diff --git a/apps/desktop/electron/windows-child-process.test.cjs b/apps/desktop/electron/windows-child-process.test.cjs index 0194464d641d..c081cc7de1e7 100644 --- a/apps/desktop/electron/windows-child-process.test.cjs +++ b/apps/desktop/electron/windows-child-process.test.cjs @@ -23,6 +23,19 @@ function requireHiddenChildOptions(source, needle) { ) } +function requireAllHiddenChildOptions(source, needle) { + const matches = [...source.matchAll(needle)] + assert.ok(matches.length > 0, `missing call site: ${needle}`) + for (const match of matches) { + const snippet = source.slice(match.index, match.index + 700) + assert.match( + snippet, + /hiddenWindowsChildOptions\(/, + `expected ${needle} call site to wrap child-process options with hiddenWindowsChildOptions` + ) + } +} + test('desktop background child processes opt into hidden Windows consoles', () => { const source = readElectronFile('main.cjs') @@ -37,6 +50,7 @@ test('desktop background child processes opt into hidden Windows consoles', () = requireHiddenChildOptions(source, /spawn\(\s*backend\.command,\s*backend\.args/) requireHiddenChildOptions(source, /hermesProcess = spawn\(\s*backend\.command,\s*backend\.args/) requireHiddenChildOptions(source, /spawn\(\s*py,\s*\['-m', 'hermes_cli\.main', 'uninstall', '--gui-summary'\]/) + requireAllHiddenChildOptions(source, /spawn\(\s*updater,\s*updaterArgs,/g) assert.match(source, /function unwrapWindowsVenvHermesCommand\(command, dashboardArgs\)/) assert.match(source, /existing Hermes no-console Python at/) @@ -56,7 +70,6 @@ test('desktop background child processes opt into hidden Windows consoles', () = test('intentional or interactive desktop child processes stay documented', () => { const source = readElectronFile('main.cjs') - assert.match(source, /windowsHide: false/) assert.match(source, /handOffWindowsBootstrapRecovery/) assert.match(source, /'--repair', '--branch'/) assert.match(source, /'--update', '--branch'/) @@ -68,5 +81,5 @@ test('bootstrap PowerShell runner hides Windows console children', () => { const source = readElectronFile('bootstrap-runner.cjs') assert.match(source, /function hiddenWindowsChildOptions\(options = \{\}\)/) - requireHiddenChildOptions(source, 'spawn(ps, fullArgs') + requireHiddenChildOptions(source, /spawn\(\s*ps,\s*fullArgs/) }) diff --git a/hermes_bootstrap.py b/hermes_bootstrap.py index ae23cc976296..cb5ead451c87 100644 --- a/hermes_bootstrap.py +++ b/hermes_bootstrap.py @@ -50,10 +50,69 @@ from __future__ import annotations import os +import subprocess import sys _IS_WINDOWS = sys.platform == "win32" _bootstrap_applied = False +_subprocess_defaults_applied = False +_original_popen = subprocess.Popen + + +def _hidden_windows_startupinfo(): + if not _IS_WINDOWS: + return None + startupinfo = subprocess.STARTUPINFO() + startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW + startupinfo.wShowWindow = subprocess.SW_HIDE + return startupinfo + + +def _apply_windows_subprocess_defaults(kwargs: dict) -> dict: + """Default Windows child processes to hidden unless caller is explicit.""" + if not _IS_WINDOWS: + return kwargs + if os.environ.get("HERMES_ALLOW_VISIBLE_SUBPROCESSES") == "1": + return kwargs + creationflags = kwargs.get("creationflags", 0) or 0 + create_new_console = getattr(subprocess, "CREATE_NEW_CONSOLE", 0x00000010) + if not creationflags & create_new_console: + kwargs["creationflags"] = creationflags | getattr( + subprocess, "CREATE_NO_WINDOW", 0 + ) + if kwargs.get("startupinfo") is None: + startupinfo = _hidden_windows_startupinfo() + if startupinfo is not None: + kwargs["startupinfo"] = startupinfo + return kwargs + + +def apply_windows_subprocess_defaults() -> bool: + """Hide Python-spawned child process windows by default on Windows. + + Hermes' desktop/gateway backends launch many short-lived helpers while the + UI is connecting, loading sessions, resolving providers, or discovering + tools. Any raw ``subprocess.run`` / ``Popen`` call that forgets + ``CREATE_NO_WINDOW`` can briefly flash a Windows Terminal/cmd window. + + This process-wide wrapper makes hidden launches the default for Hermes + Python entry points. Callers that need special process behavior remain in + control by passing explicit ``creationflags`` and/or ``startupinfo``. + """ + global _subprocess_defaults_applied + + if not _IS_WINDOWS: + return False + if _subprocess_defaults_applied: + return False + + class HermesHiddenPopen(_original_popen): + def __init__(self, *args, **kwargs): + super().__init__(*args, **_apply_windows_subprocess_defaults(kwargs)) + + subprocess.Popen = HermesHiddenPopen + _subprocess_defaults_applied = True + return True def apply_windows_utf8_bootstrap() -> bool: @@ -79,6 +138,7 @@ def apply_windows_utf8_bootstrap() -> bool: # (or PYTHONIOENCODING=something-else) if they really want to. os.environ.setdefault("PYTHONUTF8", "1") os.environ.setdefault("PYTHONIOENCODING", "utf-8") + apply_windows_subprocess_defaults() # 2. Reconfigure the current process's stdio to UTF-8. Needed # because os.environ changes don't retroactively rebind sys.stdout diff --git a/hermes_cli/_subprocess_compat.py b/hermes_cli/_subprocess_compat.py index 607a9a3e6a4d..5f0e05c21df8 100644 --- a/hermes_cli/_subprocess_compat.py +++ b/hermes_cli/_subprocess_compat.py @@ -28,12 +28,15 @@ from __future__ import annotations import shutil +import subprocess import sys -from typing import Sequence +from typing import Any, Sequence __all__ = [ "IS_WINDOWS", "resolve_node_command", + "run", + "popen", "windows_detach_flags", "windows_detach_flags_without_breakaway", "windows_hide_flags", @@ -201,6 +204,41 @@ def windows_hide_flags() -> int: return _CREATE_NO_WINDOW +def _hidden_startupinfo() -> Any | None: + """Return STARTUPINFO that hides a child console on Windows.""" + if not IS_WINDOWS: + return None + + startupinfo = subprocess.STARTUPINFO() + startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW + startupinfo.wShowWindow = 0 + return startupinfo + + +def _with_hidden_windows_defaults(kwargs: dict[str, Any]) -> dict[str, Any]: + """Apply no-window defaults without changing POSIX behavior.""" + if not IS_WINDOWS: + return kwargs + + out = dict(kwargs) + creationflags = int(out.get("creationflags", 0) or 0) + if not creationflags & subprocess.CREATE_NEW_CONSOLE: + out["creationflags"] = creationflags | windows_hide_flags() + if out.get("startupinfo") is None: + out["startupinfo"] = _hidden_startupinfo() + return out + + +def run(*popenargs: Any, **kwargs: Any) -> subprocess.CompletedProcess: + """Run a subprocess with Hermes' Windows hidden-console defaults.""" + return subprocess.run(*popenargs, **_with_hidden_windows_defaults(kwargs)) + + +def popen(*popenargs: Any, **kwargs: Any) -> subprocess.Popen: + """Open a subprocess with Hermes' Windows hidden-console defaults.""" + return subprocess.Popen(*popenargs, **_with_hidden_windows_defaults(kwargs)) + + def windows_detach_popen_kwargs() -> dict: """Return a dict of Popen kwargs that detach a child on Windows and fall back to the POSIX equivalent (``start_new_session=True``) on diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index 217eb2bb9656..3376c76e97c0 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -11,9 +11,11 @@ import threading import time from pathlib import Path +from typing import TYPE_CHECKING, Dict, List, Optional from urllib.parse import urlparse + +from hermes_cli import _subprocess_compat from hermes_constants import get_hermes_home -from typing import TYPE_CHECKING, Dict, List, Optional # rich and prompt_toolkit are imported lazily (inside the functions that use # them) rather than at module level. Importing this module is on the TUI @@ -218,7 +220,7 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]: if is_shallow: fetch_args += ["--depth", "1"] fetch_args.append("--quiet") - subprocess.run( + _subprocess_compat.run( fetch_args, capture_output=True, timeout=10, cwd=str(repo_dir), diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index e6f63a1557c9..7a3e152cd9f7 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -27,6 +27,8 @@ from pathlib import Path from typing import Optional +from hermes_cli import _subprocess_compat + logger = logging.getLogger(__name__) # OAuth device code flow constants (same client ID as opencode/Copilot CLI) @@ -135,7 +137,7 @@ def _try_gh_cli_token() -> Optional[str]: if hostname: cmd += ["--hostname", hostname] try: - result = subprocess.run( + result = _subprocess_compat.run( cmd, capture_output=True, text=True, diff --git a/hermes_cli/main.py b/hermes_cli/main.py index ab56d9986d60..d82c5af32c7e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -258,6 +258,11 @@ def _try_termux_ultrafast_version() -> bool: import shutil import stat import subprocess +from hermes_cli import _subprocess_compat +from hermes_cli._subprocess_compat import ( + windows_detach_popen_kwargs, + windows_hide_flags, +) from pathlib import Path from typing import Optional @@ -4451,7 +4456,7 @@ def _clear_bytecode_cache(root: Path) -> int: def _capture_head_sha(git_cmd, cwd) -> str | None: """Return the current HEAD SHA, or None if it can't be resolved.""" try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["rev-parse", "HEAD"], cwd=cwd, capture_output=True, @@ -6281,7 +6286,7 @@ def _update_via_zip(args): def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[str]: - status = subprocess.run( + status = _subprocess_compat.run( git_cmd + ["status", "--porcelain"], cwd=cwd, capture_output=True, @@ -6295,7 +6300,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st # git stash will fail with "needs merge / could not write index". Clear the # conflict state with `git reset` so the stash can proceed. Working-tree # changes are preserved; only the index conflict markers are dropped. - unmerged = subprocess.run( + unmerged = _subprocess_compat.run( git_cmd + ["ls-files", "--unmerged"], cwd=cwd, capture_output=True, @@ -6303,7 +6308,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st ) if unmerged.stdout.strip(): print("→ Clearing unmerged index entries from a previous conflict...") - subprocess.run(git_cmd + ["reset"], cwd=cwd, capture_output=True) + _subprocess_compat.run(git_cmd + ["reset"], cwd=cwd, capture_output=True) from datetime import datetime, timezone @@ -6311,12 +6316,12 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st "hermes-update-autostash-%Y%m%d-%H%M%S" ) print("→ Local changes detected — stashing before update...") - subprocess.run( + _subprocess_compat.run( git_cmd + ["stash", "push", "--include-untracked", "-m", stash_name], cwd=cwd, check=True, ) - stash_ref = subprocess.run( + stash_ref = _subprocess_compat.run( git_cmd + ["rev-parse", "--verify", "refs/stash"], cwd=cwd, capture_output=True, @@ -6329,7 +6334,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st def _resolve_stash_selector( git_cmd: list[str], cwd: Path, stash_ref: str ) -> Optional[str]: - stash_list = subprocess.run( + stash_list = _subprocess_compat.run( git_cmd + ["stash", "list", "--format=%gd %H"], cwd=cwd, capture_output=True, @@ -6384,7 +6389,7 @@ def _restore_stashed_changes( return False print("→ Restoring local changes...") - restore = subprocess.run( + restore = _subprocess_compat.run( git_cmd + ["stash", "apply", stash_ref], cwd=cwd, capture_output=True, @@ -6392,7 +6397,7 @@ def _restore_stashed_changes( ) # Check for unmerged (conflicted) files — can happen even when returncode is 0 - unmerged = subprocess.run( + unmerged = _subprocess_compat.run( git_cmd + ["diff", "--name-only", "--diff-filter=U"], cwd=cwd, capture_output=True, @@ -6420,7 +6425,7 @@ def _restore_stashed_changes( # Always reset to clean state — leaving conflict markers in source # files makes hermes completely unrunnable (SyntaxError on import). # The user's changes are safe in the stash for manual recovery. - subprocess.run( + _subprocess_compat.run( git_cmd + ["reset", "--hard", "HEAD"], cwd=cwd, capture_output=True, @@ -6442,7 +6447,7 @@ def _restore_stashed_changes( ) _print_stash_cleanup_guidance(stash_ref) else: - drop = subprocess.run( + drop = _subprocess_compat.run( git_cmd + ["stash", "drop", stash_selector], cwd=cwd, capture_output=True, @@ -6494,7 +6499,7 @@ def _discard_stashed_changes( _print_stash_cleanup_guidance(stash_ref) return False - drop = subprocess.run( + drop = _subprocess_compat.run( git_cmd + ["stash", "drop", stash_selector], cwd=cwd, capture_output=True, @@ -6531,7 +6536,7 @@ def _discard_stashed_changes( def _get_origin_url(git_cmd: list[str], cwd: Path) -> Optional[str]: """Get the URL of the origin remote, or None if not set.""" try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["remote", "get-url", "origin"], cwd=cwd, capture_output=True, @@ -6564,7 +6569,7 @@ def _is_fork(origin_url: Optional[str]) -> bool: def _has_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: """Check if an 'upstream' remote already exists.""" try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["remote", "get-url", "upstream"], cwd=cwd, capture_output=True, @@ -6578,7 +6583,7 @@ def _has_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: def _add_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: """Add the official repo as the 'upstream' remote. Returns True on success.""" try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["remote", "add", "upstream", OFFICIAL_REPO_URL], cwd=cwd, capture_output=True, @@ -6592,7 +6597,7 @@ def _add_upstream_remote(git_cmd: list[str], cwd: Path) -> bool: def _count_commits_between(git_cmd: list[str], cwd: Path, base: str, head: str) -> int: """Count commits on `head` that are not on `base`. Returns -1 on error.""" try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["rev-list", "--count", f"{base}..{head}"], cwd=cwd, capture_output=True, @@ -6628,7 +6633,7 @@ def _sync_fork_with_upstream(git_cmd: list[str], cwd: Path) -> bool: Returns True if push succeeded, False otherwise. """ try: - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["push", "origin", "main", "--force-with-lease"], cwd=cwd, capture_output=True, @@ -6691,7 +6696,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: print() print("→ Fetching upstream...") try: - subprocess.run( + _subprocess_compat.run( git_cmd + ["fetch", "upstream", "main", "--quiet"], cwd=cwd, capture_output=True, @@ -6731,7 +6736,7 @@ def _sync_with_upstream_if_needed(git_cmd: list[str], cwd: Path) -> None: print("→ Pulling from upstream...") try: - subprocess.run( + _subprocess_compat.run( git_cmd + ["pull", "--ff-only", "upstream", "main"], cwd=cwd, check=True, @@ -7363,6 +7368,11 @@ def _run_quarantined_install( if scripts_dir is not None: _restore_quarantined_exes(moved) raise + if scripts_dir is not None: + # A successful installer exit does not always guarantee the entry-point + # shim was recreated. Keep the command usable if uv/pip skipped writing + # the replacement after quarantine. + _restore_quarantined_exes(moved) def _cleanup_quarantined_exes(scripts_dir: Path | None = None) -> None: @@ -7381,6 +7391,11 @@ def _cleanup_quarantined_exes(scripts_dir: Path | None = None) -> None: try: for stale in scripts_dir.glob("*.exe.old.*"): try: + original_name = stale.name.split(".old.", 1)[0] + original = stale.with_name(original_name) + if not original.exists(): + stale.rename(original) + continue stale.unlink() except OSError: pass # still locked or in use — try again next run @@ -8164,7 +8179,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): # would then report a huge bogus "behind" number. Detect shallow up front: # fetch with --depth 1 to preserve the boundary and report presence-only. is_shallow = ( - subprocess.run( + _subprocess_compat.run( git_cmd + ["rev-parse", "--is-shallow-repository"], cwd=PROJECT_ROOT, capture_output=True, @@ -8176,7 +8191,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if branch == "main": print("→ Fetching from upstream...") - fetch_result = subprocess.run( + fetch_result = _subprocess_compat.run( git_cmd + ["fetch"] + depth_args + ["upstream", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -8185,7 +8200,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if fetch_result.returncode != 0: # Fallback to origin if upstream doesn't exist print("→ Fetching from origin...") - fetch_result = subprocess.run( + fetch_result = _subprocess_compat.run( git_cmd + ["fetch"] + depth_args + ["origin", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -8199,7 +8214,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): else: # Non-default branch: compare against origin/ directly. print("→ Fetching from origin...") - fetch_result = subprocess.run( + fetch_result = _subprocess_compat.run( git_cmd + ["fetch"] + depth_args + ["origin", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -8224,7 +8239,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): # Without this, `git rev-list HEAD..origin/ --count` exits 128 and # (with check=True) raises CalledProcessError, surfacing a Python # traceback. Friendlier to detect-and-report. - verify_result = subprocess.run( + verify_result = _subprocess_compat.run( git_cmd + ["rev-parse", "--verify", "--quiet", compare_branch], cwd=PROJECT_ROOT, capture_output=True, @@ -8237,11 +8252,11 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if is_shallow: # No history to count across the shallow boundary. Compare tip SHAs and # report presence-only (mirrors the banner's _check_via_local_git). - head_sha = subprocess.run( + head_sha = _subprocess_compat.run( git_cmd + ["rev-parse", "HEAD"], cwd=PROJECT_ROOT, capture_output=True, text=True, ).stdout.strip() - target_sha = subprocess.run( + target_sha = _subprocess_compat.run( git_cmd + ["rev-parse", compare_branch], cwd=PROJECT_ROOT, capture_output=True, text=True, ).stdout.strip() @@ -8254,7 +8269,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): print(f" Run '{recommended_update_command()}' to install.") return - rev_result = subprocess.run( + rev_result = _subprocess_compat.run( git_cmd + ["rev-list", f"HEAD..{compare_branch}", "--count"], cwd=PROJECT_ROOT, capture_output=True, @@ -8770,7 +8785,7 @@ def _discard_lockfile_churn(git_cmd, repo_root): Best-effort; only ever touches files named ``package-lock.json``. """ try: - diff = subprocess.run( + diff = _subprocess_compat.run( git_cmd + ["diff", "--name-only"], cwd=repo_root, capture_output=True, @@ -8791,7 +8806,7 @@ def _discard_lockfile_churn(git_cmd, repo_root): ] if not dirty: return - subprocess.run( + _subprocess_compat.run( git_cmd + ["checkout", "--", *dirty], cwd=repo_root, capture_output=True, @@ -9057,7 +9072,7 @@ def _cmd_update_impl(args, gateway_mode: bool): branch = _resolve_update_branch(args) print("→ Fetching updates...") - fetch_result = subprocess.run( + fetch_result = _subprocess_compat.run( git_cmd + ["fetch", "origin", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -9081,7 +9096,7 @@ def _cmd_update_impl(args, gateway_mode: bool): sys.exit(1) # Get current branch (returns literal "HEAD" when detached) - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["rev-parse", "--abbrev-ref", "HEAD"], cwd=PROJECT_ROOT, capture_output=True, @@ -9104,7 +9119,7 @@ def _cmd_update_impl(args, gateway_mode: bool): print(f" ⚠ Currently on {label} — switching to {branch} for update...") # Stash before checkout so uncommitted work isn't lost auto_stash_ref = _stash_local_changes_if_needed(git_cmd, PROJECT_ROOT) - checkout_result = subprocess.run( + checkout_result = _subprocess_compat.run( git_cmd + ["checkout", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -9115,7 +9130,7 @@ def _cmd_update_impl(args, gateway_mode: bool): # it up as a tracking branch of origin/. This is # the common case when the requested branch exists upstream # but was never checked out locally. - track_result = subprocess.run( + track_result = _subprocess_compat.run( git_cmd + ["checkout", "-B", branch, f"origin/{branch}"], cwd=PROJECT_ROOT, capture_output=True, @@ -9146,7 +9161,7 @@ def _cmd_update_impl(args, gateway_mode: bool): ) # Check if there are updates - result = subprocess.run( + result = _subprocess_compat.run( git_cmd + ["rev-list", f"HEAD..origin/{branch}", "--count"], cwd=PROJECT_ROOT, capture_output=True, @@ -9172,7 +9187,7 @@ def _cmd_update_impl(args, gateway_mode: bool): input_fn=gw_input_fn, ) if current_branch not in {branch, "HEAD"}: - subprocess.run( + _subprocess_compat.run( git_cmd + ["checkout", current_branch], cwd=PROJECT_ROOT, capture_output=True, @@ -9211,7 +9226,7 @@ def _cmd_update_impl(args, gateway_mode: bool): # the bad commit and the fix landing). pre_pull_sha = _capture_head_sha(git_cmd, PROJECT_ROOT) try: - pull_result = subprocess.run( + pull_result = _subprocess_compat.run( git_cmd + ["pull", "--ff-only", "origin", branch], cwd=PROJECT_ROOT, capture_output=True, @@ -9224,7 +9239,7 @@ def _cmd_update_impl(args, gateway_mode: bool): print( " ⚠ Fast-forward not possible (history diverged), resetting to match remote..." ) - reset_result = subprocess.run( + reset_result = _subprocess_compat.run( git_cmd + ["reset", "--hard", f"origin/{branch}"], cwd=PROJECT_ROOT, capture_output=True, @@ -9260,7 +9275,7 @@ def _cmd_update_impl(args, gateway_mode: bool): if pre_pull_sha: print() print(f"→ Rolling back to {pre_pull_sha[:10]}...") - rollback_result = subprocess.run( + rollback_result = _subprocess_compat.run( git_cmd + ["reset", "--hard", pre_pull_sha], cwd=PROJECT_ROOT, capture_output=True, diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index aab35394964a..2d534ff3c26a 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -41,6 +41,7 @@ save_env_value, ) from hermes_cli.cli_output import prompt as _prompt_input +from hermes_cli import _subprocess_compat _MANIFEST_VERSION = 1 @@ -397,7 +398,7 @@ def _do_git_install(entry: CatalogEntry) -> Path: is_sha_ref = bool(re.fullmatch(r"[0-9a-f]{7,40}", install.ref)) if not is_sha_ref: - proc = subprocess.run( + proc = _subprocess_compat.run( [git, "clone", "--depth", "1", "--branch", install.ref, install.url, str(dest)], ) if proc.returncode == 0: @@ -410,10 +411,10 @@ def _do_git_install(entry: CatalogEntry) -> Path: is_sha_ref = True # treat the same as a SHA ref from here if is_sha_ref: - proc = subprocess.run([git, "clone", install.url, str(dest)]) + proc = _subprocess_compat.run([git, "clone", install.url, str(dest)]) if proc.returncode != 0: raise CatalogError(f"git clone failed for {install.url}") - proc = subprocess.run([git, "-C", str(dest), "checkout", install.ref]) + proc = _subprocess_compat.run([git, "-C", str(dest), "checkout", install.ref]) if proc.returncode != 0: raise CatalogError(f"git checkout {install.ref} failed") diff --git a/tests/hermes_cli/test_update_autostash.py b/tests/hermes_cli/test_update_autostash.py index be1a5f1acf4c..2fdb3015e648 100644 --- a/tests/hermes_cli/test_update_autostash.py +++ b/tests/hermes_cli/test_update_autostash.py @@ -394,6 +394,19 @@ def _setup_update_mocks(monkeypatch, tmp_path): monkeypatch.setattr(hermes_config, "check_config_version", lambda: (5, 5)) monkeypatch.setattr(hermes_config, "migrate_config", lambda **kw: {"env_added": [], "config_added": []}) monkeypatch.setattr(hermes_main, "_refresh_active_lazy_features", lambda: None) + monkeypatch.setattr(hermes_main, "_pause_windows_gateways_for_update", lambda: None) + monkeypatch.setattr(hermes_main, "_resume_windows_gateways_after_update", lambda *_args, **_kwargs: None) + + +def _logical_git_cmd(cmd): + """Strip Windows-only git config prefix from update test comparisons.""" + if ( + len(cmd) >= 4 + and cmd[0] == "git" + and cmd[1:3] == ["-c", "windows.appendAtomically=false"] + ): + return ["git", *cmd[3:]] + return cmd def test_cmd_update_retries_optional_extras_individually_when_all_fails(monkeypatch, tmp_path, capsys): @@ -407,13 +420,14 @@ def test_cmd_update_retries_optional_extras_individually_when_all_fails(monkeypa def fake_run(cmd, **kwargs): recorded.append(cmd) - if cmd == ["git", "fetch", "origin", "main"]: + logical = _logical_git_cmd(cmd) + if logical == ["git", "fetch", "origin", "main"]: return SimpleNamespace(stdout="", stderr="", returncode=0) - if cmd == ["git", "rev-parse", "--abbrev-ref", "HEAD"]: + if logical == ["git", "rev-parse", "--abbrev-ref", "HEAD"]: return SimpleNamespace(stdout="main\n", stderr="", returncode=0) - if cmd == ["git", "rev-list", "HEAD..origin/main", "--count"]: + if logical == ["git", "rev-list", "HEAD..origin/main", "--count"]: return SimpleNamespace(stdout="1\n", stderr="", returncode=0) - if cmd == ["git", "pull", "--ff-only", "origin", "main"]: + if logical == ["git", "pull", "--ff-only", "origin", "main"]: return SimpleNamespace(stdout="Updating\n", stderr="", returncode=0) if cmd == ["/usr/bin/uv", "pip", "install", "-e", ".[all]"]: raise CalledProcessError(returncode=1, cmd=cmd) @@ -456,13 +470,14 @@ def test_cmd_update_succeeds_with_extras(monkeypatch, tmp_path): def fake_run(cmd, **kwargs): recorded.append(cmd) - if cmd == ["git", "fetch", "origin", "main"]: + logical = _logical_git_cmd(cmd) + if logical == ["git", "fetch", "origin", "main"]: return SimpleNamespace(stdout="", stderr="", returncode=0) - if cmd == ["git", "rev-parse", "--abbrev-ref", "HEAD"]: + if logical == ["git", "rev-parse", "--abbrev-ref", "HEAD"]: return SimpleNamespace(stdout="main\n", stderr="", returncode=0) - if cmd == ["git", "rev-list", "HEAD..origin/main", "--count"]: + if logical == ["git", "rev-list", "HEAD..origin/main", "--count"]: return SimpleNamespace(stdout="1\n", stderr="", returncode=0) - if cmd == ["git", "pull", "--ff-only", "origin", "main"]: + if logical == ["git", "pull", "--ff-only", "origin", "main"]: return SimpleNamespace(stdout="Updating\n", stderr="", returncode=0) return SimpleNamespace(returncode=0, stdout="", stderr="") @@ -578,7 +593,7 @@ def test_cmd_update_falls_back_to_reset_when_ff_only_fails(monkeypatch, tmp_path hermes_main.cmd_update(SimpleNamespace()) - reset_calls = [c for c in recorded if "reset" in c and "--hard" in c] + reset_calls = [_logical_git_cmd(c) for c in recorded if "reset" in c and "--hard" in c] assert len(reset_calls) == 1 assert reset_calls[0] == ["git", "reset", "--hard", "origin/main"] @@ -699,9 +714,9 @@ def test_cmd_update_fetch_is_scoped_to_target_branch(monkeypatch, tmp_path): hermes_main.cmd_update(SimpleNamespace()) - fetch_calls = [c for c in recorded if "fetch" in c] + fetch_calls = [_logical_git_cmd(c) for c in recorded if "fetch" in c] assert fetch_calls == [["git", "fetch", "origin", "main"]] - assert ["git", "fetch", "origin"] not in recorded + assert ["git", "fetch", "origin"] not in [_logical_git_cmd(c) for c in recorded] # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_update_concurrent_quarantine.py b/tests/hermes_cli/test_update_concurrent_quarantine.py index 5345319bb498..335c7e34c01c 100644 --- a/tests/hermes_cli/test_update_concurrent_quarantine.py +++ b/tests/hermes_cli/test_update_concurrent_quarantine.py @@ -447,6 +447,65 @@ def always_fails(self, target): assert "Hermes Desktop" in captured or "gateway" in captured.lower() +def test_quarantined_install_restores_missing_shim_after_success(tmp_path, monkeypatch): + """A successful installer exit must still leave the command shim usable.""" + shim = tmp_path / "hermes.exe" + quarantined = tmp_path / "hermes.exe.old.123" + quarantined.write_bytes(b"old shim") + + monkeypatch.setattr( + cli_main, + "_quarantine_running_hermes_exe", + lambda scripts_dir: [(shim, quarantined)], + ) + monkeypatch.setattr( + cli_main, + "_run_install_with_heartbeat", + lambda cmd, env=None: None, + ) + + cli_main._run_quarantined_install(["uv", "pip", "install"], scripts_dir=tmp_path) + + assert shim.read_bytes() == b"old shim" + assert not quarantined.exists() + + +def test_quarantined_install_preserves_fresh_replacement(tmp_path, monkeypatch): + """Do not overwrite a replacement shim that the installer actually wrote.""" + shim = tmp_path / "hermes.exe" + quarantined = tmp_path / "hermes.exe.old.123" + quarantined.write_bytes(b"old shim") + + monkeypatch.setattr( + cli_main, + "_quarantine_running_hermes_exe", + lambda scripts_dir: [(shim, quarantined)], + ) + + def fake_install(cmd, env=None): + shim.write_bytes(b"fresh shim") + + monkeypatch.setattr(cli_main, "_run_install_with_heartbeat", fake_install) + + cli_main._run_quarantined_install(["uv", "pip", "install"], scripts_dir=tmp_path) + + assert shim.read_bytes() == b"fresh shim" + assert quarantined.exists() + + +@patch.object(cli_main, "_is_windows", return_value=True) +def test_cleanup_quarantined_exes_restores_stranded_missing_original(_winp, tmp_path): + """Cleanup should recover, not delete, the only remaining shim backup.""" + shim = tmp_path / "hermes.exe" + quarantined = tmp_path / "hermes.exe.old.123" + quarantined.write_bytes(b"old shim") + + cli_main._cleanup_quarantined_exes(tmp_path) + + assert shim.read_bytes() == b"old shim" + assert not quarantined.exists() + + # --------------------------------------------------------------------------- # Windows gateway pause/resume before update mutation # --------------------------------------------------------------------------- diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 6b73fac7ad4b..00fd4735141b 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -91,6 +91,7 @@ import os import re import shutil +import subprocess import sys import threading import time @@ -197,9 +198,75 @@ def _write_stderr_log_header(server_name: str) -> None: # Streamable HTTP was introduced by 2025-03-26, so this remains valid for the # HTTP transport path even on older-but-supported SDK versions. LATEST_PROTOCOL_VERSION = "2025-03-26" + + +def _hidden_windows_startupinfo(): + if sys.platform != "win32": + return None + startupinfo = subprocess.STARTUPINFO() + startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW + startupinfo.wShowWindow = subprocess.SW_HIDE + return startupinfo + + +def _patch_mcp_stdio_windows_process_factory() -> None: + """Keep MCP stdio server launches from opening console windows on Windows.""" + if sys.platform != "win32": + return + try: + import anyio + import mcp.client.stdio as mcp_stdio + from mcp.os.win32 import utilities as win32_utilities + from hermes_cli._subprocess_compat import windows_hide_flags + except Exception as exc: + logger.debug("Could not patch MCP Windows stdio launcher: %s", exc) + return + + if getattr(mcp_stdio, "_hermes_hidden_windows_process_factory", False): + return + + async def _create_hidden_windows_process( + command: str, + args: List[str], + env: Optional[Dict[str, str]] = None, + errlog: Any = sys.stderr, + cwd: Any = None, + ): + process = None + job = win32_utilities._create_job_object() + process_kwargs = { + "env": env, + "stderr": errlog, + "cwd": cwd, + "creationflags": windows_hide_flags(), + "startupinfo": _hidden_windows_startupinfo(), + } + try: + process = await anyio.open_process([command, *args], **process_kwargs) + except Exception: + popen_obj = subprocess.Popen( + [command, *args], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=errlog, + env=env, + cwd=cwd, + bufsize=0, + creationflags=windows_hide_flags(), + startupinfo=_hidden_windows_startupinfo(), + ) + process = win32_utilities.FallbackProcess(popen_obj) + win32_utilities._maybe_assign_process_to_job(process, job) + return process + + mcp_stdio.create_windows_process = _create_hidden_windows_process + mcp_stdio._hermes_hidden_windows_process_factory = True + + try: from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client + _patch_mcp_stdio_windows_process_factory() _MCP_AVAILABLE = True try: from mcp.client.streamable_http import streamablehttp_client diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ec61aed6d57e..9ec18f82fc47 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -17,6 +17,7 @@ from pathlib import Path from typing import Any, Optional +from hermes_cli import _subprocess_compat from hermes_constants import ( get_hermes_home, get_hermes_home_override, @@ -9076,7 +9077,7 @@ def _(rid, params: dict) -> dict: str(pdf_path), str(out_prefix), ] try: - res = subprocess.run(argv, capture_output=True, text=True, timeout=120, stdin=subprocess.DEVNULL) + res = _subprocess_compat.run(argv, capture_output=True, text=True, timeout=120, stdin=subprocess.DEVNULL) except subprocess.TimeoutExpired: return _err(rid, 5028, "pdftoppm timed out (>120s)") if res.returncode != 0: @@ -11097,7 +11098,7 @@ def _(rid, params: dict) -> dict: if hint: return _ok(rid, {"blocked": True, "hint": hint, "code": -1, "output": ""}) try: - r = subprocess.run( + r = _subprocess_compat.run( [sys.executable, "-m", "hermes_cli.main", *argv], capture_output=True, text=True, @@ -11159,7 +11160,7 @@ def _(rid, params: dict) -> dict: if name in qcmds: qc = qcmds[name] if qc.get("type") == "exec": - r = subprocess.run( + r = _subprocess_compat.run( qc.get("command", ""), shell=True, capture_output=True, @@ -11619,7 +11620,7 @@ def _list_repo_files(root: str) -> list[str]: files: list[str] = [] try: - top_result = subprocess.run( + top_result = _subprocess_compat.run( ["git", "-C", root, "rev-parse", "--show-toplevel"], capture_output=True, timeout=2.0, @@ -11628,7 +11629,7 @@ def _list_repo_files(root: str) -> list[str]: ) if top_result.returncode == 0: top = top_result.stdout.decode("utf-8", "replace").strip() - list_result = subprocess.run( + list_result = _subprocess_compat.run( [ "git", "-C", @@ -13499,7 +13500,7 @@ def _(rid, params: dict) -> dict: except ImportError: return _err(rid, 5001, "shell.exec unavailable: approval safety module not importable") try: - r = subprocess.run( + r = _subprocess_compat.run( cmd, shell=True, capture_output=True, text=True, timeout=30, cwd=os.getcwd(), stdin=subprocess.DEVNULL, )