Skip to content

Conversation

@devcrocod
Copy link
Contributor

Motivation and Context

Removed the WASM example, as it duplicates the JVM example and only adds unnecessary complexity

How Has This Been Tested?

Locally

Breaking Changes

No

Types of changes

  • 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

  • I have read the MCP Documentation
  • 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

Copy link
Contributor

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 the kotlin-mcp-server example by consolidating the multiplatform setup into a simpler JVM-only configuration and updates SDK versions across all samples. The WASM implementation has been removed to reduce complexity while maintaining all core functionality.

Key changes:

  • Updated dependency versions (Kotlin 2.2.20→2.2.21, MCP SDK 0.7.3→0.7.4, Ktor 3.3.x→3.2.3)
  • Consolidated kotlin-mcp-server from multiplatform to JVM-only with standard Gradle application plugin
  • Moved STDIO functionality from platform-specific to main source set

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
samples/weather-stdio-server/gradle/libs.versions.toml Updated dependency versions
samples/kotlin-mcp-server/src/main/kotlin/io/modelcontextprotocol/sample/server/server.kt Fixed package name, improved SSE error handling, moved STDIO function from JVM-specific code
samples/kotlin-mcp-server/src/main/kotlin/io/modelcontextprotocol/sample/server/main.kt Updated package declaration and moved main function to common source
samples/kotlin-mcp-server/src/jvmMain/kotlin/main.jvm.kt Removed JVM-specific entry point (functionality moved to main)
samples/kotlin-mcp-server/gradle/libs.versions.toml Updated versions and switched from multiplatform to JVM plugin
samples/kotlin-mcp-server/build.gradle.kts Simplified from multiplatform to single-platform JVM configuration
samples/kotlin-mcp-server/README.md Updated documentation to reflect JVM-only setup and improved examples
samples/kotlin-mcp-client/gradle/libs.versions.toml Updated dependency versions

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

kpavlov
kpavlov previously approved these changes Oct 29, 2025
Copy link
Contributor

@kpavlov kpavlov left a comment

Choose a reason for hiding this comment

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

👍

}
dependencies {
implementation(libs.mcp.kotlin.server)
implementation(libs.ktor.server.cio)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to use Netty as server engine, because CIO does not support Http2 and Netty is de-facto a standard for high-performance servers for decades.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This example is essentially crossplatform
Therefore, I think it’s better to keep cio as the default ktor engine, since ktor itself uses it in its own examples

"--sse-server" -> runSseMcpServerWithPlainConfiguration(port)
else -> {
error("Unknown command: $command")
System.err.println("Unknown command: $command")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it exit with 0? Is it the right behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this seems to be an unexpected behavior after the refactoring
I’ll fix it

@@ -1,4 +1,4 @@
package shared
package io.modelcontextprotocol.sample.server
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@devcrocod devcrocod force-pushed the devcrocod/update-samples branch from 25364df to a13636a Compare October 30, 2025 12:19
@devcrocod devcrocod requested a review from kpavlov October 30, 2025 12:19
@devcrocod devcrocod merged commit 458a49a into main Oct 31, 2025
6 checks passed
@devcrocod devcrocod deleted the devcrocod/update-samples branch October 31, 2025 11:21
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.

3 participants