You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(shared): read MCP event streams whatever the case of the content type - #35815
The review item is correct. The previous head lowercased the whole Content-Type header and used includes("text/event-stream"). A JSON response with Content-Type: application/json; profile="https://example.com/Text/Event-Stream" was therefore routed to the SSE reader and yielded no payload, which is worse than main. responsePayloads now takes only the media type before the first ';', trims it, and compares it case-insensitively with equality to 'text/event-stream'. This keeps the original fix (Text/Event-Stream; charset=utf-8 is read as SSE, per RFC 9110 case-insensitive media types) and stops parameters from selecting the reader. No regex or parser was added: split(';', 1), trim and toLowerCase are linear in the header length. The reviewer's case was added to the existing mixed-case test in mcpResponsePayloads.test.ts (renamed to 'picks the SSE reader from the media type alone, whatever its case'). It failed on the old head and passes now.
On origin/main 6f21b0f with locked deps installed (vp i), ran the reporter's repro-media.mjs (copied from the issue) against the checkout under Node v26.13.1 / effect 4.1.3. It drives the production mcpToolSession from apps/server/src/mcp/AcpMcpStdioBridge.ts with a fake fetch. Output: 'lowercase SSE: tool result ok', then 'mixed-case SSE: Unexpected token 'd', "data: {"js"... is not valid JSON', 'RESULT: defect reproduced', exit 1. Also added a unit test in packages/shared/src/mcpResponsePayloads.test.ts feeding a Response with content-type 'Text/Event-Stream; charset=utf-8' and body 'data: {"id":1}\n\n'; vp test run fails on main with the same JSON.parse error.
Evidence
Run the production acp-mcp-call echo {} CLI against a real loopback HTTP MCP fixture. Initialize with JSON, acknowledge initialization with HTTP 202, then return the same SSE tool result using lowercase, mixed-case, and mixed-case with charset Content-Type values. Environment on both sides: Linux x86_64, Node v26.12.1, locked dependencies, synthetic data only.
Replay web scenarios do not exercise the ACP MCP CLI transport; evidence covers the real production CLI instead of a web interaction.
Did not separately demonstrate JSON responses containing an event-stream string in a media-type parameter, or the stdio bridge path; the captured reproduction covers mixed-case SSE through the production helper-session CLI.
Tests
vp test run packages/shared/src/mcpResponsePayloads.test.ts: with only the new assertion added (before the code change) 1 failed ('expected [] to deeply equal [ { jsonrpc: "2.0", id: 1, … } ]'); after the fix 2/2 passed. vp run check:changed: format, lint, knip 1/2 and 2/2, typecheck (35 packages), and test (2 files) all pass.
Not in this change
packages/provider-pi/src/server/mcpExtensionSource.ts:81 has the same case-sensitive contentType.includes("text/event-stream") check in its own response reader
apps/server/src/peer/PeerMcpClient.ts:100 has the same case-sensitive event-stream check for peer MCP responses
packages/provider-pi/src/server/mcpExtensionSource.ts:81 still uses a case-sensitive contentType.includes("text/event-stream") in its own reader
apps/server/src/peer/PeerMcpClient.ts:100 still uses a case-sensitive includes("text/event-stream") check for peer MCP responses
Who did the work
Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5
…type
Media types are case-insensitive (RFC 9110), but responsePayloads matched
"text/event-stream" case-sensitively, so a header like Text/Event-Stream sent
the SSE body to JSON.parse and failed the MCP tool call. Lowercase the header
before matching.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lowercasing the whole Content-Type and using includes let a parameter
value such as profile=".../Text/Event-Stream" on a JSON response select
the SSE reader and drop the JSON envelope. Compare only the trimmed media
type before the first semicolon, case-insensitively.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Macroscope's review found this PR approvable — MCP responses now select the event-stream decoder regardless of media-type casing, without matching text inside Content-Type parameters. The change is limited to decoder selection and leaves parsing and error handling unchanged.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
The shared MCP response reader now checks the trimmed, lowercased media type before any parameters to select SSE decoding. Tests cover mixed-case SSE content types and JSON responses whose profile parameter contains the SSE media-type text.
Changes: MCP response decoding
Layer / File(s)
Summary
Media type detection and tests packages/shared/src/mcpResponsePayloads.ts, packages/shared/src/mcpResponsePayloads.test.ts
The reader compares the trimmed, lowercased media type before parameters with text/event-stream. Tests verify mixed-case SSE selection and JSON selection when the text appears only in a profile parameter.
pingdotgg/t3code#19387: Changes the same media-type check and covers the same mixed-case SSE behavior.
Suggested labels:bugfix
Suggested reviewers:romil-bijarnia, macroscopeapp
Merge Risk: Minimal · up to d0d7e
Mixed-case SSE responses from MCP servers now decode correctly, and a JSON response whose parameters merely mention the SSE media type is no longer misread. No merge-blocking risk remains.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title "fix(shared): read MCP event streams whatever the case of the content type" states the main change. It names the affected area (shared MCP response reading), the fix type, and the case-insen…
Linked Issues check
Issue #35792 requires that a valid mixed-case Text/Event-Stream response decode as SSE and yield the same tool result as lowercase text/event-stream. At the reviewed head, packages/shared/src/mcpRespo…
Out of Scope Changes check
The whole-PR change set is limited to packages/shared/src/mcpResponsePayloads.ts (the media-type comparison) and packages/shared/src/mcpResponsePayloads.test.ts (the test extension). Both directly imp…
Description check
The description covers the required template sections with substance. Problem: the previous head lowercased the whole Content-Type header and used a substring match, so a JSON response whose profile p…
Full details: Title check
Explanation
The title "fix(shared): read MCP event streams whatever the case of the content type" states the main change. It names the affected area (shared MCP response reading), the fix type, and the case-insensitive behavior. The diff matches this: the reader now compares the media type case-insensitively, and the test covers mixed-case SSE.
Full details: Linked Issues check
Explanation
Issue #35792 requires that a valid mixed-case Text/Event-Stream response decode as SSE and yield the same tool result as lowercase text/event-stream. At the reviewed head, packages/shared/src/mcpResponsePayloads.ts takes the media type before the first ';' (split(";", 1)[0]), trims it, and compares it with toLowerCase() === "text/event-stream". That selects the SSE reader for mixed-case media types. The same reader is used by the stdio bridge, as the issue states. Parameters no longer select the reader. The test 'picks the SSE reader from the media type alone, whatever its case' in mcpResponsePayloads.test.ts covers both 'Text/Event-Stream; charset=utf-8' (read as SSE) and 'application/json; profile="...Text/Event-Stream"' (read as JSON). The PR description reports that the new assertion failed on the previous head. The issue's non-coding items (manual reproduction on a live deployment, the evidence capture) do not affect this verdict. The end-to-end before/after evidence is pending, which is non-coding and outside this assessment. The verdict rests on the unit-level source and test evidence, which I read directly.
Full details: Out of Scope Changes check
Explanation
The whole-PR change set is limited to packages/shared/src/mcpResponsePayloads.ts (the media-type comparison) and packages/shared/src/mcpResponsePayloads.test.ts (the test extension). Both directly implement issue #35792's objective. The change does not touch the similar readers in packages/provider-pi/src/server/mcpExtensionSource.ts or apps/server/src/peer/PeerMcpClient.ts. The PR explicitly lists them as unchanged, and the issue does not require them. No unrelated changes were found in the inspected files.
Full details: Description check
Explanation
The description covers the required template sections with substance. Problem: the previous head lowercased the whole Content-Type header and used a substring match, so a JSON response whose profile parameter contained "Text/Event-Stream" was routed to the SSE reader and yielded no payload. Change: responsePayloads now reads only the media type before the first ';', trims it, and compares it case-insensitively for equality with 'text/event-stream'. Scope: it fixes the linked issue #35792 (a bug report, not a feature), and the description explains the bug and the link. Verification: the author gives a reproduction on main, a new mixed-case assertion that fails before the fix and passes after, a before/after CLI evidence table, and lists what was not checked. The Problem section is not under its own heading in the usual form, but the content is present under the summary text. The template's Scope and Verification requirements are met. Minor gaps: the description repeats the same two unfinished points under 'Not in this change' (duplicate bullets), and it does not state the conventional commit title in the PR body. These are non-critical.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between 5ceabb3 and d0d7e9e.
Files selected for processing: 2
packages/shared/src/mcpResponsePayloads.test.ts
packages/shared/src/mcpResponsePayloads.ts
Limit details: You’ve used all 10 included reviews currently available. Your 13 included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.
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
size:XS0-9 changed lines (additions + deletions).vouch:trustedAuthor is trusted by repo permissions or the VOUCHED list.
1 participant
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.
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
The review item is correct. The previous head lowercased the whole Content-Type header and used includes("text/event-stream"). A JSON response with
Content-Type: application/json; profile="https://example.com/Text/Event-Stream"was therefore routed to the SSE reader and yielded no payload, which is worse than main. responsePayloads now takes only the media type before the first ';', trims it, and compares it case-insensitively with equality to 'text/event-stream'. This keeps the original fix (Text/Event-Stream; charset=utf-8is read as SSE, per RFC 9110 case-insensitive media types) and stops parameters from selecting the reader. No regex or parser was added: split(';', 1), trim and toLowerCase are linear in the header length. The reviewer's case was added to the existing mixed-case test in mcpResponsePayloads.test.ts (renamed to 'picks the SSE reader from the media type alone, whatever its case'). It failed on the old head and passes now.Fixes #35792
Reproduction on main
On origin/main 6f21b0f with locked deps installed (vp i), ran the reporter's repro-media.mjs (copied from the issue) against the checkout under Node v26.13.1 / effect 4.1.3. It drives the production mcpToolSession from apps/server/src/mcp/AcpMcpStdioBridge.ts with a fake fetch. Output: 'lowercase SSE: tool result ok', then 'mixed-case SSE: Unexpected token 'd', "data: {"js"... is not valid JSON', 'RESULT: defect reproduced', exit 1. Also added a unit test in packages/shared/src/mcpResponsePayloads.test.ts feeding a Response with content-type 'Text/Event-Stream; charset=utf-8' and body 'data: {"id":1}\n\n';
vp test runfails on main with the same JSON.parse error.Evidence
Run the production
acp-mcp-call echo {}CLI against a real loopback HTTP MCP fixture. Initialize with JSON, acknowledge initialization with HTTP 202, then return the same SSE tool result using lowercase, mixed-case, and mixed-case with charset Content-Type values. Environment on both sides: Linux x86_64, Node v26.12.1, locked dependencies, synthetic data only.Before transcript · After transcript · Reproduction driver
Not shown:
Tests
vp test run packages/shared/src/mcpResponsePayloads.test.ts: with only the new assertion added (before the code change) 1 failed ('expected [] to deeply equal [ { jsonrpc: "2.0", id: 1, … } ]'); after the fix 2/2 passed. vp run check:changed: format, lint, knip 1/2 and 2/2, typecheck (35 packages), and test (2 files) all pass.
Not in this change
Who did the work
Run by the t3code factory in T3 Code.