fix(routing): keep unbound account quota unknown (#1195) - #1288
Conversation
Policy profiles choose a provider and model before the request path resolves Pool/Direct identity, thread affinity, Anthropic session affinity, or round-robin/fill-first selection. Attaching the process-global active account during policy evaluation could therefore score or exclude a candidate using account A's quota and then execute the request on account B. An unbound candidate now stays quota-unknown in both the live route trace and the management dry-run, which is more accurate than inventing an account reference and keeps account selection, cooldowns, and session affinity authoritative. Unknown quota already has an explicit profile policy. Explicit `codexAccountId` and account-ref evidence remains unchanged. Republished from #1195 by luvs01, whose branch was 300 commits behind dev. Rebased onto f5147cb with no conflicts; authorship preserved below. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
… unknown Maintainer-added coverage for the #1195 republish. The contributor's patch deletes the same block from the live router and the management dry-run path, but only the live path had a regression. The existing dry-run test covers a candidate with an explicitly supplied codexAccountId, which stays known and is unaffected by the fix, so the dry-run half of the parity claim was unproven. These two tests exercise an unbound Codex candidate with an active pool account, and an unbound Anthropic candidate with an active account, and assert both stay quota-unknown with no accountRef. Restoring either deleted block fails them.
📝 WalkthroughWalkthroughRouting policy quota evidence now requires an explicitly identified candidate account. Active Codex and Anthropic accounts no longer populate unbound candidates. Execution-path and dry-run tests verify absent account references and unknown quota status. ChangesQuota evidence handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs-site/src/content/docs/reference/configuration/routing.md`:
- Around line 156-166: The localized routing documentation pages for ja, ko, ru,
and zh-cn need the same quota-evidence behavior described in the English routing
page. Add localized text covering account-bound Codex and Anthropic quota
caches, including the dry-run fields candidates[].codexAccountId,
candidates[].accountRef, and candidates[].quota, while explicitly stating that
an active account is not a substitute and quota evidence does not affect account
selection or switching 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: 683daa22-a041-4ddf-8611-ebd4c301f9f0
📒 Files selected for processing (5)
docs-site/src/content/docs/reference/configuration/routing.mdsrc/router.tssrc/server/management/routing-profile-routes.tstests/quota-scoring.test.tstests/routing-profile.test.ts
💤 Files with no reviewable changes (2)
- src/router.ts
- src/server/management/routing-profile-routes.ts
| account-keyed Codex and Anthropic quota caches. A runtime candidate receives cached quota only when | ||
| the evidence already identifies the account. Unbound canonical `openai` and Anthropic candidates | ||
| remain unknown during policy evaluation because Pool selection, Direct caller identity, provider | ||
| rotation, and thread affinity are resolved after the policy chooses a provider/model; a process-active | ||
| account is not used as a substitute. | ||
| Quota evidence never changes account selection, session affinity, cooldowns, or switching behavior — | ||
| it only feeds policy scoring. To see quota-aware behavior in an API dry-run, supply account refs in | ||
| the candidate evidence sent to `POST /api/routing-profiles/dry-run`: | ||
| `candidates[].codexAccountId` (Codex pool, provider `openai`) or `candidates[].accountRef` | ||
| (Anthropic) derives the matching cached account quota; an explicit `candidates[].quota` object is | ||
| echoed as given. The CLI dry-run cannot supply these per-candidate account fields. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List routing documentation pages, including localized variants.
fd -t f -g 'routing.md' docs-site/src/content/docs | sort
# Inspect quota and account-routing statements in all routing pages.
rg -n -C 3 --glob 'routing.md' \
'quota|accountRef|codexAccountId|active account|Pool|Anthropic|affinity|switching' \
docs-site/src/content/docsRepository: lidge-jun/opencodex
Length of output: 19417
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the quota/account-routing paragraphs across all routing docs, with line numbers.
for file in docs-site/src/content/docs/reference/configuration/routing.md \
docs-site/src/content/docs/ja/reference/configuration/routing.md \
docs-site/src/content/docs/ko/reference/configuration/routing.md \
docs-site/src/content/docs/ru/reference/configuration/routing.md \
docs-site/src/content/docs/zh-cn/reference/configuration/routing.md; do
echo "===== $file ====="
rg -n -C 4 --glob 'routing.md' \
'quota|accountRef|codexAccountId|active account|active Pool account|Pool selection|Anthropic|affinity|switching|dry-run|Candidates' \
"$file" || true
done
# Extract only routing docs into a small JSON report for deterministic comparison by file and quoted key terms.
python3 - <<'PY'
from pathlib import Path
terms = [
"quota", "accountRef", "codexAccountId", "active account", "active Pool account",
"Pool selection", "Anthropic", "affinity", "switching", "dry-run", "active"
]
for locale in ["", "ja", "ko", "ru", "zh-cn"]:
path = Path("docs-site/src/content/docs") / f"{locale}reference/configuration/routing.md".replace("//", "/")
if not path.exists():
print(locale or "en", "missing")
continue
text = path.read_text()
print(f"locale={locale or 'en'} lines={len(text.splitlines())}")
for term in terms:
hits = [i+1 for i,line in enumerate(text.splitlines()) if term in line]
if hits:
print(f" {term}: {hits[:10]}{'...' if len(hits)>10 else ''}")
PYRepository: lidge-jun/opencodex
Length of output: 18122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Compare the exact routing profile evidence/quota sections across locales against the English source.
for locale in ja ko ru zh-cn; do
en="docs-site/src/content/docs/reference/configuration/routing.md"
loc="docs-site/src/content/docs/${locale}/reference/configuration/routing.md"
lo=$(wc -l < "$loc")
echo "===== $locale: $loc $lo lines ====="
sed -n '94,170p' "$loc"
echo
done
# Find any active/account quota phrasing that could contradict the English contract.
rg -n -i --glob 'routing.md' \
'quota.*(account|active|process)|active.*(quota|accountRef|codexAccountId)|account.*(quota|cache)|can.*quota|supply.*quota|active Pool account|process-active' \
docs-site/src/content/docs | sed -n '1,200p'Repository: lidge-jun/opencodex
Length of output: 10717
Add the localized quota-evidence paragraphs to the routing docs.
The localized routing pages end before the English optimize.quota behavior, so docs-site/src/content/docs/{ja,ko,ru,zh-cn}/reference/configuration/routing.md can now contradict the source by omission. Add the matching account-bound quote and dry-run evidence text; do not present an active Codex or Anthropic account as a substitute for candidates[].codexAccountId / candidates[].accountRef.
🤖 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 `@docs-site/src/content/docs/reference/configuration/routing.md` around lines
156 - 166, The localized routing documentation pages for ja, ko, ru, and zh-cn
need the same quota-evidence behavior described in the English routing page. Add
localized text covering account-bound Codex and Anthropic quota caches,
including the dry-run fields candidates[].codexAccountId,
candidates[].accountRef, and candidates[].quota, while explicitly stating that
an active account is not a substitute and quota evidence does not affect account
selection or switching behavior.
Sources: Path instructions, Learnings
![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: 3fc962f2cf
ℹ️ 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".
| quota: quotaEvidenceForCandidate({ | ||
| provider: candidate.provider, | ||
| model: candidate.model, | ||
| ...(candidate.provider === OPENAI_CODEX_PROVIDER_ID | ||
| && providerCodexAccountMode( | ||
| OPENAI_CODEX_PROVIDER_ID, | ||
| config.providers[OPENAI_CODEX_PROVIDER_ID], | ||
| ) === "pool" | ||
| ? (() => { | ||
| const codexAccountId = getEffectiveActiveCodexAccountId(config); | ||
| return { | ||
| codexAccountId, | ||
| codexAccountPlan: codexAccountId | ||
| ? config.codexAccounts?.find(account => account.id === codexAccountId)?.plan | ||
| : undefined, | ||
| }; | ||
| })() | ||
| : {}), | ||
| accountRef: candidate.provider === "anthropic" | ||
| ? getAccountSet("anthropic")?.activeAccountId | ||
| : undefined, | ||
| }), |
There was a problem hiding this comment.
For any live Pool-mode profile, this call now receives neither codexAccountId nor codexAccountPlan, and routing-profile candidates cannot configure those fields, so an openai candidate always gets { known: false } even when the reconciled pool has fresh quota data. The deleted path previously used the plan as a sentinel for quotaEvidenceForCandidate to aggregate every cached pool account via codexPoolQuotaEvidence—it did not attribute only account A's quota—so configurations using unknownEvidence.quota: "exclude" can now reject a healthy pool, while quota weighting and minQuotaHeadroom cease working on live Pool routes. Remove the active-account attribution but preserve provider-level aggregate evidence for the unbound pool, as already specified by tests/routing-policy-pool-quota.test.ts.
Useful? React with 👍 / 👎.
1 participant
Summary
Republishes @luvs01's #1195 on current
dev. Their branch was 300 commits behind, so this is a maintainer rebase; the fix commit carries aCo-authored-bytrailer and the routing change is theirs.Policy profiles choose a provider and model before the request path resolves Pool/Direct identity, thread affinity, Anthropic session affinity, or round-robin/fill-first selection. Attaching the process-global active account during policy evaluation could therefore score or exclude a candidate using account A's quota and then execute the request on account B.
An unbound candidate now stays quota-unknown in both the live route trace and the management dry-run. Unknown quota already has an explicit profile policy, so leaving it unknown is more accurate than inventing an account reference, and it keeps account selection, cooldowns, and session affinity authoritative. Explicit
codexAccountIdand account-ref evidence is unchanged and still resolves to known quota.This PR contains one maintainer-authored commit that was not in #1195.
test(routing): prove the management dry-run leaves unbound candidates unknownadds two regressions totests/routing-profile.test.ts. The contributor's patch deletes the same block from the live router and the management dry-run path, but only the live path had a regression; the existing dry-run test covers a candidate with an explicitly suppliedcodexAccountId, which stays known and is unaffected by the fix. Without those two tests the live/dry-run parity claim was unproven, so it is called out here rather than folded silently into the contributor's commit.Supersedes #1195, which can be closed once this lands.
Verification
Rebased onto
f5147cbc8and re-verified after that rebase:bun run test— 9992 pass / 7 skip / 0 fail across 625 filesbun test tests/routing-profile.test.ts tests/quota-scoring.test.ts— 31 pass / 0 failbun run typecheck— cleandev(git checkout origin/dev -- src/router.ts src/server/management/routing-profile-routes.ts) gives 27 pass / 4 fail over the same 31 tests. The four failures are the two contributor runtime tests and the two maintainer dry-run tests, which is the intended coverage split. Restoring the patch returns 31 pass / 0 fail.Checklist
Summary by CodeRabbit
Bug Fixes
Documentation