fix(tui): prevent slash_worker from spawning duplicate MCP servers#15796
fix(tui): prevent slash_worker from spawning duplicate MCP servers#15796thapecroth wants to merge 3 commits into
Conversation
…ousResearch#15275) Slash worker imports model_tools (via cli), which eagerly calls discover_mcp_tools() at module level. This spawned a second set of 'hermes mcp serve' children per TUI session — doubling subprocess count and wasting resources. Fix: gate MCP discovery behind HERMES_SKIP_MCP_DISCOVERY env var. - model_tools.py: check env var before discover_mcp_tools() - slash_worker.py: set env var before importing cli - tests: 5 dedicated tests verify guard logic and ordering Tactical fix. See PR #XXXX for the architectural (connection pool) alternative that prevents duplicates by design.
The HERMES_SKIP_MCP_DISCOVERY env var check at module level used os.environ but model_tools.py did not import os, causing NameError in any test that imported model_tools.
- Replace no-op test_discover_mcp_tools_skipped_when_env_set with actual assertion that the env var is set - Fix test_env_var_truthy_values to actually set the env var before checking - Add test_slash_worker_env_var_value to verify the assigned value is truthy - Replace misleading integration test with real subprocess-based tests that verify discover_mcp_tools is NOT called when env var is set, and IS called when env var is not set - Add test_env_var_cleared_after_test to prevent env leakage
|
Closing — the bug this fixes no longer exists on The eager module-level Verified empirically: constructing the slash_worker's Your analysis of the original duplicate-spawn mechanism was correct against the code at the time. Thanks for the careful root-cause work and the test hardening. No salvage needed since the guarded call site is gone. |
Summary
Fixes #15275
Problem: A single TUI session spawns two sets of
hermes mcp servechild processes — one fromtui_gateway.entryand one fromtui_gateway.slash_worker. Both paths independently importmodel_tools.py, which eagerly callsdiscover_mcp_tools()at module level, spawning a full set of MCP server subprocesses each time.On resource-constrained systems (e.g. Raspberry Pi), this doubles the subprocess count per session and can cause resource exhaustion or cron job failures.
Root cause: Eager
discover_mcp_tools()at import time +slash_workercreating its ownHermesCLIinstance = both paths doing independent tool bootstrap.Approach: Env var guard (tactical fix)
Gate MCP discovery behind
HERMES_SKIP_MCP_DISCOVERYso lightweight consumers that never need MCP-backed tools can opt out:model_tools.py: Check env var before callingdiscover_mcp_tools()slash_worker.py: Set env var to"1"before importingcli(which transitively importsmodel_tools)test_slash_worker_mcp.pyverify guard logic, source ordering, and behavioral skipChanges
model_tools.pydiscover_mcp_tools()in env var checktui_gateway/slash_worker.pyHERMES_SKIP_MCP_DISCOVERY=1before cli importtests/tui_gateway/test_slash_worker_mcp.pyAlternative: Shared connection pool
See #XXXX for an architectural alternative that replaces eager import-time discovery with a lazy singleton
MCPConnectionPool— inspired by Claude Code'sSocketPoolpattern. That approach prevents duplicates by design (connection reuse) rather than by opt-out, and is more robust against future consumers forgetting to set the env var.Both approaches fix the same bug; the env var guard is the minimal tactical fix, while the connection pool is the strategic long-term solution.
Test results