fix(codex): fall back to caller main during Pool cooldown - #3997
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughPool routing now falls back to a validated caller-owned main credential when a selected stored account is cooling down and no recovery probe is available. Tests verify routing, policy limits, forwarded credentials, and preserved Pool state. English and Korean guides document the behavior. ChangesPool cooldown fallback
Priority: ➖ Normal — Impact reflects medium issue severity. Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to When a Pool account is cooling down without a recovery probe, eligible unbound requests can use their validated caller-owned main credential instead of failing. The documented fallback preserves account bindings, quota policy, and Pool cooldown state, with focused regression coverage for those boundaries. Sequence Diagram(s)sequenceDiagram
participant CodexRequest
participant resolveCodexAuthContext
participant PoolState
participant CallerMainCredential
CodexRequest->>resolveCodexAuthContext: Submit request with validated main credential
resolveCodexAuthContext->>PoolState: Check selected account cooldown and probe lease
PoolState-->>resolveCodexAuthContext: No eligible stored account or probe
resolveCodexAuthContext->>CallerMainCredential: Resolve caller-owned main context
CallerMainCredential-->>resolveCodexAuthContext: Return main authentication context
resolveCodexAuthContext-->>CodexRequest: Route request without changing Pool state
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (2 skipped: 2 unsupported.)
✨ 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 |
github-actions
Bot
added
the
intake: hygiene-blocked
|
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
|
@coderabbitai full review Please review head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== HEAD AND BASE ==='
git rev-parse HEAD
git merge-base HEAD origin/dev 2>/dev/null || true
printf '%s\n' '=== CHANGED FILES ==='
git diff --name-status origin/dev...HEAD
printf '%s\n' '=== DIFF STAT ==='
git diff --stat origin/dev...HEAD
printf '%s\n' '=== AUTH-CONTEXT OUTLINE ==='
ast-grep outline src/codex/auth-context.ts --items all 2>/dev/null || true
printf '%s\n' '=== RELEVANT SYMBOL LOCATIONS ==='
rg -n -C 3 --glob '*.ts' \
'CodexAccountCooldownError|cooldown|recovery probe|recoveryProbe|caller.*main|main.*caller|99%|0\.99|0\.9899|resolve.*Main|main.*quota|exact.*binding' \
src tests 2>/dev/null || trueLength of output: 1266 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='402be7c1f88283eb8465c3aec8437ccecd2542ec'
head='094e509f042f573cf4104d91562c249b2310cb0c'
printf '%s\n' '=== BASE OBJECT ==='
git cat-file -t "$base"
printf '%s\n' '=== CHANGED FILES ==='
git diff --name-status "$base" "$head"
printf '%s\n' '=== DIFF STAT ==='
git diff --stat "$base" "$head"
printf '%s\n' '=== FULL DIFF ==='
git diff --find-renames --find-copies --unified=80 "$base" "$head"
printf '%s\n' '=== AUTH-CONTEXT OUTLINE ==='
ast-grep outline src/codex/auth-context.ts --items all 2>/dev/null || true
printf '%s\n' '=== COOLDOWN AND FALLBACK LOCATIONS ==='
rg -n -C 8 --glob '*.ts' \
'CodexAccountCooldownError|cooldown|recoveryProbe|recovery probe|resolve.*[Mm]ain|[Mm]ain.*quota|caller.*credential|credential.*caller|0\.99|98\.99|99%' \
src test tests 2>/dev/null || trueLength of output: 201065 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='402be7c1f88283eb8465c3aec8437ccecd2542ec'
head='094e509f042f573cf4104d91562c249b2310cb0c'
printf '%s\n' '=== CHANGED FILES ==='
git diff --name-only "$base" "$head"
printf '%s\n' '=== AUTH-CONTEXT DIFF ==='
git diff --unified=35 "$base" "$head" -- src/codex/auth-context.ts
printf '%s\n' '=== CHANGED TEST DIFF FILES ==='
git diff --name-only "$base" "$head" | rg '(^|/)(test|tests)/|\.test\.' || true
printf '%s\n' '=== FOCUSED TEST DIFF ==='
git diff --unified=25 "$base" "$head" -- \
'tests/**/*auth*context*.test.ts' \
'tests/**/*hard*lock*.test.ts' \
'tests/**/*auth*.test.ts'
printf '%s\n' '=== RESOLVER SYMBOL MAP ==='
ast-grep outline src/codex/auth-context.ts --items all 2>/dev/null || true
printf '%s\n' '=== FOCUSED TEST LOCATIONS ==='
rg -n -i -C 4 \
'98\.99|99%|99 percent|caller-owned main|caller.*main|main.*caller|recovery probe|recoveryProbe|physical main|cooldown branch' \
tests --glob '*.test.ts' 2>/dev/null || trueLength of output: 115881 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='402be7c1f88283eb8465c3aec8437ccecd2542ec'
head='094e509f042f573cf4104d91562c249b2310cb0c'
printf '%s\n' '=== AUTH-CONTEXT: RESOLVER ==='
nl -ba src/codex/auth-context.ts | sed -n '585,970p'
printf '%s\n' '=== AUTH-CONTEXT: MAIN POLICY AND CALLER MATCHING ==='
nl -ba src/codex/auth-context.ts | sed -n '430,585p'
printf '%s\n' '=== DOCUMENTATION DIFF ==='
git diff --unified=20 "$base" "$head" -- \
docs-site/src/content/docs/guides/codex-integration.md \
docs-site/src/content/docs/ko/guides/codex-integration.md
printf '%s\n' '=== DIFF WHITESPACE CHECK ==='
git diff --check "$base" "$head"Length of output: 1014 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='402be7c1f88283eb8465c3aec8437ccecd2542ec'
head='094e509f042f573cf4104d91562c249b2310cb0c'
printf '%s\n' '=== AUTH-CONTEXT: POLICY AND CALLER HELPER (LINES 430-585) ==='
sed -n '430,585p' src/codex/auth-context.ts
printf '%s\n' '=== AUTH-CONTEXT: RESOLVER (LINES 585-970) ==='
sed -n '585,970p' src/codex/auth-context.ts
printf '%s\n' '=== ENGLISH DOCUMENTATION DIFF ==='
git diff --unified=20 "$base" "$head" -- docs-site/src/content/docs/guides/codex-integration.md
printf '%s\n' '=== KOREAN DOCUMENTATION DIFF ==='
git diff --unified=20 "$base" "$head" -- docs-site/src/content/docs/ko/guides/codex-integration.md
printf '%s\n' '=== DIFF WHITESPACE CHECK ==='
git diff --check "$base" "$head"Length of output: 33601
I found no blocking issues. The change in
I performed static inspection only. The sandbox does not run the test suite. Full contributor CI and the requested maintainer security review remain required gates. ✅ Action performedFull review finished. |
리뷰 · 우선순위 72 / 80이 PR은 이슈 #3996을 고칩니다. 지금 고치는 코드는 작습니다. probe lease가 없을 때, 현재 라인 src/codex/auth-context.ts 쿨다운 분기 - HEAD에는 probe 실패 시 즉시 throw만 있고 caller-main fallback이 없다. 이 PR의 7줄이 그 구멍이다 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
2 participants
Summary
Closes #3996.
A stored Pool account's cooldown can reject a fresh native request before it considers that request's eligible main credential. The existing post-upstream-rejection path can use the same credential, leaving successive requests with inconsistent behavior.
Use the existing caller-owned-main resolver when the selected stored account is cooling down and no recovery probe is available. The production change is seven lines and preserves caller/model validation, main quota policy, exact account bindings, Pool selection and cooldown state. English/Korean integration guidance describes the behavior.
This is independent of manual reset-credit reconciliation (#3973): it spends no credit and clears no cooldown. It also preserves the existing post-upstream retry path rather than adding another upstream retry loop.
Verification
Head
094e509f042f573cf4104d91562c249b2310cb0c, based ondev402be7c1f88283eb8465c3aec8437ccecd2542ec.CodexAccountCooldownError; after the fix, the authentication-context suite passed 71 tests / 286 assertions with the project-pinned Bun 1.4.0 on Windows.bun run typecheck,bun run privacy:scan, andgit diff --check: passed on the final head.devproduced an identical topic range-diff. The intervening upstream changes do not overlap these five files; the focused tests and documentation inputs were unchanged.Full contributor CI passed all 26 jobs on this exact head. Full-suite evidence comes from that contributor run; the focused checks above ran locally. CodeRabbit's exact-head review found no blocking issue. Maintainer sponsorship for
src/codex/auth-context.tsand explicit maintainer security review remain pending. This PR stays in draft until those gates are satisfied; agent review is not maintainer sponsorship.Checklist
Review readiness checklist
Summary by CodeRabbit
New Features
Documentation