Uh oh!
There was an error while loading. Please reload this page.
feat: Add allowed_tools and require_approval to option transforms - #328
feat: Add allowed_tools and require_approval to option transforms#328dark-panda wants to merge 1 commit into
allowed_tools and require_approval to option transforms#328Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for MCP tool allowlisting and approval requirements during provider-specific option normalization, improving interoperability with LLMs that require explicit tool enablement.
Changes:
- OpenAI Responses: propagate
require_approvalandallowed_toolsthrough MCP server normalization. - Anthropic: introduce
normalize_mcp_toolsand auto-extracttools(asmcp_toolset) from MCP servers’allowed_toolswhentoolsis not provided. - Add/extend provider transform test coverage around these new MCP options.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/active_agent/providers/open_ai/responses/transforms.rb | Passes require_approval and allowed_tools through OpenAI MCP server normalization. |
| lib/active_agent/providers/anthropic/transforms.rb | Extracts MCP allowed_tools into Anthropic tools (mcp_toolset) during param normalization. |
| test/providers/open_ai/responses/transforms_test.rb | Adds tests for OpenAI MCP normalization with require_approval and allowed_tools. |
| test/providers/anthropic/transforms_test.rb | Adds tests for Anthropic MCP tool extraction and normalize_params integration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
623a217 to
43a4fedCompareTonsOfFun
commented
Jun 9, 2026
I think we may need to update the VCR cassettes here |
43a4fed to
957bd29CompareTonsOfFun
commented
Jul 29, 2026
I dug into the CI failures on this PR. There are two separate causes, and I've pushed a fixed version of this change to the Why the 50 test failures happenEvery failing request body contains The chain is:
The fix is to only assign ifparams[:tools].nil?mcp_tools=normalize_mcp_tools(mcps)params[:tools]=mcp_toolsifmcp_tools.present?endWhy the lint job failsThe repo's RuboCop config (rails-omakase) enforces Also included on the fix branchWhile fixing the above I also addressed the Copilot review feedback:
Feel free to cherry-pick Generated by Claude Code |
Some LLMs will not execute tools unless explicitly told that they are allowed to do so. This feature normalizes the options and transforms them for specific LLMs as necessary.