fix(responses): refuse oversized input and stop compounding replayed history - #1412
fix(responses): refuse oversized input and stop compounding replayed history#1412HoshimiRox1 wants to merge 2 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds canonical overlap detection for chained response history and a pre-forwarding context-window guard. Full-history requests avoid duplicate items, delta requests retain stored history, and oversized inputs return HTTP 413. ChangesResponse replay overlap
Responses input context guard
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleResponses
participant expandPreviousResponseInput
participant Upstream
Client->>handleResponses: Submit chained response request
handleResponses->>expandPreviousResponseInput: Provide previous_response_id and input
expandPreviousResponseInput->>expandPreviousResponseInput: Compare canonical replay prefix
expandPreviousResponseInput-->>handleResponses: Preserve full history or prepend stored history
handleResponses->>Upstream: Forward expanded input
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
1/4 boxes ticked. This PR stays in draft until every box above is ticked. |
HoshimiRox1
changed the title
1598a96 to
1bab097
Compare
A chained-turn replay can balloon a request far past the model's context window (observed: a 4x expansion pushed a ~400k-token conversation to 1.6M input tokens). The proxy forwarded it verbatim; processing it on Windows ballooned bun RSS and native-crashed the whole service (upstream Bun memory bug, lidge-jun#314), taking every active thread down until restart. Reject the request with a clean 413 before any upstream I/O when the parsed input exceeds the model's configured modelContextWindows value. The client compacts well before the window, so the guard only fires on abnormal duplication.
1bab097 to
67379fd
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/responses/state.ts`:
- Around line 732-735: Update canonicalReplayItemKey in src/responses/state.ts
(lines 732-735) to recursively sort retained object keys, including nested
objects, before serialization so equivalent items produce identical canonical
keys; preserve the existing excluded fields. Add a regression case in
tests/responses-replay-overlap.test.ts (lines 118-133) using stored and resent
items with different retained key order, and assert they overlap without
duplicating history.
- Around line 886-898: Update the replay merge logic in replayedPrefixOverlap
handling within src/responses/state.ts lines 886-898 to preserve request
unchanged only for complete overlap; otherwise append
requestItems.slice(overlap) after storedItems, avoiding duplicated matched
prefixes and omitted stored items. Add a regression test in
tests/responses-replay-overlap.test.ts lines 100-116 covering a partial prefix
plus delta and asserting each history item appears exactly once.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c9d3db0b-6cee-4313-9fba-da97765c85e4
📒 Files selected for processing (4)
src/responses/state.tssrc/server/responses/core.tstests/responses-input-guard.test.tstests/responses-replay-overlap.test.ts
67379fd to
9994bea
Compare
HoshimiRox1
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/responses/state.ts`:
- Around line 909-913: In src/responses/state.ts lines 909-913, update the
full-history classification in the replay handling around replayedPrefixOverlap
to require complete canonical stored-prefix overlap, removing the
requestItems.length-only condition; any compatibility fallback must validate
item identity rather than count. In tests/responses-replay-overlap.test.ts lines
101-117, add coverage for a delta continuation whose request length is at least
the stored-history length and assert upstream input contains the stored history
followed by every delta item.
In `@tests/responses-replay-overlap.test.ts`:
- Around line 186-201: Set statelessResponses to true in
statelessDeepseekConfig() so postResponses() routes these fixtures through the
stateless adapter branch when passed directly to handleResponses().
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b5826d00-4332-4961-b9e0-b2c493671195
📒 Files selected for processing (2)
src/responses/state.tstests/responses-replay-overlap.test.ts
9994bea to
562c553
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/responses/state.ts`:
- Around line 753-758: Update replayedPrefixOverlap to canonicalize stored[n]
and requestInput[n] inside the comparison loop instead of mapping both arrays
upfront. Return the accumulated prefix length immediately when either canonical
key is undefined or the keys differ, while preserving the existing
matching-prefix result.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c01cb42-49cb-4d5b-8774-76de7efce0f6
📒 Files selected for processing (2)
src/responses/state.tstests/responses-replay-overlap.test.ts
562c553 to
3e5768c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/responses/state.ts`:
- Around line 735-738: Update the canonicalization block in canonicalReplayValue
to create out with a null prototype instead of a normal object, preserving any
own JSON __proto__ key as a serializable property while leaving the sorted-key
traversal and recursive canonicalization unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a5014d0b-b026-40ee-8f78-30b7c9f5db34
📒 Files selected for processing (1)
src/responses/state.ts
A chained /v1/responses turn may carry the full conversation (stateless upstreams such as DeepSeek force the client to resend it every turn). expandPreviousResponseInput prepended the stored history unconditionally, so a full-body request duplicated it, and recording the duplicated body made the bloat sticky across turns: 1x -> 2x -> 3x -> ... (observed 1,333,682 input tokens on 2026-08-10, ~10x the real ~127k conversation). Detect the overlap via canonical item keys (ignoring volatile ids/status) plus an item-count rule, keep the request's own input when it already begins with the stored history, and only prepend for genuine delta continuations.
3e5768c to
4e3c4ff
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Two fixes for the Codex desktop context/compaction failure chain (reported upstream in #1128).
expandPreviousResponseInputprepended the stored history unconditionally. Stateless upstreams (DeepSeek documents "every turn must resend the full history") make the client carry the full conversation ininputwhile still chaining withprevious_response_id; prepending then duplicates it, and recording the duplicated body makes the bloat sticky: 1x → 2x → 3x → … The expansion now detects the overlap via canonical item keys (ignoring volatile ids/status) plus an item-count rule, keeps the request's own input when it already begins with the stored history, and still expands genuine delta continuations.Context and reproduction evidence
The trigger is the continuation turn right after a tool result: the client resends the full conversation plus
previous_response_id, and the old expansion prepended the stored copy again. This is not web-search-specific — shell results, hosted search results, and any other tool-result round-trip share the same shape. Web-search/tool loops are the high-frequency scenario because they produce many consecutive tool-result continuations.Live observations (stock 2.11.1, all requests returned 200):
c527a04a: input239,957→485,943(~2.0x), then back to252,901on the next request. 23:59:28, another conversation:565,484.1,333,682(cached1,325,824, ~99.4% cache hit) while the real conversation was ~127k tokens; the session log shows acompactedevent immediately after.1,609,389(cached1,604,224), immediately after ashell_commandresult (GitHub API check), followed by compaction failure (stream closed before response.completed) and a proxy crash.Verification
bun test tests/responses-replay-overlap.test.ts tests/responses-input-guard.test.ts— 7 pass, 0 fail (full-history chained turns stay 1x; delta turns still expand; stateless DeepSeek end-to-end keeps upstream at 1x).EPERMsandbox cases inresponses-state.test.ts, unrelated to this change.bun run typecheck— pass.bun run privacy:scan— pass.git diff --check— pass.bun run testwas attempted earlier on Windows: unrelated codex-journal restoration tests remained red in isolation and the run ended in a Bun 1.3.14 index-out-of-bounds crash. Focused and related suites stay green, so this PR remains draft for CI confirmation.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Request Handling
413 request_too_largeerror before processing.