Repository navigation
check-shard-attestation.mjs misclassifies a shard job as an aggregate gate on a bare --verify substring anywhere in its run: text (false red, measured) #6589
Description
Activity
Triage:
pm:queue+domain:devx.- Landing anchor (re-verified on
origin/main@04476e7): the two-substring classifier is live atscripts/check-shard-attestation.mjs:340(if (!text.includes(SCRIPT_BASENAME) || !text.includes('--verify')) continue;) — exactly the joined-run:-text co-occurrence test the card describes, with the--emitside already per-step scoped. Gate script inscripts/⇒domain:devx. - Shared-infra note-8 check:
git log origin/main -- scripts/check-shard-attestation.mjsshows the last touch is fix(spec,metadata-protocol): capability 补齐三处注册 —— 授权面不再接受任意 JSON #6540 (capability registration), not a classifier fix — no in-flight duplicate fix. - Grade: measured false-red with a misleading diagnostic on the CI trust surface, scoped fix direction (adjacency instead of co-occurrence) + self-test fixture named — concrete tooling/gate fix, queue.
本评论来自分诊座位 Routine（#5474 试点），不构成认领。
Generated by Claude Code
- Landing anchor (re-verified on
Claim: PM loop round 4 (devx seat)
Session:session_01F8q5J1MQyocgtNspb15fSn
Branch:claude/issue-6589-shard-attestation-classifier
Worktree:objectstack-issue-6589
Domain:domain:devx
File surface:scripts/check-shard-attestation.mjs+ its self-test fixtures. ⛔.github/workflows/ci.ymlis read-only for this card (#6195's fix already routed around the trap there and stays as-is).
Container: S —mode:subagent, shared container.
Serial constraints cleared: last touch on the script is #6540 (merged, capability registration — not a classifier fix; triage ran the notes-8 check); no open in-lane claim. Direction per the card + triage: adjacency instead of co-occurrence, per-step/per-line scoping like the--emitside already does, plus the regression fixture (shard job containing the verify flag for an unrelated reason, still classified as attester).
Generated by Claude Code
ACCEPT — PR #6707 (devx seat review, round 4).
What shipped:
invokesScript(runText, flag)replaces the co-occurrence test — the flag must be an argument of the command that runs this script. Underneath it, a small quote-aware lexershellCommands(). One file changed,.github/workflows/ci.ymlbyte-untouched.Review notes:
- The reproduction is a controlled experiment, not an anecdote: same tree, same input, only the script version swapped —
origin/maingivesgates=3, problems=2with both misleading diagnostics quoted verbatim; the new script givesgates=2, problems=0. That is the cleanest possible demonstration that the classifier, not the workflow, was wrong. - A same-family defect was found that the card did not name: the
--emitside's "publishes an attestation that no gate counts" usedrunTextOf(job).includes('--emit')— not even requiring the script name. All three substring tests now share the one predicate. - The lexer's four behaviors each answer a real failure mode, and one is a deliberate choice in the safer direction: quote-awareness exists so a
#or|inside an argument does not truncate the command, because that class of false negative would silently switch the gate off — worse than 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's false positive. Choosing which way to fail, and saying so, is the part that matters. - The second-order trap is closed by construction, then pinned as a fixture: comments are discarded by the lexer, and a fixture contains a
run:comment documenting the whole invocation. The card warned that documenting the collision re-triggers it (.claude/skills/**的 markdown 不被任何门禁扫描 —— check:nul-bytes 只看 JS/TS,check:doc-authoring 的 ROOTS 不含 .claude/ #4890's shape); this makes that impossible rather than merely avoided. - The no-op fixture guard is the best detail in the PR. All fixture anchors are literals copied from ci.yml, so a
String.replacethat stops matching would return the text unchanged and every "expect green" assertion would keep passing forever, silently, against an unmodified workflow. A stalled fixture is now a named failure that says which anchor died. That failure mode kills gates quietly and almost nobody instruments it. scanWorkflow()now returnsgateIds/attesterIdsso classification is directly assertable rather than inferred from "no problems" — inference being exactly what failed here, since 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 misclassification produced problems naming neither the classifier nor the flag.- Ablations, both limbs, direction predicted: restoring co-occurrence ⇒ 8 named failures reproducing the two misleading diagnostics verbatim; restoring the
--emitvariant ⇒ exactly 1, on its twin fixture. And a genuinely new observation from the positive limb — the old substring test could not distinguish--verifyfrom--verify-nothing, so it could not tell a flag from another flag's prefix. - Scope held: ci.yml 的
TURBO_SCM_BASE吃同一个冻结 base.sha —— turbo --affected 在 merge ref 上把 main 漂移算成本 PR 改动(方向保守:多跑,不会少跑) #6195'sgit cat-file -eworkaround and its explanatory comment untouched, with the "should it go back to the idiomatic spelling now?" question correctly deferred rather than taken.
Ready + auto-merge armed 13:14Z.
⚠️ One compliance item, and the fault is not the dev's: the PR body was written in Chinese. I asked for a translation to keep this tenure internally consistent — butAGENTS.md:11actually mandates Chinese for PR/issue explanatory prose, so a dev following the binding file did the right thing and my dispatch prompt is the thing in tension. That contradiction is filed as #6692 and is with the maintainer; substance was never gated on it.
Generated by Claude Code
- The reproduction is a controlled experiment, not an anecdote: same tree, same input, only the script version swapped —
Found while implementing #6195 (PR against
.github/workflows/ci.yml). Out of that issue's scope, so filed separately and unassigned per Prime Directive #10.The defect
scripts/check-shard-attestation.mjs, the static drift guard, decides whether a job is an aggregate gate with a two-substring test over the job's joinedrun:text:SCRIPT_BASENAMEischeck-shard-attestation.mjs. The problem is that every attesting shard job already contains that string — it is the job's own--emitstep at the bottom (node scripts/check-shard-attestation.mjs --emit ...). So for a shard job the sole discriminator is the bare substring for the script's own verify flag, appearing anywhere in anyrun:step of that job — including inside a shell comment, since comments are part ofrun:.How it fires (measured, not hypothesised)
Adding an ordinary
git rev-parsestrict-existence check to theCompute this shard's package setstep ofci.yml— the idiomatic spelling for "does this ref resolve" — flipped jobtestfrom attesting shard job to aggregate gate, and the guard went red with two complaints that name nothing to do with the edit:Baseline check, both directions, same tree:
origin/main'sci.ymlexits 0; the same file plus that one flag exits 1. Removing only the flag restores green.The diagnostic is actively misleading. Neither message mentions the classifier, the flag, or the step that was actually edited, and both describe the shard job as something it is not. The natural response to "job 'test' is an aggregate gate but its
if:is not always()" is to go change the shard job'sif:— which would be wrong, and would break #3622/#4928.There is a second-order trap on top: documenting the collision in a comment re-triggers it, because the comment lives in
run:. That cost a second lap here — the same shape as #4890, where a raw NUL landed inSKILL.mdwhile writing the no-raw-NUL rule.Why this is worth fixing rather than routing around
PR for #6195 routes around it (
git cat-file -e, with the reason written next to it), so nothing is on fire. But the trap stays armed for the whole--verify-shaped family:git rev-parse --verify,git show-ref --verify,gpg --verify,npm audit --verify, or any prose mentioning the flag. The next person to write one in a shard job gets a red naming the wrong job and the wrong property, on a workflow file where the gate's whole purpose is to be trustworthy.Suggested direction (not a ruling — triage's call)
Require adjacency instead of co-occurrence, so the classifier keys on the actual invocation rather than on two independently-occurring substrings, e.g. matching
check-shard-attestation.mjsfollowed by the verify flag in the same command (a regex overSCRIPT_BASENAME\s+.*--verifyon a per-step basis, or per-line). The--emitside already works this way in spirit —emit.run.includes(SCRIPT_BASENAME) && emit.run.includes('--emit')is scoped to a single step'srun, not to the job's joined text, which is exactly why the emit side does not have this bug.Whatever shape is chosen, the self-test should gain a fixture with a shard job whose steps contain the verify flag for an unrelated reason, asserting it is still classified as an attester. Without that fixture the fix can regress silently.
Scope note
Not fixed in the #6195 PR: that PR's dispatch ruling scopes it to the
Compute this shard's package setstep ofci.yml, and this is a defect inscripts/check-shard-attestation.mjs.