Skip to content

Commit 0b0fcf1

Browse files
refactor(sdk): replace AgentBase.mcp_config dict with MCPConfig | None
Replace the untyped dict[str, Any] field with the typed MCPConfig Pydantic model from fastmcp, aligning AgentBase with AgentSettings which already uses MCPConfig | None. Changes: - AgentBase.mcp_config: dict[str, Any] -> MCPConfig | None (default None) - Add field_validator (mode='before') to coerce dicts to MCPConfig for backward compatibility and normalize empty values to None - Update field_serializer to dump MCPConfig to dict before sanitizing - Update local_conversation.py and plugin/loader.py to bridge between dict (for plugin merge operations) and MCPConfig - Update settings/model.py to pass MCPConfig directly (no more .model_dump()) - Update all test files for MCPConfig type The field_validator ensures backward compatibility: callers passing plain dicts still work. Sanitization still prevents secret leakage through all serialization paths. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 2f4bff6 commit 0b0fcf1

File tree

10 files changed

+153
-66
lines changed

10 files changed

+153
-66
lines changed

openhands-sdk/openhands/sdk/agent/base.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
from concurrent.futures import ThreadPoolExecutor
99
from typing import TYPE_CHECKING, Any
1010

11+
from fastmcp.mcp_config import MCPConfig
1112
from pydantic import (
1213
BaseModel,
1314
ConfigDict,
1415
Field,
1516
PrivateAttr,
1617
field_serializer,
18+
field_validator,
1719
model_validator,
1820
)
1921

@@ -82,9 +84,9 @@ class AgentBase(DiscriminatedUnionMixin, ABC):
8284
},
8385
],
8486
)
85-
mcp_config: dict[str, Any] = Field(
86-
default_factory=dict,
87-
description="Optional MCP configuration dictionary to create MCP tools.",
87+
mcp_config: MCPConfig | None = Field(
88+
default=None,
89+
description="Optional MCP configuration to create MCP tools.",
8890
examples=[
8991
{"mcpServers": {"fetch": {"command": "uvx", "args": ["mcp-server-fetch"]}}}
9092
],
@@ -230,9 +232,24 @@ def _validate_system_prompt_fields(cls, data: Any) -> Any:
230232
),
231233
)
232234

235+
@field_validator("mcp_config", mode="before")
236+
@classmethod
237+
def _normalize_mcp_config(cls, v: Any) -> Any:
238+
"""Coerce dicts to MCPConfig and normalise empty values to None."""
239+
if v is None or v == {}:
240+
return None
241+
if isinstance(v, dict):
242+
return MCPConfig.model_validate(v)
243+
if isinstance(v, MCPConfig) and not v.mcpServers:
244+
return None
245+
return v
246+
233247
@field_serializer("mcp_config")
234248
@classmethod
235-
def _sanitize_mcp_config(cls, v: dict[str, Any]) -> dict[str, Any]:
249+
def _sanitize_mcp_config(
250+
cls,
251+
v: MCPConfig | None,
252+
) -> dict[str, Any]:
236253
"""Sanitize MCP config during serialization to prevent secret leakage.
237254
238255
MCP config may contain secrets in server headers (e.g. Authorization
@@ -243,7 +260,10 @@ def _sanitize_mcp_config(cls, v: dict[str, Any]) -> dict[str, Any]:
243260
The runtime agent always provides a fresh mcp_config on resume, so
244261
redacting here does not break conversation restore.
245262
"""
246-
return sanitize_config(v) if v else v
263+
if v is None:
264+
return {}
265+
raw = v.model_dump(exclude_none=True, exclude_defaults=True)
266+
return sanitize_config(raw) if raw else raw
247267

248268
# Runtime materialized tools; private and non-serializable
249269
_tools: dict[str, ToolDefinition] = PrivateAttr(default_factory=dict)
@@ -363,7 +383,7 @@ def _initialize(self, state: ConversationState):
363383
futures.append(future)
364384

