Skip to content

fix(sdk): pause an agent after three consecutive failed inferences - #68

Merged
matej21 merged 4 commits into
mainfrom
fix/no-reset-permanently-failed-agents
Nov 11, 2026
Merged

matej21 merged 4 commits into
mainfrom
fix/no-reset-permanently-failed-agents

Conversation

@matej21

@matej21 matej21 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

Why

In a long-lived production session (15 days, 15 session_restarted events from sandbox resumes), two coding sub-agents failed with the provider error "Provider returned error". One failed after a series of page snapshots with images, the other after reading a large JPEG. Both failed on a tool-result turn. From then on, every request with their conversation failed the same way.

checkRecoveryNeeded puts every inferring or errored agent in resetAgentIds, and the session_restarted reducer resets them. A failure drops the turn's message but keeps its pending tool results. So on every restart both agents went back to pending, rebuilt the same tool-result turn, failed again, and each failure woke the parent with "Agent encountered an error" (a cold, ~48k-token orchestrator prompt). In total: 31 failed inferences and 37 orchestrator calls with no result.

The existing failure streak (consecutiveInferenceFailures) could not catch this, because session_restarted resets it to start a fresh backoff episode. With one failure per restart it never grew past 1–2.

What

  • inference_failed gets an optional errorType, the LLMError type (llmErrorTypeSchema in core/llm/schema.ts; LLMError.type now uses it).
  • A new AgentState.consecutiveRejectedTurns holds non-retryable failures of a turn whose input survives the failure:
    • the reducer increments it on inference_failed only when errorType is non-retryable (invalid_request, context_length) and the agent still has pending tool results;
    • a rejected message is consumed by the failure, so it never counts, and neither do retryable failures or older events without errorType;
    • inference_completed and agent_conversation_spliced clear it, while session_restarted and agent_resumed leave it as it is.
  • Agent.runInference: once consecutiveRejectedTurns reaches 3 (MAX_CONSECUTIVE_REJECTED_TURNS, a static constant like MAX_INFERENCE_RETRIES), the agent emits agent_paused with reason 'limit' and the message The provider rejected this turn 3 times in a row and the agent will not retry it on its own. Last error: …. It then runs the onPause hooks instead of onError.
  • A paused agent is not in resetAgentIds (no change to checkRecoveryNeeded was needed). The mailbox onPause hook sends the parent one <child-paused agent="…"> message with the error. No further restart touches the agent.
  • emitHandlerPause(message) becomes emitPause(reason, message). Existing callers pass 'handler'.
  • consecutiveInferenceFailures, the error-resume backoff and the retryable-failure notifications are unchanged.

Why a pause, not a new status

The existing pause already does what is needed. Restarts do not reset it, decide() returns paused, the parent is notified once through onPause, and the parent's system prompt already explains <child-paused> and resume_agent. A new AgentStatus would have to be mirrored in the shared projections, the conformance test, the protocol status, the CLI and the debug UI. Reason 'limit' is documented as "agent hit a hard limit" and no emitter used it, so its enum and the agentStopped notification enum stay unchanged.

Behaviour changes

  • The 3rd rejection of the same tool-result turn pauses the agent. It stays paused through restarts and new messages until resume_agent / agents.resume.
  • Rejected messages never pause. An agent whose messages the provider rejects (a user retrying an oversized image) answers the next valid message, as before.
  • After a resume the agent gets one more try. It retries the kept turn once. If that is rejected too, it pauses again and the parent gets one more <child-paused>. A retryable failure after a resume does not pause it.
  • The alert kind changes. The final failure arrives as agentStopped kind: 'paused', reason: 'limit', not kind: 'errored'. A consumer that alerts only on errored does not see it.
  • Old logs are not counted. A session log written by an older SDK has no errorType, so its existing failures do not count. An agent that is already a zombie pauses after 3 more rejected restarts under this version.

Tests

  • session.test.ts › "a rejected turn across restarts":
    • after 1 or 2 rejections the agent is in resetAgentIds, it is pending after the restart, the count is kept, and the backoff counter is reset as before;
    • after the 3rd rejection plus the pause it is not in resetAgentIds and stays paused;
    • retryable failures, failures without errorType and rejected messages (no pending tool results) do not count;
    • the count survives a resume and is cleared by a committed turn.
  • error-recovery.integration.test.ts › "an agent the provider keeps rejecting". Each case uses real restarts: a new TestHarness over the same event store plus loadAllSessions.
    • The rejected child is paused once. A child whose tool-result turn is always rejected fails once per load and is paused on the 3rd failure. A 4th load does not call it (4 LLM calls in total). The parent gets 2 "Agent encountered an error" messages and exactly 1 <child-paused> message with the provider error. The pausing load emits exactly one agentStopped for the child, paused / limit.
    • A resume gets one more try. resumeAgent on the paused child retries the turn once, pauses again and sends one more <child-paused>.
    • An outage after a resume is not a rejection. When the resumed child hits an outage it ends errored, not paused, and the next restart retries the turn successfully.
    • Rejected messages never pause. A root agent gets 3 rejected messages, then a valid one, and answers the valid one without pausing (4 LLM calls). This fails on the round-1 head.
  • bun run ts:build: exit 0.
  • bunx tsc -p packages/sdk/tsconfig.test.json --noEmit: exit 0.
  • bun run lint: clean.
  • bun run test: 2183 pass, 89 skip, 0 fail.
  • I also replayed the production session's exported event log locally through the new reducer. The script was not committed and no data is in the repo. It marks the two agents' failures as non-retryable and simulates the pause. Every one of their failures had pending tool results. Both agents pause on their 3rd failure, which falls right after restart feat(sdk): resolve ServiceConfig.cwd lazily via callback #1 and restart Parent is told to resume a paused child but has no resume tool #5 respectively. They are then absent from resetAgentIds at every later restart (14 and 10 later restarts, all of which reset them in the recorded log). 24 of the 31 failed inferences and 24 parent error notices would not have happened.

Not verified

  • Whether the production failures were non-retryable. The event only records "Provider returned error". The failures that consumed the mailbox show the non-retryable path. The ones after restarts show nothing either way, and the replay assumes they were non-retryable. feat(sdk): keep the provider's error response in inference_failed #69, which keeps the provider error body, will show this. From now on errorType records it too.
  • The real provider cause (image size or count is a hypothesis).

Status

  • Review round 1 (34915d2): changes, 1 major, 2 minor, 2 nits. All addressed in 49576ef (see the review-response comment).
  • Rebased on main after feat(sdk): keep the provider's error response in inference_failed #69 (aacd1b7). Conflicts resolved: inference_failed keeps both providerError and errorType. Fully re-verified after the rebase.
  • Review round 2 (03c0541): approve, 1 minor. Fixed in 09ec260 (see the review-response comment). Still on the current main, no rebase needed.
  • Checks on 09ec260: bun run ts:build exit 0, bunx tsc -p packages/sdk/tsconfig.test.json --noEmit exit 0, bun run lint clean, bun run test 2192 pass, 89 skip, 0 fail.
  • CI: green on 09ec260.
  • Left before merge: green CI, merge.

🤖 Generated with Claude Code

@matej21

matej21 commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

Review round 1

  • Reviewer: Claude Opus 5.5, independent review agent
  • Commit: 34915d2
  • Verdict: changes

I checked the diff against the description, read the consumers of consecutiveInferenceFailures, pauseReason and agent_paused (reducers, checkRecoveryNeeded, decide(), Session.resumeAgent, agents.resume, releaseHold, the mailbox and agent-status hooks, limits-guard, the shared projections, and the debug Dashboard and AgentDetail pages). I ran error-recovery.integration.test.ts and session.test.ts: 53 pass. I also ran one scratch scenario against this head and against the base 3f2dc5847. It is described under the major finding and was not committed.

Major

1. The pause also fires when the failing input was already dropped, so an agent that used to recover on the next message now stops answering. packages/sdk/src/core/agents/agent.ts:932

A non-retryable failure consumes the dequeued plugin messages (agent.ts:911-913). When the rejected input was a mailbox or user-chat message, the turn that failed is gone: the next message builds a different, valid request. On main, an errored agent with new queued input goes to resume_from_error and recovers. With this PR, three such failures in a row (no committed turn between them, which is what resume_from_error gives) pause the agent. A paused agent ignores new input (decide() returns paused, and pauseResumesOnInput is not set), so only an explicit agents.resume brings it back.

Reproduced with the test harness: the entry agent gets three user messages that the provider rejects (invalid_request), then a plain text message.

