Skip to content

[Server] Accept a POST without requests with 202 and no body - #597

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-post-drains-outgoing-queue
Oct 14, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-post-drains-outgoing-queue

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

Stacked on #581 - merge after it, only the last commit belongs here.

A POST carrying only responses or notifications was answered with 200 and the session's queued messages (e.g. resources/updated), while the spec requires 202 Accepted with no body.

  • createJsonResponse() answers such a POST with 202 and no body, without draining the outgoing queue
  • The queued messages stay for a later answer

Part of chr-hertel#73
Delivering session-wide messages on a plain request (it still answers with a JSON array) is still open.

🤖 Generated with Claude Code

@chr-hertel chr-hertel added this to the 0.9.0 milestone Oct 13, 2026
@chr-hertel
chr-hertel requested a review from soyuka as a code owner October 13, 2026 23:34
@chr-hertel chr-hertel added bug Something isn't working Server Issues & PRs related to the Server component labels Oct 13, 2026
@chr-hertel
chr-hertel requested a balanced review from Copilot October 13, 2026 23:39

Copilot AI 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.

🔵 Needs a closer look

The stacked diff includes cross-transport concurrency changes and public API breaks that warrant final human validation.

0 open findings

What changed in this PR

Ensures response/notification-only HTTP POSTs return 202 Accepted without draining queued session messages, atop concurrency fixes from #581.

Changes:

  • Return an empty 202 when a POST produces no request responses.
  • Use UUID outbound request IDs and transport-specific fiber queues.
  • Expand HTTP, stdio, timeout, and concurrency coverage.
File Description
src/​Server/​Transport/​StreamableHttpTransport.php Preserves queued messages for response/notification-only POSTs.
src/​Server/​Protocol.php Adds UUID IDs and per-transport outgoing queues.
src/​Server/​Transport/​StdioTransport.php Polls UUID-based requests.
src/​Server/​Transport/​BaseTransport.php Updates response lookup to UUIDs.
src/​Server/​Transport/​TransportInterface.php Updates callback contracts.
src/​Server/​Transport/​ManagesTransportCallbacks.php Updates callback type declarations.
tests/​Unit/​Server/​Transport/​StreamableHttpTransportTest.php Tests 202 responses and queue preservation.
tests/​Unit/​Server/​Transport/​StdioSuspendedFibersTest.php Tests dynamically generated request IDs.
tests/​Unit/​Server/​ProtocolTest.php Covers UUID and stream-isolation behavior.
tests/​Unit/​Server/​ProtocolSessionRaceTest.php Updates race tests for UUID IDs.
tests/​Unit/​Fixtures/​PollingLoopTransport.php Exposes stream-specific outgoing messages.
CHANGELOG.md Documents fixes and breaking API changes.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@chr-hertel
chr-hertel force-pushed the fix-post-drains-outgoing-queue branch from e7a9988 to 912779d Compare October 14, 2026 22:56
@chr-hertel
chr-hertel requested a balanced review from Copilot October 14, 2026 22:57

Copilot AI 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.

🟢 Approval recommended

The focused change preserves existing request-response handling, includes regression coverage, and has no identified blocking issues.

0 open findings

🧠 Review effort: Balanced

@chr-hertel
chr-hertel merged commit 0008c00 into modelcontextprotocol:main Oct 14, 2026
28 checks passed
@chr-hertel
chr-hertel deleted the fix-post-drains-outgoing-queue branch October 14, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants