diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 579005dd..7d5c5816 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -418,7 +418,9 @@ class MCPToolWrapper(_MCPWrapperBase): logger.warning( "MCP tool '{}' timed out after {}s", self._name, self._tool_timeout ) - return f"(MCP tool call timed out after {self._tool_timeout}s)" + return ToolResult.error( + f"(MCP tool call timed out after {self._tool_timeout}s)" + ) except asyncio.CancelledError: # MCP SDK's anyio cancel scopes can leak CancelledError on timeout/failure. # Re-raise only if our task was externally cancelled (e.g. /stop). @@ -426,7 +428,7 @@ class MCPToolWrapper(_MCPWrapperBase): if task is not None and task.cancelling() > 0: raise logger.warning("MCP tool '{}' was cancelled by server/SDK", self._name) - return "(MCP tool call was cancelled)" + return ToolResult.error("(MCP tool call was cancelled)") except Exception as exc: if await self._refresh_session_after_termination( exc, @@ -451,20 +453,35 @@ class MCPToolWrapper(_MCPWrapperBase): self._name, type(exc).__name__, ) - return f"(MCP tool call failed after retry: {type(exc).__name__})" + return ToolResult.error( + f"(MCP tool call failed after retry: {type(exc).__name__})" + ) logger.exception( "MCP tool '{}' failed: {}: {}", self._name, type(exc).__name__, exc, ) - return f"(MCP tool call failed: {type(exc).__name__})" + return ToolResult.error( + f"(MCP tool call failed: {type(exc).__name__})" + ) else: # Success — extract text and persist any image content as artifacts. - rendered = self._render_call_result(result.content, kwargs) - if getattr(result, "isError", False): - return ToolResult.error(rendered) - return rendered + try: + rendered = self._render_call_result(result.content, kwargs) + if getattr(result, "isError", False): + return ToolResult.error(rendered) + return rendered + except Exception as exc: + logger.exception( + "MCP tool '{}' failed while rendering result: {}: {}", + self._name, + type(exc).__name__, + exc, + ) + return ToolResult.error( + f"(MCP tool returned malformed content: {type(exc).__name__})" + ) return "(MCP tool call failed)" # Unreachable, but satisfies type checkers diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 7fe486d0..1a915f90 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -320,6 +320,35 @@ async def test_execute_wraps_mcp_is_error_result() -> None: assert is_tool_error_result(wrapper.name, result) +@pytest.mark.asyncio +async def test_execute_contains_malformed_success_result() -> None: + async def call_tool(_name: str, arguments: dict) -> object: + return SimpleNamespace(content=None) + + wrapper = _make_wrapper(SimpleNamespace(call_tool=call_tool)) + + result = await wrapper.execute() + + assert result == "(MCP tool returned malformed content: TypeError)" + assert is_tool_error_result(wrapper.name, result) + + +@pytest.mark.asyncio +async def test_registry_adds_retry_hint_to_malformed_mcp_result() -> None: + async def call_tool(_name: str, arguments: dict) -> object: + return SimpleNamespace(content=None) + + wrapper = _make_wrapper(SimpleNamespace(call_tool=call_tool)) + registry = ToolRegistry() + registry.register(wrapper) + + result = await registry.execute(wrapper.name, {}) + + assert is_tool_error_result(wrapper.name, result) + assert "MCP tool returned malformed content" in result + assert "Analyze the error above and try a different approach" in result + + @pytest.mark.asyncio async def test_execute_preserves_success_text_that_starts_with_error() -> None: async def call_tool(_name: str, arguments: dict) -> object: @@ -401,6 +430,7 @@ async def test_execute_returns_timeout_message() -> None: result = await wrapper.execute() assert result == "(MCP tool call timed out after 0.01s)" + assert is_tool_error_result(wrapper.name, result) @pytest.mark.asyncio @@ -413,6 +443,7 @@ async def test_execute_handles_server_cancelled_error() -> None: result = await wrapper.execute() assert result == "(MCP tool call was cancelled)" + assert is_tool_error_result(wrapper.name, result) @pytest.mark.asyncio @@ -444,6 +475,7 @@ async def test_execute_handles_generic_exception() -> None: result = await wrapper.execute() assert result == "(MCP tool call failed: RuntimeError)" + assert is_tool_error_result(wrapper.name, result) def _make_tool_def(name: str) -> SimpleNamespace: