From 24bba009047646d04e4e93b62069e62d9760f5c5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Tue, 8 Sep 2026 12:52:34 -0700 Subject: [PATCH 1/2] Honor Retry-After header on 429 retry, fix terminal message --- src/agent/retry-policy.test.ts | 61 ++++++++++++++++++-- src/agent/retry-policy.ts | 10 ++++ src/inference-error-message.test.ts | 20 +++++++ src/inference-error-message.ts | 5 ++ src/inference-gateway-error.ts | 8 ++- src/provider/codex-responses-adapter.test.ts | 7 +++ src/provider/codex-responses-adapter.ts | 22 +++++++ src/provider/grok-responses-adapter.ts | 2 + src/provider/openai-responses-adapter.ts | 2 + src/tui/stream-event-map.test.ts | 2 +- 10 files changed, 130 insertions(+), 9 deletions(-) diff --git a/src/agent/retry-policy.test.ts b/src/agent/retry-policy.test.ts index 7b0f589fc..7193b9d34 100644 --- a/src/agent/retry-policy.test.ts +++ b/src/agent/retry-policy.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "bun:test"; import type { AdmissionQueue } from "../subagent/admission.js"; -import { createCorbitsRetryPolicy, type CorbitsRetryPolicyOptions } from "./retry-policy.js"; +import { + createCorbitsRetryPolicy, + MAX_BLIND_WAIT_MS, + type CorbitsRetryPolicyOptions, +} from "./retry-policy.js"; const HTML_503 = `503 Service Unavailable Cloudflare`; @@ -104,8 +108,9 @@ describe("createCorbitsRetryPolicy", () => { raw: { error: { message: "Too Many Requests" } }, }, }); - // Remapped to retryable -> default backoff, not abort on moderate Retry-After. - expect(decision).toEqual({ kind: "retry", delayMs: 500 }); + // Remapped to retryable -> paced retry capped at the blind-wait ceiling, + // not abort on moderate Retry-After. + expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); }); test("stamped Codex usage-limit 429 retries as retryable, not long-quota abort", async () => { @@ -120,7 +125,7 @@ describe("createCorbitsRetryPolicy", () => { raw: "You have hit your ChatGPT usage limit", }, }); - expect(decision).toEqual({ kind: "retry", delayMs: 500 }); + expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); }); test("stamped xAI usage/quota body still aborts on long retryAfterMs", async () => { @@ -174,7 +179,7 @@ describe("createCorbitsRetryPolicy", () => { }; expect(await decide(bare429)).toEqual({ kind: "abort" }); current = "xai/thegreataxios"; - expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 500 }); + expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); }); // CL-6910: the harness only surfaces `inference.error` to the director @@ -242,7 +247,7 @@ describe("createCorbitsRetryPolicy", () => { raw: { error: { message: "Too Many Requests" } }, }, }; - expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 500 }); + expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); current = "openai"; expect(await decide(bare429)).toEqual({ kind: "abort" }); }); @@ -298,4 +303,48 @@ describe("createCorbitsRetryPolicy", () => { }); expect(notes).toHaveLength(0); }); + + test("retryable 429 honors Retry-After instead of the fixed 500/1000ms backoff", async () => { + const decide = policy({ providerId: "codex/abk-labs" }); + const situation = (attempt: number) => ({ + attempt, + elapsedMs: 0, + error: { + category: "retryable" as const, + message: "Too Many Requests", + statusCode: 429, + retryAfterMs: 5_000, + }, + }); + expect(await decide(situation(1))).toEqual({ kind: "retry", delayMs: 5_000 }); + expect(await decide(situation(2))).toEqual({ kind: "retry", delayMs: 5_000 }); + expect(await decide(situation(3))).toEqual({ kind: "abort" }); + }); + + test("retryable 429 caps a long Retry-After at the blind-wait ceiling", async () => { + const decide = policy({ providerId: "codex/abk-labs" }); + const decision = await decide({ + attempt: 1, + elapsedMs: 0, + error: { + category: "retryable" as const, + message: "Too Many Requests", + statusCode: 429, + retryAfterMs: 120_000, + }, + }); + expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + }); + + test("retryable 429 without Retry-After keeps the fixed backoff", async () => { + const decide = policy(); + const situation = (attempt: number) => ({ + attempt, + elapsedMs: 0, + error: { category: "retryable" as const, message: "boom", statusCode: 429 }, + }); + expect(await decide(situation(1))).toEqual({ kind: "retry", delayMs: 500 }); + expect(await decide(situation(2))).toEqual({ kind: "retry", delayMs: 1000 }); + expect(await decide(situation(3))).toEqual({ kind: "abort" }); + }); }); diff --git a/src/agent/retry-policy.ts b/src/agent/retry-policy.ts index dd4f08660..c11589c5a 100644 --- a/src/agent/retry-policy.ts +++ b/src/agent/retry-policy.ts @@ -49,6 +49,16 @@ export function createCorbitsRetryPolicy(options?: CorbitsRetryPolicyOptions): R const pauseMs = Math.min(error.retryAfterMs ?? DEFAULT_PRESSURE_PAUSE_MS, MAX_BLIND_WAIT_MS); const provider = withProvider.providerId ?? stampedProviderId ?? "unknown"; admission.notePressure(provider, now() + pauseMs); + // The vendored default retries `retryable` on a fixed 500/1000ms + // schedule and ignores Retry-After. A 429 carries the server's pacing + // instruction: honor it (capped at the blind-wait ceiling like the + // quota path) so a short rate limit waits itself out instead of + // burning all three attempts in ~1.5s and aborting. The 3-attempt cap + // mirrors the vendored MAX_ATTEMPTS in retry-policy.ts. + if (error.retryAfterMs !== undefined) { + if (situation.attempt >= 3) return { kind: "abort" }; + return { kind: "retry", delayMs: pauseMs }; + } } if ( error.category === "quota_exhausted" && diff --git a/src/inference-error-message.test.ts b/src/inference-error-message.test.ts index 32746e59a..9f76174af 100644 --- a/src/inference-error-message.test.ts +++ b/src/inference-error-message.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; +import { normalizeInferenceErrorForTerminal } from "./inference-gateway-error.js"; import { inferenceErrorMessage, terminalProviderFailureMessage, @@ -211,6 +212,25 @@ describe("terminalProviderFailureMessage", () => { ); }); + test("terminal Codex short-429 failure does not claim to still be retrying", () => { + const normalized = normalizeInferenceErrorForTerminal( + { category: "quota_exhausted", message: "Too Many Requests", statusCode: 429 }, + "codex/default", + ); + const message = terminalProviderFailureMessage("codex/default", normalized); + expect(message.toLowerCase()).toMatch(/rate limit/); + expect(message.toLowerCase()).not.toContain("retrying"); + }); + + test("retryable 429 guidance asks the operator to wait before trying again", () => { + const message = terminalProviderFailureMessage("codex/default", { + category: "retryable", + message: "Rate limited", + statusCode: 429, + }); + expect(message).toContain("Wait a moment and try again."); + }); + test("uses a safe label when the provider id contains only control sequences", () => { const message = terminalProviderFailureMessage("\u001b[31m\u001b[0m", { category: "fatal", diff --git a/src/inference-error-message.ts b/src/inference-error-message.ts index 44bd0c159..44232dd3e 100644 --- a/src/inference-error-message.ts +++ b/src/inference-error-message.ts @@ -130,6 +130,11 @@ export function terminalProviderFailureMessage( function terminalProviderFailureGuidance(error: InferenceErrorLike, category: string): string { if (category === "credential_failure") return CREDENTIAL_FAILURE_USER_MESSAGE; if (category === "context_overflow") return "Try /clear to start fresh."; + // A 429 that survived the harness's paced retries is a wait-it-out rate + // limit, not a generic flake: say so instead of the bare "Try again." + if (category === "retryable" && error.statusCode === 429) { + return "Wait a moment and try again."; + } if ( category === "retryable" || (error.statusCode !== undefined && error.statusCode >= 500 && error.statusCode <= 599) diff --git a/src/inference-gateway-error.ts b/src/inference-gateway-error.ts index 818432ab4..8fbfdddfe 100644 --- a/src/inference-gateway-error.ts +++ b/src/inference-gateway-error.ts @@ -49,8 +49,12 @@ const GATEWAY_OVERLOAD_TEXT_MARKERS = [ /** User-visible line while the harness retries a transient gateway overload. */ export const GATEWAY_OVERLOAD_USER_MESSAGE = "Inference gateway overloaded — retrying…"; -/** User-visible line while the harness retries a short known-provider HTTP 429. */ -export const RATE_LIMIT_USER_MESSAGE = "Rate limited — retrying…"; +/** + * User-visible line for a short known-provider HTTP 429. Worded without + * "retrying": this message also surfaces terminally after the harness has + * exhausted its retries, where claiming an ongoing retry is wrong. + */ +export const RATE_LIMIT_USER_MESSAGE = "Rate limited"; /** Body markers that mean a real usage/quota window, not a short rate limit. */ const XAI_QUOTA_BODY_MARKERS = [ diff --git a/src/provider/codex-responses-adapter.test.ts b/src/provider/codex-responses-adapter.test.ts index 41ba27c58..6b7728c6b 100644 --- a/src/provider/codex-responses-adapter.test.ts +++ b/src/provider/codex-responses-adapter.test.ts @@ -63,6 +63,13 @@ describe("createCodexResponsesAdapter", () => { const adapter = createCodexResponsesAdapter(source); expect(adapter.isStreamTerminal).toBe(isResponsesStreamTerminal); }); + + test("extracts Retry-After pacing from response headers", () => { + const adapter = createCodexResponsesAdapter(source); + expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000); + expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500); + expect(adapter.extractRetryAfterMs?.(new Headers({}))).toBeUndefined(); + }); }); describe("createCodexResponsesAdapter usage parsing", () => { diff --git a/src/provider/codex-responses-adapter.ts b/src/provider/codex-responses-adapter.ts index 1c5f40aaa..c2a1a4112 100644 --- a/src/provider/codex-responses-adapter.ts +++ b/src/provider/codex-responses-adapter.ts @@ -635,6 +635,27 @@ export function isResponsesStreamTerminal(sseData: string): boolean { return typeof eventType === "string" && RESPONSES_TERMINAL_EVENTS.has(eventType); } +// Responses backends (Codex, Grok, OpenAI) signal 429 pacing with the same +// `retry-after` / `retry-after-ms` headers the Chat Completions adapter +// already reads. The shared Responses adapters never extracted them, so +// every 429 arrived with retryAfterMs undefined and the retry policy fell +// back to blind fixed backoff instead of waiting out the server's window. +export function extractResponsesRetryAfterMs(headers: Headers): number | undefined { + const retryMs = headers.get("retry-after-ms"); + if (retryMs !== null) { + const ms = Number(retryMs); + if (Number.isFinite(ms) && ms > 0) return Math.ceil(ms); + } + const raw = headers.get("retry-after"); + if (raw !== null) { + const seconds = Number(raw); + if (Number.isFinite(seconds) && seconds > 0) { + return Math.ceil(seconds * 1000); + } + } + return undefined; +} + export function createCodexResponsesAdapter(source: LastCycleSource): ProviderAdapter { // Re-created per request in buildRequest, not just once here — otherwise // block indices accumulate across every request the adapter instance ever @@ -648,5 +669,6 @@ export function createCodexResponsesAdapter(source: LastCycleSource): ProviderAd parseResponse: (sseData) => parseResponse(sseData, indexer, source), parseJSONResponse, isStreamTerminal: isResponsesStreamTerminal, + extractRetryAfterMs: extractResponsesRetryAfterMs, }; } diff --git a/src/provider/grok-responses-adapter.ts b/src/provider/grok-responses-adapter.ts index 2cca85ae8..365d19066 100644 --- a/src/provider/grok-responses-adapter.ts +++ b/src/provider/grok-responses-adapter.ts @@ -19,6 +19,7 @@ import { import { RESPONSES_TOOL_NAME_LIMIT, createResponsesBlockIndexer, + extractResponsesRetryAfterMs, parseJSONResponse, parseResponse, signatureForModel, @@ -251,5 +252,6 @@ export function createGrokResponsesAdapter(source: LastCycleSource): ProviderAda }, parseResponse: (sseData) => parseResponse(sseData, indexer, source, GROK_RESPONSES_PROVIDER), parseJSONResponse, + extractRetryAfterMs: extractResponsesRetryAfterMs, }; } diff --git a/src/provider/openai-responses-adapter.ts b/src/provider/openai-responses-adapter.ts index 21e29202b..84497c6e0 100644 --- a/src/provider/openai-responses-adapter.ts +++ b/src/provider/openai-responses-adapter.ts @@ -13,6 +13,7 @@ import type { import { RESPONSES_TOOL_NAME_LIMIT, createResponsesBlockIndexer, + extractResponsesRetryAfterMs, isResponsesStreamTerminal, parseJSONResponse, parseResponse, @@ -233,5 +234,6 @@ export function createOpenAIResponsesAdapter(source: LastCycleSource): ProviderA parseResponse: (sseData) => parseResponse(sseData, indexer, source, OPENAI_RESPONSES_PROVIDER), parseJSONResponse, isStreamTerminal: isResponsesStreamTerminal, + extractRetryAfterMs: extractResponsesRetryAfterMs, }; } diff --git a/src/tui/stream-event-map.test.ts b/src/tui/stream-event-map.test.ts index 9538cc41e..d544c579d 100644 --- a/src/tui/stream-event-map.test.ts +++ b/src/tui/stream-event-map.test.ts @@ -421,7 +421,7 @@ describe("inference.error text", () => { mapProductionEvent({ type: "connector.reply", data: { content: "generic reply" } }, ctx), ).toContainEqual({ type: "assistant", - text: "Work Provider failed (retryable): Rate limited — retrying…. Try again.", + text: "Work Provider failed (retryable): Rate limited. Wait a moment and try again.", }); }); From 39e88fa818aad96121bb21f8ccd4f6ae000f8d60 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 9 Sep 2026 10:59:39 -0700 Subject: [PATCH 2/2] Honor full Retry-After on remapped 429 retries Capping the wait at the blind-wait ceiling retried while the server was still closed. Attempt abort now follows defaultPolicy so the cap cannot drift from the vendored maximum. --- src/agent/retry-policy.test.ts | 37 +++++++++++++-------- src/agent/retry-policy.ts | 19 +++++++---- src/provider/grok-responses-adapter.test.ts | 7 ++++ tests/unit/openai-responses-adapter.test.ts | 9 +++++ 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/agent/retry-policy.test.ts b/src/agent/retry-policy.test.ts index 7193b9d34..0feaed8d4 100644 --- a/src/agent/retry-policy.test.ts +++ b/src/agent/retry-policy.test.ts @@ -1,10 +1,6 @@ import { describe, expect, test } from "bun:test"; import type { AdmissionQueue } from "../subagent/admission.js"; -import { - createCorbitsRetryPolicy, - MAX_BLIND_WAIT_MS, - type CorbitsRetryPolicyOptions, -} from "./retry-policy.js"; +import { createCorbitsRetryPolicy, type CorbitsRetryPolicyOptions } from "./retry-policy.js"; const HTML_503 = `503 Service Unavailable Cloudflare`; @@ -108,9 +104,9 @@ describe("createCorbitsRetryPolicy", () => { raw: { error: { message: "Too Many Requests" } }, }, }); - // Remapped to retryable -> paced retry capped at the blind-wait ceiling, - // not abort on moderate Retry-After. - expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + // Remapped to retryable -> paced retry honors the server's Retry-After, + // not abort on moderate Retry-After and not a capped 30s wait. + expect(decision).toEqual({ kind: "retry", delayMs: 45_000 }); }); test("stamped Codex usage-limit 429 retries as retryable, not long-quota abort", async () => { @@ -125,7 +121,7 @@ describe("createCorbitsRetryPolicy", () => { raw: "You have hit your ChatGPT usage limit", }, }); - expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + expect(decision).toEqual({ kind: "retry", delayMs: 45_000 }); }); test("stamped xAI usage/quota body still aborts on long retryAfterMs", async () => { @@ -179,7 +175,7 @@ describe("createCorbitsRetryPolicy", () => { }; expect(await decide(bare429)).toEqual({ kind: "abort" }); current = "xai/thegreataxios"; - expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 45_000 }); }); // CL-6910: the harness only surfaces `inference.error` to the director @@ -247,7 +243,7 @@ describe("createCorbitsRetryPolicy", () => { raw: { error: { message: "Too Many Requests" } }, }, }; - expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 45_000 }); current = "openai"; expect(await decide(bare429)).toEqual({ kind: "abort" }); }); @@ -321,7 +317,7 @@ describe("createCorbitsRetryPolicy", () => { expect(await decide(situation(3))).toEqual({ kind: "abort" }); }); - test("retryable 429 caps a long Retry-After at the blind-wait ceiling", async () => { + test("retryable 429 honors a Retry-After above the blind-wait ceiling", async () => { const decide = policy({ providerId: "codex/abk-labs" }); const decision = await decide({ attempt: 1, @@ -333,7 +329,22 @@ describe("createCorbitsRetryPolicy", () => { retryAfterMs: 120_000, }, }); - expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS }); + expect(decision).toEqual({ kind: "retry", delayMs: 120_000 }); + }); + + test("retryable 429 with a day-long Retry-After aborts instead of hanging", async () => { + const decide = policy({ providerId: "codex/abk-labs" }); + const decision = await decide({ + attempt: 1, + elapsedMs: 0, + error: { + category: "retryable" as const, + message: "Too Many Requests", + statusCode: 429, + retryAfterMs: 86_400_000, + }, + }); + expect(decision).toEqual({ kind: "abort" }); }); test("retryable 429 without Retry-After keeps the fixed backoff", async () => { diff --git a/src/agent/retry-policy.ts b/src/agent/retry-policy.ts index c11589c5a..093126542 100644 --- a/src/agent/retry-policy.ts +++ b/src/agent/retry-policy.ts @@ -13,6 +13,7 @@ import { getProcessAdmissionQueue, type AdmissionQueue } from "../subagent/admis // so the user can switch providers or decide when to retry manually. export const MAX_BLIND_WAIT_MS = 30_000; const DEFAULT_PRESSURE_PAUSE_MS = 1_000; +const RATE_LIMIT_HANG_MS = 86_400_000; export interface CorbitsRetryPolicyOptions { /** @@ -51,13 +52,19 @@ export function createCorbitsRetryPolicy(options?: CorbitsRetryPolicyOptions): R admission.notePressure(provider, now() + pauseMs); // The vendored default retries `retryable` on a fixed 500/1000ms // schedule and ignores Retry-After. A 429 carries the server's pacing - // instruction: honor it (capped at the blind-wait ceiling like the - // quota path) so a short rate limit waits itself out instead of - // burning all three attempts in ~1.5s and aborting. The 3-attempt cap - // mirrors the vendored MAX_ATTEMPTS in retry-policy.ts. + // instruction: honor the full window. Capping at MAX_BLIND_WAIT_MS and + // retrying early burns the attempt budget while the server is still + // closed (the 45s xAI/Codex fixtures). Days-long Retry-After is a hang + // — abort rather than park the session. Attempt abort comes from + // defaultPolicy so this path cannot drift from MAX_ATTEMPTS. if (error.retryAfterMs !== undefined) { - if (situation.attempt >= 3) return { kind: "abort" }; - return { kind: "retry", delayMs: pauseMs }; + if (error.retryAfterMs >= RATE_LIMIT_HANG_MS) return { kind: "abort" }; + const retryAfterMs = error.retryAfterMs; + const honorRetryAfter = (decision: RetryDecision): RetryDecision => + decision.kind === "retry" ? { kind: "retry", delayMs: retryAfterMs } : decision; + const decision = defaultPolicy({ ...situation, error }); + if (decision instanceof Promise) return decision.then(honorRetryAfter); + return honorRetryAfter(decision); } } if ( diff --git a/src/provider/grok-responses-adapter.test.ts b/src/provider/grok-responses-adapter.test.ts index 26a44bb17..fb43f397b 100644 --- a/src/provider/grok-responses-adapter.test.ts +++ b/src/provider/grok-responses-adapter.test.ts @@ -162,4 +162,11 @@ describe("createGrokResponsesAdapter", () => { expect(body.reasoning).toEqual({ summary: "detailed" }); expect(body.reasoning?.effort).toBeUndefined(); }); + + test("extracts Retry-After pacing from response headers", () => { + const adapter = createGrokResponsesAdapter(source); + expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000); + expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500); + expect(adapter.extractRetryAfterMs?.(new Headers({}))).toBeUndefined(); + }); }); diff --git a/tests/unit/openai-responses-adapter.test.ts b/tests/unit/openai-responses-adapter.test.ts index 523c5dfa6..14274935e 100644 --- a/tests/unit/openai-responses-adapter.test.ts +++ b/tests/unit/openai-responses-adapter.test.ts @@ -89,3 +89,12 @@ describe("openai-responses x-opencode-session header", () => { expect(req.headers["x-opencode-session"]).toBeUndefined(); }); }); + +describe("openai-responses Retry-After extraction", () => { + test("extracts Retry-After pacing from response headers", () => { + const responses = adapter(); + expect(responses.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000); + expect(responses.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500); + expect(responses.extractRetryAfterMs?.(new Headers({}))).toBeUndefined(); + }); +});