From 67571e88618a392e920f478e53c2795bd3778087 Mon Sep 17 00:00:00 2001 From: Hashwanth S Date: Wed, 1 Apr 2026 22:50:25 -0700 Subject: [PATCH 1/2] Python: Support MCP tool binding by reference in declarative YAML (#4927) Add binding resolution for McpTool in AgentFactory._parse_tool(), matching the existing FunctionTool pattern. MCP tools can now be resolved either by explicit bindings list or by matching the tool name against factory bindings, allowing pre-built MCP tool dicts to be passed via AgentFactory(bindings=...) instead of requiring full connection details in every YAML file. Fixes #4927 --- .../agent_framework_declarative/_loader.py | 15 ++ .../tests/test_declarative_loader.py | 134 ++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/python/packages/declarative/agent_framework_declarative/_loader.py b/python/packages/declarative/agent_framework_declarative/_loader.py index a9c534ee2d..9495666b17 100644 --- a/python/packages/declarative/agent_framework_declarative/_loader.py +++ b/python/packages/declarative/agent_framework_declarative/_loader.py @@ -782,6 +782,21 @@ def _parse_tool(self, tool_resource: Tool) -> AFFunctionTool | dict[str, Any]: result["description"] = tool_resource.description return result case McpTool(): + # Check bindings first - allow MCP tools to be resolved by reference + bound_tool: Any = None + if self.bindings: + # Check explicit bindings list first + if tool_resource.bindings: + for binding in tool_resource.bindings: + if binding.name and (bound_tool := self.bindings.get(binding.name)): + break + # Fall back to matching by tool name + if bound_tool is None and tool_resource.name: + bound_tool = self.bindings.get(tool_resource.name) + + if bound_tool is not None: + return bound_tool + result = { "type": "mcp", "server_label": tool_resource.name.replace(" ", "_") if tool_resource.name else "", diff --git a/python/packages/declarative/tests/test_declarative_loader.py b/python/packages/declarative/tests/test_declarative_loader.py index 02485b9e9c..0f4a6c1596 100644 --- a/python/packages/declarative/tests/test_declarative_loader.py +++ b/python/packages/declarative/tests/test_declarative_loader.py @@ -1335,6 +1335,140 @@ def my_function(): tools = agent.default_options.get("tools", []) assert len(tools) == 1 + def test_parse_mcp_tool_with_explicit_bindings(self): + """Test parsing McpTool with explicit bindings resolves from factory bindings.""" + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: Test agent +tools: + - kind: mcp + name: product-mcp + bindings: + - name: my_mcp_binding +""" + + prebuilt_mcp_tool = { + "type": "mcp", + "server_label": "product_mcp", + "server_url": "https://product.example.com/mcp", + "allowed_tools": ["search", "lookup"], + } + + mock_client = MagicMock() + factory = AgentFactory(client=mock_client, bindings={"my_mcp_binding": prebuilt_mcp_tool}) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + assert tools[0] is prebuilt_mcp_tool + + def test_parse_mcp_tool_with_name_binding(self): + """Test parsing McpTool resolved by tool name when no explicit bindings list.""" + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: Test agent +tools: + - kind: mcp + name: product-mcp +""" + + prebuilt_mcp_tool = { + "type": "mcp", + "server_label": "product_mcp", + "server_url": "https://product.example.com/mcp", + } + + mock_client = MagicMock() + factory = AgentFactory(client=mock_client, bindings={"product-mcp": prebuilt_mcp_tool}) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + assert tools[0] is prebuilt_mcp_tool + + def test_parse_mcp_tool_binding_by_name_with_allowed_tools(self): + """Regression test for issue #4927: MCP tool binding by reference with allowedTools. + + When YAML specifies an MCP tool by name with allowedTools but no url, + the tool should resolve from bindings rather than producing server_url: ''. + """ + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: You are a helpful assistant. +tools: + - kind: mcp + name: product-mcp + allowedTools: + - customer_search +""" + + prebuilt_mcp_tool = { + "type": "mcp", + "server_label": "product_mcp", + "server_url": "https://product.example.com/mcp", + "allowed_tools": ["customer_search"], + "headers": {"X-API-Key": "secret"}, + "require_approval": "never", + } + + mock_client = MagicMock() + factory = AgentFactory(client=mock_client, bindings={"product-mcp": prebuilt_mcp_tool}) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + # Must return the pre-built tool, not a dict with server_url: '' + assert tools[0] is prebuilt_mcp_tool + assert tools[0]["server_url"] == "https://product.example.com/mcp" + assert tools[0]["headers"] == {"X-API-Key": "secret"} + + def test_parse_mcp_tool_without_binding_falls_through(self): + """Test that McpTool without matching binding falls through to dict-building logic.""" + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: Test agent +tools: + - kind: mcp + name: other-mcp + url: https://api.example.com/mcp +""" + + prebuilt_mcp_tool = { + "type": "mcp", + "server_label": "product_mcp", + "server_url": "https://product.example.com/mcp", + } + + mock_client = MagicMock() + factory = AgentFactory(client=mock_client, bindings={"product-mcp": prebuilt_mcp_tool}) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + # Should have built the dict from YAML, not used the binding + assert tools[0]["server_label"] == "other-mcp" + assert tools[0]["server_url"] == "https://api.example.com/mcp" + def test_parse_file_search_tool_with_all_options(self): """Test parsing FileSearchTool with ranker and filters.""" from unittest.mock import MagicMock From ec38198b784f043f02801c2a261388d04ab67a43 Mon Sep 17 00:00:00 2001 From: Hashwanth S Date: Sun, 19 Apr 2026 23:38:24 -0700 Subject: [PATCH 2/2] fix: resolve Mypy no-any-return in MCP tool binding resolution Cast bound_tool return value to the declared return type to satisfy mypy strict mode (warn_return_any). Add edge case tests for explicit binding priority and None bindings fallthrough. --- .../agent_framework_declarative/_loader.py | 2 +- .../tests/test_declarative_loader.py | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/python/packages/declarative/agent_framework_declarative/_loader.py b/python/packages/declarative/agent_framework_declarative/_loader.py index 9495666b17..ce113a49b5 100644 --- a/python/packages/declarative/agent_framework_declarative/_loader.py +++ b/python/packages/declarative/agent_framework_declarative/_loader.py @@ -795,7 +795,7 @@ def _parse_tool(self, tool_resource: Tool) -> AFFunctionTool | dict[str, Any]: bound_tool = self.bindings.get(tool_resource.name) if bound_tool is not None: - return bound_tool + return cast("AFFunctionTool | dict[str, Any]", bound_tool) result = { "type": "mcp", diff --git a/python/packages/declarative/tests/test_declarative_loader.py b/python/packages/declarative/tests/test_declarative_loader.py index 0f4a6c1596..db881ecf65 100644 --- a/python/packages/declarative/tests/test_declarative_loader.py +++ b/python/packages/declarative/tests/test_declarative_loader.py @@ -1469,6 +1469,71 @@ def test_parse_mcp_tool_without_binding_falls_through(self): assert tools[0]["server_label"] == "other-mcp" assert tools[0]["server_url"] == "https://api.example.com/mcp" + def test_parse_mcp_tool_explicit_binding_takes_priority_over_name(self): + """Test that explicit bindings list takes priority over name-based lookup.""" + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: Test agent +tools: + - kind: mcp + name: product-mcp + url: https://product.example.com/mcp + bindings: + - name: my_binding +""" + + explicit_binding_tool = { + "type": "mcp", + "server_label": "explicit_binding", + "server_url": "https://explicit.example.com/mcp", + } + name_binding_tool = { + "type": "mcp", + "server_label": "name_binding", + "server_url": "https://name.example.com/mcp", + } + + mock_client = MagicMock() + factory = AgentFactory( + client=mock_client, + bindings={"my_binding": explicit_binding_tool, "product-mcp": name_binding_tool}, + ) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + assert tools[0] is explicit_binding_tool + + def test_parse_mcp_tool_no_bindings_on_factory_falls_through(self): + """Test that MCP tool without factory bindings falls through to dict-building.""" + from unittest.mock import MagicMock + + from agent_framework_declarative import AgentFactory + + yaml_content = """ +kind: Prompt +name: TestAgent +instructions: Test agent +tools: + - kind: mcp + name: my-mcp-server + url: https://api.example.com/mcp +""" + + mock_client = MagicMock() + factory = AgentFactory(client=mock_client, bindings=None) + agent = factory.create_agent_from_yaml(yaml_content) + + tools = agent.default_options.get("tools", []) + assert len(tools) == 1 + assert tools[0]["server_label"] == "my-mcp-server" + assert tools[0]["server_url"] == "https://api.example.com/mcp" + def test_parse_file_search_tool_with_all_options(self): """Test parsing FileSearchTool with ranker and filters.""" from unittest.mock import MagicMock