Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Kotlin MCP SDK capability model to add first-class tasks capabilities and to make elicitation capabilities structured/typed (rather than an opaque JsonObject), along with updating client/server strict-capability assertions and related tests.
Changes:
- Add typed
ClientCapabilities.Elicitation(form/url sub-capabilities) and introduceClientCapabilities.Tasks/ServerCapabilities.Tasks. - Enforce tasks capability checks for
tasks/*requests andnotifications/tasks/statusinClientandServerSession. - Update DSL/tests and API surface snapshots to reflect the new typed capability shapes.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| kotlin-sdk-server/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerSessionAssertCapabilityTest.kt | Adds tests covering server-side capability assertions for tasks methods/notifications. |
| kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/ServerSession.kt | Adds tasks capability enforcement for outgoing tasks/* requests and notifications/tasks/status. |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/dsl/InitializeDslTest.kt | Updates DSL tests to use typed elicitation capabilities. |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/dsl/CapabilitiesDslTest.kt | Updates capabilities DSL tests for typed elicitation. |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/types/CapabilitiesTest.kt | Expands serialization/deserialization coverage for tasks + typed elicitation. |
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/capabilities.kt | Introduces typed elicitation + tasks capability models (client/server) and deprecation bridge. |
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types/capabilities.dsl.kt | Updates DSL to accept typed elicitation (but currently lacks tasks support). |
| kotlin-sdk-core/api/kotlin-sdk-core.api | Updates public API snapshot for new capability types and signature changes. |
| kotlin-sdk-client/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientAssertCapabilityTest.kt | Adds tests covering client-side capability assertions for tasks methods/notifications. |
| kotlin-sdk-client/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt | Adds tasks capability enforcement for outgoing tasks/* requests and notifications/tasks/status. |
| integration-test/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/client/ClientTest.kt | Updates integration tests to use typed elicitation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public class ClientCapabilitiesBuilder @PublishedApi internal constructor() { | ||
| private var sampling: ClientCapabilities.Sampling? = null | ||
| private var roots: ClientCapabilities.Roots? = null | ||
| private var elicitation: JsonObject? = null | ||
| private var elicitation: ClientCapabilities.Elicitation? = null | ||
| private var extensions: Map<String, JsonObject>? = null | ||
| private var experimental: JsonObject? = null |
There was a problem hiding this comment.
ClientCapabilities now has a tasks capability, but the DSL builder doesn’t expose any way to set it (and build() never populates it). This means users of buildInitializeRequest { capabilities { ... } } cannot declare task support via the DSL. Consider adding a private var tasks: ClientCapabilities.Tasks?, a fun tasks(value: ClientCapabilities.Tasks) setter, and wiring it into build() (and updating the builder KDoc list accordingly).
| Method.Defined.TasksGet, | ||
| Method.Defined.TasksResult, | ||
| Method.Defined.TasksList, | ||
| Method.Defined.TasksCancel, | ||
| -> assertTasksCapabilityForMethod(method) |
There was a problem hiding this comment.
Tasks methods were added to assertCapabilityForMethod, but assertRequestHandlerCapability still has no gating for tasks/* handlers. Since tasks requests can be received by either side, this allows registering handlers for tasks/get|result|list|cancel without advertising ClientCapabilities.tasks (and without checking list/cancel sub-capabilities). Consider adding corresponding tasks/* cases in assertRequestHandlerCapability that validate capabilities.tasks and, for list/cancel, capabilities.tasks.list/capabilities.tasks.cancel.
| Defined.TasksGet, | ||
| Defined.TasksResult, | ||
| Defined.TasksList, | ||
| Defined.TasksCancel, | ||
| -> assertTasksCapabilityForMethod(method) |
There was a problem hiding this comment.
Tasks methods were added to assertCapabilityForMethod (remote capability check), but assertRequestHandlerCapability still doesn’t enforce ServerCapabilities.tasks when registering local handlers for tasks/* requests. Since tasks requests are bidirectional, this can result in the server handling tasks/get|result|list|cancel without advertising tasks support (and without checking list/cancel sub-capabilities). Consider adding tasks/* cases to assertRequestHandlerCapability that validate serverCapabilities.tasks and the relevant sub-capability fields for list/cancel.
closes #544
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context