-
Notifications
You must be signed in to change notification settings - Fork 176
Add metadata support to callTool method #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
devcrocod
merged 8 commits into
modelcontextprotocol:main
from
maeryo:add-meta-in-call-tool-method
Oct 23, 2025
+500
−20
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ad37e5d
feat: Add meta parameter support to callTool method
e6e5d54
feat: implement complete _meta support with MCP specification validation
135ffd7
fix: correct _meta key validation according to MCP spec
b9b3ba2
test: add comprehensive meta parameter tests for callTool
3f22a9a
feat: Add comprehensive meta parameter support to MCP client
4a1a29e
refactor: simplify validation and JSON conversion
7e087b4
Fix compilation and tests for meta.
devcrocod de5f8b5
Add `MockTransport` for testability and improve meta field validation…
devcrocod File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,10 +50,14 @@ import kotlinx.atomicfu.update | |
| import kotlinx.collections.immutable.minus | ||
| import kotlinx.collections.immutable.persistentMapOf | ||
| import kotlinx.collections.immutable.toPersistentSet | ||
| import kotlinx.serialization.ExperimentalSerializationApi | ||
| import kotlinx.serialization.json.JsonElement | ||
| import kotlinx.serialization.json.JsonNull | ||
| import kotlinx.serialization.json.JsonObject | ||
| import kotlinx.serialization.json.JsonPrimitive | ||
| import kotlinx.serialization.json.add | ||
| import kotlinx.serialization.json.buildJsonArray | ||
| import kotlinx.serialization.json.buildJsonObject | ||
| import kotlin.coroutines.cancellation.CancellationException | ||
|
|
||
| private val logger = KotlinLogging.logger {} | ||
|
|
@@ -210,17 +214,13 @@ public open class Client(private val clientInfo: Implementation, options: Client | |
| } | ||
| } | ||
|
|
||
| Method.Defined.ToolsCall, | ||
| Method.Defined.ToolsList, | ||
| -> { | ||
| Method.Defined.ToolsCall, Method.Defined.ToolsList -> { | ||
| if (serverCapabilities?.tools == null) { | ||
| throw IllegalStateException("Server does not support tools (required for $method)") | ||
| } | ||
| } | ||
|
|
||
| Method.Defined.Initialize, | ||
| Method.Defined.Ping, | ||
| -> { | ||
| Method.Defined.Initialize, Method.Defined.Ping -> { | ||
| // No specific capability required | ||
| } | ||
|
|
||
|
|
@@ -405,10 +405,14 @@ public open class Client(private val clientInfo: Implementation, options: Client | |
| ): EmptyRequestResult = request(request, options) | ||
|
|
||
| /** | ||
| * Calls a tool on the server by name, passing the specified arguments. | ||
| * Calls a tool on the server by name, passing the specified arguments and metadata. | ||
| * | ||
| * @param name The name of the tool to call. | ||
| * @param arguments A map of argument names to values for the tool. | ||
| * @param meta A map of metadata key-value pairs. Keys must follow MCP specification format. | ||
| * - Optional prefix: dot-separated labels followed by slash (e.g., "api.example.com/") | ||
| * - Name: alphanumeric start/end, may contain hyphens, underscores, dots, alphanumerics | ||
| * - Reserved prefixes starting with "mcp" or "modelcontextprotocol" are forbidden | ||
| * @param compatibility Whether to use compatibility mode for older protocol versions. | ||
| * @param options Optional request options. | ||
| * @return The result of the tool call, or `null` if none. | ||
|
|
@@ -417,23 +421,19 @@ public open class Client(private val clientInfo: Implementation, options: Client | |
| public suspend fun callTool( | ||
| name: String, | ||
| arguments: Map<String, Any?>, | ||
| meta: Map<String, Any?> = emptyMap(), | ||
| compatibility: Boolean = false, | ||
| options: RequestOptions? = null, | ||
| ): CallToolResultBase? { | ||
| val jsonArguments = arguments.mapValues { (_, value) -> | ||
| when (value) { | ||
| is String -> JsonPrimitive(value) | ||
| is Number -> JsonPrimitive(value) | ||
| is Boolean -> JsonPrimitive(value) | ||
| is JsonElement -> value | ||
| null -> JsonNull | ||
| else -> JsonPrimitive(value.toString()) | ||
| } | ||
| } | ||
| validateMetaKeys(meta.keys) | ||
|
|
||
| val jsonArguments = convertToJsonMap(arguments) | ||
| val jsonMeta = convertToJsonMap(meta) | ||
|
|
||
| val request = CallToolRequest( | ||
| name = name, | ||
| arguments = JsonObject(jsonArguments), | ||
| _meta = JsonObject(jsonMeta), | ||
| ) | ||
| return callTool(request, compatibility, options) | ||
| } | ||
|
|
@@ -588,4 +588,116 @@ public open class Client(private val clientInfo: Implementation, options: Client | |
| val rootList = roots.value.values.toList() | ||
| return ListRootsResult(rootList) | ||
| } | ||
|
|
||
| /** | ||
| * Validates meta keys according to MCP specification. | ||
| * | ||
| * Key format: [prefix/]name | ||
| * - Prefix (optional): dot-separated labels + slash | ||
| * - Reserved prefixes contain "modelcontextprotocol" or "mcp" as complete labels | ||
| * - Name: alphanumeric start/end, may contain hyphens, underscores, dots (empty allowed) | ||
| */ | ||
| private fun validateMetaKeys(keys: Set<String>) { | ||
| 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])?") | ||
|
|
||
| keys.forEach { key -> | ||
| require(key.isNotEmpty()) { "Meta key cannot be empty" } | ||
|
|
||
| val (prefix, name) = key.split('/', limit = 2).let { parts -> | ||
| when (parts.size) { | ||
| 1 -> null to parts[0] | ||
| 2 -> parts[0] to parts[1] | ||
| else -> throw IllegalArgumentException("Unexpected split result for key: $key") | ||
| } | ||
| } | ||
|
|
||
| // Validate prefix if present | ||
| prefix?.let { | ||
| require(it.isNotEmpty()) { "Invalid _meta key '$key': prefix cannot be empty" } | ||
|
|
||
| val labels = it.split('.') | ||
| require(labels.all { label -> label.matches(labelPattern) }) { | ||
| "Invalid _meta key '$key': prefix labels must start with a letter, end with letter/digit, and contain only letters, digits, or hyphens" | ||
| } | ||
|
|
||
| require( | ||
| labels.none { label -> | ||
| label.equals("modelcontextprotocol", ignoreCase = true) || | ||
| label.equals("mcp", ignoreCase = true) | ||
| }, | ||
| ) { | ||
| "Invalid _meta key '$key': prefix cannot contain reserved labels 'modelcontextprotocol' or 'mcp'" | ||
| } | ||
| } | ||
|
|
||
| // Validate name (empty allowed) | ||
| require(name.isEmpty() || name.matches(namePattern)) { | ||
| "Invalid _meta key '$key': name must start and end with alphanumeric characters, and contain only alphanumerics, hyphens, underscores, or dots" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun convertToJsonMap(map: Map<String, Any?>): Map<String, JsonElement> = map.mapValues { (key, value) -> | ||
| try { | ||
| convertToJsonElement(value) | ||
| } catch (e: Exception) { | ||
| logger.warn { "Failed to convert value for key '$key': ${e.message}. Using string representation." } | ||
| JsonPrimitive(value.toString()) | ||
| } | ||
| } | ||
|
|
||
| @OptIn(ExperimentalUnsignedTypes::class, ExperimentalSerializationApi::class) | ||
| private fun convertToJsonElement(value: Any?): JsonElement = when (value) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes sense to extract JSON-related code to a separate class. It could be a separate PR with unit tests
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be moved to separate class
maeryo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| null -> JsonNull | ||
|
|
||
| is JsonElement -> value | ||
|
|
||
| is String -> JsonPrimitive(value) | ||
|
|
||
| is Number -> JsonPrimitive(value) | ||
|
|
||
| is Boolean -> JsonPrimitive(value) | ||
|
|
||
| is Char -> JsonPrimitive(value.toString()) | ||
|
|
||
| is Enum<*> -> JsonPrimitive(value.name) | ||
|
|
||
| is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), convertToJsonElement(v)) } } | ||
|
|
||
| is Collection<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } | ||
|
|
||
| is Array<*> -> buildJsonArray { value.forEach { add(convertToJsonElement(it)) } } | ||
|
|
||
| // Primitive arrays | ||
| is IntArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is LongArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is FloatArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is DoubleArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is BooleanArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is ShortArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is ByteArray -> buildJsonArray { value.forEach { add(it) } } | ||
|
|
||
| is CharArray -> buildJsonArray { value.forEach { add(it.toString()) } } | ||
|
|
||
| // Unsigned arrays | ||
| is UIntArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } | ||
|
|
||
| is ULongArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } | ||
|
|
||
| is UShortArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } | ||
|
|
||
| is UByteArray -> buildJsonArray { value.forEach { add(JsonPrimitive(it)) } } | ||
|
|
||
| else -> { | ||
| logger.debug { "Converting unknown type ${value::class} to string: $value" } | ||
| JsonPrimitive(value.toString()) | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex patterns should be static constants