Skip to content

fix(codex): show neutral wording for exhausted HTTP 429 retries - #26532

Open
samaluk wants to merge 2 commits into
pingdotgg:mainfrom
samaluk:fix/codex-429-limit-wording-24885
Open

samaluk wants to merge 2 commits into
pingdotgg:mainfrom
samaluk:fix/codex-429-limit-wording-24885

Conversation

@samaluk

@samaluk samaluk commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #24885.

Codex can reduce an upstream HTTP 429 to responseTooManyFailedAttempts with exceeded retry limit, last status: 429 Too Many Requests. T3 deliberately routes that failure through usage_limit recovery, but its Usage limit reached explanation treats an ambiguous rate limit as exhausted allowance. Codex has already discarded the upstream reason; T3 cannot infer capacity or quota exhaustion from that status alone (openai/codex#30471).

Change

Use one shared title decision for the existing failure class and code. Exhausted Codex HTTP 429 retries say Provider rate limit (HTTP 429); explicit usageLimitExceeded failures retain Usage limit reached. Missing-reset guidance says No reset time reported; retry manually.

Apply that decision to new stored terminal/retry items, web/desktop timeline and recovery banners, sidebar tooltips and notifications, and mobile feed, timeline and recovery cards. Clients also correct the displayed title of older items whose stored title still says usage limit. Show the retained provider message in warning rows instead of hiding it.

The adapter classification, Limited state, manual Resume controls, reset timestamps, retry policy and provider/model selection are unchanged. Completed retries keep their recovered presentation. This does not extend #25611's separate handling of explicit serverOverloaded failures or change the ACP ingestion concern mentioned in the issue comments.

Scope and approval

This is a focused correction of an inaccurate explanation on an established recovery path, under CONTRIBUTING.md's exception for a focused obvious bug. The server and client edits address that same explanation; they add no recovery workflow or wire-contract change. The problem and diagnostic-loss constraint are documented in #24885 and its comments. No maintainer approval of a broader behavior change is claimed.

Verification

Linux, Node 26.12.0, Vite+ 1.4.0; base 5211acfe95a0.

  • Added the regressions before production edits: five failures and 99 passing tests. The failures demonstrated the incorrect stored title, web/mobile projection, and recovery-banner wording. After the fix, all 104 tests in those four files passed:
    vp test run packages/provider-core/src/server/failure.test.ts apps/web/src/session-logic.test.ts apps/mobile/src/lib/threadActivity.test.ts apps/web/src/components/chat/UsageLimitRecoveryBanner.test.tsx.
    Coverage distinguishes exhausted 429s from explicit quota errors, preserves provider details/class/reset evidence, handles legacy titles, and keeps recovered retries labeled as recovered.
  • 13 Codex protocol replays passed, including explicit quota versus structured 429/500 exhaustion and completion-status precedence over cached notifications:
    vp test run apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts -t 'classifies Codex terminal failures|honors completion HTTP status' (run as two focused invocations).
  • 17 existing recovery tests passed, including manual Resume and reset-based recovery guards:
    vp test run apps/server/src/orchestration-v2/runtimeLayer.test.ts -t 'usage-limit recovery'.
  • Ran vp run check:changed with browser/render projects excluded by a temporary subprocess hook outside the repository (Vitest arguments --project '!*browser*' --project '!*renders*'). Format, lint, both knip passes, typechecks of the 31 affected packages/dependents, and 162 tests in eight non-browser files passed. Removed the check runner's result cache afterward so an excluded run cannot be reused as a complete browser-inclusive result. No repository check tooling was changed.

Limitations: browser/computer use was explicitly excluded from this task, so there are no before/after UI screenshots or integrated web/desktop/mobile captures; this does not meet the repository's UI screenshot evidence requirement. An initial attempt to exclude browser files still made Vitest initialize its browser project and failed because Chromium was absent; no browser launched. The final run excluded the projects themselves. No live upstream 429 was induced, and no historical stored-title backfill is included. Diagnostic details that Codex has discarded remain unavailable to T3.

Model: GPT-6.1 Sol, extra-high reasoning, personal profile. Harness: Codex through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 28, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ea55947

Macroscope's review found this PR approvable — Exhausted HTTP 429 retries receive neutral provider-rate-limit wording across web and mobile, with the provider’s explanation visible in work logs. Retry and recovery behavior remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

A shared helper now selects provider failure titles from the failure class and code. Server failure items and web and mobile presentations use those titles. Warning rows now include failure messages, and recovery banners use updated wording when no reset time is reported.

Changes: Provider failure presentation

Layer / File(s) Summary
Shared titles and server failure items
packages/shared/src/orchestrationV2ThreadError.ts, packages/provider-core/src/server/failure.ts, packages/provider-core/src/server/failure.test.ts
providerFailureTitle distinguishes responseTooManyFailedAttempts usage-limit failures from other usage-limit failures and provider errors. Terminal and failed-retry items use the helper. Tests check titles and retained failure data.
Web failure presentations
apps/web/src/session-logic.ts, apps/web/src/session-logic.test.ts, apps/web/src/components/ThreadNotificationCoordinator.tsx, apps/web/src/components/chat/*, apps/web/src/components/sidebar/SidebarThreadRow.tsx
Session labels, notifications, banners, timeline rows, and tooltips use titles derived from failure class and code. The banner updates its missing-reset message and tests cover both usage-limit codes. Warning timeline rows now display the failure message.
Mobile failure presentations
apps/mobile/src/features/threads/*, apps/mobile/src/lib/threadActivity.ts, apps/mobile/src/lib/threadActivity.test.ts
Recovery cards, work-log entries, and activity summaries use titles derived from failure class and code. Warning work-log entries now include the failure message. Activity tests cover multiple failure class and code cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

Suggested reviewers: t3dotgg, juliusmarminge, maria-rcks

Merge Risk: Minimal · up to 9d0b1

No actionable merge-blocking risk remains. Explicit rate-limit wording remains a follow-up.

Pre-merge checks: Passed 3 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Linked Issues check Inconclusive Requirements from #24885 (direct link) that this PR meets: (1) An ambiguous HTTP 429 must not be presented as proof of exhausted allowance. providerFailureTitle in packages/shared/src/orchestrationV… Establish whether the Codex adapter at head can receive a known capacity explanation (for example, the upstream error.message for turn_rate_limit_exceeded) for the affected turn, and if so, which code path carries it to the stored failu…
Passed Passed checks: 3 passed

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: neutral wording for exhausted HTTP 429 retries.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the affected behavior, implementation scope, tests, and known limitations. UI screenshot e…
Out of Scope Changes check Passed Every changed file applies the same explanation correction, or supports its tests. The shared providerFailureTitle helper is used by the server failure builder (failure.ts), the web session logic (s…
Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the affected behavior, implementation scope, tests, and known limitations. UI screenshot evidence is missing, but the omission is explicitly disclosed.

Full details: Out of Scope Changes check

Explanation

Every changed file applies the same explanation correction, or supports its tests. The shared providerFailureTitle helper is used by the server failure builder (failure.ts), the web session logic (session-logic.ts), the sidebar (SidebarThreadRow.tsx), the notification coordinator, the timeline and banner components (TimelineWorkEntryRow.tsx, UsageLimitRecoveryBanner.tsx), and the mobile projection and components (threadActivity.ts, thread-work-log.tsx, UsageLimitRecoveryCard.tsx). The reset-time wording change is part of the same explanation. Tests cover each touched surface (failure.test.ts, session-logic.test.ts, threadActivity.test.ts, UsageLimitRecoveryBanner.test.tsx). The PR does not change the CodexAdapterV2.ts diff, retry policy, Limited state, manual Resume controls, reset timestamps, provider or model selection, serverOverloaded handling, or the ACP ingestion concern. The PR description and objectives both state these exclusions. The merge with main keeps the PR's helper and reset wording, and the reviewed changes list shows no unrelated files. No unrelated change is demonstrated.

Full details: Linked Issues check

Explanation

Requirements from #24885 (direct link) that this PR meets: (1) An ambiguous HTTP 429 must not be presented as proof of exhausted allowance. providerFailureTitle in packages/shared/src/orchestrationV2ThreadError.ts maps usage_limit with responseTooManyFailedAttempts to Provider rate limit (HTTP 429). The title is applied in packages/provider-core/src/server/failure.ts to terminal and failed-retry items, and in the web and mobile projections. (2) Provider details are kept. The warning rows in TimelineWorkEntryRow.tsx and thread-work-log.tsx now show the retained failure message. (3) Manual retry stays available and no reset time is invented. The missing-reset text is now No reset time reported; retry manually, and resetAt remains absent for the exhausted-retry case (failure.test.ts). Requirement not met: #24885 states that a known model-capacity error should retain its capacity explanation. This PR does not add that path. CodexAdapterV2.ts has no diff in this PR. The reported stored failure carries only exceeded retry limit, last status: 429 Too Many Requests. The PR description and objectives state that Codex discards the upstream reason before T3 receives it. The issue also says it does not establish whether T3 received the original upstream body. I did not establish whether any adapter path at head receives the turn_rate_limit_exceeded / capacity text for this failure. My read of CodexAdapterV2.ts was truncated, so that question stays open. Reviewed tests cover the title, retained message, legacy title, and recovered-retry cases. No test covers a known capacity message reaching T3. Explicit rateLimitExceeded (TPM) remains labeled Usage limit reached by design. It is not part of #24885's stated requirements, so it does not decide this assessment.

Resolution

Establish whether the Codex adapter at head can receive a known capacity explanation (for example, the upstream error.message for turn_rate_limit_exceeded) for the affected turn, and if so, which code path carries it to the stored failure. If the path can carry it, add a focused test that a known capacity message reaches the failure item and its presentation. If Codex never delivers that text to T3, state that limitation in the issue and PR as an external constraint so the reviewer can accept the neutral label as the complete fix for #24885.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between ea55947 and 9d0b12f.

Files selected for processing: 10

  • apps/mobile/src/features/threads/thread-work-log.tsx
  • apps/mobile/src/lib/threadActivity.test.ts
  • apps/mobile/src/lib/threadActivity.ts
  • apps/web/src/components/ThreadNotificationCoordinator.tsx
  • apps/web/src/components/chat/TimelineWorkEntryRow.tsx
  • apps/web/src/components/chat/UsageLimitRecoveryBanner.test.tsx
  • apps/web/src/components/chat/UsageLimitRecoveryBanner.tsx
  • apps/web/src/components/sidebar/SidebarThreadRow.tsx
  • packages/provider-core/src/server/failure.test.ts
  • packages/provider-core/src/server/failure.ts

Limit details: You’ve used all 10 included reviews currently available.

maria-rcks commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Takeover update

merged current main into the branch at 9d0b12f855 (merge commit, no rebase). the change's intent and size are unchanged.

  • conflicts were in UsageLimitRecoveryBanner.tsx, its test, and ChatView.tsx. main added a ChatGPT-branded notice and an errorCode prop for the same data this pr passed as failureCode.
  • resolution: kept main's errorCode name and its ChatGPT branch (partner-guideline copy) unchanged. the generic branch now uses providerFailureTitle("usage_limit", errorCode), so exhausted responseTooManyFailedAttempts retries show "Provider rate limit (HTTP 429)". the reset fallback uses this pr's "No reset time reported; retry manually".
  • vp run check:changed passes on a blacksmith testbox: format, lint, knip, typecheck (34 packages), tests (9 files), react compiler.

Not changed

  • explicit rateLimitExceeded (the TPM case reported above) still shows "Usage limit reached". covering it would widen this pr, so it is left for a follow-up.
  • the exhausted-429 ui state is still unverified in a real app. no real 429 was available, same as the earlier note.

Earlier verification (gpt-6.1-sol, at ea55947ac8)

partial verification only. the changed http 429 behavior is unverified.

  • started github/main (32ecd71726) and then the pr with isolated data, one dev server at a time, repo playwright and /usr/bin/google-chrome.
  • created projects pointing at real git checkouts, selected codex gpt-6.1-sol, submitted the same prompt, and saw verification baseline ok. on both revisions. main needed a reload to display its already-persisted answer.
  • on the pr, ran a real codex terminal command (sleep 60), clicked stop, saw Run interrupted · Stopped by user, then saw follow-up after stop ok. in the same thread. persisted run statuses were completed, interrupted, completed.
  • the four changed regression files passed through blacksmith: 104 tests in 4 files.

short provider/server transcript

main, normal provider turn (this does not reproduce the 429 bug):

main real codex answer

pr, matching prompt and answer:

pr real codex answer

shortened capture of the real command, stop and follow-up; sampled frames, not timing evidence:

pr command stop and successful follow-up

not verified then: the original bug on main and the exhausted-429 wording on the pr (the real proxy returned successes, and no mocks or injected errors were used), the limited-state banner, warning row, sidebar tooltip, notification, manual resume, reset scheduling, recovered-retry presentation, desktop electron, native mobile, relay/tunnel, and other operating systems.

@4dwaffle

4dwaffle commented Nov 5, 2026

Copy link
Copy Markdown

Verified an additional case on Windows with T3 0.1.8-nightly.20261104.4165, using Codex through ChatGPT sign-in on a Pro subscription. The user reports 99% of their weekly allowance remaining; I did not independently verify that percentage.

The captured terminal turn/completed error is explicitly a throughput rejection:

codexErrorInfo: rateLimitExceeded
Rate limit reached for gpt-6.1-sol ... on tokens per min (TPM):
Limit 1000000, Used 732209, Requested 360332.
Please try again in 5.552s.

I fetched this PR at ea55947ac8e0f3948ff7ccd400e73b1adc16c87c and executed its exact codexUsageLimitError and providerFailureTitle function bodies against a captured terminal TPM error. It still becomes class: usage_limit with the title Usage limit reached. The added failure.test.ts table explicitly expects that title for rateLimitExceeded; only responseTooManyFailedAttempts receives the neutral rate-limit title.

Could this wording fix also cover explicit rateLimitExceeded? In this incident the provider retains the specific TPM explanation, so the headline can distinguish rate throttling from subscription allowance exhaustion without guessing. Showing the retained message in the timeline is helpful, but the recovery banner and other shared-title consumers would still use the misleading headline.

Verification was limited to executing the PR's classifier/title logic with the captured error and inspecting its web/mobile consumer call sites. No browser test, full adapter replay, or live throttling reproduction was performed. No retry-policy change is requested here.

Posted by Codex (GPT-6.1 Sol) through T3 Code on behalf of the user.

…ording-24885

# Conflicts:
#	apps/web/src/components/ChatView.tsx
#	apps/web/src/components/chat/UsageLimitRecoveryBanner.test.tsx
#	apps/web/src/components/chat/UsageLimitRecoveryBanner.tsx

This branch has not been deployed

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

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Codex capacity HTTP 429 is shown as "Usage limit reached" while allowance remains

3 participants