Refresh working tree diff after turns#3906
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces a new caching mechanism for diff previews that changes runtime behavior across server and client. The three open review comments raise valid design concerns about potential edge cases where the cache key may not properly invalidate stale diffs. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68f23c90ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| workingTree: gitStatusQuery.data?.workingTree ?? null, | ||
| }); |
There was a problem hiding this comment.
When the current ref changes while the working tree summary is unchanged, this cache key does not change even though getReviewDiffPreview recomputes the branch-range diff from the current HEAD/branch. For example, switching between two clean branches leaves workingTree as an empty summary and latestTurnId unchanged, so the branch diff panel can keep reusing the previous ${baseRef}...HEAD preview. Include ref/HEAD-relevant status (or a VCS generation) in the cache key, not just the working-tree summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 601d479ebe9a522c2bdff2ba0ba3f3e094b19044.
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| workingTree: gitStatusQuery.data?.workingTree ?? null, | ||
| }); |
There was a problem hiding this comment.
When an already-listed changed file is edited without changing its path or numstat summary, this key stays identical while the patch returned by getReviewDiffPreview changes. This is especially easy for untracked files because the status summary records them as the same path with 0 insertions/deletions, so editing the file contents after the preview is cached leaves the Working tree panel rendering the old patch until something else changes the key. Use a content/diff generation or refreshed diff hash rather than only the lossy workingTree summary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 601d479ebe9a522c2bdff2ba0ba3f3e094b19044.
Dismissing prior approval to re-evaluate 601d479
github-actions
Bot
added
size:M
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 601d479ebe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const latestTurn = orderedTurnDiffSummaries[0]; | ||
| const diffPreviewCacheKey = JSON.stringify({ | ||
| latestTurnId: latestTurn?.turnId ?? null, | ||
| localGeneration: gitStatusQuery.data?.localGeneration ?? null, |
There was a problem hiding this comment.
When the branch preview is based on a remote ref, a fetch can move that base ref while the working tree and HEAD stay unchanged; VcsStatusBroadcaster.updateCachedRemoteStatus publishes only remoteUpdated, and applyGitStatusStreamEvent preserves the old localGeneration, so this cache key stays identical even though getReviewDiffPreview recomputes ${baseRef}...HEAD against the new ref. Include a remote generation/fingerprint in the key for branch-range previews so the panel refreshes after remote status updates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in fe250edfc44c7333230992e613175367d91442d5.
github-actions
Bot
added
size:L
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d83415d7d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const remoteRefs = yield* runGitStdout("GitVcsDriver.statusDetailsRemote.remoteRefs", cwd, [ | ||
| "for-each-ref", | ||
| "--sort=refname", | ||
| "--format=%(refname)%00%(objectname)", | ||
| "refs/remotes", | ||
| ]); |
There was a problem hiding this comment.
In repositories with many remote-tracking refs, this new for-each-ref refs/remotes output can exceed the default 1 MB stdout limit enforced by runGitStdout/executeGit; when that happens collectOutput fails with a GitCommandError, so every remote status refresh fails and the Git UI loses status even though the repository is otherwise healthy. Hash the ref stream/bounded chunks, or otherwise avoid materializing all remote refs under the default cap, before putting this on the status path.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 87b2fd2. Configure here.
| _tag: "snapshot" as const, | ||
| local: initialLocal, | ||
| remote: initialRemote, | ||
| localGeneration: cachedStatus?.localGeneration, |
There was a problem hiding this comment.
Snapshot local generation mismatch
Medium Severity
In streamStatus, the initial snapshot takes local from getOrLoadLocalStatus but reads localGeneration from a later getCachedStatus. A concurrent refreshLocalStatus can advance the cached generation while the snapshot still carries the earlier local payload, so clients can briefly see mismatched generation and working-tree summary data.
Reviewed by Cursor Bugbot for commit 87b2fd2. Configure here.
1 participant
What changed
Why
The Working tree diff preview could reuse a cached response after a later agent turn changed the repository. This was especially visible when a file created in one turn was deleted in the next: Git correctly omitted the deleted untracked file from a fresh preview, but the UI could continue rendering the earlier cached patch.
The updated cache identity selects a fresh preview after completed turns and working-tree status changes, while keeping the refresh declarative in the query layer.
Impact
Working tree and branch diff views now reflect the latest repository state after agent turns and Git status changes instead of briefly showing a stale patch.
Validation
vp test packages/client-runtime/src/state/runtime.test.ts apps/server/src/vcs/GitVcsDriverCore.test.tsvp checkvp run typecheckNote
Medium Risk
Changes VCS stream semantics (more
localUpdatedevents) and adds a git for-each-ref on remote status reads; scope is limited to status/diff caching with regression tests.Overview
Fixes stale working-tree and branch diff previews when the repo changes but the summarized Git status payload looks the same (e.g. an untracked file created in one turn then deleted in the next).
VCS status now exposes
localGeneration(increments on every local refresh;localUpdatedis always published) andremoteRefHash(SHA-256 ofrefs/remotesfromgit for-each-ref). Remote status still uses fingerprinting forremoteUpdated.DiffPanelbuilds adiffPreviewCacheKeyfrom the latest turn id,localGeneration, andremoteRefHash, and passes it into branch/working-treediffPreviewenvironment queries.environmentRpcKeyaccepts an optional client-onlycacheKeyso query cache identity can change without altering the RPC input.Reviewed by Cursor Bugbot for commit 87b2fd2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Refresh working tree diff after turns by tracking local and remote ref changes
localGenerationcounter to VCS status that increments on every local status update, replacing fingerprint-based deduplication inVcsStatusBroadcaster.remoteRefHashto remote status by hashing all remote-tracking refs viagit for-each-ref; the hash changes when any remote ref moves.diffPreviewCacheKeyinDiffPanelcomposed fromlatestTurnId,localGeneration, andremoteRefHash, so diff preview queries re-execute after each turn or remote ref change.environmentRpcKeyinruntime.tswith an optionalcacheKeyto isolate environment queries when the cache key differs.localUpdatedevents than before.Macroscope summarized 87b2fd2.