Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions openhands-sdk/openhands/sdk/agent/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from openhands.sdk.llm import LLM
from openhands.sdk.llm.utils.model_prompt_spec import get_model_prompt_spec
from openhands.sdk.logger import get_logger
from openhands.sdk.mcp import create_mcp_tools
from openhands.sdk.mcp import create_mcp_tools_graceful
from openhands.sdk.tool import (
BUILT_IN_TOOL_CLASSES,
BUILT_IN_TOOLS,
Expand Down Expand Up @@ -318,6 +318,7 @@ def _initialize(self, state: ConversationState):
return

tools: list[ToolDefinition] = []
mcp_errors: list[str] = []

# Use ThreadPoolExecutor to parallelize tool resolution
with ThreadPoolExecutor(max_workers=4) as executor:
Expand All @@ -328,16 +329,33 @@ def _initialize(self, state: ConversationState):
future = executor.submit(resolve_tool, tool_spec, state)
futures.append(future)

# Submit MCP tools creation if configured
# Submit MCP tools creation if configured (using graceful degradation)
mcp_future = None
if self.mcp_config:
future = executor.submit(create_mcp_tools, self.mcp_config, 30)
futures.append(future)
mcp_future = executor.submit(
create_mcp_tools_graceful, self.mcp_config, 30
)

# Collect results as they complete
# Collect regular tool results
for future in futures:
result = future.result()
tools.extend(result)

# Collect MCP results with graceful degradation
if mcp_future is not None:
mcp_result = mcp_future.result()
tools.extend(mcp_result.tools)
if mcp_result.has_errors:
for err in mcp_result.errors:
mcp_errors.append(f"{err.server_name}: {err}")

# Log MCP errors but continue with available tools
if mcp_errors:
logger.warning(
"Some MCP servers failed to initialize:\n"
+ "\n".join(f" - {e}" for e in mcp_errors)
)

logger.info(
f"Loaded {len(tools)} tools from spec: {[tool.name for tool in tools]}"
)
Expand Down
7 changes: 6 additions & 1 deletion openhands-sdk/openhands/sdk/mcp/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

from openhands.sdk.mcp.client import MCPClient
from openhands.sdk.mcp.definition import MCPToolAction, MCPToolObservation
from openhands.sdk.mcp.exceptions import MCPError, MCPTimeoutError
from openhands.sdk.mcp.exceptions import MCPError, MCPServerError, MCPTimeoutError
from openhands.sdk.mcp.tool import (
MCPToolDefinition,
MCPToolExecutor,
)
from openhands.sdk.mcp.utils import (
MCPToolsResult,
create_mcp_tools,
create_mcp_tools_graceful,
)


Expand All @@ -18,7 +20,10 @@
"MCPToolAction",
"MCPToolObservation",
"MCPToolExecutor",
"MCPToolsResult",
"create_mcp_tools",
"create_mcp_tools_graceful",
"MCPError",
"MCPServerError",
"MCPTimeoutError",
]
15 changes: 15 additions & 0 deletions openhands-sdk/openhands/sdk/mcp/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,18 @@ def __init__(self, message: str, timeout: float, config: dict | None = None):
self.timeout = timeout
self.config = config
super().__init__(message)


class MCPServerError(MCPError):
"""Exception raised when an individual MCP server fails to initialize.

Contains details about which server failed and the underlying cause.
"""

server_name: str
cause: Exception | None

def __init__(self, message: str, server_name: str, cause: Exception | None = None):
self.server_name = server_name
self.cause = cause
super().__init__(message)
133 changes: 132 additions & 1 deletion openhands-sdk/openhands/sdk/mcp/utils.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,50 @@
"""Utility functions for MCP integration."""

import logging
from dataclasses import dataclass, field
from typing import TYPE_CHECKING

import mcp.types
from fastmcp.client.logging import LogMessage
from fastmcp.mcp_config import MCPConfig

from openhands.sdk.logger import get_logger
from openhands.sdk.mcp.client import MCPClient
from openhands.sdk.mcp.exceptions import MCPTimeoutError
from openhands.sdk.mcp.exceptions import MCPServerError, MCPTimeoutError
from openhands.sdk.mcp.tool import MCPToolDefinition


if TYPE_CHECKING:
from fastmcp.mcp_config import MCPServerTypes

logger = get_logger(__name__)
LOGGING_LEVEL_MAP = logging.getLevelNamesMapping()


@dataclass
class MCPToolsResult:
"""Result of creating MCP tools with graceful degradation.

Contains both the successfully initialized tools and any errors that occurred
during initialization of individual servers.
"""

tools: list[MCPToolDefinition] = field(default_factory=list)
errors: list[MCPServerError] = field(default_factory=list)

@property
def has_errors(self) -> bool:
"""Return True if any server failed to initialize."""
return len(self.errors) > 0

