Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/server/responses/codex-auth-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
20 changes: 19 additions & 1 deletion tests/codex-integration/codex-account-unusable-reason.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading