diff --git a/contributors/emails/yukinomon@users.noreply.github.com b/contributors/emails/yukinomon@users.noreply.github.com new file mode 100644 index 0000000000..43ee4d7d5c --- /dev/null +++ b/contributors/emails/yukinomon@users.noreply.github.com @@ -0,0 +1 @@ +yukinomon diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index 0b6cf10de8..57cff41fdc 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -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 diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index dd3a8c5fbc..b6dde9bcfe 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -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