From bc1330eebc0aa8a443501b5f62586eb7361353a5 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:32:23 +0530 Subject: [PATCH] refactor(update): name the success invariant in _receipt_looks_unfinished; one predicate contract The tail clause was correct only by ordering (exit_code != 0 there meant exit_code is None). Name what it encodes: a stop_reason counts only when nothing vouched for success. Same truth table. The three literal-dict tests on the predicate collapse into one parametrized contract; the handoff-exit test binds to COMMAND_BOUNDARY_STOP_REASON instead of re-spelling it. --- hermes_cli/update_cmd_fleet.py | 19 +++++----- .../test_update_fleet_restart_pending.py | 35 ++++++------------- tests/hermes_cli/test_update_handoff_exit.py | 5 +-- 3 files changed, 21 insertions(+), 38 deletions(-) diff --git a/hermes_cli/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index 8aa42deeeb..d637db9556 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -80,25 +80,22 @@ def _current_checkout_sha() -> str | None: def _receipt_looks_unfinished(receipt: dict) -> bool: """True when *receipt* is from an update that did not finish cleanly. - ``stop_reason`` records *how* the command boundary closed the receipt - (``completed at command boundary``, ``sys.exit(0)``, or a refusal code from - ``update_contract``). A truthy stop_reason must not make a + The command boundary stamps a ``stop_reason`` on every receipt, including clean + ones (``completed at command boundary``, ``sys.exit(0)``); it must not make a successful receipt look unfinished, or the next ``hermes update`` retriggers - ``fleet_restart_pending`` from pre-pull plan SHAs. + ``fleet_restart_pending`` from pre-pull plan SHAs (#98022). """ exit_code = receipt.get("exit_code") outcome = receipt.get("outcome") - if exit_code not in (0, None): - return True - if outcome in ("failed", "partial", "running"): + if exit_code not in (0, None) or outcome in ("failed", "partial", "running"): return True gateway_restart = receipt.get("gateway_restart") if isinstance(gateway_restart, dict) and gateway_restart.get("incomplete"): return True - stop_reason = receipt.get("stop_reason") - if stop_reason and outcome != "success" and exit_code != 0: - return True - return False + # A stop_reason alone (update_contract refusals: outcome="refused", no exit_code) + # counts only when nothing else vouched for success. + succeeded = exit_code == 0 or outcome == "success" + return bool(receipt.get("stop_reason")) and not succeeded def _receipt_reports_stale_runtime(expected_sha: str | None = None) -> bool: diff --git a/tests/hermes_cli/test_update_fleet_restart_pending.py b/tests/hermes_cli/test_update_fleet_restart_pending.py index 321fa25bf8..4056d73bc6 100644 --- a/tests/hermes_cli/test_update_fleet_restart_pending.py +++ b/tests/hermes_cli/test_update_fleet_restart_pending.py @@ -300,34 +300,19 @@ def test_successful_command_boundary_receipt_without_fleet_does_not_retrigger( @pytest.mark.parametrize( - "receipt", + ("receipt", "unfinished"), [ - {"outcome": "success", "exit_code": 0, "stop_reason": "sys.exit(0)"}, - {"outcome": "success", "stop_reason": "KeyboardInterrupt: "}, - {"exit_code": 0, "stop_reason": "sys.exit(0)"}, + pytest.param({"outcome": "success", "exit_code": 0, "stop_reason": "sys.exit(0)"}, False, id="success-sys-exit-0"), + pytest.param({"outcome": "success", "stop_reason": "KeyboardInterrupt: "}, False, id="success-no-exit-code"), + pytest.param({"exit_code": 0, "stop_reason": "sys.exit(0)"}, False, id="exit-0-no-outcome"), + # update_contract writes {"outcome": "refused", "stop_reason": } with no exit_code; + # the stop_reason clause is what keeps that receipt unfinished. + pytest.param({"outcome": "refused", "stop_reason": "not_updatable_in_place"}, True, id="refused-stop-reason-only"), + pytest.param({"outcome": "failed", "exit_code": 1, "stop_reason": "KeyboardInterrupt: "}, True, id="failed-interrupt"), ], ) -def test_successful_non_boundary_stop_reasons_are_finished(receipt): - """Successful sys.exit(0)/KeyboardInterrupt must not look unfinished.""" - assert update_cmd._receipt_looks_unfinished(receipt) is False - - -def test_refused_receipt_with_only_a_stop_reason_is_unfinished(): - # update_contract writes {"outcome": "refused", "stop_reason": } with no exit_code; - # the stop_reason clause is what keeps that receipt unfinished. - assert update_cmd._receipt_looks_unfinished( - {"outcome": "refused", "stop_reason": "not_updatable_in_place"} - ) is True - - -def test_failed_interrupt_stop_reason_is_unfinished(): - assert update_cmd._receipt_looks_unfinished( - { - "outcome": "failed", - "exit_code": 1, - "stop_reason": "KeyboardInterrupt: ", - } - ) is True +def test_stop_reason_only_marks_unfinished_when_nothing_vouches_for_success(receipt, unfinished): + assert update_cmd._receipt_looks_unfinished(receipt) is unfinished def test_stale_fleet_matrix_on_latest_receipt_is_pending(monkeypatch): diff --git a/tests/hermes_cli/test_update_handoff_exit.py b/tests/hermes_cli/test_update_handoff_exit.py index 4b963c1ba8..9d79986c7e 100644 --- a/tests/hermes_cli/test_update_handoff_exit.py +++ b/tests/hermes_cli/test_update_handoff_exit.py @@ -23,6 +23,7 @@ import pytest import hermes_cli.main as main_mod from hermes_cli import update_cmd from hermes_cli.main import cmd_update +from hermes_cli.update_receipt import COMMAND_BOUNDARY_STOP_REASON class _FakeLock: @@ -89,7 +90,7 @@ def _noop_impl(args, gateway_mode=False): def test_handoff_child_hard_exits_zero_after_success(monkeypatch): events = _run_cmd_update(monkeypatch, _noop_impl, reexec=True) assert events["exit_codes"] == [0] - assert events["receipts"] == [(0, "completed at command boundary")] + assert events["receipts"] == [(0, COMMAND_BOUNDARY_STOP_REASON)] # The hard exit is the last thing, after lock release and stdio restore. assert events["order"] == ["acquire", "impl", "release", "restore-stdio", "hard-exit"] @@ -98,7 +99,7 @@ def test_non_handoff_run_never_hard_exits(monkeypatch): events = _run_cmd_update(monkeypatch, _noop_impl, reexec=False) assert events["exit_codes"] == [] assert "hard-exit" not in events["order"] - assert events["receipts"] == [(0, "completed at command boundary")] + assert events["receipts"] == [(0, COMMAND_BOUNDARY_STOP_REASON)] def test_handoff_child_propagates_early_systemexit_code(monkeypatch):