From 3b7b5cbe7cbc2e44f5a4c58fb59185cbcd7c002b Mon Sep 17 00:00:00 2001 From: tancou Date: Thu, 24 Sep 2026 09:53:01 +0200 Subject: [PATCH] fix(mcp): reconcile an emptied profile config on reload; list portable plugin tools Review gate on #7720 (two regressions reproduced against master with the installed Agent): 1. Deleting a profile's last MCP server left ADOPTED tools callable after /reload-mcp. The scoped teardown only stops connections the profile owns, and the Agent's discover_mcp_tools() returns before its reconcile step when the profile's config has no server, so an adopted shared connection was never judged against the config. /reload-mcp now calls register_mcp_servers({}) under the profile's scope before rediscovery: the Agent's overlay heal detaches only this profile's overlay for servers its config no longer has; the owner's connection keeps running (master's wildcard shutdown stopped the owner too). 2. Portable plugin MCP servers are merged by the Agent at runtime and never appear in the profile's raw ``mcp_servers``; the registry fallback required a config entry and dropped every plugin tool from /api/mcp/tools and the Notes inventory. The profile's own registry slot (registry_tool_owned_by_view) is the isolation check; the config is not an allowlist any more. Tests (red before, green after): agent-free unit tests for both (the fake agent now models the overlay heal and discovery's empty-config early return), and a real-Agent integration test for the adopted-then-deleted case (owner connection survives, adopter slot emptied, "Removed" reported). Co-Authored-By: Claude Fable 5.1 --- api/commands.py | 15 ++++ api/routes.py | 12 +-- tests/test_mcp_runtime_profile_scope.py | 25 +++++++ tests/test_mcp_runtime_profile_scope_unit.py | 78 ++++++++++++++++++++ 4 files changed, 125 insertions(+), 5 deletions(-) diff --git a/api/commands.py b/api/commands.py index f4d52ff5..2bb85eae 100644 --- a/api/commands.py +++ b/api/commands.py @@ -286,6 +286,12 @@ def _run_reload_mcp_command() -> str: discover_mcp_tools = agent_attr( "tools.mcp_tool", "discover_mcp_tools", "tools.mcp_tool_discovery" ) + try: + register_mcp_servers = agent_attr( + "tools.mcp_tool", "register_mcp_servers", "tools.mcp_tool_discovery" + ) + except Exception: + register_mcp_servers = None # older agent: no overlay reconciliation except Exception as exc: logger.warning("Failed to import MCP runtime for /reload-mcp", exc_info=True) raise RuntimeError("MCP runtime unavailable") from exc @@ -332,6 +338,15 @@ def _run_reload_mcp_command() -> str: # The wildcard also dropped connect backoff; keep that for this # profile's own failed servers so the reload retries them now. clear_profile_connect_cooldowns(_core, view) + # A connection this profile ADOPTED (another profile's identical + # route) is not torn down above, and ``discover_mcp_tools()`` + # returns before its reconcile step when the profile's config + # has no server left, so the adopted tools would stay callable. + # ``register_mcp_servers({})`` judges every server serving this + # scope against the profile's config on disk and detaches only + # this profile's overlay; the owner's connection keeps running. + if register_mcp_servers is not None and view.registry_scope is not None: + register_mcp_servers({}) new_tools = discover_mcp_tools() with _lock: diff --git a/api/routes.py b/api/routes.py index cc9ff399..69c8f0af 100644 --- a/api/routes.py +++ b/api/routes.py @@ -29559,8 +29559,11 @@ def _mcp_tools_from_runtime_status(runtime_by_name, server_summaries): def _mcp_tools_from_registry(server_summaries, view=None): """Read already-registered MCP tool schemas without probing MCP servers. - With a profile ``view`` (see ``api.mcp_runtime``), only tools of servers the - profile configures, registered in that profile's own registry slot, are listed. + With a profile ``view`` (see ``api.mcp_runtime``), only tools registered in + that profile's own registry slot are listed. The slot is the isolation check; + the raw ``mcp_servers`` config is not an allowlist: the agent merges portable + plugin servers into the running config at runtime, and their tools are + registered in the same slot without a ``config.yaml`` entry. """ try: from tools.registry import registry @@ -29580,9 +29583,8 @@ def _mcp_tools_from_registry(server_summaries, view=None): if not isinstance(toolset, str) or not toolset.startswith("mcp-"): continue server_name = toolset[len("mcp-"):] - if view is not None and not view.legacy and ( - server_name not in server_summaries - or not registry_tool_owned_by_view(registry, tool_name, view) + if view is not None and not view.legacy and not registry_tool_owned_by_view( + registry, tool_name, view ): continue schema = registry.get_schema(tool_name) or {} diff --git a/tests/test_mcp_runtime_profile_scope.py b/tests/test_mcp_runtime_profile_scope.py index 2288b445..fdc3a2d0 100644 --- a/tests/test_mcp_runtime_profile_scope.py +++ b/tests/test_mcp_runtime_profile_scope.py @@ -276,6 +276,31 @@ def test_reloading_an_adopting_profile_keeps_the_owners_connection(mcp_env): assert WRITE_TOOL not in _tool_names(mcp_env.tools("profile-read")) +@requires_process_home_pin +def test_deleting_an_adopting_profiles_last_server_detaches_its_tools_on_reload(mcp_env): + """A profile that adopted another profile's identical connection deletes its only + server: its reload must drop its tools (they were callable after reload before) + while the owner's connection keeps running.""" + mcp_env.configure("profile-write", atlassian=mcp_env.atlassian("profile-read", read_only=True)) + mcp_env.chat_turn("profile-read") + mcp_env.chat_turn("profile-write") + owner_conn = mcp_env.connection("profile-read") + assert mcp_env.connection("profile-write") is None # adopted, not owned + assert READ_TOOLS <= _tool_names(mcp_env.tools("profile-write")) + + mcp_env.configure("profile-write") # no server left + output = mcp_env.reload("profile-write") + + assert "Removed: atlassian" in output and "Reconnected" not in output + assert mcp_env.tools("profile-write")["total"] == 0 + from tools.registry import registry + write_scope = hermes_constants.hermes_home_key(mcp_env.home("profile-write")) + assert all(registry.snapshot_registration(t, scope=write_scope) is None for t in READ_TOOLS) + assert mcp_env.connection("profile-read") is owner_conn + assert owner_conn.session is not None + assert READ_TOOLS <= _tool_names(mcp_env.tools("profile-read")) + + @requires_process_home_pin def test_root_profile_reload_leaves_named_profiles_running(mcp_env): mcp_env.configure("default", atlassian=mcp_env.atlassian("default", read_only=True)) diff --git a/tests/test_mcp_runtime_profile_scope_unit.py b/tests/test_mcp_runtime_profile_scope_unit.py index d1887dc1..49837895 100644 --- a/tests/test_mcp_runtime_profile_scope_unit.py +++ b/tests/test_mcp_runtime_profile_scope_unit.py @@ -217,12 +217,35 @@ class FakeAgent: agent.retry_after.pop(key, None) agent.failures.pop(key, None) + def register_mcp_servers(servers): + """The agent's overlay heal for the current scope: a server serving this + scope through adoption whose config entry is gone (or omitted from + ``servers`` and absent from the profile's config) loses only this + profile's overlay; the owner's connection is untouched.""" + agent.calls.append(("register", agent.override.get(), dict(servers))) + scope = agent.scope() if scoped else None + if scope is None: + return [] + cfg = yaml.safe_load(Path(agent.home(), "config.yaml").read_text()) or {} + configured = cfg.get("mcp_servers") or {} + with agent.lock: + for key, adopters in list(agent.adopters.items()): + name = scope_mod._key_name(key) + if scope in adopters and name not in servers and name not in configured: + adopters.discard(scope) + slot = agent.scoped_tools.get(scope, {}) + for tool in [t for t, (ts, _) in slot.items() if ts == f"mcp-{name}"]: + del slot[tool] + return sorted(agent._merged(scope)) + def discover_mcp_tools(): home = agent.home() agent.calls.append(("discover", agent.override.get())) cfg = yaml.safe_load(Path(home, "config.yaml").read_text()) or {} scope = agent.scope() if scoped else None names = [] + if not cfg.get("mcp_servers"): + return names # as the agent: nothing configured, no reconcile pass for name, srv in (cfg.get("mcp_servers") or {}).items(): key = agent.ledger_key(name, scope) served = any(scope_mod._key_name(k) == name and agent.visible(k, scope) and scope is not None @@ -243,6 +266,7 @@ class FakeAgent: mcp_mod.get_mcp_status = get_mcp_status mcp_mod.shutdown_mcp_servers = shutdown_mcp_servers mcp_mod.discover_mcp_tools = discover_mcp_tools + mcp_mod.register_mcp_servers = register_mcp_servers return { "hermes_constants": hc, "agent": agent_pkg, @@ -422,6 +446,60 @@ def test_adopted_connection_serves_status_inventory_and_reload(monkeypatch, tmp_ assert write_scope in agent.adopters[(read_scope, "atlassian")] +def test_reload_with_an_empty_config_detaches_adopted_tools_but_keeps_the_owner(monkeypatch, tmp_path): + """Deleting a profile's last server must stop its tools on reload, even when the + connection was adopted from another profile: the adopter's overlay goes, the + owner's connection stays (master's wildcard shutdown stopped the owner too).""" + from api.routes import _handle_mcp_tools_list + + agent = _install(monkeypatch, tmp_path) + read_scope = _profile_scope(agent, "profile-read") + write_scope = _profile_scope(agent, "profile-write") + owner_conn = agent.connect("atlassian", READ, scope=read_scope) + agent.adopt("atlassian", owner=read_scope, adopter=write_scope) + assert {t["name"] for t in _call("profile-write", _handle_mcp_tools_list)["tools"]} == set(READ) + + _configure(agent, "profile-write", {}) # the user deleted the profile's only server + output = _reload("profile-write") + + assert "Removed: atlassian" in output and "Reconnected" not in output + assert _call("profile-write", _handle_mcp_tools_list)["total"] == 0 + assert not agent.scoped_tools.get(write_scope) + assert write_scope not in agent.adopters.get((read_scope, "atlassian"), ()) + # The owner is untouched: same connection, same tools. + assert agent.servers[(read_scope, "atlassian")] is owner_conn + assert {t["name"] for t in _call("profile-read", _handle_mcp_tools_list)["tools"]} == set(READ) + # The teardown stayed scoped to this profile (never the process-wide wildcard) and the + # overlay reconcile ran under this profile's home before rediscovery. + shutdowns = [c for c in agent.calls if c[0] == "shutdown"] + assert shutdowns == [("shutdown", write_scope, {"atlassian"})] + assert ("register", str(agent.base / "profiles" / "profile-write"), {}) in agent.calls + + +def test_portable_plugin_tools_are_listed_for_their_profile_without_a_config_entry(monkeypatch, tmp_path): + """The agent merges plugin-provided (portable) MCP servers into the running config; + they never appear in the profile's raw ``mcp_servers``. Their tools must still be + listed for the profile whose registry slot holds them, and for no other profile.""" + from api.routes import _handle_mcp_tools_list, _handle_notes_sources_list + + agent = _install(monkeypatch, tmp_path) + write_scope = _profile_scope(agent, "profile-write") + agent.connect("notes-plugin", ["mcp__notes-plugin__search_notes"], scope=write_scope) + + inventory = _call("profile-write", _handle_mcp_tools_list) + assert [(t["name"], t["server"]) for t in inventory["tools"]] == [ + ("mcp__notes-plugin__search_notes", "notes-plugin") + ] + assert inventory["source"] == "tool_registry" + monkeypatch.setenv("HERMES_WEBUI_EXTERNAL_NOTES_SOURCES", "1") + notes = _call("profile-write", _handle_notes_sources_list) + assert [s["name"] for s in notes["sources"]] == ["notes-plugin"] + assert notes["sources"][0]["tool_count"] == 1 + # Isolation still holds: another profile's slot does not list them. + assert _call("profile-read", _handle_mcp_tools_list)["total"] == 0 + assert _call("default", _handle_mcp_tools_list)["total"] == 0 + + def test_skewed_routing_hides_runtime_and_refuses_reload(monkeypatch, tmp_path): from api.routes import ( _handle_mcp_servers_list,