fix(codex): rotate pool accounts on model capacity - #1313
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCodex passthrough requests detect capacity rejections before substantive output and retry across eligible Pool accounts once per request. Multi-account exclusions flow through authentication and routing. Direct mode, exact account selection, and post-output failures do not rotate. ChangesCapacity failover
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Passthrough
participant CapacityInspector
participant AuthContext
participant PoolAccount
Client->>Passthrough: Send Codex request
Passthrough->>CapacityInspector: Inspect response before output
CapacityInspector-->>Passthrough: Capacity rejection
Passthrough->>AuthContext: Exclude tried account
AuthContext->>PoolAccount: Select next eligible account
PoolAccount-->>Passthrough: Retry response
Passthrough-->>Client: Accepted response or first capacity rejection
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 review\n\nPlease focus on the request-local account exclusion semantics, first-error preservation, and the SSE pre-output commit boundary. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata ==='
gh pr view 1313 --repo lidge-jun/opencodex --json number,title,baseRefName,headRefName,files --jq '{number,title,baseRefName,headRefName,files:[.files[].path]}'
echo '=== Changed-file diff ==='
gh pr diff 1313 --repo lidge-jun/opencodex --patch
echo '=== Candidate files ==='
fd -t f -i 'codex-capacity|codex.*pool|account.*select|account.*health|server-auth|capacity.*retry' . || true
echo '=== Structural maps ==='
while IFS= read -r f; do
echo "--- $f"
ast-grep outline "$f" --items all --type function,method,class,interface,type --view expanded || true
done < <(fd -t f -e ts -e tsx . | xargs -r rg -l -i 'server_is_overloaded|slow_down|capacity|excludedAccounts|exclude.*account|thread.affinity|account.health' || true)Length of output: 54408 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Relevant source maps ==='
for f in \
src/server/responses/core.ts \
src/server/responses/codex-capacity.ts \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/lib/upstream-retry.ts \
tests/server-auth.test.ts \
tests/codex-capacity-retry.test.ts
do
echo "--- $f"
ast-grep outline "$f" --items all --view expanded || true
done
echo '=== Core retry and callback context ==='
rg -n -C 8 \
'retryCodexPoolOnAlternateAccount|onCodexAuthContextResolved|capacityRetryExhausted|recordCodexUpstreamOutcome|codexForwardTerminalOutcomeRecorder|applyAccountQuotaFromUpstreamHeaders' \
src/server/responses/core.ts
echo '=== Auth-context selection and side effects ==='
rg -n -C 10 \
'resolveCodexAuthContext|excludeAccountIds|bindThreadAffinity|beginCodexAccountSelection|recordCodex|activeCodexAccountId|pickAlternateCodexAccountExcluding' \
src/codex/auth-context.ts src/codex/routing.ts
echo '=== Capacity inspector full numbered source ==='
nl -ba src/server/responses/codex-capacity.ts
echo '=== Callback definition and all call sites ==='
rg -n -C 5 'onCodexAuthContextResolved\s*[:=]|\bonCodexAuthContextResolved\?\.' src tests
echo '=== Account durable-state writers ==='
rg -n -C 4 \
'bindThreadAffinity|recordCodexUpstreamOutcome|applyAccountQuotaFromUpstreamHeaders|set.*Active|activeCodexAccountId|update.*Health|setCodex.*Health' \
src/codex src/server/responses/core.tsLength of output: 80653 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_range() {
local file="$1" start="$2" end="$3"
echo "=== $file, Lines $start-$end ==="
sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%6d %s\n", n++, $0}'
}
show_range src/codex/auth-context.ts 251 442
show_range src/codex/routing.ts 879 905
show_range src/codex/routing.ts 1150 1205
show_range src/codex/routing.ts 1495 1615
show_range src/server/responses/core.ts 397 535
show_range src/server/responses/core.ts 2045 2300
show_range src/server/responses/codex-capacity.ts 1 267
show_range src/lib/upstream-retry.ts 350 410
echo '=== Tests that assert affinity, active selection, or exact response preservation ==='
rg -n -C 6 \
'getThread|thread.*affinity|clearThread|activeCodexAccountId|x-capacity-attempt|capacity exhaustion|preserves the first|capacity rejection' \
tests src/codex
echo '=== All references to the account-resolution callback outside core ==='
rg -n -C 5 'onCodexAuthContextResolved' src -g '!src/server/responses/core.ts'
echo '=== Capacity retry and account-selection test coverage ==='
rg -n -C 5 \
'pickAlternateCodexAccountExcluding|excludeAccountIds|commitRoundRobin|capacity' \
tests/codex-pool-rotation.test.ts tests/codex-convergence-account-selectors.test.ts tests/server-auth.test.tsLength of output: 184558
Request-local capacity rejection still writes thread affinity
If the initial account returns capacity, The alternate selection uses
This violates the documented contract that capacity-rejected accounts do not receive thread-affinity writes. It also makes an affined thread repeat a known capacity attempt on each new request. Use a side-effect-free initial selection for requests that can use capacity rotation. Commit affinity only after a non-capacity response is accepted for relay. Alternatively, capture the previous affinity and restore it when capacity classification rejects an attempt, then bind the final successful account. Add a regression test in I verified the other requested boundaries in the inspected code:
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/configuration/providers.md`:
- Around line 153-160: Fix the Pool selection lifecycle so a capacity-rejected
account does not persist thread affinity before the upstream response is
classified; update the runtime selection/affinity code responsible for initial
Pool selection while preserving request-local exclusions and existing
direct/exact-selector behavior. In
docs-site/src/content/docs/reference/configuration/providers.md lines 153-160,
document the corrected no-affinity lifecycle. In
docs-site/src/content/docs/reference/cli/providers-accounts.md lines 167-170 and
structure/08_openai-provider-tiers.md lines 26-32, clarify that the next request
starts without affinity to the capacity-rejected account and keep the provider
contract consistent.
In `@tests/server-auth.test.ts`:
- Line 151: Replace the local CAPACITY_MESSAGE literal in the test with an
import of CODEX_MODEL_CAPACITY_MESSAGE from the production codex-capacity
module, adding it alongside the existing src imports. Update the fixture
references to use the imported constant and remove the duplicate declaration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 04dcd318-141e-48f1-ad5f-1048ce4a00c6
📒 Files selected for processing (11)
docs-site/src/content/docs/reference/cli/providers-accounts.mddocs-site/src/content/docs/reference/configuration/providers.mdsrc/codex/auth-context.tssrc/codex/routing.tssrc/lib/upstream-retry.tssrc/server/responses/codex-capacity.tssrc/server/responses/core.tsstructure/08_openai-provider-tiers.mdtests/codex-capacity-retry.test.tstests/server-auth.test.tstests/upstream-transient-retry.test.ts
|
Addressed in
Verification: typecheck passed; 20 focused capacity/retry tests passed; 104 pool-routing/session-affinity tests passed; privacy scan passed. @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Commit metadata and changed files ==='
git show --no-ext-diff --format=fuller --stat 0495da70a
git diff --no-ext-diff 0495da70a^ 0495da70a -- \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/server/responses/core.ts \
src/server/responses/codex-capacity.ts \
tests/server-auth.test.ts
echo '=== Structural maps of changed candidate files ==='
for f in src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts tests/server-auth.test.ts; do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all --view expanded || true
fi
done
echo '=== Affinity selection, deferred binding, and capacity retry paths ==='
rg -n -C 10 \
'bindThreadAffinity|resolveCodexAccountForThreadDetailed|onCodexAuthContextResolved|excludeAccountIds|retryCodexPoolOnAlternateAccount|capacityRetryExhausted|excludedAccountIds|recordCodexUpstreamOutcome' \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts
echo '=== Capacity response inspection and first-response retention ==='
rg -n -C 10 \
'inspectCodexCapacityBeforeOutput|firstCapacity|first.*capacity|kind === "capacity"|capacityRetryExhausted|unknown|output' \
src/server/responses/core.ts src/server/responses/codex-capacity.ts
echo '=== Same-thread regression and nearby capacity tests ==='
rg -n -C 12 \
'parent-thread|parentThread|pool-a.*pool-b|capacity.*affinity|affinity.*capacity|second request|same-thread' \
tests/server-auth.test.tsLength of output: 190623 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_range() {
local file="$1" start="$2" end="$3"
echo "=== $file: Lines $start-$end ==="
nl -ba "$file" | sed -n "${start},${end}p"
}
show_range src/codex/routing.ts 1060 1112
show_range src/codex/routing.ts 1210 1275
show_range src/codex/routing.ts 1510 1640
show_range src/server/responses/core.ts 840 870
show_range src/server/responses/core.ts 2088 2180
show_range tests/server-auth.test.ts 230 370
show_range tests/server-auth.test.ts 2370 2405
echo '=== Active-account writer definitions and call sites ==='
rg -n -C 5 \
'\brememberActiveCodexAccount\s*\(|\bsetActiveCodexAccount\s*\(|\bpromoteActiveCodexAccount\s*\(|activeCodexAccountId\s*=' \
src/codex src/server/responses/core.ts tests/server-auth.test.ts
echo '=== Deferred-affinity option call sites ==='
rg -n -C 5 \
'deferThreadAffinityCommit|commitThreadAffinity|bindCodexThreadAffinityForAcceptedResponse' \
src testsLength of output: 1326 🏁 Script executed: #!/bin/bash
set -euo pipefail
show_range() {
local file="$1" start="$2" end="$3"
echo "=== $file: Lines $start-$end ==="
sed -n "${start},${end}p" "$file" | awk -v line="$start" '{ printf "%6d %s\n", line++, $0 }'
}
show_range src/codex/routing.ts 1060 1112
show_range src/codex/routing.ts 1210 1275
show_range src/codex/routing.ts 1510 1640
show_range src/server/responses/core.ts 840 870
show_range src/server/responses/core.ts 2088 2180
show_range tests/server-auth.test.ts 230 370
show_range tests/server-auth.test.ts 2370 2405
echo '=== Active-account writer definitions and call sites ==='
rg -n -C 5 \
'\brememberActiveCodexAccount\s*\(|\bsetActiveCodexAccount\s*\(|\bpromoteActiveCodexAccount\s*\(|activeCodexAccountId\s*=' \
src/codex src/server/responses/core.ts tests/server-auth.test.ts
echo '=== Deferred-affinity option call sites ==='
rg -n -C 5 \
'deferThreadAffinityCommit|commitThreadAffinity|bindCodexThreadAffinityForAcceptedResponse' \
src testsLength of output: 53090
I found one remaining issue. Capacity-rejected initial selections still update active-account state
Therefore, with
This does not affect the new thread-affinity test because the harness uses the default strategy and starts with Defer all selection-state commits for capacity-rotatable requests, not only ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/auth-context.ts`:
- Around line 237-238: Replace the affinity-only deferThreadAffinityCommit
option in src/codex/auth-context.ts:237-238 with a deferred selection contract
covering active-account, strategy cursor, rotation, affinity, and affinity
re-evaluation writes. Update the routing selection flow in
src/codex/routing.ts:1567-1588 to preview without committing state, then update
the capacity acceptance path in src/server/responses/core.ts:2109-2113 to commit
that selection exactly once after inspectCodexCapacityBeforeOutput returns a
non-capacity result. Add focused three-account round-robin regression coverage
in tests/server-auth.test.ts:2379-2400 verifying the rejected first account does
not advance or remain active and later requests derive state from the accepted
account.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9c175f58-a887-4e35-a52a-821d8c25b9d6
📒 Files selected for processing (4)
src/codex/auth-context.tssrc/codex/routing.tssrc/server/responses/core.tstests/server-auth.test.ts
|
@coderabbitai review Please review the incremental fixes in |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Commit metadata ==='
git show --no-ext-diff --format=fuller --stat 0495da70a
git show --no-ext-diff --format=fuller --stat 7bfbfb1f3
echo '=== Incremental diffs ==='
git diff --no-ext-diff 0495da70a^ 0495da70a -- \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts \
src/server/responses/codex-capacity.ts tests/server-auth.test.ts \
tests/codex-pool-rotation.test.ts
git diff --no-ext-diff 7bfbfb1f3^ 7bfbfb1f3 -- \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts \
src/server/responses/codex-capacity.ts tests/server-auth.test.ts \
tests/codex-pool-rotation.test.ts
echo '=== Relevant structural maps ==='
for f in \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/server/responses/core.ts \
src/server/responses/codex-capacity.ts \
tests/server-auth.test.ts \
tests/codex-pool-rotation.test.ts
do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all --view expanded || true
fi
done
echo '=== State commits, capacity retry, and exclusions ==='
rg -n -C 8 \
'deferThreadAffinityCommit|defer.*[Ss]election|commitRoundRobin|rememberActiveCodexAccount|notePoolRotationSuccess|activeCodexAccountId|bindCodexThreadAffinityForAcceptedResponse|excludeAccountIds|excludedAccountIds|retryCodexPoolOnAlternateAccount|firstCapacity|capacityRetryExhausted' \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts
echo '=== Capacity SSE inspection boundary ==='
rg -n -C 8 \
'inspectCodexCapacityBeforeOutput|kind: "capacity"|kind === "capacity"|unknown|output|lifecycle|ReadableStream|CODEX_MODEL_CAPACITY_MESSAGE' \
src/server/responses/codex-capacity.ts src/server/responses/core.ts
echo '=== Regression coverage ==='
rg -n -C 10 \
'round-robin|fill-first|capacity.*(active|cursor|rotation)|active.*capacity|cursor.*capacity|same-thread|parent-thread|first capacity|exhaustion|SSE' \
tests/server-auth.test.ts tests/codex-pool-rotation.test.tsLength of output: 207958
I found one remaining issue. Capacity-rejected strategy selections still commit active routing state
For an initial Pool selection,
For The new Therefore, a capacity-rejected initial
Commit Use a selection-commit control that covers all initial strategy mutations, not only thread affinity. For capacity-rotatable Pool requests, make the initial selection and alternate selection previews. After a response passes capacity inspection, commit the accepted account’s active-account state and round-robin state once. Do not commit state when every attempted account returns capacity. Add regression coverage for
Also add the equivalent I rechecked the requested boundaries:
✅ Action performedReview finished.
|
|
@coderabbitai review Please review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
commit="8b0d860f2"
echo "=== Commit metadata and changed files ==="
git show --no-ext-diff --format=fuller --stat "$commit"
git diff --no-ext-diff "${commit}^" "$commit" -- \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/server/responses/core.ts \
src/server/responses/codex-capacity.ts \
tests/server-auth.test.ts \
tests/codex-pool-rotation.test.ts
echo "=== Structural maps ==="
for f in \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/server/responses/core.ts \
src/server/responses/codex-capacity.ts \
tests/server-auth.test.ts \
tests/codex-pool-rotation.test.ts
do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all --view expanded || true
fi
done
echo "=== Selection commit controls and capacity retry paths ==="
rg -n -C 10 \
'commitRoundRobin|commitThreadAffinity|defer.*[Ss]election|rememberActiveCodexAccount|notePoolRotationSuccess|bindCodexThreadAffinityForAcceptedResponse|resolveCodexAccountForThreadDetailed|pickAlternateCodexAccountExcluding|excludeAccountIds|excludedAccountIds|inspectCodexCapacityBeforeOutput|firstCapacity|capacityRetryExhausted' \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts
echo "=== Focused test coverage ==="
rg -n -C 12 \
'round-robin|fill-first|capacity.*(active|cursor|rotation)|active.*capacity|cursor.*capacity|three-account|pool-a|pool-b|pool-c|first capacity|exhaustion|SSE' \
tests/server-auth.test.ts tests/codex-pool-rotation.test.tsLength of output: 182892 🏁 Script executed: #!/bin/bash
set -euo pipefail
commit="8b0d860f2"
echo "=== Exact incremental diff ==="
git diff --no-ext-diff --unified=80 "${commit}^" "$commit" -- \
src/codex/auth-context.ts \
src/codex/routing.ts \
src/server/responses/core.ts \
tests/server-auth.test.ts
echo "=== Commit helper and all call sites ==="
rg -n -C 12 \
'\bcommitCodexAcceptedAccountSelection\s*\(|\bdeferSelectionCommit\b|\bcommitCodexAccount.*Selection\b|\bcommit.*RoundRobin\b|\bnotePoolRotationSuccess\b|\brememberActiveCodexAccount\b' \
src/codex/auth-context.ts src/codex/routing.ts src/server/responses/core.ts tests
echo "=== Narrow source ranges ==="
for spec in \
"src/codex/auth-context.ts:275:345" \
"src/codex/routing.ts:1040:1135" \
"src/codex/routing.ts:1180:1305" \
"src/codex/routing.ts:1490:1660" \
"src/server/responses/core.ts:820:880" \
"src/server/responses/core.ts:2080:2190" \
"tests/server-auth.test.ts:2398:2445"
do
file="${spec%%:*}"
rest="${spec#*:}"
start="${rest%%:*}"
end="${rest#*:}"
echo "=== $file, Lines $start-$end ==="
nl -ba "$file" | sed -n "${start},${end}p"
doneLength of output: 123076 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Round-robin commit primitive ==="
rg -n -C 18 \
'\bcommitRoundRobinAccountSuccess\s*\(|\bpeekRoundRobinAccount\s*\(|\bpickRoundRobinAccount\s*\(|\bnotePoolRotationSuccess\s*\(' \
src/codex/pool-rotation.ts tests/codex-pool-rotation.test.ts
echo "=== Alternate retry resolution ==="
rg -n -C 16 \
'\bretryCodexPoolOnAlternateAccount\s*\(|\bresolveCodexAuthContext\s*\(|excludeAccountIds:|deferSelectionCommit:' \
src/server/responses/core.ts
echo "=== Exact accepted-selection implementation ==="
awk 'NR >= 1500 && NR <= 1585 { printf "%6d %s\n", NR, $0 }' src/codex/routing.ts
echo "=== Focused regression source ==="
awk 'NR >= 2400 && NR <= 2445 { printf "%6d %s\n", NR, $0 }' tests/server-auth.test.tsLength of output: 24698
The deferred selection-state finding is addressed.
I also rechecked the requested boundaries. The request-local exclusion set remains confined to the capacity-retry loop. Capacity exhaustion still restores the first response. The SSE inspection boundary remains before substantive output. You are interacting with an AI system. |
|
@lidge-jun When convenient, could you provide the explicit security review required for this auth/account-selection change? The review focus is the request-local exclusion boundary, exact-account isolation, and ensuring capacity-rejected attempts do not write durable health or selection state. The PR remains draft pending that review. |
|
Hello, thanks for this PR. I am on that, implementing this, but even more on evidence about pricing, availability, effectiveness and speed. But since I am already working on it this will interfere with it, which why I will close this today. |
2 participants
Summary
server_is_overloaded,slow_down, or the exact standard capacity message.Verification
bun run typecheckbun test tests/codex-capacity-retry.test.ts tests/upstream-transient-retry.test.ts(12 passed)bun test tests/server-auth.test.ts -t "capacity|stalled 400"(7 passed)bun run privacy:scancd docs-site && bun install --frozen-lockfile && bun run build(221 pages built)bun run prepushreached 10,022 passed / 7 skipped / 22 failed. The failures are one local-serverECONNRESETcluster in management/data-plane auth and attribution tests; the same failure class reproduces from a clean upstream worktree at the pre-change SHA under Bun 1.3.14.Review focus
codex-capacity.ts: only lifecycle-only SSE frames may be replayed; an unknown or output-bearing event permanently commits the attempt.Draft readiness
devbefore publication.Checklist
Summary by CodeRabbit
New Features
Documentation