fix(mcp): standardize MCP tool name formatting and improve error handling
- Add prefixedToolName utility method to ensure consistent tool name formatting - E nforce alphanumeric, underscore, and hyphen characters only in tool names - Limit tool names to 64 characters maximum - Use original tool name in actual calls while using formatted names in definitions - Add error handling for tool call responses in SyncMcpToolCallback - Update tests to reflect the changes Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
This commit is contained in:
committed by
Christian Tzolov
parent
ded9facfe5
commit
3f7f2f1bff
@@ -86,7 +86,7 @@ public class AsyncMcpToolCallback implements ToolCallback {
|
||||
@Override
|
||||
public ToolDefinition getToolDefinition() {
|
||||
return ToolDefinition.builder()
|
||||
.name(this.asyncMcpClient.getClientInfo().name() + "-" + this.tool.name())
|
||||
.name(McpToolUtils.prefixedToolName(this.asyncMcpClient.getClientInfo().name(), this.tool.name()))
|
||||
.description(this.tool.description())
|
||||
.inputSchema(ModelOptionsUtils.toJsonString(this.tool.inputSchema()))
|
||||
.build();
|
||||
@@ -107,7 +107,9 @@ public class AsyncMcpToolCallback implements ToolCallback {
|
||||
@Override
|
||||
public String call(String functionInput) {
|
||||
Map<String, Object> arguments = ModelOptionsUtils.jsonToMap(functionInput);
|
||||
return this.asyncMcpClient.callTool(new CallToolRequest(this.getToolDefinition().name(), arguments))
|
||||
// Note that we use the original tool name here, not the adapted one from
|
||||
// getToolDefinition
|
||||
return this.asyncMcpClient.callTool(new CallToolRequest(this.tool.name(), arguments))
|
||||
.map(response -> ModelOptionsUtils.toJsonString(response.content()))
|
||||
.block();
|
||||
}
|
||||
|
||||
@@ -56,6 +56,26 @@ public final class McpToolUtils {
|
||||
private McpToolUtils() {
|
||||
}
|
||||
|
||||
public static String prefixedToolName(String prefix, String toolName) {
|
||||
|
||||
String input = prefix + "-" + toolName;
|
||||
|
||||
if (input == null || input.isEmpty()) {
|
||||
throw new IllegalArgumentException("Input string cannot be null or empty");
|
||||
}
|
||||
|
||||
// Replace any character that isn't alphanumeric, underscore, or hyphen with
|
||||
// concatenation
|
||||
String formatted = input.replaceAll("[^a-zA-Z0-9_-]", "");
|
||||
|
||||
// If the string is longer than 64 characters, keep the last 64 characters
|
||||
if (formatted.length() > 64) {
|
||||
formatted = formatted.substring(formatted.length() - 64);
|
||||
}
|
||||
|
||||
return formatted;
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts a list of Spring AI tool callbacks to MCP synchronous tool registrations.
|
||||
* <p>
|
||||
|
||||
@@ -17,7 +17,6 @@
|
||||
package org.springframework.ai.mcp;
|
||||
|
||||
import java.util.Map;
|
||||
import java.util.UUID;
|
||||
|
||||
import io.modelcontextprotocol.client.McpSyncClient;
|
||||
import io.modelcontextprotocol.spec.McpSchema.CallToolRequest;
|
||||
@@ -42,7 +41,9 @@ import org.springframework.ai.tool.definition.ToolDefinition;
|
||||
* <li>Manages JSON serialization/deserialization of tool inputs and outputs</li>
|
||||
* </ul>
|
||||
* <p>
|
||||
* Example usage: <pre>{@code
|
||||
* Example usage:
|
||||
*
|
||||
* <pre>{@code
|
||||
* McpSyncClient mcpClient = // obtain MCP client
|
||||
* Tool mcpTool = // obtain MCP tool definition
|
||||
* ToolCallback callback = new McpToolCallback(mcpClient, mcpTool);
|
||||
@@ -88,7 +89,7 @@ public class SyncMcpToolCallback implements ToolCallback {
|
||||
@Override
|
||||
public ToolDefinition getToolDefinition() {
|
||||
return ToolDefinition.builder()
|
||||
.name(mcpClient.getClientInfo().name() + "-" + this.tool.name())
|
||||
.name(McpToolUtils.prefixedToolName(this.mcpClient.getClientInfo().name(), this.tool.name()))
|
||||
.description(this.tool.description())
|
||||
.inputSchema(ModelOptionsUtils.toJsonString(this.tool.inputSchema()))
|
||||
.build();
|
||||
@@ -109,8 +110,12 @@ public class SyncMcpToolCallback implements ToolCallback {
|
||||
@Override
|
||||
public String call(String functionInput) {
|
||||
Map<String, Object> arguments = ModelOptionsUtils.jsonToMap(functionInput);
|
||||
CallToolResult response = this.mcpClient
|
||||
.callTool(new CallToolRequest(this.getToolDefinition().name(), arguments));
|
||||
// Note that we use the original tool name here, not the adapted one from
|
||||
// getToolDefinition
|
||||
CallToolResult response = this.mcpClient.callTool(new CallToolRequest(this.tool.name(), arguments));
|
||||
if (response.isError()) {
|
||||
throw new IllegalStateException("Error calling tool: " + response.content());
|
||||
}
|
||||
return ModelOptionsUtils.toJsonString(response.content());
|
||||
}
|
||||
|
||||
|
||||
@@ -63,7 +63,8 @@ class SyncMcpToolCallbackTests {
|
||||
@Test
|
||||
void callShouldHandleJsonInputAndOutput() {
|
||||
|
||||
when(mcpClient.getClientInfo()).thenReturn(new Implementation("testClient", "1.0.0"));
|
||||
// when(mcpClient.getClientInfo()).thenReturn(new Implementation("testClient",
|
||||
// "1.0.0"));
|
||||
|
||||
when(tool.name()).thenReturn("testTool");
|
||||
CallToolResult callResult = mock(CallToolResult.class);
|
||||
@@ -79,7 +80,8 @@ class SyncMcpToolCallbackTests {
|
||||
|
||||
@Test
|
||||
void callShoulIngroeToolContext() {
|
||||
when(mcpClient.getClientInfo()).thenReturn(new Implementation("testClient", "1.0.0"));
|
||||
// when(mcpClient.getClientInfo()).thenReturn(new Implementation("testClient",
|
||||
// "1.0.0"));
|
||||
|
||||
when(tool.name()).thenReturn("testTool");
|
||||
CallToolResult callResult = mock(CallToolResult.class);
|
||||
|
||||
Reference in New Issue
Block a user