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 () => {