mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-16 20:22:21 +03:00
4b76d3b76f
* fix(sse): close the synthetic keepalive reasoning item's output_item RESPONSES_STARTUP_THINKING_FRAME (the /v1/responses early-keepalive placeholder for slow-starting reasoning models) opened a synthetic "rs_keepalive" reasoning item at output_index 0 and closed its nested summary part (response.reasoning_summary_part.done), but never sent response.output_item.done to close the item itself. The comment claimed it was "closed within this one frame" — that was true for the part, not the item. Since this placeholder has no real upstream counterpart (the real response starts an independent response.created lifecycle later and never touches it), nothing else ever closes it. A client tracking open items by output_index (as the Responses API spec requires — this is exactly what OpenClaw's parser does) sees index 0 still open when the real response's own output_item.added later reuses that same index, and throws a collision. Live incident (2026-08-13, reliably reproducing by 2026-08-14): traced via a live tcpdump capture on the OmniRoute-dev container's network namespace, correlated against the OpenClaw gateway journal and 10 separate real request/response pairs (all wire-clean on the response side, ruling out provider corruption). The failing request's own outbound payload confirmed a replayed reasoning item without encrypted_content feeding a continuation call; the response wire bytes for that exact exchange showed rs_keepalive's output_item.added at index 0, then response.created/response.in_progress arriving *after* it, then a second output_item.added reusing index 0 for the real reasoning item — never preceded by an output_item.done for rs_keepalive. Reported upstream as OpenClaw issue #123342 before the OmniRoute-side root cause was found. Fix: emit response.output_item.done for the synthetic item, matching its already-buffered summary text, right after the summary part closes and before the frame ends. Test plan: - tests/unit/early-stream-keepalive.test.ts: updated the frame-shape test to assert the full 5-event closed sequence (added the missing output_item.done and its field assertions); confirmed it fails against pre-fix code (only 4 events) and passes after - node --test tests/unit/early-stream-keepalive.test.ts, tests/unit/earlyStreamKeepalive.test.ts, tests/unit/keepalive-cleanup-8140.test.ts, tests/unit/chat-body-admission.test.ts: 58 passed, 2 pre-existing skips unrelated to this change (Node test runner ReadableStream-error-simulation limitation) - tsgo --noEmit: clean on both touched files * fix(sse): allocate the keepalive output_index from a stack, not a literal Follow-up to 03f8345ac. That commit patched the specific symptom (added the missing response.output_item.done). This commit fixes the class: RESPONSES_STARTUP_THINKING_FRAME hardcoded output_index: 0 as a literal across five hand-written events, which is exactly how the missing-close bug happened in the first place — nothing enforced that every open got a matching close, so it silently didn't for months. ResponsesOutputIndexStack (open-sse/utils/responsesOutputIndexStack.ts) makes that structural: open() allocates the next sequential index, close() must name the index being closed and throws if it doesn't match the stack's top, and assertAllClosed() throws if anything is still open. The keepalive frame now calls assertAllClosed() at module load, so a future regression of this exact shape fails at import/boot time instead of shipping a malformed stream to production and surfacing days later as a live incident. Also adds tests/helpers/assertResponsesOutputIndexLifecycle.ts: a reusable version of the same invariant for replaying a full SSE event sequence (not just checking one frame's own shape), mirroring what a real client's output-index tracker enforces. Existing coverage for this bug class (responses-reasoning-close-before-message-466.test.ts) only asserted it by hand for one specific emitter (the real translator); nothing generic existed for a hand-rolled synthetic frame like this keepalive to be checked against, which is why its own test could pass while the actual downstream contract still failed. Wired into early-stream-keepalive.test.ts, including a test that concatenates the keepalive frame with a plausible real subsequent response and asserts no collision — the scenario that actually reproduced live, not just the frame's own internal shape. Test plan: - tests/unit/responses-output-index-stack.test.ts (new): open/close/ assertAllClosed behavior, including the exact mismatch and never-closed shapes this incident hit - tests/unit/early-stream-keepalive.test.ts: existing frame-shape test plus new collision-simulation test, both passing - node --test across responses-output-index-stack, early-stream-keepalive, earlyStreamKeepalive, keepalive-cleanup-8140, chat-body-admission: 65 passed, 2 pre-existing skips unrelated to this change - tsgo --noEmit: clean on all touched files --------- Co-authored-by: adevwithpurpose <adevwithpurpose@users.noreply.github.com>