diff --git a/hermes_cli/mcp_startup.py b/hermes_cli/mcp_startup.py index a8744161d563..82839d933b67 100644 --- a/hermes_cli/mcp_startup.py +++ b/hermes_cli/mcp_startup.py @@ -58,7 +58,28 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None: if not _has_configured_mcp_servers(): return + # Capture the caller's context-local HERMES_HOME override (profile + # scoping in multi-profile processes like the dashboard/desktop + # backend) and re-install it inside the discovery thread. ContextVars + # do not propagate into bare threads, so without this a session + # "switched" to profile X would discover the LAUNCH profile's + # mcp_servers instead (#67605). The config gate above already runs on + # the caller's thread, so it sees the same override. + try: + from hermes_constants import get_hermes_home_override + + home_override = get_hermes_home_override() + except Exception: + home_override = None + def _discover() -> None: + token = None + try: + from hermes_constants import set_hermes_home_override + + token = set_hermes_home_override(home_override) + except Exception: + token = None try: _discover_mcp_tools_without_interactive_oauth() try: @@ -73,6 +94,13 @@ def _discover() -> None: except Exception: logger.debug("Background MCP tool discovery failed", exc_info=True) finally: + if token is not None: + try: + from hermes_constants import reset_hermes_home_override + + reset_hermes_home_override(token) + except Exception: + pass with _mcp_discovery_lock: global _mcp_discovery_thread, _mcp_discovery_started _mcp_discovery_thread = None diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index dbdd2ec5ffc2..cd751aaa9f10 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -94,6 +94,7 @@ def test_session_context_uses_session_cwd(monkeypatch, tmp_path): session_key = "cwd-key" project = tmp_path / "project" project.mkdir() + (project / ".git").mkdir() launcher = tmp_path / "apps" / "desktop" launcher.mkdir(parents=True) @@ -566,6 +567,100 @@ def _write_profile_cfg(home: Path, cwd: str | None) -> Path: return home +def test_profile_scoped_mcp_discovery_uses_target_home(monkeypatch, tmp_path): + """MCP discovery must start under the selected profile's HERMES_HOME.""" + from hermes_cli import mcp_startup + from hermes_constants import get_hermes_home + from tui_gateway import entry + + profile_home = tmp_path / "profiles" / "sheepyr" + profile_home.mkdir(parents=True) + + (profile_home / "config.yaml").write_text( + "mcp_servers:\n" + " bluesky_sheepyr:\n" + " command: test-command\n", + encoding="utf-8", + ) + + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "default")) + token = set_hermes_home_override(str(profile_home)) + + seen = [] + + monkeypatch.setattr(mcp_startup, "_mcp_discovery_started", False) + monkeypatch.setattr(mcp_startup, "_mcp_discovery_thread", None) + # ensure_mcp_discovery_started flips this module global; monkeypatch it so + # the enablement doesn't leak into sibling tests in this file. + monkeypatch.setattr(entry, "_mcp_discovery_enabled", False) + monkeypatch.setattr( + mcp_startup, + "_discover_mcp_tools_without_interactive_oauth", + lambda: seen.append(str(get_hermes_home())), + ) + + try: + entry.ensure_mcp_discovery_started() + thread = mcp_startup._mcp_discovery_thread + assert thread is not None + thread.join(timeout=2) + finally: + reset_hermes_home_override(token) + mcp_startup._mcp_discovery_thread = None + mcp_startup._mcp_discovery_started = False + + assert seen == [str(profile_home)] + + +def test_profile_scoped_agent_build_starts_mcp_discovery_in_profile_home( + monkeypatch, tmp_path +): + """Agent construction must start MCP discovery under the selected profile.""" + import threading + + from hermes_constants import get_hermes_home + + profile_home = tmp_path / "profiles" / "sheepyr" + profile_home.mkdir(parents=True) + + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "default")) + + seen = [] + built = threading.Event() + + monkeypatch.setattr( + server, + "_make_agent", + lambda *args, **kwargs: built.set() + or type("Agent", (), {"model": "test"})(), + ) + monkeypatch.setattr( + "tui_gateway.entry.ensure_mcp_discovery_started", + lambda: seen.append(str(get_hermes_home())), + ) + monkeypatch.setattr(server, "_wire_callbacks", lambda _sid: None) + monkeypatch.setattr(server, "_SlashWorker", lambda *args: None) + monkeypatch.setattr(server, "_attach_worker", lambda *args: None) + monkeypatch.setattr(server, "_config_model_target", lambda: ("", "")) + + ready = threading.Event() + sid = "test-sid" + session = { + "agent_ready": ready, + "session_key": "test-key", + "profile_home": str(profile_home), + } + + server._sessions[sid] = session + try: + server._start_agent_build(sid, session) + assert built.wait(timeout=2) + finally: + server._sessions.pop(sid, None) + + assert seen == [str(profile_home)] + + def test_profile_configured_cwd_reads_target_profile(tmp_path): """A profile's own terminal.cwd is read from its config.yaml.""" project = tmp_path / "proj" @@ -10700,12 +10795,14 @@ def test_session_most_recent_handles_db_unavailable(monkeypatch): # ── verification.status ────────────────────────────────────────────── -def test_verification_status_returns_recorded_evidence(tmp_path): - home = tmp_path / ".hermes" - home.mkdir() - token = set_hermes_home_override(home) +def test_verification_status_returns_recorded_evidence(tmp_path, monkeypatch): + profile_home = tmp_path / "profiles" / "verify" + profile_home.mkdir(parents=True) + monkeypatch.setattr(server, "_profile_home", lambda p: profile_home if p == "verify" else None) + token = set_hermes_home_override(profile_home) project = tmp_path / "project" project.mkdir() + (project / ".git").mkdir() (project / "package.json").write_text( json.dumps({"scripts": {"test": "vitest"}}), encoding="utf-8", @@ -10726,7 +10823,7 @@ def test_verification_status_returns_recorded_evidence(tmp_path): { "id": "1", "method": "verification.status", - "params": {"cwd": str(project), "session_id": "sid"}, + "params": {"cwd": str(project), "session_id": "sid", "profile": "verify"}, } ) finally: diff --git a/tests/test_tui_gateway_ws.py b/tests/test_tui_gateway_ws.py index 3fba21777669..ad41ba9dade6 100644 --- a/tests/test_tui_gateway_ws.py +++ b/tests/test_tui_gateway_ws.py @@ -9,13 +9,14 @@ from tui_gateway import ws as ws_mod -def test_ws_startup_starts_background_mcp_discovery(monkeypatch): - """The desktop app and dashboard chat reach the agent through this WS - sidecar, not through tui_gateway.entry.main() (which spawns the discovery - thread for the stdio TUI). handle_ws must start discovery itself, otherwise - _make_agent's wait_for_mcp_discovery no-ops and the agent snapshots an - MCP-less tool list. Regression test for #38945.""" +def test_ws_does_not_own_mcp_discovery_startup(monkeypatch): + """WebSocket transport must not start MCP discovery itself. + + MCP discovery ownership belongs to the profile-scoped agent build path. + The WS layer only establishes the transport and emits gateway readiness. + """ calls = [] + monkeypatch.setattr( mcp_startup, "start_background_mcp_discovery", @@ -41,7 +42,7 @@ async def close(self): finally: server._sessions.clear() - assert calls == [{"logger": ws_mod._log, "thread_name": "tui-ws-mcp-discovery"}] + assert calls == [] def _run_disconnect(monkeypatch, seed): @@ -188,6 +189,37 @@ async def send_text(self, line): loop.close() +def test_ws_starts_mcp_discovery_before_ready(monkeypatch): + import tui_gateway.entry as entry + + calls = [] + events = [] + + monkeypatch.setattr(server, "_WS_ORPHAN_REAP_GRACE_S", 0) + monkeypatch.setattr(entry, "ensure_mcp_discovery_started", lambda: calls.append("mcp")) + + class FakeWS: + async def accept(self): + events.append("accept") + + async def send_text(self, line): + if '"gateway.ready"' in line: + events.append(f"ready_after_{len(calls)}") + + async def receive_text(self): + raise ws_mod._WebSocketDisconnect() + + async def close(self): + pass + + asyncio.run(ws_mod.handle_ws(FakeWS())) + + # Discovery moved to profile-aware agent construction. WebSocket transport + # should not start MCP discovery before a profile has been bound. + assert calls == [] + assert events == ["accept", "ready_after_0"] + + def test_ws_transport_serializes_concurrent_sends(): active_sends = 0 max_active_sends = 0 diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index e4c87be4a113..bf41c1201816 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -24,20 +24,23 @@ logger = logging.getLogger(__name__) -# Handle for the background MCP tool-discovery thread (see main()). The first -# agent build briefly joins this so already-spawning fast servers land before -# the agent snapshots its tool list (see wait_for_mcp_discovery). +# Handle for the background MCP tool-discovery thread (see +# ensure_mcp_discovery_started). The first agent build briefly joins this so +# already-spawning fast servers land before the agent snapshots its tool list +# (see wait_for_mcp_discovery). Stays None when discovery is delegated to the +# shared owner in hermes_cli.mcp_startup — the wait/in-flight/join helpers +# below consult both owners. _mcp_discovery_thread = None -# True once main() decided this TUI process has MCP servers configured and -# spawned discovery through the shared owner. Lets wait_for_mcp_discovery -# re-invoke the (idempotent) spawn on later agent builds so the -# retry-after-zero-connected allowance in -# hermes_cli.mcp_startup.start_background_mcp_discovery can actually fire for -# the stdio TUI — without this, main()'s single spawn is the only call and a -# first run that connected nothing latches the process MCP-less. Kept as a -# flag (rather than re-probing config) so non-MCP sessions never pay the -# tools.mcp_tool import on the per-agent-build wait path. +# True once ensure_mcp_discovery_started decided this process has MCP servers +# configured and spawned discovery through the shared owner. Lets +# wait_for_mcp_discovery re-invoke the (idempotent) spawn on later agent +# builds so the retry-after-zero-connected allowance in +# hermes_cli.mcp_startup.start_background_mcp_discovery can actually fire — +# without this, the single spawn is the only call and a first run that +# connected nothing latches the process MCP-less. Kept as a flag (rather than +# re-probing config) so non-MCP sessions never pay the tools.mcp_tool import +# on the per-agent-build wait path. _mcp_discovery_enabled = False @@ -243,17 +246,18 @@ def wait_for_mcp_discovery(timeout: "float | None" = None) -> None: bound = timeout if timeout is not None else 0.75 thread.join(timeout=bound) return - # The stdio TUI spawns discovery via the shared owner (see main()); wait - # on it so the first agent build still catches fast servers. Re-invoke - # the idempotent spawn first: if the previous run finished with zero - # connected servers, start_background_mcp_discovery's - # retry-after-zero-connected allowance kicks off a fresh discovery run - # here instead of leaving the TUI latched MCP-less for the session. - # Only the stdio TUI (which spawned discovery through the shared owner) - # should delegate to the startup wait here — for every other surface - # (dashboard /api/ws) _make_agent already calls - # hermes_cli.mcp_startup.wait_for_mcp_discovery directly, and delegating - # unconditionally would make that bounded wait run twice per agent build. + # Discovery is spawned via the shared owner (ensure_mcp_discovery_started + # → hermes_cli.mcp_startup); wait on it so the first agent build still + # catches fast servers. Re-invoke the idempotent spawn first: if the + # previous run finished with zero connected servers, + # start_background_mcp_discovery's retry-after-zero-connected allowance + # kicks off a fresh discovery run here instead of leaving the process + # latched MCP-less for the session. In multi-profile processes this + # retry runs under the CALLER's profile context (agent build binds the + # session profile's HERMES_HOME first), so a launch profile with no + # mcp_servers no longer starves selected profiles of discovery (#67605). + # Gated on _mcp_discovery_enabled so non-MCP sessions never pay the + # tools.mcp_tool import on the per-agent-build wait path. if not _mcp_discovery_enabled: return try: @@ -338,57 +342,71 @@ def join_mcp_discovery(timeout: float | None = None) -> bool: _recovery_times: list[float] = [] -def main(): - _install_sidecar_publisher() - # MCP tool discovery — runs in a background daemon thread so a slow or - # unreachable MCP server can't freeze TUI startup. Previously this ran - # inline before ``gateway.ready``, which meant any configured-but-down - # server stalled the whole shell on "summoning hermes…" for the full - # connect-retry backoff (e.g. a dead stdio/http server burns 1+2+4s of - # retries → ~7s of dead air before the composer appears). Discovery is - # idempotent and registers tools into the shared registry as servers - # connect. The agent isn't built until the first prompt, at which point - # ``_make_agent`` briefly joins this thread (``wait_for_mcp_discovery``, - # bounded) so already-spawning fast servers land in the tool snapshot — - # a dead server is simply not waited on past the bound. ``/reload-mcp`` - # rebuilds the snapshot for servers that connect later in the session. - # - # Cold-start guard: importing ``tools.mcp_tool`` transitively pulls the - # full MCP SDK (mcp, pydantic, httpx, jsonschema, starlette parsers — - # ~200ms on macOS). The overwhelming majority of users have no - # ``mcp_servers`` configured, in which case every byte of that import is - # wasted. Check the config first (cheap) and only spawn the discovery - # thread when there's actually MCP work to do, so the import cost stays - # off the path entirely for the common case. +def _has_configured_mcp_servers() -> bool: + """Return whether startup should attempt MCP discovery. + + Keep this cheap so non-MCP users do not pay the MCP SDK import cost. + """ try: from hermes_cli.config import read_raw_config - _mcp_servers = (read_raw_config() or {}).get("mcp_servers") - _has_mcp_servers = isinstance(_mcp_servers, dict) and len(_mcp_servers) > 0 + + mcp_servers = (read_raw_config() or {}).get("mcp_servers") + return isinstance(mcp_servers, dict) and len(mcp_servers) > 0 except Exception: # Be conservative: if we can't decide, fall back to attempting - # discovery (still backgrounded, so it can't block startup). - _has_mcp_servers = True - if _has_mcp_servers: - # Spawn via the shared owner in hermes_cli.mcp_startup instead of - # a hand-rolled thread, so the stdio TUI gets the same restart - # semantics as every other surface: a discovery run that completed - # with zero connected servers may be retried by a later spawn call - # instead of latching the process into a no-MCP-tools state. - # wait_for_mcp_discovery/mcp_discovery_in_flight/ - # join_mcp_discovery below already consult that owner. - global _mcp_discovery_enabled - _mcp_discovery_enabled = True - try: - from hermes_cli.mcp_startup import start_background_mcp_discovery + # discovery. The caller starts it in the background. + return True - start_background_mcp_discovery( - logger=logger, thread_name="tui-mcp-discovery" - ) - except Exception: - logger.warning( - "Background MCP tool discovery failed to start", exc_info=True - ) + +def ensure_mcp_discovery_started() -> None: + """Start background MCP discovery for the current profile context, once. + + ``main()`` calls this for the stdio/TUI path. WebSocket/Desktop + entrypoints can accept sessions without running ``main()``, so the + agent-build path (``server._start_agent_build``) also calls it AFTER + binding the session profile's HERMES_HOME override — the shared owner in + ``hermes_cli.mcp_startup`` captures the caller's context-local override + and propagates it into the discovery thread, so discovery reads the + SELECTED profile's ``mcp_servers``, not the launch profile's (#67605). + + Delegating to the shared owner (instead of a hand-rolled thread) keeps + the process-wide start lock, the retry-after-zero-connected allowance, + and interactive-OAuth suppression. + + Known limitation: MCP tool registration is process-global, so in a + multi-profile process the FIRST profile that builds an agent wins the + discovery slot. Full per-profile MCP registries are tracked in #67605. + """ + global _mcp_discovery_enabled + + if not _has_configured_mcp_servers(): + return + _mcp_discovery_enabled = True + try: + from hermes_cli.mcp_startup import start_background_mcp_discovery + + start_background_mcp_discovery( + logger=logger, thread_name="tui-mcp-discovery" + ) + except Exception: + logger.warning( + "Background MCP tool discovery failed to start", exc_info=True + ) + + +def main(): + _install_sidecar_publisher() + + # MCP tool discovery — backgrounded so a slow or unreachable MCP server + # can't freeze TUI startup (a dead stdio/http server burns 1+2+4s of + # connect retries → ~7s of dead air before the composer appears). The + # agent isn't built until the first prompt, at which point _make_agent + # briefly joins the discovery thread (wait_for_mcp_discovery, bounded) so + # already-spawning fast servers land in the tool snapshot. The config + # gate inside ensure_mcp_discovery_started keeps the ~200ms MCP SDK + # import cost entirely off the path for users with no mcp_servers. + ensure_mcp_discovery_started() if not write_json({ "jsonrpc": "2.0", diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ec43de410a5a..0d769f343430 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1849,6 +1849,14 @@ def _build() -> None: session_db = SessionDB(db_path=Path(profile_home) / "state.db") except Exception: session_db = None + + try: + from tui_gateway.entry import ensure_mcp_discovery_started + + ensure_mcp_discovery_started() + except Exception: + logger.warning("MCP discovery startup failed", exc_info=True) + try: # Lazy-resumed (watch) sessions carry the stored conversation # id — pass it through so the upgrade continues that session @@ -7049,6 +7057,7 @@ def _(rid, params: dict) -> dict: @method("verification.status") +@_profile_scoped def _(rid, params: dict) -> dict: """Best known coding verification evidence for a cwd/session. diff --git a/tui_gateway/ws.py b/tui_gateway/ws.py index 11469f93c20b..795f85c13e74 100644 --- a/tui_gateway/ws.py +++ b/tui_gateway/ws.py @@ -303,22 +303,6 @@ async def handle_ws(ws: Any) -> None: transport = WSTransport(ws, asyncio.get_running_loop(), peer=peer) - # The desktop app and dashboard chat reach the agent through this WS - # sidecar, NOT through tui_gateway.entry.main() (the stdio TUI path that - # spawns the background MCP discovery thread). Without starting it here, - # discovery never runs in this process: _make_agent only *waits* on the - # thread (wait_for_mcp_discovery), which no-ops when it was never - # created, so the agent snapshots an MCP-less tool list and the only way - # to surface MCP tools is a manual /reload-mcp. Start it once per - # process here (idempotent, config-gated) before gateway.ready so the - # first agent build can pick up already-spawning servers. (#38945) - from hermes_cli.mcp_startup import start_background_mcp_discovery - - start_background_mcp_discovery( - logger=_log, - thread_name="tui-ws-mcp-discovery", - ) - ready_ok = await transport.write_async( { "jsonrpc": "2.0",