Skip to content

Commit 736efe9

Browse files
kpavlovdevcrocod
andauthored
fix(server): notifications not delivered over Streamable HTTP (#587) (#599)
## Changes - fix(server): keep SSE connection open until explicitly cancelled - conformance: add logging to test_tool_with_logging and update troubleshooting documentation - chore(conformance): remove 5 passing server tests from conformance baselines: - tools-call-with-logging - tools-call-with-progress - tools-call-sampling - tools-call-elicitation - elicitation-sep1034-defaults ## Motivation and Context Two bugs in StreamableHttpServerTransport caused server-to-client notifications (e.g. notifications/message during tool calls) to be silently dropped. **Bug 1 — GET SSE stream closed immediately** `handleGetRequest` rejected GET requests with 405 when `enableJsonResponse = true` (which `mcpStreamableHttp` always sets). Since the Ktor `sse {}` handler commits response headers before the body runs, the reject failed silently and the function returned, causing Ktor to close the SSE stream. There was also no `awaitCancellation()` to keep the connection alive. **Bug 2 — Notifications discarded in JSON mode** In `send()`, notifications with a relatedRequestId entered the POST-stream path but hit `if (!isTerminated) return` without being forwarded anywhere. **Fix** - Remove the enableJsonResponse guard from handleGetRequest. The GET SSE stream is an always-required notification channel, orthogonal to how POST responses are delivered. - Add `awaitCancellation()` to keep the GET SSE connection open until the client disconnects or the transport closes. - In `send()`, route notifications with a relatedRequestId to the standalone GET SSE stream when `enableJsonResponse = true`. ## How Has This Been Tested? Verified against the MCP conformance test suite — the ToolsCallWithLogging scenario now passes. ngrep confirms the `GET /mcp` stream stays open and `notifications/message` events arrive before the `tools/call` response. ## Breaking Changes <!-- Will users need to update their code or configurations? --> ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context Fixes #587 Requires #585 to be merged first. --------- Co-authored-by: devcrocod <devcrocod@gmail.com>
1 parent e88b341 commit 736efe9

5 files changed

Lines changed: 37 additions & 24 deletions

File tree

conformance-test/README.md

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,11 @@ Tests the conformance server against all server scenarios:
114114
8 scenarios are expected to fail due to current SDK limitations (tracked in [
115115
`conformance-baseline.yml`](conformance-baseline.yml).
116116

117-
| Scenario | Suite | Root Cause |
118-
|---------------------------------------|--------|--------------------------------------------------------------------------------------------------------------------------------------------------------|
119-
| `tools-call-with-logging` | server | Notifications from tool handlers have no `relatedRequestId`; transport routes them to the standalone SSE stream instead of the request-specific stream |
120-
| `tools-call-with-progress` | server | *(same as above)* |
121-
| `tools-call-sampling` | server | *(same as above)* |
122-
| `tools-call-elicitation` | server | *(same as above)* |
123-
| `elicitation-sep1034-defaults` | server | *(same as above)* |
124-
| `elicitation-sep1330-enums` | server | *(same as above)* |
125-
| `resources-templates-read` | server | SDK does not implement `addResourceTemplate()` with URI pattern matching; resources are looked up by exact URI |
126-
| `elicitation-sep1034-client-defaults` | client | SDK does not fill in `default` values from the elicitation request schema before sending the response |
117+
| Scenario | Suite | Root Cause |
118+
|---------------------------------------|--------|----------------------------------------------------------------------------------------------------------------|
119+
| `elicitation-sep1330-enums` | server | *(same as above)* |
120+
| `resources-templates-read` | server | SDK does not implement `addResourceTemplate()` with URI pattern matching; resources are looked up by exact URI |
121+
| `elicitation-sep1034-client-defaults` | client | SDK does not fill in `default` values from the elicitation request schema before sending the response |
127122

128123
These failures reveal SDK gaps and are intentionally not fixed in this module.
129124

conformance-test/conformance-baseline.yml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
# Conformance test baseline - expected failures
22
# Add entries here as tests are identified as known SDK limitations
33
server:
4-
- tools-call-with-logging
5-
- tools-call-with-progress
6-
- tools-call-sampling
7-
- tools-call-elicitation
8-
- elicitation-sep1034-defaults
94
- elicitation-sep1330-enums
105
- resources-templates-read
116

conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceTools.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.modelcontextprotocol.kotlin.sdk.conformance
22

3+
import io.github.oshai.kotlinlogging.KotlinLogging
34
import io.modelcontextprotocol.kotlin.sdk.server.Server
45
import io.modelcontextprotocol.kotlin.sdk.types.AudioContent
56
import io.modelcontextprotocol.kotlin.sdk.types.CallToolResult
@@ -33,6 +34,8 @@ internal const val PNG_BASE64 =
3334
// Minimal WAV (base64)
3435
internal const val WAV_BASE64 = "UklGRiYAAABXQVZFZm10IBAAAAABAAEAQB8AAAB9AAACABAAZGF0YQIAAAA="
3536

37+
private val logger = KotlinLogging.logger {}
38+
3639
@Suppress("LongMethod")
3740
fun Server.registerConformanceTools() {
3841
// 1. Simple text
@@ -429,6 +432,7 @@ fun Server.registerConformanceTools() {
429432
name = "test_tool_with_logging",
430433
description = "test_tool_with_logging",
431434
) {
435+
logger.debug { "[test_tool_with_logging] Sending message 1" }
432436
sendLoggingMessage(
433437
LoggingMessageNotification(
434438
LoggingMessageNotificationParams(
@@ -439,6 +443,7 @@ fun Server.registerConformanceTools() {
439443
),
440444
)
441445
delay(50.milliseconds)
446+
logger.debug { "[test_tool_with_logging] Sending message #2" }
442447
sendLoggingMessage(
443448
LoggingMessageNotification(
444449
LoggingMessageNotificationParams(
@@ -449,6 +454,7 @@ fun Server.registerConformanceTools() {
449454
),
450455
)
451456
delay(50.milliseconds)
457+
logger.debug { "[test_tool_with_logging] Sending message 3" }
452458
sendLoggingMessage(
453459
LoggingMessageNotification(
454460
LoggingMessageNotificationParams(
@@ -458,6 +464,7 @@ fun Server.registerConformanceTools() {
458464
),
459465
),
460466
)
467+
461468
CallToolResult(listOf(TextContent("Simple text content")))
462469
}
463470

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Level of logging for the ROOT logger: ERROR, WARN, INFO, DEBUG, TRACE (default is INFO)
2+
org.slf4j.simpleLogger.defaultLogLevel=INFO
3+
org.slf4j.simpleLogger.showThreadName=true
4+
org.slf4j.simpleLogger.showDateTime=false
5+
6+
# Log level for specific packages or classes
7+
org.slf4j.simpleLogger.log.io.ktor.server=DEBUG
8+
org.slf4j.simpleLogger.log.io.modelcontextprotocol=DEBUG
9+
org.slf4j.simpleLogger.log.io.modelcontextprotocol.kotlin.sdk.conformance=INFO

kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import io.modelcontextprotocol.kotlin.sdk.types.RPCError
2828
import io.modelcontextprotocol.kotlin.sdk.types.RPCError.ErrorCode.REQUEST_TIMEOUT
2929
import io.modelcontextprotocol.kotlin.sdk.types.RequestId
3030
import io.modelcontextprotocol.kotlin.sdk.types.SUPPORTED_PROTOCOL_VERSIONS
31+
import kotlinx.coroutines.awaitCancellation
3132
import kotlinx.coroutines.job
3233
import kotlinx.coroutines.sync.Mutex
3334
import kotlinx.coroutines.sync.withLock
@@ -242,7 +243,15 @@ public class StreamableHttpServerTransport(private val configuration: Configurat
242243
}
243244

244245
val isTerminated = message is JSONRPCResponse || message is JSONRPCError
245-
if (!isTerminated) return
246+
if (!isTerminated) {
247+
if (configuration.enableJsonResponse) {
248+
// In JSON response mode there is no per-request SSE stream, so route notifications
249+
// that are logically associated with a request to the standalone GET SSE stream.
250+
val standaloneStream = streamsMapping[STANDALONE_SSE_STREAM_ID]
251+
standaloneStream?.let { emitOnStream(STANDALONE_SSE_STREAM_ID, it.session, message) }
252+
}
253+
return
254+
}
246255

247256
requestToResponseMapping[responseRequestId!!] = message
248257
val relatedIds = requestToStreamMapping.filterValues { it == streamId }.keys
@@ -411,14 +420,9 @@ public class StreamableHttpServerTransport(private val configuration: Configurat
411420

412421
@Suppress("ReturnCount")
413422
public suspend fun handleGetRequest(session: ServerSSESession?, call: ApplicationCall) {
414-
if (configuration.enableJsonResponse) {
415-
call.reject(
416-
HttpStatusCode.MethodNotAllowed,
417-
RPCError.ErrorCode.CONNECTION_CLOSED,
418-
"Method not allowed.",
419-
)
420-
return
421-
}
423+
// NOTE: enableJsonResponse only controls how POST responses are delivered (JSON vs. SSE).
424+
// The standalone GET SSE stream is always supported — it is the only channel available
425+
// for server-to-client notifications when enableJsonResponse = true.
422426
val sseSession = session ?: error("Server session can't be null for streaming GET requests")
423427

424428
val acceptHeader = call.request.header(HttpHeaders.Accept)
@@ -456,6 +460,9 @@ public class StreamableHttpServerTransport(private val configuration: Configurat
456460
sseSession.coroutineContext.job.invokeOnCompletion {
457461
streamsMapping.remove(STANDALONE_SSE_STREAM_ID)
458462
}
463+
// Keep the SSE connection open until the client disconnects or the transport is closed.
464+
// Without this, the Ktor sse{} handler returns immediately, closing the stream.
465+
awaitCancellation()
459466
}
460467

461468
public suspend fun handleDeleteRequest(call: ApplicationCall) {

0 commit comments

Comments
 (0)