Share one bounded recovery budget between MAA retries and resumes (REMOTE-2269)

[warp-agent-staging]

Description

Retries and resumes now share one bounded recovery budget, and every recovery attempt is spaced by the jittered exponential backoff that already exists in the client.

Before this change, a cloud run that hit an HTTP/2 INTERNAL_ERROR reset mid-turn had an effective recovery budget of one attempt, and that attempt fired ~1s after the reset — straight back into the rolling deploy that caused it. recovery_action (app/src/ai/blocklist/controller/response_stream.rs) offered two escapes and in a cloud run both were closed:

• The 3-attempt retry budget was gated on !has_received_client_actions. In cloud runs the server's user-message-append ClientActions always arrive before the LLM call, so that branch was unreachable and retry_count stayed 0 — the budget was never reachable, not merely unused.
• The resume was gated on can_attempt_resume_on_error, and schedule_auto_resume_after_error sent the resume with can_attempt_resume_on_error = false. So the request that died was itself the one-shot auto-resume.

It therefore fell through to Fail → terminal Error → the driver's run_completion_immediate outcome=error, never reaching the TransientError arm that holds a run open.

What changed

One shared budget. A new RecoveryBudget (a small Copy value) carries attempts_used plus whether this request kind may resume at all. A retry charges it in place inside the same ResponseStream; a resume hands the charged budget to the ResumeConversation request the controller sends next, as a PendingResume (pending_resume → schedule_auto_resume_after_error → resume_conversation_with_recovery_budget). So MAX_RECOVERY_ATTEMPTS = 3 covers a request and its recoveries however they split between retries and resumes. resume_allowed = false now means only "this request kind may never resume" (the passive-request clamp), not "recovery is over" — that conflation is the defect.

Scope is one request, not one agent turn. A turn spans many MAA requests — every tool-result round trip is its own — and each starts with a fresh budget, exactly as it did when the budget was a per-stream retry_count. This change does not bound recovery across a whole turn and does not claim to.

Because the budget is sized at the pre-existing retry budget of 3, the pre-action retry path behaves exactly as before. What changes is that post-action failures get up to 3 backed-off resumes instead of one immediate one.

Backoff, reusing what exists. The jittered exponential schedule in app/src/server/retry_strategies.rs is factored into backoff_after_attempts(attempts_made); with_bounded_retry now calls it, so there is exactly one backoff schedule in the client rather than a second mechanism. Both an in-request retry (defer_retry_after_backoff) and an auto-resume wait it: ~0.5s, ~1s, ~2s plus up to 30% jitter. The resume's wait is decided with the recovery decision and carried on PendingResume rather than recomputed at send time — the schedule is jittered, so recomputing would wait a different duration than the one that was logged. A parked offline retry still waits for connectivity rather than the backoff: the backoff exists to space attempts against a struggling server, not to delay a reconnect.

recovery_action is restructured to check recoverability, then resume eligibility, then the budget, and Fail now carries a FailReason (not_recoverable / budget_exhausted / resume_not_allowed) so a terminal failure explains itself. Eligibility is checked ahead of the budget deliberately: a passive request that spent its budget on pre-action retries and then fails post-action is blocked by both, and the ineligibility is the constraint worth reporting because it would still block it with a full budget.

Logging — before and after

This is the part the report asked for, so here it is concretely. The line that started the investigation was true and still deeply misleading: retry_count=0 while the request that died was the one-shot resume and the budget was unreachable.

Before — retries and resumes logged differently, and neither said which kind of recovery it was in a machine-readable way:

MultiAgent request failed, retrying (attempt 1/3) - Error: ErrorStatus(503, ...)
MultiAgent request failed after client actions; resuming conversation after stream finishes - Error: Transport(...)
Internal error occurred at transport layer. [has_received_client_actions=true, is_recoverable=true, will_attempt_resume=false, is_online=true, retry_count=0, error_debug=Transport(...)]

After — one greppable family (MultiAgent request failed;), the same fields in the same shape for both kinds, the attempt number always read against the one shared budget, and an explicit statement of which kind of recovery ran and whether the request that failed was the original or an automatic resume of it:

MultiAgent request failed; recovering: recovery=retry  attempt=1/3 wait=512.4ms                       failed_request=original - Error: ErrorStatus(503, ...)
MultiAgent request failed; recovering: recovery=resume attempt=2/3 wait=after_stream_finished+1.02s   failed_request=original - Error: Transport(...)
MultiAgent request failed; recovering: recovery=resume attempt=3/3 wait=after_stream_finished+2.31s   failed_request=resume   - Error: Transport(...)
MultiAgent request failed; not recovering: recovery=none reason=budget_exhausted attempt=3/3 failed_request=resume - Error: Transport(...)
MultiAgent request failed; recovering: recovery=retry  attempt=1/3 wait=connectivity                  failed_request=original - Error: Transport(...)

Both retry variants log recovery=retry; wait= distinguishes a backed-off retry from one parked on connectivity, and carries the resume's real spacing so a reader can see how far apart the attempts were. Read a single line and you know the kind, the position in the shared budget, and whether it was the original request or a resume.

The Sentry/report_error! extras change to match: retry_count is replaced by recovery_attempt + max_recovery_attempts, and failed_request is added (also as a Sentry tag). recovery_attempt is passed in from the same value the log line prints, so the two surfaces can't disagree by one about the same failure. This renames a tag — any dashboard querying retry_count on these events needs updating. The rename is deliberate: retry_count no longer means "retries" now that resumes count into it, and leaving the old name would reproduce exactly the confusion this issue is about.

The AgentModeRequestRetrySucceeded telemetry keeps its own stream-local retries_sent counter rather than reading the shared budget, so its retry_count field still means retries of that request and is not inflated by inherited resume attempts.

Answers to the two questions left for review

1. Does AUTO_RESUME_TIMEOUT = 120s need raising? No. The deadline is armed by the driver's UpdatedConversationStatus handler when an attempt fails and the conversation enters TransientError, and cancelled when the next attempt lands and flips it back to InProgress (end_run_after bumps a generation counter, so re-arming replaces rather than stacks). It therefore bounds a single recovery attempt, not the whole chain. The backoff adds at most ~2.6s to any one attempt and ~4.6s across all three, a small fraction of one 120s window, so no attempt gets closer to the deadline than it does today. There is a real consequence to name, though: worst-case total recovery wall clock is now bounded by 3 × (120s + backoff) ≈ 6 minutes rather than 1 × 120s, for a request where every attempt is sent and then hangs. That is the intended cost of allowing more attempts, and it only applies to hung attempts — the reset case this fixes fails in seconds. I've documented the per-attempt semantics on the constant and in the spec, and added a test that fails if the budget or backoff schedule ever grows enough to approach the deadline.

2. Is repeating a resume unsafe? No, and the code supports the claim. A resume is AIAgentInput::ResumeConversation, which carries only context — no tool results, no replayed inputs. The conversation state it resumes from is the server's, addressed by server_conversation_token, and ResponseStream re-derives RequestParams from compute_active_tasks() at send time rather than replaying a stored request. The server cancels incomplete tool calls on the resumed request (Adding tool call cancellation message for tool call ... (incomplete tool call found), observed on the staging run in the issue), so an interrupted tool call is closed out rather than re-run. On the client side, each resume is a fresh request through send_request_input, which asserts there is no in-flight stream for the conversation and aborts any pending auto-resume handle first — so resumes cannot overlap or stack. Two protections that already bounded repetition still apply: a user message aborts the pending resume (so the user always wins the race), and passive background requests never resume at all. The one thing repetition does cost is server turns, which is why each attempt is backed off rather than immediate.

Out of scope, as requested

• has_received_client_actions is not narrowed to "a side-effecting action was applied". Worth noting a side effect of this change, though: because a retry resets that flag for the new attempt, a chain can now legitimately be retry → (actions stream) → resume → (pre-action failure) → retry, so the pre-action retry branch becomes reachable within a recovery chain on a cloud run in a way it previously was not. That is the shared budget working as intended and is bounded by the same counter; it does not change what the flag means or when it flips.
• The checkpoint snapshot_state.json failure in the same log is untouched (REMOTE-2557).

Linked Issue

REMOTE-2269 — https://linear.app/warpdotdev/issue/REMOTE-2269/allow-multiple-resume-attempts

Reported by @seemeroland in Slack.

This raises a budget that REMOTE-1894's shipped spec deliberately promised as "at most one automatic resume", so specs/REMOTE-1894/PRODUCT.md (I2, I9, I10, I11, I13, I14) and TECH.md are updated in the same change rather than left contradicting the code.

Testing

