mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(deps): gate the runtime takeover on genuinely historical updater frames
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.
This commit is contained in:
1 parent
5e6165562b
commit
af26acab73
4 files changed
+63
-65
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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"])
|
||||
|
||||
@@ -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()
|
||||
+9
-14
@@ -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."
|
||||
)
|
||||
Reference in new issue
Block a user