From 3473b0ccc7cd00bb387a06a58c21315c223c93bf Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Mon, 15 Dec 2025 15:09:19 +0100 Subject: [PATCH 1/5] fix: PAAL-247 retreive tool description from mpc servers --- adk/agenticlayer/agent.py | 65 +++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/adk/agenticlayer/agent.py b/adk/agenticlayer/agent.py index f05f6dc..fe02397 100644 --- a/adk/agenticlayer/agent.py +++ b/adk/agenticlayer/agent.py @@ -40,7 +40,7 @@ async def load_agent(self, agent: LlmAgent, sub_agents: list[SubAgent], tools: l """ agents, agent_tools = await self.load_sub_agents(sub_agents) - mcp_tools = self.load_tools(tools) + mcp_tools, mcp_tool_descriptions = await self.load_tools(tools) all_tools: list[ToolUnion] = agent_tools + mcp_tools # The ADK currently only adds the agent as a function with the agent name to the instructions. @@ -53,6 +53,15 @@ async def load_agent(self, agent: LlmAgent, sub_agents: list[SubAgent], tools: l agent_tool_instructions += "\nYou can use them by calling the tool with the agent name.\n" agent.instruction = f"{agent.instruction}{agent_tool_instructions}" + # Add MCP tool descriptions to instructions + if mcp_tool_descriptions: + mcp_tool_instructions = "\n\nFollowing MCP tools are available:\n" + mcp_tool_instructions += "\n".join( + [f"- '{name}': {description}" for name, description in mcp_tool_descriptions] + ) + mcp_tool_instructions += "\nYou can use them by calling the tool with the tool name.\n" + agent.instruction = f"{agent.instruction}{mcp_tool_instructions}" + agent.sub_agents += agents agent.tools += all_tools return agent @@ -87,24 +96,54 @@ async def load_sub_agents(self, sub_agents: list[SubAgent]) -> tuple[list[BaseAg return agents, tools - def load_tools(self, mcp_tools: list[McpTool]) -> list[ToolUnion]: + async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], list[tuple[str, str]]]: """ - Convert Tools into McpToolsets. + Convert Tools into McpToolsets and extract their descriptions. + + This method creates McpToolset instances for runtime use and simultaneously + introspects them to extract tool descriptions. :param mcp_tools: The tools to load - :return: A list of McpToolset tools + :return: Tuple of (toolsets for runtime, tool descriptions for instructions) + :raises ConnectionError: If any MCP server is unavailable or unreachable """ - tools: list[ToolUnion] = [] - for tool in mcp_tools: - logger.info(f"Loading tool {tool.model_dump_json()}") - tools.append( - McpToolset( + toolsets: list[ToolUnion] = [] + tool_descriptions: list[tuple[str, str]] = [] + + for mcp_tool in mcp_tools: + logger.info(f"Loading tool {mcp_tool.model_dump_json()}") + + try: + # Create McpToolset for runtime use + toolset = McpToolset( connection_params=StreamableHTTPConnectionParams( - url=str(tool.url), - timeout=tool.timeout, + url=str(mcp_tool.url), + timeout=mcp_tool.timeout, ), ) - ) - return tools + # Introspect the MCP server to get tool schemas with descriptions + # This queries the MCP server's tools/list endpoint + tools = await toolset.get_tools() + + # Extract (name, description) for each tool + for tool in tools: + name = tool.name + description = tool.description + if description: # Only include tools with descriptions + tool_descriptions.append((name, description)) + else: + logger.warning(f"Tool '{name}' from {mcp_tool.name} has no description") + + # Add toolset to runtime list (keep it alive for agent execution) + toolsets.append(toolset) + + except Exception as e: + logger.error(f"Failed to load MCP tool from {mcp_tool.url}: {e}") + raise ConnectionError( + f"Could not connect to MCP server '{mcp_tool.name}' at {mcp_tool.url}. " + f"Ensure the server is running and accessible." + ) from e + + return toolsets, tool_descriptions From 3e9d0692a4cea135af6fe95a0ce4c7622881b9bc Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Tue, 16 Dec 2025 08:43:19 +0100 Subject: [PATCH 2/5] fix: PAAL-247 add unit tests with mcp mocks --- adk/tests/test_a2a_starlette.py | 76 +++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/adk/tests/test_a2a_starlette.py b/adk/tests/test_a2a_starlette.py index ea72e77..8d703cc 100644 --- a/adk/tests/test_a2a_starlette.py +++ b/adk/tests/test_a2a_starlette.py @@ -72,6 +72,19 @@ def create_agent( ) +def create_mcp_tool_config( + name: str = "test_mcp_tool", + url: str = "http://mcp-server.local/mcp", + timeout: int = 30, +) -> McpTool: + """Helper function to create McpTool configuration object.""" + return McpTool( + name=name, + url=AnyHttpUrl(url), + timeout=timeout, + ) + + @pytest_asyncio.fixture def app_factory() -> Any: @contextlib.asynccontextmanager @@ -214,20 +227,57 @@ async def test_sub_agent_unavailable_fails_startup(self, app_factory: Any) -> No ) @pytest.mark.asyncio - async def test_tools(self, app_factory: Any) -> None: - """Test that tools are integrated correctly.""" - - # When: Creating an agent with tools - tools = [ - McpTool(name="tool_1", url=AnyHttpUrl("http://tool-1.local/mcp")), - McpTool(name="tool_2", url=AnyHttpUrl("http://tool-2.local/mcp")), + async def test_mcp_tool_multiple_from_single_server(self, app_factory: Any, monkeypatch: Any) -> None: + """Test that multiple tools from a single MCP server are all added to agent instructions.""" + + # Given: Mock tools with names and descriptions + mock_tools = [ + type("MockTool", (), {"name": "get_customer", "description": "Retrieves customer information"})(), + type("MockTool", (), {"name": "update_customer", "description": "Updates customer records"})(), + type("MockTool", (), {"name": "delete_customer", "description": "Deletes customer from database"})(), ] + + # And: Mock McpToolset.get_tools to return our mock tools + async def mock_get_tools(self, readonly_context=None): + return mock_tools + + monkeypatch.setattr("google.adk.tools.mcp_tool.mcp_toolset.McpToolset.get_tools", mock_get_tools) + + # And: Agent and MCP tool configuration agent = create_agent() + tools = [create_mcp_tool_config(name="customer_api", url="http://mcp-server.local/mcp")] - # When: Requesting the agent card endpoint - async with app_factory(agent=agent, tools=tools) as app: - client = TestClient(app) - response = client.get("/.well-known/agent-card.json") + # When: Creating app with MCP tool + async with app_factory(agent=agent, tools=tools) as _: + # Then: All three tools should appear in instructions + assert "- 'get_customer': Retrieves customer information" in agent.instruction + assert "- 'update_customer': Updates customer records" in agent.instruction + assert "- 'delete_customer': Deletes customer from database" in agent.instruction - # Then: Agent card is returned - assert response.status_code == 200 + # And: MCP tools section should be present + assert "\n\nFollowing MCP tools are available:\n" in agent.instruction + + @pytest.mark.asyncio + async def test_mcp_tool_server_unavailable(self, app_factory: Any, monkeypatch: Any) -> None: + """Test that unavailable MCP server causes app startup to fail with ConnectionError.""" + + # Given: Mock McpToolset.get_tools to raise an exception + async def mock_get_tools_error(self, readonly_context=None): + raise ConnectionError("Connection refused") + + monkeypatch.setattr("google.adk.tools.mcp_tool.mcp_toolset.McpToolset.get_tools", mock_get_tools_error) + + # And: Agent and MCP tool configuration + agent = create_agent() + tools = [create_mcp_tool_config(name="unavailable_tool", url="http://unreachable-mcp.local/mcp")] + + # Expect: App creation should fail with ConnectionError containing tool name, URL, and helpful message + with pytest.raises(ConnectionError) as exc_info: + async with app_factory(agent=agent, tools=tools): + pass + + # Then: Error message should contain expected details + error_message = str(exc_info.value) + assert "Could not connect to MCP server 'unavailable_tool'" in error_message + assert "http://unreachable-mcp.local/mcp" in error_message + assert "Ensure the server is running and accessible" in error_message From 164f481639c271aa5567e185bbf6233b47889cfa Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Tue, 16 Dec 2025 08:47:31 +0100 Subject: [PATCH 3/5] fix: PAAL-247 add type annotations --- adk/tests/test_a2a_starlette.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/adk/tests/test_a2a_starlette.py b/adk/tests/test_a2a_starlette.py index 8d703cc..62f36a8 100644 --- a/adk/tests/test_a2a_starlette.py +++ b/adk/tests/test_a2a_starlette.py @@ -238,7 +238,7 @@ async def test_mcp_tool_multiple_from_single_server(self, app_factory: Any, monk ] # And: Mock McpToolset.get_tools to return our mock tools - async def mock_get_tools(self, readonly_context=None): + async def mock_get_tools(self: Any, readonly_context: Any = None) -> list[Any]: return mock_tools monkeypatch.setattr("google.adk.tools.mcp_tool.mcp_toolset.McpToolset.get_tools", mock_get_tools) @@ -249,7 +249,8 @@ async def mock_get_tools(self, readonly_context=None): # When: Creating app with MCP tool async with app_factory(agent=agent, tools=tools) as _: - # Then: All three tools should appear in instructions + # Then: Instruction should be a string with all three tools + assert isinstance(agent.instruction, str), "Agent instruction should be a string" assert "- 'get_customer': Retrieves customer information" in agent.instruction assert "- 'update_customer': Updates customer records" in agent.instruction assert "- 'delete_customer': Deletes customer from database" in agent.instruction @@ -262,7 +263,7 @@ async def test_mcp_tool_server_unavailable(self, app_factory: Any, monkeypatch: """Test that unavailable MCP server causes app startup to fail with ConnectionError.""" # Given: Mock McpToolset.get_tools to raise an exception - async def mock_get_tools_error(self, readonly_context=None): + async def mock_get_tools_error(self: Any, readonly_context: Any = None) -> Any: raise ConnectionError("Connection refused") monkeypatch.setattr("google.adk.tools.mcp_tool.mcp_toolset.McpToolset.get_tools", mock_get_tools_error) From ca6733adb8124dea16a685a7007936889da6c758 Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Tue, 16 Dec 2025 09:39:01 +0100 Subject: [PATCH 4/5] fix: PAAL-247 split tool loading into PHASE 1 (introspect + close) and PHASE 2 (create runtime) --- adk/agenticlayer/agent.py | 62 ++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/adk/agenticlayer/agent.py b/adk/agenticlayer/agent.py index fe02397..041de01 100644 --- a/adk/agenticlayer/agent.py +++ b/adk/agenticlayer/agent.py @@ -100,8 +100,19 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l """ Convert Tools into McpToolsets and extract their descriptions. - This method creates McpToolset instances for runtime use and simultaneously - introspects them to extract tool descriptions. + This method creates McpToolset instances for runtime use and introspects + MCP servers using temporary connections to extract tool descriptions. + + We use TWO separate McpToolset instances per MCP server: + 1. Temporary instance for introspection - created, used to get tool schemas, then closed + 2. Runtime instance - long-lived, kept in agent's tool list for execution + + Why separate instances are necessary: + - McpToolset internally uses anyio.create_task_group() for async connection management + - When get_tools() is called during Starlette's lifespan startup, these task groups + conflict with Starlette's own async context management + - Closing the introspection instance ensures all internal task groups are properly + cleaned up before the runtime instance is created :param mcp_tools: The tools to load :return: Tuple of (toolsets for runtime, tool descriptions for instructions) @@ -114,18 +125,21 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l for mcp_tool in mcp_tools: logger.info(f"Loading tool {mcp_tool.model_dump_json()}") - try: - # Create McpToolset for runtime use - toolset = McpToolset( - connection_params=StreamableHTTPConnectionParams( - url=str(mcp_tool.url), - timeout=mcp_tool.timeout, - ), - ) + # Create connection params (reused for both introspection and runtime) + connection_params = StreamableHTTPConnectionParams( + url=str(mcp_tool.url), + timeout=mcp_tool.timeout, + ) + # PHASE 1: Introspection using temporary toolset + # Create a temporary toolset just for introspection to avoid async context conflicts + introspection_toolset = McpToolset(connection_params=connection_params) + + try: # Introspect the MCP server to get tool schemas with descriptions # This queries the MCP server's tools/list endpoint - tools = await toolset.get_tools() + logger.debug(f"Introspecting MCP server at {mcp_tool.url}") + tools = await introspection_toolset.get_tools() # Extract (name, description) for each tool for tool in tools: @@ -136,14 +150,34 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l else: logger.warning(f"Tool '{name}' from {mcp_tool.name} has no description") - # Add toolset to runtime list (keep it alive for agent execution) - toolsets.append(toolset) - except Exception as e: logger.error(f"Failed to load MCP tool from {mcp_tool.url}: {e}") + # Attempt cleanup before raising + try: + await introspection_toolset.close() + except Exception as cleanup_error: + logger.warning(f"Error during introspection toolset cleanup for {mcp_tool.name}: {cleanup_error}") + raise ConnectionError( f"Could not connect to MCP server '{mcp_tool.name}' at {mcp_tool.url}. " f"Ensure the server is running and accessible." ) from e + finally: + # ALWAYS close the introspection toolset to clean up async task groups + try: + logger.debug(f"Closing introspection connection to {mcp_tool.url}") + await introspection_toolset.close() + except Exception as cleanup_error: + # Log but don't raise - cleanup failures shouldn't block startup + logger.warning(f"Error closing introspection toolset for {mcp_tool.name}: {cleanup_error}") + + # PHASE 2: Create separate runtime toolset + # This toolset will be kept alive for the lifetime of the agent + logger.debug(f"Creating runtime toolset for {mcp_tool.url}") + runtime_toolset = McpToolset(connection_params=connection_params) + + # Add runtime toolset to the list (keep it alive for agent execution) + toolsets.append(runtime_toolset) + return toolsets, tool_descriptions From 786bad8da8fe3b72a765fc8611693b994983fca5 Mon Sep 17 00:00:00 2001 From: Jil Kamerling Date: Wed, 17 Dec 2025 09:18:37 +0100 Subject: [PATCH 5/5] Revert "fix: PAAL-247 split tool loading into PHASE 1 (introspect + close) and PHASE 2 (create runtime)" This reverts commit ca6733adb8124dea16a685a7007936889da6c758. --- adk/agenticlayer/agent.py | 62 +++++++++------------------------------ 1 file changed, 14 insertions(+), 48 deletions(-) diff --git a/adk/agenticlayer/agent.py b/adk/agenticlayer/agent.py index 041de01..fe02397 100644 --- a/adk/agenticlayer/agent.py +++ b/adk/agenticlayer/agent.py @@ -100,19 +100,8 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l """ Convert Tools into McpToolsets and extract their descriptions. - This method creates McpToolset instances for runtime use and introspects - MCP servers using temporary connections to extract tool descriptions. - - We use TWO separate McpToolset instances per MCP server: - 1. Temporary instance for introspection - created, used to get tool schemas, then closed - 2. Runtime instance - long-lived, kept in agent's tool list for execution - - Why separate instances are necessary: - - McpToolset internally uses anyio.create_task_group() for async connection management - - When get_tools() is called during Starlette's lifespan startup, these task groups - conflict with Starlette's own async context management - - Closing the introspection instance ensures all internal task groups are properly - cleaned up before the runtime instance is created + This method creates McpToolset instances for runtime use and simultaneously + introspects them to extract tool descriptions. :param mcp_tools: The tools to load :return: Tuple of (toolsets for runtime, tool descriptions for instructions) @@ -125,21 +114,18 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l for mcp_tool in mcp_tools: logger.info(f"Loading tool {mcp_tool.model_dump_json()}") - # Create connection params (reused for both introspection and runtime) - connection_params = StreamableHTTPConnectionParams( - url=str(mcp_tool.url), - timeout=mcp_tool.timeout, - ) - - # PHASE 1: Introspection using temporary toolset - # Create a temporary toolset just for introspection to avoid async context conflicts - introspection_toolset = McpToolset(connection_params=connection_params) - try: + # Create McpToolset for runtime use + toolset = McpToolset( + connection_params=StreamableHTTPConnectionParams( + url=str(mcp_tool.url), + timeout=mcp_tool.timeout, + ), + ) + # Introspect the MCP server to get tool schemas with descriptions # This queries the MCP server's tools/list endpoint - logger.debug(f"Introspecting MCP server at {mcp_tool.url}") - tools = await introspection_toolset.get_tools() + tools = await toolset.get_tools() # Extract (name, description) for each tool for tool in tools: @@ -150,34 +136,14 @@ async def load_tools(self, mcp_tools: list[McpTool]) -> tuple[list[ToolUnion], l else: logger.warning(f"Tool '{name}' from {mcp_tool.name} has no description") + # Add toolset to runtime list (keep it alive for agent execution) + toolsets.append(toolset) + except Exception as e: logger.error(f"Failed to load MCP tool from {mcp_tool.url}: {e}") - # Attempt cleanup before raising - try: - await introspection_toolset.close() - except Exception as cleanup_error: - logger.warning(f"Error during introspection toolset cleanup for {mcp_tool.name}: {cleanup_error}") - raise ConnectionError( f"Could not connect to MCP server '{mcp_tool.name}' at {mcp_tool.url}. " f"Ensure the server is running and accessible." ) from e - finally: - # ALWAYS close the introspection toolset to clean up async task groups - try: - logger.debug(f"Closing introspection connection to {mcp_tool.url}") - await introspection_toolset.close() - except Exception as cleanup_error: - # Log but don't raise - cleanup failures shouldn't block startup - logger.warning(f"Error closing introspection toolset for {mcp_tool.name}: {cleanup_error}") - - # PHASE 2: Create separate runtime toolset - # This toolset will be kept alive for the lifetime of the agent - logger.debug(f"Creating runtime toolset for {mcp_tool.url}") - runtime_toolset = McpToolset(connection_params=connection_params) - - # Add runtime toolset to the list (keep it alive for agent execution) - toolsets.append(runtime_toolset) - return toolsets, tool_descriptions