fix: report a git index query that could not run, at four sites - #88
Conversation
`tracked_in_index` ran `git ls-files --error-unmatch` with its status discarded, so an untracked path and a damaged index were one answer. `git ls-files` exits 1 for the first and 128 for the second, so the two separate on the status alone and need no second witness the way `git cat-file` did. Per ADR 0005 decision 3 the predicate is now three-valued and each caller reports the fault instead of its ordinary negative verdict: - `check_no_disappearances` reported E-GONE for a record sitting untouched in the tree, and skipped the append-only rules on the way. It now emits E-TRACKED-SCAN and runs neither branch. - `renumbered_elsewhere` dropped a faulted candidate out of the renumber search with `|| continue`, so the real destination went unseen and the record read as erased. The fault is remembered and returned only once the loop exhausts, so a positive match on another candidate still wins. - `check_gate_files` accused the change of deleting a gate file that is present and tracked (E-GATE-TRACKED-SCAN), and exempted a declared rename whose successor it could not verify (E-GATE-SUCCESSOR-SCAN). `still_a_record` becomes three-valued with the predicate it delegates to, and `check_no_disappearances` evaluates it once rather than spelling it out twice in an if/elif chain that cannot express a third case. Seven assertions, each red against the unconverted checker. Their `git` stubs key on the path as well as the subcommand, because `tracked_in_index` is the script's only `git ls-files` caller and its call sites are reached through different fixtures. Refs #69
`is_shell_source` read a file's first line and treated any non-zero status as "not a shell source". A file it could not open reached that same answer and left the inventory silently, so a tracked script was never linted and never format-checked with nothing reported. The same status also covers a clean EOF on a last line with no trailing newline, where the line was read and the file is a shell source -- those were dropped too. The status alone cannot separate the three, so the classifier reads the two facts it does not carry: whether anything was read, and whether there was anything to read. Empty stays the ordinary negative; a non-empty file that yielded nothing is a fault, and the lister names it and exits 1. `just lint` and `just format-check` read the inventory through `while ... done < <(lister)`, which reports the loop's status and not the lister's -- so a loud failure would have been one the gates could not hear. Both capture the list first, as check-ripgrep-config.sh already does for the same reason. The new suite builds git fixtures, so it joins the isolation gate's list. Refs #69
Three fault origins reach that one code -- the record's own base-ref copy, a candidate's existence witness, and now a candidate's index query -- and the message could name only the vanished record, which is the one file the search did read successfully. An operator was pointed at the wrong file on every one of them. renumber_fault_path carries the file each origin was reading, the way gate_witness_path already does for the gate-existence witness. The record stays the message's subject, because the verdict is about the record. Refs #69
`read` reports the same non-zero status for a clean EOF on an empty file, a clean EOF on a last line with no trailing newline, and a file that could not be opened. Deciding the fault from that status plus the file's size read a tracked binary whose first byte is NUL as unreadable -- a false red on `just lint` and `just format-check` over an asset nobody was asking about -- and still missed an unreadable zero-byte file. The open is the operation that can fail and its status says only whether it did, so the fault comes from there and read's status decides nothing. The braces keep the descriptor in this shell and put bash's own "Permission denied" behind the redirection, so the script's diagnostic is its whole interface. Refs #69
Three fault origins reach that code through three different git commands -- ls-tree for the base-ref listing, cat-file for the base copy, ls-files for the index entry -- and all three travel in path_exists_status, which is named for only one of them. A bare exit status was unattributable. renumber_fault_path now names the read rather than a bare path, the way gate_witness_path already qualifies its workflow search. Refs #69
`[[ -f $path ]] || return 1` sat one line above the collapse this branch came to fix and had the same shape: a tracked path absent from the worktree, one behind an unsearchable directory, and a broken symlink all answered "not a regular file" and left the inventory as silently as an unreadable file did. git listed the path, so something was meant to be there. The open reports each of them. A submodule's gitlink is the one non-file git emits here as a matter of course, and it is a directory, so it is named as the ordinary negative rather than opened -- without that, every repo with a submodule would red its own gates. Also pins what the recipes do with the status: the suite runs this repository's real lint and format-check recipes against a fixture whose lister emits a partial list and then fails, which is the shape a status-blind recipe passes. It reddens against the recipes as they stood before this branch. The root skip now reaches the summary line, so a run that tested no permission case cannot be read as one that did. Refs #69
A candidate whose index query or base-ref witness could not run is remembered rather than returned, so a genuine match on another candidate still wins. The remembered fault was then discarded: the verdict was right and the fact that the search reached it incompletely was reported nowhere. W-RENUMBER-SCAN says so. A warning rather than an error because the verdict below it is positive -- ADR 0005's rule against reporting a fault alongside a verdict is about a negative one, which this is not. The gate's own gate_existed_at has the same shape and is untouched here: no fixture reaches "one witness faults while another succeeds", and an unfalsifiable branch in the gate's self-protection contradicts this suite's acceptance criterion. Owned by #87. Refs #69
…stic The fault applies to every tracked file, not only to scripts, because whether a file is a script is exactly what an unopenable one does not say. So deleting a tracked file without staging the deletion now reds `just lint`, `just format-check` and the `commit-check` hook -- and one message covering both causes told that developer a script had been dropped from the inventory, which reads as a broken gate. Two causes, two remedies, two messages, and a header note recording the consequence. The three cases that asserted only a non-zero exit now grep for the diagnostic naming their own path, so none of them can pass on a regression that failed for some other reason. Refs #69
Added a commit ago and wrong on two counts ADR 0005 settles. Decision 3 puts reporting in the caller, not in the predicate, and the trace was emitted from inside renumbered_elsewhere. Decision 1 specifies E-<RULE>-SCAN on the error channel, so a warning-severity scan code is a change to that decision rather than an application of it, and this branch may not amend an append-only record. Whether an outranked fault -- one a positive match or a positive witness overtook, so it never became a verdict -- is reportable at all is a question ADR 0005 does not answer. #87 owns settling it and applying the answer at both sites. Criterion 2 is unaffected: the remembered fault_status and the exhausted-loop return that satisfy it are untouched. Refs #69
Removing the `[[ -f ]]` guard let one shape through that must never reach the open: a FIFO left where a tracked file used to be. Opening a FIFO for reading blocks until a writer appears, so every gate consuming this inventory would hang. git cannot store one, but the worktree can hold anything at a tracked path. A directory stays the ordinary negative -- it is a submodule's gitlink. Anything else that exists and is not a regular file is reported. The diagnostic gains the two cases it was misrouting with it: a symlink whose target is missing reached the "stage the deletion" branch, because `-e` follows the link and is false for both. Four causes, four remedies, four messages. The header no longer claims the fault covers every tracked file -- a `.sh` name is classified without opening anything, so an unopenable one fails at shellcheck instead. Refs #69
Three faults the loud-failure rule got wrong, all reachable. A sparse checkout leaves tracked paths out of the worktree and marks them skip-worktree; assume-unchanged does the same with a lowercase tag. Both are supported configurations and both looked exactly like a deletion, so the new rule made the repository uncommittable for anyone using one. git is asked which it is, and only for a path that already faulted. That probe wrote the defect it guards against on its first attempt: `[a-z]` matches uppercase under en_US.UTF-8 collation, so the ordinary `H` tag was swallowed and every fault went silent. `[[:lower:]]` does not. `git ls-files -z` read through a process substitution reported the loop's status and never git's -- the producer of the whole inventory carrying the miss its consumers were rewired to avoid two commits ago. A symlink to a target that exists and cannot be read was reported as a broken link, sending the developer after a missing file rather than the permissions that are wrong. Each case reddens against the code as it stood before this commit. Refs #69
|
WORK:REVIEW — token
19 findings, all dispositioned: 16 Not confirmed. The iteration-5 fixes — the sparse-checkout guard, the walk capture, the symlink-target diagnostic — had no adversarial pass over them. Iteration 5 found a real regression this branch had introduced rather than churning on its own output, so the budget ran out while the loop was still productive. Suppressions (governing-ADR re-litigations, all correctly suppressed against
Deferred with owners: #83 (the Guardrails: |
|
Merge-result verification. CI runs on the branch head, not on the merge result, and Merged No file in this change set was touched by any of those 45 commits, and the helper's public interface ( |
Closes #69.
tracked_in_indexin.github/scripts/check-records.shrangit ls-files --error-unmatchwith its status discarded, so an untracked path and an index that could not be read were one answer. Every caller then reported an ordinary negative verdict off a query that never ran — the defect class ADR 0005 records. The sibling site named in the issue's follow-up comment,is_shell_sourceinscripts/list-shell-sources.sh, had the same collapse through a failedread.Measured on git 2.50.1:
git ls-files --error-unmatchexits 1 for an untracked or absent path and 128 for a real fault, so unlikegit cat-fileit separates the two on its own and needs no second witness.The record gate
tracked_in_indexis three-valued — 0 tracked, 1 not tracked, 2 the query could not run — andstill_a_record, which delegates to it, is three-valued with it. Each caller cases on the result:check_no_disappearancesreportedE-GONEfor a record sitting untouched in the tree, and skipped the append-only rules on the way. It now emitsE-TRACKED-SCANand runs neither branch. The two branches that spelled the predicate out separately —present_as_real_file && tracked_in_indexagainst! still_a_record— are onecase, because neither shape can express a third answer.renumbered_elsewheredropped a faulted candidate out of the renumber search with|| continue, so a damaged index removed the real destination and the record read as erased. The fault is remembered and returned only once the loop exhausts, so a genuine match on another candidate still wins.check_gate_filesaccused the change of deleting a gate file that is present and tracked (E-GATE-TRACKED-SCAN), and exempted a declared rename whose successor it could not verify (E-GATE-SUCCESSOR-SCAN).E-RENUMBER-SCANnow names what could not be read. Three origins reach that one code through three different git commands, and the record named as its subject is the one file the search did read.The shell-source inventory
is_shell_sourcedecided fromread's status, which reports a clean EOF on an empty file, a clean EOF on a last line with no trailing newline, and a file that could not be opened — three unrelated things, one of them a fault. The open now decides, andread's status decides nothing. The[[ -f ]]guard above it went too: it answered "not a regular file" and dropped a tracked path missing from the worktree, one behind an unsearchable directory, and a broken symlink as silently as the case below it.Four causes get four diagnostics, because "stage the deletion", "fix the symlink", "replace the FIFO" and "fix the permissions" are not one instruction. Two shapes never reach the open: a directory, which is a submodule's gitlink and the ordinary negative, and anything else that exists but is not a regular file — opening a FIFO left at a tracked path blocks until a writer appears and would hang every gate that consumes the list.
Sparse-checkout and assume-unchanged entries are absent from the worktree by design and are listed by
git ls-filesall the same, sogit ls-files -vis asked which case it is, for a path that already faulted.The lister's own
git ls-files -zwalk is captured rather than read through a process substitution, as are thelintandformat-checkrecipes that consume it:while ... done < <(cmd)reports the loop's status and never the command's, so a discovery that stopped partway produced a short inventory at exit 0.Consequence for a working tree
Deleting a tracked file without staging the deletion now reds
just lint,just format-checkand thecommit-checkhook until it is staged or the file is restored. That is the intended reading of an inventory that cannot be trusted, and the diagnostic says which case it is. A.shname is classified without opening anything, so an unopenable one fails at shellcheck instead.Tests
Seven assertions on the record gate, each red against the unconverted checker. Their
gitstubs key on the path as well as the subcommand, becausetracked_in_indexis that script's onlygit ls-filescaller and its call sites are reached through different fixtures.scripts/list-shell-sources-test.shis new and covers thirteen cases: a shebang with no trailing newline, an empty file, a binary whose first byte is NUL, a sparse-checkout entry, a walk that stops partway, a FIFO, a dangling symlink, a submodule gitlink, an unreadable file, an unreadable empty file, an unsearchable parent, a symlink to an unreadable target, and both real recipes run against a producer that emits a partial list and fails. Each was confirmed red against the code it guards. The permission cases are skipped under uid 0 and the skip reaches the summary line, so a run that tested none of them cannot read as one that did..github/scripts/check-records.shandcheck-records-test.share mirrored byte for byte underskills/tome-of-lore/assets/;just recordscompares them.Verification
just verifygreen (exit 0) on macOS arm64, bash 3.2.57.Review status — read before merging
The
$trial-loopadversarial review ran one cycle of five iterations and ended at its iteration cap with the verdictneeds-attention, notapprove. Every finding across the five passes is dispositioned and the accepted ones are fixed, but the fixes made in iteration 5 — the sparse-checkout guard, the walk capture, and the symlink-target diagnostic — did not receive a confirming adversarial pass. Iteration 5 found a genuine regression this branch had introduced (sparse checkouts), so the loop was still finding real defects when the budget ran out rather than churning on its own output.$dispelwas not run, for the same reason: the cap says do not advance to the next step without a human decision.Deferred with owners:
testrecipe keeps the status-blind process substitution thelintandformat-checkrecipes lost here. Also the sole finding of the$detect-evilpass.renumbered_elsewhereandgate_existed_at. AW-RENUMBER-SCANtrace was added and reverted on this branch: ADR 0005 puts reporting in the caller and specifies the error channel, so a warning-severity scan code emitted from inside a predicate is a change to that record rather than an application of it, and this branch may not amend an append-only record.🤖 Generated with Claude Code