Skip to content

fix(sdk): harden agent-loop error recovery, retry, and pause semantics#2

Open
matej21 wants to merge 2 commits into
mainfrom
fix/agent-loop-error-recovery
Open

fix(sdk): harden agent-loop error recovery, retry, and pause semantics#2
matej21 wants to merge 2 commits into
mainfrom
fix/agent-loop-error-recovery

Conversation

@matej21

@matej21 matej21 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Fixes the three high-severity findings from the agent-SDK architecture review (CORR-1..3), plus the two related mediums they subsume (CORR-6 crash-window message loss, CORR-10 lost manual pause).

Context

Git history shows this is one bug family patched case-by-case: 1cb6a25 introduced preserve-for-retry token semantics, then b2cfd17, c2220d6 and 245b2b1 each fixed one instance of the resulting resume_from_error ↔ infer loop (empty-response, aborted, non-retryable). This PR closes the family with three invariants instead of a fifth spot patch.

Invariant 1 — error-resume cycles back off (CORR-1)

A retryable LLM error (rate limit / server error / network / timeout) that outlives withLLMRetry's 5 attempts keeps the dequeue token unconsumed, so decide() re-entered infer immediately: an unbounded, backoff-free spin that pinned the loop and sent the parent one mailbox error message per cycle during a provider outage.

  • AgentState now tracks consecutiveInferenceFailures / lastInferenceFailureAt (reducers: inference_failed increments, inference_completed resets, session_restarted resets the episode).
  • continue()'s resume_from_error branch waits out an exponential backoff (5s base, 60s cap, per-agent errorResumeBackoff config) and yields instead of spinning; a timer re-schedules processing when the backoff elapses.
  • onError hooks (parent notification) fire once per error episode, not once per cycle.
  • Capped rather than ceilinged: the preserved message keeps retrying at the max delay until the provider recovers — dropping it would lose user work.

Invariant 2 — tokens are consumed only when the turn commits (CORR-2, CORR-6)

markConsumed ran before executeAfterInference, so a plugin returning {action:'retry'} recursed into runInference with the mailbox message already consumed: a pure-message turn stranded the agent in inferring (→ idle, unrecoverable), a tool-result turn double-appended pendingMessages and produced duplicate tool_result blocks that Anthropic rejects.

  • Consumption moved to the commit points, after the retry decision is final.
  • New inference_retried event rolls back the staged turn (clears pendingMessages, like the inference_failed rollback from 1cb6a25) so the retry re-collects the same unconsumed messages — nothing lost, nothing duplicated.
  • Ordering is commit-then-consume: a crash in the gap now re-delivers the message on restart (at-least-once) instead of silently dropping it (session_restarted wipes staged pendingMessages, so consume-first lost the message).

Invariant 3 — pause is never clobbered mid-turn (CORR-3, CORR-10)

The tool_exec loop ran all pending tools without re-checking status, and the tool_completed/tool_failed/inference_completed reducers recomputed status unconditionally — a pause from a tool hook (or a manual pauseAgent() landing mid-inference) was silently overwritten, the paused tool re-executed its side effects, and the history could end with an unpaired tool_use.

  • Reducers preserve paused; the tool_exec loop stops at the first pause (remaining tools stay pending and run after resume).
  • afterToolCall pause still emits the executed tool's result event — side effects already ran, so the committed tool_use gets its tool_result and nothing re-executes on resume. (beforeToolCall pause keeps gate semantics: tool never started, runs after resume.)
  • agent_resumed routes back to tool_exec while tool calls are pending — resuming into pending would have run inference with unresolved tool calls.

Tests

New error-recovery.integration.test.ts (6 tests): outage → backoff timing between failure/resume events, single parent notification, message survives and is delivered exactly once; afterInference retry for pure-message and tool-result turns (no stranding, no duplicate tool results in history or LLM request); beforeToolCall/afterToolCall pause (nothing runs while paused, exactly-once execution across resume); manual pause during in-flight inference (pause survives the commit). Also adds TestSession.pauseAgent/resumeAgent.

bun run ts:build clean, full sdk suite 1642 pass / 0 fail.

🤖 Generated with Claude Code

https://claude.ai/code/session_01X2Dk23As1wEZ14mie22hGA

matej21 and others added 2 commits July 15, 2026 16:25
Closes three related defects in the agent loop's error/pause/retry
paths — the same resume_from_error ↔ infer family previously patched
case-by-case in 1cb6a25, b2cfd17, c2220d6 and 245b2b1:

- Retryable LLM errors that exhaust withLLMRetry no longer spin an
  unbounded resume_from_error ↔ infer loop. Error-resume cycles back
  off exponentially (5s base, 60s cap, configurable via
  errorResumeBackoff) using new AgentState bookkeeping
  (consecutiveInferenceFailures / lastInferenceFailureAt); onError
  hooks (parent mailbox notification) fire once per error episode
  instead of once per cycle. A session restart resets the episode.

- afterInference:'retry' no longer strands the agent or corrupts
  history. Dequeue-token consumption is deferred until the turn
  commits, and a new inference_retried event rolls back the staged
  turn so the retry re-collects the same messages instead of
  double-appending tool results (Anthropic rejects duplicate
  tool_result blocks). Consuming after the commit also closes the
  crash window that dropped a consumed-but-uncommitted message on
  restart: the gap now re-delivers (at-least-once) instead of losing
  the message.

- A pause raised mid-turn is no longer clobbered. The
  tool_completed / tool_failed / inference_completed reducers preserve
  'paused', the tool_exec loop stops at the first pause instead of
  running the remaining tools, afterToolCall pause still commits the
  executed tool's result (no side-effect re-execution on resume), and
  agent_resumed routes back to tool_exec while tool calls are pending.

Adds TestSession.pauseAgent/resumeAgent helpers and an
error-recovery.integration.test.ts suite covering all three paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2Dk23As1wEZ14mie22hGA
The agent-tree and agent-detail projections duplicated the status
recompute logic that the parent commit fixed in the core reducer, so a
mid-turn pause was still clobbered in the client-side view (and resume
went to 'pending' even with tool calls pending) while the server kept
the agent paused / in tool_exec. With agent_state_changed never emitted,
nothing corrected the drift.

- Extract the transition rules into projections/agent-status.ts and use
  it in both projections: preserve 'paused' on inference_completed and
  tool_completed/tool_failed, resume into tool_exec when tool calls are
  pending, and handle the new inference_retried rollback.
- Reset 'errored' agents on session_restarted (core already did; the
  projections only reset 'inferring').
- Add a conformance test that replays identical event sequences through
  the sdk core reducer and both projections and fails on any status or
  pending-tool-call divergence. The sdk cannot import the shared helper
  (project-reference cycle), so the test is what keeps the two copies in
  lockstep. It imports sdk from source (not dist) and lives outside
  shared/src so it runs under bun test without joining the tsc project.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2Dk23As1wEZ14mie22hGA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant