From 56175cd125bb803ced0256f104dd763b835a5017 Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Wed, 14 Oct 2026 04:03:46 +0800 Subject: [PATCH 1/5] CI: let only main's tip run open or close the pinned main-failure issue Runs on main finish out of order. On 2026-10-13 the tip's green run (c67fd996) closed #3099 and the older d3fda691 run, finishing six minutes later, reopened it for a failure main no longer had. The routing job now reads main's current tip and leaves the issue alone when its run is superseded; it fails open if the tip cannot be read. --- .github/workflows/ci.yml | 19 +++++++++++++++++++ tests/ci-main-failure-routing.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68924a2271..35eb0b657b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2320,6 +2320,25 @@ jobs: } } + // Only the run for main's CURRENT tip may speak for main. Runs finish out of order: on + // 2026-10-13 the tip's green run closed the issue at 17:27Z and an older commit's run, + // finishing at 17:33Z, reopened it for a failure main no longer had. A superseded run is + // still visible in its own checks; it just must not reopen or close the pinned issue. + // Fails open: if the tip cannot be read, route as before rather than lose an alert. + try { + const { data: branch } = await github.rest.repos.getBranch({ owner, repo, branch: "main" }); + const tipSha = branch?.commit?.sha ?? ""; + if (headSha && tipSha && tipSha !== headSha) { + core.info( + `Run is for ${headSha.slice(0, 12)} but main is now at ${tipSha.slice(0, 12)}; ` + + `leaving the pinned issue to the tip's run (this run was ${red ? "red" : "green"}).`, + ); + return; + } + } catch (error) { + core.warning(`Could not read main's tip, routing this run anyway: ${error.message}`); + } + const { data: open } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); const owned = (issue) => String(issue.body ?? "").includes(MARKER) || issue.title === title; const existing = open.find(owned) ?? null; diff --git a/tests/ci-main-failure-routing.test.ts b/tests/ci-main-failure-routing.test.ts index ca219f415f..eea8258e40 100644 --- a/tests/ci-main-failure-routing.test.ts +++ b/tests/ci-main-failure-routing.test.ts @@ -309,3 +309,27 @@ describe("main CI failure routing, executed", () => { expect(created).toHaveLength(1); }); }); + +/** + * Runs on main finish out of order (2026-10-13: the tip's green run closed #3099, then an older + * commit's red run reopened it). Only the run for main's current tip may open, update or close the + * pinned issue, and the check must come before any issue is touched, and must fail open. + */ +describe("main CI failure routing ignores superseded runs", () => { + const routing = workflow.slice(workflow.indexOf(" main-failure-routing:")); + const tipCheck = routing.indexOf('github.rest.repos.getBranch({ owner, repo, branch: "main" })'); + + it("compares the run's commit with main's current tip and stops when they differ", () => { + expect(tipCheck).toBeGreaterThan(-1); + expect(routing).toMatch(/if \(headSha && tipSha && tipSha !== headSha\) \{[\s\S]*?return;/); + }); + + it("checks the tip before it reads, closes or reopens any issue", () => { + expect(tipCheck).toBeLessThan(routing.indexOf("issues.listForRepo(")); + expect(tipCheck).toBeLessThan(routing.indexOf("if (!red)")); + }); + + it("fails open so an unreadable tip never swallows an alert", () => { + expect(routing).toContain("Could not read main's tip, routing this run anyway"); + }); +}); From bdd16bb38b0dc0f4e5464f50cea3e38a0b35c018 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 02:56:45 +0000 Subject: [PATCH 2/5] CI: order main-failure routing by last routed commit, not main's tip Review found the tip-only gate drops alerts whenever merges outpace a CI run, and that the check-then-mutate sequence could still interleave. Every body the routing job writes now records the commit it last routed; a run whose commit is behind that one stands down, and one that is ahead of it speaks. The job runs serialised (cancel-in-progress false) so the read and the write cannot interleave. The tests now execute the script against a compareCommits mock instead of matching its source text. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_015r3N1AdnrkQEPgiP4DuJdN --- .github/workflows/ci.yml | 102 +++++++++++----- tests/ci-cache-safety.test.ts | 4 +- tests/ci-main-failure-routing.test.ts | 166 +++++++++++++++++++++++--- 3 files changed, 224 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35eb0b657b..44cdbef43d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2218,6 +2218,12 @@ jobs: if: ${{ always() && github.event_name == 'push' && github.ref == 'refs/heads/main' }} runs-on: ubuntu-24.04 timeout-minutes: 5 + # One routing job at a time, never cancelling the one in flight. Without this a slow older run + # could read the issue, wait, and write after a newer run had already routed. A newer waiting run + # replaces an older waiting one, which is safe: the newest run is the one that must speak. + concurrency: + group: main-failure-routing + cancel-in-progress: false permissions: contents: read issues: write @@ -2320,31 +2326,71 @@ jobs: } } - // Only the run for main's CURRENT tip may speak for main. Runs finish out of order: on + // A run older than the one that last routed must not undo it. Runs finish out of order: on // 2026-10-13 the tip's green run closed the issue at 17:27Z and an older commit's run, - // finishing at 17:33Z, reopened it for a failure main no longer had. A superseded run is - // still visible in its own checks; it just must not reopen or close the pinned issue. - // Fails open: if the tip cannot be read, route as before rather than lose an alert. - try { - const { data: branch } = await github.rest.repos.getBranch({ owner, repo, branch: "main" }); - const tipSha = branch?.commit?.sha ?? ""; - if (headSha && tipSha && tipSha !== headSha) { - core.info( - `Run is for ${headSha.slice(0, 12)} but main is now at ${tipSha.slice(0, 12)}; ` + - `leaving the pinned issue to the tip's run (this run was ${red ? "red" : "green"}).`, - ); - return; + // finishing at 17:33Z, reopened it for a failure main no longer had. Every body this job + // writes carries the commit it last routed, and a run whose commit is BEHIND that one + // stands down. Ordering by what was last routed (not by "is this main's tip") means a busy + // merge streak cannot suppress every alert: an older red run still speaks when nothing + // newer has. Fails open: an unreadable ordering routes rather than loses an alert. + const ROUTED_PATTERN = //; + const withRoutedStamp = (text) => { + const base = String(text ?? "").replace(ROUTED_PATTERN, "").trimEnd(); + return headSha ? `${base}\n` : base; + }; + async function isSuperseded(issue) { + const routedSha = ROUTED_PATTERN.exec(String(issue?.body ?? ""))?.[1]; + if (!headSha || !routedSha || routedSha === headSha) return false; + try { + const { data: comparison } = await github.rest.repos.compareCommits({ + owner, + repo, + base: routedSha, + head: headSha, + }); + return comparison?.status === "behind"; + } catch (error) { + core.warning(`Could not order this run against ${routedSha.slice(0, 12)}, routing it anyway: ${error.message}`); + return false; } - } catch (error) { - core.warning(`Could not read main's tip, routing this run anyway: ${error.message}`); } const { data: open } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); const owned = (issue) => String(issue.body ?? "").includes(MARKER) || issue.title === title; const existing = open.find(owned) ?? null; + // The most recent issue this job closed, matched by marker only so it never reopens an + // issue a person wrote. It is the ordering reference when nothing is open, and the issue a + // new red spell reopens. + async function latestClosedOwned() { + const { data: closed } = await github.rest.issues.listForRepo({ + owner, + repo, + state: "closed", + labels: label, + sort: "updated", + direction: "desc", + per_page: 20, + }); + return closed.find((issue) => !issue.pull_request && String(issue.body ?? "").includes(MARKER)) ?? null; + } + const previous = existing ? null : await latestClosedOwned(); + + if (await isSuperseded(existing ?? previous)) { + core.info( + `Run for ${headSha.slice(0, 12)} is older than the commit already routed; ` + + `leaving the pinned issue alone (this run was ${red ? "red" : "green"}).`, + ); + return; + } + if (!red) { if (!existing) { + // Record this green run on the closed issue, so a still-older red run that finishes + // after it cannot reopen the issue for a failure main no longer has. + if (previous && withRoutedStamp(previous.body) !== String(previous.body ?? "")) { + await github.rest.issues.update({ owner, repo, issue_number: previous.number, body: withRoutedStamp(previous.body) }); + } core.info("CI on main is green and no failure issue of ours is open."); return; } @@ -2364,7 +2410,14 @@ jobs: MARKER, ].join("\n"), }); - await github.rest.issues.update({ owner, repo, issue_number: existing.number, state: "closed", state_reason: "completed" }); + await github.rest.issues.update({ + owner, + repo, + issue_number: existing.number, + state: "closed", + state_reason: "completed", + body: withRoutedStamp(existing.body), + }); core.info(`Closed #${existing.number} after a green main run.`); return; } @@ -2395,9 +2448,10 @@ jobs: "is a standing defect rather than a regression this push introduced.", MARKER, ].join("\n"); + const stampedBody = withRoutedStamp(body); if (existing) { - await github.rest.issues.update({ owner, repo, issue_number: existing.number, body }); + await github.rest.issues.update({ owner, repo, issue_number: existing.number, body: stampedBody }); await github.rest.issues.createComment({ owner, repo, @@ -2410,18 +2464,8 @@ jobs: // Reopen the most recent issue this job closed rather than opening a new one per red // spell (owner decision, 2026-09-26: main flips red and green often, and 25 issues in a // day was noise). Marker-only match, so it never reopens an issue a person wrote. - const { data: closed } = await github.rest.issues.listForRepo({ - owner, - repo, - state: "closed", - labels: label, - sort: "updated", - direction: "desc", - per_page: 20, - }); - const previous = closed.find((issue) => !issue.pull_request && String(issue.body ?? "").includes(MARKER)) ?? null; if (previous) { - await github.rest.issues.update({ owner, repo, issue_number: previous.number, state: "open", title, body }); + await github.rest.issues.update({ owner, repo, issue_number: previous.number, state: "open", title, body: stampedBody }); await github.rest.issues.createComment({ owner, repo, @@ -2431,7 +2475,7 @@ jobs: await assignAlert(previous.number); core.warning(`CI on main is failing; reopened #${previous.number}.`); } else { - const created = await github.rest.issues.create({ owner, repo, title, labels: [label], body }); + const created = await github.rest.issues.create({ owner, repo, title, labels: [label], body: stampedBody }); await assignAlert(created.data.number); core.warning(`CI on main is failing; opened #${created.data.number}.`); } diff --git a/tests/ci-cache-safety.test.ts b/tests/ci-cache-safety.test.ts index 99970e2067..2f2a8f9948 100644 --- a/tests/ci-cache-safety.test.ts +++ b/tests/ci-cache-safety.test.ts @@ -581,7 +581,9 @@ describe("CI cache safety", () => { * its own case rather than left to review. */ it("never cancels an in-flight run for a base-branch push", () => { - const concurrency = sourceSegment(workflow, "concurrency:", "permissions:", { + // Anchored on the unindented key: job-level `concurrency:` blocks (the main-failure router + // serialises itself) are indented, so this stays the workflow-level contract. + const concurrency = sourceSegment(workflow, "\nconcurrency:\n", "permissions:", { label: "workflow concurrency block", }); diff --git a/tests/ci-main-failure-routing.test.ts b/tests/ci-main-failure-routing.test.ts index eea8258e40..b7c938da86 100644 --- a/tests/ci-main-failure-routing.test.ts +++ b/tests/ci-main-failure-routing.test.ts @@ -58,7 +58,7 @@ describe("main CI failure routing", () => { it("reopens its most recent closed issue instead of opening a new one", () => { expect(routing).toContain('state: "closed",'); expect(routing).toMatch( - /const previous = closed\.find\(\(issue\) => !issue\.pull_request && String\(issue\.body \?\? ""\)\.includes\(MARKER\)\)/, + /closed\.find\(\(issue\) => !issue\.pull_request && String\(issue\.body \?\? ""\)\.includes\(MARKER\)\)/, ); expect(routing).toContain('state: "open", title, body'); expect(routing.indexOf('state: "open", title, body')).toBeLessThan(routing.indexOf("issues.create(")); @@ -186,14 +186,23 @@ function routingScriptSource() { return body.join("\n"); } +const RUN_SHA = "0123456789abcdef0123456789abcdef01234567"; +const ROUTED_SHA = "fedcba9876543210fedcba9876543210fedcba98"; + async function runRouting(options: { results: { prRequired: string; matrix: string; visual: string }; jobs?: ListedJob[]; listingFails?: boolean; openIssues?: Issue[]; + closedIssues?: Issue[]; + /** What `compareCommits` reports for the run's commit against the last routed one, or an Error to reject. */ + compare?: "behind" | "ahead" | "identical" | "diverged" | Error; }) { const created: Array<{ title: string; body: string }> = []; const closed: number[] = []; + const updates: Array<{ issue_number: number; state?: string; body?: string }> = []; + const comments: number[] = []; + const compared: Array<{ base: string; head: string }> = []; const github = { paginate: async () => { if (options.listingFails) throw new Error("Resource not accessible by integration"); @@ -203,18 +212,28 @@ async function runRouting(options: { actions: { listJobsForWorkflowRun: () => undefined }, issues: { listForRepo: async ({ state }: { state: string }) => ({ - data: state === "open" ? (options.openIssues ?? []) : [], + data: state === "open" ? (options.openIssues ?? []) : (options.closedIssues ?? []), }), create: async (request: { title: string; body: string }) => { created.push(request); return { data: { number: 900 } }; }, - update: async (request: { issue_number: number; state?: string }) => { + update: async (request: { issue_number: number; state?: string; body?: string }) => { + updates.push(request); if (request.state === "closed") closed.push(request.issue_number); }, - createComment: async () => undefined, + createComment: async (request: { issue_number: number }) => { + comments.push(request.issue_number); + }, addAssignees: async () => undefined, }, + repos: { + compareCommits: async (request: { base: string; head: string }) => { + compared.push(request); + if (options.compare instanceof Error) throw options.compare; + return { data: { status: options.compare ?? "ahead" } }; + }, + }, }, }; const context = { repo: { owner: "BigSimmo", repo: "PsychSift" }, runId: 1, serverUrl: "https://github.com" }; @@ -223,7 +242,7 @@ async function runRouting(options: { PR_REQUIRED_RESULT: options.results.prRequired, BROWSER_MATRIX_RESULT: options.results.matrix, VISUAL_BASELINE_RESULT: options.results.visual, - HEAD_SHA: "0123456789abcdef", + HEAD_SHA: RUN_SHA, HEAD_MESSAGE: "Example merge", }; const previous = Object.fromEntries(Object.keys(env).map((key) => [key, process.env[key]])); @@ -236,7 +255,7 @@ async function runRouting(options: { else process.env[key] = value; } } - return { created, closed }; + return { created, closed, updates, comments, compared }; } describe("main CI failure routing, executed", () => { @@ -312,24 +331,135 @@ describe("main CI failure routing, executed", () => { /** * Runs on main finish out of order (2026-10-13: the tip's green run closed #3099, then an older - * commit's red run reopened it). Only the run for main's current tip may open, update or close the - * pinned issue, and the check must come before any issue is touched, and must fail open. + * commit's run reopened it). Every body the job writes records the commit it last routed, and a run + * whose commit is behind that one stands down. Ordering by what was last routed, not by "is this + * main's tip", means a merge streak cannot suppress every alert. */ describe("main CI failure routing ignores superseded runs", () => { - const routing = workflow.slice(workflow.indexOf(" main-failure-routing:")); - const tipCheck = routing.indexOf('github.rest.repos.getBranch({ owner, repo, branch: "main" })'); + const red = { prRequired: "failure", matrix: "skipped", visual: "skipped" }; + const green = { prRequired: "success", matrix: "success", visual: "success" }; + const failingJobs: ListedJob[] = [{ name: "Build", conclusion: "failure" }]; + const stamp = (sha: string) => ``; + const marked = (sha: string): Issue => ({ + number: 42, + title: "CI on main is failing", + body: `\n${stamp(sha)}`, + }); + + it("serialises the routing job without cancelling the run in flight", () => { + const routing = workflow.slice(workflow.indexOf(" main-failure-routing:")); + expect(routing).toMatch(/concurrency:\n {6}group: main-failure-routing\n {6}cancel-in-progress: false/); + }); + + it("leaves the issue untouched when a superseded red run finishes after a newer one routed", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(ROUTED_SHA)], + compare: "behind", + }); + expect(result.compared).toMatchObject([{ base: ROUTED_SHA, head: RUN_SHA }]); + expect(result.updates).toHaveLength(0); + expect(result.comments).toHaveLength(0); + expect(result.created).toHaveLength(0); + }); + + it("does not close the issue for a superseded green run", async () => { + const result = await runRouting({ results: green, jobs: [], openIssues: [marked(ROUTED_SHA)], compare: "behind" }); + expect(result.closed).toHaveLength(0); + expect(result.updates).toHaveLength(0); + }); + + it("does not reopen a closed issue for a red run older than the green that closed it", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + closedIssues: [marked(ROUTED_SHA)], + compare: "behind", + }); + expect(result.updates).toHaveLength(0); + expect(result.created).toHaveLength(0); + }); - it("compares the run's commit with main's current tip and stops when they differ", () => { - expect(tipCheck).toBeGreaterThan(-1); - expect(routing).toMatch(/if \(headSha && tipSha && tipSha !== headSha\) \{[\s\S]*?return;/); + it("still alerts when an older commit's run is the newest one to report (merge streak)", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(ROUTED_SHA)], + compare: "ahead", + }); + expect(result.updates).toHaveLength(1); + expect(result.updates[0]!.body).toContain(stamp(RUN_SHA)); + expect(result.updates[0]!.body).not.toContain(stamp(ROUTED_SHA)); + expect(result.comments).toEqual([42]); + }); + + it("opens an issue stamped with the run's commit when a red run finds none", async () => { + const result = await runRouting({ results: red, jobs: failingJobs }); + expect(result.created).toHaveLength(1); + expect(result.created[0]!.body).toContain(stamp(RUN_SHA)); + expect(result.compared).toHaveLength(0); + }); + + it("stamps the closing green run on the issue so a later stale red run cannot reopen it", async () => { + const result = await runRouting({ results: green, jobs: [], openIssues: [marked(ROUTED_SHA)], compare: "ahead" }); + expect(result.closed).toEqual([42]); + expect(result.updates[0]!.body).toContain(stamp(RUN_SHA)); + expect(result.updates[0]!.body).toContain(""); }); - it("checks the tip before it reads, closes or reopens any issue", () => { - expect(tipCheck).toBeLessThan(routing.indexOf("issues.listForRepo(")); - expect(tipCheck).toBeLessThan(routing.indexOf("if (!red)")); + it("records a green run on the closed issue when none is open", async () => { + const result = await runRouting({ results: green, jobs: [], closedIssues: [marked(ROUTED_SHA)], compare: "ahead" }); + expect(result.closed).toHaveLength(0); + expect(result.updates).toHaveLength(1); + expect(result.updates[0]!.state).toBeUndefined(); + expect(result.updates[0]!.body).toContain(stamp(RUN_SHA)); }); - it("fails open so an unreadable tip never swallows an alert", () => { - expect(routing).toContain("Could not read main's tip, routing this run anyway"); + it("routes a red run that has no recorded commit to order against", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [ownIssueWithoutStamp()], + compare: "behind", + }); + expect(result.compared).toHaveLength(0); + expect(result.updates).toHaveLength(1); + }); + + it("fails open: an ordering lookup that errors still routes the alert", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(ROUTED_SHA)], + compare: new Error("Not Found"), + }); + expect(result.updates).toHaveLength(1); + expect(result.comments).toEqual([42]); + }); + + it("treats a diverged history as not superseded", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(ROUTED_SHA)], + compare: "diverged", + }); + expect(result.updates).toHaveLength(1); + }); + + it("does not compare a run with itself when it is the commit last routed", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(RUN_SHA)], + compare: "behind", + }); + expect(result.compared).toHaveLength(0); + expect(result.updates).toHaveLength(1); }); }); + +function ownIssueWithoutStamp(): Issue { + return { number: 42, title: "CI on main is failing", body: "" }; +} From d17e7df4589277763448248275e6fd70cad720c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:17:47 +0000 Subject: [PATCH 3/5] CI: drop routing concurrency group, re-check routed stamp before writing A concurrency group keeps one pending job in run-finish order, so an older run could cancel a newer commit's routing. Order by the routed-sha stamp and re-read it just before each write instead. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01KpmSAVMAELX796AzEujHSw --- .github/workflows/ci.yml | 24 ++++++++++++++++++------ tests/ci-main-failure-routing.test.ts | 5 +++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44cdbef43d..6b3e8f0291 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2218,12 +2218,11 @@ jobs: if: ${{ always() && github.event_name == 'push' && github.ref == 'refs/heads/main' }} runs-on: ubuntu-24.04 timeout-minutes: 5 - # One routing job at a time, never cancelling the one in flight. Without this a slow older run - # could read the issue, wait, and write after a newer run had already routed. A newer waiting run - # replaces an older waiting one, which is safe: the newest run is the one that must speak. - concurrency: - group: main-failure-routing - cancel-in-progress: false + # No job-level `concurrency` group on purpose. A concurrency group keeps only ONE pending job and + # replaces it with whichever job became ready last, which follows run-finish order, not commit + # order: a newer red run's routing could be cancelled by an older green run, leaving the issue + # closed while main is red (and cancelling that newer run's whole CI result). Ordering is + # instead enforced by the routed-sha stamp, and re-checked immediately before each write. permissions: contents: read issues: write @@ -2376,6 +2375,14 @@ jobs: } const previous = existing ? null : await latestClosedOwned(); + // Re-read the stamp right before writing, so a run that routed while this one was reading + // is honoured. This replaces serialisation: the window is a few API calls, not a whole job. + async function supersededNow() { + const { data: nowOpen } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); + const current = nowOpen.find(owned) ?? (await latestClosedOwned()); + return isSuperseded(current); + } + if (await isSuperseded(existing ?? previous)) { core.info( `Run for ${headSha.slice(0, 12)} is older than the commit already routed; ` + @@ -2384,6 +2391,11 @@ jobs: return; } + if (await supersededNow()) { + core.info(`Run for ${headSha.slice(0, 12)} was superseded while reading; leaving the pinned issue alone.`); + return; + } + if (!red) { if (!existing) { // Record this green run on the closed issue, so a still-older red run that finishes diff --git a/tests/ci-main-failure-routing.test.ts b/tests/ci-main-failure-routing.test.ts index b7c938da86..892b0c303b 100644 --- a/tests/ci-main-failure-routing.test.ts +++ b/tests/ci-main-failure-routing.test.ts @@ -346,9 +346,10 @@ describe("main CI failure routing ignores superseded runs", () => { body: `\n${stamp(sha)}`, }); - it("serialises the routing job without cancelling the run in flight", () => { + it("does not use a job-level concurrency group that could cancel a newer commit's routing", () => { const routing = workflow.slice(workflow.indexOf(" main-failure-routing:")); - expect(routing).toMatch(/concurrency:\n {6}group: main-failure-routing\n {6}cancel-in-progress: false/); + const jobBlock = routing.slice(0, routing.indexOf(" steps:")); + expect(jobBlock).not.toMatch(/\n {4}concurrency:/); }); it("leaves the issue untouched when a superseded red run finishes after a newer one routed", async () => { From 7335e8034acc48aaf44946e4cc93530708f9a6c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 03:30:34 +0000 Subject: [PATCH 4/5] test(roster): pin the clock in the give-away test so it stops expiring The give-away sheet reads the real clock, and the fixture shift (Thu 15 Oct, 09:00 Perth) counts as urgent inside 24 hours, which swaps "Offer to 1 person" for "I can't make it". The test passed only while the real date was more than a day before the shift. From 09:00 Perth on 14 Oct 2026 it failed on every run, turning Unit coverage red on main and on every PR. Date is now pinned to two days before the shift for that one test. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_015r3N1AdnrkQEPgiP4DuJdN --- tests/roster-team-calendar-manager.dom.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/roster-team-calendar-manager.dom.test.tsx b/tests/roster-team-calendar-manager.dom.test.tsx index 6262246bcf..c38ac22730 100644 --- a/tests/roster-team-calendar-manager.dom.test.tsx +++ b/tests/roster-team-calendar-manager.dom.test.tsx @@ -297,6 +297,12 @@ describe("Team calendar, manager layer", () => { }); it("reads requests, manage and the roster again after a give-away is sent from the calendar", async () => { + // The give-away sheet reads the real clock (`useRosterNow()` is not handed the page's pinned + // `now`), and the shift (Thu 15 Oct, 09:00 Perth) counts as urgent inside 24 hours, which swaps + // "Offer to 1 person" for "I can't make it". This test only ever passed because the real date + // was more than a day before the fixture shift; from 09:00 Perth on 14 Oct 2026 it failed on + // every run. Pin Date to two days before the shift so the ordinary give-away path is the one tested. + vi.useFakeTimers({ toFake: ["Date"], now: new Date("2026-10-13T00:00:00Z") }); const { posts, reads } = mockFetch(); render(); fireEvent.click(await screen.findByRole("button", { name: /You, Day 09:00–17:00/ })); From e9a2cc111a5f9c8d73a252d732034e79c778dff8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 04:07:32 +0000 Subject: [PATCH 5/5] CI: re-check rebuilds fresh issue state for writes, and fails open if it cannot read The routed-sha re-check before writing now re-reads the open and closed issue state and the writes act on it, instead of the state read at the start. If the re-check read itself fails, warn and route anyway. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01KpmSAVMAELX796AzEujHSw --- .github/workflows/ci.yml | 29 ++++++++--- tests/ci-main-failure-routing.test.ts | 75 +++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b3e8f0291..49cf949424 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2354,9 +2354,7 @@ jobs: } } - const { data: open } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); const owned = (issue) => String(issue.body ?? "").includes(MARKER) || issue.title === title; - const existing = open.find(owned) ?? null; // The most recent issue this job closed, matched by marker only so it never reopens an // issue a person wrote. It is the ordering reference when nothing is open, and the issue a @@ -2373,14 +2371,29 @@ jobs: }); return closed.find((issue) => !issue.pull_request && String(issue.body ?? "").includes(MARKER)) ?? null; } - const previous = existing ? null : await latestClosedOwned(); - // Re-read the stamp right before writing, so a run that routed while this one was reading - // is honoured. This replaces serialisation: the window is a few API calls, not a whole job. + // `existing` is our open issue; `previous` is the latest closed one, read only when nothing + // is open. Every write below acts on these two, so the re-check rebuilds both together. + async function readIssueState() { + const { data: open } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); + const openOwned = open.find(owned) ?? null; + return { existing: openOwned, previous: openOwned ? null : await latestClosedOwned() }; + } + let { existing, previous } = await readIssueState(); + + // Re-read the issue state right before writing, so a run that routed while this one was + // reading is honoured, and the writes below act on the fresh open/closed issue rather than + // the stale one. This replaces serialisation: the window is a few API calls, not a whole + // job. Fails open like the ordering lookup: an unreadable re-check routes on the state + // already read rather than aborting and losing the alert. async function supersededNow() { - const { data: nowOpen } = await github.rest.issues.listForRepo({ owner, repo, state: "open", labels: label }); - const current = nowOpen.find(owned) ?? (await latestClosedOwned()); - return isSuperseded(current); + try { + ({ existing, previous } = await readIssueState()); + } catch (error) { + core.warning(`Could not re-read the pinned issue before writing, routing on the state already read: ${error.message}`); + return false; + } + return isSuperseded(existing ?? previous); } if (await isSuperseded(existing ?? previous)) { diff --git a/tests/ci-main-failure-routing.test.ts b/tests/ci-main-failure-routing.test.ts index 892b0c303b..05cf0e6175 100644 --- a/tests/ci-main-failure-routing.test.ts +++ b/tests/ci-main-failure-routing.test.ts @@ -195,6 +195,12 @@ async function runRouting(options: { listingFails?: boolean; openIssues?: Issue[]; closedIssues?: Issue[]; + /** Per-call open-issue reads (the last entry repeats), to model the pinned issue changing mid-run. */ + openIssuesByCall?: Issue[][]; + /** Per-call closed-issue reads (the last entry repeats). */ + closedIssuesByCall?: Issue[][]; + /** Make the nth (1-based) open-issue read reject. */ + failOpenReadOnCall?: number; /** What `compareCommits` reports for the run's commit against the last routed one, or an Error to reject. */ compare?: "behind" | "ahead" | "identical" | "diverged" | Error; }) { @@ -203,6 +209,8 @@ async function runRouting(options: { const updates: Array<{ issue_number: number; state?: string; body?: string }> = []; const comments: number[] = []; const compared: Array<{ base: string; head: string }> = []; + let openReads = 0; + let closedReads = 0; const github = { paginate: async () => { if (options.listingFails) throw new Error("Resource not accessible by integration"); @@ -211,9 +219,17 @@ async function runRouting(options: { rest: { actions: { listJobsForWorkflowRun: () => undefined }, issues: { - listForRepo: async ({ state }: { state: string }) => ({ - data: state === "open" ? (options.openIssues ?? []) : (options.closedIssues ?? []), - }), + listForRepo: async ({ state }: { state: string }) => { + if (state === "open") { + openReads += 1; + if (options.failOpenReadOnCall === openReads) throw new Error("API rate limit exceeded"); + const calls = options.openIssuesByCall; + return { data: calls ? calls[Math.min(openReads, calls.length) - 1]! : (options.openIssues ?? []) }; + } + closedReads += 1; + const calls = options.closedIssuesByCall; + return { data: calls ? calls[Math.min(closedReads, calls.length) - 1]! : (options.closedIssues ?? []) }; + }, create: async (request: { title: string; body: string }) => { created.push(request); return { data: { number: 900 } }; @@ -439,6 +455,59 @@ describe("main CI failure routing ignores superseded runs", () => { expect(result.comments).toEqual([42]); }); + it("writes to the issue state re-read just before writing, not the state read at the start", async () => { + // A newer green run closed #42 while this older-but-ahead red run was reading: the start sees + // #42 open, the re-check sees it closed. The red run must reopen the closed issue, not update + // the (now closed) one it first saw. + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssuesByCall: [[marked(ROUTED_SHA)], []], + closedIssuesByCall: [[marked(ROUTED_SHA)]], + compare: "ahead", + }); + expect(result.created).toHaveLength(0); + expect(result.updates).toHaveLength(1); + expect(result.updates[0]).toMatchObject({ issue_number: 42, state: "open" }); + expect(result.updates[0]!.body).toContain(stamp(RUN_SHA)); + }); + + it("closes the freshly opened issue on green when a red run reopened it while this run was reading", async () => { + // Start: nothing open, #42 closed. By the re-check an older red run reopened #42. This green + // run is ahead and must close it, not rewrite the closed-issue body and leave it open. + const result = await runRouting({ + results: green, + jobs: [], + openIssuesByCall: [[], [marked(ROUTED_SHA)]], + closedIssuesByCall: [[marked(ROUTED_SHA)]], + compare: "ahead", + }); + expect(result.closed).toEqual([42]); + }); + + it("stands down when the re-check shows a newer run routed while this one was reading", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssuesByCall: [[ownIssueWithoutStamp()], [marked(ROUTED_SHA)]], + compare: "behind", + }); + expect(result.updates).toHaveLength(0); + }); + + it("fails open when the first issue read works but the re-check read fails", async () => { + const result = await runRouting({ + results: red, + jobs: failingJobs, + openIssues: [marked(ROUTED_SHA)], + failOpenReadOnCall: 2, + compare: "ahead", + }); + expect(result.updates).toHaveLength(1); + expect(result.updates[0]!.body).toContain(stamp(RUN_SHA)); + expect(result.comments).toEqual([42]); + }); + it("treats a diverged history as not superseded", async () => { const result = await runRouting({ results: red,