Skip to content

Commit 402113d

Browse files
yinlianghuiclaude
andauthored
fix(ci): scope the shard-attestation verdict to the current run attempt (#12066)
Artifacts live on the RUN, not the attempt, so every attempt after the first downloads whatever the earlier ones published. `judge()` compared only the run id -- one level coarser than the artifact namespace it judges -- so after a pre-judgement `filter` death (fail-open per THE FILTER CONTRACT #4928, which is untouched here) `rerun_failed_jobs` could never converge: the legs correctly stayed skipped on attempt 2, the roster adjusted to expected-0, and attempt 1's own credentials were then refused one by one as "unexpected attestation ... that no declared leg accounts for". Measured on run 32806633489. The credential already carried `run_attempt` and the verdict already PRINTED it -- it just never COMPARED it. So this is a verdict change only: no payload change, and nothing in ci.yml moves. The rule is "latest per shard WITHIN ONE RUN", not "only this attempt's": - a credential from an earlier attempt of this run that no declared leg accounts for is discarded with a log line, granting no pass (the green comes from #4928's expected-0, which it never enters); - a credential from an earlier attempt that a declared leg DOES account for still counts. `overwrite: true` on every upload means a re-running leg replaces its own artifact, so what survives is the credential of a leg that did NOT re-run -- and that leg keeps its earlier conclusion in `needs.<job>.result`. Refusing it would turn the ordinary rerun_failed_jobs case (one flaky shard) permanently red: the same defect, one button over; - everything else keeps its old verdict. The tolerance demands a positive "this run, an earlier attempt": a foreign credential from the CURRENT attempt is still refused in the same words, so is one from another run, and an absent or unreadable attempt buys no exemption. --self-test grows the measured sequence (both gates), the current-attempt counter-limb, the partial-re-run pin that blocks the "current attempt only" mis-fix, and the fail-closed cases on both sides of the comparison. Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8d60dcd commit 402113d

1 file changed

Lines changed: 227 additions & 5 deletions

File tree

scripts/check-shard-attestation.mjs

Lines changed: 227 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,63 @@
9494
* what makes this robust to the next undocumented lifecycle value instead of
9595
* needing another word added to a list.
9696
*
97+
* ## Attempt scoping: an earlier attempt's credential is not this one's (#11998)
98+
*
99+
* Artifacts live on the RUN, not on the attempt, so every attempt after the
100+
* first downloads whatever the earlier ones published. Measured on run
101+
* 32806633489 (2026-08-25, PR #11987): attempt 1's `filter` job died
102+
* environmentally BEFORE any path judgement, THE FILTER CONTRACT (#4928,
103+
* fail-open by design) let all 9 shards run, they passed and attested, and the
104+
* rollups failed on the dead filter. `rerun_failed_jobs` — the PRESCRIBED
105+
* remedy for a pre-judgement death — re-ran filter + the two rollups; filter
106+
* then judged the diff honestly (docs-only, zero test legs) and the shards
107+
* stayed `skipped`, correctly. The rollups read expected-0 and refused attempt
108+
* 1's credentials one at a time:
109+
*
110+
* Test Core: unexpected attestation 'test-1-of-6' that no declared leg accounts for. (×6)
111+
* Dogfood Regression Gate: unexpected attestation 'dogfood-1-of-3' … (×4)
112+
*
113+
* Red again, now for the OPPOSITE reason — and no attempt of that run could
114+
* ever converge, because a full `rerun_workflow_run` hits the identical wall.
115+
* The only exits were minting a new run or deleting artifacts by hand.
116+
*
117+
* The credential already CARRIED the attempt (`--emit` has stamped
118+
* `run_attempt` from `GITHUB_RUN_ATTEMPT` all along) and `judge()` already
119+
* PRINTED it. It simply never COMPARED it, so the run_id equality below was the
120+
* entire scope check — one level coarser than the artifact namespace it judges.
121+
* The repair is therefore a verdict change: no payload change, and nothing in
122+
* ci.yml moves.
123+
*
124+
* ⭐ The rule is "latest per shard WITHIN ONE RUN", not "only this attempt's",
125+
* and that difference is the whole design:
126+
*
127+
* - a credential from an earlier attempt of THIS run that no declared leg
128+
* accounts for is DISCARDED, with a log line. It is explained by the
129+
* attempt mechanism rather than by foul play, and discarding it grants no
130+
* pass: the green in the sequence above comes from #4928's expected-0,
131+
* which that credential never entered.
132+
* - a credential from an earlier attempt that a declared leg DOES account for
133+
* still COUNTS. `overwrite: true` on every upload means a leg that re-runs
134+
* REPLACES its own artifact, so the store already holds exactly the latest
135+
* per shard; what survives into a later attempt is the credential of a leg
136+
* that did NOT re-run — and such a leg also keeps its earlier conclusion in
137+
* `needs.<job>.result`. Refusing it would turn the ORDINARY
138+
* `rerun_failed_jobs` case (one flaky shard re-runs alone, the other five
139+
* are attempt-1) permanently red: this same never-converges defect, moved
140+
* one button over onto the commoner case. Pinned in --self-test.
141+
* - everything else keeps its pre-#11998 judgement. The tolerance requires
142+
* the credential to say POSITIVELY that it is from this run and an earlier
143+
* attempt: a foreign credential from the CURRENT attempt is still refused,
144+
* so is one from another run whatever attempt it claims, and an absent or
145+
* unreadable `run_attempt` buys no exemption at all.
146+
*
147+
* The "…but its inputs may have changed" worry that makes latest-per-shard look
148+
* risky in general cannot arise within ONE run: a run is pinned to one commit
149+
* and one workflow file, and a re-run replays the same event payload. The only
150+
* way the inputs differ is a different RUN — which the run_id veto already
151+
* refuses, and which is exactly what a base merge mints (that is how PR #11987
152+
* got unblocked, leaving the class behind for this card).
153+
*
97154
* ## How the static guard decides what a job IS (#6589)
98155
*
99156
* `scanWorkflow` has to tell an aggregate GATE (`… --verify …`) from an
@@ -155,6 +212,25 @@ function rosterFor(job, total) {
155212
return Array.from({ length: total }, (_, i) => attestationId(job, i + 1, total));
156213
}
157214

215+
/**
216+
* The attempt a credential (or this process) belongs to, as a positive integer
217+
* — or `undefined` when it does not say (#11998).
218+
*
219+
* `undefined` is the FAIL-CLOSED answer, and every tolerance below demands a
220+
* readable attempt on BOTH sides before it applies. A credential predating the
221+
* field, a blank `GITHUB_RUN_ATTEMPT`, `'latest'`, `0`, `1.5` — none of them
222+
* buys an exemption with a value nobody could read. That direction matters:
223+
* an unreadable attempt keeps the strict pre-#11998 verdict, so the worst case
224+
* of a garbled field is the red this card is about, never a silent green.
225+
*/
226+
function parseAttempt(value) {
227+
if (value === undefined || value === null) return undefined;
228+
const text = String(value).trim();
229+
if (text === '') return undefined;
230+
const attempt = Number(text);
231+
return Number.isInteger(attempt) && attempt > 0 ? attempt : undefined;
232+
}
233+
158234
// ── The verdict, as a pure function ─────────────────────────────────────────
159235

160236
/**
@@ -169,18 +245,37 @@ function rosterFor(job, total) {
169245
* filterResult: string,
170246
* present: Map<string, Record<string, unknown>>,
171247
* runId: string,
248+
* runAttempt?: string,
172249
* downloadOutcome?: string,
173250
* }} input
174251
* @returns {{ ok: boolean, log: string[], errors: string[] }}
175252
*/
176-
export function judge({ gate, legs, filterResult, present, runId, downloadOutcome }) {
253+
export function judge({ gate, legs, filterResult, present, runId, runAttempt, downloadOutcome }) {
177254
const log = [];
178255
const errors = [];
179256
const allowed = new Set();
180257
let counted = 0;
258+
const thisAttempt = parseAttempt(runAttempt);
259+
260+
/**
261+
* Was this credential published by an EARLIER attempt of THIS run (#11998)?
262+
*
263+
* Both halves are positive assertions, deliberately: the credential must name
264+
* this run's id AND a readable attempt lower than the one now judging. A
265+
* missing run_id, a missing attempt, or an unreadable `GITHUB_RUN_ATTEMPT`
266+
* here all answer `false`, which is the pre-#11998 judgement.
267+
*/
268+
const carriedOver = (record) => {
269+
if (!runId || record?.run_id === undefined || String(record.run_id) !== String(runId)) return false;
270+
const attempt = parseAttempt(record?.run_attempt);
271+
return attempt !== undefined && thisAttempt !== undefined && attempt < thisAttempt;
272+
};
181273

182274
log.push(`${gate}: counting per-shard positive attestations (#6082), not absence of negatives.`);
183275
log.push(` filter job result: ${filterResult || '(unreadable)'}`);
276+
// Printed so that an unreadable attempt — which silently restores the strict
277+
// pre-#11998 scope — is diagnosable instead of invisible.
278+
log.push(` run ${runId || '(unreadable)'}, attempt ${thisAttempt ?? '(unreadable)'}`);
184279
if (downloadOutcome && downloadOutcome !== 'success') {
185280
log.push(` NOTE: the attestation download step itself did not succeed (outcome: ${downloadOutcome}).`);
186281
}
@@ -236,7 +331,8 @@ export function judge({ gate, legs, filterResult, present, runId, downloadOutcom
236331
const missing = roster.filter((id) => !present.has(id));
237332
for (const id of roster) {
238333
const record = present.get(id);
239-
log.push(record ? ` + ${id} (run ${record.run_id}, attempt ${record.run_attempt})` : ` - ${id} MISSING`);
334+
const carried = record && carriedOver(record) ? ' ← carried over from an earlier attempt of this run (#11998)' : '';
335+
log.push(record ? ` + ${id} (run ${record.run_id}, attempt ${record.run_attempt})${carried}` : ` - ${id} MISSING`);
240336
}
241337
log.push(` attested ${roster.length - missing.length} / ${roster.length} declared shard(s)`);
242338

@@ -257,6 +353,19 @@ export function judge({ gate, legs, filterResult, present, runId, downloadOutcom
257353
// Foreign or stale credentials: the run must not be judged on someone else's.
258354
for (const [id, record] of present) {
259355
if (!allowed.has(id)) {
356+
// #11998: an earlier attempt of THIS run is not "someone else" — it is
357+
// this same commit, one button press ago. Its credential says nothing
358+
// about the attempt now being judged, so it is discarded rather than
359+
// treated as foul play. Discarding grants no pass: this branch only ever
360+
// ADDED an error, so removing that error cannot manufacture a green that
361+
// the roster logic above did not already reach.
362+
if (carriedOver(record)) {
363+
log.push(
364+
` discarded '${id}': published by attempt ${record.run_attempt} of this run, and this is attempt ${thisAttempt} — ` +
365+
`an earlier attempt's credential is not evidence about this one (#11998).`,
366+
);
367+
continue;
368+
}
260369
errors.push(`${gate}: unexpected attestation '${id}' that no declared leg accounts for.`);
261370
continue;
262371
}
@@ -763,6 +872,11 @@ function verify() {
763872
filterResult: argValue('--filter-result', ''),
764873
present,
765874
runId: process.env.GITHUB_RUN_ID ?? '',
875+
// Read from the environment for the same reason the run id is (#11998):
876+
// `GITHUB_RUN_ATTEMPT` is a default variable in every job, `--emit` already
877+
// stamps the credential from it, and taking both ends of the comparison
878+
// from the same source means no ci.yml step has to remember to pass it.
879+
runAttempt: process.env.GITHUB_RUN_ATTEMPT ?? '',
766880
downloadOutcome: argValue('--download-outcome', ''),
767881
});
768882
for (const line of verdict.log) console.log(line);
@@ -798,8 +912,8 @@ async function selfTest() {
798912
if (!condition) failures.push(description);
799913
};
800914

801-
const attest = (job, shard, total, runId = '99') =>
802-
[attestationId(job, shard, total), { attestation: attestationId(job, shard, total), job, shard, total, run_id: runId, run_attempt: '1' }];
915+
const attest = (job, shard, total, runId = '99', runAttempt = '1') =>
916+
[attestationId(job, shard, total), { attestation: attestationId(job, shard, total), job, shard, total, run_id: runId, run_attempt: runAttempt }];
803917

804918
const testGate = (results, ids, filterResult = 'success') =>
805919
judge({
@@ -893,6 +1007,114 @@ async function selfTest() {
8931007
forged.get('test-3-of-3').attestation = 'test-1-of-3';
8941008
assert(!judge({ gate: 'Test Core', legs: [{ job: 'test', total: 3, result: 'success' }], filterResult: 'success', present: forged, runId: '99' }).ok, 'a credential that does not describe itself ⇒ red');
8951009

1010+
// ── #11998: attempt scoping, in both directions ───────────────────────────
1011+
//
1012+
// THE MEASURED SEQUENCE (run 32806633489, attempt 2, 2026-08-25 — the run
1013+
// this card was filed from), replayed as a fixture because the live instance
1014+
// was unblocked by a base merge and cannot be re-entered. Attempt 1: the
1015+
// filter died before any path judgement, THE FILTER CONTRACT (#4928) let all
1016+
// 9 shards run, they attested. Attempt 2 via `rerun_failed_jobs`: filter
1017+
// succeeded, judged docs-only, the legs stayed `skipped` — and the gates
1018+
// refused attempt 1's own credentials one by one. The two error clusters
1019+
// below are quoted from that run's job logs verbatim.
1020+
const rerun = (legs, ids, runAttempt = '2', filterResult = 'success') =>
1021+
judge({ gate: 'Test Core', legs, filterResult, present: new Map(ids), runId: '99', runAttempt });
1022+
const testLeg = (result) => [{ job: 'test', total: 6, result }];
1023+
const attempt1Roster = (runAttempt = '1') => [1, 2, 3, 4, 5, 6].map((n) => attest('test', n, 6, '99', runAttempt));
1024+
1025+
const measured = rerun(testLeg('skipped'), attempt1Roster());
1026+
assert(measured.ok, '#11998 measured sequence: attempt-1 credentials + legs the filter skipped on attempt 2 ⇒ green, so the run can converge');
1027+
assert(
1028+
measured.log.some((l) => l.includes("discarded 'test-1-of-6'") && l.includes('attempt 2') && l.includes('#11998')),
1029+
'#11998: a carried-over credential is DISCARDED WITH A LOG LINE naming it and both attempts — never silently dropped',
1030+
);
1031+
// The dogfood twin of the same measured sequence: a matrix leg and a single
1032+
// leg under one context, both skipped, all four credentials carried over.
1033+
const measuredDogfood = judge({
1034+
gate: 'Dogfood Regression Gate',
1035+
legs: [
1036+
{ job: 'dogfood', total: 3, result: 'skipped' },
1037+
{ job: 'dogfood-verify', total: 1, result: 'skipped' },
1038+
],
1039+
filterResult: 'success',
1040+
present: new Map([attest('dogfood', 1, 3), attest('dogfood', 2, 3), attest('dogfood', 3, 3), attest('dogfood-verify', 1, 1)]),
1041+
runId: '99',
1042+
runAttempt: '2',
1043+
});
1044+
assert(measuredDogfood.ok, '#11998 measured sequence, dogfood twin: 3 shard + 1 single-leg credentials carried over ⇒ green');
1045+
1046+
// ⭐ THE OTHER DIRECTION, which is what stops this from being "the gate off":
1047+
// the SAME shape with the credentials stamped with the CURRENT attempt is
1048+
// still refused, with the same words the production log showed.
1049+
const sameAttemptForeign = rerun(testLeg('skipped'), attempt1Roster('2'));
1050+
assert(!sameAttemptForeign.ok, '#11998: a credential from the CURRENT attempt that no declared leg accounts for is still refused — the tolerance is attempt-scoped, not blanket');
1051+
assert(
1052+
sameAttemptForeign.errors.some((e) => e === "Test Core: unexpected attestation 'test-1-of-6' that no declared leg accounts for."),
1053+
'#11998: …and it is refused in the exact words run 32806633489 printed, so the fail-closed limb is unchanged',
1054+
);
1055+
// Nor may the exemption launder another RUN's credential just because it
1056+
// claims an earlier attempt: both halves of `carriedOver` are required.
1057+
const foreignRunEarlierAttempt = rerun(testLeg('skipped'), [attest('test', 1, 6, '1234', '1')]);
1058+
assert(!foreignRunEarlierAttempt.ok, '#11998: an earlier-attempt credential from ANOTHER run is still refused — the run_id half of the test is load-bearing');
1059+
const foreignRunInRoster = rerun(testLeg('success'), [...attempt1Roster().slice(0, 5), attest('test', 6, 6, '1234', '1')]);
1060+
assert(
1061+
foreignRunInRoster.errors.some((e) => e.includes('belongs to run 1234')),
1062+
'#11998: …and one that IS on the roster still trips the run_id veto (#6082), attempt notwithstanding',
1063+
);
1064+
// The tolerance must not fire on attempt 1, where there is no earlier attempt
1065+
// for a credential to have come from: the #4928 contradiction pin stands.
1066+
assert(!rerun(testLeg('skipped'), attempt1Roster(), '1').ok, '#11998: on attempt 1 a credential from a leg reported skipped is still a contradiction ⇒ red');
1067+
1068+
// ⭐ THE MIS-FIX THIS PIN EXISTS TO BLOCK: "accept only the CURRENT attempt's"
1069+
// — whether spelled as an attempt-namespaced artifact or as a verify-side
1070+
// filter — reads the ordinary `rerun_failed_jobs` case as five missing
1071+
// credentials. `overwrite: true` on every upload means a re-running leg
1072+
// REPLACES its own artifact, so what persists is exactly the credential of a
1073+
// leg that did NOT re-run, and that leg keeps its earlier conclusion in
1074+
// `needs.test.result`. Refusing it moves this same never-converges defect
1075+
// onto the commoner case instead of removing it.
1076+
const partialRerun = rerun(testLeg('success'), [...attempt1Roster().slice(0, 2), attest('test', 3, 6, '99', '2'), ...attempt1Roster().slice(3)]);
1077+
assert(partialRerun.ok, '#11998: a partial re-run (shard 3 re-ran on attempt 2, five legs carried over) ⇒ green — latest-per-shard within one run');
1078+
assert(
1079+
partialRerun.log.some((l) => l.includes('attested 6 / 6 declared shard(s)')),
1080+
'#11998: …and the carried-over credentials are COUNTED, not merely tolerated — the roster is satisfied 6/6',
1081+
);
1082+
assert(
1083+
partialRerun.log.some((l) => l.includes('test-1-of-6') && l.includes('carried over')),
1084+
'#11998: …with each carried-over credential marked in the roster listing',
1085+
);
1086+
// Counting them can still never override a declared negative (#6082).
1087+
assert(!rerun(testLeg('failure'), attempt1Roster()).ok, '#11998: a full roster of carried-over credentials never overrides a declared `failure`');
1088+
1089+
// FAIL-CLOSED on an unreadable attempt, on BOTH sides of the comparison. An
1090+
// exemption is never bought with a value nobody could read — so the worst a
1091+
// garbled attempt can do is restore the strict pre-#11998 verdict.
1092+
for (const bad of [null, '', ' ', 'latest', '0', '-1', '1.5', 'NaN']) {
1093+
assert(!rerun(testLeg('skipped'), [attest('test', 1, 6, '99', bad)]).ok, `#11998 fail-closed: a credential whose run_attempt reads ${JSON.stringify(bad)} gets no exemption`);
1094+
}
1095+
// The field ABSENT entirely — a credential minted before it was stamped, or
1096+
// by a fake. Spelled as a literal because `attest()`'s default parameter
1097+
// would silently turn `undefined` back into '1' (which is how this very
1098+
// assertion first failed: the fixture, not the verdict, was wrong).
1099+
assert(
1100+
!rerun(testLeg('skipped'), [['test-1-of-6', { attestation: 'test-1-of-6', job: 'test', shard: 1, total: 6, run_id: '99' }]]).ok,
1101+
'#11998 fail-closed: a credential with no run_attempt field at all gets no exemption',
1102+
);
1103+
for (const bad of [null, '', 'two']) {
1104+
assert(!rerun(testLeg('skipped'), attempt1Roster(), bad).ok, `#11998 fail-closed: an unreadable GITHUB_RUN_ATTEMPT (${JSON.stringify(bad)}) restores the strict verdict rather than disabling the gate`);
1105+
}
1106+
// …and a caller that passes no `runAttempt` at all — every pre-#11998 call
1107+
// site, this file's own fixtures above included — keeps the old verdict.
1108+
assert(
1109+
!judge({ gate: 'Test Core', legs: testLeg('skipped'), filterResult: 'success', present: new Map(attempt1Roster()), runId: '99' }).ok,
1110+
'#11998 fail-closed: judge() called without a runAttempt is judged exactly as before',
1111+
);
1112+
assert(
1113+
rerun(testLeg('skipped'), attempt1Roster(), '2').log.some((l) => l.includes('run 99, attempt 2')) &&
1114+
rerun(testLeg('skipped'), attempt1Roster(), '').log.some((l) => l.includes('attempt (unreadable)')),
1115+
'#11998: the attempt the gate is judging AS is printed, so an unreadable one is diagnosable instead of invisible',
1116+
);
1117+
8961118
// ── missing input is a failure, never a pass (#4690) ──────────────────────
8971119
assert(!judge({ gate: 'Test Core', legs: [], filterResult: 'success', present: new Map(), runId: '99' }).ok, 'a gate with no legs verifies nothing ⇒ red');
8981120
assert(!testGate('', []).ok, 'an aggregate result that did not interpolate ⇒ red');
@@ -1151,7 +1373,7 @@ async function selfTest() {
11511373
process.exit(1);
11521374
}
11531375
console.log(
1154-
`✓ check-shard-attestation --self-test: ${checked} assertions (dominance experiment + both #6082 counter-examples + the #4928 guard + the #6589 classifier pins + the #10889 quoting pins).`,
1376+
`✓ check-shard-attestation --self-test: ${checked} assertions (dominance experiment + both #6082 counter-examples + the #4928 guard + the #6589 classifier pins + the #10889 quoting pins + the #11998 attempt-scoping sequence).`,
11551377
);
11561378
}
11571379

0 commit comments

Comments
 (0)