diff --git a/CHANGELOG.md b/CHANGELOG.md index 2323d580a..50ed5d19e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,18 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). gate instead of the answer. ### Changed +- **The MCP client talks to the public Streamable-HTTP servers** (ADR-181). + It now offers both media types the transport requires + (`Accept: application/json, text/event-stream`) — the reference servers + answered the JSON-only header with 406 — and reads an answer a server + frames as `text/event-stream` exactly like a plain JSON one: the single + JSON-RPC response is unwrapped, server notifications on the same stream + are passed over, a stream with no response is a malformed answer. Nothing + else changes: no stream is held open or resumed, no server-initiated + request is answered, stdio stays out of scope. Measured against + `mcp.deepwiki.com`, `mcp.context7.com` and `learn.microsoft.com/api/mcp` + before the change: 406 each. + - **AGENTS.md files synchronized with the repository state.** Root slimmed from 356 to 119 lines by moving content into the scoped files it belongs to; stale inventories refreshed (TCA files, database tables, backend templates, JS modules, `Services.Dashboard.php`); phantom `TCA/Overrides/` and dead `MEMORY.md` references removed; generic workflow boilerplate in `.github/workflows/AGENTS.md` replaced with this repository's actual conventions (no local jobs, release flow, dependency automation). ## [0.30.0] - 2026-08-18 diff --git a/Classes/Service/Tool/Mcp/Exception/McpTransportException.php b/Classes/Service/Tool/Mcp/Exception/McpTransportException.php index 4be575492..e24922bd5 100644 --- a/Classes/Service/Tool/Mcp/Exception/McpTransportException.php +++ b/Classes/Service/Tool/Mcp/Exception/McpTransportException.php @@ -78,7 +78,7 @@ public static function forUnsupportedContentType(string $identifier, string $con { return new self( sprintf( - 'MCP server "%s" answered with content type "%s"; this client reads JSON responses only and does not consume an event stream.', + 'MCP server "%s" answered with content type "%s"; this client reads a JSON response, plain or framed as a single event-stream message, and nothing else.', $identifier, self::clip($contentType), ), diff --git a/Classes/Service/Tool/Mcp/McpHttpTransport.php b/Classes/Service/Tool/Mcp/McpHttpTransport.php index ef4bcd496..73a25502f 100644 --- a/Classes/Service/Tool/Mcp/McpHttpTransport.php +++ b/Classes/Service/Tool/Mcp/McpHttpTransport.php @@ -185,11 +185,15 @@ private function send(McpServerRecord $server, string $body, McpOperationDeadlin $request = $this->requestFactory ->createRequest('POST', $server->url) ->withHeader('Content-Type', 'application/json') - // JSON only, stated honestly: this client does not consume an event - // stream, so it does not claim to accept one. A server that can - // only answer with a stream rejects the request, which is a clearer - // failure than a body we would refuse after the fact. - ->withHeader('Accept', 'application/json') + // Both media types, because the Streamable HTTP transport requires + // a client to offer both on every POST — the reference servers + // answer `application/json` alone with 406. Offering the stream + // does not mean holding one: a server that frames its answer as + // `text/event-stream` gets the single JSON-RPC response unwrapped + // in {@see self::decodeResult()}, and nothing here keeps a stream + // open, resumes one, or answers a request the server initiates + // (ADR-181). + ->withHeader('Accept', 'application/json, text/event-stream') ->withBody($this->streamFactory->createStream($body)); if ($sessionId !== null && $sessionId !== '') { @@ -306,7 +310,9 @@ private function clientFor(McpServerRecord $server, int $timeoutSeconds): Client private function decodeResult(McpServerRecord $server, string $body, int $status, string $contentType): array { $mediaType = strtolower(trim(explode(';', $contentType, 2)[0])); - if ($mediaType !== '' && $mediaType !== 'application/json') { + if ($mediaType === 'text/event-stream') { + $body = $this->unframeEventStream($server, $body); + } elseif ($mediaType !== '' && $mediaType !== 'application/json') { throw McpTransportException::forUnsupportedContentType($server->identifier, $mediaType); } @@ -339,4 +345,67 @@ private function decodeResult(McpServerRecord $server, string $body, int $status /** @var array $result */ return $result; } + + /** + * The one JSON-RPC response inside an event-stream framed answer, as the + * JSON text it was sent as. + * + * The body is read as a whole — it has already been bounded by + * {@see self::readBounded()} and the server has closed it — and parsed by + * the SSE rules that matter for a single request/response exchange: events + * are separated by a blank line, the `data:` lines of one event join with a + * newline, `event:`, `id:`, `retry:` and comment lines carry nothing we + * read. A server may put its own notifications on the same stream before + * the response (the transport allows it); this client declared no + * capabilities, so anything that carries a `method` is passed over and the + * first message that is a response — a `result` or an `error` — is the + * answer. A stream with no such message is a malformed answer, named as one. + * + * @throws McpTransportException when the stream carries no response + */ + private function unframeEventStream(McpServerRecord $server, string $body): string + { + $messages = []; + $data = []; + + foreach (preg_split('/\r\n|\r|\n/', $body) ?: [] as $line) { + if ($line === '') { + if ($data !== []) { + $messages[] = implode("\n", $data); + $data = []; + } + + continue; + } + + if (str_starts_with($line, 'data:')) { + $payload = substr($line, 5); + $data[] = str_starts_with($payload, ' ') ? substr($payload, 1) : $payload; + } + + // `event:`, `id:`, `retry:` and `:` comments are not read. + } + + if ($data !== []) { + $messages[] = implode("\n", $data); + } + + foreach ($messages as $message) { + $decoded = json_decode($message, true); + if (!\is_array($decoded) || isset($decoded['method'])) { + // Not JSON, or a server-initiated notification/request: neither + // is the answer to the request this client made. + continue; + } + + if (\array_key_exists('result', $decoded) || \array_key_exists('error', $decoded)) { + return $message; + } + } + + throw McpTransportException::forMalformedResponse( + $server->identifier, + $messages === [] ? 'the event stream carried no message' : 'the event stream carried no response to the request', + ); + } } diff --git a/Documentation/Administration/McpServers.rst b/Documentation/Administration/McpServers.rst index cd0450273..9b8171397 100644 --- a/Documentation/Administration/McpServers.rst +++ b/Documentation/Administration/McpServers.rst @@ -28,6 +28,17 @@ How it works switched on one by one, exactly like the builtin tools in the :ref:`Tools module `. +What the client speaks +====================== + +Plain HTTP: one POST per JSON-RPC message to the server's endpoint, offering +both media types the Streamable HTTP transport requires (``application/json`` +and ``text/event-stream``). A server may answer a request as plain JSON or as +a single event-stream framed message; both read the same here. What the +client does **not** do: hold a stream open, resume one, answer a request the +server initiates, or speak stdio (:ref:`ADR-116 `, +:ref:`ADR-181 `). + Is the server alive? ==================== diff --git a/Documentation/Adr/Adr116CentralToolingAuthority.rst b/Documentation/Adr/Adr116CentralToolingAuthority.rst index c300c35b0..0ff00838a 100644 --- a/Documentation/Adr/Adr116CentralToolingAuthority.rst +++ b/Documentation/Adr/Adr116CentralToolingAuthority.rst @@ -6,8 +6,10 @@ ADR-116: Central tooling authority — nr_llm owns builtin + MCP tools ============================================================================ -:Status: Accepted +:Status: Accepted (its transport section no longer refuses an event-stream framed + answer — see :ref:`ADR-181 `) :Date: 2026-07-22 +:Amended: 2026-08-20 by :ref:`ADR-181 ` :Authors: Netresearch DTT GmbH .. _adr-116-context: diff --git a/Documentation/Adr/Adr161McpClientConformance.rst b/Documentation/Adr/Adr161McpClientConformance.rst index 96cbc1b4d..c6d021dd9 100644 --- a/Documentation/Adr/Adr161McpClientConformance.rst +++ b/Documentation/Adr/Adr161McpClientConformance.rst @@ -7,7 +7,8 @@ ADR-161: One conformance suite for every MCP connection we support :Status: Accepted (the timeouts row now covers the whole operation — see :ref:`ADR-170 `) :Date: 2026-08-11 -:Amended: 2026-08-13 by :ref:`ADR-170 ` +:Amended: 2026-08-13 by :ref:`ADR-170 `; 2026-08-20 by + :ref:`ADR-181 ` (the "no SSE" edge is now "no live stream") Context ======= diff --git a/Documentation/Adr/Adr181EventStreamFramedAnswers.rst b/Documentation/Adr/Adr181EventStreamFramedAnswers.rst new file mode 100644 index 000000000..118c7d6b6 --- /dev/null +++ b/Documentation/Adr/Adr181EventStreamFramedAnswers.rst @@ -0,0 +1,104 @@ +.. include:: /Includes.rst.txt + +.. _adr-181: + +============================================================================ +ADR-181: The client reads an event-stream framed answer, and holds no stream +============================================================================ + +:Status: Accepted +:Date: 2026-08-20 +:Amends: :ref:`ADR-116 ` (its transport section) and + :ref:`ADR-161 ` (its "no SSE" edge) +:Authors: Netresearch DTT GmbH + +.. _adr-181-context: + +Context +======= + +:ref:`ADR-116 ` drew the MCP client's edges as "HTTP only, no stdio, +no SSE", and the transport said so on the wire: ``Accept: application/json``, +with a comment calling that honest — a client that does not consume a stream +does not claim to accept one. + +The Streamable HTTP transport reads it differently. A client **must** offer +both media types on every POST, and a server may answer a request either as +plain JSON or as ``text/event-stream`` carrying the JSON-RPC response as a +``data:`` event. The reference server SDKs enforce the first half: measured on +2026-08-20 with a plain ``initialize`` against ``mcp.deepwiki.com/mcp``, +``mcp.context7.com/mcp`` and ``learn.microsoft.com/api/mcp``, the JSON-only +header got **406** ("Client must accept both application/json and +text/event-stream") from all three; with both offered, all three answered 200 +— and all three framed the answer as an event stream. + +So the honest header made the client unable to speak to any public MCP server, +which is exactly what the MCP Servers module exists to show. The edge ADR-116 +drew was right about what this client should *do* — not hold a stream, not +answer server-initiated requests, not speak stdio — and wrong about what it +should *say*. + +.. _adr-181-decision: + +Decision +======== + +1. The transport offers both media types: + ``Accept: application/json, text/event-stream``. + +2. An answer with content type ``text/event-stream`` is **unframed**, not + refused: the body — already bounded by the read cap and closed by the + server — is parsed by the SSE rules that matter for one request/response + exchange. Events are separated by a blank line; the ``data:`` lines of one + event join with a newline; ``event:``, ``id:``, ``retry:`` and comment lines + carry nothing that is read; CRLF is a line ending like any other. A server + may put its own notifications on the same stream before the response (the + transport allows it); this client declared no capabilities, so anything + carrying a ``method`` is passed over, and the first message that is a + response — a ``result`` or an ``error`` — is the answer. A stream with no + such message is a malformed answer and is named as one, with the reason + ("no message" versus "no response to the request"). + +3. Nothing else moves. No stream is held open, none is resumed, no + server-initiated request is answered, the operation budget + (:ref:`ADR-170 `) and the response size cap apply unchanged, and + stdio stays out of scope for the reasons ADR-116 gives. ADR-161's edge now + reads "no *live* stream" rather than "no SSE". + +4. The conformance suite (:ref:`ADR-161 `) gains the positive case — + an event-stream framed tool answer reads like a plain one — and the two + failing shapes: a stream with no message, and a stream carrying only a + server notification. The transport's own tests pin the header and the + framing rules (multi-line ``data:``, CRLF, framing lines ignored, + notification passed over). + +.. _adr-181-consequences: + +Consequences +============ + +✓ The MCP Servers module can be pointed at a public server and shown working +— the reason the typo3-demo wanted one in its seed. + +✓ The refusal vocabulary is unchanged in shape: a content type the client +cannot read is still refused by the same exception and code; only the message +stops claiming "JSON only". + +✕ The client now reads a body it previously would have rejected outright. +The read stays bounded and the parse is a line scan, so the new surface is +small; but it is a surface, and the tests for it are the contract. + +✕ A server that answers a single request with a *long-lived* stream — keeping +the connection open for notifications after the response — is read only up +to the size cap or the operation deadline, whichever comes first, and then +treated as answered or as timed out. That is acceptable for a request/response +client and is stated here so nobody mistakes it for support. + +.. _adr-181-revisit: + +Revisit when +============ + +Something on the far side needs more than one message per request — server +requests, progress notifications that must be surfaced, resumption. That is a +stream, it is what ADR-116 declined to hold, and it would be its own record. diff --git a/Documentation/Adr/Index.rst b/Documentation/Adr/Index.rst index dc0ca0f85..92249cb9d 100644 --- a/Documentation/Adr/Index.rst +++ b/Documentation/Adr/Index.rst @@ -532,3 +532,5 @@ Tools Adr178CallerSourceOnTheCostPath Adr179ADroppedForcedSourceIsRecordedOnTheRun Adr180TheSixthWriterCreatesAPage + Adr181EventStreamFramedAnswers +||||||| c6503022 diff --git a/Tests/Fixtures/Mcp/McpTestServer.php b/Tests/Fixtures/Mcp/McpTestServer.php index 29a188d0e..c64f6a7e0 100644 --- a/Tests/Fixtures/Mcp/McpTestServer.php +++ b/Tests/Fixtures/Mcp/McpTestServer.php @@ -36,7 +36,7 @@ final class McpTestServer implements ClientInterface * re-encodes as `[]` and would hide the very distinction a strict server * cares about. * - * @var list, raw: string, session: string}> + * @var list, raw: string, session: string, accept: string}> */ public array $received = []; @@ -74,6 +74,7 @@ public function sendRequest(RequestInterface $request): ResponseInterface 'body' => $body, 'raw' => $raw, 'session' => $request->getHeaderLine('Mcp-Session-Id'), + 'accept' => $request->getHeaderLine('Accept'), ]; // A notification carries no id and gets no reply, so it must not eat a diff --git a/Tests/Unit/Service/Tool/Mcp/Conformance/AbstractMcpConformanceTestCase.php b/Tests/Unit/Service/Tool/Mcp/Conformance/AbstractMcpConformanceTestCase.php index 0c3e63d8f..6efbafd93 100644 --- a/Tests/Unit/Service/Tool/Mcp/Conformance/AbstractMcpConformanceTestCase.php +++ b/Tests/Unit/Service/Tool/Mcp/Conformance/AbstractMcpConformanceTestCase.php @@ -322,6 +322,30 @@ public function executesAToolUnderItsRemoteNameAndReturnsTheResult(): void self::assertSame('mcp_' . $this->connection()->identifier . '_read_page', $tool->getSpec()->name); } + /** + * TOOL EXECUTION, answered as an event stream. + * + * The Streamable HTTP transport lets a server frame the response to a POST + * as `text/event-stream`, and the public reference servers do exactly that + * for every request. The answer must read the same as a plain JSON one: + * same result, same contact recorded — nothing about the framing is + * visible above the transport (ADR-181). + */ + #[Test] + public function readsAnEventStreamFramedAnswerLikeAPlainOne(): void + { + $fake = $this->connection()->scriptedServer()->willReturnRaw( + "event: message\ndata: {\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"content\":[{\"type\":\"text\",\"text\":\"framed: hello\"}]}}\n\n", + 200, + 'text/event-stream', + ); + + $result = $this->toolFor($fake)->execute(['uid' => 3], ToolExecutionContext::none()); + + self::assertFalse($result->isError, $result->content); + self::assertSame('framed: hello', $result->content); + } + /** * TOOL EXECUTION, failed by the TOOL rather than by the wire. * @@ -604,7 +628,8 @@ public static function serverFailures(): array 'an authentication refusal' => ['{}', 401, 'application/json'], 'a JSON-RPC error object' => ['{"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"Method not found"}}', 200, 'application/json'], 'a maintenance page' => ['maintenance', 200, 'text/html'], - 'an event stream we do not read' => ["data: {}\n\n", 200, 'text/event-stream'], + 'an event stream with no message' => [": ping\n\n", 200, 'text/event-stream'], + 'an event stream carrying only a server notification' => ["data: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/message\",\"params\":{}}\n\n", 200, 'text/event-stream'], 'a body with neither result nor error' => ['{"jsonrpc":"2.0","id":1}', 200, 'application/json'], 'an empty body' => ['', 200, 'application/json'], ]; diff --git a/Tests/Unit/Service/Tool/Mcp/McpHttpTransportTest.php b/Tests/Unit/Service/Tool/Mcp/McpHttpTransportTest.php index d0428c7a7..cda436b4f 100644 --- a/Tests/Unit/Service/Tool/Mcp/McpHttpTransportTest.php +++ b/Tests/Unit/Service/Tool/Mcp/McpHttpTransportTest.php @@ -142,9 +142,80 @@ public function turnsAJsonRpcErrorIntoATypedException(): void } #[Test] - public function refusesAnEventStreamRatherThanGuessingAtIt(): void + public function offersBothMediaTypesTheStreamableTransportRequires(): void { - $fake = (new McpTestServer())->willReturnRaw("data: {}\n\n", 200, 'text/event-stream'); + // The reference servers answer `application/json` alone with 406, so + // the header is part of the contract, not a preference (ADR-181). + $fake = (new McpTestServer())->willReturn(['tools' => []]); + + $this->transportFor($fake)->call(McpTestServer::server(), 'tools/list', [], $this->deadline()); + + self::assertSame('application/json, text/event-stream', $fake->received[0]['accept']); + } + + #[Test] + public function unframesAnEventStreamThatCarriesTheResponse(): void + { + $fake = (new McpTestServer())->willReturnRaw( + "event: message\r\nid: 7\r\ndata: {\"jsonrpc\":\"2.0\",\"id\":1,\r\ndata: \"result\":{\"tools\":[{\"name\":\"x\"}]}}\r\n\r\n", + 200, + 'text/event-stream; charset=utf-8', + ); + + $answer = $this->transportFor($fake)->call(McpTestServer::server(), 'tools/list', [], $this->deadline()); + + // Two `data:` lines of one event join with a newline; `event:` and + // `id:` lines and the leading space after `data:` are framing, not + // payload; CRLF is a line ending like any other. + self::assertSame(['tools' => [['name' => 'x']]], $answer['result']); + } + + #[Test] + public function passesOverAServerNotificationOnTheStreamAndTakesTheResponse(): void + { + $fake = (new McpTestServer())->willReturnRaw( + ": keep-alive\n\n" + . "data: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/message\",\"params\":{\"level\":\"info\"}}\n\n" + . "data: {\"jsonrpc\":\"2.0\",\"id\":1,\"result\":{\"ok\":true}}\n\n", + 200, + 'text/event-stream', + ); + + $answer = $this->transportFor($fake)->call(McpTestServer::server(), 'ping', [], $this->deadline()); + + self::assertSame(['ok' => true], $answer['result']); + } + + #[Test] + public function anEventStreamWithoutAResponseIsAMalformedAnswer(): void + { + $fake = (new McpTestServer())->willReturnRaw( + "data: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/message\",\"params\":{}}\n\n", + 200, + 'text/event-stream', + ); + + $this->expectException(McpTransportException::class); + $this->expectExceptionMessageMatches('/carried no response/'); + + $this->transportFor($fake)->call(McpTestServer::server(), 'ping', [], $this->deadline()); + } + + #[Test] + public function anEmptyEventStreamIsAMalformedAnswer(): void + { + $fake = (new McpTestServer())->willReturnRaw(": ping\n\n", 200, 'text/event-stream'); + + $this->expectException(McpTransportException::class); + $this->expectExceptionMessageMatches('/carried no message/'); + + $this->transportFor($fake)->call(McpTestServer::server(), 'ping', [], $this->deadline()); + } + + #[Test] + public function stillRefusesAContentTypeItCannotRead(): void + { + $fake = (new McpTestServer())->willReturnRaw('maintenance', 200, 'text/html'); $this->expectException(McpTransportException::class); $this->expectExceptionCode(1799990215);