Raise an error when duplicate tool names are registered
This raises an exception when duplicate tool names are registered, instead of silently overwriting tools. Tool names are required to be unique within a server, so this behavior aligns with the MCP specification. > Tool names SHOULD be unique within a server. https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names Validation for tool names could be made more strictly spec-compliant in a separate effort. Fixes #197
This commit is contained in:
parent
171baa876e
commit
dee80d20ae
2 changed files with 74 additions and 1 deletions
|
|
@ -5,6 +5,15 @@ require_relative "instrumentation"
|
||||||
require_relative "methods"
|
require_relative "methods"
|
||||||
|
|
||||||
module MCP
|
module MCP
|
||||||
|
class ToolNotUnique < StandardError
|
||||||
|
def initialize(duplicated_tool_names)
|
||||||
|
super(<<~MESSAGE)
|
||||||
|
Tool names should be unique. Use `tool_name` to assign unique names to:
|
||||||
|
#{duplicated_tool_names.join(", ")}
|
||||||
|
MESSAGE
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
class Server
|
class Server
|
||||||
DEFAULT_VERSION = "0.1.0"
|
DEFAULT_VERSION = "0.1.0"
|
||||||
|
|
||||||
|
|
@ -53,6 +62,7 @@ module MCP
|
||||||
@title = title
|
@title = title
|
||||||
@version = version
|
@version = version
|
||||||
@instructions = instructions
|
@instructions = instructions
|
||||||
|
@tool_names = tools.map(&:name_value)
|
||||||
@tools = tools.to_h { |t| [t.name_value, t] }
|
@tools = tools.to_h { |t| [t.name_value, t] }
|
||||||
@prompts = prompts.to_h { |p| [p.name_value, p] }
|
@prompts = prompts.to_h { |p| [p.name_value, p] }
|
||||||
@resources = resources
|
@resources = resources
|
||||||
|
|
@ -101,7 +111,10 @@ module MCP
|
||||||
|
|
||||||
def define_tool(name: nil, title: nil, description: nil, input_schema: nil, annotations: nil, meta: nil, &block)
|
def define_tool(name: nil, title: nil, description: nil, input_schema: nil, annotations: nil, meta: nil, &block)
|
||||||
tool = Tool.define(name:, title:, description:, input_schema:, annotations:, meta:, &block)
|
tool = Tool.define(name:, title:, description:, input_schema:, annotations:, meta:, &block)
|
||||||
@tools[tool.name_value] = tool
|
tool_name = tool.name_value
|
||||||
|
|
||||||
|
@tool_names << tool_name
|
||||||
|
@tools[tool_name] = tool
|
||||||
|
|
||||||
validate!
|
validate!
|
||||||
end
|
end
|
||||||
|
|
@ -176,6 +189,8 @@ module MCP
|
||||||
private
|
private
|
||||||
|
|
||||||
def validate!
|
def validate!
|
||||||
|
validate_tool_name!
|
||||||
|
|
||||||
# NOTE: The draft protocol version is the next version after 2025-11-25.
|
# NOTE: The draft protocol version is the next version after 2025-11-25.
|
||||||
if @configuration.protocol_version <= "2025-06-18"
|
if @configuration.protocol_version <= "2025-06-18"
|
||||||
if server_info.key?(:description)
|
if server_info.key?(:description)
|
||||||
|
|
@ -216,6 +231,12 @@ module MCP
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def validate_tool_name!
|
||||||
|
duplicated_tool_names = @tool_names.tally.filter_map { |name, count| name if count >= 2 }
|
||||||
|
|
||||||
|
raise ToolNotUnique, duplicated_tool_names unless duplicated_tool_names.empty?
|
||||||
|
end
|
||||||
|
|
||||||
def handle_request(request, method)
|
def handle_request(request, method)
|
||||||
handler = @handlers[method]
|
handler = @handlers[method]
|
||||||
unless handler
|
unless handler
|
||||||
|
|
|
||||||
|
|
@ -374,6 +374,44 @@ module MCP
|
||||||
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
|
assert_instrumentation_data({ method: "tools/call", tool_name: "tool_that_raises" })
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "registers tools with the same class name in different namespaces" do
|
||||||
|
module Foo
|
||||||
|
class Example < Tool
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
module Bar
|
||||||
|
class Example < Tool
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
error = assert_raises(MCP::ToolNotUnique) { Server.new(tools: [Foo::Example, Bar::Example]) }
|
||||||
|
assert_equal(<<~MESSAGE, error.message)
|
||||||
|
Tool names should be unique. Use `tool_name` to assign unique names to:
|
||||||
|
example
|
||||||
|
MESSAGE
|
||||||
|
end
|
||||||
|
|
||||||
|
test "registers tools with the same tool name" do
|
||||||
|
module Baz
|
||||||
|
class Example < Tool
|
||||||
|
tool_name "foo"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
module Qux
|
||||||
|
class Example < Tool
|
||||||
|
tool_name "foo"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
error = assert_raises(MCP::ToolNotUnique) { Server.new(tools: [Baz::Example, Qux::Example]) }
|
||||||
|
assert_equal(<<~MESSAGE, error.message)
|
||||||
|
Tool names should be unique. Use `tool_name` to assign unique names to:
|
||||||
|
foo
|
||||||
|
MESSAGE
|
||||||
|
end
|
||||||
|
|
||||||
test "#handle_json returns error response with isError true if the tool raises an error" do
|
test "#handle_json returns error response with isError true if the tool raises an error" do
|
||||||
request = JSON.generate({
|
request = JSON.generate({
|
||||||
jsonrpc: "2.0",
|
jsonrpc: "2.0",
|
||||||
|
|
@ -950,6 +988,20 @@ module MCP
|
||||||
assert_equal({ content: "success", isError: false }, response[:result])
|
assert_equal({ content: "success", isError: false }, response[:result])
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "#define_tool adds a tool with duplicated tool name to the server" do
|
||||||
|
error = assert_raises(MCP::ToolNotUnique) do
|
||||||
|
@server.define_tool(
|
||||||
|
name: "test_tool", # NOTE: Already registered tool name
|
||||||
|
description: "Defined tool",
|
||||||
|
input_schema: { type: "object", properties: { message: { type: "string" } }, required: ["message"] },
|
||||||
|
meta: { foo: "bar" },
|
||||||
|
) do |message:|
|
||||||
|
Tool::Response.new(message)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
assert_match(/\ATool names should be unique. Use `tool_name` to assign unique names to/, error.message)
|
||||||
|
end
|
||||||
|
|
||||||
test "#define_tool call definition allows tool arguments and server context" do
|
test "#define_tool call definition allows tool arguments and server context" do
|
||||||
@server.server_context = { user_id: "123" }
|
@server.server_context = { user_id: "123" }
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue