fix(web-search): preserve quota evidence when Retry-After would outlive sidecar deadline

src/web-search/executor.ts

@@ -49,10 +49,12 @@ export type SidecarOutcome = WebSearchResult & { error?: string };
  * The forward backend throttles burst sidecar traffic, and without a replay the 429 becomes a
  * failed tool result that poisons the query for the whole turn (see failedQueries in loop.ts).
  * 1 initial send + 2 replays; Retry-After is honored as a lower bound and capped by
- * RETRY_AFTER_CEILING_MS (an instruction past the ceiling ends with the 429 instead of
- * parking the search). Each wait releases the unread 429 body first so sockets do not
- * accumulate under a rate-limit storm. Abort or timeout ends the wait through the existing
- * catch, exactly like an abort during the SSE parse.
+ * RETRY_AFTER_CEILING_MS and the remaining sidecar deadline (an instruction past either
+ * ends with the 429 instead of parking the search). Each wait releases the unread 429 body first so sockets do not
+ * accumulate under a rate-limit storm. The release itself may take up to a second, so a
+ * deadline landing during release or backoff ends with the 429 already in hand rather than
+ * a timeout; a caller abort still ends the wait through the shared catch, exactly like an
+ * abort during the SSE parse.
  */
 const SIDECAR_429_MAX_ATTEMPTS = 3;
 const SIDECAR_429_BASE_DELAY_MS = 1_000;
@@ -98,9 +100,10 @@ export async function runWebSearch(
     stream: true,
   };
   const url = `${forwardProvider.baseUrl}/responses`;
+  // t0 precedes the deadline timer's start so the remaining-time check stays conservative.
+  const t0 = Date.now();
   const linkedSignal = signalWithTimeout(settings.timeoutMs, abortSignal);
   const sidecarExit = sidecarEnter("web-search");
-  const t0 = Date.now();
   try {
     const sendOnce = () => fetchWithResetRetry(
       // Recovery nests INSIDE the version helper: applyUpstreamRecoveryInit then always receives a
@@ -129,10 +132,19 @@ export async function runWebSearch(
       });
       // A deadline, not a clamp: an instruction past the ceiling ends the search with the
       // 429 instead of parking it at a provider that already said it would refuse.
-      if (delay > RETRY_AFTER_CEILING_MS) break;
+      if (delay > RETRY_AFTER_CEILING_MS || delay >= settings.timeoutMs - (Date.now() - t0)) break;
       console.warn(`[web-search] sidecar HTTP 429 — retrying (${attempt + 2}/${SIDECAR_429_MAX_ATTEMPTS}) after ${delay}ms`);
-      await releaseResponseBodyBestEffort(res.body, linkedSignal.signal);
-      await sleepWithAbort(delay, linkedSignal.signal);
+      try {
+        await releaseResponseBodyBestEffort(res.body, linkedSignal.signal);
+        await sleepWithAbort(delay, linkedSignal.signal);
+      } catch (e) {
+        // The release above may consume up to 1s, so the sidecar deadline can land during
+        // cleanup or mid-backoff — before the replay is dispatched. The observed 429 is
+        // already in hand: end with it rather than laundering it into a timeout. A caller
+        // abort (or a non-deadline throw) still propagates to the shared catch below.
+        if (!linkedSignal.signal.aborted || linkedSignal.signal.reason === abortSignal?.reason) throw e;
+        break;
+      }
       res = await sendOnce();
     }
     // Attach the body guard before ANY branch reads it. The success path guarded itself below,

structure/providers-and-adapters.md

@@ -183,6 +183,7 @@ turn show the destination one consistent conversation. The rewrite runs only for
 `call_id` the body already carries — leaves the replayed item untouched. Re-running the search or
 synthesizing result text is not a permitted recovery. The bridge finalizes request-scoped OpenAI sidecar authority on completion, failure, and client cancellation — cancellation releases immediately rather than waiting on an abandoned upstream read — so a recovery probe lease no search consumed is always returned.
 `tests/web-search/web-search-bridge-replay.test.ts` pins the restore and each of those refusals.
+A forward OpenAI search sidecar retries a 429 only when the requested delay fits both its retry ceiling and the remaining overall sidecar deadline. A delay that cannot fit returns and records the original 429 so pool routing retains quota evidence.
 A leg whose
 upstream terminal is `response.failed` or `response.incomplete` runs no search at all and closes
 any cell it opened rather than leaving it in progress. Assistant text is not treated as a search

structure/runtime.md

