From c3faffd8684b376324d3ab67ed1113fe38424ebe Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:32:06 +0000 Subject: [PATCH] fix: initialize tools_list and log warning in _resolve_native when no tools found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5116. MCPToolResolver._resolve_native() could raise UnboundLocalError when asyncio.run() raised a RuntimeError that didn't match the 'cancel scope' or 'task' conditions — the exception was silently swallowed leaving tools_list unbound. Changes: - Initialize tools_list to an empty list before the try block - Add early return with warning log when tools_list is empty (matching the existing behavior in _resolve_external) This covers three scenarios: 1. MCP server returns no tools 2. tool_filter removes all tools 3. RuntimeError swallowed without assigning tools_list Co-Authored-By: João --- lib/crewai/src/crewai/mcp/tool_resolver.py | 8 ++ lib/crewai/tests/mcp/test_amp_mcp.py | 113 +++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/lib/crewai/src/crewai/mcp/tool_resolver.py b/lib/crewai/src/crewai/mcp/tool_resolver.py index 92b1e488c..c828a9c97 100644 --- a/lib/crewai/src/crewai/mcp/tool_resolver.py +++ b/lib/crewai/src/crewai/mcp/tool_resolver.py @@ -354,6 +354,7 @@ class MCPToolResolver: ) from e try: + tools_list: list[dict[str, Any]] = [] try: asyncio.get_running_loop() import concurrent.futures @@ -408,6 +409,13 @@ class MCPToolResolver: cache_tools_list=mcp_config.cache_tools_list, ) + if not tools_list: + self._logger.log( + "warning", + f"No tools discovered from MCP server: {server_name}", + ) + return cast(list[BaseTool], []), [] + tools = [] for tool_def in tools_list: tool_name = tool_def.get("name", "") diff --git a/lib/crewai/tests/mcp/test_amp_mcp.py b/lib/crewai/tests/mcp/test_amp_mcp.py index f13484a8d..1a309d7cd 100644 --- a/lib/crewai/tests/mcp/test_amp_mcp.py +++ b/lib/crewai/tests/mcp/test_amp_mcp.py @@ -1,5 +1,6 @@ """Tests for AMP MCP config fetching and tool resolution.""" +import asyncio from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -503,3 +504,115 @@ class TestResolveExternalToolFilter: tools = resolver._resolve_external("https://mcp.example.com/api#nonexistent") assert len(tools) == 0 + + +class TestResolveNativeEmptyTools: + """Tests for _resolve_native when MCP server returns no tools or tools_list is unbound.""" + + @pytest.fixture + def agent(self): + return Agent( + role="Test Agent", + goal="Test goal", + backstory="Test backstory", + ) + + @pytest.fixture + def mock_logger(self): + logger = MagicMock() + logger.log = MagicMock() + return logger + + @pytest.fixture + def resolver(self, agent, mock_logger): + return MCPToolResolver(agent=agent, logger=mock_logger) + + @patch("crewai.mcp.tool_resolver.MCPClient") + def test_returns_empty_and_logs_warning_when_server_returns_no_tools( + self, mock_client_class, resolver + ): + """When the MCP server returns an empty tool list, _resolve_native should + return empty tools and log a warning instead of raising an error.""" + mock_client = AsyncMock() + mock_client.list_tools = AsyncMock(return_value=[]) + mock_client.connected = False + mock_client.connect = AsyncMock() + mock_client.disconnect = AsyncMock() + mock_client_class.return_value = mock_client + + http_config = MCPServerHTTP( + url="https://empty-server.example.com/api", + headers={"Authorization": "Bearer token"}, + ) + + tools, clients = resolver._resolve_native(http_config) + + assert tools == [] + assert clients == [] + resolver._logger.log.assert_any_call( + "warning", + "No tools discovered from MCP server: empty-server_example_com_api", + ) + + @patch("crewai.mcp.tool_resolver.MCPClient") + def test_returns_empty_and_logs_warning_when_tool_filter_removes_all( + self, mock_client_class, resolver + ): + """When the tool_filter removes all tools, _resolve_native should + return empty tools and log a warning.""" + mock_client = AsyncMock() + mock_client.list_tools = AsyncMock( + return_value=[ + {"name": "search", "description": "Search tool", "inputSchema": {}}, + ] + ) + mock_client.connected = False + mock_client.connect = AsyncMock() + mock_client.disconnect = AsyncMock() + mock_client_class.return_value = mock_client + + http_config = MCPServerHTTP( + url="https://filtered-server.example.com/api", + headers={"Authorization": "Bearer token"}, + tool_filter=lambda tool: False, # reject all tools + ) + + tools, clients = resolver._resolve_native(http_config) + + assert tools == [] + assert clients == [] + resolver._logger.log.assert_any_call( + "warning", + "No tools discovered from MCP server: filtered-server_example_com_api", + ) + + @patch("crewai.mcp.tool_resolver.MCPClient") + def test_no_unbound_local_error_when_runtime_error_swallowed( + self, mock_client_class, resolver + ): + """When asyncio.run raises a RuntimeError that doesn't match the + 'cancel scope' / 'task' conditions, tools_list must not be unbound. + Previously this caused UnboundLocalError on line 412.""" + mock_client = AsyncMock() + mock_client.connected = False + mock_client.connect = AsyncMock() + mock_client.disconnect = AsyncMock() + # Make list_tools raise a RuntimeError that does NOT match the conditions + mock_client.list_tools = AsyncMock( + side_effect=RuntimeError("some unrelated runtime error") + ) + mock_client_class.return_value = mock_client + + http_config = MCPServerHTTP( + url="https://broken-server.example.com/api", + headers={"Authorization": "Bearer token"}, + ) + + tools, clients = resolver._resolve_native(http_config) + + assert tools == [] + assert clients == [] + resolver._logger.log.assert_any_call( + "warning", + "No tools discovered from MCP server: broken-server_example_com_api", + )