fix: fail bare rate limits and show retry waits - #3115
Conversation
📝 WalkthroughProblem solvedBare DesignThe PR extends the existing live The added protocol validation, event handling, projection logic, and UI presentation form the smallest coherent solution for persistence, remount behavior, and user feedback. The added complexity is required to distinguish scheduled retries from completed content and to prevent stale retry state from returning. No safe deletion or simplification is evident from the current diff. The new tests cover protocol decoding, projection, continuity, error classification, telemetry, and UI-facing behavior. Validation and risksCoverage was added or updated for runtime, runtime-host, session projection, session continuity, protocol compatibility, retry classification, telemetry, and formatting. The runtime-host compatibility epoch increased from Repository-wide tests were not run. A live Electron remount was not verified. Required checks therefore remain unverified without direct check results. Review-relevant risks
WalkthroughThe PR makes rate-limit retries conditional on valid ChangesProvider retry flow
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to Remounting during a provider retry can show a countdown longer than the actual remaining wait because the scheduled retry time is not preserved. This is a concrete user-visible correctness issue that should be fixed before merge. Sequence Diagram(s)sequenceDiagram
participant Provider
participant Runtime
participant RuntimeHost
participant UI
Provider->>Runtime: return rate-limit error
Runtime->>Runtime: parse Retry-After
Runtime->>RuntimeHost: provide retry metadata
RuntimeHost->>RuntimeHost: publish provider_retry event
RuntimeHost->>UI: project retry snapshot
UI->>UI: render localized warning banner
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: af9b5f4d-87df-471c-86b7-6d7a4f3ac623
📒 Files selected for processing (21)
packages/core/src/agent-run.tspackages/runtime-host/src/__tests__/canonical-session-projection.test.tspackages/runtime-host/src/__tests__/protocol.test.tspackages/runtime-host/src/__tests__/session-projector.test.tspackages/runtime-host/src/adapter/session-projector.tspackages/runtime-host/src/protocol/operations.tspackages/runtime-host/src/protocol/turn.tspackages/runtime-host/src/server/canonical-turn-snapshot.tspackages/runtime/src/__tests__/model-adapter.test.tspackages/runtime/src/__tests__/provider-error-classification.test.tspackages/runtime/src/__tests__/provider-request-telemetry.test.tspackages/runtime/src/__tests__/session-manager.test.tspackages/runtime/src/agent-run.tspackages/runtime/src/provider-error-classification.tspackages/storage/src/agent-run-store.tspackages/ui/src/__tests__/provider-retry-presentation.test.tspackages/ui/src/chat-turn.tsxpackages/ui/src/conversation-copy.tspackages/ui/src/index.tspackages/ui/src/provider-retry-presentation.tspackages/ui/src/styles.css
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.
968aae0 to
d2666f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c38e715-0bfe-4131-b811-131d2fbb9cd9
📒 Files selected for processing (10)
packages/runtime-host/src/__tests__/protocol.test.tspackages/runtime-host/src/__tests__/session-continuity-coordinator.test.tspackages/runtime-host/src/__tests__/session-projector.test.tspackages/runtime-host/src/adapter/session-projector.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/protocol/operations.tspackages/runtime-host/src/protocol/turn.tspackages/runtime-host/src/server/root-turn-coordinator.tspackages/runtime-host/src/server/session-continuity-coordinator.tspackages/ui/src/chat-turn.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/runtime-host/src/tests/protocol.test.ts
- packages/ui/src/chat-turn.tsx
- packages/runtime-host/src/protocol/turn.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
d2666f5 to
3cc3de4
Compare
A bare 429, including OpenCode FreeUsageLimitError, is a quota or throttle without a recovery time. Retrying it ten times only hid the failure behind a three-minute empty wait. Keep automatic retries for rate limits that name Retry-After, and leave network or 5xx retries unchanged. Generated-by: Cursor Grok 4.5
The live retry line used the same muted working-status treatment as "thinking", so a 40-second rate-limit wait looked like a hang. Keep the existing live providerRetry object, and render its reason plus countdown through the shared Astryx Banner. Generated-by: Cursor Grok 4.5
A provider retry is still not a conversation fact. Hold the current wait on the in-memory Turn snapshot, publish it when it appears, and drop it on the next content event so remount cannot resurrect a finished wait. Generated-by: Cursor Grok 4.5
3cc3de4 to
f8eb564
Compare
M4n5ter
left a comment
There was a problem hiding this comment.
LGTM. The rate-limit classification, transient retry overlay lifecycle, protocol boundary, and UI presentation are coherent and well covered by tests.
A 429 with no Retry-After failed the Turn immediately: retryMetadataFromFacts
returned retryable:false for rate_limit unless the provider named a delay, so a
commandcode gateway 429 ({code:'rate_limit_error', "temporarily unavailable,
try again in a moment"}) ended the Turn with retry {decision:'declined',
because:'policy'} even though the same response says to retry.
The classifier is the single authority for retryability; the Turn loop only
consumes it. That loop is already bounded (10 provider attempts per step, 1s→32s
exponential backoff with jitter, SDK maxRetries:0), so a throttle with no named
delay has a safe home: the same rule provider_capacity already uses — retryable,
carrying retryAfterMs only when the header parses.
The new rule requires HTTP 429 evidence, not just the rate_limit class. That
class is also reachable from the trailing `\brate\b` word heuristic and from
bare in-stream strings, where nothing said the provider will accept a later
request; those keep the old "no parseable Retry-After, no retry" rule.
#3115's original case still fails fast, and now for the right reason. Its
concrete evidence was OpenCode Zen's FreeUsageLimitError (HTTP 429, error.type
'FreeUsageLimitError') — an exhausted free allowance, not a throttle — but the
rule it produced was written far wider than that evidence, covering every
rate_limit. The code is now in PROVIDER_BILLING_PROVIDER_CODES, so it
classifies as provider_billing and stops on the first attempt regardless of
status, the same way insufficient_quota does.
Known cost: a quota-exhaustion 429 that ships no structured billing code
(Gemini daily quota, OpenRouter free-models-per-day, Anthropic OAuth usage
limit) now burns one full backoff step before failing. That is accepted rather
than fixed by text matching: the quota/limit vocabulary in those bodies also
appears in genuine throttles, so a text rule would re-break the case above.
The numeric `status === 429` fallback in the retry branch is folded into the
rate_limit branch: the classifier already maps a 429 status or code to
rate_limit unless stronger structured evidence wins, and keeping the raw status
here would have overridden provider_billing's fail-fast on a 429.
Unchanged: aborts, Codex edge rejections (the transport already spent its
budget), provider_billing, auth, and context_overflow stay non-retryable; 408 /
409 / 5xx keep their existing rule.
Generated-by: Claude Code
A 429 with no Retry-After failed the Turn immediately. A commandcode gateway
throttle ({code:'rate_limit_error', "Upstream model provider is temporarily
unavailable. Please try again."}) ended the Turn with retry
{decision:'declined', because:'policy'} even though the same response says to
retry.
The root cause is that retryMetadataFromFacts had grown into a second
classifier, re-deciding from status and code what classifyProviderFacts had
already decided:
- a numeric `status` fallback that retried 408/409/5xx behind the kind's back,
so the kind and the retry answer could disagree;
- three different meanings for a malformed Retry-After — ignored for
provider_capacity and 429 rate limits, fatal for 5xx and network, and the
sole retry evidence for a text-only rate limit;
- RUNTIME_RETRYABLE_ERROR_CODES, which made three runtime codes retryable
before classification ran, including one that classifies as `unknown`;
- `status === 409`, introduced by #1425 with no test and no provider evidence.
The classifier is now the only authority: ModelFailureKind decides
retryability, and Retry-After only supplies the delay. RETRYABLE_FAILURE_KINDS
lists network, provider_capacity, provider_unavailable, rate_limit,
stream_truncated and timeout; every other kind fails fast. Two guards survive
because they are facts about the error rather than its class: an abort, and a
Codex edge rejection whose transport already spent its full 2/10/30-second
budget.
Behavior changes:
- a bare 429 is retryable (the reported failure);
- a 5xx or transport failure with a malformed Retry-After is retryable rather
than fatal — the header is ignored and the local backoff paces it;
- a fetch timeout (kind timeout) is retryable, matching MODEL_STREAM_TIMEOUT,
which was retryable only through the code list;
- 409 is no longer retried; it classifies as request_rejected;
- OPENAI_RESPONSES_CONTINUATION_UNAVAILABLE now classifies as network like its
sibling websocket code, instead of being retryable while classified
`unknown`;
- FreeUsageLimitError stays in PROVIDER_BILLING_PROVIDER_CODES, so #3115's
original case (an exhausted OpenCode Zen free tier on 429) still fails on the
first attempt.
Known cost, unchanged from the previous revision: a quota-exhaustion 429 that
ships no structured billing code (Gemini daily quota, OpenRouter
free-models-per-day, Anthropic OAuth usage limit) burns one full backoff step
before failing. Accepted rather than fixed by text matching — the quota/limit
vocabulary in those bodies also appears in genuine throttles, so a text rule
would re-break the reported case.
Consumer redundancy the table removes: ai-sdk-turn's `context_overflow` retry
branch duplicated the `!failure.retryable` branch immediately below it, and the
Codex history compactor's fallback gate tested `!diagnostic.retryable` on a
request_rejected diagnostic, which is now identically false.
Unchanged: the 10-attempt-per-step budget and its backoff, the abort and Codex
edge guards, durable retryable fields and protocol shapes, and @maka/eval.
Generated-by: Claude Code
A 429 with no Retry-After failed the Turn immediately. A commandcode gateway
throttle ({code:'rate_limit_error', "Upstream model provider is temporarily
unavailable. Please try again."}) ended the Turn with retry
{decision:'declined', because:'policy'} even though the same response says to
retry.
The root cause is that retryMetadataFromFacts had grown into a second
classifier, re-deciding from status and code what classifyProviderFacts had
already decided:
- a numeric `status` fallback that retried 408/409/5xx behind the kind's back,
so the kind and the retry answer could disagree;
- three different meanings for a malformed Retry-After — ignored for
provider_capacity and 429 rate limits, fatal for 5xx and network, and the
sole retry evidence for a text-only rate limit;
- RUNTIME_RETRYABLE_ERROR_CODES, which made three runtime codes retryable
before classification ran, including one that classifies as `unknown`;
- `status === 409`, introduced by #1425 with no test and no provider evidence.
The classifier is now the only authority, through one exhaustive table.
MODEL_FAILURE_RETRY maps every ModelFailureKind to the ProviderRetryReason it
retries under, or to null: network, provider_capacity, provider_unavailable,
rate_limit, stream_truncated and timeout map to their same-named reason, the
other six to null. A new kind that forgets a row fails to compile. Two guards
survive because they are facts about the error rather than its class: an abort,
and a Codex edge rejection whose transport already spent its full
2/10/30-second budget.
Behavior changes:
- a bare 429 is retryable (the reported failure);
- a 5xx or transport failure with a malformed Retry-After is retryable rather
than fatal — the header is ignored and the local backoff paces it;
- a fetch timeout (kind timeout) is retryable, matching MODEL_STREAM_TIMEOUT,
which was retryable only through the code list;
- 409 is no longer retried; it classifies as request_rejected. The upstream AI
SDK's APICallError.isRetryable defaults to treating 409 as retryable and
#1425 most likely copied that, but this repo has no path that produces a 409
and no test that ever pinned the behavior, so it follows the table;
- OPENAI_RESPONSES_CONTINUATION_UNAVAILABLE now classifies as network like its
sibling websocket code, instead of being retryable while classified
`unknown`;
- FreeUsageLimitError is newly added to PROVIDER_BILLING_PROVIDER_CODES. On
main it classified as rate_limit and failed fast only by accident, through
the "a 429 with no Retry-After is not retryable" rule this commit deletes.
#3115's case (an exhausted OpenCode Zen free tier on 429) now fails fast by
an explicit rule instead, at the cost of a kind change from rate_limit to
provider_billing — the persisted errorClass and the user-facing guidance
change with it.
Known cost: a quota-exhaustion 429 that ships no structured billing code
(Gemini daily quota, OpenRouter free-models-per-day, Anthropic OAuth usage
limit) now spends the whole 10-attempt-per-step budget before the Turn reports
a terminal failure — about 159s of local backoff, up to about 199s with
jitter, so roughly three minutes. Not fixed by vetoing 429 with
USAGE_LIMIT_TEXT_PATTERNS: Gemini's per-minute throttle reads "Quota exceeded
for quota metric…" too, so that veto would fail genuine throttles on the first
attempt — exactly the behavior this commit repairs. Only a structured billing
code is an honest signal, and these bodies carry none.
Retry-After is still capped at MAX_SAFE_TIMER_DELAY_MS (~24.8 days) and a
longer delay is discarded rather than clamped, as on main for rate_limit; this
commit neither widens that bound nor adds a clamp. A Turn abort interrupts the
wait.
Consumer redundancy the table removes: ai-sdk-turn's `context_overflow` retry
branch duplicated the `!failure.retryable` branch immediately below it;
ai-sdk-turn's own providerRetryReason switch, a second hand-maintained copy of
the kind-to-reason mapping, is now the same table; and the Codex history
compactor's fallback gate tested `!diagnostic.retryable` on a request_rejected
diagnostic, which is now identically false.
Unchanged: the 10-attempt-per-step budget and its backoff, the abort and Codex
edge guards, durable retryable fields and protocol shapes (including the
ProviderRetryReason `unknown` member, now unreachable from this call site but
still accepted by the Turn protocol), and @maka/eval.
Generated-by: Claude Code
…5318) A 429 with no Retry-After failed the Turn immediately. A commandcode gateway throttle ({code:'rate_limit_error', "Upstream model provider is temporarily unavailable. Please try again."}) ended the Turn with retry {decision:'declined', because:'policy'} even though the same response says to retry. The root cause is that retryMetadataFromFacts had grown into a second classifier, re-deciding from status and code what classifyProviderFacts had already decided: - a numeric `status` fallback that retried 408/409/5xx behind the kind's back, so the kind and the retry answer could disagree; - three different meanings for a malformed Retry-After — ignored for provider_capacity and 429 rate limits, fatal for 5xx and network, and the sole retry evidence for a text-only rate limit; - RUNTIME_RETRYABLE_ERROR_CODES, which made three runtime codes retryable before classification ran, including one that classifies as `unknown`; - `status === 409`, introduced by #1425 with no test and no provider evidence. The classifier is now the only authority, through one exhaustive table. MODEL_FAILURE_RETRY maps every ModelFailureKind to the ProviderRetryReason it retries under, or to null: network, provider_capacity, provider_unavailable, rate_limit, stream_truncated and timeout map to their same-named reason, the other six to null. A new kind that forgets a row fails to compile. Two guards survive because they are facts about the error rather than its class: an abort, and a Codex edge rejection whose transport already spent its full 2/10/30-second budget. Behavior changes: - a bare 429 is retryable (the reported failure); - a 5xx or transport failure with a malformed Retry-After is retryable rather than fatal — the header is ignored and the local backoff paces it; - a fetch timeout (kind timeout) is retryable, matching MODEL_STREAM_TIMEOUT, which was retryable only through the code list; - 409 is no longer retried; it classifies as request_rejected. The upstream AI SDK's APICallError.isRetryable defaults to treating 409 as retryable and #1425 most likely copied that, but this repo has no path that produces a 409 and no test that ever pinned the behavior, so it follows the table; - OPENAI_RESPONSES_CONTINUATION_UNAVAILABLE now classifies as network like its sibling websocket code, instead of being retryable while classified `unknown`; - FreeUsageLimitError is newly added to PROVIDER_BILLING_PROVIDER_CODES. On main it classified as rate_limit and failed fast only by accident, through the "a 429 with no Retry-After is not retryable" rule this commit deletes. #3115's case (an exhausted OpenCode Zen free tier on 429) now fails fast by an explicit rule instead, at the cost of a kind change from rate_limit to provider_billing — the persisted errorClass and the user-facing guidance change with it. Known cost: a quota-exhaustion 429 that ships no structured billing code (Gemini daily quota, OpenRouter free-models-per-day, Anthropic OAuth usage limit) now spends the whole 10-attempt-per-step budget before the Turn reports a terminal failure — about 159s of local backoff, up to about 199s with jitter, so roughly three minutes. Not fixed by vetoing 429 with USAGE_LIMIT_TEXT_PATTERNS: Gemini's per-minute throttle reads "Quota exceeded for quota metric…" too, so that veto would fail genuine throttles on the first attempt — exactly the behavior this commit repairs. Only a structured billing code is an honest signal, and these bodies carry none. Retry-After is still capped at MAX_SAFE_TIMER_DELAY_MS (~24.8 days) and a longer delay is discarded rather than clamped, as on main for rate_limit; this commit neither widens that bound nor adds a clamp. A Turn abort interrupts the wait. Consumer redundancy the table removes: ai-sdk-turn's `context_overflow` retry branch duplicated the `!failure.retryable` branch immediately below it; ai-sdk-turn's own providerRetryReason switch, a second hand-maintained copy of the kind-to-reason mapping, is now the same table; and the Codex history compactor's fallback gate tested `!diagnostic.retryable` on a request_rejected diagnostic, which is now identically false. Unchanged: the 10-attempt-per-step budget and its backoff, the abort and Codex edge guards, durable retryable fields and protocol shapes (including the ProviderRetryReason `unknown` member, now unreachable from this call site but still accepted by the Turn protocol), and @maka/eval. Generated-by: Claude Code
Summary
A bare 429, including OpenCode
FreeUsageLimitError, was retried ten times and looked like a hang. Rate limits now fail immediately unless the provider namesRetry-After. A real wait still is not a conversation fact: Host keeps the current overlay on the in-memory Turn snapshot, publishes it when it appears, and drops it on the next content event so remount cannot resurrect a finished wait. The chat shows that overlay as an Astryx warning Banner.Before — a muted status row with no reason:
After — an Astryx warning Banner with the retry reason:
Verification
packages/runtime/src/__tests__/provider-error-classification.test.ts(retries a rate limit only when the provider names a retry delay)packages/runtime/src/__tests__/session-manager.test.ts(provider retry progress reaches observers...)packages/runtime-host/src/__tests__/session-continuity-coordinator.test.ts(keeps the current provider retry on the live Turn until the next content event)packages/runtime-host/src/__tests__/session-projector.test.tspackages/runtime-host/src/__tests__/protocol.test.tspackages/runtime-host/src/__tests__/canonical-session-projection.test.tsnpm run format:checkDid not run repository-wide tests or a live Electron remount of a Retry-After wait.
AI use
Tool(s) and scope:
Cursor Grok 4.5 diagnosed the hung session, shaped the change, and authored the commits.
Checklist
Does this PR entail a change in behavior?