Skip to content

Commit 6d7f4df

Browse files
fix(pm): ci-failure reads aggregator roster jobs from their declared rosters, so the lint.yml verdicts rank last and point at their lanes (#23253)
Fixes #23249 Clause-②: no ## What changed `scripts/pm/ci-failure.mjs` only. `isRosterJob` knew roster jobs by the step-name pattern `/^Verify .* results$/`. That pattern matched the two ci.yml shard-attestation verdicts and missed both lint.yml aggregators, `Verify every lint lane succeeded` (`lint.yml:363`) and `Verify every type-check lane succeeded` (`lint.yml:6735`). Both line numbers were re-measured on `main` at `afbf960c26`. Following the card's stated preference, the names now come from the workflows' own aggregator roster instead of a wider pattern: - **`aggregatorRoster(workflows)`**: an aggregator is a job whose job-level `env:` declares `OS_AGGREGATOR_MEMBERS`. `MEMBERS_KEY` and `declaredIds` are imported from `scripts/check-aggregator-roster.mjs`, the gate that holds that declaration equal to `needs:`, so the key has one spelling and two readers. The import is safe before `pnpm install`: that module's YAML parser loads only inside its own reader. I ran this file's self-test from a `git archive` tree with no `node_modules` anywhere above it, and it exited 0. - **The verdict step**: the step of an aggregator that reads the `needs` context in a workflow expression. A step's job being an aggregator is not enough, because `Test Core` also runs `Check the per-package suite-duration ceilings`. That gate carries its own assertion, so its log is still the answer when it is the red step. - **`workflowJobs(text)`**: a hand-rolled job scanner, built the same way as `workflowSteps` and for the same reason (no `yaml` dependency). I cross-checked it once against the `yaml` parser over every workflow file at `5a9363ae28`. Over 40 files and 75 jobs, all 4 roster declarations agree on id, display name and members, with 0 mismatching files. - **Roster rows**, as the card asked: ranked last (`rankFailing`), labelled ROSTER JOB, no job log fetched (`shouldFetchJobLog`). A roster verdict step resolves to **no command of its own**. Its substitute names the member lanes that did not succeed. Those come from the check-runs the walk had already listed (`memberLanes`, matched by job display name and run id), so this costs no extra request. ### Bounded in-place fix, declared Before this change, the `Test Core` and `Dogfood Regression Gate` roster rows offered `node scripts/check-shard-attestation.mjs --verify ... --dir "$OS_ATTEST_DIR" ...` as the local reproduction. That command reads runner-only artifacts and env, so it reproduces nothing locally either. With one roster classification, these rows now get the same member-lane substitute. All four conditions for an in-place fix hold: - ① Same defect class as this card: a failed step whose own `run:` reproduces nothing. - ② The fix is mechanical, and its shape is the one this card pins. - ③ No other claim holds this file: the claim's serial-constraints line says so. - ④ Same gate family: the `ci-failure` self-test only. The self-test pins it. ## Before and after, `node scripts/pm/ci-failure.mjs --run 38373082677` Both outputs come from the same run. Before is `afbf960c26` (`main`); after is this branch. The exit is 2 / UNDETERMINED both times: no failing check on that sha carried a file-anchored assertion, either before or after. | | before | after | |---|---|---| | row order | `Lint & Repo Gates` · `Test Core (1/6)` · `Lint · repo & tooling gates` · `Test Core` | `Test Core (1/6)` · `Lint · repo & tooling gates` · `Test Core` · `Lint & Repo Gates` | | ROSTER JOB rows | 1 (`Test Core`) | 2 (`Test Core`, `Lint & Repo Gates`) | | job logs fetched | 3 (one was the aggregator's 7,950-byte log) | 2 | The before `Lint & Repo Gates` substitute. The excerpt stops just before the heredoc opener line, which is a shell here-document start reading `verify-lanes.mjs`: ```text substitute run the failing step locally — it prints the same assertion: from .github/workflows/lint.yml, step "Verify every lint lane succeeded" (run:) set -euo pipefail printf '%s' "$OS_NEEDS" > "$RUNNER_TEMP/needs.json" ``` The after row: ```text ✗ Lint & Repo Gates (check-run 115216483696 · run 38373082677) https://github.com/objectstack-ai/objectstack/actions/runs/38373082677/job/115216483696 ROSTER JOB — its failed step only judges what its declared member jobs concluded, and it holds no assertion of its own. The answer is in those members' rows, listed above; this row is kept so the count is honest. steps 3 real + 0 post — COMPLETE (contiguous from #1) failed step #2 Verify every lint lane succeeded ... No job log was fetched: a roster job's log is a verdict about the member jobs listed above, whose own rows already carry their assertions. roster verdict #2 "Verify every lint lane succeeded" judges other jobs, not this tree: job `lint` (.github/workflows/lint.yml) declares 8 member(s) in OS_AGGREGATOR_MEMBERS, and this step reads what they concluded from the runner's `needs` context, so its own command reproduces nothing locally. Member(s) that did not succeed in this walk: ✗ "Lint · repo & tooling gates" (lint-repo-tooling) — failure; its own row is above substitute none of its own — the member row(s) named above carry the answer, each with its own substitute. The roster verdict's command is not one. ``` The `Test Core` row now reads `✗ "Test Core (1/6)" (test) — failure; its own row is above` where it used to print the `--verify` command. ## Self-test `node scripts/pm/ci-failure.mjs --self-test` exits 0. Two batteries moved: - `isRosterJob` is now 19 cases (was 5). - New: `roster verdict: the substitute names the member lanes`, 13 cases. - The roster floor goes from 15 to 16. In both batteries, the pinned count equals the count registered. The card's three pins run against the **real** tree: - `Verify every lint lane succeeded` resolves to `Lint & Repo Gates`. - `Verify every type-check lane succeeded` resolves to `TypeScript Type Check`. - Control: `Verify build outputs` (ci.yml `build-core`) exists in the tree and stays unclassified. A second control: `Test Core`'s own ceiling gate exists and stays unclassified. The tests also hold this scan to `check-aggregator-roster`'s `REQUIRED_AGGREGATORS` floor (found, under its context, with a verdict step), and require every declared member to resolve to a named job. **Ablations** are one-shot runs: committed first, mutated with `scripts/ablation-replace.mjs` (anchor 1→0, blob moved), restored with the HEAD blob `fe3f35f3b26b` matched and `git diff HEAD` empty. No permanent test file was added. | mutation | self-test | |---|---| | old name pattern restored inside `rosterVerdictFor` | exit 1, **14** failures: both lint.yml pins, the shard pins, the substitute and specimen pins | | widened to `^Verify ` (the alternative the card warned about) | exit 1, **6** failures, including **both** controls | | every step of an aggregator counted as its verdict | exit 1, **4** failures, including the in-aggregator ceiling control | The first attempt at the first ablation landed but **crashed** the harness instead of reporting. The lanes fixtures had spread an absent verdict into a truthy one with no members. I fixed that in `5a9363ae28` (fixtures attach lanes only to a step that resolved as a roster verdict), and the reading above is from the re-run. ## Gates, at `5a9363ae28` - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 30 commands. All 30 ran and exited 0, including `pnpm check:pm-dispatch-gates`: fast tier, 1833 cases, 379.9 s. - `--ran` reconciliation: `30 derived, 30 run, 0 NOT-MEASURED, 0 UNRUN`. That zero is derived from the recorded exit codes. - NOT MEASURED: the ci.yml `Test Core` shard job the derivation schedules by path. That is CI's own shell, with no local invocation. - ESLint, narrowed, measured three ways: - ① The population for this diff is the one changed file. `eslint --print-config scripts/pm/ci-failure.mjs` applies 2 rules to it, so it is linted, not ignored. - ② `--format json` reports 1 file linted, 0 errors, 0 warnings, with `--no-inline-config`. - ③ `eslint.config.mjs` never enables type-aware linting: there is no `parserOptions.project` or `projectService`, and this file gets only `ecmaVersion`/`sourceType`. So this diff cannot move the verdict on any untouched file. - `skip-changeset`: `scripts/pm/**` publishes nothing. ## Acceptance notes - `verdictOf` still counts a roster row in the exit table's denominator, so a walk with a red roster job reads UNDETERMINED even when every member's assertion is in hand. That is the documented "this row is kept so the count is honest", left unchanged. Noted, not filed (no carrier). - The aggregators' own annotation sentences (for example, `lint lane ... concluded failure`) still classify as `other`. Members are named from the declared roster instead of by parsing those sentences, because they come from an inline workflow heredoc with no renderer this self-test could import and pin a reader against. Noted, not filed. - `checkNameMatcher` matches a `${{ ... }}` expression in a member's `name:` as any text. A matrix member whose `name:` has no expression gets GitHub's ` (values)` suffix and would not match. No member has that shape today: of 13 members, 2 are matrix jobs with expressions and 11 are not matrix jobs. Noted, not filed. --- _Generated by [Claude Code](https://claude.ai/code/session_012zwsLMxQBwHUYJgPyrpEvx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5da2c0f commit 6d7f4df

1 file changed

Lines changed: 584 additions & 34 deletions

File tree

0 commit comments

Comments
 (0)