Skip to content

Commit 614fdec

Browse files
fix(pm): ci-failure reads an aggregator's members from its own run's rows, so a later cancelled same-name row in another run no longer hides the red member (#23337)
Fixes #23305 Clause-②: no (internal PM tooling) ## What changed `scripts/pm/ci-failure.mjs` only, on the card's predicted landing point (`walkInto` and its `memberLanes` call). No `.github/workflows/**`. `latestPerName` is untouched, and the sha-level failing list is still read from its `kept` set. - **`checkRunListing(checkRuns)`**, new, next to `latestPerName`. It is the walk's one reading of a sha's check-runs: `{ all, kept, superseded }`. `kept` and `superseded` are exactly what `latestPerName(all)` returns. - **`memberLanes(aggregator, listing, { runId })`** now takes that whole listing instead of a flat row list: - It reads the members from `listing.all`, restricted to the aggregator's own run. - Inside that run, the newest row per name still wins (`latestPerName` over the run's rows). A job re-run within the run is still read at its latest attempt. - `listing.kept` is used for one thing only: telling whether a member's row is the one the report lists above. - **`walkInto`** builds the listing once and passes it to `memberLanes`. The failing list, `pending` and `total` are still read from `kept`. - **`renderRosterVerdict`**: a red member can be superseded on the sha by a later same-name row from another run. Its row is then not in the failing list, so "its own row is above" would point the reader at a row the report never prints. My change makes that branch reachable. That line now names the member's check-run, the row that superseded it, and its job URL. Every other lane line is unchanged. **Why the listing goes into `memberLanes`, instead of only swapping `kept` for `all` at the call site.** The walk only does network work, and the offline self-test never runs it. A fix made only at the call site could be reverted with the self-test still green. I measured exactly that, as ablation A4 below. Now the row set is chosen inside the pure function, so reading the wrong set turns the self-test red (A1). Passing the walk's `kept` array back in is a loud `TypeError`, not a silent "NONE". ## Root cause, measured On `main` at `cc14657ede`, `node scripts/pm/ci-failure.mjs --run 38411694784` exits 2 and prints `NONE of its members was found among this sha's check-runs` for `Lint & Repo Gates`. That reproduces the card. I ran a read-only probe over the sha's 106 check-runs (`GET /commits/042ef51530/check-runs`, 2 pages). It confirms the mechanism the card predicted, and the effect is wider than one row: - All 8 `Lint ·` member names have 3 rows each. - For **all 8**, `latestPerName` keeps the row from run `38411617233`. Those rows are cancelled and started at 14:15:56Z. It does not keep the rows from the asked run `38411694784`, which started at 14:14:50Z. - The kept `Lint & Repo Gates` row is the asked run's (`115353240189`). - So the run-id filter dropped every kept member row, and `matched` came back 0. The red member's three rows, `Lint · PM dispatch-gates`: - `115338637229`: success, run `38409754946`. - **`115345404875`: failure, run `38411694784`.** - `115345950501`: cancelled, run `38411617233`. This is the one `kept` holds. ## Before and after, `node scripts/pm/ci-failure.mjs --run 38411694784` - Before: `main` `cc14657ede`. After: this branch at `e29bf95658`. - Exit 2 / UNDETERMINED both times. - The sha-level reading is byte-identical: `49 check-run(s) after grouping by name (106 rows on the sha, 57 superseded)`, the same single failing row, and the same verdict line. - `diff` of the two outputs touches only the roster block. Before: ```text NONE of its members was found among this sha's check-runs (renamed, or absent from this tree?), so no row here carries the answer. Open the job URL above. ``` After: ```text Member(s) that did not succeed in this walk: ✗ "Lint · PM dispatch-gates" (lint-pm-dispatch-gates) — failure; check-run 115345404875, NOT listed above: a later same-name row from run 38411617233 (check-run 115345950501, cancelled) supersedes it on this sha. Its own annotations carry the answer: https://github.com/objectstack-ai/objectstack/actions/runs/38411694784/job/115345404875 ``` ## Self-test `node scripts/pm/ci-failure.mjs --self-test` exits 0 at `e29bf95658`. The battery `roster verdict: the substitute names the member lanes` goes from 13 to 17 cases. Its pin moves 13 → 17 and equals the count registered: I set the pin to 18 for one run, and it printed `registered 17 case(s), below its pinned floor of 18`. The fixture models the measured shape. Run 7 is the aggregator's run and run 9 is another run on the same sha: - `Fixture · lane A` is red in run 7, and a later cancelled row from run 9 supersedes it. - `Fixture shard (1/3)` failed in run 7, was re-run green inside run 7, and has a later red row in run 9. The four new cases: 1. **The trap is in the fixture.** The newest-per-name set holds run 9's rows for both names. 2. **The fix.** `memberLanes` finds the red member: 4 matched, and the one that did not succeed is check-run 101, superseded by 201. 3. **The control.** Shard 1/3's newest row in the aggregator's own run is green, so it reads as succeeded. Its older red row in that run and run 9's later red row do not count. 4. **The render.** The superseded red member is named by check-run and URL, and no line says "its own row is above". The existing cases now pass `checkRunListing(...)`, and their expected values are unchanged. ## Ablations These were one-shot runs, not kept as tests. Each one ran after the fix commit, through `node scripts/ablation-replace.mjs` in wrap mode. For each leg the anchor went 1 → 0 and the blob changed. Each restore read blob == HEAD `07b4465c88c4` with an empty `git diff HEAD`. `git status --porcelain` was empty after every leg. I wrote down the expected direction before each run. | leg | mutation | expected | observed | |---|---|---|---| | A1 | members read from `listing.kept` instead of `listing.all` (the original defect, moved into the tested function) | case 2 and the render case red | exactly those 2 red | | A2 | in-run grouping dropped (every row of the run matched) | case 2 and the control red | exactly those 2 red | | A3 | the superseded render branch disabled | the render case red | exactly 1 red | | A4 | the walk's call site handed `kept` again | offline self-test green, because the walk is not exercised offline; the live run loud | self-test exit 0. Live, 2nd attempt: `Cannot read properties of undefined (reading 'filter')`, caught by the mid-walk net, exit 2, "Nothing was judged". | The first A4 live attempt was **NOT MEASURED**. The jobs API answered `HTTP 502`, so no failed step was known and `memberLanes` was never called. I declared it and ran it once more. ## Gates All at `e29bf95658`. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 30 commands. All 30 exited 0, including `node scripts/pm/ci-failure.mjs --self-test` and `pnpm check:pm-dispatch-gates` (`fast 1839 case(s) in 370.5s`). - `--ran` reconciliation, with each line carrying its exit code: `30 derived famil(ies) accounted for — 30 run, 0 NOT-MEASURED (a DERIVED zero — all 30 recorded an exit code and none of them is 3)`. - Outside the derived set: `scripts/pm/check-governed-merges.mjs` reads this file's source in its self-test (the proxy re-arm guard-spelling mirror). `pnpm check:pm-governed-merges` exit 0, 500 assertions. - No new gate. **Lint, as a proven narrowing.** The full `pnpm lint` run is CI's. - Population: the one touched file. `eslint.config.mjs` lints it, and it is not ignored. - Count: `eslint --no-inline-config --format json scripts/pm/ci-failure.mjs` reports 1 file, 0 errors, 0 warnings. - Invariance: the resolved config for the file has no `parserOptions.project` and no `projectService`, so there is no type-aware linting. This diff cannot move any untouched file's verdict. **Changeset: none, and this PR takes `skip-changeset`.** - The root manifest is `private: true`. - None of the 71 non-private `package.json` files contains `scripts/pm/ci-failure.mjs` or lists it in `files[]`. - `git grep` for `checkRunListing` or `memberLanes` under `packages/**` exits 1 with 0 hits. - Positive control: the same `git grep` method finds `export const FieldSchema` in `packages/spec/src/data/field.zod.ts`, which `@objectstack/spec`'s `files[]` ships (`src/**/*.zod.ts`). Exit 0. - `scripts/pm/**` is not a governed surface: it has no row in `GOVERNED_SURFACES`. ## Acceptance notes - **Not changed: three general sentences are still false for this run.** - The `ROSTER JOB` banner says "The answer is in those members' rows, listed above". - The roster job-log line says "the member jobs listed above, whose own rows already carry their assertions". - The substitute line says "the member row(s) named above carry the answer, each with its own substitute". - On run `38411694784` no member row is listed above, and the new lane line says so. These sentences are outside the card's surface. They are reported to the PM seat as a finding. - **The verdict stays UNDETERMINED / exit 2.** The superseded red member's annotations are not fetched, because only `kept` rows are walked. That is the card's constraint on `latestPerName`'s role. The lane line gives the job URL instead. - **The walk's wiring stays outside the offline self-test**, as it was for the earlier roster and Lane-verdict changes to this file. With this change, a revert at the call site fails loudly. - **Seen only on the ablated tree:** the mid-walk net describes a non-transport exception in transport words ("the transport failed after 106 check-run(s) had been listed"). Noted, not filed. --- _Generated by [Claude Code](https://claude.ai/code/session_012zwsLMxQBwHUYJgPyrpEvx)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4185638 commit 614fdec

1 file changed

Lines changed: 142 additions & 18 deletions

File tree

‎scripts/pm/ci-failure.mjs‎

Lines changed: 142 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ const SELF_TEST_BATTERIES = Object.freeze({
354354
'assertionStatus: four answers, not two': 11,
355355
'the job log (#10141)': 34,
356356
'isRosterJob': 19,
357-
'roster verdict: the substitute names the member lanes': 13,
357+
'roster verdict: the substitute names the member lanes': 17,
358358
'runIdOf': 3,
359359
'stepBodies / resolveStep': 9,
360360
'Lane verdict: the red gates, not the verdict (#23223)': 22,
@@ -538,6 +538,24 @@ export function latestPerName(checkRuns) {
538538
return { kept, superseded };
539539
}
540540

541+
/**
542+
* The walk's one reading of a sha's check-runs: EVERY row (`all`) beside the
543+
* newest-per-name set (`kept`, `latestPerName`) and what that set dropped.
544+
*
545+
* The two sets answer different questions, and a reader handed only one of them
546+
* cannot answer the other (#23305). `kept` is the sha-level answer — which rows
547+
* are failing NOW, superseded rows ignored — and only it decides the report's
548+
* failing list. An aggregator's members are a run-level question: newest per
549+
* name across every run on the sha lets a LATER same-name row from ANOTHER run
550+
* (a cancelled push-event run) replace the aggregator's own red member, and a
551+
* run-id filter applied afterwards then finds nothing. So `memberLanes` takes
552+
* this whole listing and reads its members from `all`.
553+
*/
554+
export function checkRunListing(checkRuns) {
555+
const all = [...(checkRuns ?? [])];
556+
return { all, ...latestPerName(all) };
557+
}
558+
541559
function isNewer(a, b) {
542560
const at = Date.parse(a?.started_at ?? '') || 0;
543561
const bt = Date.parse(b?.started_at ?? '') || 0;
@@ -1063,16 +1081,45 @@ export function checkNameMatcher(jobName) {
10631081
* A member is matched by its job's display name and, where both are known, by
10641082
* run id: an aggregator's members run in its own workflow run, so a same-named
10651083
* job in another workflow is not one of them.
1084+
*
1085+
* `listing` is the walk's whole `checkRunListing`, and the members are read
1086+
* from its `all` rows, never from `kept` (#23305). `kept` is newest per name
1087+
* across EVERY run on the sha, so a later same-name row from another run — a
1088+
* cancelled push-event run on the same commit — had already replaced the
1089+
* aggregator's own red member there, and the run-id filter then found NONE of
1090+
* its members. Inside the aggregator's run the newest row per name still wins
1091+
* (`latestPerName` over that run's rows), so a job re-run within the run is
1092+
* read at its latest attempt, as before.
1093+
*
1094+
* `kept` still answers one thing here: whether a member's row is the one the
1095+
* report lists above. A member row that another run's later same-name row
1096+
* superseded on the sha is NOT in the failing list, so it carries its own
1097+
* check-run id and URL, and `supersededBy` names what replaced it.
10661098
*/
1067-
export function memberLanes(aggregator, checks, { runId = null } = {}) {
1099+
export function memberLanes(aggregator, listing, { runId = null } = {}) {
10681100
const members = (aggregator?.members ?? []).map((m) => ({ ...m, matches: checkNameMatcher(m.name ?? m.id) }));
1069-
const matched = [];
1070-
for (const check of checks ?? []) {
1101+
const ownRun = listing.all.filter((check) => {
10711102
const checkRun = runIdOf(check);
1072-
if (runId && checkRun && checkRun !== runId) continue;
1103+
return !(runId && checkRun && checkRun !== runId);
1104+
});
1105+
const listed = new Set(listing.kept);
1106+
const listedByName = new Map(listing.kept.map((check) => [check?.name ?? '', check]));
1107+
const matched = [];
1108+
for (const check of latestPerName(ownRun).kept) {
10731109
const member = members.find((m) => m.matches(check?.name));
10741110
if (!member) continue;
1075-
matched.push({ member: member.id, name: String(check.name), status: check.status ?? null, conclusion: check.conclusion ?? null });
1111+
const newer = listed.has(check) ? null : (listedByName.get(check?.name ?? '') ?? null);
1112+
matched.push({
1113+
member: member.id,
1114+
name: String(check.name),
1115+
status: check.status ?? null,
1116+
conclusion: check.conclusion ?? null,
1117+
checkRunId: check.id ?? null,
1118+
url: check.html_url ?? check.details_url ?? null,
1119+
supersededBy: newer
1120+
? { checkRunId: newer.id ?? null, runId: runIdOf(newer), status: newer.status ?? null, conclusion: newer.conclusion ?? null }
1121+
: null,
1122+
});
10761123
}
10771124
return { matched: matched.length, notSucceeded: matched.filter((m) => m.conclusion !== 'success') };
10781125
}
@@ -1423,12 +1470,28 @@ function renderRosterVerdict(step) {
14231470
} else {
14241471
out.push(`${pad}Member(s) that did not succeed in this walk:`);
14251472
for (const lane of lanes.notSucceeded) {
1426-
const where =
1427-
lane.conclusion === 'failure' || lane.conclusion === 'timed_out'
1428-
? 'its own row is above'
1429-
: lane.status !== 'completed'
1430-
? 'still running, so it has not answered yet'
1431-
: 'no row above: a lane that did not finish proves nothing — open its job';
1473+
const red = lane.conclusion === 'failure' || lane.conclusion === 'timed_out';
1474+
// #23305: a red member whose row another run's later same-name row
1475+
// superseded on the sha is NOT in the failing list above, so pointing
1476+
// there would send the reader to a row that is not printed.
1477+
if (red && lane.supersededBy) {
1478+
const by = lane.supersededBy;
1479+
const byRun = by.runId ? `run ${by.runId}` : 'another run';
1480+
out.push(
1481+
`${pad} ✗ "${lane.name}" (${lane.member}) — ${lane.conclusion}; check-run ${lane.checkRunId ?? 'unknown'}, NOT listed above:`,
1482+
);
1483+
out.push(
1484+
`${pad} a later same-name row from ${byRun} (check-run ${by.checkRunId ?? 'unknown'}, ` +
1485+
`${by.conclusion ?? by.status ?? 'unknown'}) supersedes it on this sha.`,
1486+
);
1487+
out.push(`${pad} Its own annotations carry the answer: ${lane.url ?? `/check-runs/${lane.checkRunId}/annotations`}`);
1488+
continue;
1489+
}
1490+
const where = red
1491+
? 'its own row is above'
1492+
: lane.status !== 'completed'
1493+
? 'still running, so it has not answered yet'
1494+
: 'no row above: a lane that did not finish proves nothing — open its job';
14321495
out.push(`${pad} ✗ "${lane.name}" (${lane.member}) — ${lane.conclusion ?? lane.status ?? 'unknown'}; ${where}`);
14331496
}
14341497
}
@@ -1856,8 +1919,10 @@ async function walk(sha) {
18561919
}
18571920

18581921
async function walkInto(sha, progress) {
1859-
const all = await checkRunsFor(sha, progress);
1860-
const { kept, superseded } = latestPerName(all);
1922+
// One listing, two readers (#23305): `kept` decides the failing list below,
1923+
// and a roster verdict's members are read from the full listing.
1924+
const listing = checkRunListing(await checkRunsFor(sha, progress));
1925+
const { all, kept, superseded } = listing;
18611926
const failingRuns = kept.filter((c) => c.conclusion === 'failure' || c.conclusion === 'timed_out');
18621927
const pending = kept.filter((c) => c.status !== 'completed').length;
18631928

@@ -1927,8 +1992,10 @@ async function walkInto(sha, progress) {
19271992
checkName: check.name,
19281993
});
19291994
// #23249: the member rows that carry a roster verdict's answer, from the
1930-
// check-runs already listed — no request.
1931-
if (resolved.rosterVerdict) resolved.rosterVerdict.lanes = memberLanes(resolved.rosterVerdict, kept, { runId });
1995+
// check-runs already listed — no request. The whole listing, not `kept`:
1996+
// the members are this run's rows, and `kept` may hold another run's
1997+
// later same-name row in their place (#23305).
1998+
if (resolved.rosterVerdict) resolved.rosterVerdict.lanes = memberLanes(resolved.rosterVerdict, listing, { runId });
19321999
return resolved;
19332000
}),
19342001
roster: rosterJob,
@@ -2682,7 +2749,7 @@ async function selfTest() {
26822749
{ name: 'Fixture · lane A', status: 'completed', conclusion: 'failure', details_url: runUrl(8) },
26832750
{ name: 'Fixture · lane AB', status: 'completed', conclusion: 'failure', details_url: runUrl(7) },
26842751
];
2685-
const lanes = memberLanes(fixtureRoster[0], fixtureChecks, { runId: '7' });
2752+
const lanes = memberLanes(fixtureRoster[0], checkRunListing(fixtureChecks), { runId: '7' });
26862753
// Lanes are attached only to a step that DID resolve as a roster verdict: a
26872754
// regression must be REPORTED by this harness, not crash it (see the job-log
26882755
// battery). Measured while ablating the roster read — spreading an absent
@@ -2737,6 +2804,63 @@ async function selfTest() {
27372804
lanesSay({ matched: 3, notSucceeded: [] }).some((l) => l.includes('Every one of the 3 member row(s) found here succeeded')),
27382805
true,
27392806
);
2807+
2808+
// #23305, the measured shape (run 38411694784 on 042ef51530): the asked run's
2809+
// red member, and a LATER cancelled same-name row from another run on the
2810+
// same sha. Run 7 is the aggregator's run, run 9 the other one. Lane A is red
2811+
// in run 7 and superseded by run 9's cancelled row. Shard 1/3 failed and was
2812+
// re-run green INSIDE run 7, and run 9 has a later red row under its name.
2813+
const crossRunUrl = (run, job) => `https://github.com/o/r/actions/runs/${run}/job/${job}`;
2814+
const crossRun = (id, name, run, conclusion, startedAt) => ({
2815+
id,
2816+
name,
2817+
status: 'completed',
2818+
conclusion,
2819+
started_at: `2026-10-13T${startedAt}:00Z`,
2820+
details_url: crossRunUrl(run, id),
2821+
html_url: crossRunUrl(run, id),
2822+
});
2823+
const crossRunChecks = [
2824+
crossRun(100, 'Fixture Gates', 7, 'failure', '14:30'),
2825+
crossRun(101, 'Fixture · lane A', 7, 'failure', '14:14'),
2826+
crossRun(102, 'Fixture shard (1/3)', 7, 'failure', '14:14'),
2827+
crossRun(103, 'Fixture shard (1/3)', 7, 'success', '14:20'),
2828+
crossRun(104, 'Fixture shard (2/3)', 7, 'success', '14:14'),
2829+
crossRun(105, 'Fixture shard (3/3)', 7, 'success', '14:14'),
2830+
crossRun(201, 'Fixture · lane A', 9, 'cancelled', '14:15'),
2831+
crossRun(202, 'Fixture shard (1/3)', 9, 'failure', '14:25'),
2832+
];
2833+
const crossRunListing = checkRunListing(crossRunChecks);
2834+
t(
2835+
'the trap is in the fixture: the newest-per-name set holds run 9\'s rows for lane A and shard 1/3, not run 7\'s',
2836+
crossRunListing.kept.filter((c) => ['Fixture · lane A', 'Fixture shard (1/3)'].includes(c.name)).map((c) => [c.id, runIdOf(c)]),
2837+
[[201, '9'], [202, '9']],
2838+
);
2839+
const crossRunLanes = memberLanes(fixtureRoster[0], crossRunListing, { runId: '7' });
2840+
t(
2841+
"memberLanes reads the aggregator's own run from every row, so a later cancelled same-name row in another run does not hide the red member",
2842+
[
2843+
crossRunLanes.matched,
2844+
crossRunLanes.notSucceeded.map((l) => [l.member, l.name, l.conclusion, l.checkRunId, l.supersededBy?.checkRunId ?? null]),
2845+
],
2846+
[4, [['lane-a', 'Fixture · lane A', 'failure', 101, 201]]],
2847+
);
2848+
t(
2849+
"the control: a member whose newest row in the aggregator's own run is green reads as succeeded — its older red row in that run and another run's later red row notwithstanding",
2850+
crossRunLanes.notSucceeded.some((l) => l.name === 'Fixture shard (1/3)'),
2851+
false,
2852+
);
2853+
const crossRunLines = lanesSay(crossRunLanes);
2854+
t(
2855+
'a red member superseded on the sha is named by check-run and URL, and is never said to have its own row above',
2856+
[
2857+
crossRunLines.some((l) => l.includes('✗ "Fixture · lane A" (lane-a) — failure; check-run 101, NOT listed above:')),
2858+
crossRunLines.some((l) => l.includes('a later same-name row from run 9 (check-run 201, cancelled) supersedes it on this sha.')),
2859+
crossRunLines.some((l) => l.includes(`Its own annotations carry the answer: ${crossRunUrl(7, 101)}`)),
2860+
crossRunLines.some((l) => l.includes('its own row is above')),
2861+
],
2862+
[true, true, true, false],
2863+
);
27402864
t(
27412865
'a roster row ranks after every other row, both groups keeping walk order',
27422866
rankFailing([
@@ -2771,7 +2895,7 @@ async function selfTest() {
27712895
{ name: 'Lint · ESLint', status: 'completed', conclusion: 'success', details_url: runUrl(specimenRun) },
27722896
];
27732897
const lintLines = renderSubstitute([
2774-
withLanes(lintVerdict, memberLanes(lintVerdict.rosterVerdict, specimenChecks, { runId: specimenRun })),
2898+
withLanes(lintVerdict, memberLanes(lintVerdict.rosterVerdict, checkRunListing(specimenChecks), { runId: specimenRun })),
27752899
]);
27762900
t(
27772901
'the specimen: the lint verdict names `Lint · repo & tooling gates` and prints no line of its heredoc',

0 commit comments

Comments
 (0)