From da382a413ed18fa1ca78aa45ba669cc3d7e70d31 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 13 Sep 2026 20:00:27 +0530 Subject: [PATCH] test(update): trim #109538 coverage to two invariant tests Four new tests overlapped: plan-time and spawn-time attested-death overrides both exercised the same predicate via monkeypatched lambdas. Collapse to: - one end-to-end test using a real attestation marker in a tmp home: dead attested gateway keeps the plan under Desktop ownership, survives the spawn-time re-check, and the marker is consumed by the spawn; - one probe test: no marker / null or non-list pids / non-dict / non-JSON all read False (fail closed), alive and clean-exit read False, read-only when it does read True. Existing #76129 tests keep their attested_gateway_died=False pins unchanged. --- .../test_gateway_start_attestation.py | 26 +++---- ...ws_gateway_cold_start_desktop_lifecycle.py | 68 ++++++++----------- 2 files changed, 44 insertions(+), 50 deletions(-) diff --git a/tests/hermes_cli/test_gateway_start_attestation.py b/tests/hermes_cli/test_gateway_start_attestation.py index 627394a4a8..9922f20b76 100644 --- a/tests/hermes_cli/test_gateway_start_attestation.py +++ b/tests/hermes_cli/test_gateway_start_attestation.py @@ -196,27 +196,29 @@ def test_breakaway_fallback_warns_even_on_success(monkeypatch, attest_home, caps # --------------------------------------------------------------------------- -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. +def test_attested_probe_fails_closed_without_a_well_formed_dead_attestation(attest_home): + """``hermes update`` uses this probe to override Desktop-owned lifecycle suppression + (#109538), so only a *detectable* death may read True: no marker, malformed ``pids``, + a gateway alive now, or a clean ledger exit all read False. The probe must also be + read-only — consuming the marker here would silence the CLI-start warning that reports + the same death to the user. """ + marker = attest_home / "state" / "gateway.start-attestation.json" assert gateway_windows.attested_gateway_died(current_pids=[]) is False # no marker yet - gateway_windows._write_start_attestation([555], "cold-start after update") + marker.parent.mkdir(exist_ok=True) + for malformed in ('{"pids": null}', '{"pids": 555}', '["not", "a", "dict"]', "not json"): + marker.write_text(malformed, encoding="utf-8") + assert gateway_windows.attested_gateway_died(current_pids=[]) is False, malformed + assert gateway_windows._attested_pids_from({"pids": [1, "x", 2]}) == [1, 2] + 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( + (marker.parent / "gateway.lifecycle.json").write_text( json.dumps({"phase": "exited", "pid": 556, "exit_reason": "graceful_shutdown"}), encoding="utf-8", ) 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 ac70d4bc32..580bd0ab8b 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 @@ -130,32 +130,6 @@ 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) @@ -173,25 +147,43 @@ def test_cold_start_aborts_when_desktop_owns_lifecycle(monkeypatch): 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 = [] +def test_attested_dead_gateway_survives_desktop_ownership_and_marker_is_consumed_on_spawn( + monkeypatch, tmp_path, capsys +): + """#109538: the Desktop hand-off can kill the running gateway moments before update + discovery runs, so a dead start attestation is the surviving "a gateway was up" + evidence. It must keep the plan AND survive the spawn-time ownership re-check. + Once the spawn happens the marker is consumed, so a stale crash marker cannot + re-authorize a cold start against Desktop ownership on a later update.""" + monkeypatch.setattr("hermes_cli.config.get_hermes_home", lambda: str(tmp_path)) 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(hermes_gateway, "find_windows_gateway_services", lambda **_k: []) + monkeypatch.setattr(gateway_windows, "is_installed", 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 - ) + gateway_windows._write_start_attestation([555], "direct spawn (PID 555)") + marker = tmp_path / "state" / "gateway.start-attestation.json" + + token = update_cmd._pause_windows_gateways_for_update() + + assert token == { + "resume_needed": True, + "profiles": {}, + "unmapped_pids": [], + "unmapped": [], + "cold_start_if_installed": True, + } + assert marker.exists() # plan-time probe is read-only + + spawned = [] + 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 - ) + assert "Gateway started via cold-start after update (PID: 4242)" in capsys.readouterr().out + assert not marker.exists() # consumed by the spawn + assert gateway_windows.attested_gateway_died() is False