fix(responses): keep the retryable main-refresh refusal an overload, not a bad key - #4269
Conversation
…not a bad key The 2.51.0 candidate reworded the retryable main-account refusal to end with 'the main Codex account needs reauthentication'. classifyError runs isAuthenticationMessage before it reaches the status === 503 arm, and that check is status-blind on the bare substring 'authentication', which 'reauthentication' contains. The body was served as authentication_error / invalid_api_key while still returning 503. Codex keys retry-after backoff on server_is_overloaded, so a transient token refresh started reading to the client as a bad API key and it stopped retrying. On 2.50.0 the same failure classified as server_error / server_is_overloaded. The pool counterpart in core.ts documents this exact trap and words itself around it; the main path walked into it anyway. It now uses the same construction: sign in to the main Codex account again. The old test asserted only the 503 and the word 'reauthentication', which is why the reclassification shipped unnoticed. The added test asserts error.type and error.code, and that the message carries no 'authentication' substring at all - the substring is the thing that reclassifies, not the phrasing.
|
✅ Deterministic PR hygiene checks passed. |
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. |
|
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 (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe retryable main-account refresh failure message no longer contains “reauthentication.” A comment documents the classification constraint. Integration tests verify the ChangesRetryable refresh error classification
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Retryable main-account refresh failures retain overload classification and client retry behavior. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
리뷰 · 우선순위 76 / 80이 PR은 2.51.0 후보에서 생긴 메인 Codex 계정 토큰 새로고침 거절 분류 회귀를 고칩니다. CURRENT 무슨 일이냐면 이렇습니다. #4212 쪽에서 재시도 가능한 메인 거절 문장을 친절하게 바꾸면서 끝에 Codex 클라이언트는 재시도 백오프를 HTTP 상태보다 고치는 내용은 작습니다. 라인 40 근처 - 재시도 가능 503 문장이 라인 27 - 터미널
tests/.../codex-account-unusable-reason.test.ts 새 테스트 - 분류( 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Maintainer integration into This is a release blocker found by the 2.51.0 pre-release regression audit (lane L1) and reproduced independently before the fix. It is going in ahead of the promotion, not waived. Exact-head verification —
Merging with a merge commit, matching the convention on |
1 participant
Summary
The 2.51.0 release candidate reworded the retryable main-account refresh refusal to end with "the main Codex account needs reauthentication". That single word changes how the body is classified.
classifyErrorrunsisAuthenticationMessage(src/lib/errors.ts:111) before it reaches thestatus === 503arm, and the check is status-blind on the bare substringauthentication— whichreauthenticationcontains. So the refusal was served asauthentication_error/invalid_api_keywhile still returning HTTP 503.Measured against this tree:
error.typeerror.codeserver_errorserver_is_overloadedauthentication_errorinvalid_api_keyserver_errorserver_is_overloadedCodex keys its retry-after backoff on
server_is_overloaded, not on the HTTP status. A transient main-token refresh — the case the 503 exists to cover, and one a default Codex install reaches — therefore started reading to the client as a bad API key, and the client stopped retrying instead of backing off. That is a regression against 2.50.0.The pool counterpart added in the same release already documents this trap in
src/server/responses/core.ts:2282-2286and deliberately words itself around it. The main path walked into it anyway. This commit gives it the same construction: sign in to the main Codex account again.The intent of #4212 is preserved — the refusal still names what is failing and what to do when retrying stops helping. It just stops saying the one word that reclassifies it.
The terminal
reason === "reauth"branch above is untouched: that one is a 401 and is meant to classify asauthentication_error.Verification
bun run typecheck— exit 0.bun test tests/codex-integration/codex-account-unusable-reason.test.ts— 10 pass / 0 fail.error.codebeinginvalid_api_key; with the fix it passes.The pre-existing test asserted only the 503, the
Retry-After, and that the message contained "reauthentication" — nevererror.typeorerror.code. That is precisely why the reclassification shipped unnoticed, so the added test asserts the classification, and asserts the message carries noauthenticationsubstring at all. The substring is the thing that reclassifies; pinning the phrasing would not have caught this and would not catch the next rewording either.Found by the 2.51.0 pre-release regression audit (lane L1), reproduced independently before the fix.
Checklist
Summary by CodeRabbit