From 90b814f6739f99544e124fd0810187e6395ab01b Mon Sep 17 00:00:00 2001 From: tancou Date: Tue, 22 Sep 2026 14:17:43 +0200 Subject: [PATCH] fix(mcp): scope MCP status, inventory and /reload-mcp to the request profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two profiles configuring a same-named MCP server shared one connection and the MCP settings panel showed another profile's runtime ("Active, N tools" next to an empty inventory); /reload-mcp stopped every profile's servers and rediscovered from the process-default config. - api.profiles._set_hermes_home() is the single writer of the process-profile home: it records get_process_profile_home() and hands it to Hermes Agent's hermes_constants.pin_process_hermes_home when available, at startup and on switch_profile(process_wide=True), so the per-turn HERMES_HOME mirror no longer makes every turn look like the process profile (bare-name, cross-profile MCP keys) and the anchor cannot drift from HERMES_HOME. - Run get_mcp_status/registry reads and /reload-mcp inside api.mcp_runtime.mcp_runtime_scope(): request profile's Hermes-home override and secret scope (root included), no os.environ mutation, restored on exit. - One ledger predicate, ledger_key_serves_view() (owned via _server_scope_keys/key scope, or adopted via _server_tool_scopes), is shared by status, inventory and the reload summary. The Agent's launch-profile view is process-wide and name-indexed, so filter_runtime_status_to_view() rebuilds connected/tools/status/error from the entries serving the profile: the root profile never shows a routed profile's connection (nor lets its own parked failed-spawn task borrow a routed profile's live same-named one). - Inventory lists only tools in the profile's own registry slot for servers it configures; responses carry runtime_scope. - Fail closed when the agent's routing does not match the resolved profile: hide runtime data (runtime_scope "unavailable", shown as a notice in the MCP panel, 14 locales) and refuse /reload-mcp. - /reload-mcp shuts down only the profile's own ledger names (parked included), drops its own connect cooldown/error entries so a failed spawn is retried (the wildcard cleared every cooldown; a scoped shutdown only clears the live keys it tears down), rediscovers under the profile's home and counts live connections only. The bare wildcard remains for agents without profile-scoped MCP. - Signature introspection is centralized (accepts_keywords) and falls back to the bare call instead of blanking every status; ledger key helpers resolve to the Agent's tools.mcp_tool_scope when present; tools.mcp_tool is resolved by full name so stubbed agents still work. - ARCHITECTURE.md §4.10 and docs/troubleshooting.md document the boundary. - Tests: agent-free contract (models adoption, parked tasks, cooldowns and the process-wide root view) and real-Agent integration (fake stdio MCP server, skips without hermes-agent + mcp SDK). Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Claude Fable 5.1 --- ARCHITECTURE.md | 46 ++ api/commands.py | 76 ++- api/mcp_runtime.py | 325 +++++++++++ api/profiles.py | 68 ++- api/routes.py | 103 +++- api/streaming.py | 13 +- docs/troubleshooting.md | 21 + static/i18n.js | 14 + static/panels.js | 7 +- tests/test_issue538_mcp_management.py | 2 +- tests/test_mcp_runtime_profile_scope.py | 454 +++++++++++++++ tests/test_mcp_runtime_profile_scope_unit.py | 557 +++++++++++++++++++ 12 files changed, 1628 insertions(+), 58 deletions(-) create mode 100644 api/mcp_runtime.py create mode 100644 tests/test_mcp_runtime_profile_scope.py create mode 100644 tests/test_mcp_runtime_profile_scope_unit.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e26d7613..d03f6eb1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -475,6 +475,52 @@ pointer-removed cases to `tests/test_agent_compat.py`. The resolver is compatibility-only: delete it, and import directly from the new homes, once the WebUI stops supporting Agents that predate the split. +### 4.10 MCP Runtime Profile Boundary + +Hermes Agent owns the in-process MCP ledger. It keys a connection by +`(profile_home_key, name)` and registers its tools in that profile's registry overlay +only when the calling task serves a *routed* profile: the context-local Hermes-home +override differs from the process home. Otherwise the connection uses the bare server +name and the global registry slot, which belong to the process profile. + +- `api.profiles._set_hermes_home()` is the single writer of the process-profile home: + startup (`init_profile_state()`) and `switch_profile(process_wide=True)` both go + through it, so `get_process_profile_home()` and the Agent pin + (`hermes_constants.pin_process_hermes_home()`, when the Agent provides it) are updated + in the same step as `HERMES_HOME`. Streaming turns still mirror their profile into + `os.environ['HERMES_HOME']` for legacy readers; without the pin, that mirror makes a + turn's own profile look like the process profile, so same-named servers of different + profiles share one bare-name connection. +- `/api/mcp/servers`, `/api/mcp/tools`, `/api/notes/sources` and `/reload-mcp` run + their Agent calls inside `api.mcp_runtime.mcp_runtime_scope()`, which binds the + request profile's home and secret scope (root profile included) without touching + `os.environ`. Status, tool count and inventory are read from one scope; the inventory + lists only tools in that profile's own registry slot for servers it configures. +- A connection *serves* a profile when that profile owns it (`_server_scope_keys`, else + the scope in the key) or adopted it (`_server_tool_scopes`, an identical shared + connection). `api.mcp_runtime.ledger_key_serves_view()` is the one predicate for + status, inventory and the reload summary. The Agent's launch-profile view (scope + `None`) is process-wide, so `filter_runtime_status_to_view()` downgrades + `get_mcp_status()` rows of servers the root profile does not serve to `configured` + rather than showing a routed profile's connection, tool count or connect error. +- The scope is trusted only when the Agent's routing decision matches the profile WebUI + resolved. When it cannot be confirmed (for example a same-profile turn's mirror on an + Agent without the pin), status and inventory withhold runtime data + (`runtime_scope: "unavailable"`, shown as a notice in the MCP panel) and + `/reload-mcp` refuses instead of resetting another owner's connection. +- `/reload-mcp` calls `shutdown_mcp_servers(scope=..., names=...)` with the profile's + own scope and the names of its live connections; `scope=None` without `names` is the + process-wide wildcard and is used only with Agents that predate profile-scoped MCP + (`runtime_scope: "legacy_process"`). A scoped shutdown only clears connect backoff for + the live keys it tears down, so WebUI also drops the profile's own cooldown/error + entries (`clear_profile_connect_cooldowns()`) before rediscovery: a server that failed + to spawn is retried by the reload, as the wildcard did, without touching another + owner's backoff. + +Status and inventory stay passive: they never start or probe an MCP server. Ledger key +helpers resolve to Hermes Agent's `tools.mcp_tool_scope` when present so the key shape +has one owner; the local fallbacks only cover Agents that predate that module. + --- ## 5. Frontend Architecture: Current State diff --git a/api/commands.py b/api/commands.py index 31f72315..f4d52ff5 100644 --- a/api/commands.py +++ b/api/commands.py @@ -257,11 +257,28 @@ def _run_codex_runtime_command(arg_string: str) -> str: def _run_reload_mcp_command() -> str: - """Execute the MCP reconnect path and return a short user-facing summary.""" + """Execute the MCP reconnect path and return a short user-facing summary. + + Scoped to the request profile: only that profile's connections are shut + down and rediscovered from that profile's config, so other profiles' + same-named servers keep running (agents with profile-scoped MCP; older + agents keep their process-wide reload). + """ with _RELOAD_MCP_LOCK: try: + import importlib from tools.mcp_tool import _servers, _lock + # Resolved by full name (like the ``from`` import above) so a stubbed + # ``tools.mcp_tool`` without a ``tools`` package still works. + _core = importlib.import_module("tools.mcp_tool") from api.agent_compat import agent_attr + from api.mcp_runtime import ( + accepts_keywords, + clear_profile_connect_cooldowns, + ledger_key_serves_view, + mcp_key_name, + mcp_runtime_scope, + ) shutdown_mcp_servers = agent_attr( "tools.mcp_tool", "shutdown_mcp_servers", "tools.mcp_tool_lifecycle" @@ -273,18 +290,55 @@ def _run_reload_mcp_command() -> str: logger.warning("Failed to import MCP runtime for /reload-mcp", exc_info=True) raise RuntimeError("MCP runtime unavailable") from exc - try: - with _lock: - old_servers = set(_servers.keys()) + with mcp_runtime_scope("/reload-mcp") as view: + if not view.trusted: + # Resetting under an unconfirmed scope could stop another profile's + # connection or rebuild this one under the wrong owner. + raise RuntimeError( + "MCP reload unavailable: this profile's MCP runtime scope could not be " + "confirmed (a chat turn on this profile may be running); retry when it finishes" + ) + try: + # Profile-scoped teardown needs both the scoped ledger and a + # shutdown that takes scope/names; otherwise keep the legacy reload. + legacy = view.legacy or not accepts_keywords(shutdown_mcp_servers, "scope", "names") - shutdown_mcp_servers() - new_tools = discover_mcp_tools() + def live_names() -> set: + # Same criteria as the status page: the connection serves this + # profile (owned or adopted) and is live. A parked task (failed + # spawn retained for retry) has no session and is not "connected". + return { + mcp_key_name(key) for key, server in _servers.items() + if (legacy or ledger_key_serves_view(_core, key, view)) + and getattr(server, "session", True) is not None + } - with _lock: - connected_servers = set(_servers.keys()) - except Exception as exc: - logger.warning("Failed to reload MCP servers", exc_info=True) - raise RuntimeError("Failed to reload MCP servers") from exc + with _lock: + old_servers = live_names() + # Every ledger entry serving this profile, parked ones included: + # a parked task left in place would make discovery skip the server. + owned_names = { + mcp_key_name(key) for key in _servers + if ledger_key_serves_view(_core, key, view) + } + + if legacy: + shutdown_mcp_servers() + else: + # ``names`` is required: ``scope=None`` alone is the process-wide + # wildcard. Only names with a ledger key are passed; shutdown + # ignores anything else, and a key of another owner is never selected. + shutdown_mcp_servers(scope=view.registry_scope, names=owned_names) + # 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) + new_tools = discover_mcp_tools() + + with _lock: + connected_servers = live_names() + except Exception as exc: + logger.warning("Failed to reload MCP servers", exc_info=True) + raise RuntimeError("Failed to reload MCP servers") from exc added = connected_servers - old_servers removed = old_servers - connected_servers diff --git a/api/mcp_runtime.py b/api/mcp_runtime.py new file mode 100644 index 00000000..c4b2658d --- /dev/null +++ b/api/mcp_runtime.py @@ -0,0 +1,325 @@ +"""Profile-bound view of Hermes Agent's in-process MCP runtime. + +The MCP status page, the tool inventory and ``/reload-mcp`` read or reset +Hermes Agent's MCP ledger from the HTTP request thread. Hermes Agent scopes +that ledger per profile (connection key ``(profile_home_key, name)``) only +when the calling task serves a *routed* profile, i.e. its context-local +Hermes-home override differs from the process home. These helpers bind the +request profile's home (without touching ``os.environ``) and check that the +agent's routing decision matches the profile WebUI resolved, so a read never +shows another profile's connection and a reload never resets one. + +Ledger helpers below take ``core`` (``tools.mcp_tool``) explicitly and read +its maps with defaults, so an agent that lacks a map degrades to "unscoped" +rather than raising inside a status read. +""" + +from __future__ import annotations + +import importlib +import inspect +import logging +from contextlib import contextmanager +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Callable, Generator, Iterable, Optional + +logger = logging.getLogger(__name__) + +# Scope label reported by the MCP endpoints. +SCOPE_PROFILE = "profile" # runtime view bound to the request profile +SCOPE_LEGACY = "legacy_process" # agent predates profile-scoped MCP; process-wide view +SCOPE_UNAVAILABLE = "unavailable" # profile scope could not be confirmed; runtime hidden + + +@dataclass(frozen=True) +class McpRuntimeView: + """How the current context sees Hermes Agent's MCP runtime. + + ``trusted``: runtime reads/writes in this context belong to ``profile_home``. + ``legacy``: the agent has no profile-scoped MCP ledger (process-wide by name). + ``registry_scope``: the tool-registry slot owning this profile's MCP tools + (the profile overlay key when routed, ``None`` for the process profile). + """ + + profile_home: Path + trusted: bool + legacy: bool + registry_scope: Optional[str] = None + + @property + def scope_label(self) -> str: + if not self.trusted: + return SCOPE_UNAVAILABLE + return SCOPE_LEGACY if self.legacy else SCOPE_PROFILE + + +def _routing_view(profile_home: Path, override_bound: bool) -> McpRuntimeView: + try: + from agent.secret_scope import serves_routed_profile + from hermes_constants import hermes_home_key + from tools.registry import registry + except ImportError: + return McpRuntimeView(profile_home, trusted=True, legacy=True) + from api.profiles import get_process_profile_home + + try: + expected_routed = hermes_home_key(profile_home) != hermes_home_key(get_process_profile_home()) + # Without the override a routed profile cannot be served; with it, the agent must + # agree. It disagrees when a streaming turn has mirrored this profile's home into + # os.environ['HERMES_HOME'] on an agent without a pinned process home. + if expected_routed and not override_bound: + return McpRuntimeView(profile_home, trusted=False, legacy=False) + if bool(serves_routed_profile()) != expected_routed: + return McpRuntimeView(profile_home, trusted=False, legacy=False) + scope = registry.current_scope_key() if expected_routed else None + except Exception: + logger.debug("Failed to resolve MCP runtime scope for %s", profile_home, exc_info=True) + return McpRuntimeView(profile_home, trusted=False, legacy=False) + return McpRuntimeView(profile_home, trusted=True, legacy=False, registry_scope=scope) + + +@contextmanager +def mcp_runtime_scope(purpose: str) -> Generator[McpRuntimeView, None, None]: + """Bind the active request profile for Hermes Agent MCP runtime calls. + + Installs the context-local Hermes-home override and secret scope for the + request profile (including the root profile) and restores them on exit, + including on exceptions. Never mutates ``os.environ``, starts, or probes + MCP servers. + """ + from api.profiles import get_active_hermes_home, profile_env_for_active_request_readonly + + profile_home = Path(get_active_hermes_home()) + with profile_env_for_active_request_readonly( + purpose, logger_override=logger, include_root=True + ) as override_bound: + yield _routing_view(profile_home, bool(override_bound)) + + +def accepts_keywords(fn: Callable, *names: str) -> bool: + """True when *fn*'s signature exposes every keyword in *names*. + + Introspection failures (builtins, C callables, exotic wrappers) count as + "not supported" so callers fall back to the bare call instead of dropping + the whole read. + """ + try: + params = inspect.signature(fn).parameters + except (TypeError, ValueError): + return False + return all(name in params for name in names) + + +# ── Ledger keys ────────────────────────────────────────────────────────────── +# Hermes Agent's own helpers (``tools.mcp_tool_scope``) are preferred so the key +# shape has one owner; the lambdas only cover agents that predate that module. + +def _key_helper(name: str, fallback: Callable) -> Callable: + try: + # Full-name import: works with a stubbed module and no ``tools`` package. + mcp_tool_scope = importlib.import_module("tools.mcp_tool_scope") + except Exception: + return fallback + helper = getattr(mcp_tool_scope, name, None) + return helper if callable(helper) else fallback + + +def mcp_key_name(key: Any) -> str: + """Server name of an MCP connection-ledger key (bare name or ``(scope, name)``).""" + return str(_key_helper("_key_name", lambda k: k[1] if isinstance(k, tuple) else k)(key)) + + +def mcp_key_scope(key: Any) -> Optional[str]: + """Owning registry scope encoded in a ledger key (``None`` for a bare key).""" + return _key_helper("_key_scope", lambda k: k[0] if isinstance(k, tuple) else None)(key) + + +def ledger_key_owner(core: Any, key: Any) -> Optional[str]: + """Scope owning the connection under *key*: recorded at adoption, else the key's own.""" + owners = getattr(core, "_server_scope_keys", None) + if isinstance(owners, dict) and key in owners: + return owners[key] + return mcp_key_scope(key) + + +def ledger_key_serves_view(core: Any, key: Any, view: McpRuntimeView) -> bool: + """True when the connection under *key* serves the profile in *view*. + + Owned by the view's registry scope, or adopted into it (a routed profile + sharing an identical connection; ``_server_tool_scopes``). Hermes Agent's + own launch-profile view (scope ``None``) is process-wide, so this predicate + is what narrows the root profile to its unscoped connections. Legacy agents + have one process-wide ledger, so every key counts. + """ + if view.legacy: + return True + if ledger_key_owner(core, key) == view.registry_scope: + return True + if view.registry_scope is None: + return False + adopters = getattr(core, "_server_tool_scopes", None) + if not isinstance(adopters, dict): + return False + return view.registry_scope in (adopters.get(key) or ()) + + +@dataclass(frozen=True) +class ServedRuntime: + """Runtime state of one server name as seen from a profile's ledger keys.""" + + live_tools: Optional[int] = None # tool count of a live (session-bearing) connection + connecting: bool = False + error: Optional[str] = None + lazy_tools: Optional[int] = None + + @property + def live(self) -> bool: + return self.live_tools is not None + + +def _served_keys(core: Any, ledger: str, view: McpRuntimeView) -> list: + keys = getattr(core, ledger, None) + if not isinstance(keys, (dict, set, frozenset, list, tuple)): + return [] + return [key for key in list(keys) if ledger_key_serves_view(core, key, view)] + + +def view_served_runtime(core: Any, view: McpRuntimeView) -> dict[str, ServedRuntime]: + """Per-name runtime state of the ledger entries serving *view*. + + Caller holds ``core._lock`` or tolerates a racy read (status surfaces). + """ + live: dict[str, int] = {} + connecting: set[str] = set() + errors: dict[str, str] = {} + lazy: dict[str, int] = {} + servers = getattr(core, "_servers", None) + if isinstance(servers, dict): + for key in _served_keys(core, "_servers", view): + server = servers.get(key) + if server is None or getattr(server, "session", None) is None: + continue # parked / mid-connect task: retained, not connected + names = getattr(server, "_registered_tool_names", None) + if not isinstance(names, (list, tuple, set)): + names = getattr(server, "_tools", None) or () + live[mcp_key_name(key)] = len(names) + for key in _served_keys(core, "_server_connecting", view): + connecting.add(mcp_key_name(key)) + error_map = getattr(core, "_server_connect_errors", None) + if isinstance(error_map, dict): + for key in _served_keys(core, "_server_connect_errors", view): + errors[mcp_key_name(key)] = str(error_map.get(key, "")) + lazy_tools = getattr(core, "_lazy_server_tool_names", None) + for key in _served_keys(core, "_lazy_server_configs", view): + count = lazy_tools.get(key) if isinstance(lazy_tools, dict) else None + lazy[mcp_key_name(key)] = len(count) if isinstance(count, (list, tuple, set)) else 0 + out: dict[str, ServedRuntime] = {} + for name in set(live) | connecting | set(errors) | set(lazy): + out[name] = ServedRuntime( + live_tools=live.get(name), + connecting=name in connecting, + error=errors.get(name), + lazy_tools=lazy.get(name), + ) + return out + + +_RUNTIME_ROW_FIELDS = ("connected", "tools", "status", "error", "sampling", "tool_schemas") + + +def filter_runtime_status_to_view(statuses: Iterable[Any], view: McpRuntimeView) -> list: + """Rebuild the runtime fields of ``get_mcp_status()`` rows from the keys serving *view*. + + Under a routed scope the agent already filters by owner/adopter; under the + launch profile (scope ``None``) it reports every profile's connection and + indexes them by bare name, so the root profile would show "Active, N tools" + for a routed profile's server it never connected (or its own parked server + as connected because another profile's same-named one is live). Rows keep + their config-derived fields (name, transport, disabled) and get + ``connected`` / ``tools`` / ``status`` / ``error`` from this profile's own + ledger entries only. + """ + rows = [row for row in statuses if isinstance(row, dict)] + if view.legacy or not rows: + return rows + try: + core = importlib.import_module("tools.mcp_tool") + except Exception: + return rows + lock = getattr(core, "_lock", None) + try: + if lock is not None: + with lock: + served = view_served_runtime(core, view) + else: + served = view_served_runtime(core, view) + except Exception: + logger.debug("Failed to read MCP ledger ownership for %s", view.profile_home, exc_info=True) + return [] + out = [] + for row in rows: + rebuilt = {k: v for k, v in row.items() if k not in _RUNTIME_ROW_FIELDS} + state = served.get(str(row.get("name"))) + disabled = bool(row.get("disabled")) or row.get("status") == "disabled" + rebuilt["connected"] = bool(state and state.live) + rebuilt["tools"] = state.live_tools if state and state.live else ( + state.lazy_tools if state and state.lazy_tools is not None and not disabled else 0 + ) + if state and state.live: + rebuilt["status"] = "connected" + elif disabled: + rebuilt["status"] = "disabled" + elif state and state.connecting: + rebuilt["status"] = "connecting" + elif state and state.error is not None: + rebuilt["status"] = "failed" + rebuilt["error"] = state.error + elif state and state.lazy_tools is not None: + rebuilt["status"] = "lazy" + else: + rebuilt["status"] = "configured" + out.append(rebuilt) + return out + + +_COOLDOWN_LEDGERS = ("_server_connect_retry_after", "_server_connect_failures", "_server_connect_errors") + + +def clear_profile_connect_cooldowns(core: Any, view: McpRuntimeView) -> None: + """Forget connect backoff/error state of the servers *view*'s profile owns. + + A server that failed to spawn never reaches ``_servers``, so a scoped + ``shutdown_mcp_servers(scope=..., names=...)`` (which clears cooldowns for + the live keys it tears down) leaves its backoff in place and the following + ``discover_mcp_tools()`` skips it. The process-wide wildcard cleared every + cooldown; this keeps ``/reload-mcp`` a retry for the profile's own servers + without touching another owner's backoff. Owned keys only, never adopted ones. + """ + lock = getattr(core, "_lock", None) + if lock is None: + return + with lock: + for ledger in _COOLDOWN_LEDGERS: + entries = getattr(core, ledger, None) + if not isinstance(entries, dict): + continue + for key in [k for k in entries if ledger_key_owner(core, k) == view.registry_scope]: + entries.pop(key, None) + + +def registry_tool_owned_by_view(registry, tool_name: str, view: McpRuntimeView) -> bool: + """True when *tool_name* is registered in this profile's own registry slot. + + The merged registry view overlays a profile's tools on the global slot, so a + routed profile would otherwise also list the process profile's MCP tools. + """ + if view.legacy: + return True + snapshot = getattr(registry, "snapshot_registration", None) + if not callable(snapshot): + return True + try: + return snapshot(tool_name, scope=view.registry_scope) is not None + except Exception: + return False diff --git a/api/profiles.py b/api/profiles.py index e56b32fd..c0a6fdb1 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -56,6 +56,11 @@ _loaded_profile_env_keys: set[str] = set() # process-global _active_profile. _tls = threading.local() +# Home of the profile this process serves as its own (set by init_profile_state). +# Streaming turns mirror their profile into os.environ['HERMES_HOME'], so the live +# env var is not a stable identity; see _pin_process_profile_home(). +_PROCESS_PROFILE_HOME: Optional[str] = None + _SKILL_HOME_MODULES = ("tools.skills_tool", "tools.skill_manager_tool") _SKILL_HOME_MODULE_PATCH_LOCK = threading.RLock() @@ -1360,6 +1365,8 @@ def profile_env_for_background_worker( def profile_env_for_active_request_readonly( purpose: str = "provider/model read", logger_override: Optional[logging.Logger] = None, + *, + include_root: bool = False, ): """Apply the active per-request profile's env to thread-local state only (#3957). @@ -1376,11 +1383,18 @@ def profile_env_for_active_request_readonly( process-global ``os.environ``. No-ops for the default/root profile, which is the common single-profile - deployment case. + deployment case, unless ``include_root`` is set: callers whose Hermes Agent + reads must not follow a streaming turn's mirrored ``HERMES_HOME`` (MCP + runtime status/reload) bind the root profile's home explicitly too. + + Yields True when the context-local Hermes-home override is installed for + the resolved profile, else False (no-op, older agent, or resolution error). """ profile = (get_active_profile_name() or "").strip() - if not profile or _is_root_profile(profile): - yield + if include_root and not profile: + profile = "default" + if not profile or (_is_root_profile(profile) and not include_root): + yield False return try: from api.config import _clear_thread_env, _set_thread_env, _thread_ctx @@ -1396,7 +1410,7 @@ def profile_env_for_active_request_readonly( purpose, exc_info=True, ) - yield + yield False return try: from hermes_constants import ( @@ -1440,7 +1454,7 @@ def profile_env_for_active_request_readonly( purpose, exc_info=True, ) - yield + yield home_override_installed finally: if _has_scope and _secret_scope_mod is not None: try: @@ -1529,8 +1543,14 @@ def profile_scope_for_detached_worker( def _set_hermes_home(home: Path): - """Set HERMES_HOME env var and monkey-patch cached module-level paths.""" + """Set HERMES_HOME env var and monkey-patch cached module-level paths. + + Every process-wide home change (startup, ``switch_profile(process_wide=True)``) + goes through here, so the process-profile pin used for MCP routing decisions + is updated in the same step and cannot drift from ``HERMES_HOME``. + """ os.environ['HERMES_HOME'] = str(home) + _pin_process_profile_home(home) patch_skill_home_modules(home) @@ -1610,11 +1630,45 @@ def init_profile_state() -> None: else: _active_profile = _read_active_profile_file() home = get_active_hermes_home() - _set_hermes_home(home) + _set_hermes_home(home) # also pins the process-profile home (MCP routing anchor) install_cron_scheduler_profile_isolation() _reload_dotenv(home) +def _pin_process_profile_home(home: Path) -> None: + """Record the profile home this process serves as its own, for WebUI and Hermes Agent. + + Called from ``_set_hermes_home()`` so startup and process-wide profile switches + keep one owner for the value. Hermes Agent keys MCP connections and registry + overlays by profile only when a task serves a *routed* profile: the context-local + Hermes-home override differs from the process home. Streaming turns mirror their + profile into ``os.environ['HERMES_HOME']`` for legacy readers, which makes every + turn's own profile look like the process profile, so same-named MCP servers of + different profiles share one bare-name connection. Agents exposing + ``hermes_constants.pin_process_hermes_home`` take a stable anchor instead; older + agents keep following the env var, and ``get_process_profile_home()`` still gives + WebUI the anchor to detect that skew. + """ + global _PROCESS_PROFILE_HOME + _PROCESS_PROFILE_HOME = str(home) + try: + import hermes_constants + pin = getattr(hermes_constants, 'pin_process_hermes_home', None) + if callable(pin): + pin(str(home)) + except Exception: + logger.debug("Hermes Agent process-home pin unavailable", exc_info=True) + + +def get_process_profile_home() -> Path: + """Return the profile home this WebUI process serves as its own (stable per process).""" + if _PROCESS_PROFILE_HOME: + return Path(_PROCESS_PROFILE_HOME) + if _INITIAL_HERMES_HOME: + return Path(_INITIAL_HERMES_HOME).expanduser() + return _DEFAULT_HERMES_HOME + + def switch_profile(name: str, *, process_wide: bool = True) -> dict: """Switch the active profile. diff --git a/api/routes.py b/api/routes.py index c0e5f92c..127ad108 100644 --- a/api/routes.py +++ b/api/routes.py @@ -29346,17 +29346,34 @@ def _parse_mcp_enabled(value) -> bool: return True -def _mcp_runtime_status_by_name() -> dict[str, dict]: +def _mcp_runtime_status_by_name(servers=None, view=None) -> dict[str, dict]: """Return already-known MCP runtime status without starting servers. ``tools.mcp_tool.get_mcp_status()`` only reads the existing MCP registry and configuration; it does not probe or spawn MCP subprocesses. If Hermes Agent is unavailable, fall back to an empty map so the API remains safe. + + Call it inside ``mcp_runtime_scope()`` and pass its ``view``: the agent + filters a routed profile's connections itself, but its launch-profile view + is process-wide, so rows are narrowed to the connections serving ``view`` + (``api.mcp_runtime.filter_runtime_status_to_view``). ``servers`` (the + profile's ``mcp_servers`` WebUI displays) is passed as ``configured`` when + supported so both read the same config. """ try: from api.agent_compat import agent_attr + from api.mcp_runtime import accepts_keywords, filter_runtime_status_to_view get_mcp_status = agent_attr("tools.mcp_tool", "get_mcp_status", "tools.mcp_tool_discovery") - statuses = get_mcp_status() + if isinstance(servers, dict) and accepts_keywords(get_mcp_status, "configured"): + # Invalid entries are summarized as invalid_config by WebUI; keep them + # out of the agent call so one bad entry cannot blank every status. + statuses = get_mcp_status(configured={ + str(name): scfg for name, scfg in servers.items() if isinstance(scfg, dict) + }) + else: + statuses = get_mcp_status() + if view is not None and isinstance(statuses, list): + statuses = filter_runtime_status_to_view(statuses, view) except Exception: return {} if not isinstance(statuses, list): @@ -29539,10 +29556,15 @@ def _mcp_tools_from_runtime_status(runtime_by_name, server_summaries): return tools -def _mcp_tools_from_registry(server_summaries): - """Read already-registered MCP tool schemas without probing MCP servers.""" +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. + """ try: from tools.registry import registry + from api.mcp_runtime import registry_tool_owned_by_view except Exception: return [] tools = [] @@ -29558,6 +29580,11 @@ def _mcp_tools_from_registry(server_summaries): 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) + ): + continue schema = registry.get_schema(tool_name) or {} server_summary = server_summaries.get(server_name, { "name": server_name, @@ -29569,22 +29596,45 @@ def _mcp_tools_from_registry(server_summaries): return tools +def _mcp_profile_runtime_inventory(servers, purpose, *, include_tools=True): + """Build server summaries and tools from ONE runtime view of the request profile. + + Status, tool count and inventory are all read inside the same + ``mcp_runtime_scope()`` so they describe the same profile's connections. + When that profile scope cannot be confirmed, runtime data is withheld rather + than showing another profile's connection. Passive: never starts or probes + MCP servers. + """ + from api.mcp_runtime import mcp_runtime_scope + + runtime = {} + tools = [] + source = "none" + with mcp_runtime_scope(purpose) as view: + if view.trusted: + runtime = _mcp_runtime_status_by_name(servers, view) + server_summaries = { + str(name): _server_summary(str(name), scfg, runtime.get(str(name))) + for name, scfg in servers.items() + } + if include_tools and view.trusted: + tools = _mcp_tools_from_runtime_status(runtime, server_summaries) + source = "mcp_runtime_status" + if not tools: + tools = _mcp_tools_from_registry(server_summaries, view) + source = "tool_registry" if tools else "none" + return server_summaries, tools, source, view.scope_label + + def _handle_mcp_tools_list(handler): """List known MCP tools from already-available runtime inventory only.""" cfg = get_config_for_profile_home(get_active_hermes_home()) servers = cfg.get("mcp_servers", {}) if not isinstance(servers, dict): servers = {} - runtime = _mcp_runtime_status_by_name() - server_summaries = { - str(name): _server_summary(str(name), scfg, runtime.get(str(name))) - for name, scfg in servers.items() - } - tools = _mcp_tools_from_runtime_status(runtime, server_summaries) - source = "mcp_runtime_status" - if not tools: - tools = _mcp_tools_from_registry(server_summaries) - source = "tool_registry" if tools else "none" + server_summaries, tools, source, runtime_scope = _mcp_profile_runtime_inventory( + servers, "/api/mcp/tools" + ) tools.sort(key=lambda row: (row.get("server", ""), row.get("name", ""))) unavailable_servers = [ summary["name"] for summary in server_summaries.values() @@ -29595,6 +29645,7 @@ def _handle_mcp_tools_list(handler): "total": len(tools), "source": source, "inventory_scope": "already_known_runtime_only", + "runtime_scope": runtime_scope, "unavailable_servers": unavailable_servers, }) @@ -29784,16 +29835,9 @@ def _handle_notes_sources_list(handler): servers = cfg.get("mcp_servers", {}) if not isinstance(servers, dict): servers = {} - runtime = _mcp_runtime_status_by_name() - server_summaries = { - str(name): _server_summary(str(name), scfg, runtime.get(str(name))) - for name, scfg in servers.items() - } - tools = _mcp_tools_from_runtime_status(runtime, server_summaries) - source = "mcp_runtime_status" - if not tools: - tools = _mcp_tools_from_registry(server_summaries) - source = "tool_registry" if tools else "none" + server_summaries, tools, source, _runtime_scope = _mcp_profile_runtime_inventory( + servers, "/api/notes/sources" + ) return j(handler, { "enabled": True, "sources": _notes_sources_from_mcp_inventory(server_summaries, tools), @@ -30089,15 +30133,14 @@ def _handle_mcp_servers_list(handler): servers = cfg.get("mcp_servers", {}) if not isinstance(servers, dict): servers = {} - runtime = _mcp_runtime_status_by_name() - result = [ - _server_summary(name, scfg, runtime.get(str(name))) - for name, scfg in servers.items() - ] + server_summaries, _tools, _source, runtime_scope = _mcp_profile_runtime_inventory( + servers, "/api/mcp/servers", include_tools=False + ) return j(handler, { - "servers": result, + "servers": list(server_summaries.values()), "toggle_supported": True, "reload_required": True, + "runtime_scope": runtime_scope, }) diff --git a/api/streaming.py b/api/streaming.py index 6f2b3b55..4c3264ab 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -10271,14 +10271,11 @@ def _run_agent_streaming( # loaded the default profile's `mcp_servers`, even when the session # was stamped with a non-default profile. See issue #1968. # - # NOTE: `_servers` in `tools/mcp_tool.py` is a process-global registry - # keyed by server name. This means once profile A registers a server - # named e.g. `postgres`, profile B's discovery sees it as already - # connected and skips it — even if B's config points at a different - # binary. Fully fixing multi-profile concurrent use requires keying - # `_servers` by `(profile_home, name)` upstream in hermes-agent; that - # lives outside this WebUI repo. This change fixes the headline bug - # for users who run a single non-default profile per WebUI process. + # NOTE: hermes-agent keys MCP connections by `(profile_home, name)` only + # for a routed profile (override != process home). The HERMES_HOME + # mirror above would make this turn's own profile look like the process + # profile, falling back to bare, cross-profile names, unless the agent + # honours the pin from `api.profiles._pin_process_profile_home()`. try: from api.agent_compat import agent_attr discover_mcp_tools = agent_attr("tools.mcp_tool", "discover_mcp_tools", "tools.mcp_tool_discovery") diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bb59d431..36fc8d56 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -309,6 +309,27 @@ Interpret the two together: --- +## MCP panel shows another profile's servers, or "Live status for this profile is unavailable" + +**Symptom.** With several profiles, the MCP settings panel of profile A shows a server as *Active* with a tool count while the tool inventory is empty (or lists profile B's tools); `/reload-mcp` on one profile stops the other profile's servers; or the panel shows the notice *"Live status for this profile is unavailable right now"* and `/reload-mcp` answers *"MCP runtime scope could not be confirmed"*. + +**Why.** Hermes Agent keeps one in-process MCP ledger per WebUI process and keys a connection by profile only when it can tell the request serves a profile other than the process's own. The WebUI binds every MCP status read and `/reload-mcp` to the request profile (`ARCHITECTURE.md` §4.10). While a chat turn is streaming, the WebUI mirrors that turn's profile into `HERMES_HOME`; Agents that predate `hermes_constants.pin_process_hermes_home` cannot distinguish that mirror from the process profile, so the WebUI withholds runtime data and refuses the reload instead of showing or resetting another profile's connection. + +**Diagnostic commands.** + +```bash +# runtime_scope: "profile" (bound), "legacy_process" (Agent without profile-scoped MCP), +# "unavailable" (scope could not be confirmed right now) +curl -s -b "hermes_profile=" http://127.0.0.1:8787/api/mcp/servers | python3 -m json.tool | grep -E '"(name|status|tool_count|runtime_scope)"' +python3 -c "import hermes_constants; print(hasattr(hermes_constants, 'pin_process_hermes_home'))" +``` + +**Fix.** `unavailable` while a turn is running is expected: refresh once the turn finishes. If it persists with no turn running, the Agent predates the process-home pin; upgrade Hermes Agent. `legacy_process` means the Agent has no profile-scoped MCP ledger at all; its `/reload-mcp` stays process-wide by design. After changing a profile's `mcp_servers`, run `/reload-mcp` **from that profile**: it only resets that profile's own connections and retries its failed servers. + +**When to file a bug.** File a WebUI bug if `runtime_scope` is `"profile"` and a server is still reported *Active* with tools you cannot see in the inventory, or if `/reload-mcp` from one profile changes another profile's `tool_count`. + +--- + ## Other troubleshooting This document grows over time. If a recurring failure mode isn't covered here yet, add it via PR. The format for each entry: **Symptom → Why → Diagnostic commands → Fix → When to file a bug**. diff --git a/static/i18n.js b/static/i18n.js index 269ffe43..44bb28b4 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -78,6 +78,7 @@ const LOCALES = { mcp_servers_title: 'MCP Servers', mcp_servers_desc: 'View MCP servers configured in config.yaml.', mcp_no_servers: 'No MCP servers configured.', + mcp_runtime_scope_unavailable: 'Live status for this profile is unavailable right now (a chat turn may be running). Refresh when it finishes.', mcp_add_server: '+ Add Server', mcp_field_name: 'Server Name', mcp_transport_label: 'Transport Type', @@ -1945,6 +1946,7 @@ const LOCALES = { mcp_servers_title: 'Server MCP', mcp_servers_desc: 'Visualizza i server MCP configurati in config.yaml.', mcp_no_servers: 'Nessun server MCP configurato.', + mcp_runtime_scope_unavailable: 'Lo stato in tempo reale di questo profilo non è al momento disponibile (potrebbe essere in corso un turno di chat). Aggiorna quando sarà terminato.', mcp_add_server: '+ Aggiungi Server', mcp_field_name: 'Nome Server', mcp_transport_label: 'Tipo Trasporto', @@ -3750,6 +3752,7 @@ const LOCALES = { mcp_servers_title: 'MCPサーバー', mcp_servers_desc: 'config.yaml で設定された MCP サーバーを管理します。', mcp_no_servers: 'MCPサーバーは設定されていません。', + mcp_runtime_scope_unavailable: 'このプロファイルのライブステータスは現在利用できません（チャットのターンが実行中の可能性があります）。完了後に更新してください。', mcp_add_server: '+ サーバーを追加', mcp_field_name: 'サーバー名', mcp_transport_label: 'トランスポート種別', @@ -5557,6 +5560,7 @@ const LOCALES = { mcp_servers_title: 'Серверы MCP', mcp_servers_desc: 'Просмотр серверов MCP, настроенных в config.yaml.', mcp_no_servers: 'Серверы MCP не настроены.', + mcp_runtime_scope_unavailable: 'Актуальный статус этого профиля сейчас недоступен (возможно, выполняется ход чата). Обновите, когда он завершится.', mcp_add_server: '+ Добавить сервер', mcp_field_name: 'Имя сервера', mcp_transport_label: 'Тип транспорта', @@ -7329,6 +7333,7 @@ const LOCALES = { mcp_servers_title: 'MCP 服务器', mcp_servers_desc: '管理 config.yaml 中配置的 MCP 服务器。', mcp_no_servers: '未配置 MCP 服务器。', + mcp_runtime_scope_unavailable: 'El estado en vivo de este perfil no está disponible ahora mismo (puede haber un turno de chat en curso). Actualiza cuando termine.', mcp_add_server: '+ 添加服务器', mcp_field_name: '服务器名称', mcp_transport_label: '传输类型', @@ -9080,6 +9085,7 @@ const LOCALES = { mcp_servers_title: 'MCP-Server', mcp_servers_desc: 'MCP-Server in config.yaml verwalten.', mcp_no_servers: 'Keine MCP-Server konfiguriert.', + mcp_runtime_scope_unavailable: 'Der Live-Status für dieses Profil ist derzeit nicht verfügbar (möglicherweise läuft gerade ein Chat-Zug). Aktualisieren Sie, wenn er abgeschlossen ist.', mcp_add_server: '+ Server hinzufügen', mcp_field_name: 'Servername', mcp_transport_label: 'Transporttyp', @@ -10825,6 +10831,7 @@ const LOCALES = { mcp_servers_title: 'MCP 服务器', mcp_servers_desc: '管理 config.yaml 中配置的 MCP 服务器。', mcp_no_servers: '未配置 MCP 服务器。', + mcp_runtime_scope_unavailable: '当前无法获取此配置的实时状态（可能有对话回合正在进行）。请在其完成后刷新。', mcp_add_server: '+ 添加服务器', mcp_field_name: '服务器名称', mcp_transport_label: '传输类型', @@ -12580,6 +12587,7 @@ const LOCALES = { mcp_servers_title: 'MCP 伺服器', mcp_servers_desc: '檢視 config.yaml 中設定的 MCP 伺服器。', mcp_no_servers: '未設定 MCP 伺服器。', + mcp_runtime_scope_unavailable: '目前無法取得此設定檔的即時狀態（可能有對話回合正在進行）。請在其完成後重新整理。', mcp_add_server: '+ 新增伺服器', mcp_field_name: '伺服器名稱', mcp_transport_label: '傳輸類型', @@ -15996,6 +16004,7 @@ const LOCALES = { mcp_servers_title: 'MCP Servers', mcp_servers_desc: 'Manage Model Context Protocol servers configured in config.yaml.', mcp_no_servers: 'No MCP servers configured.', + mcp_runtime_scope_unavailable: '이 프로필의 실시간 상태를 현재 확인할 수 없습니다(채팅 턴이 진행 중일 수 있음). 완료된 후 새로 고치세요.', mcp_add_server: '+ Add Server', mcp_field_name: 'Server Name', mcp_transport_label: 'Transport Type', @@ -17801,6 +17810,7 @@ const LOCALES = { mcp_servers_title: 'Serveurs MCP', mcp_servers_desc: 'Afficher les serveurs MCP configurés dans config.yaml.', mcp_no_servers: 'Aucun serveur MCP configuré.', + mcp_runtime_scope_unavailable: 'Le statut en direct de ce profil est indisponible pour le moment (un tour de conversation est peut-être en cours). Actualisez quand il sera terminé.', mcp_add_server: '+ Ajouter un serveur', mcp_field_name: 'Nom du serveur', mcp_transport_label: 'Type de transport', @@ -20909,6 +20919,7 @@ const LOCALES = { mcp_load_failed: 'Načtení serverů MCP se nezdařilo.', mcp_name_required: 'Je vyžadováno jméno serveru.', mcp_no_servers: 'Nejsou nakonfigurovány žádné servery MCP.', + mcp_runtime_scope_unavailable: 'Živý stav tohoto profilu není momentálně k dispozici (možná probíhá tah chatu). Obnovte, až skončí.', mcp_restart_hint: 'Změny serveru jsou zatím jen pro čtení. Upravte config.yaml a restartujte Hermes, aby se změny projevily.', mcp_save: 'Uložit', mcp_save_failed: 'Uložení serveru MCP se nezdařilo.', @@ -21350,6 +21361,7 @@ const LOCALES = { mcp_servers_title: 'MCP Sunucuları', mcp_servers_desc: 'config.yaml dosyasında yapılandırılmış MCP sunucularını görüntüleyin.', mcp_no_servers: 'Hiçbir MCP sunucusu yapılandırılmadı.', + mcp_runtime_scope_unavailable: 'Bu profilin canlı durumu şu anda kullanılamıyor (bir sohbet turu çalışıyor olabilir). Bittiğinde yenileyin.', mcp_add_server: '+ Sunucu Ekle', mcp_field_name: 'Sunucu Adı', mcp_transport_label: 'Taşıma Türü', @@ -23148,6 +23160,7 @@ const LOCALES = { mcp_servers_title: 'Serwery MCP', mcp_servers_desc: 'Wyświetl serwery MCP skonfigurowane w config.yaml.', mcp_no_servers: 'Brak skonfigurowanych serwerów MCP.', + mcp_runtime_scope_unavailable: 'Stan na żywo tego profilu jest obecnie niedostępny (może trwać tura czatu). Odśwież, gdy się zakończy.', mcp_add_server: '+ Dodaj serwer', mcp_field_name: 'Nazwa serwera', mcp_transport_label: 'Typ transportu', @@ -24941,6 +24954,7 @@ const LOCALES = { mcp_servers_title: 'Máy chủ MCP', mcp_servers_desc: 'Xem các máy chủ MCP được cấu hình trong config.yaml.', mcp_no_servers: 'Chưa có máy chủ MCP nào được cấu hình.', + mcp_runtime_scope_unavailable: 'Trạng thái trực tiếp của hồ sơ này hiện không khả dụng (có thể một lượt trò chuyện đang chạy). Hãy làm mới khi lượt đó kết thúc.', mcp_add_server: '+ Thêm máy chủ', mcp_field_name: 'Tên máy chủ', mcp_transport_label: 'Loại kết nối', diff --git a/static/panels.js b/static/panels.js index 4f708aae..c10ea35b 100644 --- a/static/panels.js +++ b/static/panels.js @@ -13234,7 +13234,12 @@ function loadMcpServers(){ list.innerHTML=`
${esc(t('mcp_no_servers'))}
`; return; } - list.innerHTML=r.servers.map(s=>{ + // Live status is withheld while the profile's runtime scope cannot be confirmed + // (e.g. a chat turn on this profile is running); say so instead of "not connected". + const scopeNotice=r.runtime_scope==='unavailable' + ?`
${esc(t('mcp_runtime_scope_unavailable'))}
` + :''; + list.innerHTML=scopeNotice+r.servers.map(s=>{ const transportLabel=s.transport==='http'?'HTTP':s.transport==='stdio'?'stdio':(''+(s.transport||'unknown')); const transportClass=s.transport==='http'?'mcp-http':s.transport==='stdio'?'mcp-stdio':'mcp-unknown'; const transportBadge=`${esc(transportLabel)}`; diff --git a/tests/test_issue538_mcp_management.py b/tests/test_issue538_mcp_management.py index dcbaa446..57f5da77 100644 --- a/tests/test_issue538_mcp_management.py +++ b/tests/test_issue538_mcp_management.py @@ -161,7 +161,7 @@ class TestMcpList: monkeypatch.setenv('HERMES_CONFIG_PATH', str(override_path)) monkeypatch.setattr(profiles, 'get_active_hermes_home', lambda: active_home) monkeypatch.setattr(routes, 'get_active_hermes_home', lambda: active_home) - monkeypatch.setattr(routes, '_mcp_runtime_status_by_name', lambda: {}) + monkeypatch.setattr(routes, '_mcp_runtime_status_by_name', lambda *_args, **_kwargs: {}) config.reload_config() h = _make_handler() diff --git a/tests/test_mcp_runtime_profile_scope.py b/tests/test_mcp_runtime_profile_scope.py new file mode 100644 index 00000000..2288b445 --- /dev/null +++ b/tests/test_mcp_runtime_profile_scope.py @@ -0,0 +1,454 @@ +"""MCP status, inventory and /reload-mcp follow the request profile. + +Two profiles configure a stdio MCP server with the same name (``atlassian``): +one read-only, one with write tools. These tests drive the real Hermes Agent MCP +runtime with a local fake MCP server (no network, no credentials) and check +that each profile sees and reloads only its own connection. + +Chat turns are simulated the way ``api/streaming.py`` runs them: the session +profile's home is mirrored into ``os.environ['HERMES_HOME']`` and installed as +the context-local Hermes-home override before ``discover_mcp_tools()``. +""" + +from __future__ import annotations + +import json +import os +import sys +import textwrap +import threading +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +try: + import mcp # noqa: F401 + import hermes_constants + from agent.secret_scope import current_secret_scope, serves_routed_profile # noqa: F401 + from tools import mcp_tool as mcp_core + from tools.mcp_tool_discovery import discover_mcp_tools + from tools.mcp_tool_lifecycle import shutdown_mcp_servers +except Exception as exc: # pragma: no cover - depends on the installed agent + pytest.skip( + f"requires hermes-agent with profile-scoped MCP (serves_routed_profile) and the mcp SDK: {exc}", + allow_module_level=True, + ) + +from api import profiles +from api.commands import execute_agent_command +from api.routes import _handle_mcp_servers_list, _handle_mcp_tools_list + +requires_process_home_pin = pytest.mark.skipif( + not hasattr(hermes_constants, "pin_process_hermes_home"), + reason="streaming turns need hermes_constants.pin_process_hermes_home for profile-scoped MCP keys", +) + +SCOPE_PROFILE = "profile" +SCOPE_UNAVAILABLE = "unavailable" +READ_TOOLS = {"mcp__atlassian__jira_get_issue", "mcp__atlassian__confluence_search"} +WRITE_TOOL = "mcp__atlassian__jira_create_issue" + +FAKE_SERVER = textwrap.dedent(''' + import os + from mcp.server.fastmcp import FastMCP + + mcp = FastMCP("fake-atlassian") + + @mcp.tool() + def jira_get_issue(key: str) -> str: + """Read a Jira issue (fake).""" + return key + + @mcp.tool() + def confluence_search(query: str) -> str: + """Search Confluence (fake).""" + return "[]" + + if os.environ.get("READ_ONLY_MODE", "true").lower() == "false": + @mcp.tool() + def jira_create_issue(summary: str) -> str: + """Create a Jira issue (fake, write).""" + return "created" + + mcp.run() +''') + + +def _server_cfg(script: Path, *, read_only: bool, account: str) -> dict: + # Distinct accounts, like distinct Atlassian credentials: identical routes would + # legitimately share one connection (hermes-agent adopts matching routes). + return { + "command": sys.executable, + "args": [str(script)], + "env": {"READ_ONLY_MODE": "true" if read_only else "false", "FAKE_ACCOUNT": account}, + "timeout": 30, + "connect_timeout": 30, + } + + +def _write_config(home: Path, servers: dict) -> None: + import yaml + + home.mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text(yaml.safe_dump({"mcp_servers": servers}), encoding="utf-8") + + +class Env: + def __init__(self, base: Path, script: Path): + self.base = base + self.script = script + + def home(self, profile: str) -> Path: + return self.base if profile == "default" else self.base / "profiles" / profile + + def configure(self, profile: str, **servers) -> None: + _write_config(self.home(profile), servers) + + def atlassian(self, profile: str, *, read_only: bool) -> dict: + return _server_cfg(self.script, read_only=read_only, account=profile) + + def chat_turn(self, profile: str) -> list[str]: + home = str(self.home(profile)) + old = os.environ.get("HERMES_HOME") + os.environ["HERMES_HOME"] = home + token = hermes_constants.set_hermes_home_override(home) + try: + return discover_mcp_tools() + finally: + hermes_constants.reset_hermes_home_override(token) + os.environ["HERMES_HOME"] = old + + def call(self, fn, profile: str) -> dict: + profiles.set_request_profile(profile) + try: + handler = MagicMock() + fn(handler) + return json.loads(handler.wfile.write.call_args[0][0]) + finally: + profiles.clear_request_profile() + + def servers(self, profile: str) -> dict: + payload = self.call(_handle_mcp_servers_list, profile) + return {srv["name"]: srv for srv in payload["servers"]} | {"_scope": payload.get("runtime_scope")} + + def tools(self, profile: str) -> dict: + return self.call(_handle_mcp_tools_list, profile) + + def reload(self, profile: str) -> str: + profiles.set_request_profile(profile) + try: + return execute_agent_command("/reload-mcp") + finally: + profiles.clear_request_profile() + + def connection(self, profile: str): + """The live connection object owned by *profile* for ``atlassian``.""" + key = (hermes_constants.hermes_home_key(self.home(profile)), "atlassian") + if profile == "default": + key = "atlassian" + with mcp_core._lock: + return mcp_core._servers.get(key) + + +@pytest.fixture +def mcp_env(tmp_path, monkeypatch): + base = tmp_path / "hermes-home" + base.mkdir() + script = tmp_path / "fake_atlassian.py" + script.write_text(FAKE_SERVER, encoding="utf-8") + + monkeypatch.setenv("HERMES_HOME", str(base)) + monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False) # suite-wide config pin + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False) + monkeypatch.setattr(profiles, "_is_root_profile", lambda name: name in ("", "default")) + monkeypatch.setattr(profiles, "_PROCESS_PROFILE_HOME", getattr(profiles, "_PROCESS_PROFILE_HOME", None), raising=False) + if hasattr(hermes_constants, "_PINNED_PROCESS_HERMES_HOME"): + monkeypatch.setattr( + hermes_constants, "_PINNED_PROCESS_HERMES_HOME", hermes_constants._PINNED_PROCESS_HERMES_HOME + ) + # What init_profile_state() does at startup for a WebUI launched on ``base``. + pin = getattr(profiles, "_pin_process_profile_home", None) or getattr( + hermes_constants, "pin_process_hermes_home", lambda _home: None + ) + pin(base) + shutdown_mcp_servers() + + env = Env(base, script) + env.configure("default") + env.configure("profile-read", atlassian=env.atlassian("profile-read", read_only=True)) + env.configure("profile-write", atlassian=env.atlassian("profile-write", read_only=False)) + yield env + shutdown_mcp_servers() + + +def _tool_names(payload: dict) -> set[str]: + return {tool["name"] for tool in payload["tools"]} + + +@requires_process_home_pin +def test_same_named_servers_stay_isolated_per_profile(mcp_env): + read_tools = set(mcp_env.chat_turn("profile-read")) + write_tools = set(mcp_env.chat_turn("profile-write")) + assert WRITE_TOOL not in read_tools + assert WRITE_TOOL in write_tools + assert mcp_env.connection("profile-read") is not mcp_env.connection("profile-write") + + for profile, expect_write in (("profile-read", False), ("profile-write", True)): + servers = mcp_env.servers(profile) + inventory = mcp_env.tools(profile) + names = _tool_names(inventory) + assert servers["atlassian"]["status"] == "active" + assert READ_TOOLS <= names + assert (WRITE_TOOL in names) is expect_write + # Status counter and inventory describe the same connection. + assert servers["atlassian"]["tool_count"] == inventory["total"] == len(names) + assert {tool["server"] for tool in inventory["tools"]} == {"atlassian"} + assert servers["_scope"] == SCOPE_PROFILE + + # The root profile configures no MCP server and must not see either profile's tools. + default_servers = mcp_env.servers("default") + assert set(default_servers) == {"_scope"} + assert mcp_env.tools("default")["total"] == 0 + + +def test_status_and_inventory_are_passive(mcp_env): + for profile in ("profile-write", "profile-read", "default"): + servers = mcp_env.servers(profile) + inventory = mcp_env.tools(profile) + if profile != "default": + assert servers["atlassian"]["status"] == "configured" + assert not servers["atlassian"]["active"] + assert inventory["unavailable_servers"] == ["atlassian"] + assert inventory["total"] == 0 + with mcp_core._lock: + assert not mcp_core._servers + assert not mcp_core._server_connecting + + +@requires_process_home_pin +def test_reload_applies_the_requested_profiles_config_only(mcp_env): + # profile-write connected while its config was still read-only (stale runtime). + mcp_env.configure("profile-write", atlassian=mcp_env.atlassian("profile-write", read_only=True)) + mcp_env.chat_turn("profile-read") + mcp_env.chat_turn("profile-write") + read_conn = mcp_env.connection("profile-read") + stale_write_conn = mcp_env.connection("profile-write") + + mcp_env.configure("profile-write", atlassian=mcp_env.atlassian("profile-write", read_only=False)) + stale = mcp_env.servers("profile-write")["atlassian"] + assert stale["env"]["READ_ONLY_MODE"] == "false" + assert WRITE_TOOL not in _tool_names(mcp_env.tools("profile-write")) + + output = mcp_env.reload("profile-write") + assert "Reconnected: atlassian" in output + + assert mcp_env.connection("profile-write") is not stale_write_conn + assert WRITE_TOOL in _tool_names(mcp_env.tools("profile-write")) + # The other profile's same-named connection was neither stopped nor replaced. + assert mcp_env.connection("profile-read") is read_conn + assert read_conn.session is not None + read_inventory = mcp_env.tools("profile-read") + assert WRITE_TOOL not in _tool_names(read_inventory) + assert mcp_env.servers("profile-read")["atlassian"]["tool_count"] == read_inventory["total"] + + +@requires_process_home_pin +def test_reloading_an_adopting_profile_keeps_the_owners_connection(mcp_env): + """Identical routes share one connection; the adopter's reload must not stop it.""" + 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 + # The adopted connection serves profile-write: status, inventory and the + # reload summary all count it (it is not "Added" from the user's point of view). + assert mcp_env.servers("profile-write")["atlassian"]["status"] == "active" + assert READ_TOOLS <= _tool_names(mcp_env.tools("profile-write")) + + mcp_env.configure("profile-write", atlassian=mcp_env.atlassian("profile-write", read_only=False)) + output = mcp_env.reload("profile-write") + assert "Reconnected: atlassian" in output and "Added" not in output + assert mcp_env.connection("profile-read") is owner_conn + assert owner_conn.session is not None + assert WRITE_TOOL in _tool_names(mcp_env.tools("profile-write")) + assert WRITE_TOOL not in _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)) + mcp_env.chat_turn("default") + mcp_env.chat_turn("profile-write") + root_conn = mcp_env.connection("default") + write_conn = mcp_env.connection("profile-write") + assert root_conn is not None and write_conn is not None + + assert "Reconnected: atlassian" in mcp_env.reload("default") + assert mcp_env.connection("default") is not root_conn + assert mcp_env.connection("profile-write") is write_conn + assert write_conn.session is not None + assert WRITE_TOOL not in _tool_names(mcp_env.tools("default")) + assert WRITE_TOOL in _tool_names(mcp_env.tools("profile-write")) + + +@requires_process_home_pin +def test_root_profile_status_never_shows_a_routed_profiles_connection(mcp_env): + """The agent's launch-profile view is process-wide; the root profile must only + report its own (unscoped) connection, never a routed profile's.""" + mcp_env.configure("default", atlassian=mcp_env.atlassian("default", read_only=True)) + mcp_env.chat_turn("profile-write") + assert mcp_env.connection("profile-write") is not None + assert mcp_env.connection("default") is None + + root = mcp_env.servers("default") + assert root["_scope"] == SCOPE_PROFILE + assert root["atlassian"]["status"] == "configured" and not root["atlassian"]["active"] + assert not root["atlassian"]["tool_count"] + assert mcp_env.tools("default")["total"] == 0 + + mcp_env.chat_turn("default") + root = mcp_env.servers("default") + root_inventory = mcp_env.tools("default") + assert root["atlassian"]["status"] == "active" + # Status counter and inventory describe the root profile's own connection + # (the agent also registers generic resource/prompt tools per server). + assert root["atlassian"]["tool_count"] == root_inventory["total"] + assert READ_TOOLS <= _tool_names(root_inventory) + assert WRITE_TOOL not in _tool_names(root_inventory) + + +@requires_process_home_pin +def test_reload_retries_a_server_that_failed_to_spawn(mcp_env): + """A failed spawn sits under the agent's connect backoff (30s+) and never enters + the ledger, so a scoped shutdown leaves that backoff alone; the profile's reload + must still retry it, without dropping another profile's backoff.""" + broken = {"command": str(mcp_env.base / "missing-mcp-binary"), "connect_timeout": 5} + mcp_env.configure("default", atlassian=broken) + mcp_env.configure("profile-write", atlassian=broken) + mcp_env.chat_turn("default") + mcp_env.chat_turn("profile-write") + # A permanent spawn failure is parked (task retained, no session) or dropped, + # depending on the agent; either way nothing is connected. + for profile in ("default", "profile-write"): + parked = mcp_env.connection(profile) + assert parked is None or parked.session is None + assert mcp_env.servers(profile)["atlassian"]["status"] == "configured" + retry_after = getattr(mcp_core, "_server_connect_retry_after", None) + if retry_after is None: + pytest.skip("agent has no connect-retry cooldown ledger") + write_key = (hermes_constants.hermes_home_key(mcp_env.home("profile-write")), "atlassian") + with mcp_core._lock: + assert write_key in retry_after and "atlassian" in retry_after + + mcp_env.configure("profile-write", atlassian=mcp_env.atlassian("profile-write", read_only=False)) + assert "Added: atlassian" in mcp_env.reload("profile-write") + assert mcp_env.connection("profile-write") is not None + assert WRITE_TOOL in _tool_names(mcp_env.tools("profile-write")) + with mcp_core._lock: + assert write_key not in retry_after + assert "atlassian" in retry_after # the root profile's own backoff is untouched + assert mcp_env.servers("default")["atlassian"]["status"] == "configured" + + +def test_unconfirmed_scope_hides_runtime_and_refuses_reload(mcp_env, monkeypatch): + """A live turn's HERMES_HOME mirror on an agent without the pin skews routing.""" + mcp_env.chat_turn("profile-read") + before = dict(mcp_core._servers) + if hasattr(hermes_constants, "_PINNED_PROCESS_HERMES_HOME"): + monkeypatch.setattr(hermes_constants, "_PINNED_PROCESS_HERMES_HOME", None) + monkeypatch.setenv("HERMES_HOME", str(mcp_env.home("profile-write"))) # mid-turn mirror + + servers = mcp_env.servers("profile-write") + inventory = mcp_env.tools("profile-write") + assert servers["atlassian"]["status"] == "configured" + assert servers["atlassian"]["tool_count"] is None + assert inventory["total"] == 0 + + with pytest.raises(RuntimeError, match="scope could not be confirmed"): + mcp_env.reload("profile-write") + assert dict(mcp_core._servers) == before + assert servers["_scope"] == inventory.get("runtime_scope") == SCOPE_UNAVAILABLE + + +@requires_process_home_pin +def test_disabled_and_failed_servers_are_reported_per_profile(mcp_env): + mcp_env.configure( + "profile-write", + atlassian=mcp_env.atlassian("profile-write", read_only=False), + off={**mcp_env.atlassian("profile-write", read_only=False), "enabled": False}, + broken={"command": str(mcp_env.base / "missing-mcp-binary"), "connect_timeout": 5}, + invalid="not-a-mapping", + ) + mcp_env.chat_turn("profile-write") + servers = mcp_env.servers("profile-write") + assert servers["atlassian"]["status"] == "active" + assert servers["off"]["status"] == "disabled" + assert servers["broken"]["status"] == "configured" + assert not servers["broken"]["active"] + assert servers["invalid"]["status"] == "invalid_config" + inventory = mcp_env.tools("profile-write") + assert inventory["unavailable_servers"] == ["broken"] + assert {tool["server"] for tool in inventory["tools"]} == {"atlassian"} + + +def test_runtime_scope_is_restored_after_an_exception(mcp_env): + from api.config import _thread_ctx + from api.mcp_runtime import mcp_runtime_scope + + previous_env = dict(getattr(_thread_ctx, "env", {}) or {}) + profiles.set_request_profile("profile-write") + try: + with pytest.raises(ValueError): + with mcp_runtime_scope("test") as view: + assert view.trusted + assert hermes_constants.get_hermes_home_override() == str(mcp_env.home("profile-write")) + assert current_secret_scope() is not None + raise ValueError("boom") + finally: + profiles.clear_request_profile() + assert hermes_constants.get_hermes_home_override() is None + assert current_secret_scope() is None + assert dict(getattr(_thread_ctx, "env", {}) or {}) == previous_env + assert os.environ["HERMES_HOME"] == str(mcp_env.base) + + +@requires_process_home_pin +def test_concurrent_profile_reads_and_reload_never_cross_profiles(mcp_env): + mcp_env.chat_turn("profile-read") + mcp_env.chat_turn("profile-write") + read_count = mcp_env.servers("profile-read")["atlassian"]["tool_count"] + write_count = mcp_env.servers("profile-write")["atlassian"]["tool_count"] + assert read_count != write_count + + barrier = threading.Barrier(3) + failures: list[str] = [] + + def poll(profile: str, expected: int): + barrier.wait() + for _ in range(20): + srv = mcp_env.servers(profile)["atlassian"] + names = _tool_names(mcp_env.tools(profile)) + allowed = (expected,) if profile == "profile-read" else (expected, 0, None) + if srv["tool_count"] not in allowed: + failures.append(f"{profile} saw {srv['tool_count']} tools") + if profile == "profile-read" and WRITE_TOOL in names: + failures.append("profile-read inventory leaked a write tool") + + def reload_write(): + barrier.wait() + mcp_env.reload("profile-write") + + threads = [ + threading.Thread(target=poll, args=("profile-read", read_count)), + threading.Thread(target=poll, args=("profile-write", write_count)), + threading.Thread(target=reload_write), + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=120) + assert not failures + assert mcp_env.servers("profile-read")["atlassian"]["tool_count"] == read_count + assert mcp_env.servers("profile-write")["atlassian"]["tool_count"] == write_count diff --git a/tests/test_mcp_runtime_profile_scope_unit.py b/tests/test_mcp_runtime_profile_scope_unit.py new file mode 100644 index 00000000..0993cc28 --- /dev/null +++ b/tests/test_mcp_runtime_profile_scope_unit.py @@ -0,0 +1,557 @@ +"""Agent-free contract tests for profile-scoped MCP status, inventory and /reload-mcp. + +``tests/test_mcp_runtime_profile_scope.py`` drives the real Hermes Agent MCP +runtime; CI has no Hermes Agent checkout, so this file models the small agent +surface WebUI relies on (context-local home override, routed-profile predicate, +scoped registry slots, scoped MCP ledger with adoption and connect cooldowns) +and pins the WebUI side of the contract: which profile is bound, which scope is +reset, what the root profile may see, and the fail-closed paths. +""" + +from __future__ import annotations + +import json +import os +import sys +import threading +import types +from contextvars import ContextVar +from pathlib import Path +from types import SimpleNamespace +from typing import Any, Optional +from unittest.mock import MagicMock + +import pytest +import yaml + +from api import profiles + + +class FakeAgent: + """Minimal model of the hermes-agent MCP surface used by WebUI. + + Mirrors the agent's visibility rules: a routed scope sees the connections it + owns or adopted; the launch profile's view (scope ``None``) is process-wide. + """ + + def __init__(self, *, pin_supported=True, scoped=True): + self.override: ContextVar[Optional[str]] = ContextVar("fake_home_override", default=None) + self.secret: ContextVar[Optional[dict]] = ContextVar("fake_secret_scope", default=None) + self.pinned = None + self.servers = {} # ledger key -> connection + self.owners = {} # ledger key -> owner scope + self.adopters = {} # ledger key -> {adopter scope} + self.connecting = set() + self.errors = {} # ledger key -> error text + self.retry_after = {} # ledger key -> deadline (connect cooldown) + self.failures = {} # ledger key -> failure count + self.lazy = {} # ledger key -> cfg + self.global_tools = {} # tool name -> (toolset, schema) + self.scoped_tools = {} # scope -> {tool name -> (toolset, schema)} + self.calls = [] + self.lock = threading.Lock() + self.modules = self._build_modules(pin_supported, scoped) + + # hermes_constants + def home(self): + return self.override.get() or os.environ.get("HERMES_HOME", "") + + @staticmethod + def key(path): + return str(Path(path).resolve()) + + def routed(self): + override = self.override.get() + anchor = self.pinned or os.environ.get("HERMES_HOME", "") + return override is not None and self.key(override) != self.key(anchor) + + def scope(self): + return self.key(self.home()) if self.routed() else None + + def _merged(self, scope): + return {**self.global_tools, **self.scoped_tools.get(scope, {})} + + def _slot(self, scope): + return self.global_tools if scope is None else self.scoped_tools.setdefault(scope, {}) + + @staticmethod + def ledger_key(name, scope): + return name if scope is None else (scope, name) + + def visible(self, key, scope): + """The agent's ``_server_visible_in_scope``: process-wide for scope ``None``.""" + if scope is None: + return True + return self.owners.get(key) == scope or scope in self.adopters.get(key, ()) + + def connect(self, name, tools, *, scope): + key = self.ledger_key(name, scope) + self.servers[key] = SimpleNamespace(name=name, session=object(), _registered_tool_names=list(tools)) + self.owners[key] = scope + for tool in tools: + self._slot(scope)[tool] = (f"mcp-{name}", {"description": f"{tool} ({scope})"}) + return self.servers[key] + + def adopt(self, name, *, owner, adopter): + """*adopter* shares *owner*'s identical connection (agent: ``_server_tool_scopes``).""" + key = self.ledger_key(name, owner) + self.adopters.setdefault(key, set()).add(adopter) + for tool in self.servers[key]._registered_tool_names: + self._slot(adopter)[tool] = (f"mcp-{name}", {"description": f"{tool} ({adopter})"}) + + def park(self, name, *, scope, error="spawn failed"): + """A permanent spawn failure the agent parks: task retained in ``_servers`` + without a session, error + backoff recorded under its key.""" + key = self.ledger_key(name, scope) + self.servers[key] = SimpleNamespace(name=name, session=None, _registered_tool_names=[]) + self.owners[key] = scope + self.fail(name, scope=scope, error=error) + return self.servers[key] + + def fail(self, name, *, scope, error="spawn failed"): + """A connect failure: never in ``_servers``; error + backoff recorded under its key.""" + key = self.ledger_key(name, scope) + self.errors[key] = error + self.failures[key] = self.failures.get(key, 0) + 1 + self.retry_after[key] = float("inf") + + def _build_modules(self, pin_supported, scoped): + agent = self + hc: Any = types.ModuleType("hermes_constants") + hc.set_hermes_home_override = lambda path: agent.override.set(None if path is None else str(path)) + hc.reset_hermes_home_override = lambda token: agent.override.reset(token) + hc.get_hermes_home_override = lambda: agent.override.get() + hc.hermes_home_key = lambda path=None: agent.key(path if path is not None else agent.home()) + if pin_supported: + def pin(path): + agent.pinned = None if path is None else str(path) + hc.pin_process_hermes_home = pin + + agent_pkg: Any = types.ModuleType("agent") + agent_pkg.__path__ = [] + secret: Any = types.ModuleType("agent.secret_scope") + secret.set_secret_scope = lambda mapping: agent.secret.set(dict(mapping)) + secret.reset_secret_scope = lambda token: agent.secret.reset(token) + secret.current_secret_scope = lambda: agent.secret.get() + if scoped: + secret.serves_routed_profile = agent.routed + + tools_pkg: Any = types.ModuleType("tools") + tools_pkg.__path__ = [] + registry_mod: Any = types.ModuleType("tools.registry") + + class Registry: + @staticmethod + def current_scope_key(): + return agent.key(agent.home()) + + def _view(self): + return agent._merged(agent.scope() if scoped else None) + + def get_all_tool_names(self): + return sorted(self._view()) + + def get_toolset_for_tool(self, name): + return self._view().get(name, (None, None))[0] + + def get_schema(self, name): + return self._view().get(name, (None, None))[1] + + def snapshot_registration(self, name, *, scope=None): + slot = agent.global_tools if scope is None else agent.scoped_tools.get(scope, {}) + return slot.get(name) + + registry_mod.registry = Registry() + + scope_mod: Any = types.ModuleType("tools.mcp_tool_scope") + scope_mod._key_name = lambda key: key[1] if isinstance(key, tuple) else key + scope_mod._key_scope = lambda key: key[0] if isinstance(key, tuple) else None + + mcp_mod: Any = types.ModuleType("tools.mcp_tool") + mcp_mod._servers = agent.servers + mcp_mod._lock = agent.lock + mcp_mod._server_scope_keys = agent.owners + mcp_mod._server_tool_scopes = agent.adopters + mcp_mod._server_connecting = agent.connecting + mcp_mod._server_connect_errors = agent.errors + mcp_mod._server_connect_retry_after = agent.retry_after + mcp_mod._server_connect_failures = agent.failures + mcp_mod._lazy_server_configs = agent.lazy + + def get_mcp_status(configured=None): + agent.calls.append(("status", agent.override.get(), dict(configured or {}))) + scope = agent.scope() if scoped else None + with agent.lock: + # As the agent: keyed by bare name, last key wins (process-wide for scope None). + active = {scope_mod._key_name(k): s for k, s in agent.servers.items() if agent.visible(k, scope)} + errors = {scope_mod._key_name(k): e for k, e in agent.errors.items() if agent.visible(k, scope)} + rows = [] + for name, cfg in (configured or {}).items(): + enabled = cfg.get("enabled", True) is not False + server = active.get(name) + live = server is not None and server.session is not None + status = ("connected" if live else "disabled" if not enabled + else "failed" if name in errors else "configured") + row = {"name": name, "connected": live, "disabled": not enabled, "status": status, + "tools": len(server._registered_tool_names) if live else 0} + if status == "failed": + row["error"] = errors[name] + rows.append(row) + return rows + + def shutdown_mcp_servers(*, scope=None, names=None, timeout=15.0): + agent.calls.append(("shutdown", scope, None if names is None else set(names))) + wildcard = scope is None and names is None + with agent.lock: + selected = [k for k in agent.servers + if wildcard or (agent.owners.get(k) == scope + and (names is None or scope_mod._key_name(k) in names))] + for key in selected: + del agent.servers[key] + agent.owners.pop(key, None) + agent.adopters.pop(key, None) + # Cooldown/error state: the wildcard drops everything, a scoped call + # only what it tore down (the agent's ``selected_status``). + for key in (list(agent.errors) if wildcard else selected): + agent.errors.pop(key, None) + agent.retry_after.pop(key, None) + agent.failures.pop(key, None) + + 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 = [] + 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 + for k in agent.servers) or key in agent.servers + if served: + names += [t for k, s in agent.servers.items() + if scope_mod._key_name(k) == name for t in s._registered_tool_names] + continue + if key in agent.retry_after: # backoff honoured: no retry + continue + if srv.get("fake_fail"): + agent.fail(name, scope=scope, error=str(srv["fake_fail"])) + continue + agent.connect(name, srv.get("fake_tools", []), scope=scope) + names += srv.get("fake_tools", []) + return names + + mcp_mod.get_mcp_status = get_mcp_status + mcp_mod.shutdown_mcp_servers = shutdown_mcp_servers + mcp_mod.discover_mcp_tools = discover_mcp_tools + return { + "hermes_constants": hc, + "agent": agent_pkg, + "agent.secret_scope": secret, + "tools": tools_pkg, + "tools.registry": registry_mod, + "tools.mcp_tool_scope": scope_mod, + "tools.mcp_tool": mcp_mod, + "tools.mcp_tool_discovery": mcp_mod, + "tools.mcp_tool_lifecycle": mcp_mod, + } + + +READ = ["mcp__atlassian__jira_get_issue"] +WRITE = READ + ["mcp__atlassian__jira_create_issue"] + + +def _install(monkeypatch, tmp_path, **agent_kwargs): + agent = FakeAgent(**agent_kwargs) + for name, module in agent.modules.items(): + monkeypatch.setitem(sys.modules, name, module) + base = tmp_path / "home" + for profile, tools in (("default", None), ("profile-read", READ), ("profile-write", WRITE)): + home = base if profile == "default" else base / "profiles" / profile + home.mkdir(parents=True, exist_ok=True) + servers = {} if tools is None else {"atlassian": {"command": "fake", "fake_tools": tools}} + (home / "config.yaml").write_text(yaml.safe_dump({"mcp_servers": servers})) + monkeypatch.setenv("HERMES_HOME", str(base)) + monkeypatch.delenv("HERMES_CONFIG_PATH", raising=False) + monkeypatch.setattr(profiles, "_DEFAULT_HERMES_HOME", base) + monkeypatch.setattr(profiles, "_is_isolated_profile_mode", lambda: False) + monkeypatch.setattr(profiles, "_is_root_profile", lambda name: name in ("", "default")) + monkeypatch.setattr(profiles, "_secret_scope_available", None) + monkeypatch.setattr(profiles, "_PROCESS_PROFILE_HOME", None) + profiles._pin_process_profile_home(base) + agent.base = base + return agent + + +def _configure(agent, profile, servers): + home = agent.base if profile == "default" else agent.base / "profiles" / profile + (home / "config.yaml").write_text(yaml.safe_dump({"mcp_servers": servers})) + + +def _as_profile(profile, fn, *args): + profiles.set_request_profile(profile) + try: + return fn(*args) + finally: + profiles.clear_request_profile() + + +def _call(profile, handler_fn): + handler = MagicMock() + _as_profile(profile, handler_fn, handler) + return json.loads(handler.wfile.write.call_args[0][0]) + + +def _reload(profile): + from api.commands import execute_agent_command + return _as_profile(profile, execute_agent_command, "/reload-mcp") + + +def _profile_scope(agent, profile): + return agent.key(agent.base / "profiles" / profile) + + +def _servers_by_name(payload): + return {row["name"]: row for row in payload["servers"]} + + +def test_init_pins_the_process_profile_home_for_the_agent(monkeypatch, tmp_path): + agent = _install(monkeypatch, tmp_path) + assert agent.pinned == str(agent.base) + assert profiles.get_process_profile_home() == agent.base + + +def test_process_wide_home_change_repins_in_the_same_step(monkeypatch, tmp_path): + """``_set_hermes_home`` is the single writer of the process home: startup and + ``switch_profile(process_wide=True)`` both go through it, so the MCP routing + anchor (WebUI and agent pin) can never drift from ``HERMES_HOME``.""" + agent = _install(monkeypatch, tmp_path) + new_home = agent.base / "profiles" / "profile-write" + profiles._set_hermes_home(new_home) + assert os.environ["HERMES_HOME"] == str(new_home) + assert profiles.get_process_profile_home() == new_home + assert agent.pinned == str(new_home) + + +def test_status_and_inventory_bind_the_request_profile(monkeypatch, tmp_path): + from api.routes import _handle_mcp_servers_list, _handle_mcp_tools_list + + agent = _install(monkeypatch, tmp_path) + agent.connect("atlassian", READ, scope=_profile_scope(agent, "profile-read")) + agent.connect("atlassian", WRITE, scope=_profile_scope(agent, "profile-write")) + + servers = _call("profile-write", _handle_mcp_servers_list) + tools = _call("profile-write", _handle_mcp_tools_list) + assert servers["runtime_scope"] == tools["runtime_scope"] == "profile" + assert servers["servers"][0]["tool_count"] == 2 + assert {t["name"] for t in tools["tools"]} == set(WRITE) + # get_mcp_status ran under the profile's home and was handed the displayed config. + status_calls = [c for c in agent.calls if c[0] == "status"] + assert all(c[1] == str(agent.base / "profiles" / "profile-write") for c in status_calls) + assert all(set(c[2]) == {"atlassian"} for c in status_calls) + # Nothing leaked into the request thread after the calls. + assert agent.override.get() is None and agent.secret.get() is None + + read_tools = _call("profile-read", _handle_mcp_tools_list) + assert {t["name"] for t in read_tools["tools"]} == set(READ) + assert _call("default", _handle_mcp_tools_list)["total"] == 0 + assert not [c for c in agent.calls if c[0] in ("discover", "shutdown")] + + +def test_inventory_excludes_the_process_profiles_global_slot(monkeypatch, tmp_path): + from api.routes import _handle_mcp_tools_list + + agent = _install(monkeypatch, tmp_path) + agent.connect("atlassian", WRITE, scope=None) # launch profile's unscoped connection + agent.connect("atlassian", READ, scope=_profile_scope(agent, "profile-read")) + names = {t["name"] for t in _call("profile-read", _handle_mcp_tools_list)["tools"]} + assert names == set(READ) + + +def test_root_profile_status_hides_routed_profiles_runtime(monkeypatch, tmp_path): + """The agent's launch-profile view is process-wide; WebUI narrows it to the + root profile's own (unscoped) connections, so a routed profile's live + connection or connect error never shows up as the root profile's.""" + from api.routes import _handle_mcp_servers_list, _handle_mcp_tools_list + + agent = _install(monkeypatch, tmp_path) + _configure(agent, "default", {"atlassian": {"command": "fake", "fake_tools": READ}}) + agent.connect("atlassian", WRITE, scope=_profile_scope(agent, "profile-write")) + agent.fail("atlassian", scope=_profile_scope(agent, "profile-read"), error="bad token") + + servers = _call("default", _handle_mcp_servers_list) + row = _servers_by_name(servers)["atlassian"] + assert servers["runtime_scope"] == "profile" + assert row["status"] == "configured" and row["active"] is False + assert not row["tool_count"] + assert _call("default", _handle_mcp_tools_list)["total"] == 0 + + # The root profile's own parked task (failed spawn, no session) must not borrow + # the routed profile's live same-named connection: the agent's process-wide + # view indexes by bare name and the last key wins. + agent.park("atlassian", scope=None, error="exec: not found") + row = _servers_by_name(_call("default", _handle_mcp_servers_list))["atlassian"] + assert row["status"] == "configured" and row["active"] is False and not row["tool_count"] + + # The root profile's own connection is still reported normally. + agent.connect("atlassian", READ, scope=None) + row = _servers_by_name(_call("default", _handle_mcp_servers_list))["atlassian"] + assert row["status"] == "active" and row["tool_count"] == len(READ) + assert {t["name"] for t in _call("default", _handle_mcp_tools_list)["tools"]} == set(READ) + + +def test_adopted_connection_serves_status_inventory_and_reload(monkeypatch, tmp_path): + """A profile that adopted another profile's identical connection sees it as + active; its reload counts it and never tears down the owner's connection.""" + from api.routes import _handle_mcp_servers_list, _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) + + row = _servers_by_name(_call("profile-write", _handle_mcp_servers_list))["atlassian"] + assert row["status"] == "active" and row["tool_count"] == len(READ) + assert {t["name"] for t in _call("profile-write", _handle_mcp_tools_list)["tools"]} == set(READ) + + output = _reload("profile-write") + assert "Reconnected: atlassian" in output + assert "Added" not in output and "No MCP servers connected" not in output + assert ("shutdown", write_scope, {"atlassian"}) in agent.calls + assert agent.servers[(read_scope, "atlassian")] is owner_conn + assert write_scope in agent.adopters[(read_scope, "atlassian")] + + +def test_skewed_routing_hides_runtime_and_refuses_reload(monkeypatch, tmp_path): + from api.routes import _handle_mcp_servers_list, _handle_mcp_tools_list + + agent = _install(monkeypatch, tmp_path, pin_supported=False) + agent.connect("atlassian", READ, scope=None) + monkeypatch.setenv("HERMES_HOME", str(agent.base / "profiles" / "profile-write")) # turn mirror + + servers = _call("profile-write", _handle_mcp_servers_list) + assert servers["runtime_scope"] == "unavailable" + assert servers["servers"][0]["status"] == "configured" + assert servers["servers"][0]["tool_count"] is None + assert _call("profile-write", _handle_mcp_tools_list)["total"] == 0 + with pytest.raises(RuntimeError, match="could not be confirmed"): + _reload("profile-write") + assert not [c for c in agent.calls if c[0] in ("discover", "shutdown")] + assert "atlassian" in agent.servers + + +def test_reload_resets_only_the_requested_profile(monkeypatch, tmp_path): + agent = _install(monkeypatch, tmp_path) + read_scope = _profile_scope(agent, "profile-read") + write_scope = _profile_scope(agent, "profile-write") + read_conn = agent.connect("atlassian", READ, scope=read_scope) + agent.connect("atlassian", READ, scope=write_scope) # stale write-profile connection + + output = _reload("profile-write") + assert "Reconnected: atlassian" in output + assert ("shutdown", write_scope, {"atlassian"}) in agent.calls + assert ("discover", str(agent.base / "profiles" / "profile-write")) in agent.calls + assert agent.servers[(read_scope, "atlassian")] is read_conn + assert agent.servers[(write_scope, "atlassian")]._registered_tool_names == WRITE + + +def test_reload_retries_the_profiles_failed_servers_only(monkeypatch, tmp_path): + """A server that failed to spawn is under connect backoff and absent from the + ledger, so a scoped shutdown leaves its cooldown alone; the reload must still + retry it (as the process-wide wildcard did) without touching other owners'.""" + agent = _install(monkeypatch, tmp_path) + write_scope = _profile_scope(agent, "profile-write") + agent.fail("atlassian", scope=write_scope, error="exec: not found") + agent.fail("atlassian", scope=None, error="root still broken") # root's own backoff + + _configure(agent, "profile-write", {"atlassian": {"command": "fake", "fake_tools": WRITE}}) + output = _reload("profile-write") + assert "Added: atlassian" in output + assert ("shutdown", write_scope, set()) in agent.calls + assert agent.servers[(write_scope, "atlassian")]._registered_tool_names == WRITE + assert (write_scope, "atlassian") not in agent.errors + assert "atlassian" in agent.retry_after and "atlassian" in agent.errors # root untouched + + +def test_root_profile_reload_never_uses_the_wildcard_shutdown(monkeypatch, tmp_path): + agent = _install(monkeypatch, tmp_path) + _configure(agent, "default", {"atlassian": {"command": "fake", "fake_tools": READ}}) + agent.connect("atlassian", READ, scope=None) + write_conn = agent.connect("atlassian", WRITE, scope=_profile_scope(agent, "profile-write")) + + assert "Reconnected: atlassian" in _reload("default") + shutdowns = [c for c in agent.calls if c[0] == "shutdown"] + assert shutdowns == [("shutdown", None, {"atlassian"})] + assert agent.servers[(_profile_scope(agent, "profile-write"), "atlassian")] is write_conn + + +def test_legacy_agent_keeps_process_wide_reload_but_reads_the_request_profile(monkeypatch, tmp_path): + from api.routes import _handle_mcp_servers_list + + agent = _install(monkeypatch, tmp_path, pin_supported=False, scoped=False) + assert _call("profile-write", _handle_mcp_servers_list)["runtime_scope"] == "legacy_process" + _reload("profile-write") + assert ("shutdown", None, None) in agent.calls + assert ("discover", str(agent.base / "profiles" / "profile-write")) in agent.calls + + +def test_status_read_survives_an_uninspectable_get_mcp_status(monkeypatch, tmp_path): + """Signature introspection failing must fall back to the bare call, not blank + every server's status.""" + from api.routes import _handle_mcp_servers_list + + agent = _install(monkeypatch, tmp_path) + agent.connect("atlassian", WRITE, scope=_profile_scope(agent, "profile-write")) + real = agent.modules["tools.mcp_tool"].get_mcp_status + + class Uninspectable: + @property + def __signature__(self): + raise ValueError("no signature") + + def __call__(self, *args, **kwargs): + assert not args and not kwargs, "bare call expected" + return real(configured={"atlassian": {"command": "fake"}}) + + monkeypatch.setattr(agent.modules["tools.mcp_tool"], "get_mcp_status", Uninspectable()) + row = _servers_by_name(_call("profile-write", _handle_mcp_servers_list))["atlassian"] + assert row["status"] == "active" and row["tool_count"] == len(WRITE) + + +def test_ledger_key_helpers_prefer_the_agents_and_fall_back_without_them(monkeypatch, tmp_path): + from api import mcp_runtime + + agent = _install(monkeypatch, tmp_path) + assert mcp_runtime.mcp_key_name(("scope-a", "atlassian")) == "atlassian" + assert mcp_runtime.mcp_key_scope(("scope-a", "atlassian")) == "scope-a" + assert mcp_runtime.mcp_key_name("atlassian") == "atlassian" + assert mcp_runtime.mcp_key_scope("atlassian") is None + monkeypatch.delitem(sys.modules, "tools.mcp_tool_scope") + assert mcp_runtime.mcp_key_name(("scope-a", "atlassian")) == "atlassian" + assert mcp_runtime.mcp_key_scope("atlassian") is None + assert mcp_runtime.accepts_keywords(len, "scope") is False + assert mcp_runtime.accepts_keywords(agent.modules["tools.mcp_tool"].shutdown_mcp_servers, "scope", "names") + + +def test_readonly_profile_scope_default_is_unchanged_and_root_is_opt_in(monkeypatch, tmp_path): + agent = _install(monkeypatch, tmp_path) + with profiles.profile_env_for_active_request_readonly("t") as bound: + assert bound is False and agent.override.get() is None + with profiles.profile_env_for_active_request_readonly("t", include_root=True) as bound: + assert bound is True and agent.override.get() == str(agent.base) + assert agent.override.get() is None + + +def test_runtime_scope_restores_context_after_exception(monkeypatch, tmp_path): + from api.mcp_runtime import mcp_runtime_scope + + agent = _install(monkeypatch, tmp_path) + with pytest.raises(KeyError): + _as_profile("profile-write", lambda: _raise_inside(mcp_runtime_scope, agent)) + assert agent.override.get() is None and agent.secret.get() is None + + +def _raise_inside(scope_factory, agent): + with scope_factory("t") as view: + assert view.trusted and agent.override.get() is not None and agent.secret.get() is not None + raise KeyError("boom")