diff --git a/.agents/skills/freehire-search/cli/src/helpers.ts b/.agents/skills/freehire-search/cli/src/helpers.ts index 290044af..c75b5193 100644 --- a/.agents/skills/freehire-search/cli/src/helpers.ts +++ b/.agents/skills/freehire-search/cli/src/helpers.ts @@ -29,9 +29,29 @@ export interface Envelope { error?: string } +// The most apiGet sleeps across all retries of one request. Its own backoff +// schedule (500ms doubling to 8s, six retries) stays under it; a Retry-After +// that would push past it fails the request at once instead, so an outage still +// degrades this source quickly rather than outlasting the caller's tool timeout. +const RETRY_BUDGET_MS = 30_000 + +/** + * The wait a 429/503 asks for via `Retry-After`, in ms, or null when absent or + * unreadable. Accepts both RFC 9110 forms: delta-seconds and an HTTP-date. + */ +export function retryAfterMs(header: string | null, now = Date.now()): number | null { + if (!header) return null + const value = header.trim() + if (/^\d+$/.test(value)) return Number(value) * 1000 + const date = Date.parse(value) + if (Number.isNaN(date)) return null + return Math.max(date - now, 0) +} + /** * GET a JSON envelope from the freehire API. Retries 429/5xx (transient server - * states) with backoff; returns `null` on a 404. A connection failure fails fast + * states) with backoff, waiting at least as long as a `Retry-After` header asks + * within a total budget of RETRY_BUDGET_MS; returns `null` on a 404. A connection failure fails fast * with a clear message — no retry, so an outage degrades this source quickly * rather than hanging the caller (the graceful-degradation contract). */ @@ -39,6 +59,7 @@ export async function apiGet(path: string): Promise | null> { const url = `${baseUrl()}${path}` const maxRetries = 6 let delay = 500 + let waited = 0 for (let attempt = 0; attempt <= maxRetries; attempt++) { let response: Response @@ -59,7 +80,17 @@ export async function apiGet(path: string): Promise | null> { if (attempt === maxRetries) { throw new Error(`freehire API request failed: ${response.status} ${response.statusText}`) } - await sleep(delay + Math.floor(Math.random() * 500)) + const backoff = delay + Math.floor(Math.random() * 500) + const retryAfter = retryAfterMs(response.headers.get("Retry-After")) + const wait = Math.max(backoff, retryAfter ?? 0) + if (waited + wait > RETRY_BUDGET_MS) { + throw new Error( + `freehire API request failed: ${response.status} ${response.statusText}; ` + + `the server asked to retry in ${Math.ceil(wait / 1000)}s, past this CLI's ${RETRY_BUDGET_MS / 1000}s retry budget`, + ) + } + await sleep(wait) + waited += wait delay = Math.min(delay * 2, 8000) continue } diff --git a/.agents/skills/freehire-search/cli/tests/retry-backoff.test.ts b/.agents/skills/freehire-search/cli/tests/retry-backoff.test.ts index fad5654f..740c9435 100644 --- a/.agents/skills/freehire-search/cli/tests/retry-backoff.test.ts +++ b/.agents/skills/freehire-search/cli/tests/retry-backoff.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { apiGet } from "../src/helpers"; +import { apiGet, retryAfterMs } from "../src/helpers"; // The portal contract requires backoff on 429/5xx. These tests pin the retry // loop offline: a stubbed fetch counts attempts, and a stubbed setTimeout @@ -70,3 +70,79 @@ describe("apiGet retry/backoff", () => { expect(state.calls).toBe(1); }); }); + +describe("Retry-After", () => { + function recordWaits(): number[] { + const waits: number[] = []; + globalThis.setTimeout = ((fn: () => void, ms?: number) => { + waits.push(ms ?? 0); + return originalSetTimeout(fn, 0); + }) as unknown as typeof setTimeout; + return waits; + } + + test("waits at least as long as the server asks before retrying a 429", async () => { + const waits = recordWaits(); + const state = stubFetch([ + () => new Response("", { status: 429, headers: { "Retry-After": "20" } }), + () => new Response('{"data":[]}', { status: 200 }), + ]); + + await apiGet("/x"); + expect(state.calls).toBe(2); + expect(waits).toEqual([20_000]); + }); + + test("fails at once, naming the wait, when Retry-After exceeds the retry budget", async () => { + const waits = recordWaits(); + const state = stubFetch([ + () => new Response("", { status: 429, headers: { "Retry-After": "40" } }), + ]); + + await expect(apiGet("/x")).rejects.toThrow(/retry in 40s, past this CLI's 30s retry budget/); + expect(state.calls).toBe(1); + expect(waits).toEqual([]); + }); + + test("bounds the TOTAL wait: repeated Retry-Afters stop once the budget is spent", async () => { + const waits = recordWaits(); + const state = stubFetch([ + () => new Response("", { status: 503, headers: { "Retry-After": "12" } }), + ]); + + await expect(apiGet("/x")).rejects.toThrow(/retry budget/); + expect(waits).toEqual([12_000, 12_000]); + expect(state.calls).toBe(3); + expect(waits.reduce((a, b) => a + b, 0)).toBeLessThanOrEqual(30_000); + }); + + test("its own backoff schedule alone fits inside the budget", async () => { + const waits = recordWaits(); + const state = stubFetch([() => new Response("", { status: 500 })]); + + await expect(apiGet("/x")).rejects.toThrow(/500/); + expect(state.calls).toBe(7); + expect(waits.reduce((a, b) => a + b, 0)).toBeLessThanOrEqual(30_000); + }); + + test("keeps its own backoff when the header asks for less", async () => { + const waits = recordWaits(); + stubFetch([ + () => new Response("", { status: 429, headers: { "Retry-After": "0" } }), + () => new Response('{"data":[]}', { status: 200 }), + ]); + + await apiGet("/x"); + expect(waits[0]).toBeGreaterThanOrEqual(500); + }); + + test("parses delta-seconds and HTTP-dates, ignores junk", () => { + const now = Date.parse("2026-10-18T12:00:00Z"); + expect(retryAfterMs("2", now)).toBe(2_000); + expect(retryAfterMs("Sun, 18 Oct 2026 12:00:30 GMT", now)).toBe(30_000); + expect(retryAfterMs("Sun, 18 Oct 2026 11:59:00 GMT", now)).toBe(0); + expect(retryAfterMs("3600", now)).toBe(3_600_000); + expect(retryAfterMs(null, now)).toBeNull(); + expect(retryAfterMs("soon", now)).toBeNull(); + }); +}); diff --git a/CHANGELOG.md b/CHANGELOG.md index 1275ee1f..9b749f3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,13 @@ per-file diff commands. that guard exists to prevent. `detail` now answers it with `UNKNOWN_FLAG` before any request; `search`, where the flag is documented and used, is unchanged. +- **`freehire-search` honours `Retry-After` on 429 and 503 within a 30-second budget** (#632, + `.agents/skills/freehire-search/cli/src/helpers.ts`) - the retry loop backed off on its own + schedule whatever the server asked for. It now waits for the longer of its backoff and + `Retry-After`, under a 30-second total sleep budget per request, and a wait that would exceed the + budget fails the request at once with a message naming the requested wait. Giving up on a + persistent 5xx still takes about 25 s over 7 calls, as before. + ## [1.7.3] - 2026-10-19 ### Added