fix: model content annotations as nested objects #242
Closed
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): |
|
Thanks for the update @LucaButBoring . Looks good. We discovered a correlation between the #CPUs and time to run the tests. GH Actions are likely running with fewer CPUs and the initialization timeout for the testListToolsWithoutInitialization exceeds 6 sec. |
tzolov
pushed a commit
that referenced
this pull request
Jun 17, 2025
Updates all Content types to represent annotations as nested objects instead of directly on the content itself. - Refactor TextContent, ImageContent, and EmbeddedResource to use Annotations object instead of separate audience/priority fields - Add backward-compatible deprecated methods for audience() and priority() accessors - Add parameterized tests for message annotations across different message types (success, error, debug) - Test both text and image content annotations with proper priority and audience validation This brings the implementation in line with the content types as modeled by the specification: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/c87a0da6d8c2436d56a6398023c80b0562224454/schema/2025-03-26/schema.json#L1970-L1973
|
Rebased, squashed and merged at fb40047 |
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