From 049533836d74b5336946100660af617063bfaec6 Mon Sep 17 00:00:00 2001 From: NekoPunch Date: Sat, 29 Aug 2026 12:09:12 -0700 Subject: [PATCH] fix(python): reject MCP servers passed as provider agent tools An MCPTool handed to ClaudeAgent or GitHubCopilotAgent was silently dropped: no error, no warning, and none of its tools reached the model. Both SDKs connect to MCP servers themselves and take their own mcp_servers config, so the framework cannot honor a framework-managed MCPTool there. It is now refused with the native configuration to write instead. --- .../claude/agent_framework_claude/_agent.py | 22 ++++++++-- .../claude/tests/test_claude_agent.py | 28 +++++++++++- .../agent_framework_github_copilot/_agent.py | 23 +++++++++- .../tests/test_github_copilot_agent.py | 44 +++++++++++++++++++ 4 files changed, 112 insertions(+), 5 deletions(-) diff --git a/python/packages/claude/agent_framework_claude/_agent.py b/python/packages/claude/agent_framework_claude/_agent.py index e34f507c07f..bbdc3305a44 100644 --- a/python/packages/claude/agent_framework_claude/_agent.py +++ b/python/packages/claude/agent_framework_claude/_agent.py @@ -29,6 +29,7 @@ normalize_messages, normalize_tools, ) +from agent_framework._mcp import MCPTool from agent_framework._telemetry import mark_feature_used from agent_framework.exceptions import AgentException, AgentInvalidRequestException from agent_framework.observability import AgentTelemetryLayer @@ -72,6 +73,15 @@ logger = logging.getLogger("agent_framework.claude") +_MCP_TOOL_MESSAGE = ( + "MCP server '{name}' cannot be passed to ClaudeAgent as a tool: the Claude Agent SDK " + "connects to MCP servers itself, so a framework-managed MCPTool would keep none of its " + "framework behavior. Configure the server natively instead, for example " + "default_options={{'mcp_servers': {{'{name}': " + "{{'type': 'stdio', 'command': 'python', 'args': ['server.py']}}}}}}, or use a ChatAgent, " + "where the framework owns the connection." +) + FINISH_REASON_MAP: dict[str, str] = { "end_turn": "stop", "stop_sequence": "stop", @@ -408,8 +418,9 @@ def _normalize_tools( return non_builtin_tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] = [] - if not isinstance(tools, list): - tools = [tools] + # Same wrapping rule as normalize_tools: any other Sequence is a collection of tools. + if isinstance(tools, (str, bytes, bytearray, Mapping)) or not isinstance(tools, Sequence): + tools = [tools] # type: ignore[assignment] for tool in tools: # type: ignore[reportUnknownVariableType] if isinstance(tool, str): self._builtin_tools.append(tool) @@ -417,7 +428,12 @@ def _normalize_tools( non_builtin_tools.append(tool) # type: ignore[union-attr, reportUnknownArgumentType] if not non_builtin_tools: return - self._custom_tools.extend(normalize_tools(non_builtin_tools)) + # Check after normalizing: it flattens tool-collection wrappers, which can hide an MCPTool. + normalized = normalize_tools(non_builtin_tools) + for tool in normalized: + if isinstance(tool, MCPTool): + raise TypeError(_MCP_TOOL_MESSAGE.format(name=tool.name)) + self._custom_tools.extend(normalized) async def __aenter__(self) -> RawClaudeAgent[OptionsT]: """Start the agent when entering async context.""" diff --git a/python/packages/claude/tests/test_claude_agent.py b/python/packages/claude/tests/test_claude_agent.py index 38359f9d152..6fdb444f825 100644 --- a/python/packages/claude/tests/test_claude_agent.py +++ b/python/packages/claude/tests/test_claude_agent.py @@ -1,10 +1,11 @@ # Copyright (c) Microsoft. All rights reserved. +from types import SimpleNamespace from typing import Any, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest -from agent_framework import AgentResponseUpdate, AgentSession, Content, Message, tool +from agent_framework import AgentResponseUpdate, AgentSession, Content, MCPStdioTool, Message, tool from agent_framework._settings import load_settings from agent_framework.exceptions import AgentInvalidRequestException @@ -174,6 +175,31 @@ def farewell(name: str) -> str: agent = ClaudeAgent(tools=[greet, farewell]) assert len(agent._custom_tools) == 2 # type: ignore[reportPrivateUsage] + def test_mcp_tool_is_rejected_with_the_native_configuration(self) -> None: + """An MCP server cannot keep its framework behavior here, so it is refused, not dropped.""" + with pytest.raises(TypeError, match="mcp_servers"): + ClaudeAgent(tools=[MCPStdioTool(name="weather", command="python")]) + + def test_mcp_tool_in_a_tuple_is_rejected(self) -> None: + """``tools`` takes any sequence, so a tuple must not slip past the refusal.""" + with pytest.raises(TypeError, match="mcp_servers"): + ClaudeAgent(tools=(MCPStdioTool(name="weather", command="python"),)) + + def test_mcp_tool_inside_a_tool_collection_is_rejected(self) -> None: + """normalize_tools flattens collection wrappers, so the refusal has to run after it.""" + + toolbox = SimpleNamespace(tools=[MCPStdioTool(name="weather", command="python")]) + + with pytest.raises(TypeError, match="mcp_servers"): + ClaudeAgent(tools=[toolbox]) + + def test_builtin_tools_in_a_tuple_are_recognized(self) -> None: + """A tuple of built-in names must classify like the list form, not fall through as custom tools.""" + agent = ClaudeAgent(tools=("Read", "Bash")) + + assert agent._builtin_tools == ["Read", "Bash"] # type: ignore[reportPrivateUsage] + assert agent._custom_tools == [] # type: ignore[reportPrivateUsage] + def test_no_tools(self) -> None: """Test agent without tools.""" agent = ClaudeAgent() diff --git a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py index c58e5edf711..811d329891f 100644 --- a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py +++ b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py @@ -30,6 +30,7 @@ add_usage_details, normalize_messages, ) +from agent_framework._mcp import MCPTool from agent_framework._settings import load_settings from agent_framework._telemetry import mark_feature_used from agent_framework._tools import FunctionTool, ToolTypes @@ -179,6 +180,22 @@ async def _resolve_function_approval( logger = logging.getLogger("agent_framework.github_copilot") +_MCP_TOOL_MESSAGE = ( + "MCP server '{name}' cannot be passed to GitHubCopilotAgent as a tool: the Copilot SDK " + "connects to MCP servers itself, so a framework-managed MCPTool would keep none of its " + "framework behavior. Configure the server natively instead, for example " + "default_options={{'mcp_servers': {{'{name}': {{'type': 'stdio', 'command': 'python', " + "'args': ['server.py'], 'tools': ['*']}}}}}}, or use a ChatAgent, where the framework owns " + "the connection." +) + + +def _reject_mcp_tools(tools: Sequence[Any]) -> None: + """Refuse MCP servers handed in as tools, from whichever option carried them.""" + for tool in tools: + if isinstance(tool, MCPTool): + raise TypeError(_MCP_TOOL_MESSAGE.format(name=tool.name)) + def _deny_all_permissions( _request: PermissionRequest, @@ -654,6 +671,7 @@ def __init__( ) self._tools = normalize_tools(tools) + _reject_mcp_tools(self._tools) self._permission_handler = on_permission_request self._on_pre_tool_use: PreToolUseHandler | None = on_pre_tool_use self._function_approval_handler: FunctionApprovalCallback | None = on_function_approval @@ -1472,7 +1490,10 @@ def _build_session_kwargs( # Merge agent-level tools with any caller-supplied tools (from default_options # or per-run options, the latter winning) and convert to SDK tools. - all_tools = list(self._tools or []) + list(kwargs.get("tools") or []) + # Normalize the option-supplied tools the way the constructor does: it converts callables + # and flattens tool-collection wrappers, which can otherwise hide an MCPTool. + all_tools = normalize_tools(list(self._tools or []) + list(kwargs.get("tools") or [])) + _reject_mcp_tools(all_tools) kwargs["tools"] = self._prepare_tools(all_tools) if all_tools else None kwargs["streaming"] = streaming diff --git a/python/packages/github_copilot/tests/test_github_copilot_agent.py b/python/packages/github_copilot/tests/test_github_copilot_agent.py index 3643ae9e591..37412e42048 100644 --- a/python/packages/github_copilot/tests/test_github_copilot_agent.py +++ b/python/packages/github_copilot/tests/test_github_copilot_agent.py @@ -9,6 +9,7 @@ import unittest.mock from collections.abc import Sequence from datetime import datetime, timezone +from types import SimpleNamespace from typing import Any, cast from unittest.mock import AsyncMock, MagicMock, patch from uuid import uuid4 @@ -24,6 +25,7 @@ Content, ContextProvider, HistoryProvider, + MCPStdioTool, Message, tool, ) @@ -1940,6 +1942,48 @@ async def test_arbitrary_option_forwarded_verbatim( assert config["reasoning_effort"] == "high" assert config["context_tier"] == "large" + async def test_mcp_tool_is_rejected_with_the_native_configuration( + self, + mock_client: MagicMock, + ) -> None: + """An MCP server cannot keep its framework behavior here, so it is refused, not dropped.""" + with pytest.raises(TypeError, match="mcp_servers"): + GitHubCopilotAgent(client=mock_client, tools=[MCPStdioTool(name="weather", command="python")]) + + async def test_mcp_tool_in_a_tuple_is_rejected( + self, + mock_client: MagicMock, + ) -> None: + """``tools`` takes any sequence, so a tuple must not slip past the refusal.""" + with pytest.raises(TypeError, match="mcp_servers"): + GitHubCopilotAgent(client=mock_client, tools=(MCPStdioTool(name="weather", command="python"),)) + + async def test_mcp_tool_from_default_options_is_rejected( + self, + mock_client: MagicMock, + ) -> None: + """Tools reach the SDK from the options too, so the refusal cannot live in the constructor alone.""" + agent = GitHubCopilotAgent( + client=mock_client, + default_options=cast(Any, {"tools": [MCPStdioTool(name="weather", command="python")]}), + ) + await agent.start() + + with pytest.raises(AgentException, match="mcp_servers"): + await agent._get_or_create_session(AgentSession()) # type: ignore[reportPrivateUsage] + + async def test_mcp_tool_inside_a_tool_collection_from_options_is_rejected( + self, + mock_client: MagicMock, + ) -> None: + """Option-supplied tools are normalized too, so a wrapper cannot hide an MCP server.""" + toolbox = SimpleNamespace(tools=[MCPStdioTool(name="weather", command="python")]) + agent = GitHubCopilotAgent(client=mock_client, default_options=cast(Any, {"tools": [toolbox]})) + await agent.start() + + with pytest.raises(AgentException, match="mcp_servers"): + await agent._get_or_create_session(AgentSession()) # type: ignore[reportPrivateUsage] + async def test_tools_from_default_options_are_honored( self, mock_client: MagicMock,