Skip to content

Commit 39e88fa

Browse files
TheGreatAxiosTheGreatAxios
committed
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.
1 parent 24bba00 commit 39e88fa

4 files changed

Lines changed: 53 additions & 19 deletions

File tree

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

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

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

@@ -108,9 +104,9 @@ describe("createCorbitsRetryPolicy", () => {
108104
raw: { error: { message: "Too Many Requests" } },
109105
},
110106
});
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 });
107+
// Remapped to retryable -> paced retry honors the server's Retry-After,
108+
// not abort on moderate Retry-After and not a capped 30s wait.
109+
expect(decision).toEqual({ kind: "retry", delayMs: 45_000 });
114110
});
115111

116112
test("stamped Codex usage-limit 429 retries as retryable, not long-quota abort", async () => {
@@ -125,7 +121,7 @@ describe("createCorbitsRetryPolicy", () => {
125121
raw: "You have hit your ChatGPT usage limit",
126122
},
127123
});
128-
expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
124+
expect(decision).toEqual({ kind: "retry", delayMs: 45_000 });
129125
});
130126

131127
test("stamped xAI usage/quota body still aborts on long retryAfterMs", async () => {
@@ -179,7 +175,7 @@ describe("createCorbitsRetryPolicy", () => {
179175
};
180176
expect(await decide(bare429)).toEqual({ kind: "abort" });
181177
current = "xai/thegreataxios";
182-
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
178+
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 45_000 });
183179
});
184180

185181
// CL-6910: the harness only surfaces `inference.error` to the director
@@ -247,7 +243,7 @@ describe("createCorbitsRetryPolicy", () => {
247243
raw: { error: { message: "Too Many Requests" } },
248244
},
249245
};
250-
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
246+
expect(await decide(bare429)).toEqual({ kind: "retry", delayMs: 45_000 });
251247
current = "openai";
252248
expect(await decide(bare429)).toEqual({ kind: "abort" });
253249
});
@@ -321,7 +317,7 @@ describe("createCorbitsRetryPolicy", () => {
321317
expect(await decide(situation(3))).toEqual({ kind: "abort" });
322318
});
323319

324-
test("retryable 429 caps a long Retry-After at the blind-wait ceiling", async () => {
320+
test("retryable 429 honors a Retry-After above the blind-wait ceiling", async () => {
325321
const decide = policy({ providerId: "codex/abk-labs" });
326322
const decision = await decide({
327323
attempt: 1,
@@ -333,7 +329,22 @@ describe("createCorbitsRetryPolicy", () => {
333329
retryAfterMs: 120_000,
334330
},
335331
});
336-
expect(decision).toEqual({ kind: "retry", delayMs: MAX_BLIND_WAIT_MS });
332+
expect(decision).toEqual({ kind: "retry", delayMs: 120_000 });
333+
});
334+
335+
test("retryable 429 with a day-long Retry-After aborts instead of hanging", async () => {
336+
const decide = policy({ providerId: "codex/abk-labs" });
337+
const decision = await decide({
338+
attempt: 1,
339+
elapsedMs: 0,
340+
error: {
341+
category: "retryable" as const,
342+
message: "Too Many Requests",
343+
statusCode: 429,
344+
retryAfterMs: 86_400_000,
345+
},
346+
});
347+
expect(decision).toEqual({ kind: "abort" });
337348
});
338349

339350
test("retryable 429 without Retry-After keeps the fixed backoff", async () => {

‎src/agent/retry-policy.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { getProcessAdmissionQueue, type AdmissionQueue } from "../subagent/admis
1313
// so the user can switch providers or decide when to retry manually.
1414
export const MAX_BLIND_WAIT_MS = 30_000;
1515
const DEFAULT_PRESSURE_PAUSE_MS = 1_000;
16+
const RATE_LIMIT_HANG_MS = 86_400_000;
1617

1718
export interface CorbitsRetryPolicyOptions {
1819
/**
@@ -51,13 +52,19 @@ export function createCorbitsRetryPolicy(options?: CorbitsRetryPolicyOptions): R
5152
admission.notePressure(provider, now() + pauseMs);
5253
// The vendored default retries `retryable` on a fixed 500/1000ms
5354
// 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.
55+
// instruction: honor the full window. Capping at MAX_BLIND_WAIT_MS and
56+
// retrying early burns the attempt budget while the server is still
57+
// closed (the 45s xAI/Codex fixtures). Days-long Retry-After is a hang
58+
// — abort rather than park the session. Attempt abort comes from
59+
// defaultPolicy so this path cannot drift from MAX_ATTEMPTS.
5860
if (error.retryAfterMs !== undefined) {
59-
if (situation.attempt >= 3) return { kind: "abort" };
60-
return { kind: "retry", delayMs: pauseMs };
61+
if (error.retryAfterMs >= RATE_LIMIT_HANG_MS) return { kind: "abort" };
62+
const retryAfterMs = error.retryAfterMs;
63+
const honorRetryAfter = (decision: RetryDecision): RetryDecision =>
64+
decision.kind === "retry" ? { kind: "retry", delayMs: retryAfterMs } : decision;
65+
const decision = defaultPolicy({ ...situation, error });
66+
if (decision instanceof Promise) return decision.then(honorRetryAfter);
67+
return honorRetryAfter(decision);
6168
}
6269
}
6370
if (

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,11 @@ describe("createGrokResponsesAdapter", () => {
162162
expect(body.reasoning).toEqual({ summary: "detailed" });
163163
expect(body.reasoning?.effort).toBeUndefined();
164164
});
165+
166+
test("extracts Retry-After pacing from response headers", () => {
167+
const adapter = createGrokResponsesAdapter(source);
168+
expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000);
169+
expect(adapter.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500);
170+
expect(adapter.extractRetryAfterMs?.(new Headers({}))).toBeUndefined();
171+
});
165172
});

‎tests/unit/openai-responses-adapter.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,12 @@ describe("openai-responses x-opencode-session header", () => {
8989
expect(req.headers["x-opencode-session"]).toBeUndefined();
9090
});
9191
});
92+
93+
describe("openai-responses Retry-After extraction", () => {
94+
test("extracts Retry-After pacing from response headers", () => {
95+
const responses = adapter();
96+
expect(responses.extractRetryAfterMs?.(new Headers({ "retry-after": "7" }))).toBe(7_000);
97+
expect(responses.extractRetryAfterMs?.(new Headers({ "retry-after-ms": "1500" }))).toBe(1_500);
98+
expect(responses.extractRetryAfterMs?.(new Headers({}))).toBeUndefined();
99+
});
100+
});

0 commit comments

Comments
 (0)