Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5093,6 +5093,7 @@ def _remove_custom_provider(config):
"_invalidate_update_cache",
"_is_android_python",
"_is_fork",
"_kill_hermes_python_processes",
"_leftover_pausable_gateway_pids",
"_log_only_write",
"_mark_skip_upstream_prompt",
Expand Down
9 changes: 9 additions & 0 deletions hermes_cli/subcommands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ def build_update_parser(subparsers, *, cmd_update: Callable) -> None:
default=False,
help="Windows: proceed with the update even when another hermes.exe is detected. The concurrent process will likely cause WinError 32 warnings. Does NOT bypass the venv-process guard (see --force-venv).",
)
update_parser.add_argument(
"--force-kill",
action="store_true",
default=False,
help="Windows: force-stop all Hermes-related Python processes (headroom proxy, "
"tui_gateway workers, etc.) before updating. These hold .pyd file locks "
"on Windows that would otherwise block the dependency update, potentially "
"leaving a broken install.",
)
update_parser.add_argument(
"--force-venv",
action="store_true",
Expand Down
79 changes: 78 additions & 1 deletion hermes_cli/update_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -4432,9 +4432,78 @@ def _format_venv_python_holders_message(matches: list[tuple[int, str, str]]) ->
" Close the Hermes desktop app / other Hermes terminals, then re-run:"
)
lines.append(" hermes update")
lines.append(" (or use `hermes update --force-venv` to proceed anyway at your own risk)")
lines.append(" (or use `hermes update --force-kill` to stop them and update, or `hermes update --force-venv` to proceed anyway at your own risk)")
return "\n".join(lines)


def _kill_hermes_python_processes() -> int:
"""Force-stop safe venv Python processes holding ``.pyd`` locks before an update.

Reuses :func:`_detect_venv_python_processes` to find candidates, then
``terminate_pid(force=True)`` (a tree-kill) on each PID so the kernel
releases native-extension handles. Best-effort: processes that refuse
to die are logged but do not block the update — ``--force-kill`` was
explicitly requested, so we proceed regardless.

**Desktop backend exclusion:** the Hermes Desktop app supervises its
``python.exe -m hermes_cli.main serve`` backend and respawns it within
seconds, so killing it would not release the lock for long enough to
let the dependency sync through. Such PIDs are skipped with a printed
hint telling the user to close the Desktop app instead (mirrors the
*caller should refuse* contract in ``_detect_venv_python_processes``'s
docstring).

Returns the number of PIDs that were actually terminated. Off-Windows
this is a no-op that always returns 0.
"""
if not _m()._is_windows():
return 0
holders = _detect_venv_python_processes()
if not holders:
return 0

from gateway.status import terminate_pid

killed = 0
refused: list[tuple[int, str, str]] = []
toxic: list[tuple[int, str, str]] = []
for pid, name, cmdline in holders:
low = cmdline.lower()
# Mirror _format_venv_python_holders_message's Desktop classification
# — "serve" / "dashboard" invocations are Desktop-supervised and
# respawn faster than we can sync deps. Refuse rather than kill.
if "serve" in low or "dashboard" in low:
refused.append((pid, name, cmdline))
continue
toxic.append((pid, name, cmdline))

if toxic:
pids = [pid for pid, _, _ in toxic]
print(f"→ Force-stopping {len(pids)} Hermes-related Python process(es) before update")
for pid, name, cmdline in toxic[:6]:
print(f" PID {pid} {name} {cmdline}")
if len(toxic) > 6:
print(f" ... and {len(toxic) - 6} more")
for pid in pids:
try:
terminate_pid(int(pid), force=True)
killed += 1
except (ProcessLookupError, PermissionError, OSError) as exc:
logger.debug("Could not force-stop venv pid %s: %s", pid, exc)

if refused:
print("⚠ Skipping Hermes Desktop backend process(es) — close the Desktop app first:")
for pid, name, cmdline in refused[:6]:
print(f" PID {pid} {name} {cmdline} ← Desktop backend (respawns if killed)")
if len(refused) > 6:
print(f" ... and {len(refused) - 6} more")

if killed:
# Brief pause so the kernel releases .pyd handles before pip/uv runs.
_time.sleep(1.0)
return killed


