From ae71d85bba6016aa8690e187b2ba216cd3e16d2c Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:36:18 -0700 Subject: [PATCH] fix(update): startup hint no longer blames an update that did restart the fleet `hermes` printed "A previous `hermes update` pulled new code but did not restart running gateways" on every launch after an update whose receipt was `failed` for a post-restart crash: `fleet` was empty (the matrix never ran), so the check fell back to the pre-pull `plan.runtimes[].code_sha` against today's checkout and the live gateway, correctly restarted onto the pulled commit, still read as "not restarted" once a manual `git pull` had moved the checkout again. Reproduced on the maintainer's box against the #112604 receipt: gateway on the update's `post_update.sha`, checkout two pulls ahead. The startup hint now holds a receipt whose restart phase completed (`gateway_restart` present, `incomplete` false, no `phase_error`) to the code it actually pulled: every owed gateway serving `post_update.sha` discharges the update. Later drift is `gateway/code_skew.py`'s job, not a warning that misreports the update. The catch-up predicate `hermes update` itself runs keeps its checkout semantics (the fleet must reach HEAD), so the same box still gets its gateway restarted on the next update. `_live_fleet_covers_receipt` compares stamped `code_sha` against the expected sha for `current` AND `stale` rows (both labels are relative to the checkout, not to what the update owed); `unknown`/`down` rows still never cover. --- hermes_cli/update_cmd_fleet.py | 56 +++++++++++++++++-- .../test_update_fleet_restart_pending.py | 38 +++++++++++++ 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/hermes_cli/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index 686a5606c9..33ce7dd278 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -171,7 +171,7 @@ def _receipt_owed_gateways() -> set[tuple[str, str]] | None: return owed -def _live_fleet_covers_receipt(expected_sha: str | None) -> bool: +def _live_fleet_covers_receipt(expected_sha: str | None, *, accept_states: tuple = ("current",)) -> bool: """Require current successors for every recorded runtime, not just any live row. A PID changes on restart; the stable identity is (runtime kind, profile). @@ -187,8 +187,12 @@ def _live_fleet_covers_receipt(expected_sha: str | None) -> bool: if not owed: return False fleet = collect_fleet_versions() + # ``current``/``stale`` are labels relative to the checkout. A caller asking about the + # code a completed update restarted the fleet onto passes ``accept_states`` with + # ``stale`` too: the row's stamped ``code_sha`` is the identity that matters there. + # ``unknown``/``down`` rows never cover. if not fleet or any( - row.get("state") != "current" or row.get("code_sha") != expected_sha + row.get("state") not in accept_states or row.get("code_sha") != expected_sha for row in fleet ): return False @@ -265,8 +269,32 @@ def _marker_only_restart_obsolete() -> bool: return True +def _receipt_restart_phase_completed(receipt: dict) -> str | None: + """``post_update.sha`` when the receipt's restart phase ran to completion, else None. + + A receipt can be ``failed`` for reasons that have nothing to do with the fleet (a + post-restart step crashed, a notice raised) after every gateway was already brought to + the pulled code. Its pre-pull ``plan.runtimes[].code_sha`` then no longer describes an + obligation; the update owed the fleet ``post_update.sha`` and that is what the live + fleet must be checked against. Any later drift (a manual ``git pull`` moving the + checkout past a running gateway) belongs to ``gateway/code_skew.py``, not to a warning + that blames an update which did restart the fleet. + """ + gateway_restart = receipt.get("gateway_restart") + if not isinstance(gateway_restart, dict) or not gateway_restart: + return None + if gateway_restart.get("incomplete") or gateway_restart.get("phase_error"): + return None + post_sha = (receipt.get("post_update") or {}).get("sha") + return str(post_sha) if post_sha else None + + def _pending_fleet_restart_needed() -> bool: - """Reconcile old restart obligations against current, identity-matched gateways.""" + """Reconcile old restart obligations against current, identity-matched gateways. + + Catch-up semantics (``hermes update`` on a current checkout): the fleet must reach the + checkout HEAD, whatever moved it there. + """ from hermes_cli.update_cmd import _current_checkout_sha # The marker has no runtime inventory and may belong to a newer, killed update @@ -281,6 +309,26 @@ def _pending_fleet_restart_needed() -> bool: return not _live_fleet_covers_receipt(_current_checkout_sha()) +def _update_owes_fleet_restart() -> bool: + """Startup-warning semantics: does the LAST UPDATE still owe the fleet a restart? + + Same evidence as :func:`_pending_fleet_restart_needed`, except that a receipt whose + restart phase completed is held to the code it pulled, not to today's checkout: the + update kept its promise, and a checkout moved later by hand is not its unfinished work. + """ + with suppress(OSError): + if _fleet_restart_pending_marker_path().is_file(): + return not _marker_only_restart_obsolete() + if not _receipt_reports_stale_runtime(): + return False + from hermes_cli.update_cmd import _current_checkout_sha + from hermes_cli.update_receipt import read_latest_receipt + restarted_to = _receipt_restart_phase_completed(read_latest_receipt() or {}) + if restarted_to: + return not _live_fleet_covers_receipt(restarted_to, accept_states=("current", "stale")) + return not _live_fleet_covers_receipt(_current_checkout_sha()) + + def _warn_pending_fleet_restart(*, startup: bool = False) -> None: """Print the specific interrupted-update fleet-restart warning.""" stream = sys.stderr if startup else sys.stdout @@ -293,7 +341,7 @@ def _warn_pending_fleet_restart(*, startup: bool = False) -> None: def _warn_pending_fleet_restart_on_startup() -> None: """Cheap CLI-startup hint. Never restarts; never raises.""" with suppress(Exception): - if _pending_fleet_restart_needed(): + if _update_owes_fleet_restart(): _warn_pending_fleet_restart(startup=True) diff --git a/tests/hermes_cli/test_update_fleet_restart_pending.py b/tests/hermes_cli/test_update_fleet_restart_pending.py index 8708616735..e5f28b1b50 100644 --- a/tests/hermes_cli/test_update_fleet_restart_pending.py +++ b/tests/hermes_cli/test_update_fleet_restart_pending.py @@ -765,3 +765,41 @@ def test_startup_warn_kept_when_receipt_owed_gateway_is_down(monkeypatch, capsys assert "did not restart running gateways" in capsys.readouterr().err assert update_cmd._fleet_restart_pending_marker_path().exists() + +def test_startup_warn_silent_when_failed_receipt_already_restarted_fleet(monkeypatch, capsys): + """#112604 aftermath: the update pulled ``pulled``, restarted every gateway onto it, then a + post-restart step crashed (receipt ``failed``, empty ``fleet`` matrix). Later a manual + ``git pull`` moved the checkout again. The startup hint must not blame that update for a + restart it performed; ``hermes update``'s catch-up still owes the fleet the checkout.""" + pre, pulled, checkout = "a" * 40, "b" * 40, "c" * 40 + _patch_marker_sha(monkeypatch, checkout) + receipt_dir = get_hermes_home() / "logs" / "update_receipts" + receipt_dir.mkdir(parents=True) + (receipt_dir / "latest.json").write_text( + json.dumps( + { + "outcome": "failed", "exit_code": 1, + "stop_reason": "AttributeError: module 'hermes_cli.main_dashboard' has no attribute 'x'", + "pre_update": {"sha": pre}, "post_update": {"sha": pulled}, + "gateway_restart": { + "restarted_services": ["hermes-gateway"], "relaunched_profiles": [], + "externally_supervised_profiles": [], "killed_pids": [], "failed_units": [], + "incomplete": False, "phase_error": "", + }, + "fleet": [], + "plan": {"runtimes": [{"kind": "gateway", "profile": "default", "code_sha": pre, "pid": 1}]}, + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + "hermes_cli.update_receipt.collect_fleet_versions", + lambda **kwargs: [ + {"profile": "default", "pid": 42, "code_sha": pulled, "code_version": "0.21.3", "state": "stale"} + ], + ) + + update_cmd._warn_pending_fleet_restart_on_startup() + + assert capsys.readouterr().err == "" + assert update_cmd_fleet._pending_fleet_restart_needed() is True