after 3 rejected messages after the valid message LLM calls
base 3f2dc5847 errored pending, answered 4
this PR paused (limit) still paused, not called 3

For a root agent nobody is told: the mailbox onPause returns early when there is no parent. In a chat UI it looks like a dead session. The trigger is ordinary: a user retries the same oversized image three times. For a child, the parent now has to call resume_agent where it used to just send another message.

The mechanism this PR targets is narrower. A restart re-runs a failed turn only when its input survives the failure, and the only such input is pendingToolResults (hasWork(), agent.ts:2084). Mailbox tokens are consumed on a non-retryable failure. Suggested fix: pause only when the failed turn will be rebuilt unchanged, i.e. the post-failure state still hasWork() (pending tool results). The production case is covered by that, because both agents failed on a tool-result turn. Please add a test for the root-agent / dropped-message case either way.

Minor

2. Retryable failures count toward the pause threshold. agent.ts:929-932

The streak now survives restarts and grows by one per error-resume cycle during an outage (one cycle per 60 s at the cap). After an outage of any length, the first non-retryable failure pauses the agent at once: 2 retryable failures plus 1 non-retryable is enough. One transient 400 after an outage (an aggregator returning "Provider returned error" for an upstream issue) used to leave the agent errored, so the next restart or the next message retried it. Now it needs a manual resume. The description lists this as intended. I still think counting only non-retryable failures toward the pause is a better fit for "the provider keeps rejecting this conversation". Finding 1's hasWork() condition narrows this case too.

3. No test covers resuming a limit-paused agent. session.test.ts / error-recovery.integration.test.ts

The description says that resume_agent / agents.resume keeps the streak, and that the next non-retryable failure pauses the agent again and notifies the parent once more. The code does that: agent_resumed does not touch consecutiveInferenceFailures, the state goes to pending, and the next failure has failures >= 3. No test pins it, and neither of the two other listed side effects is pinned: no second parent notice for a retryable failure after a restart, and a backoff that continues from the kept count. One integration case for the resume would cover the part most likely to regress.

Nit

4. Two comments now say the opposite of the code. agent.ts:351 still says "a session restart resets the episode", but the restart now keeps the streak and clears only the timestamp. core/agents/state.ts:266 documents consecutiveInferenceFailures as driving the backoff only. It now also drives the pause.

5. The alert signal for the final failure changes kind. plugins/agent-status/plugin.ts

The failure that pauses skips onError, so agentStopped arrives as kind: 'paused', reason: 'limit' instead of kind: 'errored'. A consumer that alerts only on errored loses this event. The new kind is arguably the more accurate one. It is worth a line in the description for anyone consuming the notification.

Checked, no finding

  • Outage retry loop: retryable errors never pause, tokens are preserved, and resume_from_error plus the capped backoff are unchanged. After a restart the agent is pending, so the first attempt goes straight to infer without a backoff.
  • Reusing 'limit': it had no emitter. Nothing branches on the reason value: the debug UI prints pauseMessage (falling back to the reason), the shared projections copy it, and limits-guard pauses with 'handler'. releaseHold resumes only pauseResumesOnInput agents, so it does not touch this pause. A new status would have been the bigger change, and I agree with the choice.
  • Manual resume: both Session.resumeAgent and agents.resume accept paused. The agent resumes into pending (there are no pending tool calls after a failed inference) and retries the kept tool results.
  • Old logs replaying: the streak resets on inference_completed, so only agents that are currently failing carry a high count. An idle agent whose input was dropped goes pending → idle on restart and is not paused, apart from finding 1 when a new message fails.

@matej21

matej21 commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

Review response, round 1

Everything is addressed in 49576ef.

1. Major: the pause fired when the failing input had already been dropped. Fixed in 49576ef.
The pause now counts only turns that a restart rebuilds unchanged:

  • inference_failed records errorType (the LLMError type, a new optional field).
  • The reducer keeps consecutiveRejectedTurns. It is incremented only by a non-retryable failure of a turn that still has pending tool results at the moment it fails.
  • A rejected message is consumed by the failure, so it never counts. Neither do retryable failures or events without a type.
  • A committed turn or a conversation splice clears the count. A restart or a resume keeps it.
  • The agent pauses on the 3rd such failure.

This is the criterion you suggested, applied per failure instead of at pause time. Checking hasWork() only on the 3rd failure would still let 2 rejected messages plus 1 rejected tool-result turn pause the agent. Counting per failure needs the same tool results to be rejected 3 times.

