diff --git a/scripts/github/pr-ci-sweeper.d.mts b/scripts/github/pr-ci-sweeper.d.mts index 6f4b97c3feb7..bd79f2b73746 100644 --- a/scripts/github/pr-ci-sweeper.d.mts +++ b/scripts/github/pr-ci-sweeper.d.mts @@ -20,8 +20,8 @@ export function classifyRunForRevive(params: { head_repository?: { full_name?: string }; }; prCreatedAt: string; - prHeadBranch?: string; - repoFullName?: string; + prHeadBranch: string; + repoFullName: string; }): { action: "revive" | "skip"; reason: string }; export function runPrCiSweeper(params: { github: Record; diff --git a/scripts/github/pr-ci-sweeper.mjs b/scripts/github/pr-ci-sweeper.mjs index 3316dbb95153..ef819c26314d 100644 --- a/scripts/github/pr-ci-sweeper.mjs +++ b/scripts/github/pr-ci-sweeper.mjs @@ -39,9 +39,6 @@ export function classifyPrForSweep({ pr, ciRuns, botCloseCount, now }) { if (pr.draft) { return { action: "skip", reason: "draft" }; } - if (now - Date.parse(pr.created_at) > LOOKBACK_MS) { - return { action: "skip", reason: "outside-lookback" }; - } if (now - Date.parse(pr.updated_at) < MIN_QUIET_MS) { return { action: "skip", reason: "recently-updated" }; } @@ -81,7 +78,12 @@ export function classifyRunForRevive({ run, prCreatedAt, prHeadBranch, repoFullN } // A head SHA can be reused by a later PR. Reruns replay the original event // context, so a run created before this PR existed cannot safely be revived. - if (Date.parse(run.created_at) < Date.parse(prCreatedAt)) { + const runCreatedAt = Date.parse(run.created_at); + const pullCreatedAt = Date.parse(prCreatedAt); + if (!Number.isFinite(runCreatedAt) || !Number.isFinite(pullCreatedAt)) { + return { action: "skip", reason: "unverifiable-created-at" }; + } + if (runCreatedAt < pullCreatedAt) { return { action: "skip", reason: "predates-pr" }; } if (run.run_attempt >= 3) { @@ -92,12 +94,12 @@ export function classifyRunForRevive({ run, prCreatedAt, prHeadBranch, repoFullN // triggering PR's branch. Same-repo branch names are unique, so requiring // branch + repo match ties the rerun to this PR's event context; fork-headed // or foreign-branch runs are refused rather than replayed on inference. - if (prHeadBranch !== undefined && run.head_branch !== prHeadBranch) { + if (!prHeadBranch || run.head_branch !== prHeadBranch) { return { action: "skip", reason: "different-head-branch" }; } // Absent metadata fails closed: an unverifiable head repository must never // default to "same repo" — that would replay fork-triggered privileged runs. - if (repoFullName !== undefined && run.head_repository?.full_name !== repoFullName) { + if (!repoFullName || run.head_repository?.full_name !== repoFullName) { return { action: "skip", reason: "fork-head-repository" }; } return { action: "revive", reason: "cancelled-pr-event-run" }; @@ -349,32 +351,6 @@ export async function runPrCiSweeper({ prHeadBranch: listed.head.ref, repoFullName: `${owner}/${repo}`, }); - if (verdict.action === "revive") { - workflowRunsById.set(runId, Promise.resolve(run)); - const supersession = await resolveWorkflowSupersession({ - github, - owner, - repo, - checks, - run, - prCreatedAt: listed.created_at, - prHeadBranch: listed.head.ref, - repoFullName: `${owner}/${repo}`, - workflowRunsById, - }); - if (supersession) { - core.info( - `pr-ci-sweeper: skip cancelled run ${runId} for #${listed.number} (${supersession})`, - ); - continue; - } - } - // Global suppression only once this run is actually handled: a - // PR-relative rejection (branch mismatch, predates-pr) must leave the - // run inspectable for the candidate that owns it. - if (verdict.action === "revive") { - seenRunIds.add(runId); - } if (verdict.action !== "revive") { if (verdict.reason === "revive-budget-exhausted") { core.info( @@ -387,6 +363,8 @@ export async function runPrCiSweeper({ core.info(`pr-ci-sweeper: per-sweep revive cap (${MAX_REVIVES_PER_SWEEP}) reached`); break reviveLane; } + let currentChecks = checks; + let currentRun = run; if (!dryRun) { // Revalidate immediately before mutating, mirroring the re-fire lane: // a fresh push, merge, close, or disarmed auto-merge in the scan gap @@ -411,7 +389,7 @@ export async function runPrCiSweeper({ // cancelled run would put stale checks back in flight (or cancel a // live replacement via workflow concurrency), so require the same // check to still be the head's latest cancelled entry. - const currentChecks = await listLatestChecksForHead({ + currentChecks = await listLatestChecksForHead({ github, owner, repo, @@ -423,23 +401,6 @@ export async function runPrCiSweeper({ ); continue; } - const supersession = await resolveWorkflowSupersession({ - github, - owner, - repo, - checks: currentChecks, - run, - prCreatedAt: listed.created_at, - prHeadBranch: listed.head.ref, - repoFullName: `${owner}/${repo}`, - workflowRunsById, - }); - if (supersession) { - core.info( - `pr-ci-sweeper: skip cancelled run ${runId} for #${listed.number} (${supersession})`, - ); - continue; - } // Revive only a quiescent head: any queued/in-progress Actions check // means a replacement may be underway, and rerunning now could cancel // it via workflow concurrency. Auto-merge waits for every check anyway, @@ -456,11 +417,12 @@ export async function runPrCiSweeper({ // A concurrent rerun can advance the same run id to a fresh attempt in // the scan gap; reclassify the current attempt so the budget and // cancelled-state guards judge what the rerun would actually replay. - const { data: currentRun } = await github.rest.actions.getWorkflowRun({ + const { data: freshRun } = await github.rest.actions.getWorkflowRun({ owner, repo, run_id: runId, }); + currentRun = freshRun; const currentVerdict = classifyRunForRevive({ run: currentRun, prCreatedAt: listed.created_at, @@ -474,6 +436,26 @@ export async function runPrCiSweeper({ continue; } } + workflowRunsById.set(runId, Promise.resolve(currentRun)); + const supersession = await resolveWorkflowSupersession({ + github, + owner, + repo, + checks: currentChecks, + run: currentRun, + prCreatedAt: listed.created_at, + prHeadBranch: listed.head.ref, + repoFullName: `${owner}/${repo}`, + workflowRunsById, + }); + if (supersession) { + core.info( + `pr-ci-sweeper: skip cancelled run ${runId} for #${listed.number} (${supersession})`, + ); + continue; + } + // Rejected or stale candidates must remain inspectable by their actual PR. + seenRunIds.add(runId); // Count only real (or dry-run-logged) revive attempts: stale candidates // rejected by revalidation must not exhaust the sweep-wide cap. revives += 1; diff --git a/test/scripts/pr-ci-sweeper.test.ts b/test/scripts/pr-ci-sweeper.test.ts index 7c248e2d05e8..68442d9a3bc0 100644 --- a/test/scripts/pr-ci-sweeper.test.ts +++ b/test/scripts/pr-ci-sweeper.test.ts @@ -23,108 +23,72 @@ function pr(overrides: Partial[0]["pr"]> = describe("classifyPrForSweep", () => { const cases: Array<{ name: string; - input: Parameters[0]; + prOverrides?: Partial[0]["pr"]>; + ciRuns?: Parameters[0]["ciRuns"]; + botCloseCount?: number; expected: ReturnType; }> = [ { name: "re-fires when no CI run attached", - input: { pr: pr(), ciRuns: [], botCloseCount: 0, now: NOW }, expected: { action: "refire", reason: "ci-run-missing" }, }, { name: "re-fires when only startup failures attached", - input: { - pr: pr(), - ciRuns: [{ conclusion: "startup_failure" }], - botCloseCount: 1, - now: NOW, - }, + ciRuns: [{ conclusion: "startup_failure" }], + botCloseCount: 1, expected: { action: "refire", reason: "ci-startup-failure" }, }, { name: "skips drafts", - input: { pr: pr({ draft: true }), ciRuns: [], botCloseCount: 0, now: NOW }, + prOverrides: { draft: true }, expected: { action: "skip", reason: "draft" }, }, - { - name: "skips PRs outside the 24h lookback", - input: { - pr: pr({ created_at: new Date(NOW - 25 * HOURS).toISOString() }), - ciRuns: [], - botCloseCount: 0, - now: NOW, - }, - expected: { action: "skip", reason: "outside-lookback" }, - }, { name: "skips recently updated PRs so merge-ref computation can settle", - input: { - pr: pr({ updated_at: new Date(NOW - 5 * MINUTES).toISOString() }), - ciRuns: [], - botCloseCount: 0, - now: NOW, - }, + prOverrides: { updated_at: new Date(NOW - 5 * MINUTES).toISOString() }, expected: { action: "skip", reason: "recently-updated" }, }, { name: "skips merge conflicts whose merge ref legitimately cannot exist", - input: { pr: pr({ mergeable: false }), ciRuns: [], botCloseCount: 0, now: NOW }, + prOverrides: { mergeable: false }, expected: { action: "skip", reason: "merge-conflict" }, }, { name: "skips PRs with auto-merge enabled (close would cancel it)", - input: { - pr: pr({ auto_merge: { merge_method: "squash" } }), - ciRuns: [], - botCloseCount: 0, - now: NOW, - }, + prOverrides: { auto_merge: { merge_method: "squash" } }, expected: { action: "skip", reason: "auto-merge-enabled" }, }, { name: "treats a completed run as attached", - input: { - pr: pr(), - ciRuns: [{ conclusion: "success" }], - botCloseCount: 0, - now: NOW, - }, + ciRuns: [{ conclusion: "success" }], expected: { action: "skip", reason: "ci-attached" }, }, { name: "treats a queued run (null conclusion) as attached", - input: { - pr: pr(), - ciRuns: [{ conclusion: null }, { conclusion: "startup_failure" }], - botCloseCount: 0, - now: NOW, - }, + ciRuns: [{ conclusion: null }, { conclusion: "startup_failure" }], expected: { action: "skip", reason: "ci-attached" }, }, { name: "treats a failed run as attached (rerunnable, not sweepable)", - input: { - pr: pr(), - ciRuns: [{ conclusion: "failure" }], - botCloseCount: 0, - now: NOW, - }, + ciRuns: [{ conclusion: "failure" }], expected: { action: "skip", reason: "ci-attached" }, }, { name: "stops after two bot closes", - input: { pr: pr(), ciRuns: [], botCloseCount: 2, now: NOW }, + botCloseCount: 2, expected: { action: "skip", reason: "refire-budget-exhausted" }, }, { name: "re-fires on unknown mergeability (stuck merge-ref IS the pathology)", - input: { pr: pr({ mergeable: null }), ciRuns: [], botCloseCount: 0, now: NOW }, + prOverrides: { mergeable: null }, expected: { action: "refire", reason: "ci-run-missing" }, }, ]; - it.each(cases)("$name", ({ input, expected }) => { - expect(classifyPrForSweep(input)).toEqual(expected); + it.each(cases)("$name", ({ prOverrides, ciRuns = [], botCloseCount = 0, expected }) => { + expect(classifyPrForSweep({ pr: pr(prOverrides), ciRuns, botCloseCount, now: NOW })).toEqual( + expected, + ); }); }); @@ -132,149 +96,104 @@ describe("classifyRunForRevive", () => { const prCreatedAt = new Date(NOW - 2 * HOURS).toISOString(); const cases: Array<{ name: string; - run: Parameters[0]["run"]; + runOverrides?: Partial[0]["run"]>; + pullCreatedAt?: string; + expectedHeadBranch?: string; + expectedRepoFullName?: string; expected: ReturnType; }> = [ { name: "revives a cancelled pull_request_target run", - run: { - conclusion: "cancelled", - event: "pull_request_target", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - }, + expected: { action: "revive", reason: "cancelled-pr-event-run" }, + }, + { + name: "revives a cancelled pull_request run", + runOverrides: { event: "pull_request" }, expected: { action: "revive", reason: "cancelled-pr-event-run" }, }, { name: "skips a run after two revives without progress", - run: { - conclusion: "cancelled", - event: "pull_request", - run_attempt: 3, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - }, + runOverrides: { event: "pull_request", run_attempt: 3 }, expected: { action: "skip", reason: "revive-budget-exhausted" }, }, { name: "skips a non-cancelled run", - run: { - conclusion: "success", - event: "pull_request_target", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - }, + runOverrides: { conclusion: "success" }, expected: { action: "skip", reason: "not-cancelled" }, }, { name: "skips a cancelled run from an unrelated event", - run: { - conclusion: "cancelled", - event: "workflow_dispatch", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - }, + runOverrides: { event: "workflow_dispatch" }, expected: { action: "skip", reason: "unsupported-event" }, }, + { + name: "skips a run triggered from a different head branch", + runOverrides: { head_branch: "some/other-branch" }, + expected: { action: "skip", reason: "different-head-branch" }, + }, + { + name: "skips a run with a null head branch", + runOverrides: { head_branch: null }, + expected: { action: "skip", reason: "different-head-branch" }, + }, + { + name: "fails closed when the expected and actual head branches are empty", + runOverrides: { head_branch: "" }, + expectedHeadBranch: "", + expected: { action: "skip", reason: "different-head-branch" }, + }, + { + name: "skips a run with no head repository metadata", + runOverrides: { head_repository: undefined }, + expected: { action: "skip", reason: "fork-head-repository" }, + }, + { + name: "skips a run whose head repository is a fork", + runOverrides: { head_repository: { full_name: "fork/openclaw" } }, + expected: { action: "skip", reason: "fork-head-repository" }, + }, + { + name: "fails closed when the expected and actual head repositories are empty", + runOverrides: { head_repository: { full_name: "" } }, + expectedRepoFullName: "", + expected: { action: "skip", reason: "fork-head-repository" }, + }, + { + name: "skips a run created before the current PR existed", + runOverrides: { created_at: new Date(NOW - 3 * HOURS).toISOString() }, + expected: { action: "skip", reason: "predates-pr" }, + }, + { + name: "fails closed when the workflow run creation time is invalid", + runOverrides: { created_at: "not-a-date" }, + expected: { action: "skip", reason: "unverifiable-created-at" }, + }, + { + name: "fails closed when the pull request creation time is invalid", + pullCreatedAt: "not-a-date", + expected: { action: "skip", reason: "unverifiable-created-at" }, + }, ]; - it.each(cases)("$name", ({ run, expected }) => { - expect( - classifyRunForRevive({ - run: { - head_branch: "automation/refresh", - head_repository: { full_name: "openclaw/openclaw" }, - ...run, - }, - prCreatedAt, - prHeadBranch: "automation/refresh", - repoFullName: "openclaw/openclaw", - }), - ).toEqual(expected); - }); - - it("skips a run triggered from a different head branch", () => { - expect( - classifyRunForRevive({ - run: { - conclusion: "cancelled", - event: "pull_request_target", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - head_branch: "some/other-branch", - }, - prCreatedAt, - prHeadBranch: "automation/refresh", - repoFullName: "openclaw/openclaw", - }), - ).toEqual({ action: "skip", reason: "different-head-branch" }); - }); - - it("skips a run with a null head branch", () => { - expect( - classifyRunForRevive({ - run: { - conclusion: "cancelled", - event: "pull_request", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - head_branch: null, - head_repository: { full_name: "openclaw/openclaw" }, - }, - prCreatedAt, - prHeadBranch: "automation/refresh", - repoFullName: "openclaw/openclaw", - }), - ).toEqual({ action: "skip", reason: "different-head-branch" }); - }); - - it("skips a run with no head repository metadata", () => { - expect( - classifyRunForRevive({ - run: { - conclusion: "cancelled", - event: "pull_request", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - head_branch: "automation/refresh", - }, - prCreatedAt, - prHeadBranch: "automation/refresh", - repoFullName: "openclaw/openclaw", - }), - ).toEqual({ action: "skip", reason: "fork-head-repository" }); - }); - - it("skips a run whose head repository is a fork", () => { - expect( - classifyRunForRevive({ - run: { - conclusion: "cancelled", - event: "pull_request", - run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), - head_branch: "automation/refresh", - head_repository: { full_name: "fork/openclaw" }, - }, - prCreatedAt, - prHeadBranch: "automation/refresh", - repoFullName: "openclaw/openclaw", - }), - ).toEqual({ action: "skip", reason: "fork-head-repository" }); - }); - - it("skips a run created before the current PR existed", () => { - expect( - classifyRunForRevive({ - run: { - conclusion: "cancelled", - event: "pull_request_target", - run_attempt: 1, - created_at: new Date(NOW - 3 * HOURS).toISOString(), - }, - prCreatedAt, - }), - ).toEqual({ action: "skip", reason: "predates-pr" }); - }); + it.each(cases)( + "$name", + ({ + runOverrides, + pullCreatedAt = prCreatedAt, + expectedHeadBranch = "automation/refresh", + expectedRepoFullName = "openclaw/openclaw", + expected, + }) => { + expect( + classifyRunForRevive({ + run: cancelledRun(1, runOverrides), + prCreatedAt: pullCreatedAt, + prHeadBranch: expectedHeadBranch, + repoFullName: expectedRepoFullName, + }), + ).toEqual(expected); + }, + ); }); type FakeCall = { method: string; args: Record }; @@ -326,13 +245,16 @@ function fakeGithub(options: { const pageSize = options.pageSize ?? Math.max(items.length, 1); const collected: unknown[] = []; let stopped = false; - for (let start = 0; start < items.length && !stopped; start += pageSize) { + for (let start = 0; start < items.length; start += pageSize) { record(`${endpoint.endpointName}.page`, { start }); collected.push( ...mapFn({ data: items.slice(start, start + pageSize) }, () => { stopped = true; }), ); + if (stopped) { + break; + } } return Promise.resolve(collected); }; @@ -341,9 +263,10 @@ function fakeGithub(options: { } if (endpoint.endpointName === "actions.listWorkflowRuns") { return Promise.resolve( - (options.runsBySha[args.head_sha as string] ?? []) - .map((run) => ({ ...run, event: run.event ?? "pull_request" })) - .filter((run) => !args.event || run.event === args.event), + Array.from(options.runsBySha[args.head_sha as string] ?? [], (run) => ({ + ...run, + event: run.event ?? "pull_request", + })).filter((run) => !args.event || run.event === args.event), ); } if (endpoint.endpointName === "checks.listForRef") { @@ -452,7 +375,7 @@ function cancelledRun(runId: number, overrides: Partial = {}): conclusion: "cancelled", event: "pull_request_target", run_attempt: 1, - created_at: new Date(NOW - 1 * HOURS).toISOString(), + created_at: new Date(NOW - HOURS).toISOString(), head_branch: "automation/refresh", head_repository: { full_name: "openclaw/openclaw" }, ...overrides,