Deploys leave running jobs alone; a skipped deploy no longer passes the live test; steady docs test (#66)

* Deploys leave running jobs and imports alone; cancelling a job stops it at once

A container rollout only spared containers up for less than an hour, so any job
or import running longer was sent SIGTERM and then killed 15 minutes later. The
runner and import containers' grace periods now outlast a job (6 h cap) and an
import transfer, so a deploy never replaces one that is running.

Cancelling a job, its timeout, fail-fast and concurrency stopped the container
with SIGTERM, which the agent reads as a rollout and so kept the job running for
up to 14 more minutes. They now send SIGINT, which cancels at once.

After a SIGTERM the agent drains for 13 minutes, not 14, and caps the final log
flush at 30 s, so the job reports a result before the platform's SIGKILL. A
container that stops with no exit code says so instead of "exit code 0".

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ruyx3Qr4jYrZEpPi466deb

* A run whose jobs were all skipped is skipped; steady docs route test

On a commit where CI failed, Deploy's only job was skipped but the run reported
success, so the live test ran against the previous deploy and could vouch for a
commit that never shipped. As on GitHub, a run whose jobs were all skipped now
concludes "skipped", and the live test (which waits for a successful Deploy)
doesn't run. The sync's wait script accepts skipped runs; a failed CI still
fails it. An end-to-end check covers it and fails without the fix.

The docs test that renders every page twice timed out at 5.2 s on a busy CI
runner; it gets 30 s. Its D1 stub now answers queries, so the sitemap test
checks the docs are listed instead of skipping, and logs no TypeError.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ruyx3Qr4jYrZEpPi466deb

---------

Co-authored-by: Claude <noreply@anthropic.com>

Canberk Demirci authored 13 days ago· GitHub committed 13 days agosigned1 parent 30ba0b3commit a2120c07cb6d02291a576ff7f481df509c23ff97Browse files

13 files changed+56−17

Files (13)

apps/diffinery/docs/actions/overview.md+1−1
apps/diffinery/docs/actions/runners.md+1−1
apps/diffinery/docs/self-hosting/configuration.md+1−1
apps/diffinery/docs/self-hosting/deploy.md+1−1
apps/diffinery/src/repo/actions.ts+4−1
apps/diffinery/src/runner/containers.ts+3−1
apps/diffinery/test/e2e/e2e.test.mjs+13−0
apps/diffinery/test/unit/docs.test.ts+8−5
apps/diffinery/wrangler.jsonc+6−2
containers/runner/src/agent.ts+2−1
containers/runner/src/main.ts+3−2
containers/runner/test/job.test.ts+10−0
scripts/diffinery-wait-runs.sh+3−1

modifiedapps/diffinery/docs/actions/overview.md+1−1View file

modifiedapps/diffinery/docs/actions/runners.md+1−1View file

modifiedapps/diffinery/docs/self-hosting/configuration.md+1−1View file

modifiedapps/diffinery/docs/self-hosting/deploy.md+1−1View file

modifiedapps/diffinery/src/repo/actions.ts+4−1View file

modifiedapps/diffinery/src/runner/containers.ts+3−1View file

modifiedapps/diffinery/test/e2e/e2e.test.mjs+13−0View file

modifiedapps/diffinery/test/unit/docs.test.ts+8−5View file

modifiedapps/diffinery/wrangler.jsonc+6−2View file

modifiedcontainers/runner/src/agent.ts+2−1View file

modifiedcontainers/runner/src/main.ts+3−2View file

modifiedcontainers/runner/test/job.test.ts+10−0View file

modifiedscripts/diffinery-wait-runs.sh+3−1View file

		@@ -431,7 +431,7 @@
431	431	\| A self-hosted job stays `waiting` \| No runner with all the job's labels is polling. Start `diffinery runner` with those labels. After 6 hours the job times out. \|
432	432	\| `Could not start a runner: ...` \| The job's container failed to start. Re-run the run. \|
433	433	\| `The runner stopped unexpectedly (exit code <n>).` \| The job's container exited before reporting a result. Re-run the run; if it repeats, check that the job doesn't exhaust memory or disk. \|
434		-\| `This runner is being replaced by a new version. The job keeps running for up to 14 minutes before it is cancelled.` \| Diffinery is rolling out a new runner version. Jobs that finish within 14 minutes are unaffected; longer ones are cancelled and need a re-run. \|
	434	+\| `This runner is being replaced by a new version. The job keeps running for up to 13 minutes before it is cancelled.` \| The platform is replacing this job's container. Deploys leave running jobs alone, so this is rare. Jobs that finish within 13 minutes are unaffected; longer ones are cancelled and need a re-run. \|
435	435	\| `Error evaluating 'if': ...` \| The job's `if:` expression failed at runtime. See [Expressions](/docs/actions/expressions). \|
436	436	\| A step can't reach a secret \| Secrets are configured in the repository or organization settings. See [Secrets and variables](/docs/actions/secrets-and-variables). \|
437	437	\| `git push` from a job fails \| The job's `GITHUB_TOKEN` needs `contents: write` (new repositories default to read-only tokens), can't change workflow files, and follows branch protection. See [GITHUB_TOKEN permissions](/docs/actions/workflow-permissions). \|

		@@ -141,7 +141,7 @@
141	141	\| Step timeout \| `steps[*].timeout-minutes`, no default \| The step fails with "The step has timed out after N minutes." \|
142	142	\| Matrix size \| 256 jobs per matrix \| The job fails with "A matrix may produce at most 256 jobs." \|
143	143	\| Log line length \| 16,384 characters \| Longer lines are cut and end with `…` \|
144		-\| Platform updates \| When Diffinery deploys a new runner version, a running job keeps going for up to 14 minutes \| The log shows "This runner is being replaced by a new version. The job keeps running for up to 14 minutes before it is cancelled." \|
	144	+\| Platform updates \| A deploy doesn't replace the container of a running job: running jobs finish on the version they started with. If the platform replaces a container anyway, the job keeps going for up to 13 minutes \| The log shows "This runner is being replaced by a new version. The job keeps running for up to 13 minutes before it is cancelled." \|
145	145
146	146	If the container stops unexpectedly (for example it runs out of memory), the job fails with `The runner stopped unexpectedly (exit code N).`
147	147

		@@ -114,7 +114,7 @@
114	114	\| `observability` \| Enabled, `head_sampling_rate: 1` \| Every request is logged to Workers Logs. \|
115	115	\| `upload_source_maps` \| `true` \| Readable stack traces in the logs. \|
116	116	\| `limits.cpu_ms` \| `300000` \| Up to 5 minutes of CPU time per request (large pushes, diffs and merges). \|
117		-\| `containers` \| `RunnerContainer`: `standard-2`, `max_instances: 100`; `SandboxContainer`: `standard-1`, `max_instances: 50`; `ImportContainer`: `standard-4`, `max_instances: 20`; all `rollout_active_grace_period: 3600` \| All three build `containers/runner/Dockerfile` with `containers/runner` as context. Raise `max_instances` for more concurrent jobs, sandboxes or import transfers. \|
	117	+\| `containers` \| `RunnerContainer`: `standard-2`, `max_instances: 100`; `SandboxContainer`: `standard-1`, `max_instances: 50`; `ImportContainer`: `standard-4`, `max_instances: 20`; `rollout_active_grace_period`: 23400 for runners and 25200 for imports (longer than a job or transfer can run, so a deploy never replaces a running one), 3600 for sandboxes \| All three build `containers/runner/Dockerfile` with `containers/runner` as context. Raise `max_instances` for more concurrent jobs, sandboxes or import transfers. \|
118	118	\| `triggers.crons` \| `["/5 * * *"]` \| Scheduled workflows, token expiry reminders, email log pruning, registry cleanup, and cancelling draft imports older than 7 days. \|
119	119
120	120	## The dev environment

		@@ -267,7 +267,7 @@
267	267	- Creates nothing that already exists and keeps your D1 data, R2 data, secrets and Access application.
268	268	- Applies new D1 migrations before the new Worker goes live. For example, `0010_imports.sql` adds the tables for GitHub imports, linked GitHub accounts and organization variables.
269	269	- Rebuilds the documentation from the Markdown files in your copy, so `/docs` matches the code you deploy.
270		-- Rolls out a new runner image. The first sandbox started on a machine after a deploy pulls the new image first, so it can take a few minutes to start; Diffinery waits up to 3 minutes for it. A hosted job that is running when its container is replaced gets a warning in its log ("This runner is being replaced by a new version. The job keeps running for up to 14 minutes before it is cancelled.").
	270	+- Rolls out a new runner image. The first sandbox started on a machine after a deploy pulls the new image first, so it can take a few minutes to start; Diffinery waits up to 3 minutes for it. Running hosted jobs and imports keep their containers until they finish, and the next ones start on the new image. A sandbox up for more than an hour is replaced.
271	271
272	272	Keep `MASTER_KEY` safe and never change it: it decrypts every stored Actions secret (repository, organization and environment secrets), unsubscribe links in emails already sent are signed with it, and so are the cache and artifact tokens of jobs that are running. If you delete `OIDC_PRIVATE_KEY`, the next deploy generates a new key with a new key ID; systems that trust your OIDC tokens fetch the new public key from `/.well-known/jwks`.
273	273

		@@ -949,7 +949,7 @@
949	949	async containerStopped(jobId: string, exitCode: number \| null): Promise<void> {
950	950	const j = this.sql.exec<JobRow>("SELECT * FROM jobs WHERE id = ?", jobId).toArray()[0];
951	951	if (!j \|\| j.status === "completed") return;
952		- await this.completeJob(jobId, { conclusion: "failure", outputs: {}, failure: { message: `The runner stopped unexpectedly (exit code ${exitCode ?? "unknown"}).` } });
	952	+ await this.completeJob(jobId, { conclusion: "failure", outputs: {}, failure: { message: exitCode ? `The runner stopped unexpectedly (exit code ${exitCode}).` : "The runner's container stopped before the job finished, without an exit code (the platform restarted or replaced it)." } });
953	953	}
954	954
955	955	/**
		@@ -989,6 +989,9 @@
989	989	// A job with continue-on-error may fail without failing the run.
990	990	if (jobs.some((j) => (j.conclusion === "failure" \|\| j.conclusion === "timed_out") && !j.continue_on_error)) conclusion = "failure";
991	991	else if (jobs.some((j) => j.conclusion === "cancelled")) conclusion = "cancelled";
	992	+ // As on GitHub, a run whose jobs were all skipped is skipped, not a success, so a
	993	+ // workflow_run follower that checks for success (a live test after a deploy) doesn't run.
	994	+ else if (jobs.length && jobs.every((j) => j.conclusion === "skipped")) conclusion = "skipped";
992	995	}
993	996	const t = now();
994	997	this.sql.exec("UPDATE runs SET status = 'completed', conclusion = ?, updated_at = ? WHERE id = ?", conclusion, t, runId);

		@@ -24,7 +24,9 @@
24	24	}
25	25
26	26	async stopJob(): Promise<void> {
27		- if (this.ctx.container?.running) await this.stop("SIGTERM");
	27	+ // SIGINT cancels the job at once. The agent reads SIGTERM as a platform rollout and
	28	+ // keeps the job running for up to 14 minutes, which a cancelled job must not do.
	29	+ if (this.ctx.container?.running) await this.stop("SIGINT");
28	30	}
29	31
30	32	override async onStop(params: StopParams): Promise<void> {

		@@ -668,6 +668,19 @@
668	668	});
669	669	assert.equal(done.conclusion, "success");
670	670	assert.equal((await runs()).length, 2, "the follower does not trigger itself");
	671	+
	672	+ // A run whose jobs were all skipped is skipped, as on GitHub, so a follower that waits for success doesn't run.
	673	+ writeFileSync(join(src, ".github/workflows/ci.yml"), `name: CI\non: push\njobs:\n build:\n if: \${{ false }}\n runs-on: [self-hosted, linux]\n steps:\n - run: echo built\n`);
	674	+ git(src, ["commit", "-q", "-am", "skip the build"]);
	675	+ git(src, ["push", "-q", "origin", "main"]);
	676	+ const settled = await until("skipped CI and its follower", async () => {
	677	+ const all = await runs();
	678	+ return all.length === 4 && all.every((r) => r.status === "completed") ? all : null;
	679	+ });
	680	+ const sha = git(src, ["rev-parse", "HEAD"]).trim();
	681	+ const skippedCi = settled.find((r) => r.name === "CI" && r.head_sha === sha);
	682	+ assert.equal(skippedCi?.conclusion, "skipped");
	683	+ assert.equal(settled.find((r) => r.event === "workflow_run" && r.id !== follower.id)?.conclusion, "skipped");
671	684	});
672	685
673	686	test("triggers from workflows: a job's repository_dispatch starts runs, a follower of agent code stays an agent's", { timeout: 600_000 }, async () => {

		@@ -18,7 +18,9 @@
18	18	app.route("/", web);
19	19
20	20	const ORIGIN = "https://diffinery.example";
21		-const env = { PUBLIC_URL: ORIGIN, DB: {} } as unknown as Env;
	21	+// An empty D1: the docs need no data, and the sitemap finds no public repositories.
	22	+const stmt = { bind: () => stmt, all: async () => ({ results: [] }), first: async () => null, run: async () => ({ success: true, meta: {} }) };
	23	+const env = { PUBLIC_URL: ORIGIN, DB: { prepare: () => stmt } } as unknown as Env;
22	24	const ctx = { waitUntil() {}, passThroughOnException() {}, props: {} } as unknown as ExecutionContext;
23	25	const get = (path: string, headers: Record<string, string> = {}) => app.fetch(new Request(`${ORIGIN}${path}`, { headers }), env, ctx);
24	26
		@@ -191,7 +193,8 @@
191	193	}
192	194	const neg = await get("/docs/quickstart", { accept: "text/markdown, text/html;q=0.8" });
193	195	expect(neg.headers.get("content-type")).toContain("text/markdown");
194		- });
	196	+ // Renders and highlights every page twice: about 1 s alone, several times that on a busy CI runner.
	197	+ }, 30_000);
195	198
196	199	it("serves llms.txt, llms-full.txt, OpenAPI, the search index and the CLI", async () => {
197	200	for (const [path, type] of [
		@@ -214,8 +217,8 @@
214	217	});
215	218
216	219	it("lists the docs in the sitemap", async () => {
217		- const xml = await (await get("/sitemap.xml")).text().catch(() => "");
218		- // The sitemap also lists public repositories from D1, which this test has no database for.
219		- if (xml.includes("<urlset")) expect(xml).toContain(`${ORIGIN}/docs/quickstart`);
	220	+ const res = await get("/sitemap.xml");
	221	+ expect(res.status).toBe(200);
	222	+ expect(await res.text()).toContain(`${ORIGIN}/docs/quickstart`);
220	223	});
221	224	});

		@@ -62,11 +62,15 @@
62	62	{ "name": "diffinery-run", "binding": "RUN_WORKFLOW", "class_name": "RunWorkflow" },
63	63	{ "name": "diffinery-import", "binding": "IMPORT_WORKFLOW", "class_name": "ImportWorkflow" }
64	64	],
	65	+ // A deploy that changes the image rolls the running containers: once a container has been up
	66	+ // longer than rollout_active_grace_period seconds, it gets SIGTERM and is killed 15 minutes later.
	67	+ // Runner and import containers are covered for their whole life (a job is capped at 6 hours, an
	68	+ // import transfer too), so a deploy never cuts one short; they pick up the new image next start.
65	69	"containers": [
66		- { "class_name": "RunnerContainer", "image": "../../containers/runner/Dockerfile", "image_build_context": "../../containers/runner", "instance_type": "standard-2", "max_instances": 100, "rollout_active_grace_period": 3600 },
	70	+ { "class_name": "RunnerContainer", "image": "../../containers/runner/Dockerfile", "image_build_context": "../../containers/runner", "instance_type": "standard-2", "max_instances": 100, "rollout_active_grace_period": 23400 },
67	71	{ "class_name": "SandboxContainer", "image": "../../containers/runner/Dockerfile", "image_build_context": "../../containers/runner", "instance_type": "standard-1", "max_instances": 50, "rollout_active_grace_period": 3600 },
68	72	// GitHub imports: a mirror clone of a large repository needs disk and memory.
69		- { "class_name": "ImportContainer", "image": "../../containers/runner/Dockerfile", "image_build_context": "../../containers/runner", "instance_type": "standard-4", "max_instances": 20, "rollout_active_grace_period": 3600 }
	73	+ { "class_name": "ImportContainer", "image": "../../containers/runner/Dockerfile", "image_build_context": "../../containers/runner", "instance_type": "standard-4", "max_instances": 20, "rollout_active_grace_period": 25200 }
70	74	],
71	75	// Housekeeping, and a check that the workflow scheduler's alarm is set (src/scheduler.ts).
72	76	"triggers": { "crons": ["/5 * * *"] },

		@@ -62,7 +62,8 @@
62	62	completion = { conclusion: "failure", outputs: {}, failure: { message: `Runner error: ${errMsg(e)}` } };
63	63	}
64	64	if (batcher) {
65		- const ok = await batcher.close(120_000);
	65	+ // After a SIGTERM the platform kills the container soon, so the result must not wait long on logs.
	66	+ const ok = await batcher.close(draining ? 30_000 : 120_000);
66	67	if (!ok) process.stderr.write(`[diffinery-runner] gave up shipping ${batcher.pending} log lines\n`);
67	68	}
68	69	try {

		@@ -139,8 +139,9 @@
139	139	toolCache: env.RUNNER_TOOL_CACHE,
140	140	echoLogs: env.DIFFINERY_ECHO_LOGS === "1" \|\| apiBase === null,
141	141	handleSignals: true,
142		- // Cloudflare gives a container 15 minutes after SIGTERM during a rollout.
143		- drainOnSigtermMs: args.spec ? undefined : 14 * 60_000,
	142	+ // Cloudflare gives a container 15 minutes after SIGTERM during a rollout; the last
	143	+ // two are for cancelling the steps and reporting the result before SIGKILL.
	144	+ drainOnSigtermMs: args.spec ? undefined : 13 * 60_000,
144	145	});
145	146	process.stdout.write(`[diffinery-runner] job finished: ${completion.conclusion}\n`);
146	147	process.exit(completion.conclusion === "success" ? 0 : 1);

		@@ -821,5 +821,15 @@
821	821	expect(api.completion).toMatchObject({ conclusion: "cancelled" });
822	822	expect(api.lastReport(0)?.conclusion).toBe("cancelled");
823	823	expect(api.lastReport(1)?.conclusion).toBe("cancelled");
	824	+
	825	+ // SIGINT (Diffinery cancelling the job): cancel at once, with no drain.
	826	+ const t1 = Date.now();
	827	+ const cancel = startJob("job-5", [{ run: "echo started; sleep 60" }]);
	828	+ await waitForLog("started");
	829	+ cancel.child.kill("SIGINT");
	830	+ expect(await cancel.exited).toBe(1);
	831	+ expect(Date.now() - t1).toBeLessThan(15_000);
	832	+ expect(api.completion).toMatchObject({ conclusion: "cancelled" });
	833	+ expect(api.logText()).not.toContain("being replaced");
824	834	}, 60_000);
825	835	});

		@@ -44,7 +44,9 @@
44	44	jq -r 'def secs: try ((.completed_at \| fromdateiso8601) - (.started_at \| fromdateiso8601)) catch null;
45	45	.jobs[] \| " \(.name): \(.conclusion)\(if .failure then " - " + (.failure.message // "") else "" end)\(if secs then " (\(secs)s)" else "" end)",
46	46	(.steps[]? \| select((secs // 0) > 20) \| " \(secs)s \(.name)")' <<<"$jobs"
47		- if [ "$(jq -r .conclusion <<<"$run")" != success ]; then
	47	+ # A skipped run (every job's `if` was false) is fine; a skipped Deploy after a failed CI
	48	+ # still fails the sync through CI.
	49	+ if ! grep -qxE 'success\|skipped' <<<"$(jq -r .conclusion <<<"$run")"; then
48	50	failed=1
49	51	for job in $(jq -r '.jobs[] \| select(.conclusion == "failure" or .conclusion == "timed_out") \| .id' <<<"$jobs"); do
50	52	echo "::group::Last log lines of job $job"

codician/diffinerypublic

Deploys leave running jobs alone; a skipped deploy no longer passes the live test; steady docs test (#66)