diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 2d10dc4d13..59a7756bd7 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -1344,6 +1344,7 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str]) -> bool: import sys import time from hermes_cli._subprocess_compat import ( + _WINDOWS_GATEWAY_BREAKAWAY_ENV, windows_detach_flags, windows_detach_flags_without_breakaway, ) @@ -1361,6 +1362,24 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str]) -> bool: break time.sleep(0.2) + # Route stray stdout/stderr from the respawned gateway to the same + # sidecar log _spawn_detached uses. DEVNULL here meant a gateway + # killed moments after respawn (e.g. parent Job Object teardown when + # breakaway is denied, #48820 4th repro) left ZERO trace anywhere — + # no gateway.log line, no exit-diag record, nothing. Best-effort: + # fall back to DEVNULL when the log dir is unavailable. + _stdio_target = subprocess.DEVNULL + _stdio_fh = None + try: + from hermes_cli.config import get_hermes_home + from pathlib import Path + _log_dir = Path(get_hermes_home()) / "logs" + _log_dir.mkdir(parents=True, exist_ok=True) + _stdio_fh = open(_log_dir / "gateway-stdio.log", "ab", buffering=0) + _stdio_target = _stdio_fh + except Exception: + pass + # Platform-appropriate detach for the respawned gateway. On POSIX # start_new_session=True maps to os.setsid; on Windows we need # explicit creationflags because start_new_session is a no-op there. @@ -1369,8 +1388,8 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str]) -> bool: # without breakaway the respawned gateway would die when that job # tears down. See _subprocess_compat.windows_detach_flags(). _popen_kwargs = {{ - "stdout": subprocess.DEVNULL, - "stderr": subprocess.DEVNULL, + "stdout": _stdio_target, + "stderr": _stdio_target, }} # Anchor the respawned gateway at the stable working dir and overlay # the env (VIRTUAL_ENV / PYTHONPATH / HERMES_HOME) the windowless @@ -1378,23 +1397,45 @@ def _spawn_gateway_restart_watcher(old_pid: int, run_argv: list[str]) -> bool: # the venv python resolves imports without help. if _respawn_cwd: _popen_kwargs["cwd"] = _respawn_cwd - if _respawn_env_overlay: - _popen_kwargs["env"] = {{**os.environ, **_respawn_env_overlay}} - if sys.platform == "win32": - try: - _popen_kwargs["creationflags"] = windows_detach_flags() + _base_env = {{**os.environ, **_respawn_env_overlay}} + try: + if sys.platform == "win32": + try: + _popen_kwargs["creationflags"] = windows_detach_flags() + # Stamp the breakaway state exactly like the canonical + # gateway_windows._spawn_detached, so the respawned + # gateway's exit-diag / lifecycle records show whether it + # escaped the parent Job Object (#48820 4th repro: + # without the stamp, a job-teardown kill was + # indistinguishable from any other silent death). + _popen_kwargs["env"] = {{ + **_base_env, _WINDOWS_GATEWAY_BREAKAWAY_ENV: "1", + }} + subprocess.Popen(cmd, **_popen_kwargs) + except OSError: + # CREATE_BREAKAWAY_FROM_JOB can be rejected with + # ERROR_ACCESS_DENIED when the parent's job object refuses + # breakaway. Retry without it — DETACHED_PROCESS et al. + # alone are enough in most setups. Mirrors the canonical + # fallback in gateway_windows._spawn_detached. + _popen_kwargs["creationflags"] = ( + windows_detach_flags_without_breakaway() + ) + _popen_kwargs["env"] = {{ + **_base_env, _WINDOWS_GATEWAY_BREAKAWAY_ENV: "0", + }} + subprocess.Popen(cmd, **_popen_kwargs) + else: + if _respawn_env_overlay: + _popen_kwargs["env"] = _base_env + _popen_kwargs["start_new_session"] = True subprocess.Popen(cmd, **_popen_kwargs) - except OSError: - # CREATE_BREAKAWAY_FROM_JOB can be rejected with - # ERROR_ACCESS_DENIED when the parent's job object refuses - # breakaway. Retry without it — DETACHED_PROCESS et al. - # alone are enough in most setups. Mirrors the canonical - # fallback in gateway_windows._spawn_detached. - _popen_kwargs["creationflags"] = windows_detach_flags_without_breakaway() - subprocess.Popen(cmd, **_popen_kwargs) - else: - _popen_kwargs["start_new_session"] = True - subprocess.Popen(cmd, **_popen_kwargs) + finally: + if _stdio_fh is not None: + try: + _stdio_fh.close() + except OSError: + pass """ ).strip().format( respawn_cwd_literal=respawn_cwd_literal, diff --git a/hermes_cli/gateway_windows.py b/hermes_cli/gateway_windows.py index b2ddf9fea6..3f86247613 100644 --- a/hermes_cli/gateway_windows.py +++ b/hermes_cli/gateway_windows.py @@ -1187,7 +1187,8 @@ def install( def _confirm_gateway_stable( - initial_pids: list[int], confirm_s: float, interval_s: float + initial_pids: list[int], confirm_s: float, interval_s: float, + all_profiles: bool = False, ) -> list[int]: """Re-check a freshly detected gateway for ``confirm_s`` seconds. @@ -1206,7 +1207,7 @@ def _confirm_gateway_stable( confirm_deadline = time.monotonic() + confirm_s while time.monotonic() < confirm_deadline: time.sleep(interval_s) - pids = list(find_gateway_pids()) + pids = list(find_gateway_pids(all_profiles=all_profiles)) if not pids: return [] return pids @@ -1216,6 +1217,7 @@ def _wait_for_gateway_ready( timeout_s: float = 6.0, interval_s: float = 0.4, confirm_s: float = 2.0, + all_profiles: bool = False, ) -> list[int]: """Poll for a live gateway process for up to ``timeout_s`` seconds. @@ -1225,6 +1227,10 @@ def _wait_for_gateway_ready( after spawn must not earn a ✓, #91675). If it vanishes during the confirmation window, polling resumes until the deadline. + ``all_profiles`` widens the scan across every profile's gateway — the + post-update resume path relaunches the whole fleet, not just the active + profile. + Returns the list of PIDs found. Empty list means nothing (stable) came up in time — the caller should surface that to the user as a failed start. @@ -1233,9 +1239,11 @@ def _wait_for_gateway_ready( deadline = time.monotonic() + timeout_s while time.monotonic() < deadline: - pids = list(find_gateway_pids()) + pids = list(find_gateway_pids(all_profiles=all_profiles)) if pids: - confirmed = _confirm_gateway_stable(pids, confirm_s, interval_s) + confirmed = _confirm_gateway_stable( + pids, confirm_s, interval_s, all_profiles=all_profiles + ) if confirmed: return confirmed continue # died during confirmation — keep polling until deadline diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index f0a825b32b..b185b07bcd 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -7451,6 +7451,52 @@ def _resume_windows_gateways_after_update(token: dict | None) -> None: token["unmapped"] = failed_unmapped if failed_profiles or failed_unmapped: raise RuntimeError("Could not restart every paused Windows gateway") + + # A truthy return from the launch helpers only proves the detached + # watcher process was created — not that the gateway it respawns + # survived. A parent Job Object that denies CREATE_BREAKAWAY_FROM_JOB + # kills the freshly respawned gateway on updater teardown before it + # writes a single log line, yet "✓ Restarting" was printed anyway + # (#48820, 3rd/4th repro). Verify a stable gateway process actually + # exists before vouching for the resume, using the same + # provisional-hit + confirmation-window poll every other spawn path + # uses (#91675). all_profiles=True because the resume covers the fleet. + if relaunched or unmapped_relaunched: + try: + from hermes_cli import gateway_windows + except Exception as exc: + raise RuntimeError( + f"Could not load Windows gateway liveness helpers: {exc}" + ) from exc + ready_pids = gateway_windows._wait_for_gateway_ready( + timeout_s=30.0, all_profiles=True + ) + if not ready_pids: + token["profiles"] = dict(profiles) + token["unmapped"] = list(unmapped) + print() + print( + " ⚠ Windows gateway restart could not be verified — no stable " + "gateway process appeared after relaunch." + ) + print( + " (The respawned gateway may have been killed by a parent " + "Job Object during updater teardown, #48820.)" + ) + print(" Recover with: hermes gateway restart") + raise RuntimeError( + "Windows gateway relaunch after update was not verified alive" + ) + # Persist the PIDs this ✓ vouches for so a death AFTER the updater + # exits (parent Job Object teardown, #91675) is reported by the next + # CLI invocation instead of staying silent. Best-effort. + try: + gateway_windows._write_start_attestation( + ready_pids, "post-update relaunch" + ) + except Exception: + pass + token["resume_needed"] = False if relaunched: diff --git a/tests/hermes_cli/test_gateway_job_teardown_live.py b/tests/hermes_cli/test_gateway_job_teardown_live.py new file mode 100644 index 0000000000..e6245c10c6 --- /dev/null +++ b/tests/hermes_cli/test_gateway_job_teardown_live.py @@ -0,0 +1,335 @@ +"""LIVE Windows E2E for #48820 (4th repro): Job-Object teardown vs the +gateway restart watcher, with real processes on a real windows-latest runner. + +Three live proofs (no mocks of the code under test): + +1. ``TestJobObjectMechanismLive`` — the mechanism everything rests on: + a child spawned with ``windows_detach_flags()`` (CREATE_BREAKAWAY_FROM_JOB) + from inside a kill-on-close Job Object SURVIVES the job teardown, while a + child spawned with ``windows_detach_flags_without_breakaway()`` is killed + by it. This is exactly the reporter's suspected kill path. + +2. ``TestWatcherRespawnLive`` — drives the REAL + ``hermes_cli.gateway._spawn_gateway_restart_watcher`` end to end with a + real stub gateway process, against a temp HERMES_HOME: + - the respawned process's stderr must land in ``logs/gateway-stdio.log`` + (on unfixed main it went to DEVNULL: a job-teardown kill left ZERO trace); + - the respawn env must carry ``_HERMES_GATEWAY_BREAKAWAY=1`` (the stamp + that makes a later job-teardown death diagnosable in exit-diag). + +3. ``TestResumeVerificationLive`` — the user-visible symptom: the updater's + ``_resume_windows_gateways_after_update`` must NOT print + "✓ Restarting Windows gateway profile(s)" when the relaunched gateway is + dead. The relaunch chain runs for real; the "gateway" is a stub that exits + immediately (standing in for the job-teardown kill). On unfixed main the ✓ + is printed anyway; after the fix the resume raises "not verified alive". +""" + +from __future__ import annotations + +import ctypes +import os +import subprocess +import sys +import time +from ctypes import wintypes +from pathlib import Path + +import pytest + +pytestmark = [ + pytest.mark.windows_only, + pytest.mark.skipif(sys.platform != "win32", reason="native Windows only"), +] + +_REPO_ROOT = Path(__file__).resolve().parents[2] + +JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x00002000 +JOB_OBJECT_LIMIT_BREAKAWAY_OK = 0x00000800 +JobObjectExtendedLimitInformation = 9 +PROCESS_ALL_ACCESS = 0x001FFFFF + + +class IO_COUNTERS(ctypes.Structure): + _fields_ = [ + ("ReadOperationCount", ctypes.c_ulonglong), + ("WriteOperationCount", ctypes.c_ulonglong), + ("OtherOperationCount", ctypes.c_ulonglong), + ("ReadTransferCount", ctypes.c_ulonglong), + ("WriteTransferCount", ctypes.c_ulonglong), + ("OtherTransferCount", ctypes.c_ulonglong), + ] + + +class JOBOBJECT_BASIC_LIMIT_INFORMATION(ctypes.Structure): + _fields_ = [ + ("PerProcessUserTimeLimit", ctypes.c_longlong), + ("PerJobUserTimeLimit", ctypes.c_longlong), + ("LimitFlags", wintypes.DWORD), + ("MinimumWorkingSetSize", ctypes.c_size_t), + ("MaximumWorkingSetSize", ctypes.c_size_t), + ("ActiveProcessLimit", wintypes.DWORD), + ("Affinity", ctypes.c_size_t), + ("PriorityClass", wintypes.DWORD), + ("SchedulingClass", wintypes.DWORD), + ] + + +class JOBOBJECT_EXTENDED_LIMIT_INFORMATION(ctypes.Structure): + _fields_ = [ + ("BasicLimitInformation", JOBOBJECT_BASIC_LIMIT_INFORMATION), + ("IoInfo", IO_COUNTERS), + ("ProcessMemoryLimit", ctypes.c_size_t), + ("JobMemoryLimit", ctypes.c_size_t), + ("PeakProcessMemoryUsed", ctypes.c_size_t), + ("PeakJobMemoryUsed", ctypes.c_size_t), + ] + + +def _make_kill_on_close_job(allow_breakaway: bool) -> int: + kernel32 = ctypes.windll.kernel32 + job = kernel32.CreateJobObjectW(None, None) + assert job, "CreateJobObjectW failed" + info = JOBOBJECT_EXTENDED_LIMIT_INFORMATION() + flags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE + if allow_breakaway: + flags |= JOB_OBJECT_LIMIT_BREAKAWAY_OK + info.BasicLimitInformation.LimitFlags = flags + ok = kernel32.SetInformationJobObject( + job, + JobObjectExtendedLimitInformation, + ctypes.byref(info), + ctypes.sizeof(info), + ) + assert ok, "SetInformationJobObject failed" + return job + + +def _assign_to_job(job: int, proc: subprocess.Popen) -> None: + kernel32 = ctypes.windll.kernel32 + ok = kernel32.AssignProcessToJobObject(job, int(proc._handle)) + assert ok, f"AssignProcessToJobObject failed (winerror={ctypes.GetLastError()})" + + +def _pid_alive(pid: int) -> bool: + kernel32 = ctypes.windll.kernel32 + PROCESS_QUERY_LIMITED_INFORMATION = 0x1000 + h = kernel32.OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, False, pid) + if not h: + return False + try: + code = wintypes.DWORD() + kernel32.GetExitCodeProcess(h, ctypes.byref(code)) + return code.value == 259 # STILL_ACTIVE + finally: + kernel32.CloseHandle(h) + + +_SLEEPER = "import time; time.sleep(120)" + + +def _wait_for(predicate, timeout_s: float = 30.0, interval_s: float = 0.25): + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval_s) + return False + + +class TestJobObjectMechanismLive: + """Real Job Objects, real children — the #48820 kill mechanism.""" + + def _driver_source(self, flags_helper: str, pid_file: str) -> str: + # The driver runs INSIDE the job and spawns a grandchild "gateway" + # with the flag bundle under test, then exits — mirroring the + # updater/watcher exiting while its job tears down. + return ( + "import subprocess, sys, pathlib\n" + "sys.path.insert(0, r'%s')\n" + "from hermes_cli._subprocess_compat import (\n" + " windows_detach_flags, windows_detach_flags_without_breakaway)\n" + "flags = %s()\n" + "p = subprocess.Popen([sys.executable, '-c', %r],\n" + " creationflags=flags, stdin=subprocess.DEVNULL,\n" + " stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)\n" + "pathlib.Path(r'%s').write_text(str(p.pid), encoding='utf-8')\n" + ) % (str(_REPO_ROOT), flags_helper, _SLEEPER, pid_file) + + def _run_in_job(self, tmp_path: Path, flags_helper: str) -> int: + pid_file = tmp_path / f"{flags_helper}.pid" + job = _make_kill_on_close_job(allow_breakaway=True) + kernel32 = ctypes.windll.kernel32 + try: + driver = subprocess.Popen( + [ + sys.executable, + "-c", + # Handshake: wait until the test has assigned us to the + # job before spawning the grandchild. + "import pathlib, sys, time\n" + f"go = pathlib.Path(r'{tmp_path / 'go.marker'}')\n" + "deadline = time.monotonic() + 30\n" + "while not go.exists():\n" + " assert time.monotonic() < deadline, 'no go marker'\n" + " time.sleep(0.1)\n" + + self._driver_source(flags_helper, str(pid_file)), + ], + cwd=str(_REPO_ROOT), + ) + _assign_to_job(job, driver) + (tmp_path / "go.marker").write_text("go", encoding="utf-8") + assert _wait_for(pid_file.exists), "driver never wrote the pid file" + gw_pid = int(pid_file.read_text(encoding="utf-8")) + assert _wait_for(lambda: driver.poll() is not None), ( + "driver did not exit" + ) + assert _pid_alive(gw_pid), "grandchild died before job teardown" + # THE teardown: closing the last job handle fires + # KILL_ON_JOB_CLOSE against every process still in the job. + kernel32.CloseHandle(job) + job = None + time.sleep(2.0) + return gw_pid + finally: + (tmp_path / "go.marker").unlink(missing_ok=True) + if job: + kernel32.CloseHandle(job) + + def test_breakaway_child_survives_job_teardown(self, tmp_path): + pid = self._run_in_job(tmp_path, "windows_detach_flags") + try: + assert _pid_alive(pid), ( + "CREATE_BREAKAWAY_FROM_JOB child must survive the parent " + "job's kill-on-close teardown" + ) + finally: + subprocess.run( + ["taskkill", "/PID", str(pid), "/T", "/F"], capture_output=True + ) + + def test_non_breakaway_child_killed_by_job_teardown(self, tmp_path): + """The #48820 kill path, reproduced live: no breakaway → the job's + teardown reaps the freshly spawned gateway.""" + pid = self._run_in_job(tmp_path, "windows_detach_flags_without_breakaway") + try: + assert not _pid_alive(pid), ( + "child without breakaway must be killed by kill-on-close " + "job teardown — this is the silent gateway death of #48820" + ) + finally: + subprocess.run( + ["taskkill", "/PID", str(pid), "/T", "/F"], capture_output=True + ) + + +class TestWatcherRespawnLive: + """Drive the real ``_spawn_gateway_restart_watcher`` with real processes.""" + + def _run_watcher_cycle(self, tmp_path: Path, monkeypatch) -> tuple[Path, Path]: + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + (tmp_path / "home").mkdir(parents=True, exist_ok=True) + + marker = tmp_path / "respawned.marker" + # The stub "gateway": records its breakaway stamp env, screams on + # stderr (so the stdio sidecar has something to capture), then exits. + stub = ( + "import os, pathlib, sys\n" + f"pathlib.Path(r'{marker}').write_text(\n" + " os.environ.get('_HERMES_GATEWAY_BREAKAWAY', 'MISSING'),\n" + " encoding='utf-8')\n" + "print('stub-gateway-stderr-trace', file=sys.stderr)\n" + ) + + # A real old-pid that exits immediately — the watcher's poll loop + # sees it die and respawns. + old = subprocess.Popen([sys.executable, "-c", "pass"]) + old.wait(timeout=30) + + import hermes_cli.gateway as gateway + + assert gateway._spawn_gateway_restart_watcher( + old.pid, [sys.executable, "-c", stub] + ), "watcher spawn returned False" + + assert _wait_for(marker.exists, timeout_s=60), ( + "watcher never respawned the stub gateway" + ) + stdio_log = tmp_path / "home" / "logs" / "gateway-stdio.log" + return marker, stdio_log + + def test_respawn_stamps_breakaway_and_leaves_stdio_trace( + self, tmp_path, monkeypatch + ): + marker, stdio_log = self._run_watcher_cycle(tmp_path, monkeypatch) + + # (a) Breakaway stamp: on unfixed main the respawn env carried no + # stamp, so a job-teardown death was undiagnosable. + stamp = marker.read_text(encoding="utf-8").strip() + assert stamp in {"1", "0"}, ( + f"respawned gateway must carry the breakaway stamp, got {stamp!r}" + ) + + # (b) Stdio trace: on unfixed main stderr went to DEVNULL — a dying + # gateway left zero trace (#48820 4th repro). + assert _wait_for( + lambda: stdio_log.exists() + and "stub-gateway-stderr-trace" + in stdio_log.read_text(encoding="utf-8", errors="replace"), + timeout_s=30, + ), "respawned gateway stderr must land in logs/gateway-stdio.log" + + +class TestResumeVerificationLive: + """The user-visible lie: '✓ Restarting' printed for a dead gateway.""" + + def test_dead_relaunch_is_not_reported_as_success(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + (tmp_path / "home").mkdir(parents=True, exist_ok=True) + + import hermes_cli.gateway as gateway + import hermes_cli.main as hm + from hermes_cli.update_cmd import _resume_windows_gateways_after_update + + # Peripheral only: don't regenerate launcher scripts into the temp home. + monkeypatch.setattr(hm, "_refresh_windows_gateway_launchers", lambda: None) + + # Real relaunch chain, real watcher, real spawn — but the respawned + # "gateway" exits immediately, standing in for the Job-Object + # teardown kill. It never registers in the process table as a + # gateway, exactly like the dead pid 48452 / 50456 of #48820. + def _relaunch(profile, old_pid): + dead = subprocess.Popen([sys.executable, "-c", "pass"]) + dead.wait(timeout=30) + return gateway._spawn_gateway_restart_watcher( + dead.pid, [sys.executable, "-c", "pass"] + ) + + monkeypatch.setattr( + gateway, "launch_detached_profile_gateway_restart", _relaunch + ) + + token = { + "resume_needed": True, + "profiles": {"default": 999999}, + "unmapped_pids": [], + "unmapped": [], + } + + printed: list = [] + real_print = print + monkeypatch.setattr( + "builtins.print", lambda *a, **k: printed.append(" ".join(map(str, a))) + ) + try: + with pytest.raises(RuntimeError, match="not verified alive"): + _resume_windows_gateways_after_update(token) + finally: + monkeypatch.setattr("builtins.print", real_print) + + text = "\n".join(printed) + assert "✓ Restarting" not in text, ( + "the updater must not vouch for a gateway that is not alive " + f"(#48820). Printed:\n{text}" + ) + assert "could not be verified" in text diff --git a/tests/hermes_cli/test_windows_gateway_job_teardown_48820.py b/tests/hermes_cli/test_windows_gateway_job_teardown_48820.py new file mode 100644 index 0000000000..285f0b1b6c --- /dev/null +++ b/tests/hermes_cli/test_windows_gateway_job_teardown_48820.py @@ -0,0 +1,189 @@ +"""Regression tests for #48820 (4th repro): job-object teardown killed the +post-update respawned gateway silently, and the updater printed +"✓ Restarting Windows gateway profile(s)" anyway. + +Two fixes under test: + +1. ``_spawn_gateway_restart_watcher``'s inlined watcher source must + (a) route the respawned gateway's stray stdout/stderr to + ``logs/gateway-stdio.log`` (it was ``DEVNULL`` — a gateway killed by + parent Job Object teardown left ZERO trace anywhere), and + (b) stamp ``_HERMES_GATEWAY_BREAKAWAY`` =1/0 on the respawn env exactly + like the canonical ``gateway_windows._spawn_detached``, so the + lifecycle/exit-diag records show whether the gateway escaped the + parent's Job Object. + +2. ``_resume_windows_gateways_after_update`` must verify a stable gateway + process actually exists (via ``gateway_windows._wait_for_gateway_ready``) + before printing the ✓ — a truthy launch return only proves the watcher + process was created, not that the respawned gateway survived the + updater's Job Object teardown. +""" + +from unittest.mock import patch + +import pytest + +import hermes_cli.gateway as gateway +import hermes_cli.gateway_windows as gateway_windows +import hermes_cli.main as hm +from hermes_cli._subprocess_compat import _WINDOWS_GATEWAY_BREAKAWAY_ENV +from hermes_cli.update_cmd import _resume_windows_gateways_after_update + + +# --------------------------------------------------------------------------- +# 1. Watcher template contract +# --------------------------------------------------------------------------- + + +def _captured_watcher_source(monkeypatch) -> str: + """Spawn the watcher with a mocked Popen and return the inlined -c source.""" + captured = {} + + def fake_popen(argv, **kwargs): + captured["argv"] = argv + captured["kwargs"] = kwargs + + class _P: + pid = 12345 + + return _P() + + monkeypatch.setattr(gateway.subprocess, "Popen", fake_popen) + assert gateway._spawn_gateway_restart_watcher( + 999999, ["python", "-m", "hermes_cli.main", "gateway", "run"] + ) + argv = captured["argv"] + assert argv[1] == "-c" + return argv[2] + + +class TestWatcherRespawnTemplate: + def test_respawn_stdio_routed_to_sidecar_log_not_devnull(self, monkeypatch): + """DEVNULL swallowed the dying gateway's last words (#48820 4th + repro: 'Zero trace anywhere ... because the watcher respawns with + stdout=DEVNULL, stderr=DEVNULL').""" + src = _captured_watcher_source(monkeypatch) + assert "gateway-stdio.log" in src, ( + "watcher respawn must route stray stdout/stderr to the same " + "sidecar log _spawn_detached uses, so a gateway killed moments " + "after respawn leaves a trace" + ) + # DEVNULL remains only as the fallback when the log dir is + # unavailable — the popen kwargs must not be hardwired to it. + assert '"stdout": _stdio_target' in src + assert '"stderr": _stdio_target' in src + + def test_respawn_stamps_breakaway_state_like_spawn_detached( + self, monkeypatch + ): + """The respawned gateway must carry _HERMES_GATEWAY_BREAKAWAY=1 on + the primary (breakaway) spawn and =0 on the no-breakaway fallback, + mirroring gateway_windows._spawn_detached — without the stamp, a + job-teardown kill is indistinguishable from any other silent death + in the exit diagnostics.""" + src = _captured_watcher_source(monkeypatch) + assert "_WINDOWS_GATEWAY_BREAKAWAY_ENV" in src + assert _WINDOWS_GATEWAY_BREAKAWAY_ENV == "_HERMES_GATEWAY_BREAKAWAY" + # Primary stamps "1", the OSError fallback stamps "0". + assert '_WINDOWS_GATEWAY_BREAKAWAY_ENV: "1"' in src + assert '_WINDOWS_GATEWAY_BREAKAWAY_ENV: "0"' in src + + def test_respawn_source_compiles(self, monkeypatch): + """The inlined -c template is built via str.format over a + dedented literal — guard against brace/indentation regressions.""" + src = _captured_watcher_source(monkeypatch) + compile(src, "", "exec") + + def test_watcher_fallback_retry_preserved(self, monkeypatch): + """The ERROR_ACCESS_DENIED retry without breakaway must survive.""" + src = _captured_watcher_source(monkeypatch) + assert "windows_detach_flags_without_breakaway" in src + + +# --------------------------------------------------------------------------- +# 2. Post-update resume liveness gate +# --------------------------------------------------------------------------- + + +def _token(profiles: dict) -> dict: + return { + "resume_needed": True, + "profiles": profiles, + "unmapped_pids": [], + "unmapped": [], + } + + +class TestResumeLivenessGate: + @pytest.fixture(autouse=True) + def _windows(self, monkeypatch): + monkeypatch.setattr(hm, "_is_windows", lambda: True) + monkeypatch.setattr(hm, "_refresh_windows_gateway_launchers", lambda: None) + monkeypatch.setattr( + gateway, "launch_detached_profile_gateway_restart", lambda *_a: True + ) + monkeypatch.setattr( + gateway, "launch_detached_gateway_restart_by_cmdline", lambda *_a: True + ) + + def test_dead_respawn_fails_the_resume_instead_of_printing_check( + self, monkeypatch + ): + """No stable gateway after the relaunch → the resume raises (update + marked incomplete) instead of printing '✓ Restarting'. This is the + exact #48820 3rd/4th-repro hole: spawn succeeded, gateway died + within seconds, success was reported, platforms were offline for + 12.5 hours.""" + monkeypatch.setattr( + gateway_windows, "_wait_for_gateway_ready", lambda **_kw: [] + ) + token = _token({"default": 1111}) + printed = [] + with patch("builtins.print", side_effect=lambda *a, **k: printed.append(a)): + with pytest.raises(RuntimeError, match="not verified alive"): + _resume_windows_gateways_after_update(token) + + text = " ".join(str(a) for a in printed) + assert "✓ Restarting" not in text + assert "could not be verified" in text + # The profile stays on the token so retry/reporting still sees it. + assert token["profiles"] == {"default": 1111} + assert token["resume_needed"] is True + + def test_live_respawn_prints_check_and_writes_attestation(self, monkeypatch): + monkeypatch.setattr( + gateway_windows, "_wait_for_gateway_ready", lambda **_kw: [777] + ) + attested = {} + monkeypatch.setattr( + gateway_windows, + "_write_start_attestation", + lambda pids, via: attested.update(pids=pids, via=via), + ) + token = _token({"default": 1111}) + printed = [] + with patch("builtins.print", side_effect=lambda *a, **k: printed.append(a)): + _resume_windows_gateways_after_update(token) + + text = " ".join(str(a) for a in printed) + assert "✓ Restarting" in text + assert attested == {"pids": [777], "via": "post-update relaunch"} + assert token["resume_needed"] is False + + def test_liveness_poll_scans_all_profiles(self, monkeypatch): + """The resume relaunches the whole fleet; the verification must not + be scoped to the active profile.""" + seen = {} + + def fake_wait(**kwargs): + seen.update(kwargs) + return [777] + + monkeypatch.setattr(gateway_windows, "_wait_for_gateway_ready", fake_wait) + monkeypatch.setattr( + gateway_windows, "_write_start_attestation", lambda *_a, **_kw: None + ) + with patch("builtins.print"): + _resume_windows_gateways_after_update(_token({"work": 2222})) + assert seen.get("all_profiles") is True diff --git a/tests/hermes_cli/test_windows_update_restart_reconciliation.py b/tests/hermes_cli/test_windows_update_restart_reconciliation.py index 0de2d6bbd9..4b3279ab56 100644 --- a/tests/hermes_cli/test_windows_update_restart_reconciliation.py +++ b/tests/hermes_cli/test_windows_update_restart_reconciliation.py @@ -24,6 +24,7 @@ from unittest.mock import patch import pytest import hermes_cli.gateway as gateway +import hermes_cli.gateway_windows as gateway_windows import hermes_cli.main as hm from hermes_cli.update_cmd import _resume_windows_gateways_after_update from hermes_cli.update_inventory import ( @@ -43,6 +44,21 @@ def _token(profiles: dict) -> dict: } +@pytest.fixture(autouse=True) +def _stub_post_relaunch_liveness(monkeypatch): + """The resume path now verifies a stable gateway process actually exists + before vouching for the relaunch (#48820 3rd/4th repro — a parent Job + Object killing the respawned gateway made '✓ Restarting' a lie). These + reconciliation tests exercise the token bookkeeping, not the liveness + poll, so stub it as 'gateway came up'.""" + monkeypatch.setattr( + gateway_windows, "_wait_for_gateway_ready", lambda **_kw: [4242] + ) + monkeypatch.setattr( + gateway_windows, "_write_start_attestation", lambda *_a, **_kw: None + ) + + def test_resume_records_successfully_relaunched_profiles_on_the_token(monkeypatch): monkeypatch.setattr(hm, "_is_windows", lambda: True) monkeypatch.setattr(hm, "_refresh_windows_gateway_launchers", lambda: None)