diff --git a/gateway/restart.py b/gateway/restart.py index a4a5bcee6c..742519cddb 100644 --- a/gateway/restart.py +++ b/gateway/restart.py @@ -356,11 +356,15 @@ def resolve_systemd_timeout_stop_sec( def resolve_restart_exit_wait_budget( - drain_timeout: float, after_turn_timeout: float, cron_drain_timeout: float = 0.0, *, headroom: float = 15.0, + drain_timeout: float, after_turn_timeout: float, cron_drain_timeout: float, *, headroom: float = 15.0, ) -> float: - """Observer budget for in-band deferral, the full stop envelope, and replacement startup. + """Seconds a CLI waits for the gateway PID to exit after SIGUSR1: the in-band after-turn + deferral, then the longest supervisor stop envelope — the one systemd's ``TimeoutStopSec`` is + sized from (cron drain + cleanup reserve, supervisor headroom, floor; #94759) — then + observer ``headroom``. - The stop envelope includes cron cleanup reserve, supervisor headroom and the floor; - deferral precedes it, while observer headroom follows it. Cron zero opts out. + A non-finite drain means "wait indefinitely", not a crash in the integer envelope. """ + if not all(math.isfinite(_seconds(value)) for value in (drain_timeout, cron_drain_timeout)): + return math.inf return _seconds(after_turn_timeout) + resolve_systemd_timeout_stop_sec(drain_timeout, cron_drain_timeout) + _seconds(headroom) diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 1d379af8a5..82723f9afc 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -264,8 +264,8 @@ def _graceful_restart_via_sigusr1(pid: int, drain_timeout: float, *, on_progress """SIGUSR1 (drain-aware restart) a gateway PID and wait for exit; False if unsent or it outlived the timeout. gateway/run.py maps SIGUSR1 to ``request_restart(via_service=True)``: refuse new turns, drain, - ``stop()``, exit; the supervisor relaunches. ``drain_timeout`` must cover after-turn wait + drain - — pass ``resolve_restart_exit_wait_budget(...)``. ``on_progress`` (zero-arg) runs on every poll so + ``stop()``, exit; the supervisor relaunches. ``drain_timeout`` must cover after-turn wait + the full stop + envelope — pass ``resolve_restart_exit_wait_budget(...)``. ``on_progress`` (zero-arg) runs on every poll so a long wait can report what the gateway is still holding for (``update_cmd_drain_report``). """ if not hasattr(signal, "SIGUSR1") or pid <= 0: diff --git a/hermes_cli/update_cmd_drain_report.py b/hermes_cli/update_cmd_drain_report.py index bace46b779..7e386e501e 100644 --- a/hermes_cli/update_cmd_drain_report.py +++ b/hermes_cli/update_cmd_drain_report.py @@ -81,7 +81,7 @@ def read_active_work(home: Optional[Path] = None) -> Optional[list]: def format_drain_report(work: Optional[list], *, remaining_s: float, home: Optional[Path] = None) -> str: """Multi-line progress block: what the gateway is waiting on plus how to stop waiting.""" - lines = [f" ⏳ still draining — {int(max(remaining_s, 0))}s left before the forced restart"] + lines = [f" ⏳ still draining — {max(remaining_s, 0):.0f}s left before the forced restart"] if work is None: lines.append(" (gateway did not report what it is waiting on — pre-update gateway or unreadable state file)") elif not work: diff --git a/hermes_cli/update_cmd_fleet.py b/hermes_cli/update_cmd_fleet.py index 0075ba5835..31e9b77869 100644 --- a/hermes_cli/update_cmd_fleet.py +++ b/hermes_cli/update_cmd_fleet.py @@ -1003,7 +1003,7 @@ def _restart_macos_launchd_gateways( continue # A profile without an installed job has no restart target. graceful_ok = False if old_pid is not None and old_pid > 0: - print(f" → {label}: draining (up to {int(drain_budget)}s)...") + print(f" → {label}: draining (up to {drain_budget:.0f}s)...") from hermes_cli.update_cmd_drain_report import drain_progress_reporter graceful_ok = _graceful_restart_via_sigusr1( old_pid, drain_timeout=drain_budget, @@ -1179,7 +1179,7 @@ def _drain_or_signal_gateway_for_update( print(f" ⚠ {label}: gateway event loop is unresponsive — skipping drain, forcing a bounded stop...") _escalate_wedged_gateway(pid) return True - print(f" → {label}: draining (up to {int(drain_budget)}s)...") + print(f" → {label}: draining (up to {drain_budget:.0f}s)...") from hermes_cli.update_cmd_drain_report import drain_progress_reporter return _graceful_restart_via_sigusr1( pid, drain_timeout=drain_budget, diff --git a/tests/gateway/test_restart_after_turn.py b/tests/gateway/test_restart_after_turn.py index b79a8b0cf1..86d9e24f51 100644 --- a/tests/gateway/test_restart_after_turn.py +++ b/tests/gateway/test_restart_after_turn.py @@ -1,5 +1,7 @@ """Unit tests for in-band restart after-turn deferral helpers (#77184).""" +import math + from gateway.restart import ( DEFAULT_GATEWAY_RESTART_AFTER_TURN_TIMEOUT, parse_restart_after_turn_timeout, @@ -18,18 +20,13 @@ def test_parse_restart_after_turn_timeout_defaults_and_clamps(): assert parse_restart_after_turn_timeout("120") == 120.0 - - -def test_resolve_restart_exit_wait_budget_covers_both_phases(): - assert resolve_restart_exit_wait_budget(0, 0, 0, headroom=15) == resolve_systemd_timeout_stop_sec(0, 0) + 15 - assert resolve_restart_exit_wait_budget(180, 21600, 0, headroom=15) == 21600 + resolve_systemd_timeout_stop_sec(180, 0) + 15 - for chat, cron in ((2, 80), (80, 2), (2, 0), (0, 0)): - stop_envelope = resolve_systemd_timeout_stop_sec(chat, cron) - assert resolve_restart_exit_wait_budget(chat, 3, cron, headroom=15) == 3 + stop_envelope + 15 - # A bounded observer uses the longer stop path, not the sum of independent drains. - assert resolve_restart_exit_wait_budget(80, 3, 80, headroom=15) == 3 + resolve_systemd_timeout_stop_sec(80, 80) + 15 - assert resolve_restart_exit_wait_budget(2, 3, 0, headroom=15) < resolve_restart_exit_wait_budget(2, 3, 80, headroom=15) - assert resolve_restart_exit_wait_budget("bad", "bad", 0, headroom="x") == 60.0 +def test_restart_exit_wait_budget_outlasts_deferral_plus_stop_envelope(): + for chat, after_turn, cron in ((0, 0, 0), (0, 1800, 30), (2, 3, 80), (80, 3, 2), (180, 21600, 0)): + # The observer must never give up before the supervisor's own stop deadline would. + assert resolve_restart_exit_wait_budget(chat, after_turn, cron) > after_turn + resolve_systemd_timeout_stop_sec(chat, cron) + # A non-finite drain waits indefinitely instead of crashing the integer envelope. + assert resolve_restart_exit_wait_budget(float("inf"), 0, 0) == math.inf + assert resolve_restart_exit_wait_budget(0, 0, float("inf")) == math.inf def test_cli_restart_wait_covers_configured_cron_drain(tmp_path, monkeypatch): @@ -38,16 +35,13 @@ def test_cli_restart_wait_covers_configured_cron_drain(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) for key in ("HERMES_RESTART_DRAIN_TIMEOUT", "HERMES_RESTART_AFTER_TURN_TIMEOUT", "HERMES_CRON_DRAIN_TIMEOUT"): monkeypatch.delenv(key, raising=False) - (tmp_path / "config.yaml").write_text( - "agent:\n restart_drain_timeout: 2\n restart_after_turn_timeout: 3\n cron_drain_timeout: 80\n", - encoding="utf-8", - ) - assert gateway_cli._get_restart_exit_wait_budget() == 3 + resolve_systemd_timeout_stop_sec(2, 80) + 15 - (tmp_path / "config.yaml").write_text( - "agent:\n restart_drain_timeout: 2\n restart_after_turn_timeout: 3\n cron_drain_timeout: 0\n", - encoding="utf-8", - ) - assert gateway_cli._get_restart_exit_wait_budget() == 3 + resolve_systemd_timeout_stop_sec(2, 0) + 15 + config = tmp_path / "config.yaml" + config.write_text("agent:\n restart_drain_timeout: 2\n restart_after_turn_timeout: 3\n cron_drain_timeout: 80\n") + with_cron = gateway_cli._get_restart_exit_wait_budget() + config.write_text("agent:\n restart_drain_timeout: 2\n restart_after_turn_timeout: 3\n cron_drain_timeout: 0\n") + # The configured cron drain reaches the CLI wait, which outlasts the stop it can take. + assert with_cron > 3 + resolve_systemd_timeout_stop_sec(2, 80) + assert with_cron > gateway_cli._get_restart_exit_wait_budget() def test_load_restart_after_turn_timeout_preserves_zero(tmp_path, monkeypatch): diff --git a/tests/hermes_cli/test_fleet_matrix_self_restart_pending.py b/tests/hermes_cli/test_fleet_matrix_self_restart_pending.py index 7ecbaccd92..1a09291130 100644 --- a/tests/hermes_cli/test_fleet_matrix_self_restart_pending.py +++ b/tests/hermes_cli/test_fleet_matrix_self_restart_pending.py @@ -55,10 +55,8 @@ def test_ancestor_with_accepted_self_restart_is_pending_while_other_stale_rows_s # Without the sibling, the pending row alone is not a failure — and the same pid NOT recorded # as pending (the restart phase never reached the ancestor branch) is a plain stale verdict. only_ancestor = [row for row in fleet if row["profile"] == "default"] - with contextlib.redirect_stdout(io.StringIO()) as pending_out: + with contextlib.redirect_stdout(io.StringIO()): assert ur.print_fleet_version_matrix(only_ancestor) is False - assert "as soon as this update exits" not in pending_out.getvalue() - assert "restart pending" in pending_out.getvalue() plain = ur.collect_fleet_versions(pre_restart_pids=[ancestor, sibling]) assert {row["state"] for row in plain} == {"stale"}