Skip to content

[Server] Keep inbound request intact when queueing an outbound one - #486

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-27-handlerequest-reassign
Sep 4, 2026
Merged

[Server] Keep inbound request intact when queueing an outbound one#486
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-27-handlerequest-reassign

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

Protocol::handleRequest() reassigned $request to the outbound (id-less) request when a fiber suspends, so a failure while queueing it hit an uninitialized Request::$id in the catch blocks instead of answering under the inbound request's id.

Renames the local and adds a regression test.

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 targeted fix is correct and adequately covered by the regression test.

Pull request overview

Preserves the inbound request during outbound fiber suspension so failures retain the correct response ID.

Changes:

  • Uses a distinct $outboundRequest variable.
  • Adds regression coverage for transport attachment failures.
File summaries
File Description
src/Server/Protocol.php Prevents overwriting the inbound request.
tests/Unit/Server/ProtocolTest.php Verifies errors use the inbound request ID.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: 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 merged commit 749c035 into modelcontextprotocol:main Sep 4, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants