fix(update): a successful update no longer credits or ignores surviving unmanaged serve runtimes (#100479, salvage #100490 + #100493)

contributors/emails/nguyenngoctinh011258@gmail.com

@@ -0,0 +1 @@
+twotnguyen

hermes_cli/update_cmd.py

@@ -10814,6 +10814,26 @@ def _on_unit_timeout(svc_name: str, exc: subprocess.TimeoutExpired) -> None:
             node_failures, already_restarted_units=set(restarted_services)
         )
 
+        # Check if any pre-update serve/dashboard runtimes survived on
+        # 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:
+                _warn_stale_serve_runtimes(_stale_serve_rows)
+        except Exception as _serve_warn_exc:
+            logger.debug("Failed to check for surviving serve runtimes: %s", _serve_warn_exc)
+
         print()
         print("Tip: You can now select a provider and model:")
         print("  hermes model              # Select provider and model")
@@ -10923,6 +10943,13 @@ def _on_unit_timeout(svc_name: str, exc: subprocess.TimeoutExpired) -> None:
                     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

hermes_cli/update_inventory.py

@@ -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,23 +514,57 @@ 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
+            # profile is a different process the gateway restart never
+            # touched, and must not borrow its outcome (#100479).
             if r.profile in relaunched or r.profile in external:
                 outcome = "restarted"
             elif r.pid is not None and r.pid in killed:
                 outcome = "stopped"
             elif any(
-                r.profile in unit or (r.profile == "default" and "hermes-gateway" in unit)
+                r.profile in unit
+                or (
+                    r.kind == "gateway"
+                    and r.profile == "default"
+                    and "hermes-gateway" in unit
+                )
                 for unit in failed_set
             ):
                 outcome = "failed"
             elif any(
-                r.profile in svc or (r.profile == "default" and "hermes-gateway" in svc)
+                r.profile in svc
+                or (
+                    r.kind == "gateway"
+                    and r.profile == "default"
+                    and "hermes-gateway" in svc
+                )
                 for svc in restarted_set
             ):
                 outcome = "restarted"
@@ -511,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
 
 

tests/hermes_cli/test_restart_plan_reconciliation.py

@@ -159,6 +159,124 @@ def test_external_supervisor_counts_as_restarted():
     assert outcomes[0]["outcome"] == "restarted"
 
 
+def test_unmanaged_serve_runtime_under_default_profile_is_unaccounted():
+    """#100479: an sshd-spawned `serve --isolated` has no systemd unit and
+    shares the default profile with the gateway. A gateway-only restart
+    must not be read as covering it — it must trip the tripwire instead."""
+    serve_runtime = RuntimeRecord(
+        kind="serve",
+        profile="default",
+        pid=900,
+        supervisor="manual-serve",
+        restart_via=_restart_mechanism("manual-serve", "default"),
+    )
+    outcomes = match_runtime_outcomes(
+        _plan(_rt("default", 100, supervisor="systemd"), serve_runtime),
+        restarted_services=["hermes-gateway"], relaunched_profiles=[],
+        externally_supervised_profiles=[], killed_pids=set(), failed_units=[],
+    )
+    by_pid = {o["pid"]: o["outcome"] for o in outcomes}
+    assert by_pid[100] == "restarted"
+    assert by_pid[900] == "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(