vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-02 10:01:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ci): simplify fail-closed workflow revival (#115420)

Browse Source
This commit is contained in:
Peter Steinberger
2026-07-28 17:47:48 -04:00
committed by GitHub
parent ed62e2d0bb
commit bd33a1c45f
3 changed files with 140 additions and 235 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
scripts/github/pr-ci-sweeper.d.mts
View File

@@ -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<string, unknown>;

84
scripts/github/pr-ci-sweeper.mjs
View File

@@ -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;

287
test/scripts/pr-ci-sweeper.test.ts
View File

@@ -23,108 +23,72 @@ function pr(overrides: Partial<Parameters<typeof classifyPrForSweep>[0]["pr"]> =
describe("classifyPrForSweep", () => {
const cases: Array<{
name: string;
input: Parameters<typeof classifyPrForSweep>[0];
prOverrides?: Partial<Parameters<typeof classifyPrForSweep>[0]["pr"]>;
ciRuns?: Parameters<typeof classifyPrForSweep>[0]["ciRuns"];
botCloseCount?: number;
expected: ReturnType<typeof classifyPrForSweep>;
}> = [
{
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<typeof classifyRunForRevive>[0]["run"];
runOverrides?: Partial<Parameters<typeof classifyRunForRevive>[0]["run"]>;
pullCreatedAt?: string;
expectedHeadBranch?: string;
expectedRepoFullName?: string;
expected: ReturnType<typeof classifyRunForRevive>;
}> = [
{
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<string, unknown> };
@@ -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<FakeWorkflowRun> = {}):
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,