New unit tests in app/src/ai/blocklist/controller/response_stream_tests.rs, on top of the reworked recovery_action matrix:

• resume_failures_consume_the_shared_budget — a resume failure charges the budget, so repeated post-action resets get 3 resumes and then Fail(BudgetExhausted), not 1 resume as before.
• retries_and_resumes_share_one_budget — walks the exact REMOTE-2269 sequence (pre-action retry → post-action resume → resume → exhausted) and asserts the fourth failure is terminal whichever kind of recovery it would have used, pinning that the counter is shared and not per-kind.
• a_scheduled_resume_inherits_a_charged_budget — the boundary the controller consumes: the budget handed to a scheduled resume is charged one attempt and carries resume eligibility over unchanged. A fresh budget here would restart recovery from scratch; a dropped resume_allowed would silently re-enable resumes for a passive request.
• spending_an_attempt_preserves_resume_eligibility — charging an attempt must not quietly flip eligibility in either direction.
• ineligibility_is_reported_ahead_of_an_exhausted_budget — when both constraints bind, the reported reason is the one that would still bind with a full budget.
• the_recovery_backoff_fits_inside_the_cloud_run_recovery_window — ties the backoff schedule to AUTO_RESUME_TIMEOUT, so question 1's answer breaks loudly if the budget or backoff grows.
• The existing matrix is updated for the new signature, including post_action_recoverable_failures_resume, which previously asserted that "the in-request retry budget is irrelevant once actions have executed" — that assertion is exactly what this change reverses.

Validation. Full CI is green on this branch: run 32201774179 — clippy on Linux/macOS/Windows/wasm, Verify compilation with release flags on all four targets and with eval features, and the test suites on Linux (10893 passed / 0 failed), macOS and Windows. Each new test is confirmed passing by name in the Linux log.

Two caveats on how that was obtained, since they affect how you read the checks on this PR:

• CI skips the compile/clippy/test jobs on draft PRs by design, so the checks shown on the PR itself are the cheap ones. The green run above was dispatched manually via workflow_dispatch against this branch at 1d8b634; marking the PR ready for review will re-run it as PR checks.
• I could not compile locally: the sandbox has a hard ~4GB memory cap and rustc is OOM-killed on the warp crate even at -j 1 with debug info off. Only cargo fmt --check ran locally. That is why the dispatched run matters — and it earned its keep, catching a private_interfaces error under -D warnings (RecoveryBudget was pub(crate) while the pub fn new taking it is pub) that formatting alone could never have surfaced.

No manual ./script/run testing: this is headless cloud-run recovery behavior with no UI surface, and reproducing it needs a mid-stream transport reset (the TransportReset LLM mock REMOTE-1894 used against oz-local), which the sandbox can't build a client for.

• I have manually tested my changes locally with ./script/run

Agent Mode

• Warp Agent Mode - This PR was created via Warp's AI Agent Mode

[warp-agent-staging]

This PR was generated with Warp.

Comment @warp-factory on this PR to send it follow-up work.

 

[warp-agent-staging]

Overview

Shares one bounded recovery budget between MAA retries and resumes with backoff, so a stream reset during a server deploy no longer kills a cloud run on the second failure. An adversarial review pass found no correctness, security, or build defect and verified the per-attempt deadline claim against the code; its findings have all been addressed, and one question is left for your judgment.

Concerns

• Coverage of the charge-crossing wiring rests on unit tests plus code reading. a_scheduled_resume_inherits_a_charged_budget pins the budget boundary the controller consumes, but the async path recovery_budget_for_resume → schedule_auto_resume_after_error → resume_conversation_with_recovery_budget → ResponseStream::new has no end-to-end test, and the REMOTE-1894 e2e that exercises it was not re-run because this sandbox cannot build a client. Your call whether that e2e should be re-run somewhere with a build before this merges, given it is the sole cloud-recovery path.

Verdict

Checks: build pass, tests pass (10893 passed / 0 failed), CI green (run 32201774179), visual proof n/a

Note that ci.yml skips compile, clippy and test for draft PRs, so the checks displayed on this PR are only the cheap ones; the green signal above is a dispatched run against the branch head. Marking the PR ready re-runs it as PR checks.

Found: 0 critical, 0 important, 0 suggestions, 0 nits, 1 question. Review findings (1 important, 4 suggestions, 3 nits) were addressed in 1d8b634 before this was posted.

Responding as wilson: Open session · View factory task