diff --git a/agent/lsp/install.py b/agent/lsp/install.py index 418cc510c70..f4f36dd7dcc 100644 --- a/agent/lsp/install.py +++ b/agent/lsp/install.py @@ -34,6 +34,7 @@ import sys import threading from pathlib import Path from typing import Any, Dict, Optional +from hermes_cli import _subprocess_compat logger = logging.getLogger("agent.lsp.install") @@ -343,7 +344,7 @@ def _install_pip(pkg: str, bin_name: str) -> Optional[str]: pip_target.mkdir(parents=True, exist_ok=True) try: logger.info("[install] pip install --target %s %s", pip_target, pkg) - proc = subprocess.run( + proc = _subprocess_compat.run( [sys.executable, "-m", "pip", "install", "--target", str(pip_target), "--quiet", pkg], check=False, capture_output=True, diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index 3706a79ac0f..671959871a1 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -219,7 +219,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/gateway.py b/hermes_cli/gateway.py index ed0caacc777..35b4a0f383c 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -839,8 +839,26 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str]) -> bool: respawn_env_literal=respawn_env_literal, ) + # The watcher process itself must ALSO run under the windowless + # interpreter. ``sys.executable`` during ``hermes update`` is the venv's + # console ``python.exe``; uv's venv launcher re-execs the base console + # interpreter, which allocates a conhost that DETACHED_PROCESS / + # CREATE_NO_WINDOW cannot suppress. That persistent console then becomes + # the parent of the respawned gateway — so even though the gateway argv is + # rewritten to pythonw, closing the leftover console window kills the + # gateway. Resolve a windowless pythonw for the watcher too (no-op on + # POSIX, where sys.executable has no console problem). + watcher_python = sys.executable + if sys.platform == "win32": + try: + from hermes_cli.gateway_windows import _resolve_detached_python + + watcher_python = _resolve_detached_python(sys.executable)[0] + except Exception: + watcher_python = sys.executable + watcher_argv = [ - sys.executable, + watcher_python, "-c", watcher, str(old_pid), diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 919d658e516..f0b5dff4a51 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -5339,7 +5339,7 @@ def _ensure_git_worktree(repo_root: Path, target: Path, branch_name: str) -> Non "git", "-C", str(repo_root), "worktree", "add", "-b", branch_name, str(target), "HEAD", ] - 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 efa116df71f..1bf83647853 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -262,6 +262,7 @@ from hermes_cli._subprocess_compat import ( windows_detach_popen_kwargs, windows_hide_flags, ) +from hermes_cli import _subprocess_compat from pathlib import Path from typing import Optional @@ -1160,7 +1161,7 @@ def _probe_container(cmd: list, backend: str, via_sudo: bool = False): all other exceptions propagate naturally. """ try: - return subprocess.run(cmd, capture_output=True, text=True, timeout=15) + return _subprocess_compat.run(cmd, capture_output=True, text=True, timeout=15) except subprocess.TimeoutExpired: label = f"sudo {backend}" if via_sudo else backend print( @@ -4456,7 +4457,7 @@ _UPDATE_CRITICAL_FILES = ( 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, @@ -4635,7 +4636,7 @@ def _run_with_idle_timeout( lock = threading.Lock() try: - proc = subprocess.Popen( + proc = _subprocess_compat.popen( cmd, cwd=cwd, stdout=subprocess.PIPE, @@ -6224,7 +6225,7 @@ def _update_via_zip(args): # Some environments lose pip inside the venv; bootstrap it back with # ensurepip before trying the editable install. try: - subprocess.run( + _subprocess_compat.run( pip_cmd + ["--version"], cwd=PROJECT_ROOT, check=True, @@ -6291,7 +6292,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, @@ -6305,7 +6306,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, @@ -6313,7 +6314,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 @@ -6327,7 +6328,7 @@ def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[st check=True, creationflags=windows_hide_flags(), ) - stash_ref = subprocess.run( + stash_ref = _subprocess_compat.run( git_cmd + ["rev-parse", "--verify", "refs/stash"], cwd=cwd, capture_output=True, @@ -6340,7 +6341,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, @@ -6395,7 +6396,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, @@ -6403,7 +6404,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, @@ -6431,7 +6432,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, @@ -6453,7 +6454,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, @@ -6505,7 +6506,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, @@ -6542,7 +6543,7 @@ SKIP_UPSTREAM_PROMPT_FILE = ".skip_upstream_prompt" 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, @@ -6575,7 +6576,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, @@ -6589,7 +6590,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, @@ -6603,7 +6604,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, @@ -6639,7 +6640,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, @@ -6702,7 +6703,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, @@ -6935,7 +6936,7 @@ def _recover_from_interrupted_install() -> None: # no pip module at all, and uv may also be gone. ensurepip restores a # known-good pip so at least the plain-pip path below can proceed. try: - subprocess.run( + _subprocess_compat.run( [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], cwd=PROJECT_ROOT, capture_output=True, @@ -8071,7 +8072,7 @@ def _run_logged_subprocess(cmd, *, cwd=None, env=None): Returns the ``CompletedProcess`` (with ``stdout`` populated) so the caller can decide whether to surface the captured output on failure. """ - result = subprocess.run( + result = _subprocess_compat.run( cmd, cwd=cwd, env=env, @@ -8178,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, @@ -8238,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, @@ -8251,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() @@ -8268,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, @@ -8784,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, @@ -8805,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, @@ -9071,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, @@ -9095,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, @@ -9118,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, @@ -9129,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, @@ -9160,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, @@ -9186,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, @@ -9225,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, @@ -9238,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, @@ -9274,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, @@ -9380,7 +9381,7 @@ def _cmd_update_impl(args, gateway_mode: bool): # ensurepip before trying the editable install. pip_cmd = [sys.executable, "-m", "pip"] try: - subprocess.run( + _subprocess_compat.run( pip_cmd + ["--version"], cwd=PROJECT_ROOT, check=True, diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 65d3d73dbe1..26c082c4024 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -32,6 +32,7 @@ from pathlib import Path, PurePosixPath, PureWindowsPath from typing import List, Optional, Tuple from agent.skill_utils import is_excluded_skill_path +from hermes_cli import _subprocess_compat _PROFILE_ID_RE = re.compile(r"^[a-z0-9][a-z0-9_-]{0,63}$") @@ -1058,7 +1059,7 @@ def seed_profile_skills(profile_dir: Path, quiet: bool = False) -> Optional[dict } project_root = Path(__file__).parent.parent.resolve() try: - result = subprocess.run( + result = _subprocess_compat.run( [sys.executable, "-c", "import json; from tools.skills_sync import sync_skills; " "r = sync_skills(quiet=True); print(json.dumps(r))"], diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index f27f35fac19..ac9ec4d4826 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -161,6 +161,7 @@ from hermes_cli.cli_output import ( # noqa: E402 ) from hermes_cli.secret_prompt import masked_secret_prompt # noqa: E402 from hermes_cli._subprocess_compat import windows_hide_flags +from hermes_cli import _subprocess_compat def is_interactive_stdin() -> bool: @@ -1316,7 +1317,7 @@ def setup_terminal_backend(config: dict): text=True, ) else: - result = subprocess.run( + result = _subprocess_compat.run( [sys.executable, "-m", "pip", "install", "modal"], capture_output=True, text=True, @@ -1369,7 +1370,7 @@ def setup_terminal_backend(config: dict): text=True, ) else: - result = subprocess.run( + result = _subprocess_compat.run( [sys.executable, "-m", "pip", "install", "daytona"], capture_output=True, text=True, diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 7db29368c45..f90bb368d0f 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -229,6 +229,7 @@ def _checklist_toolset_keys(platform: str) -> Set[str]: # compatibility with existing ``PLATFORMS[key]["label"]`` access patterns. from hermes_cli.platforms import PLATFORMS as _PLATFORMS_REGISTRY from hermes_cli._subprocess_compat import windows_hide_flags +from hermes_cli import _subprocess_compat PLATFORMS = { k: {"label": info.label, "default_toolset": info.default_toolset} @@ -653,7 +654,7 @@ def _pip_install( pip_cmd = [sys.executable, "-m", "pip"] try: # Probe for pip; bootstrap via ensurepip if missing (uv venv lacks it). - probe = subprocess.run( + probe = _subprocess_compat.run( pip_cmd + ["--version"], capture_output=True, text=True, timeout=15, ) @@ -661,7 +662,7 @@ def _pip_install( raise FileNotFoundError("pip not in venv") except (subprocess.TimeoutExpired, FileNotFoundError): try: - subprocess.run( + _subprocess_compat.run( [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], capture_output=True, text=True, timeout=120, check=True, ) @@ -672,7 +673,7 @@ def _pip_install( stderr=f"pip not available and ensurepip failed: {e}", ) - return subprocess.run( + return _subprocess_compat.run( pip_cmd + ["install", *args], capture_output=capture_output, text=True, timeout=timeout, ) @@ -889,7 +890,7 @@ def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) - if verbose: result = subprocess.run(install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env(), creationflags=windows_hide_flags()) else: - result = subprocess.run( + result = _subprocess_compat.run( install_cmd, shell=use_shell, timeout=300, env=_cua_driver_env(), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, encoding="utf-8", errors="replace", @@ -1031,7 +1032,7 @@ def _run_post_setup(post_setup_key: str): else [npx_bin, "-y", "agent-browser", "install", "--with-deps"] ) try: - result = subprocess.run( + result = _subprocess_compat.run( install_cmd, capture_output=True, text=True, cwd=str(PROJECT_ROOT), timeout=600, ) diff --git a/plugins/google_meet/process_manager.py b/plugins/google_meet/process_manager.py index 0709c6a1f94..400eab669e5 100644 --- a/plugins/google_meet/process_manager.py +++ b/plugins/google_meet/process_manager.py @@ -21,6 +21,7 @@ from pathlib import Path from typing import Any, Dict, Optional from hermes_constants import get_hermes_home +from hermes_cli import _subprocess_compat # File + directory layout (under $HERMES_HOME): # @@ -160,7 +161,7 @@ def start( # signals don't propagate. log_fh = open(log_path, "ab", buffering=0) try: - proc = subprocess.Popen( + proc = _subprocess_compat.popen( [sys.executable, "-m", "plugins.google_meet.meet_bot"], stdin=subprocess.DEVNULL, stdout=log_fh, diff --git a/plugins/memory/honcho/cli.py b/plugins/memory/honcho/cli.py index 8fc37448fd4..9ba90908a38 100644 --- a/plugins/memory/honcho/cli.py +++ b/plugins/memory/honcho/cli.py @@ -13,6 +13,7 @@ from pathlib import Path from hermes_constants import get_hermes_home from plugins.memory.honcho.client import _host_block, profile_host_key, resolve_active_host, resolve_config_path, HOST from hermes_cli.config import cfg_get +from hermes_cli import _subprocess_compat def clone_honcho_for_profile(profile_name: str) -> bool: @@ -517,7 +518,7 @@ def _ensure_sdk_installed() -> bool: import subprocess print(" Installing honcho-ai...", flush=True) - result = subprocess.run( + result = _subprocess_compat.run( [sys.executable, "-m", "pip", "install", "honcho-ai>=2.0.1"], capture_output=True, text=True, diff --git a/plugins/platforms/google_chat/oauth.py b/plugins/platforms/google_chat/oauth.py index 3d481b3ead7..480f80eac46 100644 --- a/plugins/platforms/google_chat/oauth.py +++ b/plugins/platforms/google_chat/oauth.py @@ -92,6 +92,7 @@ except (ModuleNotFoundError, ImportError): return str(home) from utils import atomic_replace +from hermes_cli._subprocess_compat import windows_hide_flags def _hermes_home() -> Path: @@ -382,6 +383,7 @@ def install_deps() -> bool: subprocess.check_call( [sys.executable, "-m", "pip", "install", "--quiet"] + _REQUIRED_PACKAGES, stdout=subprocess.DEVNULL, + creationflags=windows_hide_flags(), ) print("Dependencies installed.") return True diff --git a/scripts/check-windows-footguns.py b/scripts/check-windows-footguns.py index 35a9b4103d5..d93a03fd66a 100644 --- a/scripts/check-windows-footguns.py +++ b/scripts/check-windows-footguns.py @@ -463,7 +463,9 @@ def _call_attr_name(node: ast.Call) -> str | None: return f.attr -def _suppresses_window(node: ast.Call, func_name: str) -> bool: +def _suppresses_window( + node: ast.Call, func_name: str, var_progs: dict[str, str] | None = None +) -> bool: """True if this subprocess call cannot create a new console window. The honest invariant (corrected after review of PR #53791): capturing or @@ -492,11 +494,11 @@ def _suppresses_window(node: ast.Call, func_name: str) -> bool: return True if any(kw.arg is None for kw in node.keywords): # **kwargs spread return True - if _is_posix_only_program(node): + if _is_posix_only_program(node, var_progs): return True # Capture/redirect is only a safety boundary for programs that don't # allocate a Windows console — NOT for git/npm/node/python/ffmpeg/etc. - if not _is_windows_flashing_program(node): + if not _is_windows_flashing_program(node, var_progs): if func_name == "check_output": return True if explicit & {"stdout", "stderr", "capture_output"}: @@ -504,41 +506,120 @@ def _suppresses_window(node: ast.Call, func_name: str) -> bool: return False -def _argv_head(node: ast.Call) -> str | None: - """Return the path-stripped first argv element if it's a string literal.""" - if not node.args: +def _head_of_list_literal(seq) -> str | None: + """Path-stripped first element of a list/tuple literal, if a str constant. + + Also recognises ``[sys.executable, ...]`` and ``[sys.executable, "-m", …]`` + as the ``python`` program — the launcher interpreter flashes a console just + like a bare ``python`` does. + """ + if not isinstance(seq, (ast.List, ast.Tuple)) or not seq.elts: return None - first = node.args[0] - if isinstance(first, (ast.List, ast.Tuple)) and first.elts: - head = first.elts[0] - if isinstance(head, ast.Constant) and isinstance(head.value, str): - return head.value.rsplit("/", 1)[-1].rsplit("\\", 1)[-1] + head = seq.elts[0] + if isinstance(head, ast.Constant) and isinstance(head.value, str): + return head.value.rsplit("/", 1)[-1].rsplit("\\", 1)[-1] + # sys.executable -> the running Python interpreter (a flashing console exe). + if ( + isinstance(head, ast.Attribute) + and head.attr == "executable" + and isinstance(head.value, ast.Name) + and head.value.id == "sys" + ): + return "python" return None -def _is_windows_flashing_program(node: ast.Call) -> bool: +def _argv_head(node: ast.Call, var_progs: dict[str, str] | None = None) -> str | None: + """Return the path-stripped program name of the call's argv, or None. + + Resolves three shapes so the rule sees through the indirection that real + update/build code uses (this was the blind spot behind the recurring + Windows flash storm — ``git_cmd = ["git", "-c", …]`` then + ``subprocess.run(git_cmd + ["fetch"], capture_output=True)`` was invisible): + + * literal: ``subprocess.run(["git", "status"])`` + * variable: ``cmd = ["git", …]; subprocess.run(cmd, …)`` + * concat: ``subprocess.run(git_cmd + ["fetch"], …)`` (BinOp Add) + + ``var_progs`` maps a variable name to the program head inferred from its + nearest ``name = [, …]`` assignment in the same module. Dynamic + argv that can't be resolved still returns None (treated as unknown -> + flagged when not otherwise window-safe). + """ + if not node.args: + return None + first = node.args[0] + # literal list/tuple + lit = _head_of_list_literal(first) + if lit is not None: + return lit + # bare variable: cmd + if isinstance(first, ast.Name) and var_progs: + return var_progs.get(first.id) + # concat: + [...] or [...] + + if isinstance(first, ast.BinOp) and isinstance(first.op, ast.Add): + left = first.left + if isinstance(left, ast.Name) and var_progs: + return var_progs.get(left.id) + lit_left = _head_of_list_literal(left) + if lit_left is not None: + return lit_left + return None + + +def _build_var_progs(tree: ast.AST) -> dict[str, str]: + """Map variable names to the program head of the list literal assigned to + them, e.g. ``git_cmd = ["git", "-c", …]`` -> ``{"git_cmd": "git"}``. + + A name assigned more than once with conflicting heads is dropped (we can't + prove which value reaches the call site, so we don't claim to know it). + Scans the whole module — call sites and their ``name = [...]`` definitions + are routinely in different functions, and the names used for command lists + (``git_cmd``, ``pip_cmd``, ``npm``…) don't collide across scopes in + practice. False positives here only ever ADD a flag, which an inline + ``# windows-footgun: ok`` can suppress. + """ + progs: dict[str, str] = {} + conflicted: set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + head = _head_of_list_literal(node.value) + if head is None: + continue + for tgt in node.targets: + if not isinstance(tgt, ast.Name): + continue + if tgt.id in conflicted: + continue + if tgt.id in progs and progs[tgt.id] != head: + # Conflicting definitions — give up on this name. + del progs[tgt.id] + conflicted.add(tgt.id) + continue + progs[tgt.id] = head + return progs + + +def _is_windows_flashing_program( + node: ast.Call, var_progs: dict[str, str] | None = None +) -> bool: """True if the call's program is a known cross-platform console exe that allocates a Windows console window (so capture is NOT a safe boundary).""" - prog = _argv_head(node) + prog = _argv_head(node, var_progs) return prog is not None and prog in _WINDOWS_FLASHING_PROGRAMS -def _is_posix_only_program(node: ast.Call) -> bool: +def _is_posix_only_program( + node: ast.Call, var_progs: dict[str, str] | None = None +) -> bool: """True if the call's program is a statically-known POSIX-only executable. - Only inspects a literal list/tuple first arg whose first element is a - string constant (e.g. ``["launchctl", "bootout", target]``). Dynamic - argv (variables, f-strings) is treated as unknown and still flagged. + Resolves literal, variable, and concat argv shapes (see :func:`_argv_head`). + Dynamic argv that can't be resolved is treated as unknown and still flagged. """ - if not node.args: - return False - first = node.args[0] - if isinstance(first, (ast.List, ast.Tuple)) and first.elts: - head = first.elts[0] - if isinstance(head, ast.Constant) and isinstance(head.value, str): - prog = head.value.rsplit("/", 1)[-1] - return prog in _POSIX_ONLY_PROGRAMS - return False + prog = _argv_head(node, var_progs) + return prog is not None and prog in _POSIX_ONLY_PROGRAMS def scan_subprocess_window_footguns( @@ -555,6 +636,7 @@ def scan_subprocess_window_footguns( except SyntaxError: return [] lines = text.splitlines() + var_progs = _build_var_progs(tree) rule = Footgun( name=SUBPROCESS_FOOTGUN_NAME, pattern=re.compile(r"^$"), # unused; AST-driven @@ -568,7 +650,7 @@ def scan_subprocess_window_footguns( func_name = _call_attr_name(node) if func_name is None: continue - if _suppresses_window(node, func_name): + if _suppresses_window(node, func_name, var_progs): continue lineno = node.lineno line = lines[lineno - 1] if 0 <= lineno - 1 < len(lines) else "" diff --git a/scripts/contributor_audit.py b/scripts/contributor_audit.py index d23cecce53f..935ad39678e 100644 --- a/scripts/contributor_audit.py +++ b/scripts/contributor_audit.py @@ -30,6 +30,7 @@ SCRIPT_DIR = Path(__file__).resolve().parent sys.path.insert(0, str(SCRIPT_DIR)) from release import resolve_author # noqa: E402 +from hermes_cli import _subprocess_compat REPO_ROOT = SCRIPT_DIR.parent @@ -78,7 +79,7 @@ def is_ignored(handle: str, email: str = "") -> bool: def git(*args, cwd=None): """Run a git command and return stdout.""" - result = subprocess.run( + result = _subprocess_compat.run( ["git"] + list(args), capture_output=True, text=True, diff --git a/scripts/release.py b/scripts/release.py index 3ae7440e586..a3b29e49ae0 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -29,6 +29,7 @@ import sys from collections import defaultdict from datetime import datetime from pathlib import Path +from hermes_cli import _subprocess_compat REPO_ROOT = Path(__file__).resolve().parent.parent VERSION_FILE = REPO_ROOT / "hermes_cli" / "__init__.py" @@ -1711,7 +1712,7 @@ AUTHOR_MAP = { def git(*args, cwd=None): """Run a git command and return stdout.""" - result = subprocess.run( + result = _subprocess_compat.run( ["git"] + list(args), capture_output=True, text=True, cwd=cwd or str(REPO_ROOT), @@ -1724,7 +1725,7 @@ def git(*args, cwd=None): def git_result(*args, cwd=None): """Run a git command and return the full CompletedProcess.""" - return subprocess.run( + return _subprocess_compat.run( ["git"] + list(args), capture_output=True, text=True, @@ -1862,7 +1863,7 @@ def build_release_artifacts(semver: str) -> list[Path]: else: cmd = [sys.executable, "-m", "build", "--sdist", "--wheel"] - result = subprocess.run( + result = _subprocess_compat.run( cmd, cwd=str(REPO_ROOT), capture_output=True, @@ -2280,7 +2281,7 @@ def main(): gh_bin = shutil.which("gh") if gh_bin: - result = subprocess.run( + result = _subprocess_compat.run( gh_cmd, capture_output=True, text=True, cwd=str(REPO_ROOT), diff --git a/scripts/run_tests_parallel.py b/scripts/run_tests_parallel.py index 0128bcfbef9..20c4adb9913 100755 --- a/scripts/run_tests_parallel.py +++ b/scripts/run_tests_parallel.py @@ -47,6 +47,7 @@ import time from concurrent.futures import ThreadPoolExecutor, Future from pathlib import Path from typing import Dict, List, Tuple +from hermes_cli import _subprocess_compat # Default test discovery roots. @@ -243,7 +244,7 @@ def _run_one_file( subproc_start = time.monotonic() # launch the pytest process - proc = subprocess.Popen( + proc = _subprocess_compat.popen( cmd, cwd=repo_root, stdout=subprocess.PIPE, diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index cec730dcb2b..5f384c101d2 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -78,6 +78,7 @@ import sysconfig from dataclasses import dataclass from pathlib import Path from typing import Any, Callable, Optional +from hermes_cli import _subprocess_compat logger = logging.getLogger(__name__) @@ -635,7 +636,7 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install # Tier 2: python -m pip (with ensurepip bootstrap if needed) pip_cmd = [sys.executable, "-m", "pip"] try: - probe = subprocess.run( + probe = _subprocess_compat.run( pip_cmd + ["--version"], capture_output=True, text=True, timeout=15, stdin=subprocess.DEVNULL, @@ -644,7 +645,7 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install raise FileNotFoundError("pip not in venv") except (subprocess.TimeoutExpired, FileNotFoundError): try: - subprocess.run( + _subprocess_compat.run( [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], capture_output=True, text=True, timeout=120, check=True, stdin=subprocess.DEVNULL, @@ -654,7 +655,7 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install f"pip not available and ensurepip failed: {e}") try: - r = subprocess.run( + r = _subprocess_compat.run( pip_cmd + ["install", *target_args, *constraint_args, *specs], capture_output=True, text=True, timeout=timeout, stdin=subprocess.DEVNULL,