Skip to content

fix(codex): answer the main hard lock as a usage limit, clamp retry to pool recovery (#7046) - #7076

Merged
lidge-jun merged 3 commits into
devfrom
fix/7046-hard-lock-usage-limit
Oct 13, 2026
Merged

lidge-jun merged 3 commits into
devfrom
fix/7046-hard-lock-usage-limit

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

The main-account quota guard (codexMainAccountHardLock) refused locally with 429 rate_limit_error / rate_limit_exceeded and a Retry-After pointing at main's weekly reset. Codex treats that as transient: in the reported incident it scheduled a retry 5 days out (Retry-After: 446329) and kept the Goal active for hours, even though a sibling pool account had recovered about 1.5 hours after the refusal.

After this change:

  • A hard-lock refusal is 429 with error.type = error.code = usage_limit_reached, the same policy message, and resets_at (epoch seconds), with no plan_type. Codex treats this as final: the turn fails and the Goal becomes usageLimited (the issue's isolated app-server run showed exactly this transition). It uses the same body shape as the Anthropic pool fix (fix(anthropic-pool): answer spent quota as a usage limit and lift Retry-After quota cooldowns on fresh reads #6995), via formatUsageLimitReachedResponse. HTTP (cooldownErrorResponse) and the Codex WebSocket error frame both use it.
  • Retry-After and resets_at are clamped to the next account-pool recovery time when an unpinned pool request fell through to the locked main and another stored pool account is known to recover sooner. Both name the same instant. When nothing is known, both are omitted as before.
  • The guard, its thresholds, account routing, credits consent and pins are unchanged. Other cooldown classes (pool cooldowns, credits-off, Reserve) keep their retryable rate_limit_error contract byte for byte.

Before → after for the incident: rate_limit_exceeded, Retry-After: 446329, Goal active → usage_limit_reached, Retry-After/resets_at ≈ the sibling's recovery (~1.5 h after the refusal, about 14:04 local in the issue's timeline), Goal usageLimited.

Pool recovery policy (decided in this PR; Jun delegated the call)

nextCodexPoolRecoveryAt (src/codex/routing/pool-recovery.ts) is deliberately conservative. It only predicts a retry time; it selects nothing and changes no state:

Rule Why
Applied only for pool mode, unpinned requests (no explicit account, not Reserve, main not excluded). Pins, Direct, Reserve and the dispatch-time recheck keep main's own reset. A pinned or main-only request can't be served by a sibling, so a pool time would be a false promise.
Candidates: stored pool accounts that are not paused, plan-excluded, sign-in-quarantined, otherwise unusable, or model-denied for this request. For an account-gated model, only accounts in the resolved entitlement roster count, and with no resolved roster none do. Those are operator, credential or entitlement states no clock lifts.
An account's recovery = the latest of every routing hold: quota-health cooldown, quota avoidance, transient soft avoidance, pool-refresh backoff, credits-off usage-limit reset. Selection only picks the account once every hold has ended.
Accounts with a hold whose end is unknown, under the sliding failure-window guard (its end moves with every failure), or with no recorded hold, are skipped. Guessing would invite a retry that just meets the same refusal.
The pool time is used only when it comes before main's reset, and never less than 60 s out. It never extends the wait, and it avoids a tight retry loop on a hold about to lapse.
It is a hint only: no automatic Goal continuation, no account switch, no change to the main guard. The issue notes a reset timestamp isn't proof of fresh quota. The retry runs ordinary selection from fresh state, and that decides.

I chose these rules to keep the existing safety boundaries (eligibility, pins, replay safety, credits consent, the main guard). The fix gives a truthful terminal state plus the earliest honest "try again at" time. Automatic Goal resumption stays a client concern.

Closes #7046

Verification

  • bun test tests/codex-integration/main-account-hard-lock-usage-limit.test.ts (new, 8 tests): the incident's exact 446329 deadline → usage_limit_reached + matching resets_at; missing/past/NaN/∞ reset → neither header nor field; earlier pool recovery clamps both, later does not; 60 s floor; past hint ignored; pool hint alone when main's reset is unknown; pool cooldown and credits-off keep rate_limit_error; estimator rules (latest hold wins incl. soft avoid, denied/needs-reauth/paused/out-of-roster excluded, no-hold → none); a gated model (Daybreak) takes the hint only from a roster-admitted sibling; end-to-end resolveCodexAuthContext in pool mode carries the sibling's recovery, and a main-pinned request does not.
  • The hint reuses the request's own fenced model evidence (the gated roster and the read-fenced denial set computed during selection); it never re-reads entitlements outside that fence.
  • Individually: every tests/codex-integration/main-account-*.test.ts, codex-credits-after-limit-main, reserve-auth-context, every test file referencing cooldownErrorResponse / the hard lock / websocket-handler, file-size ratchet, test-layout guards, and every test that reads auth-context.ts, routing/health-store.ts or websocket-handler.ts as source (incl. codex-model-entitlements): pass.
  • bun run typecheck, bun run structure:check, bun run privacy:scan: pass.
  • Not run locally: full bun run test / test:changed. auth-context.ts pulls in nearly the whole suite, and the shared Bun test lock was held by other worktrees on this machine. The ubuntu test shards on this PR cover it.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (structure/providers/openai-accounts.md, docs-site/.../reference/codex-retry-diagnostics.md)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (no account ids in bodies; the guard and credential handling are unchanged)

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 13, 2026 17:07
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T17:11:41.817418Z 34d6112 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: bd901e3f-6b84-442e-9888-f1edfe35bb63
📥 Commits

Reviewing files that changed from the base of the PR and between 34d6112 and 312df15.

📒 Files selected for processing (4)
  • src/codex/auth-context.ts
  • src/codex/routing/pool-recovery.ts
  • structure/providers/openai-accounts.md
  • tests/codex-integration/main-account-hard-lock-usage-limit.test.ts

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

📝 Walkthrough

Hard-lock refusals now use usage_limit_reached and include retry details when a valid reset time is known. Eligible unpinned Pool requests can use another eligible pool account’s recovery time. WebSocket responses and documentation reflect the updated contract.

Changes: Codex hard-lock retry handling

Layer / File(s) Summary
Estimate eligible pool recovery times
src/codex/routing/pool-recovery.ts, src/codex/routing/health-store.ts, tests/codex-integration/main-account-hard-lock-usage-limit.test.ts
Adds nextCodexPoolRecoveryAt to return the earliest future recovery among eligible accounts. It combines known holds per account and skips accounts without a qualifying recovery time.
Attach recovery hints and format hard-lock responses
src/codex/auth-context.ts, tests/codex-integration/main-account-hard-lock-usage-limit.test.ts
Eligible unpinned Pool requests can attach a sibling account’s recovery time to a hard-lock error. Hard-lock responses use usage_limit_reached; the retry time is the earlier of the main reset and the adjusted pool recovery hint. Other cooldown errors retain their existing response path.
Expose and document retry details
src/server/index/websocket-handler.ts, structure/providers/openai-accounts.md, docs-site/src/content/docs/reference/codex-retry-diagnostics.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
WebSocket hard-lock frames include the response body and headers. Documentation describes the retry contract, and the test-layout mappings include the new integration test.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 312df

Hard-lock refusals now return a terminal usage-limit 429 with accurate retry details, and routing and eligibility are unchanged. No actionable merge-blocking risk was found.

Pre-merge checks · Passed 3 · Failed 2

Failed Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check Warning Issue #7046's terminal contract is implemented. src/codex/auth-context.ts maps CodexMainAccountHardLockError to usage_limit_reached and carries a finite reset or an earlier eligible pool recover… Implement the missing Goal or equivalent control-plane integration for #7046. After a terminal main hard-lock refusal, expose the waiting or unavailable state with the computed recovery time and re-evaluate eligible pool accounts when that …
Docstring Coverage Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check Passed The reviewed changes stay within issue #7046. src/codex/auth-context.ts changes the hard-lock response contract and propagates the pool recovery hint. src/codex/routing/pool-recovery.ts computes t…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately and concisely describes the main changes: returning main hard-lock refusals as usage limits and clamping retries to pool recovery.
Full details: Linked Issues check

Explanation

Issue #7046's terminal contract is implemented. src/codex/auth-context.ts maps CodexMainAccountHardLockError to usage_limit_reached and carries a finite reset or an earlier eligible pool recovery hint. src/codex/routing/pool-recovery.ts preserves eligibility and hold checks without changing selection. The focused tests cover the contract, pool eligibility, pins, and other cooldown classes. The issue also requires end-to-end pool-aware recovery. The current implementation only stores poolRecoveryAt on the refusal and tells the client to retry. nextCodexPoolRecoveryAt explicitly selects no account and changes no state. The reviewed changes do not resume or re-evaluate an active Goal when the eligible pool account recovers. This leaves the broader recovery requirement unmet.

Resolution

Implement the missing Goal or equivalent control-plane integration for #7046. After a terminal main hard-lock refusal, expose the waiting or unavailable state with the computed recovery time and re-evaluate eligible pool accounts when that time is reached. Preserve model and account eligibility, explicit pins, replay safety, credits consent, and the main-account guard. Add an end-to-end test that demonstrates the Goal does not remain active and that a recovered eligible pool account is selected on re-evaluation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 13, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34d611269e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/codex/auth-context.ts
Comment thread src/codex/routing/pool-recovery.ts

@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.

🔇 Additional comments (12)
src/codex/routing/pool-recovery.ts (1)

45-60: Pool recovery estimate can return a time earlier than a still-active hold.

Lines 46-50 and 51-52 collect holds only when they are live (> now). Line 53 collects the usage-limit reset only when isCodexAccountHeldAtUsageLimit is true. Line 59 then takes Math.max(...holds). These sources are consistent with each other, so the computed maximum is correct for the holds that were detected.

One gap remains. Line 53 depends on isCodexAccountHeldAtUsageLimit, which uses the credit policy. If an account has only a transient softAvoidUntil hold (or a probe/transient hold), nothing is added to holds. Line 58 then skips the account. This is a safe, conservative result and matches the documented policy. No defect.

The module header documents the intended semantics, and the tests cover cooldown, avoidance, denial, reauth, and pause. The logic matches the documented contract.

tests/codex-integration/main-account-hard-lock-usage-limit.test.ts (1)

189-199: Test compares an absolute timestamp captured after the call setup, which can flake.

Line 192 captures now after lockMain(). Line 193 sets the cooldown from that now. Line 194 calls resolveCodexAuthContext, which reads its own Date.now() at line 1095 of src/codex/auth-context.ts. The expected value on line 196 is now + 2 * 3_600_000. That value matches the stored cooldownUntil exactly, so the equality holds.

Line 198 is the weaker assertion. cooldownErrorResponse(..., now) uses the captured now. The expected retry-after is 2 * 3_600. This is deterministic because now is the same value used to set the cooldown. No flake risk.

No defect found.

src/codex/routing/health-store.ts (1)

166-166: LGTM!

src/codex/auth-context.ts (3)

944-959: LGTM!


967-974: LGTM!


1091-1093: 🎯 Functional Correctness

The request-scoped main credential remains rejected while main is hard-locked, but poolRecoveryAt is only a retry hint. A later pool retry can select an eligible sibling account. The code and tests require the main reset to be preserved for an exact main pin, not for every request-scoped credential. No defect remains.

src/server/index/websocket-handler.ts (1)

373-379: The await refusal.json() call can throw and fall through to the generic error handling.

Line 376 awaits refusal.json() inside the existing try block. If it throws, the outer catch at line 391 swallows it. No frame is sent, and the client gets no refusal. The body comes from formatUsageLimitReachedResponse, which always produces valid JSON, so this is unlikely in practice.

The log was finalized with status 429 on line 367, so logging stays consistent. No defect.

structure/providers/openai-accounts.md (1)

544-556: LGTM!

scripts/test-layout/layout.json (1)

1175-1175: LGTM!

tests/fixtures/test-layout-expected.json (1)

751-751: LGTM!

docs-site/src/content/docs/reference/codex-retry-diagnostics.md (1)

44-49: 📐 Maintainability & Code Quality

No locale contradiction is established.

The inspected locale trees do not contain a translated codex-retry-diagnostics.md page. Therefore, this English-only documentation change does not leave an existing locale page describing the old behavior. The proposed locale update is not actionable for this file.

src/codex/auth-context.ts-1094-1099 (1)

1094-1099: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Swallowed errors hide failures of the recovery lookup.

The empty catch on line 1099 discards every error from nextCodexPoolRecoveryAt and cachedDeniedCodexAccountIdsForModel. The comment states the hint is optional, so swallowing is acceptable for the response. A systematic bug in the helper would then be invisible, because no log records the failure.

Add a debug-level log in the catch block. Do not log credentials or account identifiers beyond what the codebase already masks.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b07e2789-039b-4510-8eeb-d2c705628988
📥 Commits

Reviewing files that changed from the base of the PR and between aca5ccf and 34d6112.

📒 Files selected for processing (9)
  • docs-site/src/content/docs/reference/codex-retry-diagnostics.md
  • scripts/test-layout/layout.json
  • src/codex/auth-context.ts
  • src/codex/routing/health-store.ts
  • src/codex/routing/pool-recovery.ts
  • src/server/index/websocket-handler.ts
  • structure/providers/openai-accounts.md
  • tests/codex-integration/main-account-hard-lock-usage-limit.test.ts
  • tests/fixtures/test-layout-expected.json

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev under the MAINTAINERS.md maintainer bypass (non-UI change, author has admin access). Jun delegated the pool-recovery policy to this PR; the rules and reasoning are in the description.

Head commit: 312df1520. Every PR check completed successfully on it: 22 pass, 8 skipped by CI policy (macOS/Windows/desktop). The earlier test 1/4 failure on 8bc46abb2 was a real read-fence regression, fixed in 312df1520. Both Codex findings (gated-model eligibility, all routing holds) were fixed in 8bc46abb2 and resolved. The main-account guard, its thresholds, credentials and account eligibility are unchanged; only the refusal's error contract and retry hint change.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant