fix(gateway): survive Windows Job-Object teardown across gateway restarts (#48820)
Fourth reproduction on #48820: the updater's post-update resume respawned the gateway through _spawn_gateway_restart_watcher, the process died within seconds (parent Job Object denying CREATE_BREAKAWAY_FROM_JOB kills the child on job teardown), and "✓ Restarting Windows gateway profile(s)" was printed anyway — 12.5h of silent platform downtime, with zero trace because the watcher respawned with stdout/stderr=DEVNULL. Three surgical changes: 1. Watcher respawn stdio → logs/gateway-stdio.log (hermes_cli/gateway.py). The inlined watcher now routes the respawned gateway's stray stdout/stderr to the same sidecar log gateway_windows._spawn_detached uses (DEVNULL only as fallback), so a gateway killed moments after respawn leaves a trace. Direct implementation of the 4th repro's hardening suggestion (1). 2. Watcher respawn stamps _HERMES_GATEWAY_BREAKAWAY=1/0 exactly like the canonical _spawn_detached, so the respawned gateway's exit-diag / lifecycle records show whether it escaped the parent Job Object — a job-teardown kill is no longer indistinguishable from any other silent death. 3. Post-update resume verifies liveness before vouching (hermes_cli/update_cmd.py). _resume_windows_gateways_after_update now runs the same provisional-hit + 2s-confirmation liveness poll every other spawn path uses (gateway_windows._wait_for_gateway_ready, widened with all_profiles= for the fleet) before printing ✓, writes the #91675 start attestation for the verified PIDs, and fails the resume with a "restart could not be verified" warning + recovery hint when no stable gateway appears. Suggestion (2) of the 4th repro; closes the last silent-success hole in the family (#84185 fixed the cold-start leg, #91675 the direct-start leg; this is the relaunch leg). Live proof on windows-latest (wine2e lane): real kill-on-close Job Objects confirm breakaway children survive teardown and non-breakaway children die (the exact #48820 mechanism); the real watcher respawn cycle leaves the stdio trace + breakaway stamp; and the resume path refuses to print ✓ for a dead relaunch. Fixes the Bug-1 relaunch-trust leg of #48820.
This commit is contained in:
Teknium
+59
-18
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
@@ -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, "<watcher>", "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
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user