Repository navigation
fix(command-code): rotate on insufficient-credit refusals - #7558
Conversation
Command Code reports exhausted credits as HTTP 400/BAD_REQUEST, which bypasses the generic OAuth 429 rotation path. Normalize only the exact refusal to 429 while preserving the original body and status in headers.
CodeRabbit noted the regex also accepted near-miss messages. Require the exact upstream prefix and cover the near misses.
Cover the normalized retry response's original 400 status and preserved JSON body, as requested in review.
Review noted that non-fatal UTF-8 decoding could still classify a malformed 400 body as an insufficient-credit refusal. Decode fatally so the existing fail-open path preserves the original response.
- Re-throw the caller's abort reason from refusal inspection. - Register the new test in both layout maps and keep layout.json below the file-size ratchet. - Document the narrow 400-to-429 normalization in the adapter contract. - Cover the abort path with a focused test.
…ation A genuine upstream 429 must keep its own Retry-After and reach rotation unlabelled, and a 402 stays a 402 even when its body repeats the credit wording. Pin both so the narrow 400 normalization cannot widen. Co-authored-by: Yushi Chen <chenyushi63@gmail.com>
With two stored Command Code accounts, the exact credit refusal on the first account now completes the same request on the sibling, while an ordinary BAD_REQUEST stays on the account and surfaces as a 400. The rotation case fails against the adapter without the normalization. Co-authored-by: Yushi Chen <chenyushi63@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
OAuth response normalization and send paths src/adapters/command-code.ts, tests/providers/command-code-credit-failover.test.ts, structure/providers-and-adapters.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json |
The adapter converts only the specified OAuth HTTP 400 response to 429 and records the upstream status and refusal type. It applies the conversion to initial sends and reasoning-effort retries. Tests cover authentication modes, other statuses, malformed or unsafe bodies, and caller aborts. The documentation and test-layout mappings reflect the behavior and new tests. |
Sibling-account rotation validation tests/providers/command-code-credit-rotation.test.ts |
Dispatch-path tests verify that a credit refusal retries with a sibling OAuth account and that an ordinary BAD_REQUEST does not rotate accounts. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
- lidge-jun/opencodex#7587 — The PR normalizes the specified OAuth credit refusal so account rotation can select another account. The issue also covers desktop admission, usage authority, and account safeguards.
- lidge-jun/opencodex#7684 — The PR adds refusal-aware handling for insufficient credits. It does not handle the issue’s
MODEL_NOT_IN_PLAN403 response.
Suggested reviewers: lcxhh521
Merge Risk: 
65b29
-
No actionable merge-blocking risk remains in the reviewed changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "fix(command-code): rotate on insufficient-credit refusals" matches the main change. The diff adds OAuth-only normalization in src/adapters/command-code.ts that rewrites the exact insufficie… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "fix(command-code): rotate on insufficient-credit refusals" matches the main change. The diff adds OAuth-only normalization in src/adapters/command-code.ts that rewrites the exact insufficient-credit HTTP 400 to 429, so OAuth rotation can take over. The title names the adapter scope and the behavior. It omits the 400-to-429 rewrite and the OAuth-only gate, which are details the reader needs, but it does not mislead. Changes to layout.json, the fixture, the docs, and the two new test files support this same change.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 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.
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aaaadad07
ℹ️ 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".
A canonical key-auth Command Code row gets a same-key 429 retry ladder from rateLimitRetryPolicyFor, so rewriting its permanent credit 400 into a 429 would replay a credential with no credits left before surfacing the error. Only OAuth rows have sibling accounts to rotate to, so key and forward rows now keep the original 400. Co-authored-by: Yushi Chen <chenyushi63@gmail.com>
|
✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Reviewed the complete carry at 3fe73f6b45756dd113936fac5888efe916348afc, including the original five author-preserved commits and the matching Co-authored-by trailer. The 400-to-429 classification is narrowly gated to OAuth and the exact bounded/fatal-UTF-8 credit-refusal shape. Key/forward rows, real 429 Retry-After and 402 retain their original semantics; the effort-repair response is covered as well. Rotation reuses the existing send-budget/account owner rather than adding an unbounded replay loop.
Independent capped Linux/Bun 1.4.3: bun test tests/providers/command-code-credit-failover.test.ts tests/providers/command-code-credit-rotation.test.ts — 9 pass / 0 fail / 89 assertions, including actual dispatch with two synthetic stored accounts and a fully stubbed upstream. The ordinary BAD_REQUEST sends once and stays on its account. Current-head CI 39848926882 and gates/Typecheck succeeded; the OAuth-only finding is resolved and no standing objection remains in the thread snapshot.
The full local suite/typecheck were not repeated under the shared host limits; hosted Ubuntu CI supplies broader coverage. This approval is head/diff-bound, not a later union, live billing statement or native-platform certification. No real account, credential, credit spend, provider call or service change was used.
Local CI-equivalent (Linux) at 3119e9dOverall: PASS (temporary rule; three raw shard commands failed BASE-IDENTICALLY) under the temporary local-verification rule. This is local Linux evidence, not hosted CI.
Failure classification (failed commands remain failed):
Base comparisons (same Bun/environment; once per distinct failing file, reused where recorded):
NOT RUN: Shard 1/4 batches 11–64, shard 2/4 batches 5–59, and shard 4/4 batches 12–65: original CI script stops after the first failed batch; no retries or manual skips were added; Hosted CI and nonrequested jobs/platforms; docs-site build is outside this non-docs PR scope All requested command invocations completed. Every non-shard gate, GUI test/build, structure check, storage-policy and api-usage command exited 0. No INTRODUCED failures were observed in executed coverage. Overall disposition follows the manager’s explicit base-identical exception; this does not claim the unexecuted shard remainders passed. Fresh subsequent PR checkouts will use a private scratch path under Head re-fetched before posting: Recorded UTC: 2026-10-24T09:54:11.493958+00:00 |
Local CI-equivalent (macOS) at 3119e9dOverall: PROVISIONAL — full-suite coverage pending under the updated verification rule. Host: Git setup: fetch pull head, resolve FETCH_HEAD, confirm detached HEAD, and compute merge-base completed with exit 0; no test counts apply. Head was re-fetched after verification to check for movement. Workflow source: Environment: Suite environment:
Failures and base comparisons: Base scratch setup:
NOT RUN:
Raw command logs and structured results retained on this host in |
Local CI-equivalent (Linux) coverage addendum at 3119e9dFinal verdict: PASS (temporary rule; raw base-identical test failures retained). This supersedes the earlier verdict’s incomplete coverage: #7558 (comment) Host All new suite/base invocations use documented Batch file lists are generated by the exact head’s original planner. Only its script-directory anchors point back to the original location; its test command/deadlines are unchanged. Every previously executed batch file list was checked against the reconstructed manifest. Remaining batches ran serially; any new failure stayed failed and did not suppress later missing batches.
Remaining primary batches — each command is
Remainder totals: 33523 pass / 0 fail / 90 skip. Combined original + remainder command counts: 55641 pass / 2 fail / 167 skip. Failed checks remain failed; final disposition uses only the explicit BASE-IDENTICAL exception. Failure classification across original and remainder evidence:
Base comparisons (cached once per distinct file/exact failed batch):
Every original non-test gate result is retained in #7558 (comment). No source edits, pushes, approvals or merges were performed. Source status: clean ( NOT RUN: Hosted CI and platforms/jobs outside the requested Linux scope Exact manifests and raw logs (retained on the named Linux host):
UTC: 2026-10-24T11:06:25.206140+00:00 |
Local CI-equivalent (macOS) at 3119e9d — full coverage addendumFinal verdict: PASS (owner’s BASE-IDENTICAL test-failure exception). This supersedes the coverage limitation in the initial result. Host All suite runs after the manager’s coverage instruction use The remainder plans are computed by each exact head’s own batch-planning code (all original code before its final execution loop). The only planner path substitution points Each remaining batch ran sequentially as Coverage: 508/508 batches have all planned file headers confirmed. Original gates/build/lint/typechecks/GUI/storage/API results are retained from the initial comment.
Failure classification:
Per-batch remainder results (counts reflect that batch only):
NOT RUN: No test batches remain unexecuted. Original job-tail CLI-help steps after a failing suite were not repeated; the identical gates CLI-help command already exited 0. Original raw logs: |
|
Integration evidence for exact head 65b29fa (only a dev merge on top of the verified 3119e9d):
Static merge simulation on current dev e8ff1b2 (this Mac): dev=e8ff1b2061465782a61aafbba4ffa15f66b97a21 base=e8ff1b2061465782a61aafbba4ffa15f66b97a21 pr_files= 6 overlap=[]; merge clean; install exit=0; gui install exit=0; tsc exit=0; tsc doctor exit=0; structure:check exit=0; privacy:scan exit=0; Merged by the triage manager as maintainer integration (MAINTAINERS.md, dev only) under the owner's static-judgment rule while GitHub Actions is degraded; dev push CI will be checked when Actions recovers. |
Summary
Carries #7471 by @scy7796 onto the current
devtip. Related to #7426 (the Command Code half of that report only).Command Code's
/alpha/generatereports exhausted monthly credits as HTTP 400 withsuccess: false,error.code: "BAD_REQUEST"and a message beginningYou have insufficient credits to make this request.Generic OAuth account rotation only reacts to 429, so before this change that refusal failed the turn even when a sibling Command Code account still had credits.On OAuth rows the adapter now rewrites only that exact refusal to 429, adds
x-opencodex-refusal: insufficient-credits, and keeps the original body plusx-opencodex-upstream-status: 400. The existingrotateGenericOAuthAccountOn429path, cooldowns and send budget then move the same request to the next account; there is no new retry loop. The same rewrite applies to the retry after a reasoning-effort rejection.Before: account A out of credits → client sees a 400 and the turn ends.
After: account A out of credits → the same request completes on account B, and B stays active.
What this carry adds on top of the original five commits (kept with their authorship):
tests/providers/command-code-credit-failover.test.ts: a genuine upstream 429 (with or without the credit wording) and a 402 (with the credit body or a plain payment body) pass through unchanged, keep their ownRetry-After, and never getx-opencodex-refusalorx-opencodex-upstream-status. The ordinary-400 case now also asserts no upstream-status header.tests/providers/command-code-credit-rotation.test.ts(new): with two stored Command Code accounts,handleResponsesturns the credit refusal on account A into a completed response from account B; an ordinaryBAD_REQUEST400 stays on A, surfaces as 400, and sends once.structure/providers-and-adapters.md: the Command Code row names the 429/402 boundary and both tests.Review threads on #7471
request!,requestand other casingsfatalUtf8: true, tested)tests/fixtures/test-layout-expected.json, adapter contract undocumentedReview threads on #7558
authMode === "oauth"; key and forward rows keep the 400 (tested, red without the gate)createCommandCodeAdapterfactory, and the new normalizer already carries a doc comment explaining the OAuth-only ruleNo other finding was raised; CodeRabbit reported no actionable comments at 3fe73f6.
Decisions
rateLimitRetryPolicyFor(src/providers/key-failover.ts), so a rewritten 429 would replay a credential with no credits left before the error surfaced. Forward rows carry the client's own credential and have nothing to rotate to.src/server/responses/adapter-dispatch.ts429 arm); the 400/402 loop there serves API-key pools and returns early for OAuth (hasKeyPoolFailover). Rewriting to 402 would not rotate. Thex-opencodex-refusalheader keeps the credit refusal distinguishable from a real rate limit.src/reads the two new headers yet, so the only place a widening could happen is the adapter's status check and matcher. The new adapter cases fail if the normalizer accepts 429 or 402 (checked by mutating the status guard to[400, 402, 429]: two tests went red).scripts/test-layout/layout.jsonsits at its 1999-line cap, so the new file is appended to the line that already holdscommand-code-credit-failover.test.ts, as the original PR did.Retry-After, so the exhausted account cools for the default 60 s (or until its quota reset when quota data already shows it exhausted). Round-robin can return to it after that and spend one send to rediscover the refusal. A credit-aware cooldown keyed onx-opencodex-refusalwould be a separate change to the shared failover module.Security summary
The change sits next to OAuth account rotation but does not touch credential storage, token refresh or logging. It reads at most 8 KiB of an already-received 400 body from a clone, fails open on parse, size or UTF-8 errors, and adds two fixed header values that carry no account or request data. The body is passed through unchanged, as before.
Verification
After 3fe73f6 (OAuth gate): credit tests 9 pass / 0 fail; removing the gate turns the new key/forward test red (
Expected 400, Received 429);bun run typecheck,structure:check,privacy:scanandgit diff --checkpass.bun run test:changedreported 63 failures outside Command Code during a busy window; the 10 affected files then passed 437 / 0 serially on this head, matching 437 / 0 on the previous head 7aaaada, andclaude-messages-endpoint.test.tspassed 54 / 0 with and without the change.Earlier runs at 7aaaada: run on macOS with Bun 1.4.3, isolated
HOME/CLAUDE_CONFIG_DIR/OPENCODEX_HOMEfrommktemp -d, branch ondev516aaaaf84.bun test tests/providers/command-code-credit-failover.test.ts tests/providers/command-code-credit-rotation.test.ts tests/providers/command-code-provider.test.ts tests/providers/command-code-efforts.test.ts tests/providers/command-code-error-finish.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 142 pass, 0 failRed→green: the rotation test fails (
Expected 200, Received 400) againstdev'ssrc/adapters/command-code.tsand passes with this branch; the 429/402 boundary test fails when the status guard is widened to 429/402bun run test:changed(under the shared lockf): 17504 pass, 34 skip, 0 fail across 727 filesbun run typecheck: passbun run structure:check: passbun run privacy:scan: passgit diff --check origin/dev...HEAD: cleanbun run test(full suite, under the shared lockf while other worktrees were testing): the parallel pass reported 34 failures, all outside Command Code; most were 5-40 s timeouts and the runner flagged a busy CPU. Rerunning the 15 affected files with--parallel=1: 364 pass, 3 skip, 2 fail. The two remaining failures (tests/claude-integration/claude-desktop-remote-hub.test.ts"connected Desktop uses hub IDs across a cold restart",tests/codex-integration/codex-runtime.test.ts"treats missing persisted and resolved versions as the same selection") fail identically on a cleandev516aaaaf84checkout on this host: the runtime test finds the host's installed Codex, so they are host-environment failures unrelated to this change.The local pre-push hook (
bun run prepush) was skipped with--no-verify: it runs in the unisolated shell with Bun 1.3.14 and exited in itsteststep within seconds. Every step it runs was executed above in the isolated Bun 1.4.3 environment; there are nogui/changes for its GUI steps. No live Command Code account was used for this carry; the original PR describes a live two-account check on 2.88.0.Windows-sensitive: no. No changed path matches the
windowsfilter in.github/workflows/ci.yml, and no added line undersrc/ortests/contains a marker fromscripts/ci/windows-sensitive-diff.sh.Checklist
structure/; no user-facing config or docs change.)Co-authored-by: Yushi Chen chenyushi63@gmail.com
Summary by CodeRabbit