diff --git a/CHANGELOG.md b/CHANGELOG.md index c89fb978..c73d845a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 record's `tools` and `instructions` columns. Dashboard-authored agents, which name no class, keep using the dynamic runtime either way; a class name that no longer resolves falls back to it rather than failing the run. +- MCP servers take `allowed_tools` and `require_approval` in the common + format. OpenAI's Responses API receives both as given; Anthropic receives + `allowed_tools` as an `mcp_toolset` entry in `tools` (every other tool of + the server disabled), beside any tools the request already declares + (#328, by @dark-panda). ### Changed diff --git a/lib/active_agent/providers/anthropic/transforms.rb b/lib/active_agent/providers/anthropic/transforms.rb index ff173a8f..32d026b4 100644 --- a/lib/active_agent/providers/anthropic/transforms.rb +++ b/lib/active_agent/providers/anthropic/transforms.rb @@ -38,10 +38,20 @@ def normalize_params(params) end # Handle mcps parameter (common format) -> transforms to mcp_servers (provider format) - if params[:mcps] - params[:mcp_servers] = normalize_mcp_servers(params.delete(:mcps)) - elsif params[:mcp_servers] - params[:mcp_servers] = normalize_mcp_servers(params[:mcp_servers]) + if params[:mcps] || params[:mcp_servers] + mcps = if params[:mcps] + params.delete(:mcps) + else + params[:mcp_servers] + end + + params[:mcp_servers] = normalize_mcp_servers(mcps) + + # A server's allowed_tools become an mcp_toolset entry beside the + # request's own tools. Added only when there is one: an empty + # list would send `"tools": null` on every MCP request. + mcp_tools = normalize_mcp_tools(mcps) + params[:tools] = Array(params[:tools]) + mcp_tools if mcp_tools.present? end params @@ -111,6 +121,49 @@ def normalize_mcp_servers(mcp_servers) end end + # Builds Anthropic mcp_toolset tool entries from MCP server allowed_tools. + # + # Accepts allowed_tools entries as tool-name strings/symbols or hashes + # with a :name key; entries in any other format are ignored. + # + # @param mcp_servers [Array] + # @return [Array, nil] toolset entries, or nil when none were extracted + def normalize_mcp_tools(mcp_servers) + return nil unless mcp_servers.is_a?(Array) + + result = mcp_servers.filter_map do |server| + next unless server.is_a?(Hash) + + server_hash = server.deep_symbolize_keys + allowed_tools = server_hash[:allowed_tools] + next unless allowed_tools.is_a?(Array) + + configs = allowed_tools.filter_map { |tool| + name = case tool + when String, Symbol + tool.to_s + when Hash + (tool[:name] || tool["name"]).to_s + end + + [ name, { enabled: true } ] if name.present? + }.to_h + + next if configs.empty? + + { + type: "mcp_toolset", + mcp_server_name: server_hash[:name], + default_config: { + enabled: false + }, + configs: configs + } + end + + result.presence + end + # Normalizes tool_choice from common format to Anthropic gem model objects. # # The Anthropic gem expects tool_choice to be a model object (ToolChoiceAuto, diff --git a/lib/active_agent/providers/open_ai/responses/transforms.rb b/lib/active_agent/providers/open_ai/responses/transforms.rb index 86b06c97..5b365149 100644 --- a/lib/active_agent/providers/open_ai/responses/transforms.rb +++ b/lib/active_agent/providers/open_ai/responses/transforms.rb @@ -96,6 +96,14 @@ def normalize_mcp_servers(mcp_servers) server_url: server_hash[:url] || server_hash[:server_url] } + if server_hash[:require_approval] + result[:require_approval] = server_hash[:require_approval] + end + + if server_hash[:allowed_tools] + result[:allowed_tools] = server_hash[:allowed_tools] + end + # Keep authorization field (OpenAI uses 'authorization', not 'authorization_token') if server_hash[:authorization] result[:authorization] = server_hash[:authorization] diff --git a/test/providers/anthropic/transforms_test.rb b/test/providers/anthropic/transforms_test.rb index 3c223a7b..5f694221 100644 --- a/test/providers/anthropic/transforms_test.rb +++ b/test/providers/anthropic/transforms_test.rb @@ -691,6 +691,262 @@ def transforms assert_equal "not an array", result end + + # normalize_mcp_tools tests + test "normalize_mcp_tools converts allowed_tools to mcp_toolset format" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ + { name: "create_payment" }, + { name: "get_payment" } + ] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 1, result.size + assert_equal "mcp_toolset", result[0][:type] + assert_equal "stripe", result[0][:mcp_server_name] + assert_equal false, result[0][:default_config][:enabled] + assert_equal 2, result[0][:configs].size + assert_equal true, result[0][:configs]["create_payment"][:enabled] + assert_equal true, result[0][:configs]["get_payment"][:enabled] + end + + test "normalize_mcp_tools handles multiple servers with allowed_tools" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ + { name: "create_payment" } + ] + }, + { + name: "github", + url: "https://api.github.com", + allowed_tools: [ + { name: "search_repos" }, + { name: "create_issue" } + ] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 2, result.size + assert_equal "stripe", result[0][:mcp_server_name] + assert_equal 1, result[0][:configs].size + assert_equal "github", result[1][:mcp_server_name] + assert_equal 2, result[1][:configs].size + end + + test "normalize_mcp_tools skips servers without allowed_tools" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ + { name: "create_payment" } + ] + }, + { + name: "public", + url: "https://public.api.com" + # No allowed_tools + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 1, result.size + assert_equal "stripe", result[0][:mcp_server_name] + end + + test "normalize_mcp_tools handles allowed_tools as array of strings" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ "create_payment", "get_payment" ] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 1, result.size + assert_equal "mcp_toolset", result[0][:type] + assert_equal "stripe", result[0][:mcp_server_name] + assert_equal 2, result[0][:configs].size + assert_equal true, result[0][:configs]["create_payment"][:enabled] + assert_equal true, result[0][:configs]["get_payment"][:enabled] + end + + test "normalize_mcp_tools handles mixed string and hash allowed_tools" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ "create_payment", { name: "get_payment" }, :list_payments, 42 ] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 1, result.size + assert_equal 3, result[0][:configs].size + assert_equal true, result[0][:configs]["create_payment"][:enabled] + assert_equal true, result[0][:configs]["get_payment"][:enabled] + assert_equal true, result[0][:configs]["list_payments"][:enabled] + end + + test "normalize_mcp_tools returns nil for empty allowed_tools array" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_nil result + end + + test "normalize_mcp_tools returns nil for servers all without allowed_tools" do + mcp_servers = [ + { + name: "public", + url: "https://public.api.com" + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_nil result + end + + test "normalize_mcp_tools returns nil for nil input" do + result = transforms.normalize_mcp_tools(nil) + + assert_nil result + end + + test "normalize_mcp_tools returns nil for empty array" do + result = transforms.normalize_mcp_tools([]) + + assert_nil result + end + + test "normalize_mcp_tools returns nil for non-array input" do + result = transforms.normalize_mcp_tools("not an array") + + assert_nil result + end + + test "normalize_mcp_tools handles single tool" do + mcp_servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ + { name: "create_payment" } + ] + } + ] + + result = transforms.normalize_mcp_tools(mcp_servers) + + assert_equal 1, result.size + assert_equal 1, result[0][:configs].size + assert_equal true, result[0][:configs]["create_payment"][:enabled] + end + + # Integration test for normalize_params with allowed_tools + test "normalize_params extracts mcp_tools from mcps with allowed_tools" do + params = { + mcps: [ + { + name: "stripe", + url: "https://mcp.stripe.com", + authorization: "sk_test_123", + allowed_tools: [ + { name: "create_payment" }, + { name: "get_payment" } + ] + } + ] + } + + result = transforms.normalize_params(params) + + # Should have mcp_servers + assert_equal 1, result[:mcp_servers].size + assert_equal "stripe", result[:mcp_servers][0][:name] + + # Should have extracted tools from allowed_tools + assert result[:tools].present? + assert_equal 1, result[:tools].size + assert_equal "mcp_toolset", result[:tools][0][:type] + assert_equal "stripe", result[:tools][0][:mcp_server_name] + assert_equal 2, result[:tools][0][:configs].size + end + + test "normalize_params does not set tools key when mcps have no allowed_tools" do + params = { + mcps: [ + { + name: "stripe", + url: "https://mcp.stripe.com" + } + ] + } + + result = transforms.normalize_params(params) + + assert_equal 1, result[:mcp_servers].size + assert_not result.key?(:tools) + end + + test "normalize_params does not set tools key for empty mcp_servers default" do + params = { + mcp_servers: [] + } + + result = transforms.normalize_params(params) + + assert_not result.key?(:tools) + end + + test "normalize_params keeps existing tools and adds the mcp toolset beside them" do + params = { + tools: [ + { name: "existing_tool", input_schema: { type: "object" } } + ], + mcps: [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ + { name: "create_payment" } + ] + } + ] + } + + result = transforms.normalize_params(params) + + # The request's own tools stay, and the server's allowed_tools still + # restrict what it exposes rather than being dropped. + assert_equal 2, result[:tools].size + assert_equal "existing_tool", result[:tools][0][:name] + assert_equal "mcp_toolset", result[:tools][1][:type] + assert_equal({ "create_payment" => { enabled: true } }, result[:tools][1][:configs]) + end end end end diff --git a/test/providers/open_ai/responses/transforms_test.rb b/test/providers/open_ai/responses/transforms_test.rb index c36eea00..ae9ecaae 100644 --- a/test/providers/open_ai/responses/transforms_test.rb +++ b/test/providers/open_ai/responses/transforms_test.rb @@ -523,6 +523,163 @@ def serializable.serialize assert_equal [], result end + + # require_approval tests + test "normalize_mcp_servers handles require_approval always" do + servers = [ + { name: "stripe", url: "https://mcp.stripe.com", require_approval: "always" } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal "always", result[0][:require_approval] + end + + test "normalize_mcp_servers handles require_approval never" do + servers = [ + { name: "stripe", url: "https://mcp.stripe.com", require_approval: "never" } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal "never", result[0][:require_approval] + end + + test "normalize_mcp_servers handles require_approval with hash" do + servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + require_approval: { + always: [ "payment_methods" ], + never: [ "read_operations" ] + } + } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal [ "payment_methods" ], result[0][:require_approval][:always] + assert_equal [ "read_operations" ], result[0][:require_approval][:never] + end + + test "normalize_mcp_servers without require_approval omits field" do + servers = [ + { name: "stripe", url: "https://mcp.stripe.com" } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_nil result[0][:require_approval] + end + + # allowed_tools tests + test "normalize_mcp_servers handles allowed_tools array" do + servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + allowed_tools: [ "create_payment", "get_payment" ] + } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal [ "create_payment", "get_payment" ], result[0][:allowed_tools] + end + + test "normalize_mcp_servers handles empty allowed_tools array" do + servers = [ + { name: "stripe", url: "https://mcp.stripe.com", allowed_tools: [] } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal [], result[0][:allowed_tools] + end + + test "normalize_mcp_servers without allowed_tools omits field" do + servers = [ + { name: "stripe", url: "https://mcp.stripe.com" } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_nil result[0][:allowed_tools] + end + + # Combined require_approval and allowed_tools tests + test "normalize_mcp_servers handles both require_approval and allowed_tools" do + servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + authorization: "sk_test_123", + require_approval: "always", + allowed_tools: [ "create_payment", "get_payment" ] + } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 1, result.size + assert_equal "mcp", result[0][:type] + assert_equal "stripe", result[0][:server_label] + assert_equal "https://mcp.stripe.com", result[0][:server_url] + assert_equal "sk_test_123", result[0][:authorization] + assert_equal "always", result[0][:require_approval] + assert_equal [ "create_payment", "get_payment" ], result[0][:allowed_tools] + end + + test "normalize_mcp_servers handles different require_approval for multiple servers" do + servers = [ + { + name: "stripe", + url: "https://mcp.stripe.com", + require_approval: "always", + allowed_tools: [ "create_payment" ] + }, + { + name: "github", + url: "https://api.githubcopilot.com/mcp/", + require_approval: "never", + allowed_tools: [ "search_repos", "create_issue" ] + } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal 2, result.size + assert_equal "always", result[0][:require_approval] + assert_equal [ "create_payment" ], result[0][:allowed_tools] + assert_equal "never", result[1][:require_approval] + assert_equal [ "search_repos", "create_issue" ], result[1][:allowed_tools] + end + + test "normalize_mcp_servers preserves require_approval and allowed_tools in already normalized format" do + servers = [ + { + type: "mcp", + server_label: "stripe", + server_url: "https://mcp.stripe.com", + require_approval: "always", + allowed_tools: [ "create_payment" ] + } + ] + + result = transforms.normalize_mcp_servers(servers) + + assert_equal servers, result + assert_equal "always", result[0][:require_approval] + assert_equal [ "create_payment" ], result[0][:allowed_tools] + end end end end