-
Notifications
You must be signed in to change notification settings - Fork 862
feat: Add SEP-1034 elicitation schema default values #908
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,17 +11,19 @@ | |
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
| import com.fasterxml.jackson.annotation.JsonInclude; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.fasterxml.jackson.annotation.JsonSubTypes; | ||
| import com.fasterxml.jackson.annotation.JsonTypeInfo; | ||
|
|
||
| import io.modelcontextprotocol.json.McpJsonMapper; | ||
| import io.modelcontextprotocol.json.TypeRef; | ||
| import io.modelcontextprotocol.util.Assert; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * Based on the <a href="http://www.jsonrpc.org/specification">JSON-RPC 2.0 | ||
|
|
@@ -431,19 +433,34 @@ public record Sampling() { | |
| * @param url support for out-of-band URL-based elicitation | ||
| */ | ||
| @JsonInclude(JsonInclude.Include.NON_ABSENT) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record Elicitation(@JsonProperty("form") Form form, @JsonProperty("url") Url url) { | ||
|
|
||
| /** | ||
| * Marker record indicating support for form-based elicitation mode. | ||
| * Record indicating support for form-based elicitation mode. | ||
| * | ||
| * @param applyDefaults Whether the client should apply default values from | ||
| * the schema to the elicitation result content when fields are missing. When | ||
| * true, the SDK will automatically fill in missing fields with their | ||
| * schema-defined defaults before returning the result to the server. | ||
| */ | ||
| @JsonInclude(JsonInclude.Include.NON_ABSENT) | ||
| public record Form() { | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record Form(@JsonProperty("applyDefaults") Boolean applyDefaults) { | ||
|
|
||
| /** | ||
| * Creates a Form with default settings (no applyDefaults). | ||
| */ | ||
| public Form() { | ||
| this(null); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Marker record indicating support for URL-based elicitation mode. | ||
| */ | ||
| @JsonInclude(JsonInclude.Include.NON_ABSENT) | ||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public record Url() { | ||
| } | ||
|
|
||
|
|
@@ -507,6 +524,31 @@ public Builder elicitation(boolean form, boolean url) { | |
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Enables elicitation capability with form mode and applyDefaults setting. | ||
| * <p> | ||
| * Note: {@code applyDefaults} is an SDK-level behavior flag that controls | ||
| * whether the client automatically fills in missing fields from schema | ||
| * defaults. It is serialized as part of the capabilities sent to the server | ||
| * during initialization, consistent with the TypeScript SDK behavior. Servers | ||
| * should tolerate unknown capability fields per the MCP specification. | ||
| * @param form whether to support form-based elicitation | ||
| * @param url whether to support URL-based elicitation | ||
| * @param applyDefaults whether the client should apply schema defaults to | ||
| * elicitation results. Requires {@code form} to be {@code true}. | ||
| * @return this builder | ||
| * @throws IllegalArgumentException if {@code applyDefaults} is {@code true} | ||
| * but {@code form} is {@code false} | ||
| */ | ||
| public Builder elicitation(boolean form, boolean url, boolean applyDefaults) { | ||
| if (!form && applyDefaults) { | ||
| throw new IllegalArgumentException("applyDefaults requires form to be true"); | ||
| } | ||
| this.elicitation = new Elicitation(form ? new Elicitation.Form(applyDefaults) : null, | ||
| url ? new Elicitation.Url() : null); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new builder overload At minimum, add a guard: if (!form && applyDefaults) {
throw new IllegalArgumentException("applyDefaults requires form to be true");
}Or document the behavior clearly in the Javadoc.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. Added an |
||
| return this; | ||
| } | ||
|
|
||
| public ClientCapabilities build() { | ||
| return new ClientCapabilities(experimental, roots, sampling, elicitation); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| /* | ||
| * Copyright 2024-2025 the original author or authors. | ||
| */ | ||
|
|
||
| package io.modelcontextprotocol.client; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * Tests for {@link McpAsyncClient#applyElicitationDefaults(Map, Map)}. | ||
| * | ||
| * Verifies that the client-side default application logic correctly fills in missing | ||
| * fields from schema defaults, matching the behavior specified in SEP-1034. | ||
| */ | ||
| class McpAsyncClientElicitationDefaultsTests { | ||
|
|
||
| @Test | ||
| void appliesStringDefault() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("name", Map.of("type", "string", "default", "Guest"))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("name", "Guest"); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesNumberDefault() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("age", Map.of("type", "integer", "default", 18))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("age", 18); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesBooleanDefault() { | ||
| Map<String, Object> schema = Map.of("properties", | ||
| Map.of("subscribe", Map.of("type", "boolean", "default", true))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("subscribe", true); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesEnumDefault() { | ||
| Map<String, Object> schema = Map.of("properties", | ||
| Map.of("color", Map.of("type", "string", "enum", List.of("red", "green"), "default", "green"))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("color", "green"); | ||
| } | ||
|
|
||
| @Test | ||
| void doesNotOverrideExistingValues() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("name", Map.of("type", "string", "default", "Guest"))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| content.put("name", "Alice"); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("name", "Alice"); | ||
| } | ||
|
|
||
| @Test | ||
| void skipsPropertiesWithoutDefault() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("email", Map.of("type", "string"))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).doesNotContainKey("email"); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesMultipleDefaults() { | ||
| Map<String, Object> schema = Map.of("properties", | ||
| Map.of("name", Map.of("type", "string", "default", "Guest"), "age", | ||
| Map.of("type", "integer", "default", 18), "subscribe", | ||
| Map.of("type", "boolean", "default", true), "color", | ||
| Map.of("type", "string", "enum", List.of("red", "green"), "default", "green"))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("name", "Guest") | ||
| .containsEntry("age", 18) | ||
| .containsEntry("subscribe", true) | ||
| .containsEntry("color", "green"); | ||
| } | ||
|
|
||
| @Test | ||
| void handlesNullSchema() { | ||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(null, content); | ||
|
|
||
| assertThat(content).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void handlesNullContent() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("name", Map.of("type", "string", "default", "Guest"))); | ||
|
|
||
| // Should not throw | ||
| McpAsyncClient.applyElicitationDefaults(schema, null); | ||
| } | ||
|
|
||
| @Test | ||
| void handlesSchemaWithoutProperties() { | ||
| Map<String, Object> schema = Map.of("type", "object"); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesDefaultsOnlyToMissingFields() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("name", Map.of("type", "string", "default", "Guest"), | ||
| "age", Map.of("type", "integer", "default", 18))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| content.put("name", "John"); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("name", "John").containsEntry("age", 18); | ||
| } | ||
|
|
||
| @Test | ||
| void appliesFloatingPointDefault() { | ||
| Map<String, Object> schema = Map.of("properties", Map.of("score", Map.of("type", "number", "default", 95.5))); | ||
|
|
||
| Map<String, Object> content = new HashMap<>(); | ||
| McpAsyncClient.applyElicitationDefaults(schema, content); | ||
|
|
||
| assertThat(content).containsEntry("score", 95.5); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applyDefaultsis a client-side SDK behavior flag, but because it lives insideClientCapabilities.Elicitation.Form, it gets serialized and sent to the server in theinitializerequest. A strict MCP server that validates capabilities against the protocol schema will receive an unknown field it never declared. This could break interoperability.Consider keeping
applyDefaultspurely in the SDK layer (e.g., a separateMcpClientOptions/ builder flag) rather than embedding it in the protocol capabilities record. Alternatively, document explicitly that this is an SDK extension field and that servers must tolerate unknown capability fields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the interoperability concern.
This follows the TypeScript SDK's design —
applyDefaultsis defined insideFormElicitationCapabilitySchemaand gets serialized to the server during initialization:https://github.com/modelcontextprotocol/typescript-sdk/blob/main/packages/core/src/types/schemas.ts#L312
The Java SDK already uses
@JsonIgnoreProperties(ignoreUnknown = true)on capability records, so servers built with this SDK will tolerate it. I've added Javadoc documenting this as an SDK-level behavior flag, consistent with the TypeScript reference implementation.That said, if the maintainers prefer keeping it outside the protocol record (separate builder flag), happy to refactor.