fix(update): reload process-scan modules at the dashboard-cleanup entry point
Widen PR #87757 to cover the ZIP path: _update_via_zip() also calls _finish_dashboard_update_cleanup() but never runs _reload_config_modules, so the Windows git-broken fallback would still crash with the same ImportError (cannot import name 'bounded_probe_run' from the stale cached hermes_cli._subprocess_compat). - new _reload_process_scan_modules() called inside _finish_dashboard_update_cleanup itself, so every current and future call site is covered; reloads dependency-first (_subprocess_compat, then dashboard_procs) - reload failures log at warning (a miss surfaces seconds later as an ImportError in the same process) - regression tests: reload-before-kill ordering, node-failure skip, stale-module symbol restoration (the exact #87134 boundary state), nonfatal reload failure, and the #87757 reload-list contract
This commit is contained in:
@@ -0,0 +1 @@
|
||||
yukinomon
|
||||
@@ -632,6 +632,43 @@ def _format_time_ago(iso_ts: str) -> str:
|
||||
except Exception:
|
||||
return "recently"
|
||||
|
||||
def _reload_process_scan_modules() -> None:
|
||||
"""Force-reload the process-scan modules from disk after an update.
|
||||
|
||||
``_finish_dashboard_update_cleanup`` runs in the PRE-update Python
|
||||
process, but ``_scan_dashboard_processes`` does a function-level
|
||||
``from hermes_cli._subprocess_compat import bounded_probe_run``. If the
|
||||
update added a new symbol to ``_subprocess_compat`` (as #87134 did with
|
||||
``bounded_probe_run``), the cached OLD module object doesn't have it and
|
||||
the cleanup step crashes with ImportError — after the code update itself
|
||||
already succeeded. Reload dependency-first so ``dashboard_procs`` binds
|
||||
against the fresh ``_subprocess_compat``.
|
||||
|
||||
Lives here (called from the cleanup entry point) rather than only in
|
||||
``_reload_config_modules`` so EVERY caller — the git-update path, the
|
||||
Windows ZIP fallback path, and any future one — is covered.
|
||||
"""
|
||||
import importlib
|
||||
|
||||
importlib.invalidate_caches()
|
||||
for mod_name in (
|
||||
"hermes_cli._subprocess_compat",
|
||||
"hermes_cli.dashboard_procs",
|
||||
):
|
||||
mod = sys.modules.get(mod_name)
|
||||
if mod is not None:
|
||||
try:
|
||||
importlib.reload(mod)
|
||||
except Exception as exc:
|
||||
# warning, not debug: a failed reload here surfaces seconds
|
||||
# later as an ImportError in the same process — leave a trail.
|
||||
logger.warning(
|
||||
"Could not reload %s for post-update cleanup: %s",
|
||||
mod_name,
|
||||
exc,
|
||||
)
|
||||
|
||||
|
||||
def _finish_dashboard_update_cleanup(node_failures: list[str]) -> None:
|
||||
"""Refresh managed dashboards or stop stale manual ones after an update."""
|
||||
if node_failures:
|
||||
@@ -640,6 +677,10 @@ def _finish_dashboard_update_cleanup(node_failures: list[str]) -> None:
|
||||
print(" Node.js dependency refresh did not complete.")
|
||||
return
|
||||
|
||||
# The scan path lazy-imports symbols from _subprocess_compat; make sure
|
||||
# both modules reflect the freshly-updated source before touching them.
|
||||
_reload_process_scan_modules()
|
||||
|
||||
stop_result = _m()._kill_stale_dashboard_processes(restart_managed=True)
|
||||
if not stop_result.get("unrecovered"):
|
||||
return
|
||||
|
||||
@@ -661,3 +661,87 @@ class TestCmdlineCapture:
|
||||
"""
|
||||
live = self._live()
|
||||
assert live._dashboard_cmdline_for_pid(123) is None
|
||||
|
||||
|
||||
class TestPostUpdateStaleModuleReload:
|
||||
"""Regression tests for the post-update stale-module ImportError.
|
||||
|
||||
``hermes update`` runs in the PRE-pull Python process. When the update
|
||||
adds a new symbol to ``hermes_cli._subprocess_compat`` (as #87134 added
|
||||
``bounded_probe_run``), the post-update dashboard cleanup's lazy
|
||||
``from hermes_cli._subprocess_compat import bounded_probe_run`` hits the
|
||||
stale cached module and crashes with ImportError — after the code update
|
||||
itself already succeeded. The cleanup entry point must force-reload the
|
||||
process-scan modules first (PR #87757 + ZIP-path widening).
|
||||
"""
|
||||
|
||||
def test_cleanup_reloads_before_scanning(self):
|
||||
"""_finish_dashboard_update_cleanup must reload the process-scan
|
||||
modules BEFORE calling _kill_stale_dashboard_processes, on every
|
||||
call path (git update and ZIP fallback both route here)."""
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
order: list[str] = []
|
||||
with patch.object(
|
||||
update_cmd, "_reload_process_scan_modules",
|
||||
side_effect=lambda: order.append("reload"),
|
||||
), patch(
|
||||
"hermes_cli.main._kill_stale_dashboard_processes",
|
||||
side_effect=lambda **kw: order.append("kill") or {"unrecovered": []},
|
||||
):
|
||||
update_cmd._finish_dashboard_update_cleanup([])
|
||||
|
||||
assert order == ["reload", "kill"]
|
||||
|
||||
def test_node_failures_skip_reload_and_kill(self):
|
||||
"""A failed Node refresh leaves the running dashboard untouched —
|
||||
no reload, no kill (existing safety rule preserved)."""
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
with patch.object(update_cmd, "_reload_process_scan_modules") as mock_reload, \
|
||||
patch("hermes_cli.main._kill_stale_dashboard_processes") as mock_kill:
|
||||
update_cmd._finish_dashboard_update_cleanup(["dashboard"])
|
||||
|
||||
mock_reload.assert_not_called()
|
||||
mock_kill.assert_not_called()
|
||||
|
||||
def test_reload_restores_missing_symbol(self):
|
||||
"""Simulate the stale-module state: strip ``bounded_probe_run`` off
|
||||
the cached module object (what an old pre-#87134 module looks like)
|
||||
and verify the reload restores it from disk — the exact state the
|
||||
Windows update crash came from."""
|
||||
import hermes_cli._subprocess_compat as compat
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
assert hasattr(compat, "bounded_probe_run")
|
||||
try:
|
||||
delattr(compat, "bounded_probe_run")
|
||||
assert not hasattr(compat, "bounded_probe_run")
|
||||
|
||||
update_cmd._reload_process_scan_modules()
|
||||
|
||||
stale = sys.modules["hermes_cli._subprocess_compat"]
|
||||
assert hasattr(stale, "bounded_probe_run")
|
||||
finally:
|
||||
importlib.reload(sys.modules["hermes_cli._subprocess_compat"])
|
||||
importlib.reload(sys.modules["hermes_cli.dashboard_procs"])
|
||||
|
||||
def test_reload_failure_is_nonfatal(self):
|
||||
"""A reload failure must log and continue, never raise — the cleanup
|
||||
step runs after the update already succeeded."""
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
with patch("importlib.reload", side_effect=RuntimeError("boom")):
|
||||
update_cmd._reload_process_scan_modules() # must not raise
|
||||
|
||||
def test_config_reload_list_includes_process_scan_modules(self):
|
||||
"""PR #87757's half: the git-path pre-cleanup reload also refreshes
|
||||
the process-scan modules (belt to the entry-point suspenders)."""
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
reloaded: list[str] = []
|
||||
with patch("importlib.reload", side_effect=lambda m: reloaded.append(m.__name__)):
|
||||
update_cmd._reload_config_modules()
|
||||
|
||||
assert "hermes_cli._subprocess_compat" in reloaded
|
||||
assert "hermes_cli.dashboard_procs" in reloaded
|
||||
|
||||
Reference in New Issue
Block a user