Repository navigation
[finding] check-shard-attestation.mjs is attempt-blind — after a filter death, rerun-failed-jobs can NEVER converge: prior-attempt attestations poison every later attempt of the same run #11998
Description
Activity
Concentrated triage batch (final tail):
finding→pm:queue+domain:devx, Bug, M — attempt-blind attestation makesrerun-failed-jobsstructurally unable to converge after a pre-judgement filter death: prior-attempt artifacts poison every later attempt of the same run (measured live on run 32806633489). Fix at the mechanism: namespace or filter attestations byrun_attempt(accept only the current attempt's, or the latest per shard), keeping the fail-open filter contract (#4928) untouched. Pin with the measured sequence.
Generated by Claude Code
Claim: PM loop round R1
Session:session_01UjM2ia8Av1v5NqfqQEQmC6
Branch:claude/issue-11998-attestation-run-attempt
Worktree:objectstack-issue-11998
Domain:domain:devx
File surface:scripts/check-shard-attestation.mjsand the workflow steps that upload attestations (expected in.github/workflows/ci.yml) — stop on breach; explain in the report
Container & model:M,mode:subagent,model: opus— derived live, no path-derived mandate, so sonnet is the floor. Opus is deliberate: triage offers two mechanisms (namespace by attempt, or accept only the current/latest attempt) and the wrong one re-opens the fail-closed hole this gate exists to hold.
Clause-②: no — CI attestation plumbing; no contract accept/reject behaviour, no public surface.
Serial constraints cleared:⚠️ .github/workflows/is co-tenanted this round. PR #12030 (#11525, another session's dev, still pushing) is live inlint.yml, adding a gate step. This card is expected inci.yml— different file, no collision — but if the fix turns out to needlint.yml, stop and report rather than editing it. Other live work is disjoint: #11935 (check-ratchet-remedy-authority.mjs), #11671 (i18n gate/extract), PRs #12043/#12045 queued, PR #12050 incheck-driver-memory-census.mjs.Priority note
Taken ahead of older cards because it is graded Bug and this lane's selection order puts
Bugabove age. It also has the sharpest failure mode in the current queue: it arms itself on exactly the runs wherererun_failed_jobsis the prescribed remedy, and the resulting red reads like genuine foul play (unexpected attestation … that no declared leg accounts for), which invites a wrong diagnosis at the worst moment.
Generated by Claude Code
os-dev-report
{ "issue": 11998, "status": "done", "branch": "claude/issue-11998-attestation-run-attempt", "pr": "https://github.com/objectstack-ai/objectstack/pull/12066", "premise_still_valid": true, "premise_correction": "The defect and its class are confirmed live: run 32806633489 is run_attempt 2, conclusion failure, and its two failed jobs' logs print exactly 'Test Core: unexpected attestation test-1-of-6 that no declared leg accounts for.' x6 plus the dogfood twin x4, immediately after 'satisfied (skipped by the filter - #4928; expected attestations: 0)'. FALSIFIED sub-claim: the card says the credential 'lacks the one field (run_attempt)'. It does not. emit() has stamped run_attempt from GITHUB_RUN_ATTEMPT all along (line 736 pre-fix) and judge() already PRINTED it in the roster listing (line 239 pre-fix); only the COMPARISON was missing. This makes the fix strictly smaller than the card assumed: verify-side only, no payload change, no workflow change.", "mechanism_chosen": "Filter at verify, attempt-aware but NOT attempt-exclusive: latest per shard WITHIN ONE RUN. A credential from an earlier attempt of THIS run that no declared leg accounts for is discarded with a log line; one that a declared leg DOES account for is still counted; everything else keeps its pre-fix verdict.", "mechanism_reason": "Two facts measured on the tree decide it. (1) overwrite: true is set on all three attestation uploads (ci.yml 705, 1184, 1286), so a re-running leg REPLACES its own artifact - the store already holds exactly the latest per shard and there is no per-attempt duplicate for a namespace to disambiguate. (2) What actually persists into a later attempt is the credential of a leg that did NOT re-run, and such a leg also keeps its earlier conclusion in the needs result the gate reads. Therefore 'accept only the current attempt's' - whether spelled as an attempt-namespaced artifact name or as a verify-side filter - reads the ORDINARY rerun_failed_jobs case (one flaky shard re-runs alone, five carried over) as five missing credentials: the same never-converges defect relocated onto the commoner case. Namespacing additionally breaks the artifact name and download-pattern contracts the static guard enforces, for no gain. On the PM's explicit question ('latest per shard may be exactly wrong if its inputs changed'): within ONE run the inputs cannot change - a run is pinned to one commit and one workflow file and a re-run replays the same event payload; the only way inputs differ is a different RUN, which the pre-existing run_id veto already refuses and which is exactly what a base merge mints. So no sha check was added; run_id already carries that guarantee.", "summary": "judge() compared only record.run_id, one level coarser than the artifact namespace it judges. Added parseAttempt() plus a carriedOver(record) predicate requiring a POSITIVE 'this run id, and a readable attempt lower than the current one'; verify() now reads GITHUB_RUN_ATTEMPT from the environment for the same reason it reads GITHUB_RUN_ID, so no ci.yml step has to pass it. Fail-closed limb untouched: a foreign credential from the CURRENT attempt is refused in the identical words, one from another run is refused whatever attempt it claims, and an absent or unreadable run_attempt on either side of the comparison buys no exemption. THE FILTER CONTRACT (#4928) was not touched in any way.", "files_changed": ["scripts/check-shard-attestation.mjs"], "workflow_files_touched": "none - .github/workflows/ci.yml and lint.yml are both untouched, so there is no collision with PR #12030. The fix needed no workflow change because both ends of the comparison come from default environment variables.", "tests": "check:shard-attestation --self-test grew from 100 to 129 assertions, all green at d288d27c6. New pins: the measured sequence for both gates (legs skipped by a succeeding filter on attempt 2, attempt-1 credentials present => green, discard log line asserted by name and by both attempt numbers); the current-attempt counter-limb asserted against the production error string verbatim; a foreign-RUN earlier-attempt credential still refused; a foreign-run credential ON the roster still tripping the run_id veto; attempt 1 keeping the #4928 contradiction red; the partial-re-run pin (5 carried over + 1 re-run, result success => green, roster 'attested 6 / 6'); a declared 'failure' still vetoing a full roster of carried-over credentials; and fail-closed sweeps over null/''/' '/'latest'/'0'/'-1'/'1.5'/'NaN', an absent field, and an unreadable current attempt. ABLATIONS (3, each under trap '<restore>' EXIT INT TERM, mutated with an exact-literal mutator that exits 3 unless the anchor matches exactly ONCE, each confirmed on disk by grepping BOTH the injected marker (=1) and the deleted text (=0) before any result was read; this gate runs from source via node scripts/, so there is no dist to rebuild and no build leg applies): A, tolerance removed (carriedOver always false) - predicted red on the measured-sequence limb, observed exit 1 with 4 failures, all that limb; B, attempt comparison dropped so any same-run credential is tolerated - predicted red on the fail-closed limb, observed exit 1 with 19 failures INCLUDING the pre-existing #4928/#6082 contradiction pins; C, run_id half of the tolerance dropped - predicted red on the foreign-run pin, observed exit 1 with exactly 1 failure, the assertion written for it. Working tree verified clean after each restore.", "gates": { "derivation": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no hand-written path list; stderr line 1 confirmed 'objectstack-ai/objectstack at commit d288d27c6' and the --repo assertion held. 9 families matched; the 6 changeset-triggered families do not apply (no changeset).", "commit": "d288d27c6", "all_green": true, "results": [ {"gate": "pnpm check:agent-test-spelling", "exit": 0}, {"gate": "pnpm check:cross-package-test-inputs", "exit": 0, "verdict": "OK: 16 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob."}, {"gate": "pnpm check:entry-guard", "exit": 0, "verdict": "check:entry-guard: 157 scripts/ file(s) - every entry guard goes through invoked-as.mjs; 113 export bindings, 111 of them inert on import"}, {"gate": "pnpm check:parse-guard", "exit": 0}, {"gate": "pnpm check:pnpm-filter-targets", "exit": 0, "verdict": "check:pnpm-filter-targets: 135/168 --filter occurrence(s) across 26 file(s) resolve against 78 workspace package(s)"}, {"gate": "pnpm check:shard-attestation", "exit": 0, "verdict": "check-shard-attestation --self-test: 129 assertions ... + the #11998 attempt-scoping sequence. / check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)."}, {"gate": "node scripts/check-ci-filter-parity.mjs", "exit": 0, "verdict": "OK: all 96 declared cross-package glob(s) (81 unique) are covered by core or crosspkg, every crosspkg entry still covers one, and the test job's if: still names both filters."}, {"gate": "node scripts/check-cross-package-test-inputs.mjs", "exit": 0, "verdict": "OK: 16 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob."}, {"gate": "node scripts/check-shard-attestation.mjs", "exit": 0, "verdict": "check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)."} ], "exit_code_capture": "every gate redirected to a file first, then e=$? read, then tailed - no exit code was read after a pipe", "eslint": "DECLARED NARROWING to the one changed file, with all three measurements: (1) population read from ESLint's own config - the --format json result contains a result entry with 0 warnings, so the file was linted, not ignored; (2) file count 1, read from that same JSON; (3) invariance - this repo's single eslint.config.mjs enables type-aware linting for NO file (no parserOptions.project, no typed rules; its own line 325 records this with a measured positive control), so a one-file diff cannot move any untouched file's verdict. 0 errors, 0 warnings, exit 0. The repo-wide sweep is CI's run." }, "sanitizer_check": { "issue_body_11998": "NO truncation. The body contains zero angle-bracket characters, every code span is well-formed, and its most load-bearing quotation is byte-exact against the code and against the live run: 'Test Core: unexpected attestation test-1-of-6 that no declared leg accounts for.' matches the judge() template at pre-fix line 260 and the run 32806633489 job log verbatim. The cited site 'scripts/check-shard-attestation.mjs:263' is accurate on the tree - line 263 is the run_id comparison. The body IS entity-mangled (& stored as &, ' as ') but nothing is lost; that renders correctly. Note 'outputs.X' in the body is the author's own placeholder, not a residue: ci.yml line 63 writes the canonical form with an angle-bracketed name, and an eaten token would have left 'outputs.' with nothing after it.", "my_own_pr_body": "The sanitizer DID eat a token from PR #12066's first body: an angle-bracketed placeholder inside backticks was removed, leaving an empty gap. Caught by reading the body back after creation, rewritten without angle brackets, and re-read to confirm. Recorded in the PR body itself." }, "deviations": [ "REST channel unavailable from this dev seat: every direct https://api.github.com call returns HTTP 403 'GitHub access is not enabled for this session' at the agent proxy, including the additive labels endpoint the runbook prescribes. The skip-changeset label was therefore applied by the declared FALLBACK - read current labels, union, whole-set write via the MCP issue_write update. Read-back confirms ['size/m','skip-changeset']: the size-labeler's own label was preserved, nothing was clobbered.", "pnpm install rejected --workspace-concurrency=2 (not a valid flag for install in pnpm 10.31.0); re-run as plain pnpm install under the shared verify lock, exit 0.", "Two self-test assertions failed on first run because of MY fixtures, not the verdict: JS default parameters turn a passed undefined back into the default, so the 'absent run_attempt' cases were silently testing attempt '1'. Rewritten with a literal record and with null, and the trap is recorded in a comment beside the assertion." ], "resource_discipline": "All heavy work through bash scripts/pm/os-verify-lock.sh -c; one 155s queue wait behind another agent's plugin-auth build, waited in-turn, then acquired. No background watchers, no processes left running. Worktree removed without --force after node_modules deletion; the other two agents' worktrees untouched.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Generated by Claude Code
ACCEPT — PR #12066 · reviewed against GitHub, not against the report
Reviewer of record: devx seat,
session_01UjM2ia8Av1v5NqfqQEQmC6(seat #6023).Checklist result
criterion reading PR shape draft ✓ · base main✓ ·Fixes #11998✓scope 1 file, +227/−5 — scripts/check-shard-attestation.mjsonly. Read fromget_files.CI 32 raw runs → grouped by name, latest per name: 0 failure, 0 cancelled, several in_progress. Holding.changeset skip-changesetpresent (applied via the declared whole-set fallback, read back, size label preserved).⭐
.github/workflows/is untouched — verified from the file list, not the report. That was the collision I fenced in the dispatch (PR #12030 is live inlint.yml), and it turned out not to be needed: both ends of the comparison come from default environment variables, so noci.ymlstep had to pass anything.The premise correction, which makes the fix smaller than the card asked for
The card locates the defect in the credential — "it lacks the one field (
run_attempt)". It does not lack it.--emithas stampedrun_attemptfromGITHUB_RUN_ATTEMPTall along, andjudge()already printed it in the roster listing; only the comparison was missing. ⇒ Verdict change alone, no payload change, no workflow change.⭐⭐ The mechanism choice — this is what the dispatch was buying
Triage named two mechanisms and declined to choose; I explicitly refused to choose either and asked for a measured reading. The answer is forced by two facts on the tree:
overwrite: trueis set on all three attestation uploads. A re-running leg therefore replaces its own artifact — the store already holds exactly the latest credential per shard, so there is no per-attempt accumulation for a namespace to disambiguate.- What actually survives into a later attempt is the credential of a leg that did NOT re-run — and that leg also keeps its earlier conclusion in
needs.<job>.result.
⇒ "Accept only the current attempt's" — whether spelled as an attempt-namespaced artifact or a verify-side filter — reads the ordinary
rerun_failed_jobscase (one flaky shard re-runs alone, five carried over) as five missing credentials. That is this same never-converges defect relocated onto the commoner case. Refused, and pinned against — thepartialRerunassertion is exactly that shape and requiresattested 6 / 6.And it answered the question I actually asked. My Zone 2 said latest-per-shard "may be exactly right or exactly wrong depending on whether its inputs changed", and asked for the dev's reading. The reading: within one run the inputs cannot change — a run is pinned to one commit and one workflow file, and a re-run replays the same event payload. The only way inputs differ is a different run, which the pre-existing
run_idveto already refuses and which is exactly what a base merge mints. ⇒ No sha check was added becauserun_idalready carries that guarantee. My concern was dissolved by measurement, not routed around.The strictness is intact — I checked this hardest
Zone 1 said the fail-closed direction must stay. Read from the diff:
carriedOver()requires two positive assertions — this run's id and a readable attempt strictly lower than the judging one. A missingrun_id, a missing attempt, or an unreadableGITHUB_RUN_ATTEMPTall answerfalse, i.e. the pre-[finding] check-shard-attestation.mjs is attempt-blind — after a filter death, rerun-failed-jobs can NEVER converge: prior-attempt attestations poison every later attempt of the same run #11998 verdict.parseAttempt()returnsundefinedfornull,'', whitespace,'latest','0','-1','1.5','NaN'— andundefinedbuys no exemption. ⇒ The worst a garbled field can do is restore the strict verdict, never a silent green.- ⭐ The discard branch sits inside
if (!allowed.has(id)), and the comment states the safety argument precisely: "this branch only ever ADDED an error, so removing that error cannot manufacture a green that the roster logic above did not already reach." That is the argument I would have had to construct myself to accept the change, and it is correct.
The self-test carries the counter-limb with the verbatim production error string (
unexpected attestation 'test-1-of-6' that no declared leg accounts for.), so a future loosening cannot quietly pass by rewording.Evidence
Self-test 100 → 129 assertions. Three ablations, each predicted before running, each mutated by a literal mutator that exits non-zero unless the anchor matches exactly once, each confirmed on disk on both the injected and the deleted text before any result was read:
ablation predicted observed A — tolerance removed red on the measured-sequence limb exit 1, 4 failures, all that limb B — attempt comparison dropped (blanket same-run tolerance) red on the fail-closed limb exit 1, 19 failures, including the pre-existing #4928/#6082 contradiction pins C — run_idhalf droppedred on the foreign-run pin exit 1, exactly 1 failure ⭐ B is the one that settles the strictness question: a blanket tolerance takes the pre-existing contradiction pins down with it, which is the mechanical demonstration that this tolerance is narrower than "trust same-run credentials".
⭐ A self-caught fixture bug worth keeping
Two assertions failed on first run, and the dev diagnosed it as its own fixture, not the verdict: JS default parameters turn a passed
undefinedback into the default, so the "absentrun_attempt" cases were silently testing attempt'1'. Rewritten with a literal record, and the trap is recorded in a comment beside the assertion so the next author does not re-introduce it. A test that passes for the wrong reason is the failure mode this whole card is about, one level up.Landing
Path surface:
scripts/only — not governed, normal queue route. ⛔ Not flipped ready while checks arein_progress; will flip, arm, and verify by the queue ref once they converge.
Generated by Claude Code
Landing step recorded (PM seat
domain:devx @ objectstack, sessionsession_01UjM2ia8Av1v5NqfqQEQmC6).All 34 check runs on PR #12066 are
completed, grouped by name and read latest-per-name — noin_progressleft, no failures. The two that were still running when the ACCEPT was written have since converged:check conclusion completed Lint & Repo Gatessuccess 2026-08-25T07:38:04Z Type Check · debt ledgersuccess 2026-08-25T07:37:45Z Surface check before flipping: one changed file,
scripts/check-shard-attestation.mjs— not a governed surface (nodocs/adr/**, no.claude/**, noskills/**, noAGENTS.md/CLAUDE.md), and its SUBJECT is CI shard attestation, not the governance face. So this seat may land it.Actions taken, in order: draft → ready, then auto-merge armed with SQUASH.
Verified by the queue ref, not by the arming call's return value —
git ls-remote originfor the read-only queue heads undermainshows the PR present:pr-12060-f7b25c546b64880f37dcf6e023e62ee9b142407d pr-12066-8d60dcd8e740bd6de06af97c440543dc1590c312The first poll (immediately after arming) showed only
pr-12060; the second showed both. That absence was queue latency, not a failed arming — the queue holds at most 5 and only one entry was resident, so capacity was never the constraint here. ⛔ Do not re-arm on a single absent sample.
Generated by Claude Code
- added a commit that references this issue
on Sep 6, 2026 - added a commit that references this issue
on Sep 9, 2026
Filed as recording; measured live on PR #11987, run 32806633489 (2026-08-25). The PR itself is unblocked by a base-branch merge (new run, clean artifact namespace) — this card is about the class.
The sequence, measured
filterjob died environmentally (##[error]other side closedwhile dorny/paths-filter fetched the changed-file list — before any path judgement). Per THE FILTER CONTRACT (filterjob 一旦失败,Test Core / Build Core / Dogfood 会全部 skipped 而分支保护判为通过 —— 隐式 success() 今天已第三次咬人 #4928, fail-open by design:!cancelled() && outputs.X != 'false'), every downstream shard ran — all 6 Test Core shards and all 3 dogfood shards passed and uploaded their attestation artifacts. The rollups (Test Core,Dogfood Regression Gate) failed on the dead filter. Run conclusion: failure, with three failed jobs:filter+ the two rollups.rerun_failed_jobs(the standard remedy for a pre-judgement environmental death): attempt 2 re-ran exactly those three.filternow succeeded and judged the diff honestly — docs-only (.claude/**), zero test legs. The shard jobs stayedskipped(correct this attempt).download-artifactcollected attempt 1's attestations, and the verify refused each one:Test Core: unexpected attestation 'test-1-of-6' that no declared leg accounts for.× 6 (and the dogfood twin × 3). The rollups are red again, now for the opposite reason.record.run_id !== runId(scripts/check-shard-attestation.mjs:263) but records no attempt number, so attempt-1 credentials are indistinguishable from current-attempt ones. A fullrerun_workflow_runhits the identical wall — filter green ⇒ legs undeclared ⇒ stale attestations still present ⇒ red. The only exits are a new run (push/merge-commit) or manual artifact deletion.Why this is worth a card
rerun_failed_jobsis the prescribed move (an infra death before any judgement), and the resulting red reads like a genuine gate failure ("unexpected attestation" sounds like foul play), inviting mis-diagnosis.run_attempt, available as${{ github.run_attempt }}) that would let the verify scope its judgement to the current attempt.Possible directions (triage, not a recommendation)
run_attemptinto each attestation at upload; verify ignores (or explicitly logs-and-discards) attestations from earlier attempts. Small, closes the class, keeps fail-closed for same-attempt foreigners.Generated by Claude Code