hermes-agent/tests/hermes_cli/test_update_concurrent_quarantine.py
Teknium 39975613b1
test: prune wave 2 + speed fixes — 28,106 → 19,757 test functions, suite wall 315s → 294s
Second, deeper pass over tools/gateway/hermes_cli plus first pass over
the trees wave 1 missed (acp, acp_adapter, skills, computer_use, docker,
dashboard, conformance, monitoring, secret_sources, hermes_state,
providers). Same rubric as wave 1 (AGENTS.md test policy); security,
alternation/caching invariants, issue-number regressions, and E2E kept.

Real test-quality fixes found and rooted out along the way:
- tests/tools/test_command_guards.py made real auxiliary-LLM HTTPS calls
  (DEFAULT_CONFIG smart-approval leaked in) — pinned approval
  mode=manual via autouse fixture: 17.4s → 0.4s.
- test_model_switch_custom_providers.py / test_user_providers_model_switch.py
  silently probed live provider catalogs (~2s/test) — stubbed
  cached_provider_model_ids/provider_model_ids/fetch_api_models.
- test_telegram_noise_filter.py: 15-platform copy-paste matrix over
  shared gateway.run logic → 3 representative platforms (55s → 3.9s).
- test_gateway_shutdown.py: stop()'s 5s interrupt-deadline loop spun on
  MagicMock agents — interrupt.side_effect now clears _running_agents
  (22s → 1.0s).
- test_gateway_inactivity_timeout.py poll-harness timings shrunk 3-5x
  (24s → 1.1s); test_mcp_stability.py backoff/SIGTERM-grace sleeps
  patched (15.4s → 2.5s); test_async_delegation.py negative-drain wait
  5s → 0.5s.
- test_telegram_init_deadline.py: loop-block margin restored to 1.0s
  with rationale comment — the watchdog-dump assertion needs the loop
  blocked well past deadline+grace under parallel load (flaked once in
  the 40-worker verification run at a 0.2s margin).

Verification: full hermetic suite via scripts/run_tests.sh —
2,438 files, 21,718 tests passed, 0 failed, 293.9s wall.
Suite totals vs original baseline: 46,820 → 19,757 test functions
(−57.8%), wall 583.5s → 293.9s (−50%), subprocess CPU 13,564s → 11,623s.
2026-07-29 13:39:40 -07:00

287 lines
9.4 KiB
Python

