mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-07 02:51:50 +00:00
fix(tui): suppress MCP discovery in slash_worker to prevent duplicate serve children (#15275)
The slash_worker creates a HermesCLI which imports model_tools, triggering discover_mcp_tools() at module scope. Meanwhile, the TUI server also calls MCP discovery independently. Both paths spawn ``hermes mcp serve`` child processes per session. Fix: defer the cli import in slash_worker to main() and set HERMES_MCP_DISCOVERY=0 beforehand. model_tools now checks this env var and skips MCP discovery when suppressed.
This commit is contained in:
parent
00c3d848d8
commit
c8e3c02c2b
3 changed files with 124 additions and 13 deletions
|
|
@ -137,12 +137,19 @@ def _run_async(coro):
|
|||
|
||||
discover_builtin_tools()
|
||||
|
||||
# MCP tool discovery (external MCP servers from config)
|
||||
try:
|
||||
# MCP tool discovery (external MCP servers from config).
|
||||
# Skipped when HERMES_MCP_DISCOVERY=0 -- the TUI slash_worker sets this because
|
||||
# the TUI server already handles MCP; running it again would spawn duplicate
|
||||
# ``hermes mcp serve`` children per session (#15275).
|
||||
import os as _os_mod
|
||||
if _os_mod.environ.get("HERMES_MCP_DISCOVERY") != "0":
|
||||
try:
|
||||
from tools.mcp_tool import discover_mcp_tools
|
||||
discover_mcp_tools()
|
||||
except Exception as e:
|
||||
except Exception as e:
|
||||
logger.debug("MCP tool discovery failed: %s", e)
|
||||
else:
|
||||
logger.debug("MCP tool discovery suppressed (HERMES_MCP_DISCOVERY=0)")
|
||||
|
||||
# Plugin tool discovery (user/project/pip plugins)
|
||||
try:
|
||||
|
|
|
|||
84
tests/tui_gateway/test_slash_worker_mcp.py
Normal file
84
tests/tui_gateway/test_slash_worker_mcp.py
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
"""Verify the TUI slash_worker suppresses MCP discovery at import time.
|
||||
|
||||
Regression test for #15275 -- without the HERMES_MCP_DISCOVERY=0 guard in
|
||||
slash_worker.py, importing the cli module triggers discover_mcp_tools() via
|
||||
model_tools, spawning duplicate ``hermes mcp serve`` children per session.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import os
|
||||
|
||||
|
||||
def test_slash_worker_sets_mcp_discovery_before_cli_import():
|
||||
"""slash_worker.main() must set HERMES_MCP_DISCOVERY=0 before importing cli."""
|
||||
import importlib
|
||||
src = importlib.util.find_spec("tui_gateway.slash_worker")
|
||||
assert src is not None and src.origin is not None
|
||||
with open(src.origin) as fh:
|
||||
lines = fh.readlines()
|
||||
|
||||
# Find the line that sets the env var and the first cli import.
|
||||
env_line_idx = None
|
||||
cli_import_idx = None
|
||||
for i, line in enumerate(lines):
|
||||
stripped = line.strip()
|
||||
if "HERMES_MCP_DISCOVERY" in stripped and "environ" in stripped:
|
||||
env_line_idx = i
|
||||
# Only match actual import statements, not comments or strings
|
||||
if cli_import_idx is None and stripped.startswith(("import cli", "from cli ")):
|
||||
cli_import_idx = i
|
||||
|
||||
assert env_line_idx is not None, (
|
||||
"slash_worker.py must set HERMES_MCP_DISCOVERY env var"
|
||||
)
|
||||
assert cli_import_idx is not None, (
|
||||
"slash_worker.py must import cli"
|
||||
)
|
||||
assert env_line_idx < cli_import_idx, (
|
||||
"HERMES_MCP_DISCOVERY must be set before cli is imported "
|
||||
f"(env var at line {env_line_idx + 1}, cli import at line {cli_import_idx + 1})"
|
||||
)
|
||||
|
||||
|
||||
def test_slash_worker_does_not_import_cli_at_module_level():
|
||||
"""cli must NOT be imported at module scope -- it must be lazy in main()."""
|
||||
import importlib
|
||||
src = importlib.util.find_spec("tui_gateway.slash_worker")
|
||||
assert src is not None and src.origin is not None
|
||||
with open(src.origin) as fh:
|
||||
source = fh.read()
|
||||
|
||||
tree = ast.parse(source)
|
||||
for node in ast.iter_child_nodes(tree):
|
||||
if isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
assert not alias.name.startswith("cli"), (
|
||||
f"cli must not be imported at module level (line {node.lineno})"
|
||||
)
|
||||
elif isinstance(node, ast.ImportFrom):
|
||||
assert node.module is None or not node.module.startswith("cli"), (
|
||||
f"cli must not be imported at module level (line {node.lineno})"
|
||||
)
|
||||
|
||||
|
||||
def test_model_tools_skips_mcp_when_env_var_set(monkeypatch):
|
||||
"""model_tools must skip discover_mcp_tools() when HERMES_MCP_DISCOVERY=0."""
|
||||
monkeypatch.setenv("HERMES_MCP_DISCOVERY", "0")
|
||||
|
||||
assert os.environ.get("HERMES_MCP_DISCOVERY") == "0"
|
||||
|
||||
# Simulate the guard check from model_tools.py
|
||||
should_discover = os.environ.get("HERMES_MCP_DISCOVERY") != "0"
|
||||
assert should_discover is False, (
|
||||
"MCP discovery should be suppressed when HERMES_MCP_DISCOVERY=0"
|
||||
)
|
||||
|
||||
|
||||
def test_model_tools_runs_mcp_when_env_var_absent(monkeypatch):
|
||||
"""model_tools must run discover_mcp_tools() when env var is not set."""
|
||||
monkeypatch.delenv("HERMES_MCP_DISCOVERY", raising=False)
|
||||
|
||||
should_discover = os.environ.get("HERMES_MCP_DISCOVERY") != "0"
|
||||
assert should_discover is True, (
|
||||
"MCP discovery should run when HERMES_MCP_DISCOVERY is not set"
|
||||
)
|
||||
|
|
@ -1,6 +1,12 @@
|
|||
"""Persistent slash-command worker — one HermesCLI per TUI session.
|
||||
"""Persistent slash-command worker -- one HermesCLI per TUI session.
|
||||
|
||||
Protocol: reads JSON lines from stdin {id, command}, writes {id, ok, output|error} to stdout.
|
||||
|
||||
The slash_worker only needs CLI command processing, not MCP tool bootstrapping.
|
||||
The TUI server already handles MCP discovery and spawns hermes mcp serve
|
||||
children; without the HERMES_MCP_DISCOVERY guard both code paths independently
|
||||
spawn duplicate MCP serve processes per session (#15275). The cli module is
|
||||
imported lazily inside main() so the env var is set before model_tools runs.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
|
|
@ -10,12 +16,14 @@ import json
|
|||
import os
|
||||
import sys
|
||||
|
||||
import cli as cli_mod
|
||||
from cli import HermesCLI
|
||||
from rich.console import Console
|
||||
|
||||
# cli module reference -- populated lazily in main() after setting the
|
||||
# HERMES_MCP_DISCOVERY env var to prevent duplicate MCP serve children.
|
||||
_cli_mod = None
|
||||
|
||||
def _run(cli: HermesCLI, command: str) -> str:
|
||||
|
||||
def _run(cli, command: str) -> str:
|
||||
cmd = (command or "").strip()
|
||||
if not cmd:
|
||||
return ""
|
||||
|
|
@ -29,21 +37,23 @@ def _run(cli: HermesCLI, command: str) -> str:
|
|||
# underlying file to our buffer so self.console.print() is captured.
|
||||
cli.console = Console(file=buf, force_terminal=True, width=120)
|
||||
|
||||
old = getattr(cli_mod, "_cprint", None)
|
||||
old = getattr(_cli_mod, "_cprint", None)
|
||||
if old is not None:
|
||||
cli_mod._cprint = lambda text: print(text)
|
||||
_cli_mod._cprint = lambda text: print(text)
|
||||
|
||||
try:
|
||||
with contextlib.redirect_stdout(buf), contextlib.redirect_stderr(buf):
|
||||
cli.process_command(cmd)
|
||||
finally:
|
||||
if old is not None:
|
||||
cli_mod._cprint = old
|
||||
_cli_mod._cprint = old
|
||||
|
||||
return buf.getvalue().rstrip()
|
||||
|
||||
|
||||
def main():
|
||||
global _cli_mod
|
||||
|
||||
p = argparse.ArgumentParser(add_help=False)
|
||||
p.add_argument("--session-key", required=True)
|
||||
p.add_argument("--model", default="")
|
||||
|
|
@ -52,6 +62,16 @@ def main():
|
|||
os.environ["HERMES_SESSION_KEY"] = args.session_key
|
||||
os.environ["HERMES_INTERACTIVE"] = "1"
|
||||
|
||||
# Suppress MCP discovery -- the TUI server already handles MCP servers;
|
||||
# importing cli triggers model_tools which calls discover_mcp_tools() at
|
||||
# module scope. This env var tells model_tools to skip that step.
|
||||
os.environ.setdefault("HERMES_MCP_DISCOVERY", "0")
|
||||
|
||||
import cli as cli_mod
|
||||
from cli import HermesCLI
|
||||
|
||||
_cli_mod = cli_mod
|
||||
|
||||
with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()):
|
||||
cli = HermesCLI(model=args.model or None, compact=True, resume=args.session_key, verbose=False)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue