Repository navigation
fix(responses): surface provider 413 as terminal context overflow - #3177
Conversation
|
✅ Deterministic PR hygiene checks passed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d5eea6a16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The bounded reader owns the original body, deadline, abort settlement, and lock. | ||
| // Unsafe partial data falls back to #452's non-empty status-only JSON. | ||
| const errorText = await readDisplaySafeErrorText(upstreamResponse, upstream.signal, ""); | ||
| if (upstreamResponse.status === 413 && clientRequestedStream) { |
There was a problem hiding this comment.
Preserve 413 for translated non-streaming callers
When a request enters through /v1/chat/completions or /v1/messages with stream:false, both wrappers force the internal Responses body to stream:true, so this condition—and the equivalent translated-adapter condition below—converts the provider's 413 into an HTTP-200 Responses SSE failure. The wrappers then fold that failure into HTTP 502, changing a terminal client-input error into a retryable server error instead of retaining the upstream 413 contract documented by this change. Restrict the conversion to genuine streaming Responses ingress (for example via options.inboundWire) or carry the original caller's stream mode through the replay.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
…dies 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.
…dies (#3196) * 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. * docs(devlog): record live inventory after 3190 --------- Co-authored-by: jun <jun@lidge.dev>
…dge-jun#3177) Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
…dies (lidge-jun#3196) * feat(responses): opt-in ceiling for oversized outbound passthrough bodies Reimplements lidge-jun#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. lidge-jun#2473 acts on that by falling back to HTTP rather than refusing, and lidge-jun#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 (lidge-jun#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 lidge-jun#3142. Related to lidge-jun#2511, which asks for per-provider downscaling and pruning and is deliberately not implemented here. * docs(devlog): record live inventory after 3190 --------- Co-authored-by: jun <jun@lidge.dev>
…dge-jun#3177) Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
…dies (lidge-jun#3196) * feat(responses): opt-in ceiling for oversized outbound passthrough bodies Reimplements lidge-jun#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. lidge-jun#2473 acts on that by falling back to HTTP rather than refusing, and lidge-jun#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 (lidge-jun#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 lidge-jun#3142. Related to lidge-jun#2511, which asks for per-provider downscaling and pruning and is deliberately not implemented here. * docs(devlog): record live inventory after 3190 --------- Co-authored-by: jun <jun@lidge.dev>
Summary
response.failedwithcontext_length_exceeded, so Codex stops its 5/5 reconnect loop and can apply next-turn compactionVerification
Exact head
6d5eea6a163106232242af14335f69f34ca6cad5on base22a643a00b5974fa53b084a04491f60d56ec9ee2:bun run typecheck— passedbun test ./tests/responses-context-overflow.test.ts— 6 passed, 0 failedbun run test:changed— 2,887 passed, 4 skipped, 0 failed across 147 import-connected filescd docs-site && bun run build— passed, 409 pages builtshutdown-launcher.test.ts20-second startup timeouts failing under the constrained runner; the same three failures reproduced unchanged on cleanorigin/devat22a643a00b5974fa53b084a04491f60d56ec9ee2git diff --check origin/dev...HEAD— passedHOME,OPENCODEX_HOME, andCODEX_HOME,nice -n 10, and no more than two CPUs; protected runtime configuration hashes remained unchangedChecklist
Closes #3170.