diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 73e66a7c5f..17dc5f8ac4 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -49,7 +49,7 @@ from hermes_cli.update_cmd_fleet import ( # noqa: F401 _FLEET_RESTART_PENDING_NAME, _FRESH_RESTART_SUPERVISORS, _GatewayRestartOutcome, _clear_fleet_restart_pending_marker, _current_checkout_sha, _drain_or_signal_gateway_for_update, _fleet_probe_expected_runtimes, - _fleet_restart_pending_marker_path, _for_each_systemd_gateway_unit, + _fleet_restart_pending_marker_path, _fleet_restart_skip_reason, _for_each_systemd_gateway_unit, _gateway_recovery_partition, _gateway_service_matches_profile, _pending_fleet_restart_needed, _receipt_looks_unfinished, _receipt_reports_stale_runtime, _resolve_manage_cmd, _restart_gateway_fleet_after_update, _restart_launchd_gateway_after_update, @@ -1373,6 +1373,9 @@ def _finish_already_up_to_date( _git_run(git_cmd, ["checkout", current_branch]) if completion_request is not None: + # Same code, same host obligation: an SHA-less arm would REPLACE the standing record + # (and its restarted proof), so a sibling profile's no-op update re-kills the multiplexer. + completion_request["expected_sha"] = _capture_head_sha(git_cmd, _m().PROJECT_ROOT) or "" completion_request["completion_message"] = ( "✓ Already up to date!" if _plan.upstream_checked else "✓ Up to date with your fork (official repo not checked).") diff --git a/hermes_cli/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index 8d51e19498..7a2aee1551 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -567,6 +567,30 @@ def _restart_identity_sha() -> str: return "" +def _fleet_restart_skip_reason(plan) -> str | None: + """Why the completion tail may leave the fleet alone, or ``None`` when a restart is owed. + + Every route (pulled, already-current, ZIP) now finishes through the same completion + tail, so the guards the old catch-up path carried live here: one host runs ONE + multiplexing gateway, so a second profile's ``hermes update`` attaches to the restart the + first one already stamped (#95294), and a fleet already serving the checkout code (a + no-op update, a manual ``hermes gateway restart`` seconds ago) is not re-killed (#117051). + + The second guard needs BOTH the pre-update plan and the live probe: the live matrix only + lists gateways, so a planned ``serve`` still on pre-update code (or any runtime without a + stamped identity) keeps the restart — the reconciliation there is what surfaces it. + """ + from hermes_cli.update_host_obligation import host_restart_already_completed + checkout_sha = _restart_identity_sha() + if host_restart_already_completed(checkout_sha): + return "this host's gateway was already restarted for this update" + if (checkout_sha and plan is not None and plan.runtimes + and all(str(runtime.code_sha) == checkout_sha for runtime in plan.runtimes) + and _live_fleet_current_rows() is not None): + return "every running gateway already serves the checkout code" + return None + + def _run_pending_fleet_restart() -> bool: """Historical retry hook; new retries use the ordinary completion owner.""" from hermes_cli._old_updater import stop_for_relaunch @@ -1550,6 +1574,11 @@ def _restart_gateway_fleet_after_update(_pre_update_plan, gateway_mode: bool): ) out.restarted_scoped_units = set(restarted_scoped_units) + if not out.incomplete: + # Stamp the HOST obligation so every other profile's CLI knows this update's restart + # already happened; without it each profile re-kills the one shared multiplexer. + from hermes_cli.update_host_obligation import mark_host_restart_completed + mark_host_restart_completed(_restart_identity_sha()) return out diff --git a/hermes_cli/update_completion.py b/hermes_cli/update_completion.py index 40ed6fe4f9..fb68ca6772 100644 --- a/hermes_cli/update_completion.py +++ b/hermes_cli/update_completion.py @@ -189,6 +189,20 @@ def _complete_selected(request: dict) -> None: if not complete: raise SystemExit(1) return + skip = update_cmd._fleet_restart_skip_reason(plan) + if skip: + from hermes_cli.update_receipt import record_skip + + record_skip("gateway_restart", skip) + print(f" ✓ Gateway restart skipped: {skip}.") + # Discharges the obligation this run armed when the live fleet vouches for it; a + # fleet still owing the restart fails closed exactly like a stale matrix would. + if update_cmd._pending_fleet_restart_needed(): + print(" ⚠ Gateways are still off the checkout code. Recover with: hermes gateway restart") + raise SystemExit(1) + if not complete: + raise SystemExit(1) + return restart = update_cmd._restart_gateway_fleet_after_update(plan, request["gateway_mode"]) update_cmd._resume_windows_gateways_and_merge_outcome(restart, request["windows_resume"], request["gateway_mode"]) update_cmd._verify_fleet_after_update( diff --git a/tests/hermes_cli/test_update_completion_process.py b/tests/hermes_cli/test_update_completion_process.py index f02247a908..419f89080f 100644 --- a/tests/hermes_cli/test_update_completion_process.py +++ b/tests/hermes_cli/test_update_completion_process.py @@ -98,6 +98,7 @@ def transition(tmp_path): "_sweep_bytecode_after_update = lambda branch: event('bytecode')\n" "_write_fleet_restart_pending_marker = lambda **kw: event('pending')\n" "_write_gateway_update_exit_code = lambda ok: event('exit_marker', ok=ok)\n" + "_fleet_restart_skip_reason = lambda plan: None\n" "def _restart_gateway_fleet_after_update(plan, gateway_mode):\n" " event('restart', profiles=[r.profile for r in plan.runtimes])\n" " return object()\n" diff --git a/tests/hermes_cli/test_update_fleet_restart_pending.py b/tests/hermes_cli/test_update_fleet_restart_pending.py index 41b7aa3a62..943d1d0155 100644 --- a/tests/hermes_cli/test_update_fleet_restart_pending.py +++ b/tests/hermes_cli/test_update_fleet_restart_pending.py @@ -929,3 +929,90 @@ def test_pending_fleet_restart_cleared_instead_of_exit_1(monkeypatch, tmp_path): assert seen["ran"] is False assert not marker.exists() + + +# ── Completion tail guards (#117051 / #95294): a no-op update must not re-kill the fleet ── + + +def _current_row(sha): + return [{"profile": "default", "pid": 42, "code_sha": sha, "code_version": "0.21.0", "state": "current"}] + + +def _plan_with_current_gateway(monkeypatch, sha): + """Pre-update inventory: one gateway already stamped with the checkout code.""" + import hermes_cli.update_inventory as ui + + plan = ui.UpdatePlan() + plan.runtimes = [ui.RuntimeRecord(kind="gateway", profile="default", pid=42, supervisor="systemd", + code_sha=sha, restart_via=ui._restart_mechanism("systemd", "default"))] + monkeypatch.setattr(ui, "collect_runtime_inventory", lambda: plan) + + +def _spy_fleet_restart(monkeypatch): + """Record restart-phase entries; the phase itself (drain waits, unit restarts) is not under test.""" + calls = [] + + def _spy(plan, gateway_mode): + calls.append(plan) + raise SystemExit(3) + + monkeypatch.setattr(update_cmd, "_restart_gateway_fleet_after_update", _spy) + return calls + + +def test_up_to_date_update_leaves_current_fleet_alone(monkeypatch, tmp_path, capsys): + """Every live gateway already serves the checkout code: `hermes update` (cron, a second + profile) must not drain and restart the shared multiplexer again, and must still discharge + the obligation it armed and finish clean.""" + args = _update_args() + _patch_update_deps(monkeypatch, tmp_path, _make_up_to_date_side_effect("abc123")) + _patch_marker_sha(monkeypatch, "abc123") + _plan_with_current_gateway(monkeypatch, "abc123") + monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", lambda **k: _current_row("abc123")) + restarts = _spy_fleet_restart(monkeypatch) + + hermes_main.cmd_update(args) + + assert restarts == [] + assert not update_cmd_fleet._fleet_restart_obligation_armed() + assert "Gateway restart skipped" in capsys.readouterr().out + from hermes_cli.update_receipt import read_latest_receipt + receipt = read_latest_receipt() + assert receipt["outcome"] == "success" + assert any(skip.get("name") == "gateway_restart" for skip in receipt.get("skips", [])) + + +def test_up_to_date_update_still_restarts_a_stale_gateway(monkeypatch, tmp_path): + """The guard is evidence-based: one gateway still on pre-update code means the restart runs.""" + args = _update_args() + _patch_update_deps(monkeypatch, tmp_path, _make_up_to_date_side_effect("abc123")) + _patch_marker_sha(monkeypatch, "abc123") + _plan_with_current_gateway(monkeypatch, "abc123") + monkeypatch.setattr( + "hermes_cli.update_receipt.collect_fleet_versions", + lambda **k: _current_row("abc123") + [ + {"profile": "work", "pid": 43, "code_sha": "0" * 40, "code_version": "0.20.0", "state": "stale"}], + ) + restarts = _spy_fleet_restart(monkeypatch) + + with pytest.raises(SystemExit) as error: + hermes_main.cmd_update(args) + + assert error.value.code == 3 + assert len(restarts) == 1 + + +def test_second_profile_attaches_to_completed_host_restart(monkeypatch): + """One host process serves every profile: once its restart is stamped for this checkout, + another profile's completion skips the restart instead of killing the multiplexer again.""" + _patch_marker_sha(monkeypatch, "abc123") + monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", lambda **k: []) + update_cmd._write_fleet_restart_pending_marker(expected_sha="abc123") + assert update_cmd_fleet._fleet_restart_skip_reason(None) is None + + host_obligation.mark_host_restart_completed("abc123") + assert update_cmd_fleet._fleet_restart_skip_reason(None) + + # A stamp for other code proves nothing about this checkout. + host_obligation.mark_host_restart_completed("def456") + assert update_cmd_fleet._fleet_restart_skip_reason(None) is None