From ceaf622c6d348c570b141898bee5916ca115a8d5 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 11 Sep 2026 08:34:03 -0700 Subject: [PATCH] fix(mcp): same-named MCP servers with different credentials connect per profile; owner /reload-mcp keeps adopters' tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under gateway.multiplex_profiles every connection ledger in tools/mcp_tool.py (_servers, _server_scope_keys/_server_tool_scopes, connecting/error/cooldown maps, the circuit breaker, lazy schema-cache configs, trust metadata) was keyed by the bare server NAME. The common per-tenant layout — each profile names its server `github`/`notion` with its own token — gave only the first profile a connection: the second profile's register_mcp_servers saw the name as "already connected", refused to adopt it (different credentials, 4ddbcbd35ef), and left the profile silently tool-less with a healthy-looking `configured` status (#106005 Bug 1/2, #91654). Siblings of the same bug: profile A's failing `x` put profile B's healthy `x` into A's 10-minute connect cooldown and A's open circuit breaker short-circuited B's calls; toolsets._resolve_toolset_memo was not scope-keyed, so B resolved A's `mcp-` tool names. Keys are now the connection key from the new tools/mcp_tool_scope.py: the bare name outside a multiplexer (single-profile processes are unchanged) and (owner_scope, name) under one. Call-time lookups (_resolve_server_key) prefer the calling scope's own connection, then a shared connection it adopted, so identical-route profiles still share one subprocess. _select_new_servers, the cooldown/breaker/trust maps, lazy registration and get_mcp_status all read and write through the composite key; teardown resolves a task's key by identity (the MCP loop has no profile context). The toolset memo key includes registry.current_scope_key(). An owner's scoped /reload-mcp tore down its connection and, with it, every adopting profile's tool overlay; nothing re-ran the adopters' discovery until they reloaded. shutdown_mcp_servers(scope=) now records the orphaned adopters and register_mcp_servers re-registers them under their own home + secret scope once the owner's rediscovery pass completes. Docs: multi-profile-gateways.md states the per-profile connection rule. Fixes #106005 Fixes #91654 Co-authored-by: Bergmann89 Co-authored-by: Izzy-Gottz --- tests/test_toolsets.py | 2 +- .../test_mcp_multiplex_connection_keys.py | 134 ++++++++++++++++++ tools/mcp_tool.py | 77 ++++++---- tools/mcp_tool_discovery.py | 129 ++++++++++------- tools/mcp_tool_handlers.py | 22 ++- tools/mcp_tool_health.py | 2 +- tools/mcp_tool_lifecycle.py | 70 +++++++-- tools/mcp_tool_loop.py | 3 +- tools/mcp_tool_registration.py | 111 +++++++++------ tools/mcp_tool_scope.py | 63 ++++++++ tools/mcp_tool_server_run.py | 2 +- tools/mcp_tool_transport.py | 7 +- toolsets.py | 10 +- .../docs/user-guide/multi-profile-gateways.md | 6 +- 14 files changed, 481 insertions(+), 157 deletions(-) create mode 100644 tests/tools/test_mcp_multiplex_connection_keys.py create mode 100644 tools/mcp_tool_scope.py diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index daf06b469b..d716c696d6 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -337,7 +337,7 @@ class TestResolveToolsetMemo: f"got {get_toolset_calls['n']} calls" ) assert ( - "hermes-cli", True, registry_id, generation + "hermes-cli", True, registry_id, generation, registry.current_scope_key() ) in toolsets_mod._resolve_toolset_memo def test_generation_bump_invalidates_memo(self, monkeypatch): diff --git a/tests/tools/test_mcp_multiplex_connection_keys.py b/tests/tools/test_mcp_multiplex_connection_keys.py new file mode 100644 index 0000000000..9acff00127 --- /dev/null +++ b/tests/tools/test_mcp_multiplex_connection_keys.py @@ -0,0 +1,134 @@ +"""Two multiplexed profiles that name the same MCP server with different credentials are two +connections (#106005, #91654): the ledgers in ``tools.mcp_tool`` are keyed per owning profile +scope, and an owner's scoped reload re-registers the profiles that had adopted its connection.""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from hermes_constants import hermes_home_key, reset_hermes_home_override, set_hermes_home_override + + +def _tool(): + return SimpleNamespace(name="t", description="d", inputSchema={"type": "object", "properties": {}}, + annotations=None) + + +def _server(name, cfg): + return SimpleNamespace(name=name, session=object(), _config=cfg, _tools=[_tool()], tool_timeout=30, + initialize_result=None, _registered_tool_names=[], _sampling=None) + + +@pytest.fixture +def two_profiles(tmp_path, monkeypatch): + """Multiplex on, clean MCP ledgers, a scope switcher for homes A and B; restores everything.""" + import tools.mcp_tool as core + from tools import mcp_tool_config as _config + from tools.registry import registry + + homes = {k: tmp_path / "profiles" / k for k in ("a", "b")} + for home in homes.values(): + home.mkdir(parents=True) + monkeypatch.setattr("agent.secret_scope.is_multiplex_active", lambda: True) + monkeypatch.setattr(core, "_ensure_mcp_sdk", lambda: True) + monkeypatch.setattr(_config, "_filter_suspicious_mcp_servers", lambda servers: servers) + ledgers = ("_servers", "_server_scope_keys", "_server_tool_scopes", "_server_connecting", + "_server_connect_errors", "_server_connect_retry_after", "_server_connect_failures", + "_server_error_counts", "_server_breaker_opened_at", "_lazy_server_configs", + "_mcp_tool_server_names", "_orphaned_adopters") + saved = {n: type(getattr(core, n))(getattr(core, n)) for n in ledgers} + for n in ledgers: + getattr(core, n).clear() + tokens = [] + + def enter(which): + tokens.append(set_hermes_home_override(homes[which])) + return hermes_home_key(homes[which]) + + yield enter + for tool_name in list(registry.get_tool_names_for_toolset("mcp-x")): + for home in homes.values(): + registry.deregister(tool_name, scope=hermes_home_key(home)) + for token in reversed(tokens): + reset_hermes_home_override(token) + for n in ledgers: + getattr(core, n).clear() + getattr(core, n).update(saved[n]) + + +def test_same_named_server_with_other_credentials_is_a_separate_connection(two_profiles): + import tools.mcp_tool as core + from tools import mcp_tool_discovery as disc, mcp_tool_handlers as handlers + from tools import mcp_tool_registration as reg + from tools.registry import registry + import toolsets + + cfg_a = {"url": "https://mcp.example/x", "headers": {"Authorization": "Bearer A"}} + cfg_b = {"url": "https://mcp.example/x", "headers": {"Authorization": "Bearer B"}} + + two_profiles("a") + srv_a = _server("x", cfg_a) + disc._adopt_server("x", srv_a) + srv_a._registered_tool_names = reg._register_server_tools("x", srv_a, cfg_a) + assert toolsets.resolve_toolset("mcp-x") == ["mcp__x__t"] + for _ in range(core._CIRCUIT_BREAKER_THRESHOLD): + core._bump_server_error("x") + disc._note_connect_failure("y", RuntimeError("boom")) + + two_profiles("b") + # B's own view: no tools yet, its memo is not A's, and A's connection is not "connected" for B. + assert registry.get_tool_names_for_toolset("mcp-x") == [] + assert toolsets.resolve_toolset("mcp-x") == [] + assert disc.get_mcp_status({"x": cfg_b})[0]["status"] == "configured" + # B's differently-authenticated 'x' is a connect candidate, not shadowed by A's ledger entries. + assert "x" in disc._select_new_servers({"x": cfg_b}) + assert not disc._connect_cooldown_active("y") + assert handlers._check_circuit_breaker("x") is None + + +def test_owner_reload_reregisters_profiles_that_adopted_its_connection(two_profiles): + import tools.mcp_tool as core + from tools import mcp_tool_discovery as disc, mcp_tool_lifecycle as lifecycle + from tools import mcp_tool_registration as reg + from tools.registry import registry + + cfg = {"url": "https://mcp.example/x", "headers": {"Authorization": "Bearer shared"}} + scope_a = two_profiles("a") + srv_a = _server("x", cfg) + disc._adopt_server("x", srv_a) + srv_a._registered_tool_names = reg._register_server_tools("x", srv_a, cfg) + + two_profiles("b") + assert reg.register_connected_into_current_scope({"x": cfg}) == 1 + assert registry.get_tool_names_for_toolset("mcp-x") == ["mcp__x__t"] + + # Owner A: scoped shutdown (no MCP loop here, so emulate the task teardown), then rediscovery. + two_profiles("a") + with patch.object(lifecycle._loop, "_stop_mcp_loop", lambda **_kw: False): + lifecycle.shutdown_mcp_servers(scope=scope_a) + for tool_name in list(srv_a._registered_tool_names): + reg._deregister_mcp_tool_all_scopes(srv_a, tool_name) + with core._lock: + for key in [k for k, v in core._servers.items() if v is srv_a]: + core._servers.pop(key) + core._server_scope_keys.pop(key, None) + core._server_tool_scopes.pop(key, None) + + def fake_pass(new_servers): + for name, config in new_servers.items(): + srv = _server(name, config) + disc._adopt_server(name, srv) + srv._registered_tool_names = reg._register_server_tools(name, srv, config) + + with patch.object(disc, "_run_discovery_pass", fake_pass), \ + patch.object(disc._loop, "_ensure_mcp_loop", lambda: None), \ + patch("tools.mcp_tool_config._load_mcp_config", lambda: {"x": cfg}): + disc.register_mcp_servers({"x": cfg}) + + # B never reloaded, yet has its tools back on the owner's new identical connection. + two_profiles("b") + assert registry.get_tool_names_for_toolset("mcp-x") == ["mcp__x__t"] + assert disc.get_mcp_status({"x": cfg})[0]["status"] == "connected" diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 7509aa9115..98354693bd 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -396,22 +396,30 @@ class MCPServerTask(MCPServerRunMixin, MCPServerTransportMixin, MCPServerHealthM # ---- Module-level state (every mutation under ``_lock``) ---- +# +# Every ledger below is keyed by the CONNECTION KEY from ``tools.mcp_tool_scope``: the bare +# server name outside a multiplexer, ``(owner_scope, name)`` under one. Two profiles naming the +# same server with their own credentials are two connections; a name-keyed ledger let the first +# profile's connection shadow the second's (never connected, silently tool-less — #106005). -_servers: Dict[str, MCPServerTask] = {} +_servers: Dict[Any, MCPServerTask] = {} # Profile registry scope per live connection (None outside multiplex) so a multiplexed # /reload-mcp tears down only its own profile's servers. -_server_scope_keys: Dict[str, Optional[str]] = {} +_server_scope_keys: Dict[Any, Optional[str]] = {} # Registry scopes that have adopted a live server connection. The owning scope above remains # authoritative for connection teardown; this set preserves visibility for shared connections. -_server_tool_scopes: Dict[str, set] = {} -_server_connecting: set[str] = set() -_server_connect_errors: Dict[str, str] = {} +_server_tool_scopes: Dict[Any, set] = {} +_server_connecting: set = set() +_server_connect_errors: Dict[Any, str] = {} +# adopter scope -> server names whose shared connection an owner's scoped shutdown tore down; +# drained by the next discovery pass so the adopter is re-registered (see mcp_tool_lifecycle). +_orphaned_adopters: Dict[str, set] = {} # Lazy startup: servers registered from the schema cache without connecting; popped on # first real connection. -# Keyed by server name; entries are popped once a real connection is established on first use. See #56832. -_lazy_server_configs: Dict[str, dict] = {} -_lazy_server_fingerprints: Dict[str, str] = {} -_lazy_server_tool_names: Dict[str, List[str]] = {} +# Keyed by connection key; entries are popped once a real connection is established on first use. See #56832. +_lazy_server_configs: Dict[Any, dict] = {} +_lazy_server_fingerprints: Dict[Any, str] = {} +_lazy_server_tool_names: Dict[Any, List[str]] = {} # Task-local claim around ``_connect_server``: discovery retains a recoverable parked task # while standalone probes never publish failed servers into module-global ownership. _connect_server_claim: contextvars.ContextVar[Optional[Callable[[MCPServerTask], None]]] = ( @@ -432,8 +440,8 @@ _connect_server_claim: contextvars.ContextVar[Optional[Callable[[MCPServerTask], # ``retry_after`` deadline with exponential backoff. ``register_mcp_servers`` skips a server whose cooldown # has not elapsed, so a chronically failing server is retried on a backoff schedule instead of on every # worker session -- isolating it from the rest of the bridge. A successful connection clears the state. -_server_connect_retry_after: Dict[str, float] = {} # name -> monotonic deadline -_server_connect_failures: Dict[str, int] = {} # name -> consecutive failures +_server_connect_retry_after: Dict[Any, float] = {} # connection key -> monotonic deadline +_server_connect_failures: Dict[Any, int] = {} # connection key -> consecutive failures _CONNECT_RETRY_BASE_BACKOFF_SEC, _CONNECT_RETRY_MAX_BACKOFF_SEC = 30.0, 600.0 # Per-server circuit breaker: closed -> open (calls short-circuit until the cooldown) -> @@ -446,8 +454,8 @@ _CONNECT_RETRY_BASE_BACKOFF_SEC, _CONNECT_RETRY_MAX_BACKOFF_SEC = 30.0, 600.0 # ``_server_breaker_opened_at`` records the monotonic timestamp when the breaker most recently transitioned # into the open state. Use the ``_bump_server_error`` / ``_reset_server_error`` helpers to mutate this state # — they keep the count and timestamp in sync. -_server_error_counts: Dict[str, int] = {} -_server_breaker_opened_at: Dict[str, float] = {} +_server_error_counts: Dict[Any, int] = {} +_server_breaker_opened_at: Dict[Any, float] = {} _CIRCUIT_BREAKER_THRESHOLD, _CIRCUIT_BREAKER_COOLDOWN_SEC = 3, 60.0 # Trust-tier gating (``trust: full | untrusted``): on an untrusted server every write-capable @@ -456,24 +464,29 @@ _CIRCUIT_BREAKER_THRESHOLD, _CIRCUIT_BREAKER_COOLDOWN_SEC = 3, 60.0 # already warned about, never widen access. Missing trust = full; unrecognized = untrusted (a # typo must never disable the gate). Classified at CALL time from DISCOVERY data: no schema # mutation, prompt cache intact. -_server_trust_levels: Dict[str, str] = {} -_tool_read_only_hints: Dict[str, Dict[str, bool]] = {} +_server_trust_levels: Dict[Any, str] = {} +_tool_read_only_hints: Dict[Any, Dict[str, bool]] = {} _TRUST_FULL, _TRUST_UNTRUSTED = "full", "untrusted" def _bump_server_error(server_name: str) -> None: - """Count a failure; at the threshold (re)stamp the breaker-open time.""" - n = _server_error_counts.get(server_name, 0) + 1 - _server_error_counts[server_name] = n + """Count a failure; at the threshold (re)stamp the breaker-open time. Keyed by the calling + scope's connection so one profile's failing server never opens another profile's breaker.""" + from tools.mcp_tool_scope import _resolve_server_key + key = _resolve_server_key(server_name) + n = _server_error_counts.get(key, 0) + 1 + _server_error_counts[key] = n if n >= _CIRCUIT_BREAKER_THRESHOLD: - _server_breaker_opened_at[server_name] = time.monotonic() + _server_breaker_opened_at[key] = time.monotonic() def _reset_server_error(server_name: str) -> None: """Close the breaker on any unambiguous success signal.""" - _server_error_counts[server_name] = 0 - _server_breaker_opened_at.pop(server_name, None) + from tools.mcp_tool_scope import _resolve_server_key + key = _resolve_server_key(server_name) + _server_error_counts[key] = 0 + _server_breaker_opened_at.pop(key, None) # Raw server names opted into parallel tool calls (``foo-bar``/``foo_bar`` sanitize alike but @@ -622,20 +635,22 @@ def _mcp_registry_scope() -> Optional[str]: return registry.current_scope_key() -def _server_registry_scope(name: str) -> Optional[str]: - """Scope owning *name*'s tools: the one captured at adoption (teardown runs on the MCP - loop without the discovering profile's context), else the current one.""" - if name in _server_scope_keys: - return _server_scope_keys[name] - return _mcp_registry_scope() +def _server_registry_scope(key) -> Optional[str]: + """Scope owning the connection under *key*'s tools: the one captured at adoption (teardown + runs on the MCP loop without the discovering profile's context), else the current one.""" + if key in _server_scope_keys: + return _server_scope_keys[key] + from tools.mcp_tool_scope import _key_scope + return _key_scope(key) or _mcp_registry_scope() -def _server_visible_in_scope(name: str, scope: Optional[str]) -> bool: - """Whether a live server is visible from ``scope`` without changing its teardown owner.""" +def _server_visible_in_scope(key, scope: Optional[str]) -> bool: + """Whether the live connection under *key* is visible from ``scope`` without changing its + teardown owner.""" if scope is None: return True - return (_server_scope_keys.get(name) == scope - or scope in _server_tool_scopes.get(name, ())) + return (_server_scope_keys.get(key) == scope + or scope in _server_tool_scopes.get(key, ())) # Cross-process discovery guard: advisory file lock so gateway + CLI + TUI don't all discover. diff --git a/tools/mcp_tool_discovery.py b/tools/mcp_tool_discovery.py index 7f25687caa..c602384b52 100644 --- a/tools/mcp_tool_discovery.py +++ b/tools/mcp_tool_discovery.py @@ -16,27 +16,31 @@ from tools import mcp_tool_lifecycle as _lifecycle from tools import mcp_tool_loop as _loop from tools import mcp_tool_registration as _registration from tools.mcp_tool_schema import MCP_TOOL_NAME_PREFIX +from tools.mcp_tool_scope import _key_name, _resolve_server_key, _server_key logger = logging.getLogger("tools.mcp_tool") def _record_connect_failure(server_name: str) -> None: """Stamp a geometric, capped retry cooldown after a failed connect (under ``_lock``).""" - n = _core._server_connect_failures.get(server_name, 0) + 1 - _core._server_connect_failures[server_name] = n + key = _server_key(server_name) + n = _core._server_connect_failures.get(key, 0) + 1 + _core._server_connect_failures[key] = n backoff = min(_core._CONNECT_RETRY_BASE_BACKOFF_SEC * (2 ** (n - 1)), _core._CONNECT_RETRY_MAX_BACKOFF_SEC) - _core._server_connect_retry_after[server_name] = time.monotonic() + backoff + _core._server_connect_retry_after[key] = time.monotonic() + backoff def _clear_connect_failure(server_name: str) -> None: """Clear the connect-cooldown state after a successful connection.""" - _core._server_connect_failures.pop(server_name, None) - _core._server_connect_retry_after.pop(server_name, None) + key = _server_key(server_name) + _core._server_connect_failures.pop(key, None) + _core._server_connect_retry_after.pop(key, None) def _connect_cooldown_active(server_name: str) -> bool: - """True if ``server_name`` is still within its retry cooldown.""" - deadline = _core._server_connect_retry_after.get(server_name) + """True if ``server_name`` is still within its retry cooldown (this scope's connection: one + profile's failing ``x`` must not shadow another profile's healthy ``x``).""" + deadline = _core._server_connect_retry_after.get(_server_key(server_name)) return deadline is not None and time.monotonic() < deadline @@ -111,8 +115,9 @@ def _note_connect_failure(name: str, exc: BaseException) -> str: """Record a failed connect (under ``_lock``): error text for status, cooldown stamp.""" message = _errors._format_connect_error(exc) with _core._lock: - _core._server_connecting.discard(name) - _core._server_connect_errors[name] = message + key = _server_key(name) + _core._server_connecting.discard(key) + _core._server_connect_errors[key] = message _record_connect_failure(name) return message @@ -120,16 +125,18 @@ def _note_connect_failure(name: str, exc: BaseException) -> str: def _note_connect_success(name: str) -> None: """Clear connecting/error/cooldown state after a successful connect (under ``_lock``).""" with _core._lock: - _core._server_connecting.discard(name) - _core._server_connect_errors.pop(name, None) + key = _server_key(name) + _core._server_connecting.discard(key) + _core._server_connect_errors.pop(key, None) _clear_connect_failure(name) def _adopt_server(name: str, server: _core.MCPServerTask) -> None: - """Publish *server* into ``_servers`` with its owning registry scope (under ``_lock``).""" + """Publish *server* into ``_servers`` under the connecting scope's key (under ``_lock``).""" with _core._lock: - _core._servers[name] = server - _core._server_scope_keys[name] = _core._mcp_registry_scope() + key = _server_key(name) + _core._servers[key] = server + _core._server_scope_keys[key] = _core._mcp_registry_scope() def _ensure_lazy_server_connected(server_name: str) -> bool: @@ -140,15 +147,16 @@ def _ensure_lazy_server_connected(server_name: str) -> bool: See #50394. """ with _core._lock: - server = _core._servers.get(server_name) + key = _resolve_server_key(server_name) + server = _core._servers.get(key) if server is not None and server.session is not None: return True - config = _core._lazy_server_configs.get(server_name) + config = _core._lazy_server_configs.get(key) if (not config or _connect_cooldown_active(server_name) - or server_name in _core._server_connecting): + or key in _core._server_connecting): return False - _core._server_connecting.add(server_name) - _core._server_connect_errors.pop(server_name, None) + _core._server_connecting.add(key) + _core._server_connect_errors.pop(key, None) logger.info("MCP server '%s': lazy start on first use", server_name) _loop._ensure_mcp_loop() connect_timeout = config.get("connect_timeout", _core._DEFAULT_CONNECT_TIMEOUT) @@ -160,16 +168,16 @@ def _ensure_lazy_server_connected(server_name: str) -> bool: return False _note_connect_success(server_name) with _core._lock: - _core._lazy_server_configs.pop(server_name, None) - stale_fingerprint = _core._lazy_server_fingerprints.pop(server_name, None) - cached_names = _core._lazy_server_tool_names.pop(server_name, None) or [] - server = _core._servers.get(server_name) + _core._lazy_server_configs.pop(key, None) + stale_fingerprint = _core._lazy_server_fingerprints.pop(key, None) + cached_names = _core._lazy_server_tool_names.pop(key, None) or [] + server = _core._servers.get(key) live_names = set(getattr(server, "_registered_tool_names", []) or []) # The cached manifest may advertise tools the live server no longer serves. phantom_names = [n for n in cached_names if n not in live_names] if phantom_names: for tool_name in phantom_names: - _registration._deregister_mcp_tool_all_scopes(server_name, tool_name) + _registration._deregister_mcp_tool_all_scopes(key, tool_name) logger.info("MCP server '%s': deregistered %d phantom cached tool(s) not served live (stale schema-cache " "fingerprint %s): %s", server_name, len(phantom_names), stale_fingerprint, ", ".join(phantom_names)) return server is not None and server.session is not None @@ -183,8 +191,9 @@ def _get_connected_server_for_call(server_name: str) -> Optional[_core.MCPServer AND the resource/prompt utility handlers all trigger the deferred spawn (#56832). """ with _core._lock: - server = _core._servers.get(server_name) - is_lazy = server_name in _core._lazy_server_configs + key = _resolve_server_key(server_name) + server = _core._servers.get(key) + is_lazy = key in _core._lazy_server_configs if is_lazy and (server is None or server.session is None): _ensure_lazy_server_connected(server_name) elif server is not None and server.session is None and server._is_recycled_stdio(): @@ -192,7 +201,7 @@ def _get_connected_server_for_call(server_name: str) -> Optional[_core.MCPServer else: return server with _core._lock: - return _core._servers.get(server_name) + return _core._servers.get(key) async def _discover_and_register_server(name: str, config: dict) -> List[str]: @@ -217,8 +226,9 @@ async def _discover_and_register_server(name: str, config: dict) -> List[str]: finally: _core._connect_server_claim.reset(claim_token) with _core._lock: - _core._server_connecting.discard(name) - _core._server_connect_errors.pop(name, None) + key = _server_key(name) + _core._server_connecting.discard(key) + _core._server_connect_errors.pop(key, None) _adopt_server(name, server) registered_names = _registration._register_server_tools(name, server, config) server._registered_tool_names = list(registered_names) @@ -232,21 +242,25 @@ def _select_new_servers(servers: Dict[str, dict]) -> Dict[str, dict]: refresh per-server bookkeeping. Known servers without a live session are parked or mid-reconnect with tools deregistered, so nothing else can nudge them: signal a reconnect.""" with _core._lock: - connecting = set(_core._server_connecting) + current_scope = _core._mcp_registry_scope() + # This scope's own connections OR shared ones it adopted (``register_connected_into_current_scope`` + # ran first): a same-named server owned by ANOTHER profile with other credentials is not + # "connected" for us and must be a candidate, or this profile ends up silently tool-less. + keys = {k: _resolve_server_key(k, current_scope, current=False) for k in servers} # Only attempt servers that aren't already connected (or currently connecting) and are enabled. # Checking ``_server_connecting`` prevents duplicate subprocess spawns when ``discover_mcp_tools()`` # is called from multiple entry-points before the first batch finishes (#58862). new_servers = { k: v for k, v in servers.items() - if k not in _core._servers and k not in connecting and k not in _core._lazy_server_configs + if keys[k] not in _core._servers and keys[k] not in _core._server_connecting + and keys[k] not in _core._lazy_server_configs and _enabled(v) and not _connect_cooldown_active(k)} - stale_cached = [_core._servers[k] for k in servers - if k in _core._servers and getattr(_core._servers[k], "session", None) is None] - _core._server_connecting.update(new_servers) - current_scope = _core._mcp_registry_scope() + stale_cached = [_core._servers[keys[k]] for k in servers + if keys[k] in _core._servers and getattr(_core._servers[keys[k]], "session", None) is None] for srv_name in new_servers: - _core._server_scope_keys[srv_name] = current_scope - _core._server_connect_errors.pop(srv_name, None) + _core._server_connecting.add(keys[srv_name]) + _core._server_scope_keys[keys[srv_name]] = current_scope + _core._server_connect_errors.pop(keys[srv_name], None) # Track which servers opt-in to parallel tool calls (idempotent). for srv_name, srv_cfg in servers.items(): if _parse_boolish(srv_cfg.get("supports_parallel_tool_calls", False), default=False): @@ -278,13 +292,13 @@ def _register_lazy_from_cache(new_servers: Dict[str, dict]) -> Tuple[Dict[str, d if not entry: continue with _core._lock: - _core._server_connecting.discard(name) + _core._server_connecting.discard(_server_key(name)) try: names = _registration._register_from_cache_sync(name, cfg, entry) except Exception as exc: logger.warning("Failed lazy MCP registration for '%s': %s", name, exc) with _core._lock: - _core._server_connecting.add(name) + _core._server_connecting.add(_server_key(name)) continue eager_servers.pop(name, None) lazy_registered += len(names) @@ -321,13 +335,14 @@ def _run_discovery_pass(new_servers: Dict[str, dict]) -> None: # Stranded _server_connecting entries would block future reconnects. how = "timed out" if isinstance(_e, TimeoutError) else "interrupted" with _core._lock: - stale = [n for n in new_servers if n in _core._server_connecting] + stale = [n for n in new_servers if _server_key(n) in _core._server_connecting] if stale: logger.warning("MCP discovery %s while %d server(s) were still connecting; clearing stale " "connecting set: %s", how, len(stale), ", ".join(stale)) - _core._server_connecting.difference_update(stale) for _sn in stale: - _core._server_connect_errors.setdefault(_sn, f"Connection attempt {how} during discovery") + _core._server_connecting.discard(_server_key(_sn)) + _core._server_connect_errors.setdefault( + _server_key(_sn), f"Connection attempt {how} during discovery") raise finally: if _was_interrupted: @@ -337,8 +352,10 @@ def _run_discovery_pass(new_servers: Dict[str, dict]) -> None: def _connected_summary(names, *, lazy_tools: int = 0, lazy_servers: int = 0) -> Tuple[int, int, int]: """(tool count, connected count, failed count) for candidate names, plus lazy servers.""" with _core._lock: - connected = [n for n in names if n in _core._servers and n not in _core._server_connect_errors] - tool_count = sum(len(getattr(_core._servers[n], "_registered_tool_names", [])) for n in connected) + keys = {n: _server_key(n) for n in names} + connected = [n for n in names + if keys[n] in _core._servers and keys[n] not in _core._server_connect_errors] + tool_count = sum(len(getattr(_core._servers[keys[n]], "_registered_tool_names", [])) for n in connected) failed = len(names) - len(connected) return tool_count + lazy_tools, len(connected) + lazy_servers, failed @@ -360,6 +377,15 @@ def register_mcp_servers(servers: Dict[str, dict]) -> List[str]: logger.debug("MCP SDK not available -- skipping explicit MCP registration") return [] servers = _config._filter_suspicious_mcp_servers(servers) + try: + return _register_mcp_servers(servers) + finally: + # An owner's scoped reload orphaned adopters of its shared connections: now that this + # pass (its rediscovery) is done, give them their tools back under their own scope. + _lifecycle._reregister_orphaned_adopters() + + +def _register_mcp_servers(servers: Dict[str, dict]) -> List[str]: scoped_healed = _registration.register_connected_into_current_scope(servers) if not servers: logger.debug("No explicit MCP servers provided") @@ -434,9 +460,10 @@ def discover_mcp_tools(allowed_mcp_names: Optional[List[str]] = None) -> List[st cookie = _acquire_discovery_lock_with_retry() try: with _core._lock: - connecting = set(_core._server_connecting) + keys = {name: _resolve_server_key(name) for name in servers} new_server_names = [name for name, cfg in servers.items() - if name not in _core._servers and name not in connecting and _enabled(cfg)] + if keys[name] not in _core._servers and keys[name] not in _core._server_connecting + and _enabled(cfg)] tool_names = register_mcp_servers(servers) if new_server_names: _log_summary(" MCP:", new_server_names) @@ -465,15 +492,15 @@ def get_mcp_status(configured: Optional[Dict[str, dict]] = None, *, include_runt return [] current_scope = _core._mcp_registry_scope() with _core._lock: - def visible(name: str) -> bool: + def visible(key) -> bool: # Runtime state belongs to the profile that adopted it; under a multiplexer only that # profile's view may show it, and ``include_runtime=False`` hides the launch profile's # servers from a status read scoped to a different profile. - return include_runtime and _core._server_visible_in_scope(name, current_scope) + return include_runtime and _core._server_visible_in_scope(key, current_scope) - active_servers = {n: s for n, s in _core._servers.items() if visible(n)} - connecting = {n for n in _core._server_connecting if visible(n)} - connect_errors = {n: e for n, e in _core._server_connect_errors.items() if visible(n)} + active_servers = {_key_name(k): s for k, s in _core._servers.items() if visible(k)} + connecting = {_key_name(k) for k in _core._server_connecting if visible(k)} + connect_errors = {_key_name(k): e for k, e in _core._server_connect_errors.items() if visible(k)} result: List[dict] = [] for name, cfg in configured.items(): diff --git a/tools/mcp_tool_handlers.py b/tools/mcp_tool_handlers.py index 8b21774229..240b6d78f1 100644 --- a/tools/mcp_tool_handlers.py +++ b/tools/mcp_tool_handlers.py @@ -41,8 +41,10 @@ _STDIO_OUTCOME_UNCERTAIN_MSG = ( def _trust_gate_check(server_name: str, tool_name: str) -> Optional[str]: """Approval gate for write-capable tools on ``trust: untrusted`` servers. None to proceed, else a ``tool_error``. Fail-closed: approval-system errors block.""" - if (_core._server_trust_levels.get(server_name, _core._TRUST_FULL) != _core._TRUST_UNTRUSTED - or _core._tool_read_only_hints.get(server_name, {}).get(tool_name) is True): + from tools.mcp_tool_scope import _resolve_server_key + key = _resolve_server_key(server_name) + if (_core._server_trust_levels.get(key, _core._TRUST_FULL) != _core._TRUST_UNTRUSTED + or _core._tool_read_only_hints.get(key, {}).get(tool_name) is True): return None try: # lazy: tools.approval routes the prompt to whichever surface owns the session from tools.approval_prompt import request_elicitation_consent @@ -67,8 +69,10 @@ def _trust_gate_check(server_name: str, tool_name: str) -> Optional[str]: def _check_circuit_breaker(server_name: str) -> Optional[str]: """Open-breaker error, or None when calls may proceed. After the cooldown the breaker is half-open: the next call probes; success resets, failure re-bumps and re-arms the cooldown.""" - failures = _core._server_error_counts.get(server_name, 0) - age = time.monotonic() - _core._server_breaker_opened_at.get(server_name, 0.0) + from tools.mcp_tool_scope import _resolve_server_key + key = _resolve_server_key(server_name) + failures = _core._server_error_counts.get(key, 0) + age = time.monotonic() - _core._server_breaker_opened_at.get(key, 0.0) if failures < _core._CIRCUIT_BREAKER_THRESHOLD or age >= _core._CIRCUIT_BREAKER_COOLDOWN_SEC: return None return tool_error(f"MCP server '{server_name}' is unreachable after {failures} consecutive failures. " @@ -120,8 +124,9 @@ def _mcp_loop_running() -> bool: def _lookup_reconnectable_server(server_name: str, require_loop: bool = False): """The registered server object when it can be signalled to reconnect, else None. With *require_loop*, also None unless the MCP loop is running (nothing to wait on).""" + from tools.mcp_tool_scope import _resolve_server_key with _core._lock: - srv = _core._servers.get(server_name) + srv = _core._servers.get(_resolve_server_key(server_name)) ok = srv is not None and hasattr(srv, "_reconnect_event") and (_mcp_loop_running() or not require_loop) return srv if ok else None @@ -591,9 +596,12 @@ _make_get_prompt_handler = _make_utility_handler( def _make_check_fn(server_name: str): """Connection-alive check; lazy (schema-cache registered) servers count as available.""" + from tools.mcp_tool_scope import _resolve_server_key + def _check() -> bool: with _core._lock: - server = _core._servers.get(server_name) + key = _resolve_server_key(server_name) + server = _core._servers.get(key) return ((server is not None and (server.session is not None or server._is_recycled_stdio())) - or server_name in _core._lazy_server_configs) + or key in _core._lazy_server_configs) return _check diff --git a/tools/mcp_tool_health.py b/tools/mcp_tool_health.py index 6db839bb5c..fa39a6d0b8 100644 --- a/tools/mcp_tool_health.py +++ b/tools/mcp_tool_health.py @@ -127,7 +127,7 @@ class MCPServerHealthMixin: from tools.registry import registry for tool_name in tool_names: if registry.get_toolset_for_tool(tool_name) == f"mcp-{self.name}": - _registration._deregister_mcp_tool_all_scopes(self.name, tool_name) + _registration._deregister_mcp_tool_all_scopes(self, tool_name) async def _refresh_tools(self): """Re-fetch tools on ``tools/list_changed`` and update the registry. The lock serializes rapid-fire diff --git a/tools/mcp_tool_lifecycle.py b/tools/mcp_tool_lifecycle.py index 0cde565f30..bca4c5390e 100644 --- a/tools/mcp_tool_lifecycle.py +++ b/tools/mcp_tool_lifecycle.py @@ -84,16 +84,47 @@ def _filter_mcp_children(pids: set) -> set: return kept -def _clear_connect_cooldowns(names=None) -> None: +def _clear_connect_cooldowns(keys=None) -> None: """Drop connect-retry cooldowns: a restart must re-attempt every server immediately, not honour a stale per-server backoff. Caller holds ``_core._lock``.""" - if names is None: + if keys is None: _core._server_connect_retry_after.clear() _core._server_connect_failures.clear() else: - for name in names: - _core._server_connect_retry_after.pop(name, None) - _core._server_connect_failures.pop(name, None) + for key in keys: + _core._server_connect_retry_after.pop(key, None) + _core._server_connect_failures.pop(key, None) + + +def _reregister_orphaned_adopters() -> None: + """Re-run MCP registration for profiles whose ADOPTED shared connection an owner's + ``/reload-mcp`` just tore down. Their tools vanished with the owner's teardown and nothing + re-runs their discovery until THEY reload, so they sat tool-less behind a healthy-looking + status (#106005). Runs after the owner's rediscovery, under each adopter's own home + secret + scope (its ``${VAR}`` refs must resolve to ITS credentials): the adopter re-adopts the owner's + new identical connection or connects its own.""" + with _core._lock: + pending = dict(_core._orphaned_adopters) + _core._orphaned_adopters.clear() + if not pending: + return + from pathlib import Path + from agent.secret_scope import build_profile_secret_scope, reset_secret_scope, set_secret_scope + from hermes_constants import reset_hermes_home_override, set_hermes_home_override + from tools import mcp_tool_discovery as _discovery + from tools.mcp_tool_config import _load_mcp_config + for adopter, names in pending.items(): + home_token = set_hermes_home_override(adopter) + secret_token = set_secret_scope(build_profile_secret_scope(Path(adopter))) + try: + servers = {n: c for n, c in (_load_mcp_config() or {}).items() if n in names} + if servers: + _discovery.register_mcp_servers(servers) + except Exception: + logger.debug("MCP: re-registration for profile scope %s failed", adopter, exc_info=True) + finally: + reset_secret_scope(secret_token) + reset_hermes_home_override(home_token) def shutdown_mcp_servers(*, scope: Optional[str] = None): @@ -103,23 +134,32 @@ def shutdown_mcp_servers(*, scope: Optional[str] = None): (its ``/reload-mcp`` must not kill other profiles') and leaves the shared loop running if anything else is still connected.""" with _core._lock: - selected = [name for name in _core._servers if scope is None or _core._server_scope_keys.get(name) == scope] - servers_snapshot = [_core._servers[name] for name in selected] + selected = [key for key in _core._servers if scope is None or _core._server_scope_keys.get(key) == scope] + servers_snapshot = [_core._servers[key] for key in selected] selected_status = ( set(_core._servers) | set(_core._server_scope_keys) | set(_core._server_tool_scopes) | set(_core._server_connecting) | set(_core._server_connect_errors) if scope is None else { - name for name, owner in _core._server_scope_keys.items() if owner == scope + key for key, owner in _core._server_scope_keys.items() if owner == scope } ) + # Adopters of the connections being torn down lose their overlays with the tasks' own + # ``_deregister_tools``; remember them so the next discovery pass re-registers them + # (``_reregister_orphaned_adopters``). + if scope is not None: + from tools.mcp_tool_scope import _key_name + for key in selected: + for adopter in _core._server_tool_scopes.get(key, ()): + if adopter != scope: + _core._orphaned_adopters.setdefault(adopter, set()).add(_key_name(key)) def clear_selected_status(): _core._server_connecting.difference_update(selected_status) - for name in selected_status: - _core._server_connect_errors.pop(name, None) - _core._server_scope_keys.pop(name, None) - _core._server_tool_scopes.pop(name, None) + for key in selected_status: + _core._server_connect_errors.pop(key, None) + _core._server_scope_keys.pop(key, None) + _core._server_tool_scopes.pop(key, None) # Fast path: nothing to shut down. The connect-cooldown maps can still be populated here — a server that # failed to connect is never recorded in ``_servers`` (that is the very premise of the #50394 cooldown), @@ -132,9 +172,9 @@ def shutdown_mcp_servers(*, scope: Optional[str] = None): if isinstance(result, Exception): logger.debug("Error closing MCP server '%s': %s", server.name, result) with _core._lock: - for name in selected: - _core._servers.pop(name, None) - _core._server_scope_keys.pop(name, None) + for key in selected: + _core._servers.pop(key, None) + _core._server_scope_keys.pop(key, None) clear_selected_status() _clear_connect_cooldowns(None if scope is None else selected_status) diff --git a/tools/mcp_tool_loop.py b/tools/mcp_tool_loop.py index 3337b278f5..d2154d8fff 100644 --- a/tools/mcp_tool_loop.py +++ b/tools/mcp_tool_loop.py @@ -194,8 +194,9 @@ def _signal_reconnect(server: Any) -> bool: def reconnect_mcp_server(server_name: str) -> bool: """Ask a currently-live MCP server to rebuild after external re-auth.""" + from tools.mcp_tool_scope import _resolve_server_key with _core._lock: - server = _core._servers.get(server_name) + server = _core._servers.get(_resolve_server_key(server_name)) return server is not None and _signal_reconnect(server) diff --git a/tools/mcp_tool_registration.py b/tools/mcp_tool_registration.py index abe0d4b025..df3a4aad33 100644 --- a/tools/mcp_tool_registration.py +++ b/tools/mcp_tool_registration.py @@ -17,6 +17,7 @@ from tools.mcp_tool_handlers import ( _make_list_resources_handler, _make_read_resource_handler) from tools.mcp_tool_schema import ( _UTILITY_CAPABILITY_ATTRS, _build_utility_schemas, _normalize_name_filter, matches_name_filter) +from tools.mcp_tool_scope import _key_name, _resolve_server_key, _server_key if TYPE_CHECKING: # pragma: no cover from tools.mcp_tool import MCPServerTask @@ -54,8 +55,9 @@ def _record_tool_trust_metadata(server_name: str, config: dict, tools: List[Any] """Capture per-server trust and per-tool readOnlyHint at discovery — the security boundary: the call-time gate classifies from data we control, never re-read server-supplied state.""" with _core._lock: - _core._server_trust_levels[server_name] = _normalize_server_trust((config or {}).get("trust")) - hints = _core._tool_read_only_hints.setdefault(server_name, {}) + key = _resolve_server_key(server_name) + _core._server_trust_levels[key] = _normalize_server_trust((config or {}).get("trust")) + hints = _core._tool_read_only_hints.setdefault(key, {}) hints.update({t.name: _annotation_read_only_hint(t) for t in tools if getattr(t, "name", None)}) @@ -71,49 +73,65 @@ def _forget_mcp_tool_server(tool_name: str) -> None: _core._mcp_tool_server_names.pop(tool_name, None) -def _deregister_mcp_tool_all_scopes(server_name: str, tool_name: str) -> None: - """Deregister one server tool from every profile overlay that owns it.""" +def _server_key_for_task(server) -> object: + """Connection key of a live ``MCPServerTask`` (teardown runs on the MCP loop without the + discovering profile's context, so the key is found by identity, never re-derived).""" + with _core._lock: + for key, live in _core._servers.items(): + if live is server: + return key + return _server_key(server.name) + + +def _deregister_mcp_tool_all_scopes(server, tool_name: str) -> None: + """Deregister one server tool from every profile overlay that owns it. *server* is the + live task or a connection key.""" from tools.registry import registry + key = server if isinstance(server, (str, tuple)) else _server_key_for_task(server) with _core._lock: - scopes = set(_core._server_tool_scopes.get(server_name, ())) + scopes = set(_core._server_tool_scopes.get(key, ())) if not scopes: - scopes = {_core._server_registry_scope(server_name)} + scopes = {_core._server_registry_scope(key)} for scope in scopes: registry.deregister(tool_name, scope=scope) _forget_mcp_tool_server(tool_name) - _restore_server_toolset_alias(server_name) + _restore_server_toolset_alias(key) -def _restore_server_toolset_alias(server_name: str) -> None: - """Keep the process-global alias while any profile still owns this server's tools.""" +def _restore_server_toolset_alias(key) -> None: + """Keep the process-global alias while any profile still owns tools of a server with this + name — including another profile's same-named connection (the alias is per NAME; the + registry deregister dropped it after checking only one scope).""" from tools.registry import registry + server_name = _key_name(key) with _core._lock: - server = _core._servers.get(server_name) - scopes = set(_core._server_tool_scopes.get(server_name, ())) - tool_names = list(getattr(server, "_registered_tool_names", ()) if server is not None else ()) + owned = [(server, set(_core._server_tool_scopes.get(k, ()))) + for k, server in _core._servers.items() if _key_name(k) == server_name] if any( registry.snapshot_registration(tool_name, scope=scope) is not None - for scope in scopes for tool_name in tool_names + for server, scopes in owned for scope in scopes + for tool_name in getattr(server, "_registered_tool_names", ()) ): registry.register_toolset_alias(server_name, f"mcp-{server_name}") -def _remove_server_scope(server_name: str, scope: str) -> None: +def _remove_server_scope(key, scope: str) -> None: """Remove one profile's MCP overlay for a shared live connection.""" from tools.registry import registry + server_name = _key_name(key) for tool_name in registry.get_tool_names_for_toolset(f"mcp-{server_name}"): registry.deregister(tool_name, scope=scope) with _core._lock: - scopes = set(_core._server_tool_scopes.get(server_name, ())) + scopes = set(_core._server_tool_scopes.get(key, ())) scopes.discard(scope) if scopes: - _core._server_tool_scopes[server_name] = scopes + _core._server_tool_scopes[key] = scopes else: - _core._server_tool_scopes.pop(server_name, None) - _restore_server_toolset_alias(server_name) + _core._server_tool_scopes.pop(key, None) + _restore_server_toolset_alias(key) def _select_utility_schemas(server_name: str, server: "MCPServerTask", config: dict) -> List[dict]: @@ -153,12 +171,12 @@ def _existing_tool_names() -> List[str]: with _core._lock: server_names = [ - name for name in _core._servers - if _core._server_visible_in_scope(name, scope) + _key_name(key) for key in _core._servers + if _core._server_visible_in_scope(key, scope) ] server_names.extend( - name for name in _core._lazy_server_tool_names - if name not in _core._servers + _key_name(key) for key in _core._lazy_server_tool_names + if key not in _core._servers and _core._server_visible_in_scope(key, scope) ) return sorted({ tool_name @@ -171,8 +189,8 @@ def _existing_tool_names() -> List[str]: names.extend(server._registered_tool_names if hasattr(server, "_registered_tool_names") else (_schema._convert_mcp_schema(server.name, t)["name"] for t in server._tools)) with _core._lock: - names.extend(n for sname, tool_names in _core._lazy_server_tool_names.items() - if sname not in _core._servers for n in tool_names) + names.extend(n for key, tool_names in _core._lazy_server_tool_names.items() + if key not in _core._servers for n in tool_names) return names @@ -288,14 +306,17 @@ def _resolve_name_collisions(name: str, candidates: List[_Candidate]) -> List[_C def _register_candidates(name: str, candidates: List[_Candidate], *, check_fn: Callable, - scope: Callable[[], Optional[str]], lazy: bool) -> List[str]: + scope: Callable[[], Optional[str]], lazy: bool, key=None) -> List[str]: """Register candidates under toolset ``mcp-{name}``; returns the names that landed. The ownership pre-check is advisory (servers connect in parallel): ``registry.register()`` is - the atomic gate and its verdict is re-read after every call.""" + the atomic gate and its verdict is re-read after every call. *key* is the connection whose + ``_server_tool_scopes`` records the registering scope (default: this scope's own).""" from tools.registry import registry toolset_name = f"mcp-{name}" registered: List[str] = [] scope_value = scope() + if key is None: + key = _server_key(name, scope_value, current=False) for c in candidates: existing_toolset = registry.get_toolset_for_tool(c.registry_name) if existing_toolset and existing_toolset != toolset_name: # foreign owner: skip, preserve it @@ -317,7 +338,7 @@ def _register_candidates(name: str, candidates: List[_Candidate], *, check_fn: C _track_mcp_tool_server(c.registry_name, name) if scope_value is not None: with _core._lock: - _core._server_tool_scopes.setdefault(name, set()).add(scope_value) + _core._server_tool_scopes.setdefault(key, set()).add(scope_value) registered.append(c.registry_name) elif not lazy: logger.error("MCP server '%s': registration of %s as '%s' was rejected by the registry; " @@ -362,12 +383,13 @@ def _register_server_tools(name: str, server: "MCPServerTask", config: dict) -> refresh); returns the names. Toolset aliases derive from the live registry, not ``toolsets.TOOLSETS``; lossy normalization collisions (``read-file``/``read_file``) fail closed.""" should_register = _make_tool_filter(name, config) + key = _server_key_for_task(server) _record_tool_trust_metadata(name, config, server._tools) candidates = _tool_candidates(name, server._tools, should_register, server.tool_timeout) candidates += _utility_candidates(name, _select_utility_schemas(name, server, config), server.tool_timeout) registered = _register_candidates( name, _resolve_name_collisions(name, candidates), - check_fn=_make_check_fn(name), scope=lambda: _core._server_registry_scope(name), lazy=False) + check_fn=_make_check_fn(name), scope=lambda: _core._server_registry_scope(key), lazy=False, key=key) if registered: _write_schema_cache(name, server, config, should_register) return registered @@ -420,28 +442,34 @@ def _register_connected_into_current_scope(servers: dict) -> int: with _core._lock: stale = [] - for name, scopes in _core._server_tool_scopes.items(): + for key, scopes in _core._server_tool_scopes.items(): if scope not in scopes: continue - server = _core._servers.get(name) - config = servers.get(name) + server = _core._servers.get(key) + config = servers.get(_key_name(key)) if (config is None or not _server_enabled(config) or server is None or getattr(server, "session", None) is None or not _same_server_route(server, config)): - stale.append(name) - for name in stale: - _remove_server_scope(name, scope) + stale.append(key) + for key in stale: + _remove_server_scope(key, scope) registered_servers = 0 for name, config in servers.items(): if not _server_enabled(config): continue with _core._lock: - server = _core._servers.get(name) - if server is None or getattr(server, "session", None) is None or not _same_server_route(server, config): + if _server_key(name, scope, current=False) in _core._servers: + continue # this profile has its own connection for the name + # Any other profile's live connection with the same route AND credentials is shareable. + shared = [(key, live) for key, live in _core._servers.items() + if _key_name(key) == name and getattr(live, "session", None) is not None + and _same_server_route(live, config)] + if not shared: continue + key, server = shared[0] # Visibility for this profile: the owner keeps teardown, this scope sees the connection. with _core._lock: - _core._server_tool_scopes.setdefault(name, set()).add(scope) + _core._server_tool_scopes.setdefault(key, set()).add(scope) if registry.get_tool_names_for_toolset(f"mcp-{name}"): continue candidates = _tool_candidates(name, server._tools, _make_tool_filter(name, config), server.tool_timeout) @@ -449,7 +477,7 @@ def _register_connected_into_current_scope(servers: dict) -> int: name, _select_utility_schemas(name, server, config), server.tool_timeout) names = _register_candidates( name, _resolve_name_collisions(name, candidates), - check_fn=_make_check_fn(name), scope=lambda: scope, lazy=False) + check_fn=_make_check_fn(name), scope=lambda: scope, lazy=False, key=key) if names: registered_servers += 1 with _core._lock: @@ -476,8 +504,9 @@ def _register_from_cache_sync(name: str, config: dict, entry: dict) -> List[str] name, candidates, check_fn=_make_check_fn(name), scope=_core._mcp_registry_scope, lazy=True) if registered: with _core._lock: - _core._lazy_server_configs[name] = dict(config) - _core._lazy_server_fingerprints[name] = config_fingerprint(config) - _core._lazy_server_tool_names[name] = list(registered) + key = _server_key(name) + _core._lazy_server_configs[key] = dict(config) + _core._lazy_server_fingerprints[key] = config_fingerprint(config) + _core._lazy_server_tool_names[key] = list(registered) logger.info("MCP server '%s' (lazy): registered %d tool(s) from schema cache", name, len(registered)) return registered diff --git a/tools/mcp_tool_scope.py b/tools/mcp_tool_scope.py new file mode 100644 index 0000000000..7e6904b872 --- /dev/null +++ b/tools/mcp_tool_scope.py @@ -0,0 +1,63 @@ +"""Connection-ledger keys for tools.mcp_tool under a profile multiplexer. + +Every ledger in ``tools.mcp_tool`` (``_servers``, connecting/error/cooldown maps, circuit +breaker, lazy configs, trust metadata) is keyed by a *connection key*: the bare server name +outside a multiplexer (single-profile processes are unchanged, byte for byte), and +``(owner_scope, name)`` under one. Two profiles that both configure ``github`` with their own +token are two connections; keying by name alone let the first profile's connection shadow the +second's forever — its ``register_mcp_servers`` saw the name as "already connected", adopted +nothing (different credentials) and left the profile silently tool-less (#106005, #91654). + +A profile may still *adopt* another profile's live connection when the route and credentials +match (``mcp_tool_registration._same_server_route``); ``_server_tool_scopes[key]`` records every +scope that has done so, and ``_resolve_server_key`` finds that shared connection for a caller +whose own scope has none. +""" + +from __future__ import annotations + +from typing import Optional, Tuple, Union + +from tools.mcp_tool_common import _core + +ServerKey = Union[str, Tuple[str, str]] + + +def _server_key(name: str, scope: Optional[str] = None, *, current: bool = True) -> ServerKey: + """Connection key for *name* owned by *scope* (the current registry scope when *current*). + ``None`` scope (no multiplexer) keeps the bare name.""" + if scope is None and current: + scope = _core._mcp_registry_scope() + return name if scope is None else (scope, name) + + +def _key_name(key: ServerKey) -> str: + return key[1] if isinstance(key, tuple) else key + + +def _key_scope(key: ServerKey) -> Optional[str]: + """Owning registry scope encoded in *key* (None for a bare, unscoped key).""" + return key[0] if isinstance(key, tuple) else None + + +def _key_visible_in_scope(key: ServerKey, scope: Optional[str]) -> bool: + """Whether the connection under *key* serves *scope*: owned by it or adopted into it. + Caller holds ``_core._lock`` or tolerates a racy read (status surfaces).""" + if scope is None: + return True + return _key_scope(key) == scope or scope in _core._server_tool_scopes.get(key, ()) + + +def _resolve_server_key(name: str, scope: Optional[str] = None, *, current: bool = True) -> ServerKey: + """The connection key a call to *name* from *scope* must use: the scope's own connection + (live, connecting or lazily registered) when it has one, else a shared connection it + adopted, else its own (not yet existing) key so bookkeeping lands under this scope.""" + if scope is None and current: + scope = _core._mcp_registry_scope() + own = _server_key(name, scope, current=False) + if scope is None or own in _core._servers or own in _core._lazy_server_configs: + return own + for key, scopes in _core._server_tool_scopes.items(): + if scope in scopes and _key_name(key) == name and key in _core._servers: + return key + return own diff --git a/tools/mcp_tool_server_run.py b/tools/mcp_tool_server_run.py index 4daded5437..2363aff4a1 100644 --- a/tools/mcp_tool_server_run.py +++ b/tools/mcp_tool_server_run.py @@ -414,5 +414,5 @@ class MCPServerRunMixin: """Drop this server's tools from the registry (idempotent); on shutdown AND budget exhaustion, so a dead server never leaves phantom tools in the prompt.""" for tool_name in list(getattr(self, "_registered_tool_names", [])): - _registration._deregister_mcp_tool_all_scopes(self.name, tool_name) + _registration._deregister_mcp_tool_all_scopes(self, tool_name) self._registered_tool_names = [] diff --git a/tools/mcp_tool_transport.py b/tools/mcp_tool_transport.py index 1324179788..ef28cdc4a8 100644 --- a/tools/mcp_tool_transport.py +++ b/tools/mcp_tool_transport.py @@ -449,10 +449,11 @@ class MCPServerTransportMixin: if self._registered_tool_names: return with _core._lock: - owned = _core._servers.get(self.name) is self + owned = [key for key, live in _core._servers.items() if live is self] if not owned and not self._ready.is_set(): return self._registered_tool_names = _registration._register_server_tools(self.name, self, self._config) with _core._lock: # a retained initial-failure server that just published tools has recovered - if _core._servers.get(self.name) is self: - _core._server_connect_errors.pop(self.name, None) + for key in owned: + if _core._servers.get(key) is self: + _core._server_connect_errors.pop(key, None) diff --git a/toolsets.py b/toolsets.py index 3da2993c07..24c58b9677 100644 --- a/toolsets.py +++ b/toolsets.py @@ -325,9 +325,11 @@ def bundle_non_core_tools(toolset_name: str) -> Set[str]: return to_remove - core -# Memo keyed on (name, include_registry, id(registry), registry generation); -# engages only at the public entry (visited is None). -_resolve_toolset_memo: Dict[Tuple[str, bool, int, int], List[str]] = {} +# Memo keyed on (name, include_registry, id(registry), registry generation, profile scope); +# engages only at the public entry (visited is None). The scope is part of the key because a +# multiplexed process resolves ``mcp-`` per profile overlay: without it profile B got +# profile A's tool names for a server B never connected (#106005). +_resolve_toolset_memo: Dict[Tuple[str, bool, int, int, str], List[str]] = {} def _plugin_platform_bundle(name: str) -> List[str]: @@ -361,7 +363,7 @@ def resolve_toolset(name: str, visited: Set[str] = None, *, include_registry: bo """ external_call = visited is None if external_call: - memo_key = (name, include_registry, *_registry_generation()) + memo_key = (name, include_registry, *_registry_generation(), _registry_call("current_scope_key", "")) cached = _resolve_toolset_memo.get(memo_key) if cached is not None: return list(cached) diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index fbc691088e..e2d09a49b9 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -237,7 +237,11 @@ into a shared environment. Subprocesses like MCP servers and Kanban workers only ever see their own profile's secrets — including credentials injected by an external secret source (1Password, Bitwarden, …): a stdio MCP server started for profile B receives B's value for such a name, or nothing if B has none, never the -default profile's. Terminal settings +default profile's. MCP servers are connected **per profile**: two profiles that +both name a server `github` with their own token get two connections and each +sees only its own tools; profiles whose `mcp_servers` entry is identical (same +route *and* credentials) share one connection, and an owner's `/reload-mcp` +re-registers the sharing profiles' tools without them reloading. Terminal settings (`terminal.backend`, `terminal.cwd`, `terminal.docker_volumes`, `terminal.docker_shared_container_key`, SSH targets, …) are likewise resolved per profile on every routed turn: a profile that omits a terminal key gets the