Skip to content

Commit 24bba00

Browse files
TheGreatAxiosTheGreatAxios
committed
Honor Retry-After header on 429 retry, fix terminal message
1 parent 00577f3 commit 24bba00

10 files changed

Lines changed: 130 additions & 9 deletions

‎src/agent/retry-policy.test.ts‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { describe, expect, test } from "bun:test";
22
import type { AdmissionQueue } from "../subagent/admission.js";
3-
import { createCorbitsRetryPolicy, type CorbitsRetryPolicyOptions } from "./retry-policy.js";
3+
import {
4+
createCorbitsRetryPolicy,
5+
MAX_BLIND_WAIT_MS,
6+
type CorbitsRetryPolicyOptions,
7+
} from "./retry-policy.js";
48

59
const HTML_503 = `<!DOCTYPE html><html><body>503 Service Unavailable Cloudflare</body></html>`;
610

@@ -104,8 +108,9 @@ describe("createCorbitsRetryPolicy", () => {
104108
raw: { error: { message: "Too Many Requests" } },
105109
},
106110
});
107-
// Remapped to retryable -> default backoff, not abort on moderate Retry-After.
108-
expect(decision).toEqual({ kind: "retry", delayMs: 500 });
111+
// Remapped to retryable -> paced retry capped at the blind-wait ceiling,
112+
// not abort on moderate Retry-After.
113+
expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
109114
});
110115

111116
test("stamped Codex usage-limit 429 retries as retryable, not long-quota abort", async () => {
@@ -120,7 +125,7 @@ describe("createCorbitsRetryPolicy", () => {
120125
raw: "You have hit your ChatGPT usage limit",
121126
},
122127
});
123-
expect(decision).toEqual({ kind: "retry", delayMs: 500 });
128+
expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
124129
});
125130

126131
test("stamped xAI usage/quota body still aborts on long retryAfterMs", async () => {
@@ -174,7 +179,7 @@ describe("createCorbitsRetryPolicy", () => {
174179
};
175180
expect(await decide(bare429)).toEqual({ kind: "abort" });
176181
current = "xai/thegreataxios";
177-
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 500 });
182+
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
178183
});
179184

180185
// CL-6910: the harness only surfaces `inference.error` to the director
@@ -242,7 +247,7 @@ describe("createCorbitsRetryPolicy", () => {
242247
raw: { error: { message: "Too Many Requests" } },
243248
},
244249
};
245-
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 500 });
250+
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
246251
current = "openai";
247252
expect(await decide(bare429)).toEqual({ kind: "abort" });
248253
});
@@ -298,4 +303,48 @@ describe("createCorbitsRetryPolicy", () => {
298303
});
299304
expect(notes).toHaveLength(0);
300305
});
306+
307+
test("retryable 429 honors Retry-After instead of the fixed 500/1000ms backoff", async () => {
308+
const decide = policy({ providerId: "codex/abk-labs" });
309+
const situation = (attempt: number) => ({
310+
attempt,
311+
elapsedMs: 0,
312+
error: {
313+
category: "retryable" as const,
314+
message: "Too Many Requests",
315+
statusCode: 429,
316+
retryAfterMs: 5_000,
317+
},
318+
});
319+
expect(await decide(situation(1))).toEqual({ kind: "retry", delayMs: 5_000 });
320+
expect(await decide(situation(2))).toEqual({ kind: "retry", delayMs: 5_000 });
321+
expect(await decide(situation(3))).toEqual({ kind: "abort" });
322+
});
323+
324+
test("retryable 429 caps a long Retry-After at the blind-wait ceiling", async () => {
325+
const decide = policy({ providerId: "codex/abk-labs" });
326+
const decision = await decide({
327+
attempt: 1,
328+
elapsedMs: 0,
329+
error: {
330+
category: "retryable" as const,
331+
message: "Too Many Requests",
332+
statusCode: 429,
333+
retryAfterMs: 120_000,
334+
},
335+
});
336+
expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
337+
});
338+
339+
test("retryable 429 without Retry-After keeps the fixed backoff", async () => {
340+
const decide = policy();
341+
const situation = (attempt: number) => ({
342+
attempt,
343+
elapsedMs: 0,
344+
error: { category: "retryable" as const, message: "boom", statusCode: 429 },
345+
});
346+
expect(await decide(situation(1))).toEqual({ kind: "retry", delayMs: 500 });
347+
expect(await decide(situation(2))).toEqual({ kind: "retry", delayMs: 1000 });
348+
expect(await decide(situation(3))).toEqual({ kind: "abort" });
349+
});
301350
});

