From 2c7028f606a05d6a668ad3e59f9e1fd5ece7fcad Mon Sep 17 00:00:00 2001 From: Matt Sichterman Date: Sat, 22 Aug 2026 13:36:35 -0400 Subject: [PATCH] fix(runner): do not fail a job on a transient cache-assignment status The job-started hook treated every status except 200 and 202 as a verdict and exited 1 on the first sample. Two real conditions reach that branch without meaning the assignment is invalid: - 401, when the container polls before the Worker's assignment authorization is visible through Cloudflare's edge. The record exists; the reader is early. - 000, curl's output for a connection failure, so one network blip ends the job. Both surface to the user as a failed "Set up runner" step before any repository code runs, and both clear on a re-run. Observed on a private repository across about 16 job attempts: 3 failures, split between this branch and the bounded wait window. Keep polling on 000, 401, 408, 429, and 5xx inside the existing bounded window. Continue to fail closed on every other status and when the window expires, so a job still never runs with a cache capability the Worker did not confirm. Report the last observed status in the timeout message, because "not observed within N seconds" alone does not distinguish a slow assignment from a stuck one. Tests: a 401 that resolves to 200 now starts the job; a 401 that never resolves still fails closed and still never prints the capability. Co-Authored-By: Claude Opus 5 (1M context) --- docker/job-started-hook.sh | 21 ++++++++--- tests/job-started-hook.test.ts | 64 ++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/docker/job-started-hook.sh b/docker/job-started-hook.sh index e056bee..8b7d8bc 100644 --- a/docker/job-started-hook.sh +++ b/docker/job-started-hook.sh @@ -43,13 +43,24 @@ while [ "$attempt" -le "$assignment_max_attempts" ]; do if [ "$status" = '200' ]; then exit 0 fi - if [ "$status" != '202' ]; then - printf '%s\n' "::error title=Cloudflare runner cache assignment::The Worker returned HTTP $status while waiting for GitHub's runner assignment." - exit 1 - fi + case "$status" in + 202) ;; + # Transient conditions, not verdicts. The assignment record is written by + # the Worker and read back through Cloudflare's edge, so a container can + # poll before its authorization is visible and see 401. 000 is curl's + # output for a connection failure. Keep polling inside the existing + # bounded window instead of failing the job on the first sample. + 000|401|408|429|500|502|503|504) ;; + *) + printf '%s\n' "::error title=Cloudflare runner cache assignment::The Worker returned HTTP $status while waiting for GitHub's runner assignment." + exit 1 + ;; + esac attempt=$((attempt + 1)) sleep "$assignment_poll_seconds" done -printf '%s\n' "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within ${assignment_max_attempts} seconds." +# Still fail closed. The job must not run with a cache capability whose +# assignment the Worker never confirmed. +printf '%s\n' "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within ${assignment_max_attempts} seconds (last Worker status: ${status})." exit 1 diff --git a/tests/job-started-hook.test.ts b/tests/job-started-hook.test.ts index 997bcea..863634f 100644 --- a/tests/job-started-hook.test.ts +++ b/tests/job-started-hook.test.ts @@ -67,7 +67,7 @@ describe("runner cache assignment hook", () => { }, 10_000); it("fails closed on an unexpected Worker status and never prints the runner capability", async () => { - const server = createServer((_request, response) => response.writeHead(401).end()); + const server = createServer((_request, response) => response.writeHead(403).end()); const port = await listen(server); const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); const configurationPath = join(directory, "cache-assignment"); @@ -78,7 +78,7 @@ describe("runner cache assignment hook", () => { await expect(runHook(configurationPath)).resolves.toEqual({ code: 1, stdout: - "::error title=Cloudflare runner cache assignment::The Worker returned HTTP 401 while waiting for GitHub's runner assignment.\n", + "::error title=Cloudflare runner cache assignment::The Worker returned HTTP 403 while waiting for GitHub's runner assignment.\n", stderr: "", }); await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); @@ -110,7 +110,7 @@ describe("runner cache assignment hook", () => { ).resolves.toEqual({ code: 1, stdout: - "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds.\n", + "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds (last Worker status: 202).\n", stderr: "", }); expect(attempts).toBe(2); @@ -121,6 +121,64 @@ describe("runner cache assignment hook", () => { } }); + it("keeps polling when the assignment authorization is not yet visible", async () => { + // The Worker writes the assignment record and the container reads it back + // through Cloudflare's edge, so the first samples can 401 before the + // authorization propagates. That is a race, not a verdict. + let attempts = 0; + const server = createServer((_request, response) => { + attempts += 1; + response.writeHead(attempts < 3 ? 401 : 200).end(); + }); + const port = await listen(server); + const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); + const configurationPath = join(directory, "cache-assignment"); + await writeFile(configurationPath, `http://127.0.0.1:${port}/v1/runner-cache\nBearer runner-capability\n`, { + mode: 0o600, + }); + + try { + await expect( + runHook(configurationPath, { + CF_RUNNER_CACHE_ASSIGNMENT_MAX_ATTEMPTS: "5", + CF_RUNNER_CACHE_ASSIGNMENT_POLL_SECONDS: "0", + }), + ).resolves.toEqual({ code: 0, stdout: "", stderr: "" }); + expect(attempts).toBe(3); + await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await close(server); + await rm(directory, { recursive: true, force: true }); + } + }); + + it("still fails closed when the authorization never becomes visible", async () => { + const server = createServer((_request, response) => response.writeHead(401).end()); + const port = await listen(server); + const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); + const configurationPath = join(directory, "cache-assignment"); + const capability = "Bearer capability-that-must-not-leak"; + await writeFile(configurationPath, `http://127.0.0.1:${port}/v1/runner-cache\n${capability}\n`, { mode: 0o600 }); + + try { + const result = await runHook(configurationPath, { + CF_RUNNER_CACHE_ASSIGNMENT_MAX_ATTEMPTS: "2", + CF_RUNNER_CACHE_ASSIGNMENT_POLL_SECONDS: "0", + }); + expect(result).toEqual({ + code: 1, + stdout: + "::error title=Cloudflare runner cache assignment::GitHub's runner assignment was not observed within 2 seconds (last Worker status: 401).\n", + stderr: "", + }); + expect(result.stdout).not.toContain(capability); + await expect(readFile(configurationPath)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await close(server); + await rm(directory, { recursive: true, force: true }); + } + }); + it("does not start a job when the one-time cache configuration is malformed", async () => { const directory = await mkdtemp(join(tmpdir(), "runner-job-hook-test-")); const configurationPath = join(directory, "cache-assignment");