docs: streamable server sample - #595
Conversation
…rt and tool/resource definitions
…sure consistency in transport and connection details
There was a problem hiding this comment.
Pull request overview
Adds a new Streamable HTTP server sample (with optional bearer auth) and refreshes the repository docs/samples documentation to prefer Streamable HTTP over the older SSE quickstart path.
Changes:
- Added
simple-streamable-serversample showcasing Streamable HTTP, tools/prompts/resources, and logging notifications. - Updated existing sample READMEs and the root README quickstart to use Streamable HTTP.
- Added/updated README files for the
samples/index and the notebook sample.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/weather-stdio-server/README.md | Simplifies and updates weather STDIO server documentation and integration steps. |
| samples/simple-streamable-server/src/main/kotlin/io/modelcontextprotocol/sample/server/server.kt | Implements the Streamable HTTP Ktor server sample with optional authenticated routing. |
| samples/simple-streamable-server/src/main/kotlin/io/modelcontextprotocol/sample/server/main.kt | Adds runnable entrypoint with optional --auth mode. |
| samples/simple-streamable-server/settings.gradle.kts | Defines standalone Gradle build settings for the new sample. |
| samples/simple-streamable-server/gradlew.bat | Adds Windows Gradle wrapper script for the new sample. |
| samples/simple-streamable-server/gradlew | Adds POSIX Gradle wrapper script for the new sample. |
| samples/simple-streamable-server/gradle/wrapper/gradle-wrapper.properties | Configures Gradle wrapper distribution for the new sample. |
| samples/simple-streamable-server/gradle/wrapper/gradle-wrapper.jar | Adds the Gradle wrapper JAR for the new sample. |
| samples/simple-streamable-server/gradle/libs.versions.toml | Declares dependency versions used by the new sample. |
| samples/simple-streamable-server/gradle.properties | Enables Gradle performance flags for the new sample build. |
| samples/simple-streamable-server/build.gradle.kts | Adds dependencies/plugins and application entrypoint for the new sample. |
| samples/simple-streamable-server/README.md | Documents how to run and use the new Streamable HTTP server sample. |
| samples/notebooks/README.md | Documents the notebook-based Streamable HTTP client walkthrough. |
| samples/kotlin-mcp-server/README.md | Repositions SSE modes as backward-compatible and points readers to the new streamable sample. |
| samples/kotlin-mcp-client/README.md | Simplifies and refreshes client sample documentation and links. |
| samples/README.md | Adds a samples index/overview table and navigation entrypoints. |
| README.md | Updates the main quickstart server example to Streamable HTTP and points to the samples overview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… headers, and enforce required `MCP_AUTH_TOKEN` for authentication.
kpavlov
left a comment
There was a problem hiding this comment.
Looks good in general; however, the example of a streamable server appears awkward.
Please check my comments before merging
| @@ -0,0 +1,28 @@ | |||
| package io.modelcontextprotocol.sample.server | |||
|
|
|||
| import io.ktor.server.cio.CIO | |||
There was a problem hiding this comment.
On the JVM target, it's not a good idea to use an exotic server engine that doesn't support HTTP/2. Let's switch to something widely used like Netty or Jetty to showcase a production use case.
There was a problem hiding this comment.
This remains an example rather than part of the production code
For example, cors and auth are not configured as they would be in production
I can change the engine, that’s not a problem
There was a problem hiding this comment.
The file has a bit too much information. Let’s keep the server configuration here, and move the Ktor glue code to a different file.
There was a problem hiding this comment.
Maybe, but for now I think I'll leave it as is
Later I would refactor part of the code, especially the auth. Then we could either remove it or move it to a separate file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sample: | ||
| - kotlin-mcp-client | ||
| - kotlin-mcp-server | ||
| - simple-streamable-server |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val sessionId = call.request.header(MCP_SESSION_ID_HEADER) | ||
| if (sessionId != null) { | ||
| val transport = transports[sessionId] | ||
| if (transport == null) { | ||
| call.respond(HttpStatusCode.NotFound, "Session not found") | ||
| } | ||
| return transport | ||
| } |
There was a problem hiding this comment.
getOrCreateTransport treats an empty mcp-session-id header as a real session ID (it only checks for null), which leads to a misleading 404 "Session not found" instead of a 400 like findTransport does. Consider using the same isNullOrEmpty() validation (and returning 400) or treating blank as absent and creating a new session, to keep behavior consistent across endpoints.
closes #170
How Has This Been Tested?
knit and inspector
Breaking Changes
NaN
Types of changes
Checklist