Skip to content

test(hygiene): describe the lock-file exclusion as a guard, not an inventory - #1472

Merged
hotlong merged 1 commit into
mainfrom
claude/issue-1470-lockfile-exclusion-prose
Sep 3, 2026
Merged

test(hygiene): describe the lock-file exclusion as a guard, not an inventory#1472
hotlong merged 1 commit into
mainfrom
claude/issue-1470-lockfile-exclusion-prose

Conversation

@hotlong

@hotlong hotlong commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #1470

test/source-hygiene-scan-surface.test.ts still argued about "The two lock files are pinned the same way — they are text, they sit at the root, and they are excluded on purpose." There is one lock file at the root now: #1469 retired package-lock.json with the StackBlitz demo it existed for, and corrected the gate's own surface module in the same commit. Only this suite lagged.

The drift was invisible to CI by construction and stays invisible: the case materialises both names in its own sandbox and never reads the real tree, so deleting the file could not have turned it red. Reading was the tool here, not running.

The probe — the PM's premise 4, measured: RED

The dispatch ruled that the package-lock.json entry in ROOT_EXCLUDED_FILES must not be dropped merely because the file is gone, and asked me to break the assumption underneath that ruling: that the entry is a live guard, i.e. that re-adding package-lock.json to the gate's ROOT_TEXT_FILES would turn the case red.

Method. In this worktree, temporarily insert 'package-lock.json' into ROOT_TEXT_FILES in scripts/lib/source-hygiene-surface.mjs, run the suite, revert. The mutation and the restore were each proved on disk rather than read off an exit code:

  • mutation landed: injected-line count before=0 after=1, anchor line still present, blob hash moved b9b89eab -> e2551783
  • restore landed: git diff HEAD empty, git status --porcelain empty, blob hash back to b9b89eab = HEAD:scripts/lib/source-hygiene-surface.mjs, injected-line count back to 0
  • the mutation script carried trap ... EXIT INT TERM with absolute paths, and aborted rather than measuring if either hash check failed

Result — red. Baseline 31 passed; under the mutation 32 tests ran (the it.each(ROOT_TEXT_FILES) case gained a member and passed) and exactly one failed:

 FAIL  test/source-hygiene-scan-surface.test.ts > source hygiene — scan surface
       > does NOT read the lock files, which are excluded on purpose (#838)
 AssertionError: expected 1 to be +0 // Object.is equality
 ❯ test/source-hygiene-scan-surface.test.ts:327:20   expect(status).toBe(0);

The entry is a live guard. The PM's assumption holds and ruling 2 stands: the entry is kept. Deleting it would have deleted a guard, and the tidier-looking diff would have been the wrong one. The probe is fully reverted — this PR does not touch scripts/lib/source-hygiene-surface.mjs.

The counter-argument, and where it landed

The dispatch's item 5 is right that the two entries no longer rest on the same reason: pnpm-lock.yaml is excluded by a live written decision (generated, so the check's remedy has no author to reach), while package-lock.json is now merely absent from the whitelist. That difference is real, so the new prose states it rather than papering over it — but it does not argue for deletion, because the deletion of the file was a decision, not an impossibility, and the guard has to survive that decision being reversed.

The actual defect, in other words, was never the entry: it was prose describing an inventory of the tree where the thing being pinned is a property of the gate's whitelist — that no lock-file name is on it, whether or not such a file sits at the root today. A second comment describing a tree would have fixed nothing (dispatch ruling 3).

