Skip to content

Commit 3a1a1b4

Browse files
tzolovtzolov
committed
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 <christian.tzolov@broadcom.com>
1 parent 37c342c commit 3a1a1b4

File tree

5 files changed

+285
-3
lines changed

5 files changed

+285
-3
lines changed

‎mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,76 @@ void testAddDuplicateTool() {
153153
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
154154
}
155155

156+
@Test
157+
void testAddDuplicateToolCall() {
158+
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
159+
160+
var mcpAsyncServer = McpServer.async(createMcpTransportProvider())
161+
.serverInfo("test-server", "1.0.0")
162+
.capabilities(ServerCapabilities.builder().tools(true).build())
163+
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
164+
.build();
165+
166+
StepVerifier
167+
.create(mcpAsyncServer.addTool(new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
168+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))))
169+
.verifyErrorSatisfies(error -> {
170+
assertThat(error).isInstanceOf(McpError.class)
171+
.hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists");
172+
});
173+
174+
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
175+
}
176+
177+
@Test
178+
void testDuplicateToolCallDuringBuilding() {
179+
Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building",
180+
emptyJsonSchema);
181+
182+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
183+
.serverInfo("test-server", "1.0.0")
184+
.capabilities(ServerCapabilities.builder().tools(true).build())
185+
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
186+
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
187+
.build()).isInstanceOf(IllegalArgumentException.class)
188+
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
189+
}
190+
191+
@Test
192+
void testDuplicateToolsInBatchListRegistration() {
193+
Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema);
194+
List<McpServerFeatures.AsyncToolCallSpecification> specs = List.of(
195+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
196+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))),
197+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
198+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
199+
);
200+
201+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
202+
.serverInfo("test-server", "1.0.0")
203+
.capabilities(ServerCapabilities.builder().tools(true).build())
204+
.toolCalls(specs)
205+
.build()).isInstanceOf(IllegalArgumentException.class)
206+
.hasMessage("Tool with name 'batch-list-tool' is already registered.");
207+
}
208+
209+
@Test
210+
void testDuplicateToolsInBatchVarargsRegistration() {
211+
Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema);
212+
213+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
214+
.serverInfo("test-server", "1.0.0")
215+
.capabilities(ServerCapabilities.builder().tools(true).build())
216+
.toolCalls(
217+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
218+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))),
219+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
220+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
221+
)
222+
.build()).isInstanceOf(IllegalArgumentException.class)
223+
.hasMessage("Tool with name 'batch-varargs-tool' is already registered.");
224+
}
225+
156226
@Test
157227
void testRemoveTool() {
158228
Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);

‎mcp-test/src/main/java/io/modelcontextprotocol/server/AbstractMcpSyncServerTests.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ void testAddToolCall() {
140140
}
141141

142142
@Test
143+
@Deprecated
143144
void testAddDuplicateTool() {
144145
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
145146

@@ -157,6 +158,73 @@ void testAddDuplicateTool() {
157158
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
158159
}
159160

161+
@Test
162+
void testAddDuplicateToolCall() {
163+
Tool duplicateTool = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);
164+
165+
var mcpSyncServer = McpServer.sync(createMcpTransportProvider())
166+
.serverInfo("test-server", "1.0.0")
167+
.capabilities(ServerCapabilities.builder().tools(true).build())
168+
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
169+
.build();
170+
171+
assertThatThrownBy(() -> mcpSyncServer.addTool(new McpServerFeatures.SyncToolCallSpecification(duplicateTool,
172+
(exchange, request) -> new CallToolResult(List.of(), false))))
173+
.isInstanceOf(McpError.class)
174+
.hasMessage("Tool with name '" + TEST_TOOL_NAME + "' already exists");
175+
176+
assertThatCode(() -> mcpSyncServer.closeGracefully()).doesNotThrowAnyException();
177+
}
178+
179+
@Test
180+
void testDuplicateToolCallDuringBuilding() {
181+
Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building",
182+
emptyJsonSchema);
183+
184+
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
185+
.serverInfo("test-server", "1.0.0")
186+
.capabilities(ServerCapabilities.builder().tools(true).build())
187+
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false))
188+
.toolCall(duplicateTool, (exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate!
189+
.build()).isInstanceOf(IllegalArgumentException.class)
190+
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
191+
}
192+
193+
@Test
194+
void testDuplicateToolsInBatchListRegistration() {
195+
Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema);
196+
List<McpServerFeatures.SyncToolCallSpecification> specs = List.of(
197+
new McpServerFeatures.SyncToolCallSpecification(duplicateTool,
198+
(exchange, request) -> new CallToolResult(List.of(), false)),
199+
new McpServerFeatures.SyncToolCallSpecification(duplicateTool,
200+
(exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate!
201+
);
202+
203+
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
204+
.serverInfo("test-server", "1.0.0")
205+
.capabilities(ServerCapabilities.builder().tools(true).build())
206+
.toolCalls(specs)
207+
.build()).isInstanceOf(IllegalArgumentException.class)
208+
.hasMessage("Tool with name 'batch-list-tool' is already registered.");
209+
}
210+
211+
@Test
212+
void testDuplicateToolsInBatchVarargsRegistration() {
213+
Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema);
214+
215+
assertThatThrownBy(() -> McpServer.sync(createMcpTransportProvider())
216+
.serverInfo("test-server", "1.0.0")
217+
.capabilities(ServerCapabilities.builder().tools(true).build())
218+
.toolCalls(
219+
new McpServerFeatures.SyncToolCallSpecification(duplicateTool,
220+
(exchange, request) -> new CallToolResult(List.of(), false)),
221+
new McpServerFeatures.SyncToolCallSpecification(duplicateTool,
222+
(exchange, request) -> new CallToolResult(List.of(), false)) // Duplicate!
223+
)
224+
.build()).isInstanceOf(IllegalArgumentException.class)
225+
.hasMessage("Tool with name 'batch-varargs-tool' is already registered.");
226+
}
227+
160228
@Test
161229
void testRemoveTool() {
162230
Tool tool = new McpSchema.Tool(TEST_TOOL_NAME, "Test tool", emptyJsonSchema);

‎mcp/src/main/java/io/modelcontextprotocol/server/McpServer.java‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,12 @@ private AsyncSpecification(McpServerTransportProvider transportProvider) {
208208
this.transportProvider = transportProvider;
209209
}
210210

211+
private void AssertNotDupplicateTool(String toolName) {
212+
if (this.tools.stream().anyMatch(toolSpec -> toolSpec.tool().name().equals(toolName))) {
213+
throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered.");
214+
}
215+
}
216+
211217
/**
212218
* Sets the URI template manager factory to use for creating URI templates. This
213219
* allows for custom URI template parsing and variable extraction.
@@ -329,6 +335,7 @@ public AsyncSpecification tool(McpSchema.Tool tool,
329335
BiFunction<McpAsyncServerExchange, Map<String, Object>, Mono<CallToolResult>> handler) {
330336
Assert.notNull(tool, "Tool must not be null");
331337
Assert.notNull(handler, "Handler must not be null");
338+
AssertNotDupplicateTool(tool.name());
332339

333340
this.tools.add(new McpServerFeatures.AsyncToolSpecification(tool, handler).toToolCall());
334341

@@ -353,6 +360,7 @@ public AsyncSpecification toolCall(McpSchema.Tool tool,
353360

354361
Assert.notNull(tool, "Tool must not be null");
355362
Assert.notNull(handler, "Handler must not be null");
363+
AssertNotDupplicateTool(tool.name());
356364

357365
this.tools.add(new McpServerFeatures.AsyncToolCallSpecification(tool, handler));
358366

@@ -374,6 +382,12 @@ public AsyncSpecification toolCall(McpSchema.Tool tool,
374382
@Deprecated
375383
public AsyncSpecification tools(List<McpServerFeatures.AsyncToolSpecification> toolSpecifications) {
376384
Assert.notNull(toolSpecifications, "Tool handlers list must not be null");
385+
386+
for (var tool : toolSpecifications) {
387+
AssertNotDupplicateTool(tool.tool().name());
388+
this.tools.add(tool.toToolCall());
389+
}
390+
377391
this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList());
378392
return this;
379393
}
@@ -390,7 +404,11 @@ public AsyncSpecification tools(List<McpServerFeatures.AsyncToolSpecification> t
390404
*/
391405
public AsyncSpecification toolCalls(List<McpServerFeatures.AsyncToolCallSpecification> toolCallSpecifications) {
392406
Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null");
393-
this.tools.addAll(toolCallSpecifications);
407+
408+
for (var tool : toolCallSpecifications) {
409+
AssertNotDupplicateTool(tool.tool().name());
410+
this.tools.add(tool);
411+
}
394412
return this;
395413
}
396414

@@ -415,7 +433,9 @@ public AsyncSpecification toolCalls(List<McpServerFeatures.AsyncToolCallSpecific
415433
@Deprecated
416434
public AsyncSpecification tools(McpServerFeatures.AsyncToolSpecification... toolSpecifications) {
417435
Assert.notNull(toolSpecifications, "Tool handlers list must not be null");
436+
418437
for (McpServerFeatures.AsyncToolSpecification tool : toolSpecifications) {
438+
AssertNotDupplicateTool(tool.tool().name());
419439
this.tools.add(tool.toToolCall());
420440
}
421441
return this;
@@ -430,7 +450,9 @@ public AsyncSpecification tools(McpServerFeatures.AsyncToolSpecification... tool
430450
*/
431451
public AsyncSpecification toolCalls(McpServerFeatures.AsyncToolCallSpecification... toolCallSpecifications) {
432452
Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null");
453+
433454
for (McpServerFeatures.AsyncToolCallSpecification tool : toolCallSpecifications) {
455+
AssertNotDupplicateTool(tool.tool().name());
434456
this.tools.add(tool);
435457
}
436458
return this;
@@ -763,6 +785,12 @@ private SyncSpecification(McpServerTransportProvider transportProvider) {
763785
this.transportProvider = transportProvider;
764786
}
765787

788+
private void AssertNotDupplicateTool(String toolName) {
789+
if (this.tools.stream().anyMatch(toolSpec -> toolSpec.tool().name().equals(toolName))) {
790+
throw new IllegalArgumentException("Tool with name '" + toolName + "' is already registered.");
791+
}
792+
}
793+
766794
/**
767795
* Sets the URI template manager factory to use for creating URI templates. This
768796
* allows for custom URI template parsing and variable extraction.
@@ -883,6 +911,7 @@ public SyncSpecification tool(McpSchema.Tool tool,
883911
BiFunction<McpSyncServerExchange, Map<String, Object>, McpSchema.CallToolResult> handler) {
884912
Assert.notNull(tool, "Tool must not be null");
885913
Assert.notNull(handler, "Handler must not be null");
914+
AssertNotDupplicateTool(tool.name());
886915

887916
this.tools.add(new McpServerFeatures.SyncToolSpecification(tool, handler).toToolCall());
888917

@@ -906,6 +935,7 @@ public SyncSpecification toolCall(McpSchema.Tool tool,
906935
BiFunction<McpSyncServerExchange, McpSchema.CallToolRequest, McpSchema.CallToolResult> handler) {
907936
Assert.notNull(tool, "Tool must not be null");
908937
Assert.notNull(handler, "Handler must not be null");
938+
AssertNotDupplicateTool(tool.name());
909939

910940
this.tools.add(new McpServerFeatures.SyncToolCallSpecification(tool, handler));
911941

@@ -927,7 +957,14 @@ public SyncSpecification toolCall(McpSchema.Tool tool,
927957
@Deprecated
928958
public SyncSpecification tools(List<McpServerFeatures.SyncToolSpecification> toolSpecifications) {
929959
Assert.notNull(toolSpecifications, "Tool handlers list must not be null");
930-
this.tools.addAll(toolSpecifications.stream().map(s -> s.toToolCall()).toList());
960+
961+
for (var tool : toolSpecifications) {
962+
String toolName = tool.tool().name();
963+
AssertNotDupplicateTool(toolName); // Check against existing tools
964+
this.tools.add(tool.toToolCall()); // Add the tool call specification
965+
// directly
966+
}
967+
931968
return this;
932969
}
933970

@@ -942,7 +979,12 @@ public SyncSpecification tools(List<McpServerFeatures.SyncToolSpecification> too
942979
*/
943980
public SyncSpecification toolCalls(List<McpServerFeatures.SyncToolCallSpecification> toolCallSpecifications) {
944981
Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null");
945-
this.tools.addAll(toolCallSpecifications);
982+
983+
for (var tool : toolCallSpecifications) {
984+
AssertNotDupplicateTool(tool.tool().name());
985+
this.tools.add(tool);
986+
}
987+
946988
return this;
947989
}
948990

@@ -969,7 +1011,9 @@ public SyncSpecification toolCalls(List<McpServerFeatures.SyncToolCallSpecificat
9691011
@Deprecated
9701012
public SyncSpecification tools(McpServerFeatures.SyncToolSpecification... toolSpecifications) {
9711013
Assert.notNull(toolSpecifications, "Tool handlers list must not be null");
1014+
9721015
for (McpServerFeatures.SyncToolSpecification tool : toolSpecifications) {
1016+
AssertNotDupplicateTool(tool.tool().name());
9731017
this.tools.add(tool.toToolCall());
9741018
}
9751019
return this;
@@ -985,7 +1029,9 @@ public SyncSpecification tools(McpServerFeatures.SyncToolSpecification... toolSp
9851029
*/
9861030
public SyncSpecification toolCalls(McpServerFeatures.SyncToolCallSpecification... toolCallSpecifications) {
9871031
Assert.notNull(toolCallSpecifications, "Tool handlers list must not be null");
1032+
9881033
for (McpServerFeatures.SyncToolCallSpecification tool : toolCallSpecifications) {
1034+
AssertNotDupplicateTool(tool.tool().name());
9891035
this.tools.add(tool);
9901036
}
9911037
return this;

‎mcp/src/test/java/io/modelcontextprotocol/server/AbstractMcpAsyncServerTests.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,55 @@ void testAddDuplicateToolCall() {
174174
assertThatCode(() -> mcpAsyncServer.closeGracefully().block(Duration.ofSeconds(10))).doesNotThrowAnyException();
175175
}
176176

177+
@Test
178+
void testDuplicateToolCallDuringBuilding() {
179+
Tool duplicateTool = new Tool("duplicate-build-toolcall", "Duplicate toolcall during building",
180+
emptyJsonSchema);
181+
182+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
183+
.serverInfo("test-server", "1.0.0")
184+
.capabilities(ServerCapabilities.builder().tools(true).build())
185+
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false)))
186+
.toolCall(duplicateTool, (exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
187+
.build()).isInstanceOf(IllegalArgumentException.class)
188+
.hasMessage("Tool with name 'duplicate-build-toolcall' is already registered.");
189+
}
190+
191+
@Test
192+
void testDuplicateToolsInBatchListRegistration() {
193+
Tool duplicateTool = new Tool("batch-list-tool", "Duplicate tool in batch list", emptyJsonSchema);
194+
List<McpServerFeatures.AsyncToolCallSpecification> specs = List.of(
195+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
196+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))),
197+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
198+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
199+
);
200+
201+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
202+
.serverInfo("test-server", "1.0.0")
203+
.capabilities(ServerCapabilities.builder().tools(true).build())
204+
.toolCalls(specs)
205+
.build()).isInstanceOf(IllegalArgumentException.class)
206+
.hasMessage("Tool with name 'batch-list-tool' is already registered.");
207+
}
208+
209+
@Test
210+
void testDuplicateToolsInBatchVarargsRegistration() {
211+
Tool duplicateTool = new Tool("batch-varargs-tool", "Duplicate tool in batch varargs", emptyJsonSchema);
212+
213+
assertThatThrownBy(() -> McpServer.async(createMcpTransportProvider())
214+
.serverInfo("test-server", "1.0.0")
215+
.capabilities(ServerCapabilities.builder().tools(true).build())
216+
.toolCalls(
217+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
218+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))),
219+
new McpServerFeatures.AsyncToolCallSpecification(duplicateTool,
220+
(exchange, request) -> Mono.just(new CallToolResult(List.of(), false))) // Duplicate!
221+
)
222+
.build()).isInstanceOf(IllegalArgumentException.class)
223+
.hasMessage("Tool with name 'batch-varargs-tool' is already registered.");
224+
}
225+
177226
@Test
178227
void testRemoveTool() {
179228
Tool too = new McpSchema.Tool(TEST_TOOL_NAME, "Duplicate tool", emptyJsonSchema);

0 commit comments

Comments
 (0)