fix(responses): reassemble split SSE event/data frames before streaming#2492
Merged
luispater merged 2 commits intorouter-for-me:mainfrom Apr 2, 2026
Merged
fix(responses): reassemble split SSE event/data frames before streaming#2492luispater merged 2 commits intorouter-for-me:mainfrom
luispater merged 2 commits intorouter-for-me:mainfrom
Conversation
Line-oriented upstream executors can emit `event:` and `data:` as separate chunks, but the Responses handler had started terminating each incoming chunk as a full SSE event. That split `response.created` into an empty event plus a later data block, which broke downstream clients like OpenClaw. This keeps the fix in the handler layer: a small stateful framer now buffers standalone `event:` lines until the matching `data:` arrives, preserves already-framed events, and ignores delimiter-only leftovers. The regression suite now covers split event/data framing, full-event passthrough, terminal errors, and the bootstrap path that forwards line-oriented openai-response streams from non-Codex executors too. Constraint: Keep the fix localized to Responses handler framing instead of patching every executor Rejected: Revert to v6.9.7 chunk writing | would reintroduce data-only framing regressions Rejected: Patch each line-oriented executor separately | duplicates fragile SSE assembly logic Confidence: high Scope-risk: narrow Reversibility: clean Directive: Do not assume incoming Responses stream chunks are already complete SSE events; preserve handler-layer reassembly for split `event:`/`data:` inputs Tested: /tmp/go1.26.1/go/bin/go test ./sdk/api/handlers/openai -count=1 Tested: /tmp/go1.26.1/go/bin/go test ./sdk/api/handlers -count=1 Tested: /tmp/go1.26.1/go test ./sdk/api/handlers/... -count=1 Tested: /tmp/go1.26.1/go/bin/go vet ./sdk/api/handlers/... Tested: Temporary patched server on 127.0.0.1:18317 -> /v1/models 200, /v1/responses non-stream 200, /v1/responses stream emitted combined `event:` + `data:` frames Not-tested: Full repository test suite outside sdk/api/handlers packages
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a responsesSSEFramer to correctly reassemble split Server-Sent Events (SSE) in the OpenAI responses handler, ensuring that partial chunks are buffered until a complete frame is formed. It also updates line-ending handling and adds unit tests for split-event scenarios. The review feedback highlights potential issues with eager flushing of incomplete frames, redundant newline injections when chunks already contain line breaks, and performance optimizations for line-prefix checks to avoid unnecessary allocations.
Follow-up review found two real framing hazards in the handler-layer framer: it could flush a partial `data:` payload before the JSON was complete, and it could inject an extra newline before chunks that already began with `\n`/`\r\n`. This commit tightens the framer so it only emits undelimited events when the buffered `data:` payload is already valid JSON (or `[DONE]`), skips newline injection for chunks that already start with a line break, and avoids the heavier `bytes.Split` path while scanning SSE fields. The regression suite now covers split `data:` payload chunks, newline-prefixed chunks, and dropping incomplete trailing data on flush, so the original Responses fix remains intact while the review concerns are explicitly locked down. Constraint: Keep the follow-up limited to handler-layer framing and tests Rejected: Ignore the review and rely on current executor chunk shapes | leaves partial data payload corruption possible Rejected: Build a fully generic SSE parser | wider change than needed for the identified risks Confidence: high Scope-risk: narrow Reversibility: clean Directive: Do not emit undelimited Responses SSE events unless buffered `data:` content is already complete and valid Tested: /tmp/go1.26.1/go/bin/go test ./sdk/api/handlers/openai -count=1 Tested: /tmp/go1.26.1/go/bin/go test ./sdk/api/handlers -count=1 Tested: /tmp/go1.26.1/go/bin/go vet ./sdk/api/handlers/... Not-tested: Full repository test suite outside sdk/api/handlers packages
luispater
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reassemble split
event:+data:SSE fragments in the/v1/responseshandler before flushing them downstream, so line-oriented upstream executors no longer emit malformed Responses stream events.Problem
In the current Responses streaming path, some upstream executors can emit SSE line-by-line rather than as complete framed events. When
event:anddata:arrive as separate chunks, the handler was incorrectly closing each chunk as its own SSE event, producing malformed output like:Downstream clients such as OpenClaw then fail to parse the stream and report JSON errors like:
This was not limited to a single executor implementation. Any line-oriented openai-response stream routed through the Responses handler could hit the same framing bug.
Fix
Added a small stateful
responsesSSEFramerin the Responses handler layer.It now:
event:lines until the matchingdata:line arrives\n\nand\r\n\r\nframingAlso added regression coverage for:
event:+data:chunks in Responses stream forwardingExecuteStreamWithAuthManageracceptance of split openai-response SSE event lines from non-Codex executorsTesting
/tmp/go1.26.1/go/bin/go test ./sdk/api/handlers/openai -count=1/tmp/go1.26.1/go/bin/go test ./sdk/api/handlers -count=1/tmp/go1.26.1/go/bin/go test ./sdk/api/handlers/... -count=1/tmp/go1.26.1/go/bin/go vet ./sdk/api/handlers/...127.0.0.1:8317:/v1/modelsreturned 200/v1/responsesnon-stream returned 200/v1/responsesstream returned valid combinedevent:+data:framesevent: response.created\n\ndata:output no longer appearedFixes #2484