Skip to content

Add support for elicitation #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ptitjes
Copy link
Contributor

@ptitjes ptitjes commented Jul 6, 2025

Fixes #137

In this PR, I add:

  • the request and result objects for the elicitation mechanism
  • adds the client capabilities and capability checking on the client and server sides
  • adds tests to ClientTest

Motivation and Context

To support the new elicitation feature of the 2025-06-18 protocol version.

How Has This Been Tested?

I added two tests in ClientTest. The tests are not extensive yet as I only check:

  • that the server throws when the client doesn't support elicitation
  • that the client correctly handles a simple elicitation request

Maybe should I add some more tests? Please advise me.

Breaking Changes

N/A

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

Additional context

  1. I was not sure whether to add the tests to ClientTest or ServerTest. (It seems to me that the tests should be divided by feature and not by client/server side, as most features involve both client and server, but I respected how it is done today.)

  2. I was not sure whether to add a method to Client to register an elicitation handler (i.e. wrap the setRequestHandler(...) call). Please advise me on what you think is best. (If that is best to add a wrapper method, maybe this can be implemented as a follow-up?)

  3. The CreateElicitationRequest.RequestedSchema class I introduced is duplicated from Tool.Input. Maybe that should be factored-out in a JsonSchema top-level class in types.kt. I was not sure you would approve that, so I did not factor it out. Please tell me if you'd prefer I factor that out. (The support for the new Tool output schema in the latest protocol version will also need the same type.)

@devcrocod devcrocod requested review from Copilot and devcrocod July 8, 2025 19:06
Copy link
Contributor

@Copilot 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

Adds initial support for the new “elicitation” feature in the MCP protocol, including data types, capability checks, client/server handling, and basic tests.

  • Introduces CreateElicitationRequest and CreateElicitationResult types and updates Method.Defined
  • Adds client/server capability checks and a Server.createElicitation helper
  • Adds two new tests in ClientTest for reject and success paths

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/jvmTest/kotlin/client/ClientTest.kt New tests for server-initiated elicitation accept/reject paths
src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types.kt Added elicitation types and client capability field
src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt Added createElicitation and server-side capability assertion
src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/client/Client.kt Added client-side capability assertion for elicitation requests
api/kotlin-sdk.api Updated public API signature dump to include elicitation types
Comments suppressed due to low confidence (1)

src/jvmTest/kotlin/client/ClientTest.kt:846

  • [nitpick] Consider adding tests for the decline and cancel branches of CreateElicitationResult.Action to ensure all response paths are handled correctly.
    fun `should handle server elicitation`() = runTest {

Method.Defined.ElicitationCreate -> {
if (capabilities.elicitation == null) {
throw IllegalStateException(
"Client does not support elicitation capability (required for $method)"
Copy link
Preview

Copilot AI Jul 8, 2025

Choose a reason for hiding this comment

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

The exception message is inconsistent with the server-side text and the test expectation. Consider matching it to "Client does not support elicitation (required for elicitation/create)" and use method.value rather than $method for clarity.

Suggested change
"Client does not support elicitation capability (required for $method)"
"Client does not support elicitation (required for elicitation/create)"

Copilot uses AI. Check for mistakes.

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.

Support Elicitation
1 participant