Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): check-required-contexts reads comments as prose, not as wiring (#10818) - #10878
Conversation
… wiring (#10818) `uncommentedYaml` dropped a line only when its first non-space character was `#`, so a TRAILING `# --verify-required-set` on a live line — and a trailing shell comment inside a `run:` block scalar — survived the strip and reddened `Lint & Repo Gates` on prose. A workflow stacks two comment grammars and one line filter was wrong about both. Each now goes to the thing that knows it: `yaml.parse` for the YAML layer, and check-shard-attestation's `shellCommands()` lexer — imported, not re-typed — for the shell inside each `run:`. Both limbs are pinned in `--self-test`: four prose shapes that must NOT read as wiring, and six live shapes that must. The trailing-comment pair differs by the `#` alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
✅ ACCEPT — reviewer of record: |
| flag | files naming it on origin/main |
|---|---|
bare --verify | 20 — many programs take it (git rev-parse, git show-ref, gpg, …) |
--verify-required-set | 3 — lint.yml, required-set-patrol.yml, and the script itself |
So for the sibling, adjacency removes real false positives; here there is no other program to collide with, and adjacency would trade a false positive that cannot occur for false negatives that can — the flag arriving via a shell variable, or an invocation spelling nobody enumerated.
⭐ Copying a neighbour's mechanism because it is stronger is the easy move, and it would have been wrong. Keeping width outside run:on purpose, and pinning it, is the part that makes this a decision rather than an omission.
The reuse choice is equally measured
Imported the already-exported shellCommands() rather than hand-rolling a second lexer — citing the js-comment-mask.mjs history where two private stripComments families drifted apart in opposite directions — and rather than extracting a shared module, which #10628 had to undo. Result: one file changed (+301/−10), check-shard-attestation.mjsuntouched — confirmed in the diff.
Handing each of the two stacked grammars to the thing that knows it (yaml.parse for the YAML layer, the shell lexer for each run:) is the right decomposition. A single regex was never going to hold two grammars.
Both limbs pinned, and the pairs differ by one character
4 prose shapes that must not read as wiring, 6 live shapes that must — and "the same command minus the # — the pair differs by the comment marker alone." That construction is what makes the pins falsifying rather than decorative: nothing but the thing under test varies between the two.
⭐ And the ablation was placed at the recognizer, not the fixtures — you refactored so fixtures are workflow source and wired() is the whole pipeline, letting the recognizer be swapped without touching a fixture. Predicted exactly two red, observed exactly two, both named. Baseline confirmed by running origin/main's copy standalone: 124 assertions, green — the defect was latent exactly as the card said. Branch: 135 (+11).
Two orphans you re-homed rather than left dangling
Deleting the stripper orphaned two consumers, and you fixed both rather than repointing them at the nearest thing:
- the patrol's
merge_groupcheck now reads the trigger block through this file's already-pinnedtriggersOf(), and ⭐ stops being vacuously true when there is no readableon:— that is a second latent false-green, found and closed in passing; - an unparseable workflow is now a named failure instead of a silent classification (check:react-declaration-parity 是唯一没接进任何 workflow 的源码审计门禁,且无 MANIFEST 时静默 skip 退出 0 —— 它现在永远不可能红 #4690's rule).
#10877 — correctly filed, and it is the harder half
the self-test proves its own WIRING with text a comment can supply — a second, distinct
/^\s*#/filter feeding a PRESENCE assertion
⭐ Opposite failure direction: this PR fixes a false red; that one is a false green. And you noted the sting — its fix wants the narrow recognizer this PR argues against for the absence half. So the two halves of this file may need different strictness, which is exactly the kind of thing that must not be decided as a rider. Filing it unlabeled for triage rather than riding it along was right; I am triaging it now.
Flipping ready and arming.
Generated by Claude Code
Uh oh!
There was an error while loading. Please reload this page.
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 32497810118 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
跨 PR 相同签名（24h，按失败测试文件聚合）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes#10818
The defect
scripts/check-required-contexts.mjsstripped comments withwhich drops a line only when its first non-space character is
#. A workflow filestacks two comment grammars, and that one line filter is wrong about both:
inside a quoted scalar — so a trailing
# --verify-required-setsurvived whole;run:block scalar is not YAML at all, it is shell —#opens a comment at a wordboundary,
#inside quotes is an argument, and a backslash-newline continues the command.A trailing shell comment survived there too.
Both surviving shapes reddened two assertions on
Lint & Repo Gates— the absence assertion("must not RUN the live required-set read") and the caller sweep (a second caller that does
not exist) — on prose that runs nothing. Latent today, and the trigger is writing a warning
about this very flag, which is what the neighbouring
ci.ymlblock already does forcheck-shard-attestation's sibling flag.The fix: each grammar goes to the thing that knows it
yaml.parseanswers the YAML layer (comments are not part of a parsed document, leading ortrailing; a
#inside a quoted scalar stays in the string).shellCommands()—check-shard-attestation's lexer, imported, not re-typed — answers the shell layer inside
each
run:. Nothing here parses either grammar by hand.The reuse decision, measured
Triage left the choice open. Three routes were on the table:
stripCommentsfamilies drifting apart in opposite directions is the measured historyjs-comment-mask.mjsexists to have ended; this would be the third family. And it would be wrong: neither YAML comments nor shell comments are line-shaped.shellCommands()/invokesScript()into a new shared moduleshellCommandsis already exported and already import-safe by construction (check-shard-attestation.mjs's own #10667 note: "an import for those exports alone must run nothing"). Ninescripts/check-*.mjsfiles already import from a siblingcheck-*— this is the tree's own convention, so a new module would add a hop without removing one.It also deliberately does not adopt check-shard-attestation's stricter
invokesScript()adjacency test. That test earns its keep there because
--verifyand--emitare flagsother programs genuinely take (
git rev-parse --verifyis what #6589 misread).--verify-required-setis spelled nowhere else in this tree, so adjacency would remove afalse positive that cannot occur, while introducing false negatives that can — the flag
passed through a shell variable, or by a spelling of the invocation the file did not think to
enumerate. Erring wide is the safe direction for an absence pin: a false red names a file
and a line, a false green is a gate that quietly stopped guarding. Outside a
run:block thescalar is therefore kept whole, so a flag arriving via an
env:value, awith:input or amatrix entry still reads as wiring — pinned as case (h).
Two call sites moved from text shapes to structure
Both were consumers of the deleted stripper, so both had to be re-homed:
merge_groupcheck now reads the trigger block through this file's ownalready-pinned
triggersOf()instead of/^\s{0,4}merge_group\s*:/m. It also stopsbeing vacuously true when there is no readable
on:block at all;(check:react-declaration-parity 是唯一没接进任何 workflow 的源码审计门禁,且无 MANIFEST 时静默 skip 退出 0 —— 它现在永远不可能红 #4690). It falls back to the widest possible reading first, so it can only ever
over-report a caller, never hide one.
Evidence
Both limbs are pinned, because ⭐ a comment-stripper that swallowed the genuine mention
would be a strictly worse defect than the false positive it fixes — the live read wired into
a required job with this gate green about it. Ten cases: four prose shapes that must not read
as wiring, six live shapes that must. The trailing-comment pair
ls # --verify-required-set/
ls --verify-required-setdiffers by the#alone.Reverse verification (single variable, at the recognizer, not at the fixtures). The
fixtures are workflow source and
wired()is the whole pipeline, so the recognizer can beswapped without touching a fixture. Predicted before running: exactly two pins red — the
trailing YAML comment and the trailing shell comment — with the whole-line pins and all six
live limbs staying green. Mutation confirmed on disk by marker count (
const wired = (source) => wiresLiveRead(parse(source));1 → 0,ABLATION0 → 1), not by an editor's exit code.Observed, verbatim:
Restored from the commit (
ABLATIONmarker back to 0, worktree clean). That the other eightpins stayed green under the old filter is the point: the two new pins isolate exactly the two
shapes the card names, and the six live limbs guard the new recognizer rather than
restating the old one.
Pristine
origin/mainself-test: 124 assertions, green — the defect is latent, as filed.This branch: 135.
Gates, all at
b5ef0d6c9dwith a clean worktree, exit codes captured before any pipe(
cmd > log 2>&1; ec=$?). Derived withnode scripts/pm/dispatch-gates.mjs(no paths — ittakes its own change set from the merge base), plus the two it is structurally blind to:
check:required-contexts✓ … --self-test: 135 assertions·✓ …: 6 required context name(s) pinned across 2 workflow(s)check:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declaredcheck-ci-filter-parity.mjsOK: all 82 declared cross-package glob(s) (71 unique) are coveredcheck:entry-guard(hand-run)✓ …: 129 scripts/ file(s) — every entry guard goes through invoked-as.mjs; 87 export bindings, 77 of them inert on importcheck:parse-guard(hand-run)dispatch-gatesreports this one unreachable by construction, which is why it is hand-runcheck:nul-bytescheck-nul-bytes: OK (scanned 6292 text file(s) … no raw ASCII control bytes)check:shard-attestation✓ …: 92 assertions·✓ …: 2 aggregate gate(s) count 3 declared leg(s)— run because this PR now imports from it; the file itself is unchangedeslint scripts/check-required-contexts.mjs --no-inline-config— exit 0, no output.No changeset:
scripts/**only, nothing published changes.Out of scope, filed separately
Filed as #10877 — not touched here. The block immediately below carries a second, distinct
/^\s*#/filter, and its failure direction is the opposite one: a presence assertionthat a trailing comment could satisfy, i.e. a false green about wiring that is not there.
Repairing it is not mechanical (it reads raw job/step text by indentation) and it wants the
narrow recognizer this PR argues against for the absence half — two halves of one function
pulling in opposite directions, which is a design call rather than a swap.
Generated by Claude Code