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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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": <code>} 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": <code>} 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):
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user