def _venv_launcher_ancestors(pids: list[int]) -> list[int]:
"""Return venv-interpreter ancestors of *pids* that hold the install open.

Expand Down Expand Up @@ -5987,6 +6056,14 @@ def _cmd_update_impl(args, gateway_mode: bool):
# and app.asar — a non-desktop venv python holding a .pyd would sail
# through and corrupt the sync (the exact failure this guard exists for).
# --force-venv is the explicit escape hatch.
# --force-kill is the user-facing "shut everything down then update"
# path: it terminates the venv holders so the dependency sync can
# proceed, rather than aborting (default) or racing past the lock
# (--force-venv). Executed AFTER gateway pause/snapshot so relaunch
# state is preserved.
if _m()._is_windows() and getattr(args, "force_kill", False):
_kill_hermes_python_processes()

if _m()._is_windows() and not getattr(args, "force_venv", False):
_venv_holders = _m()._detect_venv_python_processes()
if _venv_holders:
Expand Down
170 changes: 170 additions & 0 deletions tests/hermes_cli/test_update_concurrent_quarantine.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,176 @@ def test_unreadable_argv_falls_back_to_the_captured_prefix(monkeypatch):



# ---------------------------------------------------------------------------
# _kill_hermes_python_processes — --force-kill path (issue #63300)
#
# The kill helper must:
# 1. Be a no-op off-Windows (so tests on Linux don't spawn taskkill).
# 2. Be a no-op when no venv holders are detected.
# 3. Call terminate_pid(force=True) for every holder PID (tree-kill).
# 4. Survive individual terminate_pid failures without raising.
# 5. _cmd_update_impl must invoke it BEFORE the venv-holder guard so
# the guard sees an empty holder set and lets the update proceed.
# ---------------------------------------------------------------------------


import hermes_cli.update_cmd as _update_cmd


@patch.object(cli_main, "_is_windows", return_value=True)
def test_force_kill_no_op_when_no_holders(_winp, monkeypatch):
"""No holders → no terminate_pid calls; doesn't raise."""
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes", lambda **_k: []
)
killed: list[int] = []
import gateway.status as _status_mod
monkeypatch.setattr(
_status_mod, "terminate_pid",
lambda pid, force=False: killed.append(int(pid)),
)
monkeypatch.setattr(_update_cmd, "_time", types.SimpleNamespace(sleep=lambda _s: None))

_update_cmd._kill_hermes_python_processes()
assert killed == []


@patch.object(cli_main, "_is_windows", return_value=False)
def test_force_kill_no_op_off_windows(_winp, monkeypatch):
"""Off-Windows → no terminate_pid calls at all."""
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes",
lambda **_k: [(999, "python.exe", "leak")],
)
killed: list[int] = []
import gateway.status as _status_mod
monkeypatch.setattr(
_status_mod, "terminate_pid",
lambda pid, force=False: killed.append(int(pid)),
)
_update_cmd._kill_hermes_python_processes()
assert killed == []


@patch.object(cli_main, "_is_windows", return_value=True)
def test_force_kill_terminates_every_holder_pid(_winp, monkeypatch):
"""Every *non-Desktop* holder PID gets a force tree-kill; Desktop
backends (cmdline contains ``serve`` / ``dashboard``) are refused and
skipped, mirroring the ``_detect_venv_python_processes`` docstring which
says the Desktop app supervises that backend and respawns it within
seconds — killing it would not release the lock long enough.
"""
holders = [
(500, "python.exe", r"C:\\x\\venv\\Scripts\\python.exe -m headroom.cli proxy"),
(600, "pythonw.exe", r"C:\\x\\venv\\Scripts\\pythonw.exe -m tui_gateway.slash_worker"),
(700, "python.exe", r"C:\\x\\venv\\Scripts\\python.exe -m hermes_cli.main serve"),
]
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes", lambda **_k: list(holders)
)
killed: list[tuple[int, bool]] = []
import gateway.status as _status_mod
monkeypatch.setattr(
_status_mod, "terminate_pid",
lambda pid, force=False: killed.append((int(pid), bool(force))),
)
monkeypatch.setattr(_update_cmd, "_time", types.SimpleNamespace(sleep=lambda _s: None))

n = _update_cmd._kill_hermes_python_processes()

