Uh oh!
There was an error while loading. Please reload this page.
docs(ci): correct the shard-classifier comment to describe the post-#6589 classifier - #10817
Merged
Merged
Conversation
…6589 classifier The "Compute this shard's package set" step explained its `git cat-file -e` spelling with three claims about check-shard-attestation.mjs, all of which describe the classifier as it stood BEFORE #6589: - "the job's joined `run:` text" -> classification is `jobInvokes`, applied per STEP; the joined text is built only after a job is already classified a gate, to extract its --leg tokens. - the flag counts "as a bare substring" -> `invokesScript()` tokenises each command and requires the flag to be an exact argument token following the script's own name IN THE SAME COMMAND. - "including in a comment, since comments are part of `run:`" -> the `shellCommands()` lexer drops `#` comments outright. The comment was authored in e4737b6 (#6195); the classifier was rewritten later the same day in 4e271b2 (#6589), so it has described a classifier that no longer exists ever since. The step's behaviour is untouched: `git cat-file -e` stays. Only its stated reason changes, because the hazard it cites is now closed by construction. The corrected comment deliberately spells `--verify` literally, which the old comment avoided. That is the load-bearing check: under the pre-#6589 co-occurrence rule this text would reclassify job `test` as an aggregate gate, and `pnpm check:shard-attestation` reports the identical verdict before and after -- "2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
os-zhuang
marked this pull request as ready for review
August 21, 2026 13:18
Uh oh!
There was an error while loading. Please reload this page.
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#10713
The Compute this shard's package set step in
.github/workflows/ci.ymljustified itsgit cat-file -espelling with three claims aboutscripts/check-shard-attestation.mjs.All three describe the classifier as it stood before #6589. This corrects the comment
only — the step's behaviour is untouched and
git cat-file -estays.The comment was authored in
e4737b6ac2(#6195) and the classifier was rewritten laterthe same day in
4e271b2c67(#6589), so the text has described a classifier that nolonger exists ever since.
The three claims, re-derived from the classifier source
Each row was settled against the code, not against the script's docblock (which is prose
and could itself have gone stale) and not against the card's table.
run:text"scanWorkflowbuildsjobInvokes = (job, flag) => stepsOf(job).some((step) => invokesScript(step?.run, flag))and classifies withif (!jobInvokes(job, '--verify')) continue;invokesScriptlexes the text into commands, locates the token equal toSCRIPT_BASENAME(or ending/+ it), then requirestoken === flag || token.startsWith(flag + '=')among the tokens after it in that same commandrun:"shellCommandshitsif (ch === '#' && atWordStart), skips to the newline and flushes#comments are droppedTwo details worth recording, because neither is visible from the docblock alone:
runTextOf(job)is still built, but only aftera job has already been classified as a gate, and only to extract its
--legtokens. Itno longer participates in classification at all.
runTextOfjoins with a newline, andthe lexer already treats a newline as a command boundary. Either guard alone would defeat
the check-shard-attestation.mjs misclassifies a shard job as an aggregate gate on a bare
--verifysubstring anywhere in its run: text (false red, measured) #6589 shape.Why the corrected comment now spells
--verifyliterallyThe old comment went out of its way to write "dash-dash-verify" instead of the flag. That
avoidance is no longer necessary, and demonstrating it is the point — so the replacement
names the flag three times, in the very step that already carries the script's basename in
its
--emitstep. That makes this the load-bearing measurement rather than a vacuous one:The pre-#6589 rule quoted there (
text.includes(BASENAME) && text.includes('--verify')) isthe one the classifier's own docblock records as the defect #6589 removed. The safety of
this text is pinned upstream by two
--self-testassertions — a--verify-in-a-commentfixture and a trailing-comment fixture.
Evidence
pnpm check:shard-attestation, before and after, verdict lines quoted — the shard job'sclassification is unchanged:
Gate union re-derived by
node scripts/pm/dispatch-gates.mjs(no paths passed — it takesits own change set) and re-run at final commit
db6e53c3bd. Exit codes captured withcmd > file 2>&1; ec=$?, never through a pipe:pnpm check:node-versionpnpm check:required-contextspnpm check:shard-attestationpnpm check:workflow-status-functionsnode scripts/check-aggregator-roster.mjsnode scripts/check-required-contexts.mjsnode scripts/check-shard-attestation.mjspnpm check:nul-bytesdispatch-gatesnamed exactly the four workflow-parsing families the card predicted, plusthe shard/required-contexts duplicates from their other workflows; it added nothing my diff
did not touch. No
scripts/**path is touched, so thecheck:entry-guard/check-parse-guard.mjsblind spot does not apply here.No ablation applies. A comment correction has no guard under test — there is no mutation
whose removal could turn anything red, and the counterfactual above is the honest substitute:
it shows the edit would have flipped the classification under the classifier the comment
describes.
Scope
git cat-file -evsgit rev-parsewas explicitly out of review;the command is byte-identical.
filterjob 一旦失败,Test Core / Build Core / Dogfood 会全部 skipped 而分支保护判为通过 —— 隐式 success() 今天已第三次咬人 #4928) governs thefilterjob; this is thetestshard job, andnothing inside that contract's boundary is touched.
scripts/check-shard-attestation.mjsis not edited. The classifier is correct; thecomment describing it was not.
skip-changesetapplied.Generated by Claude Code
Generated by Claude Code