fix(update): clear the fleet-restart obligation on empty gateway inventories
A pre-update plan that records zero runtimes (Desktop-hosted `serve` with no gateway services, or a fleet of only manually-deferred serve/dashboard processes) armed `fleet_restart_pending` with an empty inventory. Nothing navigates the marker's fail-closed discharge path for an empty `owed` set — `_marker_only_restart_obsolete` returned False before ever probing, so every later `hermes update` on the host reported "Fleet restart incomplete" and exited 1 with no gateway to restart. - `_write_fleet_restart_pending_marker`: never arm a marker for an explicit empty inventory (`runtimes=[]`); `None` still arms the conservative "unknown obligation" breadcrumb. - `_marker_only_restart_obsolete`: build the owed set before the SHA/probe gates; an inventory owing nothing is discharged and cleared immediately, without a fleet probe. Inventories that DO owe restarts keep the existing fail-closed probe-until-proven-current path; legacy/malformed inventories stay fail-closed. Regression tests: marker not armed for `runtimes=[]`; pre-fix empty-inventory marker discharged without touching the fleet probe; an already-up-to-date `hermes update` carrying the legacy marker exits 0 with no restart run. Fixes #115311
This commit is contained in:
@@ -53,6 +53,11 @@ def _fleet_restart_pending_marker_path() -> Path:
|
||||
|
||||
def _write_fleet_restart_pending_marker(*, expected_sha: str = "", runtimes: list[dict] | None = None) -> None:
|
||||
"""Drop the pull→restart obligation breadcrumb. Never raises."""
|
||||
if runtimes == []:
|
||||
# An explicit empty inventory owes no restart (e.g. Desktop-hosted `serve` with no
|
||||
# gateway services). Arming the marker here leaves a breadcrumb nothing can discharge:
|
||||
# a no-gateway host would then fail every later ``hermes update`` (#115311).
|
||||
return
|
||||
from hermes_cli.update_cmd import _m
|
||||
path = _fleet_restart_pending_marker_path()
|
||||
if _m()._pytest_owns_live_checkout(path.parent):
|
||||
@@ -215,8 +220,14 @@ def _live_fleet_covers_receipt(expected_sha: str | None, receipt: dict, owed: se
|
||||
def _marker_only_restart_obsolete() -> bool:
|
||||
"""Settle only the inventory stored with this marker's target SHA.
|
||||
|
||||
Historical receipts cannot narrow this obligation. Malformed or unsupported inventories stay fail-closed; empty discovery never proves a stopped gateway recovered.
|
||||
An inventory-less marker records no obligation (#115638), so it discharges when the live fleet provably serves the marker's expected SHA.
|
||||
Historical receipts cannot narrow this obligation. Malformed or unsupported inventories stay
|
||||
fail-closed; empty discovery never proves a stopped gateway recovered. Two shapes record no
|
||||
obligation and settle without one: an explicit empty inventory (a pull that found no gateway,
|
||||
#115311) clears outright, and an inventory-less marker (the pre-inventory writer, or a tail
|
||||
that died before its inventory was recorded, #115638) clears once every live gateway is
|
||||
current on the checkout — there is no recorded owed set, so the fleet running the code on disk
|
||||
is the whole of the evidence the marker's warning can be about, even after HEAD moved past
|
||||
``expected_sha`` by an out-of-band pull.
|
||||
"""
|
||||
from hermes_cli.update_serve_obligations import defer_manual_serve
|
||||
|
||||
@@ -230,13 +241,11 @@ def _marker_only_restart_obsolete() -> bool:
|
||||
expected_sha = fields.get("expected_sha", "").strip()
|
||||
inventory = json.loads(fields.get("inventory", "null"))
|
||||
owed: set[tuple[str, str]] | None = None
|
||||
if inventory is None:
|
||||
pass # inventory-less marker: no recorded obligation; live-fleet evidence alone settles it
|
||||
else:
|
||||
if inventory is not None:
|
||||
if not isinstance(inventory, dict) or inventory.get("version") != 1:
|
||||
return False
|
||||
runtimes = inventory.get("runtimes")
|
||||
if not isinstance(runtimes, list) or not runtimes:
|
||||
if not isinstance(runtimes, list):
|
||||
return False
|
||||
owed = set()
|
||||
for runtime in runtimes:
|
||||
@@ -252,11 +261,20 @@ def _marker_only_restart_obsolete() -> bool:
|
||||
owed.add(("gateway", profile))
|
||||
except (OSError, UnicodeError, ValueError):
|
||||
return False
|
||||
if owed is not None and not owed:
|
||||
# A pull that recorded no gateway runtime owes no restart; clearing avoids the
|
||||
# stuck "Fleet restart incomplete" loop on Desktop-hosted (no-service) installs.
|
||||
_clear_fleet_restart_pending_marker()
|
||||
logger.debug("Fleet-restart-pending marker discharged: no gateway obligation recorded")
|
||||
return True
|
||||
if not expected_sha:
|
||||
return False
|
||||
checkout_sha = _current_checkout_sha()
|
||||
if checkout_sha != expected_sha:
|
||||
if owed is not None and checkout_sha != expected_sha:
|
||||
return False # a newer pull moved HEAD; it owns a fresh obligation
|
||||
target_sha = expected_sha if owed is not None else checkout_sha
|
||||
if not target_sha:
|
||||
return False
|
||||
try:
|
||||
from hermes_cli.update_receipt import collect_fleet_versions
|
||||
fleet = collect_fleet_versions()
|
||||
@@ -269,14 +287,14 @@ def _marker_only_restart_obsolete() -> bool:
|
||||
if covered is None:
|
||||
return False # unidentified runtime: the matrix cannot vouch for it
|
||||
for row in fleet:
|
||||
if row.get("state") != "current" or str(row.get("code_sha")) != expected_sha:
|
||||
if row.get("state") != "current" or str(row.get("code_sha")) != target_sha:
|
||||
return False # stale / down / unknown-identity row still owes the restart
|
||||
if owed is not None and not owed <= covered:
|
||||
return False # A gateway this marker owns is absent (down) or unidentifiable.
|
||||
_clear_fleet_restart_pending_marker()
|
||||
logger.debug(
|
||||
"Fleet-restart-pending marker discharged: %d gateway(s) already serve %s",
|
||||
len(fleet), expected_sha[:10],
|
||||
len(fleet), target_sha[:10],
|
||||
)
|
||||
return True
|
||||
|
||||
|
||||
@@ -943,3 +943,48 @@ def test_startup_warn_kept_when_inventory_less_marker_fleet_stale(monkeypatch, c
|
||||
|
||||
assert "did not restart running gateways" in capsys.readouterr().err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
# ── Empty-inventory marker: a pull that recorded no gateway owes nothing (#115311) ──
|
||||
|
||||
def _write_marker_with_inventory(expected_sha, runtimes):
|
||||
marker = update_cmd_fleet._fleet_restart_pending_marker_path()
|
||||
marker.write_text(
|
||||
f"started=0\npid=1\nexpected_sha={expected_sha}\n"
|
||||
f"inventory={json.dumps({'version': 1, 'runtimes': runtimes})}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
return marker
|
||||
|
||||
|
||||
def test_empty_inventory_does_not_arm_marker():
|
||||
"""A pre-update plan with zero runtimes must never arm the marker: on a no-gateway
|
||||
(Desktop-hosted) install every later update would otherwise hit the unbeatable
|
||||
'Fleet restart incomplete' exit 1 (#115311)."""
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha="e" * 40, runtimes=[])
|
||||
assert not update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_pending_fleet_restart_cleared_instead_of_exit_1(monkeypatch, tmp_path):
|
||||
"""Repro: an already-up-to-date host carrying an empty-inventory marker must exit 0 with
|
||||
no restart run — not print 'Fleet restart incomplete' and exit 1 (#115311)."""
|
||||
args = _update_args()
|
||||
_patch_update_deps(monkeypatch, tmp_path, _make_up_to_date_side_effect())
|
||||
marker = _write_marker_with_inventory("abc123", [])
|
||||
|
||||
seen = {"ran": False}
|
||||
monkeypatch.setattr(update_cmd_fleet, "_current_checkout_sha", lambda: "abc123")
|
||||
monkeypatch.setattr(
|
||||
update_cmd,
|
||||
"_run_pending_fleet_restart",
|
||||
lambda: seen.__setitem__("ran", True) or True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
update_cmd_fleet,
|
||||
"_run_pending_fleet_restart",
|
||||
lambda: seen.__setitem__("ran", True) or True,
|
||||
)
|
||||
|
||||
hermes_main.cmd_update(args)
|
||||
|
||||
assert seen["ran"] is False
|
||||
assert not marker.exists()
|
||||
|
||||
Reference in New Issue
Block a user