fix(sdk): tolerate unavailable MCP servers

Treat MCP connection failures as optional by default while retaining an explicit strict mode, diagnostic server targets, and cleanup that preserves the original error. Add regressions for mixed-server discovery and LocalConversation startup.

Co-authored-by: openhands <openhands@all-hands.dev>

Author: jstar0

openhands-sdk/openhands/sdk/mcp/client.py

@@ -103,7 +103,7 @@ def sync_close(self) -> None:
         if hasattr(self, "close") and inspect.iscoroutinefunction(self.close):
             try:
                 self._executor.run_async(self.close, timeout=10.0)
-            except Exception:
+            except BaseException:  # noqa: BLE001 - cleanup must never mask the original error
                 pass  # Ignore close errors during cleanup
 
         # Always cleanup the executor

openhands-sdk/openhands/sdk/mcp/utils.py

@@ -22,8 +22,9 @@
     enabled_mcp_servers,
     to_fastmcp_mcp_config,
 )
-from openhands.sdk.mcp.exceptions import MCPTimeoutError
+from openhands.sdk.mcp.exceptions import MCPError, MCPTimeoutError
 from openhands.sdk.mcp.tool import MCPToolDefinition
+from openhands.sdk.utils.redact import redact_url_params
 
 
 logger = get_logger(__name__)
@@ -46,6 +47,7 @@ def create_tools(
         mcp_config: dict[str, MCPServer],
         timeout: float = 30.0,
         *,
+        strict: bool = False,
         on_tools_changed: ToolsChangedCallback | None = None,
         on_tools_reconciled: ToolsReconciledCallback | None = None,
     ) -> MCPClient: ...
@@ -59,12 +61,14 @@ def create_tools(
         mcp_config: dict[str, MCPServer],
         timeout: float = 30.0,
         *,
+        strict: bool = False,
         on_tools_changed: ToolsChangedCallback | None = None,
         on_tools_reconciled: ToolsReconciledCallback | None = None,
     ) -> MCPClient:
         return create_mcp_tools(
             mcp_config,
             timeout,
+            strict=strict,
             on_tools_changed=on_tools_changed,
             on_tools_reconciled=on_tools_reconciled,
         )
@@ -312,10 +316,43 @@ async def _refresh_tools(self) -> None:
             )
 
 
