test: give every tracked evidence file exactly one writer - #4
Merged
Merged
Conversation
Two audit tests appended to files another test owns and rewrites whole: gaps.test.ts into evidence-notes.txt, icc-check.test.ts into review-attacks.txt. Vitest runs test files in parallel, so what the tracked file ended up holding depended on which of the pair finished last. Running the appender and then the owner destroyed four recorded lines in the first case and two in the second, silently, with every test still green. Both appenders record real evidence - findings, verdicts, canary survival - not timings, so they get their own tracked file rather than the ignored run log. The lines move across verbatim; no existing evidence text is reworded, reordered or dropped, and neither of the two long-standing files is restructured. EVIDENCE_OWNERS in the harness names the owner of each artifact, and evidence-ownership.test.ts checks that table against what the sources actually write. It counts every write call against the ones whose path it can read, so a path assembled at runtime fails the check instead of slipping past it. Verified by reintroducing both mistakes and watching it go red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #3.
The defect, reproduced rather than assumed
Two audit tests appended to a file another test owns and rewrites whole. Vitest runs
test files in parallel, so what the tracked file ended up holding depended on which of
the pair finished last. Forcing the adverse order with vitest's own file selection — no
test hack — destroyed recorded evidence, silently, with every test still green:
gaps.test.ts→evidence.test.tsicc-check.test.ts→review-attacks.test.tsA full inventory of every
writeFileSync/appendFileSyncinaudit/andsrc/confirmed these were the only two cases — the judgement is not based on filenames.
The fix
Both appenders record real evidence — findings, verdicts, canary survival — not timings,
so they get their own tracked file rather than the ignored run log.
audit/evidence-notes.txtevidence.test.ts+gaps.test.tsevidence.test.tsaudit/review-attacks.txtreview-attacks.test.ts+icc-check.test.tsreview-attacks.test.tsaudit/gaps-notes.txt(new)gaps.test.tsaudit/icc-notes.txt(new)icc-check.test.tsNo citable evidence text changed. The five lines move across verbatim into the file of
the test that produced them. Nothing is reworded, reordered or dropped, and neither
long-standing file is restructured — the remaining diff lines are "no newline at end of
file" shifts.
Regression guard
EVIDENCE_OWNERSin the harness names the owner of each artifact;audit/evidence-ownership.test.tschecks that table against what the sources actuallywrite. It counts every write call against the ones whose path it can read, so a path
assembled at runtime fails the check instead of slipping past it.
Verified by reintroducing both mistakes:
review-attacks.txtAn earlier version of the guard asserted that each declared artifact exists. That check was
itself order-dependent — it ran before the new files had been produced — which is precisely
the fault this PR removes. It was dropped.
Evidence
Windows,
core.autocrlf=true:tsc --noEmitvite buildnpm audit --audit-level=highgit diff --checkgit status --porcelainafter a full audit run on the committed treenpm cicould not run in the working checkout: Windows held an EPERM lock on@rolldown/binding-win32-x64-msvc. Rather than kill unattributable node processes, thelockfile was verified by running
npm ciagainst a copy ofpackage.jsonandpackage-lock.jsonin a scratch directory — clean, 0 vulnerabilities. CI runs the realnpm cion a fresh runner.Merge gate
Full product and audit suites pass, every tracked evidence file has exactly one writer, and
a complete audit run leaves the tracked working tree clean on Windows with
core.autocrlf=true— including under the orderings that previously destroyed evidence.Not in this PR
package-lock.jsonrecords"license": "ISC"whilepackage.jsonsays"MIT", so anynpm installrewrites the lockfile. Same class as this sprint, but out of scope; the changewas reverted and is left for a separate decision.
No product, parser or verification logic is touched. No test removed or weakened. No tag,
release or deploy proposed.
🤖 Generated with Claude Code