Merge pull request #199 from koic/fix_tool_name_colisions_across_namespaces
Raise an error when duplicate tool names are registered
This commit is contained in:
commit
8d68fc0717
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