+def _server_target(name: str, server: MCPServer) -> str:
+    if server.url is not None:
+        return f"{name!r} at {redact_url_params(server.url)!r}"
+    if server.command is not None:
+        return f"{name!r} using command {server.command!r}"
+    return repr(name)
+
+
+def _connection_failure_message(
+    mcp_config: Mapping[str, MCPServer], error: BaseException
+) -> str:
+    targets = ", ".join(
+        _server_target(name, server) for name, server in mcp_config.items()
+    )
+    detail = str(error.__cause__ or error)
+    return (
+        f"Failed to connect to MCP server(s): {targets}. {detail}\n"
+        "Possible solutions:\n"
+        "  1. Check if the MCP server is running and responding\n"
+        "  2. Verify network connectivity to the MCP server"
+    )
+
+
+def _close_client_quietly(client: MCPClient) -> None:
+    try:
+        client.sync_close()
+    except BaseException as close_error:  # noqa: BLE001 - cleanup must not mask the original error
+        logger.debug(
+            "Failed to close MCP client during error cleanup", exc_info=close_error
+        )
+
+
 def create_mcp_tools(
     mcp_config: dict[str, MCPServer],
     timeout: float = 30.0,
     *,
+    strict: bool = False,
     on_tools_changed: ToolsChangedCallback | None = None,
     on_tools_reconciled: ToolsReconciledCallback | None = None,
     mcp_oauth_token_storage: AsyncKeyValue | None = None,
@@ -330,6 +367,10 @@ def create_mcp_tools(
                 # use tool
         # Connection automatically closed
 
+    By default, an unavailable server is logged and skipped so an optional
+    MCP server cannot prevent the agent from starting. Pass ``strict=True``
+    to preserve fail-fast behavior for configurations that require MCP.
+
     The client subscribes to ``notifications/tools/list_changed`` and
     reconciles its tool list whenever the server signals a change. When
     ``on_tools_changed`` is provided, the client invokes it with newly added
@@ -365,7 +406,7 @@ def create_mcp_tools(
             _connect_and_list_tools, timeout=timeout, client=client
         )
     except TimeoutError as e:
-        client.sync_close()
+        _close_client_quietly(client)
         # Extract server names from config for better error message
         server_names = (
             list(config.mcpServers.keys()) if config.mcpServers else ["unknown"]
@@ -381,13 +422,18 @@ def create_mcp_tools(
         raise MCPTimeoutError(
             error_msg, timeout=timeout, config=config.model_dump()
         ) from e
+    except (MCPError, ConnectionError) as e:
+        error_msg = _connection_failure_message(mcp_config, e)
+        _close_client_quietly(client)
+        if strict:
+            raise MCPError(error_msg) from e
+        logger.warning(
+            "%s. Continuing without MCP tools; pass strict=True to fail fast.",
+            error_msg,
+        )
+        return client
     except BaseException:
-        try:
-            client.sync_close()
-        except Exception as close_exc:
-            logger.warning(
-                "Failed to close MCP client during error cleanup", exc_info=close_exc
-            )
+        _close_client_quietly(client)
         raise
 
     logger.info("Created %d MCP tools", len(client.tools))

tests/sdk/mcp/test_create_mcp_tool.py

@@ -19,7 +19,12 @@
 from key_value.aio.stores.memory import MemoryStore
 from pydantic import SecretStr
 
+from openhands.sdk.agent import Agent
+from openhands.sdk.conversation.impl.local_conversation import LocalConversation
+from openhands.sdk.conversation.state import ConversationExecutionStatus
+from openhands.sdk.llm import Message, TextContent
 from openhands.sdk.mcp import create_mcp_tools
+from openhands.sdk.mcp.client import MCPClient
 from openhands.sdk.mcp.config import (
     MCPApiKeyAuthCredential,
     MCPBasicAuthCredential,
@@ -32,6 +37,7 @@
 )
 from openhands.sdk.mcp.exceptions import MCPError, MCPTimeoutError
 from openhands.sdk.mcp.utils import _prepare_mcp_config
+from openhands.sdk.testing import TestLLM
 
 
 logger = logging.getLogger(__name__)
@@ -649,6 +655,170 @@ def test_create_mcp_tools_connection_to_nonexistent_server():
         pass  # Expected connection errors are acceptable
 
 
+def test_unreachable_server_is_skipped_with_diagnostic_warning(caplog):
+    """An unavailable optional server must not prevent tool creation."""
+    config = native_mcp_config(
+        {
+            "mcpServers": {
+                "broken": {
+                    "transport": "http",
+                    "url": "http://127.0.0.1:59999/mcp?api_key=secret",
+                }
+            }
+        }
+    )
+
+    with caplog.at_level(logging.WARNING):
+        tools = create_mcp_tools(config, timeout=5.0)
+
+    assert len(tools) == 0
+    assert "broken" in caplog.text
+    assert "http://127.0.0.1:59999/mcp?api_key=%3Credacted%3E" in caplog.text
+    assert "secret" not in caplog.text
+    assert "Possible solutions" in caplog.text
+    assert "strict=True" in caplog.text
+
+
+def test_unreachable_server_can_fail_fast_in_strict_mode():
+    """Strict mode retains the opt-in fail-fast behavior with context."""
+    config = native_mcp_config(
+        {
+            "mcpServers": {
+                "broken": {
+                    "transport": "http",
+                    "url": "http://127.0.0.1:59999/mcp",
+                }
+            }
+        }
+    )
+
+    with pytest.raises(MCPError) as exc_info:
+        create_mcp_tools(config, timeout=5.0, strict=True)
+
+    assert "broken" in str(exc_info.value)
+    assert "http://127.0.0.1:59999/mcp" in str(exc_info.value)
+    assert exc_info.value.__cause__ is not None
+    assert exc_info.value.__cause__.__cause__ is not None
+
+
+def test_reachable_server_tools_survive_unreachable_server(
+    http_mcp_server: MCPTestServer,
+):
+    """A failed optional server must not discard tools from a healthy server."""
+    config = native_mcp_config(
+        {
+            "mcpServers": {
+                "healthy": {
+                    "transport": "http",
+                    "url": f"http://127.0.0.1:{http_mcp_server.port}/mcp",
+                },
+                "broken": {
+                    "transport": "http",
+                    "url": "http://127.0.0.1:59999/mcp",
+                },
+            }
+        }
+    )
+
+    tools = create_mcp_tools(config, timeout=10.0)
+
+    assert {tool.name for tool in tools} == {"healthy_greet", "healthy_add_numbers"}
+
+
+def test_local_conversation_runs_with_unreachable_mcp_server(tmp_path: Path, caplog):
+    """An unavailable MCP source must not block the agent's LLM path."""
+    llm = TestLLM.from_messages(
+        [Message(role="assistant", content=[TextContent(text="done")])]
+    )
+    agent = Agent(
+        llm=llm,
+        tools=[],
+        include_default_tools=[],
+        mcp_config=native_mcp_config(
+            {
+                "mcpServers": {
+                    "broken": {
+                        "transport": "http",
+                        "url": "http://127.0.0.1:59999/mcp",
+                    }
+                }
+            }
+        ),
+    )
+    conversation = LocalConversation(
+        agent=agent,
+        workspace=str(tmp_path),
+        visualizer=None,
+    )
+
+    try:
+        with caplog.at_level(logging.WARNING):
+            conversation.send_message("hello")
+            conversation.run()
+    finally:
+        conversation.close()
+
+    assert conversation.state.execution_status == ConversationExecutionStatus.FINISHED
+    assert llm.call_count == 1
+    assert "broken" in caplog.text
+
+
+def test_cleanup_failure_does_not_mask_connection_failure():
+    """Cleanup errors must not replace the original MCP connection error."""
+    config = native_mcp_config(
+        {
+            "mcpServers": {
+                "broken": {
+                    "transport": "http",
+                    "url": "http://127.0.0.1:59999/mcp",
+                }
+            }
+        }
+    )
+
+    with patch("openhands.sdk.mcp.utils.MCPClient") as mock_client_class:
+        mock_client = MagicMock()
+        mock_client_class.return_value = mock_client
+        mock_client.call_async_from_sync.side_effect = MCPError(
+            "MCP Connection Failure"
+        )
+        mock_client.sync_close.side_effect = BaseException("cleanup failed")
+
+        with pytest.raises(MCPError, match="broken") as exc_info:
+            create_mcp_tools(config, timeout=5.0, strict=True)
+
+    assert "MCP Connection Failure" in str(exc_info.value.__cause__)
+
+
+def test_sync_close_suppresses_base_exception_from_async_close():
+    """Client cleanup must handle cancellation-style BaseException values."""
+    client = MCPClient(
+        FastMCPConfig.model_validate(
+            {
+                "mcpServers": {
+                    "server": {
+                        "transport": "http",
+                        "url": "http://127.0.0.1:59999/mcp",
+                    }
+                }
+            }
+        )
+    )
+    with (
+        patch.object(
+            client._executor,
+            "run_async",
+            side_effect=BaseException("cleanup failed"),
+        ) as run_async,
+        patch.object(client._executor, "close") as executor_close,
+    ):
+        client.sync_close()
+
+    run_async.assert_called_once()
+    executor_close.assert_called_once()
+    assert client._closed is True
+
+
 def test_create_mcp_tools_stdio_server():
     """Test creating MCP tools from a native server map."""
     mcp_config = stdio_fetch_mcp_config()