From b83a0db8bc30daa7c389b6fdd642c00e31c9af91 Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 11 Sep 2026 19:30:27 +0900 Subject: [PATCH] fix(responses): keep the retryable main-refresh refusal an overload, 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. --- src/server/responses/codex-auth-error.ts | 14 ++++++++++++- .../codex-account-unusable-reason.test.ts | 20 ++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 8ae85e837b..8cddd70d6c 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -33,11 +33,23 @@ export function nativeMainRefreshFailureResponse(error: unknown): Response { // concluded the proxy had broken while one account was the thing that needed them. The refusal // stays a retryable 503 because the refresh genuinely may succeed, but it now names what is // failing and what to do when retrying stops helping. + // + // It says "sign in to the main Codex account again" and deliberately does NOT say + // "reauthentication", for the same reason the pool counterpart does not — see + // `poolCredentialRefreshIncompleteResponse` in ./core.ts. `classifyError` runs + // `isAuthenticationMessage` before it reaches the `status === 503` arm, and that check is + // status-blind on the bare substring "authentication", which "reauthentication" contains. + // A body carrying that word is reclassified to `authentication_error` / `invalid_api_key` + // even though the HTTP status stays 503, and Codex keys its retry-after backoff on + // `server_is_overloaded` — so the word alone turns a transient refresh into what reads as a + // bad API key and the client stops retrying. The pool path documented this trap and this one + // walked into it anyway, which is why the test below now asserts the classification and not + // just the sentence. const response = formatErrorResponse( 503, "server_busy", "Codex main credential refresh did not complete; retry this request. " - + "If it keeps failing, the main Codex account needs reauthentication.", + + "If it keeps failing, sign in to the main Codex account again.", ); const headers = new Headers(response.headers); headers.set("Retry-After", "1"); diff --git a/tests/codex-integration/codex-account-unusable-reason.test.ts b/tests/codex-integration/codex-account-unusable-reason.test.ts index fbb4c452fe..ca71c4adc9 100644 --- a/tests/codex-integration/codex-account-unusable-reason.test.ts +++ b/tests/codex-integration/codex-account-unusable-reason.test.ts @@ -130,7 +130,25 @@ describe("native main refresh refusal", () => { expect(response.headers.get("Retry-After")).toBe("1"); const message = ((await response.json()) as { error: { message: string } }).error.message; expect(message).toContain("Codex main credential refresh did not complete"); - expect(message).toContain("reauthentication"); + expect(message).toContain("sign in to the main Codex account again"); + }); + + // The regression this pins is not the sentence, it is the CLASSIFICATION the sentence causes. + // `classifyError` runs `isAuthenticationMessage` before the `status === 503` arm, and that check + // is status-blind on the bare substring "authentication" -- which "reauthentication" contains. + // A retryable refusal that says that word is served as `authentication_error` / + // `invalid_api_key` while still returning 503, and Codex keys its retry-after backoff on + // `server_is_overloaded`, so the client reads a transient refresh as a bad API key and stops + // retrying. The previous version of the test above asserted only the status and the word, which + // is exactly why the reclassification shipped unnoticed. + test("the retryable refusal is served as an overload, not as a bad key", async () => { + const response = nativeMainRefreshFailureResponse(new MainAccountTokenRefreshError("transient")); + const error = ((await response.json()) as { error: { type: string; code: string; message: string } }).error; + expect(response.status).toBe(503); + expect(error.type).toBe("server_error"); + expect(error.code).toBe("server_is_overloaded"); + // Load-bearing: the substring, not the phrasing, is what reclassifies the body. + expect(error.message.toLowerCase()).not.toContain("authentication"); }); test("a terminal reauth failure still refuses with 401 rather than a retry promise", async () => {