fix(anthropic): replay a stream that reports overload before any output - #4085
Conversation
Anthropic answers HTTP 200 and then delivers `overloaded_error` as the first SSE event. `requestStream` builds that error with `retryable: true` but cannot act on it: it hands the body off the moment headers arrive, and past that point `if (streamOwnsDeadline) throw error` surfaces every failure because a reader already owns the body. The run therefore ends in RUN_ERROR on a transient upstream 529. Replay it where the request body is still available. The bound (2) and the backoff (1s, 2s) match the pre-header loop, so the worst case adds 3 seconds and a hosted child fork stays inside its 45-second idle watchdog. Two guards keep the replay safe, both mutation-proven: - nothing may have been yielded for the current request, so a mid-turn failure can never duplicate output - the failure must be a typed `ProviderError` with `retryable: true`, so an authentication error or caller cancellation is never replayed Observed on the staging health lane: `claude-opus-4-6` failed both the text and the tool probe with OVERLOADED_ERROR across four runs between 2026-08-24 06:45Z and 08:11Z, from `buildAnthropicSseError` inside `Object.pull`.
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49a0e4f98f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThe PR adds bounded replay of retryable Anthropic SSE failures that occur before output and consolidates stream issuance so initial, replayed, and
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| extensions/ext-llm-anthropic/src/anthropic-provider.ts | Implements guarded in-stream replay, abortable backoff, independent post-output replay allowances, and shared header budgeting; both previously reported defects are fixed. |
| extensions/ext-llm-anthropic/src/anthropic-provider.test.ts | Adds comprehensive tests for replay eligibility, bounds, continuation request bodies, cancellation, and timing budgets without introducing a review-eligible defect. |
| src/provider/runtime-loader/provider-http.ts | Exports the existing abort-aware retry wait helper and provider stream timeout constants for safe reuse. |
| src/provider/runtime-loader.ts | Re-exports the shared stream retry and timeout primitives through the runtime loader. |
| src/provider/shared/index.ts | Exposes the new shared primitives to provider extensions through the stable shared barrel. |
| docs/api-reference/veryfront/provider.md | Updates generated provider API documentation for the newly exported stream timeout and retry symbols. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Issue Anthropic stream request] --> B{Headers/request succeeded?}
B -- No --> C[requestStream pre-header retry handling]
B -- Yes --> D[Parse SSE body]
D --> E{Retryable provider error?}
E -- No --> F[Yield parts or surface error]
E -- Yes --> G{Any output yielded in this attempt?}
G -- Yes --> H[Surface error without replay]
G -- No --> I{Replay count and header budget available?}
I -- No --> H
I -- Yes --> J[Abortable backoff]
J --> K[Reissue current request body]
K --> D
F --> L{pause_turn continuation?}
L -- Yes --> M[Build continuation request body]
M --> A
L -- No --> N[Emit final result]
Reviews (10): Last reviewed commit: "Prove replay header caps without wall-cl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ble guard Review of the replay found three things to clean up. The local `waitForAnthropicStreamReplay` duplicated `waitForProviderStreamRetry` in provider-http.ts, and duplicated it worse: it did not check `aborted` up front, so an already-cancelled request burned the whole delay before anything noticed. Export the existing helper instead. The PR claims the replay's backoff matches the pre-header loop; sharing the wait makes that structural rather than coincidental. The tool-name registry snapshot was unreachable. Every mutation in the parser (`set` at anthropic-stream.ts:1165, `delete` at :1214) is immediately followed by a yield, so any registry change implies `yieldedThisAttempt`, which blocks the replay. Restoring it was dead code presented as a guard. Replaying the wrong request body was untested: the stub answered positionally and ignored what was sent, so substituting `body` for `requestBody` survived. The pause_turn test now asserts the message count of every request, and that mutation reddens it. Adds coverage for two reachable behaviours that had none: a replay of a pause_turn continuation, and the replay budget being shared across the whole call rather than reset per request.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.\n\nVerified one blocker: the in-stream replay path resets the shared header/first-output budget on every retry, so repeated overload replays can exceed the intended total admission budget. Carry one deadline across the initial request and all replays, with a regression covering delayed replay headers.
Exporting `waitForProviderStreamRetry` from `veryfront/provider/shared` adds it to the public surface, which `ci (lint)` caught as a stale generated reference. The rest of the diff is line-number churn from the edits above it. Making it public is a deliberate trade. `veryfront/provider/shared` is the toolbox provider implementations build against, and it already exports `requestStream`, `buildProviderError`, and `parseRetryAfterMs`; a cancellation-aware retry wait belongs in that set. The alternative was a second private copy in the extension, which is exactly what let the missing `aborted` check through the first time.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Constraint: In-stream Anthropic overload replays must share the same admission budget as the original streamed call. Rejected: Resetting requestStream budget per in-stream replay | lets repeated provider stalls exceed the hosted idle boundary. Confidence: high Scope-risk: narrow Directive: Keep provider-specific stream replay budgets aligned with shared provider HTTP defaults. Tested: deno test -A extensions/ext-llm-anthropic/src/anthropic-provider.test.ts; deno fmt --check touched files; git diff --check Not-tested: Full repository test suite; pre-commit verify:quick blocked by local Deno 2.7.11 vs pinned 2.7.7
kwakayama
added
ready-for-review
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e01871c80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…eplays Builds on 8e01871, which landed the shared header budget in parallel with my own version of the same fix. Its anchor is the better idea: it ties the bound to the 45-second hosted child-fork watchdog directly, which mine did not. Two defects remained. The anchor never reset. It is set once at doStream entry and `performance.now()` keeps advancing while the stream is consumed, so any turn longer than 40 seconds issues its pause_turn continuation with a spent budget. That silently strips the continuation of the pre-header retry veryfront-code#3993 added. Demonstrated before fixing: a continuation whose first attempt gets a transient 529 was not retried and the run died. It is re-anchored before each continuation, because that request opens a new idle window; a replay stays inside the window it failed in, since a replay only happens when nothing was yielded. The budget was also unproven. Deleting `totalHeadersBudgetMs` entirely left every test green, because the existing test exercises the pre-backoff guard rather than the value handed to `requestStream`. The cause is that `requestStream` never shortens a request's *first* attempt to fit the total budget, so a replay still waited the full 30-second per-attempt deadline. Replays now pass both the per-attempt deadline and the budget, capped at 10 seconds. Mutation-proven, with the timings naming the mechanism: remove the continuation re-anchor -> RED drop the replay clamp entirely -> RED at 40s (fresh default budget) drop only headersTimeoutMs from it -> RED at 31s (unshortened first attempt) Also regenerates the API reference. 8e01871 exported DEFAULT_PROVIDER_STREAM_TOTAL_HEADERS_BUDGET_MS without it, which `ci (lint)` would have failed on.
Constraint: In-stream Anthropic overload replays must not receive a fresh full header wait after the shared first-output budget has already been spent. Rejected: Passing only totalHeadersBudgetMs to requestStream | each Anthropic replay is a new requestStream first attempt, so that does not shorten its header deadline. Confidence: high Scope-risk: narrow Directive: Preserve caller abort propagation through providerAbortScope when adjusting Anthropic stream replay timing. Tested: deno test --allow-env --allow-net --allow-read --allow-write extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: deno test --allow-env --allow-net --allow-read --allow-write --allow-run=deno extensions/ext-llm-anthropic/src/anthropic-provider.test.ts extensions/ext-llm-anthropic/src/anthropic-stream.test.ts extensions/ext-llm-anthropic/src/anthropic-request-builder.test.ts extensions/ext-llm-anthropic/src/anthropic-native-content.test.ts extensions/ext-llm-anthropic/src/anthropic-mcp-request.test.ts extensions/ext-llm-anthropic/src/index.test.ts Tested: deno fmt --check extensions/ext-llm-anthropic/src/anthropic-provider.ts extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: deno lint extensions/ext-llm-anthropic/src/anthropic-provider.ts extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: git diff --check Not-tested: Full pre-commit verify:quick; blocked locally by Deno 2.7.11 while repository pins 2.7.7.
|
@codex review |
Closes the last open review finding. Greptile flagged that `streamReplayCount` spans pause_turn continuations, so a request that spends both replays leaves a later continuation with none and kills a run the replay exists to save. I declined this earlier for a reason that no longer holds. The argument was that per-request counts let repeated stalls grow without bound, which was true while the count was the only bound. It is not any more: the header budget anchored per idle window, plus the 10-second replay clamp, bound the time independently. With time bounded, resetting the count per window is safe, and leaving it shared is now incoherent with the header budget, which already resets there. Both budgets therefore reset at the same place, for the same reason: a continuation opens a new idle window. Neither resets on the replay path. That distinction is load-bearing, not stylistic. Mutation-proven: remove the per-window count reset -> RED (continuation gets no replay) also reset on the replay path -> RED, as an unbounded loop The test asserting the old shared-count behaviour is replaced rather than kept, because this commit deliberately reverses that decision.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 509f44a1c4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Review findings: all five addressedHead is
On #2 and #4Both are the same defect, and both are gone because the code they describe is gone. The local On #3, which I declined earlier and now acceptMy original reasoning was that per-request replay counts let repeated stalls grow without bound. That was true while the count was the only bound. Finding #1 changed that: the header budget now resets per idle window and replays are clamped to 10 seconds, so time is bounded independently of the count. With time bounded, a shared count buys nothing and costs a run: a first request that spends both replays leaves a healthy Both budgets now reset at the same place for the same reason: a continuation opens a new idle window. Neither resets on the replay path. That distinction is load-bearing rather than stylistic, and it is pinned: The test that asserted the old shared-count behaviour is replaced, not kept, because this reverses that decision deliberately. On #5Worth recording why the first attempt at this did not work. Passing The mutation timings name the mechanism rather than just failing: One defect found while reconciling, not raised by either reviewer
Demonstrated before fixing: a continuation whose first attempt got a transient 529 was not retried and the run died. Covered by still grants a pause_turn continuation its pre-header retry budget after a long turn. Queue note - correctionThis PR was in the merge queue at position 23. I dequeued it to push #3, because a queued branch cannot be updated. It was not merged at any point. An earlier version of this comment said I had re-queued it. That was wrong. Dequeuing also cleared auto-merge, and I did not restore either. The PR is currently OPEN, out of the queue, with auto-merge DISABLED, and it will not merge until someone re-enables it. Re-arming it is a deliberate choice for a human to make, not something to restore silently - especially since CI on this head is not clean yet. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
The stub for the header-wait cap returned a promise that never settled. A real fetch rejects when its signal aborts, so the stub now does too. This is hygiene, not a fix. I suspected it caused the `coverage shard 6/8` failure on this head, which reported "Promise resolution is still pending but the event loop has already resolved". It did not: the anthropic suite runs in shard 2, which passed, and re-running shard 6 was green, so that failure was flaky. Running this file under `--trace-leaks` with the old stub does not flag it either. Keeping the change anyway because a permanently pending promise is a poor stub whether or not a sanitizer catches it today. Verified the reworked stub still detects the mutation it exists for: dropping the replay clamp reddens it at 39 seconds, the same as before.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0530611bc7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
kwakayama
removed
the
ready-for-review
…tions Re-anchor the stream header budget (and the replay count) at the moment a part is yielded to the consumer, because that is when the hosted watchdog's idle window restarts. A pause_turn continuation no longer gets a fresh budget: it draws on what remains of the window opened by the last yielded part, fails fast with a retryable error when that window is already spent, and clamps its per-attempt header deadline to the remaining window so a request issued late in a window cannot wait for headers beyond it. Export DEFAULT_PROVIDER_STREAM_HEADERS_TIMEOUT_MS through the provider barrels so the Anthropic runtime can apply the clamp. Trim the exported JSDoc on requestStream and the total header budget so the generated public API reference no longer names hosted watchdog internals; the rationale stays in the source as a non-doc comment, and the API reference is regenerated to match. Claude-Session: https://claude.ai/code/session_016wezkD1Xnjjk5gQiCDHGGZ
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f15b24f9f4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The replay timeout test used elapsed wall time to prove the 10s header cap, which made the focused Anthropic suite spend roughly eleven seconds in one assertion. Drive the retry delay and replay header timeout with FakeTime instead, and assert the replay signal remains active until the exact fake deadline before it aborts. Also remove review/internal wording from the touched Anthropic source comments and make the replay comment distinguish 3s of backoff from the shared header/idle-window budget and per-replay 10s header cap. Constraint: Preserve the 10s production replay header budget and existing replay behavior Rejected: Add timeout injection to production code | test-only determinism is enough and avoids widening the public surface Confidence: high Scope-risk: narrow Tested: deno task test:file extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: deno fmt --check extensions/ext-llm-anthropic/src/anthropic-provider.ts extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: deno check --frozen extensions/ext-llm-anthropic/src/anthropic-provider.ts extensions/ext-llm-anthropic/src/anthropic-provider.test.ts Tested: git diff --check
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
2 participants
Summary
A transient Anthropic overload ends an agent run terminally instead of retrying, because the error arrives inside the SSE body rather than as an HTTP status.
Anthropic answers
HTTP 200, then sendsevent: error/{"type":"overloaded_error"}as the first stream event.buildAnthropicSseErrorclassifies it correctly (ProviderOverloadedError,retryable: true), but nothing acts on the flag: the only retry loop isrequestStream, which hands the body off the moment headers arrive and then rethrows everything viaif (streamOwnsDeadline) throw error— correct on its own terms, since a reader already owns the body.So the replay has to live in the provider, where the request body is still available to re-issue.
What ships
continuePausedStreamnow replays the current request when the parse throws, under two guards:ProviderErrorwithretryable: true— an authentication error is not replayed, and caller cancellation surfaces asAbortError, so it cannot reach the branchThe bound (2 replays) and backoff (1s, 2s) match the pre-header loop in
provider/runtime-loader/provider-http.ts. The backoff adds at most 3 seconds. Replay header attempts remain inside the shared 40-second idle-window budget, and each replay header wait is capped at 10 seconds.The two
requestStreamcall sites collapsed into oneissueStream(requestBody)helper so the replay and thepause_turncontinuation issue requests identically. That is the only refactor.TDD
Red first — 2 of the first 4 cases failed before the change:
Every guard is mutation-proven, each mutation verified to have landed before the run:
yieldedThisAttempt ||!isReplayableAnthropicStreamFailure(...)->falseMAX_ANTHROPIC_STREAM_REPLAYS2 -> 5bodyinstead ofrequestBodystreamReplayCountinside the loopTwo of those need their history stated rather than just their result.
The retryable-classification mutation was wrong the first time. Deleting the term left a dangling
||and the file failed to parse, so no test ran — an empty result that reads like a pass. Redone as afalsesubstitution, and the RED confirmed.Replaying the wrong body originally SURVIVED. The fetch stub answered positionally and ignored what was sent, so the test could not tell the continuation from the original turn. The stub now records every request body and the pause_turn case asserts the message count of each (
[1, 2, 2]); the mutation reddens it. This is why the pause_turn case exists at all.Caveat on the last row. Moving
streamReplayCountinside the loop makes the replay unbounded, and the suite hangs rather than failing an assertion. That is a real RED, but a hang is a weaker signal than a failed expectation, and it is worth knowing that this particular regression would show up as a timeout.Green:
src/provider/fails locally without provider credentials this machine does not have. That is pre-existing, not introduced here — verified against a pristineorigin/mainworktree:The set of failing test names is byte-for-byte identical between the two (
diffof the sorted names is empty). The branch adds 7 passing steps and no new failures.Second commit: review cleanups
Self-review after the first commit found three things worth fixing rather than defending.
waitForAnthropicStreamReplayduplicatedwaitForProviderStreamRetry, and duplicated it worse: no up-frontabortedcheck, so an already-cancelled request burned the full delay. The existing helper is now exported and reused.setatanthropic-stream.ts:1165,deleteat:1214) is immediately followed by a yield, so any registry change impliesyieldedThisAttempt, which blocks the replay. Dead code presented as a guard; removed.pause_turncontinuation, and the budget being shared across the call.Evidence
Staging health lane,
claude-opus-4-6failing both the text and the tool probe:OVERLOADED_ERROROVERLOADED_ERROROVERLOADED_ERROROVERLOADED_ERROREvents on every one:
AGENT_RUN_DEFAULT_CHAT_START_ENQUEUED, RUN_STARTED, CUSTOM, STEP_STARTED, RUN_ERROR— no output before the failure, which is exactly the window this replay covers.The staging log for
run_4dfbef51-907c-464c-b5d2-cb2f65e6961bnames the path:Object.pullis the tell: the failure is raised while reading the body, so it is structurally out of reach of the pre-header loop.Five such events in staging today between 06:45Z and 07:24Z, four in production over the preceding week. The lane passed at 06:43Z and 06:58Z with runs seconds either side of a failure, so the overload is per-request and transient — the case a bounded replay is for.
Note on the health gate
This is the defect behind the red lane, but landing it does not turn staging green by itself: the fix reaches staging only once a
veryfront-coderelease is promoted intoveryfront-server, which currently runs0.1.36-rc.53.Related
Same shape as veryfront-issue-inbox#710 (
retryable: truebuilt and then ignored), which veryfront-code#3993 closed for the pre-header window. This closes the post-header, pre-output window that #3993 could not reach.