Diffinery

codician/diffinerypublic

Open-source, agent-first GitHub and GitHub Actions alternative that runs on your own Cloudflare account: git hosting, pull requests, code review and drop-in GitHub Actions YAML on Workers, Durable Objects, R2 and Containers.

Merge pull request #252 from claude/cancel-runs-cleanup

A cancelled job runs its always() and cancelled() steps before it ends, like GitHub
cnbrkdmrci authored 2 parents 661e3099466de6commit 7bde4a51b5fa2d01cc57d1e3ba47f961d956d2b4Browse files
9 files changed+338−48
Files (9)
modifiedapps/diffinery/docs/actions/expressions.md+3−1View file
@@ -145,10 +145,12 @@
145145 | Function | True when |
146146 | --- | --- |
147147 | `success()` | No earlier step of the job failed (steps with `continue-on-error: true` don't count) and the job isn't being cancelled or timing out. |
148-| `failure()` | An earlier step failed. |
148+| `failure()` | An earlier step failed, and the job isn't being cancelled or timing out. |
149149 | `cancelled()` | The job is being cancelled or has hit its timeout. |
150150 | `always()` | Always. |
151151
152+When a job on a Diffinery-hosted runner is cancelled or hits its `timeout-minutes`, the step that is running stops, and then, as on GitHub, the remaining steps whose `if:` is true still run: `if: always()`, `if: cancelled()` and post steps such as the cache save with `save-always`. They have 5 minutes together. Use them to clean up what the job created, for example a test environment or a temporary repository. Cancelling the run a second time stops them at once. The job still ends as `cancelled` (or failed, after a timeout).
153+
152154 An `if:` that doesn't call any of these is treated as `success() && (<condition>)`. So `if: github.ref == 'refs/heads/main'` is skipped after a failure, while `if: always() && github.ref == 'refs/heads/main'` still runs. Inside a composite action, the functions look at the action's own steps.
153155
154156 ```yaml
modifiedapps/diffinery/docs/actions/overview.md+1−1View file
@@ -164,7 +164,7 @@
164164 - The run's **Status** and duration, what **Triggered** it and who, the **Commit** and ref (and pull request), and the **Workflow file** at that commit.
165165 - One box per job id, listing the jobs it produced (a matrix job says how many, for example `4 jobs (matrix)`), each with its status, duration and, for a failed job, the first line of the failure reason.
166166 - **Artifacts**, when jobs uploaded any that haven't expired, with their size and age. Click a name to download it: a `.zip` file, or the file itself for an artifact uploaded with `archive: false`. See [Caching and artifacts](/docs/actions/caching-and-artifacts).
167-- **Cancel run** while the run is not completed (you confirm with **Cancel this run?**), and **Re-run all jobs** once it has completed. Both need write access.
167+- **Cancel run** while the run is not completed (you confirm with **Cancel this run?**), and **Re-run all jobs** once it has completed. Both need write access. A cancelled job on a Diffinery-hosted runner first runs its `if: always()` and `if: cancelled()` steps, for up to 5 minutes (see [Status check functions](/docs/actions/expressions#status-check-functions)); cancelling again stops them.
168168
169169 ### Job page and logs
170170
modifiedapps/diffinery/src/repo/actions.ts+53−9View file
@@ -46,11 +46,19 @@
4646 /**
4747 * How long past its timeout-minutes (from when its runner fetched it) a running job may go without
4848 * reporting before Diffinery ends it itself. The runner stops a job at its timeout and reports within
49- * seconds (it cancels the steps and skips post steps), so this only ends jobs whose runner agent froze
50- * or vanished, which would otherwise hold their container, quota slot and minutes for up to 6 hours.
49+ * seconds, or within CANCEL_GRACE_MS when steps run after the timeout (`if: always()`), so this only ends
50+ * jobs whose runner agent froze or vanished, which would otherwise hold their container, quota slot and
51+ * minutes for up to 6 hours.
5152 */
5253 export const JOB_DEADLINE_GRACE_MS = 10 * 60_000;
5354 /**
55+ * A cancelled hosted job's runner stops the step under way and then, like GitHub's, runs the steps that
56+ * run after a cancel (`if: always()`, `if: cancelled()`, post steps) for up to 5 minutes, so cleanup such
57+ * as deleting what the job created still happens. Diffinery waits this long for it to report (its 5
58+ * minutes, and time for the logs and the result), then stops its container and ends the job itself.
59+ */
60+export const CANCEL_STOP_MS = 7 * 60_000;
61+/**
5462 * A run finishes when its workflow (runner/workflow.ts) has had every job's completion event. An event
5563 * lost while the platform updates (completeJob's sendEvent failing during a deploy) leaves the workflow
5664 * waiting up to 6 hours, and a workflow instance that failed never finishes the run: either way the run
@@ -261,6 +269,9 @@
261269 runner_info: string | null;
262270 /** Set on a job a partial re-run copied from an earlier attempt: the job that actually ran. */
263271 carried_from?: string | null;
272+ /** When a running hosted job was cancelled (ms since the epoch) and why, while its runner finishes (CANCEL_STOP_MS). */
273+ cancel_at?: number | null;
274+ cancel_reason?: string | null;
264275 restarts: number;
265276 /** Hosted runner size runs-on asked for (runner/sizes.ts); null when it is chosen from the job's earlier runs. */
266277 runner_size?: string | null;
@@ -1751,6 +1762,10 @@
17511762 await this.completeJob(jobId, { conclusion: "failure", outputs: {}, failure: { message: `The job exceeded its timeout of ${j.timeout_minutes} minutes.` } }, "timed_out");
17521763 }
17531764
1765+ private earlierDeadline(a: number | null, b: number | null): number | null {
1766+ return a === null ? b : b === null ? a : Math.min(a, b);
1767+ }
1768+
17541769 /** When a running job's runner has had its timeout and JOB_DEADLINE_GRACE_MS to report, in ms since the epoch. */
17551770 private jobDeadline(j: Pick<JobRow, "started_at" | "timeout_minutes">): number | null {
17561771 const from = Date.parse(j.started_at ?? "");
@@ -1762,8 +1777,8 @@
17621777 /** The earliest deadline of a running job (RepoDO's alarm wakes up for it), or null. */
17631778 nextJobDeadline(): number | null {
17641779 let next: number | null = null;
1765- for (const j of this.sql.exec<JobRow>("SELECT started_at, timeout_minutes FROM jobs WHERE status = 'in_progress' AND started_at IS NOT NULL").toArray()) {
1766- const d = this.jobDeadline(j);
1780+ for (const j of this.sql.exec<JobRow>("SELECT started_at, timeout_minutes, cancel_at FROM jobs WHERE status = 'in_progress' AND started_at IS NOT NULL").toArray()) {
1781+ const d = this.earlierDeadline(this.jobDeadline(j), j.cancel_at ? j.cancel_at + CANCEL_STOP_MS : null);
17671782 if (d !== null && (next === null || d < next)) next = d;
17681783 }
17691784 return next;
@@ -1851,6 +1866,12 @@
18511866
18521867 async enforceJobDeadlines(at = Date.now()): Promise<void> {
18531868 for (const j of this.sql.exec<JobRow>("SELECT * FROM jobs WHERE status = 'in_progress' AND started_at IS NOT NULL ORDER BY rowid").toArray()) {
1869+ if (j.cancel_at && j.cancel_at + CANCEL_STOP_MS <= at) {
1870+ await this.stopContainer(j);
1871+ const message = `${j.cancel_reason ?? "Cancelled"}. Its runner didn't report within ${CANCEL_STOP_MS / 60_000} minutes of the cancel, so Diffinery stopped it.`;
1872+ await this.completeJob(j.id, { conclusion: "cancelled", outputs: {}, failure: { message } });
1873+ continue;
1874+ }
18541875 const d = this.jobDeadline(j);
18551876 if (d === null || d > at) continue;
18561877 const minutes = Number(j.timeout_minutes) > 0 ? Number(j.timeout_minutes) : DEFAULT_TIMEOUT_MINUTES;
@@ -1868,6 +1889,8 @@
18681889 if (!j || j.status === "completed") return;
18691890 // A container the job no longer runs in (it was replaced, and the job restarted in another).
18701891 if (container && j.container !== container) return;
1892+ // Cancelled, and its container stopped before the runner reported: the job ends as cancelled.
1893+ if (j.cancel_at) return this.completeJob(jobId, { conclusion: "cancelled", outputs: {}, failure: { message: j.cancel_reason ?? "Cancelled" } });
18711894 // Already back in line (placementFailed): a container given up on, which started after all,
18721895 // finds its token revoked and exits. The job's next placement starts a new one.
18731896 if (j.status === "queued") return;
@@ -1975,11 +1998,15 @@
19751998 if (!this.sql.exec("DELETE FROM runners WHERE id = ? RETURNING id", id).toArray().length) throw new HttpError(404, "runner not found");
19761999 }
19772000
1978- private async stopContainer(j: JobRow): Promise<void> {
2001+ /**
2002+ * Stops a hosted job's container, for a job Diffinery ends itself. `graceful` instead asks its runner to
2003+ * cancel the job, running the steps that run after a cancel, and report (cancelInternal).
2004+ */
2005+ private async stopContainer(j: JobRow, graceful = false): Promise<void> {
19792006 if (!j.container || j.container.startsWith("self-hosted:")) return;
19802007 try {
19812008 const ns = this.runnerNamespace(j.container);
1982- await ns.get(ns.idFromName(j.container)).stopJob();
2009+ await ns.get(ns.idFromName(j.container)).stopJob(graceful);
19832010 } catch (e) {
19842011 console.warn("stop container failed", e);
19852012 }
@@ -2079,19 +2106,31 @@
20792106
20802107 // ---------------------------------------------------------------- cancellation and re-runs
20812108
2082- private async cancelInternal(runId: string, reason: string): Promise<void> {
2109+ /**
2110+ * Cancels a run's unfinished jobs. With `graceful` (someone cancelled the run), a running hosted job's
2111+ * runner gets to run the steps that run after a cancel first, as on GitHub (CANCEL_STOP_MS); the job
2112+ * ends when it reports. Otherwise, or when it was already cancelled that way, every job ends at once.
2113+ */
2114+ private async cancelInternal(runId: string, reason: string, graceful = false): Promise<void> {
20832115 const run = this.runRow(runId);
20842116 if (run.status === "completed") return;
20852117 this.sql.exec("UPDATE runs SET conclusion = 'cancelled', updated_at = ? WHERE id = ?", now(), run.id);
20862118 for (const j of this.sql.exec<JobRow>("SELECT * FROM jobs WHERE run_id = ? AND status != 'completed'", run.id).toArray()) {
2119+ if (graceful && !j.cancel_at && j.status === "in_progress" && !j.labels && j.container && !j.container.startsWith("self-hosted:")) {
2120+ const at = Date.now();
2121+ this.sql.exec("UPDATE jobs SET cancel_at = ?, cancel_reason = ? WHERE id = ?", at, reason, j.id);
2122+ await this.stopContainer(j, true);
2123+ this.repo.alarmBy(at + CANCEL_STOP_MS);
2124+ continue;
2125+ }
20872126 await this.stopContainer(j);
2088- await this.completeJob(j.id, { conclusion: "cancelled", outputs: {}, failure: { message: reason } });
2127+ await this.completeJob(j.id, { conclusion: "cancelled", outputs: {}, failure: { message: j.cancel_reason ?? reason } });
20892128 }
20902129 }
20912130
20922131 async cancel(runId: string, caller: Caller): Promise<void> {
20932132 if (!atLeast(caller.role, "write")) throw new HttpError(403, "permission denied");
2094- await this.cancelInternal(runId, `Cancelled by ${caller.login}`);
2133+ await this.cancelInternal(runId, `Cancelled by ${caller.login}`, true);
20952134 }
20962135
20972136 /**
@@ -2425,6 +2464,11 @@
24252464
24262465 async runnerComplete(jobId: string, token: string, completion: JobCompletion, deployed: string | null = null): Promise<void> {
24272466 const job = await this.authJob(jobId, token);
2467+ // Cancelled while it ran: it ends here whatever stopped it, never starting again, and says who cancelled it.
2468+ if (job.cancel_at) {
2469+ const { replaced: _, ...rest } = completion;
2470+ return this.completeJob(jobId, rest.conclusion === "cancelled" ? { ...rest, failure: { message: job.cancel_reason ?? "Cancelled" } } : rest);
2471+ }
24282472 if (completion.replaced && completion.conclusion !== "success" && !job.labels) {
24292473 // The runner can't tell a rollout's SIGTERM from the platform's own; its image can: on the
24302474 // deployed one, no deploy replaced it (the platform stopped or moved its container).
modifiedapps/diffinery/src/repo/schema.ts+3−0View file
@@ -216,6 +216,9 @@
216216 ["jobs", "runner_size", "TEXT"],
217217 ["jobs", "size_note", "TEXT"],
218218 ["jobs", "usage", "TEXT"],
219+ // When a running hosted job was cancelled and why, while its runner runs the steps that run after a cancel (repo/actions.ts CANCEL_STOP_MS).
220+ ["jobs", "cancel_at", "INTEGER"],
221+ ["jobs", "cancel_reason", "TEXT"],
219222 // The GitHub import that wrote this row (null for everything else); a retry may only overwrite its own rows.
220223 ["issues", "imported", "TEXT"],
221224 ["pulls", "imported", "TEXT"],
modifiedapps/diffinery/src/runner/containers.ts+10−4View file
@@ -75,13 +75,19 @@
7575 );
7676 }
7777
78- async stopJob(): Promise<void> {
78+ /**
79+ * Diffinery ended the job (a timeout, fail-fast, a newer run in its concurrency group): the container
80+ * goes at once. `graceful` (someone cancelled the run) sends SIGINT instead: the runner stops the step
81+ * under way, runs the steps that run after a cancel and reports. The agent reads SIGTERM as a platform
82+ * rollout and keeps the job running for up to 14 minutes, which a cancelled job must not do.
83+ */
84+ async stopJob(graceful = false): Promise<void> {
7985 // Noted first, so onStop can tell Diffinery's own stop (a cancel, a timeout) from the platform's.
8086 const job = await this.ctx.storage.get<RunnerJob>("job");
8187 if (job) await this.ctx.storage.put("job", { ...job, stopRequested: true });
82- // SIGINT cancels the job at once. The agent reads SIGTERM as a platform rollout and
83- // keeps the job running for up to 14 minutes, which a cancelled job must not do.
84- if (this.ctx.container?.running) await this.stop("SIGINT");
88+ if (!this.ctx.container?.running) return;
89+ if (graceful) await this.stop("SIGINT");
90+ else await this.destroy();
8591 }
8692
8793 override async onStop(params: StopParams): Promise<void> {
modifiedapps/diffinery/test/unit/actions-runtime.test.ts+102−2View file
@@ -1,7 +1,7 @@
11 import { describe, expect, it } from "vitest";
22 import { BodyTooLarge, STEP_OPS, batchParts, blockKey, chunkKey, compose, finalKey, listStaged, parseBlockList, parseRange, planCompose, putStream, runComposeStep, stagingPrefix, type BlobBucket, type ComposeLimits, type ComposeStep } from "../../src/lib/blob-store.js";
33 import { DatabaseSync } from "node:sqlite";
4-import { Actions, JOB_DEADLINE_GRACE_MS, MAX_ACTIVE_RUNS, MAX_PLACEMENT_TRIES, MAX_RUNNER_RESTARTS, parseRunnerInfo, placementDelay, QUICK_RETRY_SECONDS, STALE_IMAGE_RETRY_SECONDS, STALE_IMAGE_TRIES } from "../../src/repo/actions.js";
4+import { Actions, CANCEL_STOP_MS, JOB_DEADLINE_GRACE_MS, MAX_ACTIVE_RUNS, MAX_PLACEMENT_TRIES, MAX_RUNNER_RESTARTS, parseRunnerInfo, placementDelay, QUICK_RETRY_SECONDS, STALE_IMAGE_RETRY_SECONDS, STALE_IMAGE_TRIES } from "../../src/repo/actions.js";
55 import { fleetLimits } from "../../src/lib/quotas.js";
66 import { testD1 } from "./stubs/d1.js";
77 import { DEFAULT_ARTIFACT_QUOTA, Runtime, artifactFileName, artifactObjectKeys } from "../../src/repo/runtime.js";
@@ -10,7 +10,7 @@
1010 import type { Env } from "../../src/env.js";
1111 import { archiveKey, deletePrefixes, legacyArtifactKeys, legacyCacheKeys, logKeys, transientPrefixes } from "../../src/lib/storage-keys.js";
1212 import { signBlobUrl, signRuntimeToken, verifyBlobUrl, verifyRuntimeToken } from "../../src/lib/runtime-auth.js";
13-import { b64, fromB64url } from "../../src/lib/util.js";
13+import { b64, fromB64url, sha256 } from "../../src/lib/util.js";
1414 import { completePayload, issuePayload, releasePayload } from "../../src/repo/event-payloads.js";
1515 import { TokenPolicy } from "../../src/repo/token-policy.js";
1616 import type { Caller, RepoMeta } from "../../src/repo/types.js";
@@ -1029,6 +1029,106 @@
10291029 expect(actions.nextJobDeadline()).toBeNull();
10301030 });
10311031
1032+ it("cancelling a run lets running hosted jobs run their steps that run after a cancel, and ends them after CANCEL_STOP_MS", async () => {
1033+ const sql = sqlStorage();
1034+ sql.exec(REPO_SCHEMA);
1035+ migrateRepoSchema(sql as unknown as SqlStorage);
1036+ const events: { type: string; payload: Record<string, unknown> }[] = [];
1037+ const stopped: [string, boolean][] = [];
1038+ const alarms: number[] = [];
1039+ const repo = {
1040+ sql,
1041+ meta: { repoId: "r", owner: "acme", ownerId: "acme", name: "app", defaultBranch: "main" },
1042+ upsertCheck: () => {},
1043+ broadcast: () => {},
1044+ deploys: { jobCompleted: () => {} },
1045+ runtime: { sweepJob: async () => {}, grantFor: () => ({ role: "read", permissions: {} }) },
1046+ alarmBy: (at: number) => void alarms.push(at),
1047+ } as unknown as RepoDO;
1048+ const env = {
1049+ PUBLIC_URL: "https://d.test",
1050+ BUCKET: memoryBucket().bucket,
1051+ RUNNER: { idFromName: (n: string) => n, get: (n: string) => ({ startJob: async () => {}, stopJob: async (graceful = false) => void stopped.push([n, graceful]) }) },
1052+ RUN_WORKFLOW: { get: async () => ({ sendEvent: async (e: { type: string; payload: Record<string, unknown> }) => void events.push(e) }) },
1053+ } as unknown as Env;
1054+ const actions = new Actions(repo, env);
1055+ const t = new Date().toISOString();
1056+ sql.exec("INSERT INTO runs (id, run_number, workflow_path, workflow_name, event, ref, sha, actor, status, created_at, updated_at, event_payload) VALUES ('run1', 1, 'w.yml', 'w', 'push', 'refs/heads/main', 'abc', '{}', 'in_progress', ?, ?, '{}')", t, t);
1057+ // a and b run on hosted runners (a reports after its cleanup steps, b's runner never does); s runs
1058+ // on a self-hosted runner; w's container hasn't fetched the job yet; q waits in line.
1059+ const token = (id: string) => `djt_r_${id}_secret`;
1060+ const add = async (id: string, status: string, container: string | null) =>
1061+ sql.exec(
1062+ "INSERT INTO jobs (id, run_id, job_key, name, status, timeout_minutes, container, started_at, token_hash, token_at, strategy) VALUES (?, 'run1', ?, ?, ?, 60, ?, ?, ?, ?, '{\"fail-fast\":false}')",
1063+ id, id, id, status, container, status === "in_progress" ? t : null, await sha256(token(id)), t,
1064+ );
1065+ await add("a", "in_progress", "job-a");
1066+ await add("b", "in_progress", "job-b");
1067+ await add("s", "in_progress", "self-hosted:box");
1068+ await add("w", "waiting", "job-w");
1069+ await add("q", "queued", null);
1070+ const job = (id: string) => sql.exec<{ status: string; conclusion: string | null; failure: string | null }>("SELECT status, conclusion, failure FROM jobs WHERE id = ?", id).one();
1071+ const before = Date.now();
1072+ await actions.cancel("run1", { kind: "user", login: "octo", role: "write" } as unknown as Caller);
1073+ // a and b get SIGINT and keep running; the rest end at once, w's container stopped outright.
1074+ expect(stopped).toEqual([["job-a", true], ["job-b", true], ["job-w", false]]);
1075+ expect([job("a").status, job("b").status]).toEqual(["in_progress", "in_progress"]);
1076+ for (const id of ["s", "w", "q"]) expect(job(id)).toMatchObject({ status: "completed", conclusion: "cancelled", failure: JSON.stringify({ message: "Cancelled by octo" }) });
1077+ expect(alarms.at(-1)).toBeGreaterThanOrEqual(before + CANCEL_STOP_MS);
1078+ const deadline = actions.nextJobDeadline()!;
1079+ expect(deadline - before).toBeGreaterThanOrEqual(CANCEL_STOP_MS);
1080+ expect(deadline - Date.now()).toBeLessThanOrEqual(CANCEL_STOP_MS);
1081+ // a's runner reports once its cleanup steps ran: cancelled, saying who cancelled it, never started again.
1082+ await actions.runnerComplete("a", token("a"), { conclusion: "cancelled", outputs: {}, failure: { message: "The job was cancelled." }, replaced: true });
1083+ expect(job("a")).toMatchObject({ status: "completed", conclusion: "cancelled", failure: JSON.stringify({ message: "Cancelled by octo" }) });
1084+
1085+ // b's runner never reports: at CANCEL_STOP_MS its container is stopped and the job ends.
1086+ await actions.enforceJobDeadlines(deadline - 1);
1087+ expect(job("b").status).toBe("in_progress");
1088+ await actions.enforceJobDeadlines(deadline);
1089+ expect(stopped.at(-1)).toEqual(["job-b", false]);
1090+ expect(job("b").conclusion).toBe("cancelled");
1091+ expect(JSON.parse(job("b").failure!).message).toBe(`Cancelled by octo. Its runner didn't report within ${CANCEL_STOP_MS / 60_000} minutes of the cancel, so Diffinery stopped it.`);
1092+ expect(actions.nextJobDeadline()).toBeNull();
1093+ });
1094+
1095+ it("cancelling a cancelled run again stops its jobs at once, and a cancelled job whose container stops before its runner reports ends as cancelled", async () => {
1096+ const sql = sqlStorage();
1097+ sql.exec(REPO_SCHEMA);
1098+ migrateRepoSchema(sql as unknown as SqlStorage);
1099+ const repo = {
1100+ sql,
1101+ meta: { repoId: "r", owner: "acme", ownerId: "acme", name: "app", defaultBranch: "main" },
1102+ upsertCheck: () => {},
1103+ broadcast: () => {},
1104+ deploys: { jobCompleted: () => {} },
1105+ runtime: { sweepJob: async () => {} },
1106+ alarmBy: () => {},
1107+ } as unknown as RepoDO;
1108+ const env = {
1109+ PUBLIC_URL: "https://d.test",
1110+ BUCKET: memoryBucket().bucket,
1111+ RUNNER_IMAGE: "diffinery-runner:new",
1112+ RUNNER: { idFromName: (n: string) => n, get: (n: string) => ({ startJob: async () => {}, stopJob: async (graceful = false) => void stopped.push([n, graceful]) }) },
1113+ RUN_WORKFLOW: { get: async () => ({ sendEvent: async () => {} }) },
1114+ } as unknown as Env;
1115+ const stopped: [string, boolean][] = [];
1116+ const actions = new Actions(repo, env);
1117+ const t = new Date().toISOString();
1118+ const job = (id: string) => sql.exec("SELECT status, conclusion, failure FROM jobs WHERE id = ?", id).one();
1119+ for (const run of ["run1", "run2"]) sql.exec("INSERT INTO runs (id, run_number, workflow_path, workflow_name, event, ref, sha, actor, status, created_at, updated_at, event_payload) VALUES (?, 1, 'w.yml', 'w', 'push', 'refs/heads/main', 'abc', '{}', 'in_progress', ?, ?, '{}')", run, t, t);
1120+ // On an older runner image than the deployed one, which would otherwise read as a rollout and start the job again.
1121+ for (const [id, run] of [["j1", "run1"], ["j2", "run2"]]) sql.exec("INSERT INTO jobs (id, run_id, job_key, name, status, container, started_at, runner_info, strategy) VALUES (?, ?, ?, ?, 'in_progress', ?, ?, '{\"image\":\"diffinery-runner:old\"}', '{}')", id, run, id, id, `job-${id}`, t);
1122+ const octo = { kind: "user", login: "octo", role: "write" } as unknown as Caller;
1123+ await actions.cancel("run1", octo);
1124+ await actions.cancel("run1", { kind: "user", login: "hubot", role: "write" } as unknown as Caller);
1125+ expect(stopped).toEqual([["job-j1", true], ["job-j1", false]]);
1126+ expect(job("j1")).toMatchObject({ status: "completed", conclusion: "cancelled", failure: JSON.stringify({ message: "Cancelled by octo" }) });
1127+ await actions.cancel("run2", octo);
1128+ await actions.containerStopped("j2", null, "job-j2", true);
1129+ expect(job("j2")).toMatchObject({ status: "completed", conclusion: "cancelled", failure: JSON.stringify({ message: "Cancelled by octo" }) });
1130+ });
1131+
10321132 it("a job whose runner a rollout replaced starts again on a new runner before its steps ran or when it can only read, and fails otherwise", async () => {
10331133 const sql = sqlStorage();
10341134 sql.exec(REPO_SCHEMA);
modifiedcontainers/runner/src/agent.ts+6−2View file
@@ -9,7 +9,10 @@
99 workRoot: string;
1010 toolCache?: string;
1111 echoLogs?: boolean;
12- /** Install a SIGTERM/SIGINT handler that cancels the job. */
12+ /**
13+ * Install a SIGTERM/SIGINT handler that cancels the job: the step under way stops and the steps that
14+ * still run after a cancel (`if: always()`, `if: cancelled()`, post steps) run. A second signal stops them too.
15+ */
1316 handleSignals?: boolean;
1417 /**
1518 * Hosted runners: on SIGTERM (a platform rollout replacing this container), keep
@@ -50,7 +53,8 @@
5053 o.onRunner?.(runner);
5154 const onSignal = (sig: string) => {
5255 runner.logger.line(-1, `##[error]Received ${sig}; cancelling the job.`);
53- runner.cancel("cancelled");
56+ // While the platform replaces this runner, its container stops soon: no steps run after the cancel.
57+ runner.cancel("cancelled", runner.replacing ? 0 : undefined);
5458 };
5559 let draining: NodeJS.Timeout | null = null;
5660 const sigterm = () => {
modifiedcontainers/runner/src/job.ts+77−23View file
@@ -19,6 +19,12 @@
1919
2020 export type Outcome = "success" | "failure" | "cancelled" | "skipped";
2121
22+/**
23+ * After a cancel or a job timeout, how long the steps that still run (`if: always()`, `if: cancelled()`,
24+ * post steps) get before they are stopped too: GitHub's runner gives them 5 minutes.
25+ */
26+export const CANCEL_GRACE_MS = 5 * 60_000;
27+
2228 export interface StepContextEntry {
2329 outputs: Record<string, string>;
2430 outcome: Outcome;
@@ -102,8 +108,11 @@
102108 readonly summaries: string[] = [];
103109 readonly posts: PostStep[] = [];
104110 readonly children = new Set<ChildProcess>();
105- /** Aborted when the job is cancelled or times out, for work the runner does itself (downloads). */
106- private readonly aborter = new AbortController();
111+ /**
112+ * Aborted when the job is cancelled or times out, for work the runner does itself (downloads). A new
113+ * one takes its place for the steps that still run after a cancel.
114+ */
115+ private aborter = new AbortController();
107116 readonly stepErrors = new Map<number, Annotation[]>();
108117 readonly stepReasons = new Map<number, string>();
109118 /** Job and service containers, and Docker actions (self-hosted runners with Docker). */
@@ -112,7 +121,15 @@
112121 /** The Actions runtime services client (null when the server sent none). */
113122 readonly runtime: RuntimeClient | null;
114123
124+ /** Why the job is ending early: the job's status for status functions and its conclusion. */
115125 private stopping: null | "cancelled" | "timeout" = null;
126+ /**
127+ * Whether the work under way must stop now: set by a cancel, cleared once the steps that still
128+ * run after it begin (resumeAfterCancel), and set for good when their grace runs out (`forced`).
129+ */
130+ private halting = false;
131+ private forced = false;
132+ private graceTimer: NodeJS.Timeout | undefined;
116133 /** Set when the platform signals that it is replacing this runner (a container rollout). */
117134 replacing = false;
118135 private firstFailure: { step?: number; message: string } | null = null;
@@ -165,18 +182,55 @@
165182 return this.aborter.signal;
166183 }
167184
185+ /** Whether the step under way must stop (the job was cancelled or timed out while it ran). */
168186 get isStopping(): boolean {
169- return this.stopping !== null;
187+ return this.halting;
170188 }
171189
172- /** SIGTERM / external cancellation. */
173- cancel(reason: "cancelled" | "timeout" = "cancelled"): void {
174- if (this.stopping) return;
190+ /**
191+ * SIGINT / SIGTERM / external cancellation, or the job's timeout. Like GitHub, the step under way
192+ * stops, and the steps that still run after a cancel (`if: always()`, `if: cancelled()`, post steps)
193+ * get `graceMs` before they are stopped too. A second cancel stops them at once.
194+ */
195+ cancel(reason: "cancelled" | "timeout" = "cancelled", graceMs = CANCEL_GRACE_MS): void {
196+ if (this.stopping) return this.force();
175197 this.stopping = reason;
198+ this.halt();
199+ if (graceMs <= 0) return this.force();
200+ this.graceTimer = setTimeout(() => {
201+ this.logger.line(-1, `##[error]The steps that run after a cancel took longer than ${graceMs / 60_000} minutes; stopping them.`);
202+ this.force();
203+ }, graceMs);
204+ this.graceTimer.unref();
205+ }
206+
207+ private force(): void {
208+ if (this.forced) return;
209+ this.forced = true;
210+ if (this.graceTimer) clearTimeout(this.graceTimer);
211+ this.halt();
212+ }
213+
214+ private halt(): void {
215+ this.halting = true;
176216 this.aborter.abort();
177217 for (const c of this.children) killTree(c);
178218 }
179219
220+ /** A process the job started: stopped with the job, or at once when it started after the stop. */
221+ private track(c: ChildProcess): void {
222+ this.children.add(c);
223+ c.once("close", () => this.children.delete(c));
224+ if (this.halting) killTree(c);
225+ }
226+
227+ /** Once the cancelled step has stopped: let the steps that still run after a cancel do their work. */
228+ private resumeAfterCancel(): void {
229+ if (!this.halting || this.forced) return;
230+ this.halting = false;
231+ this.aborter = new AbortController();
232+ }
233+
180234 async run(): Promise<JobCompletion> {
181235 const spec = this.spec;
182236 let jobTimer: NodeJS.Timeout | undefined;
@@ -196,10 +250,11 @@
196250 for (let i = 0; i < spec.steps.length; i++) {
197251 const step = spec.steps[i]!;
198252 const fallbackName = defaultStepName(step);
199- if (this.stopping) {
253+ if (this.forced) {
200254 await this.report(i, { status: "completed", conclusion: "cancelled", name: safeName(step, fallbackName), completedAt: nowIso() });
201255 continue;
202256 }
257+ this.resumeAfterCancel();
203258 const startedAt = nowIso();
204259 let started = false;
205260 const res = await this.executeStep(step, top, i, async (name) => {
@@ -208,13 +263,14 @@
208263 });
209264 await this.report(i, {
210265 status: "completed",
211- conclusion: res.conclusion,
266+ // After a cancel, a step whose condition kept it from running shows as cancelled, as before.
267+ conclusion: this.stopping && res.conclusion === "skipped" ? "cancelled" : res.conclusion,
212268 name: res.name ?? fallbackName,
213269 ...(started ? { startedAt } : {}),
214270 completedAt: nowIso(),
215271 });
216272 }
217- if (!this.stopping) await this.runPosts(top);
273+ if (!this.forced) await this.runPosts(top);
218274 } catch (e) {
219275 const msg = e instanceof Error ? e.message : String(e);
220276 this.logger.line(-1, `##[error]${msg}`);
@@ -222,6 +278,7 @@
222278 this.firstFailure ??= { message: msg };
223279 } finally {
224280 if (jobTimer) clearTimeout(jobTimer);
281+ if (this.graceTimer) clearTimeout(this.graceTimer);
225282 await this.prefetch.stop();
226283 try {
227284 await this.docker.cleanup({ logIndex: -1, outputs: {}, state: {} });
@@ -422,7 +479,8 @@
422479 statusFns(scope: Scope): StatusFns {
423480 return {
424481 success: () => !scope.failed && this.stopping === null,
425- failure: () => scope.failed,
482+ // GitHub: once the job is cancelled its status is cancelled, so failure() is false.
483+ failure: () => scope.failed && this.stopping === null,
426484 cancelled: () => this.stopping !== null,
427485 always: () => true,
428486 };
@@ -564,9 +622,9 @@
564622 } else {
565623 throw new StepFailure("Step has neither `run` nor `uses`.");
566624 }
567- if (this.stopping) outcome = "cancelled";
625+ if (this.halting) outcome = "cancelled";
568626 } catch (e) {
569- if (this.stopping) {
627+ if (this.halting) {
570628 outcome = "cancelled";
571629 } else {
572630 outcome = "failure";
@@ -673,10 +731,7 @@
673731 env,
674732 timeoutMs: o.timeoutMs,
675733 onLine: (l) => proc.handle(l),
676- onSpawn: (c) => {
677- this.children.add(c);
678- c.once("close", () => this.children.delete(c));
679- },
734+ onSpawn: (c) => this.track(c),
680735 });
681736 await this.applyFileCommands(files, o.io);
682737 if (res.signal && this.replacing && !this.stopping) {
@@ -685,7 +740,8 @@
685740 const msg = "The platform stopped this step's processes while stopping the runner.";
686741 this.logger.line(o.io.logIndex, `##[error]${msg}`);
687742 this.addAnnotation(o.io.logIndex, { level: "error", message: msg });
688- this.cancel("cancelled");
743+ // No steps run after this cancel: the platform is stopping the container, and the job may start again elsewhere.
744+ this.cancel("cancelled", 0);
689745 }
690746 if (res.timedOut) {
691747 const mins = (o.timeoutMs ?? 0) / 60000;
@@ -757,10 +813,7 @@
757813 capture: o.capture,
758814 input: o.input,
759815 onLine: o.silent ? undefined : (l) => this.logger.line(o.io.logIndex, l),
760- onSpawn: (c) => {
761- this.children.add(c);
762- c.once("close", () => this.children.delete(c));
763- },
816+ onSpawn: (c) => this.track(c),
764817 });
765818 }
766819
@@ -773,7 +826,8 @@
773826 let index = this.spec.steps.length;
774827 for (const p of posts) {
775828 const logIndex = index++;
776- if (this.stopping) break;
829+ if (this.forced) break;
830+ this.resumeAfterCancel();
777831 let ok: boolean;
778832 try {
779833 ok = evaluateIf(p.condition, this.contexts(top, this.baseEnvContext(top)), this.statusFns(top), this.extra());
@@ -796,7 +850,7 @@
796850 top.failed = true;
797851 this.noteFailure(logIndex, msg);
798852 }
799- if (this.stopping) conclusion = "cancelled";
853+ if (this.halting) conclusion = "cancelled";
800854 await this.report(logIndex, { status: "completed", conclusion, name: p.name, startedAt, completedAt: nowIso() });
801855 }
802856 }
modifiedcontainers/runner/test/job.test.ts+83−6View file
@@ -478,22 +478,99 @@
478478 expect(api.logText(1)).toContain("after-timeout");
479479 });
480480
481- it("cancels on request: kills the process group and reports cancelled", async () => {
481+ const cancelOnce = async (steps: StepSpec[], cancel: (r: JobRunner) => void) => {
482482 let runner: JobRunner | undefined;
483- const spec = makeSpec(api, [{ run: "echo started; (sleep 60 & wait)" }, { run: "echo never" }, { if: "always()", run: "echo never2" }]);
484- const p = run(spec, { onRunner: (r) => (runner = r) });
483+ const p = run(makeSpec(api, steps), { onRunner: (r) => (runner = r) });
485484 const t0 = Date.now();
486485 while (!api.logText(0).includes("started") && Date.now() - t0 < 10_000) await new Promise((r) => setTimeout(r, 50));
487- runner!.cancel();
488- const c = await p;
486+ cancel(runner!);
487+ return { c: await p, ms: Date.now() - t0 };
488+ };
489+
490+ it("cancels on request: kills the process group, then runs only the steps that run after a cancel", async () => {
491+ const { c, ms } = await cancelOnce(
492+ [
493+ { run: "echo started; (sleep 60 & wait)" },
494+ { run: "echo never" },
495+ { if: "failure()", run: "echo never-failure" },
496+ { if: "always()", run: "echo always-ran" },
497+ { if: "${{ cancelled() }}", run: 'echo "cancelled-ran $JOB_STATUS"', env: { JOB_STATUS: "${{ job.status }}" } },
498+ { if: "!cancelled()", run: "echo never-not-cancelled" },
499+ ],
500+ (r) => r.cancel(),
501+ );
489502 expect(c.conclusion).toBe("cancelled");
503+ expect(c.failure?.message).toBe("The job was cancelled.");
490504 expect(api.lastReport(0)?.conclusion).toBe("cancelled");
491505 expect(api.lastReport(1)?.conclusion).toBe("cancelled");
492506 expect(api.lastReport(2)?.conclusion).toBe("cancelled");
507+ expect(api.lastReport(3)?.conclusion).toBe("success");
508+ expect(api.lastReport(4)?.conclusion).toBe("success");
509+ expect(api.lastReport(5)?.conclusion).toBe("cancelled");
510+ expect(api.logText(3)).toContain("always-ran");
511+ expect(api.logText(4)).toContain("cancelled-ran cancelled");
512+ expect(api.logText()).not.toContain("never");
493513 expect(api.completion?.conclusion).toBe("cancelled");
494- expect(Date.now() - t0).toBeLessThan(10_000);
514+ expect(ms).toBeLessThan(10_000);
495515 });
496516
517+ it("a second cancel stops the steps that run after a cancel", async () => {
518+ const steps: StepSpec[] = [{ run: "echo started; sleep 60" }, { if: "always()", run: "echo cleanup-started; sleep 60" }, { if: "always()", run: "echo never" }];
519+ let runner: JobRunner | undefined;
520+ const p = run(makeSpec(api, steps), { onRunner: (r) => (runner = r) });
521+ const t0 = Date.now();
522+ while (!api.logText(0).includes("started") && Date.now() - t0 < 10_000) await new Promise((r) => setTimeout(r, 50));
523+ runner!.cancel();
524+ const t1 = Date.now();
525+ while (!api.logText(1).includes("cleanup-started") && Date.now() - t1 < 15_000) await new Promise((r) => setTimeout(r, 50));
526+ expect(api.logText(1)).toContain("cleanup-started");
527+ runner!.cancel();
528+ const c = await p;
529+ expect(c.conclusion).toBe("cancelled");
530+ expect(api.lastReport(1)?.conclusion).toBe("cancelled");
531+ expect(api.lastReport(2)?.conclusion).toBe("cancelled");
532+ expect(api.logText(2)).not.toContain("never");
533+ expect(Date.now() - t1).toBeLessThan(15_000);
534+ });
535+
536+ it("a process a step starts after the job was stopped is stopped at once", async () => {
537+ let runner: JobRunner | undefined;
538+ const p = run(makeSpec(api, [{ run: "echo started; sleep 60" }, { if: "always()", run: "echo cleanup-started; sleep 60" }]), { onRunner: (r) => (runner = r) });
539+ const t0 = Date.now();
540+ while (!api.logText(0).includes("started") && Date.now() - t0 < 10_000) await new Promise((r) => setTimeout(r, 50));
541+ // Both cancels before the cleanup step's process starts: it must not run its 60 seconds.
542+ runner!.cancel();
543+ runner!.cancel();
544+ const c = await p;
545+ expect(c.conclusion).toBe("cancelled");
546+ expect(api.lastReport(1)?.conclusion).toBe("cancelled");
547+ expect(Date.now() - t0).toBeLessThan(20_000);
548+ });
549+
550+ it("stops the steps that run after a cancel once their grace runs out", async () => {
551+ let runner: JobRunner | undefined;
552+ const p = run(makeSpec(api, [{ run: "echo started; sleep 60" }, { if: "always()", run: "echo cleanup-started; sleep 60" }, { if: "always()", run: "echo never" }]), { onRunner: (r) => (runner = r) });
553+ const t0 = Date.now();
554+ while (!api.logText(0).includes("started") && Date.now() - t0 < 10_000) await new Promise((r) => setTimeout(r, 50));
555+ runner!.cancel("cancelled", 1500);
556+ const c = await p;
557+ expect(c.conclusion).toBe("cancelled");
558+ expect(api.logText(1)).toContain("cleanup-started");
559+ expect(api.lastReport(1)?.conclusion).toBe("cancelled");
560+ expect(api.lastReport(2)?.conclusion).toBe("cancelled");
561+ expect(api.logText()).toContain("took longer than");
562+ expect(Date.now() - t0).toBeLessThan(15_000);
563+ });
564+
565+ it("runs always() steps and post steps after the job's timeout", async () => {
566+ const c = await run(makeSpec(api, [{ run: "echo started; sleep 60" }, { run: "echo never" }, { if: "always()", run: "echo after-job-timeout" }], { timeoutMinutes: 0.03 }));
567+ expect(c.conclusion).toBe("failure");
568+ expect(c.failure?.message).toContain("maximum execution time");
569+ expect(api.lastReport(1)?.conclusion).toBe("cancelled");
570+ expect(api.lastReport(2)?.conclusion).toBe("success");
571+ expect(api.logText(2)).toContain("after-job-timeout");
572+ });
573+
497574 it("never loses log lines when the log endpoint flakes", async () => {
498575 api.failLogs = 2;
499576 const c = await run(makeSpec(api, [{ run: "for i in $(seq 1 500); do echo \"n$i\"; done" }]));