fix: detect responses SSE without content type - #6082
Conversation
WalkthroughThe chat-to-responses relay now detects SSE payloads by inspecting response bodies when ChangesResponses SSE Detection
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@relay/chat_completions_via_responses.go`:
- Around line 199-213: Increase the Peek window in the response stream detection
logic to a fixed size larger than len("event:"), sufficient for the BOM and
leading whitespace handled by TrimLeft. Update the reader.Peek call in the
relevant detection block while preserving the existing short-body and error
handling, then continue trimming and checking for "event:" or "data:" prefixes.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97e3388f-d23f-48d6-9192-b908e275bd46
📒 Files selected for processing (2)
relay/chat_completions_via_responses.gorelay/chat_completions_via_responses_test.go
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 `@relay/chat_completions_via_responses.go`:
- Around line 203-205: Replace the fixed-size Peek(64) in the response sniffing
flow with incremental probing that can identify event: or data: from a short
live SSE prefix without waiting for EOF or a full buffer. Preserve existing
whitespace/BOM and JSON detection behavior, and add a regression test covering a
short SSE prefix such as event: ping\n\n with no EOF.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: afb16774-87c8-4073-aa86-6b9145d1c2ec
📒 Files selected for processing (2)
relay/chat_completions_via_responses.gorelay/chat_completions_via_responses_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/chat_completions_via_responses_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
relay/chat_completions_via_responses_test.go (1)
70-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a timeout branch to the
selectstatement.The
selectat Lines 70-75 waits only onresultandreader.secondRead. IfdetectResponsesEventStreamregresses to a state where it neither returns nor triggers a secondRead(for example, a deadlock inside the detection loop), this test hangs until the Go test binary's global timeout instead of failing fast with a clear message.Add a
time.Afterbranch so a regression fails quickly with a clear message.♻️ Proposed fix to add a timeout branch
+ "time" ... select { case got := <-result: assert.True(t, got) case <-reader.secondRead: require.Fail(t, "stream detection waited for more bytes after receiving an SSE prefix") + case <-time.After(2 * time.Second): + require.Fail(t, "detectResponsesEventStream did not complete in time") }🤖 Prompt for 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. In `@relay/chat_completions_via_responses_test.go` around lines 70 - 76, Add a timeout case to the select in the test around detectResponsesEventStream, using time.After to fail promptly with a clear deadlock or detection-timeout message. Preserve the existing result-success and secondRead-failure branches.
🤖 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.
Nitpick comments:
In `@relay/chat_completions_via_responses_test.go`:
- Around line 70-76: Add a timeout case to the select in the test around
detectResponsesEventStream, using time.After to fail promptly with a clear
deadlock or detection-timeout message. Preserve the existing result-success and
secondRead-failure branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ba8aac20-8473-441c-a18c-42933f2f41c8
📒 Files selected for processing (2)
relay/chat_completions_via_responses.gorelay/chat_completions_via_responses_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/chat_completions_via_responses.go
51fdfc5 to
2b6f1df
Compare
1 participant
Important
📝 变更描述 / Description
This PR was AI-assisted by Codex automation because the configured git author is not a recurring core maintainer in recent repository history.
Fixes the chat/completions → responses conversion path for upstream Responses SSE bodies that omit
Content-Type, as reported for Codex/ChatGPT subscription channels. The relay now peeks at the beginning of the response body when the header does not identify an event stream, treatsevent:ordata:prefixes as SSE, and preserves the buffered bytes so the selected stream handler receives the full original body. JSON-looking bodies remain on the non-stream path.🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
relay/chat_completions_via_responses.goand its regression tests changed.📸 运行证明 / Proof of Work
Summary by CodeRabbit
Bug Fixes
Content-Typeheader is missing or unclear by detecting event-stream–formatted payloads from the body.Tests
Content-Type, including leading whitespace and UTF-8 BOM cases.