From d55d404f6d566fd5292731baf0269027ecfe26a5 Mon Sep 17 00:00:00 2001 From: Luca Chang Date: Fri, 16 May 2025 15:49:43 -0700 Subject: [PATCH 1/2] fix: model annotations as nested objects Updates all Content types to represent annotations as nested objects instead of directly on the content itself. The existing ctors have been moved to deprecated shim ctors for backwards-compat. This brings the implementation in line with the content types as modeled by the specification. An integration test has been added to ensure this doesn't drift in the future. https://github.com/modelcontextprotocol/modelcontextprotocol/blob/c87a0da6d8c2436d56a6398023c80b0562224454/schema/2025-03-26/schema.json#L1970-L1973 --- .../modelcontextprotocol/spec/McpSchema.java | 42 +++++++++---- .../client/StdioMcpSyncClientTests.java | 60 +++++++++++++++++++ 2 files changed, 92 insertions(+), 10 deletions(-) diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java index 8df8a158..052e8074 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java @@ -1277,30 +1277,52 @@ else if (this instanceof EmbeddedResource) { @JsonInclude(JsonInclude.Include.NON_ABSENT) @JsonIgnoreProperties(ignoreUnknown = true) public record TextContent( // @formatter:off - @JsonProperty("audience") List audience, - @JsonProperty("priority") Double priority, - @JsonProperty("text") String text) implements Content { // @formatter:on + @JsonProperty("annotations") Annotations annotations, + @JsonProperty("text") String text) implements Annotated, Content { // @formatter:on public TextContent(String content) { - this(null, null, content); + this(null, content); + } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link TextContent#TextContent(Annotations, String)} instead. + */ + public TextContent(List audience, Double priority, String content) { + this(audience != null || priority != null ? new Annotations(audience, priority) : null, content); } } @JsonInclude(JsonInclude.Include.NON_ABSENT) @JsonIgnoreProperties(ignoreUnknown = true) public record ImageContent( // @formatter:off - @JsonProperty("audience") List audience, - @JsonProperty("priority") Double priority, + @JsonProperty("annotations") Annotations annotations, @JsonProperty("data") String data, - @JsonProperty("mimeType") String mimeType) implements Content { // @formatter:on + @JsonProperty("mimeType") String mimeType) implements Annotated, Content { // @formatter:on + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link ImageContent#ImageContent(Annotations, String, String)} instead. + */ + public ImageContent(List audience, Double priority, String data, String mimeType) { + this(audience != null || priority != null ? new Annotations(audience, priority) : null, data, mimeType); + } } @JsonInclude(JsonInclude.Include.NON_ABSENT) @JsonIgnoreProperties(ignoreUnknown = true) public record EmbeddedResource( // @formatter:off - @JsonProperty("audience") List audience, - @JsonProperty("priority") Double priority, - @JsonProperty("resource") ResourceContents resource) implements Content { // @formatter:on + @JsonProperty("annotations") Annotations annotations, + @JsonProperty("resource") ResourceContents resource) implements Annotated, Content { // @formatter:on + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link EmbeddedResource#EmbeddedResource(Annotations, ResourceContents)} + * instead. + */ + public EmbeddedResource(List audience, Double priority, ResourceContents resource) { + this(audience != null || priority != null ? new Annotations(audience, priority) : null, resource); + } } // --------------------------- diff --git a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java index 706aa9b2..c096ef16 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java @@ -5,6 +5,7 @@ package io.modelcontextprotocol.client; import java.time.Duration; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -12,12 +13,17 @@ import io.modelcontextprotocol.client.transport.ServerParameters; import io.modelcontextprotocol.client.transport.StdioClientTransport; import io.modelcontextprotocol.spec.McpClientTransport; +import io.modelcontextprotocol.spec.McpSchema; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import reactor.core.publisher.Sinks; import reactor.test.StepVerifier; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; /** * Tests for the {@link McpSyncClient} with {@link StdioClientTransport}. @@ -67,6 +73,60 @@ void customErrorHandlerShouldReceiveErrors() throws InterruptedException { StepVerifier.create(transport.closeGracefully()).expectComplete().verify(Duration.ofSeconds(5)); } + @ParameterizedTest + @ValueSource(strings = { "success", "error", "debug" }) + void testMessageAnnotations(String messageType) { + McpClientTransport transport = createMcpTransport(); + + withClient(transport, client -> { + client.initialize(); + + McpSchema.CallToolResult result = client.callTool(new McpSchema.CallToolRequest("annotatedMessage", + Map.of("messageType", messageType, "includeImage", true))); + + assertThat(result).isNotNull(); + assertThat(result.isError()).isNotEqualTo(true); + assertThat(result.content()).isNotEmpty(); + assertThat(result.content()).allSatisfy(content -> { + switch (content.type()) { + case "text": + McpSchema.TextContent textContent = assertInstanceOf(McpSchema.TextContent.class, content); + assertThat(textContent.text()).isNotEmpty(); + assertThat(textContent.annotations()).isNotNull(); + + switch (messageType) { + case "error": + assertThat(textContent.annotations().priority()).isEqualTo(1.0); + assertThat(textContent.annotations().audience()).containsOnly(McpSchema.Role.USER, + McpSchema.Role.ASSISTANT); + break; + case "success": + assertThat(textContent.annotations().priority()).isEqualTo(0.7); + assertThat(textContent.annotations().audience()).containsExactly(McpSchema.Role.USER); + break; + case "debug": + assertThat(textContent.annotations().priority()).isEqualTo(0.3); + assertThat(textContent.annotations().audience()) + .containsExactly(McpSchema.Role.ASSISTANT); + break; + default: + throw new IllegalStateException("Unexpected value: " + content.type()); + } + break; + case "image": + McpSchema.ImageContent imageContent = assertInstanceOf(McpSchema.ImageContent.class, content); + assertThat(imageContent.data()).isNotEmpty(); + assertThat(imageContent.annotations()).isNotNull(); + assertThat(imageContent.annotations().priority()).isEqualTo(0.5); + assertThat(imageContent.annotations().audience()).containsExactly(McpSchema.Role.USER); + break; + default: + fail("Unexpected content type: " + content.type()); + } + }); + }); + } + protected Duration getInitializationTimeout() { return Duration.ofSeconds(6); } From 253e360db545e84a8ef48dbb445517c32cc9b5f5 Mon Sep 17 00:00:00 2001 From: Luca Chang Date: Fri, 16 May 2025 16:12:38 -0700 Subject: [PATCH 2/2] fix: add backwards compat shims for old annotations --- .../modelcontextprotocol/spec/McpSchema.java | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java b/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java index 052e8074..2725a782 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java +++ b/mcp/src/main/java/io/modelcontextprotocol/spec/McpSchema.java @@ -5,10 +5,7 @@ package io.modelcontextprotocol.spec; import java.io.IOException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; @@ -1291,6 +1288,22 @@ public TextContent(String content) { public TextContent(List audience, Double priority, String content) { this(audience != null || priority != null ? new Annotations(audience, priority) : null, content); } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link TextContent#annotations()} instead. + */ + public List audience() { + return annotations == null ? null : annotations.audience(); + } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link TextContent#annotations()} instead. + */ + public Double priority() { + return annotations == null ? null : annotations.priority(); + } } @JsonInclude(JsonInclude.Include.NON_ABSENT) @@ -1307,6 +1320,22 @@ public record ImageContent( // @formatter:off public ImageContent(List audience, Double priority, String data, String mimeType) { this(audience != null || priority != null ? new Annotations(audience, priority) : null, data, mimeType); } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link ImageContent#annotations()} instead. + */ + public List audience() { + return annotations == null ? null : annotations.audience(); + } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link ImageContent#annotations()} instead. + */ + public Double priority() { + return annotations == null ? null : annotations.priority(); + } } @JsonInclude(JsonInclude.Include.NON_ABSENT) @@ -1323,6 +1352,22 @@ public record EmbeddedResource( // @formatter:off public EmbeddedResource(List audience, Double priority, ResourceContents resource) { this(audience != null || priority != null ? new Annotations(audience, priority) : null, resource); } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link EmbeddedResource#annotations()} instead. + */ + public List audience() { + return annotations == null ? null : annotations.audience(); + } + + /** + * @deprecated Only exists for backwards-compatibility purposes. Use + * {@link EmbeddedResource#annotations()} instead. + */ + public Double priority() { + return annotations == null ? null : annotations.priority(); + } } // ---------------------------