Skip to content

fix(server): Codex usage-limit failures on a fresh session carry their reset time - #29283

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

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

Conversation

@rwspatin

@rwspatin rwspatin commented Nov 2, 2026 •

Copy link
Copy Markdown

Fixes #29277

Problem

A Codex session that starts already limited never gets account/rateLimits/updated, so its usage-limit failure goes out with resetAt: null. Without a reset time the thread never becomes a recovery candidate, and auto-resume (or Resume at reset) never arms.

The existing late back-fill can't cover this case. RunExecutionService stops ingesting a run's events at its failed terminal (Stream.takeUntilEffect(shouldStopProviderEventIngestion)), so a turn_item.updated that arrives after the terminal is never stored.

Fix

In CodexAdapterV2, when a root terminal is a usage_limit failure with no reset time, and the rate-limit snapshot can't supply one, the adapter reads account/rateLimits/read and merges the result through the same path the rateLimits/updated handler uses (applyRateLimits). Only then does it publish the terminal, so the terminal event itself carries resetAt.

  • The read runs in a forked fiber. Notification handlers run on the client's reader fiber, so awaiting the reply inline would deadlock.
  • The adapter's own bookkeeping (terminalized turns, idle root thread) still happens immediately. Only the outgoing event waits.
  • The read is bounded by a 3s timeout. On error or timeout the terminal is published with resetAt: null, as before.
  • The extra read happens only for limited turns whose reset time is unknown.

Tests

  • A new read-reset row in the existing "classifies Codex terminal failures" table replays a fresh limited session: error with usageLimitExceeded, no rateLimits/updated, and a read answered with the weekly window at 100%. It fails on main (expected null to equal '2033-05-19T07:20:00.000Z') and passes with the fix.
  • The other usage_limit rows now answer the read with an error. usage asserts that resetAt stays null, which covers the read-failure path.
  • CodexAdapterV2.test.ts: 134 passed. vp run check:changed: all steps pass.

Runtime transcript

Real Codex CLI 0.163.0 on a ChatGPT Plus account whose weekly window was exhausted. Dev server from this branch with isolated worktree state, Resume after usage limits on, model gpt-5.6-luna:

before (Codex adapter identical to main):
  thread start --wait … → stopped with status failed: You've hit your usage limit … try again at Nov 4th, 2026 3:39 AM.
  error item failure: {class: usage_limit, code: usageLimitExceeded, resetAt: null}
  thread limitRecovery: (none)

this branch, same account and command:
  thread start --wait … → stopped with status failed: You've hit your usage limit … try again at Nov 4th, 2026 3:39 AM.
  error item failure: {class: usage_limit, code: usageLimitExceeded, resetAt: 2026-11-04T06:39:47.000Z}
  thread limitRecovery: {runId: …ordinal:1, resetAt: 2026-11-04T06:39:47.000Z, autoResume: true, snooze: false}

Notes

Implemented with Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

…r reset time

A Codex session that starts already limited never receives
account/rateLimits/updated, so the usage-limit failure was published with
resetAt null and auto-resume never armed. A later back-fill could not help:
runs stop ingesting at a failed terminal.

When the failure has no reset time and the snapshot cannot provide one, read
account/rateLimits/read before publishing the terminal (forked, 3s timeout,
falls back to null on error).

Fixes pingdotgg#29277

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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 Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 58b9c19

Macroscope's review found this PR approvable — Codex usage-limit failures now fetch missing reset times before publishing, restoring existing recovery behavior without changing user preferences or defaults. The read is bounded to three seconds and preserves the previous fallback on failure.

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

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

When a failed Codex usage-limit terminal has no reset time in the cached snapshot, the adapter reads account rate limits before publishing the terminal. Rate-limit notification handling uses a shared function to update snapshots and limited-turn items. Replay tests cover read responses and reset-time outcomes.

Changes: Codex rate-limit terminal handling

Layer / File(s) Summary
Rate-limit notification handling
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
A shared handler merges rate-limit snapshots, fills missing reset times on limited-turn items, emits item updates, and calls onUsageLimits when limits change.
Terminal reset-time lookup and replay coverage
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
When a failed usage-limit terminal lacks a reset time in the cached snapshot, the adapter reads account rate limits with a three-second timeout, applies the response, then publishes the terminal. Tests cover read responses, reset-time assertions, and usage-limit classification.

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Adapter as CodexAdapterV2
  participant AppServer as Codex app-server
  participant Apply as applyRateLimits
  participant Publish as publishRootTerminal
  Adapter->>AppServer: Read account rate limits when reset time is missing
  AppServer-->>Adapter: Return rate-limit response
  Adapter->>Apply: Apply returned limits
  Adapter->>Publish: Publish terminal
Loading

Possibly related issues

Suggested labels: bugfix

Suggested reviewers: yash-singh1, juliusmarminge

Merge Risk: Minimal · up to 71cf4

The reset-time lookup supports recovery from a first-turn usage-limit failure, with no actionable merge-blocking issue established.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely identifies the Codex usage-limit failure fix and the missing reset time addressed by the changes.
Description check Passed The description explains the problem, fix, scope, implementation constraints, tests, observed results, and runtime verification. The Fix and Tests headings cover the template's Change and Verification…
Linked Issues check Passed Issue [#29277] requires a reset time on a Codex usage-limit failure from a fresh, already-limited session. The reviewed head meets this in CodexAdapterV2.ts. The new emitRootTerminal checks four c…
Out of Scope Changes check Passed All changes in the whole-PR diff (merge base 67eebac to head 71cf4c7) connect to issue [#29277]. (1) applyRateLimits extracts the existing account/rateLimits/updated handler body without changin…
Full details: Description check

Explanation

The description explains the problem, fix, scope, implementation constraints, tests, observed results, and runtime verification. The Fix and Tests headings cover the template's Change and Verification sections, and the linked issue supports the focused scope.

Full details: Linked Issues check

Explanation

Issue [#29277] requires a reset time on a Codex usage-limit failure from a fresh, already-limited session. The reviewed head meets this in CodexAdapterV2.ts. The new emitRootTerminal checks four conditions: the terminal is a failed usage_limit, failure.resetAt is null, the code is not a managed sharing-cap usage limit, and codexUsageLimitResetAt of the cached snapshot is null. When all four hold, it calls account/rateLimits/read with a 3-second Effect.timeout. It merges the result through the shared applyRateLimits handler, which the account/rateLimits/updated handler also uses. Only then does it publish the terminal, in a fork (Effect.forkIn(scope)). Error and timeout paths log at debug level and publish with the original null reset time. The bookkeeping (terminalized native turns, idle root thread) runs right after the fork starts, so it does not wait for the read. The test table adds a read-reset row. That row replays no rateLimits/updated, answers the read with a weekly window at 100%, and asserts terminal.failure.resetAt equals the expected reset. The usage row asserts null when the read fails. Coding requirements are met by the diff and tests. I did not run the tests. The reported 134 passing tests come from the PR description.

Full details: Out of Scope Changes check

Explanation

All changes in the whole-PR diff (merge base 67eebac to head 71cf4c7) connect to issue [#29277]. (1) applyRateLimits extracts the existing account/rateLimits/updated handler body without changing its behavior. The read path reuses it, so this extraction is needed. (2) The CODEX_RATE_LIMITS_READ_TIMEOUT constant and the read block serve the fix. (3) Splitting emitRootTerminal into publishRootTerminal and the bookkeeping wrapper keeps the idle-root and terminalized-turn updates immediate while the publish waits. (4) The classifyCodexManagedError(...)?.usageLimit guard skips the read for managed sharing-cap errors. The PR description says this guard came from merging main for the #25341 conflict. It keeps a null reset time for those errors, so it does not add unrelated behavior. (5) Test changes add the codexRateLimitsReadEntries helper, the read-reset row, and read-failure replays for other usage-limit rows. These changes test the new path. The raw summary says the prior usage-limit failure-item storage and emission was removed. The diff does not show that: the makeProviderFailureTurnItem block for usage-limit failures remains in publishRootTerminal, and the removed lines are only the bookkeeping moved to the wrapper. I rely on the diff. No unrelated product behavior or unrelated files appear in the reviewed hunks.

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 58b9c19 and 71cf4c7.

Files selected for processing: 2

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

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

@rwspatin

rwspatin commented Nov 4, 2026

Copy link
Copy Markdown
Author

Follow-up with a real, unmocked run of this branch (58b9c1902b), local dev server, real Codex account that hit its usage limit:

  1. Limit hit (2026-11-02 12:14Z): the run failed with usage_limit. With this fix, the adapter read account/rateLimits/read before publishing the terminal, so the failure carried resetAt: 2026-11-04T06:39:47.000Z. limitRecovery got armed (autoResume: true). On main the same failure has resetAt: null and nothing gets armed.
  2. Reset reached (2026-11-04 06:39:47.156Z): UsageLimitRecoveryWorker fired at the reset second and created continuation run ordinal 2 (Continue where you left off., userMessageId: limit-resume:…). The dev server had been restarted in the meantime, so this also shows the armed state surviving a restart.
run ordinal 1  failed  requested 2026-11-02T12:14:36Z  (usage_limit, resetAt 2026-11-04T06:39:47Z)
run ordinal 2  failed  requested 2026-11-04T06:39:47.156Z  ← auto-resume, triggered by the worker

Run 2 failed for a reason outside this PR. At 03:39 local time the Mac had no network access for apps (Codex's own log: application network policy is unavailable). Because of that, codex app-server rejected the config with failed to load configuration: Failed to synchronize managed preferences. The same call works normally once the machine is awake. So the part this PR fixes is shown end to end: the reset time is captured, the thread is armed, and the worker fires on time.

A side observation for maintainers, not something this PR touches: the provider-turn.start effect spent all 5 attempts within about 2 seconds. A transient provider startup failure right at the reset then ends the auto-resume for good. A short backoff there might be worth considering.

@rwspatin

rwspatin commented Nov 4, 2026

Copy link
Copy Markdown
Author

Full end-to-end confirmation, again unmocked, on this branch (58b9c1902b) with a real Codex account. This time the machine stayed awake through the reset:

run ordinal 1  failed     requested 2026-11-04T09:56:54Z  (usage_limit, resetAt 2026-11-04T11:46:05Z)
run ordinal 2  completed  requested 2026-11-04T11:46:09Z  ← auto-resume at the reset, no user action
[user]
Reply with exactly: resumed after limit. Do not use tools or change files.

[user]
Continue where you left off.

[assistant]
resumed after limit.

The resetAt on the failure (11:46:05Z) matches the time Codex's own error message gave ("try again at 8:46 AM", UTC-3). limitRecovery armed with autoResume: true, and the worker started the continuation 4s after the reset. Codex answered in the resumed run. On main this same failure has resetAt: null, so nothing would be armed and the thread would just stay failed.

…-reset-at

# Conflicts:
#	apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
#	apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

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

Merged current main (merge commit 71cf4c7232, no rebase) to clear the conflict with #25341 in CodexAdapterV2.ts and its test.

Resolution:

  • applyRateLimits keeps this PR's shared handler and uses main's per-item failureResetAt, so sharing caps still never get an allowance reset time.
  • emitRootTerminal skips the account/rateLimits/read for managed sharing caps (classifyCodexManagedError(...).usageLimit). Their reset time stays null by design, so the read would only delay the terminal by up to 3s.
  • The test adds the read replay only for non-sharing usage-limit scenarios, and keeps main's resetAfter snapshot.

vp run check:changed passes (format, lint, knip, typecheck, 146 adapter tests).

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 on a fresh session have no reset time, so auto-resume never arms

2 participants