fix(offload-test): a stage probes for its checkout before it runs - #128
Conversation
Container disk is ephemeral, and a staged run spanning forty minutes of durable steps is long enough for the instance behind it to be recycled. The runtime already names this exactly — `working directory '<dir>' was missing at exec time — the checkout did not survive to this step (container recycled)` — and raises it as `ExecFailed`. Which is precisely what `retryOn` retries. So the platform re-ran the same 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. Every stage now probes `test -d <dir>/.git` inside its own retryable step and re-clones when the probe fails — one extra exec of about a second on the happy path, a clone and an install on a recycled container, which is what the stage needed anyway. The single-exec path is deliberately untouched: its contract is pinned byte for byte and its exposure is one step, not five. Observed on a consumer's Rust gate, where the stage that died left no log at all and three identical retries followed it.
There was a problem hiding this comment.
AI code review — 💬 Comment
Risk tier: lite · 0 critical · 2 warnings · 0 suggestions
Reviewers: security 0 · code-quality 0 · performance 1 · documentation 1
1. ⚠️ Warning — Rebuilt shared workspace is not retained for later stages
📍 runs/offload-test.ts:777-790
When 'ensureWorkspace(shared)' detects a recycled container, it acquires a replacement workspace only in the local 'ws' variable. The immutable 'shared' reference remains pointed at the missing container, so every subsequent shared stage probes the stale container and performs another clone/install. Retain the rebuilt workspace for later stages (with appropriate synchronization for concurrent stages) to avoid repeated expensive workspace setup.
2. ⚠️ Warning — README overstates checkout probing for isolated stages
The documentation says every stage runs 'test -d dir/.git' and re-clones when that probe fails, but isolated stages do not run this probe; they acquire a fresh workspace directly inside the retryable step. Clarify that probing applies to shared-stage workspaces, while isolated stages rebuild their workspace on each retry.
…ebuild carries nothing Both from pr-review. The README said every stage probes; isolated stages do not — they acquire inside the retryable step already, so a retry rebuilds by construction. And the rebuilt workspace is deliberately not written back over `shared`: on this runtime it is value-identical (`acquire` hands back the per-execution sandbox id, the clone lands at the same path), so there is nothing to carry forward and a mutable cell shared across concurrent stages would cost synchronisation for no gain.
…ble step (#130) #128 gave `offload-test`'s staged path a checkout probe and deliberately left the single-exec runs alone — their contract was pinned and their exposure was one step. The evidence disagreed within the hour: `check` posted ExecFailed: exec failed (exit -1): working directory '/workspace/<repo>' was missing at exec time — the checkout did not survive to this step (container recycled) as a red verdict on a 40-second run, with all three platform retries spent re-running the command in the same absent directory. So the probe becomes a primitive — `ensureWorkspace` beside `workspace` — and `check`, `oxlint` and `offload-test` all call it inside the step that retries. One `test -d` on the happy path; a clone and an install on a recycled container, which is what the step needed anyway. `offload-test`'s local copy is deleted in favour of the shared one. Exposure is not one step: a container can be recycled between ANY two durable steps, and the checkout is a step earlier than every exec by construction.
#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/<repo>' 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.
…uild the checkout on every path (#134) * fix(runs): a bare StepFailed is the platform, so retry it 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. * fix(offload-test): the single-exec path re-establishes its checkout too #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/<repo>' 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.
Problem & Insight
A staged run's retry policy is still a lie after #127, for a reason #127's isolation cannot reach on the current runtime.
The runtime already names the failure precisely:
Container disk is ephemeral. A staged run spanning forty minutes of durable steps is long enough for the instance behind it to be recycled, and the next stage then execs into a directory that is gone. That is raised as
ExecFailed— exactly the classretryOnretries. So the platform re-ran the same 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.#127's isolation does not fix this, because the CF runtime cannot give a run two containers.
acquire: () => Effect.succeed({ id: sandboxId })returns one id per execution, andexecroutes through aboxclient closed over at Layer build — thecontainerhandle a caller passes is not what selects the container. Turned on for five stages, all five failed in under five seconds withCheckoutFailed: they did not get five containers, they got one, and raced to wipe each other's checkout (git cloneclears its target directory first). Per-stage containers need the runtime to route by handle — a real change to the "V0 = one container per execution" design, and a separate one.Take
A stage stops assuming its workspace. It probes
test -d <dir>/.gitinside its own retryable step and re-clones when the probe fails.Isolated mode keeps acquiring per stage (unchanged from #127), so the same code path serves both once the runtime can honour it.
The single-exec path is deliberately untouched. Its contract is pinned byte for byte by the suite, and its exposure is one step rather than five — the same fix belongs there, but not in a change whose effect on other consumers cannot be observed from here.
Key actions
execby the container handle — the prerequisite forstageConcurrency > 1actually working — stays open