fix(phi-scan): refuse a scan root the walk never observed - #38
Merged
Conversation
…BSERVED-NOTHING-IS-GLOBAL) `pnpm phi-scan` with no arguments, which is what CI runs, printed `OK, no hits` and exited 0 over a corpus it never opened. Each scan root's walk is now reconciled against `git ls-files`, and two independent conditions refuse at exit 2: the root contributed nothing, or git tracks an in-scope file under it that the walk did not open. Six states measured on this tree, every one of them previously exiting 0 with `OK, no hits`: the fixture root missing; the fixture root emptied; the fixture root a dangling symbolic link; the fixture root a live symbolic link to a directory outside the repository; one tracked fixture removed from the working tree with the rest of the root still opened; and `src` moved away. 8 of the suite's tests run red on cd221a0 and green here. Neither condition subsumes the other, so both ship: an emptied root opens nothing, a swapped one opens plenty, and refusing only a MISSING root leaves the other half open, because existence is not observation. The dangling case is why a kind check cannot stand in: `existsSync` FOLLOWS the link and answers false, so `walk()` returns before `readdirSync` and the not-a-regular-file refusal never fires, leaving nothing about the entry ever inspected. A denominator was deliberately NOT added. A count counts the roots and the files that did exist, so a healthy-looking total is exactly what a starved root produces. Exit 2 was derived from this scanner's own contract, not ported from a sibling: `1` means "hits found" here, `walk()` already raises an unreadable root as an invocation failure, and a root replaced by a regular file already exits 2 through `readdirSync` (re-measured, not assumed). Sibling scanners disagree on this code. The rule is one-directional (a tracked file the walk missed refuses, an untracked one it found does not) and scoped to all mode: `--staged` is a diff rather than a corpus, and widening it would change what a commit is blocked on. `git ls-files` failing refuses rather than answering the empty set. Two of the three disclosed escapes are narrowed, not closed, and the module header says so: a scan root that is itself a live link is still followed and survives only where git tracks nothing under it, and an ancestor of a scan root is still out of the staged route's scope. Paths mode is untouched. Also fixed: a present-but-unreadable `phi-scan-overrides.md` threw a raw EACCES past every handler and node exited 1, this contract's code for "hits found". It now exits 2, matching the allow-list reader beside it. The two other minors a sibling named were re-derived and measured NOT open here. Measured for the record: 122 tracked files, 34 opened by the all-mode walk, 88 scanned by neither route, 6 of those carrying an inline `PID|` literal. All six were read by hand and are placeholder shapes, no real identifier. That 88 is an enumeration gap and a different item; this change widened no root, so the recogniser needed no widening.
…ed path once Three findings from the conformance gate on e72dba6, all INTRODUCED by that commit and none of them a change to what the rule refuses. 1. THE RESIDUAL DISCLOSURE WAS FALSE AND WAS FALSIFIED IN ONE RUN. `CLAUDE.md`, `CHANGELOG.md` and `documentation/agent-notes.md` all said the live-link scan-root escape "survives only where git tracks nothing under it". It does not. The reconciliation compares PATH SETS, not the bytes git carries at those paths, so a target directory mirroring this repo's own seven tracked fixture NAMES satisfies both conditions and the gate prints `OK, no hits` at exit 0 over decoy contents. Measured. The module header had it right; the three restatements dropped the qualifier and inverted it. All four surfaces now name the surviving shape, and a root git tracks nothing under is stated as the degenerate case of it rather than the whole of it. An inaccurate disclosure is a defect in its own right here, so the remedy is the claim and deliberately not a bigger guard. 2. THE COUNTS WERE STALE IN THE COMMIT THAT SHIPPED THEM. 122 tracked, 88 in neither route and 6 carrying an inline `PID|` literal are all correct as of cd221a0 and all move the moment this slice adds a file, including the note that states them, which contains the literal it counts. They are now anchored to that sha, with the reason written next to them. 3. AN UNMERGED PATH WAS NAMED ONCE PER STAGE. `git ls-files` emits three records for a conflicted path, so one missing fixture was reported as three. `trackedUnder` de-duplicates. The refusal and its exit code were right either way; only the diagnostic was wrong. Two tests added: one pins the surviving live-link shape so the disclosure cannot quietly revert to the absolute wording, and one pins the unmerged count, asserting the three stages exist before asserting the one line.
Two minors from the second gate pass, both stale text and neither a change to behaviour. The red-test count went stale INSIDE the paragraph that had just been added to stop counts going stale. It read "8 of the suite's tests run red on cd221a0", true at e72dba6 when the suite held 45 tests; the remedy's own added test moved it. Re-derived against a clean tree: 9 of 47 red under cd221a0's scanner, 38 green. The denominator is now named, and so is the reason, because this is the second time in one slice. The module header's residual list said TWO while shape (1) sixty lines above it had just been corrected to describe a third: the reconciliation compares path sets rather than the bytes git carries at those paths, so a directory mirroring the tracked names clears both conditions with decoy contents. That is now listed as a residual in its own right, with the tracks-nothing case stated as its degenerate form, and with the note that comparing blobs would be a different and larger rule that is deliberately not taken here.
Contributor
Author
|
Merging on pass 2's NOT REFUTED. Flagging plainly: commit Separately, and NOT fixed here: |
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.
What was wrong
pnpm phi-scanwith no arguments, which is what CI runs, printedOK, no hitsand exited 0 over a corpus it never opened. A PHI tripwire that has never been pointed at an input has not cleared that input.Six states, every one measured on this tree and every one previously exiting 0 with
OK, no hits:test/__fixtures__missingtest/__fixtures__emptiedtest/__fixtures__a dangling symbolic linktest/__fixtures__a live link to an outside directorysrcmoved awayWhat it does
Each scan root's walk is reconciled against
git ls-files, and two independent conditions refuse at exit 2: the root contributed nothing, or git tracks an in-scope file under the root that the walk did not open.existsSyncFOLLOWS the link and answers false, sowalk()returns beforereaddirSyncand the existing not-a-regular-file refusal never fires. Nothing about the entry is ever inspected.1means "hits found" here;walk()already raises an unreadable root as an invocation failure, and a root replaced by a regular file already exits 2 throughreaddirSync(re-measured, not assumed). Sibling scanners disagree on this code.git ls-filesfailing REFUSES rather than answering the empty set, which would be indistinguishable from "this root tracks nothing".--stagedis a diff and not a corpus, and widening it would change what a commit is blocked on.Also fixed: a present-but-unreadable
phi-scan-overrides.mdthrew a rawEACCESpast every handler and node exited 1, this scanner's code for "hits found". It now exits 2, matching the allow-list reader beside it. The two other minors a sibling item named were re-derived and measured not open here.What is NOT closed, and is disclosed on four surfaces
The live-link scan-root escape is narrowed, not closed. The reconciliation compares path sets, not the bytes git carries at those paths, so a target directory mirroring the tracked names clears both conditions with decoy contents, at exit 0. A test pins that so the disclosure cannot quietly revert to the absolute wording. An ancestor of a scan root is still out of the staged route's scope, and paths mode still follows a link a caller names.
Measurement
As of
cd221a0: 122 tracked files, 34 opened by the all-mode walk (7 fixtures + 27 sources), 88 scanned by neither route, and 6 of those carry an inlinePID|literal. All six were read by hand and are placeholder shapes (DOE^JANE,X^^^H^MR,123^^^HOSP); no real identifier. That 88 is an enumeration gap belonging to a different item; this change widens no root, so the recogniser needed no widening and none was added.9 of test/scripts/phi-scan.test.ts's 47 tests run red againstcd221a0's scanner and green against this one.Gates
scripts/verify.sh cligreen on every step except the pre-existingpnpm auditadvisories under@modelcontextprotocol/sdktransitives (this branch touches neitherpackage.jsonnorpnpm-lock.yaml). 403 tests, coverage 97.7%.Conformance gate: pass 1 REFUTED (a false residual disclosure, stale counts, an unmerged path named once per stage), pass 2 NOT REFUTED, pass 3 narrow on the remedy diff.