‎src/agent/retry-policy.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ export function createCorbitsRetryPolicy(options?: CorbitsRetryPolicyOptions): R
4949
const pauseMs = Math.min(error.retryAfterMs ?? DEFAULT_PRESSURE_PAUSE_MS, MAX_BLIND_WAIT_MS);
5050
const provider = withProvider.providerId ?? stampedProviderId ?? "unknown";
5151
admission.notePressure(provider, now() + pauseMs);
52+
// The vendored default retries `retryable` on a fixed 500/1000ms
53+
// schedule and ignores Retry-After. A 429 carries the server's pacing
54+
// instruction: honor it (capped at the blind-wait ceiling like the
55+
// quota path) so a short rate limit waits itself out instead of
56+
// burning all three attempts in ~1.5s and aborting. The 3-attempt cap
57+
// mirrors the vendored MAX_ATTEMPTS in retry-policy.ts.
58+
if (error.retryAfterMs !== undefined) {
59+
if (situation.attempt >= 3) return { kind: "abort" };
60+
return { kind: "retry", delayMs: pauseMs };
61+
}
5262
}
5363
if (
5464
error.category === "quota_exhausted" &&

‎src/inference-error-message.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test } from "bun:test";
22

3+
import { normalizeInferenceErrorForTerminal } from "./inference-gateway-error.js";
34
import {
45
inferenceErrorMessage,
56
terminalProviderFailureMessage,
@@ -211,6 +212,25 @@ describe("terminalProviderFailureMessage", () => {
211212
);
212213
});
213214

