From aaed68a815420cd05c37e1338ee1dfbdf2d9597f Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 13 Sep 2026 15:54:50 +0900 Subject: [PATCH 1/4] fix(bridge): fail an in-flight search at a truncated terminal [skip ci] Carry #4381 from aad1d75bcfe8c80efdff5e8e6d9ce3dff16b79c0 onto the current dev tip. When an adapter ends a turn with a recognized truncated stop reason, the streaming path already keeps an open function, custom, or tool-search call incomplete, and the error and explicit-incomplete terminals already close an in-flight provider web search as failed. The plain truncated `done` path did not: it closed the search as `completed`, so a client showed a finished search for a turn the provider cut short before any result arrived. That search takes the same failed status now. The classifier and bridge terminal tests gain the exact `max_output_tokens` stop-reason value, which the vocabulary and mapping cases previously omitted even though `src/responses/truncated-stop-reason.ts` classifies it. Without that case, dropping it from the classifier would silently let the bridge emit `response.completed` with an incomplete call or search attached. This is the remainder of #4312's tool-finalization scope after #4341 landed the open function, custom, and tool-search call handling upstream; nothing from that PR is duplicated here. Verification on this carry: bun test tests/adapters/bridge-nonstreaming-terminal.test.ts, bun run typecheck, bun run structure:check, bun run privacy:scan. Local full suite: NOT RUN. Hosted CI on the lane tip is the suite proof. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/bridge.ts | 4 +++- structure/adapters/registry.md | 2 ++ .../bridge-nonstreaming-terminal.test.ts | 22 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/bridge.ts b/src/bridge.ts index 612b60ba36..f0cf2f0a2c 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -1293,7 +1293,9 @@ export function bridgeToResponsesSSE( if (isTruncatedStopReason(event.stopReason)) failCurrentToolCall(); else closeCurrentToolCall(); } - if (currentWebSearch) closeCurrentWebSearch("completed", []); + // A search still in flight when upstream truncates never returned results, so it + // takes the same "failed" status as the error/incomplete terminals below. + if (currentWebSearch) closeCurrentWebSearch(isTruncatedStopReason(event.stopReason) ? "failed" : "completed", []); releasePendingWebSources(); // Redacted-only turns (or hidden thinking without a trailing signature event) still // need their envelope-only reasoning item so the blocks replay next turn. diff --git a/structure/adapters/registry.md b/structure/adapters/registry.md index 8c23e38eef..38ae9d79bb 100644 --- a/structure/adapters/registry.md +++ b/structure/adapters/registry.md @@ -81,6 +81,8 @@ Listener startup diagnostics follow [the runtime lifecycle contract](../runtime. The bridge keeps an open function, custom, or tool-search call incomplete when an adapter ends with a recognized truncated stop reason. Streaming emits no argument/input completion frame for that open call, and buffered JSON applies the same status. A call already closed by its own tool-call end retains its completed state. The response remains incomplete, partial output is preserved, and truncated compaction never replaces history. +A provider web search still in flight at that truncated terminal is finalized as `failed`, the same status it already receives from the error and explicit-incomplete terminals. It never returned results, so reporting it as `completed` would leave the client showing a finished search for a turn the provider cut short. + Chat helper admission in `src/server/responses/core.ts` follows the [deferred stored-main contract](../providers/openai-tiers.md): only a needed Direct OpenAI helper claims stored main, after terminal vision, routed vision and search exclusions. diff --git a/tests/adapters/bridge-nonstreaming-terminal.test.ts b/tests/adapters/bridge-nonstreaming-terminal.test.ts index de6bfb37c9..2367950586 100644 --- a/tests/adapters/bridge-nonstreaming-terminal.test.ts +++ b/tests/adapters/bridge-nonstreaming-terminal.test.ts @@ -268,6 +268,7 @@ describe("truncated-stop-reason classifier", () => { "length", "content-filter", // Command Code / AI SDK "pause_turn", // Anthropic: turn needs continuation "refusal", "model_context_window_exceeded", // Anthropic + "max_output_tokens", // Anthropic: same spelling as the mapped reason "MAX_TOKENS", "SAFETY", "MALFORMED_FUNCTION_CALL", "IMAGE_SAFETY", "LANGUAGE", // Gemini "Safety", "safety", // mixed case must not slip through ]) { @@ -285,6 +286,7 @@ describe("truncated-stop-reason classifier", () => { test("truncation maps to the right incomplete_details reason", () => { expect(truncationReasonFor("length")).toBe("max_output_tokens"); expect(truncationReasonFor("model_context_window_exceeded")).toBe("max_output_tokens"); + expect(truncationReasonFor("max_output_tokens")).toBe("max_output_tokens"); expect(truncationReasonFor("refusal")).toBe("content_filter"); expect(truncationReasonFor("SAFETY")).toBe("content_filter"); expect(truncationReasonFor("end_turn")).toBeUndefined(); @@ -318,6 +320,7 @@ describe("truncated done preserves open tool integrity (#4312)", () => { ["content_filter", "content_filter"], ["max_tokens", "max_output_tokens"], ["length", "max_output_tokens"], + ["max_output_tokens", "max_output_tokens"], ] as const; for (const [stopReason, reason] of cases) { for (const kind of ["function_call", "custom_tool_call", "tool_search_call"] as const) { @@ -376,5 +379,24 @@ describe("truncated done preserves open tool integrity (#4312)", () => { expect(text).toContain("event: response.function_call_arguments.done"); expect(text).toContain('"arguments":"{\\"arg\\":\\"complete\\"}","status":"completed"'); }); + + test(`${stopReason}: a search still in flight is failed, not completed`, async () => { + // The provider cut the turn short, so the search never returned results. Reporting it as + // completed would leave the client showing a finished search for a truncated turn. + const text = await sseText([ + { type: "web_search_call_begin", id: "search_in_flight" }, + { type: "done", stopReason }, + ]); + const item = text.split("\n\n") + .flatMap(frame => { + const data = frame.split("\n").find(line => line.startsWith("data: {"))?.slice(6); + return data ? [JSON.parse(data)] : []; + }) + .find(frame => frame.type === "response.output_item.done" + && frame.item?.type === "web_search_call")?.item; + + expect(terminalEventNames(text)).toEqual(["response.incomplete"]); + expect(item).toMatchObject({ type: "web_search_call", status: "failed" }); + }); } }); From b1453a38385a52eece45112799202f6b42da71fd Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 13 Sep 2026 15:58:40 +0900 Subject: [PATCH 2/4] fix(images): bound bridge iteration buffering [skip ci] Carry #4388 from 9518b528166122e775ef4246e81dff4d1b903064 onto the lane B chain above the #4381 carry. A hidden image/video iteration collected adapter events into an unbounded array. Both the runTurn emit path and ordinary stream collection kept every non-heartbeat event until the scanner ran, so an oversized tool argument or a long text stream grew one iteration without limit even though src/lib/translator-budget.ts already defines the turn and call-argument ceilings. Each iteration now owns an independent budget: 32 MiB of serialized retained events including array framing, and 2 MiB of UTF-8 arguments per open call, with surrogate pairs that straddle two deltas counted once. Overflow aborts the producer and surfaces translation_buffer_limit instead of a truncated success. Folded in the Codex review finding left on the source head, which was not addressed there. createAdapterEventQueue merges adjacent text and thinking deltas into chunks of up to 64 KiB while no reader is waiting, so a synchronous producer's token-granular deltas survive as a handful of strings. Charging each pre-merge envelope billed about 31 bytes for every one-character delta, so roughly 1 MiB of retained output crossed the 32 MiB turn limit and aborted a healthy turn. push now reports whether it merged the event into its buffered tail, and a merged delta is charged only for the payload it appended. The return value is additive: every other caller passes push as a void callback and is unaffected. That accounting is deliberately conservative in one place. JSON escaping is per character, so a merged delta costs its quoted form minus the quotes; a surrogate pair split across two deltas over-counts by eight bytes and never under-counts. Verification on this carry: bun test tests/images/loop.test.ts tests/adapters/run-turn-queue.test.ts (74 pass, 0 fail), bun run typecheck, bun run structure:check, bun run privacy:scan. The new coalescing regression was driven red against the unfixed emit path first, where it reproduced the spurious translation_buffer_limit response. Local full suite: NOT RUN. Hosted CI on the lane tip is the suite proof. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/adapters/run-turn-queue.ts | 19 +- src/images/loop.ts | 155 ++++++++++++---- structure/runtime.md | 3 + structure/transports/inventory.md | 18 ++ tests/images/loop.test.ts | 288 +++++++++++++++++++++++++++++- 5 files changed, 438 insertions(+), 45 deletions(-) diff --git a/src/adapters/run-turn-queue.ts b/src/adapters/run-turn-queue.ts index 4b63e387de..253407baf2 100644 --- a/src/adapters/run-turn-queue.ts +++ b/src/adapters/run-turn-queue.ts @@ -12,7 +12,13 @@ export const PREFLIGHT_HEARTBEAT_RETAIN_LIMIT = 16; export const COALESCE_MAX_CHUNK_LENGTH = 64 * 1024; export interface AdapterEventQueue { - push(event: AdapterEvent): void; + /** + * Returns true when the event was merged into the buffered tail instead of + * becoming its own retained item. A caller that charges a memory budget for + * what the queue holds needs that distinction: a merged delta costs only its + * appended payload, while a new item costs a whole serialized event. + */ + push(event: AdapterEvent): boolean; close(): void; stream(): AsyncIterable; collect(): Promise; @@ -98,21 +104,22 @@ export function createAdapterEventQueue(opts?: { return false; }; - const push = (event: AdapterEvent): void => { - if (closed) return; + const push = (event: AdapterEvent): boolean => { + if (closed) return false; const reader = readers.shift(); if (reader) { reader({ done: false, value: event }); - return; + return false; } - if (coalesceIntoTail(event)) return; + if (coalesceIntoTail(event)) return true; if (queued.length >= maxBacklog) { opts?.onBacklogExceeded?.(); queued.push({ type: "error", message: "consumer stalled: adapter event backlog exceeded — turn aborted" }); close(); - return; + return false; } queued.push(event); + return false; }; const close = (): void => { diff --git a/src/images/loop.ts b/src/images/loop.ts index e33ae02b41..6f9eacad1f 100644 --- a/src/images/loop.ts +++ b/src/images/loop.ts @@ -24,7 +24,9 @@ import { readBoundedResponseBody } from "../lib/bounded-body"; import { applyUpstreamRecoveryInit, fetchWithResetRetry, prepareSameTarget429Wait } from "../lib/upstream-retry"; import { rateLimitRetryDelayMs } from "../providers/key-failover"; import { + createTranslatorBudget, isTranslatorBudgetExceededError, + TRANSLATOR_MAX_CALL_ARGUMENT_BYTES, TRANSLATOR_MAX_TURN_BYTES, TranslatorBudgetExceededError, } from "../lib/translator-budget"; @@ -101,6 +103,57 @@ interface ImageCall { providerMetadata?: OcxProviderOpaqueToolCallMetadata; } +/** Independent retention owner: adapter leases and final SSE buffers have separate lifetimes. */ +function createIterationEventBudget() { + const budget = createTranslatorBudget(); + let firstEvent = true; + let argumentBytes: number | undefined; + let trailingHighSurrogate = false; + return { + retain(event: AdapterEvent, coalescedIntoTail = false): void { + // Heartbeats are never retained or passed to scanEventsForImageCall. + if (event.type === "heartbeat") return; + if (event.type === "tool_call_start") { + argumentBytes = 0; + trailingHighSurrogate = false; + } else if (event.type === "tool_call_delta" && argumentBytes !== undefined) { + const chunk = event.arguments; + argumentBytes += Buffer.byteLength(chunk); + // A surrogate pair may straddle adapter deltas; count the concatenated UTF-8 string. + if (trailingHighSurrogate && /^[\uDC00-\uDFFF]/.test(chunk)) argumentBytes -= 2; + if (chunk.length > 0) trailingHighSurrogate = /[\uD800-\uDBFF]$/.test(chunk); + if (argumentBytes > TRANSLATOR_MAX_CALL_ARGUMENT_BYTES) { + throw new TranslatorBudgetExceededError("tool_args", TRANSLATOR_MAX_CALL_ARGUMENT_BYTES); + } + } else { + argumentBytes = undefined; + trailingHighSurrogate = false; + } + // A delta the queue merged into its buffered tail leaves one object behind, not two, + // so it costs the appended payload rather than another envelope. Charging the whole + // event here would bill over 32 MiB for the ~1 MiB that a million one-character text + // deltas actually retain, and abort a turn far below the documented limit. + const appended = coalescedIntoTail + ? event.type === "text_delta" ? event.text : event.type === "thinking_delta" ? event.thinking : undefined + : undefined; + if (appended !== undefined) { + // JSON escaping is per character, so the tail grows by the quoted form minus its + // quotes. A surrogate pair split across two deltas is the one over-count, by eight + // bytes, which bounds memory conservatively and never under-charges. + budget.chargeRetained(Buffer.byteLength(JSON.stringify(appended)) - 2, { + kind: "retained_collectors", + }); + return; + } + budget.chargeRetained(Buffer.byteLength(JSON.stringify(event)) + (firstEvent ? 2 : 1), { + kind: "retained_collectors", + }); + firstEvent = false; + }, + dispose: () => budget.dispose(), + }; +} + /** * Split an iteration's adapter events into (a) the image-generation tool calls to intercept and * (b) the events to pass through to Codex. An image tool-call's own start/delta/end events are @@ -370,6 +423,8 @@ export async function runWithImageBridge(deps: ImageBridgeDeps): Promise; @@ -397,29 +452,31 @@ export async function runWithImageBridge(deps: ImageBridgeDeps): Promise internalAbort.abort("runTurn backlog exceeded"), }); - // Attempt telemetry must fire at dispatch time (parity with fetchOnce), not after collect. - deps.onAttemptSend?.(); - void adapter - .runTurn( - iterParsed, - { - headers: deps.forwardHeaders ? new Headers(deps.forwardHeaders) : new Headers(), - abortSignal: signal, - translatorBudget, - }, - queue.push, - ) - .then(() => queue.close()) - .catch(err => { - queue.push({ type: "error", message: err instanceof Error ? err.message : String(err) }); - queue.close(); - }); + const iterationBudget = createIterationEventBudget(); + let accepting = true; + let collectionError: unknown; + const closeOnAbort = (): void => { accepting = false; queue.close(); }; + signal.addEventListener("abort", closeOnAbort, { once: true }); + const emit = (event: AdapterEvent): void => { + if (!accepting || signal.aborted) return; + try { + // Check at emission, before even a synchronous producer can fill the queue. push + // reports whether it merged this delta into the buffered tail, which is what the + // iteration actually retains once the consumer drains it. + iterationBudget.retain(event, queue.push(event)); + } catch (error) { + collectionError = error; + closeOnAbort(); + internalAbort.abort(error); + throw error; + } + }; // Bound collect with a real *idle* deadline that resets on each emitted event. // A fixed wall-clock race would abort legitimate long Cursor turns that keep @@ -439,14 +496,34 @@ export async function runWithImageBridge(deps: ImageBridgeDeps): Promise { + if (accepting) { + collectionError = err; + if (isTranslatorBudgetExceededError(err)) internalAbort.abort(err); + } + closeOnAbort(); + }); idle.reset(); for await (const event of queue.stream()) { if (timedOut) break; idle.reset(); - events.push(event); + if (event.type !== "heartbeat") events.push(event); } } finally { + accepting = false; idle.cancel(); + signal.removeEventListener("abort", closeOnAbort); + iterationBudget.dispose(); + } + if (collectionError) { + if (isTranslatorBudgetExceededError(collectionError)) throw collectionError; + throw new LoopError(502, collectionError instanceof Error ? collectionError.message : String(collectionError)); } if (timedOut) { throw new LoopError(504, `runTurn inactivity timeout after ${stallTimeoutMs}ms during image-bridge`); @@ -466,14 +543,9 @@ export async function runWithImageBridge(deps: ImageBridgeDeps): Promise { - const events: AdapterEvent[] = []; + const events: AdapterEvent[] = prepared.collectedEvents ?? []; + const iterationBudget = prepared.collectedEvents ? undefined : createIterationEventBudget(); try { - const parse = prepared.responseAdapter.parseStream.bind(prepared.responseAdapter); - for await (const event of parseStreamWithProgress(prepared.response, parse, { - signal, - inactivityTimeoutMs: stallTimeoutMs, - translatorBudget, - })) { - if (event.type === "heartbeat") yield event; - else events.push(event); + if (iterationBudget) { + const parse = prepared.responseAdapter.parseStream.bind(prepared.responseAdapter); + for await (const event of parseStreamWithProgress(prepared.response, parse, { + signal, + inactivityTimeoutMs: stallTimeoutMs, + translatorBudget, + })) { + if (event.type === "heartbeat") yield event; + else { + iterationBudget.retain(event); + events.push(event); + } + } } } catch (error) { - if (isTranslatorBudgetExceededError(error)) throw error; + if (isTranslatorBudgetExceededError(error)) { + internalAbort.abort(error); + throw error; + } if (signal.aborted) throw new LoopError(499, "client closed request during image-bridge"); if (error instanceof RoutedModelInactivityError) throw new LoopError(504, error.message); if (error instanceof WebSearchStreamProtocolError) throw new LoopError(502, error.message); throw new LoopError(502, `Provider stream error: ${error instanceof Error ? error.message : String(error)}`); + } finally { + iterationBudget?.dispose(); } const terminalIndexes = events.flatMap((event, index) => diff --git a/structure/runtime.md b/structure/runtime.md index 35b1e1f31a..c03c14d914 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -173,6 +173,9 @@ The server exposes `POST /api/stop` which restores native Codex config, stops an Adapter output must stay in internal `AdapterEvent` form until `bridge.ts` converts it back to Responses SSE or WebSocket frames. +The image/video loop bounds each hidden iteration before replay or fulfillment; see +[media iteration retention](transports/inventory.md#media-iteration-retention). + Live model discovery is bounded and registry-driven through `src/providers/model-discovery.ts`. Custom providers keep the conventional `${baseUrl}/models` request; canonical presets may select a trusted URL/path/query and declarative eligibility filter without persisting that policy into user diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index b2ba773ba1..ff966bb1bc 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -38,6 +38,24 @@ Native Composer/MCP behavior and text-only historical replay remain unchanged. The shared Responses path follows the [bounded multipart recovery contract](../subagents.md#multipart-encrypted-task-recovery); credential admission and retry policy remain unchanged. +## Media iteration retention + +`src/images/loop.ts` admits at most 32 MiB of serialized non-heartbeat adapter events per +iteration, including array framing, and 2 MiB of UTF-8 arguments per current tool call. Both +`runTurn` emission and ordinary stream collection enforce the shared translator limits before +retaining another event. Overflow aborts the producer and surfaces `translation_buffer_limit`. +The iteration budget is separate from adapter leases and final response buffers; collected +`runTurn` events reach the scanner directly without a second charge. Heartbeats do not reset +argument accounting, and each new iteration receives a fresh retention budget. The charge follows +what `src/adapters/run-turn-queue.ts` keeps: `push` reports whether it merged a text or thinking +delta into its buffered tail, and a merged delta costs only its appended payload. Billing every +pre-merge envelope would abort a turn on roughly a thirtieth of the documented limit whenever a +producer streams token-granular deltas ahead of its consumer. These bounds do +not cap process RSS or the conversation messages accumulated across completed media iterations. +`tests/images/loop.test.ts` covers early producer cancellation, byte boundaries, iteration reset, +opaque metadata, normal tool passthrough, coalesced-tail accounting, and consumer cancellation on +both execution paths. + ## Provider diagnostic outbound safety Provider connection tests and live model discovery share the GET-only provider outbound wrapper. diff --git a/tests/images/loop.test.ts b/tests/images/loop.test.ts index 4b9d6423b7..9dbddf1a68 100644 --- a/tests/images/loop.test.ts +++ b/tests/images/loop.test.ts @@ -7,6 +7,12 @@ import type { AdapterEvent, OcxParsedRequest } from "../../src/types"; import type { ImageBridgePlan, ImageCallResult } from "../../src/images/types"; import type { ImageBridgeDeps } from "../../src/images/loop"; import { createTestTranslatorBudget } from "../helpers/translator-budget"; +import { parseStreamWithProgress, type ParseStreamWithProgressOptions } from "../../src/web-search/progress-stream"; +import { TRANSLATOR_MAX_CALL_ARGUMENT_BYTES, TRANSLATOR_MAX_TURN_BYTES, translatorLiveBudgetCountForTests } from "../../src/lib/translator-budget"; + +const realParseStreamWithProgress = parseStreamWithProgress; +let useRealProgressStream = false; +let fulfillCallCount = 0; const PREV_HOME = process.env.OPENCODEX_HOME; let runWithImageBridgeProduction: typeof import("../../src/images/loop")["runWithImageBridge"]; @@ -23,14 +29,15 @@ beforeAll(async () => { process.env.OPENCODEX_HOME = join(tmpdir(), "ocx-test-" + randomUUID()); mock.restore(); mock.module("../../src/web-search/progress-stream", () => ({ - parseStreamWithProgress: async function* (_resp: Response, parse: (r: Response) => AsyncGenerator, _opts: unknown) { - for await (const e of parse(_resp)) yield e; + parseStreamWithProgress: async function* (_resp: Response, parse: ProviderAdapter["parseStream"], opts: ParseStreamWithProgressOptions) { + if (useRealProgressStream) yield* realParseStreamWithProgress(_resp, parse, opts); + else for await (const e of parse(_resp, opts.translatorBudget)) yield e; }, RoutedModelInactivityError: class extends Error { readonly timeoutMs = 0; }, WebSearchStreamProtocolError: class extends Error { /* */ }, })); mock.module("../../src/images/fulfill", () => ({ - fulfillImageCall: async (): Promise => fulfillResult, + fulfillImageCall: async (): Promise => { fulfillCallCount++; return fulfillResult; }, })); ({ runWithImageBridge: runWithImageBridgeProduction, @@ -62,11 +69,195 @@ const defaultFulfillResult: ImageCallResult = { files: ["/test/img.png"], count: 1, markdown: "![image](/test/img.png)", }; beforeEach(() => { + useRealProgressStream = false; + fulfillCallCount = 0; fulfillResult = { ...defaultFulfillResult, files: [...defaultFulfillResult.files] }; buildRequestCalls = 0; streamQueue = []; }); +describe.each(["runTurn", "parseStream"] as const)("image-loop collection bounds — %s", mode => { + beforeEach(() => { useRealProgressStream = true; }); + + function streamingAdapter(events: () => Generator) { + const state = { produced: 0, terminalProduced: false, closed: false, cancelled: false, signal: undefined as AbortSignal | undefined, requests: [] as OcxParsedRequest[] }; + async function* source(): AsyncGenerator { + try { + for (const event of events()) { + if (state.signal?.aborted) return; + state.produced++; + if (event.type === "done") state.terminalProduced = true; + yield event; + // Keep queue backlog small: the regression is cumulative iteration retention. + await Bun.sleep(1); + } + } finally { state.closed = true; } + } + const adapter: ProviderAdapter = { + name: "bounded-media-fixture", + buildRequest: async (_parsed, incoming) => { + state.signal = incoming.abortSignal; + state.requests.push(_parsed); + return { url: "https://example.invalid/model", method: "POST", headers: {}, body: "{}" }; + }, + fetchResponse: async () => new Response(new ReadableStream({ + cancel() { state.cancelled = true; }, + })), + parseStream: source, + ...(mode === "runTurn" ? { + runTurn: async (_parsed: OcxParsedRequest, incoming: IncomingMeta, emit: (event: AdapterEvent) => void) => { + state.signal = incoming.abortSignal; + state.requests.push(_parsed); + for await (const event of source()) emit(event); + }, + } : {}), + }; + return { adapter, state }; + } + + test("aborts retained-event overflow before the producer reaches its terminal", async () => { + const { adapter, state } = streamingAdapter(function* () { + const text = "x".repeat(1024 * 1024); + for (let i = 0; i < 40; i++) yield { type: "text_delta", text }; + yield { type: "done" }; + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + await Bun.sleep(5); + expect(state.terminalProduced).toBe(false); + expect(state.produced).toBeLessThan(40); + expect(state.signal?.aborted).toBe(true); + expect(state.closed).toBe(true); + if (mode === "parseStream") expect(state.cancelled).toBe(true); + expect(sse).toContain('"code":"translation_buffer_limit"'); + expect(sse).not.toContain("event: response.completed"); + expect(fulfillCallCount).toBe(0); + }); + + test("aborts cumulative UTF-8 arguments before media fulfillment or terminal", async () => { + const { adapter, state } = streamingAdapter(function* () { + yield { type: "tool_call_start", id: "oversize", name: "image_gen" }; + const argumentsChunk = "한".repeat(Math.floor(TRANSLATOR_MAX_CALL_ARGUMENT_BYTES / 6)); + for (let i = 0; i < 4; i++) { + yield { type: "tool_call_delta", arguments: argumentsChunk }; + yield { type: "heartbeat" }; + } + yield { type: "tool_call_end" }; + yield { type: "done" }; + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + await Bun.sleep(5); + expect(state.terminalProduced).toBe(false); + expect(state.produced).toBeLessThan(9); + expect(state.signal?.aborted).toBe(true); + expect(state.closed).toBe(true); + if (mode === "parseStream") expect(state.cancelled).toBe(true); + expect(sse).toContain('"code":"translation_buffer_limit"'); + expect(sse).not.toContain("event: response.completed"); + expect(fulfillCallCount).toBe(0); + expect(translatorLiveBudgetCountForTests()).toBe(1); // Only the caller-owned budget remains. + }); + + test.each([0, 1])("retained JSON array boundary plus %i byte", async extra => { + const first: AdapterEvent[] = [{ type: "text_delta", text: "" }, ...imageCallEvents]; + const overhead = Buffer.byteLength(JSON.stringify(first)); + first[0] = { type: "text_delta", text: "x".repeat(TRANSLATOR_MAX_TURN_BYTES - overhead + extra) }; + let iteration = 0; + const { adapter } = streamingAdapter(function* () { + if (iteration++ === 0) yield* first; + else { yield { type: "text_delta", text: "finished" }; yield { type: "done" }; } + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + expect(sse.includes('"code":"translation_buffer_limit"')).toBe(extra === 1); + expect(sse.includes("event: response.completed")).toBe(extra === 0); + expect(fulfillCallCount).toBe(extra === 0 ? 1 : 0); + }); + + test("resets the retained-event budget between media iterations", async () => { + let iteration = 0; + const { adapter } = streamingAdapter(function* () { + if (iteration++ < 2) { + yield { type: "text_delta", text: "x".repeat(18 * 1024 * 1024) }; + yield* imageCallEvents; + } else { yield { type: "text_delta", text: "finished" }; yield { type: "done" }; } + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + expect(sse).toContain("event: response.completed"); + expect(sse).not.toContain("translation_buffer_limit"); + expect(fulfillCallCount).toBe(2); + }); + + test("accepts exact UTF-8 argument limits per call and preserves opaque metadata", async () => { + let iteration = 0; + const signatures = ["first-synthetic-signature", "second-synthetic-signature"]; + const { adapter, state } = streamingAdapter(function* () { + if (iteration++ > 0) { yield { type: "done" }; return; } + for (const signature of signatures) { + yield { type: "tool_call_start", id: signature, name: "image_gen", providerMetadata: { google: { thoughtSignature: signature } } }; + const prefix = '{"prompt":"'; + const suffix = '"}'; + yield { type: "tool_call_delta", arguments: prefix + "x".repeat(TRANSLATOR_MAX_CALL_ARGUMENT_BYTES - prefix.length - suffix.length - 4) }; + yield { type: "tool_call_delta", arguments: "\uD83D" }; + yield { type: "heartbeat" }; + yield { type: "tool_call_delta", arguments: "\uDE00" + suffix }; + yield { type: "tool_call_end" }; + } + yield { type: "done" }; + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + expect(sse).toContain("event: response.completed"); + expect(sse).not.toContain("translation_buffer_limit"); + expect(fulfillCallCount).toBe(2); + const assistant = state.requests[1]?.context.messages.find(message => message.role === "assistant"); + const calls = assistant?.role === "assistant" ? assistant.content.filter(part => part.type === "toolCall") : []; + expect(calls.map(call => call.providerMetadata?.google?.thoughtSignature)).toEqual(signatures); + expect(calls.map(call => Buffer.byteLength(JSON.stringify(call.arguments)))).toEqual([TRANSLATOR_MAX_CALL_ARGUMENT_BYTES, TRANSLATOR_MAX_CALL_ARGUMENT_BYTES]); + }); + + test("passes normal real tool calls through without media fulfillment", async () => { + const { adapter } = streamingAdapter(function* () { + yield { type: "tool_call_start", id: "real", name: "read_file", providerMetadata: { google: { thoughtSignature: "real-call-signature" } } }; + yield { type: "tool_call_delta", arguments: '{"path":"example.txt"}' }; + yield { type: "tool_call_end" }; + yield { type: "done" }; + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const sse = await response.text(); + expect(sse).toContain("event: response.completed"); + expect(sse).toContain('"name":"read_file"'); + expect(sse).toContain('"thought_signature":"real-call-signature"'); + expect(fulfillCallCount).toBe(0); + }); + + test("consumer cancellation aborts and releases an active collector", async () => { + const { adapter, state } = streamingAdapter(function* () { + for (let i = 0; i < 100; i++) yield { type: "text_delta", text: "pending" }; + yield { type: "done" }; + }); + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const reader = response.body!.getReader(); + const draining = (async () => { while (!(await reader.read()).done) { /* keep demanding SSE */ } })(); + try { + for (let i = 0; i < 20 && state.produced === 0; i++) await Bun.sleep(1); + expect(state.produced).toBeGreaterThan(0); + } finally { + await reader.cancel("synthetic consumer closed"); + await draining; + } + await Bun.sleep(5); + expect(state.signal?.aborted).toBe(true); + expect(state.closed).toBe(true); + expect(state.terminalProduced).toBe(false); + if (mode === "parseStream") expect(state.cancelled).toBe(true); + expect(fulfillCallCount).toBe(0); + expect(translatorLiveBudgetCountForTests()).toBe(1); + }); +}); + const mockAdapter: ProviderAdapter = { name: "test", buildRequest: async () => { buildRequestCalls++; return { url: "https://test/v1/chat", method: "POST", headers: {}, body: "{}" }; }, @@ -731,6 +922,97 @@ describe("runWithImageBridge", () => { // --------------------------------------------------------------------------- describe("runWithImageBridge — runTurn adapter", () => { + test("charges the queue's coalesced tail, not each delta it discarded", async () => { + // createAdapterEventQueue merges adjacent text deltas into chunks while no reader is + // scheduled, so a synchronous producer's one-character deltas survive as a handful of + // strings. Charging each original event's envelope instead billed ~31 bytes apiece and + // tripped the 32 MiB turn limit on roughly 1 MiB of retained output. + const deltas = 1_200_000; + const response = await runWithImageBridge({ + parsed: makeParsed(), plan, + adapter: { + ...mockAdapter, + runTurn: async (_parsed, _incoming, emit) => { + for (let i = 0; i < deltas; i++) emit({ type: "text_delta", text: "x" }); + emit({ type: "done" }); + }, + }, + }); + const sse = await response.text(); + expect(Buffer.byteLength(JSON.stringify({ type: "text_delta", text: "x" })) * deltas) + .toBeGreaterThan(TRANSLATOR_MAX_TURN_BYTES); + expect(sse).not.toContain("translation_buffer_limit"); + expect(sse).toContain("event: response.completed"); + }); + + test("queue backlog overflow keeps its upstream error instead of becoming client cancellation", async () => { + const response = await runWithImageBridge({ + parsed: makeParsed(), plan, + adapter: { + ...mockAdapter, + runTurn: async (_parsed, _incoming, emit) => { + for (let i = 0; i < 1100; i++) emit({ type: "tool_call_start", id: `call_${i}`, name: "read_file" }); + emit({ type: "done" }); + }, + }, + }); + const sse = await response.text(); + expect(sse).toContain("adapter event backlog exceeded"); + expect(sse).not.toContain("client closed request"); + expect(sse).not.toContain("event: response.completed"); + }); + + test("a completed batch is not fulfilled after its turn signal aborts", async () => { + const abort = new AbortController(); + const adapter: ProviderAdapter = { + ...mockAdapter, + runTurn: async (_parsed, _incoming, emit) => { + for (const event of imageCallEvents) emit(event); + abort.abort("synthetic cancelled turn"); + }, + }; + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan, abortSignal: abort.signal }); + const sse = await response.text(); + expect(sse).not.toContain("event: response.completed"); + expect(fulfillCallCount).toBe(0); + }); + + test("emits after runTurn settles cannot recharge its collection", async () => { + let lateEmit!: (event: AdapterEvent) => void; + let resolveRun!: () => void; + let incomingSignal: AbortSignal | undefined; + const finished = new Promise(resolve => { resolveRun = resolve; }); + const adapter: ProviderAdapter = { + ...mockAdapter, + runTurn: (_parsed, incoming, emit) => { + incomingSignal = incoming.abortSignal; + lateEmit = emit; + // Alternate event types so the queue still has a batch to drain after producer settlement. + for (let i = 0; i < 32; i++) { + emit({ type: "text_delta", text: "finished" }); + emit({ type: "thinking_delta", thinking: "synthetic thought" }); + } + emit({ type: "done" }); + resolveRun(); + return finished; + }, + }; + const response = await runWithImageBridge({ parsed: makeParsed(), adapter, plan }); + const reading = response.text(); + await finished; + // Queue the emit after the bridge's promise completion handler, while the batch can still drain. + await Promise.resolve(); + const abortedBeforeLateEmit = incomingSignal?.aborted; + expect(abortedBeforeLateEmit).toBe(false); + expect(() => lateEmit({ type: "tool_call_start", id: "late", name: "image_gen" })).not.toThrow(); + expect(() => lateEmit({ type: "tool_call_delta", arguments: "x".repeat(TRANSLATOR_MAX_CALL_ARGUMENT_BYTES + 1) })).not.toThrow(); + expect(incomingSignal?.aborted).toBe(abortedBeforeLateEmit); + const sse = await reading; + expect(sse).toContain("event: response.completed"); + expect(sse).not.toContain("translation_buffer_limit"); + expect(fulfillCallCount).toBe(0); + }); + let runTurnEventQueue: AdapterEvent[][] = []; const runTurnAdapter: ProviderAdapter = { ...mockAdapter, From 2fb74c151b44ea249b4837bf23c5dde50231c103 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 13 Sep 2026 16:01:43 +0900 Subject: [PATCH 3/4] fix(codex): distinguish model availability from auth failure Carry #4460 from eefa07442478ef85d258de8693c5d243ad8d0613 onto the lane B chain above the #4388 carry. Lane tip. Account-gated native model selection threw CodexPoolAuthenticationError whenever no configured account carried the entitlement, so Responses, Images, Live and Search all reported HTTP 401 authentication_error with code invalid_api_key. A healthy credential that simply lacks the model looked broken, and users re-logged in and rotated keys for a condition that needed a different model or a reopened quota window. CodexModelAvailabilityError now carries the reason. An entitlement no account holds is 400 invalid_request_error; a model whose capable accounts are all temporarily exhausted is 429 rate_limit_error with code rate_limit_exceeded and no Retry-After, since the reopening time is not known here. Real credential failures keep their 401. Two carry-specific changes on top of the source head. The source branch shipped devlog/_fin/260913_model_availability_errors. A _fin unit records work already visible in public git history, and this work has not landed, so it moves to devlog/_plan and gains the catch-order audit below. The maintainer review asked for one more pass over catches outside Responses, because CodexModelAvailabilityError extends CodexPoolAuthenticationError and a parent-first branch would fold 400 and 429 back into 401. The shared mapper and the Images, Live and Search surfaces all test the subclass first. context-history folds the parent straight to 401 and is left unchanged: it resolves with modelId "context_history", and every throw site is gated on ACCOUNT_GATED_NATIVE_OPENAI_MODELS membership, directly or through modelEligibleAccountIds, which is only populated for a gated model. A new assertion pins that membership so a future gated "context_history" cannot silently restore the invalid_api_key report. encrypted-payload and collaboration import the parent but never branch on it. The new test file is registered in scripts/test-layout/layout.json and tests/fixtures/test-layout-expected.json; the source head omitted both, which fails tests/test-layout-tooling.test.ts. This touches src/codex/auth-context.ts, so pr-hygiene reports unsponsored_surface and MAINTAINERS.md requires security review before it can leave draft. That is expected and unresolved here. Verification on this carry: bun test over tests/codex-integration/codex-model-availability-error.test.ts, tests/codex-integration/codex-auth-context.test.ts, tests/images/loop.test.ts, tests/adapters/run-turn-queue.test.ts and tests/adapters/bridge-nonstreaming-terminal.test.ts (205 pass, 0 fail), plus tests/test-layout.test.ts and tests/test-layout-tooling.test.ts (17 pass), bun run typecheck, bun run structure:check, bun run privacy:scan. Local full suite: NOT RUN. Hosted CI on this tip is the lane's suite proof. Co-authored-by: Shawn <77868633+AgenticLab-SH@users.noreply.github.com> --- .../000_summary.md | 46 +++++++++++++ .../src/content/docs/guides/model-ordering.md | 5 ++ scripts/test-layout/layout.json | 1 + src/codex/auth-context.ts | 55 +++++++++++----- src/server/images.ts | 4 ++ src/server/live.ts | 4 ++ src/server/responses/codex-auth-error.ts | 11 ++++ src/server/search.ts | 3 + structure/runtime.md | 7 +- structure/transports/responses.md | 2 +- .../codex-auth-context.test.ts | 23 +++++-- .../codex-model-availability-error.test.ts | 65 +++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + 13 files changed, 205 insertions(+), 22 deletions(-) create mode 100644 devlog/_plan/260913_model_availability_errors/000_summary.md create mode 100644 tests/codex-integration/codex-model-availability-error.test.ts diff --git a/devlog/_plan/260913_model_availability_errors/000_summary.md b/devlog/_plan/260913_model_availability_errors/000_summary.md new file mode 100644 index 0000000000..3bdeb86214 --- /dev/null +++ b/devlog/_plan/260913_model_availability_errors/000_summary.md @@ -0,0 +1,46 @@ +# Model availability error classification + +Carried from #4460 (AgenticLab-SH) as the tip of lane B in the contributor carry +train. This unit stays in `_plan` until the carry lands on `dev`; a `_fin` record +describes work already visible in public git history, which this is not yet. + +## Problem + +Account-gated native model selection inherited `CodexPoolAuthenticationError`, so every local +compatibility or capacity failure became HTTP 401 `invalid_api_key`. A healthy pool account that +did not support the selected model therefore looked like a broken credential. + +## Change + +- Added typed `unsupported` and `temporarily_unavailable` model-availability reasons. +- Mapped unsupported selections to 400 `invalid_request_error`. +- Mapped temporarily unavailable model-capable pools to 429 `rate_limit_error` with code + `rate_limit_exceeded`. +- Reused the mapping on Responses, Images, Live, and Search surfaces. +- Preserved existing 401 behavior for actual pool credential failures. + +## Verification + +- Focused mapping tests cover 400, 429, and unchanged 401 behavior. +- The existing auth-context regression suite covers account-gated detours, exact selection, + cooldowns, affinity, and reauthentication behavior. + +## Catch-order audit + +`CodexModelAvailabilityError` extends `CodexPoolAuthenticationError`, so any `catch` that +tests the parent first would fold the new 400 and 429 back into 401. Every such site was +audited during the carry: + +- `src/server/responses/codex-auth-error.ts`, `images.ts`, `live.ts` and `search.ts` test the + subclass before the parent. That order is the contract. +- `src/server/context-history.ts` folds the parent straight to 401 and was left unchanged. It + resolves with `modelId: "context_history"`, and every `CodexModelAvailabilityError` throw + site is gated on `ACCOUNT_GATED_NATIVE_OPENAI_MODELS` membership — directly, or through + `modelEligibleAccountIds`, which is only populated for a gated model. The subclass therefore + cannot reach that catch. `tests/codex-integration/codex-model-availability-error.test.ts` + pins the membership that keeps this true. +- `src/server/responses/encrypted-payload.ts` and `collaboration.ts` import the parent but + never branch on it. + +Whether the context-history surface should adopt the same mapping outright is a maintainer +decision recorded on #4460, not a defect in this carry. diff --git a/docs-site/src/content/docs/guides/model-ordering.md b/docs-site/src/content/docs/guides/model-ordering.md index 45ec0666d6..b3ca477259 100644 --- a/docs-site/src/content/docs/guides/model-ordering.md +++ b/docs-site/src/content/docs/guides/model-ordering.md @@ -116,6 +116,11 @@ Use `subagentModels` to choose and order the leading models that Codex also adve choice can expand into multiple selector-qualified catalog rows, so configured choices and advertised rows are not necessarily one-to-one. +If no configured account supports an account-gated native model, the request fails as an invalid +model choice. If supporting accounts exist but are temporarily exhausted or unavailable, it fails +as a retryable rate limit. These states are never reported as an invalid API key; choose another +available model or wait for the capable account's quota window to reopen. + Use `modelPickerOrder` for display-only ordering of routed `/` rows beyond that featured block: diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index ed73c48686..b570c5e4f2 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -475,6 +475,7 @@ "codex-management-convergence.test.ts": "codex-integration", "codex-metadata-integrity.test.ts": "codex-integration", "codex-model-entitlements.test.ts": "codex-integration", + "codex-model-availability-error.test.ts": "codex-integration", "codex-models-cache-invalidate.test.ts": "codex-integration", "codex-native-residue.test.ts": "codex-integration", "codex-plan.test.ts": "codex-integration", diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index 12a1dd6e17..32d124b7fd 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -204,6 +204,19 @@ export class CodexPoolAuthenticationError extends Error { } } +export type CodexModelAvailabilityReason = "unsupported" | "temporarily_unavailable"; + +/** A model/account compatibility failure is not a credential failure. */ +export class CodexModelAvailabilityError extends CodexPoolAuthenticationError { + reason: CodexModelAvailabilityReason; + + constructor(reason: CodexModelAvailabilityReason, message: string) { + super(message); + this.name = "CodexModelAvailabilityError"; + this.reason = reason; + } +} + class CodexAccountValidationPendingError extends CodexPoolAuthenticationError { constructor() { super("Codex account validation is pending; refresh quota after recovery to validate it"); @@ -703,7 +716,10 @@ export async function resolveCodexAuthContext( options.isDirectCallerEntitledToCodexModel ?? isDirectCallerEntitledToCodexModel )(headers, options.modelId); if (!entitled) { - throw new CodexPoolAuthenticationError("The selected ChatGPT account does not support this model"); + throw new CodexModelAvailabilityError( + "unsupported", + "The selected ChatGPT account does not support this model", + ); } } if (callerMatchesObservedMain(headers)) assertMainAccountPolicy(policy); @@ -737,7 +753,10 @@ export async function resolveCodexAuthContext( options.modelId, )?.has(MAIN_CODEX_ACCOUNT_ID) === true; if (!entitled) { - throw new CodexPoolAuthenticationError("The selected ChatGPT account does not support this model"); + throw new CodexModelAvailabilityError( + "unsupported", + "The selected ChatGPT account does not support this model", + ); } } assertMainAccountPolicy(policy); @@ -864,11 +883,13 @@ export async function resolveCodexAuthContext( return await resolveCallerOwnedMainContext(); } if (fixedAccountId !== undefined) { - throw new CodexPoolAuthenticationError( - modelEligibleAccountIds && !modelEligibleAccountIds.has(fixedAccountId) - ? "Selected Codex account does not support this model" - : "Selected Codex account is unavailable", - ); + if (modelEligibleAccountIds && !modelEligibleAccountIds.has(fixedAccountId)) { + throw new CodexModelAvailabilityError( + "unsupported", + "Selected Codex account does not support this model", + ); + } + throw new CodexPoolAuthenticationError("Selected Codex account is unavailable"); } // Recovery or a turn drain deliberately makes physical main unobservable. // If no healthy pool route is available, report the temporary fence rather @@ -884,13 +905,16 @@ export async function resolveCodexAuthContext( && (!modelEligibleAccountIds || modelEligibleAccountIds.has(MAIN_CODEX_ACCOUNT_ID))) { assertMainAccountPolicy(policy); } - throw new CodexPoolAuthenticationError( - modelEligibleAccountIds === undefined - ? undefined - : entitledAccountIds?.size === 0 && !mainModelGrantUnobserved - ? "No eligible Codex account supports this model" - : "Codex accounts that support this model are currently unavailable", - ); + if (modelEligibleAccountIds !== undefined) { + const unsupported = entitledAccountIds?.size === 0 && !mainModelGrantUnobserved; + throw new CodexModelAvailabilityError( + unsupported ? "unsupported" : "temporarily_unavailable", + unsupported + ? "No eligible Codex account supports this model" + : "Codex accounts that support this model are currently unavailable", + ); + } + throw new CodexPoolAuthenticationError(); } accountId = selected; if (accountId === MAIN_CODEX_ACCOUNT_ID) assertMainAccountPolicy(policy); @@ -909,7 +933,8 @@ export async function resolveCodexAuthContext( // Model entitlement is different: sending the request would spend a turn on an account whose // authenticated roster already denied the model. Reassert this boundary after every selector. if (modelEligibleAccountIds && !modelEligibleAccountIds.has(accountId)) { - throw new CodexPoolAuthenticationError( + throw new CodexModelAvailabilityError( + "unsupported", fixedAccountId !== undefined ? "Selected Codex account does not support this model" : "No eligible Codex account supports this model", diff --git a/src/server/images.ts b/src/server/images.ts index 25ff02f3d4..cf9c4d516f 100644 --- a/src/server/images.ts +++ b/src/server/images.ts @@ -19,6 +19,7 @@ import { cooldownErrorResponse, CodexAuthContextError, CodexMainProfileDrainingError, + CodexModelAvailabilityError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, } from "../codex/auth-context"; @@ -49,6 +50,7 @@ import { findXaiProvider, resolveXaiImageAuthToken } from "../images/plan"; import { callXaiImages } from "../images/xai-client"; import type { AdmissionLease } from "../lib/admission"; import { codexAccountSelectionForTurn } from "./lifecycle"; +import { codexModelAvailabilityErrorResponse } from "./responses/codex-auth-error"; export type ImagesEndpoint = "generations" | "edits"; @@ -674,6 +676,8 @@ export async function handleImages( const safeAccountLabel = formatCodexProviderForLog("openai", err.accountId, config); console.error(`[images] Pool account ${safeAccountLabel} token failed; reauthentication required`); forwardAuthError = formatErrorResponse(401, "authentication_error", "Selected Codex account needs reauthentication"); + } else if (err instanceof CodexModelAvailabilityError) { + forwardAuthError = codexModelAvailabilityErrorResponse(err); } else if (err instanceof CodexPoolAuthenticationError) { forwardAuthError = formatErrorResponse(401, "authentication_error", err.message); } else { diff --git a/src/server/live.ts b/src/server/live.ts index 15322f225f..eaca5f14ca 100644 --- a/src/server/live.ts +++ b/src/server/live.ts @@ -35,6 +35,7 @@ import { cooldownErrorResponse, CodexAuthContextError, CodexMainProfileDrainingError, + CodexModelAvailabilityError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, } from "../codex/auth-context"; @@ -48,6 +49,7 @@ import type { RequestLogContext } from "./request-log"; import { codexLogAccountId } from "./responses"; import type { AdmissionLease } from "../lib/admission"; import { codexAccountSelectionForTurn } from "./lifecycle"; +import { codexModelAvailabilityErrorResponse } from "./responses/codex-auth-error"; /** Voice call create can wait on SDP negotiation; bound a hung upstream. */ const LIVE_UPSTREAM_TIMEOUT_MS = 120_000; @@ -565,6 +567,8 @@ export async function resolveLiveRelay( "authentication_error", "Selected Codex account needs reauthentication", ); + } else if (err instanceof CodexModelAvailabilityError) { + forwardAuthError = codexModelAvailabilityErrorResponse(err); } else if (err instanceof CodexPoolAuthenticationError) { forwardAuthError = formatErrorResponse(401, "authentication_error", err.message); } else { diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 8cddd70d6c..12ab9a40fa 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -7,6 +7,7 @@ import { CodexDirectAuthenticationError, CodexMainProfileDrainingError, CodexMainSubstitutionUnavailableError, + CodexModelAvailabilityError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, } from "../../codex/auth-context"; @@ -22,6 +23,13 @@ export interface CodexAuthContextErrorResponseOptions { now: number; } +export function codexModelAvailabilityErrorResponse(error: CodexModelAvailabilityError): Response { + if (error.reason === "temporarily_unavailable") { + return formatErrorResponse(429, "rate_limit_error", error.message); + } + return formatErrorResponse(400, "invalid_request_error", error.message); +} + export function nativeMainRefreshFailureResponse(error: unknown): Response { if (error instanceof MainAccountTokenRefreshError && error.reason === "reauth") { return formatErrorResponse(401, "authentication_error", "Codex main account needs reauthentication"); @@ -86,6 +94,9 @@ export function mapCodexAuthContextErrorToResponse( "Selected Codex account needs reauthentication", ); } + if (error instanceof CodexModelAvailabilityError) { + return codexModelAvailabilityErrorResponse(error); + } if (error instanceof CodexPoolAuthenticationError || error instanceof CodexDirectAuthenticationError) { return formatErrorResponse(401, "authentication_error", error.message); } diff --git a/src/server/search.ts b/src/server/search.ts index 808bcd86c6..681c44254d 100644 --- a/src/server/search.ts +++ b/src/server/search.ts @@ -15,6 +15,7 @@ import { cooldownErrorResponse, CodexAuthContextError, CodexMainProfileDrainingError, + CodexModelAvailabilityError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, } from "../codex/auth-context"; @@ -39,6 +40,7 @@ import type { RequestLogContext } from "./request-log"; import { codexLogAccountId, decodeRequestErrorResponse } from "./responses"; import type { AdmissionLease } from "../lib/admission"; import { codexAccountSelectionForTurn } from "./lifecycle"; +import { codexModelAvailabilityErrorResponse } from "./responses/codex-auth-error"; /** * Default TOTAL deadline for one search relay. alpha/search is non-streaming JSON — response @@ -143,6 +145,7 @@ export async function handleSearch( console.error(`[search] Pool account ${safeAccountLabel} token failed; reauthentication required`); return formatErrorResponse(401, "authentication_error", "Selected Codex account needs reauthentication"); } + if (err instanceof CodexModelAvailabilityError) return codexModelAvailabilityErrorResponse(err); if (err instanceof CodexPoolAuthenticationError) return formatErrorResponse(401, "authentication_error", err.message); throw err; } diff --git a/structure/runtime.md b/structure/runtime.md index c03c14d914..d9134b8aac 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -43,8 +43,11 @@ there. Feature code is grouped by responsibility: `src/server/` is split by responsibility: `index.ts` owns the listener and route ordering; `responses.ts` owns Responses handling and compaction; `images.ts` owns the standalone Images relay; -`responses/codex-auth-error.ts` owns the shared Responses/compact Codex auth-context HTTP mapping, -while account selection, credential materialization, logging, and transport stay in their existing handlers; +`responses/codex-auth-error.ts` owns the shared Responses/compact Codex auth-context HTTP mapping. +Model entitlement denial is a 400 request error and temporary exhaustion of every model-capable +account is a retryable 429; neither is reported as an invalid API key. Images, Live, and Search +reuse that model-availability mapping while retaining their existing credential handling. Account +selection, credential materialization, logging, and transport stay in their existing handlers; `management-api.ts` owns `/api/*`; `lifecycle.ts`, `request-log.ts`, `relay.ts` (incl. the shared `createSseInspector` SSE inspection factory), `relay-eager.ts` (#314 gated eager bounded passthrough relay), `memory-watchdog.ts` diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 96ef06d3c0..41ca530e79 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -519,7 +519,7 @@ policies; kiro imports the shared abort/sleep helpers from this module. ## Same-provider combo quota fallback -For a failover combo with multiple models on the same Codex-login OpenAI provider, a pre-stream +Native account-gated model selection maps no grant to 400, temporary capable-account exhaustion to 429, and actual credential failures to 401; Images, Live, and Search reuse this distinction. For a failover combo with multiple models on the same Codex-login OpenAI provider, a pre-stream 429/402 carrying only `x-codex-*-reset-at` may advance to the later model on the same account. The failed physical combo target still enters its normal target cooldown. An explicit `Retry-After` remains an account-wide instruction and blocks the later target; a quota response with neither an diff --git a/tests/codex-integration/codex-auth-context.test.ts b/tests/codex-integration/codex-auth-context.test.ts index bca4e30808..c23a1cda2c 100644 --- a/tests/codex-integration/codex-auth-context.test.ts +++ b/tests/codex-integration/codex-auth-context.test.ts @@ -10,6 +10,7 @@ import { CodexAuthContextError, CodexDirectAuthenticationError, CodexMainProfileDrainingError, + CodexModelAvailabilityError, CodexPoolAuthenticationError, CodexThreadAffinityExpiredError, codexMainProfileDrainingResponse, @@ -676,9 +677,17 @@ describe("Codex auth context", () => { }); await expect(resolve(["gpt-daybreak-blue-latest"])) - .rejects.toThrow("Codex accounts that support this model are currently unavailable"); + .rejects.toMatchObject({ + name: "CodexModelAvailabilityError", + reason: "temporarily_unavailable", + message: "Codex accounts that support this model are currently unavailable", + } satisfies Partial); await expect(resolve(["gpt-5.6-sol"])) - .rejects.toThrow("No eligible Codex account supports this model"); + .rejects.toMatchObject({ + name: "CodexModelAvailabilityError", + reason: "unsupported", + message: "No eligible Codex account supports this model", + } satisfies Partial); const mainExcludedSnapshot: CodexModelEntitlementSnapshot = { modelsByAccount: new Map(), @@ -697,7 +706,10 @@ describe("Codex auth context", () => { expect(options?.excludeAccountIds?.has(MAIN_CODEX_ACCOUNT_ID)).toBeTrue(); return mainExcludedSnapshot; }, - })).rejects.toThrow("Codex accounts that support this model are currently unavailable"); + })).rejects.toMatchObject({ + reason: "temporarily_unavailable", + message: "Codex accounts that support this model are currently unavailable", + } satisfies Partial); }); test("auth resolution preserves per-model detours without replacing ordinary affinity", async () => { @@ -784,7 +796,10 @@ describe("Codex auth context", () => { accountId: "pool-a", modelId: "gpt-daybreak-blue-latest", resolveCodexModelEntitlements: async () => entitlementSnapshot, - })).rejects.toThrow("Selected Codex account does not support this model"); + })).rejects.toMatchObject({ + reason: "unsupported", + message: "Selected Codex account does not support this model", + } satisfies Partial); }); test("ordinary native models do not pay the entitlement discovery path", async () => { diff --git a/tests/codex-integration/codex-model-availability-error.test.ts b/tests/codex-integration/codex-model-availability-error.test.ts new file mode 100644 index 0000000000..375d3df13d --- /dev/null +++ b/tests/codex-integration/codex-model-availability-error.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from "bun:test"; +import { + CodexModelAvailabilityError, + CodexPoolAuthenticationError, +} from "../../src/codex/auth-context"; +import { ACCOUNT_GATED_NATIVE_OPENAI_MODELS } from "../../src/codex/catalog/native-models"; +import { + codexModelAvailabilityErrorResponse, + mapCodexAuthContextErrorToResponse, +} from "../../src/server/responses/codex-auth-error"; + +describe("Codex model availability HTTP errors", () => { + test("unsupported model is a request error, not invalid_api_key", async () => { + const error = new CodexModelAvailabilityError( + "unsupported", + "No eligible Codex account supports this model", + ); + expect(error).toBeInstanceOf(CodexPoolAuthenticationError); + + const response = mapCodexAuthContextErrorToResponse(error, { now: Date.now() }); + expect(response?.status).toBe(400); + expect(await response?.json()).toEqual({ + error: { + type: "invalid_request_error", + code: "invalid_request_error", + message: "No eligible Codex account supports this model", + }, + }); + }); + + test("temporarily unavailable model is retryable quota capacity", async () => { + const response = codexModelAvailabilityErrorResponse(new CodexModelAvailabilityError( + "temporarily_unavailable", + "Codex accounts that support this model are currently unavailable", + )); + expect(response.status).toBe(429); + expect(response.headers.has("retry-after")).toBeFalse(); + expect(await response.json()).toEqual({ + error: { + type: "rate_limit_error", + code: "rate_limit_exceeded", + message: "Codex accounts that support this model are currently unavailable", + }, + }); + }); + + test("ordinary pool credential failures retain authentication semantics", async () => { + const response = mapCodexAuthContextErrorToResponse( + new CodexPoolAuthenticationError(), + { now: Date.now() }, + ); + expect(response?.status).toBe(401); + expect(await response?.json()).toMatchObject({ + error: { type: "authentication_error", code: "invalid_api_key" }, + }); + }); + + test("the context-history model stays outside the account-gated set", () => { + // src/server/context-history.ts folds CodexPoolAuthenticationError straight to 401, and + // CodexModelAvailabilityError extends it. That catch is safe only because every throw + // site is gated on this membership, so a gated "context_history" would silently restore + // the invalid_api_key report this change exists to remove. + expect(ACCOUNT_GATED_NATIVE_OPENAI_MODELS.has("context_history")).toBeFalse(); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 21bf29eb78..abe6ea2736 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -308,6 +308,7 @@ "codex-management-convergence.test.ts": "codex-integration", "codex-metadata-integrity.test.ts": "codex-integration", "codex-model-entitlements.test.ts": "codex-integration", + "codex-model-availability-error.test.ts": "codex-integration", "codex-models-cache-invalidate.test.ts": "codex-integration", "codex-native-residue.test.ts": "codex-integration", "codex-plan.test.ts": "codex-integration", From bd39d3b5c847fad688fc9ba7f32fb30992fbe392 Mon Sep 17 00:00:00 2001 From: JUN Date: Sun, 13 Sep 2026 16:15:42 +0900 Subject: [PATCH 4/4] test(server): expect 400 for an account-gated model with no grant This branch reports an unsupported account-gated model as 400 invalid_request_error instead of 401 invalid_api_key, and one local API auth case still pinned the old status. It was the only assertion that failed on the hosted suite: ubuntu shard 1/4 and macos 1/2 both failed on it and on nothing else. What the case exists to prove is that the request fails before upstream dispatch, which the empty dispatch list still shows. The status was incidental to that intent and the message assertion is unchanged. Verification: bun test tests/server/server-auth.test.ts (112 pass, 0 fail), bun run typecheck. Co-authored-by: Shawn <77868633+AgenticLab-SH@users.noreply.github.com> --- tests/server/server-auth.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/server/server-auth.test.ts b/tests/server/server-auth.test.ts index b715ff3ede..1e226dd0b8 100644 --- a/tests/server/server-auth.test.ts +++ b/tests/server/server-auth.test.ts @@ -2754,7 +2754,10 @@ describe("server local API auth", () => { ); try { const response = await harness.request({ model }); - expect(response.status).toBe(401); + // No roster carries this model, so the failure is an unsupported selection rather than a + // credential problem: 400 invalid_request_error, not the 401 invalid_api_key this used to + // report. What the case actually pins is the empty dispatch list below. + expect(response.status).toBe(400); expect(await response.text()).toContain("No eligible Codex account supports this model"); expect(harness.dispatches).toEqual([]); } finally {