New test: a root agent gets 3 messages that are rejected (invalid_request), then a valid one. It answers the valid one, makes 4 LLM calls and never pauses. On the round-1 head this test fails.

2. Minor: retryable failures counted toward the 3. Fixed in 49576ef.
They no longer count (see 1). This also made the restart change unnecessary. session_restarted resets consecutiveInferenceFailures again, as on main. So the two side effects listed in round 1 are gone:

  • a retryable failure after a restart notifies the parent again, as before;
  • the backoff after a restart starts fresh, as before.

A reducer test pins both: after a restart, consecutiveInferenceFailures is 0 and lastInferenceFailureAt is cleared.

3. Minor: no test covered resuming a paused agent. Fixed in 49576ef.

  • New integration test: resumeAgent on the paused child retries the kept turn once, is rejected, pauses again, and sends exactly one more <child-paused> (5 LLM calls, 2 pause events).
  • New reducer test: the count survives agent_resumed and is cleared by the next committed turn.

4. Nit: two stale comments. Fixed in 49576ef.

  • agent.ts:351 ("a session restart resets the episode") is accurate again, because the restart change was reverted.
  • consecutiveInferenceFailures again drives only the backoff, so its doc in core/agents/state.ts is accurate too.
  • The new consecutiveRejectedTurns field documents that it drives the pause.

5. Nit: the alert kind for the final failure changed. Kept, and now tested and documented in 49576ef.
The pausing failure still skips onError, so agentStopped arrives as kind: 'paused', reason: 'limit' and not as errored. Running both hooks would send the parent two messages for one failure. A pause is also the more accurate signal for an agent that will not run again on its own. The integration test asserts that the pausing load emits exactly one agentStopped for the child, ['paused', 'limit']. The PR description has a line on this for consumers that alert only on errored.

Checks on 49576ef:

  • bun run ts:build: exit 0.
  • bunx tsc -p packages/sdk/tsconfig.test.json --noEmit: exit 0.
  • bun run lint: clean.
  • bun run test: 2183 pass, 89 skip, 0 fail.
  • The production replay (local only, assuming the failures were non-retryable): both agents still pause on their 3rd failure. Every one of their failures had pending tool results.

matej21 and others added 3 commits November 11, 2026 16:15
A session restart reset every errored agent, so an agent whose conversation
the provider rejects failed again on each restart and woke its parent each
time. The failure streak was also cleared by the restart, so it never grew.

- session_restarted keeps consecutiveInferenceFailures and clears only the
  backoff timestamp, so the first attempt after a restart is still immediate.
- When a non-retryable failure is the third in a row, the agent pauses
  (reason 'limit') instead of staying errored. A paused agent is not in
  resetAgentIds, and the mailbox onPause hook tells the parent once, with
  the last error. Retryable failures keep retrying as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 1: the pause also fired when the failing input was a message,
which a non-retryable failure consumes. An agent that used to answer the
next message stayed paused instead, and a root agent had nobody to tell.

- inference_failed records the error type. The reducer counts
  consecutiveRejectedTurns: non-retryable failures of a turn whose pending
  tool results survive the failure, which is the turn a restart rebuilds
  unchanged. Retryable failures, rejected messages and events without a
  type do not count. A committed turn or a conversation splice clears it;
  restarts and resumes keep it.
- The agent pauses on the third such failure.
- session_restarted resets consecutiveInferenceFailures again, as on main,
  so the backoff and retryable-notification behaviour is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matej21
matej21 force-pushed the fix/no-reset-permanently-failed-agents branch from 49576ef to 03c0541 Compare November 11, 2026 15:18
@matej21

matej21 commented Nov 11, 2026 •

Copy link
Copy Markdown
Member Author

Review round 2

I reviewed git diff aacd1b7a9..03c054183 and checked it against the round-1 response. I ran error-recovery.integration.test.ts, session.test.ts, openrouter.test.ts and anthropic.test.ts: 141 pass. CI is green. I also ran three harness scenarios against this head; they are described below and were not committed.

