feat(coord): distinguish an untracked-but-recoverable file from one genuinely at risk (BACKLOG #1298) - #580
Open
wshallwshall wants to merge 4 commits into
Open
feat(coord): distinguish an untracked-but-recoverable file from one genuinely at risk (BACKLOG #1298)#580wshallwshall wants to merge 4 commits into
wshallwshall wants to merge 4 commits into
Conversation
…LOG #1298) The archive dialog warns that untracked files "will be permanently discarded". It reasons from "not in THIS WORKTREE'S index" straight to "will be lost", and skips the question that decides it: is the content somewhere else. The two come apart for an ordinary and constant reason. A tree branched behind `main` does not have the files that landed since, so a copy of one is untracked THERE while tracked on `main` -- recoverable, and warned about anyway. Every session branched behind `main` meets this, on every file that landed since, and the prompt arrives when a seat is finishing. REPRODUCED BEFORE BUILDING, on a worktree detached at 720f943, one commit before tests/test_ci_retry_native_crash.py landed at 6e758a8, holding main's copy of that file: git status --porcelain -> ?? tests/test_ci_retry_native_crash.py git hash-object <file> -> 5498a64 git rev-parse origin/main:<file> -> 5498a64 Identical. The dialog called that permanent loss. scripts/coord/recoverable.ps1 answers it: every untracked file classified RECOVERABLE, AT-RISK (absent from the ref), or AT-RISK (on the ref but MODIFIED here). Exit 1 if any is at risk, so it works as a check and not only as something to read. THE THIRD VERDICT IS WHY AN EXISTENCE CHECK IS NOT ENOUGH. "Is it on main" answers YES for a file whose local edit is the only thing that would be lost. I nearly shipped a test that missed it -- my first contrast case used a NEW FILENAME, which exercises "absent from main", not "on main but modified". Different arms; I redid it against the same path. TWO RULES, BOTH ABOUT WHICH WAY TO BE WRONG: * Anything the script cannot read is reported AT-RISK, never clean -- the direction occupancy.ps1 states for its own fence. A false AT-RISK costs a look; a false RECOVERABLE costs the file. * -NoFetch is safe for the same reason: a stale ref can only fail to contain something that has since landed, so it can only move a file toward AT-RISK. It cannot invent a match. The ref and its sha are printed with every run, because a verdict quoted without the ref it was computed against cannot be re-checked by whoever reads it. --untracked-files=all, because git's default collapses an untracked DIRECTORY to one entry and every file beneath it would go unexamined while the run still printed a verdict. -z, because porcelain v1 QUOTES paths containing spaces and parsing that form is a second, silently different unescaper. Both are pinned by tests. THE PARTITION GUARD WAS RUN AS A POSITIVE CONTROL, NOT ASSUMED. With the new module unregistered, tests/test_tooling_partition.py went RED naming it; after adding the manifest line, `-m tooling` selects all 7 and `-m "not tooling"` deselects all 7. An unclassified test module is BACKLOG #1262's defect and it does not announce itself. Verification, with scope: tests/test_coord_recoverable.py (7), plus test_tooling_partition.py, test_ci_tooling_gate.py, test_citation_line_check.py, test_dangling_citation_check.py and test_backlog_citation_check.py -- 114 passed, on .venv/Scripts/python.exe (CPython 3.14.6 non-freethreaded, the seven CI extras). ruff 0.15.22 (== the constraints.lock pin) check and format --check clean; mypy strict clean. recoverable.ps1 parses with 0 AST errors. NOT the full suite. SCOPE THE ROW STATES AND THIS COMMIT HONOURS: the dialog is the Claude Code harness and is NOT this repository's code. Nothing here changes its wording; this answers the question it raises but cannot itself answer. No test was removed or weakened. The ledger row is not mine to author and is not in this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (BACKLOG #1298) Follow-up to the #1298 commit, applying a four-angle review that completed after it. The review ran 4 agents over the change; none returned empty. ONE GIT PROCESS FEWER PER FILE. `rev-parse <ref>:<path>` already exits non-zero when the path is absent from the ref, so the `cat-file -e` probe that preceded it asked a question the next line answered on its way to the sha. Two review angles found this independently. Verified: `git rev-parse origin/main:no/such/path` exits 128, `origin/main:README.md` exits 0. AND IT HAS A SHARP EDGE THE EXIT-CODE CHECK IS LOAD-BEARING FOR: rev-parse PRINTS THE SPEC ITSELF TO STDOUT on failure. `origin/main:no/such/zzq.txt` comes back on stdout with exit 128, so a truthiness test on the captured value alone would read a missing path as a valid sha. The guard tests $LASTEXITCODE first and then nulls the variable explicitly. THE PAYLOAD NOW MATCHES WHAT THE DOCS SELL. `Verdict` is deliberately BINARY -- would this be lost -- but three of its four causes are AT-RISK, so the three-way distinction the docs table advertised existed only inside `Detail`, an English sentence with the ref name interpolated into it. A consumer wanting to tell "absent" from "modified" had to parse prose. `Reason` is now a closed set: identical, absent, modified, unreadable. THE ARM THE DOCS CALLED THE WHOLE POINT WAS UNTESTED, and now is not. Mutation-verified with the harness carrying its own controls -- anchor asserted unique, file hash asserted CHANGED before scoring: collapsing `modified` into `absent` (two causes that share the AT-RISK verdict, so no Verdict assertion can see it) reds 1 of 7. Before this commit that mutation SURVIVED. STATED ONCE, NOT THREE TIMES. The measured reproduction with its shas lived in the script header, the docs subsection and the test docstring -- six sha citations maintained in two files. The docs copy is now the only one; the header points at it. (SDS-3.5.) THE DOCS SUBSECTION IS A `##`, NOT A `###` UNDER "Remove one". The question governs every way a tree goes away -- remove.ps1, prune-merged.ps1, a rescue, the archive dialog, a manual delete -- so a 53-line subsection nested under the 32-line section about the one script it does NOT concern is hidden from exactly the reader looking for it. Also: dead `$verdict = $null` / `$detail = $null` removed (every branch assigns both), while the `$wtHash` / `$refHash` resets are KEPT with a comment saying why -- without them a hash from the previous file survives into a row nobody hashed. The two pairs looked identical, which is how the next reader deletes all four. And the test's two inline row unpackers are replaced by one `by_path` helper, so the isinstance dance is written once. DELIBERATELY NOT APPLIED, and each is reported rather than silently dropped: * BATCHING THE GIT CALLS. The two angles disagree and both measured. REUSE: 300 untracked files cost 41,220 ms today against 427 ms batched, and `alloc.ps1:126` already documents this exact mistake. EFFICIENCY: the real population is N<=2, where batching buys nothing and costs the per-file UNREADABLE isolation. Both are right; which wins depends on whether this is ever pointed at a large tree, and that is not settled here. * MOVING THE FILE to scripts/worktree/. The altitude case is strong -- zero mefor-coord hits, no seat, claim, lock, mail or registry read, and its documented neighbours are remove.ps1 and prune-merged.ps1. But the dispatch brief named scripts/coord/ explicitly, and moving it against that is a scope call that is not mine to make unasked. * THE LARGEST FINDING, which is out of this item's scope entirely: fleet.ps1:367, prune-merged.ps1:570-577 and unbacked_check.ps1:273 all still assert the index-as-loss claim this script refutes, and unlike the archive dialog they ARE this repository's code. Reported to the dispatcher as content for its own row. Widening a claimed item to three unrelated files -- one of them the most destructive script in the tree -- is the cross-lane collision the file grouping exists to prevent. Verification, with scope: tests/test_coord_recoverable.py, test_tooling_partition.py, test_citation_line_check.py, test_dangling_citation_check.py -- 70 passed, on .venv/Scripts/python.exe (CPython 3.14.6 non-freethreaded, seven CI extras). ruff 0.15.22 (== the constraints.lock pin) check and format --check clean; mypy strict clean. NOT the full suite. No test was removed or weakened. The ledger row is not mine to author and is not in this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 25, 2026 17:41
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
1 participant
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.
New script (scripts/coord/recoverable.ps1) classifying every untracked file in a worktree as RECOVERABLE, AT-RISK-absent, or AT-RISK-modified, and exiting 1 if anything is at risk. docs/BACKLOG.md untouched (verified, count 0) -- the #1298 row is not this PR's to author.
What it fixes: the worktree-removal safety dialog's 'will be permanently discarded' warning is an INDEX test presented as a LOSS test. A tree branched behind main and holding a file that landed since main advanced reads as untracked there while tracked on main -- byte-identical, fully recoverable -- and gets warned about as if it were unique. Reproduced before building: a tree detached behind main, holding main's own copy of a since-landed file, reported
??while its blob matched origin/main's blob exactly.Two commits: the feature, then the quality-review follow-up applied before handoff (author held it for exactly this):
cat-file -ecollapsed into the followingrev-parse).Reasonfield (identical/absent/modified/unreadable) replacing a classification that only existed inside an interpolated English sentence.git rev-parse <ref>:<missing-path>prints the pathspec itself to stdout and exits 128 -- a truthiness check on the captured value alone reads a missing path as a valid sha. Only checking $LASTEXITCODE first catches it.Deliberately not in scope, named so a reviewer isn't surprised: batching the git calls (two measurement angles disagreed -- huge win at 300 files, no win and a real cost at the actual population size of <=2); relocating the script to scripts/worktree/ (a Dispatcher scope call, not this branch's); fleet.ps1/prune-merged.ps1/unbacked_check.ps1 still assert the claim this script refutes elsewhere (reported to the Dispatcher as separate content).
Verification, explicitly scoped: 70 tests pass across test_coord_recoverable + three citation/partition gates, on the CI-matched lane venv (seven extras). ruff/ruff format/mypy clean on every changed file. Full suite not run.