From e948274dcddee88e7a874a9189d4242e0d81b492 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:35:21 +0900 Subject: [PATCH 1/3] fix(codex): retain cooldown after account retry --- src/server/responses/core.ts | 50 ++++++++++------ tests/server-auth.test.ts | 108 +++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 18 deletions(-) diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 01a58b794..248dc5b53 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -435,7 +435,11 @@ async function retryCodexPoolOnAlternateAccount( firstAuthCtx.writerGeneration, ); } - if (!shouldDeferCodexResetDerivedCooldown(firstResponse, options.deferCodexResetDerivedCooldown)) { + const deferFirstOutcome = shouldDeferCodexResetDerivedCooldown( + firstResponse, + options.deferCodexResetDerivedCooldown, + ); + const recordFirstOutcome = (): void => { recordCodexUpstreamOutcome(config, firstAuthCtx.accountId, outcomeStatus, { ...quotaMeta, threadId: req.headers.get("x-codex-parent-thread-id"), @@ -446,8 +450,10 @@ async function retryCodexPoolOnAlternateAccount( // Retry already advanced the RR ring via excludeAccountId — reuse for promotion. ...(retryAuthCtx.accountId ? { promoteAccountId: retryAuthCtx.accountId } : {}), }); - } - + }; + // Only a combo reset-derived outcome is deferred. Retry-After, defaults, and + // ordinary requests must block the first account before the alternate send. + if (!deferFirstOutcome) recordFirstOutcome(); const retryHeaders = headersForCodexAuthContext(req.headers, retryAuthCtx); const retryProvider = applyCodexAuthContextToProvider( stripCodexRuntimeProviderFields(route.provider), @@ -474,8 +480,9 @@ async function retryCodexPoolOnAlternateAccount( ); noteAttemptSend(logCtx.activeAttempt, passthroughEstimate); + let upstreamResponse: Response; try { - const upstreamResponse = await fetchWithHeaderTimeout( + upstreamResponse = await fetchWithHeaderTimeout( request.url, { method: request.method, @@ -490,26 +497,33 @@ async function retryCodexPoolOnAlternateAccount( // dead-host rejection after the credential was seen (#914). route.provider.authMode === "forward", ); - // A real HTTP response proves the host was reached (#914). - const retryHostKey = upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url)); - if (normalizeUpstreamHostCircuitThreshold(config.upstreamHostCircuitThreshold) > 0) { - resetUpstreamHostHealth(retryHostKey, null); - } else { - resetUpstreamHostHealth(retryHostKey); - } - return { - kind: "retried", - authCtx: retryAuthCtx, - request, - upstreamResponse, - selectedForwardHeaders: retryHeaders, - }; } catch (error) { // Attribute the transport failure to the alternate account (already selected). return { kind: "transport", error, authCtx: retryAuthCtx }; } finally { request.releaseBodyObservation?.(); } + // A real HTTP response proves the host was reached (#914). + const retryHostKey = upstreamHostHealthKey(route.providerName, safeOriginLabel(request.url)); + if (normalizeUpstreamHostCircuitThreshold(config.upstreamHostCircuitThreshold) > 0) { + resetUpstreamHostHealth(retryHostKey, null); + } else { + resetUpstreamHostHealth(retryHostKey); + } + if (deferFirstOutcome && upstreamResponse.ok) { + // Deferral keeps the first account eligible for a later combo model while an + // alternate attempt is still fallible. Commit its quota outcome only once the + // alternate account returns a successful HTTP response; otherwise the combo may + // still need the first account for its next target. + recordFirstOutcome(); + } + return { + kind: "retried", + authCtx: retryAuthCtx, + request, + upstreamResponse, + selectedForwardHeaders: retryHeaders, + }; } diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 2d9a8929b..80099b98f 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -13,6 +13,7 @@ import { CODEX_THREAD_AFFINITY_IDLE_TTL_MS, clearCodexUpstreamHealth, clearThreadAccountMap, + getCodexQuotaHealthSnapshot, getCodexUpstreamHealth, isCodexAccountSoftAvoided, recordCodexUpstreamOutcome, @@ -194,6 +195,7 @@ async function startPoolRetryHarness( pausedAccountIds?: string[]; reauthAccountIds?: string[]; omitCredentialAccountIds?: string[]; + combos?: OcxConfig["combos"]; } = {}, ): Promise { await removeTestDirBestEffort(TEST_DIR); @@ -250,6 +252,7 @@ async function startPoolRetryHarness( ...(options.visionSidecarModel ? { visionSidecar: { model: options.visionSidecarModel } } : {}), ...(options.websockets ? { websockets: true } : {}), ...(options.streamMode ? { streamMode: options.streamMode } : {}), + ...(options.combos ? { combos: options.combos } : {}), } as OcxConfig; saveConfig(config); if (!options.omitCredentialAccountIds?.includes("pool-a")) { @@ -2318,6 +2321,111 @@ describe("server local API auth", () => { } }); + test("#584: Retry-After cools the first account even when its account retry fails", async () => { + const harness = await startPoolRetryHarness(accountId => accountId === "acct-pool-a" + ? new Response(JSON.stringify({ error: { message: "rate limited" } }), { + status: 429, + headers: { "content-type": "application/json", "retry-after": "60" }, + }) + : new Response(JSON.stringify({ error: { message: "upstream unavailable" } }), { + status: 503, + headers: { "content-type": "application/json" }, + })); + try { + const response = await harness.request(); + expect(response.status).toBe(503); + expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b"]); + expect(getCodexUpstreamHealth("pool-a")).toMatchObject({ + cooldownUntil: expect.any(Number), + cooldownSource: "retry-after", + }); + } finally { + await stopPoolRetryHarness(harness); + } + }, { timeout: SERVER_BUDGET_MS }); + + test("combo reset deferral still cools the first account when its account retry succeeds", async () => { + const harness = await startPoolRetryHarness( + accountId => accountId === "acct-pool-a" + ? new Response(JSON.stringify({ error: { message: "rate limited" } }), { + status: 429, + headers: { + "content-type": "application/json", + "x-codex-primary-reset-at": String(Math.floor(Date.now() / 1000) + 3600), + }, + }) + : Response.json({ id: "combo-account-retry-success", status: "completed", output: [] }), + { + combos: { + quota: { + strategy: "failover", + targets: [ + { provider: "openai", model: POOL_RETRY_MODEL }, + { provider: "openai", model: `${POOL_RETRY_MODEL}-fallback` }, + ], + }, + }, + }, + ); + try { + const response = await harness.request({ model: "combo/quota" }); + expect(response.status).toBe(200); + expect((await response.json() as { id: string }).id).toBe("combo-account-retry-success"); + expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b"]); + expect(getCodexQuotaHealthSnapshot("pool-a", "shared")).toMatchObject({ + cooldownUntil: expect.any(Number), + cooldownSource: "reset-derived", + quotaScope: "shared", + }); + expect(loadConfig().activeCodexAccountId).toBe("pool-b"); + } finally { + await stopPoolRetryHarness(harness); + } + }, { timeout: SERVER_BUDGET_MS }); + + test("combo reset deferral preserves the first account when its account retry also fails", async () => { + const harness = await startPoolRetryHarness( + async (accountId, request) => { + const body = await request.json() as { model?: string }; + if (body.model === `${POOL_RETRY_MODEL}-fallback`) { + return Response.json({ id: "combo-later-model-success", status: "completed", output: [] }); + } + if (accountId === "acct-pool-a") { + return new Response(JSON.stringify({ error: { message: "rate limited" } }), { + status: 429, + headers: { + "content-type": "application/json", + "x-codex-primary-reset-at": String(Math.floor(Date.now() / 1000) + 3600), + }, + }); + } + return new Response(JSON.stringify({ error: { message: "retry later" } }), { + status: 429, + headers: { "content-type": "application/json", "retry-after": "60" }, + }); + }, + { + combos: { + quota: { + strategy: "failover", + targets: [ + { provider: "openai", model: POOL_RETRY_MODEL }, + { provider: "openai", model: `${POOL_RETRY_MODEL}-fallback` }, + ], + }, + }, + }, + ); + try { + const response = await harness.request({ model: "combo/quota" }); + expect(response.status).toBe(200); + expect((await response.json() as { id: string }).id).toBe("combo-later-model-success"); + expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b", "acct-pool-a"]); + } finally { + await stopPoolRetryHarness(harness); + } + }, { timeout: SERVER_BUDGET_MS }); + test("#584: pre-stream 429 with one eligible account preserves the original 429", async () => { const body = JSON.stringify({ error: { message: "rate limited" } }); const harness = await startPoolRetryHarness( From fd25a2ba50156fcc2bdedeb0bd60fc7e8894e881 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:34:26 +0900 Subject: [PATCH 2/3] test(codex): require future retry cooldown --- tests/server-auth.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 80099b98f..95f22e359 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -2335,10 +2335,11 @@ describe("server local API auth", () => { const response = await harness.request(); expect(response.status).toBe(503); expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b"]); - expect(getCodexUpstreamHealth("pool-a")).toMatchObject({ - cooldownUntil: expect.any(Number), + const health = getCodexUpstreamHealth("pool-a"); + expect(health).toMatchObject({ cooldownSource: "retry-after", }); + expect(health?.cooldownUntil).toBeGreaterThan(Date.now()); } finally { await stopPoolRetryHarness(harness); } From d9119fc4085d31761b0e4ab9e45e75dea96c353d Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:08:38 +0900 Subject: [PATCH 3/3] test(codex): assert deferred quota health directly --- tests/server-auth.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/server-auth.test.ts b/tests/server-auth.test.ts index 95f22e359..4e1d9e9b4 100644 --- a/tests/server-auth.test.ts +++ b/tests/server-auth.test.ts @@ -2422,6 +2422,7 @@ describe("server local API auth", () => { expect(response.status).toBe(200); expect((await response.json() as { id: string }).id).toBe("combo-later-model-success"); expect(harness.dispatches).toEqual(["acct-pool-a", "acct-pool-b", "acct-pool-a"]); + expect(getCodexQuotaHealthSnapshot("pool-a", "shared")).toBeNull(); } finally { await stopPoolRetryHarness(harness); }