Stop premature sandbox aborts and offer retry on late failures - #1026
Merged
Conversation
Sandbox creation was capped at three minutes, duplicated as a literal in all five spawn-*-worker modules. Under sandbox-creation rate limiting that deadline fires while creation is still queued: the run is marked failed, and then the sandbox comes up anyway and the agent posts its work minutes later. The roomote nightly run 3734 failed at exactly 3m00s and its assistant output arrived 12 minutes after; 4 of 42 failed runs that week did the same, leaving users an error banner on a finished task. Creation now waits ten minutes, from one shared constant so it can be tuned once rather than five times. A slow start still renders the startup sequence; a premature abort throws away real work. A run that fails before any output shows the startup sequence's retry, but one whose sandbox came up late lands in the historical view — which had no way to continue a dead thread. That view now offers the same retry, reusing the existing retryFailedStart command and hook, with the eligibility predicate lifted into a shared module so the two views cannot drift. Note the relaunch allowlist is unchanged: github_pr_review_sync (run 3734's kind) stays excluded, since relaunching a review that already posted would post it twice.
Contributor
|
No code issues found. See task
Reviewed f7819cb |
Removing the local eligibility predicate left RunStatus imported as a value with only its type alias still used. Caught by CI's oxlint, which this repo lints with — not the eslint I checked locally.
Review caught that enqueueTaskRelaunch rejects any failed run carrying a non-kickoff message — which is exactly the late-transcript case the button was written for, so it could only ever raise an error toast. Verified against the real run: 3734's messages carry source 'pr_review_notification', which trips that guard. The guard is right, and the button was wrong: relaunching a run whose work already landed would redo it, posting a second review on the PR. Retry is now offered only for a failed start with an empty transcript. The gate is on transcript emptiness rather than message source, which is stricter than the server — it can hide the button for a kickoff-only run (where the startup view still offers one) and can never show a button that fails. The late-transcript case keeps its real fix: the ten-minute creation deadline, which stops the run from being declared failed at all.
The previous gate hid Retry for chat-launched runs whose transcript holds only the provider kickoff message — history the relaunch command deliberately accepts — so a Slack-started failed run lost its recovery control in this view. Both client approximations are gone. task-run-queue exports canRetryFailedStart, sharing the exact non-kickoff-message check with enqueueTaskRelaunch, and the sandbox session decorates its run with the verdict the same way it already decorates prRepo/prNumber. The failure message just reads it, so the button appears exactly where the command would accept it and nowhere else. Startup.tsx keeps its own predicate and is byte-identical to develop again: its runs come from SSE and initial props that carry no server verdict, and its behaviour was never in question.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the failure behind task 1e3wtt1oxrofl on the nightly.
What was happening
Sandbox creation was capped at three minutes (
createInstanceTimeoutMs: 180_000, duplicated as a literal in all fivespawn-*-workermodules). Under sandbox-creation rate limiting that deadline fires while creation is still queued, so the run is markedfailed— and then the sandbox comes up anyway and the agent posts its work minutes later.Measured on the nightly: run 3734 failed at exactly 3m00s after
provision_started_at, and its assistant output arrived at 13:30, twelve minutes after the abort. A sibling run the same afternoon did the same eight minutes late, and a third failed outright withSandbox creation rate limit exceeded. Over seven days, 4 of 42 failed runs produced real work after being declared failed — each one leaving the user an error banner sitting on top of a completed task whose thread is dead.Changes
1. Creation deadline 3 min → 10 min, moved into a single shared
timeouts.tsso it can be tuned once instead of five times. Bootstrap stays at 2 min — it runs after the provider has already returned the machine, so it is not subject to creation-queue contention. Waiting longer is the cheaper mistake: a slow start still shows the startup sequence, while a premature abort discards finished work.2. Retry in the historical view. A start that fails before producing output shows the startup sequence's retry button; a run whose sandbox came up late lands in the historical view instead, which had no way to continue. It now offers the same retry, reusing the existing
retryFailedStartcommand anduseRetryFailedTaskStarthook. The eligibility predicate moved to@/lib/task-run-retryso the two views cannot drift apart.Deliberately not changed
The relaunch allowlist. Run 3734's payload kind is
github_pr_review_sync, which is not relaunchable — so this PR does not put a retry button on that specific task. Adding it would mean a retry re-runs a review that may have already posted, double-commenting on the PR. Worth deciding separately; the timeout fix is what prevents that class of task from failing in the first place.Tests
Controller compute-providers 86 passed; startup 9 passed; HistoricalContent 8 passed including two new cases — a retryable failed start renders the button and fires the mutation with the right task/run ids, and a
snapshot_resumefailure renders no button (it has its own resume path). Repo type-check clean.