fix(bridge): preserve rate-limit retry advice for Codex (carry #6225)

docs-site/src/content/docs/reference/adapters.md

@@ -610,6 +610,20 @@ configuration that names the old id is rewritten at startup.
   allowance on standalone turns surface the original 429 without an early retry. The
   final 429 preserves the stated delay as a cooldown hint. A `~` in its message marks a delay recovered
   from a secondhand trailer sentence rather than an exact header value.
+- For Codex Responses streams, known typed rate-limit failures with a valid delay are normalized to
+  `rate_limit_exceeded` with `Please try again in Ns.` before the original redacted detail.
+  This lets Codex honor the stated delay and use its native reconnect notification without
+  adding a reasoning item to conversation history. Client retries are finite and controlled by
+  the client's `stream_max_retries`; this does not promise recovery after app shutdown or restart.
+  Leave `OPENCODEX_DEVIN_STATED_RESET_WAIT_MS` unset or `0` to let the client own the wait.
+  A positive proxy allowance keeps the existing proxy-owned wait; the client only learns of a
+  final refusal afterwards, and client retries can multiply the proxy's per-request attempts.
+  Combo target/account failover and Grok HTTP 429 handling retain their existing ordering.
+  The exact UI placement and text depend on the Codex version; this is not a custom countdown.
+  Message-only rate-limit errors use the same longest-delay-first formatting. Typed errors
+  without a usable delay retain their original code. Client retries create new HTTP requests;
+  they do not share one proxy request's send counter or cumulative wait allowance. This
+  compatibility mapping does not replace the controls of an explicitly enabled proxy wait.
 - Experimental unofficial bridge; not shown in the dashboard preset by default. See the
   [provider guide](/guides/providers/) for login instructions.
 

docs-site/src/content/docs/reference/codex-retry-diagnostics.md

@@ -0,0 +1,96 @@
+---
+title: Codex retry diagnostics
+description: Distinguish rate-limit advice, automatic retransmission, provider recovery, and Desktop notifications.
+---
+
+OpenCodex's [Devin adapter](/reference/adapters/#devin) preserves a usable retry
+hint in the error contract Codex understands. That is not a guarantee that
+Desktop displays a reconnect row, or that the provider accepts the next request.
+
+## Who waits
+
+With `OPENCODEX_DEVIN_STATED_RESET_WAIT_MS` unset or `0`, the Devin adapter does
+not hold a refused request for the stated cooldown. Codex receives the final
+failure and owns its bounded retry policy. A positive value opts into the
+existing proxy-owned wait; it delays delivery of the final failure and can
+compound with client retries. This compatibility mapping changes neither that
+setting nor Codex's retry count. A process-only override need not survive restart.
+
+`rate_limit_exceeded` plus `Please try again in Ns.` is retry advice, not a
+countdown event. Codex's own retry policy, cancellation and lifecycle still
+apply. Do not add synthetic reasoning, tool, assistant or success items to make
+waiting visible: those items can contaminate history or imply work happened.
+Do not shorten the provider delay or provoke an extra request to advance the
+client's retry counter.
+
+## Why the first reconnect row can be absent
+
+The public Codex `rust-v0.158.0-alpha.2.1` source gates the ordinary stream-retry
+notification in
+[`handle_response_stream_error`](https://github.com/openai/codex/blob/rust-v0.158.0-alpha.2.1/codex-rs/core/src/responses_retry.rs):
+
+```rust
+let report_error = retry_count > 1
+    || cfg!(debug_assertions)
+    || !sess.services.model_client.responses_websocket_enabled();
+```
+
+In a release build, the first retry in this loop can therefore wait without
+emitting `Reconnecting...` when the internal WebSocket-enabled predicate is
+true. The selected delay is not part of that notification predicate, and the
+sleep is outside the `if report_error` block. A long server-advised wait can be
+silent too. This is a version-specific source observation, not proof that any
+particular Desktop request took that branch. Do not infer the internal predicate
+from a model name, an HTTP status, or the transport seen on one proxy hop.
+
+An enabled notification subscription and a renderer capable of drawing the row
+only establish that the receiving path exists. They do not prove that the engine
+emitted an event for the affected turn. Likewise, collapsed provider details can
+explain missing detail text, but not a missing reconnect row by themselves.
+
+The corresponding app-server
+[`ErrorNotification`](https://github.com/openai/codex/blob/rust-v0.158.0-alpha.2.1/codex-rs/app-server-protocol/schema/typescript/v2/ErrorNotification.ts)
+contains `willRetry`, `threadId` and `turnId`. Observe emission, delivery to the
+matching turn, and rendering separately. A proxy `response.failed` frame is not
+itself that app-server notification. Changing this engine-side first-notification
+policy requires a Codex change; an OpenCodex error-message rewrite cannot force
+an event through a branch that does not emit it.
+
+## Read-only verification
+
+Keep the existing app, proxy, settings and test turn unchanged while collecting
+evidence. Restarting, sending a new turn, or switching transports changes the
+experiment. In an isolated follow-up, compare the same controlled failure under
+both internal WebSocket states and inspect the first and subsequent retries;
+do not call that an actual Desktop-rendering test unless the UI is observed.
+
+For the affected conversation and active turn, distinguish these outcomes:
+
+| Question | Evidence required |
+| --- | --- |
+| Was usable advice delivered? | The final failure's code, normalized delay and response-end time. |
+| Did automatic retransmission occur? | A correlated next request after that failure, without a manual send. Measure from the failure response end to the next request start, not from the first request start. |
+| Did the provider recover? | A successful response and continuation of the affected work, not merely another request or an unrelated successful turn. |
+| Was a retry notification emitted and delivered? | The matching app-server error event and its retry flag, not subscription configuration alone. |
+| Was the notice visible? | Observation of the affected Desktop turn; protocol or source-code checks alone are insufficient. |
+
+Approximate provider advice and scheduling overhead can produce a small timing
+difference. If retransmission reaches a socket-close error and a later attempt
+receives a new rate limit, record retransmission as working but provider recovery
+as incomplete. The new refusal's delay is a new observation, not a timer inherited
+from the previous refusal. An `inProgress` turn by itself proves neither recovery
+nor correct rendering. Retry exhaustion, cancellation and app-restart recovery
+must be assessed separately.
+
+Publish only the validation scope and aggregate durations/outcomes. Keep raw logs,
+request bodies, credentials, account data, private paths, and thread/conversation/
+request/trace identifiers out of public PRs and diagnostic reports.
+
+## Regression-test scope
+
+`tests/server/retry-delay-hardening.test.ts` checks that 120-, 900-, 1,800-,
+2,460- and 3,600-second advice survives formatting unchanged; a longer hint stays
+first when a shorter one also exists; and an intervening untimed disconnect does
+not make a later refusal inherit an old delay. These are pure parser/formatter
+checks. They do not wait for an hour, exercise Codex's notification predicate,
+verify an app restart, or prove recovery from a live provider failure.

src/bridge/internal.ts

@@ -15,6 +15,7 @@ import {
   type OcxErrorPayload,
 } from "../lib/errors";
 import { redactSecretString } from "../lib/redact";
+import { formatRetryAfterAdvice } from "../lib/retry-delay";
 import { usageDisplayTotalTokens } from "../usage/totals";
 
 export function uuid(): string {
@@ -144,6 +145,17 @@ export function adapterFailureFromEvent(event: Extract<AdapterEvent, { type: "er
   const error = classifyError(httpStatus, event.errorType ?? fallback.error.type, message);
   if (event.errorType !== undefined) error.type = event.errorType;
   if (event.code !== undefined) error.code = event.code;
+  // Codex only parses a streamed retry delay for these canonical rate-limit codes.
+  // Preserve other typed verdicts (quota, auth, local send budgets) verbatim.
+  if (httpStatus === 429 && error.type === "rate_limit_error"
+    && ["resource_exhausted", "rate_limit_exceeded", "slow_down"].includes(error.code ?? "")) {
+    // Without a usable delay, retain the original code and existing client behavior.
+    const advice = formatRetryAfterAdvice(message);
+    if (advice !== undefined) {
+      error.code = "rate_limit_exceeded";
+      error.message = advice;
+    }
+  }
   // Codex maps cyber_policy on HTTP 400 (body) or mid-stream code; never leave it as 502.
   if (isCyberPolicyCode(error.code) || isCyberPolicyCode(event.code)) {
     error.code = CYBER_POLICY_ERROR_CODE;

src/lib/errors.ts

@@ -1,4 +1,4 @@
-import { parseRetryAfterFromMessage } from "./retry-delay";
+import { formatRetryAfterAdvice, parseRetryAfterFromMessage } from "./retry-delay";
 
 export interface OcxErrorPayload {
   message: string;
@@ -631,10 +631,12 @@ export function adapterFailureFromMessage(message: string): { httpStatus: number
             : httpStatus === 400
               ? "invalid_request_error"
               : "upstream_error";
-  return {
-    httpStatus,
-    error: classifyError(httpStatus, errorType, finalMessage),
-  };
+  const error = classifyError(httpStatus, errorType, finalMessage);
+  if (httpStatus === 429 && error.type === "rate_limit_error"
+    && ["rate_limit_exceeded", "slow_down"].includes(error.code ?? "")) {
+    error.message = formatRetryAfterAdvice(message) ?? error.message;
+  }
+  return { httpStatus, error };
 }
 
 /** Map a terminal Responses error object to the HTTP status we record in /api/logs. */

src/lib/retry-delay.ts

@@ -73,3 +73,13 @@ export function parseRetryAfterFromMessage(message: string): number | undefined
   }
   return result;
 }
+
+/** Put the longest valid lower bound first for clients that read only one hint. */
+export function formatRetryAfterAdvice(message: string): string | undefined {
+  const seconds = parseRetryAfterFromMessage(message);
+  if (seconds === undefined) return undefined;
+  const advice = `Please try again in ${seconds}s.`;
+  if (message.startsWith(advice)) return message;
+  const detail = /try\s+again\s+in/i.test(message) ? `Provider detail: ${message}` : message;
+  return `${advice} ${detail}`;
+}

structure/transports/responses-wire-shapes.md

@@ -1,5 +1,14 @@
 # Responses Wire Shapes
 
+## Client rate-limit retry advice
+
+With a valid delay, `src/bridge/internal.ts` maps typed HTTP 429 `rate_limit_error` codes
+`resource_exhausted`, `rate_limit_exceeded`, and `slow_down` to `rate_limit_exceeded`.
+`src/lib/retry-delay.ts` formats typed and message-only rate-limit advice with the longest
+lower bound first; competing original hints are retained under an explicit provider-detail label.
+Other explicit verdicts, proxy replay budgets and event ordering are unchanged. This creates
+no reasoning/history item; client retry policy, Grok HTTP status and combo preflight stay intact.
+
 ## Compaction image input
 
 For translated routed compaction, `src/responses/compaction-images.ts` replaces earlier user and

structure/transports/responses.md

@@ -584,8 +584,8 @@ count their own frames. `src/protocols/encoders/adapter-events.ts` ports the bri
 machine for those encoders, so a change to item boundaries, tool naming or terminal handling in
 `sse.ts` has to be made there too; the parity tests fail when the two diverge. `src/bridge/errors.ts` (`formatErrorResponse`) formats error responses and
 keeps only allowlisted transport verdict codes. Adapter error events take a different path:
-`src/bridge/internal.ts` carries an event's own `code` into the SSE and JSON failure, after
-mapping cyber-policy codes to HTTP 400. The same file holds the shared usage shaping; `input_tokens_details` and
+`src/bridge/internal.ts` preserves explicit verdicts except cyber-policy and known rate-limit mappings
+([client retry advice](responses-wire-shapes.md#client-rate-limit-retry-advice)). The shared usage shaping's `input_tokens_details` and
 `output_tokens_details` are always emitted, with zero defaults, because strict Responses clients
 deserialize them as required fields.
 

tests/adapters/adapter-error-inline.test.ts

@@ -1,7 +1,9 @@
 import { describe, expect, test } from "bun:test";
 import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../../src/adapters/openai-chat";
 import { createGoogleAdapter as createGoogleAdapterProduction } from "../../src/adapters/google";
-import { adapterFailureFromMessage, bridgeToResponsesSSE } from "../../src/bridge";
+import { adapterFailureFromMessage, bridgeToResponsesSSE, buildResponseJSON } from "../../src/bridge";
+import { adapterFailureFromEvent } from "../../src/bridge/internal";
+import { preflightComboStreamResponse } from "../../src/server/responses/combo-stream-preflight";
 import type { AdapterEvent } from "../../src/types";
 import { withTestTranslatorBudget } from "../helpers/translator-budget";
 
@@ -12,6 +14,83 @@ const createGoogleAdapter = (...args: Parameters<typeof createGoogleAdapterProdu
 
 const provider = { adapter: "openai-chat", baseUrl: "https://example.test/v1", apiKey: "key" };
 
+describe("typed rate-limit retry advice", () => {
+  const refusal = (code = "resource_exhausted", message = "Devin cloud error resource_exhausted; retry after ~900s") => ({
+    type: "error" as const, status: 429, errorType: "rate_limit_error", code, message, retryable: true,
+  });
+
+  test.each(["resource_exhausted", "rate_limit_exceeded", "slow_down"])("canonicalizes %s without changing the original event", code => {
+    const event = refusal(code);
+    const before = JSON.stringify(event);
+    const result = adapterFailureFromEvent(event);
+    expect(result).toMatchObject({ httpStatus: 429, error: {
+      type: "rate_limit_error", code: "rate_limit_exceeded",
+      message: `Please try again in 900s. ${event.message}`,
+    } });
+    expect(JSON.stringify(event)).toBe(before);
+  });
+
+  test("uses the longest lower bound before any shorter client-readable hint", () => {
+    const event = refusal("resource_exhausted", "Please try again in 1s. Retry-After: 1h30m");
+    expect(adapterFailureFromEvent(event).error.message).toStartWith("Please try again in 5400s.");
+    expect(adapterFailureFromEvent(refusal("rate_limit_exceeded", "Please try again in 1800s.")).error.message)
+      .toBe("Please try again in 1800s.");
+  });
+
+  test("does not invent a delay or rewrite unrelated explicit verdicts", () => {
+    for (const message of ["busy", "retry after ~0s", "retry after 2 months"]) {
+      expect(adapterFailureFromEvent(refusal("resource_exhausted", message)).error)
+        .toMatchObject({ code: "resource_exhausted", message });
+    }
+    for (const code of ["insufficient_quota", "request_send_budget_exhausted", "invalid_argument", "vendor_custom"]) {
+      const event = refusal(code);
+      expect(adapterFailureFromEvent(event).error).toMatchObject({ code, message: event.message });
+    }
+    const not429 = { ...refusal(), status: 503, errorType: "server_error" };
+    expect(adapterFailureFromEvent(not429).error).toMatchObject({ code: "resource_exhausted", message: not429.message });
+  });
+
+  test("typed and message-only errors share the same longest-first client advice", () => {
+    const message = "rate limit exceeded: Please try again in 1s. retry after ~900s";
+    const typed = adapterFailureFromEvent(refusal("resource_exhausted", message));
+    const untyped = adapterFailureFromMessage(message);
+    expect(typed).toEqual(untyped);
+    expect(typed.error.message).toBe(`Please try again in 900s. Provider detail: ${message}`);
+    expect(adapterFailureFromMessage(typed.error.message).error.message).toBe(typed.error.message);
+  });
+
+  test("redacts the retained provider detail before appending retry advice", () => {
+    const secret = "sk-" + "a".repeat(32);
+    const event = refusal("resource_exhausted", `retry after ~30s; api_key=${secret}`);
+    const result = adapterFailureFromEvent(event);
+    expect(result.error.message).toStartWith("Please try again in 30s.");
+    expect(result.error.message).not.toContain(secret);
+  });
+
+  test("SSE and buffered failures agree and do not synthesize reasoning or success", async () => {
+    const event = refusal();
+    async function* source() { yield event; }
+    const frames = await collectSse(bridgeToResponsesSSE(source(), "devin/swe-2"));
+    const response = frames.find(frame => frame.event === "response.failed")!.data.response as Record<string, unknown>;
+    const buffered = buildResponseJSON([event], "devin/swe-2");
+    expect(response.error).toEqual(buffered.error);
+    expect(response.error).toMatchObject({ code: "rate_limit_exceeded", message: `Please try again in 900s. ${event.message}` });
+    expect(frames.some(frame => frame.event?.includes("reasoning") || frame.event === "response.completed")).toBe(false);
+    expect(buffered.status).toBe("failed");
+  });
+
+  test("the canonical failure remains pre-output and available to combo failover", async () => {
+    async function* source() { yield refusal(); }
+    const response = new Response(bridgeToResponsesSSE(source(), "devin/swe-2"), {
+      headers: { "content-type": "text/event-stream" },
+    });
+    const preflight = await preflightComboStreamResponse(response, { provider: "devin", model: "swe-2" });
+    expect(preflight.kind).toBe("failed");
+    expect(preflight.response.status).toBe(429);
+    expect((await preflight.response.json()).error.code).toBe("rate_limit_exceeded");
+  });
+});
+
 async function collect(gen: AsyncGenerator<AdapterEvent>): Promise<AdapterEvent[]> {
   const out: AdapterEvent[] = [];
   for await (const e of gen) out.push(e);