fix: keep connection open after responseStream to enable keep-alive ping#833
fix: keep connection open after responseStream to enable keep-alive ping#833JGoP-L wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
There was a problem hiding this comment.
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.
| @@ -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(); | |||
| })); | |||
There was a problem hiding this comment.
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.
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
e001542 to
e82fee0
Compare
Problem
When a Streamable HTTP client (e.g. Cursor) sends a JSON-RPC request such as
tools/listwithout establishing a separate GET SSE listening stream,responseStream()callstransport.closeGracefully()immediately after sendingthe response. This closes the SSE connection, preventing
KeepAliveSchedulerfrom 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:.then(transport.closeGracefully()),closing the SSE connection immediately.
sendMessage()without promotingthe transport to a listening stream.
In both cases,
listeningStreamRefremains pointing toMissingMcpTransportSession,causing
KeepAliveSchedulerping attempts to fail with "Stream unavailable".