365385
# Submit MCP tools creation if configured
366-
if self.mcp_config:
386+
if self.mcp_config and self.mcp_config.mcpServers:
367387
future = executor.submit(create_mcp_tools, self.mcp_config, 30)
368388
futures.append(future)
369389

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,13 @@ def _ensure_plugins_loaded(self) -> None:
335335

336336
# Start with agent's existing context and MCP config
337337
merged_context = self.agent.agent_context
338-
merged_mcp = dict(self.agent.mcp_config) if self.agent.mcp_config else {}
338+
merged_mcp = (
339+
self.agent.mcp_config.model_dump(
340+
exclude_none=True, exclude_defaults=True
341+
)
342+
if self.agent.mcp_config
343+
else {}
344+
)
339345

340346
for spec in self._plugin_specs:
341347
# Fetch plugin and get resolved commit SHA
@@ -368,11 +374,17 @@ def _ensure_plugins_loaded(self) -> None:
368374
if plugin.agents:
369375
all_plugin_agents.extend(plugin.agents)
370376

371-
# Update agent with merged content
377+
# Update agent with merged content; coerce merged dict to
378+
# MCPConfig so the typed field is satisfied (model_copy
379+
# bypasses validators).
380+
from fastmcp.mcp_config import MCPConfig
381+
372382
self.agent = self.agent.model_copy(
373383
update={
374384
"agent_context": merged_context,
375-
"mcp_config": merged_mcp,
385+
"mcp_config": (
386+
MCPConfig.model_validate(merged_mcp) if merged_mcp else None
387+
),
376388
}
377389
)
378390

