lidge-jun · lidge-jun · Sep 1, 2026 · Sep 1, 2026 · Sep 1, 2026
@@ -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).
@@ -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.
@@ -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`.
@@ -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. |

@@ -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)

@@ -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,