From fdd40b36188e360e1424111aade0d3155c674274 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov <1517853+kpavlov@users.noreply.github.com> Date: Fri, 21 Nov 2025 13:50:58 +0200 Subject: [PATCH 1/4] Ensure proper exception handling and improve JSON deserialization stability. - Add custom `asJsonObject` method for stricter `JsonObject` validation. - Introduce tests to validate `SerializationException` behavior on invalid JSON inputs. --- .../kotlin/sdk/types/serializers.kt | 20 ++++++++++++++----- .../kotlin/sdk/types/JsonRpcTest.kt | 19 ++++++++++++++++++ .../kotlin/sdk/types/ResourcesTest.kt | 14 +++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt b/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt index 721a2aac..e5b9cece 100644 --- a/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt +++ b/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt @@ -11,6 +11,7 @@ import kotlinx.serialization.encoding.Decoder import kotlinx.serialization.encoding.Encoder import kotlinx.serialization.json.JsonContentPolymorphicSerializer import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive @@ -40,6 +41,15 @@ private fun JsonElement.getTypeOrNull(): String? = jsonObject["type"]?.jsonPrimi */ private fun JsonElement.getType(): String = requireNotNull(getTypeOrNull()) { "Missing required 'type' field" } +@Throws(SerializationException::class) +private fun JsonElement.asJsonObject(): JsonObject { + if (this !is JsonObject) { + throw SerializationException("Invalid response. JsonObject expected, got: $this") + } + val jsonObject = this.jsonObject + return jsonObject +} + // ============================================================================ // Method Serializer // ============================================================================ @@ -131,7 +141,7 @@ internal object MediaContentPolymorphicSerializer : internal object ResourceContentsPolymorphicSerializer : JsonContentPolymorphicSerializer(ResourceContents::class) { override fun selectDeserializer(element: JsonElement): DeserializationStrategy { - val jsonObject = element.jsonObject + val jsonObject = element.asJsonObject() return when { "text" in jsonObject -> TextResourceContents.serializer() "blob" in jsonObject -> BlobResourceContents.serializer() @@ -284,7 +294,7 @@ internal object ServerNotificationPolymorphicSerializer : * Returns EmptyResult serializer if the JSON object is empty or contains only metadata. */ private fun selectEmptyResult(element: JsonElement): DeserializationStrategy? { - val jsonObject = element.jsonObject + val jsonObject = element.asJsonObject() return when { jsonObject.isEmpty() || (jsonObject.size == 1 && "_meta" in jsonObject) -> EmptyResult.serializer() else -> null @@ -296,7 +306,7 @@ private fun selectEmptyResult(element: JsonElement): DeserializationStrategy? { - val jsonObject = element.jsonObject + val jsonObject = element.asJsonObject() return when { "model" in jsonObject && "role" in jsonObject -> CreateMessageResult.serializer() "roots" in jsonObject -> ListRootsResult.serializer() @@ -310,7 +320,7 @@ private fun selectClientResultDeserializer(element: JsonElement): Deserializatio * Returns null if the structure doesn't match any known server result type. */ private fun selectServerResultDeserializer(element: JsonElement): DeserializationStrategy? { - val jsonObject = element.jsonObject + val jsonObject = element.asJsonObject() return when { "protocolVersion" in jsonObject && "capabilities" in jsonObject -> InitializeResult.serializer() "completion" in jsonObject -> CompleteResult.serializer() @@ -378,7 +388,7 @@ internal object ServerResultPolymorphicSerializer : internal object JSONRPCMessagePolymorphicSerializer : JsonContentPolymorphicSerializer(JSONRPCMessage::class) { override fun selectDeserializer(element: JsonElement): DeserializationStrategy { - val jsonObject = element.jsonObject + val jsonObject = element.asJsonObject() return when { "error" in jsonObject -> JSONRPCError.serializer() "result" in jsonObject -> JSONRPCResponse.serializer() diff --git a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt index d8d20583..b2aa9a20 100644 --- a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt +++ b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt @@ -1,8 +1,10 @@ package io.modelcontextprotocol.kotlin.sdk.types import io.kotest.assertions.json.shouldEqualJson +import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldBe import io.kotest.matchers.types.shouldBeSameInstanceAs +import kotlinx.serialization.SerializationException import kotlinx.serialization.json.boolean import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.int @@ -43,6 +45,7 @@ class JsonRpcTest { @Test fun `should convert JSONRPCRequest to Request`() { + // language=json val jsonRpc = McpJson.decodeFromString( """ { @@ -101,6 +104,7 @@ class JsonRpcTest { @Test fun `should convert JSONRPCNotification to Notification`() { + // language=json val jsonRpc = McpJson.decodeFromString( """ { @@ -164,6 +168,7 @@ class JsonRpcTest { @Test fun `should deserialize JSONRPCRequest with numeric id`() { + // language=json val json = """ { "id": 42, @@ -212,6 +217,7 @@ class JsonRpcTest { @Test fun `should deserialize JSONRPCNotification`() { + // language=json val json = """ { "method": "notifications/progress", @@ -257,6 +263,7 @@ class JsonRpcTest { @Test fun `should deserialize JSONRPCResponse with EmptyResult`() { + // language=json val json = """ { "id": 7, @@ -308,6 +315,7 @@ class JsonRpcTest { @Test fun `should deserialize JSONRPCError`() { + // language=json val json = """ { "id": "req-404", @@ -336,6 +344,7 @@ class JsonRpcTest { @Test fun `should decode JSONRPCMessage as request`() { + // language=json val json = """ { "id": "msg-1", @@ -359,6 +368,7 @@ class JsonRpcTest { @Test fun `should decode JSONRPCMessage as error response`() { + // language=json val json = """ { "id": 123, @@ -401,6 +411,15 @@ class JsonRpcTest { """.trimIndent() } + @Test + fun `JSONRPCMessage should throw on non-object JSON`() { + val exception = shouldThrow { + McpJson.decodeFromString("[\"just a string\"]") + } + + exception.message shouldBe "Invalid response. JsonObject expected, got: [\"just a string\"]" + } + @Test fun `should create JSONRPCRequest with string ID`() { val params = buildJsonObject { diff --git a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt index 0313b758..c517bd78 100644 --- a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt +++ b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt @@ -1,6 +1,9 @@ package io.modelcontextprotocol.kotlin.sdk.types import io.kotest.assertions.json.shouldEqualJson +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.shouldBe +import kotlinx.serialization.SerializationException import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.int import kotlinx.serialization.json.jsonPrimitive @@ -250,6 +253,15 @@ class ResourcesTest { """.trimIndent() } + @Test + fun `ResourceContents should throw on non-object JSON`() { + val exception = shouldThrow { + McpJson.decodeFromString("\"just a string\"") + } + + exception.message shouldBe "Invalid response. JsonObject expected, got: \"just a string\"" + } + @Test fun `should serialize ListResourcesRequest with cursor`() { val request = ListResourcesRequest( @@ -316,6 +328,7 @@ class ResourcesTest { @Test fun `should deserialize ListResourcesResult`() { + // language=json val json = """ { "resources": [ @@ -370,6 +383,7 @@ class ResourcesTest { @Test fun `should deserialize ReadResourceResult with mixed contents`() { + // language=json val json = """ { "contents": [ From 5900d0958b995f1038fca34589f649ef61ee7be9 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov <1517853+kpavlov@users.noreply.github.com> Date: Fri, 21 Nov 2025 13:51:08 +0200 Subject: [PATCH 2/4] Update AGENTS.md with test writing guidelines. Improve consistency and add details to AGENTS.md, including test-writing and best practices. --- AGENTS.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5b49a6db..7f83a1fa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,6 +3,7 @@ MCP Kotlin SDK — Kotlin Multiplatform implementation of the Model Context Protocol. ## Repository Layout + - `kotlin-sdk-core`: Core protocol types, transport abstractions, WebSocket implementation - `kotlin-sdk-client`: Client transports (STDIO, SSE, StreamableHttp, WebSocket) - `kotlin-sdk-server`: Server transports + Ktor integration (STDIO, SSE, WebSocket) @@ -11,6 +12,7 @@ MCP Kotlin SDK — Kotlin Multiplatform implementation of the Model Context Prot - `samples/`: Three sample projects (composite builds) ## General Guidance + - Avoid changing public API signatures. Run `./gradlew apiCheck` before every commit. - **Explicit API mode** is strict: all public APIs must have explicit visibility modifiers and return types. - Anything under an `internal` directory is not part of the public API and may change freely. @@ -18,6 +20,7 @@ MCP Kotlin SDK — Kotlin Multiplatform implementation of the Model Context Prot - The SDK targets Kotlin 2.2+ and JVM 1.8+ as minimum. ## Building and Testing + 1. Build the project: ```bash ./gradlew build @@ -43,7 +46,17 @@ MCP Kotlin SDK — Kotlin Multiplatform implementation of the Model Context Prot ``` ## Tests -- All tests for each module are located in `src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/` + +- Write comprehensive tests for new features +- **Prioritize test readability** +- Avoid creating too many test methods. If multiple parameters can be tested in one scenario, go for it. +- In case of similar scenarios/use-cases, consider using parametrized tests. + But never make a parametrized test for only one use-case +- Use function `Names with backticks` for test methods in Kotlin, e.g. "fun `should return 200 OK`()" +- Avoid writing KDocs for tests, keep code self-documenting +- When running tests on a Kotlin Multiplatform project, run only JVM tests, + if not asked to run tests on another platform either. +- Common tests for each module are located in `src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/` - Platform-specific tests go in `src/jvmTest/`, `src/jsTest/`, etc. - Use Kotest assertions (`shouldBe`, `shouldContain`, etc.) for readable test failures. - Use `shouldMatchJson` from Kotest for JSON validation. @@ -53,41 +66,48 @@ MCP Kotlin SDK — Kotlin Multiplatform implementation of the Model Context Prot ## Code Conventions ### Multiplatform Patterns + - Use `expect`/`actual` pattern for platform-specific implementations in `utils.*` files. - Test changes on JVM first, then verify platform-specific behavior if needed. - Supported targets: JVM (1.8+), JS/Wasm, iOS, watchOS, tvOS. ### Serialization + - Use Kotlinx Serialization with explicit `@Serializable` annotations. - Custom serializers should be registered in the companion object. - JSON config is defined in `McpJson.kt` — use it consistently. ### Concurrency and State + - Use `kotlinx.atomicfu` for thread-safe operations across platforms. - Prefer coroutines over callbacks where possible. - Transport implementations extend `AbstractTransport` for consistent callback management. ### Error Handling + - Use sealed classes for representing different result states. - Map errors to JSON-RPC error codes in protocol handlers. - Log errors using `io.github.oshai.kotlinlogging.KotlinLogging`. ### Logging + - Use `KotlinLogging.logger {}` for structured logging. - Never log sensitive data (credentials, tokens). ## Pull Requests + - Base all PRs on the `main` branch. - PR title format: Brief description of the change - Include in PR description: - - **What changed?** - - **Why? Motivation and context** - - **Breaking changes?** (if any) + - **What changed?** + - **Why? Motivation and context** + - **Breaking changes?** (if any) - Run `./gradlew apiCheck` before submitting. - Link PR to related issue (except for minor docs/typo fixes). - Follow [Kotlin Coding Conventions](https://kotlinlang.org/docs/reference/coding-conventions.html). ## Review Checklist + - `./gradlew apiCheck` must pass. - `./gradlew test` must succeed for affected modules. - New tests added for any new feature or bug fix. From 29a925c92b6bf7ec289c3253fcb98ad2785fc822 Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov <1517853+kpavlov@users.noreply.github.com> Date: Fri, 21 Nov 2025 14:15:47 +0200 Subject: [PATCH 3/4] Avoid exposing raw response to exception message --- .../io/modelcontextprotocol/kotlin/sdk/types/serializers.kt | 2 +- .../io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt | 2 +- .../io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt b/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt index e5b9cece..961a2ffb 100644 --- a/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt +++ b/kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/serializers.kt @@ -44,7 +44,7 @@ private fun JsonElement.getType(): String = requireNotNull(getTypeOrNull()) { "M @Throws(SerializationException::class) private fun JsonElement.asJsonObject(): JsonObject { if (this !is JsonObject) { - throw SerializationException("Invalid response. JsonObject expected, got: $this") + throw SerializationException("Invalid response. JsonObject expected, got: ${this::class.simpleName}") } val jsonObject = this.jsonObject return jsonObject diff --git a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt index b2aa9a20..7c357c00 100644 --- a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt +++ b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/JsonRpcTest.kt @@ -417,7 +417,7 @@ class JsonRpcTest { McpJson.decodeFromString("[\"just a string\"]") } - exception.message shouldBe "Invalid response. JsonObject expected, got: [\"just a string\"]" + exception.message shouldBe "Invalid response. JsonObject expected, got: JsonArray" } @Test diff --git a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt index c517bd78..bcb12a54 100644 --- a/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt +++ b/kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/ResourcesTest.kt @@ -259,7 +259,7 @@ class ResourcesTest { McpJson.decodeFromString("\"just a string\"") } - exception.message shouldBe "Invalid response. JsonObject expected, got: \"just a string\"" + exception.message shouldBe "Invalid response. JsonObject expected, got: JsonLiteral" } @Test From fed6589cef927924bfe9bac75d768e8502413a1f Mon Sep 17 00:00:00 2001 From: Konstantin Pavlov <1517853+kpavlov@users.noreply.github.com> Date: Fri, 21 Nov 2025 14:15:57 +0200 Subject: [PATCH 4/4] Enable JUnit parallel test execution and configure logging properties for JVM tests. --- .../src/jvmTest/resources/junit-platform.properties | 6 ++++++ .../src/jvmTest/resources/simplelogger.properties | 8 ++++++++ 2 files changed, 14 insertions(+) create mode 100644 kotlin-sdk-core/src/jvmTest/resources/junit-platform.properties create mode 100644 kotlin-sdk-core/src/jvmTest/resources/simplelogger.properties diff --git a/kotlin-sdk-core/src/jvmTest/resources/junit-platform.properties b/kotlin-sdk-core/src/jvmTest/resources/junit-platform.properties new file mode 100644 index 00000000..9af79875 --- /dev/null +++ b/kotlin-sdk-core/src/jvmTest/resources/junit-platform.properties @@ -0,0 +1,6 @@ +## https://docs.junit.org/5.3.0-M1/user-guide/index.html#writing-tests-parallel-execution +junit.jupiter.execution.parallel.enabled=true +junit.jupiter.execution.parallel.config.strategy=dynamic +junit.jupiter.execution.parallel.mode.default=concurrent +junit.jupiter.execution.parallel.mode.classes.default=concurrent +junit.jupiter.execution.timeout.default=2m diff --git a/kotlin-sdk-core/src/jvmTest/resources/simplelogger.properties b/kotlin-sdk-core/src/jvmTest/resources/simplelogger.properties new file mode 100644 index 00000000..2064ae33 --- /dev/null +++ b/kotlin-sdk-core/src/jvmTest/resources/simplelogger.properties @@ -0,0 +1,8 @@ +# Level of logging for the ROOT logger: ERROR, WARN, INFO, DEBUG, TRACE (default is INFO) +org.slf4j.simpleLogger.defaultLogLevel=INFO +org.slf4j.simpleLogger.showThreadName=true +org.slf4j.simpleLogger.showDateTime=false + +# Log level for specific packages or classes +org.slf4j.simpleLogger.log.io.ktor.server=INFO +org.slf4j.simpleLogger.log.io.modelcontextprotocol=DEBUG