[REVIEW] Full-codebase Greptile audit

[claudepc42]

Overview

Not a normal feature PR — this diffs the entire current codebase against a
disposable, empty baseline branch (greptile-audit-base, an orphan commit
with zero files), so the full project shows up as one diff for a
comprehensive first-ever Greptile pass, rather than reviewing it piecemeal
across incremental PRs. Same trick used for
claudepc42/audiobookshelf-app-cloudflare-zero-trust PR #7 and #9, adapted
for a project with no upstream fork to diff against — the baseline here is
a genuinely empty orphan branch instead of a copied upstream/old-tag branch,
since PyGoose is an original project, not a fork.

Scope

The entire PyGoose codebase as of main (95 files, ~7,371 lines): the
pygoose/ package (engine, goose behavior state machine, config, cursor
tracking, overlay rendering, renderer, sound, window subsystems — meme
window, movable window, notepad window), plus tests/test_engine.py.

Testing Done

Not applicable in the usual sense — this is a first full-codebase audit
pass, not a review of a specific recent change. Existing test coverage is
in tests/test_engine.py.

Specific Concerns for Review

1. General code quality, bugs, and architectural issues across the whole
   codebase — this is intentionally broad, not scoped to one recent change.
2. The goose behavior state machine (pygoose/goose/goose.py, the largest
   file) — check for state-transition bugs, race conditions between timers
   and user input, and any resource leaks in long-running behavior loops.
3. Window subsystems (meme_window.py, movable_window.py,
   notepad_window.py) — check for consistent cleanup/teardown, especially
   around window close and app shutdown.
4. config.py and paths.py — check for path-handling issues that could
   break on different OS/user setups.
5. Security: this is a desktop app running arbitrary local behavior — flag
   anything that reads/writes files or executes commands without proper
   path validation.

Not for merging

greptile-audit-base is a disposable orphan branch that exists only to
give this PR something to diff against — it has no relationship to the
project's real history. This PR itself is not intended to be merged; close
both this PR and the greptile-audit-base branch once the review is done.

Greptile Summary

This change adds a cross-platform desktop-goose application, including its Qt overlay, behavior state machine, props, cursor control, packaging, and release automation.

Executable checks reproduced four problems that should be fixed before merge: animated props can stop repainting while the goose is idle; macOS cursor capture can target the wrong virtual-desktop coordinates; an invalid developer-forced task aborts behavior selection; and the release publisher runs a mutable third-party action with repository write permission.

Merge safety: unsafe until the identified behavior, cursor, configuration, and release-publishing issues are resolved.

Confidence Score: 3/5

The change is not safe to merge because it contains reproducible desktop behavior failures and a release-publishing supply-chain exposure.

Independent executable checks reproduced a skipped repaint path, an incorrect macOS global-coordinate calculation, an uncaught configuration exception in task selection, and mutable third-party code running with release write access.

Files Needing Attention: pygoose/goose/goose.py needs repaint invalidation and forced-task error handling; pygoose/goose/cursor.py needs global cursor coordinates on macOS; .github/workflows/release.yml needs immutable action pins.

Security Review

The release workflow invokes softprops/action-gh-release@v2 through a mutable tag while granting contents: write. If that upstream tag changes, unreviewed action code can execute with permission to publish or modify release artifacts. Pin both release-action invocations to reviewed full commit SHAs.

T-Rex Logs

What T-Rex did

• T-Rex produced a P1 finding proof about the narrow production prop animation and dirty-rect harness affecting repaint decisions, supported by before- and after-render data.
• T-Rex validated macOS cursor origin handling by running the non-zero-origin cursor test harness and comparing baseline against non-zero origin results.
• T-Rex compared DEV ForceTask outputs, distinguishing the invalid observed output from the valid control path, with harness outputs attached.
• T-Rex validated the release workflow pin by executing the validator and confirming the exact release workflow source and the assertion-backed pin outcome.
• T-Rex performed general contract validation confirming the prop repaint decision path and the resulting render, including before and after evidence of the prop position.

_{Ran code and verified through T-Rex}

Comments Outside Diff (4)

1. General comment

   Animated props are omitted from the repaint signature

   • Bug
     • After the initial paint, a falling prop changed from (330.0, 205.0, z=46.0, angle=0.0) to (357.0, 205.0, z=0.0, angle=108.0) across six production physics ticks. On every one of those frames Goose.dirty_rect() returned None, which makes Overlay._tick() skip update() when the goose is otherwise stationary. The painted prop therefore remains frozen until another signature input changes, then appears at its later state.
   • Cause
     • sig contains goose, feet, rig, and sleep values but no prop state. Lines 421-425 compare only that signature and footmark animation state.
   • Fix
     • Include all render-affecting prop state in the dirty/repaint decision—at minimum type, position, surface_z, z, angle, scale, state, and any rendered prop-specific data—or explicitly mark active/falling/animated props dirty and invalidate a rectangle covering both their old and new rendered bounds.

   _{Ran code and verified through T-Rex}

2. General comment

   macOS cursor placement omits the primary display's virtual-desktop origin

   • Bug
     • When the primary macOS display begins at a non-zero virtual-desktop coordinate, set_cursor_clip invokes QCursor.setPos with local gameplay coordinates. Qt interprets them as global coordinates, so the cursor is displaced by the primary display origin and can land on a different display.
   • Cause
     • pygoose/goose/cursor.py:36 computes only x + w / 2 and y + h / 2; it does not add QGuiApplication.primaryScreen().geometry().x() and .y() before calling the global-coordinate QCursor.setPos overload.
   • Fix
     • Convert the primary-display-local center to global coordinates before calling QCursor.setPos, for example by adding the primary screen geometry's x() and y() offsets (or ensure all inputs are made global at the call boundary).

   _{Ran code and verified through T-Rex}

3. General comment

   Invalid DEV_ForceTask crashes next-task selection

   • Bug
     • A stale, misspelled, or unsupported non-empty DEV_ForceTask value causes _choose_next_task to raise ValueError instead of selecting a task, aborting the behavior-loop call path.
   • Cause
     • pygoose/goose/goose.py:690 directly constructs Task(self.config.dev_force_task) with no validation or ValueError handling.
   • Fix
     • Validate against supported Task values (or catch ValueError) and fall back to normal task selection or Task.WANDER; optionally surface a non-fatal warning.

   _{Ran code and verified through T-Rex}

4. General comment

   Mutable release-publishing action is granted repository contents write permission

   • Bug
     • Confirmed at .github/workflows/release.yml:85: softprops/action-gh-release@v2 is referenced by mutable major-version tag v2, not a reviewed immutable 40-character commit SHA. The workflow is tag-triggered and has contents: write at .github/workflows/release.yml:9; it uploads release files at lines 86–89. If the action's upstream tag changes, the altered action code executes with permission to publish or modify release contents.
   • Cause
     • The workflow pins the third-party softprops/action-gh-release action to a mutable version tag rather than a specific reviewed commit SHA while assigning repository write capability.
   • Fix
     • Replace softprops/action-gh-release@v2 at line 85 (and the identical line 110 invocation) with the reviewed full 40-character commit SHA for the intended action release, and retain the SHA pin through dependency updates.

   _{Ran code and verified through T-Rex}

Prompt To Fix All With AI

### Issue 1
pygoose/goose/goose.py:421-425
**Animated props are omitted from the repaint signature**

`dirty_rect()` only compares goose, rig, foot, and sleep state. It does not account for `self.props`, even though prop physics and rendering continue independently. Once the goose is stationary, a falling or settling prop can change position, height, or angle while this method returns `None`, so the overlay does not repaint it. The prop remains visually frozen until another goose change triggers a paint, then jumps to its later position. Include active prop state in invalidation, or explicitly invalidate both the previous and current bounds of animated props.

### Issue 2
pygoose/goose/cursor.py:36
**macOS cursor placement omits the virtual-desktop origin**

`QCursor.setPos` expects global virtual-desktop coordinates, but this passes the center of the supplied display-local rectangle directly. When the primary display has a non-zero virtual-desktop origin, the cursor is displaced by that origin and can land on the wrong display. Convert the local center to global coordinates by adding the target screen geometry's `x()` and `y()` offsets before calling `setPos`.

