Skip to content

fix(responses-continuation): fail closed on a log-truncated stored input/output array - #11473

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
hartmark:fix/continuation-truncated-array-marker
Aug 25, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
hartmark:fix/continuation-truncated-array-marker

Conversation

@hartmark

Copy link
Copy Markdown
Contributor

What Problem This Solves

previous_response_id continuation went from "doesn't help" to "actively breaks the turn" once a real conversation's logged history grew past ~24 items: the client sent a valid previous_response_id, OmniRoute accepted it, reconstructed the request server-side, and forwarded a genuinely malformed body upstream — a live 400: Unsupported Responses API feature: input item type 'missing' cannot be represented in Chat Completions. Confirmed live on a real gateway's production traffic.

Why This Change Was Made

resolvePreviousResponseState (added in #11434, building on the continuation-virtualization work in #10262) reads a stored call-log artifact's input/output arrays back as if they were always the complete, untruncated conversation. But both array-bounding implementations that clip a stored artifact for log-storage size — cloneBoundedChatLogPayload (open-sse/handlers/chatCore/logTruncation.ts) and cloneBoundedForLog (open-sse/utils/requestLogger.ts) — prepend an _omniroute_truncated_array sentinel object in place of the items they drop once an array exceeds their tail-item cap (MAX_LOG_ARRAY_ITEMS = 24). That's routine for any conversation that's been running a while, not an edge case.

Reading that sentinel back as a real Responses-API item and forwarding the reconstructed request upstream produced the malformed-input 400 above. This function already fails closed (returns null, forcing a full-history resend) for a size-limit-omitted payload — this extends the same treatment to a truncated-array sentinel in either the cached input or output.

User Impact

Any client relying on previous_response_id continuation now correctly falls back to resending full history once its conversation's logged arrays exceed the tail-item cap, instead of getting a corrupted reconstruction and a hard 400 on that turn.

Evidence

  • New regression test (resolvePreviousResponseState fails closed when the stored input array was log-truncated) reproduces the exact sentinel shape and asserts null. Confirmed it fails on pre-fix code (returns the corrupted {input, output} including the sentinel object) and passes post-fix.
  • Full tests/unit/responses-continuation-store.test.ts suite green (9/9) on both the branch this was developed against and release/v3.8.51.
  • Live-verified directly against the exact response_id that produced the real 400 in production (resp_gen-1787640505-UFPwZTrkLcHJY4O3CRmA): resolvePreviousResponseState now returns null for it instead of the malformed reconstruction that broke the turn.

…put/output array

resolvePreviousResponseState read the client-facing artifact's input/output
arrays as if they were always the complete, untruncated conversation -- but
both array-bounding implementations that clip a stored artifact for
log-storage size (cloneBoundedChatLogPayload in
open-sse/handlers/chatCore/logTruncation.ts, cloneBoundedForLog in
open-sse/utils/requestLogger.ts) prepend an `_omniroute_truncated_array`
sentinel in place of the items they drop once an array exceeds their
tail-item cap (~24 items) -- routine for any conversation that's been going
a while, not an edge case.

Live impact: reading that sentinel back as a real Responses-API item and
forwarding the reconstructed request upstream produced a genuine 400
("input item type 'missing' cannot be represented in Chat Completions"),
observed live on Ping's gateway once a real conversation grew past the cap.
That's worse than the plain cache-miss this function is otherwise designed
to fail into -- previous_response_id continuation went from "doesn't help"
to "actively breaks the turn."

Fail closed (return null, exactly like the existing size-limit-omitted
case) whenever either array contains the sentinel, so the client falls
back to full history instead of getting a corrupted reconstruction.
@hartmark
hartmark requested a review from diegosouzapw as a code owner August 25, 2026 07:11
@diegosouzapw
diegosouzapw merged commit fff62f1 into diegosouzapw:release/v3.8.51 Aug 25, 2026
7 of 16 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 25, 2026
…uplicate implementations (#11499)

Validated in a combined 2-PR batch worktree off release/v3.8.51 tip (companion fix to #11473, merged first).
- Focused tests: chat-log-array-tail-items-default, chatcore-log-truncation, request-logger-bounded-clone, request-logger-bounded-idempotence, repro-7847-bound-client-raw-request — part of batch's 46/46 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for unifying the two independently-drifted truncation caps onto one configurable source — measured evidence that the storage ceiling comes from retention days, not the per-item cap, makes the 128→1000 raise a clear correctness improvement.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…put/output array (diegosouzapw#11473)

Validated in a combined 2-PR batch worktree off release/v3.8.51 tip.
- Focused test: responses-continuation-store.test.ts — part of batch's 46/46 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for the live-verified root cause — a truncation sentinel getting forwarded upstream as a real Responses-API item, breaking the turn with a genuine 400, is exactly the kind of defect that's easy to miss without production traffic to reproduce against.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…uplicate implementations (diegosouzapw#11499)

Validated in a combined 2-PR batch worktree off release/v3.8.51 tip (companion fix to diegosouzapw#11473, merged first).
- Focused tests: chat-log-array-tail-items-default, chatcore-log-truncation, request-logger-bounded-clone, request-logger-bounded-idempotence, repro-7847-bound-client-raw-request — part of batch's 46/46 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for unifying the two independently-drifted truncation caps onto one configurable source — measured evidence that the storage ceiling comes from retention days, not the per-item cap, makes the 128→1000 raise a clear correctness improvement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants