fix(update): discharge fleet_restart_pending when the fleet provably serves expected_sha
_pending_fleet_restart_needed() returns True on marker existence alone. A
supervisor-level gateway restart (`systemctl --user restart`, launchctl, an ops
script) never passes through _clear_fleet_restart_pending_marker(), so a marker
written by a pulled update survives a restart that DID bring every live gateway
to the new code — and every later CLI call prints the interrupted-update warning
forever, a permanent false positive that trains operators to ignore it.
The comment on that branch is right that "an older receipt cannot discharge that
unknown obligation" — but the obligation is not unknown. The marker records its
own expected_sha, so it can be verified against the live fleet directly, with no
receipt involved. _live_fleet_covers_receipt() cannot substitute: it is
receipt-anchored and returns False when `owed` is empty, which is exactly the
marker-only case.
Hold the marker to the same evidence bar _live_fleet_covers_receipt() applies to
a receipt: at least one row, and every row a `current` gateway under a known
profile whose code_sha equals the marker's expected_sha, with the checkout HEAD
not moved past the marker.
Still keeps the marker (warns) on:
- stale / down rows — the restart genuinely is owed
- an all-`unknown` fleet — pre-code-identity gateways cannot prove currency
(same conservatism as #88848/#74973)
- a marker with no expected_sha — nothing to verify against
- a newer pull that moved the checkout — it owns a fresh obligation
- a probe that raises or answers empty — no proof either way
Discharging deletes the marker only after every row passes; the historical
receipt is left intact, since a supervisor restart is still not a successful
update.
Tests: discharge on a provably-current fleet; keep on stale, empty probe,
unknown identity, and newer-pull-moved-checkout (which asserts the fleet is
never probed once HEAD has moved).
This commit is contained in:
@@ -183,6 +183,64 @@ def _live_fleet_covers_receipt(expected_sha: str | None) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _read_fleet_marker_expected_sha() -> str:
|
||||
"""``expected_sha`` recorded in the pending marker ("" when absent/unreadable)."""
|
||||
with suppress(OSError):
|
||||
for line in _fleet_restart_pending_marker_path().read_text(encoding="utf-8").splitlines():
|
||||
if line.startswith("expected_sha="):
|
||||
return line.split("=", 1)[1].strip()
|
||||
return ""
|
||||
|
||||
|
||||
def _marker_only_restart_obsolete() -> bool:
|
||||
"""True when the pending marker is a leftover: the fleet already runs the expected code.
|
||||
|
||||
A supervisor-level restart (``systemctl --user restart``, launchctl, ops scripts) never
|
||||
goes through this module's clear path, so the marker survives a restart that DID bring
|
||||
every live gateway to the pulled code — and every later CLI call then prints the
|
||||
interrupted-update warning forever (false positive).
|
||||
|
||||
The marker is not an *unknown* obligation: it records its own ``expected_sha``, so it can
|
||||
be checked against the live fleet directly — no receipt required. Hold it to the same
|
||||
evidence bar ``_live_fleet_covers_receipt`` applies to one: at least one row, and every
|
||||
row a ``current`` gateway under a known profile whose ``code_sha`` equals that
|
||||
``expected_sha``, with the checkout HEAD not moved past the marker.
|
||||
|
||||
Keep the marker on stale/down rows, on an all-``unknown`` fleet (pre-code-identity
|
||||
gateways cannot prove currency — same conservatism as the silent-failure class
|
||||
#88848/#74973), on a marker with no ``expected_sha``, when a newer pull moved the
|
||||
checkout, and when the probe fails or answers empty.
|
||||
"""
|
||||
expected_sha = _read_fleet_marker_expected_sha()
|
||||
if not expected_sha:
|
||||
return False # pre-expected_sha marker: nothing to verify against
|
||||
checkout_sha = _current_checkout_sha()
|
||||
if checkout_sha and checkout_sha != expected_sha:
|
||||
return False # a newer pull moved HEAD; it owns a fresh obligation
|
||||
try:
|
||||
from hermes_cli.update_receipt import collect_fleet_versions
|
||||
fleet = collect_fleet_versions()
|
||||
except Exception as exc:
|
||||
logger.debug("Fleet probe failed; keeping fleet-restart-pending marker: %s", exc)
|
||||
return False
|
||||
if not fleet:
|
||||
return False # probe answered empty: no proof either way
|
||||
for row in fleet:
|
||||
if not isinstance(row, dict):
|
||||
return False
|
||||
profile = row.get("profile")
|
||||
if not profile or profile == "unknown":
|
||||
return False # unidentified runtime: the matrix cannot vouch for it
|
||||
if row.get("state") != "current" or str(row.get("code_sha")) != expected_sha:
|
||||
return False # stale / down / unknown-identity row still owes the restart
|
||||
_clear_fleet_restart_pending_marker()
|
||||
logger.debug(
|
||||
"Fleet-restart-pending marker discharged: %d gateway(s) already serve %s",
|
||||
len(fleet), expected_sha[:10],
|
||||
)
|
||||
return True
|
||||
|
||||
|
||||
def _pending_fleet_restart_needed() -> bool:
|
||||
"""Reconcile old restart obligations against current, identity-matched gateways."""
|
||||
from hermes_cli.update_cmd import _current_checkout_sha
|
||||
@@ -191,6 +249,8 @@ def _pending_fleet_restart_needed() -> bool:
|
||||
# than latest.json. An older receipt cannot discharge that unknown obligation.
|
||||
with suppress(OSError):
|
||||
if _fleet_restart_pending_marker_path().is_file():
|
||||
if _marker_only_restart_obsolete():
|
||||
return False
|
||||
return True
|
||||
if not _receipt_reports_stale_runtime():
|
||||
return False
|
||||
|
||||
@@ -609,3 +609,110 @@ def test_startup_warn_silent_when_nothing_pending(capsys):
|
||||
captured = capsys.readouterr()
|
||||
assert captured.err == ""
|
||||
assert captured.out == ""
|
||||
|
||||
|
||||
# ── Self-heal: marker left behind by a supervisor-level restart (TRA-1180 class) ──
|
||||
#
|
||||
# `systemctl --user restart hermes-gateway` (weekly-update fallback, manual ops) never
|
||||
# runs this module's clear path, so the marker survives a restart that DID bring the
|
||||
# fleet to the pulled code — and every later CLI call warns forever. The marker must be
|
||||
# discharged when (and only when) the fleet provably serves expected_sha.
|
||||
|
||||
|
||||
def _patch_marker_sha(monkeypatch, disk_sha):
|
||||
monkeypatch.setattr(update_cmd, "_current_checkout_sha", lambda: disk_sha)
|
||||
monkeypatch.setattr(update_cmd_fleet, "_current_checkout_sha", lambda: disk_sha)
|
||||
|
||||
|
||||
def test_startup_warn_discharged_when_fleet_current(monkeypatch, capsys):
|
||||
disk_sha = "e" * 40
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha=disk_sha)
|
||||
_patch_marker_sha(monkeypatch, disk_sha)
|
||||
monkeypatch.setattr(
|
||||
update_cmd_fleet,
|
||||
"_marker_only_restart_obsolete",
|
||||
update_cmd_fleet._marker_only_restart_obsolete,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **kwargs: [
|
||||
{"profile": "default", "pid": 42, "code_sha": disk_sha, "code_version": "0.21.0", "state": "current"}
|
||||
],
|
||||
)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
captured = capsys.readouterr()
|
||||
assert captured.err == ""
|
||||
assert not update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_startup_warn_kept_when_fleet_stale(monkeypatch, capsys):
|
||||
disk_sha = "e" * 40
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha=disk_sha)
|
||||
_patch_marker_sha(monkeypatch, disk_sha)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **kwargs: [
|
||||
{"profile": "default", "pid": 42, "code_sha": "7" * 40, "code_version": "0.20.0", "state": "stale"}
|
||||
],
|
||||
)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "did not restart running gateways" in err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_startup_warn_kept_when_fleet_probe_empty(monkeypatch, capsys):
|
||||
disk_sha = "e" * 40
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha=disk_sha)
|
||||
_patch_marker_sha(monkeypatch, disk_sha)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **kwargs: [],
|
||||
)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "did not restart running gateways" in err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_startup_warn_kept_when_fleet_identity_unknown(monkeypatch, capsys):
|
||||
disk_sha = "e" * 40
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha=disk_sha)
|
||||
_patch_marker_sha(monkeypatch, disk_sha)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **kwargs: [
|
||||
{"profile": "default", "pid": 42, "code_sha": None, "code_version": None, "state": "unknown"}
|
||||
],
|
||||
)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "did not restart running gateways" in err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_marker_kept_when_newer_pull_moved_checkout_past_marker(monkeypatch, capsys):
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha="d" * 40)
|
||||
_patch_marker_sha(monkeypatch, "e" * 40) # checkout advanced after the marker
|
||||
seen = {"called": False}
|
||||
|
||||
def _collect(**kwargs):
|
||||
seen["called"] = True
|
||||
return [{"profile": "default", "pid": 42, "code_sha": "d" * 40, "code_version": None, "state": "current"}]
|
||||
|
||||
monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", _collect)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
err = capsys.readouterr().err
|
||||
assert "did not restart running gateways" in err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
assert seen["called"] is False # short-circuits before probing the fleet
|
||||
|
||||
Reference in New Issue
Block a user