From 81b74d6fa1c577216bed630ff2c327505c22582d Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 24 Aug 2026 00:27:44 +0900 Subject: [PATCH] refactor: centralize Codex auth error responses --- src/server/responses/codex-auth-error.ts | 55 +++++++++ src/server/responses/compact.ts | 30 +---- src/server/responses/core.ts | 39 +----- structure/01_runtime.md | 2 + tests/responses-compaction-routing.test.ts | 136 +++++++++++++++++++++ 5 files changed, 205 insertions(+), 57 deletions(-) create mode 100644 src/server/responses/codex-auth-error.ts diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts new file mode 100644 index 0000000000..cb7ce8609c --- /dev/null +++ b/src/server/responses/codex-auth-error.ts @@ -0,0 +1,55 @@ +import { formatErrorResponse } from "../../bridge"; +import { + CodexAccountCooldownError, + codexMainProfileDrainingResponse, + cooldownErrorResponse, + CodexAuthContextError, + CodexDirectAuthenticationError, + CodexMainProfileDrainingError, + CodexMainSubstitutionUnavailableError, + CodexPoolAuthenticationError, + CodexThreadAffinityExpiredError, +} from "../../codex/auth-context"; + +export interface CodexAuthContextErrorResponseOptions { + accountSelector?: string; + now: number; +} + +/** Shared HTTP contract for Codex auth-context failures on Responses surfaces. */ +export function mapCodexAuthContextErrorToResponse( + error: unknown, + options: CodexAuthContextErrorResponseOptions, +): Response | undefined { + if (error instanceof CodexAccountCooldownError) { + return cooldownErrorResponse(error, options.now, options.accountSelector); + } + if (error instanceof CodexMainProfileDrainingError) { + return codexMainProfileDrainingResponse(); + } + if (error instanceof CodexThreadAffinityExpiredError) { + return formatErrorResponse( + 409, + "invalid_request_error", + "Codex thread account affinity expired; start a new session", + ); + } + if (error instanceof CodexAuthContextError) { + return formatErrorResponse( + 401, + "authentication_error", + "Selected Codex account needs reauthentication", + ); + } + if (error instanceof CodexPoolAuthenticationError || error instanceof CodexDirectAuthenticationError) { + return formatErrorResponse(401, "authentication_error", error.message); + } + if (error instanceof CodexMainSubstitutionUnavailableError) { + return formatErrorResponse( + 401, + "authentication_error", + "No usable Codex main credential to serve this request", + ); + } + return undefined; +} diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index 5a9558277f..4416173312 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -42,17 +42,9 @@ import { describeImagesInPlace, planVisionSidecar, shouldResolveOpenAiVisionSide import { createAdapterEventQueue, preflightAdapterEvents } from "../../adapters/run-turn-queue"; import { applyCodexAuthContextToProvider, - CodexAccountCooldownError, - codexMainProfileDrainingResponse, - cooldownErrorResponse, - CodexAuthContextError, - CodexDirectAuthenticationError, CodexMainProfileDrainingError, - CodexPoolAuthenticationError, - CodexThreadAffinityExpiredError, headersForCodexAuthContext, materializeCodexUpstreamAuth, - CodexMainSubstitutionUnavailableError, isCodexAuthContextUsable, resolveCodexAuthContext, codexProbeLeaseId, @@ -134,6 +126,7 @@ import { usesCodexForwardPoolAuth, } from "./core"; import { fetchWithHeaderTimeout, providerFetch, safeHostLabel, safeOriginLabel } from "./fetch-helpers"; +import { mapCodexAuthContextErrorToResponse } from "./codex-auth-error"; export const COMPACT_RESPONSE_MAX_BYTES = 32 * 1024 * 1024; @@ -395,22 +388,11 @@ export async function handleResponsesCompact( } } } catch (err) { - if (err instanceof CodexAccountCooldownError) { - return cooldownErrorResponse(err, Date.now(), route.codexAccountNamespace); - } - if (err instanceof CodexMainProfileDrainingError) return codexMainProfileDrainingResponse(); - if (err instanceof CodexThreadAffinityExpiredError) { - return formatErrorResponse(409, "invalid_request_error", "Codex thread account affinity expired; start a new session"); - } - if (err instanceof CodexAuthContextError) { - return formatErrorResponse(401, "authentication_error", "Selected Codex account needs reauthentication"); - } - if (err instanceof CodexMainSubstitutionUnavailableError) { - return formatErrorResponse(401, "authentication_error", "No usable Codex main credential to serve this request"); - } - if (err instanceof CodexPoolAuthenticationError || err instanceof CodexDirectAuthenticationError) { - return formatErrorResponse(401, "authentication_error", err.message); - } + const response = mapCodexAuthContextErrorToResponse(err, { + accountSelector: route.codexAccountNamespace, + now: Date.now(), + }); + if (response) return response; throw err; } const base = isCanonicalOpenAiForwardProvider(compactProvider) diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index a8f9f70a3a..67005be579 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -115,16 +115,12 @@ import { applyCodexAuthContextToProvider, codexPoolAffinityKey, CodexAccountCooldownError, - codexMainProfileDrainingResponse, - cooldownErrorResponse, CodexAuthContextError, - CodexDirectAuthenticationError, CodexMainProfileDrainingError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, headersForCodexAuthContext, materializeCodexUpstreamAuth, - CodexMainSubstitutionUnavailableError, isCodexAuthContextUsable, resolveCodexAuthContext, codexProbeLeaseId, @@ -274,6 +270,7 @@ import { createResponsesModelPayloadRewrite, rewriteResponsesModelJson } from ". import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration"; +import { mapCodexAuthContextErrorToResponse } from "./codex-auth-error"; import { hasUnreadableEncryptedAgentTask, looksLikeBackendCiphertext, sanitizeEncryptedContentInPlace } from "./encrypted-payload"; import { fetchWithHeaderTimeout, providerFetch, safeHostLabel, safeOriginLabel } from "./fetch-helpers"; import { classifyTransportFailureKind, transportErrorCode } from "../../lib/upstream-reachability"; @@ -1541,44 +1538,20 @@ async function resolveResponsesCodexAuth( substituteMainCredential, }; } catch (err) { - if (err instanceof CodexAccountCooldownError) { - return { ok: false, response: cooldownErrorResponse(err, Date.now(), route.codexAccountNamespace) }; - } - if (err instanceof CodexMainProfileDrainingError) { - return { ok: false, response: codexMainProfileDrainingResponse() }; - } - if (err instanceof CodexThreadAffinityExpiredError) { - return { - ok: false, - response: formatErrorResponse(409, "invalid_request_error", "Codex thread account affinity expired; start a new session"), - }; - } if (err instanceof CodexAuthContextError) { const safeAccountLabel = route.codexAccountNamespace ? `${route.providerName}-${route.codexAccountNamespace}` : formatCodexProviderForLog(route.providerName, err.accountId, config); console.error(`[codex-auth] Pool account ${safeAccountLabel} token failed; reauthentication required`); - return { - ok: false, - response: formatErrorResponse(401, "authentication_error", "Selected Codex account needs reauthentication"), - }; - } - if (err instanceof CodexPoolAuthenticationError) { - return { ok: false, response: formatErrorResponse(401, "authentication_error", err.message) }; - } - if (err instanceof CodexDirectAuthenticationError) { - return { ok: false, response: formatErrorResponse(401, "authentication_error", err.message) }; } if (err instanceof ForwardAdmissionCredentialError) { return { ok: false, response: formatErrorResponse(401, "authentication_error", err.message) }; } - if (err instanceof CodexMainSubstitutionUnavailableError) { - // Fail BEFORE any upstream I/O. The alternative is forwarding the admission secret. - return { - ok: false, - response: formatErrorResponse(401, "authentication_error", "No usable Codex main credential to serve this request"), - }; - } + const response = mapCodexAuthContextErrorToResponse(err, { + accountSelector: route.codexAccountNamespace, + now: Date.now(), + }); + if (response) return { ok: false, response }; throw err; } } diff --git a/structure/01_runtime.md b/structure/01_runtime.md index b839e0ccb5..12995b8c93 100644 --- a/structure/01_runtime.md +++ b/structure/01_runtime.md @@ -35,6 +35,8 @@ there. Feature code is grouped by responsibility: `src/server/` is split by responsibility: `index.ts` owns the listener and route ordering; `responses.ts` owns Responses handling and compaction; `images.ts` owns the standalone Images relay; +`responses/codex-auth-error.ts` owns the shared Responses/compact Codex auth-context HTTP mapping, +while account selection, credential materialization, logging, and transport stay in their existing handlers; `management-api.ts` owns `/api/*`; `lifecycle.ts`, `request-log.ts`, `relay.ts` (incl. the shared `createSseInspector` SSE inspection factory), `relay-eager.ts` (#314 gated eager bounded passthrough relay), `memory-watchdog.ts` diff --git a/tests/responses-compaction-routing.test.ts b/tests/responses-compaction-routing.test.ts index 3e2cb72528..6ec6473503 100644 --- a/tests/responses-compaction-routing.test.ts +++ b/tests/responses-compaction-routing.test.ts @@ -171,6 +171,142 @@ describe("supportsNativeResponsesCompactEndpoint (#422)", () => { }); }); +describe("Codex auth-context error parity (#2392)", () => { + const cases: Array<{ + label: string; + createError: () => Error; + status: number; + retryAfter?: string; + regularLog: boolean; + }> = [ + { + label: "account cooldown", + createError: () => new authContextModule.CodexAccountCooldownError( + "sensitive-account-id", + Date.now() + 120_000, + "retry-after", + ), + status: 429, + regularLog: false, + }, + { + label: "native-main drain", + createError: () => new authContextModule.CodexMainProfileDrainingError(), + status: 503, + retryAfter: "1", + regularLog: false, + }, + { + label: "expired thread affinity", + createError: () => new authContextModule.CodexThreadAffinityExpiredError("sensitive-account-id"), + status: 409, + regularLog: false, + }, + { + label: "pool credential refresh failure", + createError: () => new authContextModule.CodexAuthContextError( + "sensitive-account-id", + new Error("private refresh detail"), + ), + status: 401, + regularLog: true, + }, + { + label: "pool authentication failure", + createError: () => new authContextModule.CodexPoolAuthenticationError("Pool credential is unavailable"), + status: 401, + regularLog: false, + }, + { + label: "direct authentication failure", + createError: () => new authContextModule.CodexDirectAuthenticationError(), + status: 401, + regularLog: false, + }, + { + label: "main credential substitution failure", + createError: () => new authContextModule.CodexMainSubstitutionUnavailableError(), + status: 401, + regularLog: false, + }, + ]; + + function regularAuthRequest(): Request { + return compactionRequest({ model: "gpt-5.6-sol", input: "hello", stream: false }); + } + + function compactAuthRequest(): Request { + return compactionRequest(baseCompactionBody({ model: "gpt-5.6-sol" })); + } + + test.each(cases)("maps $label identically on regular and compact Responses", async testCase => { + let upstreamCalls = 0; + globalThis.fetch = (async () => { + upstreamCalls += 1; + return jsonResponse(completedPayload("unexpected upstream response")); + }) as typeof fetch; + const error = testCase.createError(); + const authSpy = spyOn(authContextModule, "resolveCodexAuthContext").mockRejectedValue(error); + const errorLog = spyOn(console, "error").mockImplementation(() => {}); + try { + const regular = await handleResponses(regularAuthRequest(), nativePoolConfig(), { model: "", provider: "" }); + const regularLogCount = errorLog.mock.calls.length; + const compact = await handleResponsesCompact(compactAuthRequest(), nativePoolConfig(), { model: "", provider: "" }); + + expect(regular.status).toBe(testCase.status); + expect(compact.status).toBe(testCase.status); + expect(regular.headers.get("content-type")).toBe("application/json"); + expect(compact.headers.get("content-type")).toBe("application/json"); + expect(await regular.text()).toBe(await compact.text()); + expect(compact.headers.get("retry-after")).toBe(regular.headers.get("retry-after")); + if (testCase.retryAfter) expect(regular.headers.get("retry-after")).toBe(testCase.retryAfter); + if (testCase.label === "account cooldown") expect(regular.headers.get("retry-after")).not.toBeNull(); + if (testCase.label === "main credential substitution failure") expect(upstreamCalls).toBe(0); + + expect(regularLogCount).toBe(testCase.regularLog ? 1 : 0); + expect(errorLog.mock.calls.length).toBe(regularLogCount); + if (testCase.regularLog) { + const line = errorLog.mock.calls[0]!.join(" "); + expect(line).toContain("[codex-auth] Pool account openai token failed; reauthentication required"); + expect(line).not.toContain("sensitive-account-id"); + expect(line).not.toContain("private refresh detail"); + } + } finally { + errorLog.mockRestore(); + authSpy.mockRestore(); + } + }); + + test("unknown auth-resolution errors reject on both handlers instead of being mapped", async () => { + let upstreamCalls = 0; + globalThis.fetch = (async () => { + upstreamCalls += 1; + return jsonResponse(completedPayload("unexpected upstream response")); + }) as typeof fetch; + const authSpy = spyOn(authContextModule, "resolveCodexAuthContext"); + try { + const regularError = new Error("unmapped regular auth failure"); + authSpy.mockRejectedValueOnce(regularError); + await expect(handleResponses( + regularAuthRequest(), + nativePoolConfig(), + { model: "", provider: "" }, + )).rejects.toBe(regularError); + + const compactError = new Error("unmapped compact auth failure"); + authSpy.mockRejectedValueOnce(compactError); + await expect(handleResponsesCompact( + compactAuthRequest(), + nativePoolConfig(), + { model: "", provider: "" }, + )).rejects.toBe(compactError); + expect(upstreamCalls).toBe(0); + } finally { + authSpy.mockRestore(); + } + }); +}); + describe("native compact usage reporting", () => { test("the buffered upstream body fills the request log usage and stays intact for the client", async () => { const config = {