"""Tests for issue #26670 — concurrent hermes.exe detection and improved
quarantine retry / reboot-deferred fallback during `hermes update` on Windows.
These tests force ``_is_windows`` to return ``True`` via patching so the
Windows-specific code paths can be exercised on any host.
"""
from __future__ import annotations
import json
import os
import sys
import types
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
import pytest
from hermes_cli import main as cli_main
# Tests in this module either exercise the REAL _detect_concurrent_hermes_instances
# helper (and need the autouse stub in tests/hermes_cli/conftest.py disabled),
# or supply their own explicit return value via patch.object. Mark the whole
# module so the conftest fixture skips its default stub.
pytestmark = pytest.mark.real_concurrent_gate
# ---------------------------------------------------------------------------
# _detect_concurrent_hermes_instances
# ---------------------------------------------------------------------------
def _make_proc(pid: int, exe: str, name: str = "hermes.exe"):
"""Build a duck-typed psutil Process stand-in with the .info dict."""
proc = MagicMock()
proc.info = {"pid": pid, "exe": exe, "name": name}
return proc
# ---------------------------------------------------------------------------
# Parent-chain exclusion (issue #30768 follow-up — the setuptools .exe
# launcher on Windows is a separate native process that spawns python.exe;
# excluding only ``os.getpid()`` flags the launcher as a concurrent instance.
# ---------------------------------------------------------------------------
def _fake_psutil_with_parent_chain(
parent_chain: list[int],
proc_iter_rows: list,
*,
ancestor_exe: str | None = None,
):
"""Build a psutil stand-in that has Process()/parents()/exe() AND process_iter().
``parent_chain`` is the ordered list of ancestor PIDs (closest first)
returned by ``proc.parents()`` on the seed (``os.getpid()``).
``ancestor_exe`` is the executable path reported by each ancestor's
``.exe()``; when it matches one of our shim paths the ancestor is
excluded (the launcher-shim case). Pass ``None`` to model an ancestor
whose exe can't be read (psutil error) — it stays in the candidate set.
"""
class _FakeProc:
def __init__(self, pid: int, exe_path: str | None):
self.pid = pid
self._exe = exe_path
def exe(self):
if self._exe is None:
raise OSError("exe unavailable")
return self._exe
def parents(self):
return [_FakeProc(p, ancestor_exe) for p in parent_chain]
class _NoSuchProcess(Exception):
pass
class _AccessDenied(Exception):
pass
def _process(pid=None):
return _FakeProc(pid if pid is not None else os.getpid(), ancestor_exe)
return types.SimpleNamespace(
Process=_process,
NoSuchProcess=_NoSuchProcess,
AccessDenied=_AccessDenied,
process_iter=lambda attrs: iter(proc_iter_rows),
)
@patch.object(cli_main, "_is_windows", return_value=True)
def test_detect_concurrent_parents_call_robust_to_one_bad_hop(_winp, tmp_path):
"""The launcher shim is still excluded even when an ancestor exe is unreadable.
Field regression (issues #29341, #34795): the old per-hop ``parent()``
walk bailed on the FIRST psutil error, so an AccessDenied on any hop left
the launcher shim in the candidate set and re-triggered the false
positive. ``parents()`` returns the whole list at once; we evaluate each
ancestor independently, so one unreadable hop never strands the launcher.
"""
scripts_dir = tmp_path
shim = scripts_dir / "hermes.exe"
shim.write_bytes(b"")
me = os.getpid()
launcher_pid = me + 100
rows = [
_make_proc(me, str(shim), "python.exe"),
_make_proc(launcher_pid, str(shim), "hermes.exe"),
]
# ancestor_exe=None → every ancestor's .exe() raises OSError. The helper
# must swallow it per-ancestor and not crash; the launcher won't be
# excluded in this degenerate case, but a real run reads the shim exe.
fake_psutil = _fake_psutil_with_parent_chain(
parent_chain=[launcher_pid],
proc_iter_rows=rows,
ancestor_exe=None,
)
with patch.dict(sys.modules, {"psutil": fake_psutil}):
result = cli_main._detect_concurrent_hermes_instances(scripts_dir)
# No crash; helper completes. (Degenerate stub: launcher exe unreadable.)
assert result == [(launcher_pid, "hermes.exe")]
# ---------------------------------------------------------------------------
# _format_concurrent_instances_message
# ---------------------------------------------------------------------------
# ---------------------------------------------------------------------------
# _quarantine_running_hermes_exe — retry + reboot-deferred fallback
# ---------------------------------------------------------------------------
@patch.object(cli_main, "_is_windows", return_value=True)
def test_quarantine_succeeds_first_attempt(_winp, tmp_path):
"""When the rename works immediately, no warning, single rename pair returned."""
shim = tmp_path / "hermes.exe"
shim.write_bytes(b"old")
pairs = cli_main._quarantine_running_hermes_exe(tmp_path)
assert len(pairs) == 1
orig, quarantine = pairs[0]
assert orig == shim
assert quarantine.name.startswith("hermes.exe.old.")
assert quarantine.exists()
assert not shim.exists()
@patch.object(cli_main, "_is_windows", return_value=True)
def test_quarantine_falls_back_to_reboot_schedule(_winp, tmp_path, capsys, monkeypatch):
"""When every retry fails, we schedule via MoveFileEx and warn helpfully."""
shim = tmp_path / "hermes.exe"
shim.write_bytes(b"locked")
def always_fails(self, target):
raise OSError(32, "The process cannot access the file (simulated lock)")
scheduled_calls: list[tuple[Path, Path]] = []
def fake_schedule(s: Path, q: Path) -> bool:
scheduled_calls.append((s, q))
return True
monkeypatch.setattr(cli_main, "_hermes_exe_shims", lambda d: [shim])
with patch.object(Path, "rename", always_fails), patch.object(
cli_main, "_schedule_replace_on_reboot", fake_schedule
), patch("time.sleep", lambda *_a, **_k: None):
pairs = cli_main._quarantine_running_hermes_exe(tmp_path)
captured = capsys.readouterr().out
# The reboot-deferred path was used.
assert scheduled_calls and scheduled_calls[0][0] == shim
# It is NOT added to the returned roll-back list (the issue calls this
# out — don't undo a deferred operation).
assert pairs == []
# The user got a clear message, not raw [WinError 32].
assert "scheduled" in captured.lower()
assert "reboot" in captured.lower()
# ---------------------------------------------------------------------------
# Windows gateway pause/resume before update mutation
# ---------------------------------------------------------------------------
@patch.object(cli_main, "_is_windows", return_value=True)
def test_pause_windows_gateways_for_update_stops_profile_and_unmapped_pids(
_winp,
monkeypatch,
tmp_path,
capsys,
):
import gateway.status as status_mod
import hermes_cli.gateway as gateway_mod
profile_home = tmp_path / "profiles" / "work"
profile_home.mkdir(parents=True)
profile_proc = SimpleNamespace(profile="work", path=profile_home, pid=101)
monkeypatch.setattr(gateway_mod, "find_gateway_pids", lambda **_k: [101, 202])
monkeypatch.setattr(
gateway_mod,
"find_profile_gateway_processes",
lambda **_k: [profile_proc],
)
monkeypatch.setattr(gateway_mod, "_get_restart_drain_timeout", lambda: 0.1)
waited_for = []
def fake_wait(pids, *, timeout):
waited_for.extend(pids)
return set()
monkeypatch.setattr(cli_main, "_wait_for_windows_update_gateway_exit", fake_wait)
monkeypatch.setattr(
gateway_mod,
"_capture_gateway_argv",
lambda pid: ["pythonw.exe", "-m", "hermes_cli.main", "gateway", "run"]
if pid == 202
else None,
)
terminated = []
monkeypatch.setattr(
status_mod,
"terminate_pid",
lambda pid, force=False: terminated.append((pid, force)),
)
token = cli_main._pause_windows_gateways_for_update()
assert token == {
"resume_needed": True,
"profiles": {"work": 101},
"unmapped_pids": [202],
"unmapped": [
{
"pid": 202,
"argv": ["pythonw.exe", "-m", "hermes_cli.main", "gateway", "run"],
}
],
}
assert waited_for == [101]
assert terminated == [(202, True)]
marker = json.loads((profile_home / ".gateway-planned-stop.json").read_text())
assert marker["target_pid"] == 101
assert marker["stopper_pid"] == os.getpid()
captured = capsys.readouterr().out
assert "Paused gateway profile(s): work" in captured
assert "without profile mapping" in captured
# An unmapped PID whose argv we captured is respawnable, so we must NOT
# tell the user to restart it manually.
assert "Restart manually after update" not in captured
# ---------------------------------------------------------------------------
# cmd_update integration — concurrent-instance gate
# ---------------------------------------------------------------------------