Add metadata support to callTool method#289
Add metadata support to callTool method#289devcrocod merged 8 commits intomodelcontextprotocol:mainfrom
Conversation
| } | ||
|
|
||
| @OptIn(ExperimentalUnsignedTypes::class, ExperimentalSerializationApi::class) | ||
| private fun convertToJsonElement(value: Any?): JsonElement = when (value) { |
There was a problem hiding this comment.
It makes sense to extract JSON-related code to a separate class. It could be a separate PR with unit tests
There was a problem hiding this comment.
Should be moved to separate class
|
Thank you for your feedback, @kpavlov |
|
Thank you, @maeryo |
|
@kpavlov, I just fixed ktlint. 🚀 |
5a73759 to
2b957a9
Compare
devcrocod
left a comment
There was a problem hiding this comment.
Please take a look at my comments.
Also, it seems that your changes are breaking the tests
|
Sorry, the integration tests broke because of the new field added. I’ll add a fix soon |
|
@devcrocod I've addressed all your comments. Please let me know if any further changes are needed. |
- Add meta parameter support to callTool method with validation - Implement MCP-compliant meta key validation (reserved prefixes, format rules) - Enhance JSON conversion for complex data types with better error handling - Add comprehensive test suite for meta parameter functionality - Improve MockTransport to simulate proper initialization flow - Update API signatures to include meta parameter support
small refactoring
127b270 to
7e087b4
Compare
| Method.Defined.PromptsList, | ||
| Method.Defined.CompletionComplete, | ||
| -> { | ||
| Method.Defined.PromptsGet, Method.Defined.PromptsList, Method.Defined.CompletionComplete -> { |
There was a problem hiding this comment.
It's actually better to have each case on a new line, because add/delete is more visible in diff
| val labelPattern = Regex("[a-zA-Z]([a-zA-Z0-9-]*[a-zA-Z0-9])?") | ||
| val namePattern = Regex("[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?") |
There was a problem hiding this comment.
Regex patterns should be static constants
| } | ||
|
|
||
| @OptIn(ExperimentalUnsignedTypes::class, ExperimentalSerializationApi::class) | ||
| private fun convertToJsonElement(value: Any?): JsonElement = when (value) { |
There was a problem hiding this comment.
Should be moved to separate class
| "formattedResult" : "11,000", | ||
| "precision" : 3, | ||
| "tags" : [ ] | ||
| "tags" : ["test", "calculator", "integration"] |
There was a problem hiding this comment.
Yes, previously the arrays were serialized as a string, and for some reason they were completely skipped in the process
| client.callTool("test-tool", mapOf("arg" to "value"), validMeta) | ||
| } | ||
|
|
||
| assertTrue(result.isSuccess, "Valid meta keys should not cause exceptions") |
There was a problem hiding this comment.
assertion for meta content is missing
| } | ||
| } | ||
|
|
||
| class MockTransport : Transport { |
There was a problem hiding this comment.
MockTransport should be moved to separate file. It's a nice reusable stuff
| } | ||
|
|
||
| class MockTransport : Transport { | ||
| private val _sentMessages = mutableListOf<JSONRPCMessage>() |
There was a problem hiding this comment.
not a thread-safe collection
| } | ||
|
|
||
| override fun onMessage(block: suspend (JSONRPCMessage) -> Unit) { | ||
| onMessageBlock = block |
There was a problem hiding this comment.
should also capture and expose received messages (thread-safe)
kpavlov
left a comment
There was a problem hiding this comment.
Thank you, folks!
I'm fine with merging this PR and making betterments in follow-up PR(s)
|
@kpavlov Thanks for all the feedback. I'll prepare a follow-up PR after this is merged. |
## Changes - Updated `DecimalFormatSymbols` to use period as decimal separator (Locale.US) - Fixed test expectations in `testComplexInputSchemaTool`: - `formattedResult`: `"11,000"` → `"11.000"` - `tags`: Added actual test values `["test", "calculator", "integration"]` - Fixed test expectations in `testComplexNestedSchema`: - `formattedResult`: `"0,00"` → `"0.00"` ## Motivation 1. **Standard Decimal Notation**: Using a comma as a decimal separator could be confusing as it's commonly used for thousands separators in many locales. Standardizing on the period (`.`) follows international conventions and makes the output clearer. 2. **Test Data Correction**: During the Validate PR workflow for #289, discovered that the `tags` array in the `expectedContent` variable of `testComplexInputSchemaTool()` was incorrectly set to an empty array, which didn't match the actual input values being tested. --------- Co-authored-by: MAERYO <maeryo@hanwha.com>
According to https://modelcontextprotocol.io/specification/2025-06-18/basic/index#meta, the _meta parameter should be passable when calling tools. While the CallToolRequest class (in types.kt) currently includes _meta in its definition, SDK users cannot actually input _meta when invoking the callTool method.
This Pull Request adds a metadata parameter to the CallTool method and includes functionality to convert various data formats without loss.
Additionally, to encourage the use of valid Key name formats as defined by the protocol, validation for prefixes has been implemented.