diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index 1852afbd8b..7d30531cf8 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -2525,7 +2525,15 @@ def _detect_venv_dir() -> Path | None: def get_python_path() -> str: venv = _detect_venv_dir() if venv is not None: - from hermes_constants import venv_python_path + try: + from hermes_constants import venv_python_path + except ImportError: + # Update-boundary: a gateway restarted mid-update can hold a + # hermes_constants cached from before this symbol existed. See + # _reload_hermes_constants() in hermes_cli/managed_uv.py. + from hermes_cli.managed_uv import _reload_hermes_constants + + venv_python_path = _reload_hermes_constants().venv_python_path venv_python = venv_python_path(venv, windows=is_windows()) if venv_python.exists(): diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index 85256e880a..8c42cdcb41 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -19,6 +19,7 @@ releases it. from __future__ import annotations +import importlib import json import logging import os @@ -383,10 +384,35 @@ def update_managed_uv( # --------------------------------------------------------------------------- -def _venv_python(venv_dir: Path) -> Path: - from hermes_constants import venv_python_path +def _reload_hermes_constants(): + """Re-execute ``hermes_constants`` from disk and return the fresh module. - return venv_python_path(venv_dir, windows=platform.system() == "Windows") + ``hermes update`` imports ``hermes_constants`` from the OLD checkout, + ``git pull`` then replaces that file, and this freshly-pulled module runs + its lazy imports against the module object Python already cached in + ``sys.modules`` — the pre-upgrade one. A symbol added by the update is + absent there while the file named in the resulting ``ImportError`` plainly + contains it, which is what made this read as a contradiction: + + cannot import name 'venv_python_path' from 'hermes_constants' + (~/.hermes/hermes-agent/hermes_constants.py) + + Reloading picks up the definitions actually on disk, so callers keep using + the shared helper instead of hand-rolling a second copy of its logic. Same + update-boundary class as the ``ensure_uv()`` arity skew on :class:`_UvResult`. + """ + import hermes_constants + + return importlib.reload(hermes_constants) + + +def _venv_python(venv_dir: Path) -> Path: + windows = platform.system() == "Windows" + try: + from hermes_constants import venv_python_path + except ImportError: + venv_python_path = _reload_hermes_constants().venv_python_path + return venv_python_path(venv_dir, windows=windows) def _remove_tree(path: Path, *, boundary: Path) -> None: diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 8cc34c40c8..1c60866101 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -1026,3 +1026,77 @@ class TestDefaultLiveVenv: result = repair_vulnerable_runtime("uv", project_root=root) assert result.status == "not-applicable" + +class TestVenvPythonUpdateBoundary: + """``_venv_python`` must survive a hermes_constants predating its symbol. + + ``hermes update`` imports hermes_constants from the OLD checkout, ``git + pull`` replaces that file, and the freshly-pulled managed_uv then runs its + lazy ``from hermes_constants import venv_python_path`` against the module + object already cached in ``sys.modules``. That cached module has no such + symbol, so the import raises — while naming the NEW file on disk, which + plainly contains it, which is what made the error so confusing: + + cannot import name 'venv_python_path' from 'hermes_constants' + (~/.hermes/hermes-agent/hermes_constants.py) + + It aborted the managed-Python runtime repair on the first update from any + release older than the symbol. Same class as the ``ensure_uv()`` arity skew + documented on ``_UvResult``. + """ + + def test_recovers_when_the_cached_module_predates_the_symbol(self, monkeypatch): + import hermes_constants + + from hermes_cli.managed_uv import _venv_python + + # The stale in-memory module: the symbol the new code wants is absent, + # exactly as on an install that booted the pre-upgrade checkout. The + # file on disk is the current one, so a reload recovers the real helper. + monkeypatch.delattr(hermes_constants, "venv_python_path", raising=False) + monkeypatch.setattr("platform.system", lambda: "Linux") + + assert _venv_python(Path("/opt/hermes/venv")) == Path( + "/opt/hermes/venv/bin/python" + ) + + def test_recovery_uses_the_shared_helper_not_a_second_copy(self, monkeypatch): + """The reload must resolve through hermes_constants, not open-code it. + + Hand-rolling `Scripts`/`bin` here is what #76105 deduped away and what + `test_no_open_coded_venv_layout_remains_in_hermes_cli` bans. + """ + import hermes_constants + + from hermes_cli.managed_uv import _venv_python + + monkeypatch.delattr(hermes_constants, "venv_python_path", raising=False) + monkeypatch.setattr("platform.system", lambda: "Linux") + + sentinel = Path("/sentinel/from/shared/helper") + real_reload = __import__("importlib").reload + + def _reload_with_marker(module): + fresh = real_reload(module) + monkeypatch.setattr( + fresh, "venv_python_path", lambda *a, **k: sentinel, raising=False + ) + return fresh + + monkeypatch.setattr("importlib.reload", _reload_with_marker) + assert _venv_python(Path("/opt/hermes/venv")) == sentinel + + def test_uses_the_real_helper_when_it_is_importable(self, monkeypatch): + """The normal path never reloads — recovery stays a fallback.""" + from hermes_cli.managed_uv import _venv_python + + def _no_reload(module): # pragma: no cover - must not run + raise AssertionError("reload must not run when the import succeeds") + + monkeypatch.setattr("importlib.reload", _no_reload) + monkeypatch.setattr("platform.system", lambda: "Linux") + + assert _venv_python(Path("/opt/hermes/venv")) == Path( + "/opt/hermes/venv/bin/python" + ) +