fix(runs): survive a platform kill — retry a bare StepFailed, and rebuild the checkout on every path - #134
Conversation
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.
There was a problem hiding this comment.
AI code review — 💬 Comment
Risk tier: lite · 0 critical · 1 warnings · 1 suggestions
Reviewers: security 1 · code-quality 1 · performance 0 · documentation 0
1. ⚠️ Warning — Retrying StepFailed can replay non-idempotent commands
Adding 'StepFailed' to 'retryOn' causes the configured repository command to run again for any failure classified at the workflow boundary, including ambiguous failures where the process may already have started or completed. If 'check.command:repo' performs side effects, this can duplicate deployments, mutations, or other privileged actions up to three times. Restrict this retry to a proven platform-kill classification or require commands to be idempotent before enabling it.
2. 💡 Suggestion — Update stale retry policy comment
The assertion now expects both 'ExecFailed' and 'StepFailed', but the nearby comment still describes 'retryOn: ExecFailed' as the complete policy. Update the comment to mention the new 'StepFailed' fallback so the test rationale matches the behavior.
#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.
Two halves of one mechanism, both found by the same afternoon's failures.
Problem & Insight
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 —
ExecFailedandExecTimeout— because a command that runs and exits non-zero comes back as a normalExecResultfor the run body to decide on. When the platform kills the step outright there is no EffectCauseto read a tag from:errorTagOffalls back to"StepFailed",rethrowForRetryPolicyfinds it absent fromretryOn: ["ExecFailed"], wraps it inNonRetryableError— and the one failure mode that is purely the platform's is the one the platform is never asked to retry.Observed on a consumer: a 70-second TypeScript stage died as
StepFailedafter ~80s, withcheckandoxlintgreen beside it, and no retry attempted.And #128's carve-out was wrong. It gave the staged path a checkout probe and left the single-exec path alone, reasoning that its "exposure is one step rather than five". A container can be recycled between any two durable steps, and
checkoutis a step earlier thanexecby construction — the exposure is one boundary, which every run has. This repo's own gate then died exactly that way on exactly that path:Neither is resource pressure being papered over. The consumer'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 — which is the shape a retry is the right answer to, provided the retry can actually run.
Take
RETRY_ONbecomes["ExecFailed", "StepFailed"]on all three PR runs. Neither class can reach a verdict: a red test or a lint error never fails the Effect.StepFailedhere means "the step died and left nothing behind", which is the platform by construction.ExecTimeoutstays out, deliberately: its tag survives the boundary whenever there is a Cause to read, so it arrives as itself, and a command that outran its ceiling will outrun it again.ensureWorkspaceinside its retryable step, like every other path now does — onetest -don the happy path, a clone and an install on a recycled container.Key actions
["ExecFailed"]updated to pin the pair