fix(update): keep the Windows cold-start plan for a dead attested gateway
A Desktop self-update hand-off exits the app before the updater runs and can kill the messaging gateway in those same seconds (#109538), so the updater's discovery finds no live PID while the one-shot start attestation still vouches for the dead one. Both Desktop-ownership checks then read "nothing running" as "nothing to restore" and the bot stayed down until a manual start. Consult the attestation non-destructively before Desktop-owned lifecycle suppresses a cold-start: a vouched-for PID gone without a clean ledger exit keeps the plan and is restored; no attested death preserves the #76129 skip unchanged.
This commit is contained in:
@@ -889,6 +889,21 @@ def _clear_start_attestation() -> None:
|
||||
pass
|
||||
|
||||
|
||||
def _read_start_attestation() -> object | None:
|
||||
"""Parsed attestation payload (any JSON type), or ``None`` when absent/unreadable. Never raises."""
|
||||
try:
|
||||
return json.loads(_start_attestation_path().read_text(encoding="utf-8"))
|
||||
except (OSError, ValueError):
|
||||
return None
|
||||
|
||||
|
||||
def _attested_pids_from(data: object) -> list[int]:
|
||||
"""PID list from an attestation payload; empty for anything malformed."""
|
||||
if not isinstance(data, dict):
|
||||
return []
|
||||
return [p for p in data.get("pids", []) if isinstance(p, int)]
|
||||
|
||||
|
||||
def _attested_pid_exited_cleanly(pid: int) -> bool:
|
||||
"""True when the lifecycle ledger shows a clean exit for ``pid``."""
|
||||
try:
|
||||
@@ -900,14 +915,34 @@ def _attested_pid_exited_cleanly(pid: int) -> bool:
|
||||
return isinstance(data, dict) and data.get("phase") == "exited" and data.get("pid") == pid
|
||||
|
||||
|
||||
def attested_gateway_died(current_pids: list[int] | None = None) -> bool:
|
||||
"""True when the start attestation vouches for gateway PID(s) that are gone without a clean exit.
|
||||
|
||||
Read-only twin of :func:`check_start_attestation` for callers that must not consume the
|
||||
one-shot marker — ``hermes update`` consults it to decide whether a Desktop-owned install
|
||||
still owes a gateway cold-start (#109538). ``False`` for anything undecidable (no marker, a
|
||||
gateway running now, a clean ledger exit, or discovery failure): "unknown" must never read
|
||||
as "dead"."""
|
||||
attested = _attested_pids_from(_read_start_attestation())
|
||||
if not attested:
|
||||
return False
|
||||
if current_pids is None:
|
||||
try:
|
||||
from hermes_cli.gateway import find_gateway_pids
|
||||
|
||||
current_pids = list(find_gateway_pids())
|
||||
except Exception:
|
||||
return False
|
||||
return not current_pids and not any(_attested_pid_exited_cleanly(pid) for pid in attested)
|
||||
|
||||
|
||||
def check_start_attestation(current_pids: list[int] | None = None) -> str | None:
|
||||
"""Surface (once) a gateway that died after a ✓ was printed for it. Never raises. Gateway running
|
||||
or a clean-exit ledger record: clear silently; otherwise return a warning and consume the marker."""
|
||||
try:
|
||||
data = json.loads(_start_attestation_path().read_text(encoding="utf-8"))
|
||||
except (OSError, ValueError):
|
||||
data = _read_start_attestation()
|
||||
if data is None:
|
||||
return None
|
||||
attested = [p for p in data.get("pids", []) if isinstance(p, int)] if isinstance(data, dict) else []
|
||||
attested = _attested_pids_from(data)
|
||||
if not attested:
|
||||
_clear_start_attestation()
|
||||
return None
|
||||
@@ -923,7 +958,10 @@ def check_start_attestation(current_pids: list[int] | None = None) -> str | None
|
||||
_clear_start_attestation()
|
||||
if current_pids or any(_attested_pid_exited_cleanly(pid) for pid in attested):
|
||||
return None
|
||||
return _format_attestation_warning(attested, data)
|
||||
|
||||
|
||||
def _format_attestation_warning(attested: list[int], data: dict) -> str:
|
||||
via = data.get("via") or "direct spawn"
|
||||
ts = data.get("ts") or "unknown time"
|
||||
lines = [
|
||||
|
||||
@@ -746,16 +746,22 @@ def _windows_cold_start_plan() -> dict | None:
|
||||
An installed autostart entry is an explicit "I want a gateway" signal; a gateway that died between
|
||||
updates would otherwise stay down until next login (resume only relaunches what was running).
|
||||
Desktop-owned lifecycle -> ``None`` (spawning ``gateway run`` beside Desktop races ports/state);
|
||||
the skip is ownership, not liveness."""
|
||||
the skip is ownership, not liveness — except when the start attestation reports a vouched-for
|
||||
gateway that died without a clean exit. The Desktop hand-off exits the app before the updater starts
|
||||
and can kill the running gateway in those same seconds, so discovery finds no live PID to pause
|
||||
(#109538) — the dead attestation is the only surviving "a gateway was up" evidence, and the Desktop
|
||||
does not restart the messaging gateway itself. Keep the plan; the spawn-time re-check in
|
||||
``_cold_start_windows_gateway_after_update`` stays the ownership authority."""
|
||||
from hermes_cli.update_cmd import _desktop_owns_gateway_lifecycle
|
||||
with _best_effort('Could not check Desktop gateway-lifecycle ownership before update: %s'):
|
||||
if _desktop_owns_gateway_lifecycle():
|
||||
logger.debug("Skipping Windows gateway cold-start plan: Desktop owns gateway lifecycle")
|
||||
return None
|
||||
from hermes_cli import gateway_windows
|
||||
with _best_effort('Could not check Windows gateway autostart state before update: %s'):
|
||||
from hermes_cli import gateway_windows
|
||||
if gateway_windows.is_installed():
|
||||
return {"resume_needed": True, "profiles": {}, "unmapped_pids": [], "unmapped": [], "cold_start_if_installed": True}
|
||||
if not gateway_windows.is_installed():
|
||||
return None
|
||||
with _best_effort('Could not check Desktop gateway-lifecycle ownership before update: %s'):
|
||||
if _desktop_owns_gateway_lifecycle() and not gateway_windows.attested_gateway_died():
|
||||
logger.debug("Skipping Windows gateway cold-start plan: Desktop owns gateway lifecycle")
|
||||
return None
|
||||
return {"resume_needed": True, "profiles": {}, "unmapped_pids": [], "unmapped": [], "cold_start_if_installed": True}
|
||||
return None
|
||||
|
||||
|
||||
@@ -921,6 +927,10 @@ def _cold_start_windows_gateway_after_update() -> bool:
|
||||
same post-spawn liveness poll every other ``_spawn_detached`` caller uses
|
||||
(``gateway_windows._report_gateway_start``), instead of being printed unconditionally from the returned
|
||||
PID.
|
||||
|
||||
Desktop-owned lifecycle suppresses the spawn only while nothing attests a gateway is expected: an
|
||||
attested gateway that died without a clean exit is restored even then (#109538) — the Desktop does
|
||||
not restart the messaging gateway itself.
|
||||
"""
|
||||
from hermes_cli.update_cmd import _desktop_owns_gateway_lifecycle, _m
|
||||
if not _m()._is_windows():
|
||||
@@ -932,7 +942,7 @@ def _cold_start_windows_gateway_after_update() -> bool:
|
||||
if list(find_gateway_pids(all_profiles=True)):
|
||||
return True
|
||||
with _abort_on_error("Could not re-check Desktop gateway-lifecycle ownership before cold-start"):
|
||||
if _desktop_owns_gateway_lifecycle():
|
||||
if _desktop_owns_gateway_lifecycle() and not gateway_windows.attested_gateway_died():
|
||||
logger.debug("Skipping Windows gateway cold-start: Desktop owns gateway lifecycle")
|
||||
return True
|
||||
with _abort_on_error("Could not cold-start Windows gateway after update"):
|
||||
|
||||
@@ -189,3 +189,36 @@ def test_breakaway_fallback_warns_even_on_success(monkeypatch, attest_home, caps
|
||||
assert "✓" in out
|
||||
assert "could not break away" in out
|
||||
assert "schtasks /Run /TN Hermes_Gateway" in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #109538: read-only death probe for the update path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_attested_probe_is_read_only_and_never_reads_unknown_as_dead(attest_home):
|
||||
"""The update path needs the death verdict without consuming the one-shot marker the
|
||||
next CLI start still owes the user; and only a *detectable* death may read True.
|
||||
|
||||
``hermes update`` consults this probe to keep the cold-start plan when Desktop owns
|
||||
the lifecycle (#109538) — consuming the marker here would silence the CLI-start
|
||||
warning that reports the same death to the user.
|
||||
"""
|
||||
assert gateway_windows.attested_gateway_died(current_pids=[]) is False # no marker yet
|
||||
|
||||
gateway_windows._write_start_attestation([555], "cold-start after update")
|
||||
|
||||
assert gateway_windows.attested_gateway_died(current_pids=[555]) is False # alive
|
||||
assert gateway_windows.attested_gateway_died(current_pids=[]) is True # dead, unclean
|
||||
marker = attest_home / "state" / "gateway.start-attestation.json"
|
||||
assert marker.exists() # unconsumed — the CLI start below still reports it
|
||||
assert gateway_windows.check_start_attestation(current_pids=[]) is not None
|
||||
|
||||
state = attest_home / "state"
|
||||
state.mkdir(exist_ok=True)
|
||||
(state / "gateway.lifecycle.json").write_text(
|
||||
json.dumps({"phase": "exited", "pid": 556, "exit_reason": "graceful_shutdown"}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
gateway_windows._write_start_attestation([556], "cold-start after update")
|
||||
assert gateway_windows.attested_gateway_died(current_pids=[]) is False # planned stop
|
||||
|
||||
@@ -7,6 +7,13 @@ control plane, the updater must not spawn a competing messaging daemon.
|
||||
Serve/dashboard are the control plane, not the messaging gateway (#92091).
|
||||
``looks_like_gateway_command_line`` stays strict; ownership is a separate
|
||||
predicate.
|
||||
|
||||
#109538: ownership alone must not hide a gateway that *died*. The Desktop
|
||||
hand-off exits the app before the updater starts and can kill the running
|
||||
gateway in those same seconds, so discovery finds no live PID while a start
|
||||
attestation still vouches for the dead one. In that case the cold-start
|
||||
survives both the plan-time and the spawn-time ownership check — the Desktop
|
||||
does not restart the messaging gateway itself.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -94,6 +101,7 @@ def test_pause_skips_cold_start_plan_when_desktop_owns_lifecycle(monkeypatch):
|
||||
hermes_gateway, "find_windows_gateway_services", lambda **_k: []
|
||||
)
|
||||
monkeypatch.setattr(gateway_windows, "is_installed", lambda: True)
|
||||
monkeypatch.setattr(gateway_windows, "attested_gateway_died", lambda: False)
|
||||
monkeypatch.setattr(update_cmd, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(update_cmd_windows, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
|
||||
@@ -122,11 +130,38 @@ def test_pause_still_cold_starts_when_autostart_and_no_desktop_owner(monkeypatch
|
||||
}
|
||||
|
||||
|
||||
def test_pause_keeps_cold_start_plan_when_desktop_owns_but_attested_gateway_died(monkeypatch):
|
||||
"""#109538: the Desktop hand-off can kill the running gateway moments before this
|
||||
discovery runs, so a dead start attestation is the surviving "a gateway was up"
|
||||
evidence. The plan must keep the cold-start instead of silently leaving the bot down."""
|
||||
monkeypatch.setattr(cli_main, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(main_install_repair, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(hermes_gateway, "find_gateway_pids", lambda **_k: [])
|
||||
monkeypatch.setattr(
|
||||
hermes_gateway, "find_windows_gateway_services", lambda **_k: []
|
||||
)
|
||||
monkeypatch.setattr(gateway_windows, "is_installed", lambda: True)
|
||||
monkeypatch.setattr(gateway_windows, "attested_gateway_died", lambda: True)
|
||||
monkeypatch.setattr(update_cmd, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(update_cmd_windows, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
|
||||
token = update_cmd._pause_windows_gateways_for_update()
|
||||
|
||||
assert token == {
|
||||
"resume_needed": True,
|
||||
"profiles": {},
|
||||
"unmapped_pids": [],
|
||||
"unmapped": [],
|
||||
"cold_start_if_installed": True,
|
||||
}
|
||||
|
||||
|
||||
def test_cold_start_aborts_when_desktop_owns_lifecycle(monkeypatch):
|
||||
spawned = []
|
||||
monkeypatch.setattr(cli_main, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(main_install_repair, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(hermes_gateway, "find_gateway_pids", lambda **_k: [])
|
||||
monkeypatch.setattr(gateway_windows, "attested_gateway_died", lambda: False)
|
||||
monkeypatch.setattr(update_cmd, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(update_cmd_windows, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(
|
||||
@@ -136,3 +171,27 @@ def test_cold_start_aborts_when_desktop_owns_lifecycle(monkeypatch):
|
||||
update_cmd._cold_start_windows_gateway_after_update()
|
||||
|
||||
assert spawned == []
|
||||
|
||||
|
||||
def test_cold_start_restores_attested_dead_gateway_despite_desktop_ownership(monkeypatch, capsys):
|
||||
"""#109538: the same dead attestation must also survive the spawn-time ownership
|
||||
re-check — nothing else brings the messaging gateway back on this install."""
|
||||
spawned = []
|
||||
monkeypatch.setattr(cli_main, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(main_install_repair, "_is_windows", lambda: True)
|
||||
monkeypatch.setattr(hermes_gateway, "find_gateway_pids", lambda **_k: [])
|
||||
monkeypatch.setattr(gateway_windows, "attested_gateway_died", lambda: True)
|
||||
monkeypatch.setattr(update_cmd, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(update_cmd_windows, "_desktop_owns_gateway_lifecycle", lambda: True)
|
||||
monkeypatch.setattr(
|
||||
gateway_windows, "_spawn_detached", lambda: spawned.append(1) or 4242
|
||||
)
|
||||
monkeypatch.setattr(gateway_windows, "_wait_for_gateway_ready", lambda *a, **k: [4242])
|
||||
monkeypatch.setattr(gateway_windows, "_write_start_attestation", lambda *a, **k: None)
|
||||
|
||||
assert update_cmd._cold_start_windows_gateway_after_update() is True
|
||||
assert spawned == [1]
|
||||
assert (
|
||||
"Gateway started via cold-start after update (PID: 4242)"
|
||||
in capsys.readouterr().out
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user