fix(ci): stop reviving superseded pull request workflows (#115378)

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Peter Steinberger
2026-07-28 11:56:19 -07:00
committed by GitHub
parent 8799a97efc
commit 723423ffe7
2 changed files with 471 additions and 10 deletions

View File

@@ -142,8 +142,8 @@ async function listLatestChecksForHead({ github, owner, repo, headSha }) {
// Checks are the PR association GitHub's merge box and auto-merge actually
// wait on. pull_request_target workflow runs can have a base-side head_sha
// and an empty pull_requests array, so neither run field identifies the PR.
// filter=latest also omits cancelled checks superseded by a newer same-name
// check, leaving only cancelled work that still blocks this head.
// filter=latest is scoped to a check suite, not a workflow: cancelled checks
// from older runs may coexist with their completed replacements on this head.
// Accepted tradeoff: checks are commit-scoped, so a second PR sharing this
// exact head (a duplicate PR off the same automation branch) could have its
// run revived under our candidate's eligibility. GitHub exposes no trigger
@@ -159,12 +159,82 @@ async function listLatestChecksForHead({ github, owner, repo, headSha }) {
});
}
function workflowRunIdForCheck(check) {
if (check.conclusion !== "cancelled" || check.app?.slug !== "github-actions") {
function githubActionsWorkflowRunIdForCheck(check) {
if (check.app?.slug !== "github-actions") {
return undefined;
}
const match = check.details_url?.match(/\/actions\/runs\/(\d+)(?:\/|$)/);
return match ? Number(match[1]) : undefined;
if (!match) {
return undefined;
}
const runId = Number(match[1]);
return Number.isSafeInteger(runId) && runId > 0 ? runId : undefined;
}
function workflowRunIdForCheck(check) {
if (check.conclusion !== "cancelled") {
return undefined;
}
return githubActionsWorkflowRunIdForCheck(check);
}
async function resolveWorkflowSupersession({
github,
owner,
repo,
checks,
run,
prCreatedAt,
prHeadBranch,
repoFullName,
workflowRunsById,
}) {
if (!Number.isSafeInteger(run.workflow_id) || run.workflow_id <= 0) {
return "unverifiable-workflow";
}
for (const check of checks) {
if (check.app?.slug !== "github-actions") {
continue;
}
const replacementRunId = githubActionsWorkflowRunIdForCheck(check);
if (replacementRunId === undefined) {
return "unverifiable-workflow";
}
if (replacementRunId <= run.id) {
continue;
}
let replacementRun = workflowRunsById.get(replacementRunId);
if (!replacementRun) {
// A run's owning workflow is immutable, so cache only this exact run-id
// lookup; current cancellation/attempt state is still fetched fresh.
replacementRun = github.rest.actions
.getWorkflowRun({ owner, repo, run_id: replacementRunId })
.then(({ data }) => data);
workflowRunsById.set(replacementRunId, replacementRun);
}
const replacement = await replacementRun;
if (!Number.isSafeInteger(replacement?.workflow_id) || replacement.workflow_id <= 0) {
return "unverifiable-workflow";
}
// Workflow identity alone is shared by dispatches, pushes, and other PRs.
// Match the candidate's trusted PR-event lineage, not head_sha: target
// workflows can report their base SHA rather than the checked PR head.
if (
replacement.workflow_id === run.workflow_id &&
REVIVABLE_EVENTS.has(replacement.event) &&
replacement.event === run.event &&
replacement.head_branch === prHeadBranch &&
replacement.head_repository?.full_name === repoFullName &&
Number.isFinite(Date.parse(replacement.created_at)) &&
Date.parse(replacement.created_at) >= Date.parse(prCreatedAt)
) {
return "superseded-workflow";
}
}
return null;
}
function isExpectedReviveSkip(error) {
@@ -245,6 +315,7 @@ export async function runPrCiSweeper({
let revives = 0;
const openPrs = await listRecentOpenPrs({ github, owner, repo, now });
const seenRunIds = new Set();
const workflowRunsById = new Map();
reviveLane: for (const listed of openPrs) {
if (now - Date.parse(listed.created_at) > LOOKBACK_MS) {
break;
@@ -278,6 +349,26 @@ 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.
@@ -332,6 +423,23 @@ 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,

View File

@@ -278,12 +278,17 @@ describe("classifyRunForRevive", () => {
});
type FakeCall = { method: string; args: Record<string, unknown> };
type FakeWorkflowRun = Parameters<typeof classifyRunForRevive>[0]["run"] & { id: number };
type FakeWorkflowRun = Parameters<typeof classifyRunForRevive>[0]["run"] & {
id: number;
workflow_id: number | null;
};
type FakeCheckRun = {
id: number;
name: string;
status?: string;
conclusion: string | null;
app: { slug: string } | null;
details_url: string | null;
details_url: string | null | undefined;
};
function fakeGithub(options: {
@@ -292,14 +297,16 @@ function fakeGithub(options: {
string,
Array<{ conclusion: string | null; event?: string; id?: number; status?: string }>
>;
checksByRef?: Record<string, FakeCheckRun[]>;
checksByRef?: Record<string, FakeCheckRun[] | FakeCheckRun[][]>;
workflowRunsById?: Record<number, FakeWorkflowRun>;
workflowRunErrorsById?: Record<number, Error>;
pullsGetByNumber?: Record<number, Record<string, unknown> | Array<Record<string, unknown>>>;
events?: Array<Record<string, unknown>>;
pageSize?: number;
}) {
const calls: FakeCall[] = [];
const pullsGetCallCounts = new Map<number, number>();
const checksListCallCounts = new Map<string, number>();
const record = (method: string, args: Record<string, unknown>) => {
calls.push({ method, args });
};
@@ -340,7 +347,15 @@ function fakeGithub(options: {
);
}
if (endpoint.endpointName === "checks.listForRef") {
return Promise.resolve(options.checksByRef?.[args.ref as string] ?? []);
const ref = args.ref as string;
const configured = options.checksByRef?.[ref] ?? [];
if (Array.isArray(configured[0])) {
const snapshots = configured as FakeCheckRun[][];
const callIndex = checksListCallCounts.get(ref) ?? 0;
checksListCallCounts.set(ref, callIndex + 1);
return Promise.resolve(snapshots[Math.min(callIndex, snapshots.length - 1)] ?? []);
}
return Promise.resolve(configured as FakeCheckRun[]);
}
if (endpoint.endpointName === "issues.listEvents") {
return Promise.resolve(options.events ?? []);
@@ -370,7 +385,12 @@ function fakeGithub(options: {
listWorkflowRuns: { endpointName: "actions.listWorkflowRuns" },
getWorkflowRun: (args: Record<string, unknown>) => {
record("actions.getWorkflowRun", args);
return Promise.resolve({ data: options.workflowRunsById?.[args.run_id as number] });
const runId = args.run_id as number;
const error = options.workflowRunErrorsById?.[runId];
if (error) {
return Promise.reject(error);
}
return Promise.resolve({ data: options.workflowRunsById?.[runId] });
},
reRunWorkflow: (args: Record<string, unknown>) => {
record("actions.reRunWorkflow", args);
@@ -415,6 +435,8 @@ function autoMergePr(number: number, headSha: string) {
function githubActionsCheck(runId: number, overrides: Partial<FakeCheckRun> = {}): FakeCheckRun {
return {
id: runId,
name: "proof",
conclusion: "cancelled",
status: "completed",
app: { slug: "github-actions" },
@@ -426,6 +448,7 @@ function githubActionsCheck(runId: number, overrides: Partial<FakeCheckRun> = {}
function cancelledRun(runId: number, overrides: Partial<FakeWorkflowRun> = {}): FakeWorkflowRun {
return {
id: runId,
workflow_id: 10,
conclusion: "cancelled",
event: "pull_request_target",
run_attempt: 1,
@@ -696,6 +719,336 @@ describe("runPrCiSweeper", () => {
expect(calls.filter((call) => call.method === "pulls.update")).toEqual([]);
});
it.each([
{
name: "same-name successful",
replacementName: "proof",
status: "completed",
conclusion: "success",
},
{
name: "queued",
replacementName: "proof",
status: "queued",
conclusion: null,
},
{
name: "in-progress",
replacementName: "proof",
status: "in_progress",
conclusion: null,
},
{
name: "renamed",
replacementName: "renamed proof",
status: "completed",
conclusion: "success",
},
])(
"does not revive a cancelled run superseded by a $name workflow run",
async ({ replacementName, status, conclusion }) => {
const generated = autoMergePr(40, "4".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { name: replacementName, status, conclusion }),
],
},
workflowRunsById: {
100: cancelledRun(100),
200: cancelledRun(200, { conclusion }),
},
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
},
);
it("does not let a newer run from another workflow suppress a cancelled run", async () => {
const generated = autoMergePr(41, "5".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { conclusion: "success" }),
],
},
workflowRunsById: {
100: cancelledRun(100),
200: cancelledRun(200, { workflow_id: 20, conclusion: "success" }),
},
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([
{
method: "actions.reRunWorkflow",
args: { owner: "openclaw", repo: "openclaw", run_id: 100 },
},
]);
expect(
calls.filter((call) => call.method === "actions.getWorkflowRun" && call.args.run_id === 200),
).toHaveLength(1);
});
it.each([
{ name: "manual dispatch", replacement: { event: "workflow_dispatch" } },
{ name: "push event", replacement: { event: "push" } },
{ name: "different pull-request event", replacement: { event: "pull_request" } },
{ name: "different head branch", replacement: { head_branch: "automation/another-pr" } },
{ name: "missing head branch", replacement: { head_branch: null } },
{
name: "fork head repository",
replacement: { head_repository: { full_name: "someone-else/openclaw" } },
},
{ name: "missing head repository", replacement: { head_repository: undefined } },
{
name: "run predating the pull request",
replacement: { created_at: new Date(NOW - 3 * HOURS).toISOString() },
},
])(
"does not let a newer $name from the same workflow suppress the PR run",
async ({ replacement }) => {
const generated = autoMergePr(48, "c".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { conclusion: "success" }),
],
},
workflowRunsById: {
100: cancelledRun(100),
200: cancelledRun(200, { conclusion: "success", ...replacement }),
},
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([
{
method: "actions.reRunWorkflow",
args: { owner: "openclaw", repo: "openclaw", run_id: 100 },
},
]);
expect(
calls.filter(
(call) => call.method === "actions.getWorkflowRun" && call.args.run_id === 200,
),
).toHaveLength(1);
},
);
it.each([
{ name: "successful", status: "completed", conclusion: "success" },
{ name: "queued", status: "queued", conclusion: null },
{ name: "in-progress", status: "in_progress", conclusion: null },
])("does not report a $name replacement as a dry-run revive", async ({ status, conclusion }) => {
const generated = autoMergePr(42, "6".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { status, conclusion }),
],
},
workflowRunsById: {
100: cancelledRun(100),
200: cancelledRun(200, { conclusion }),
},
});
const { core: loggedCore, logs } = recordingCore();
await runPrCiSweeper({
github: github as never,
context: context as never,
core: loggedCore as never,
dryRun: true,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
expect(logs.some((line) => line.includes("would revive cancelled run 100"))).toBe(false);
});
it("does not revive a run superseded during pre-mutation revalidation", async () => {
const generated = autoMergePr(43, "7".repeat(40));
const cancelled = githubActionsCheck(100);
const replacement = githubActionsCheck(200, { conclusion: "success" });
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [[cancelled], [cancelled, replacement]],
},
workflowRunsById: {
100: cancelledRun(100),
200: cancelledRun(200, { conclusion: "success" }),
},
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "checks.listForRef")).toHaveLength(2);
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
});
it.each([
{ name: "cancelled run", cancelledWorkflowId: null, replacementWorkflowId: 10 },
{ name: "replacement run", cancelledWorkflowId: 10, replacementWorkflowId: null },
])(
"does not revive when the $name has no verifiable workflow identity",
async ({ cancelledWorkflowId, replacementWorkflowId }) => {
const generated = autoMergePr(44, "8".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { conclusion: "success" }),
],
},
workflowRunsById: {
100: cancelledRun(100, { workflow_id: cancelledWorkflowId }),
200: cancelledRun(200, {
workflow_id: replacementWorkflowId,
conclusion: "success",
}),
},
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
},
);
it.each([
{ name: "missing", detailsUrl: null },
{ name: "undefined", detailsUrl: undefined },
{ name: "malformed", detailsUrl: "https://github.com/openclaw/openclaw/actions/runs/nope" },
])("does not revive when an Actions replacement has a $name run URL", async ({ detailsUrl }) => {
const generated = autoMergePr(46, "a".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { conclusion: "success", details_url: detailsUrl }),
],
},
workflowRunsById: { 100: cancelledRun(100) },
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
});
it("does not let a malformed non-Actions check suppress a cancelled workflow", async () => {
const generated = autoMergePr(47, "b".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, {
app: { slug: "external-ci" },
conclusion: "success",
details_url: null,
}),
],
},
workflowRunsById: { 100: cancelledRun(100) },
});
await runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
});
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([
{
method: "actions.reRunWorkflow",
args: { owner: "openclaw", repo: "openclaw", run_id: 100 },
},
]);
});
it("fails closed when replacement workflow metadata cannot be loaded", async () => {
const generated = autoMergePr(45, "9".repeat(40));
const { github, calls } = fakeGithub({
prs: [generated],
runsBySha: {},
checksByRef: {
[generated.head.sha]: [
githubActionsCheck(100),
githubActionsCheck(200, { conclusion: "success" }),
],
},
workflowRunsById: { 100: cancelledRun(100) },
workflowRunErrorsById: { 200: new Error("replacement workflow unavailable") },
});
await expect(
runPrCiSweeper({
github: github as never,
context: context as never,
core: core as never,
now: NOW,
}),
).rejects.toThrow("replacement workflow unavailable");
expect(calls.filter((call) => call.method === "actions.reRunWorkflow")).toEqual([]);
});
it("does not revive a run triggered from a different branch on a shared commit", async () => {
const generated = autoMergePr(14, "f".repeat(40));
const { github, calls } = fakeGithub({