Skip to content

fix(mcp): offer both media types and unframe an event-stream answer (ADR-181) - #835

Merged
CybotTM merged 3 commits into
mainfrom
fix/mcp-accept-sse
Aug 20, 2026
Merged

fix(mcp): offer both media types and unframe an event-stream answer (ADR-181)#835
CybotTM merged 3 commits into
mainfrom
fix/mcp-accept-sse

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 20, 2026

Copy link
Copy Markdown
Member

Closes #833.

What

The MCP client sent Accept: application/json alone; the public Streamable-HTTP servers answer that with 406 ("Client must accept both application/json and text/event-stream") — measured before this change against mcp.deepwiki.com/mcp, mcp.context7.com/mcp and learn.microsoft.com/api/mcp, all three. The transport now offers both media types, as the spec requires of every POST, and reads an answer a server frames as text/event-stream like a plain JSON one: the single JSON-RPC response is unwrapped (events split on a blank line, the data: lines of one event join with a newline, event:/id:/retry:/comment lines ignored, CRLF accepted), a server notification on the same stream is passed over, and a stream with no response is a malformed answer that says which way ("no message" vs "no response to the request").

Nothing else moves: no stream is held open or resumed, no server-initiated request is answered, the response size cap and the operation budget (ADR-170) apply unchanged, stdio stays out of scope. The unsupported-content-type refusal keeps its exception and code; only its message stops claiming "JSON only".

Records

ADR-181 (new) — amends ADR-116's transport section ("HTTP only" was right about what the client does, wrong about what it says) and ADR-161's "no SSE" edge (now "no live stream"). McpServers.rst gains a "What the client speaks" paragraph. CHANGELOG under Unreleased/Changed.

Tests

Transport: the header is pinned; a CRLF, multi-data:, event:+id:-framed response unframes to the result; a server notification before the response is passed over; an empty stream and a notification-only stream are malformed answers; text/html is still refused with the same code. Conformance suite (ADR-161): one positive case — an event-stream framed tool answer reads like a plain one — and two new failing shapes in serverFailures. McpTestServer records the Accept header.

Gates run locally (one matrix cell, PHP 8.5 / TYPO3 14)

cgl ✓ · phpstan (level 10) ✓ · unit (7226 tests, includes the conformance suites) ✓ · changelog ✓. Rector cannot run in this worktree (.Build resolved under 8.4, see Tests/AGENTS.md); the functional suite is untouched by this change and left to CI.

Assisted by claude-code:claude-fable-5 — Session

The transport sent Accept: application/json alone, and the public
Streamable-HTTP servers answer that with 406 — measured against
mcp.deepwiki.com, mcp.context7.com and learn.microsoft.com/api/mcp,
all three, before this change. The spec requires a client to offer
both media types on every POST, and the reference SDKs enforce it.

The client now offers both and reads an answer framed as
text/event-stream like a plain JSON one: the single JSON-RPC response
is unwrapped by the SSE rules that matter for one exchange (events
split on a blank line, data: lines of one event join with a newline,
framing and comment lines ignored, CRLF accepted); a server
notification on the same stream is passed over; a stream with no
response is a malformed answer and says which way. Nothing else
moves — no stream is held open, no server request is answered, the
size cap and the operation budget apply unchanged. ADR-181 records
it and amends ADR-116's transport section and ADR-161's edge.

Conformance suite: one positive case (a framed tool answer reads
like a plain one) and two failing shapes; transport tests pin the
header and the framing rules.

Closes #833

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_0124WZK5UoAM9oYQoSYF1Y8z
Agent-Host: 32116e
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
@CybotTM
CybotTM requested a review from a team as a code owner August 20, 2026 11:17
Copilot AI lite review requested due to automatic review settings August 20, 2026 11:17
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test-related changes labels Aug 20, 2026
github-actions[bot]
github-actions Bot previously approved these changes Aug 20, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.75%. Comparing base (cb7e823) to head (41bb809).

Files with missing lines Patch % Lines
...rvice/Tool/Mcp/Exception/McpTransportException.php 0.00% 1 Missing ⚠️
Classes/Service/Tool/Mcp/McpHttpTransport.php 97.14% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #835   +/-   ##
=========================================
  Coverage     86.74%   86.75%           
- Complexity     9865     9880   +15     
=========================================
  Files           573      573           
  Lines         31920    31949   +29     
=========================================
+ Hits          27689    27717   +28     
- Misses         4231     4232    +1     
Flag Coverage Δ
unit 65.09% <94.44%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rvice/Tool/Mcp/Exception/McpTransportException.php 0.00% <0.00%> (ø)
Classes/Service/Tool/Mcp/McpHttpTransport.php 88.72% <97.14%> (+2.18%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

NewlineAfterStatementRector wants the comment that follows the
`data:` branch separated from it by an empty line.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_0124WZK5UoAM9oYQoSYF1Y8z
Agent-Host: 32116e
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
github-actions[bot]
github-actions Bot previously approved these changes Aug 20, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Self-review: 28390be

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 20, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Aug 20, 2026
Both ADR-180 (#832) and ADR-181 appended to the ADR toctree; the
merge keeps both entries in order.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_0124WZK5UoAM9oYQoSYF1Y8z
Agent-Host: 32116e
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

@CybotTM

CybotTM commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Self-review: 41bb809

The review this pull request demands is unsatisfiable (Copilot quota wall or repeated bot failures on this head). Per the documented fallback, the diff on this head was reviewed by the PR author; this comment is the on-the-record attestation the merge gate reads back. It stops matching on the next push.

@CybotTM
CybotTM added this pull request to the merge queue Aug 20, 2026
Merged via the queue into main with commit 681d342 Aug 20, 2026
92 checks passed
@CybotTM
CybotTM deleted the fix/mcp-accept-sse branch August 20, 2026 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation tests Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP client: send Accept: application/json, text/event-stream and unframe an SSE-framed response — public servers answer 406 today

2 participants