Conversation
…eline file for expected test failures
…esponse format in ConformanceTools
…deFlow implementation
… directory resolution and disabling redundant shellcheck warning
…bump conformance tool version to 0.1.15
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ow implementation
fixes #589 ## How Has This Been Tested? conformance tests ## Breaking Changes NaN ## Types of changes - [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 - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed
fixes #588 ## How Has This Been Tested? conformance test ## Breaking Changes NaN ## Types of changes - [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 - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed
…ared factory method
…rios to matrix, and update conformance test baseline
…fine JWT assertion audience handling
| done | ||
|
|
||
| - name: Run conformance tests | ||
| uses: modelcontextprotocol/conformance@v0.1.15 |
There was a problem hiding this comment.
Let's run the same run-conformance.sh here so it is consistent with the local testing.
There was a problem hiding this comment.
doen't work for uses field
| runs-on: ${{ matrix.os }} | ||
| name: Run Conformance Tests on ${{ matrix.os }} | ||
| server: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Observation: Conformance tests are no longer being run on different platforms.
Add configurable SSE reconnection with exponential backoff and server-driven retry delays to `StreamableHttpClientTransport` closes #590 closes #420 ## How Has This Been Tested? New unit tests and pass conformance test ## Breaking Changes old constructors are Deprecated `close` no longer calls `terminateSession` ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed
- Introduced `list` command in `run-conformance.sh` to display available scenarios. - Updated README with details about the new `list` command and its purpose.
There was a problem hiding this comment.
Pull request overview
Adds a new conformance-test module and CI workflow to run the MCP conformance suite against the Kotlin SDK, while updating Streamable HTTP client/server behavior and protocol version constants to support newer conformance scenarios (notably SSE retry/resumption behavior and priming events).
Changes:
- Introduces a standalone conformance test harness (server + scenario-based client, including OAuth/auth scenarios), baseline expected-failures file, and a local runner script.
- Updates Streamable HTTP client transport reconnection logic (server
retryparsing, backoff, reconnection options) and server priming-event behavior. - Bumps protocol version constants/tests to
2025-11-25and updates CI to execute conformance suites.
Reviewed changes
Copilot reviewed 41 out of 42 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/StreamableHttpServerTransport.kt | Gates priming-event emission based on client protocol version header. |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/CommonTypeTest.kt | Updates assertions for latest/supported protocol versions. |
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt | Adjusts JSON-RPC message polymorphic selection for result-without-id cases. |
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/common.kt | Updates protocol version constants and supported version list. |
| kotlin-sdk-client/src/jvmTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpClientTest.kt | Adds explicit terminate-session test and adjusts unsubscribe expectations. |
| kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/streamable/http/StreamableHttpClientTransportTest.kt | Adds tests for inline SSE retry parsing, priming tracking, reconnection behavior, and deprecated ctor. |
| kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpMcpKtorClientExtensions.kt | Adds ReconnectionOptions overloads and deprecates duration-based overloads. |
| kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/StreamableHttpClientTransport.kt | Refactors SSE reconnection, retry/backoff, inline SSE parsing, and removes auto-terminate-on-close. |
| kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ReconnectionOptions.kt | Introduces configurable reconnection options type. |
| kotlin-sdk-client/api/kotlin-sdk-client.api | Updates public API surface for new overloads and ReconnectionOptions. |
| gradle/libs.versions.toml | Adds ktor-client-auth dependency entry. |
| conformance-test/src/test/resources/simplelogger.properties | Removes simplelogger configuration from old test harness. |
| conformance-test/src/test/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/WebSocketConformanceClient.kt | Removes old WebSocket conformance client entry point. |
| conformance-test/src/test/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceTest.kt | Removes old JUnit-driven conformance runner harness. |
| conformance-test/src/test/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceServer.kt | Removes old embedded conformance server implementation. |
| conformance-test/src/test/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceClient.kt | Removes old embedded conformance client implementation. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/utils.kt | Adds shared OAuth/auth utilities (context parsing, discovery helpers, bearer injection). |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/tokenExchange.kt | Adds auth-code token exchange and redirect-following helpers. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/scopeHandling.kt | Adds scope selection and step-up parsing logic for WWW-Authenticate. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/registration.kt | Registers auth scenario handlers with the scenario router. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/pkce.kt | Adds PKCE verifier/challenge generation and AS metadata validation. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/discovery.kt | Adds PRM + AS metadata discovery logic. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/crossAppAccessScenario.kt | Implements SEP-990 cross-app access flow scenario. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/clientRegistration.kt | Implements client credential resolution (pre-reg, CIMD, dynamic registration). |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/basicScenario.kt | Implements client-credentials basic scenario. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/authCodeFlow.kt | Implements auth-code flow scenario with HttpSend interceptor retries. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/auth/JWTScenario.kt | Implements client-credentials JWT assertion scenario. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/InMemoryEventStore.kt | Adds EventStore implementation for SSE resumption tests. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceTools.kt | Registers server-side conformance tools used by the runner suites. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceServer.kt | New conformance server entry point using mcpStreamableHttp with DNS rebinding protection + EventStore. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceResources.kt | Registers conformance resources (static, binary, template placeholder, watched, dynamic). |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformancePrompts.kt | Registers conformance prompts (simple, args, image, embedded, dynamic). |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceCompletions.kt | Adds completion/complete handler registration for conformance. |
| conformance-test/src/main/kotlin/io/modelcontextprotocol/kotlin/sdk/conformance/ConformanceClient.kt | New scenario-based conformance client entry point (routes by env MCP_CONFORMANCE_SCENARIO). |
| conformance-test/run-conformance.sh | Adds local script to build + run server/client/auth conformance suites and store results. |
| conformance-test/detekt-baseline.xml | Removes detekt baseline used by the old test harness. |
| conformance-test/conformance-baseline.yml | Adds expected failures baseline file for known SDK limitations. |
| conformance-test/build.gradle.kts | Converts conformance-test into an application module with installDist start scripts for server + client. |
| conformance-test/README.md | Adds documentation for running the conformance suites locally and understanding structure/known failures. |
| conformance-test/.gitignore | Ignores conformance run result output directory. |
| build.gradle.kts | Skips detekt for conformance-test and docs subprojects. |
| .github/workflows/conformance.yml | Adds/updates CI workflow to run server/client/auth conformance suites using the conformance action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val store = configuration.eventStore | ||
| if (store == null || session == null) return | ||
| // Priming events have empty data which older clients cannot handle. | ||
| // Only send priming events to clients with protocol version >= 2025-11-25 | ||
| // which includes the fix for handling empty SSE data. | ||
| if (clientProtocolVersion != null && clientProtocolVersion < MIN_PRIMING_EVENT_PROTOCOL_VERSION) return |
There was a problem hiding this comment.
maybeSendPrimingEvent currently sends the priming event when mcp-protocol-version is missing. Since older clients are likely to omit this header, this defeats the version gate and can still send empty-data events to clients that can't handle them. Consider treating a missing/unknown protocol version as older than MIN_PRIMING_EVENT_PROTOCOL_VERSION (i.e., skip priming unless the client explicitly advertises support), and/or parse/validate the version instead of relying on nullable string comparison.
| val jsonObj = element.jsonObject | ||
| return when { | ||
| "error" in jsonObject -> JSONRPCError.serializer() | ||
| "result" in jsonObject -> JSONRPCResponse.serializer() | ||
| "method" in jsonObject && "id" in jsonObject -> JSONRPCRequest.serializer() | ||
| "method" in jsonObject -> JSONRPCNotification.serializer() | ||
| jsonObject.isEmpty() || jsonObject.keys == setOf("jsonrpc") -> JSONRPCEmptyMessage.serializer() | ||
| else -> throw SerializationException("Invalid JSONRPCMessage type: ${jsonObject.keys}") | ||
| "error" in jsonObj -> JSONRPCError.serializer() | ||
| "result" in jsonObj && "id" in jsonObj -> JSONRPCResponse.serializer() | ||
| "result" in jsonObj && jsonObj["result"]?.jsonObject?.isEmpty() == true -> JSONRPCEmptyMessage.serializer() | ||
| "method" in jsonObj && "id" in jsonObj -> JSONRPCRequest.serializer() |
There was a problem hiding this comment.
JSONRPCMessagePolymorphicSerializer uses jsonObj["result"]?.jsonObject?.isEmpty() which will throw if result is not a JSON object (JSON-RPC results can be primitives/arrays). Also, the KDoc says "result" -> JSONRPCEmptyMessage, but the implementation only treats empty object results with missing id as empty. Use a type check (e.g., jsonObj["result"] is JsonObject) and align the selection logic with the documented behavior (either classify any missing-id result as empty, or update the KDoc and throw a SerializationException deterministically).
| val session = client.sseSession(urlString = url, showRetryEvents = true) { | ||
| method = HttpMethod.Get | ||
| applyCommonHeaders(this) | ||
| // sseSession will add ContentType.Text.EventStream automatically | ||
| accept(ContentType.Application.Json) | ||
| (resumptionToken ?: lastEventId)?.let { headers.append(MCP_RESUMPTION_TOKEN_HEADER, it) } | ||
| lastEventId?.let { headers.append(MCP_RESUMPTION_TOKEN_HEADER, it) } | ||
| requestBuilder() |
There was a problem hiding this comment.
The GET/SSE connection builder adds accept(ContentType.Application.Json). Since Accept influences content negotiation, this can cause an SSE-capable server to legitimately choose application/json, which you then interpret as "JSON-only mode" and stop reconnecting. Consider removing the explicit JSON accept (let the SSE plugin set text/event-stream), or explicitly prefer/limit to text/event-stream for the GET stream.
| var attempt = 0 | ||
| var needsDelay = initialServerRetryDelay != null | ||
|
|
||
| @Suppress("LoopWithTooManyJumpStatements") | ||
| while (isActive) { | ||
| // Delay before (re)connection: skip only for first fresh SSE connection | ||
| if (needsDelay) { | ||
| delay(getNextReconnectionDelay(attempt, serverRetryDelay)) | ||
| } |
There was a problem hiding this comment.
The backoff attempt counter is incremented before the next loop iteration's delay, so the first retry waits initialReconnectionDelay * multiplier^1 instead of the configured initialReconnectionDelay. If initialReconnectionDelay is intended to be the delay before the first retry, adjust the attempt math (e.g., delay using attempt - 1, or increment attempt after delaying).
| session.incoming.collect { event -> | ||
| event.retry?.let { localServerRetryDelay = it.milliseconds } | ||
| event.id?.let { | ||
| lastEventId = it | ||
| localLastEventId = it | ||
| hasPrimingEvent = true | ||
| onResumptionToken?.invoke(it) |
There was a problem hiding this comment.
hasPrimingEvent is set to true for any SSE event that has an id. In SSE, id can be present on regular events as well, so this can incorrectly trigger the POST-to-GET reconnection path when inline SSE contains normal events with ids but no response. Consider detecting priming events more precisely (e.g., id != null && data.isNullOrEmpty() for the empty-data priming event) instead of using id presence alone.
| # ============================================================================ | ||
|
|
||
| COMMAND="${1:-}" | ||
| shift 2>/dev/null || true |
There was a problem hiding this comment.
Argument forwarding is broken: COMMAND consumes $1, but the script does shift 2, which drops the first [extra-args...] value. This prevents callers from passing through runner flags as documented. Use shift 1 here (and keep the remaining args intact).
| shift 2>/dev/null || true | |
| shift 1 2>/dev/null || true |
| build | ||
|
|
||
| EXIT_CODE=0 |
There was a problem hiding this comment.
The script doesn't fail fast if build fails (no set -e and build isn't checked). This can lead to confusing downstream errors when installDist didn't produce the binaries. Consider enabling set -e (or set -euo pipefail) and/or checking build's return code before proceeding.
| val response = execute(request) | ||
| val status = response.response.status | ||
|
|
||
| // Determine if we need to (re-)authorize | ||
| val needsAuth = status == HttpStatusCode.Unauthorized | ||
| val wwwAuth = response.response.headers[HttpHeaders.WWWAuthenticate] ?: "" | ||
| val stepUpScope = if (status == HttpStatusCode.Forbidden) parseStepUpScope(wwwAuth) else null | ||
| val needsStepUp = stepUpScope != null | ||
|
|
||
| if ((needsAuth || needsStepUp) && authAttempts < 3) { | ||
| authAttempts++ | ||
|
|
||
| // Discover metadata (cache across retries) | ||
| if (cachedDiscovery == null) { | ||
| val resourceMetadataUrl = extractParam(wwwAuth, "resource_metadata") | ||
| cachedDiscovery = discoverOAuthMetadata(httpClient, serverUrl, resourceMetadataUrl) | ||
| } | ||
| val discovery: DiscoveryResult = cachedDiscovery |
There was a problem hiding this comment.
In the HttpSend interceptor, when you decide to re-authorize (401/403), the original response from execute(request) is not consumed or closed before performing additional HTTP calls and retrying execute(request) again. In Ktor this can leak resources/keep connections busy; consider calling response.close() (or consuming the body) before starting the auth flow and retrying the request.
…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>
Adds a comprehensive conformance test suite for the Kotlin MCP SDK, covering core protocol operations, tool calls, elicitation, resources, prompts, and 20 OAuth/auth scenarios
fixes:
Remaining known failures (tracked issues, will be fixed directly in
main)tools-call-with-logging,tools-call-with-progress,tools-call-sampling,tools-call-elicitation,elicitation-sep1034-defaults- see fix(server): notifications not delivered over Streamable HTTP (#587) #599,elicitation-sep1330-enums- Server-initiated notifications from tool handlers are not delivered to the client #587 fix(conformance): correct SEP-1330 enum schemas in test_elicitation_sep1330_enums #600initialize- Client fails to deserialize JSON-RPC response withoutidfield #588tools_call,auth/scope-step-up,auth/scope-retry-limit- SSE GET session crashes on 404 instead of treating it as "SSE not supported" #589elicitation-sep1034-client-defaults- Implement SEP-1034: Default Values for Elicitation Schemas #414sse-retry- Client does not respect SSEretryfield or sendLast-Event-IDon reconnection #590resources-templates-read- Server does not support resource template parameter substitution #591Breaking Changes
from #596
StreamableHttpClientTransportandmcpStreamableHttp/mcpStreamableHttpTransport: old constructors acceptingDurationtimeout are now@Deprecated— use the new overloads withReconnectionOptionsinsteadStreamableHttpClientTransport.close()no longer callsterminateSession()automaticallyTypes of changes
Checklist