def error_summary(self) -> str:
"""Return a human-readable summary of all errors."""
if not self.errors:
return ""
parts = [f"- {err.server_name}: {err}" for err in self.errors]
return "MCP server initialization failures:\n" + "\n".join(parts)


async def log_handler(message: LogMessage):
"""
Handles incoming logs from the MCP server and forwards them
Expand Down Expand Up @@ -89,3 +118,105 @@ def create_mcp_tools(

logger.info(f"Created {len(client.tools)} MCP tools: {[t.name for t in client]}")
return client


def _create_single_server_tools(
server_name: str,
server_config: "MCPServerTypes",
timeout: float,
) -> MCPClient:
"""Create MCP tools for a single server.

Internal helper used by create_mcp_tools_graceful.
Raises exceptions on failure (caller handles graceful degradation).
"""
single_server_config = MCPConfig(mcpServers={server_name: server_config})
client = MCPClient(single_server_config, log_handler=log_handler)

try:
client.call_async_from_sync(
_connect_and_list_tools, timeout=timeout, client=client
)
except TimeoutError as e:
client.sync_close()
raise MCPTimeoutError(
f"MCP server '{server_name}' timed out after {timeout} seconds",
timeout=timeout,
config=single_server_config.model_dump(),
) from e
except BaseException:
try:
client.sync_close()
except Exception as close_exc:
logger.warning(
f"Failed to close MCP client for '{server_name}' during error cleanup",
exc_info=close_exc,
)
raise

return client


def create_mcp_tools_graceful(
config: dict | MCPConfig,
timeout: float = 30.0,
) -> MCPToolsResult:
"""Create MCP tools with per-server graceful degradation.

Unlike create_mcp_tools() which fails if ANY server fails, this function
attempts to initialize each MCP server individually and continues even
if some servers fail.

Args:
config: MCP configuration dictionary or MCPConfig object containing
server definitions.
timeout: Timeout in seconds for each server connection (default: 30.0).

Returns:
MCPToolsResult containing:
- tools: List of successfully loaded MCPToolDefinitions from all servers
- errors: List of MCPServerError for any servers that failed

Example:
result = create_mcp_tools_graceful(config)
if result.has_errors:
logger.warning(result.error_summary())
for tool in result.tools:
# use tool
"""
if isinstance(config, dict):
config = MCPConfig.model_validate(config)

if not config.mcpServers:
return MCPToolsResult()

result = MCPToolsResult()

for server_name, server_config in config.mcpServers.items():
try:
client = _create_single_server_tools(server_name, server_config, timeout)
result.tools.extend(client.tools)
logger.info(
f"MCP server '{server_name}' connected: {len(client.tools)} tools"
)
except Exception as e:
error = MCPServerError(
message=str(e),
server_name=server_name,
cause=e,
)
result.errors.append(error)
logger.warning(f"MCP server '{server_name}' failed to initialize: {e}")

if result.tools:
logger.info(
f"Created {len(result.tools)} MCP tools from "
f"{len(config.mcpServers) - len(result.errors)} servers"
)
if result.errors:
logger.warning(
f"{len(result.errors)} MCP server(s) failed: "
f"{[e.server_name for e in result.errors]}"
)

return result
15 changes: 10 additions & 5 deletions tests/sdk/conversation/test_local_conversation_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,21 @@ def test_plugin_mcp_config_is_initialized(
This is a regression test for a bug where MCP tools from plugins were not
being created because the agent was initialized before plugins were loaded.
"""
# Mock create_mcp_tools to avoid actually starting MCP servers in tests
from openhands.sdk.mcp import MCPToolsResult

# Mock create_mcp_tools_graceful to avoid actually starting MCP servers
mcp_tools_created = []

def mock_create_mcp_tools(config, timeout):
def mock_create_mcp_tools_graceful(config, timeout):
mcp_tools_created.append(config)
return [] # Return empty list for testing
return MCPToolsResult(tools=[], errors=[]) # Return empty result

import openhands.sdk.agent.base

monkeypatch.setattr(
openhands.sdk.agent.base, "create_mcp_tools", mock_create_mcp_tools
openhands.sdk.agent.base,
"create_mcp_tools_graceful",
mock_create_mcp_tools_graceful,
)

plugin_dir = create_test_plugin(
Expand Down Expand Up @@ -269,7 +273,8 @@ def mock_create_mcp_tools(config, timeout):
assert "test-server" in conversation.agent.mcp_config["mcpServers"]

# The agent should have been initialized with the complete MCP config
# This verifies that create_mcp_tools was called with the plugin's MCP config
# This verifies that create_mcp_tools_graceful was called with the plugin's
# MCP config
assert len(mcp_tools_created) > 0
assert "mcpServers" in mcp_tools_created[-1]
assert "test-server" in mcp_tools_created[-1]["mcpServers"]
Expand Down
Loading
Loading