fix: model content annotations as nested objects #242
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
9 tasks
tzolov
requested changes
Jun 15, 2025
There was a problem hiding this comment.
Thank you @LucaButBoring ! An important alignment of the schema to comply with the spec!
I've left few recommendations to improve the formatting and the test coverage.
mcp/src/test/java/io/modelcontextprotocol/client/StdioMcpSyncClientTests.java
Outdated
Show resolved
Hide resolved
- Moves annotation test from stdio suite to abstract suite - Adds async version of sync annotation test - Fixes wildcard import
… into fix/content-annotations # Conflicts: # mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java # mcp-test/src/main/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java # mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpAsyncClientTests.java # mcp/src/test/java/io/modelcontextprotocol/client/AbstractMcpSyncClientTests.java
|
Is it possible to rerun workflows? The failing test here appears to be unrelated to the modified code (and appears to be flaky, based on local runs): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Motivation and Context
Content annotations have been incorrect for at least six months. The integration test added in this PR fails without the related schema changes.
How Has This Been Tested?
Newly-implemented integration test, leveraging the appropriate tool from everything.
Breaking Changes
None; the old constructor has been maintained for backwards-compatibility purposes. The old auto-generated record properties (
.audience()and.priority()) have also been maintained.Types of changes
Checklist
Additional context