From 9983492ef0312a77b1e1070648adb3eb06d3a106 Mon Sep 17 00:00:00 2001 From: debuggingfuture Date: Sat, 15 Aug 2026 21:42:36 +0800 Subject: [PATCH 1/2] fix(runs): a bare StepFailed is the platform, so retry it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry policy was inert on the failure it was written for. A step body in these runs can fail in exactly two typed ways — `ExecFailed` and `ExecTimeout` — because a command that RUNS and exits non-zero comes back as a normal `ExecResult`. When the platform kills the step outright, no Effect `Cause` survives the Workflow boundary, `errorTagOf` falls back to `"StepFailed"`, and `retryOn: ["ExecFailed"]` classified that as non-retryable. The one failure mode that is purely the platform's was the one the platform was never asked to retry. Observed on a consumer: a 70-second TypeScript stage died as `StepFailed` after ~80s with the two fast checks green beside it, and no retry was attempted. Not resource pressure being papered over — that same gate's heaviest stage peaks at 2.2 GiB of 11.9 GiB with 8.4 GB of disk free, and its deaths land at 80s, 137s, 647s and 1284s against successes at 666s, 2128s and 2176s. No resource is scarce and no duration is safe. `ExecTimeout` stays out, deliberately: its tag survives the boundary whenever there is a Cause to read, so it arrives as itself rather than as `StepFailed`, and a command that outran its ceiling will outrun it again. --- runs/check.test.ts | 2 +- runs/check.ts | 12 ++++++++++-- runs/offload-test.test.ts | 8 ++++---- runs/offload-test.ts | 27 ++++++++++++++++++++++++++- runs/oxlint.test.ts | 2 +- runs/oxlint.ts | 12 ++++++++++-- 6 files changed, 52 insertions(+), 11 deletions(-) diff --git a/runs/check.test.ts b/runs/check.test.ts index d5fe9e5..feb65cb 100644 --- a/runs/check.test.ts +++ b/runs/check.test.ts @@ -121,7 +121,7 @@ describe("check", () => { const execStep = handles.executions.steps.find((s) => s.name === "exec"); expect(execStep?.metadata?.["stepOpts.timeoutSec"]).toBe(1800 + 120); expect(execStep?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); }).pipe(Effect.provide(layer)); }, ); diff --git a/runs/check.ts b/runs/check.ts index 2b0a0a8..fd5fa29 100644 --- a/runs/check.ts +++ b/runs/check.ts @@ -162,9 +162,17 @@ const DEFAULT_TIMEOUT_SEC = 600; */ const STEP_TIMEOUT_HEADROOM_SEC = 120; -/** Platform-failure retries — see the `exec` step. A verdict is never retried. */ +/** + * Platform-failure retries — see the `exec` step. A verdict is never retried: + * a command that runs and exits non-zero is a normal `ExecResult`, so neither + * class here can reach one. + * + * `StepFailed` is included because a platform kill leaves no Effect `Cause` to + * read a tag from, and the runner falls back to that name — so listing only + * `ExecFailed` left the purely-platform failure as the one thing not retried. + */ const PLATFORM_RETRIES = 3; -const RETRY_ON = ["ExecFailed"] as const; +const RETRY_ON = ["ExecFailed", "StepFailed"] as const; /** CONFIG_KV key — strictly per-repo (see header: no global fallback). */ const commandKey = (repo: string): string => `check.command:${repo}`; diff --git a/runs/offload-test.test.ts b/runs/offload-test.test.ts index 3e3b306..fba3f11 100644 --- a/runs/offload-test.test.ts +++ b/runs/offload-test.test.ts @@ -126,7 +126,7 @@ describe("offload-test", () => { yield* offloadTest.run(baseInput); const execStep = handles.executions.steps.find((st) => st.name === "exec"); expect(execStep?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); }).pipe(Effect.provide(layer)); }); @@ -232,7 +232,7 @@ describe("offload-test", () => { // raised by the engine, so `retryOn` cannot gate it: a wedged exec is // replayed for the whole budget. expect(execStep?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); }).pipe(Effect.provide(layer)); }, ); @@ -725,7 +725,7 @@ describe("offload-test staged mode", () => { const execWorkspace = handles.executions.steps.find((s) => s.name === "exec-workspace"); expect(execWorkspace?.metadata?.["stepOpts.timeoutSec"]).toBe(900 + 120); expect(execWorkspace?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execWorkspace?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execWorkspace?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); // The suspicious labelled-key-missing fallback is recorded on the // stage's step metadata — `workspace` resolved its own key, so only // `features` is flagged. @@ -1129,7 +1129,7 @@ describe("offload-test isolated stages", () => { // The retry contract is unchanged — it is the UNIT that changed. const execA = handles.executions.steps.find((s) => s.name === "exec-a"); expect(execA?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execA?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execA?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); expect(result.exitCode).toBe(0); expect(result.durationMs).toBe(300); diff --git a/runs/offload-test.ts b/runs/offload-test.ts index 4b65956..cbee483 100644 --- a/runs/offload-test.ts +++ b/runs/offload-test.ts @@ -250,7 +250,32 @@ const STEP_TIMEOUT_HEADROOM_SEC = 120; const PLATFORM_RETRIES = 3; -const RETRY_ON = ["ExecFailed"] as const; +/** + * The classes a stage step retries — both of which are the PLATFORM, never a + * verdict. + * + * `ExecFailed` is the obvious one: the command could not run. `StepFailed` is + * the one that was missing, and its absence made the whole retry policy inert + * on the failure it was written for. + * + * A step body here can fail in exactly two typed ways — `ExecFailed` and + * `ExecTimeout` — because a command that RUNS and exits non-zero comes back as a + * normal `ExecResult`. When the platform kills the step outright, no Effect + * `Cause` survives the Workflow boundary, `errorTagOf` falls back to + * `"StepFailed"`, and a `retryOn` listing only `ExecFailed` classified that as + * non-retryable — so the one failure mode that is purely the platform's was the + * one the platform was never asked to retry. + * + * Observed: a 70-second TypeScript stage died as `StepFailed` after ~80s with + * `check` and `oxlint` green beside it, and no retry was attempted. The same + * consumer's heaviest stage peaks at 2.2 GiB of 11.9 GiB with 8.4 GB of disk + * free, so this is not resource pressure being papered over. + * + * `ExecTimeout` stays OUT, deliberately. Its tag survives the boundary intact + * whenever there is a Cause to read, so it lands here as itself rather than as + * `StepFailed` — and a command that outran its ceiling will outrun it again. + */ +const RETRY_ON = ["ExecFailed", "StepFailed"] as const; const stepTimeoutFor = (execTimeoutSec: number): number => execTimeoutSec + STEP_TIMEOUT_HEADROOM_SEC; diff --git a/runs/oxlint.test.ts b/runs/oxlint.test.ts index 60d6a7c..56750ea 100644 --- a/runs/oxlint.test.ts +++ b/runs/oxlint.test.ts @@ -289,7 +289,7 @@ describe("oxlint source determinism", () => { // oxlint exiting non-zero is a normal ExecResult decided by the run body, // so `retryOn: ExecFailed` can only ever cover the container. expect(execStep?.metadata?.["stepOpts.retries"]).toBe(3); - expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed"]); + expect(execStep?.metadata?.["stepOpts.retryOn"]).toEqual(["ExecFailed", "StepFailed"]); }).pipe(Effect.provide(layer)); }); }); diff --git a/runs/oxlint.ts b/runs/oxlint.ts index 13ce53e..aa95c86 100644 --- a/runs/oxlint.ts +++ b/runs/oxlint.ts @@ -95,9 +95,17 @@ const OxlintOutput = Schema.Struct({ /** Default `exec` timeout — lint is fast, so a tighter ceiling than tests. */ const TIMEOUT_SEC_DEFAULT = 300; -/** Platform-failure retries — see the `exec` step. A verdict is never retried. */ +/** + * Platform-failure retries — see the `exec` step. A verdict is never retried: + * a command that runs and exits non-zero is a normal `ExecResult`, so neither + * class here can reach one. + * + * `StepFailed` is included because a platform kill leaves no Effect `Cause` to + * read a tag from, and the runner falls back to that name — so listing only + * `ExecFailed` left the purely-platform failure as the one thing not retried. + */ const PLATFORM_RETRIES = 3; -const RETRY_ON = ["ExecFailed"] as const; +const RETRY_ON = ["ExecFailed", "StepFailed"] as const; export const oxlint = defineRun({ name: "oxlint", From e9a4def76932bbe63434d27a9c5c2167e540558c Mon Sep 17 00:00:00 2001 From: debuggingfuture Date: Sat, 15 Aug 2026 21:47:55 +0800 Subject: [PATCH 2/2] fix(offload-test): the single-exec path re-establishes its checkout too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #128 covered the staged path and left this one alone, reasoning that a single-exec run's "exposure is one step rather than five". Wrong twice over: a container can be recycled between ANY two durable steps, and `checkout` is a step earlier than `exec` by construction — so the exposure is one BOUNDARY, which every run has, staged or not. This repo's own gate then died exactly that way, on this exact path: `working directory '/workspace/' was missing at exec time — the checkout did not survive to this step (container recycled)`. Same primitive, same placement: one `test -d` on the happy path, a clone and an install on a recycled container. --- runs/README.md | 5 +++-- runs/offload-test.test.ts | 26 +++++++++++++++++++++++++ runs/offload-test.ts | 40 ++++++++++++++++++++++++++++++--------- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/runs/README.md b/runs/README.md index 99810e9..a1fbe95 100644 --- a/runs/README.md +++ b/runs/README.md @@ -276,8 +276,9 @@ command in the same missing directory three times and reported a failure about a missing directory rather than anything about the code. The retry could never have worked: the thing it needed was the thing that was gone. -So **every** PR run — `offload-test`'s shared-container stages, `check`, and -`oxlint` — calls the `ensureWorkspace` primitive inside its retryable step: it +So **every** PR run and every path within it — `offload-test` staged and +single-exec, `check`, and `oxlint` — calls the `ensureWorkspace` primitive +inside its retryable step: it probes `test -d /.git` and re-clones when the probe fails. On the happy path that is one extra exec of about a second; on a recycled container it is a clone and an install, which is what the step was going to need anyway. diff --git a/runs/offload-test.test.ts b/runs/offload-test.test.ts index fba3f11..6101e57 100644 --- a/runs/offload-test.test.ts +++ b/runs/offload-test.test.ts @@ -658,6 +658,32 @@ describe("offload-test", () => { // earlier stage's log; a stage step that dies gets a one-line marker uploaded // under its log name. ABSENT key → the 1.1.0 behaviour, byte-identical — the // unstaged tests above are the pin for that. +describe("offload-test single-exec", () => { + it.effect("a recycled container is re-cloned rather than retried into", () => { + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { + // The probe answers non-zero: the container was recycled between + // `checkout` and `exec`, which is one boundary every run has. + "test -d /workspace/name/.git": { exitCode: 1 }, + "pnpm test": { exitCode: 0 }, + }, + }); + + return Effect.gen(function* () { + const result = yield* offloadTest.run(baseInput); + + // TWO clones: the run's own checkout, then the rebuild inside the exec + // step. #128 left this path uncovered on the grounds that its "exposure + // is one step"; the exposure is one BOUNDARY, and every run has one. + expect(handles.sandbox.clones).toEqual([ + { repo: "owner/name", sha: "abc123" }, + { repo: "owner/name", sha: "abc123" }, + ]); + expect(result.exitCode).toBe(0); + }).pipe(Effect.provide(layer)); + }); +}); + describe("offload-test staged mode", () => { // Webhook-shaped input — staged mode only exists on the path that omits // `command` (the resolve step is where the stages key is read). diff --git a/runs/offload-test.ts b/runs/offload-test.ts index cbee483..afdb1ae 100644 --- a/runs/offload-test.ts +++ b/runs/offload-test.ts @@ -1163,15 +1163,37 @@ export const offloadTest = defineRun({ const result = yield* step( "exec", () => - sandbox.exec({ - cwd: soleWorkspace.dir, - container: soleWorkspace.container, - command: soleCommand, - // Per-dispatch `env` wins over a same-named config-store secret — - // the more specific source overrides the global one. - env: { ...secretEnv, ...input.env }, - timeoutSec, - }), + // The checkout is re-established INSIDE the retryable step, for the + // same reason the staged path does it: a container recycled between + // `checkout` and here takes the checkout with it, and the retry would + // otherwise re-run the command in a directory that is gone. + // + // #128 left this path alone, reasoning that a single-exec run's + // "exposure is one step rather than five". That was wrong twice over. + // A container can be recycled between ANY two durable steps, and + // `checkout` is a step earlier than this one by construction — so the + // exposure is one BOUNDARY, which every run has. This repo's own gate + // then died exactly that way: `working directory + // '/workspace/' was missing at exec time`. + ensureWorkspace({ + current: soleWorkspace, + repo: input.repo, + sha: input.sha, + ...(input.image !== undefined ? { image: input.image } : {}), + install, + }).pipe( + Effect.flatMap((ws) => + sandbox.exec({ + cwd: ws.dir, + container: ws.container, + command: soleCommand, + // Per-dispatch `env` wins over a same-named config-store secret + // — the more specific source overrides the global one. + env: { ...secretEnv, ...input.env }, + timeoutSec, + }), + ), + ), { timeoutSec: stepTimeoutFor(timeoutSec), retries: PLATFORM_RETRIES, retryOn: RETRY_ON }, );