Skip to content

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

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

samaluk wants to merge 1 commit 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

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 5211acf and ea55947.

Files selected for processing: 15
  • apps/mobile/src/features/threads/UsageLimitRecoveryCard.tsx
  • 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/ChatView.tsx
  • 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
  • apps/web/src/session-logic.test.ts
  • apps/web/src/session-logic.ts
  • packages/provider-core/src/server/failure.test.ts
  • packages/provider-core/src/server/failure.ts
  • packages/shared/src/orchestrationV2ThreadError.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The change adds a shared failure-title function and uses it in server failure items and mobile and web presentations. For responseTooManyFailedAttempts, usage-limit failures display “Provider rate limit (HTTP 429).” Recovery messaging and warning-row failure details are also updated.

Changes: Provider failure presentation

Layer / File(s) Summary
Shared titles and terminal failure items
packages/shared/src/orchestrationV2ThreadError.ts, packages/provider-core/src/server/failure.ts, packages/provider-core/src/server/failure.test.ts
A shared function selects provider failure titles from the failure class and code. Terminal and failed-retry items use those titles, with tests for multiple combinations.
Web failure presentations
apps/web/src/components/chat/UsageLimitRecoveryBanner.tsx, apps/web/src/components/chat/UsageLimitRecoveryBanner.test.tsx, apps/web/src/components/ChatView.tsx, apps/web/src/components/ThreadNotificationCoordinator.tsx, apps/web/src/components/chat/TimelineWorkEntryRow.tsx, apps/web/src/components/sidebar/SidebarThreadRow.tsx, apps/web/src/session-logic.ts, apps/web/src/session-logic.test.ts
Web presentations use failure titles derived from class and code. The recovery banner receives the runtime error code and changes its no-reset message. Warning timeline rows now display the failure message. Tests cover both usage-limit codes and retry metadata.
Mobile failure presentations
apps/mobile/src/features/threads/UsageLimitRecoveryCard.tsx, apps/mobile/src/features/threads/thread-work-log.tsx, apps/mobile/src/lib/threadActivity.ts, apps/mobile/src/lib/threadActivity.test.ts
Mobile recovery and work-log labels use failure titles. Warning work-log rows now display the failure message, and historical activity summaries use the derived title. Tests cover failure codes and recovered errors.

Priority: Low

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#23842: Implements the bounded Codex HTTP 429 retry path and terminal exhausted-retry failure that this change presents with a distinct title.
  • pingdotgg/t3code#25341: Adds usage-limit failure classes and codes consumed by the shared title function.

Suggested reviewers: t3dotgg, juliusmarminge, maria-rcks

Merge Risk: Minimal · up to ea559

HTTP 429 failures receive neutral rate-limit wording. No merge-blocking regression is evident.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description includes all required sections and gives detailed problem, scope, implementation, and verification information. However, the change affects multiple UI surfaces and the required before… Add clear before/after screenshots for the affected UI changes, and include a recording if timing, transitions, or interaction details require it. If screenshots cannot be provided, obtain and document an approved exception to the repositor…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: neutral wording for exhausted Codex HTTP 429 retries.
Linked Issues check Passed The PR addresses the coding requirements in #24885. providerFailureTitle maps usage_limit with responseTooManyFailedAttempts to Provider rate limit (HTTP 429) and keeps usageLimitExceeded as…
Out of Scope Changes check Passed The changed server, web, desktop, and mobile presentation paths support the same explanation correction requested by #24885. The shared helper and projection changes remove inconsistent hard-coded tit…
Full details: Linked Issues check

Explanation

The PR addresses the coding requirements in #24885. providerFailureTitle maps usage_limit with responseTooManyFailedAttempts to Provider rate limit (HTTP 429) and keeps usageLimitExceeded as Usage limit reached. failure.ts applies the title to terminal and failed-retry items while retaining the failure object. The web and mobile changes apply the same decision and display the retained failure message in warning rows. Missing reset guidance uses No reset time reported; retry manually. The changes preserve manual recovery, reset evidence, retry policy, and recovered-retry presentation. The added tests cover ambiguous 429 failures, explicit quota failures, retained messages, legacy titles, and recovery behavior.

Full details: Out of Scope Changes check

Explanation

The changed server, web, desktop, and mobile presentation paths support the same explanation correction requested by #24885. The shared helper and projection changes remove inconsistent hard-coded titles. The tests verify the related behavior. No unrelated recovery workflow, automatic retry, wire-contract change, serverOverloaded handling, provider selection, or historical backfill appears in the reviewed scope.

Full details: Description check

Explanation

The description includes all required sections and gives detailed problem, scope, implementation, and verification information. However, the change affects multiple UI surfaces and the required before/after screenshot evidence is missing; the description acknowledges this limitation.

Resolution

Add clear before/after screenshots for the affected UI changes, and include a recording if timing, transitions, or interaction details require it. If screenshots cannot be provided, obtain and document an approved exception to the repository requirement.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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

1 participant