Skip to content

fix(server): codex usage-limit failures keep their reset time - #27787

Open
imluckii wants to merge 5 commits into
pingdotgg:mainfrom
imluckii:fix/codex-usage-limit-reset-at
Open

imluckii wants to merge 5 commits into
pingdotgg:mainfrom
imluckii:fix/codex-usage-limit-reset-at

Conversation

@imluckii

Copy link
Copy Markdown

Problem

On codex threads, a usage-limit failure that kills a run's FIRST model request carries no resetAt: the mobile failure card shows a bare "Usage limit reached." and the recovery card says "The provider did not report a reset time", so auto-resume cannot be scheduled. Failures in runs that had prior successful responses do carry the reset time. Tracked in #27773 (with protocol-log evidence of both shapes from one live thread).

The adapter derives failure.resetAt only from its per-run rate-limit snapshot ref, which is fed solely by account/rateLimits/updated notifications. Those ride successful streaming responses; a 429 that ends a run before any response delivers only an empty non-codex family snapshot (upstream codex parses the x-codex-active-limit family), which mergeCodexRateLimits correctly ignores. The existing retro-patch of limited items never fires because no codex-family notification arrives while the account stays exhausted.

Change

When a turn terminates as usage_limit with no resetAt, the adapter now reads fresh limits straight from the app-server (account/rateLimits/read, the same request the snapshot probe already uses, with the same 3s deadline) and feeds the result through the existing merge + retro-patch path, so the failure item gains resetAt and UsageLimitRecoveryWorker can schedule auto-resume. The notification handler's inline patch logic is extracted into a shared applyRateLimitSnapshot (behavior unchanged); the read is forked into the adapter scope because its response arrives on the protocol reader fiber the terminal handler runs on. A failed or timed-out read degrades to today's behavior.

Scope and approval

Fixes #27773. The issue was filed with reproduction evidence before this PR; the change is limited to the codex adapter's failure path plus exporting the existing probe timeout constant.

Verification

  • New regression test in CodexAdapterV2.test.ts (recovers resetAt from a fresh rate-limits read when the 429 never primed the snapshot) replays the live failure shape through the existing replay harness: error (usageLimitExceeded) -> empty premium rateLimits notification -> failed turn/completed -> adapter issues account/rateLimits/read -> response carries the codex window -> turn_item.updated with failure.resetAt set. It fails on main (no read issued, no resetAt) and passes with the change.
  • vp test run apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts: 117 passed (117).
  • vp run check:changed: format, lint, knip, graphql, typecheck (3 packages), test (3 files) - all pass.

Not verified: live behavior against a real exhausted ChatGPT-plan account (card date, "Resume at reset", an actual auto-resume) - verified at the adapter layer instead, via the same turn_item.updated path the proven late-reset case uses end to end.

Written with zai glm-5.3 in 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 30, 2026
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

When a Codex usage-limit failure has no reset time, the adapter starts a bounded rate-limit read. It merges the returned snapshot and emits a marked enrichment when a reset time is available. The session manager routes qualifying events to the turn item’s thread, and the event sink preserves an existing reset time during normalization.

Changes: Codex usage-limit reset backfill

Layer / File(s) Summary
Rate-limit snapshot and backfill
apps/server/src/provider/CodexProvider.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
The adapter uses the exported 3,000 ms timeout for a rate-limit read when a usage-limit failure has no reset time. It merges the snapshot and emits an enriched failure item when a reset time is available. The regression test checks the cold-429 flow and verifies that enrichment follows the terminal event and occurs once.
Enrichment event routing and persistence
packages/provider-core/src/server/ProviderAdapter.ts, apps/server/src/orchestration-v2/ProviderSessionManager.ts, apps/server/src/orchestration-v2/EventSink.ts, apps/server/src/orchestration-v2/ProviderSessionManager.test.ts
The event payload can mark usage-limit reset-time enrichment. The session manager routes qualifying events using the turn item’s thread ID. The event sink retains an existing reset time when a normalized usage-limit error has none. Tests check persisted updates, newer-run items, other threads, and subscriber updates.

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant CodexAdapterV2
  participant CodexAppServer
  participant ProviderSessionManager
  participant EventSink
  CodexAdapterV2->>CodexAppServer: account/rateLimits/read
  CodexAppServer-->>CodexAdapterV2: rateLimits snapshot
  CodexAdapterV2->>ProviderSessionManager: marked reset-time enrichment event
  ProviderSessionManager->>EventSink: ingest event for turn item's thread
Loading

