Skip to content

fix(core): recover partial provider failures - #44002

Open
kitlangton wants to merge 6 commits into
v2from
partial-failure-recovery
Open

fix(core): recover partial provider failures#44002
kitlangton wants to merge 6 commits into
v2from
partial-failure-recovery

Conversation

@kitlangton

@kitlangton kitlangton commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What

Automatically recover retryable provider-internal and rate-limit failures that arrive after partial model output. Recovery can cross eagerly executed local tools once their outcomes are durable, but stops at provider-hosted activity that cannot be replayed uniformly.

Before / After

Before: A provider could emit reasoning, text, or a local tool call and then fail with a retryable provider error. Because output had already started, the ordinary transparent retry path no longer applied, so the assistant response ended in error and the user had to prompt again manually.

After: The runner joins eager local tool executions, durably settles every observed call, and then schedules its existing bounded retry for incomplete streams, transport-read failures, retryable rate limits, and retryable provider-internal failures. It records the failed attempt, appends the existing continuation prompt, reloads projected history, and starts a fresh model request. Durable local tool results are included, so side effects are not repeated. Provider-hosted activity remains terminal because hosted results are not uniformly replayable across protocols and storage modes.

How

  • packages/core/src/session/runner/llm.ts classifies eligible post-output failures separately from transparent pre-output retries and requires every observed call to be settled and locally executed.
  • Recovery reuses the durable partial-failure continuation and existing bounded retry schedule rather than adding another retry loop.
  • packages/ai/src/protocols/openai-responses.ts surfaces hosted activity at response.output_item.added, ensuring a stream failure cannot hide a provider-side tool boundary before output_item.done.
  • Core tests cover reasoning-only provider recovery, retryable rate limits, settled local tool recovery without repeated execution, hosted-tool refusal, and omission of failed encrypted reasoning state.

Scope

  • Does not retry failures marked non-retryable by provider policy.
  • Does not recover across provider-hosted tool activity, even when the provider reported a result.
  • Does not treat request-delivery transport failures as post-output continuation candidates.
  • Preserves existing incomplete-stream and transport-read continuation behavior.

Testing

  • bun run test test/session-runner.test.ts from packages/core: 165 passed
  • bun run test from packages/core: 1950 passed
  • bun run test test/provider/openai-responses.test.ts from packages/ai: 94 passed
  • bun typecheck from packages/core: passed
  • bun typecheck from packages/ai: passed
  • Push hook repository typecheck: 33 tasks passed
  • git diff --check origin/v2: passed
  • Full packages/ai suite: 542 passed, 6 unrelated existing OpenRouter fixture/reasoning failures

Flow

flowchart TD
  A[Retryable failure after output starts] --> B[Join eager local tools]
  B --> C[Durably close every observed call]
  C --> D{Failure eligible?}
  D -->|No| E[Keep failure terminal]
  D -->|Yes| F{Any hosted or unsettled call?}
  F -->|Yes| E
  F -->|No| G[Schedule bounded retry]
  G --> H[Append continuation prompt]
  H --> I[Reload projected history and local tool results]
  I --> J[Start fresh model request]
Loading

@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

  1. packages/core/src/session/runner/llm.ts:66 — The providerRetry arm (RateLimit / ProviderInternal + isRetryable) extends the auto-continue path beyond stream interruption, and each continue spawns a fresh provider request. Nothing in this hunk bounds how many times recovery can re-fire for a flapping provider. Why it matters: repeated 429→continue cycles burn billed turns in a loop. Suggestion: confirm the enclosing runner still caps consecutive recoveries (and surface that count in the synthetic event or logs); otherwise thread a recovery-attempt budget into shouldContinueAfterFailure.

  2. packages/core/src/session/runner/llm.ts:69 — record.calls.every((call) => call.settled && !call.providerExecuted) is all-or-nothing: one hosted (provider-executed) call alongside nine locally-settled calls disables recovery for the entire step. Conservative and correct today, but OpenAI Responses-style protocols can replay hosted items by ID. Suggestion: leave as-is for now; a follow-up could recover the local subset and re-request only the hosted tail.

  3. packages/ai/src/protocols/openai-responses.ts:181 — onHostedToolAdded emits toolInputStart on output_item.added; if a stream truncates between added and done (or a provider duplicates the added event), consumers see an orphan toolInputStart with no matching result/done. Why it matters: transcript/UI code that pairs starts with completions can wedge. Suggestion: assert (in tests) that an added-without-done sequence yields a benign terminal state, or synthesize a close-out on stream end.

  4. packages/core/test/session-provider-recovery.test.ts — The recorded fixtures give excellent cross-provider accept/reject coverage. Missing: a direct unit matrix for the pure predicate shouldContinueAfterFailure (non-retryable rate limit ⇒ false; any unsettled call ⇒ false; any providerExecuted call ⇒ false; Transport-read ⇒ true only when calls settled). Why it matters: it's now the single gate deciding whether a failed step silently continues.

  5. packages/core/src/session/runner/llm.ts:130 (docs/changelog) — Auto-continue now also triggers after retryable RateLimit/ProviderInternal failures, i.e. users will see the synthetic "previous response was interrupted" turn in cases where the run previously just failed. Why it matters: surprising extra turns/cost without a changelog note. Suggestion: mention the widened trigger in release notes and make the synthetic text distinguish interruption vs provider retry if cheap.

— automated review (ox-alpha, round2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants