Adds optional _meta field to Tool #339
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds an optional _meta field to the Tool class to align with the OpenAI Apps SDK specification, which includes metadata fields on Tool descriptors. The change maintains backward compatibility by defaulting to an empty JSON object.
- Adds
_metafield toTooldata class by implementing theWithMetainterface - Updates
Server.addTool()method to accept an optional_metaparameter - Adds comprehensive serialization tests for the new field
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types.kt | Adds _meta field to Tool class and implements WithMeta interface |
| kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt | Updates addTool() method to accept optional _meta parameter |
| kotlin-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/ToolSerializationTest.kt | Adds test coverage for _meta field serialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
kotlin-sdk-core/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/types.kt
Show resolved
Hide resolved
...n-sdk-core/src/commonTest/kotlin/io/modelcontextprotocol/kotlin/sdk/ToolSerializationTest.kt
Show resolved
Hide resolved
kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt
Outdated
Show resolved
Hide resolved
Adds support for tool descriptor metadata to misk-mcp. This is required by then OpenAI Apps SDK: https://developers.openai.com/apps-sdk/reference This is a temporary solution until modelcontextprotocol/kotlin-sdk#339 can be pulled in.
My pleasure @kpavlov, looking forward to this merge |
|
#289 |
|
ping @kpavlov and @devcrocod , would love to get this merged |
# Conflicts: # kotlin-sdk-server/src/commonMain/kotlin/io/modelcontextprotocol/kotlin/sdk/server/Server.kt
| public class Builder { | ||
| public var name: String? = null | ||
| public var title: String? = null | ||
| public var description: String? = null | ||
| public var inputSchema: Input = Input() | ||
| public var outputSchema: Output? = null | ||
| public var annotations: ToolAnnotations? = null | ||
|
|
||
| @Suppress("PropertyName") | ||
| public var _meta: JsonObject = EmptyJsonObject | ||
|
|
||
| public fun build(): Tool = Tool( | ||
| name = requireNotNull(name) { "Tool name is required" }, | ||
| title = title, | ||
| description = description, | ||
| inputSchema = inputSchema, | ||
| outputSchema = outputSchema, | ||
| annotations = annotations, | ||
| _meta = _meta, | ||
| ) | ||
| } | ||
|
|
||
| public companion object { | ||
| public fun build(builder: Builder.() -> Unit): Tool = Builder().apply(builder).build() | ||
| } |
There was a problem hiding this comment.
I think adding a builder in this PR feels a bit out of place compared to the rest of the API
There was a problem hiding this comment.
Ok, that was a PR suggestion from @kpavlov . I can revert the builder in order to get the too descriptor _meta field merged. Please advise @devcrocod
There was a problem hiding this comment.
@devcrocod I went ahead and removed the builder. If its still desired, it can be, more appropriately, in a separate PR.
4 participants
Motivation and Context
The recently released openAI Apps SDK adds a '_meta' field on the Tool Descriptor. This would be in addition to it being available on Messages. It remains optional and defaults to an empty json object, following existing conventions.
How Has This Been Tested?
Unit tests for serialization added
Breaking Changes
N/A
Types of changes
Checklist
Additional context
https://developers.openai.com/apps-sdk
https://developers.openai.com/apps-sdk/reference/#_meta-fields-the-client-provides