fix(update): warn surviving pre-update serve and dashboard runtimes on success (#100479)

[twotnguyen]

What does this PR do?

On the clean success path of hermes update, _surviving_pre_update_serve_runtimes(_pre_update_plan) and _warn_stale_serve_runtimes() were never called (they were only wired into the abort recovery except Exception: block).

As a result, non-unit serve and dashboard runtimes (such as Desktop SSH serve --isolated or manual CLI serve sessions with no systemd unit) that survived the update continue running on pre-update sys.modules graphs without warning the user. Because these serve processes run cron tickers, later agent-mode cron runs fail with ImportError on symbols added in the update.

This PR adds the check for surviving serve/dashboard runtimes on the normal update path after dashboard cleanup, reporting any stale processes and the commands to restart them.

Related Issue

Fixes #100479

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_cli/update_cmd.py: Call _surviving_pre_update_serve_runtimes(_pre_update_plan) and _warn_stale_serve_runtimes() on the normal update path so non-unit serve/dashboard runtimes surviving an update are reported.
• tests/hermes_cli/test_update_fleet_restart_pending.py: Added an update-path regression test that drives cmd_update through a successful mocked pull and asserts the stale-runtime warning.

How to Test

1. Run the update-path regression test:
   python -m pytest tests/hermes_cli/test_update_fleet_restart_pending.py::test_clean_update_warns_about_surviving_pre_update_serve_runtime
2. Run the relevant suites in separate pytest processes:
   python -m pytest tests/hermes_cli/test_update_serve_generation_recovery.py
python -m pytest tests/hermes_cli/test_update_fleet_restart_pending.py

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/hermes_cli/test_update_serve_generation_recovery.py and tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11 / Python 3.11.15

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[teamster22]

Thanks for the fast turnaround — I verified the branch on a clone and the production change is correct. One issue with the test, though: it passes with the fix reverted, so it does not guard this regression.

The production fix is right

_surviving_pre_update_serve_runtimes + _warn_stale_serve_runtimes at the new call site sit in _cmd_update_impl (lines 7921–10832), outside every except handler — confirmed by walking the AST for enclosing handler ranges, not by reading. That is exactly the placement the issue asked for, and _pre_update_plan is assigned at 7995/8002, well before the call, so the argument is populated on the clean path.

The regression test is vacuous

test_update_warns_surviving_serve_runtimes_on_clean_path calls the two helpers directly:

stale_rows = update_cmd._surviving_pre_update_serve_runtimes(plan)
update_cmd._warn_stale_serve_runtimes(stale_rows)

Both helpers already existed and already worked — they were merged in #92145. Their behavior was never the bug. The bug is that nothing on the success path calls them. A test that invokes them itself asserts the part that was never broken.

Verified on the branch:

$ git checkout HEAD~1 -- hermes_cli/update_cmd.py   # revert prod fix, keep the new test
$ pytest -k test_update_warns_surviving_serve_runtimes_on_clean_path
1 passed, 59 deselected

Green with the fix reverted. The suite is 60 passed either way, so nothing in it would catch a future refactor that drops the call — the same "only reachable from except" state this issue reported, which #92145's own suite also failed to catch for the same reason.

Despite the test name, nothing in it exercises an update path: no _cmd_update_impl, no git mock, no capsys assertion against a real update run.

A guard that fails without the fix

The property worth pinning is reachability — the survivor check is called from somewhere that is not an exception handler. That is assertable statically, with no git/network mocking:

import ast

def test_clean_update_path_calls_surviving_serve_check():
    src = open("hermes_cli/update_cmd.py").read()
    tree = ast.parse(src)
    fn = next(n for n in ast.walk(tree)
              if isinstance(n, ast.FunctionDef) and n.name == "_cmd_update_impl")

    handlers = [(h.lineno, h.end_lineno)
                for h in ast.walk(fn) if isinstance(h, ast.ExceptHandler)]

    calls = [n.lineno for n in ast.walk(fn)
             if isinstance(n, ast.Call)
             and (getattr(n.func, "id", None) or getattr(n.func, "attr", None))
                 == "_surviving_pre_update_serve_runtimes"]
    assert calls, "survivor check is never called in _cmd_update_impl"

    outside = [ln for ln in calls
               if not any(a <= ln <= b for a, b in handlers)]
    assert outside, (
        "survivor check is only reachable from an except handler; "
        "a successful update never reports stale serve runtimes (#100479)"
    )

Both directions verified on your branch:

State	Result
PR as-is	1 passed
git checkout HEAD~1 -- hermes_cli/update_cmd.py	1 failed — assert []

Full suites still green with it added: test_update_serve_generation_recovery.py + test_cmd_update.py = 107 passed.

An end-to-end test driving _cmd_update_impl through a mocked git (the _make_head_moved_side_effect pattern in test_update_fleet_restart_pending.py) and asserting the warning text via capsys would be stronger still, since it would also cover the call surviving a refactor that renames the helper. The AST guard is the cheap version that at least fails when the call is removed. Either is fine by me — please take whichever you prefer, no attribution needed.

Happy to test another push.

---

Reported from a 944-commit v0.20.6 → v0.21.0 update. Verification ran against a local clone of fix/warn-surviving-serve-runtimes-on-update @ 3d5ecc574; the live install was never modified.

[twotnguyen]

Addressed in 3c959d4438.

The helper-only test has been replaced with an update-path regression test in test_update_fleet_restart_pending.py. It drives cmd_update through a successful mocked HEAD advance and asserts the operator-facing stale serve warning from the real success path.

Mutation check:

• PR call site present: 1 passed
• exact production call site removed: test fails at assert "pid 5555" in out
• call site restored: 1 passed

Fresh relevant suites, run in separate pytest processes because the update recovery suite intentionally purges cached Hermes modules:

• test_update_serve_generation_recovery.py: 59 passed
• test_update_fleet_restart_pending.py: 14 passed

The branch was also rebased onto current main. The PR body now points to the new regression test.

[teknium1]

Merged via #100928 (2b7132c) with your commit and authorship preserved, @twotnguyen. Thanks!