feat: Command Code OAuth account pool with Codex-style rotation - #1552
feat: Command Code OAuth account pool with Codex-style rotation#1552dbc-hbin wants to merge 7 commits into
Conversation
github-actions
Bot
added
enhancement
|
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Review readiness checklist
✅ 4/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
📝 WalkthroughWalkthroughAdds opt-in Command Code OAuth account pools with quota routing, priorities, sticky sessions, cooldowns, bounded 429 failover, management APIs, CLI support, and manual GUI authentication. ChangesCommand Code OAuth account pools
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟡 Moderate · up to The PR adds account-pool rotation and failover, but the current head still contains a syntax error in a changed test file, lacks focused regression coverage for repeated 429 failover, and combines the feature with an unrelated security-sensitive change. These issues should be fixed or explicitly accepted before merge. Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesCore
participant commandCodeRouting
participant oauthPoolRouting
participant OAuthAccount
Client->>ResponsesCore: send Responses request
ResponsesCore->>commandCodeRouting: resolve session account
commandCodeRouting->>oauthPoolRouting: select eligible account
oauthPoolRouting->>OAuthAccount: check quota, credentials, and cooldown
OAuthAccount-->>oauthPoolRouting: return account state
oauthPoolRouting-->>ResponsesCore: return account and token
ResponsesCore-->>Client: stream response
ResponsesCore->>commandCodeRouting: rotate account after 429
commandCodeRouting->>oauthPoolRouting: select failover account
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
5ff3b64 to
b49500b
Compare
dbc-hbin
changed the title
github-actions
Bot
changed the title
github-actions
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/account-api.ts (1)
241-255: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve and test OAuth account priorities when reading account rows.
cmdPriorityreadsrow.priority, butfetchOAuthRows()drops this field. The command therefore reports priority0after a successful Command Code or Anthropic priority update. Expose the configured priority from/api/oauth/accounts, add it toOAuthAccountDto, and map it intoFamilyRows.
src/cli/account-api.ts#L241-L255: addprioritytoOAuthAccountDtoand the projected row after the management API returns it.tests/cli-account.test.ts#L843-L851: run the priority command without a value after the write and assert that it reports priority2; make the mock persist and return the priority.🤖 Prompt for 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. In `@src/cli/account-api.ts` around lines 241 - 255, The OAuth account row projection drops the configured priority, causing cmdPriority to report zero after updates. In src/cli/account-api.ts lines 241-255, add priority to OAuthAccountDto and map the API-provided value into each projected row. In tests/cli-account.test.ts lines 843-851, persist and return the mock priority, then invoke the priority command without a value after writing it and assert that it reports 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 `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx`:
- Around line 351-376: Update the manual code input guarded by
authHandlers.onSubmitManualCode in ProviderAuthPanel to use type="password"
instead of type="text", while preserving its existing value, submission, and
conditional hint behavior.
- Around line 202-219: Update the catch block in submitManualCode to capture the
thrown error and pass its message to t("prov.pasteFail", { error }), using error
instanceof Error ? error.message : String(error) instead of the hardcoded
"network error".
In `@README.md`:
- Around line 72-77: Update the README account-pool configuration description to
document the explicit runtime keys, including config.anthropicAccountPool and
config.commandCodeAccountPool, so users can enable both providers; replace the
invalid generic config.<provider>AccountPool.enabled reference while
preserving the surrounding feature details.
In `@src/lib/state-store-registrations.ts`:
- Line 85: Export a provider-agnostic sweeper alongside
sweepExpiredOAuthPoolRoutingHealth that iterates every provider in
stateByProvider, invokes the existing per-provider sweeper with the same
timestamp, and returns the total removed count. Update the
oauth-pool-routing-health registration to import and use
sweepExpiredAllOAuthPoolRoutingHealth instead of binding the "command-code"
provider, while preserving the registration name.
In `@src/oauth/command-code-routing.ts`:
- Around line 51-59: Update priorityOf’s accountId priority resolution to
validate priorities[accountId] is a finite number before applying
Math.max/Math.min clamping; return the default tier 0 for strings, NaN,
infinities, or other malformed values while preserving the existing -100 to 100
bounds for valid numbers.
In `@src/oauth/oauth-pool-routing.ts`:
- Around line 56-64: Extend OAuthAccountPoolConfig with the accountPriorities
and activeAccountPinned fields using the same types and optionality declared on
commandCodeAccountPool in src/types.ts. Keep commandCodeAccountPoolConfig() and
all existing pool-provider consumers typed through this shared interface so the
priority map and pin are exposed without requiring raw configuration access.
In `@src/providers/quota.ts`:
- Around line 1308-1316: The monthly quota calculation in
fetchCommandCodeUsageQuota incorrectly divides credits.monthlyCredits by itself,
always producing 100. In src/providers/quota.ts lines 1308-1316, use distinct
used and cap fields when available, otherwise omit the monthly row when no
denominator exists; preserve the empty-payload guard when only credits are
present. In tests/provider-account-quota.test.ts lines 436-446, assert the
expected quota.monthlyPercent for the creditsBody fixture.
- Around line 1393-1395: Update the dispatch in fetchProviderAccountQuotas to
use an explicit provider switch instead of treating every non-anthropic provider
as Command Code. Call fetchAnthropicUsageQuota for "anthropic",
fetchCommandCodeUsageQuota for "command-code", and return null for unrecognized
providers so the existing !quota unavailable-account handling applies.
In `@src/server/management/oauth-account-routes.ts`:
- Around line 577-581: Update the account deletion path around the provider ===
"command-code" branch to remove the deleted id from
config.commandCodeAccountPool.activeAccountPinned, persist the updated
configuration, and reconcile Command Code routing state alongside
clearCommandCodeAccountCooldown and clearCommandCodeSessionAffinityForAccount.
Ensure this cleanup occurs whenever a Command Code account is deleted so
automatic and manual routing cannot retain the removed account.
In `@src/server/responses/core.ts`:
- Around line 3745-3778: Add a focused Responses regression test covering the
reachable Command Code flow around the registry route: return three consecutive
429 responses, assert the first two retries use distinct account bearer tokens,
verify invalidateSameTargetRequest() is invoked, and confirm the third 429 is
surfaced without a fourth rotation. In the continuation failover block using
rotateCommandCodeAccountOn429 and getCommandCodePoolAccessToken, remove the
unsupported branch unless an explicit supported routing contract is added, since
modelAdapters only permits openai-chat/openai-responses and the Anthropic pin
applies only to opencode-go.
In `@tests/command-code-account-pool.test.ts`:
- Around line 376-380: Correct the setup in the test “higher-priority account
with headroom preempts active under quota strategy”: remove the redundant
setActiveAccount call and update its misleading comment to reflect that
seedTwoAccounts already activates A, or explicitly describe the call as a
defensive guard if retaining it.
In `@tests/provider-account-quota.test.ts`:
- Around line 436-446: Add monthlyPercent coverage to the tests using
creditsBody, first ensuring the source quota calculation derives the correct
monthly denominator from the provider data rather than dividing monthlyCredits
by itself. Assert the expected quota.monthlyPercent value so the self-division
regression is detected while preserving the existing five-hour and weekly
assertions.
---
Outside diff comments:
In `@src/cli/account-api.ts`:
- Around line 241-255: The OAuth account row projection drops the configured
priority, causing cmdPriority to report zero after updates. In
src/cli/account-api.ts lines 241-255, add priority to OAuthAccountDto and map
the API-provided value into each projected row. In tests/cli-account.test.ts
lines 843-851, persist and return the mock priority, then invoke the priority
command without a value after writing it and assert that it reports 2.
🪄 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: 25a97e0b-ed93-4c61-8626-246d6b924c06
📒 Files selected for processing (31)
README.mdgui/src/components/provider-workspace/ProviderAuthPanel.tsxgui/src/components/provider-workspace/types.tsgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh-TW.tsgui/src/i18n/zh.tsgui/src/pages/Providers.tsxgui/src/pages/use-providers-oauth.tsgui/src/styles/provider-workspace-settings.csssrc/cli/account-api.tssrc/cli/account-extended.tssrc/codex/pool-rotation.tssrc/lib/state-store-registrations.tssrc/oauth/command-code-routing.tssrc/oauth/health.tssrc/oauth/oauth-pool-routing.tssrc/providers/quota.tssrc/server/management/oauth-account-routes.tssrc/server/responses/core.tssrc/types.tssrc/usage/log.tstests/account-pool-management-api.test.tstests/cli-account.test.tstests/command-code-account-pool.test.tstests/provider-account-quota.test.tstests/state-store-sweeper.test.ts
| priorityOf: config => { | ||
| const priorities = config.commandCodeAccountPool?.accountPriorities; | ||
| if (!priorities) return () => 0; | ||
| return accountId => ( | ||
| Object.hasOwn(priorities, accountId) | ||
| ? Math.max(-100, Math.min(100, priorities[accountId] ?? 0)) | ||
| : 0 | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-finite priority values before clamping.
priorities[accountId] comes from operator-edited config.json, so it is not guaranteed to be a number. If the value is a string such as "10", Math.min(100, "10") coerces and works, but a value such as "high" or NaN produces NaN. Every comparison against NaN is false, so the account lands in an undefined position inside selectPriorityTier instead of the documented default tier 0. Add a finite-number check so a malformed entry degrades to 0.
🛡️ Proposed guard
priorityOf: config => {
const priorities = config.commandCodeAccountPool?.accountPriorities;
if (!priorities) return () => 0;
- return accountId => (
- Object.hasOwn(priorities, accountId)
- ? Math.max(-100, Math.min(100, priorities[accountId] ?? 0))
- : 0
- );
+ return accountId => {
+ if (!Object.hasOwn(priorities, accountId)) return 0;
+ const raw = priorities[accountId];
+ if (typeof raw !== "number" || !Number.isFinite(raw)) return 0;
+ return Math.max(-100, Math.min(100, raw));
+ };
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| priorityOf: config => { | |
| const priorities = config.commandCodeAccountPool?.accountPriorities; | |
| if (!priorities) return () => 0; | |
| return accountId => ( | |
| Object.hasOwn(priorities, accountId) | |
| ? Math.max(-100, Math.min(100, priorities[accountId] ?? 0)) | |
| : 0 | |
| ); | |
| }, | |
| priorityOf: config => { | |
| const priorities = config.commandCodeAccountPool?.accountPriorities; | |
| if (!priorities) return () => 0; | |
| return accountId => { | |
| if (!Object.hasOwn(priorities, accountId)) return 0; | |
| const raw = priorities[accountId]; | |
| if (typeof raw !== "number" || !Number.isFinite(raw)) return 0; | |
| return Math.max(-100, Math.min(100, raw)); | |
| }; | |
| }, |
🤖 Prompt for 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.
In `@src/oauth/command-code-routing.ts` around lines 51 - 59, Update priorityOf’s
accountId priority resolution to validate priorities[accountId] is a finite
number before applying Math.max/Math.min clamping; return the default tier 0 for
strings, NaN, infinities, or other malformed values while preserving the
existing -100 to 100 bounds for valid numbers.
| if ( | ||
| response.status === 429 | ||
| && commandCodePoolAccountId | ||
| && isCommandCodeAccountPoolEnabled(config) | ||
| && commandCodePoolFailovers < COMMAND_CODE_POOL_MAX_FAILOVERS_PER_REQUEST | ||
| ) { | ||
| const nextAccountId = rotateCommandCodeAccountOn429( | ||
| config, | ||
| commandCodePoolAccountId, | ||
| response.headers.get("retry-after"), | ||
| commandCodeSessionKey, | ||
| ); | ||
| if (nextAccountId) { | ||
| try { void response.body?.cancel().catch(() => {}); } catch { /* already closed */ } | ||
| try { | ||
| const accessToken = await getCommandCodePoolAccessToken(nextAccountId); | ||
| commandCodePoolAccountId = nextAccountId; | ||
| commandCodePoolFailovers += 1; | ||
| route.provider = { ...route.provider, apiKey: accessToken }; | ||
| invalidateSameTargetRequest(); | ||
| promoteCommandCodeActiveAccount(nextAccountId); | ||
| logCtx.provider = formatCommandCodeProviderForLog("command-code", nextAccountId, config); | ||
| activeAdapter = resolveAdapter( | ||
| resolveWireProtocolOverride(route.providerName, route.modelId, route.provider, inboundWire), | ||
| config.cacheRetention, | ||
| ); | ||
| sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, activeAdapter.name, logCtx.accountLogLabel); | ||
| nextContinuationRecoveryKind = "command-code-oauth-429"; | ||
| continue; | ||
| } catch { | ||
| // fall through to emit continuation error below | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether a command-code route can resolve to the anthropic adapter,
# and whether any test drives the Responses failover loops.
set -uo pipefail
echo "== wire-override inputs: pinned wires, model defaults, allow-list =="
rg -nP --type=ts -C6 '\b(pinnedWireAdapter|providerModelWireDefault|MODEL_ADAPTER_OVERRIDE_ALLOWED)\b' src
echo
echo "== any command-code provider configured onto a non-command-code adapter? =="
rg -nP --type=ts -C4 'modelAdapters' src
echo
echo "== tests referencing the new recovery labels or the core failover loops =="
rg -nP -C4 'command-code-oauth-429|anthropic-oauth-429' tests
echo
echo "== tests that drive handleResponses with a command-code provider =="
rg -nPl --type=ts 'command-code' tests | while IFS= read -r f; do
rg -nP -C3 'handleResponses|/v1/responses' "$f" && echo "--- $f"
doneRepository: lidge-jun/opencodex
Length of output: 20394
🏁 Script executed:
#!/bin/bash
set -u
echo "== relevant core functions and guards =="
rg -n -C8 'fetchTerminalGuardContinuation|terminalGuardEnabled|commandCodePoolFailovers|invalidateSameTargetRequest|command-code-oauth-429|rotateCommandCodeAccountOn429' src/server/responses/core.ts
echo
echo "== Command Code provider definitions and pinned Anthropic models =="
rg -n -C8 'command-code|COMMAND_CODE|ANTHROPIC_WIRE_MODELS|isWirePinnedModel' src/providers src/types.ts src/config.ts
echo
echo "== adapter resolution and Command Code adapter identity =="
cat -n src/server/adapter-resolve.ts | sed -n '1,100p'
rg -n -C8 'createCommandCodeAdapter|name:.*command-code|name:.*anthropic' src/adapters src/server
echo
echo "== existing Responses test harnesses and Command Code references =="
rg -n -C6 'handleResponsesInner|fetchTerminalGuardContinuation|command-code|/v1/responses' tests src/server/responses | head -1200Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "== Command Code pool initialization and request routing =="
sed -n '1680,1855p' src/server/responses/core.ts
sed -n '3140,3470p' src/server/responses/core.ts
echo
echo "== continuation branch context =="
sed -n '3538,3798p' src/server/responses/core.ts
echo
echo "== focused tests and test utilities =="
fd -t f -e ts tests | sort | rg 'responses|command-code|server'
rg -n -C5 'handleResponses\(|handleResponsesInner|new Request|fetchResponse|createCommandCodeAdapter|rotate.*429' tests --glob '*.ts' | head -1000Repository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
types = Path("src/types.ts").read_text()
resolver = Path("src/server/adapter-resolve.ts").read_text()
registry = Path("src/providers/registry.ts").read_text()
core = Path("src/server/responses/core.ts").read_text()
allowed = set(re.findall(r'"([^"]+)"', types[types.index("MODEL_ADAPTER_OVERRIDE_ALLOWED"):types.index("]);", types.index("MODEL_ADAPTER_OVERRIDE_ALLOWED"))])
pinned = set(re.findall(r'"([^"]+)"', types[types.index("ANTHROPIC_WIRE_MODELS"):types.index("};", types.index("ANTHROPIC_WIRE_MODELS"))]))
print("allowed modelAdapters wires:", sorted(allowed))
print("pinned Anthropic provider/model entries:", sorted(pinned))
print("Command Code registry adapter:", re.search(r'id: "command-code",\s+label:.*?\s+adapter: "([^"]+)"', registry, re.S).group(1))
print("resolver can return Anthropic from modelAdapters:", '"anthropic"' in allowed)
print("resolver has Command Code-specific Anthropic pin:", "command-code" in pinned)
print("reachable pre-stream Command Code loop:", "while (\\n upstreamResponse.status === 429" in core and "commandCodePoolAccountId" in core)
print("continuation Command Code loop present:", "nextContinuationRecoveryKind = \"command-code-oauth-429\"" in core)
PYRepository: lidge-jun/opencodex
Length of output: 361
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
types = Path("src/types.ts").read_text()
resolver = Path("src/server/adapter-resolve.ts").read_text()
registry = Path("src/providers/registry.ts").read_text()
core = Path("src/server/responses/core.ts").read_text()
allowed = set(re.findall(r'"([^"]+)"', types[types.index("MODEL_ADAPTER_OVERRIDE_ALLOWED"):types.index("]);", types.index("MODEL_ADAPTER_OVERRIDE_ALLOWED"))])
pinned = set(re.findall(r'"([^"]+)"', types[types.index("ANTHROPIC_WIRE_MODELS"):types.index("};", types.index("ANTHROPIC_WIRE_MODELS"))]))
print("allowed modelAdapters wires:", sorted(allowed))
print("pinned Anthropic provider/model entries:", sorted(pinned))
print("Command Code registry adapter:", re.search(r'id: "command-code",\s+label:.*?\s+adapter: "([^"]+)"', registry, re.S).group(1))
print("resolver can return Anthropic from modelAdapters:", '"anthropic"' in allowed)
print("resolver has Command Code-specific Anthropic pin:", "command-code" in pinned)
print("reachable pre-stream Command Code loop:", "while (\n upstreamResponse.status === 429" in core and "commandCodePoolAccountId" in core)
print("continuation Command Code loop present:", "nextContinuationRecoveryKind = \"command-code-oauth-429\"" in core)
PYRepository: lidge-jun/opencodex
Length of output: 361
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
types = Path("src/types.ts").read_text()
registry = Path("src/providers/registry.ts").read_text()
core = Path("src/server/responses/core.ts").read_text()
allowed_start = types.index("MODEL_ADAPTER_OVERRIDE_ALLOWED")
allowed_end = types.index("]);", allowed_start)
allowed = set(re.findall(r'"([^"]+)"', types[allowed_start:allowed_end]))
pinned_start = types.index("ANTHROPIC_WIRE_MODELS")
pinned_end = types.index("};", pinned_start)
pinned = set(re.findall(r'"([^"]+)"', types[pinned_start:pinned_end]))
match = re.search(r'id: "command-code",\s+label:.*?\s+adapter: "([^"]+)"', registry, re.S)
print("allowed modelAdapters wires:", sorted(allowed))
print("pinned Anthropic provider/model entries:", sorted(pinned))
print("Command Code registry adapter:", match.group(1) if match else "not found")
print("resolver can return Anthropic from modelAdapters:", "anthropic" in allowed)
print("resolver has Command Code-specific Anthropic pin:", "command-code" in pinned)
print("continuation Command Code loop present:", 'nextContinuationRecoveryKind = "command-code-oauth-429"' in core)
print("pre-stream Command Code pool guard present:",
"upstreamResponse.status === 429" in core
and "commandCodePoolAccountId" in core
and "rebuildAndRefetch(\"command-code-oauth-429\")" in core)
PYRepository: lidge-jun/opencodex
Length of output: 1110
🏁 Script executed:
#!/bin/bash
set -u
echo "== provider adapter validation and normalization =="
rg -n -C8 'adapter.*(command-code|anthropic|openai-chat)|provider.*adapter|adapterConfigError|providerConfigError' src/config.ts src/types.ts src/providers src/server | head -500
echo
echo "== Command Code pool enablement and account selection tests =="
rg -n -C6 'isCommandCodeAccountPoolEnabled|command-code.*adapter|adapter: "anthropic"|providers.*command-code' tests src/oauth src/config.ts | head -500Repository: lidge-jun/opencodex
Length of output: 50375
Add a focused Responses regression test for Command Code pool failover.
src/server/responses/core.ts:3425-3460 is the reachable Command Code path. tests/command-code-account-pool.test.ts only tests routing in isolation. Add a Responses test that returns three consecutive 429 responses, asserts the first two retries use different account bearer tokens, calls invalidateSameTargetRequest(), and surfaces the third 429 without a fourth rotation.
The continuation branch at src/server/responses/core.ts:3745-3777 is not reached by the registry Command Code route. modelAdapters permits only openai-chat and openai-responses, and the Anthropic pin applies only to opencode-go. Remove this unsupported branch or add an explicit supported routing contract and a separate test for it.
🤖 Prompt for 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.
In `@src/server/responses/core.ts` around lines 3745 - 3778, Add a focused
Responses regression test covering the reachable Command Code flow around the
registry route: return three consecutive 429 responses, assert the first two
retries use distinct account bearer tokens, verify invalidateSameTargetRequest()
is invoked, and confirm the third 429 is surfaced without a fourth rotation. In
the continuation failover block using rotateCommandCodeAccountOn429 and
getCommandCodePoolAccessToken, remove the unsupported branch unless an explicit
supported routing contract is added, since modelAdapters only permits
openai-chat/openai-responses and the Anthropic pin applies only to opencode-go.
Source: Path instructions
| function creditsBody(fiveHourUsed: number, fiveHourCap: number, weeklyUsed: number, weeklyCap: number): string { | ||
| return JSON.stringify({ | ||
| credits: { monthlyCredits: 80, purchasedCredits: 0, freeCredits: 0 }, | ||
| windowLimits: { | ||
| limited: true, | ||
| exceeded: null, | ||
| fiveHour: { used: fiveHourUsed, cap: fiveHourCap, exceeded: false, resetAt: "2026-07-05T12:00:00Z" }, | ||
| weekly: { used: weeklyUsed, cap: weeklyCap, exceeded: false, resetAt: "2026-07-08T12:00:00Z" }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a monthlyPercent assertion to this fixture's tests.
creditsBody sets credits.monthlyCredits: 80 and supplies no monthly cap or monthly-used field. No test in this file asserts quota.monthlyPercent, which is why the self-division defect in src/providers/quota.ts lines 1309-1316 passes CI: that code computes percent(monthlyCredits, monthlyCredits), which is always 100.
Once the denominator question is settled on the source side, pin the expected value here so the constant cannot come back.
🤖 Prompt for 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.
In `@tests/provider-account-quota.test.ts` around lines 436 - 446, Add
monthlyPercent coverage to the tests using creditsBody, first ensuring the
source quota calculation derives the correct monthly denominator from the
provider data rather than dividing monthlyCredits by itself. Assert the expected
quota.monthlyPercent value so the self-division regression is detected while
preserving the existing five-hour and weekly assertions.
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx`:
- Around line 212-215: Replace the hardcoded "network error" fallback in the
manual-code submission catch block with a translated value from the prov locale
entries, resolving it through t(...) before interpolating into prov.pasteFail.
Preserve the existing Error message when available and update all relevant
locale files with the fallback entry.
In `@src/server/management/oauth-account-routes.ts`:
- Around line 251-253: Update the priorities selection in the OAuth
account-listing handler to return config.anthropicAccountPool?.accountPriorities
when provider is "anthropic", while preserving the existing command-code
behavior and undefined fallback for other providers. Add an integration test
that configures an Anthropic account priority and verifies the account-list
response includes that value.
🪄 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: 27198ddb-4008-4982-b316-ef6b481ba9cd
📒 Files selected for processing (13)
README.mdgui/src/components/provider-workspace/ProviderAuthPanel.tsxgui/tests/provider-auth-manual-code.test.tsxsrc/cli/account-api.tssrc/lib/state-store-registrations.tssrc/oauth/command-code-routing.tssrc/oauth/oauth-pool-routing.tssrc/providers/quota.tssrc/server/management/oauth-account-routes.tstests/account-pool-management-api.test.tstests/cli-account.test.tstests/command-code-account-pool.test.tstests/provider-account-quota.test.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gui/src/components/provider-workspace/ProviderAuthPanel.tsx (1)
387-390: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose manual-code feedback to assistive technologies.
The feedback element is added after an asynchronous submission, but it has no
roleoraria-liveattribute. Screen readers may not announce the success or error message.Render success as
role="status"and failure asrole="alert". Add an assertion for the role ingui/tests/provider-auth-manual-code.test.tsx.Proposed fix
- {manualCodeMsg && ( - <div className="text-label" style={{ color: manualCodeOk ? "var(--accent-hover)" : "var(--amber)" }}> + {manualCodeMsg && ( + <div + role={manualCodeOk ? "status" : "alert"} + aria-atomic="true" + className="text-label" + style={{ color: manualCodeOk ? "var(--accent-hover)" : "var(--amber)" }} + > {manualCodeMsg} </div> )}🤖 Prompt for 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. In `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx` around lines 387 - 390, Update the manualCodeMsg feedback element in ProviderAuthPanel to use role="status" when manualCodeOk is true and role="alert" otherwise, preserving the existing message and styling. Add an assertion in the manual-code test covering the rendered role for success and failure feedback.
🤖 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.
Outside diff comments:
In `@gui/src/components/provider-workspace/ProviderAuthPanel.tsx`:
- Around line 387-390: Update the manualCodeMsg feedback element in
ProviderAuthPanel to use role="status" when manualCodeOk is true and
role="alert" otherwise, preserving the existing message and styling. Add an
assertion in the manual-code test covering the rendered role for success and
failure feedback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2a62ad6e-235a-4018-a282-4695b196a233
📒 Files selected for processing (2)
gui/src/components/provider-workspace/ProviderAuthPanel.tsxgui/tests/provider-auth-manual-code.test.tsx
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review against current head 31cb7e8: requesting changes before merge.
-
Branch is stale against
dev. The PR is currently 45 commits behind the latestdev, while the PR's own readiness checklist requires being pushed to latestdev. Please update/rebase first, then rerun validation because this is a large cross-cutting change. -
Anthropic priority support is currently an incomplete contract. This PR exposes
ocx account priority anthropic ...and the management API persists Anthropic priorities, but the existing Anthropic routing implementation still does not consumeaccountPrioritiesoractiveAccountPinned. So the command can report success while actual routing behaviour does not change. The account listing also currently projects priorities only forcommand-code, so an Anthropic priority read can fall back to 0. Please either migrate Anthropic to the generic priority-aware router, implement equivalent priority/pin handling in the Anthropic router, or remove Anthropic priority support from the public CLI/API in this PR. -
Add an end-to-end Responses regression test for Command Code 429 failover. The routing unit tests are useful, but they do not cover the new retry loop in
src/server/responses/core.ts. Please test multiple consecutive 429 responses, verify retries use different account bearer tokens, verify same-target request state is invalidated, and verify the failover bound surfaces the final 429 without another rotation. Also either remove the Command Code continuation failover branch if it is unreachable under the supported routing contract, or document/support that route explicitly and test it. -
Manual-code feedback should be announced to assistive technology. The async success/error message in
ProviderAuthPanelhas no live-region semantics. Please userole="status"for success androle="alert"for failure (or equivalent) and cover it in the manual-code test.
Two currently open older findings appear already fixed at this head and can be resolved: the finite-number guard for Command Code priorities is present, and monthlyPercent is now correctly omitted and asserted as undefined when the provider does not expose a denominator.
Finally, the current GitHub Actions runs are action_required, not green. That may only be fork approval gating rather than a test failure, but this head still needs an actual green CI run after the branch is brought up to date.
|
Follow-up on scope: this PR is titled and framed as a Command Code account-pool feature, but it also expands Anthropic behavior in the management API and CLI (auto-switch / priority / cooldown handling). I do not think those Anthropic changes belong in this PR. Please either remove the Anthropic-facing changes here and keep this PR strictly Command Code-scoped, or split the Anthropic work into a separate PR with its own routing contract, tests, and review. Mixing the two makes the change harder to reason about and has already created a partial-contract problem: Anthropic priority can be written through the new API/CLI path while the existing Anthropic router does not implement that priority/pin behavior. So I would prefer this PR to stay focused on Command Code only. Any Anthropic parity work should be reviewed independently. |
31cb7e8 to
c7ebcb3
Compare
Add an opt-in Command Code OAuth account pool mirroring the Codex pool rotation strategy (quota / round-robin / fill-first, sticky session affinity, 429 failover, selection-order priority, manual pin): - Generic OAuth pool router (oauth-pool-routing.ts) shared by providers; command-code adapter (command-code-routing.ts) wires config hooks - Per-account 5h + weekly quota from GET /alpha/billing/credits (fiveHourPercent / weeklyPercent), with provider-level report seeding - Management API: pool config GET/PUT/PATCH, priority, clear-cooldown, cooldown in health projection; CLI auto-switch/priority/clear-cooldown extended to anthropic + command-code - Responses core: pool selection per session, 429 failover (bounded) - GUI: account panel Add account shows a paste box accepting a redirect URL / auth code / raw API key (Command Code), wired through the existing /api/oauth/login/code path - Tests: 21 pool-routing unit tests, pool management API tests, CLI auto-switch/priority tests, per-account quota tests
c7ebcb3 to
4260619
Compare
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The Command Code-only scope correction is good, and the prior malformed-priority and fake monthly-percentage defects are fixed on exact head 4260619. I am still requesting changes because the account-token failover contract is not proven at the actual Responses boundary.
-
Add a focused end-to-end Responses regression for the reachable Command Code adapter path: seed at least three OAuth accounts, return consecutive 429s, prove the first retry uses a different bearer/account, prove same-target request state is invalidated before reconstruction, enforce the per-request rotation bound, and surface the final 429 without another rotation. The current pool-unit tests do not exercise route mutation, adapter reconstruction, request headers, response-body settlement, or the terminal request log together.
-
Remove the Command Code branch inside
fetchTerminalGuardContinuationunless you add a supported reachability contract. That continuation generator is selected only whenactiveAdapter.name === "anthropic"; the canonical Command Code route resolves to adaptercommand-code, so the new continuation branch is currently dead and creates an untested maintenance promise. -
Replace the placeholder screenshot URL with a real screenshot from this exact GUI head or remove the visual claim and request an explicit owner waiver. This PR adds a raw-API-key input surface; a generated placeholder is not UI evidence.
Keep this draft and do not apply maintainer-sponsored until those authentication/failover boundaries, the new CodeRabbit threads, typecheck/privacy/GUI checks, and full exact-head cross-platform CI are green. The direction remains potentially valuable after the runtime proof is complete.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/responses/core.ts (1)
22-22: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit encrypted agent-task recovery from this Command Code pool PR.
These changes add request decryption recovery, persistence exclusion, and a second model-routing pass for general Responses thread-spawn traffic. They are independent of Command Code account selection and 429 failover.
Move this feature to a separate PR, or revert it from this PR. This keeps the Command Code routing contract reviewable and limits the security-sensitive change surface.
Also applies to: 193-196, 1707-1799
🤖 Prompt for 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. In `@src/server/responses/core.ts` at line 22, Remove the encrypted agent-task recovery changes from this PR, including markBodyNonPersistable and the related request decryption, persistence exclusion, and second model-routing pass in the affected response-handling logic. Revert these changes while preserving the Command Code account selection and 429 failover behavior.
🤖 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 `@gui/src/i18n/de.ts`:
- Line 368: Update the German translation value for prov.pasteCommandCodeHint so
the API-key instruction and redirect-URL instruction are expressed as two
separate, grammatically complete sentences, clearly distinguishing the
alternatives while preserving the existing meaning.
---
Outside diff comments:
In `@src/server/responses/core.ts`:
- Line 22: Remove the encrypted agent-task recovery changes from this PR,
including markBodyNonPersistable and the related request decryption, persistence
exclusion, and second model-routing pass in the affected response-handling
logic. Revert these changes while preserving the Command Code account selection
and 429 failover behavior.
🪄 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: 9a4eaa4c-2536-4800-9a80-18225e92d722
📒 Files selected for processing (18)
README.mdgui/src/components/provider-workspace/ProviderAuthPanel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh-TW.tsgui/src/i18n/zh.tsgui/tests/provider-auth-manual-code.test.tsxsrc/cli/account-extended.tssrc/server/management/oauth-account-routes.tssrc/server/responses/core.tssrc/types.tstests/account-pool-management-api.test.tstests/cli-account.test.tstests/server-rate-limit-retry-e2e.test.ts
| "prov.pasteRedirectHint": "Zeigt der Browser einen localhost-Fehler, kopiere die vollständige URL aus der Adressleiste und füge sie hier ein (oder den Autorisierungscode).", | ||
| "prov.pasteRedirectHint": "Falls der Browser einen localhost-Fehler zeigt, kopieren Sie die vollständige URL aus der Adressleiste und fügen Sie sie hier ein (oder fügen Sie den Autorisierungscode ein).", | ||
| "prov.pasteCommandCodePlaceholder": "Command-Code-API-Schlüssel oder Redirect-URL einfügen", | ||
| "prov.pasteCommandCodeHint": "Fügen Sie einen Command-Code-API-Schlüssel (user_… aus ~/.commandcode/auth.json) ein, um ihn als weiteres Konto hinzuzufügen, oder die Redirect-URL aus dem Browser.", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the German Command Code authentication hint.
Line [368] joins two alternatives without a verb. Split the alternatives into separate sentences so users can distinguish an API key from a redirect URL.
Proposed wording
- "prov.pasteCommandCodeHint": "Fügen Sie einen Command-Code-API-Schlüssel (user_… aus ~/.commandcode/auth.json) ein, um ihn als weiteres Konto hinzuzufügen, oder die Redirect-URL aus dem Browser.",
+ "prov.pasteCommandCodeHint": "Fügen Sie einen Command-Code-API-Schlüssel (user_… aus ~/.commandcode/auth.json) ein, um ihn als weiteres Konto hinzuzufügen. Alternativ können Sie die Redirect-URL aus dem Browser einfügen.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "prov.pasteCommandCodeHint": "Fügen Sie einen Command-Code-API-Schlüssel (user_… aus ~/.commandcode/auth.json) ein, um ihn als weiteres Konto hinzuzufügen, oder die Redirect-URL aus dem Browser.", | |
| "prov.pasteCommandCodeHint": "Fügen Sie einen Command-Code-API-Schlüssel (user_… aus ~/.commandcode/auth.json) ein, um ihn als weiteres Konto hinzuzufügen. Alternativ können Sie die Redirect-URL aus dem Browser einfügen.", |
🤖 Prompt for 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.
In `@gui/src/i18n/de.ts` at line 368, Update the German translation value for
prov.pasteCommandCodeHint so the API-key instruction and redirect-URL
instruction are expressed as two separate, grammatically complete sentences,
clearly distinguishing the alternatives while preserving the existing meaning.
- gui: split de pasteCommandCodeHint into two sentences - responses: remove dead Command Code branch from fetchTerminalGuardContinuation (anthropic-only guard, no reachable command-code adapter path)
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
The Anthropic scope is still not actually removed from this PR.
The PR body says anthropic scoping removed per review — strictly command-code in this PR, but the current head still rewrites shared management behavior around Anthropic. In src/server/management/oauth-account-routes.ts, the changed code now explicitly handles both anthropic and command-code for /api/oauth/accounts/pool, chooses between anthropicAccountPool and commandCodeAccountPool, and dispatches clear-cooldown back into clearAnthropicAccountCooldown() for Anthropic.
That is not the scope correction I asked for. Removing the new Anthropic priority/pin behavior was good, but it is not the same as removing Anthropic from this Command Code PR.
Please keep the existing Anthropic path untouched and add Command Code alongside it, or split the generic Anthropic/Command Code management refactor into a separate PR. I do not want a Command Code feature PR to also rewrite/generalize Anthropic behavior, even if the intended Anthropic behavior is preserved.
In particular, please remove the new Anthropic-facing branches introduced by this PR from the shared management-path changes rather than presenting them as part of the Command Code implementation. Any generic OAuth-pool refactor that changes how Anthropic is routed through these endpoints should get its own scope, tests, and review.
…ongside Wibias: previous pool/clear-cooldown handlers rewrote the shared anthropic path to a generic poolKey dispatch. Keep anthropic exactly as on upstream/dev (verbatim) and handle command-code in separate branches alongside it, so this Command Code PR does not generalize anthropic management behavior. No anthropic contract change. Co-authored-by: Wibias review 2026-08-13
3 participants
Summary
Add an opt-in Command Code OAuth account pool mirroring the Codex pool rotation strategy: quota / round-robin / fill-first new-session picking, sticky session affinity, 429 failover, selection-order priority, and manual pin. This enables multi-account Command Code use with auto-switch, exactly like the Codex pool.
Design
src/oauth/oauth-pool-routing.ts) — provider-parameterized, shared engine capturing the Codex strategy (priority tiers viaselectPriorityTier, quota headroom, affinity, failover streak, 429 cooldown, RR/fill-first). Command Code adapter (src/oauth/command-code-routing.ts) wires config hooks.GET /alpha/billing/credits(5h + weekly used/cap →fiveHourPercent/weeklyPercent), plus provider-report seeding of the active account cache./api/oauth/accounts/poolGET/PUT/PATCH (enabled/threshold/strategy/sticky + accountPriorities + pin),/api/oauth/accounts/pool/priority,/api/oauth/accounts/clear-cooldown, cooldown surfaced in health projection.ocx account auto-switch command-code,priority,clear-cooldownextended to the Command Code OAuth pool (anthropic scoping removed per review — strictly command-code in this PR)./api/oauth/login/codepath. All 8 locales updated.Verification
tsc --noEmitclean; GUItsc -b+vite buildcleanocx account auto-switch command-code on/statusGUI change
The
ProviderAuthPanelpaste box now usestype="password"(masked API key) androle="status"/role="alert"witharia-atomic="true"for screen-readerannouncement. This is asserted in
gui/tests/provider-auth-manual-code.test.tsx.Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit