From af26acab737db4382815aadcd73c06755471ab52 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:19:00 +0530 Subject: [PATCH] fix(deps): gate the runtime takeover on genuinely historical updater frames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The salvaged matcher accepted any cmd_update/_cmd_update_impl frame in hermes_cli.update_cmd/main, so the CURRENT updater's entrypoints (which carry pre_update_version on opts, not as a local) would also hand off and exit the process mid-`hermes update`. Extract _old_updater.in_historical_update() sharing _historical_context's exact frame matcher (module, name, and pre_update_version in co_varnames) so the shim and the context detector cannot drift apart. - tools/lazy_deps: use the shared predicate; accept the constraints/ dry_run kwargs historical updaters pass so they reach the handoff instead of dying on TypeError; drop the vague message advice. - tests: collapse 29 parametrize cases to the real invariants and add the missing one — the frame walk is now exercised through a nested helper (a getframe(1)-only implementation fails), and a current-updater-shaped frame (no pre_update_version local) is asserted NOT to hand off. --- hermes_cli/_old_updater.py | 39 +++++++++++++++++++-- tests/hermes_cli/test_old_updater_shims.py | 26 +++++++++----- tests/tools/test_lazy_deps_runtime.py | 40 +--------------------- tools/lazy_deps.py | 23 +++++-------- 4 files changed, 63 insertions(+), 65 deletions(-) diff --git a/hermes_cli/_old_updater.py b/hermes_cli/_old_updater.py index 6b2b7f4187..1e4f264081 100644 --- a/hermes_cli/_old_updater.py +++ b/hermes_cli/_old_updater.py @@ -17,6 +17,41 @@ from typing import Any, NoReturn _result: int | None = None +# The historical updater entrypoints. `_historical_context` reads capture data +# from these frames; `tools.lazy_deps.install_specs` hands off only inside them. +# Keep in sync with scripts/audit-old-updater-imports.py UPDATE_ENTRYPOINTS: the +# audit list is wider on purpose (helpers run *under* these frames and are found +# by the frame walk). +_HISTORICAL_UPDATE_FRAMES = ( + ("hermes_cli.update_cmd", "hermes_cli.main"), + ("_cmd_update_impl", "cmd_update"), +) + + +def _is_historical_update_frame(frame: Any) -> bool: + modules, names = _HISTORICAL_UPDATE_FRAMES + return (frame.f_globals.get("__name__") in modules + and frame.f_code.co_name in names) + + +def in_historical_update(depth: int = 1) -> bool: + """True when an ancestor frame is a historical updater entrypoint. + + Only the old on-disk updaters define these entrypoints with + ``pre_update_version`` as a local; code loaded in a CURRENT tree must keep + running instead of handing control to a takeover child. + """ + frame = sys._getframe(depth + 1) + try: + while frame is not None: + if (_is_historical_update_frame(frame) + and "pre_update_version" in frame.f_code.co_varnames): + return True + frame = frame.f_back + finally: + del frame + return False + def _historical_context() -> tuple[dict, list[dict], Any]: """Carry data already held by old frames, without importing their modules. @@ -35,9 +70,7 @@ def _historical_context() -> tuple[dict, list[dict], Any]: # Capture hooks can be imported BEFORE the old updater has pulled. # A declared-but-unassigned version in its innermost known frame # proves early entry; a None value is still a post-capture value. - if (restart_update is None - and frame.f_globals.get("__name__") in ("hermes_cli.update_cmd", "hermes_cli.main") - and frame.f_code.co_name in ("_cmd_update_impl", "cmd_update") + if (restart_update is None and _is_historical_update_frame(frame) and "pre_update_version" in frame.f_code.co_varnames): restart_update = "pre_update_version" not in frame.f_locals for name in names: diff --git a/tests/hermes_cli/test_old_updater_shims.py b/tests/hermes_cli/test_old_updater_shims.py index 522ceaecea..51c5ed70bc 100644 --- a/tests/hermes_cli/test_old_updater_shims.py +++ b/tests/hermes_cli/test_old_updater_shims.py @@ -98,7 +98,6 @@ from tests.compat.old_updater_support import ( ("hermes_cli.update_cmd", "get_default_hermes_root", (), {}, None), ("hermes_cli.tools_config", "_pip_install", (["--quiet", "honcho-ai"],), {}, None), ("hermes_cli.tools_config", "_pip_install", (["--quiet", "honcho-ai"],), {"timeout": 120, "capture_output": False}, None), - ], ) def test_retired_dependency_entrypoints_handoff_without_fallback(module, name, args, kwargs, cached, fresh_child, monkeypatch): @@ -120,19 +119,28 @@ def test_retired_dependency_entrypoints_handoff_without_fallback(module, name, a assert (args, kwargs) == before -@pytest.mark.parametrize("module", ["hermes_cli.main", "hermes_cli.update_cmd"]) -@pytest.mark.parametrize("name", ["cmd_update", "_cmd_update_impl"]) -@pytest.mark.parametrize("specs", [[], ["honcho-ai"]]) -@pytest.mark.parametrize("status", [0, 19]) -def test_lazy_installer_inside_historical_update_hands_off(module, name, specs, status, fresh_child): +@pytest.mark.parametrize("module,name,specs", [ + ("hermes_cli.main", "cmd_update", []), + ("hermes_cli.update_cmd", "_cmd_update_impl", ["honcho-ai"]), +]) +def test_lazy_installer_inside_historical_update_hands_off(module, name, specs, fresh_child): from tools.lazy_deps import install_specs # Run a frozen old-caller-shaped function, not a mocked context detector. + # The historical entrypoints declare pre_update_version; the walk crosses + # the intermediate helper frame to reach the matching frame. namespace = {"__name__": module, "install_specs": install_specs} - exec(f"def {name}(specs):\n install_specs(specs, timeout=120)\n raise AssertionError('old fallback resumed')\n", namespace) - fresh_child.returncode = status + exec( + f"def {name}():\n" + f" pre_update_version = '1.0'\n" + f" _helper()\n" + f"def _helper():\n" + f" install_specs({specs!r}, timeout=120)\n", + namespace, + ) + fresh_child.returncode = 0 with fresh_child.exits(): - namespace[name](specs) + namespace[name]() @pytest.mark.parametrize("unpack", [False, True], ids=["path-era", "tuple-era"]) diff --git a/tests/tools/test_lazy_deps_runtime.py b/tests/tools/test_lazy_deps_runtime.py index 8bf249ac6b..98b4b82eb9 100644 --- a/tests/tools/test_lazy_deps_runtime.py +++ b/tests/tools/test_lazy_deps_runtime.py @@ -4,30 +4,10 @@ from unittest.mock import Mock import pytest -@pytest.mark.parametrize("specs", [[], ["hindsight-all"], ("honcho-ai",)]) -@pytest.mark.parametrize("timeout", [0, 120, 300]) -def test_install_specs_reports_unavailable_without_update(specs, timeout, monkeypatch): - from hermes_cli import _old_updater - from tools.lazy_deps import install_specs - - # Exercise the public API and real handoff code, stopping only at its child - # boundary so an unfixed checkout cannot run a real install or Desktop build. - child = Mock(return_value=(0, {})) - monkeypatch.setattr(_old_updater, "_run_child", child) - monkeypatch.setattr(_old_updater, "_result", None) - failure = None - try: - install_specs(specs, timeout=timeout) - except BaseException as exc: - failure = exc - assert isinstance(failure, ImportError), repr(failure) - child.assert_not_called() - - @pytest.mark.parametrize("module,name", [ ("community_plugin", "cmd_update"), ("hermes_cli.main", "serve"), - ("hermes_cli.update_cmd", "unrelated"), + ("hermes_cli.update_cmd", "cmd_update"), # current updater: no pre_update_version local ]) def test_runtime_lookalikes_do_not_start_updates(module, name, monkeypatch): from hermes_cli import _old_updater @@ -41,21 +21,3 @@ def test_runtime_lookalikes_do_not_start_updates(module, name, monkeypatch): with pytest.raises(ImportError, match="retired"): namespace[name]() child.assert_not_called() - - -def test_runtime_caller_can_keep_running_after_retired_installer(monkeypatch): - from hermes_cli import _old_updater - from tools.lazy_deps import install_specs - - child = Mock(return_value=(0, {})) - monkeypatch.setattr(_old_updater, "_run_child", child) - monkeypatch.setattr(_old_updater, "_result", None) - # The plugin's ordinary exception handler must see failure, not SystemExit. - try: - install_specs(["hindsight-all"]) - except Exception: - available = False - else: - available = True - assert available is False - child.assert_not_called() diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index 0a438337d4..81c089dd58 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -1,9 +1,8 @@ """Shims to suppress old updater work until relaunch. New code must not use these.""" -import sys from typing import NoReturn -from hermes_cli._old_updater import stop_for_relaunch +from hermes_cli._old_updater import in_historical_update, stop_for_relaunch def ensure(feature: str, *, prompt: bool = True) -> NoReturn: @@ -12,20 +11,16 @@ def ensure(feature: str, *, prompt: bool = True) -> NoReturn: raise ImportError("Dependencies are unknown to this old updater. Please relaunch Hermes.") -def install_specs(specs: list[str] | tuple[str, ...], *, timeout: int = 300) -> NoReturn: +def install_specs(specs: list[str] | tuple[str, ...], *, timeout: int = 300, + constraints: list | None = None, dry_run: bool = False) -> NoReturn: # Plugins still call this retired API during normal agent construction. # Only an actual updater call stack may transfer control to the updater; # argv can still say "serve" or "gateway" when /update runs in-process. - frame = sys._getframe(1) - try: - while frame is not None: - if (frame.f_globals.get("__name__") in ("hermes_cli.update_cmd", "hermes_cli.main") - and frame.f_code.co_name in ("_cmd_update_impl", "cmd_update")): - stop_for_relaunch() - frame = frame.f_back - finally: - del frame + # Historical updaters also passed constraints/dry_run; accepted and ignored. + if in_historical_update(): + # never returns: hands off to the takeover child and exits + stop_for_relaunch() raise ImportError( - "tools.lazy_deps.install_specs is retired; runtime dependency installation " - "is unavailable. Update the plugin to use Hermes package management." + "tools.lazy_deps.install_specs is retired; runtime dependency " + "installation is unavailable." )