mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(windows): stop persistent gateway console after update + close checker variable-argv blind spot
Two distinct Windows console bugs that survived #53791/#53810/#53829: 1. Persistent console after update that kills the gateway when closed. _spawn_gateway_restart_watcher rewrote the respawned gateway argv to windowless pythonw, but launched the WATCHER process itself with sys.executable — the venv console python.exe during `hermes update`. uv's venv launcher re-execs the base console interpreter, allocating a conhost that DETACHED_PROCESS/CREATE_NO_WINDOW can't suppress; the respawned gateway inherits it. Resolve the watcher interpreter to pythonw too (no-op on POSIX). 2. The AST footgun checker only resolved a flashing program when argv[0] was a string literal, so the update path's `git_cmd = ["git", ...]` then `subprocess.run(git_cmd + [...], capture_output=True)` calls were invisible and shipped unflagged — one console flash per call. Teach _argv_head to resolve variable and concat (BinOp Add) argv via a module-wide name->program map. Route the 39 newly-visible calls in main.py + 23 across 12 other files through the _subprocess_compat chokepoint (or creationflags=windows_hide_flags()). Also fixes a latent NameError: main.py called _subprocess_compat.run with no module import in scope — the update path would have crashed on Windows. Tests: checker 32/32, watcher/restart/detach 5/5. Update suite failures drop 29->7 (remaining 7 are pre-existing baseline failures).
This commit is contained in:
Teknium
parent
2ecca1e7d3
commit
4c677698ff
16 changed files with 202 additions and 89 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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/<bogus> --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/<branch>. 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,
|
||||
|
|
|
|||
|
|
@ -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))"],
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 = [<literal>, …]`` 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: <var> + [...] or [...] + <var>
|
||||
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 ""
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue