Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): six source-scanning gates share one string-aware comment masker - #9445
Merged
Merged
Conversation
…t masker Each gate answered "is this span a comment or code?" with its own private `stripComments`, and the copies had drifted into two families that fail in opposite, silent directions: - naive regex (dispatcher-error-vocabulary, error-code-casing, platform-checklist) -- string-blind, so a `/*` inside a glob or route wildcard, or a `//` inside a URL, opens a phantom comment that deletes real code. The gate reports clean over source it never read. - string-aware scanner, regex-blind (error-status-conformance, examples-live-imports, test-source-alias) -- a regex literal holding a quote character opens a phantom STRING, and because a scanner SKIPS string spans, comments inside one are never removed. The gate reads commented-out text as live code. The scanner that gets both right already existed inside scripts/pm/dispatch-gates.mjs, but that module runs CLI code at import and exits(2), so no gate could import it. Moved to scripts/js-comment-mask.mjs with no import-time side effect; dispatch-gates re-exports `maskComments` so its own self-test still drives the shared masker rather than a copy. Two projections over one scanner: `maskComments` blanks (offsets and line numbers survive) and `stripComments` deletes the comment characters but keeps newlines (line numbers survive, offsets do not). check-test-source-alias takes the deleting one -- its import regex is lazy, and blanking is quadratic over the whitespace it leaves: measured 6.4s -> 5m27s before the projection split, same verdict. The self-test pins the shapes rather than the corpus: a green run over today's tree proves only that today's tree lacks the shape.
os-steve
marked this pull request as ready for review
August 18, 2026 01:34
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 18, 2026
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 freeto 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.
Fixes#9367
Six source-scanning gates each carried a private
stripComments. They now share onestring-, template- and regex-aware scanner in
scripts/js-comment-mask.mjs.What the card assumed, and what measuring found
The card lists six gates "using the vulnerable spelling". Re-derived from the source on
51a46a440, they are not one defect — they are two, failing in opposite directions:check-dispatcher-error-vocabulary.mjscheck-error-code-casing.mjscheck-platform-checklist.mjscheck-error-status-conformance.mjscheck-examples-live-imports.mjscheck-test-source-alias.mjsThe three the card describes are string-blind, so a
/*in a glob or route wildcard, or a//in a URL, opens a phantom comment that deletes real code. The other three alreadytrack strings — the card's prose is stale for them. Their bug is the mirror image: a regex
literal whose class holds a quote character opens a phantom string, and because a
scanner skips string spans, comments inside one are never removed. Those gates read
commented-out text as live code.
All six are defective; none was already fixed.
check-test-source-alias.mjswas closest —it knew about regex literals, but decided regex-vs-division from the preceding character
alone, which misses the keyword forms -- a regex literal opening right after
returnorcase, whose character class holds a backtick.Reachability, per gate — two are latent
Each gate's own strip was run against its own real corpus and diffed against the shared
scanner:
check-dispatcher-error-vocabularycheck-error-code-casingcheck-examples-live-importscheck-error-status-conformancecheck-platform-checklistcheck-test-source-aliasSo the hazard is live for four gates and latent for two. For
check-platform-checklistall eight pinned enum slices extract an identical member list either way; the earlier
whole-file difference does not reach what the gate actually reads.
The verification that matters
A green run over today's corpus proves only that today's corpus lacks the shape, so each
gate was driven end to end against a planted fixture, once with the new script and once
with the original from
51a46a440:check-error-code-casingcheck-dispatcher-error-vocabularycheck-error-status-conformancecheck-examples-live-importscheck-test-source-aliascheck-platform-checklistEvery OLD failure above is manufactured out of, or hidden by, a comment. The two
fabrication rows are the sharper result: the old gate fails on source that is correct.
The shapes themselves are pinned as self-test cases in the shared module (15 cases,
node scripts/js-comment-mask.mjs --self-test), driven through both projections, andeach case is valid JavaScript so the expected answer is the language's, not an
implementation's. An earlier draft case was discarded because the reference disagreed with
it — a
*+/inside a docblock genuinely does close the comment.Why the helper moved instead of being imported
The card proposes importing
maskCommentsfromscripts/pm/dispatch-gates.mjs. That doesnot work: the module runs CLI code at import and calls
process.exit(2), so any gateimporting it dies with a usage error. The scanner moved to
scripts/js-comment-mask.mjswith no import-time side effect;
dispatch-gates.mjsre-exportsmaskComments, so its own260-case self-test still drives the shared masker rather than a copy.
Two projections, and a 51x regression that measuring caught
maskCommentsblanks comment spans (offsets and line numbers survive).stripCommentsdeletes the characters but keeps newlines (line numbers survive, offsets do not).
This split is not tidiness. Converting
check-test-source-alias.mjsto the blankingprojection alone took it from 6.4s to 5m27s — same verdict, 51x the cost. Its import
matcher is lazy (
(?:import|export)\s+([\s\S]*?)\s*from), and a lazy[\s\S]*?draggedacross the whitespace blanking leaves behind is quadratic in the comment bytes. On the
deleting projection it is back to 6.7s. Gates that report a line or an offset take
maskComments; gates that report neither takestripComments.Scope
Only the six gates the card names, plus the shared module and the
dispatch-gates.mjsre-export the move requires.
scripts/docs-audit/affected-docs.mjsis not touched — PR#9394 holds it. No seventh gate was swept. One more script does carry the same defect class --
scripts/check-org-identifier.mjstruncates each line at the first double slash, so a URL onthat line hides the read it looks for -- and per the card's rulings it is reported, not fixed:
filed as #9444.
Note
check:platform-checklistis not a CI gate — lint.yml runs it by maintainer decision,on demand — so that gate's change is covered by the local run below, not by this PR's checks.
Gates run locally, on
9083e1c75check:dispatcher-error-vocabulary,check:error-code-casing,check:error-status-conformance,check:examples-live-imports,check:test-source-alias,check:pm-dispatch-gates,check:platform-checklist,check:nul-bytes— all exit 0, plusnode scripts/js-comment-mask.mjs --self-test(15 cases). Families re-derived from theactual changed paths with
node scripts/pm/dispatch-gates.mjs.No changeset: root
scripts/only, nothing published.Generated by Claude Code
Generated by Claude Code