Skip to content

[Server] Fix StreamableHttpTransport mutable state and clock - #487

Open
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-22-transport-state
Open

[Server] Fix StreamableHttpTransport mutable state and clock#487
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-22-transport-state

Conversation

@chr-hertel

Copy link
Copy Markdown
Member
  • Immediate response state is consumed once in handlePostRequest(), so a reused transport instance no longer replays the first POST's response.
  • $this->request is no longer reassigned mid-flight; the middleware-processed request is threaded as a parameter and the property is now readonly.
  • ClockInterface is injected (default NativeClock, matching the session stores) and used for pending-request timeouts, making the polling loop's timeout testable without a 120s sleep.

The echo/flush SSE body in CallbackStream is left as-is — it is inherent to the CallbackStream design and out of scope here.

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 changes are consistent with existing patterns and adequately covered by regression tests.

Pull request overview

Fixes stale transport state and makes timeout behavior deterministic and testable.

Changes:

  • Consumes immediate responses once and preserves the original request.
  • Injects a PSR clock for pending-request timeout checks.
  • Adds regression tests for response replay and clock-driven timeout handling.
File summaries
File Description
src/Server/Transport/StreamableHttpTransport.php Corrects state handling, request threading, and timeout clock usage.
tests/Unit/Server/Transport/StreamableHttpTransportTest.php Covers response consumption and injected-clock timeout behavior.
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