### Issue 3
pygoose/goose/goose.py:688-691
**Invalid forced tasks crash task selection**

A stale, misspelled, or unsupported non-empty `DEV_ForceTask` value is passed directly to `Task(...)`. The enum constructor raises `ValueError` before `_set_task` runs, aborting the timer-driven behavior update instead of allowing the goose to continue normally. Validate the configured value or catch `ValueError` and fall back to normal task selection or `Task.WANDER`.

### Issue 4
.github/workflows/release.yml:85
**Mutable release action has repository write access**

`softprops/action-gh-release@v2` is a mutable third-party tag rather than a reviewed immutable commit SHA. This tag-triggered workflow grants `contents: write`, so a changed upstream tag could execute altered code with permission to publish or modify release artifacts. Pin this invocation and the identical source-release invocation to reviewed full 40-character commit SHAs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "release: v0.37 — knife threat, SilenceMu..." | Re-trigger Greptile}

Greptile also left 4 inline comments on this PR.

[claudepc42]

@greptileai Please perform a comprehensive full-codebase audit covering:

1. General code quality, bugs, and architectural issues across the whole PyGoose package.
2. The goose behavior state machine (pygoose/goose/goose.py and pygoose/goose/behaviors/*) - state-transition bugs, race conditions between timers and user input, resource leaks in long-running behavior loops.
3. Window subsystems (meme_window.py, movable_window.py, notepad_window.py) - consistent cleanup/teardown, especially around window close and app shutdown.
4. config.py and paths.py - path-handling issues that could break on different OS/user setups.
5. Security: this is a desktop app running arbitrary local behavior - flag anything that reads/writes files or executes commands without proper path validation.
6. Physics/props system (pygoose/goose/props/*) and rendering (renderer.py, overlay.py) - performance issues or unbounded resource growth.

[greptile-apps]

Animated props are omitted from the repaint signature

dirty_rect() only compares goose, rig, foot, and sleep state. It does not account for self.props, even though prop physics and rendering continue independently. Once the goose is stationary, a falling or settling prop can change position, height, or angle while this method returns None, so the overlay does not repaint it. The prop remains visually frozen until another goose change triggers a paint, then jumps to its later position. Include active prop state in invalidation, or explicitly invalidate both the previous and current bounds of animated props.

Artifacts

Narrow production prop animation and dirty-rect harness source

• The executable harness drives production prop physics and production dirty-rect logic with a stationary goose, ending with reproducible assertions of the missing invalidation.

Before execution log with skipped repaint decisions

• The before run executed the production failure path and recorded six changed falling-prop frames for which production dirty_rect returned None, proving repaint is skipped.

Before rendering evidence showing stationary goose and retained old prop visual

• The before capture visualizes the executed skipped-repaint result: the stationary goose remains while the prior prop visual is retained instead of following its advanced physics state.

After execution log with prop-inclusive repaint decisions

• The comparison run recorded that each changed prop frame schedules repaint when the prop state is included in the invalidation decision, showing the required behavior.

After rendering evidence showing the prop at its advanced position with stationary goose

• The after capture visualizes the prop-inclusive repaint result: the goose stays fixed and the prop is drawn at its final physics position, demonstrating the correction.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: pygoose/goose/goose.py
Line: 421-425

Comment:
**Animated props are omitted from the repaint signature**

`dirty_rect()` only compares goose, rig, foot, and sleep state. It does not account for `self.props`, even though prop physics and rendering continue independently. Once the goose is stationary, a falling or settling prop can change position, height, or angle while this method returns `None`, so the overlay does not repaint it. The prop remains visually frozen until another goose change triggers a paint, then jumps to its later position. Include active prop state in invalidation, or explicitly invalidate both the previous and current bounds of animated props.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

macOS cursor placement omits the virtual-desktop origin

QCursor.setPos expects global virtual-desktop coordinates, but this passes the center of the supplied display-local rectangle directly. When the primary display has a non-zero virtual-desktop origin, the cursor is displaced by that origin and can land on the wrong display. Convert the local center to global coordinates by adding the target screen geometry's x() and y() offsets before calling setPos.

Artifacts

Executable harness source for the macOS non-zero-origin cursor test

• The exact Python source executes cursor.py's Darwin branch with an intercepted QCursor call and compares its coordinates with the required global virtual-desktop location, confirming the origin omission.

Cursor coordinate baseline with zero virtual-desktop origin

• The executed zero-origin comparison shows line 36 produces `(110, 205)`, which matches the required global coordinate when no origin offset exists, establishing the baseline.

Cursor coordinate result with non-zero primary-display origin

• The executed Darwin-branch harness records line 36 sending `(110, 205)` while the required global position is `(2030, 205)`, confirming the cursor is offset by `(-1920, 0)`.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: pygoose/goose/cursor.py
Line: 36

Comment:
**macOS cursor placement omits the virtual-desktop origin**

`QCursor.setPos` expects global virtual-desktop coordinates, but this passes the center of the supplied display-local rectangle directly. When the primary display has a non-zero virtual-desktop origin, the cursor is displaced by that origin and can land on the wrong display. Convert the local center to global coordinates by adding the target screen geometry's `x()` and `y()` offsets before calling `setPos`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Invalid forced tasks crash task selection

A stale, misspelled, or unsupported non-empty DEV_ForceTask value is passed directly to Task(...). The enum constructor raises ValueError before _set_task runs, aborting the timer-driven behavior update instead of allowing the goose to continue normally. Validate the configured value or catch ValueError and fall back to normal task selection or Task.WANDER.

Artifacts

Narrow DEV ForceTask harness source

• The executable harness extracts the exact Task enum and _choose_next_task method from the repository source and invokes the method with a probe config, showing the targeted runtime scope.

Invalid DEV ForceTask execution output

• The captured command run with DEV_ForceTask set to WanderTypo shows ValueError before any task is set, confirming the crash condition.

Valid DEV ForceTask control execution output

• The captured command run with DEV_ForceTask set to wander returns normally and sets the wander task, showing the contrasting supported-value behavior.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: pygoose/goose/goose.py
Line: 688-691

Comment:
**Invalid forced tasks crash task selection**

A stale, misspelled, or unsupported non-empty `DEV_ForceTask` value is passed directly to `Task(...)`. The enum constructor raises `ValueError` before `_set_task` runs, aborting the timer-driven behavior update instead of allowing the goose to continue normally. Validate the configured value or catch `ValueError` and fall back to normal task selection or `Task.WANDER`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Mutable release action has repository write access

softprops/action-gh-release@v2 is a mutable third-party tag rather than a reviewed immutable commit SHA. This tag-triggered workflow grants contents: write, so a changed upstream tag could execute altered code with permission to publish or modify release artifacts. Pin this invocation and the identical source-release invocation to reviewed full 40-character commit SHAs.

Artifacts

Executed narrow workflow action-pin validator

• The exact dependency-free Python validator executed against the workflow; it checks the tag trigger, contents permission, action line 85, and full-SHA pin requirement, confirming the insecure reference.

Exact release workflow source validated

• An unmodified snapshot of `.github/workflows/release.yml` used as the validation input; it shows the write permission and mutable release-action references, confirming the affected configuration.

Raw release workflow observation

• Captured output of the raw runtime workflow inspection, including command, working directory, and exit code; it shows `contents: write` and line 85 `v2` is not an immutable commit SHA, confirming the condition.

Assertion-backed release action pin result

• Captured output of the assertion-backed runtime validator, including command, working directory, and exit code 0; it reports `RESULT=CONFIRMED`, confirming the security bug.

_{Ran code and verified through T-Rex}

Prompt To Fix With AI

This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 85

Comment:
**Mutable release action has repository write access**

`softprops/action-gh-release@v2` is a mutable third-party tag rather than a reviewed immutable commit SHA. This tag-triggered workflow grants `contents: write`, so a changed upstream tag could execute altered code with permission to publish or modify release artifacts. Pin this invocation and the identical source-release invocation to reviewed full 40-character commit SHAs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!