feat/Issue-13: elicitation support by krtkvrm · Pull Request #188 · modelcontextprotocol/go-sdk

krtkvrm · 2025-07-29T10:59:46Z

fixes: #13

Implements elicitation functionality from MCP 2025-06-18 specification.

Changes

Add ElicitParams and ElicitResult protocol types
Add ServerSession.Elicit() method and ClientOptions.ElicitationHandler
Schema validation enforces top-level properties only
Support for accept/decline/cancel actions with progress tokens

Tests

Integration test in TestEndToEnd
Schema validation, error handling, and capability declaration tests

mcp/client.go

jba · 2025-07-29T15:52:10Z

The PR comment is too wordy. See other PR comments by jba, findleyr and samthanawalla to get the style.

The example is useful but it should be an actual example. See the files named *example*.go in the mcp directory.

jba

Add a test to mcp_test.go.

.gitignore

mcp/client.go

mcp/protocol.go

.gitignore

mcp/protocol.go

mcp/client.go

jba · 2025-08-01T18:37:56Z

mcp/client.go

+		// TODO: wrap or annotate this error? Pick a standard code?
+		return nil, &jsonrpc2.WireError{Code: CodeUnsupportedMethod, Message: "client does not support elicitation"}
+	}
+	return c.opts.ElicitationHandler(ctx, cs, params)


A good place to check the restriction on the schema.

added, please check if its as expected

@jba Correct me if I'm wrong but I believe we can also check that the ElicitResult.Content matches the Params.RequestSchema before returning.

It would look like this:

r, err := params.RequestedSchema.Resolve(nil) res, err := c.opts.ElicitationHandler(ctx, cs, params) err := r.Validate(res.Content) if err != nil { panic(elicitationhandler violated the requestedschema) }

Seems about right. Instead of panicking can the RPC return an error?

This would pass the error to the server instead of the client but I think it should be a client side error since it originated from the ElicitationHandler. What do you think?

There's no way to surface the error on the client.
I think this is what we do with other handlers.

samthanawalla · 2025-08-07T20:24:01Z

@krtkvrm It would be great to get this in soon if you are able to resolve all the comments, thanks in advance!

jba · 2025-08-11T21:02:27Z

mcp/client.go

+}
+
+// validateElicitSchema validates that the schema only contains top-level properties without nesting.
+func validateElicitSchema(schema *jsonschema.Schema) error {


Actually, I just read the spec and there is more to check. Let's check it all.
https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#supported-schema-types

zackchandler · 2025-08-20T04:12:26Z

mcp/elicitation_example_test.go

+			return &mcp.ElicitResult{
+				Action: "accept",
+				Content: map[string]any{
+					"apiKey":     "sk-test123",


Probably don't want to use this field as an example since it is prohibited in the spec:

https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#user-interaction-model

jba · 2025-08-20T16:14:08Z

@krtkvrm can you wrap this up by tomorrow, Thursday Aug 20? We would like it to land before our next release, which will probably be Friday.

samthanawalla · 2025-08-21T14:49:58Z

@krtkvrm Will you be able to complete it today? We want to release this feature soon so I will take it over unless you say otherwise. (And give you credit) Thank you!

krtkvrm · 2025-08-21T14:56:47Z

I'll be pushing changes in next few hours, sorry for delays, I am currently travelling

findleyr · 2025-08-21T14:58:28Z

Thanks. We're hoping to tag tomorrow, so if it would help you, we'd still be happy to take over. We will of course add a Co-Authored-By line to give you credit.

samthanawalla · 2025-08-21T20:32:21Z

LGTM once you resolve the conflicts, thanks!

krtkvrm · 2025-08-21T21:00:55Z

@samthanawalla @jba @findleyr thanks for helping with reviews on this PR. again sorry for delays, I was caught up with something personal. Please feel free to take over if the deadline is close and things are pending.

Looking forward to contribute more :)

findleyr · 2025-08-21T21:03:38Z

Thanks @krtkvrm! Do you think you could look into the staticcheck lint failure? It looks trivial, and is blocking our merging.

krtkvrm · 2025-08-21T21:03:50Z

pushed the fix for lint

fixes: modelcontextprotocol#13 Implements elicitation functionality from MCP 2025-06-18 specification. Changes - Add ElicitParams and ElicitResult protocol types - Add ServerSession.Elicit() method and ClientOptions.ElicitationHandler - Schema validation enforces top-level properties only - Support for accept/decline/cancel actions with progress tokens Tests - Integration test in TestEndToEnd - Schema validation, error handling, and capability declaration tests

krtkvrm changed the title ~~Feat/issue 13 elicitation support~~ feat/issue 13 elicitation support Jul 29, 2025

krtkvrm mentioned this pull request Jul 29, 2025

Elicitation support #13

Closed

krtkvrm commented Jul 29, 2025

View reviewed changes

mcp/client.go Show resolved Hide resolved

krtkvrm changed the title ~~feat/issue 13 elicitation support~~ feat/Issue-13: elicitation support Jul 29, 2025

jba requested changes Jul 29, 2025

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

mcp/client.go Show resolved Hide resolved

mcp/protocol.go Outdated Show resolved Hide resolved

samthanawalla reviewed Jul 29, 2025

View reviewed changes

mcp/protocol.go Outdated Show resolved Hide resolved

krtkvrm force-pushed the feat/issue-13-elicitation-support branch from 7ce6c7d to 498871c Compare July 31, 2025 14:49

samthanawalla suggested changes Jul 31, 2025

View reviewed changes

mcp/protocol.go Outdated Show resolved Hide resolved

mcp/protocol.go Show resolved Hide resolved

.gitignore Outdated Show resolved Hide resolved

mcp/protocol.go Show resolved Hide resolved

jba requested changes Aug 1, 2025

View reviewed changes

krtkvrm force-pushed the feat/issue-13-elicitation-support branch from ed7c7f2 to 824e0ce Compare August 10, 2025 16:14

krtkvrm requested review from jba and samthanawalla August 11, 2025 10:01

jba reviewed Aug 11, 2025

View reviewed changes

zackchandler reviewed Aug 20, 2025

View reviewed changes

krtkvrm added 6 commits August 22, 2025 01:59

feat: add support for elicitation

29c7838

fix: resolve descriptions

c4ae2f8

fix: add descrption to ElicitResult

6781696

fix: fix description to ElicitParams

27a6c68

fix: rebase failures

0702092

fix: use jsonrpc2.NewError

7de6f37

krtkvrm added 3 commits August 22, 2025 02:05

fix: add validation

953424e

fix: add tests

d573192

feat: adds example

7811366

krtkvrm added 2 commits August 22, 2025 02:23

fix: add validation tests

903792c

fix: add correct elicit example

87d139d

krtkvrm force-pushed the feat/issue-13-elicitation-support branch from 9e488ed to 87d139d Compare August 21, 2025 20:58

fix: lint

83b0f0a

krtkvrm requested a review from jba August 21, 2025 21:05

jba approved these changes Aug 22, 2025

View reviewed changes

samthanawalla approved these changes Aug 22, 2025

View reviewed changes

samthanawalla merged commit c058c6a into modelcontextprotocol:main Aug 22, 2025
5 checks passed

Conversation

krtkvrm commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jba commented Jul 29, 2025

Uh oh!

jba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samthanawalla Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samthanawalla commented Aug 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jba commented Aug 20, 2025

Uh oh!

samthanawalla commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krtkvrm commented Aug 21, 2025

Uh oh!

findleyr commented Aug 21, 2025

Uh oh!

samthanawalla commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krtkvrm commented Aug 21, 2025

Uh oh!

findleyr commented Aug 21, 2025

Uh oh!

krtkvrm commented Aug 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

krtkvrm commented Jul 29, 2025 •

edited

Loading

samthanawalla Aug 11, 2025 •

edited

Loading

samthanawalla commented Aug 21, 2025 •

edited

Loading

samthanawalla commented Aug 21, 2025 •

edited

Loading