diff --git a/.changes/pr-pipeline/fix-reconcile-list-lost-by-status-2960f534.md b/.changes/pr-pipeline/fix-reconcile-list-lost-by-status-2960f534.md new file mode 100644 index 0000000000..9c7901ddc6 --- /dev/null +++ b/.changes/pr-pipeline/fix-reconcile-list-lost-by-status-2960f534.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The lost-run reconciler lists pr-verify runs once per lost conclusion and per not-yet-completed status instead of all runs of the day, so a day with more than 500 runs no longer fails the page cap. diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs index 6251d9dd4a..782e5ea2df 100755 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs @@ -72,9 +72,14 @@ export const MAX_IN_FLIGHT = 3; export const WRITE_GAP_MS = 1000; // An outage loss is re-run within a day; an older one is left to a person (judgment). export const LOOKBACK_HOURS = 24; -// Pages of 100 read from a lane's run listing; a day of pr-verify runs fits in -// them, and a listing that does not is a failure, never a partial read. +// Pages of 100 read per status query of a lane's run listing; a listing that +// does not fit is a failure, never a partial read. export const LANE_RUN_PAGES = 5; +// The lane listing is read once per status, so the many succeeded runs are +// never listed: the lost conclusions, and every status short of completed (the +// runs a re-run may have put in flight). Earlier statuses are read first, so a +// run that starts between two queries is still listed by the later one. +export const LANE_LIST_STATUSES = [...LOST, "requested", "pending", "waiting", "queued", "in_progress"]; class RateLimited extends Error {} @@ -179,17 +184,24 @@ async function plan({ repo, call, say, since }) { for (const lanePath of RECORDED_LANES) { try { const file = lanePath.slice(lanePath.lastIndexOf("/") + 1); - const listed = await paginate( - async (...args) => { - const page = await call(...args); - if (!Array.isArray(page?.workflow_runs)) { - throw new Error("its run listing is not a page of runs"); - } - return page.workflow_runs; - }, - `/repos/${repo}/actions/workflows/${file}/runs?created=${created}`, - LANE_RUN_PAGES, - ); + const byId = new Map(); + for (const status of LANE_LIST_STATUSES) { + const listed = await paginate( + async (...args) => { + const page = await call(...args); + if (!Array.isArray(page?.workflow_runs)) { + throw new Error("its run listing is not a page of runs"); + } + return page.workflow_runs; + }, + `/repos/${repo}/actions/workflows/${file}/runs?created=${created}&status=${status}`, + LANE_RUN_PAGES, + ); + for (const r of listed) { + byId.set(r.id, r); + } + } + const listed = [...byId.values()]; const runs = listed.filter((r) => r.path === lanePath); inFlight += runs.filter((r) => r.status !== "completed" && r.run_attempt > 1).length; const unpublished = []; diff --git a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs index f88730e714..4b2bb54467 100644 --- a/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs +++ b/.github/scripts/maintenance-reconcile-lost-runs/reconcile.test.mjs @@ -76,10 +76,13 @@ function fakeGitHub({ pulls, runs = [], fail = {} }) { const found = runs.filter((r) => r.head_sha === listing[1]); return { total_count: found.length, workflow_runs: found }; } - const byWorkflow = /^\/repos\/[^/]+\/[^/]+\/actions\/workflows\/([a-z0-9-]+\.yml)\/runs\?created=%3E%3D2026-10-18T20%3A00%3A00Z&per_page=100(?:&page=([0-9]+))?$/.exec(path); + const byWorkflow = /^\/repos\/[^/]+\/[^/]+\/actions\/workflows\/([a-z0-9-]+\.yml)\/runs\?created=%3E%3D2026-10-18T20%3A00%3A00Z(?:&status=([a-z_]+))?&per_page=100(?:&page=([0-9]+))?$/.exec(path); if (method === "GET" && byWorkflow) { - const found = runs.filter((r) => r.path === `.github/workflows/${byWorkflow[1]}`); - const page = Number(byWorkflow[2] ?? 1); + // Like GitHub, `status` matches a run's status or its conclusion. + const found = runs.filter( + (r) => r.path === `.github/workflows/${byWorkflow[1]}` && (byWorkflow[2] === undefined || r.status === byWorkflow[2] || r.conclusion === byWorkflow[2]), + ); + const page = Number(byWorkflow[3] ?? 1); return { total_count: found.length, workflow_runs: found.slice((page - 1) * 100, page * 100) }; } const jobs = /^\/repos\/[^/]+\/[^/]+\/actions\/runs\/([0-9]+)\/attempts\/1\/jobs\?per_page=100$/.exec(path); @@ -454,18 +457,65 @@ test("a non-operational Actions status records nothing", async () => { const verifyDay = (count, lost) => Array.from({ length: count }, (_, i) => lost.get(i) ?? { ...run(10_000 + i, MAIN_TIP, { path: VERIFY, event: "workflow_run", conclusion: "success" }), head_branch: "main" }); -test("a lane listing of 150 runs over 2 pages finds a lost run on page 2", async () => { - const lost = verifyRun(201); - const { code, reruns, calls, outputs } = await reconcile({ pulls: [], runs: verifyDay(150, new Map([[120, lost]])) }); +// Lost pr-verify runs, oldest first. +const lostDay = (count) => + Array.from({ length: count }, (_, i) => verifyRun(1000 + i, { created: new Date(Date.parse("2026-10-19T00:00:00Z") + i * 60_000).toISOString().replace(/\.\d{3}Z$/, "Z") })); +const lostQueries = (calls) => calls.filter((c) => c.includes("/workflows/pr-verify.yml/runs?") && c.includes("&status=cancelled&")); + +test("a lost-run listing of 150 runs over 2 pages is read in full", async () => { + const { code, reruns, calls } = await reconcile({ pulls: [], runs: lostDay(150) }); + assert.equal(code, 0); + assert.deepEqual(reruns, [1000, 1001, 1002]); + assert.equal(lostQueries(calls).length, 2); +}); + +test("700 succeeded verify runs and 2 lost ones plan the 2 lost runs without hitting the page cap", async () => { + const { code, reruns, outputs, calls } = await reconcile({ + pulls: [], + runs: [...verifyDay(700, new Map()), verifyRun(201), verifyRun(202, { created: "2026-10-19T19:00:00Z" })], + }); + assert.equal(code, 0); + assert.deepEqual(reruns, [201, 202]); + assert.deepEqual(records(outputs), [`reconcile-rerun-201-2-${MAIN_TIP}`, `reconcile-rerun-202-2-${MAIN_TIP}`, ""]); + assert.ok(calls.every((c) => !c.includes("&page=2"))); +}); + +test("with 700 succeeded runs listed, verify re-runs already queued or in progress still take places", async () => { + const { code, reruns } = await reconcile({ + pulls: [], + runs: [ + ...verifyDay(700, new Map()), + verifyRun(209, { status: "in_progress", attempt: 2 }), + verifyRun(210, { status: "queued", attempt: 2 }), + verifyRun(201), + verifyRun(202, { created: "2026-10-19T19:00:00Z" }), + ], + }); assert.equal(code, 0); assert.deepEqual(reruns, [201]); - assert.deepEqual(records(outputs).slice(0, 1), [`reconcile-rerun-201-2-${MAIN_TIP}`]); - assert.equal(calls.filter((c) => c.includes("/workflows/pr-verify.yml/runs?")).length, 2); +}); + +test("verify re-runs that start while the listing is read still take places", async () => { + const queued = [209, 210, 211].map((id) => verifyRun(id, { status: "queued", attempt: 2 })); + const { code, reruns } = await reconcile({ + pulls: [], + runs: [...queued, verifyRun(201)], + github: (g) => async (method, path, body) => { + if (path.includes("/workflows/pr-verify.yml/runs?") && path.includes("&status=queued&")) { + for (const r of queued) { + r.status = "in_progress"; + } + } + return g(method, path, body); + }, + }); + assert.equal(code, 0); + assert.deepEqual(reruns, []); }); test("a lane listing past the page cap is a failure, and nothing is called lost", async (t) => { const log = t.mock.method(console, "log", () => {}); - const { code, reruns } = await reconcile({ pulls: [], runs: verifyDay(501, new Map([[0, verifyRun(201)]])) }); + const { code, reruns } = await reconcile({ pulls: [], runs: lostDay(501) }); const lines = log.mock.calls.map((c) => String(c.arguments[0])); assert.equal(code, 1); assert.deepEqual(reruns, []); @@ -476,7 +526,7 @@ test("a lane listing past the page cap is a failure, and nothing is called lost" test("act says why it exits 1 when the plan recorded listing failures", async (t) => { const log = t.mock.method(console, "log", () => {}); - const { code } = await reconcile({ pulls: [], runs: verifyDay(501, new Map()) }); + const { code } = await reconcile({ pulls: [], runs: lostDay(501) }); const errors = log.mock.calls.map((c) => String(c.arguments[0])).filter((l) => l.startsWith("::error title=reconcile::the plan step")); assert.equal(code, 1); assert.deepEqual(errors, ["::error title=reconcile::the plan step recorded 1 listing failures and 0 re-runs failed; see the Find lost runs step"]);