# Only PIDs 500 and 600 are terminated; 700 is a Desktop backend → refused.
assert sorted(killed) == [(500, True), (600, True)]
assert n == 2


@patch.object(cli_main, "_is_windows", return_value=True)
def test_force_kill_refuses_desktop_backend(_winp, monkeypatch):
"""A ``serve`` cmdline is refused (not killed) and the rest still fire."""
holders = [
(800, "python.exe", r"...python.exe -m hermes_cli.main dashboard --no-open"),
(900, "python.exe", r"...python.exe -m hermes_cli.main serve"),
(950, "python.exe", r"...python.exe -m headroom.cli proxy"),
]
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes", lambda **_k: list(holders)
)
killed: list[int] = []
import gateway.status as _status_mod
monkeypatch.setattr(_status_mod, "terminate_pid", lambda pid, force=False: killed.append(int(pid)))
monkeypatch.setattr(_update_cmd, "_time", types.SimpleNamespace(sleep=lambda _s: None))

_update_cmd._kill_hermes_python_processes()
# Only the headroom proxy gets killed; the two Desktop backends are refused.
assert killed == [950]


@patch.object(cli_main, "_is_windows", return_value=True)
def test_force_kill_survives_terminate_failures(_winp, monkeypatch):
"""A refused kill on one PID doesn't stop termination of the rest."""
holders = [(500, "python.exe", "..."), (600, "python.exe", "...")]
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes", lambda **_k: list(holders)
)
killed: list[int] = []
import gateway.status as _status_mod

def _flaky_terminate(pid, force=False):
pid = int(pid)
if pid == 500:
raise OSError("access denied")
killed.append(pid)

monkeypatch.setattr(_status_mod, "terminate_pid", _flaky_terminate)
monkeypatch.setattr(_update_cmd, "_time", types.SimpleNamespace(sleep=lambda _s: None))

_update_cmd._kill_hermes_python_processes()
assert killed == [600] # 500 raised, 600 still got killed


@patch.object(cli_main, "_is_windows", return_value=True)
def test_force_kill_flag_decision_is_correct(_winp, monkeypatch):
"""The --force-kill branch fires iff Windows AND args.force_kill; the
venv-holder guard fires iff Windows AND not --force-venv (regardless of
--force-kill). This stubs out _kill_hermes_python_processes and asserts
the kill branch sees the call while the guard would see no holders.

We do NOT call _cmd_update_impl (it would run real git/pip); instead we
exercise the actual decision helper used by that function. This isolates
the wiring contract without coupling to network side effects.
"""
# Track whether the kill helper actually fired.
kill_invoked = []
monkeypatch.setattr(
_update_cmd, "_kill_hermes_python_processes",
lambda: kill_invoked.append(1),
)
# The post-kill detect (i.e. what the guard re-scan would see) returns [].
monkeypatch.setattr(_update_cmd, "_detect_venv_python_processes", lambda **_k: [])

# WITH --force-kill: killer runs. Simulate the branch _cmd_update_impl takes.
args_kill = SimpleNamespace(force=False, force_kill=True, force_venv=False)
if cli_main._is_windows() and getattr(args_kill, "force_kill", False):
_update_cmd._kill_hermes_python_processes()
# Post-kill venv-holder guard: should see nothing now.
post_kill_holders = _update_cmd._detect_venv_python_processes()
assert kill_invoked == [1], "--force-kill must invoke the killer"
assert post_kill_holders == [], "post-kill scan must be empty"

# WITHOUT --force-kill: killer does NOT run; venv guard sees the holders.
kill_invoked.clear()
monkeypatch.setattr(
_update_cmd, "_detect_venv_python_processes",
lambda **_k: [(500, "python.exe", "venv\\python.exe")],
)
args_no_kill = SimpleNamespace(force=False, force_kill=False, force_venv=False)
if cli_main._is_windows() and getattr(args_no_kill, "force_kill", False):
_update_cmd._kill_hermes_python_processes()
# But the guard would fire: holders exist, no --force-venv escape.
guard_holders = _update_cmd._detect_venv_python_processes()
assert kill_invoked == [], "without --force-kill, the killer must NOT run"
assert guard_holders, "without --force-kill, the guard must see the holders"


# ---------------------------------------------------------------------------
# cmd_update integration — concurrent-instance gate
# ---------------------------------------------------------------------------
Expand Down