Fix phantom file carry-forward causing lingering shadow branches#586
Fix phantom file carry-forward causing lingering shadow branches#586
Conversation
Agent transcript parsers capture file paths from all tool calls, including ones that were attempted but never created on disk (e.g. agent writes src/types.go but creates src/types/types.go instead). These phantom paths in FilesTouched caused filesWithRemainingAgentChanges to always return non-empty results, creating an infinite carry-forward loop where shadow branches were never cleaned up. Fix: for non-committed files, also check the shadow branch tree before carrying forward. buildTreeWithChanges already skips non-existent files, so the shadow tree is the ground truth of what was actually checkpointed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 95bb0b3d0af0
PR SummaryCursor Bugbot is generating a summary for commit 7da41e6. Configure here. |
There was a problem hiding this comment.
Pull request overview
Fixes carry-forward logic in the manual-commit shadow-branch strategy by preventing “phantom” file paths (captured from agent transcripts but never materialized in the shadow tree) from being repeatedly carried forward, which can create lingering shadow branches and E2E failures.
Changes:
- Update
filesWithRemainingAgentChangesto skip uncommitted paths that aren’t present in the shadow tree (treating the shadow tree as ground truth for what the agent actually produced). - Add a unit test covering the phantom-file scenario to prevent infinite carry-forward loops.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cmd/entire/cli/strategy/content_overlap.go |
Adds a shadow-tree existence check before carrying forward uncommitted touched paths. |
cmd/entire/cli/strategy/content_overlap_test.go |
Adds a unit test ensuring phantom transcript paths don’t get carried forward. |
| // If file wasn't committed, check if it actually exists in the shadow tree. | ||
| // Transcript parsers capture file paths from all tool calls, including ones | ||
| // that were attempted but never created (e.g. agent writes src/types.go then | ||
| // creates src/types/types.go instead). buildTreeWithChanges skips non-existent | ||
| // files, so the shadow tree is the ground truth. Without this check, phantom | ||
| // paths cause infinite carry-forward loops. | ||
| if _, wasCommitted := committedFiles[filePath]; !wasCommitted { | ||
| if _, err := shadowTree.File(filePath); err != nil { | ||
| logging.Debug(logCtx, "filesWithRemainingAgentChanges: file not committed and not in shadow tree, skipping", | ||
| slog.String("file", filePath), | ||
| ) | ||
| continue | ||
| } |
There was a problem hiding this comment.
The new "phantom file" check treats any path missing from the shadow tree as skippable when it wasn’t committed. That also matches legitimate agent deletions (deleted files are included in FilesTouched), causing uncommitted deletions to be dropped from carry-forward. Consider only skipping when the path is missing from BOTH the shadow tree and the HEAD/commit tree (or otherwise explicitly handle deletions), and include the underlying shadowTree.File error in the debug log. Adding a unit test for an uncommitted deletion would prevent regressions.
Summary
TestMixedNewAndModifiedFiles,TestPartialStaging) caused by phantom file paths infilesTouchedcreating infinite carry-forward loopssrc/types.goin a tool call but actually createssrc/types/types.gofor Go package structure)filesWithRemainingAgentChangesnow checks shadow tree existence before carrying forward non-committed files — the shadow tree is the ground truth sincebuildTreeWithChangesalready skips non-existent filesFailing E2E runs
Test plan
TestFilesWithRemainingAgentChanges_PhantomFileunit testmise run test:ci)mise run lint)TestMixedNewAndModifiedFilesandTestPartialStagingshould stop failing for gemini-cli🤖 Generated with Claude Code