Skip to content

[Server] Stop the handshake-era SSE loop once the client disconnected - #569

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-sse-abandoned-stream
Oct 13, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix-sse-abandoned-stream

Conversation

@chr-hertel

@chr-hertel chr-hertel commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

A client that disconnects while its call waits on an elicitation kept the PHP worker polling until the request timed out (120s by default) - the loop never checked connection_aborted(), StatelessProtocol already does.

  • the idle stream writes a : keep-alive comment every 250ms (same pace as the stateless keep-alive), since PHP only notices a dropped peer when it writes
  • once the connection is aborted, the suspended fiber is dropped and the loop ends
  • the check sits behind a protected isConnectionAborted() to keep it testable

Fixes chr-hertel#60

🤖 Generated with Claude Code

@chr-hertel
chr-hertel requested a review from Nyholm as a code owner October 10, 2026 01:54
@chr-hertel chr-hertel added this to the 0.9.0 milestone Oct 10, 2026
@chr-hertel
chr-hertel force-pushed the fix-sse-abandoned-stream branch from c3aa3ec to eee8070 Compare October 10, 2026 01:55
@chr-hertel chr-hertel added bug Something isn't working Server Issues & PRs related to the Server component labels Oct 10, 2026
@chr-hertel
chr-hertel requested a balanced review from Copilot October 12, 2026 20:09

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.

🟡 Changes recommended

The abort path leaves the awaited request persisted in session state.

1 open finding
What changed in this PR

Stops handshake-era SSE polling after client disconnection, preventing PHP worker exhaustion.

Changes:

  • Sends periodic SSE keep-alive comments to detect disconnects.
  • Exits polling when the connection aborts.
  • Adds regression coverage and changelog documentation.
File Description
src/​Server/​Transport/​StreamableHttpTransport.php Adds keep-alives and disconnect handling.
tests/​Unit/​Server/​Transport/​StreamableHttpTransportTest.php Tests disconnected-client behavior.
CHANGELOG.md Documents the fix.

🧠 Review effort: Balanced


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

Comment on lines +350 to +353
if ($this->isConnectionAborted()) {
$this->logger->info('SSE: Client disconnected, dropping the suspended request.');

return;
@chr-hertel
chr-hertel force-pushed the fix-sse-abandoned-stream branch 2 times, most recently from 980d5a9 to da06f4a Compare October 12, 2026 23:27
soyuka
soyuka previously approved these changes Oct 13, 2026
@chr-hertel
chr-hertel force-pushed the fix-sse-abandoned-stream branch from da06f4a to 022503f Compare October 13, 2026 22:00
@chr-hertel
chr-hertel merged commit 9c279f9 into modelcontextprotocol:main Oct 13, 2026
54 of 56 checks passed
@chr-hertel
chr-hertel deleted the fix-sse-abandoned-stream branch October 13, 2026 22:05
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.

[Server] Handshake-era SSE loop keeps polling after the client disconnected

3 participants