Repository navigation
Avoid phantom turn-end checkpoints for CRLF working files - #2376
Closed
gtrrz-victor wants to merge 5 commits into
Closed
gtrrz-victor wants to merge 5 commits into
gtrrz-victor wants to merge 5 commits into
Conversation
filterToUncommittedFiles is the turn-end guard that drops transcript-reported files an agent committed mid-turn, so SaveStep does not mint a shadow branch after post-commit has already condensed the session and deleted the old one. It compared raw working-tree bytes with the HEAD blob, so under core.autocrlf=true a CRLF working copy of the LF-normalized blob counted as uncommitted. Copilot CLI writes CRLF on Windows, and PR #2341 made its transcript analyzer report the files it writes, which turned the latent mismatch into a shadow branch that nothing condenses (nightly run 34583415795, TestMultiSessionSequential/copilot-cli on windows-latest; Linux passes only because Copilot writes LF there). The post-commit carry-forward path already answers the same question with native git hash-object (#2336). Extract that comparison into strategy.WorktreeMatchesCommitted and use it from both call sites; the subagent capture path shares the guard and is fixed by the same change. Two cases hash-object alone gets wrong keep the raw-bytes comparison as a second acceptance path rather than a fallback: a blob that already carries CRLF, which git status exempts from autocrlf conversion while hash-object converts unconditionally, and a symlink blob checked out as a regular file under core.symlinks=false, where Readlink has nothing to read. Both are pinned by tests alongside the turn-end autocrlf case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M283MCNP3S1ZZ13MRJP910BS
Contributor
There was a problem hiding this comment.
馃煛 Changes recommended
A critical custom clean-filter fallback issue remains unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates turn-end and post-commit file matching to follow Git鈥檚 filter-aware comparisons, preventing false shadow branches on CRLF worktrees.
Changes:
- Centralizes worktree-versus-commit matching.
- Reuses it for turn-end and carry-forward filtering.
- Adds autocrlf, legacy-CRLF, symlink tests, and documentation.
Review findings remain in content_overlap.go for custom clean-filter handling and in state.go for an inaccurate error-handling comment.
File summaries
| File | Summary |
|---|---|
docs/architecture/checkpoint-scenarios.md |
Documents shared Git-style comparison behavior. |
cmd/entire/cli/strategy/content_overlap.go |
Implements shared committed-content matching. |
cmd/entire/cli/strategy/content_overlap_test.go |
Adds matching and symlink regression tests. |
cmd/entire/cli/state.go |
Applies filter-aware matching during turn-end filtering. |
cmd/entire/cli/state_test.go |
Adds autocrlf turn-end coverage. |
cmd/entire/cli/gitrepo/worktree_hash.go |
Expands hash budget documentation. |
Review details
Suppressed comments (1)
cmd/entire/cli/state.go:335
- This comment no longer matches the implementation:
HashWorktreeFilespreserves successful hashes when another candidate fails, soWorktreeMatchesCommittedcan remove successfully matched paths even when a git hash operation returned an error; it does not return the original list unchanged for any error. Please document the per-path fail-open behavior so callers do not rely on the stronger all-or-nothing contract.
// or with different content in the working tree are kept. Fails open: if any git operation
// errors, returns the original list unchanged.
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Entire-Checkpoint: 01M28DTM49NMHTQ80896EWTPQC
Entire-Checkpoint: 01M2JKHAM59PGA9D07WH5E17NS
Entire-Checkpoint: 01M2JPQ0HQ6T83TBFDHVQFK6RR
Entire-Checkpoint: 01M2JX5B55M5NCW8SMEJX7AH0B
Contributor
Author
|
Done properly here -> #2482 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/1309
Summary
Prevent turn-end capture from recreating a shadow branch for files an agent already committed mid-turn when the working copy uses CRLF and Git stores LF. This addresses the Windows Copilot failure in nightly install smoke run 34583415795,
TestMultiSessionSequential/copilot-cli.The turn-end guard previously compared raw bytes against HEAD, so line-ending conversion made committed files appear uncommitted. #2341 exposed this by teaching Copilot's transcript analyzer to read
restrictedProperties.filePaths.Why Copilot exposed this on Windows
Windows does not force every file to use CRLF. The agent's file-writing tool chooses the line endings. With
core.autocrlf=true,git addnormalizes the stored blob to LF without rewriting the working file:The original Windows smoke artifacts show different paths through this shared code:
restrictedProperties.filePaths.This is a shared Entire bug. Any agent that writes CRLF and reports the file again at turn end after committing it can hit the same faulty comparison. Passing Windows tests for other agents do not establish that those agents are immune.
What Changed
Main independently merged a CRLF fix in #2482. After merging main, this PR retains the raw-byte fast path and batch-normalizes only raw mismatches. It uses main's shared
worktreedir.HashableEntrysafety check and retains main's CRLF and symlink regression tests.filterToUncommittedFiles.gitrepo.HashWorktreeFilesto check whether Git normalization makes the content equal. Hash candidates in a batch; keep actual changes and paths whose normalized content cannot be confirmed.Verification
Merge update
776568da2incorporates main atc21ef7f00. The resolved tree passed the focused turn-end tests, formatting, lint, andmise run test:ci, including race-enabled unit/integration tests and deterministic canaries. A final standardmise run lintpassed after committing. The real-agent matrix below tested the earlier97ace0cc2commit and was not rerun for this merge.mise run fmt,mise run lint(0 issues), andmise run test:cipassed, including race-enabled unit/integration tests and deterministic E2E canaries. Focused Windows CopilotTestMultiSessionSequential(git-refs) passed on the fix on its first attempt. The same scenario failed on the unfixed base, including its retry, with a shadow branch recreated after post-commit cleanup. On the fix, both sessions instead reported no files modified at turn end. Full cross-platform E2E validation completed across 32 combinations. 26 passed; six Windows combinations failed. This is not a clean cross-platform result.Linux Cursor's installer returned HTTP 500 in the original matrix; its isolated retry passed the full suite. macOS Copilot
TestRapidSequentialCommitsand macOS CursorTestMultiSessionSequentialeach passed after one test rerun. Existing unsupported-test skips remain in the reports.Windows failure investigation:
tmuxand a separate existing post-commit CRLF mismatch: ended-session matching compares an LF commit blob with a CRLF shadow blob and declines to condense. The relevant comparison code is unchanged from the base. An isolated temporary Go test reproduces it without an agent: an LF shadow file matches after native Git commits it, while the same CRLF content does not. The original multi-session cleanup scenario passes on the fix with both backends.WaitDelayerrors and missingtmux; which tests hit the subprocess error varies between runs.tmux. The existing Windows dependency-link fallback also makes each test install dependencies again. The unfixed baseline also failed: git-branch completed with 37 failed tests, and git-refs hit the suite鈥檚 30-minute timeout after recording 33 failures, leaving tests unfinished. The baseline shows the same agent-exit and missing-tmuxfailure classes; it does not provide a complete git-refs comparison.The Windows failures prevent a clean cross-platform conclusion. No new failure class was identified, but the existing failures and incomplete OpenCode baseline limit regression confidence.
The workflow expansion lives on
validation/turn-end-autocrlf-20260915, outside this PR. Every fixed-branch job built commit97ace0cc20c8b208a4fbdc77f4b08b314e1f517b.The turn-end regression test covers a CRLF working copy of an LF commit, genuinely modified CRLF content, staged changes compared against HEAD, missing paths, legacy CRLF raw matches, and input ordering.
BenchmarkFilterToUncommittedFilesmeasures the complete turn-end filter, including repository and blob reads. Example local medians on Apple M2, three samples of ten iterations each:Raw matches need no hashing subprocess. Raw mismatches reuse the existing batched Git hashing helper.
In the focused Windows Copilot run, the logged filter-and-normalize step took 20 ms and 46 ms for the two sessions; total agent-stop hooks took 206 ms and 232 ms. These are individual observations, not a performance bound.