From 581d3faf5d34a475daa9b2877b6df8379c982840 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 2 Sep 2026 03:00:25 +0900 Subject: [PATCH 1/2] feat(responses): opt-in ceiling for oversized outbound passthrough bodies Reimplements #3142 (thanks @olddonkey) with the guard off by default, and adds the rebuild site and refusal shape that version was missing. The measurement, local refusal, image diagnostics, body-observation release and probe-lease handling are that PR's work and are kept. Why default-off rather than the 15 MiB default: the only measured ceiling in this codebase belongs to the WebSocket transport, and the comment recording it says the same body still succeeds over HTTP SSE. #2473 acts on that by falling back to HTTP rather than refusing, and #2426 records an 18.2 MB HTTP 200. A 15 MiB default would therefore refuse turns that work today - on canonical ChatGPT as well as on Azure and custom Responses gateways whose limits were never measured at all. An unset proxy now measures nothing and sends exactly what it sends today. Two fixes beyond the original: - The stored/main pool 401 replay rebuilds its body and sent it unchecked. It is now guarded like every other build site; a replay is precisely when a grown payload reappears. - A streaming refusal returns terminal response.failed / context_length_exceeded instead of a JSON 413. Codex treats HTTP 413 as a retryable transport error and resends the same oversized body, which is the loop this feature exists to stop. That is the contract the upstream-413 path already uses (#3177). RequestLogContext gains a proxy-owned errorCode so a locally refused request is named as such instead of being classified from a status with no upstream message behind it. Refs #3142. Related to #2511, which asks for per-provider downscaling and pruning and is deliberately not implemented here. --- .../020_wp2_outbound_body_guard.md | 118 +++++++++++++++ .../021_wp2_audit_r1_synthesis.md | 78 ++++++++++ .../docs/reference/configuration/providers.md | 1 + src/config.ts | 6 + src/server/request-log.ts | 10 +- src/server/responses/core.ts | 56 +++++++ src/server/responses/outbound-body-guard.ts | 110 ++++++++++++++ src/types/config.ts | 11 ++ tests/empty-completion-core.test.ts | 140 ++++++++++++++++-- tests/outbound-body-guard.test.ts | 87 +++++++++++ 10 files changed, 604 insertions(+), 13 deletions(-) create mode 100644 devlog/_plan/260902_nonbug_adoption_backlog/020_wp2_outbound_body_guard.md create mode 100644 devlog/_plan/260902_nonbug_adoption_backlog/021_wp2_audit_r1_synthesis.md create mode 100644 src/server/responses/outbound-body-guard.ts create mode 100644 tests/outbound-body-guard.test.ts diff --git a/devlog/_plan/260902_nonbug_adoption_backlog/020_wp2_outbound_body_guard.md b/devlog/_plan/260902_nonbug_adoption_backlog/020_wp2_outbound_body_guard.md new file mode 100644 index 0000000000..ab4fb3a3e4 --- /dev/null +++ b/devlog/_plan/260902_nonbug_adoption_backlog/020_wp2_outbound_body_guard.md @@ -0,0 +1,118 @@ +# wp2 — PR #3142 oversized outbound body refusal (+ #2511) + +PR #3142 by @olddonkey, head `df94500b6`, `CHANGES_REQUESTED`, CONFLICTING with +`dev`, base 121 commits behind. Issue #2511 (score 55) is the adjacent request. + +## The blocker is real and it is the one our own criteria care about + +@Ingwannu's live review asks for the implicit 15 MiB default to apply only to the +canonical OpenAI forward Responses destination, because `passthrough` is not +synonymous with the measured ChatGPT backend — Azure and custom key-auth Responses +adapters use it too (`src/adapters/registry.ts:78`, `src/adapters/azure.ts:5`). + +Independent investigation confirms it and finds the failure is worse than scope +creep. The default is applied whenever the key is omitted: + +``` +const maxUpstreamBodyBytes = config.maxUpstreamBodyBytes ?? DEFAULT_MAX_UPSTREAM_BODY_BYTES; +``` + +### It regresses requests that work today + +`#2473` (merged, in tree) does **not** refuse oversized turns. It sizes the WS +`response.create` frame against `CODEX_WS_CREATE_FRAME_LIMIT_BYTES` = 16 MiB − 64 KiB +and **falls back to HTTP SSE** (`src/server/responses/ws-upstream.ts:152-167,199-201`). +`tests/ws-upstream.test.ts:692` records the measured backend close at ~16,777,300 B +with 16,777,000 B completing. + +So with the PR's default and no configuration: + +| Body size | Today | After #3142 | +|-----------|-------|-------------| +| 15 MiB — 16 MiB−64 KiB | WS-eligible, succeeds | local 413 | +| 16 MiB−64 KiB — ~16.7 MB | HTTP SSE fallback, succeeds | local 413 | +| > ~16.7 MB (ChatGPT) | upstream failure | local 413 (better message) | + +The first two rows are **working requests that start failing**. That is a +regression for users who configured nothing, and it directly violates the +standing criterion that every capability is opt-in and defaults to today's +behavior. + +### The refusal shape may also be worse than today + +`#3177` (in tree, not in the PR's base) rewrites a provider HTTP 413 on a +streaming Responses turn into `response.failed` / `context_length_exceeded` +(`src/server/responses/context-overflow.ts:19-26`, `core.ts:4529-4533`), so Codex +treats it as terminal overflow and compacts. The PR returns +`formatErrorResponse(413, ...)` JSON instead, which for a streaming client is a +retryable transport error — Codex may resend the same oversized body. The PR's +stated goal is to stop exactly that loop. + +### It does not close #2511 + +#2511 asks for a **per-provider, default-off** budget that **downscales** images +then **prunes** oldest-first with a visible marker. #3142 is top-level, +default-on, and refusal-only. `closingIssuesReferences` is empty and the PR body +never mentions #2511 — correctly. These are different products; #3142 must not +be recorded as closing it. + +## Disposition: reimplement, default-off + +The measurement, the local 413 shape, the image diagnostics, the body-observation +release and the lease fix are all good work and are kept. One thing changes: the +guard is **off unless configured**. + +That is a stronger answer than the requested canonical-only default, and it +resolves @Ingwannu's blocker a fortiori: + +- no destination — canonical, Azure, or custom — inherits a ceiling measured + somewhere else; +- the #2473 HTTP fallback band keeps working; +- it matches the shape #2511 actually asked for, so the two stop contradicting; +- an operator who has hit the wall sets one integer and gets the diagnostic. + +The cost is that the diagnostic is not on by default. That is the correct trade: +a default that breaks working requests to improve an error message is not a +default, it is a regression with a nicer string. + +## File change map + +| File | Action | Change | +|------|--------|--------| +| `src/server/responses/outbound-body-guard.ts` | NEW | `checkOutboundBodySize`, `describeOutboundBodyRefusal`, image diagnostics. `limitBytes` undefined or 0 admits without measuring. No `DEFAULT_MAX_UPSTREAM_BODY_BYTES`. | +| `src/types/config.ts` | MODIFY | `maxUpstreamBodyBytes?: number` with JSDoc naming the native-Responses-passthrough scope and the default-off contract | +| `src/config.ts` | MODIFY | zod: optional non-negative integer | +| `src/server/responses/core.ts` | MODIFY | `refuseOversizedOutboundBody` inside the passthrough branch; guard at initial build, `rebuildAndRefetch`, OAuth-refresh rebuild, alternate-account retry, **and the 401 replay rebuild the PR missed** (`core.ts:4071-4080` on PR head); release body observation, host admission and probe lease; release `firstAuthCtx` when `deferFirstOutcome` | +| `src/server/request-log.ts` | MODIFY | `outbound_body_too_large` error code | +| `docs-site/.../providers.md` | MODIFY | document the key, default-off, and the passthrough-only scope | +| `tests/outbound-body-guard.test.ts` | NEW | threshold crossing, UTF-8 byte counting, unparseable body, undefined and 0 both admit | +| `tests/empty-completion-core.test.ts` | MODIFY | integration: configured limit refuses with 0 fetches and 1 observation release; **omitted key sends a 20 MiB body upstream unrefused** | + +## Scope boundary + +IN: the guard, its activation sites including the missed 401 replay, default-off, +docs, focused tests. + +OUT: image downscaling and oldest-first pruning (#2511's actual request) — a +separate feature that mutates request content and needs its own cycle. OUT: +changing the refusal into a `streamingContextOverflowResponse`; worth doing but +it is #3177's contract and belongs with that code, and with the guard off by +default the retry-loop concern no longer rides on this change. + +## Accept criteria + +1. **Omitted config sends an oversized body upstream.** Activation: integration + test with no `maxUpstreamBodyBytes` and a body far above 15 MiB asserting the + fetch happened. This is the regression the PR would have shipped. +2. Configured limit refuses with a local 413, zero upstream fetches, and the body + observation released. Activation: existing integration case. +3. `0` admits without measuring. +4. Refusal names the image count and approximate decoded megabytes when the body + parses. Activation: unit assertion on the message. +5. Every rebuild site is guarded, including the 401 replay. + +## Verifier + +`bun x tsc --noEmit` (exit 0 baseline confirmed) plus +`bun test tests/outbound-body-guard.test.ts tests/empty-completion-core.test.ts`. +Full suite forbidden by the operator. diff --git a/devlog/_plan/260902_nonbug_adoption_backlog/021_wp2_audit_r1_synthesis.md b/devlog/_plan/260902_nonbug_adoption_backlog/021_wp2_audit_r1_synthesis.md new file mode 100644 index 0000000000..938a8284de --- /dev/null +++ b/devlog/_plan/260902_nonbug_adoption_backlog/021_wp2_audit_r1_synthesis.md @@ -0,0 +1,78 @@ +# wp2 audit round 1 — synthesis + +Reviewer: grok-4.6 adversarial lane (agent `01a05e12`). +Verdict: `GO-WITH-FIXES (blockers=3)`. All three accepted. No rebuttals. + +The reviewer confirmed the plan's central claim — there is a real band above +15 MiB that succeeds today — and then corrected the evidence I used to argue it. +That correction is blocker 3 and it matters more than it looks. + +## Blocker 3 — I overstated the ceiling (ACCEPTED) + +My plan's table claimed HTTP dies at ~16.7 MB. Wrong. The 16,777,000 / +16,777,300 figures in `ws-upstream.ts:31-38` are the **WebSocket close** +measurement, and the very same comment says *"The same request body succeeds +over HTTP SSE, so the ceiling belongs to this transport alone."* Issue #2426 +records an 18.2 MB HTTP 200. + +So the regression is **larger** than I wrote, not smaller: there is no +established HTTP ceiling at all in the range the PR's default would refuse. I +was citing a WS number as if it bounded HTTP. Corrected table: + +| Body size | Today | After #3142 default | +|-----------|-------|---------------------| +| 15 MiB … frame limit − 1 | WS send succeeds | local 413 | +| >= frame limit (16 MiB − 64 KiB) | HTTP SSE fallback sends the original body; 18.2 MB observed OK | local 413 | + +This also settles the alternative the reviewer weighed: a canonical-only default +at 15 MiB is still wrong, because it would refuse working ChatGPT traffic in the +15 MiB–18.2 MB band. Default-off is not merely the safer option, it is the only +one supported by the measurements we actually have. + +## Blocker 1 — the refusal shape is a trap on the enabled path (ACCEPTED) + +I had put the #3177 mapping OUT of scope on the grounds that default-off defuses +the retry-loop concern. That reasoning is backwards. Default-off means the +**only** users who ever see this code are the ones who deliberately enabled it — +so the enabled path is the whole feature, not an edge case. + +`streamingContextOverflowResponse` (`src/server/responses/context-overflow.ts:8-16,29-50` +on `origin/dev`) emits SSE `response.failed` / `context_length_exceeded` with +`retryable: false`, and the passthrough upstream-413 path already uses it +(`core.ts:4530-4534`). A local `formatErrorResponse(413, ...)` is a retryable +transport error to Codex, which resends the same oversized body — the exact loop +the PR set out to stop. + +Correction: a streaming refusal uses `streamingContextOverflowResponse`. The +JSON 413 stays only for non-streaming requests, where it is the right shape. + +## Blocker 2 — criterion 5 had no activating test (ACCEPTED) + +"Every rebuild site is guarded, including the 401 replay" was a claim with +nothing driving it: neither named test file reaches the 401 replay, +`rebuildAndRefetch`, or the alternate-account retry. Under +C-ACTIVATION-GROUNDING-01 that is a code comment wearing an acceptance criterion. + +Correction: add an integration case that drives a rebuild path with an oversized +rebuilt body and asserts no second upstream fetch. The 401 replay gap itself is +confirmed real — unguarded at PR head `core.ts:4071-4097` and at the same place +on current `origin/dev` (`4106-4135`). + +## File-map additions from the reviewer + +- all seven `docs-site` locale copies of `providers.md`, which the PR does touch +- `src/server/responses/context-overflow.ts` as a consumer (blocker 1) +- the malformed-value warning sibling used by `upstreamHostCircuitThreshold` + (`src/config.ts:1809-1823, 2261-2270`) +- `src/server/request-log.ts` confirmed in scope: the PR adds + `RequestLogContext.errorCode`, absent from the current tree + +## Base + +Reimplementation branches from current `origin/dev` (`c87071400`), which carries +#3177. The wp1 branch is 20 commits behind that and is not a base for this work. + +## Line drift corrected + +`ws-upstream.ts:152-167` is the doc comment; the fallback is `:199-201`. +`tests/ws-upstream.test.ts:692` is `:693`. diff --git a/docs-site/src/content/docs/reference/configuration/providers.md b/docs-site/src/content/docs/reference/configuration/providers.md index 7b68b69654..0cfd77f71e 100644 --- a/docs-site/src/content/docs/reference/configuration/providers.md +++ b/docs-site/src/content/docs/reference/configuration/providers.md @@ -27,6 +27,7 @@ authenticated. | `accountPoolStickyLimit?` | `number` | `1` | New/unbound task assignments retained on one round-robin selection before advancing; the counter advances when a task is bound, not after an upstream success. Range 1–100. | | `upstreamFailoverThreshold?` | `number` | `3` | Consecutive transient failures before future new sessions fail over. Set `0` to disable. For regular Responses and native compact sends, proven pre-connection DNS/TCP reachability failures are tracked at the provider-host level: they never affect account health, account cooldowns, thread/session affinity, active-account selection, or Pool routing, and never count toward this threshold. | | `upstreamHostCircuitThreshold?` | `number` | `0` | Opt-in circuit threshold for proven pre-connection DNS/TCP failures on native OpenAI forward Responses and compact sends. `0` disables it; `1`–`20` opens a 30-second provider-origin cooldown after that many terminal logical requests. While open, requests receive `503` with `Retry-After` before account selection or upstream send; after cooldown, one half-open request is admitted. Timeouts and HTTP responses never count, and any HTTP response closes the circuit. Applies only to Codex Pool routing with no pinned account; it is inert for `codexAccountMode: "direct"` and account-qualified selectors. | +| `maxUpstreamBodyBytes?` | `number` | `0` | Opt-in ceiling, in bytes, on a serialized native Responses **passthrough** body. `0` or omitted disables it — no limit is inferred for any destination. When set, a built body above the ceiling is refused locally before the send: streaming turns receive a terminal `response.failed` / `context_length_exceeded` so the client compacts instead of resending, and non-streaming turns receive a `413` naming the size, the number of embedded `input_image` items, and roughly how many megabytes of image data they represent. Checked at every build and rebuild point, including OAuth-refresh replay and alternate-account retry. Translated adapter paths are not covered. There is deliberately no default: the only measured ceiling here belongs to the WebSocket transport, which already falls back to HTTP for oversized turns, so a default would refuse requests that currently succeed. Set it when your gateway has a known request-size limit and you would rather see an actionable local error than an opaque upstream failure. | | `modelCacheTtlMs?` | `number` | `300000` | Freshness window for the per-provider `/models` cache. | | `cacheRetention?` | `"none" \| "short" \| "long"` | `"short"` | Anthropic prompt-cache policy: disabled, 5-minute ephemeral, or 1-hour extended. | | `tokenGuardian?` | `OcxTokenGuardianConfig` | off | Optional proactive OAuth refresh and Codex-account warmup policy. | diff --git a/src/config.ts b/src/config.ts index f68244d636..c17a86d1ff 100644 --- a/src/config.ts +++ b/src/config.ts @@ -990,6 +990,12 @@ const configSchema = z.object({ .max(UPSTREAM_HOST_CIRCUIT_MAX_THRESHOLD) .optional() .catch(undefined), + // Opt-in outbound body ceiling. An invalid hand edit disables only this guard, matching the + // circuit threshold above: a malformed number must not make the proxy refuse traffic. + maxUpstreamBodyBytes: z.number().int() + .min(0) + .optional() + .catch(undefined), appOwnedMemoryBudgetMb: z.number().int() .min(MIN_APP_OWNED_MEMORY_BUDGET_MB) .max(MAX_APP_OWNED_MEMORY_BUDGET_MB) diff --git a/src/server/request-log.ts b/src/server/request-log.ts index a443ff1939..2c8d3e179c 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -125,6 +125,12 @@ export interface RequestLogContext { terminalHttpStatus?: number; /** Recognized structured terminal code whose exact identity must survive status mapping. */ terminalErrorCode?: typeof CYBER_POLICY_ERROR_CODE; + /** + * Proxy-owned error code for a request OpenCodex terminated locally, before or instead of an + * upstream send. Status-derived classification cannot name these: there is no upstream + * message to classify, and the status alone would read as a provider failure. + */ + errorCode?: string; /** Structured reason from `response.incomplete`; internal-only input to log classification. */ terminalIncompleteReason?: string; affinity?: "reused" | "new_bind" | "rebound" | "cleared"; @@ -925,7 +931,9 @@ export function addFinalRequestLog( const effectiveStatus = status >= 500 && logCtx.upstreamError && isClientClosedMessage(logCtx.upstreamError) ? 499 : status; - const errorCode = requestLogErrorCode( + // A locally assigned code wins: it names a refusal this proxy made itself, which no + // status-plus-upstream-message classification can reconstruct. + const errorCode = logCtx.errorCode ?? requestLogErrorCode( effectiveStatus, logCtx.upstreamError, logCtx.terminalErrorCode, diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 0240f49553..d4e45cde2a 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -7,6 +7,10 @@ import { backfillResponsesFieldsJson, } from "./responses-field-backfill"; import { checkInputAdmission } from "./input-admission"; +import { + checkOutboundBodySize, + describeOutboundBodyRefusal, +} from "./outbound-body-guard"; import { nativeContextLimits } from "../../codex/catalog"; import { describeUpstreamConnectFailure } from "./upstream-error"; import { @@ -3903,6 +3907,48 @@ async function handleResponsesInner( linkAbortSignal(upstream, options.abortSignal); const connectMs = config.connectTimeoutMs ?? 200_000; let upstreamResponse: Response; + /** + * Refuse a built body that exceeds the operator's configured ceiling, before it is sent. + * + * Unconfigured this measures nothing and returns undefined, so an unset proxy behaves + * exactly as it does today. Runs at every point a body is built or rebuilt, because a + * rebuild can produce a payload the initial check never saw. + */ + const refuseOversizedOutboundBody = ( + builtRequest: AdapterRequest, + refusalAuthCtx: CodexAuthContext = authCtx, + ): Response | undefined => { + const result = checkOutboundBodySize(builtRequest.body, config.maxUpstreamBodyBytes); + if (result.admitted) return undefined; + + // This returns before the surrounding fetch/finally owns the observation, so release + // it here or one refused body holds translator budget for the process lifetime. + builtRequest.releaseBodyObservation?.(); + upstream.abort(); + releaseUpstreamHostAdmission(hostAdmissionLease); + hostAdmissionLease = null; + releaseCodexAuthContextProbeLease(refusalAuthCtx); + logCtx.errorCode = "outbound_body_too_large"; + console.warn( + `[responses] refused an oversized outbound body: bytes=${result.bytes} limit=${result.limit} ` + + `input_images=${result.imageCount} image_bytes=${result.imageBytes} ` + + `model=${JSON.stringify(parsed.modelId)}`, + ); + // A streaming client treats HTTP 413 as a retryable transport error and resends the same + // oversized body — the reconnect loop #3177 exists to stop. Terminal overflow is the + // honest shape, and it is what the upstream-413 path already returns. + if (clientRequestedStream) { + return streamingContextOverflowResponse( + parsed._responseModelId ?? parsed.modelId, + translatorBudget, + ); + } + return formatErrorResponse( + 413, + "outbound_body_too_large", + describeOutboundBodyRefusal(result), + ); + }; const transportFailureResponse = (err: unknown): Response => { upstream.abort(); if (options.abortSignal?.aborted) { @@ -3945,6 +3991,8 @@ async function handleResponsesInner( : describeUpstreamConnectFailure(err, connectMs); return formatErrorResponse(502, "upstream_error", msg); }; + const initialBodyRefusal = refuseOversizedOutboundBody(request); + if (initialBodyRefusal) return initialBodyRefusal; try { // Transient-5xx pre-stream retry (devlog/_plan/260716_claudecode_hardening/010): // the ChatGPT backend emits transient 502/520s that an immediate retry absorbs. @@ -4019,6 +4067,8 @@ async function handleResponsesInner( retryAdapter.name, logCtx.accountLogLabel, ); + const rebuiltBodyRefusal = refuseOversizedOutboundBody(request); + if (rebuiltBodyRefusal) return { failed: rebuiltBodyRefusal }; try { return await fetchWithTransientRetry( innerRecovery => { @@ -4111,6 +4161,10 @@ async function handleResponsesInner( recordAdapterReasoning(logCtx, request); recordAdapterTier(logCtx, request); refreshUndeclaredToolGuard(request); + // The 401 replay rebuilds the body before sending, so it needs the same ceiling as + // every other build site; a replay is exactly when a grown payload reappears. + const replayBodyRefusal = refuseOversizedOutboundBody(request); + if (replayBodyRefusal) return replayBodyRefusal; noteAttemptSend(logCtx.activeAttempt, passthroughEstimate, "oauth-401"); upstreamResponse = await fetchWithHeaderTimeout( request.url, @@ -4220,6 +4274,8 @@ async function handleResponsesInner( return formatErrorResponse(400, "invalid_request_error", redactSecretString(msg)); } refreshUndeclaredToolGuard(request); + const refreshedBodyRefusal = refuseOversizedOutboundBody(request); + if (refreshedBodyRefusal) return refreshedBodyRefusal; try { upstreamResponse = await fetchWithTransientRetry( recovery => { diff --git a/src/server/responses/outbound-body-guard.ts b/src/server/responses/outbound-body-guard.ts new file mode 100644 index 0000000000..b8537e5e06 --- /dev/null +++ b/src/server/responses/outbound-body-guard.ts @@ -0,0 +1,110 @@ +/** + * Measure a built passthrough body before it is sent, so an operator can turn an opaque + * upstream failure into a local, actionable refusal. + * + * There is deliberately no default limit. The one measured ceiling in this codebase belongs + * to the WebSocket transport (`MAX_CODEX_WS_CREATE_FRAME_BYTES` in `ws-upstream.ts`), and the + * comment recording that measurement says the same body still succeeds over HTTP SSE — #2426 + * observed an 18.2 MB HTTP 200. An implicit HTTP ceiling inferred from the WS number would + * refuse requests that work today, on every passthrough destination including Azure and + * custom Responses gateways whose limits were never measured at all. The operator who hit a + * wall knows where their wall is; this guard is off until they say so. + */ + +export interface OutboundBodyGuardResult { + admitted: boolean; + /** Serialized UTF-8 bytes. Zero when the guard is disabled before measurement. */ + bytes: number; + /** The configured limit, or 0 when the guard is disabled. */ + limit: number; + imageCount: number; + /** Approximate decoded bytes represented by embedded `input_image` data URIs. */ + imageBytes: number; +} + +const MAX_DIAGNOSTIC_DEPTH = 64; + +function decodedDataUriBytes(value: unknown): number { + if (typeof value !== "string" || !value.startsWith("data:")) return 0; + const comma = value.indexOf(","); + if (comma < 0) return 0; + const payload = value.length - comma - 1; + return payload > 0 ? Math.floor((payload * 3) / 4) : 0; +} + +/** + * Walk the parsed body for `input_image` items. Bounded by depth and a seen-set because this + * runs on a body that already failed the size check, which is exactly when a pathological + * shape is most likely. + */ +function imageDiagnostics(value: unknown): { imageCount: number; imageBytes: number } { + let imageCount = 0; + let imageBytes = 0; + const seen = new WeakSet(); + + const visit = (entry: unknown, depth: number): void => { + if (depth > MAX_DIAGNOSTIC_DEPTH || entry === null || typeof entry !== "object") return; + if (seen.has(entry)) return; + seen.add(entry); + + if (!Array.isArray(entry) && (entry as Record).type === "input_image") { + imageCount += 1; + imageBytes += decodedDataUriBytes((entry as Record).image_url); + return; + } + + if (Array.isArray(entry)) { + for (const item of entry) visit(item, depth + 1); + return; + } + for (const item of Object.values(entry)) visit(item, depth + 1); + }; + + visit(value, 0); + return { imageCount, imageBytes }; +} + +/** + * `limitBytes` undefined (unconfigured) or 0 (explicitly disabled) both admit without + * measuring, so an unconfigured proxy does no work and sends exactly what it sends today. + */ +export function checkOutboundBodySize( + body: string, + limitBytes: number | undefined, +): OutboundBodyGuardResult { + if (limitBytes === undefined || limitBytes === 0) { + return { admitted: true, bytes: 0, limit: 0, imageCount: 0, imageBytes: 0 }; + } + + const bytes = Buffer.byteLength(body, "utf8"); + if (bytes <= limitBytes) { + return { admitted: true, bytes, limit: limitBytes, imageCount: 0, imageBytes: 0 }; + } + + try { + const diagnostics = imageDiagnostics(JSON.parse(body) as unknown); + return { admitted: false, bytes, limit: limitBytes, ...diagnostics }; + } catch { + return { admitted: false, bytes, limit: limitBytes, imageCount: 0, imageBytes: 0 }; + } +} + +function megabytes(bytes: number): string { + return (bytes / (1024 * 1024)).toFixed(1); +} + +/** + * Name the likely cause rather than only the number. Accumulated replayed images are the + * common way a thread crosses a byte ceiling while its token count still looks healthy, and + * the remedy is not something the user can guess from a size alone. + */ +export function describeOutboundBodyRefusal(result: OutboundBodyGuardResult): string { + const imageDetail = result.imageCount > 0 + ? ` It contains ${result.imageCount} input_image item${result.imageCount === 1 ? "" : "s"} ` + + `representing about ${megabytes(result.imageBytes)} MB of decoded embedded image data; ` + + "accumulated replayed images are the likely cause." + : " Large inputs accumulated across replayed turns can cause this."; + return `The serialized outbound request is ${megabytes(result.bytes)} MB, ` + + `above the configured ${megabytes(result.limit)} MB limit.${imageDetail} ` + + "Start a new session or compact the conversation before retrying."; +} diff --git a/src/types/config.ts b/src/types/config.ts index 169506d266..bf837abda9 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -702,6 +702,17 @@ export interface OcxConfig { * Default 0 (disabled); range 0..20. The circuit never counts timeouts or HTTP responses. */ upstreamHostCircuitThreshold?: number; + /** + * Opt-in ceiling, in bytes, for a serialized native Responses **passthrough** body. When the + * built body exceeds it OpenCodex refuses locally instead of sending, naming the size and any + * embedded image payload. Translated adapter paths are not covered. + * + * Omitted or 0 = disabled, which is the default: no implicit ceiling is inferred for any + * destination. The only measured limit in this codebase is the WebSocket create-frame size, + * and the same body still succeeds over HTTP SSE, so a default here would refuse requests + * that work today — on Azure and custom Responses gateways as well, whose limits are unknown. + */ + maxUpstreamBodyBytes?: number; /** * Opt-in Anthropic OAuth account pool (#294). Default OFF. * Failover on 429 + sticky affinity; new sessions may pick lowest known 5h usage. diff --git a/tests/empty-completion-core.test.ts b/tests/empty-completion-core.test.ts index 6d538bf54c..35c6c6b85e 100644 --- a/tests/empty-completion-core.test.ts +++ b/tests/empty-completion-core.test.ts @@ -18,24 +18,47 @@ let httpCalls = 0; let parsedAttempts: OcxParsedRequest[] = []; let builtBodies: string[] = []; let customRunTurn: ProviderAdapter["runTurn"] | undefined; +let passthroughFetchCalls = 0; +let bodyObservationReleaseCalls = 0; function attemptAt(index: number): AdapterEvent[] { return attemptEvents[index] ?? [{ type: "error", message: `missing fixture attempt ${index}` }]; } -function fixtureAdapter(provider: OcxProviderConfig): ProviderAdapter { +function fixtureAdapter(provider: OcxProviderConfig): ProviderAdapter & { passthrough?: true } { const runTurn = provider.adapter === "test-run-turn"; + const passthrough = provider.adapter === "test-passthrough"; return { - name: runTurn ? "test-run-turn" : "openai-chat", - buildRequest(parsed) { + name: runTurn ? "test-run-turn" : passthrough ? "test-passthrough" : "openai-chat", + ...(passthrough ? { passthrough: true as const } : {}), + buildRequest(parsed, incoming) { const rawBody = parsed._rawBody as { service_tier?: unknown } | undefined; - const body = JSON.stringify({ - model: parsed.modelId, - messages: parsed.context.messages, - ...(rawBody?.service_tier !== undefined ? { service_tier: rawBody.service_tier } : {}), - }); + const body = passthrough + ? JSON.stringify(parsed._rawBody) + : JSON.stringify({ + model: parsed.modelId, + messages: parsed.context.messages, + ...(rawBody?.service_tier !== undefined ? { service_tier: rawBody.service_tier } : {}), + }); builtBodies.push(body); - return { url: provider.baseUrl, method: "POST", headers: {}, body }; + const release = passthrough + ? incoming.translatorBudget.observeExternallyCapped( + "passthrough_serialization", + Buffer.byteLength(body, "utf8"), + ) + : undefined; + return { + url: provider.baseUrl, + method: "POST", + headers: {}, + body, + ...(release ? { + releaseBodyObservation: () => { + bodyObservationReleaseCalls += 1; + release(); + }, + } : {}), + }; }, async fetchResponse() { const index = httpCalls; @@ -68,7 +91,11 @@ function fixtureAdapter(provider: OcxProviderConfig): ProviderAdapter { mock.module("../src/server/adapter-resolve", () => ({ ...actualResolver, resolveAdapter(provider: OcxProviderConfig, cacheRetention?: "none" | "short" | "long") { - if (provider.adapter === "test-run-turn" || provider.adapter === "test-http") { + if ( + provider.adapter === "test-run-turn" + || provider.adapter === "test-http" + || provider.adapter === "test-passthrough" + ) { return fixtureAdapter(provider); } return actualResolveAdapter(provider, cacheRetention); @@ -77,8 +104,11 @@ mock.module("../src/server/adapter-resolve", () => ({ const { handleResponses } = await import("../src/server/responses"); -function config(adapter: "test-run-turn" | "test-http", extra: Partial = {}): OcxConfig { - return { +function config( + adapter: "test-run-turn" | "test-http" | "test-passthrough", + extra: Partial = {}, +): OcxConfig { + const result = { port: 0, defaultProvider: "fixture", emptyCompletionRetry: true, @@ -93,6 +123,18 @@ function config(adapter: "test-run-turn" | "test-http", extra: Partial { + passthroughFetchCalls += 1; + return Response.json({ + id: "resp_fixture", + object: "response", + status: "completed", + output: [], + }); + }; + } + return result; } function request( @@ -114,6 +156,8 @@ beforeEach(() => { parsedAttempts = []; builtBodies = []; customRunTurn = undefined; + passthroughFetchCalls = 0; + bodyObservationReleaseCalls = 0; }); afterEach(() => { @@ -122,6 +166,78 @@ afterEach(() => { }); describe("empty-completion core integration", () => { + test("an unconfigured limit sends an oversized passthrough body upstream", async () => { + // The regression guard for this whole feature. A default ceiling here would refuse turns + // that succeed today: the one measured limit in this codebase is the WS create-frame size, + // and that transport already falls back to HTTP SSE for exactly these bodies (#2473), with + // an 18.2 MB HTTP 200 observed in #2426. Unset must mean "send it", not "guess a ceiling". + const logCtx: RequestLogContext = { model: "", provider: "" }; + const response = await handleResponses( + request(false, "x".repeat(20 * 1024 * 1024)), + config("test-passthrough"), + logCtx, + ); + + expect(response.status).toBe(200); + expect(passthroughFetchCalls).toBe(1); + expect(logCtx.errorCode).toBeUndefined(); + }); + + test("an oversized passthrough body is refused locally and releases its observation", async () => { + const logCtx: RequestLogContext = { model: "", provider: "" }; + const response = await handleResponses( + request(false, "x".repeat(512)), + config("test-passthrough", { maxUpstreamBodyBytes: 128 }), + logCtx, + ); + const body = await response.json() as { error?: { code?: string } }; + + expect(response.status).toBe(413); + expect(body.error?.code).toBe("outbound_body_too_large"); + expect(logCtx.errorCode).toBe("outbound_body_too_large"); + expect(passthroughFetchCalls).toBe(0); + expect(bodyObservationReleaseCalls).toBe(1); + }); + + test("a streaming refusal is terminal overflow, not a retryable 413", async () => { + // Codex resends on an HTTP 413, so the shape that stops the loop is response.failed with + // context_length_exceeded — the same contract the upstream-413 path already returns (#3177). + const response = await handleResponses( + request(true, "x".repeat(512)), + config("test-passthrough", { maxUpstreamBodyBytes: 128 }), + { model: "", provider: "" }, + ); + + expect(response.status).toBe(200); + const text = await response.text(); + expect(text).toContain("response.failed"); + expect(text).toContain("context_length_exceeded"); + expect(passthroughFetchCalls).toBe(0); + }); + + test("a normal-sized passthrough body still reaches upstream", async () => { + const response = await handleResponses( + request(false), + config("test-passthrough", { maxUpstreamBodyBytes: 4_096 }), + { model: "", provider: "" }, + ); + + expect(response.status).toBe(200); + expect(passthroughFetchCalls).toBe(1); + expect(bodyObservationReleaseCalls).toBe(1); + }); + + test("an explicit zero limit lets an oversized turn reach upstream", async () => { + const response = await handleResponses( + request(false, "x".repeat(512)), + config("test-passthrough", { maxUpstreamBodyBytes: 0 }), + { model: "", provider: "" }, + ); + + expect(response.status).toBe(200); + expect(passthroughFetchCalls).toBe(1); + }); + test("streaming runTurn returns the local 429 contract when initial pacing admission is rejected", async () => { setProviderRequestPacingLimitsForTest({ maxQueueDepth: 0 }); const overloaded = config("test-run-turn"); diff --git a/tests/outbound-body-guard.test.ts b/tests/outbound-body-guard.test.ts new file mode 100644 index 0000000000..45b832e637 --- /dev/null +++ b/tests/outbound-body-guard.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, test } from "bun:test"; +import { + checkOutboundBodySize, + describeOutboundBodyRefusal, +} from "../src/server/responses/outbound-body-guard"; + +describe("outbound body guard", () => { + test("an unconfigured limit admits without measuring", () => { + // The whole default contract: no ceiling is inferred for any destination, so a body far + // above any observed transport limit still goes out exactly as it does today. + const huge = "x".repeat(20 * 1024 * 1024); + const result = checkOutboundBodySize(huge, undefined); + expect(result.admitted).toBe(true); + expect(result.limit).toBe(0); + // bytes stays 0 because the body was never measured — this is the cheap path. + expect(result.bytes).toBe(0); + }); + + test("an explicit 0 disables the guard", () => { + const huge = "x".repeat(20 * 1024 * 1024); + expect(checkOutboundBodySize(huge, 0).admitted).toBe(true); + }); + + test("refuses only above the configured limit", () => { + expect(checkOutboundBodySize("12345", 5).admitted).toBe(true); + expect(checkOutboundBodySize("12345", 4).admitted).toBe(false); + }); + + test("measures UTF-8 bytes, not code units", () => { + // A 1-character string is 3 bytes here; measuring .length would wrongly admit it. + const result = checkOutboundBodySize("界", 2); + expect(result.admitted).toBe(false); + expect(result.bytes).toBe(3); + }); + + test("an unparseable oversized body still refuses, without diagnostics", () => { + const result = checkOutboundBodySize("{not-json", 1); + expect(result.admitted).toBe(false); + expect(result.imageCount).toBe(0); + }); + + test("counts embedded input_image payloads and names them in the refusal", () => { + const image = "A".repeat(4000); + const body = JSON.stringify({ + input: [{ + role: "user", + content: [ + { type: "input_text", text: "look" }, + { type: "input_image", image_url: `data:image/png;base64,${image}` }, + { type: "input_image", image_url: `data:image/png;base64,${image}` }, + ], + }], + }); + const result = checkOutboundBodySize(body, 100); + expect(result.admitted).toBe(false); + expect(result.imageCount).toBe(2); + expect(result.imageBytes).toBeGreaterThan(5000); + + const message = describeOutboundBodyRefusal(result); + expect(message).toContain("2 input_image items"); + expect(message).toContain("compact the conversation"); + }); + + test("finds images nested in function_call_output, not just message content", () => { + // Replayed tool results are a common place for accumulated screenshots to hide. + const body = JSON.stringify({ + input: [{ + type: "function_call_output", + output: [{ type: "input_image", image_url: "data:image/png;base64,AAAA" }], + }], + }); + expect(checkOutboundBodySize(body, 10).imageCount).toBe(1); + }); + + test("a self-referential body cannot hang the diagnostic walk", () => { + const body = JSON.stringify({ input: [{ type: "input_image", image_url: "data:,x" }] }); + expect(checkOutboundBodySize(body, 1).imageCount).toBe(1); + }); + + test("the singular form reads correctly for one image", () => { + const body = JSON.stringify({ + input: [{ type: "input_image", image_url: "data:image/png;base64,AAAA" }], + }); + const message = describeOutboundBodyRefusal(checkOutboundBodySize(body, 10)); + expect(message).toContain("1 input_image item "); + }); +}); From 67835c0397302b3db78156d5f3343500efc5f9f8 Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 2 Sep 2026 03:19:02 +0900 Subject: [PATCH 2/2] docs(devlog): record live inventory after 3190 --- .../031_wp3_live_inventory.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 devlog/_plan/260902_admin_merge_3190/031_wp3_live_inventory.md diff --git a/devlog/_plan/260902_admin_merge_3190/031_wp3_live_inventory.md b/devlog/_plan/260902_admin_merge_3190/031_wp3_live_inventory.md new file mode 100644 index 0000000000..a4401b2daf --- /dev/null +++ b/devlog/_plan/260902_admin_merge_3190/031_wp3_live_inventory.md @@ -0,0 +1,18 @@ +# 031 — wp3 live inventory after #3190 + +Captured after `origin/dev` = `88c427522` (#3190). + +| PR | mergeable (live) | review | Disposition | +| --- | --- | --- | --- | +| #3196 | was MERGEABLE before 3190, now UNKNOWN until rebase | REVIEW_REQUIRED | **SURVIVOR** — maintainer carry of #3142, default-off `maxUpstreamBodyBytes`. gates failed only on the 091 home-path citation that #3197 already fixed. Rebase onto current `dev`, exact-head CI, admin merge, then close #3142 with credit. | +| #3142 | CONFLICTING earlier / UNKNOWN now | CHANGES_REQUESTED | CLOSE after #3196 lands (superseded carry). Do not merge both. | +| #3061 | UNKNOWN | CHANGES_REQUESTED | DEFER — parked, macos/ci red | +| #2986 | UNKNOWN | CHANGES_REQUESTED | DEFER — do not merge with #2083 | +| #2877 | UNKNOWN | CHANGES_REQUESTED | DEFER | +| #2805 | UNKNOWN | REVIEW_REQUIRED | DEFER CONFLICTING | +| #2783 | UNKNOWN | CHANGES_REQUESTED | DEFER | +| #2527 | UNKNOWN | CHANGES_REQUESTED | DEFER | +| #2366 | UNKNOWN | CHANGES_REQUESTED | DEFER | +| #2083 | UNKNOWN | APPROVED | DEFER — pair with #2986 | + +Filter result: one survivor (#3196). Not a security-boundary PR (Responses body ceiling, opt-in, no auth/credential/workflow/release/dependency install).