@@ -378,6 +378,8 @@ Automatic Codex pool selection and account status share the [plan exclusion cont
 ### Empty forced search answers
 
 `src/web-search/loop.ts` makes at most one extra answer attempt after a clean forced-answer terminal with no visible output or tool call. The recovery has no tools and reuses gathered search results. Malformed calls fail before refusal/truncation passthrough, and well-formed recognized refusal/truncation terminals pass through unchanged, including empty or partial answers. The extra generation may incur provider usage.
+
+OpenAI sidecar 429 replays run only when their backoff fits the remaining sidecar deadline; otherwise the original 429 remains the routing-health outcome rather than becoming a timeout.
 ## Scoped provider quota for Combo selection
 
 `src/providers/quota/report-cache.ts` publishes routing evidence only when a producer explicitly supplies its

tests/adapters/openai/openai-chat-dangling-toolcalls.test.ts

@@ -12,6 +12,10 @@ const provider: OcxProviderConfig = {
   baseUrl: "https://example.test/v1",
   apiKey: "sk-test",
   authMode: "key",
+  // The wire role folds to `system` unless a destination is recorded as accepting
+  // `developer`; these cases are about where the barrier lands, so the destination
+  // declares the role rather than asserting the unrecorded default.
+  foldDeveloperRoleToSystem: false,
 };
 
 interface ChatMsg {

tests/responses/chat-inline-document-bytes.test.ts

@@ -175,12 +175,19 @@ describe("inline document bytes reach a wire that can hold them", () => {
       stream: false,
       options: {},
     } as unknown as OcxParsedRequest;
-    const outbound = JSON.parse(createOpenAIChatAdapter(chatProvider).buildRequest(parsed).body) as {
-      messages: Array<{ role: string; content: unknown }>;
-    };
-    expect(outbound.messages).toEqual([{
+    const buildBody = (provider: OcxProviderConfig) => JSON.parse(
+      createOpenAIChatAdapter(provider).buildRequest(parsed).body,
+    ) as { messages: Array<{ role: string; content: unknown }> };
+    // Carrying a document must not demote the turn to `user`; which role the slot shows on the
+    // wire is the destination's recorded answer, so the accepting destination keeps `developer`
+    // and the unrecorded one folds to `system` in place.
+    expect(buildBody({ ...chatProvider, foldDeveloperRoleToSystem: false }).messages).toEqual([{
       role: "developer",
       content: [{ type: "file", file: { file_data: PDF_DATA_URL, filename: "spec" } }],
     }]);
+    expect(buildBody(chatProvider).messages).toEqual([{
+      role: "system",
+      content: [{ type: "file", file: { file_data: PDF_DATA_URL, filename: "spec" } }],
+    }]);
   });
 });

tests/web-search/web-search-sidecar-429.test.ts

@@ -31,14 +31,20 @@ describe("web-search sidecar 429 replays", () => {
     return new Response("data: [DONE]\n\n", { headers: { "content-type": "text/event-stream" } });
   }
 
-  function searchWith(fetchImpl: () => Promise<Response>) {
+  function searchWith(
+    fetchImpl: () => Promise<Response>,
+    timeoutMs = 30_000,
+    recordOutcome?: (outcome: number | "connect_error" | "connect_neutral" | "timeout") => void,
+  ) {
     globalThis.fetch = fetchImpl as unknown as typeof fetch;
     return runOpenAiWebSearch(
       "current docs",
       { type: "web_search" },
       sidecarProvider(),
       new Headers({ authorization: "Bearer selected-token" }),
-      { model: "gpt-5.6-luna", reasoning: "low", timeoutMs: 30_000 },
+      { model: "gpt-5.6-luna", reasoning: "low", timeoutMs },
+      undefined,
+      recordOutcome,
     );
   }
 
@@ -72,4 +78,42 @@ describe("web-search sidecar 429 replays", () => {
     expect(calls).toBe(1);
     expect(outcome.error).toContain("429");
   });
+
+  test("a Retry-After that cannot fit the sidecar deadline preserves the 429", async () => {
+    let calls = 0;
+    const recorded: Array<number | string> = [];
+    const outcome = await searchWith(async () => {
+      calls += 1;
+      return new Response("slow down", { status: 429, headers: { "retry-after": "0.1" } });
+    }, 50, value => recorded.push(value));
+    expect(calls).toBe(1);
+    expect(outcome.error).toContain("429");
+    expect(recorded).toEqual([429]);
+  });
+
+  test("a deadline expiring during pre-retry body cleanup preserves the 429", async () => {
+    // The never-settling body is a worse leak than the other mocks leave behind: restore
+    // fetch so a later file's shared search loop does not inherit a 1s release per retry.
+    const originalFetch = globalThis.fetch;
+    try {
+      let calls = 0;
+      const recorded: Array<number | string> = [];
+      const outcome = await searchWith(async () => {
+        calls += 1;
+        // A cancel() that never settles makes the bounded 1s release run to its cap; the
+        // remaining deadline then cannot fit the backoff, so the wait ends mid-sleep. The
+        // observed 429 must survive that expiry instead of being recorded as a timeout.
+        const body = new ReadableStream<Uint8Array>({
+          start: controller => controller.enqueue(new TextEncoder().encode("rate limited")),
+          cancel: () => new Promise<void>(() => {}),
+        });
+        return new Response(body, { status: 429, headers: { "retry-after": "1" } });
+      }, 1_500, value => recorded.push(value));
+      expect(calls).toBe(1);
+      expect(outcome.error).toContain("429");
+      expect(recorded).toEqual([429]);
+    } finally {
+      globalThis.fetch = originalFetch;
+    }
+  });
 });