Possibly related issues

  • pingdotgg/t3code#29277 — Reports a fresh-session usage-limit failure without resetAt; this change adds a rate-limit read when the failure lacks one.
  • pingdotgg/t3code#28284 — Reports a missing resetAt when the cached snapshot does not show an exhausted window; this change adds a fresh rate-limit read for failures without a reset time.

Possibly related PRs

  • pingdotgg/t3code#29283 — Both changes address Codex usage-limit failures without resetAt; this PR backfills after the terminal event, while #29283 reads and applies limits before publishing the terminal.
  • pingdotgg/t3code#27791 — Both changes target first-request Codex usage-limit failures without a reset time; this PR performs an asynchronous read and routes the later enrichment.
  • pingdotgg/t3code#28291 — Both changes address unknown reset times with a bounded account-limit read; this PR also routes late enrichment and handles persistence races.

Suggested labels: bugfix

Suggested reviewers: macroscopeapp, juliusmarminge

Merge Risk: Minimal · up to 6e4a8

The reset-time backfill and automatic-resume scheduling path appear mergeable after normal checks; live automatic resumption remains unverified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "fix(server): codex usage-limit failures keep their reset time" matches the main change. The PR makes the Codex adapter read fresh rate limits when a usage-limit turn failure has no reset ti…
Description check Passed The description covers the required template sections: Problem, Change, Scope and approval, and Verification. It explains the root cause (a first-request 429 yields no resetAt because the snapshot is …
Linked Issues check Passed [#27773] requires a Codex usage_limit failure to carry resetAt when the reset time is obtainable, including when the run's first request receives a 429. Evidence at the reviewed head: (1) CodexAdapter…
Out of Scope Changes check Passed Changes since the earlier review and the whole-PR summary map to #27773. CodexAdapterV2.ts: the read path and the applyRateLimitSnapshot extraction (behavior unchanged for notifications). CodexProvide…
Full details: Title check

Explanation

The title "fix(server): codex usage-limit failures keep their reset time" matches the main change. The PR makes the Codex adapter read fresh rate limits when a usage-limit turn failure has no reset time, so the failure keeps a non-null resetAt. The title follows the conventional commit format and is specific enough for a teammate scanning history.

Full details: Description check

Explanation

The description covers the required template sections: Problem, Change, Scope and approval, and Verification. It explains the root cause (a first-request 429 yields no resetAt because the snapshot is only fed by successful streaming responses) and the fix (a fresh account/rateLimits/read with the existing 3s deadline, merged through the existing retro-patch path). It links issue #27773 and cites a specific regression test with pass/fail evidence and a 117-test run. It also states what was not verified (live exhausted-account behavior). The description predates later pushed changes noted in the objectives, but it still matches the core change and is mostly complete.

Full details: Linked Issues check

Explanation

[#27773] requires a Codex usage_limit failure to carry resetAt when the reset time is obtainable, including when the run's first request receives a 429. Evidence at the reviewed head: (1) CodexAdapterV2.ts, per the change summary, issues account/rateLimits/read when a usage_limit terminal has no resetAt. It applies the result through the shared applyRateLimitSnapshot path with the existing 3s RATE_LIMITS_PROBE_TIMEOUT_MS (exported from CodexProvider.ts). A failed or timed-out read leaves the snapshot unchanged. (2) EventSink.ts (read at head) keeps a non-null resetAt from the existing projected usage_limit error item when an incoming codex turn-item error has resetAt == null. This covers late enrichment after the terminal. (3) ProviderSessionManager.ts routes marked enrichment events (usageLimitResetAtEnrichment) so they persist after run settlement. (4) CodexAdapterV2.test.ts adds the regression test 'recovers resetAt from a fresh rate-limits read when the 429 never primed the snapshot'. The PR also adds persistence tests in ProviderSessionManager.test.ts. The live exhausted-account test reported a persisted resetAt matching the Codex reset time and a card with resume/snooze actions. Manual 'Resume at reset' worked. Automatic resume was not observed. UsageLimitRecoveryWorker itself is unchanged. The issue's expected behavior is met at the data and card layers. Two limits remain: the turn error message source ('try again at <time>') the issue lists is not parsed, and a failed read still yields null resetAt. The issue's wording 'whenever obtainable' is met by the fresh read path that the issue names as a source.

Full details: Out of Scope Changes check

Explanation

Changes since the earlier review and the whole-PR summary map to #27773. CodexAdapterV2.ts: the read path and the applyRateLimitSnapshot extraction (behavior unchanged for notifications). CodexProvider.ts: exports the existing 3_000 ms probe constant. packages/provider-core ProviderAdapter.ts: adds an optional usageLimitResetAtEnrichment?: true marker so the session manager can route enrichment. EventSink.ts: the persistence guard is gated to codex, usage_limit, incoming resetAt == null, and an existing non-null resetAt (verified in the file at head). ProviderSessionManager.ts: routes turn_item.updated only for marked enrichment with non-null resetAt. The related tests cover these paths. Each change supports persisting the enriched resetAt. The PR description's 'Scope and approval' section is stale. It lists only the adapter and constant export, and omits the EventSink, ProviderSessionManager, and provider-core changes. This is a documentation mismatch, not an out-of-scope change. The incremental diff between previous reviewed heads and head was unavailable in this review. Scope was judged from the whole-PR summary and the EventSink.ts file read at head.

Finishing Touches

Generate unit tests (beta)

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

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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 dc900ce and 6e4a8fd.

Files selected for processing: 3

  • apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
  • apps/server/src/orchestration-v2/ProviderSessionManager.test.ts

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

maria-rcks commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Note

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

Takeover update (head 6e4a8fd32e)

  • merged current main (merge commit, no rebase). one conflict in CodexAdapterV2.ts: main's fix(codex): sharing usage limits offer the existing recovery options #25341 added failureResetAt, which leaves sharing-cap usage limits without a reset time. kept this PR's shared applyRateLimitSnapshot (used by both the notification and the backfill read) and made it use failureResetAt per item, so both sides hold.
  • the CodeRabbit finding about late enrichment being overwritten was already fixed in dc900cef7b.
  • vp run check:changed passes (format, lint, knip, typecheck of 26 packages, tests). CodexAdapterV2.test.ts, ProviderSessionManager.test.ts and codexUsageLimits.test.ts pass (212 tests).
Earlier verification

Verified in a real dev environment

partial verification only. the reset-time fix remains unverified.

  • installed github/main at 5fe18d79ac in a separate worktree, then installed the exact pr head 07623f8b35f6ccd7c5715dbadaabd9f75419b075. ran one background dev server at a time with separate empty state, stopping main before starting this head.
  • drove the actual web app with repo playwright and /usr/bin/google-chrome. created projects pointing at the real repository through the app. no mocks, fixtures, replay provider, or database injection.
  • real codex gpt-6.1-sol turns completed on main and this head. on this head, a second turn on the same native codex thread completed, and both responses survived a browser reload. provider protocol logs confirm both sol turns completed on the same native thread.
  • opening usage > limits on both versions displays Codex could not read usage (JSON-RPC -32600) without preventing normal conversation.
  • the focused existing regression test, recovers resetAt from a fresh rate-limits read, passed through blacksmith-check (1 passed, 116 filtered out). this is adapter coverage, not proof of the live reset flow. the pr reports 31 successful checks.

main usage view:

main: account-rate-limit read is unavailable

pr usage view with the same project path, prompt, and model:

pr: account-rate-limit read remains unavailable

real sol continuation, from submission through response:

pr: real codex gpt-6.1-sol continuation completes

short provider transcript:

account/read: {"requiresOpenaiAuth":false}
account/rateLimits/read:
  error -32600: codex account authentication required to read rate limits
main: turn/start model=gpt-6.1-sol -> turn/completed status=completed
pr: turn/start model=gpt-6.1-sol -> turn/completed status=completed
pr: same native thread, turn/start model=gpt-6.1-sol -> turn/completed status=completed

Not verified

the original first-request usage-limit failure could not be reproduced on main. this vm uses the local proxy without a codex account login, and the real app-server rejects account/rateLimits/read with the error above. successful proxy turns do not establish behavior for an exhausted chatgpt-plan account.

therefore the new failure-triggered read, failure.resetAt backfill, reset date display, resume-at-reset scheduling, and actual automatic continuation remain unverified. these need a real exhausted chatgpt-plan codex account, first on main and then on this head. native ios/android, electron, macos/windows, and remote/relay connections were not exercised on this linux vm.

no source changes or pushes were made. the running-app evidence supports the neighboring paths only; it does not establish that the changed reset-time behavior works.

@imluckii

imluckii commented Nov 2, 2026

Copy link
Copy Markdown
Author

Real-account verification, as asked in #27787 (comment): a real ChatGPT Plus codex account was driven to 100% of its 5h window by normal traffic (no artificial burn), then one tiny turn was sent per side. Setup: two stock installs on one Linux box, current nightly 0.1.7-nightly.20261101.4035 (main behavior) and this PR's head 07623f8b, separate t3 homes and ports, native codex login, no proxy between the driver and the app-server (the part that was not testable before).

What happened (probes at 2026-11-01 20:19 UTC, account already at primary 100%):

  • main: turn fails, failure classifies as usage_limit, failure.resetAt stays null, card shows no reset time. The reported bug, reproduced.
  • this PR: the new backfill does fire. account/rateLimits/read goes out 120 ms after the failure and the app-server answers 1.6 s later (inside the 3 s timeout) with a valid decoded snapshot: primary 100%, resetsAt present, and it matches the "try again at 5:15 AM" text in the error.
  • but nothing happens with the response: no turn_item.updated is emitted, the persisted turn item keeps resetAt: null, a fresh page load 40 minutes later still shows "Reset time unavailable; retry manually", and nothing auto-resumed at the boundary either (checked 4+ hours past it).

Both cards, identical UI:

main (nightly) PR head 07623f8b
main nightly card: Usage limit reached, Reset time unavailable; retry manually PR head card: Usage limit reached, Reset time unavailable; retry manually

Why the replay test does not catch this. "recovers resetAt from a fresh rate-limits read when the 429 never primed the snapshot" passes on the exact tree I tested. The harness answers the read instantly, so the backfill's continuation is still alive when it arrives. In the real run the response lands 1.6 s after the failure, and the run has already settled by then. Unproven suspicion (no source instrumented on my side): the forked backfill's continuation is interrupted at settle, so the response has nowhere to land. Worth checking which scope the forkIn(scope) at the backfill call site survives.

Happy to re-run with an instrumented build or more scenarios (mid-turn 429, secondary-window exhaustion) if that helps. Everything above is from logs and persisted state, not memory.

Verification rig and write-up produced with gpt-6.1-sol in T3 Code.

@imluckii

imluckii commented Nov 2, 2026

Copy link
Copy Markdown
Author

Correction to my earlier comment: the "forked backfill loses the race" theory was wrong. Real cause: when a run fails, event ingestion stops at settlement, so the backfill enrichment that arrived 1.6s after the failure had zero subscribers. fix(server): persist Codex reset enrichment after run settlement (now pushed on top of this PR's head) routes the marked enrichment through the session pump's direct-ingestion lane instead.

Live verification from today on a genuinely exhausted ChatGPT-plan codex account (natural exhaustion, watcher-confirmed at 95-99%, resetAt 17:22 IST):

  • Fired a probe turn at 95% window usage. Projection row now records the failure with failure.resetAt = 2026-11-02T11:52:22Z (non-null, matches codex's "try again at 5:22 PM"). On main nightly (Nov 1 A/B) this field stayed empty.
  • The failure card renders the reset time: "Usage limit reached. Retry after 5:22 PM.", plus a composer banner "Usage limit reached · Resets 5:22 PM" with Resume at reset / Snooze until reset actions (main showed "reset time unavailable; retry manually"):

card

  • After the boundary passed, Resume at reset resumed the thread and the queued turn answered ok; the banner cleared and the sidebar badge flipped from Limited to fresh:

resumed

Not verified / follow-ups: auto-resume did not fire on its own ~15 min past the boundary (headless rig, no browser tab open at the boundary, so a client-side scheduler would not have been alive — worth confirming whether auto-resume should be server-driven); single account, primary 5h window only; secondary weekly window not exercised; no native/electron client.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

@imluckii

imluckii commented Nov 3, 2026

Copy link
Copy Markdown
Author

Fix pushed for the CodeRabbit finding (verified real): dc900cef7b "fix(server): preserve enriched Codex usage-limit reset times".

The race is exactly as flagged. The adapter's in-memory guard (applyRateLimitSnapshot skips items that already carry a resetAt) protects the tracked item, but persistence had no such guard: the marked enrichment commits through the direct-ingestion lane immediately, while the terminal failure enters subscriber queues without waiting for consumer persistence. Late subscription has a fresh queue with no replay, and terminal normalization recreates the failure item from the terminal's own resetAt: null, which the projection then stores unconditionally - wiping the enriched resetAt back to null.

The guard lives in the persistence path itself (EventSink.ts): when a codex usage_limit terminal with null resetAt is about to be written and the persisted turn item already carries a non-null one, the terminal payload is enriched with the existing value before the write, so the event log and projection stay consistent instead of diverging. Behavior is unchanged when no prior resetAt exists.

Evidence: regression test in ProviderSessionManager.test.ts (extends the subscribed/unsubscribed harness) failed 2/2 before the fix and passes after; full affected suites green (ProviderSessionManager + CodexAdapterV2 + ProviderEventIngestor, 220 tests); vp run check:changed green. Not verified live: real codex timing to hit the race window in production - the trace and test reproduce it deterministically on the server side.

# Conflicts:
#	apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts

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 usage-limit failures lose resetAt when a run's first request hits the 429 (no auto-resume, no reset time on the card)

2 participants