Round-1 findings

  1. Major (the pause fired when the failing input was already dropped): fixed. The round-1 scenario on this head: three rejected user messages to the entry agent, then a valid one. The agent stays errored, answers the valid message (pending, 4 LLM calls) and is never paused. That matches the base result from round 1. The counter goes up only when the error is non-retryable and pendingToolResults is non-empty. Since tool results are cleared only by a committed turn (which resets the counter) or a splice (which resets it too), a counted streak really means "the same tool results rejected N times". Counting per failure, not checking at pause time, is the better version of my suggestion.
  2. Minor (retryable failures counted): fixed. They no longer count, and session_restarted resets consecutiveInferenceFailures again, as on main.
  3. Minor (no resume test): fixed. There are now an integration test and a reducer test for it.
  4. Nit (stale comments): fixed. The restart change was reverted, so the comments are accurate again.
  5. Nit (the alert kind changes): kept on purpose, tested and documented. Fine.

Conflict resolution against #69: correct

  • inference_failed carries providerError and the new errorType. Both are optional, so old events still parse.
  • feat(sdk): keep the provider's error response in inference_failed #69's rule for the first retryable failure still holds. repeatsRetryableFailure is computed from the pre-emit consecutiveInferenceFailures (agent.ts:921), and the parent-notification rule (isRepeatRetryableFailure) is unchanged from main. Scenario: an outage of two inference cycles (server_error, with a provider body) followed by recovery. It produced inference_failed events [["server_error",502],["server_error",null]]: the body is kept on the first failure only, errorType is on both, the agent recovered, and consecutiveRejectedTurns stayed unset.
  • Retryability now has one definition, isRetryableLLMErrorType in core/llm/schema.ts. Both isRetryableLLMError and the reducer use it, so the runtime and the replayed state cannot disagree.

New findings

Minor: a resumed agent pauses on a retryable error, and the pause message blames the provider for rejecting the turn. packages/sdk/src/core/agents/agent.ts:936

The pause check reads only consecutiveRejectedTurns >= 3. That count survives agent_resumed by design. So after resume_agent, any failure pauses again, including a retryable one, and the outage retry loop with its preserved message never starts. Reproduced: a tool-result turn is rejected 3 times, the agent is paused, it is resumed, and the provider then returns server_error. The agent is paused (limit) again, there are 2 pause events, and the message says: "The provider rejected this turn 3 times in a row … Last error: upstream 502". The parent gets a second <child-paused> that names a transient error as a permanent rejection. Fix: pause only when this failure is itself a counted rejection, e.g. && !isRetryableLLMError(llmResponse.error) in the condition. Add a test with a retryable error after a resume.

Checked, no finding

  • Events written before errorType existed never count (documented). An agent that is already a zombie pauses after 3 more rejected restarts.
  • A splice (agents.rewind) clears the count, and so does a committed turn. A restart keeps the count and still resets the backoff.
  • An aborted failure returns before inference_failed, so it never counts.

Review round 2: the rejected-turn count survives a resume, so any failure
after resume_agent paused the agent again, an outage included, with a
message saying the provider rejected the turn. The pause now also requires
the current failure to be non-retryable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matej21

matej21 commented Nov 11, 2026

Copy link
Copy Markdown
Member Author

Review response, round 2

Minor: a resumed agent paused again on a retryable error. Fixed in 09ec260.

The pause condition now also requires the current failure to be non-retryable:

if (errorState && !isRetryableLLMError(llmResponse.error) && rejected >= Agent.MAX_CONSECUTIVE_REJECTED_TURNS)

A non-retryable failure at or above the threshold is always itself a counted rejection. The count is non-zero only while the same tool results are pending, because a committed turn or a splice clears both.

New integration test: "a resumed agent that hits an outage is retried after it, not paused again".

  • A child is paused after 3 rejected restarts, then resumed.
  • The provider returns server_error until the LLM retry gives up after 5 attempts. The child ends errored, not paused.
  • The provider recovers. The next restart resets the child and the kept turn commits, which clears the count.
  • Result: 1 pause event in total and 10 child LLM calls. The child's errorType sequence is invalid_request ×3, server_error.
  • I put the old condition back locally and this test fails.

Checks on 09ec260:

  • bun run ts:build: exit 0.
  • bunx tsc -p packages/sdk/tsconfig.test.json --noEmit: exit 0.
  • bun run lint: clean.
  • bun run test: 2192 pass, 89 skip, 0 fail.
  • The branch is still on the current origin/main (aacd1b7a9). No rebase was needed.

@matej21
matej21 merged commit f551ec4 into main Nov 11, 2026
1 check passed
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