| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
nevermind changelog conflicts. they will happen all the time, and i can easily solve them while merging. just put the PR to ready for review when you think you're done :) |
Sorry, something went wrong.
There was a problem hiding this comment.
The response race is addressed without changing other transports, with focused deterministic coverage for both interleavings.
0 open findings
What changed in this PRMoves Streamable HTTP responses out of shared session queues, preventing concurrent requests from losing or consuming each other’s responses.
Changes:
| File | Description |
|---|---|
| CHANGELOG.md | Documents the concurrency fix. |
| src/Server/Protocol.php | Sends eligible responses inline and avoids unnecessary saves. |
| src/Server/Transport/InlineResponseTransportInterface.php | Defines inline-response transport capability. |
| src/Server/Transport/StreamableHttpTransport.php | Collects inline responses for JSON and SSE output. |
| tests/Unit/Server/Transport/Fixture/InterleavingSessionStore.php | Simulates deterministic concurrent session writes. |
| tests/Unit/Server/Transport/StreamableHttpTransportTest.php | Tests concurrent POSTs and streamed batches. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
|
@guillaume-sainthillier Please rebase and have a look if the test added in PR #545 is valuable to add here - thanks already! |
Sorry, something went wrong.
|
Rebased on main and added the test from #545, adapted to share the store fixture. |
Sorry, something went wrong.
…n (Streamable HTTP) Fixes modelcontextprotocol#275 and modelcontextprotocol#467. A response to a POST went through the session's outgoing queue, which concurrent requests of the same session read and write back whole. One request could then answer with another's response, and the other with an empty 202. StreamableHttpTransport now implements InlineResponseTransportInterface: Protocol hands it the responses through send() and the transport answers the POST with them. The queue keeps server-initiated requests and notifications.
Ported from modelcontextprotocol#545: one worker polls the session for a client's answer while another stores it. Saving the session on every poll used to overwrite that answer. The store fixture moves to Session/Fixture and gains a hook after the next read. Co-authored-by: Christopher Hertel <mail@christopher-hertel.de>
There was a problem hiding this comment.
Thanks @guillaume-sainthillier!
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #275, fixes #467. Implements the approach discussed in #275 (comment), replacing the filtering of #508.
Symptom
On Streamable HTTP (handshake era, JSON responses), concurrent requests on one session lose responses. A POST answers 202 with an empty body instead of its JSON-RPC response, or answers with another request's response, sometimes inside an array. The client waits until it times out (TypeScript SDK: MCP error -32001: Request timed out). Parallel tool calls from LLM agents (n8n / LangChain) and Claude Code's concurrent tools/list and resources/list on connect run into it.
Reproduction
Any server with several workers and a shared session store. Initialize a session, then send N POSTs at once with the same Mcp-Session-Id. Measured with https://github.com/chr-hertel/mcp-concurrency-test (file store, loopback):
In production on FrankenPHP worker mode (mcp/sdk 0.8.1 through symfony/mcp-bundle), 1 call in 10 got a 202.
Root cause
A response was not returned by the POST that carried its request. It went through the session:
With two requests A and B on one session:
Filtering the queue by request id (#508) fixes the second case but not the first.
Fix
Tests
StreamableHttpTransportTest::testConcurrentPostsOfOneSessionEachGetTheirOwnResponse runs two tools/call POSTs through real Server and StreamableHttpTransport instances that share one store. The fixture InterleavingSessionStore runs B from inside A's session save, so the interleaving is deterministic and needs no real concurrency. There are two cases:
Each POST must answer 200 with its own id and the session header. Both cases fail on main, where A answers 202.
testStreamedBatchCarriesInlineResponses: a batch whose tool call suspends to send progress still streams the ping response.
ProtocolSessionRaceTest, ported from [Server] Stop overwriting client responses while polling for them #545: a client's answer stored by one worker while another polls the session for it is not overwritten. It fails without the consumeOutgoingMessages() change.
Unit and integration suites pass, PHPStan (level 8) and php-cs-fixer are clean.
Backward compatibility
Not in this PR