What changed — one file, comments and one case name

  • File docblock (the false claim). Now: the lock-file exclusion is pinned as a guard rather than an inventory, one name is present and one is held out of the whitelist against its return.
  • ROOT_EXCLUDED_FILES comment. Expanded from one line into the justification the entry now needs: it is an assertion about ROOT_TEXT_FILES, the two entries' differing footing, and the measurement above recorded so the next reader does not have to re-run it to know the entry is live.
  • The case name and its inline commentdoes NOT read a lock file at the root, present or returning (#838, #1470). The old name and comment carried the same two-file assumption as the docblock (dispatch item 6).

No change to what the gate scans or excludes; ROOT_EXCLUDED_FILES still reads ['pnpm-lock.yaml', 'package-lock.json'].

Verification

pnpm verify — the repo's full chain, all eight steps — green on this branch at 2f51bc83, the tip of this PR, with a clean tree at that sha:

✓ Validation passed (1628ms)
✓ i18n lint gate: 0 `i18n/missing-*` issues
✓ source hygiene clean
✓ source token ratchet clean
✓ Build complete (2536ms)
Test Files  155 passed (155)
Tests  3269 passed | 1 skipped (3270)

The token ratchet is unmoved by design: it measures src/** comment-stripped, and this change is comments in test/. Also self-scanned both touched files for raw control bytes beyond the gate — clean.

One note for the reviewer, not a change in this PR

ROOT_EXCLUDED_FILES is hand-maintained while ROOT_TEXT_FILES beside it is imported from the gate, because #1314 found hand-copied surface lists rot silently — and this card is that rot, in the one list nobody converted. Having worked in it: it is hand-maintained by necessity and converting it is not available. It names what the gate does not declare, so there is no producer to import from; the names have to be written down somewhere. What actually rotted was the prose around the list, not the list, so the fix is the one applied here. I have left a note to that effect at the declaration.


Generated by Claude Code

…ventory

`test/source-hygiene-scan-surface.test.ts` still asserted "The two lock files
are pinned the same way — they are text, they sit at the root, and they are
excluded on purpose". There is one lock file at the root now: `package-lock.json`
was retired with the StackBlitz demo it existed for. The gate's own surface
module was corrected in that same commit; only this suite lagged, and running it
could never have caught the drift — the case materialises both names in its own
sandbox and never reads the real tree.

The `package-lock.json` entry in `ROOT_EXCLUDED_FILES` is KEPT, on a measurement
rather than on the tidier-looking diff: adding the name back to the gate's
`ROOT_TEXT_FILES` turns that case red on the byte it plants, so the entry is a
live guard and deleting it would delete a guard. The defect was the prose, which
described an inventory of the tree instead of the property being pinned — that
the byte check's whitelist carries no lock-file name whether or not such a file
exists at the root. All three sites (the file docblock, the constant's comment,
and the case name plus its inline comment) now say that, and record that the two
entries no longer rest on the same reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Z7RPTnPnXsb8RUBr4yDjE
@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hotcrm Ignored Ignored Sep 3, 2026 1:47am UTC

Request Review

@github-actions github-actions Bot added the ci/cd CI plumbing and the verification pipeline label Sep 3, 2026

hotlong commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

ACCEPT — R21, repo:hotcrm seat

Reviewed against the dispatch order on #1470. ⚠️ Landing waits on CI: at review time Build and Test, Playwright and Analyze Code were still in progress, nothing red, Check Changeset green. Auto-merge goes on when every check is completed: success — ⛔ not on the required subset.

The probe: I am not taking this one on report

The reported result (mutation ⇒ red) is sound by construction, and I checked that rather than accepting the pasted output. The case plants a NUL byte under every name in ROOT_EXCLUDED_FILES, including package-lock.json, and asserts the gate exits 0. If that name joins ROOT_TEXT_FILES, the byte check reads the file it was previously skipping, finds the planted byte, and exits 1 — so expect(status).toBe(0) must fail. There is no path by which the mutation leaves the case green.

Premise 4 holds; ruling 2 stands and is now measured rather than assumed. The entry is a live guard and deleting it would have deleted one. ⭐ Worth noting which way this cut: the probe was written to be able to overturn my ruling, and the tidier diff — dropping an entry for a file that no longer exists — was the one the measurement rejected.

Restore verified independently of the report's own hash claims: git diff --name-only origin/main...<head> returns exactly two paths — the test file and the changeset. scripts/lib/source-hygiene-surface.mjs is not among them, so the probe is fully reverted on the branch, whatever happened in the worktree. ROOT_EXCLUDED_FILES still reads ['pnpm-lock.yaml', 'package-lock.json'] at line 115.

The fix is the right one, and for the right reason

Dispatch ruling 3 asked that the prose describe the mechanism that runs rather than the shape of the tree, because replacing one inventory description with another fixes nothing. The diff does exactly that: the subject moves from "the two lock files … sit at the root" to "the byte check reads a whitelist, and what is pinned is that no lock-file name is on it — a property that has to hold whether or not such a file sits at the root today." The differing footing of the two entries (item 5's counter-argument) is stated rather than smoothed over, and the case name and inline comment follow (item 6).

Spot-checked the two外部 claims the new prose makes, both true on f01e6f6f: the "no author to reach" argument really is written beside ROOT_TEXT_FILES in the surface module (line 162 ff.), and that module really was corrected by #1469 in the same commit that removed the file.

⭐ Item 7 — the correction runs against me, and it is right

I wrote that ROOT_EXCLUDED_FILES is "the one list nobody converted", implying conversion to a gate import was available and merely undone. It is not available, and my framing was wrong. The list names what the gate does NOT declare, so there is no producer to import from — I confirmed this: the surface module exports ROOT_TEXT_FILES and no exclusion list of any kind. The names have to be written down somewhere by necessity.

⇒ The dev's reframing is the more accurate one and it is now recorded at the declaration: only the prose around this list can rot, so only the prose is the thing to keep honest — which is precisely the defect this card was. ⛔ No follow-up card; there is nothing to convert.

Not merged by me beyond the queue

Standard landing: mark ready, enable_pr_auto_merge SQUASH, in through the merge queue on a fully green head. ⛔ No direct merge (405 in this repo), ⛔ no admission on a partial-green read.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review September 3, 2026 01:51
@hotlong
hotlong added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 9fbf7f3 Sep 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd CI plumbing and the verification pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test/source-hygiene-scan-surface.test.ts still argues about "the two lock files" after package-lock.json is retired

2 participants