openhands-sdk/openhands/sdk/plugin/loader.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,14 @@ def load_plugins(
6666
if not plugin_specs:
6767
return agent, None
6868

69-
# Start with agent's existing context and MCP config
69+
# Start with agent's existing context and MCP config (as dict for
70+
# plugin merge operations, then coerced back to MCPConfig).
7071
merged_context: AgentContext | None = agent.agent_context
71-
merged_mcp: dict[str, Any] = dict(agent.mcp_config) if agent.mcp_config else {}
72+
merged_mcp: dict[str, Any] = (
73+
agent.mcp_config.model_dump(exclude_none=True, exclude_defaults=True)
74+
if agent.mcp_config
75+
else {}
76+
)
7277
all_hooks: list[HookConfig] = []
7378

7479
for spec in plugin_specs:
@@ -100,11 +105,16 @@ def load_plugins(
100105
# Combine all hook configs (concatenation semantics)
101106
combined_hooks = HookConfig.merge(all_hooks)
102107

108+
# Coerce merged dict back to MCPConfig (model_copy bypasses validators).
109+
from fastmcp.mcp_config import MCPConfig
110+
103111
# Create updated agent with merged content
104112
updated_agent = agent.model_copy(
105113
update={
106114
"agent_context": merged_context,
107-
"mcp_config": merged_mcp,
115+
"mcp_config": (
116+
MCPConfig.model_validate(merged_mcp) if merged_mcp else None
117+
),
108118
}
109119
)
110120

openhands-sdk/openhands/sdk/settings/model.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ def create_agent(self) -> Agent:
552552
return Agent(
553553
llm=self.llm,
554554
tools=self.tools,
555-
mcp_config=self._serialize_mcp_config(self.mcp_config),
555+
mcp_config=self.mcp_config,
556556
agent_context=self.agent_context,
557557
condenser=self.build_condenser(self.llm),
558558
critic=self.build_critic(),

tests/sdk/agent/test_agent_serialization.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
"""Test agent JSON serialization with DiscriminatedUnionMixin."""
22

33
import json
4-
from typing import cast
54
from unittest.mock import Mock
65

76
import mcp.types
87
import pytest
8+
from fastmcp.mcp_config import MCPConfig
99
from pydantic import BaseModel
1010

1111
from openhands.sdk.agent import Agent
@@ -62,16 +62,20 @@ def test_mcp_tool_serialization():
6262
def test_agent_serialization_should_include_mcp_tool() -> None:
6363
# Create a simple LLM instance and agent with empty tools
6464
llm = LLM(model="test-model", usage_id="test-llm")
65-
mcp_config = {
65+
mcp_config_dict = {
6666
"mcpServers": {
6767
"dummy": {"command": "echo", "args": ["dummy-mcp"]},
6868
}
6969
}
70-
agent = Agent(llm=llm, tools=[], mcp_config=cast(dict[str, object], mcp_config))
70+
agent = Agent(
71+
llm=llm,
72+
tools=[],
73+
mcp_config=MCPConfig.model_validate(mcp_config_dict),
74+
)
7175

7276
# Serialize to JSON (excluding non-serializable fields)
7377
agent_dump = agent.model_dump()
74-
assert agent_dump.get("mcp_config") == mcp_config
78+
assert agent_dump.get("mcp_config") == mcp_config_dict
7579
agent_json = agent.model_dump_json()
7680

7781
# Deserialize from JSON using the base class

tests/sdk/agent/test_mcp_config_sanitization.py

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import uuid
1212
from pathlib import Path
1313

14+
from fastmcp.mcp_config import MCPConfig, RemoteMCPServer
1415
from pydantic import SecretStr
1516

1617
from openhands.sdk import Agent
@@ -22,10 +23,43 @@
2223
from openhands.sdk.workspace import LocalWorkspace
2324

2425

25-
def _make_agent(**kwargs) -> Agent:
26+
def _make_agent(
27+
mcp_config: dict | MCPConfig | None = None,
28+
) -> Agent:
2629
"""Helper to create an Agent with default LLM."""
2730
llm = LLM(model="gpt-4o-mini", api_key=SecretStr("test-key"), usage_id="test-llm")
28-
return Agent(llm=llm, tools=[], **kwargs)
31+
cfg: MCPConfig | None = None
32+
if isinstance(mcp_config, dict):
33+
cfg = MCPConfig.model_validate(mcp_config) if mcp_config else None
34+
else:
35+
cfg = mcp_config
36+
return Agent(llm=llm, tools=[], mcp_config=cfg)
37+
38+
39+
# -- Tests for MCPConfig type coercion --
40+
41+
42+
def test_mcp_config_dict_coerced_to_mcpconfig():
43+
"""Passing a raw dict is coerced to MCPConfig via the field validator."""
44+
agent = _make_agent(
45+
mcp_config={
46+
"mcpServers": {"fetch": {"command": "uvx", "args": ["mcp-server-fetch"]}}
47+
}
48+
)
49+
assert isinstance(agent.mcp_config, MCPConfig)
50+
assert "fetch" in agent.mcp_config.mcpServers
51+
52+
53+
def test_mcp_config_none_stays_none():
54+
"""None mcp_config remains None."""
55+
agent = _make_agent(mcp_config=None)
56+
assert agent.mcp_config is None
57+
58+
59+
def test_mcp_config_empty_dict_normalised_to_none():
60+
"""Empty dict is normalised to None."""
61+
agent = _make_agent(mcp_config={})
62+
assert agent.mcp_config is None
2963

3064

3165
# -- Tests for model_dump / model_dump_json redaction --
@@ -97,8 +131,8 @@ def test_mcp_config_api_key_field_redacted_on_dump():
97131
)
98132

99133

100-
def test_mcp_config_empty_dict_preserved():
101-
"""Empty mcp_config is preserved as-is (no crash)."""
134+
def test_mcp_config_empty_serialises_to_empty_dict():
135+
"""None mcp_config serialises as empty dict for backward compat."""
102136
agent = _make_agent(mcp_config={})
103137
dumped = agent.model_dump(mode="json")
104138
assert dumped["mcp_config"] == {}
@@ -164,15 +198,10 @@ def test_mcp_config_roundtrip_preserves_structure():
164198
json_str = agent.model_dump_json()
165199
restored = AgentBase.model_validate_json(json_str)
166200

167-
# Structure preserved, secrets redacted
168-
assert "shttp_server" in restored.mcp_config["mcpServers"]
169-
assert "stdio_server" in restored.mcp_config["mcpServers"]
170-
shttp = restored.mcp_config["mcpServers"]["shttp_server"]
171-
assert shttp["url"] == "https://example.com/mcp"
172-
assert shttp["headers"]["Authorization"] == "<redacted>"
173-
stdio = restored.mcp_config["mcpServers"]["stdio_server"]
174-
assert stdio["command"] == "npx"
175-
assert stdio["env"]["API_TOKEN"] == "<redacted>"
201+
# Structure preserved, secrets redacted; restored as MCPConfig
202+
assert isinstance(restored.mcp_config, MCPConfig)
203+
assert "shttp_server" in restored.mcp_config.mcpServers
204+
assert "stdio_server" in restored.mcp_config.mcpServers
176205

177206

178207
# -- Tests for ConversationStateUpdateEvent pathway --
@@ -282,28 +311,30 @@ def test_persisted_base_state_has_redacted_mcp_config():
282311

283312

284313
def test_mcp_config_runtime_value_unaffected():
285-
"""The in-memory mcp_config dict is NOT modified by serialization.
314+
"""The in-memory MCPConfig is NOT modified by serialization.
286315
287316
field_serializer only affects the output of model_dump/model_dump_json,
288317
not the actual field value on the instance.
289318
"""
290-
secret_config = {
291-
"mcpServers": {
292-
"slack": {
293-
"headers": {"Authorization": "Bearer real-token"},
319+
agent = _make_agent(
320+
mcp_config={
321+
"mcpServers": {
322+
"slack": {
323+
"url": "https://mcp.example.com",
324+
"headers": {"Authorization": "Bearer real-token"},
325+
}
294326
}
295327
}
296-
}
297-
agent = _make_agent(mcp_config=secret_config)
328+
)
298329

299330
# Serialize (triggers field_serializer)
300331
_ = agent.model_dump(mode="json")
301332

302-
# In-memory value is untouched (frozen model, so original dict is intact)
303-
assert (
304-
agent.mcp_config["mcpServers"]["slack"]["headers"]["Authorization"]
305-
== "Bearer real-token"
306-
)
333+
# In-memory MCPConfig is untouched
334+
assert isinstance(agent.mcp_config, MCPConfig)
335+
slack = agent.mcp_config.mcpServers["slack"]
336+
assert isinstance(slack, RemoteMCPServer)
337+
assert slack.headers["Authorization"] == "Bearer real-token"
307338

308339

309340
def test_multiple_servers_all_sanitized():

tests/sdk/conversation/test_local_conversation_plugins.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -256,23 +256,21 @@ def mock_create_mcp_tools(config, timeout):
256256
)
257257

258258
# Before loading plugins, no MCP config should exist
259-
assert (
260-
conversation.agent.mcp_config is None or conversation.agent.mcp_config == {}
261-
)
259+
assert conversation.agent.mcp_config is None
262260

263261
# Trigger plugin loading and agent initialization
264262
conversation._ensure_agent_ready()
265263

266264
# After loading, MCP config should be merged
267265
assert conversation.agent.mcp_config is not None
268-
assert "mcpServers" in conversation.agent.mcp_config
269-
assert "test-server" in conversation.agent.mcp_config["mcpServers"]
266+
assert "test-server" in conversation.agent.mcp_config.mcpServers
270267

271268
# The agent should have been initialized with the complete MCP config
272269
# This verifies that create_mcp_tools was called with the plugin's MCP config
273270
assert len(mcp_tools_created) > 0
274-
assert "mcpServers" in mcp_tools_created[-1]
275-
assert "test-server" in mcp_tools_created[-1]["mcpServers"]
271+
last_config = mcp_tools_created[-1]
272+
assert hasattr(last_config, "mcpServers")
273+
assert "test-server" in last_config.mcpServers
276274

277275
conversation.close()
278276

0 commit comments

Comments
 (0)