From 5edc0c492b5dc830c06f897152d32b6c3c10e11b Mon Sep 17 00:00:00 2001 From: Mike Smith Date: Fri, 7 Aug 2026 12:56:12 -0400 Subject: [PATCH] fix(cli): skip wrapper-side MCP discovery when chat launches the TUI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each TUI instance spawned three stdio MCP server copies: one in the CLI wrapper, one in tui_gateway.entry, one in the slash worker. The wrapper's copy is dead weight — _launch_tui blocks in subprocess.call until the TUI exits, so its registered MCP tools are never invoked, yet the server process (35-85 MB) lives for the whole session. Root cause: _is_tui_chat_launch() only detected --tui / HERMES_TUI=1, so bare `hermes` with display.interface: tui fell through to background MCP discovery in the wrapper while the TUI gateway (spawned moments later) ran a second discovery. Fix: _is_tui_chat_launch() now consults _resolve_use_tui() — the exact TUI-vs-classic decision cmd_chat makes — for chat commands only (command in {None, "chat"}), leaving mcp serve / gateway / acp / cron discovery behavior untouched. Verified: unit tests (RED->GREEN); E2E with a canary stdio MCP server in a scratch HERMES_HOME counted 2 spawned copies pre-fix vs 1 post-fix (gateway's only), and the wrapper's RSS dropped ~43 MB. Related: #71928 (same per-process duplication class), #11115 (lazy non-core discovery). --- hermes_cli/main.py | 14 ++++- tests/hermes_cli/test_mcp_startup.py | 90 ++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index d3a73ae146..8281cbdd63 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -12791,7 +12791,19 @@ _AGENT_SUBCOMMANDS = { def _is_tui_chat_launch(args) -> bool: - return bool(getattr(args, "tui", False) or os.environ.get("HERMES_TUI") == "1") + if getattr(args, "tui", False) or os.environ.get("HERMES_TUI") == "1": + return True + # The chat path decides TUI-vs-classic via _resolve_use_tui (--cli/--tui + # flags, TTY gate, HERMES_TUI env, display.interface config). Bare + # `hermes`/`hermes chat` with a TUI display config was previously missed + # here, so the wrapper pre-warmed its own MCP discovery while the TUI + # gateway (spawned moments later) ran a second one — an idle stdio MCP + # server copy held dead for the whole session. Only chat commands can + # launch the TUI; other commands (mcp serve, gateway, acp, cron) keep + # their own discovery behavior untouched. + if getattr(args, "command", None) not in {None, "chat"}: + return False + return _resolve_use_tui(args) def _command_has_dedicated_mcp_startup(args) -> bool: diff --git a/tests/hermes_cli/test_mcp_startup.py b/tests/hermes_cli/test_mcp_startup.py index 76bd1661e3..f9be0410ac 100644 --- a/tests/hermes_cli/test_mcp_startup.py +++ b/tests/hermes_cli/test_mcp_startup.py @@ -102,6 +102,96 @@ def test_prepare_agent_startup_backgrounds_blocking_mcp_for_chat(monkeypatch): stop.set() +def test_prepare_agent_startup_skips_discovery_when_chat_resolves_to_tui( + monkeypatch, +): + """Bare ``hermes`` / ``hermes chat`` on a TTY with ``display.interface: + tui`` resolves to the TUI via ``_resolve_use_tui``, but does NOT pass + ``--tui`` or ``HERMES_TUI``. Discovery must be skipped in the wrapper: + the TUI gateway owns it, and the wrapper would otherwise hold a dead + MCP server for the entire session (3 copies per TUI instance). + """ + calls = {"background": 0, "inline": 0} + + monkeypatch.setattr(main_mod, "_resolve_use_tui", lambda _args: True) + monkeypatch.setattr( + mcp_startup, + "start_background_mcp_discovery", + lambda **_kwargs: calls.__setitem__("background", calls["background"] + 1), + ) + monkeypatch.setitem( + sys.modules, + "hermes_cli.plugins", + types.SimpleNamespace(discover_plugins=lambda: None), + ) + monkeypatch.setitem( + sys.modules, + "hermes_cli.config", + types.SimpleNamespace( + read_raw_config=lambda: {"mcp_servers": {"demo": {"transport": "stdio"}}}, + load_config=lambda: {}, + ), + ) + monkeypatch.setitem( + sys.modules, + "agent.shell_hooks", + types.SimpleNamespace(register_from_config=lambda *_a, **_k: None), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_tool", + types.SimpleNamespace( + discover_mcp_tools=lambda: calls.__setitem__("inline", calls["inline"] + 1), + ), + ) + + main_mod._prepare_agent_startup(_agent_args(command=None)) + + assert calls["background"] == 0 + assert calls["inline"] == 0 + assert mcp_startup._mcp_discovery_thread is None + + +def test_prepare_agent_startup_keeps_discovery_for_non_chat_commands( + monkeypatch, +): + """Non-chat commands never launch the TUI, so they must keep their own + MCP discovery even when the ambient display config resolves to TUI — + ``_is_tui_chat_launch`` must not consult ``_resolve_use_tui`` there.""" + calls = {"inline": 0} + + monkeypatch.setattr(main_mod, "_resolve_use_tui", lambda _args: True) + monkeypatch.setitem( + sys.modules, + "hermes_cli.plugins", + types.SimpleNamespace(discover_plugins=lambda: None), + ) + monkeypatch.setitem( + sys.modules, + "hermes_cli.config", + types.SimpleNamespace( + read_raw_config=lambda: {"mcp_servers": {"demo": {"transport": "stdio"}}}, + load_config=lambda: {}, + ), + ) + monkeypatch.setitem( + sys.modules, + "agent.shell_hooks", + types.SimpleNamespace(register_from_config=lambda *_a, **_k: None), + ) + monkeypatch.setitem( + sys.modules, + "tools.mcp_tool", + types.SimpleNamespace( + discover_mcp_tools=lambda: calls.__setitem__("inline", calls["inline"] + 1), + ), + ) + + main_mod._prepare_agent_startup(_agent_args(command="mcp", mcp_action="serve")) + + assert calls["inline"] == 1 + + def test_background_mcp_discovery_suppresses_interactive_oauth(monkeypatch): state = {"active": False, "during_discover": None}