From 864f13a34760f36386c89d8f0263c28aed99c0ff Mon Sep 17 00:00:00 2001 From: Charlie Holland Date: Wed, 4 Nov 2026 19:04:35 +0000 Subject: [PATCH] mcp: apply elicitation defaults when an accept has no content Client.elicit applied requestedSchema defaults (SEP-1034) only when the handler's result had non-nil Content. A handler that accepts what the form offered returns &ElicitResult{Action: "accept"}, so the server received an accept with no values, and the schema was not checked either. Treat an accept without content as an empty answer: start from an empty map, then validate and apply defaults as before. A required property without a default is still rejected. TestElicitationAcceptWithoutContent covers both; each case fails without the change. docs/client.md now says an accept without content takes every default. Fixes #1330 Signed-off-by: Charlie Holland --- docs/client.md | 5 +++ internal/docs/client.src.md | 5 +++ mcp/client.go | 7 +++- mcp/mcp_test.go | 73 +++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/docs/client.md b/docs/client.md index 2e6917a7..211a8a1f 100644 --- a/docs/client.md +++ b/docs/client.md @@ -313,6 +313,11 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. +An elicitation handler that accepts the form as offered can return +`&ElicitResult{Action: "accept"}` with no `Content`. The client treats that +as an empty answer: every field takes its default, and a required field +without one is rejected. + An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is diff --git a/internal/docs/client.src.md b/internal/docs/client.src.md index 72aaa2e6..de0c711c 100644 --- a/internal/docs/client.src.md +++ b/internal/docs/client.src.md @@ -108,6 +108,11 @@ defaulted field `Required` defeats it: accepted content is validated against the schema before defaults are applied, so an answer that omits the field is rejected rather than defaulted. +An elicitation handler that accepts the form as offered can return +`&ElicitResult{Action: "accept"}` with no `Content`. The client treats that +as an empty answer: every field takes its default, and a required field +without one is rejected. + An `Enum` ([SEP-1330](https://modelcontextprotocol.io/seps/1330-elicitation-enum-schema-improvements-and-standards)) restricts a field to a fixed set of values, which the client renders as a choice. Enums are supported only on `"string"` fields; declaring one on another type is diff --git a/mcp/client.go b/mcp/client.go index 1c642a0d..c0f19966 100644 --- a/mcp/client.go +++ b/mcp/client.go @@ -775,8 +775,13 @@ func (c *Client) elicit(ctx context.Context, req *ElicitRequest) (*ElicitResult, if res == nil { return nil, fmt.Errorf("%w: ElicitationHandler returned a nil result", jsonrpc2.ErrInternal) } + // An accept without content answers nothing, so every field takes its + // default. + if res.Action == "accept" && schema != nil && res.Content == nil { + res.Content = map[string]any{} + } // Validate elicitation result content against requested schema. - if res.Action == "accept" && schema != nil && res.Content != nil { + if res.Action == "accept" && schema != nil { resolved, err := schema.Resolve(nil) if err != nil { return nil, &jsonrpc.Error{Code: jsonrpc.CodeInvalidParams, Message: fmt.Sprintf("failed to resolve requested schema: %v", err)} diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index 9e830c7b..404c57ec 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -2006,6 +2006,79 @@ func TestElicitationDefaultValues(t *testing.T) { } } +// TestElicitationAcceptWithoutContent checks that an accept with nil content +// is treated as an empty answer: defaults are applied and the schema is still +// enforced. +func TestElicitationAcceptWithoutContent(t *testing.T) { + ctx := context.Background() + testcases := []struct { + name string + schema *jsonschema.Schema + expected map[string]any + wantErr bool + }{ + { + name: "defaults are applied", + schema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "color": {Type: "string", Default: json.RawMessage(`"blue"`)}, + "count": {Type: "integer", Default: json.RawMessage("3")}, + }, + }, + expected: map[string]any{"color": "blue", "count": float64(3)}, + }, + { + name: "required property is enforced", + schema: &jsonschema.Schema{ + Type: "object", + Required: []string{"name"}, + Properties: map[string]*jsonschema.Schema{ + "name": {Type: "string"}, + }, + }, + wantErr: true, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ct, st := NewInMemoryTransports() + ss, err := NewServer(testImpl, nil).Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + defer ss.Close() + c := NewClient(testImpl, &ClientOptions{ + ElicitationHandler: func(context.Context, *ElicitRequest) (*ElicitResult, error) { + return &ElicitResult{Action: "accept", Content: nil}, nil + }, + }) + cs, err := c.Connect(ctx, ct, &ClientSessionOptions{ProtocolVersion: protocolVersion20251125}) + if err != nil { + t.Fatal(err) + } + defer cs.Close() + + res, err := ss.Elicit(ctx, &ElicitParams{Message: tc.name, RequestedSchema: tc.schema}) + if tc.wantErr { + if err == nil { + t.Fatalf("Elicit succeeded with content %v, want a validation error", res.Content) + } + if !strings.Contains(err.Error(), "does not match requested schema") { + t.Fatalf("Elicit error = %v, want a schema validation error", err) + } + return + } + if err != nil { + t.Fatalf("Elicit: %v", err) + } + if diff := cmp.Diff(tc.expected, res.Content); diff != "" { + t.Errorf("content mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestKeepAliveFailure(t *testing.T) { synctest.Test(t, func(t *testing.T) { ctx := context.Background()