fix(update): reconcile serve/dashboard runtimes in their own vocabulary and escalate survivors (#100479)
Widen the two salvaged fixes (#100490, #100493) to the whole class: - match_runtime_outcomes: serve/dashboard rows never borrow gateway bookkeeping at ANY site — not just the bare hermes-gateway unit name (#100490) but also relaunched_profiles / externally_supervised_profiles and the profile-substring unit match (hermes-gateway-work credited the 'work' serve). They reconcile against hermes-serve*/hermes-dashboard* units (exact names, scope prefix tolerated) or, when the caller passes the (pid, create_time) survivor probe result, by incarnation liveness. - update_cmd success path: the survivor rows from #100493's new call now feed the Phase-2 reconciliation, so a surviving unmanaged serve is 'unaccounted' -> exit 1 + 'partial' receipt, not warn-and-exit-0. - report_unaccounted_runtimes: a serve/dashboard miss names the serve remedy instead of 'hermes gateway restart', which cannot reach it. Tests: 6 reconciliation cases (sibling sites, unit vocabulary, exact-name guard, incarnation probe, remedy text) + an end-to-end cmd_update case asserting warn + unaccounted + exit 1 + receipt runtime_outcomes.
This commit is contained in:
@@ -10815,7 +10815,18 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
)
|
||||
|
||||
# Check if any pre-update serve/dashboard runtimes survived on
|
||||
# pre-update code generations (#100479).
|
||||
# pre-update code generations (#100479). This is the SUCCESS-path
|
||||
# twin of the abort-recovery probe above: the restart phase only
|
||||
# restarts units, so an sshd-spawned `serve --isolated` or a manual
|
||||
# `hermes serve` (no unit) is left running its pre-update
|
||||
# sys.modules graph — and its cron ticker keeps firing agent jobs
|
||||
# that ImportError on every symbol added in the pulled range. Runs
|
||||
# AFTER the dashboard cleanup so a manual dashboard that cleanup
|
||||
# killed and respawned is (correctly) not a survivor. The rows also
|
||||
# feed the plan-vs-execution reconciliation below, so a survivor is
|
||||
# escalated (exit 1) instead of merely printed. ``None`` means the
|
||||
# probe itself failed; the reconciliation then stays fail-closed.
|
||||
_stale_serve_rows: "list | None" = None
|
||||
try:
|
||||
_stale_serve_rows = _surviving_pre_update_serve_runtimes(_pre_update_plan)
|
||||
if _stale_serve_rows:
|
||||
@@ -10932,6 +10943,13 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
externally_supervised_profiles=externally_supervised_profiles,
|
||||
killed_pids=killed_pids,
|
||||
failed_units=failed_or_stale_units,
|
||||
# Serve/dashboard runtimes reconcile by incarnation
|
||||
# liveness, not by the gateway's unit names (#100479).
|
||||
stale_serve_pids=(
|
||||
{row.get("pid") for row in _stale_serve_rows}
|
||||
if _stale_serve_rows is not None
|
||||
else None
|
||||
),
|
||||
)
|
||||
if report_unaccounted_runtimes(_runtime_outcomes):
|
||||
gateway_fleet_restart_incomplete = True
|
||||
|
||||
@@ -425,6 +425,49 @@ def print_update_plan(plan: UpdatePlan) -> None:
|
||||
)
|
||||
|
||||
|
||||
_SERVE_KINDS = ("serve", "dashboard")
|
||||
|
||||
|
||||
def _serve_unit_matches_profile(profile: str, unit: object) -> bool:
|
||||
"""Does *unit* name a ``hermes-serve*``/``hermes-dashboard*`` unit for *profile*?
|
||||
|
||||
Serve/dashboard runtimes have their OWN unit vocabulary; the gateway's
|
||||
``hermes-gateway*`` names never cover them (#100479). Exact names only —
|
||||
``work`` must not claim ``hermes-serve-workbench`` — and a scope prefix
|
||||
(``user/hermes-serve``) is tolerated because the restart phase records
|
||||
scope-qualified identities in some lists.
|
||||
"""
|
||||
name = str(unit).removesuffix(".service")
|
||||
if "/" in name:
|
||||
name = name.rsplit("/", 1)[-1]
|
||||
if profile == "default":
|
||||
return name in {"hermes-serve", "hermes-dashboard"}
|
||||
return name in {f"hermes-serve-{profile}", f"hermes-dashboard-{profile}"}
|
||||
|
||||
|
||||
def _serve_runtime_outcome(
|
||||
r: RuntimeRecord,
|
||||
*,
|
||||
killed: set,
|
||||
failed_set: set,
|
||||
restarted_set: set,
|
||||
stale_serves: "set | None",
|
||||
) -> str:
|
||||
"""Outcome for one serve/dashboard runtime — never the gateway's."""
|
||||
if r.pid is not None and r.pid in killed:
|
||||
return "stopped"
|
||||
if any(_serve_unit_matches_profile(r.profile, u) for u in failed_set):
|
||||
return "failed"
|
||||
if stale_serves is not None:
|
||||
# Incarnation-verified: the pre-update process is gone (replaced by
|
||||
# its unit / the dashboard cleanup respawn / the Desktop app) or it
|
||||
# is still alive on pre-update code.
|
||||
return "unaccounted" if r.pid in stale_serves else "restarted"
|
||||
if any(_serve_unit_matches_profile(r.profile, s) for s in restarted_set):
|
||||
return "restarted"
|
||||
return "unaccounted"
|
||||
|
||||
|
||||
def match_runtime_outcomes(
|
||||
plan: "UpdatePlan",
|
||||
*,
|
||||
@@ -433,6 +476,7 @@ def match_runtime_outcomes(
|
||||
externally_supervised_profiles: list,
|
||||
killed_pids: set,
|
||||
failed_units: list,
|
||||
stale_serve_pids: "set | None" = None,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Reconcile the plan's runtimes against what the restart phase DID.
|
||||
|
||||
@@ -450,6 +494,18 @@ def match_runtime_outcomes(
|
||||
``unaccounted`` — the plan saw it and NO bookkeeping mentions it: the
|
||||
blind-spot tripwire (same philosophy as the fleet matrix's DOWN row).
|
||||
Never raises; on any probe error returns what it has.
|
||||
|
||||
Serve/dashboard runtimes are reconciled in their OWN vocabulary
|
||||
(#100479): a ``hermes-serve*``/``hermes-dashboard*`` unit, a killed
|
||||
PID, or — when the caller passes ``stale_serve_pids`` (the
|
||||
``(pid, create_time)``-verified survivor probe,
|
||||
:func:`hermes_cli.update_abort_recovery._surviving_pre_update_serve_runtimes`)
|
||||
— liveness: a pre-update serve whose incarnation is gone was replaced
|
||||
(unit restart, dashboard cleanup respawn, Desktop respawn) and counts as
|
||||
``restarted``; one still alive is ``unaccounted``. They never borrow the
|
||||
gateway's outcome: ``relaunched_profiles`` and ``hermes-gateway*`` name a
|
||||
different process that shares the profile, nothing more. Without the
|
||||
probe result, an untouched serve stays ``unaccounted`` (fail closed).
|
||||
"""
|
||||
outcomes: list[dict[str, Any]] = []
|
||||
try:
|
||||
@@ -458,11 +514,31 @@ def match_runtime_outcomes(
|
||||
relaunched = set(relaunched_profiles or [])
|
||||
external = set(externally_supervised_profiles or [])
|
||||
killed = {int(p) for p in (killed_pids or set())}
|
||||
stale_serves = (
|
||||
{int(p) for p in stale_serve_pids} if stale_serve_pids is not None else None
|
||||
)
|
||||
|
||||
for runtime in plan.runtimes:
|
||||
r = runtime if isinstance(runtime, RuntimeRecord) else None
|
||||
if r is None:
|
||||
continue
|
||||
if r.kind in _SERVE_KINDS:
|
||||
outcomes.append(
|
||||
{
|
||||
"kind": r.kind,
|
||||
"profile": r.profile,
|
||||
"pid": r.pid,
|
||||
"mechanism": r.restart_via,
|
||||
"outcome": _serve_runtime_outcome(
|
||||
r,
|
||||
killed=killed,
|
||||
failed_set=failed_set,
|
||||
restarted_set=restarted_set,
|
||||
stale_serves=stale_serves,
|
||||
),
|
||||
}
|
||||
)
|
||||
continue
|
||||
outcome = "unaccounted"
|
||||
# The bare "hermes-gateway" unit name is gateway-specific: a
|
||||
# serve/dashboard runtime that merely shares the default
|
||||
@@ -525,8 +601,14 @@ def report_unaccounted_runtimes(outcomes: list[dict[str, Any]]) -> bool:
|
||||
f" — planned mechanism: {o['mechanism']}"
|
||||
)
|
||||
print(" Restart them manually, then verify:")
|
||||
print(" hermes gateway restart # active profile")
|
||||
print(" hermes -p <profile> gateway restart # named profile")
|
||||
if any(o.get("kind") not in _SERVE_KINDS for o in missed):
|
||||
print(" hermes gateway restart # active profile")
|
||||
print(" hermes -p <profile> gateway restart # named profile")
|
||||
if any(o.get("kind") in _SERVE_KINDS for o in missed):
|
||||
# A serve/dashboard is not reachable by any `gateway restart`
|
||||
# command (#100479): name the process, not the wrong verb.
|
||||
print(" systemctl --user restart hermes-serve.service # unit-managed serve")
|
||||
print(" relaunch `hermes serve` / `hermes dashboard` / the Desktop app")
|
||||
return True
|
||||
|
||||
|
||||
|
||||
@@ -181,6 +181,102 @@ def test_unmanaged_serve_runtime_under_default_profile_is_unaccounted():
|
||||
assert report_unaccounted_runtimes(outcomes) is True
|
||||
|
||||
|
||||
def _serve(profile: str, pid: int, kind: str = "serve") -> RuntimeRecord:
|
||||
return RuntimeRecord(
|
||||
kind=kind,
|
||||
profile=profile,
|
||||
pid=pid,
|
||||
supervisor="manual-serve",
|
||||
restart_via=_restart_mechanism("manual-serve", profile),
|
||||
)
|
||||
|
||||
|
||||
def test_serve_never_borrows_relaunched_or_external_gateway_profile():
|
||||
"""Sibling site of #100479: the relaunched_profiles / external-supervisor
|
||||
bookkeeping is gateway vocabulary too. A manual gateway relaunch under
|
||||
``default`` (or a named profile) says nothing about a serve that shares
|
||||
the profile name."""
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(_rt("default", 100), _serve("default", 900),
|
||||
_rt("work", 101), _serve("work", 901, kind="dashboard")),
|
||||
restarted_services=[], relaunched_profiles=["default"],
|
||||
externally_supervised_profiles=["work"], killed_pids=set(), failed_units=[],
|
||||
)
|
||||
by_pid = {o["pid"]: o["outcome"] for o in outcomes}
|
||||
assert by_pid == {
|
||||
100: "restarted", 900: "unaccounted", 101: "restarted", 901: "unaccounted"
|
||||
}
|
||||
|
||||
|
||||
def test_named_profile_serve_does_not_match_gateway_profile_unit():
|
||||
"""``hermes-gateway-work.service`` restarted must not credit the ``work``
|
||||
serve — the old substring match (``"work" in unit``) did exactly that."""
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(_rt("work", 101, supervisor="systemd"), _serve("work", 901)),
|
||||
restarted_services=["hermes-gateway-work.service"], relaunched_profiles=[],
|
||||
externally_supervised_profiles=[], killed_pids=set(), failed_units=[],
|
||||
)
|
||||
by_pid = {o["pid"]: o["outcome"] for o in outcomes}
|
||||
assert by_pid == {101: "restarted", 901: "unaccounted"}
|
||||
|
||||
|
||||
def test_serve_reconciles_against_its_own_unit_vocabulary():
|
||||
"""A serve IS covered when a ``hermes-serve*`` unit for its profile was
|
||||
restarted (or failed) — scope-qualified identities included."""
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(_serve("default", 900), _serve("work", 901),
|
||||
_serve("ops", 902, kind="dashboard"), _serve("qa", 903)),
|
||||
restarted_services=["hermes-gateway", "user/hermes-serve",
|
||||
"hermes-serve-work.service", "hermes-dashboard-ops"],
|
||||
relaunched_profiles=[], externally_supervised_profiles=[],
|
||||
killed_pids=set(), failed_units=["hermes-serve-qa.service"],
|
||||
)
|
||||
by_pid = {o["pid"]: o["outcome"] for o in outcomes}
|
||||
assert by_pid == {900: "restarted", 901: "restarted", 902: "restarted", 903: "failed"}
|
||||
# exact names: ``work`` must not claim ``hermes-serve-workbench``
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(_serve("work", 901)),
|
||||
restarted_services=["hermes-serve-workbench.service"], relaunched_profiles=[],
|
||||
externally_supervised_profiles=[], killed_pids=set(), failed_units=[],
|
||||
)
|
||||
assert outcomes[0]["outcome"] == "unaccounted"
|
||||
|
||||
|
||||
def test_serve_outcome_follows_incarnation_probe_when_provided():
|
||||
"""With the (pid, create_time) survivor probe result, liveness decides:
|
||||
a pre-update serve that is gone was replaced (restarted); one still
|
||||
alive is unaccounted — even when a hermes-serve unit was restarted."""
|
||||
plan = _plan(_serve("default", 900), _serve("default", 901, kind="dashboard"))
|
||||
outcomes = match_runtime_outcomes(
|
||||
plan, restarted_services=["hermes-serve.service"], relaunched_profiles=[],
|
||||
externally_supervised_profiles=[], killed_pids=set(), failed_units=[],
|
||||
stale_serve_pids={900},
|
||||
)
|
||||
by_pid = {o["pid"]: o["outcome"] for o in outcomes}
|
||||
assert by_pid == {900: "unaccounted", 901: "restarted"}
|
||||
# killed pid still wins as "stopped"; probe None => fail closed
|
||||
outcomes = match_runtime_outcomes(
|
||||
plan, restarted_services=[], relaunched_profiles=[],
|
||||
externally_supervised_profiles=[], killed_pids={901}, failed_units=[],
|
||||
stale_serve_pids=None,
|
||||
)
|
||||
by_pid = {o["pid"]: o["outcome"] for o in outcomes}
|
||||
assert by_pid == {900: "unaccounted", 901: "stopped"}
|
||||
|
||||
|
||||
def test_unaccounted_serve_report_names_serve_remedy_not_gateway_restart(capsys):
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(_serve("default", 900)),
|
||||
restarted_services=["hermes-gateway"], relaunched_profiles=[],
|
||||
externally_supervised_profiles=[], killed_pids=set(), failed_units=[],
|
||||
)
|
||||
assert report_unaccounted_runtimes(outcomes) is True
|
||||
out = capsys.readouterr().out
|
||||
assert "serve [default] pid 900" in out
|
||||
assert "hermes-serve.service" in out
|
||||
assert "hermes gateway restart" not in out
|
||||
|
||||
|
||||
def test_mixed_fleet_only_the_missed_one_escalates(capsys):
|
||||
outcomes = match_runtime_outcomes(
|
||||
_plan(
|
||||
|
||||
@@ -334,6 +334,70 @@ def test_clean_update_warns_about_surviving_pre_update_serve_runtime(
|
||||
assert "pre-update code" in out
|
||||
|
||||
|
||||
def test_clean_update_escalates_surviving_serve_as_unaccounted(
|
||||
monkeypatch, tmp_path, capsys
|
||||
):
|
||||
"""#100479 end to end: the plan inventoried a gateway (restarted through
|
||||
``hermes-gateway.service``) and an unmanaged ``serve`` on the same
|
||||
default profile. The serve survives the update as the SAME process, so
|
||||
the update must (1) warn, (2) reconcile it as ``unaccounted`` instead of
|
||||
borrowing the gateway's restart, and (3) exit 1 with a ``partial``
|
||||
receipt — not print a clean success."""
|
||||
from hermes_cli.update_inventory import (
|
||||
RuntimeRecord, UpdatePlan, _restart_mechanism,
|
||||
)
|
||||
import hermes_cli.update_inventory as ui
|
||||
|
||||
args = _update_args()
|
||||
_patch_update_deps(monkeypatch, tmp_path, _make_head_moved_side_effect())
|
||||
|
||||
plan = UpdatePlan()
|
||||
plan.runtimes = [
|
||||
RuntimeRecord(kind="gateway", profile="default", pid=4444,
|
||||
supervisor="systemd",
|
||||
restart_via=_restart_mechanism("systemd", "default")),
|
||||
RuntimeRecord(kind="serve", profile="default", pid=5555,
|
||||
supervisor="manual-serve",
|
||||
restart_via=_restart_mechanism("manual-serve", "default"),
|
||||
detail={"create_time": 1000.0}),
|
||||
]
|
||||
monkeypatch.setattr(ui, "collect_runtime_inventory", lambda: plan)
|
||||
# The restart phase's own bookkeeping says the gateway unit restarted
|
||||
# (systemd branch is stubbed off in _patch_update_deps, so feed it here).
|
||||
real_match = ui.match_runtime_outcomes
|
||||
|
||||
def _match(p, **kw):
|
||||
kw["restarted_services"] = list(kw.get("restarted_services") or []) + [
|
||||
"hermes-gateway.service"
|
||||
]
|
||||
return real_match(p, **kw)
|
||||
|
||||
monkeypatch.setattr(ui, "match_runtime_outcomes", _match)
|
||||
# Real survivor probe semantics against a fake ledger: pid 5555 is still
|
||||
# the same incarnation the plan recorded.
|
||||
import hermes_cli.process_identity as pi
|
||||
|
||||
monkeypatch.setattr(
|
||||
pi, "ledger_entries",
|
||||
lambda **_k: [{"pid": 5555, "purpose": "serve", "create_time": 1000.0}],
|
||||
)
|
||||
|
||||
with pytest.raises(SystemExit) as excinfo:
|
||||
hermes_main.cmd_update(args)
|
||||
assert excinfo.value.code == 1
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "pid 5555" in out and "pre-update code" in out
|
||||
assert "Planned runtimes the restart phase never touched" in out
|
||||
assert "serve [default] pid 5555" in out
|
||||
|
||||
latest = get_hermes_home() / "logs" / "update_receipts" / "latest.json"
|
||||
receipt = json.loads(latest.read_text(encoding="utf-8"))
|
||||
assert receipt["outcome"] == "partial"
|
||||
by_pid = {o["pid"]: o["outcome"] for o in receipt["runtime_outcomes"]}
|
||||
assert by_pid == {4444: "restarted", 5555: "unaccounted"}
|
||||
|
||||
|
||||
def test_interrupt_between_pull_and_restart_leaves_marker(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user