Skip to content

Commit 6d2eecf

Browse files
refactor(sdk): isolate trusted project MCP loading
Split project MCP loading into a dedicated helper and invoke it during agent readiness before plugin merging. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 7ffda54 commit 6d2eecf

1 file changed

Lines changed: 30 additions & 21 deletions

File tree

openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import uuid
33
from collections.abc import Mapping
44
from pathlib import Path
5+
from typing import Any
56

67
from openhands.sdk.agent.acp_agent import ACPAgent
78
from openhands.sdk.agent.base import AgentBase
@@ -42,13 +43,13 @@
4243
from openhands.sdk.llm.llm_registry import LLMRegistry
4344
from openhands.sdk.logger import get_logger
4445
from openhands.sdk.observability.laminar import observe
45-
from openhands.sdk.mcp.project_config import try_load_project_mcp_config
46+
from openhands.sdk.mcp.merge import merge_mcp_configs
47+
from openhands.sdk.mcp.project_config import load_project_mcp_config
4648
from openhands.sdk.plugin import (
4749
Plugin,
4850
PluginSource,
4951
ResolvedPluginSource,
5052
fetch_plugin_with_resolution,
51-
merge_mcp_configs,
5253
)
5354
from openhands.sdk.security.analyzer import SecurityAnalyzerBase
5455
from openhands.sdk.security.confirmation_policy import (
@@ -153,8 +154,7 @@ def __init__(
153154
lowercase alphanumeric, values up to 256 characters.
154155
trust_project_mcp: When True, load ``.openhands/.mcp.json`` or root
155156
``.mcp.json`` from the workspace and merge under user/agent MCP
156-
settings. UIs should set this only after the user approves
157-
project-scoped servers.
157+
settings.
158158
"""
159159
super().__init__() # Initialize with span tracking
160160
# Mark cleanup as initiated as early as possible to avoid races or partially
@@ -316,6 +316,25 @@ def resolved_plugins(self) -> list[ResolvedPluginSource] | None:
316316
"""
317317
return self._resolved_plugins
318318

319+
def _load_project_mcp_config(self) -> dict[str, Any] | None:
320+
"""Load project ``.mcp.json`` if trusted; update agent when merged differs.
321+
322+
Call only while holding ``self._state`` (same thread as other agent updates).
323+
"""
324+
if not self._trust_project_mcp:
325+
return None
326+
327+
project_dir = Path(self.workspace.working_dir)
328+
project_mcp = load_project_mcp_config(project_dir)
329+
if project_mcp is None:
330+
return None
331+
332+
merged = merge_mcp_configs(project_mcp, self.agent.mcp_config)
333+
if merged != self.agent.mcp_config:
334+
self.agent = self.agent.model_copy(update={"mcp_config": merged})
335+
self._state.agent = self.agent
336+
return merged
337+
319338
def _ensure_plugins_loaded(self) -> None:
320339
"""Lazy load plugins and set up hooks on first use.
321340
@@ -337,13 +356,7 @@ def _ensure_plugins_loaded(self) -> None:
337356
all_plugin_hooks: list[HookConfig] = []
338357
all_plugin_agents: list[AgentDefinition] = []
339358

340-
project_dir = Path(self.workspace.working_dir)
341-
project_mcp = (
342-
try_load_project_mcp_config(project_dir)
343-
if self._trust_project_mcp
344-
else None
345-
)
346-
merged_mcp = merge_mcp_configs(project_mcp, self.agent.mcp_config)
359+
merged_mcp = dict(self.agent.mcp_config)
347360

348361
# Load plugins if specified
349362
if self._plugin_specs:
@@ -397,11 +410,6 @@ def _ensure_plugins_loaded(self) -> None:
397410

398411
logger.info(f"Loaded {len(self._plugin_specs)} plugin(s) via Conversation")
399412

400-
elif merged_mcp != self.agent.mcp_config:
401-
self.agent = self.agent.model_copy(update={"mcp_config": merged_mcp})
402-
with self._state:
403-
self._state.agent = self.agent
404-
405413
# Register file-based agents defined in plugins
406414
if all_plugin_agents:
407415
register_plugin_agents(
@@ -463,10 +471,11 @@ def _ensure_agent_ready(self) -> None:
463471
Performs one-time lazy initialization on the first `send_message()`
464472
or `run()` call. The steps executed (in order) are:
465473
466-
1. Load plugins (merges skills, MCP config, and hooks).
467-
2. Register file-based agents into the agent registry.
468-
3. Initialize the agent with complete plugin config and hooks.
469-
4. Register LLMs in the LLM registry.
474+
1. Load project MCP config when trusted (merges under user settings).
475+
2. Load plugins (merges skills, MCP config, and hooks).
476+
3. Register file-based agents into the agent registry.
477+
4. Initialize the agent with complete configuration and hooks.
478+
5. Register LLMs in the LLM registry.
470479
471480
This preserves the design principle that constructors should not perform
472481
I/O or error-prone operations, while eliminating double initialization.
@@ -487,7 +496,7 @@ def _ensure_agent_ready(self) -> None:
487496
if self._agent_ready:
488497
return
489498

490-
# Load plugins first (merges skills, MCP config, hooks)
499+
self._load_project_mcp_config()
491500
self._ensure_plugins_loaded()
492501

493502
# register file-based agents

0 commit comments

Comments
 (0)