Skip to content

feat: Add allowed_tools and require_approval to option transforms #328

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
dark-panda wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Add allowed_tools and require_approval to option transforms #328

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
39 changes: 35 additions & 4 deletions lib/active_agent/providers/anthropic/transforms.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,18 @@ def normalize_params(params)
params[:tool_choice] = normalize_tool_choice(params[:tool_choice]) if params[:tool_choice]

# 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)

if params[:tools].nil? # If tools not already provided, extract from mcps
params[:tools] = normalize_mcp_tools(mcps)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In normalize_params, when :tools is nil and MCPs are provided, params[:tools] = normalize_mcp_tools(mcps) will set :tools to nil when no allowed_tools are present (because normalize_mcp_tools returns nil via .presence). This introduces a tools: nil keyword arg that previously wouldn’t be sent and may serialize as "tools": null / fail Anthropic model validation. Consider only assigning :tools when toolsets were actually extracted (or deleting the key when the result is nil).

Suggested change
params[:tools] = normalize_mcp_tools(mcps)
mcp_tools = normalize_mcp_tools(mcps)
params[:tools] = mcp_tools if mcp_tools

Copilot uses AI. Check for mistakes.
end
end

params
Expand Down Expand Up @@ -105,6 +113,29 @@ def normalize_mcp_servers(mcp_servers)
end
end

def normalize_mcp_tools(mcp_servers)
return [] unless mcp_servers.is_a?(Array)

result = mcp_servers.map do |server|
server_hash = server.is_a?(Hash) ? server.deep_symbolize_keys : server

if server_hash[:allowed_tools].present?
{
type: "mcp_toolset",
mcp_server_name: server_hash[:name],
default_config: {
enabled: false
},
configs: server_hash[:allowed_tools].to_h { |tool|
[ tool[:name], { enabled: true } ]
}
}
end
Comment on lines +120 to +133
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalize_mcp_tools assumes allowed_tools is an array of hashes with :name (uses tool[:name]). Elsewhere (e.g., MCP docs/examples) allowed_tools is shown as an array of strings, which would raise a TypeError here. Consider supporting both formats (string tool names and {name: ...} hashes), and ignore/validate unexpected elements so tool extraction doesn't crash on valid common-format input.

Suggested change
server_hash = server.is_a?(Hash) ? server.deep_symbolize_keys : server
if server_hash[:allowed_tools].present?
{
type: "mcp_toolset",
mcp_server_name: server_hash[:name],
default_config: {
enabled: false
},
configs: server_hash[:allowed_tools].to_h { |tool|
[ tool[:name], { enabled: true } ]
}
}
end
# Only hash servers are supported for MCP tool extraction
next unless server.is_a?(Hash)
server_hash = server.deep_symbolize_keys
allowed_tools = server_hash[:allowed_tools]
next unless allowed_tools.present? && allowed_tools.is_a?(Array)
configs_array = allowed_tools.filter_map do |tool|
case tool
when String, Symbol
name = tool.to_s
next if name.empty?
[name, { enabled: true }]
when Hash
tool_hash = tool.respond_to?(:deep_symbolize_keys) ? tool.deep_symbolize_keys : tool
name = tool_hash[:name] || tool_hash["name"]
next unless name
name_str = name.to_s
next if name_str.empty?
[name_str, { enabled: true }]
else
# Ignore unsupported tool formats
next
end
end
next if configs_array.empty?
{
type: "mcp_toolset",
mcp_server_name: server_hash[:name],
default_config: {
enabled: false
},
configs: configs_array.to_h
}

Copilot uses AI. Check for mistakes.
end

result.compact.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,
Expand Down
8 changes: 8 additions & 0 deletions lib/active_agent/providers/open_ai/responses/transforms.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
190 changes: 190 additions & 0 deletions test/providers/anthropic/transforms_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,196 @@ 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" }
]
}
Comment on lines +696 to +705
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new allowed_tools behavior is only tested with allowed_tools: [{ name: ... }], but docs/examples show allowed_tools as an array of tool-name strings. Add coverage for the string-array form (and/or mixed forms) to ensure common-format input doesn’t break normalize_mcp_tools.

Copilot uses AI. Check for mistakes.
]

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 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 empty array for nil input" do
result = transforms.normalize_mcp_tools(nil)

assert_equal [], result
end

test "normalize_mcp_tools returns empty array for empty array" do
result = transforms.normalize_mcp_tools([])

assert_nil result
end
Comment on lines +802 to +806
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name says it "returns empty array for empty array" but the assertion expects nil. Please align the test description with the expected behavior (or adjust the behavior if [] is actually intended), so failures are easier to interpret.

Copilot uses AI. Check for mistakes.

test "normalize_mcp_tools returns empty array for non-array input" do
result = transforms.normalize_mcp_tools("not an array")

assert_equal [], 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 override existing tools when extracting from mcps" 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)

# Should keep existing tools, not override with mcp tools
assert_equal 1, result[:tools].size
assert_equal "existing_tool", result[:tools][0][:name]
end
end
end
end
Loading
Loading