[Server] Answer a JSON POST with only its own responses by vbcherepanov · Pull Request #508 · modelcontextprotocol/php-sdk · GitHub

Fixes #467.

With PHP-FPM, requests of the same session run in parallel and put their responses into the same session queue. createJsonResponse() took the whole queue, so one request got all responses as a JSON array and the others got 202 with nothing.

Now the transport collects the ids from its own POST body and takes only those responses. The rest stays in the queue. Batches are still answered with an array, single messages with one object, and an invalid message without a usable id still gets its error.

Protocol::consumeOutgoingMessages() gets the ids as an optional second argument. Without it nothing changes, so the SSE path and StdioTransport work as before.

The reproducer is testConcurrentPostsSharingASessionEachReceiveTheirOwnResponse: the second POST is handled completely between the first one queueing its response and reading the queue. It fails on main.

Not part of this PR:

the lost update on the session data itself ([Server][Streamable HTTP] Concurrent requests in the same session can overwrite queued responses and cause stale/unknown message IDs #275). The queue is read and written like before, and nothing is written if there is nothing to take.
notifications queued outside of an SSE stream (e.g. resource subscriptions) are not added to a JSON response anymore, since they could only go there as an invalid array. They stay in the queue.

AI disclosure: I used Claude Code to investigate the issue and help write the reproducer test. The fix itself is my own.

Independent confirmation on v0.6.0 (Apache + mod_php), driving a real MCP server rather than a unit test: 20 concurrent tools/list + resources/list pairs, each on a freshly initialized session, both requests released from a thread barrier.

build	pairs	correct	cross-delivery	response lost (empty 202)	HTTP 500
v0.6.0	40	27	11	2	0
v0.6.0 + this PR	80	70	0	6	4
v0.6.0 + this PR + #500	60	59	0	1	0

This PR removes the cross-delivery completely — 0 in 140 pairs after it, against 11 in 40 before. Both shapes described in #467 are gone: the JSON array carrying another request's response, and the 202 the other request got instead of its own.

Two notes from the same runs:

The HTTP 500s that surface once cross-delivery is gone are JsonException: Control character error from Session::readData() on a half-written session file — [Server] Concurrent requests corrupt the session file in FileSessionStore, crashing Session::readData() with JsonException #498, fixed by [Server] Stop concurrent writes from corrupting session files #500. With both PRs applied they disappear. This PR does not cause them, it just stops masking them.
One lost response survives both (1 in 60 pairs), which is the lost update of [Server][Streamable HTTP] Concurrent requests in the same session can overwrite queued responses and cause stale/unknown message IDs #275 that this PR correctly says it does not address.

Real-client impact, for the record: Claude Code issues tools/list and resources/list concurrently on every connect, so a plain connection hits this. On a hit it cancels the orphaned request after 30 s with notifications/cancelled … "Request timed out" and silently ends up with no resources.

The patch was applied by hand onto v0.6.0 (the files have moved since), so line numbers differ but the logic is unchanged.

Correction to my numbers above, and a data point on v0.8.1.

The residual-loss figure in my comment — "one lost response survives both (1 in 60 pairs)" — came from too small a sample. Larger runs put it at 6 in 180 pairs on v0.6.0 with both PRs applied, so roughly 1 in 30. That residual is still the lost update of #275 and still outside this PR's scope, so nothing here changes; I am correcting it because "1 in 60" understates how often a real client ends up waiting on a response that was silently dropped.

The cross-delivery result did not move, and it is the one I would stand behind: 0 in 180 pairs.

v0.8.1 still has the bug. Same harness against stock v0.8.1: 65 % of pairs correct, versus 67 % on stock v0.6.0. So the v0.8.0 refactor did not incidentally fix this and the PR is still needed against current releases — worth saying explicitly, because the files have moved enough that four hunks had to be finished by hand when porting it there.

base	pairs	correct	cross-delivery	response lost
stock v0.6.0	40	67 %	11	2
v0.6.0 + 508 + 500	180	97 %	0	6
stock v0.8.1	60	65 %	many	—
v0.8.1 + 508 + 500	180	91 %	0	17

One caveat on that 17, since it would be easy to misread as "v0.8.1 loses more than v0.6.0": those runs shared a busy machine and a single run contributed 5 of them. I would not claim a difference between the two bases without re-measuring on an idle box. The zero cross-delivery is the robust part of both patched rows.

Same setup as before — Apache + mod_php, 20 concurrent tools/list + resources/list pairs per run, each pair on a freshly initialized session, both requests released from a thread barrier.

Thanks for the input @michalcharvat - going with concurrency tests here was a good idea 👍

Hi @vbcherepanov, thanks for working on this, also started reviewing on detail level but got confused about the json_encode/decode handling we need to do here and i wonder if this is solving the issue on the right layer.

and ended up similar to the discussion in #275 - let's sort it there at first.

Suggested change

      
                private static function expectedResponses(string $body): array
          
                private function expectedResponses(string $body): array

Suggested change

      
                 * @param callable(Uuid $sessionId, list<int|string|null>|null $responseIds=): array<int, array{message: string, context: array<string, mixed>}> $provider
          
                 * @param callable(Uuid $sessionId, list<int|string|null>|null $responseIds): array<int, array{message: string, context: array<string, mixed>}> $provider

Suggested change

      
                private static function isResponseTo(array $message, array $responseIds): bool
          
                private function isResponseTo(array $message, array $responseIds): bool

this make sense as solution but introduces an issue of potentially piling up $remaining responses in the memory, right?

vbcherepanov requested review from CodeWithKyrian, Nyholm, chr-hertel and soyuka as code owners September 14, 2026 12:39

[Server] Answer a JSON POST with only its own responses

64f02a0

vbcherepanov force-pushed the fix/467-json-response-own-messages branch from e0fced5 to 64f02a0 Compare September 15, 2026 10:19

guillaume-sainthillier mentioned this pull request Sep 25, 2026

[Server][Streamable HTTP] Concurrent requests in the same session can overwrite queued responses and cause stale/unknown message IDs #275

Open

chr-hertel added the Server Issues & PRs related to the Server component label Oct 5, 2026

chr-hertel requested changes Oct 5, 2026

View reviewed changes

chr-hertel added needs design Valid issue but needs maintainer alignment on design or approach needs more work Not ready to be merged yet, needs additional follow-up from the author(s). labels Oct 5, 2026

guillaume-sainthillier mentioned this pull request Oct 6, 2026

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

Draft

+                   * @param array{message: string, context: array<string, mixed>} $message
+                   * @param list<int|string|null>                                 $responseIds
+                   */
+                  private static function isResponseTo(array $message, array $responseIds): bool

+                      foreach ($queue as $message) {
+                          if (self::isResponseTo($message, $responseIds)) {
+                              $consumed[] = $message;
+                          } else {
+                              $remaining[] = $message;
+                          }
+                      }
+                      if ([] !== $consumed) {
+                          $session->set(self::SESSION_OUTGOING_QUEUE, $remaining);
+                          $session->save();
+                      }

[Server] Answer a JSON POST with only its own responses by vbcherepanov · Pull Request #508 · modelcontextprotocol/php-sdk · GitHub

Repository navigation

[Server] Answer a JSON POST with only its own responses - #508

[Server] Answer a JSON POST with only its own responses#508
vbcherepanov wants to merge 1 commit into
modelcontextprotocol:mainfrom
vbcherepanov:fix/467-json-response-own-messages

vbcherepanov commented Sep 14, 2026 •
edited
Loading

Uh oh!

Uh oh!

michalcharvat commented Sep 17, 2026

Uh oh!

michalcharvat commented Sep 17, 2026

Uh oh!

chr-hertel commented Oct 5, 2026

Uh oh!

chr-hertel left a comment

Uh oh!

chr-hertel Oct 5, 2026

Uh oh!

chr-hertel Oct 5, 2026

Uh oh!

chr-hertel Oct 5, 2026

Uh oh!

chr-hertel Oct 5, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	private static function expectedResponses(string $body): array
	private function expectedResponses(string $body): array

+                   * are passed, only the responses to those requests are returned.
                    *
-                   * @param callable(Uuid $sessionId): array<int, array{message: string, context: array<string, mixed>}> $provider
+                   * @param callable(Uuid $sessionId, list<int|string|null>|null $responseIds=): array<int, array{message: string, context: array<string, mixed>}> $provider

	* @param callable(Uuid $sessionId, list<int\|string\|null>\|null $responseIds=): array<int, array{message: string, context: array<string, mixed>}> $provider
	* @param callable(Uuid $sessionId, list<int\|string\|null>\|null $responseIds): array<int, array{message: string, context: array<string, mixed>}> $provider

	private static function isResponseTo(array $message, array $responseIds): bool
	private function isResponseTo(array $message, array $responseIds): bool

[Server] Answer a JSON POST with only its own responses by vbcherepanov · Pull Request #508 · modelcontextprotocol/php-sdk · GitHub

Repository navigation

[Server] Answer a JSON POST with only its own responses - #508

[Server] Answer a JSON POST with only its own responses#508vbcherepanov wants to merge 1 commit intomodelcontextprotocol:mainmodelcontextprotocol/php-sdk:mainfrom vbcherepanov:fix/467-json-response-own-messagesvbcherepanov/php-sdk:fix/467-json-response-own-messagesCopy head branch name to clipboard

Conversation

vbcherepanov commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michalcharvat commented Sep 17, 2026

Uh oh!

michalcharvat commented Sep 17, 2026

Uh oh!

chr-hertel commented Oct 5, 2026

Uh oh!

chr-hertel left a comment

Choose a reason for hiding this comment

Uh oh!

chr-hertel Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

chr-hertel Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

chr-hertel Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

chr-hertel Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Server] Answer a JSON POST with only its own responses#508
vbcherepanov wants to merge 1 commit into
modelcontextprotocol:mainfrom
vbcherepanov:fix/467-json-response-own-messages

vbcherepanov commented Sep 14, 2026 •
edited
Loading