diff --git a/src/google/adk/tools/mcp_tool/mcp_tool.py b/src/google/adk/tools/mcp_tool/mcp_tool.py index 3b668a73..a411b8b6 100644 --- a/src/google/adk/tools/mcp_tool/mcp_tool.py +++ b/src/google/adk/tools/mcp_tool/mcp_tool.py @@ -42,6 +42,9 @@ from ...auth.auth_tool import AuthConfig from ...events.ui_widget import UiWidget from ...features import FeatureName from ...features import is_feature_enabled +from ...flows.llm_flows.functions import REQUEST_CONFIRMATION_FUNCTION_CALL_NAME +from ...flows.llm_flows.functions import REQUEST_EUC_FUNCTION_CALL_NAME +from ...flows.llm_flows.functions import REQUEST_INPUT_FUNCTION_CALL_NAME from ...utils.context_utils import find_context_parameter # `is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING)` gates the # error-boundary and transport-crash-detection behavior added in this module. @@ -52,6 +55,7 @@ from ...utils.context_utils import find_context_parameter from .._gemini_schema_util import _to_gemini_schema from ..base_authenticated_tool import BaseAuthenticatedTool from ..tool_context import ToolContext +from ..transfer_to_agent_tool import transfer_to_agent from .mcp_session_manager import _http_debug_var from .mcp_session_manager import MCPSessionManager from .mcp_session_manager import retry_on_errors @@ -59,6 +63,16 @@ from .session_context import SessionContext logger = logging.getLogger("google_adk." + __name__) +# Tool names the framework itself puts on the wire. A server advertising one of +# these would have its tool dispatched in place of the framework's own, so the +# name is refused at registration. +_RESERVED_TOOL_NAMES = frozenset({ + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, + REQUEST_INPUT_FUNCTION_CALL_NAME, + transfer_to_agent.__name__, +}) + @runtime_checkable class ProgressCallbackFactory(Protocol): @@ -176,8 +190,14 @@ class McpTool(BaseAuthenticatedTool): and modify runtime context like session state. Raises: - ValueError: If mcp_tool or mcp_session_manager is None. + ValueError: If the MCP tool name collides with a reserved ADK tool + name. """ + if mcp_tool.name in _RESERVED_TOOL_NAMES: + raise ValueError( + f"MCP tool name '{mcp_tool.name}' collides with a reserved ADK tool" + " name." + ) super().__init__( name=mcp_tool.name, diff --git a/src/google/adk/tools/mcp_tool/mcp_toolset.py b/src/google/adk/tools/mcp_tool/mcp_toolset.py index 595034a3..30df8daa 100644 --- a/src/google/adk/tools/mcp_tool/mcp_toolset.py +++ b/src/google/adk/tools/mcp_tool/mcp_toolset.py @@ -61,6 +61,7 @@ from .mcp_session_manager import retry_on_errors from .mcp_session_manager import SseConnectionParams from .mcp_session_manager import StdioConnectionParams from .mcp_session_manager import StreamableHTTPConnectionParams +from .mcp_tool import _RESERVED_TOOL_NAMES from .mcp_tool import MCPTool from .mcp_tool import ProgressCallbackFactory @@ -564,6 +565,16 @@ class McpToolset(BaseToolset): # even on a cache hit, so only the round trip is skipped. tools = [] for tool in mcp_tools: + # Skip rather than let McpTool raise: one reserved name would otherwise + # fail the whole listing and take the server's honest tools down with it. + if tool.name in _RESERVED_TOOL_NAMES: + logger.warning( + "Skipping MCP tool '%s' because it collides with a reserved ADK" + " framework tool name.", + tool.name, + ) + continue + mcp_tool = MCPTool( mcp_tool=tool, mcp_session_manager=self._mcp_session_manager, diff --git a/tests/unittests/tools/mcp_tool/test_mcp_tool.py b/tests/unittests/tools/mcp_tool/test_mcp_tool.py index d4dbe634..d6b8a6da 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_tool.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_tool.py @@ -243,6 +243,40 @@ class TestMCPTool: assert tool.description == "" + @pytest.mark.parametrize( + "reserved_name", + [ + "adk_request_credential", + "adk_request_confirmation", + "adk_request_input", + "transfer_to_agent", + ], + ) + def test_init_reserved_name(self, reserved_name): + """A tool named after a framework function call is refused.""" + mock_tool = MockMCPTool(name=reserved_name) + with pytest.raises( + ValueError, + match=( + f"MCP tool name '{reserved_name}' collides with a reserved ADK tool" + " name." + ), + ): + MCPTool( + mcp_tool=mock_tool, + mcp_session_manager=self.mock_session_manager, + ) + + def test_init_reserved_name_prefix_allowed(self): + """Only exact collisions are refused, not names that merely look alike.""" + mock_tool = MockMCPTool(name="transfer_to_agent_v2") + tool = MCPTool( + mcp_tool=mock_tool, + mcp_session_manager=self.mock_session_manager, + ) + + assert tool.name == "transfer_to_agent_v2" + @pytest.mark.asyncio async def test_run_async_impl_no_auth(self): """Test running tool without authentication.""" diff --git a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py index 1eb64818..de6e38ab 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_toolset.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_toolset.py @@ -397,6 +397,27 @@ class TestMcpToolset: assert [tool.name for tool in tools] == ["alpha", "bravo", "charlie"] + @pytest.mark.asyncio + async def test_get_tools_skips_reserved_names(self): + """A server advertising reserved names loses those, not the whole list.""" + mock_tools = [ + MockMCPTool("valid_tool"), + MockMCPTool("transfer_to_agent"), + MockMCPTool("adk_request_credential"), + MockMCPTool("adk_request_confirmation"), + MockMCPTool("adk_request_input"), + ] + self.mock_session.list_tools = AsyncMock( + return_value=MockListToolsResult(mock_tools) + ) + + toolset = McpToolset(connection_params=self.mock_stdio_params) + toolset._mcp_session_manager = self.mock_session_manager + + tools = await toolset.get_tools() + + assert [tool.name for tool in tools] == ["valid_tool"] + @pytest.mark.asyncio async def test_get_tools_with_list_filter(self): """Test getting tools with list-based filtering."""