215+
test("terminal Codex short-429 failure does not claim to still be retrying", () => {
216+
const normalized = normalizeInferenceErrorForTerminal(
217+
{ category: "quota_exhausted", message: "Too Many Requests", statusCode: 429 },
218+
"codex/default",
219+
);
220+
const message = terminalProviderFailureMessage("codex/default", normalized);
221+
expect(message.toLowerCase()).toMatch(/rate limit/);
222+
expect(message.toLowerCase()).not.toContain("retrying");
223+
});
224+
225+
test("retryable 429 guidance asks the operator to wait before trying again", () => {
226+
const message = terminalProviderFailureMessage("codex/default", {
227+
category: "retryable",
228+
message: "Rate limited",
229+
statusCode: 429,
230+
});
231+
expect(message).toContain("Wait a moment and try again.");
232+
});
233+
214234
test("uses a safe label when the provider id contains only control sequences", () => {
215235
const message = terminalProviderFailureMessage("\u001b[31m\u001b[0m", {
216236
category: "fatal",

‎src/inference-error-message.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ export function terminalProviderFailureMessage(
130130
function terminalProviderFailureGuidance(error: InferenceErrorLike, category: string): string {
131131
if (category === "credential_failure") return CREDENTIAL_FAILURE_USER_MESSAGE;
132132
if (category === "context_overflow") return "Try /clear to start fresh.";
133+
// A 429 that survived the harness's paced retries is a wait-it-out rate
134+
// limit, not a generic flake: say so instead of the bare "Try again."
135+
if (category === "retryable" && error.statusCode === 429) {
136+
return "Wait a moment and try again.";
137+
}
133138
if (
134139
category === "retryable" ||
135140
(error.statusCode !== undefined && error.statusCode >= 500 && error.statusCode <= 599)

‎src/inference-gateway-error.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,12 @@ const GATEWAY_OVERLOAD_TEXT_MARKERS = [
4949
/** User-visible line while the harness retries a transient gateway overload. */
5050
export const GATEWAY_OVERLOAD_USER_MESSAGE = "Inference gateway overloaded — retrying…";
5151

52-
/** User-visible line while the harness retries a short known-provider HTTP 429. */
53-
export const RATE_LIMIT_USER_MESSAGE = "Rate limited — retrying…";
52+
/**
53+
* User-visible line for a short known-provider HTTP 429. Worded without
54+
* "retrying": this message also surfaces terminally after the harness has
55+
* exhausted its retries, where claiming an ongoing retry is wrong.
56+
*/
57+
export const RATE_LIMIT_USER_MESSAGE = "Rate limited";
5458

5559
/** Body markers that mean a real usage/quota window, not a short rate limit. */
5660
const XAI_QUOTA_BODY_MARKERS = [

‎src/provider/codex-responses-adapter.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ describe("createCodexResponsesAdapter", () => {
6363
const adapter = createCodexResponsesAdapter(source);
6464
expect(adapter.isStreamTerminal).toBe(isResponsesStreamTerminal);
6565
});
66+
67+
test("extracts Retry-After pacing from response headers", () => {
68+
const adapter = createCodexResponsesAdapter(source);
69+
expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000);
70+
expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500);
71+
expect(adapter.extractRetryAfterMs?.(new Headers({}))).toBeUndefined();
72+
});
6673
});
6774

6875
describe("createCodexResponsesAdapter usage parsing", () => {

‎src/provider/codex-responses-adapter.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,27 @@ export function isResponsesStreamTerminal(sseData: string): boolean {
635635
return typeof eventType === "string" && RESPONSES_TERMINAL_EVENTS.has(eventType);
636636
}
637637

638+
// Responses backends (Codex, Grok, OpenAI) signal 429 pacing with the same
639+
// `retry-after` / `retry-after-ms` headers the Chat Completions adapter
640+
// already reads. The shared Responses adapters never extracted them, so
641+
// every 429 arrived with retryAfterMs undefined and the retry policy fell
642+
// back to blind fixed backoff instead of waiting out the server's window.
643+
export function extractResponsesRetryAfterMs(headers: Headers): number | undefined {
644+
const retryMs = headers.get("retry-after-ms");
645+
if (retryMs !== null) {
646+
const ms = Number(retryMs);
647+
if (Number.isFinite(ms) && ms > 0) return Math.ceil(ms);
648+
}
649+
const raw = headers.get("retry-after");
650+
if (raw !== null) {
651+
const seconds = Number(raw);
652+
if (Number.isFinite(seconds) && seconds > 0) {
653+
return Math.ceil(seconds * 1000);
654+
}
655+
}
656+
return undefined;
657+
}
658+
638659
export function createCodexResponsesAdapter(source: LastCycleSource): ProviderAdapter {
639660
// Re-created per request in buildRequest, not just once here — otherwise
640661
// block indices accumulate across every request the adapter instance ever
@@ -648,5 +669,6 @@ export function createCodexResponsesAdapter(source: LastCycleSource): ProviderAd
648669
parseResponse: (sseData) => parseResponse(sseData, indexer, source),
649670
parseJSONResponse,
650671
isStreamTerminal: isResponsesStreamTerminal,
672+
extractRetryAfterMs: extractResponsesRetryAfterMs,
651673
};
652674
}

‎src/provider/grok-responses-adapter.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import {
2020
RESPONSES_TOOL_NAME_LIMIT,
2121
createResponsesBlockIndexer,
22+
extractResponsesRetryAfterMs,
2223
parseJSONResponse,
2324
parseResponse,
2425
signatureForModel,
@@ -251,5 +252,6 @@ export function createGrokResponsesAdapter(source: LastCycleSource): ProviderAda
251252
},
252253
parseResponse: (sseData) => parseResponse(sseData, indexer, source, GROK_RESPONSES_PROVIDER),
253254
parseJSONResponse,
255+
extractRetryAfterMs: extractResponsesRetryAfterMs,
254256
};
255257
}

‎src/provider/openai-responses-adapter.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {
1313
import {
1414
RESPONSES_TOOL_NAME_LIMIT,
1515
createResponsesBlockIndexer,
16+
extractResponsesRetryAfterMs,
1617
isResponsesStreamTerminal,
1718
parseJSONResponse,
1819
parseResponse,
@@ -233,5 +234,6 @@ export function createOpenAIResponsesAdapter(source: LastCycleSource): ProviderA
233234
parseResponse: (sseData) => parseResponse(sseData, indexer, source, OPENAI_RESPONSES_PROVIDER),
234235
parseJSONResponse,
235236
isStreamTerminal: isResponsesStreamTerminal,
237+
extractRetryAfterMs: extractResponsesRetryAfterMs,
236238
};
237239
}

‎src/tui/stream-event-map.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ describe("inference.error text", () => {
421421
mapProductionEvent({ type: "connector.reply", data: { content: "generic reply" } }, ctx),
422422
).toContainEqual({
423423
type: "assistant",
424-
text: "Work Provider failed (retryable): Rate limited — retrying…. Try again.",
424+
text: "Work Provider failed (retryable): Rate limited. Wait a moment and try again.",
425425
});
426426
});
427427

0 commit comments

Comments
 (0)