Skip to content

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

Open
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-27-handlerequest-reassign
Open

[Server] Keep inbound request intact when queueing an outbound one#486
chr-hertel wants to merge 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.

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