Skip to content

[Server] Fix lost responses on concurrent requests of the same session (Streamable HTTP) - #535

Open
guillaume-sainthillier wants to merge 5 commits into
modelcontextprotocol:mainfrom
guillaume-sainthillier:fix-275-inline-responses
Open

guillaume-sainthillier wants to merge 5 commits into
modelcontextprotocol:mainfrom
guillaume-sainthillier:fix-275-inline-responses

Conversation

@guillaume-sainthillier

Copy link
Copy Markdown

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):

Server Load main this PR
FrankenPHP worker mode 100 rounds × 10 tools/call 0 rounds correct; 328 foreign, 185 lost 100 rounds correct; 0 foreign, 0 lost
php -S, 8 workers 100 rounds × 10 tools/call 10 rounds correct; 159 foreign, 80 lost 100 rounds correct; 0 foreign, 0 lost
php -S, 8 workers 200 rounds × tools/list + resources/list 104 rounds correct; 58 foreign, 38 lost 200 rounds correct; 0 foreign, 0 lost

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:

  1. Protocol::sendResponse() queued it in _mcp.outgoing_queue (Protocol.php#L476-L482, #L503-L508) in the session data loaded at the start of the request.
  2. doProcessInput() saved that whole session back (#L223).
  3. createJsonResponse() reloaded the session and took the whole queue (StreamableHttpTransport.php#L224-L234, Protocol.php#L516-L524).

With two requests A and B on one session:

  • If both load the session before either saves, the second save overwrites the first one's queued response. That response is gone (a lost update), and its POST answers 202.
  • If B saves after A, whichever consumes first takes both responses, and the other POST answers 202.

Filtering the queue by request id (#508) fixes the second case but not the first.

Fix

  • StreamableHttpTransport implements a new marker interface, InlineResponseTransportInterface. For such a transport, Protocol::sendResponse() hands every response to TransportInterface::send(), with the session id in the session_id context key, instead of queueing it. This is the path session-less errors already took.
  • The transport collects these responses and answers the POST with them: one object, or an array for a batch, as before. Responses no longer go through the session store, so no interleaving can lose them.
  • When a request in a batch suspends (SSE), the other requests' responses are written first in the stream.
  • The session queue still carries server-initiated requests and notifications: fiber yields on SSE streams, and resource-updated notifications. A JSON POST still drains it as before, so notifications are delivered as they were.
  • consumeOutgoingMessages() only saves when it took something. Saving an unchanged session on every POST could only overwrite what a concurrent request had saved in the meantime.

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:

    • B runs between A's save and A's response;
    • B loaded the session before A saved it (the lost update).

    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.

  • Unit, integration and inspector suites pass. php-cs-fixer is clean. PHPStan only reports the two errors in ElicitationSchema.php that are already there and unrelated.

Backward compatibility

  • The new behavior only applies to transports that implement InlineResponseTransportInterface. StdioTransport, InMemoryTransport and custom transports keep the queue. Stdio would work unchanged on the inline path too; making it the default for every transport could be a follow-up.
  • StreamableHttpTransport: no signature changes. createJsonResponse() and createStreamedResponse() keep their protected signatures.
  • Protocol::consumeOutgoingMessages() returns the same thing. It just skips a save that would not change anything.

Not in this PR

  • Other session keys written by concurrent requests can still be lost: pending requests, client responses waiting for a suspended handler, client info. Fixing that needs per-session serialization, e.g. an opt-in lock around load → handle → save (flock for FileSessionStore, symfony/lock for the PSR-16 store). symfony/mcp-bundle would then need an option to wire it. Happy to follow up if that direction suits you.
  • Until now, applications worked around this with a per-session lock around the whole request, e.g. a decorator of symfony/mcp-bundle's mcp.server.<name>.controller wrapping handle() in symfony/lock's LockFactory::createLock('mcp-session-'.$sessionId). With this PR, that lock is no longer needed for responses.

…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.
PHPStan 2.3.0 reports the runtime checks of fromArray() as redundant with its phpdoc shape. They guard input from the wire, so keep them.
Comment thread src/Schema/Elicitation/ElicitationSchema.php
@chr-hertel

Copy link
Copy Markdown
Member

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 :)

@chr-hertel chr-hertel added Server Issues & PRs related to the Server component P1 Significant bug affecting many users, highly requested feature bug Something isn't working labels Oct 6, 2026
@guillaume-sainthillier
guillaume-sainthillier marked this pull request as ready for review October 7, 2026 07:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P1 Significant bug affecting many users, highly requested feature Server Issues & PRs related to the Server component

Projects

None yet

2 participants