Skip to content
Closed
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
81 changes: 79 additions & 2 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8235,6 +8235,70 @@ def _format_concurrent_instances_message(
return "\n".join(lines)


def _classify_concurrent_instance(pid: int) -> str:
"""Return ``"gateway"`` if ``pid``'s command line looks like a gateway.

Mirrors the substring patterns used by
``hermes_cli.gateway._scan_gateway_pids`` so a PID classified as
``"gateway"`` here is the same set of PIDs the post-update kill+restart
block will reap. That symmetry is what lets the pre-update concurrent
gate safely skip the abort for gateway-only matches: the gateway is
going to be killed anyway, so refusing to update just to make the user
kill it manually is friction without benefit.

Returns ``"non-gateway"`` when the cmdline doesn't match the gateway
patterns, and ``"unknown"`` when psutil can't read it (process gone,
access denied, etc.). The pre-update gate treats ``"unknown"`` as
non-gateway (i.e. errs on the side of the existing abort) — we'd
rather block an update we could have completed than kill something
we couldn't positively identify as a gateway.
"""
try:
import psutil
except Exception:
return "unknown"

try:
proc = psutil.Process(int(pid))
cmdline_list = proc.cmdline()
except Exception:
return "unknown"

cmdline = " ".join(cmdline_list or [])
cmdline_lc = cmdline.lower()
gateway_markers = (
"hermes_cli.main gateway",
"hermes_cli/main.py gateway",
"hermes gateway",
"hermes.exe gateway",
"hermes-gateway.exe",
"gateway/run.py",
)
Comment on lines +8269 to +8276
if any(marker in cmdline_lc for marker in gateway_markers):
return "gateway"
return "non-gateway"


def _filter_non_gateway_concurrent_instances(
matches: list[tuple[int, str]],
) -> list[tuple[int, str]]:
"""Return only the concurrent-instance matches that are NOT the gateway.

Used by the pre-update concurrent gate to decide whether to abort
``hermes update``. If every concurrent instance is a gateway, the
post-update kill+restart block will handle it — we let the update
proceed. If anything else (a TUI shell, a Hermes Desktop backend
child, an unrelated ``hermes`` REPL) is in the list, we still abort
with the existing message, since those aren't covered by the
post-update sweep.
"""
non_gateway: list[tuple[int, str]] = []
for pid, name in matches:
if _classify_concurrent_instance(pid) != "gateway":
non_gateway.append((pid, name))
return non_gateway


def _quarantine_running_hermes_exe(
scripts_dir: Path, *, max_attempts: int = 4
) -> list[tuple[Path, Path]]:
Expand Down Expand Up @@ -9301,13 +9365,26 @@ def _cmd_update_impl(args, gateway_mode: bool):
# open. Continuing would result in a string of WinError 32 warnings and
# then either a deferred-rename leftover or a failed git-pull fast path
# that silently falls back to the slower ZIP route. See issue #26670.
#
# Exception: when the only concurrent instance is a gateway process, the
# post-update logic at the bottom of this function (the gateway kill+
# restart block) already handles it cleanly — it matches gateways by
# command line ("hermes gateway" / "hermes.exe gateway" / etc.), not by
# the venv shim .exe path, so the gateway is always stopped and respawned
# after a successful code update. Aborting the update just to make the
# user do the same kill manually is pointless friction — the gateway is
# going to be killed anyway. Skip the abort in that case and let the
# update flow continue; the gateway PID gets reaped by the post-update
# block a few seconds later.
if _is_windows() and not getattr(args, "force", False):
scripts_dir = _venv_scripts_dir()
if scripts_dir is not None:
concurrent = _detect_concurrent_hermes_instances(scripts_dir)
if concurrent:
print(_format_concurrent_instances_message(concurrent, scripts_dir))
sys.exit(2)
non_gateway = _filter_non_gateway_concurrent_instances(concurrent)
if non_gateway:
print(_format_concurrent_instances_message(non_gateway, scripts_dir))
sys.exit(2)

# Pre-update backup — runs before any git/file mutation so users can
# always roll back to the exact state they had before this update.
Expand Down
270 changes: 270 additions & 0 deletions tests/hermes_cli/test_update_concurrent_quarantine.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,273 @@ def test_cmd_update_force_bypasses_concurrent_check(_winp, tmp_path):

# When --force is set, we should not have even consulted psutil.
detect.assert_not_called()


# ---------------------------------------------------------------------------
# _classify_concurrent_instance / _filter_non_gateway_concurrent_instances
#
# These helpers were added so the pre-update concurrent-instance gate can
# let the update proceed when the only concurrent hermes.exe is a gateway
# process — the post-update kill+restart block at the bottom of cmd_update
# already handles gateway PIDs by command-line match, so refusing the
# update just to make the user kill the gateway manually is friction
# without benefit.
# ---------------------------------------------------------------------------


def _make_psutil_proc_with_cmdline(pid: int, cmdline: list[str]):
"""Mock a psutil.Process with a .cmdline() method returning ``cmdline``."""
proc = MagicMock()
proc.cmdline = MagicMock(return_value=list(cmdline))
return proc


@patch.object(cli_main, "_is_windows", return_value=True)
def test_classify_concurrent_instance_recognises_hermes_gateway(_winp):
"""A process whose cmdline contains the ``gateway`` keyword classifies
as ``"gateway"`` regardless of the launcher (python -m, hermes.exe
shim, hermes-gateway.exe shim, or direct gateway/run.py invocation)."""
from hermes_cli.main import _classify_concurrent_instance

cases = [
# python -m hermes_cli.main gateway run
["C:/venv/Scripts/python.exe", "-m", "hermes_cli.main", "gateway", "run"],
# hermes.exe gateway run (Windows console-script shim)
["C:/venv/Scripts/hermes.exe", "gateway", "run"],
# hermes-gateway.exe direct
["C:/venv/Scripts/hermes-gateway.exe"],
# direct python gateway/run.py (development launch)
["C:/venv/Scripts/python.exe", "gateway/run.py"],
# case-insensitive: uppercase GATEWAY
["hermes.exe", "GATEWAY", "run"],
]
for cmdline in cases:
fake = types.SimpleNamespace(
Process=lambda pid: _make_psutil_proc_with_cmdline(pid, cmdline)
)
with patch.dict(sys.modules, {"psutil": fake}):
result = _classify_concurrent_instance(1234)
assert result == "gateway", f"expected gateway for cmdline={cmdline!r}, got {result!r}"


@patch.object(cli_main, "_is_windows", return_value=True)
def test_classify_concurrent_instance_recognises_non_gateway(_winp):
"""Processes without ``gateway`` in their cmdline classify as
``"non-gateway"``. This is the bucket that should still trigger the
pre-update abort (TUI shells, Desktop backend, plain REPLs)."""
from hermes_cli.main import _classify_concurrent_instance

cases = [
# Interactive REPL — no gateway subcommand
["C:/venv/Scripts/hermes.exe"],
# Dashboard
["C:/venv/Scripts/hermes.exe", "dashboard"],
# Direct python -m hermes_cli.main (no subcommand)
["python", "-m", "hermes_cli.main"],
# Empty cmdline
[],
]
for cmdline in cases:
fake = types.SimpleNamespace(
Process=lambda pid: _make_psutil_proc_with_cmdline(pid, cmdline)
)
with patch.dict(sys.modules, {"psutil": fake}):
result = _classify_concurrent_instance(1234)
assert result == "non-gateway", f"expected non-gateway for cmdline={cmdline!r}, got {result!r}"


@patch.object(cli_main, "_is_windows", return_value=True)
def test_classify_concurrent_instance_returns_unknown_on_psutil_error(_winp):
"""If psutil can't read the cmdline (process gone, AccessDenied on a
session boundary, etc.) the helper returns ``"unknown"``. The pre-update
gate treats ``"unknown"`` as non-gateway — we'd rather block an update
we could have completed than kill something we couldn't positively
identify as a gateway."""
from hermes_cli.main import _classify_concurrent_instance

class _BrokenProc:
def cmdline(self): # noqa: D401 — psutil Process duck type
raise psutil.AccessDenied(pid=1234)

fake = types.SimpleNamespace(Process=lambda pid: _BrokenProc())
import psutil as _real_psutil
with patch.dict(sys.modules, {"psutil": _real_psutil}):
result = _classify_concurrent_instance(1234)
assert result == "unknown"
Comment on lines +617 to +625


def test_classify_concurrent_instance_returns_unknown_without_psutil():
"""If psutil isn't importable at all (very rare but possible in slim
installs), the helper returns ``"unknown"`` rather than crashing."""
from hermes_cli.main import _classify_concurrent_instance

with patch.dict(sys.modules, {"psutil": None}):
result = _classify_concurrent_instance(1234)
assert result == "unknown"


@patch.object(cli_main, "_is_windows", return_value=True)
def test_filter_non_gateway_concurrent_instances_splits_correctly(_winp):
"""The filter drops PIDs whose cmdline looks like a gateway and keeps
the rest. The pre-update gate then aborts only on the kept list."""
from hermes_cli.main import _filter_non_gateway_concurrent_instances

# Three PIDs: one gateway (drop), one TUI/REPL (keep), one Desktop
# backend child (keep). The gate should only block on the two
# non-gateway entries; the gateway is left for the post-update sweep.
def _make_filter_psutil():
def _proc_factory(pid: int):
if pid == 100:
return _make_psutil_proc_with_cmdline(
pid, ["hermes.exe", "gateway", "run"]
)
if pid == 200:
return _make_psutil_proc_with_cmdline(
pid, ["hermes.exe"] # interactive REPL
)
if pid == 300:
return _make_psutil_proc_with_cmdline(
pid, ["hermes.exe", "dashboard"]
)
raise AssertionError(f"unexpected pid {pid}")

return types.SimpleNamespace(Process=_proc_factory)

matches = [(100, "hermes.exe"), (200, "hermes.exe"), (300, "hermes.exe")]

with patch.dict(sys.modules, {"psutil": _make_filter_psutil()}):
kept = _filter_non_gateway_concurrent_instances(matches)

assert kept == [(200, "hermes.exe"), (300, "hermes.exe")]


@patch.object(cli_main, "_is_windows", return_value=True)
def test_filter_non_gateway_concurrent_instances_drops_gateway_only(_winp):
"""If every concurrent instance is a gateway, the filter returns an
empty list. The pre-update gate sees no non-gateway matches and lets
the update proceed — the gateway PID will be reaped by the post-update
kill+restart block."""
from hermes_cli.main import _filter_non_gateway_concurrent_instances

def _proc_factory(pid: int):
return _make_psutil_proc_with_cmdline(
pid, ["hermes.exe", "gateway", "run"]
)

matches = [(111, "hermes.exe"), (222, "hermes-gateway.exe")]

with patch.dict(sys.modules, {"psutil": types.SimpleNamespace(Process=_proc_factory)}):
kept = _filter_non_gateway_concurrent_instances(matches)

assert kept == []


# ---------------------------------------------------------------------------
# cmd_update integration with the relaxed pre-update gate
# ---------------------------------------------------------------------------


@patch.object(cli_main, "_is_windows", return_value=True)
def test_cmd_update_skips_abort_when_only_concurrent_is_gateway(
_winp, tmp_path, capsys
):
"""Regression test for the user-reported issue: when the gateway is
the only other ``hermes.exe`` running, ``hermes update`` used to abort
with exit code 2, forcing the user to kill all hermes.exe manually
before retrying. The post-update kill+restart block already handles
gateway PIDs cleanly, so the gate should now let the update proceed
instead."""
scripts_dir = tmp_path / "Scripts"
scripts_dir.mkdir()

args = SimpleNamespace(
check=False,
gateway=False,
yes=False,
force=False,
backup=False,
no_backup=True,
)

# Two concurrent matches, BOTH gateway — filter should return [].
# Short-circuit at the first call after the gate so we don't have to
# mock the entire update pipeline.
with patch.object(
cli_main, "_venv_scripts_dir", return_value=scripts_dir
), patch.object(
cli_main,
"_detect_concurrent_hermes_instances",
return_value=[(1000, "hermes.exe"), (2000, "hermes-gateway.exe")],
), patch.object(
cli_main, "_filter_non_gateway_concurrent_instances", return_value=[]
), patch.object(
cli_main, "_run_pre_update_backup"
) as mock_backup, patch.object(
cli_main, "_install_hangup_protection", return_value={}
), patch.object(
cli_main, "_finalize_update_output"
):
# Should NOT raise SystemExit. The sentinel that fires at the next
# real step (pre-update backup) lets us assert we got past the
# gate without mocking the whole pipeline.
mock_backup.side_effect = RuntimeError("reached post-gate body")
with pytest.raises(RuntimeError, match="reached post-gate body"):
cli_main.cmd_update(args)

# Pre-update backup was reached — gate let us through.
mock_backup.assert_called_once()

captured = capsys.readouterr().out
assert "Another hermes.exe is running" not in captured
assert "--force" not in captured # the abort-message override hint


@patch.object(cli_main, "_is_windows", return_value=True)
def test_cmd_update_still_aborts_on_non_gateway_concurrent(
_winp, tmp_path, capsys
):
"""The gate must STILL abort when an unrelated concurrent hermes.exe
is in the list (TUI, Desktop backend, plain REPL). Only gateway-only
matches are exempted — see test_cmd_update_skips_abort_when_only_*
for the relaxed case."""
scripts_dir = tmp_path / "Scripts"
scripts_dir.mkdir()

args = SimpleNamespace(
check=False,
gateway=False,
yes=False,
force=False,
backup=False,
no_backup=True,
)

with patch.object(
cli_main, "_venv_scripts_dir", return_value=scripts_dir
), patch.object(
cli_main,
"_detect_concurrent_hermes_instances",
return_value=[
(1000, "hermes.exe"), # would be classified as gateway
(3000, "hermes.exe"), # would be classified as non-gateway
],
), patch.object(
cli_main,
"_filter_non_gateway_concurrent_instances",
return_value=[(3000, "hermes.exe")], # simulated filter result
), patch.object(
cli_main, "_run_pre_update_backup"
) as mock_backup, patch.object(
cli_main, "_install_hangup_protection", return_value={}
), patch.object(
cli_main, "_finalize_update_output"
):
with pytest.raises(SystemExit) as excinfo:
cli_main.cmd_update(args)

assert excinfo.value.code == 2
mock_backup.assert_not_called()

captured = capsys.readouterr().out
assert "3000" in captured
assert "--force" in captured