Add missing title, size and annotation attributes on Resource/ResourceTemplate and other types#219
Conversation
faeebc5 to
4cd3f6e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds missing attributes and the Annotations type to align the Kotlin SDK with the latest MCP specification. The changes include adding missing attributes to several resource and content types, implementing the new Annotations data class, and ensuring proper serialization support.
- Adds the missing
Annotationstype with validation for priority values - Extends
Resource,ResourceTemplate, and various content types with missing attributes - Includes serialization/deserialization tests for the new
Annotationstype
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types.kt | Implements the Annotations data class and adds missing attributes to resource and content types |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/TypesTest.kt | Adds test coverage for Annotations serialization/deserialization |
| kotlin-sdk-core/api/kotlin-sdk-core.api | Updates API signatures to reflect the new attributes and Annotations type |
| val priority: Double?, | ||
| ) { | ||
| init { | ||
| require(priority == null || priority in 0.0..1.0) { "Priority must be between 0.0 and 1.0" } |
There was a problem hiding this comment.
[nitpick] The validation logic could be more explicit about inclusive bounds. Consider using a more descriptive error message that clarifies both bounds are inclusive: "Priority must be between 0.0 and 1.0 (inclusive)"
| require(priority == null || priority in 0.0..1.0) { "Priority must be between 0.0 and 1.0" } | |
| require(priority == null || priority in 0.0..1.0) { "Priority must be between 0.0 and 1.0 (inclusive)" } |
devcrocod
left a comment
There was a problem hiding this comment.
Thank you!
lgtm
I have just one small question
4cd3f6e to
c7c5b5a
Compare
This PR adds:
AnnotationstypeResource.title,Resource.sizeandResource.annotationsResourceTemplate.titleandResourceTemplate.annotationsTextContent.annotationsImageContent.annotationsAudioContent.annotationsEmeddedResource.annotationsFixes #159.
(PS: the solution I described in the issue was wrong. I checked with the specification's schema to be sure of the changes.)
Motivation and Context
Adherence to the latest specification version, cf. #159.
How Has This Been Tested?
I only added a serialization/deserialization test for
Annotationsto ensure that the serializer forkotlin.time.Instantwas indeed present. Cf. below in "Additional Context".Breaking Changes
N/A
Types of changes
Checklist
Additional context
I took two liberties:
Annotations.lastModified, as this must be encoded as an ISO 8601 date.Annotations.priorityto verify that it is between 0 and 1.