From b31dbfc21add1a99fb4d9f27c0c0adda34f4e88d Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Mon, 3 Nov 2025 12:45:47 -0500 Subject: [PATCH] CLI: make sure MCP server doesn't persist even after removal (#11602) Co-authored-by: openhands --- .../openhands_cli/tui/settings/store.py | 4 +- .../test_mcp_settings_reconciliation.py | 114 ++++++++++++++++++ 2 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 openhands-cli/tests/settings/test_mcp_settings_reconciliation.py diff --git a/openhands-cli/openhands_cli/tui/settings/store.py b/openhands-cli/openhands_cli/tui/settings/store.py index 1cd43fd74e..5b77112105 100644 --- a/openhands-cli/openhands_cli/tui/settings/store.py +++ b/openhands-cli/openhands_cli/tui/settings/store.py @@ -45,9 +45,7 @@ class AgentStore: system_message_suffix=f'You current working directory is: {WORK_DIR}', ) - additional_mcp_config = self.load_mcp_configuration() - mcp_config: dict = agent.mcp_config.copy().get('mcpServers', {}) - mcp_config.update(additional_mcp_config) + mcp_config: dict = self.load_mcp_configuration() # Update LLM metadata with current information agent_llm_metadata = get_llm_metadata( diff --git a/openhands-cli/tests/settings/test_mcp_settings_reconciliation.py b/openhands-cli/tests/settings/test_mcp_settings_reconciliation.py new file mode 100644 index 0000000000..65a5687335 --- /dev/null +++ b/openhands-cli/tests/settings/test_mcp_settings_reconciliation.py @@ -0,0 +1,114 @@ +"""Minimal tests: mcp.json overrides persisted agent MCP servers.""" + +import json +from pathlib import Path +from unittest.mock import patch +import pytest +from pydantic import SecretStr + +from openhands.sdk import Agent, LLM +from openhands_cli.locations import MCP_CONFIG_FILE, AGENT_SETTINGS_PATH +from openhands_cli.tui.settings.store import AgentStore + + +# ---------------------- tiny helpers ---------------------- + +def write_json(path: Path, obj: dict) -> None: + path.write_text(json.dumps(obj)) + + +def write_agent(root: Path, agent: Agent) -> None: + (root / AGENT_SETTINGS_PATH).write_text( + agent.model_dump_json(context={"expose_secrets": True}) + ) + + +# ---------------------- fixtures ---------------------- + +@pytest.fixture +def persistence_dir(tmp_path, monkeypatch) -> Path: + # Create root dir and point AgentStore at it + root = tmp_path / "openhands" + root.mkdir() + monkeypatch.setattr("openhands_cli.tui.settings.store.PERSISTENCE_DIR", str(root)) + return root + + +@pytest.fixture +def agent_store() -> AgentStore: + return AgentStore() + + +# ---------------------- tests ---------------------- + +@patch("openhands_cli.tui.settings.store.get_default_tools", return_value=[]) +@patch("openhands_cli.tui.settings.store.get_llm_metadata", return_value={}) +def test_load_overrides_persisted_mcp_with_mcp_json_file( + mock_meta, + mock_tools, + persistence_dir, + agent_store +): + """If agent has MCP servers, mcp.json must replace them entirely.""" + # Persist an agent that already contains MCP servers + persisted_agent = Agent( + llm=LLM(model="gpt-4", api_key=SecretStr("k"), usage_id="svc"), + tools=[], + mcp_config={ + "mcpServers": { + "persistent_server": {"command": "python", "args": ["-m", "old_server"]} + } + }, + ) + write_agent(persistence_dir, persisted_agent) + + # Create mcp.json with different servers (this must fully override) + write_json( + persistence_dir / MCP_CONFIG_FILE, + { + "mcpServers": { + "file_server": {"command": "uvx", "args": ["mcp-server-fetch"]} + } + }, + ) + + loaded = agent_store.load() + assert loaded is not None + # Expect ONLY the MCP json file's config + assert loaded.mcp_config == { + "mcpServers": { + "file_server": { + "command": "uvx", + "args": ["mcp-server-fetch"], + "env": {}, + "transport": "stdio", + } + } + } + + +@patch("openhands_cli.tui.settings.store.get_default_tools", return_value=[]) +@patch("openhands_cli.tui.settings.store.get_llm_metadata", return_value={}) +def test_load_when_mcp_file_missing_ignores_persisted_mcp( + mock_meta, + mock_tools, + persistence_dir, + agent_store +): + """If mcp.json is absent, loaded agent.mcp_config should be empty (persisted MCP ignored).""" + persisted_agent = Agent( + llm=LLM(model="gpt-4", api_key=SecretStr("k"), usage_id="svc"), + tools=[], + mcp_config={ + "mcpServers": { + "persistent_server": {"command": "python", "args": ["-m", "old_server"]} + } + }, + ) + write_agent(persistence_dir, persisted_agent) + + # No mcp.json created + + loaded = agent_store.load() + assert loaded is not None + assert loaded.mcp_config == {} # persisted MCP is ignored if file is missin