From 37c342c57e6bded8896d67b1f6c6caff2fc3cf1e Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Sun, 29 Jun 2025 07:38:38 +0200 Subject: [PATCH 1/3] feat: refactor the tool specifications - Deprecate AsyncToolSpecification in favor of AsyncToolCallSpecification - Deprecate SyncToolSpecification in favor of SyncToolCallSpecification - Tool handlers now receive CallToolRequest instead of raw arguments to enable access to additional metadata parameters like progressToken - Add toolCall() and toolCalls() builder methods alongside deprecated variants - Update addTool() methods to support new tool call specifications - Maintain backward compatibility through deprecation annotations - Add tests for new tool call functionality This change allows tool handlers to process additional metadata parameters from the full CallToolRequest object, enabling features like progress tracking. Signed-off-by: Christian Tzolov --- .../server/AbstractMcpAsyncServerTests.java | 16 ++ .../server/AbstractMcpSyncServerTests.java | 16 ++ .../server/McpAsyncServer.java | 34 ++- .../server/McpServer.java | 141 +++++++++- .../server/McpServerFeatures.java | 257 +++++++++++++----- .../server/McpSyncServer.java | 13 +- .../server/AbstractMcpAsyncServerTests.java | 38 +++ .../server/AbstractMcpSyncServerTests.java | 35 +++ 8 files changed, 457 insertions(+), 93 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index 12827f469..83ee30414 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -102,6 +102,7 @@ void testImmediateClose() { """; @Test + @Deprecated void testAddTool() { Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) @@ -116,6 +117,21 @@ void testAddTool() { assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } + @Test + void testAddToolCall() { + Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); + var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .build(); + + StepVerifier.create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolCallSpecification(newTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyComplete(); + + assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); + } + @Test void testAddDuplicateTool() { Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index eefcdf9a3..3a3abf9ee 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -109,6 +109,7 @@ void testGetAsyncServer() { """; @Test + @Deprecated void testAddTool() { var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) .serverInfo("test-server", "1.0.0") @@ -123,6 +124,21 @@ void testAddTool() { assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } + @Test + void testAddToolCall() { + var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .build(); + + Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); + assertThatCode(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolCallSpecification(newTool, + (exchange, request) -> new CallToolResult(List.of(), false)))) + .doesNotThrowAnyException(); + + assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); + } + @Test void testAddDuplicateTool() { Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java index 02ad955b9..1b0d3ff39 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java @@ -92,7 +92,7 @@ public class McpAsyncServer { private final String instructions; - private final CopyOnWriteArrayList tools = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList tools = new CopyOnWriteArrayList<>(); private final CopyOnWriteArrayList resourceTemplates = new CopyOnWriteArrayList<>(); @@ -271,15 +271,27 @@ private McpServerSession.NotificationHandler asyncRootsListChangedNotificationHa * Add a new tool specification at runtime. * @param toolSpecification The tool specification to add * @return Mono that completes when clients have been notified of the change + * @deprecated Use {@link #addTool(McpServerFeatures.AsyncToolCallSpecification)} + * instead. */ + @Deprecated public Mono addTool(McpServerFeatures.AsyncToolSpecification toolSpecification) { - if (toolSpecification == null) { + return addTool(toolSpecification.toToolCall()); + } + + /** + * Add a new tool call specification at runtime. + * @param toolCallSpecification The tool specification to add + * @return Mono that completes when clients have been notified of the change + */ + public Mono addTool(McpServerFeatures.AsyncToolCallSpecification toolCallSpecification) { + if (toolCallSpecification == null) { return Mono.error(new McpError("Tool specification must not be null")); } - if (toolSpecification.tool() == null) { + if (toolCallSpecification.tool() == null) { return Mono.error(new McpError("Tool must not be null")); } - if (toolSpecification.call() == null) { + if (toolCallSpecification.call() == null) { return Mono.error(new McpError("Tool call handler must not be null")); } if (this.serverCapabilities.tools() == null) { @@ -288,13 +300,13 @@ public Mono addTool(McpServerFeatures.AsyncToolSpecification toolSpecifica return Mono.defer(() -> { // Check for duplicate tool names - if (this.tools.stream().anyMatch(th -> th.tool().name().equals(toolSpecification.tool().name()))) { + if (this.tools.stream().anyMatch(th -> th.tool().name().equals(toolCallSpecification.tool().name()))) { return Mono - .error(new McpError("Tool with name '" + toolSpecification.tool().name() + "' already exists")); + .error(new McpError("Tool with name '" + toolCallSpecification.tool().name() + "' already exists")); } - this.tools.add(toolSpecification); - logger.debug("Added tool handler: {}", toolSpecification.tool().name()); + this.tools.add(toolCallSpecification); + logger.debug("Added tool handler: {}", toolCallSpecification.tool().name()); if (this.serverCapabilities.tools().listChanged()) { return notifyToolsListChanged(); @@ -340,7 +352,7 @@ public Mono notifyToolsListChanged() { private McpServerSession.RequestHandler toolsListRequestHandler() { return (exchange, params) -> { - List tools = this.tools.stream().map(McpServerFeatures.AsyncToolSpecification::tool).toList(); + List tools = this.tools.stream().map(McpServerFeatures.AsyncToolCallSpecification::tool).toList(); return Mono.just(new McpSchema.ListToolsResult(tools, null)); }; @@ -352,7 +364,7 @@ private McpServerSession.RequestHandler toolsCallRequestHandler( new TypeReference() { }); - Optional toolSpecification = this.tools.stream() + Optional toolSpecification = this.tools.stream() .filter(tr -> callToolRequest.name().equals(tr.tool().name())) .findAny(); @@ -360,7 +372,7 @@ private McpServerSession.RequestHandler toolsCallRequestHandler( return Mono.error(new McpError("Tool not found: " + callToolRequest.name())); } - return toolSpecification.map(tool -> tool.call().apply(exchange, callToolRequest.arguments())) + return toolSpecification.map(tool -> tool.call().apply(exchange, callToolRequest)) .orElse(Mono.error(new McpError("Tool not found: " + callToolRequest.name()))); }; } diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java index d6ec2cc30..ec615c78a 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java @@ -175,7 +175,7 @@ class AsyncSpecification { * Each tool is uniquely identified by a name and includes metadata describing its * schema. */ - private final List tools = new ArrayList<>(); + private final List tools = new ArrayList<>(); /** * The Model Context Protocol (MCP) provides a standardized way for servers to @@ -321,13 +321,40 @@ public AsyncSpecification capabilities(McpSchema.ServerCapabilities serverCapabi * map of arguments passed to the tool. * @return This builder instance for method chaining * @throws IllegalArgumentException if tool or handler is null + * @deprecated Use {@link #toolCall(McpSchema.Tool, BiFunction)} instead for tool + * calls that require a request object. */ + @Deprecated public AsyncSpecification tool(McpSchema.Tool tool, BiFunction, Mono> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - this.tools.add(new McpServerFeatures.AsyncToolSpecification(tool, handler)); + this.tools.add(new McpServerFeatures.AsyncToolSpecification(tool, handler).toToolCall()); + + return this; + } + + /** + * Adds a single tool with its implementation handler to the server. This is a + * convenience method for registering individual tools without creating a + * {@link McpServerFeatures.AsyncToolCallSpecification} explicitly. + * @param tool The tool definition including name, description, and schema. Must + * not be null. + * @param handler The function that implements the tool's logic. Must not be null. + * The function's first argument is an {@link McpAsyncServerExchange} upon which + * the server can interact with the connected client. The second argument is the + * {@link McpSchema.CallToolRequest} object containing the tool call + * @return This builder instance for method chaining + * @throws IllegalArgumentException if tool or handler is null + */ + public AsyncSpecification toolCall(McpSchema.Tool tool, + BiFunction> handler) { + + Assert.notNull(tool, "Tool must not be null"); + Assert.notNull(handler, "Handler must not be null"); + + this.tools.add(new McpServerFeatures.AsyncToolCallSpecification(tool, handler)); return this; } @@ -341,10 +368,29 @@ public AsyncSpecification tool(McpSchema.Tool tool, * @return This builder instance for method chaining * @throws IllegalArgumentException if toolSpecifications is null * @see #tools(McpServerFeatures.AsyncToolSpecification...) + * @deprecated Use {@link #toolCalls(List)} instead for adding multiple tool + * calls. */ + @Deprecated public AsyncSpecification tools(List toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); - this.tools.addAll(toolSpecifications); + this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList()); + return this; + } + + /** + * Adds multiple tools with their handlers to the server using a List. This method + * is useful when tools are dynamically generated or loaded from a configuration + * source. + * @param toolCallSpecifications The list of tool specifications to add. Must not + * be null. + * @return This builder instance for method chaining + * @throws IllegalArgumentException if toolSpecifications is null + * @see #tools(McpServerFeatures.AsyncToolCallSpecification...) + */ + public AsyncSpecification toolCalls(List toolCallSpecifications) { + Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); + this.tools.addAll(toolCallSpecifications); return this; } @@ -363,11 +409,28 @@ public AsyncSpecification tools(List t * @param toolSpecifications The tool specifications to add. Must not be null. * @return This builder instance for method chaining * @throws IllegalArgumentException if toolSpecifications is null - * @see #tools(List) + * @deprecated Use + * {@link #toolCalls(McpServerFeatures.AsyncToolCallSpecification...)} instead */ + @Deprecated public AsyncSpecification tools(McpServerFeatures.AsyncToolSpecification... toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); for (McpServerFeatures.AsyncToolSpecification tool : toolSpecifications) { + this.tools.add(tool.toToolCall()); + } + return this; + } + + /** + * Adds multiple tools with their handlers to the server using varargs. This + * method provides a convenient way to register multiple tools inline. + * @param toolSpecifications The tool specifications to add. Must not be null. + * @return This builder instance for method chaining + * @throws IllegalArgumentException if toolSpecifications is null + */ + public AsyncSpecification toolCalls(McpServerFeatures.AsyncToolCallSpecification... toolCallSpecifications) { + Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); + for (McpServerFeatures.AsyncToolCallSpecification tool : toolCallSpecifications) { this.tools.add(tool); } return this; @@ -667,7 +730,7 @@ class SyncSpecification { * Each tool is uniquely identified by a name and includes metadata describing its * schema. */ - private final List tools = new ArrayList<>(); + private final List tools = new ArrayList<>(); /** * The Model Context Protocol (MCP) provides a standardized way for servers to @@ -812,13 +875,39 @@ public SyncSpecification capabilities(McpSchema.ServerCapabilities serverCapabil * list of arguments passed to the tool. * @return This builder instance for method chaining * @throws IllegalArgumentException if tool or handler is null + * @deprecated Use {@link #toolCall(McpSchema.Tool, BiFunction)} instead for tool + * calls that require a request object. */ + @Deprecated public SyncSpecification tool(McpSchema.Tool tool, BiFunction, McpSchema.CallToolResult> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - this.tools.add(new McpServerFeatures.SyncToolSpecification(tool, handler)); + this.tools.add(new McpServerFeatures.SyncToolSpecification(tool, handler).toToolCall()); + + return this; + } + + /** + * Adds a single tool with its implementation handler to the server. This is a + * convenience method for registering individual tools without creating a + * {@link McpServerFeatures.SyncToolSpecification} explicitly. + * @param tool The tool definition including name, description, and schema. Must + * not be null. + * @param handler The function that implements the tool's logic. Must not be null. + * The function's first argument is an {@link McpSyncServerExchange} upon which + * the server can interact with the connected client. The second argument is the + * list of arguments passed to the tool. + * @return This builder instance for method chaining + * @throws IllegalArgumentException if tool or handler is null + */ + public SyncSpecification toolCall(McpSchema.Tool tool, + BiFunction handler) { + Assert.notNull(tool, "Tool must not be null"); + Assert.notNull(handler, "Handler must not be null"); + + this.tools.add(new McpServerFeatures.SyncToolCallSpecification(tool, handler)); return this; } @@ -832,10 +921,28 @@ public SyncSpecification tool(McpSchema.Tool tool, * @return This builder instance for method chaining * @throws IllegalArgumentException if toolSpecifications is null * @see #tools(McpServerFeatures.SyncToolSpecification...) + * @deprecated Use {@link #toolCalls(List)} instead for adding multiple tool + * calls. */ + @Deprecated public SyncSpecification tools(List toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); - this.tools.addAll(toolSpecifications); + this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList()); + return this; + } + + /** + * Adds multiple tools with their handlers to the server using a List. This method + * is useful when tools are dynamically generated or loaded from a configuration + * source. + * @param toolSpecifications The list of tool specifications to add. Must not be + * null. + * @return This builder instance for method chaining + * @throws IllegalArgumentException if toolSpecifications is null + */ + public SyncSpecification toolCalls(List toolCallSpecifications) { + Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); + this.tools.addAll(toolCallSpecifications); return this; } @@ -855,10 +962,30 @@ public SyncSpecification tools(List too * @return This builder instance for method chaining * @throws IllegalArgumentException if toolSpecifications is null * @see #tools(List) + * @deprecated Use + * {@link #toolCalls(McpServerFeatures.SyncToolCallSpecification...)} instead for + * tool calls that require a request object. */ + @Deprecated public SyncSpecification tools(McpServerFeatures.SyncToolSpecification... toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); for (McpServerFeatures.SyncToolSpecification tool : toolSpecifications) { + this.tools.add(tool.toToolCall()); + } + return this; + } + + /** + * Adds multiple tools with their handlers to the server using varargs. This + * method provides a convenient way to register multiple tools inline. + * @param toolSpecifications The tool specifications to add. Must not be null. + * @return This builder instance for method chaining + * @throws IllegalArgumentException if toolSpecifications is null + * @see #tools(List) + */ + public SyncSpecification toolCalls(McpServerFeatures.SyncToolCallSpecification... toolCallSpecifications) { + Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); + for (McpServerFeatures.SyncToolCallSpecification tool : toolCallSpecifications) { this.tools.add(tool); } return this; diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpServerFeatures.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpServerFeatures.java index 8311f5d41..3d9faba9d 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpServerFeatures.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpServerFeatures.java @@ -39,7 +39,7 @@ public class McpServerFeatures { * @param instructions The server instructions text */ record Async(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities serverCapabilities, - List tools, Map resources, + List tools, Map resources, List resourceTemplates, Map prompts, Map completions, @@ -59,8 +59,8 @@ record Async(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities s * @param instructions The server instructions text */ Async(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities serverCapabilities, - List tools, Map resources, - List resourceTemplates, + List tools, + Map resources, List resourceTemplates, Map prompts, Map completions, List, Mono>> rootsChangeConsumers, @@ -99,9 +99,9 @@ record Async(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities s * user. */ static Async fromSync(Sync syncSpec) { - List tools = new ArrayList<>(); + List tools = new ArrayList<>(); for (var tool : syncSpec.tools()) { - tools.add(AsyncToolSpecification.fromSync(tool)); + tools.add(AsyncToolCallSpecification.fromSync(tool)); } Map resources = new HashMap<>(); @@ -146,7 +146,7 @@ static Async fromSync(Sync syncSpec) { * @param instructions The server instructions text */ record Sync(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities serverCapabilities, - List tools, + List tools, Map resources, List resourceTemplates, Map prompts, @@ -166,7 +166,7 @@ record Sync(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities se * @param instructions The server instructions text */ Sync(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities serverCapabilities, - List tools, + List tools, Map resources, List resourceTemplates, Map prompts, @@ -213,21 +213,21 @@ record Sync(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities se * * *

- * Example tool specification:

{@code
+	 * Example tool specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.AsyncToolSpecification(
-	 *     new Tool(
-	 *         "calculator",
-	 *         "Performs mathematical calculations",
-	 *         new JsonSchemaObject()
-	 *             .required("expression")
-	 *             .property("expression", JsonSchemaType.STRING)
-	 *     ),
-	 *     (exchange, args) -> {
-	 *         String expr = (String) args.get("expression");
-	 *         return Mono.fromSupplier(() -> evaluate(expr))
-	 *             .map(result -> new CallToolResult("Result: " + result));
-	 *     }
-	 * )
+	 * 		new Tool(
+	 * 				"calculator",
+	 * 				"Performs mathematical calculations",
+	 * 				new JsonSchemaObject()
+	 * 						.required("expression")
+	 * 						.property("expression", JsonSchemaType.STRING)),
+	 * 		(exchange, args) -> {
+	 * 			String expr = (String) args.get("expression");
+	 * 			return Mono.fromSupplier(() -> evaluate(expr))
+	 * 					.map(result -> new CallToolResult("Result: " + result));
+	 * 		})
 	 * }
* * @param tool The tool definition including name, description, and parameter schema @@ -235,10 +235,17 @@ record Sync(McpSchema.Implementation serverInfo, McpSchema.ServerCapabilities se * returning results. The function's first argument is an * {@link McpAsyncServerExchange} upon which the server can interact with the * connected client. The second arguments is a map of tool arguments. + * @deprecated Use {@link AsyncToolCallSpecification} instead. */ + @Deprecated public record AsyncToolSpecification(McpSchema.Tool tool, BiFunction, Mono> call) { + public AsyncToolCallSpecification toToolCall() { + return new AsyncToolCallSpecification(tool, (exchange, toolReq) -> call.apply(exchange, toolReq.arguments()) + .subscribeOn(Schedulers.boundedElastic())); + } + static AsyncToolSpecification fromSync(SyncToolSpecification tool) { // FIXME: This is temporary, proper validation should be implemented if (tool == null) { @@ -251,6 +258,81 @@ static AsyncToolSpecification fromSync(SyncToolSpecification tool) { } } + /** + * Specification of a tool with its asynchronous handler function. Tools are the + * primary way for MCP servers to expose functionality to AI models. Each tool + * represents a specific capability, such as: + *
    + *
  • Performing calculations + *
  • Accessing external APIs + *
  • Querying databases + *
  • Manipulating files + *
  • Executing system commands + *
+ * + *

+ * Example tool specification: + * + *

{@code
+	 * new McpServerFeatures.AsyncToolCallSpecification(
+	 * 		new Tool(
+	 * 				"calculator",
+	 * 				"Performs mathematical calculations",
+	 * 				new JsonSchemaObject()
+	 * 						.required("expression")
+	 * 						.property("expression", JsonSchemaType.STRING)),
+	 * 		(exchange, request) -> Mono.fromSupplier(() -> evaluate(request.getArguments().get("expression")))
+	 * 				.map(result -> new CallToolResult("Result: " + result)))
+	 * }
+ * + * @param tool The tool definition including name, description, and parameter schema + * @param call The function that implements the tool's logic, receiving arguments and + * returning results. + */ + public record AsyncToolCallSpecification(McpSchema.Tool tool, + BiFunction> call) { + + /** + * Converts a synchronous {@link SyncToolCallSpecification} into an + * {@link AsyncToolCallSpecification} by wrapping the handler in a bounded elastic + * scheduler for safe non-blocking execution. + * @param tool the synchronous tool call specification + * @return an asynchronous wrapper of the provided sync specification, or + * {@code null} if input is null + */ + static AsyncToolCallSpecification fromSync(SyncToolCallSpecification tool) { + if (tool == null) { + return null; + } + return new AsyncToolCallSpecification(tool.tool(), + (exchange, callToolRequest) -> Mono + .fromCallable(() -> tool.call().apply(new McpSyncServerExchange(exchange), callToolRequest)) + .subscribeOn(Schedulers.boundedElastic())); + } + + /** + * Converts a synchronous {@link SyncToolSpecification} into an + * {@link AsyncToolCallSpecification} by wrapping the handler in a bounded elastic + * scheduler for safe non-blocking execution. + * @param tool the synchronous tool specification + * @deprecated the {@link SyncToolSpecification} is deprecated, use + * {@link SyncToolCallSpecification} instead. + * @return an asynchronous wrapper of the provided sync specification, or + * {@code null} if input is null + */ + @Deprecated + static AsyncToolCallSpecification fromSync(SyncToolSpecification tool) { + // FIXME: This is temporary, proper validation should be implemented + if (tool == null) { + return null; + } + return new AsyncToolCallSpecification(tool.tool(), + (exchange, callToolRequest) -> Mono.fromCallable( + () -> tool.call().apply(new McpSyncServerExchange(exchange), callToolRequest.arguments())) + .subscribeOn(Schedulers.boundedElastic())); + } + } + /** * Specification of a resource with its asynchronous handler function. Resources * provide context to AI models by exposing data such as: @@ -263,13 +345,13 @@ static AsyncToolSpecification fromSync(SyncToolSpecification tool) { * * *

- * Example resource specification:

{@code
+	 * Example resource specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.AsyncResourceSpecification(
-	 *     new Resource("docs", "Documentation files", "text/markdown"),
-	 *     (exchange, request) ->
-	 *         Mono.fromSupplier(() -> readFile(request.getPath()))
-	 *             .map(ReadResourceResult::new)
-	 * )
+	 * 		new Resource("docs", "Documentation files", "text/markdown"),
+	 * 		(exchange, request) -> Mono.fromSupplier(() -> readFile(request.getPath()))
+	 * 				.map(ReadResourceResult::new))
 	 * }
* * @param resource The resource definition including name, description, and MIME type @@ -305,16 +387,16 @@ static AsyncResourceSpecification fromSync(SyncResourceSpecification resource) { * * *

- * Example prompt specification:

{@code
+	 * Example prompt specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.AsyncPromptSpecification(
-	 *     new Prompt("analyze", "Code analysis template"),
-	 *     (exchange, request) -> {
-	 *         String code = request.getArguments().get("code");
-	 *         return Mono.just(new GetPromptResult(
-	 *             "Analyze this code:\n\n" + code + "\n\nProvide feedback on:"
-	 *         ));
-	 *     }
-	 * )
+	 * 		new Prompt("analyze", "Code analysis template"),
+	 * 		(exchange, request) -> {
+	 * 			String code = request.getArguments().get("code");
+	 * 			return Mono.just(new GetPromptResult(
+	 * 					"Analyze this code:\n\n" + code + "\n\nProvide feedback on:"));
+	 * 		})
 	 * }
* * @param prompt The prompt definition including name and description @@ -379,31 +461,23 @@ static AsyncCompletionSpecification fromSync(SyncCompletionSpecification complet /** * Specification of a tool with its synchronous handler function. Tools are the - * primary way for MCP servers to expose functionality to AI models. Each tool - * represents a specific capability, such as: - *
    - *
  • Performing calculations - *
  • Accessing external APIs - *
  • Querying databases - *
  • Manipulating files - *
  • Executing system commands - *
+ * primary way for MCP servers to expose functionality to AI models. * *

- * Example tool specification:

{@code
+	 * Example tool specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.SyncToolSpecification(
-	 *     new Tool(
-	 *         "calculator",
-	 *         "Performs mathematical calculations",
-	 *         new JsonSchemaObject()
-	 *             .required("expression")
-	 *             .property("expression", JsonSchemaType.STRING)
-	 *     ),
-	 *     (exchange, args) -> {
-	 *         String expr = (String) args.get("expression");
-	 *         return new CallToolResult("Result: " + evaluate(expr));
-	 *     }
-	 * )
+	 * 		new Tool(
+	 * 				"calculator",
+	 * 				"Performs mathematical calculations",
+	 * 				new JsonSchemaObject()
+	 * 						.required("expression")
+	 * 						.property("expression", JsonSchemaType.STRING)),
+	 * 		(exchange, args) -> {
+	 * 			String expr = (String) args.get("expression");
+	 * 			return new CallToolResult("Result: " + evaluate(expr));
+	 * 		})
 	 * }
* * @param tool The tool definition including name, description, and parameter schema @@ -411,9 +485,43 @@ static AsyncCompletionSpecification fromSync(SyncCompletionSpecification complet * returning results. The function's first argument is an * {@link McpSyncServerExchange} upon which the server can interact with the connected * client. The second arguments is a map of arguments passed to the tool. + * @deprecated Use {@link SyncToolCallSpecification} instead. */ + @Deprecated public record SyncToolSpecification(McpSchema.Tool tool, BiFunction, McpSchema.CallToolResult> call) { + + public SyncToolCallSpecification toToolCall() { + return new SyncToolCallSpecification(tool, + (exchange, toolReq) -> call.apply(exchange, toolReq.arguments())); + } + } + + /** + * Specification of a tool with its synchronous handler function. Tools are the + * primary way for MCP servers to expose functionality to AI models. + * + *

+ * Example tool specification: + * + *

{@code
+	 * new McpServerFeatures.SyncToolCallSpecification(
+	 * 		new Tool(
+	 * 				"calculator",
+	 * 				"Performs mathematical calculations",
+	 * 				new JsonSchemaObject()
+	 * 						.required("expression")
+	 * 						.property("expression", JsonSchemaType.STRING)),
+	 * 		(exchange,
+	 * 				request) -> new CallToolResult("Result: " + evaluate(request.getArguments().get("expression"))))
+	 * }
+ * + * @param tool The tool definition including name, description, and parameter schema + * @param call The function that implements the tool's logic, receiving arguments and + * returning results. + */ + public record SyncToolCallSpecification(McpSchema.Tool tool, + BiFunction call) { } /** @@ -428,14 +536,15 @@ public record SyncToolSpecification(McpSchema.Tool tool, * * *

- * Example resource specification:

{@code
+	 * Example resource specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.SyncResourceSpecification(
-	 *     new Resource("docs", "Documentation files", "text/markdown"),
-	 *     (exchange, request) -> {
-	 *         String content = readFile(request.getPath());
-	 *         return new ReadResourceResult(content);
-	 *     }
-	 * )
+	 * 		new Resource("docs", "Documentation files", "text/markdown"),
+	 * 		(exchange, request) -> {
+	 * 			String content = readFile(request.getPath());
+	 * 			return new ReadResourceResult(content);
+	 * 		})
 	 * }
* * @param resource The resource definition including name, description, and MIME type @@ -460,16 +569,16 @@ public record SyncResourceSpecification(McpSchema.Resource resource, * * *

- * Example prompt specification:

{@code
+	 * Example prompt specification:
+	 *
+	 * 
{@code
 	 * new McpServerFeatures.SyncPromptSpecification(
-	 *     new Prompt("analyze", "Code analysis template"),
-	 *     (exchange, request) -> {
-	 *         String code = request.getArguments().get("code");
-	 *         return new GetPromptResult(
-	 *             "Analyze this code:\n\n" + code + "\n\nProvide feedback on:"
-	 *         );
-	 *     }
-	 * )
+	 * 		new Prompt("analyze", "Code analysis template"),
+	 * 		(exchange, request) -> {
+	 * 			String code = request.getArguments().get("code");
+	 * 			return new GetPromptResult(
+	 * 					"Analyze this code:\n\n" + code + "\n\nProvide feedback on:");
+	 * 		})
 	 * }
* * @param prompt The prompt definition including name and description diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java index 91f8d9e4c..bd87ce47b 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpSyncServer.java @@ -66,9 +66,20 @@ public McpSyncServer(McpAsyncServer asyncServer) { /** * Add a new tool handler. * @param toolHandler The tool handler to add + * @deprecated Use {@link #addTool(McpServerFeatures.SyncToolCallSpecification)} + * instead. */ + @Deprecated public void addTool(McpServerFeatures.SyncToolSpecification toolHandler) { - this.asyncServer.addTool(McpServerFeatures.AsyncToolSpecification.fromSync(toolHandler)).block(); + this.addTool(toolHandler.toToolCall()); + } + + /** + * Add a new tool handler. + * @param toolHandler The tool handler to add + */ + public void addTool(McpServerFeatures.SyncToolCallSpecification toolHandler) { + this.asyncServer.addTool(McpServerFeatures.AsyncToolCallSpecification.fromSync(toolHandler)).block(); } /** diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index dd9f65895..39b5636e5 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -101,6 +101,7 @@ void testImmediateClose() { """; @Test + @Deprecated void testAddTool() { Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) @@ -116,6 +117,22 @@ void testAddTool() { } @Test + void testAddToolCall() { + Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); + var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .build(); + + StepVerifier.create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolCallSpecification(newTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyComplete(); + + assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); + } + + @Test + @Deprecated void testAddDuplicateTool() { Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); @@ -136,6 +153,27 @@ void testAddDuplicateTool() { assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } + @Test + void testAddDuplicateToolCall() { + Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); + + var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) + .build(); + + StepVerifier + .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyErrorSatisfies(error -> { + assertThat(error).isInstanceOf(McpError.class) + .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + }); + + assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); + } + @Test void testRemoveTool() { Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 6cbb8632c..827949d53 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -108,6 +108,7 @@ void testGetAsyncServer() { """; @Test + @Deprecated void testAddTool() { var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) .serverInfo("test-server", "1.0.0") @@ -123,6 +124,22 @@ void testAddTool() { } @Test + void testAddToolCall() { + var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .build(); + + Tool newTool = new McpSchema.Tool("new-tool", "New test tool", emptyJsonSchema); + assertThatCode(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolCallSpecification(newTool, + (exchange, request) -> new CallToolResult(List.of(), false)))) + .doesNotThrowAnyException(); + + assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); + } + + @Test + @Deprecated void testAddDuplicateTool() { Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); @@ -140,6 +157,24 @@ void testAddDuplicateTool() { assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } + @Test + void testAddDuplicateToolCall() { + Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); + + var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) + .build(); + + assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)))) + .isInstanceOf(McpError.class) + .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + + assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); + } + @Test void testRemoveTool() { Tool tool = new McpSchema.Tool(TEST_TOOL_NAME, "Test tool", emptyJsonSchema); From 3a1a1b41fa40a9151177b582e428c74e8f136e85 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Tue, 1 Jul 2025 14:43:36 +0200 Subject: [PATCH 2/3] feat: add duplicate tool name validation for MCP server registration - Add AssertNotDupplicateTool validation method to prevent duplicate tool registration - Throw IllegalArgumentException during server building when duplicate tool names detected - Throw McpError when attempting to add duplicate tools to existing server - Add test coverage for duplicate detection scenarios: - Individual tool registration during building - Batch tool registration (list and varargs) - Runtime duplicate tool addition - Apply validation to both sync and async server implementations This prevents tool name conflicts that could cause unexpected behavior or runtime errors in MCP server implementations. Signed-off-by: Christian Tzolov --- .../server/AbstractMcpAsyncServerTests.java | 70 +++++++++++++++++++ .../server/AbstractMcpSyncServerTests.java | 68 ++++++++++++++++++ .../server/McpServer.java | 52 +++++++++++++- .../server/AbstractMcpAsyncServerTests.java | 49 +++++++++++++ .../server/AbstractMcpSyncServerTests.java | 49 +++++++++++++ 5 files changed, 285 insertions(+), 3 deletions(-) diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java index 83ee30414..1c03cf83d 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java @@ -153,6 +153,76 @@ void testAddDuplicateTool() { assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } + @Test + void testAddDuplicateToolCall() { + Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); + + var mcpAsyncServer = McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) + .build(); + + StepVerifier + .create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))))) + .verifyErrorSatisfies(error -> { + assertThat(error).isInstanceOf(McpError.class) + .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + }); + + assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); + } + + @Test + void testDuplicateToolCallDuringBuilding() { + Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building", + emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'duplicate-build-toolcall' is already registered."); + } + + @Test + void testDuplicateToolsInBatchListRegistration() { + Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema); + List specs = List.of( + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))), + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + ); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls(specs) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-list-tool' is already registered."); + } + + @Test + void testDuplicateToolsInBatchVarargsRegistration() { + Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls( + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))), + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + ) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-varargs-tool' is already registered."); + } + @Test void testRemoveTool() { Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); diff --git a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 3a3abf9ee..eed358350 100644 --- a/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -140,6 +140,7 @@ void testAddToolCall() { } @Test + @Deprecated void testAddDuplicateTool() { Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); @@ -157,6 +158,73 @@ void testAddDuplicateTool() { assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } + @Test + void testAddDuplicateToolCall() { + Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); + + var mcpSyncServer = McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) + .build(); + + assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)))) + .isInstanceOf(McpError.class) + .hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists"); + + assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); + } + + @Test + void testDuplicateToolCallDuringBuilding() { + Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building", + emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'duplicate-build-toolcall' is already registered."); + } + + @Test + void testDuplicateToolsInBatchListRegistration() { + Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema); + List specs = List.of( + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)), + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + ); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls(specs) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-list-tool' is already registered."); + } + + @Test + void testDuplicateToolsInBatchVarargsRegistration() { + Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls( + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)), + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + ) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-varargs-tool' is already registered."); + } + @Test void testRemoveTool() { Tool tool = new McpSchema.Tool(TEST_TOOL_NAME, "Test tool", emptyJsonSchema); diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java index ec615c78a..5c00d2af5 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java @@ -208,6 +208,12 @@ private AsyncSpecification(McpServerTransportProvider transportProvider) { this.transportProvider = transportProvider; } + private void AssertNotDupplicateTool(String toolName) { + if (this.tools.stream().anyMatch(toolSpec -> toolSpec.tool().name().equals(toolName))) { + throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); + } + } + /** * Sets the URI template manager factory to use for creating URI templates. This * allows for custom URI template parsing and variable extraction. @@ -329,6 +335,7 @@ public AsyncSpecification tool(McpSchema.Tool tool, BiFunction, Mono> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); + AssertNotDupplicateTool(tool.name()); this.tools.add(new McpServerFeatures.AsyncToolSpecification(tool, handler).toToolCall()); @@ -353,6 +360,7 @@ public AsyncSpecification toolCall(McpSchema.Tool tool, Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); + AssertNotDupplicateTool(tool.name()); this.tools.add(new McpServerFeatures.AsyncToolCallSpecification(tool, handler)); @@ -374,6 +382,12 @@ public AsyncSpecification toolCall(McpSchema.Tool tool, @Deprecated public AsyncSpecification tools(List toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); + + for (var tool : toolSpecifications) { + AssertNotDupplicateTool(tool.tool().name()); + this.tools.add(tool.toToolCall()); + } + this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList()); return this; } @@ -390,7 +404,11 @@ public AsyncSpecification tools(List t */ public AsyncSpecification toolCalls(List toolCallSpecifications) { Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); - this.tools.addAll(toolCallSpecifications); + + for (var tool : toolCallSpecifications) { + AssertNotDupplicateTool(tool.tool().name()); + this.tools.add(tool); + } return this; } @@ -415,7 +433,9 @@ public AsyncSpecification toolCalls(List toolSpec.tool().name().equals(toolName))) { + throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); + } + } + /** * Sets the URI template manager factory to use for creating URI templates. This * allows for custom URI template parsing and variable extraction. @@ -883,6 +911,7 @@ public SyncSpecification tool(McpSchema.Tool tool, BiFunction, McpSchema.CallToolResult> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); + AssertNotDupplicateTool(tool.name()); this.tools.add(new McpServerFeatures.SyncToolSpecification(tool, handler).toToolCall()); @@ -906,6 +935,7 @@ public SyncSpecification toolCall(McpSchema.Tool tool, BiFunction handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); + AssertNotDupplicateTool(tool.name()); this.tools.add(new McpServerFeatures.SyncToolCallSpecification(tool, handler)); @@ -927,7 +957,14 @@ public SyncSpecification toolCall(McpSchema.Tool tool, @Deprecated public SyncSpecification tools(List toolSpecifications) { Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); - this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList()); + + for (var tool : toolSpecifications) { + String toolName = tool.tool().name(); + AssertNotDupplicateTool(toolName); // Check against existing tools + this.tools.add(tool.toToolCall()); // Add the tool call specification + // directly + } + return this; } @@ -942,7 +979,12 @@ public SyncSpecification tools(List too */ public SyncSpecification toolCalls(List toolCallSpecifications) { Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null"); - this.tools.addAll(toolCallSpecifications); + + for (var tool : toolCallSpecifications) { + AssertNotDupplicateTool(tool.tool().name()); + this.tools.add(tool); + } + return this; } @@ -969,7 +1011,9 @@ public SyncSpecification toolCalls(List mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException(); } + @Test + void testDuplicateToolCallDuringBuilding() { + Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building", + emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) + .toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'duplicate-build-toolcall' is already registered."); + } + + @Test + void testDuplicateToolsInBatchListRegistration() { + Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema); + List specs = List.of( + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))), + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + ); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls(specs) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-list-tool' is already registered."); + } + + @Test + void testDuplicateToolsInBatchVarargsRegistration() { + Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls( + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))), + new McpServerFeatures.AsyncToolCallSpecification(duplicateTool, + (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate! + ) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-varargs-tool' is already registered."); + } + @Test void testRemoveTool() { Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema); diff --git a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java index 827949d53..2e5a674fa 100644 --- a/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java +++ b/mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java @@ -175,6 +175,55 @@ void testAddDuplicateToolCall() { assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException(); } + @Test + void testDuplicateToolCallDuringBuilding() { + Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building", + emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) + .toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'duplicate-build-toolcall' is already registered."); + } + + @Test + void testDuplicateToolsInBatchListRegistration() { + Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema); + List specs = List.of( + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)), + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + ); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls(specs) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-list-tool' is already registered."); + } + + @Test + void testDuplicateToolsInBatchVarargsRegistration() { + Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema); + + assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider()) + .serverInfo("test-server", "1.0.0") + .capabilities(ServerCapabilities.builder().tools(true).build()) + .toolCalls( + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)), + new McpServerFeatures.SyncToolCallSpecification(duplicateTool, + (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate! + ) + .build()).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tool with name 'batch-varargs-tool' is already registered."); + } + @Test void testRemoveTool() { Tool tool = new McpSchema.Tool(TEST_TOOL_NAME, "Test tool", emptyJsonSchema); From 29ca07e9eb7da3b63d735d12b6ae0e13a0df85a1 Mon Sep 17 00:00:00 2001 From: Christian Tzolov Date: Wed, 2 Jul 2025 07:54:36 +0200 Subject: [PATCH 3/3] address review comments Signed-off-by: Christian Tzolov --- .../server/McpServer.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java index 5c00d2af5..e97df208c 100644 --- a/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java +++ b/mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java @@ -208,12 +208,6 @@ private AsyncSpecification(McpServerTransportProvider transportProvider) { this.transportProvider = transportProvider; } - private void AssertNotDupplicateTool(String toolName) { - if (this.tools.stream().anyMatch(toolSpec -> toolSpec.tool().name().equals(toolName))) { - throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); - } - } - /** * Sets the URI template manager factory to use for creating URI templates. This * allows for custom URI template parsing and variable extraction. @@ -335,7 +329,7 @@ public AsyncSpecification tool(McpSchema.Tool tool, BiFunction, Mono> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - AssertNotDupplicateTool(tool.name()); + assertNoDuplicateTool(tool.name()); this.tools.add(new McpServerFeatures.AsyncToolSpecification(tool, handler).toToolCall()); @@ -360,7 +354,7 @@ public AsyncSpecification toolCall(McpSchema.Tool tool, Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - AssertNotDupplicateTool(tool.name()); + assertNoDuplicateTool(tool.name()); this.tools.add(new McpServerFeatures.AsyncToolCallSpecification(tool, handler)); @@ -384,7 +378,7 @@ public AsyncSpecification tools(List t Assert.notNull(toolSpecifications, "Tool handlers list must not be null"); for (var tool : toolSpecifications) { - AssertNotDupplicateTool(tool.tool().name()); + assertNoDuplicateTool(tool.tool().name()); this.tools.add(tool.toToolCall()); } @@ -406,7 +400,7 @@ public AsyncSpecification toolCalls(List toolSpec.tool().name().equals(toolName))) { + throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); + } + } + /** * Registers multiple resources with their handlers using a Map. This method is * useful when resources are dynamically generated or loaded from a configuration @@ -785,12 +785,6 @@ private SyncSpecification(McpServerTransportProvider transportProvider) { this.transportProvider = transportProvider; } - private void AssertNotDupplicateTool(String toolName) { - if (this.tools.stream().anyMatch(toolSpec -> toolSpec.tool().name().equals(toolName))) { - throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); - } - } - /** * Sets the URI template manager factory to use for creating URI templates. This * allows for custom URI template parsing and variable extraction. @@ -911,7 +905,7 @@ public SyncSpecification tool(McpSchema.Tool tool, BiFunction, McpSchema.CallToolResult> handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - AssertNotDupplicateTool(tool.name()); + assertNoDuplicateTool(tool.name()); this.tools.add(new McpServerFeatures.SyncToolSpecification(tool, handler).toToolCall()); @@ -935,7 +929,7 @@ public SyncSpecification toolCall(McpSchema.Tool tool, BiFunction handler) { Assert.notNull(tool, "Tool must not be null"); Assert.notNull(handler, "Handler must not be null"); - AssertNotDupplicateTool(tool.name()); + assertNoDuplicateTool(tool.name()); this.tools.add(new McpServerFeatures.SyncToolCallSpecification(tool, handler)); @@ -960,7 +954,7 @@ public SyncSpecification tools(List too for (var tool : toolSpecifications) { String toolName = tool.tool().name(); - AssertNotDupplicateTool(toolName); // Check against existing tools + assertNoDuplicateTool(toolName); // Check against existing tools this.tools.add(tool.toToolCall()); // Add the tool call specification // directly } @@ -981,7 +975,7 @@ public SyncSpecification toolCalls(List toolSpec.tool().name().equals(toolName))) { + throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered."); + } + } + /** * Registers multiple resources with their handlers using a Map. This method is * useful when resources are dynamically generated or loaded from a configuration