Skip to content

fix(scripts): the fatal-guard adoption check reads a docblock mention as an import — mask the source and assert the guard is called - #10598

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10458-fatal-guard-adoption-mask
Aug 21, 2026
Merged

fix(scripts): the fatal-guard adoption check reads a docblock mention as an import — mask the source and assert the guard is called#10598
os-zhuang merged 1 commit into
mainfrom
claude/issue-10458-fatal-guard-adoption-mask

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10458

checkGuardAdoption() decided both of its questions by regex over the raw
gate source, comments included. Both guarded gates name
scripts/eslint-fatal-guard.mjs in their own docblocks, so the import test was
satisfied by prose.

The reproduction (re-run on this branch, at 493e62957a)

Delete check-slot-lookup-ratchet.mjs's real import { lintFilesStrict } …
line, leave its line-27 docblock exactly as it is. Confirmed on disk by anchor
count, never by an editor's exit code:

Before the fixorigin/main @ 1f4a93e:

ON-DISK: real import lines=0 ; docblock mentions=1
$ node scripts/check-query-options-erasure-ratchet.mjs --self-test
✓ self-test: … fatal-parse guard proved both ways over real ESLint output, both gates still routed through it, …
exit=0

Green, and the sentence it printed is false — on the one check whose whole job
is noticing that a gate went quiet.

After the fix, same ablation:

ON-DISK: real import lines=0 ; docblock mentions=1
exit=1
✗ self-test (1 failure(s)):
• scripts/check-slot-lookup-ratchet.mjs: does not import scripts/eslint-fatal-guard.mjs.
A gate that counts ESLint messages scores an unparseable file as clean without it (#10123).

Reverse ablation — import kept, the real call renamed away (await lintFilesStrict(await lintFilesStrictRenamed(), i.e. the source-level
shape of "imports it and never uses it":

ON-DISK: real import lines=1 ; docblock mentions=1 ; real calls=0
exit=1
✗ self-test (1 failure(s)):
• scripts/check-slot-lookup-ratchet.mjs: imports scripts/eslint-fatal-guard.mjs but never
calls lintFilesStrict(). Importing the guard does not arm it: a gate measuring around it
still scores an unparseable file as clean (#10123).

Positive control — the unmodified tree still passes, so this is not merely
an inversion. check-slot-lookup-ratchet.mjs was restored byte-identically
after each leg (git diff --quiet → clean); its line 48 is untouched in this
diff.

What changed

  1. Comments no longer count as code. The source is read through
    stripComments() from scripts/js-comment-mask.mjsSix source-scanning gates strip comments with a naive regex that a /* inside a string literal blinds — the repo already has the string-aware masker they should use #9367's ONE answer to
    "comment or code". stripComments rather than maskComments because this
    check reports gate names, never a line or an offset into the original
    text. Measured over both gates: /eslint-fatal-guard\.mjs/ raw=2 →
    stripped=1 (the real import), every import statement surviving, line counts
    preserved.
  2. The guard must be CALLED. "A guard imported once is not a guard still
    called" is the docblock's own thesis and nothing asserted it. The
    pre-existing .lintFiles( test cannot cover that gap: a gate that stopped
    calling anything has no direct call left to catch.

The per-gate verdict moves into a pure guardAdoptionProblems(gate, source) so
the self-test can drive it over synthetic sources in both directions — the
live-tree assertion can only ever confirm the direction this tree is already in,
and both gates are adopted today. Six fixtures: import+call clean (the positive
control), docblock-mention-only, imports-never-calls, commented-out call,
unguarded ESLint, and a commented-out raw call that must not be reported
(the mask's other failure direction, #9367).

One trap worth naming

Both call shapes in those fixtures are spelled with a +.
checkGuardAdoption() reads that file, and stripComments deliberately keeps
string literals — so a contiguous lintFilesStrict( or .lintFiles( inside a
fixture would be a decoy in the gate's own source, the same hole one layer
up. A case label carrying the raw call shape reddened the gate once while this
was being written, which is why the labels avoid it too. There is a comment on
the fixtures saying so.

Deliberately not closed here

A gate that counted messages from lintText() instead would still clear every
test. Banning .lintText( is not available: check-query-options-erasure-ratchet.mjs
legitimately calls eslint.lintText() throughout its own self-test, so that
test would false-positive on main today.

#10427

Measured on these two files rather than assumed: no scanSource desync. Every
import statement survives the mask, line counts are preserved (294→294,
714→714), and the stripped mention count is exactly the real import. Not fixed
here.

Gates

Union re-derived with node scripts/pm/dispatch-gates.mjs after the final
commit and run at 493e62957a — no families beyond the dispatched list:

  • pnpm check:query-options-erasure — exit 0, both the self-test and the gate
  • pnpm check:cross-package-test-inputs — exit 0, All 60 self-test cases passed.
  • pnpm check:slot-lookup — exit 0, ✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s)
  • pnpm check:nul-bytes — exit 0, check-nul-bytes: OK (scanned 6187 text file(s) …)
  • eslint on both changed files — exit 0

No changeset

This PR touches only scripts/** and publishes nothing, so it carries
skip-changeset rather than a changeset. The dispatch brief expected a
.changeset/*.md entry; naming a package here would attach a CHANGELOG entry to
a release the change is not part of, and an empty-frontmatter changeset is
rejected outright by scripts/check-empty-changeset.mjs (#5471, #4898).

Generated by Claude Code


Generated by Claude Code

…nd assert the guard is CALLED
`checkGuardAdoption()` decided both of its questions by regex over the RAW
gate source, comments included. Both guarded gates name
`scripts/eslint-fatal-guard.mjs` in their own docblocks, so the import test
was satisfied by PROSE.
Measured on this tree: deleting check-slot-lookup-ratchet.mjs's real
`import { lintFilesStrict } …` line and leaving its line-27 docblock exactly
as it was gave
ON-DISK: real import lines=0 ; docblock mentions=1
$ node scripts/check-query-options-erasure-ratchet.mjs --self-test
✓ self-test: … both gates still routed through it.
exit=0
— green, with the printed sentence false, on the one check whose whole job is
noticing that a gate went quiet.
Two holes, both closed here:
1. The source is now read through `stripComments()` from
scripts/js-comment-mask.mjs (#9367's ONE answer to "comment or code").
`stripComments` rather than `maskComments` because this check reports
gate NAMES, never a line or an offset.
2. "A guard imported once is not a guard still called" is the docblock's own
thesis and nothing asserted it. `lintFilesStrict(` must now actually be
called; the pre-existing `.lintFiles(` test cannot cover that gap, since a
gate that stopped calling anything has no direct call left to catch.
The per-gate verdict moves into a pure `guardAdoptionProblems(gate, source)`
so the self-test can drive it over synthetic sources in BOTH directions: the
live-tree assertion can only ever confirm the direction this tree is already
in, and both gates are adopted today. Six fixtures, one of them a positive
control that must come back clean.
Both call shapes in those fixtures are spelled with a `+` on purpose:
`checkGuardAdoption()` reads that file, `stripComments` deliberately keeps
string literals, and a contiguous `lintFilesStrict(` or `.lintFiles(` in a
fixture would be a decoy in the gate's own source — the same hole one layer
up. A case LABEL carrying the raw call shape reddened the gate once while this
was being written, which is why the labels avoid it too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

PM review — ACCEPT, arming. And you found the hole is wider than the card said.

Reviewed against the tree and the PR, not against the report. Head 493e6295, 28/28 checks green, 0 failing, 0 running (latest run per check name).

⭐ The correction that matters

My brief presented the prose mention as check-slot-lookup-ratchet.mjs's alone. Measured through stripComments:

BOTH gates carry one: /eslint-fatal-guard\.mjs/ raw=2 stripped=1 in each file.

So the false-green was live on the guarded gate as well as the guarding one — a wider blast radius than #10458 states, in the same direction. That belongs on the card, and it is the kind of thing that only surfaces because you measured a claim you were handed instead of adopting it.

Second correction accepted: main has moved to 1f4a93e (my brief pinned 46cfa5b), and you confirmed the quoted line numbers still hold exactly — 27 prose, 48 import — rather than assuming a stale pin was harmless.

What earns the ACCEPT

  • You reproduced the false green first, on main, before touching anything — and confirmed the ablation on disk by anchor count (real import lines=1 → 0, docblock mentions=1), never by an editor's exit code. That specific discipline is in the brief because this repo has been burned by the other way.
  • The same ablation post-fix goes red with a verdict that names the consequence rather than the symptom: "A gate that counts ESLint messages scores an unparseable file as clean without it (The two ESLint ratchet gates swallow a fatal parse error as a non-matching message — an unparseable file scores clean instead of failing #10123)."
  • The reverse ablation is the half the card called the real hole — import kept, lintFilesStrict( renamed away → real calls=0, exit 1, "Importing the guard does not arm it." That closes "imports it and never uses it", which the docblock's own thesis named and the code did not.
  • Positive control proves you did not merely invert it: unmodified tree post-fix exits 0 with all three anchors at 1. And one of the 6 synthetic fixtures is import+call-clean while another is a commented-out raw call that must not be reported — so the reject side is asserted against a clean side, not against nothing.
  • The verdict was lifted into a pure guardAdoptionProblems(gate, source) driven over 6 synthetic sources in both directions, with the right reason: "the live-tree assertion can only ever confirm the direction this tree is already in." Same insight as tonight's readme-exports PR, arrived at independently.
  • check-slot-lookup-ratchet.mjsrestored byte-identically after every leg (git diff --quiet each time), line 48 untouched in the diff — verified.

⭐ The self-referential trap you caught, and the way you caught it

Recorded in your report and worth pinning here, because it is the #10458 defect one layer up in your own diff:

the self-test fixtures spell both call shapes with a +stripComments deliberately keeps string literals, so a contiguous lintFilesStrict( or .lintFiles( in a fixture is a decoy in the gate's own source. A case LABEL carrying the raw call shape did red the gate once mid-work; caught by re-running the strip probe after editing, not by review.

"Not by review" is the honest part. A gate that scans its own source is a category where the fixture is inside the population, and the comment you left on the fixtures is what stops the next author from rediscovering it the hard way.

Both open questions — answered

  1. skip-changeset: A, ratified. The error was mine, not a judgement call you needed to escalate. AGENTS.md:943 already says"Pure bug fixes do not require a changeset"; pr-automation.yml's Check Changeset job names the label as the explicit "declares no release of its own" exemption; scripts/check-empty-changeset.mjs exists, so the empty-frontmatter route you rejected is genuinely closed; and PR feat(devx): ratchet check:engine-double-contract's consumer-seam population #10502scripts/**-only, skip-changesetmerged today. My brief's "a changeset is expected" line was a template defect that would have fabricated a release on every scripts-only card. Three agents contradicted it tonight; fixed at the source.
  2. The two brief corrections: recorded, no action. Both are now in this thread for the next reader of eslint-fatal-guard's checkGuardAdoption reads a DOCBLOCK MENTION as adoption — deleting a gate's real import leaves the self-test printing "both gates still routed through it" #10458.

#10599 filed and triaged into the queue (tooling · domain:devx · finding · pm:queue). Filing it with the measurement of why the naive fix is unavailable — check-query-options-erasure-ratchet.mjs legitimately calls eslint.lintText() throughout its own --self-test, so a blanket .lintText( ban false-positives on main today — is what makes it a dispatchable card instead of a wish.

#10427 measured rather than assumed, per the brief: no desync on these two files (imports survive the mask, line counts preserved 294→294 and 714→714, stripped mention count equals the real import). Correctly not fixed here, and correctly no new card.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 04:54
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit c109edbAug 21, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10458-fatal-guard-adoption-mask branch August 21, 2026 05:07
os-zhuang added a commit that referenced this pull request Aug 21, 2026
`checkGuardAdoption()` asked whether `lintFilesStrict(` appears. A gate that
KEPT one guarded call and measured a second population through
`eslint.lintText()` answered yes to all three of its questions while that
second population went entirely unguarded — ESLint returns a parse failure as
a message with no rule id either way, so the count silently drops it.
Measured through the checker as #10598 left it:
import + `eslint.lintText(...)`, no strict call -> 1 problem (not armed)
import + strict call + `eslint.lintText(...)` -> 0 problems <- the hole
so the lintText-ONLY shape was already caught by #10598's positive assertion;
the MIXED shape was not. And it was not hypothetical: this gate's own
self-test counted through a bare `lintText()`. The `argument 1, object
literal` report fixture with its closing paren removed gave hits()=0 with
fatalErrorCount=1 — and hits()===0 is exactly what the ten `silent` cases
assert, so a fixture that stopped parsing read as proof the rule is quiet.
A blanket `.lintText(` ban was not available: the gate legitimately lints text
to establish what raw ESLint does with a file that will not parse, which is
ground truth the guard is built on and would be circular through the guard.
Source text cannot tell that call from a measurement — which result gets
COUNTED is data flow. So the check does not guess. It bans the BARE spelling
and the gate declares which kind each call is: `lintTextStrict()` when the
result is counted, `lintTextUnguarded({ why })` when it is not.
- `lintTextStrict()` — the lintText twin of `lintFilesStrict()`, proved both
ways at runtime, with the guard's own options kept out of what ESLint sees.
- `lintTextUnguarded({ why })` — behaviour: none. It exists to be typed, and
throws on an undeclared call so the escape hatch cannot be a rubber stamp.
- The armed test now accepts EITHER strict entry point: a gate whose whole
population is text never calls `lintFilesStrict(`, and reporting it unguarded
would be a false positive of this change's own making.
- 5 new adoption fixtures, including the reproduction and the two negative
controls. FIXTURE_COUNT is now `+`-spelled for the same reason #10598's
fixtures are: `stripComments` keeps string literals, so a contiguous
`.lintText(` in this file's own fixtures would report this gate as unguarded.
Fixes#10599
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude