From 830c8f443d6ec0629924d10df18dc15ff8b2f6c3 Mon Sep 17 00:00:00 2001 From: ennheng <48635783+ennheng@users.noreply.github.com> Date: Sun, 13 Sep 2026 11:09:25 +0800 Subject: [PATCH] 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. --- hermes_cli/gateway_windows.py | 46 +++++++++++++-- hermes_cli/update_cmd_windows.py | 28 ++++++--- .../test_gateway_start_attestation.py | 33 +++++++++++ ...ws_gateway_cold_start_desktop_lifecycle.py | 59 +++++++++++++++++++ 4 files changed, 153 insertions(+), 13 deletions(-) diff --git a/hermes_cli/gateway_windows.py b/hermes_cli/gateway_windows.py index a11ab462b2..53cebed27c 100644 --- a/hermes_cli/gateway_windows.py +++ b/hermes_cli/gateway_windows.py @@ -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 = [ diff --git a/hermes_cli/update_cmd_windows.py b/hermes_cli/update_cmd_windows.py index cd3928b558..30c6eac956 100644 --- a/hermes_cli/update_cmd_windows.py +++ b/hermes_cli/update_cmd_windows.py @@ -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"): diff --git a/tests/hermes_cli/test_gateway_start_attestation.py b/tests/hermes_cli/test_gateway_start_attestation.py index 38df762d44..627394a4a8 100644 --- a/tests/hermes_cli/test_gateway_start_attestation.py +++ b/tests/hermes_cli/test_gateway_start_attestation.py @@ -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 diff --git a/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py b/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py index e1454f0fcc..ac70d4bc32 100644 --- a/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py +++ b/tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py @@ -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 + )