Skip to content

Comments

fix: keep connection open after responseStream to enable keep-alive ping#833

Open
JGoP-L wants to merge 1 commit intomodelcontextprotocol:mainfrom
JGoP-L:fix/keep-alive-response-stream
Open

fix: keep connection open after responseStream to enable keep-alive ping#833
JGoP-L wants to merge 1 commit intomodelcontextprotocol:mainfrom
JGoP-L:fix/keep-alive-response-stream

Conversation

@JGoP-L
Copy link

@JGoP-L JGoP-L commented Feb 24, 2026

Problem

When a Streamable HTTP client (e.g. Cursor) sends a JSON-RPC request such as
tools/list without establishing a separate GET SSE listening stream,
responseStream() calls transport.closeGracefully() immediately after sending
the response. This closes the SSE connection, preventing KeepAliveScheduler
from sending ping messages through it.

As a result, clients that rely on keep-alive pings to detect connection health
(e.g. Cursor) mark the MCP server as disconnected (red state) after the idle
timeout period.

Fixes #681

Root Cause

In McpStreamableServerSession.responseStream(), after sending the response:

  • The normal handler path unconditionally called .then(transport.closeGracefully()),
    closing the SSE connection immediately.
  • The method-not-found path returned after sendMessage() without promoting
    the transport to a listening stream.

In both cases, listeningStreamRef remains pointing to MissingMcpTransportSession,
causing KeepAliveScheduler ping attempts to fail with "Stream unavailable".

Copilot AI review requested due to automatic review settings February 24, 2026 03:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an issue where Streamable HTTP clients that don't establish a separate GET SSE listening stream (e.g., Cursor IDE) would be marked as disconnected after idle timeout. The root cause was that responseStream() would close the connection immediately after sending a response, preventing KeepAliveScheduler from sending ping messages.

Changes:

  • Modified responseStream() to conditionally promote the response stream to a listening stream when no listening stream exists
  • Applied the fix to both the error handler path (method-not-found) and the success handler path

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 175 to 211
@@ -189,7 +199,16 @@ public Mono<Void> responseStream(McpSchema.JSONRPCRequest jsonrpcRequest, McpStr
return Mono.just(errorResponse);
})
.flatMap(transport::sendMessage)
.then(transport.closeGracefully());
.then(Mono.defer(() -> {
McpLoggableSession currentListeningStream = this.listeningStreamRef.get();
if (currentListeningStream == this.missingMcpTransportSession) {
if (this.listeningStreamRef.compareAndSet(this.missingMcpTransportSession, stream)) {
logger.debug("Converted response stream to listening stream for session {}", this.id);
return Mono.empty();
}
}
return transport.closeGracefully();
}));
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for promoting a response stream to a listening stream is duplicated in both the error handler path (lines 175-184) and the success handler path (lines 202-211). Consider extracting this into a private helper method to reduce duplication and improve maintainability. For example:

private Mono<Void> promoteToListeningStreamOrClose(
    McpStreamableServerSessionStream stream, 
    McpStreamableServerTransport transport) {
    return Mono.defer(() -> {
        McpLoggableSession currentListeningStream = this.listeningStreamRef.get();
        if (currentListeningStream == this.missingMcpTransportSession) {
            if (this.listeningStreamRef.compareAndSet(this.missingMcpTransportSession, stream)) {
                logger.debug("Converted response stream to listening stream for session {}", this.id);
                return Mono.empty();
            }
        }
        return transport.closeGracefully();
    });
}

This would make the code easier to maintain and ensure consistent behavior across both paths.

Copilot uses AI. Check for mistakes.
After sending a response in responseStream(), if no listening stream
exists yet (i.e., the client hasn't established a GET SSE connection),
promote the current response stream to a listening stream instead of
closing it. This allows KeepAliveScheduler to send periodic ping
messages through the transport.

Clients like Cursor that don't establish a separate GET listening
stream would otherwise have the connection closed immediately after
each response, causing the MCP server to appear as disconnected
after the idle-timeout period.

Fixes modelcontextprotocol#681
@JGoP-L JGoP-L force-pushed the fix/keep-alive-response-stream branch from e001542 to e82fee0 Compare February 24, 2026 03:28
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.

WebFluxStreamableServerTransportProvider closes connection after response, blocking keep-alive ping

1 participant