Skip to content

Return tool argument validation failures as tool execution errors #333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
koic wants to merge 1 commit into
base: main
Choose a base branch
from
+88 −27
Open

Return tool argument validation failures as tool execution errors #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/mcp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ def call_tool(request, session: nil, related_request_id: nil)
add_instrumentation_data(error: :missing_required_arguments)

missing = tool.input_schema.missing_required_arguments(arguments).join(", ")
raise RequestHandlerError.new("Missing required arguments: #{missing}", request, error_type: :invalid_params)
return error_tool_response("Missing required arguments: #{missing}")
end

if configuration.validate_tool_call_arguments && tool.input_schema
Expand All @@ -542,7 +542,7 @@ def call_tool(request, session: nil, related_request_id: nil)
rescue Tool::InputSchema::ValidationError => e
add_instrumentation_data(error: :invalid_schema)

raise RequestHandlerError.new(e.message, request, error_type: :invalid_params)
return error_tool_response(e.message)
end
end

Expand Down
111 changes: 86 additions & 25 deletions test/mcp/server_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ class ServerTest < ActiveSupport::TestCase
assert_instrumentation_data({ method: "tools/call", tool_name: tool_name, tool_arguments: tool_args })
end

test "#handle tools/call returns protocol error in JSON-RPC format if required tool arguments are missing" do
test "#handle tools/call returns tool execution error if required tool arguments are missing" do
tool_with_required_argument = Tool.define(
name: "test_tool",
title: "Test tool",
Expand All @@ -336,10 +336,10 @@ class ServerTest < ActiveSupport::TestCase

response = server.handle(request)

assert_nil response[:result]
assert_equal(-32602, response[:error][:code])
assert_equal "Invalid params", response[:error][:message]
assert_includes response[:error][:data], "Missing required arguments: message"
assert_nil response[:error]
assert(response[:result][:isError])
assert_equal "text", response[:result][:content][0][:type]
assert_includes response[:result][:content][0][:text], "Missing required arguments: message"
end

test "#handle_json tools/call executes tool and returns result" do
Expand Down Expand Up @@ -1562,11 +1562,10 @@ class Example < Tool
refute response[:result].key?(:instructions)
end

test "tools/call returns protocol error in JSON-RPC format for missing arguments" do
server = Server.new(
tools: [TestTool],
configuration: Configuration.new(validate_tool_call_arguments: true),
)
test "tools/call returns tool execution error for missing arguments" do
configuration = Configuration.new(validate_tool_call_arguments: true)
configuration.instrumentation_callback = instrumentation_helper.callback
server = Server.new(tools: [TestTool], configuration: configuration)

response = server.handle(
{
Expand All @@ -1581,17 +1580,22 @@ class Example < Tool

assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id]
assert_nil response[:result]
assert_equal(-32602, response[:error][:code])
assert_equal "Invalid params", response[:error][:message]
assert_includes response[:error][:data], "Missing required arguments"
assert_nil response[:error]
assert(response[:result][:isError])
assert_equal "text", response[:result][:content][0][:type]
assert_includes response[:result][:content][0][:text], "Missing required arguments"
assert_instrumentation_data({
method: "tools/call",
tool_name: "test_tool",
tool_arguments: {},
error: :missing_required_arguments,
})
end

test "tools/call returns protocol error in JSON-RPC format for invalid arguments when validate_tool_call_arguments is true" do
server = Server.new(
tools: [TestTool],
configuration: Configuration.new(validate_tool_call_arguments: true),
)
test "tools/call returns tool execution error for invalid arguments when validate_tool_call_arguments is true" do
configuration = Configuration.new(validate_tool_call_arguments: true)
configuration.instrumentation_callback = instrumentation_helper.callback
server = Server.new(tools: [TestTool], configuration: configuration)

response = server.handle(
{
Expand All @@ -1607,10 +1611,44 @@ class Example < Tool

assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id]
assert_nil response[:result]
assert_equal(-32602, response[:error][:code])
assert_equal "Invalid params", response[:error][:message]
assert_includes response[:error][:data], "Invalid arguments"
assert_nil response[:error]
assert(response[:result][:isError])
assert_equal "text", response[:result][:content][0][:type]
assert_includes response[:result][:content][0][:text], "Invalid arguments"
assert_instrumentation_data({
method: "tools/call",
tool_name: "test_tool",
tool_arguments: { message: 123 },
error: :invalid_schema,
})
end

test "tools/call returns tool execution error for nested schema validation failure" do
server = Server.new(
tools: [ComplexTypesTool],
configuration: Configuration.new(validate_tool_call_arguments: true),
)

response = server.handle(
{
jsonrpc: "2.0",
id: 1,
method: "tools/call",
params: {
name: "complex_types_tool",
arguments: {
numbers: [1, 2, 3],
strings: ["a", "b", "c"],
objects: [{ name: 123 }],
},
},
},
)

assert_nil response[:error]
assert(response[:result][:isError])
assert_equal "text", response[:result][:content][0][:type]
assert_includes response[:result][:content][0][:text], "Invalid arguments"
end

test "tools/call skips argument validation when validate_tool_call_arguments is false" do
Expand Down Expand Up @@ -1695,7 +1733,7 @@ class Example < Tool
assert_equal "OK", response[:result][:content][0][:content]
end

test "tools/call returns protocol error in JSON-RPC format when additionalProperties set to false" do
test "tools/call returns tool execution error when additionalProperties set to false" do
server = Server.new(
tools: [TestToolWithAdditionalPropertiesSetToFalse],
configuration: Configuration.new(validate_tool_call_arguments: true),
Expand All @@ -1718,10 +1756,33 @@ class Example < Tool

assert_equal "2.0", response[:jsonrpc]
assert_equal 1, response[:id]
assert_nil response[:error]
assert(response[:result][:isError])
assert_equal "text", response[:result][:content][0][:type]
assert_includes response[:result][:content][0][:text], "Invalid arguments"
end

test "tools/call returns JSON-RPC -32602 protocol error when tool is not found" do
server = Server.new(
tools: [TestTool],
)

response = server.handle(
{
jsonrpc: "2.0",
id: 1,
method: "tools/call",
params: {
name: "unknown_tool",
arguments: {},
},
},
)

assert_nil response[:result]
assert_equal(-32602, response[:error][:code])
assert_equal "Invalid params", response[:error][:message]
assert_includes response[:error][:data], "Invalid arguments"
assert_includes response[:error][:data], "Tool not found: unknown_tool"
end

test "#handle completion/complete returns default completion result" do
Expand Down