diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index e8c5d5fe60..eea7c94cac 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -1302,6 +1302,12 @@ def delete_profile(name: str, yes: bool = False) -> Path: # into the directory before we remove it. _notify_multiplexer(canon) + # The main serve process survives this deletion. Stop only this profile's MCP + # transports and release cached stderr handles, including completed probes. + from hermes_constants import hermes_home_key + from tools.mcp_tool_lifecycle import shutdown_mcp_servers + shutdown_mcp_servers(scope=hermes_home_key(profile_dir)) + # Release this process's holographic memory-store connections into the profile. The # Desktop's main serve process opens memory_store.db for every profile and is # deliberately not stopped above; on Windows its handles fail rmtree with WinError 32. diff --git a/tests/hermes_cli/test_profile_delete_mcp_log.py b/tests/hermes_cli/test_profile_delete_mcp_log.py new file mode 100644 index 0000000000..dc6bc45b81 --- /dev/null +++ b/tests/hermes_cli/test_profile_delete_mcp_log.py @@ -0,0 +1,55 @@ +"""Profile deletion after a real stdio MCP probe must release its log file.""" + +import sys +from pathlib import Path + +import pytest + +from hermes_cli import profiles +from hermes_cli.mcp_config import _probe_single_server +from hermes_constants import reset_hermes_home_override, set_hermes_home_override +from tools import mcp_tool_config + + +@pytest.mark.windows_only +def test_delete_profile_after_stdio_probe(tmp_path, monkeypatch): + monkeypatch.setattr(Path, "home", lambda: tmp_path) + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + # Do not manage host services or scan unrelated developer processes in this test. + monkeypatch.setattr(profiles, "_cleanup_gateway_service", lambda *_: None) + monkeypatch.setattr(profiles, "_stop_profile_backends", lambda *_: None) + monkeypatch.setattr(profiles, "_notify_multiplexer", lambda *_: None) + profile = profiles.create_profile("mcp-probe-delete", no_alias=True) + server = tmp_path / "stdio_server.py" + server.write_text('''import json +import sys +for line in sys.stdin: + request = json.loads(line) + if "id" not in request: + continue + if request["method"] == "initialize": + result = {"protocolVersion": request["params"]["protocolVersion"], + "capabilities": {"tools": {}}, + "serverInfo": {"name": "probe-fixture", "version": "1"}} + elif request["method"] == "tools/list": + result = {"tools": [{"name": "echo", "description": "Echo", + "inputSchema": {"type": "object"}}]} + else: + result = {} + print(json.dumps({"jsonrpc": "2.0", "id": request["id"], "result": result}), flush=True) +''', encoding="utf-8") + token = set_hermes_home_override(profile) + try: + assert _probe_single_server("fixture", {"command": sys.executable, "args": [str(server)]}) == [("echo", "Echo")] + finally: + reset_hermes_home_override(token) + try: + profiles.delete_profile("mcp-probe-delete", yes=True) + assert not profile.exists() + finally: + # Keep the pre-fix failure from leaking a Windows handle into tempdir cleanup. + for handle in list(mcp_tool_config._mcp_stderr_log_fh.values()): + if getattr(handle, "name", None) == str(profile / "logs" / "mcp-stderr.log"): + handle.close() \ No newline at end of file diff --git a/tests/tools/test_mcp_stderr_lifecycle.py b/tests/tools/test_mcp_stderr_lifecycle.py new file mode 100644 index 0000000000..0f42f7bfe7 --- /dev/null +++ b/tests/tools/test_mcp_stderr_lifecycle.py @@ -0,0 +1,38 @@ +"""MCP shutdown releases only the selected profile's cached stderr handle.""" + +from hermes_constants import ( + hermes_home_key, + reset_hermes_home_override, + set_hermes_home_override, +) +from tools.mcp_tool_config import _get_mcp_stderr_log +from tools.mcp_tool_lifecycle import shutdown_mcp_servers + + +def test_scoped_shutdown_releases_log_and_preserves_other_profile(tmp_path): + handles = [] + for name in ("first", "second"): + token = set_hermes_home_override(tmp_path / name) + try: + handles.append(_get_mcp_stderr_log()) + finally: + reset_hermes_home_override(token) + first, second = handles + try: + shutdown_mcp_servers(scope=hermes_home_key(tmp_path / "first")) + assert first.closed + second.write("other profile remains usable\n") + second.flush() + token = set_hermes_home_override(tmp_path / "first") + try: + reopened = _get_mcp_stderr_log() + handles.append(reopened) + reopened.write("reload can reopen the log\n") + reopened.flush() + finally: + reset_hermes_home_override(token) + shutdown_mcp_servers() + assert second.closed and reopened.closed + finally: + for handle in handles: + handle.close() \ No newline at end of file diff --git a/tools/mcp_tool_config.py b/tools/mcp_tool_config.py index c7064ab32a..19dcc21051 100644 --- a/tools/mcp_tool_config.py +++ b/tools/mcp_tool_config.py @@ -20,17 +20,17 @@ _mcp_stderr_log_lock = threading.Lock() def _get_mcp_stderr_log() -> Any: - """Shared append-mode handle for MCP subprocess stderr, opened once per process PER PROFILE HOME (a + """Shared append-mode handle for MCP subprocess stderr, cached until shutdown PER PROFILE HOME (a multiplexed gateway's secondary profile must log under ITS ``logs/``, not the launch profile's). Must expose a real fd (asyncio wires the child's stderr to it); falls back to ``/dev/null``, then real stderr.""" - from hermes_constants import get_hermes_home, hermes_home_key + from hermes_constants import get_hermes_home, hermes_home_key, mkdir_under_hermes_home home_key = hermes_home_key() with _mcp_stderr_log_lock: fh = _mcp_stderr_log_fh.get(home_key) - if fh is None: + if fh is None or fh.closed: try: log_dir = get_hermes_home() / "logs" - log_dir.mkdir(parents=True, exist_ok=True) + mkdir_under_hermes_home(log_dir) # Line-buffered so output lands promptly; errors="replace" tolerates garbled binary. fh = open(log_dir / "mcp-stderr.log", "a", encoding="utf-8", errors="replace", buffering=1) fh.fileno() # confirm a real fd before committing @@ -44,6 +44,21 @@ def _get_mcp_stderr_log() -> Any: return fh +def _close_mcp_stderr_logs(*, scope: Optional[str] = None) -> None: + """Release cached parent handles after the selected MCP transports have stopped.""" + with _mcp_stderr_log_lock: + keys = list(_mcp_stderr_log_fh) if scope is None else [scope] + for key in keys: + fh = _mcp_stderr_log_fh.pop(key, None) + # The last-resort fallback is borrowed, not owned by MCP. + if fh is None or fh is sys.stderr or fh is sys.__stderr__: + continue + try: + fh.close() + except OSError: + logger.warning("Could not close MCP stderr log for %s", key, exc_info=True) + + def _write_stderr_log_header(server_name: str) -> None: """Session marker so operators can find each server's output in the shared log (per-line prefixes would need a pipe + reader thread).""" diff --git a/tools/mcp_tool_lifecycle.py b/tools/mcp_tool_lifecycle.py index cbff4e77b5..7fd0aa2976 100644 --- a/tools/mcp_tool_lifecycle.py +++ b/tools/mcp_tool_lifecycle.py @@ -201,6 +201,11 @@ def shutdown_mcp_servers(*, scope: Optional[str] = None, names: Optional[set] = clear_selected_status() _clear_connect_cooldowns(None if scope is None and names is None else selected_status) _loop._stop_mcp_loop(only_if_idle=scope is not None or names is not None) + # A removed subset still shares its profile's log with the remaining servers. + # Full/profile shutdown must also release handles left by completed CLI/UI probes. + if names is None: + from tools.mcp_tool_config import _close_mcp_stderr_logs + _close_mcp_stderr_logs(scope=scope) def _take_reapable_pids(include_active: bool, server_name: Optional[str]) -> tuple[Dict[int, str], Dict[int, int]]: