Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): the required-context self-test proves WIRING with invocations, not prose (#10877) - #10884
Merged
Merged
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
…lf-test-wiring-presence
…ations, not prose (#10877) Two presence assertions in `--self-test` decided "is this wired up?" from text a comment, a label or an `echo` could supply. Both error toward a silent GREEN — the gate reporting that a required-context pin runs when it does not. - the `lint` job block carried a second `/^\s*#/` line filter and matched `/run: pnpm check:required-contexts\b/` over a raw text slice. It is now a structural read of `jobs.lint.steps`. The slice was also bounded by `'\n typecheck:'`, which is not the job after `lint:` — measured, it covered five jobs. - the caller sweep's single equality was two assertions in opposite directions sharing one WIDE recognizer. Split: the absence half keeps `wiresLiveRead`, the presence half gets `invokesLiveRead`. The two recognizers are deliberately different widths and are pinned together, including one case where they disagree by design. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
…lf-test-wiring-presence # Conflicts: # scripts/check-required-contexts.mjs
Contributor
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32500802189 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang pushed a commit
that referenced
this pull request
Aug 21, 2026
…run (#10889) `invokesScript()` in check-shard-attestation added adjacency for #6589 but tokenized with `command.split(/\s+/)` plus a per-token quote strip, so a quoted string handed to `echo` was torn into words and the script name plus its flag INSIDE that string read as a genuine invocation. The reading is now the quote-aware one #10884 built for the sibling gate, moved to the lexer whose output it consumes rather than mirrored: `commandWords` and `invokes` live beside `shellCommands` in check-shard-attestation, and check-required-contexts imports them over the one import edge that already existed between the two files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
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#10877
The defect
--self-testhad two presence assertions that decided "is this thing wired up?" fromtext a comment, a label or an
echocan supply. Presence assertions fail toward a silentgreen: the symptom is this gate reporting that a required-context pin runs when it does
not — the
#4690shape one of the assertion messages already names.Site 1 — the
lintjob wiring blockA second
/^\s*#/line filter, ~70 lines below the one #10878 repaired, feeding a text matchover a raw slice of
lint.yml. Measured on the checked-in file, it was wrong three ways,all of them false greens about the wiring of a required context:
#- name: checkout # run: pnpm check:required-contexts is belowname:— prose that survives even a correct comment stripper'\n lint:'→'\n typecheck:'typecheck-debt:does not matchtypecheck:, so the slice covered five jobs (lint,typecheck-source-gates,typecheck-workspace,typecheck-debt,typecheck-consumers) — the pin could migrate out of the required job unnoticedIt also had a false red:
/run: pnpm check:required-contexts\b/needs that literal run ofcharacters, which a
run: |block scalar never contains. Genuinely wiring the pin as one ofseveral commands would have reddened the gate.
Now a structural read of
jobs.lint.steps. None of the three shapes above is a fact about thejob at all.
Site 2 — the caller sweep
callers.join(',') === PATROL_WORKFLOWis two assertions in opposite directions sharing onewide recognizer. Measured on
required-set-patrol.yml: of its 7 mentions of the flag, 4are whole-line YAML comments and 3 survive as live text — the genuine
node scripts/check-required-contexts.mjs --verify-required-set, and twoechosteps thatonly report on its exit code (the card predicted one). Delete the genuine invocation and either
echokeeps the patrol classified as the standing caller.Split into the two halves, each on the recognizer its direction needs.
⭐ The asymmetry — why the two halves are NOT unified
Triage made this binding, and the answer is that they must stay separate:
wiresLiveRead, wide — unchangedinvokesLiveRead, narrow — newWidth is not a quality one of them has more of. #10878 was right that wide is correct for the
absence half; the same width is exactly wrong for the presence half. One recognizer serving
both directions is what this file already had — a
/^\s*#/filter, written twice — and itwas wrong in both directions at once for this reason.
What is shared is the traversal:
liveWorkflowTextandrunCommandsare now two readingsof one
visitLivewalk, so arun:key one reading understands and the other does not cannotdrift. The reading is the part that must not be shared, and
invokesLiveRead's docblock says sowith a ⛔ for the next reader.
The disagreement is pinned as an executable fact rather than left to prose: a flag arriving via
env:is wiring for the absence half (#10878's case (h), restated from the other side) andnot an invocation for the presence half.
The reuse decision
check-shard-attestation'sinvokesScript()was the obvious candidate and is not used:it hardcodes
SCRIPT_BASENAME(parameterising it edits the sibling — the route #10878 alreadyrejected, and #10628 had to undo one mirrored helper in a neighbouring file), and it splits
commands on
/\s+/then strips quotes off each piece, so it readsecho "pnpm check:required-contexts …"as running
pnpm.commandWords()tracks which words came out of a quoted region instead —shell does not word-split inside quotes, so that
echois two words, a program and one blob.#at all and runs onshellCommands()output, where comments are already gone. Only the program word must be unquoted; arguments
may be quoted freely, because refusing
"--verify-required-set"would narrow a presenceassertion into a false red.
Evidence
Both limbs throughout. ⭐ A presence test tightened until nothing satisfies it is a worse
defect than the prose it excludes — it shows up as a permanently red self-test, and what a
permanently red self-test gets is loosened. So every prose case is paired with the same wiring
made real, and the checked-in
lint.ymland patrol are asserted as live limbs.origin/mainself-test: 124. #10878: 135. This branch: 150.Reverse verification — two ablations, at the recognizer, no fixture touched. Predicted
counts written down before running; both matched exactly. Each mutation confirmed on disk
by marker counts, never by an editor's exit code, and each restore leg confirmed the same way.
Ablation 1 — the
pinWiredInseam swapped back to the text matcher it replaced(seam text 1 → 0,
ABLATION1marker 0 → 1). Predicted exactly 6; observed 6, verbatim:The real
lint.ymlpresence assertion stayed green under the old matcher — that is thepoint: these six isolate exactly the shapes where old and new disagree, and the sixth is the
old matcher's false red.
Ablation 2 —
invokesLiveReadswapped to the widewiresLiveRead(narrow body 1 → 0,ABLATION2marker 0 → 1). Predicted exactly 2; observed 2, verbatim:Restored from the commit both times (
ABLATION1/ABLATION2markers back to 0, structuralbodies back to 1, worktree clean). No build step is involved — this file is run directly by
node, not resolved through a packagedist/.Gates, all re-run at
c20069e17f— the post-merge head — with a clean worktree, exit codescaptured before any pipe (written to a file, not read from a killed pipeline). Derived with
node scripts/pm/dispatch-gates.mjswith no paths — it takes its own change set from the mergebase.
check:entry-guardandcheck:parse-guardare no longer blind spots inthat derivation. They were hand-run on the first pass because
dispatch-gatescould not reachthem; after #10873 (
let a gate declare a scan surface, not just its baseline artifact) landed,both now derive as ordinary matched families, and both self-tests grew a case for it (47 → 52,
41 → 46). They are still in the table below, now on the derivation's own say-so.
check:required-contexts✓ … --self-test: 150 assertions·✓ …: 6 required context name(s) pinned across 2 workflow(s); 5 instruction surface(s) scannedcheck:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.check-ci-filter-parity.mjsOK: all 82 declared cross-package glob(s) (71 unique) are covered…check:entry-guard✓ …: 129 scripts/ file(s) — every entry guard goes through invoked-as.mjs; 87 export bindings, 77 of them inert on import (10 known-unsafe, ⛔ SHRINK-ONLY)check:parse-guard✓ …: 128 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.check:nul-bytescheck-nul-bytes: OK (scanned 6296 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 reasons about its lexer; that file is unchangednpx eslint scripts/check-required-contexts.mjs --no-inline-config— exit 0, no output.No changeset:
scripts/**only, nothing published changes.Out of scope
The job-level
if:is deliberately not judged, and stays that way —lint.yml's owncomment records the reason (four enrolled
ci.ymljobs carry a filter-drivenif:by design,#4928, and a skipped job still publishes a context). The re-homed assertion stays step-level,
now over
everymatching step rather than the first.Generated by Claude Code
Generated by Claude Code