fix(app): invalidate the project-skill cache when the checkout changes - #4463
fix(app): invalidate the project-skill cache when the checkout changes#4463dwyanewang wants to merge 9 commits into
Conversation
Draft command caches are only invalidated when the checkout identity moves, so an external tool that adds, removes, or edits an uncommitted project skill leaves autocomplete offering the old set until the next branch or HEAD change. Every checkout-status push now invalidates the scoped draft-command cache. Identity changes still refetch immediately so an open command menu updates in place; every other push only marks the cache stale, so working-tree churn cannot restart provider discovery while the user is typing. The query is disabled while the menu is closed, so the next open runs discovery again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deferred invalidation canceled every in-flight draft-command query, including one backing an open command menu. Cancellation reverts the fetch without starting another, so the menu sat empty until it was closed and reopened. Cancel only discovery nothing is watching. A watched fetch now lands so the menu has commands to show, then gets re-marked stale and refetched on top, which is reachable only from this collision and never while the user types into an idle menu.
|
| Filename | Overview |
|---|---|
| packages/app/src/git/checkout-status-cache.ts | Classifies checkout identity and working-tree changes and applies the corresponding checkout-scoped draft-command invalidation. |
| packages/app/src/hooks/agent-commands-query.ts | Adds scoped invalidation helpers and preserves watched in-flight discovery before refreshing stale results. |
| packages/server/src/server/workspace-git-service.ts | Tracks monotonic working-tree revisions and includes HEAD identity in workspace snapshots. |
| packages/server/src/server/checkout/status-projection.ts | Projects checkout HEAD and working-tree revision fields onto status payloads. |
| packages/protocol/src/messages.ts | Adds backward-compatible optional checkout identity and revision fields to protocol schemas. |
| packages/app/src/git/checkout-status-cache.test.ts | Covers cache behavior through React Query observers, but the existing review request for real browser or app-level coverage remains outstanding. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Git checkout observation] --> B[Checkout status update]
B --> C{Checkout identity changed?}
C -->|Branch or HEAD changed| D[Invalidate and refetch active draft-command queries]
C -->|No| E{Working-tree revision changed?}
E -->|Yes| F[Mark draft-command queries stale]
F --> G{Discovery already in flight?}
G -->|Unwatched| H[Cancel discovery and retain invalidation]
G -->|Watched| I[Allow result to land]
I --> J[Refetch if menu remains open]
E -->|No| K[Leave draft-command cache unchanged]
Reviews (2): Last reviewed commit: "test(app): drop the unused JSDOM environ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -104,6 +107,44 @@ function createQueryClient(): QueryClient { | |||
| return new QueryClient({ defaultOptions: { queries: { retry: false } } }); | |||
There was a problem hiding this comment.
These command-menu scenarios run in JSDOM and simulate the menu lifecycle directly through QueryObserver. This violates the repository directive that JSDOM must not substitute for app behavior. The open-menu refresh path needs coverage through the real browser or app interface before merging.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
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!
The pragma arrived with b0ca08a (getpaseo#1491) and nothing in the file touches the DOM, while docs/testing.md rules JSDOM out. All existing tests pass without it, and the suite stops paying for a JSDOM environment it never uses.
|
On the JSDOM finding: the The tests this PR adds there drive So I removed it (317f359) rather than adding to a file that violates docs/testing.md. The suite now runs on the node environment, which also drops the JSDOM startup cost: Happy to drop that commit if you would rather keep the pragma change out of this PR. |
Add fix/skill-cache-invalidation-scoped at 317f359 for PR getpaseo#4463 after confirming its nine feature commits remain unique to main da8c1b5 and fully replace the removed mixed cache branch. Add fix/electron-explorer-tab-menu at 99cb41d for PR getpaseo#3890 after confirming the rebased branch preserves main's plugin-panel target identity while restoring a non-drag Electron menu trigger.
1 participant
Linked issue
Fixes #4462. Follow-up to #2325 / #2423.
Type of change
What does this PR do
Draft command results are cached by provider + cwd with an infinite stale time, but project skills are read from the active checkout. Switching branches therefore keeps a skill that no longer exists, or keeps a newly available one hidden, until the app restarts.
This invalidates only the affected checkout's draft-command cache:
HEADmoved) — refetch immediately, so a menu that is open updates on the spot.To tell those apart from the daemon side, the checkout status snapshot carries the checked-out commit and a monotonic working-tree revision. Both protocol fields are optional additions.
Agent-session command caches and other projects are untouched.
Relationship to #2585
Same fix, resubmitted with the scope it should have had. That branch had a merge of an unrelated feature branch pushed into it by mistake, so its diff grew to 58 files and read as a feature PR. This one is the fix alone: 16 files, and every file is on the invalidation path.
Reviewing #2585 also surfaced a real defect that is fixed here (last commit): deferred invalidation cancelled every in-flight draft-command query, including the one backing an open command menu. Cancelling reverts the fetch without starting another, so the menu sat empty until it was closed and reopened. Now only unwatched discovery is cancelled; a watched fetch lands first so the menu has something to show, then is refetched on top of it.
How did you verify it
Automated:
The empty-menu defect above was written as a failing test first (
shows an open menu the in-flight result, then refreshes itinagent-commands-query.test.ts); it fails on the previous cancel-everything behaviour withexpected undefined to deeply equal [ '/build' ].npm run typecheckis clean for every workspace this PR touches. It fails in@getpaseo/desktopwith two pre-existing Electron typing errors that reproduce unchanged on a cleanupstream/maincheckout here, so they are environmental and not from this branch:Manual, on a packaged desktop build, in both directions:
build-paseoskill exists;/build-paseoappeared. Switched to a branch without it; it disappeared from/.fix.app._.refresh.project.skills.after.checkout.changes.by.dwyanewang.Pull.Request.2585.getpaseo_paseo.mp4
Platforms tested: Desktop (Electron, Linux). Not manually retested on iOS, Android, or browser web — the cache and its invalidation are shared app-side code with no platform branches, and the app-side tests above cover it.
Checklist
npm run lintpassesnpm run formatran (Biome)