From c136eb4de1eae6db5acf2cc35f7e1e9e4763aea3 Mon Sep 17 00:00:00 2001 From: teknium1 Date: Thu, 4 Jun 2026 02:12:46 -0700 Subject: [PATCH] fix(update): harden venv rebuild + verify core deps after install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two complementary fixes for a silent partial-install failure that bit ``hermes update`` in the wild: a fresh checkout pulled 145 commits, ``rebuild_venv`` failed to recreate the venv on Windows because ``shutil.rmtree(ignore_errors=True)`` couldn't delete files held open by the running ``hermes.exe`` shim. ``uv venv`` then refused with "A directory already exists at: venv" and the update fell back to installing on top of the stale venv. The resulting partial install missed exactly one newly-added base dep — ``pathspec==1.1.1`` — which ``hermes desktop --build-only`` imports at the top of its content-hash check. The desktop rebuild died with ModuleNotFoundError and the parent update only logged "⚠ Desktop build failed (non-fatal)". Same root cause made the "default: sync failed" line in the skill-sync stage, because that sync subprocess hit the same missing import. Fix 1: ``rebuild_venv`` retries with ``--clear`` ------------------------------------------------ If ``uv venv`` fails with "already exists" in stderr (which is what uv prints, and what uv's own hint tells you to fix with --clear), retry once with ``--clear``. Only this specific failure pattern triggers the retry — disk-full / interpreter-download failures still surface as before so we don't mask real problems. Fix 2: post-install dep verification ------------------------------------ Belt-and-suspenders so future uv resolver quirks (or any other cause of partial installs) surface immediately instead of hours later in a downstream subprocess. After ``_install_python_dependencies_with_optional_fallback`` runs, ``_verify_core_dependencies_installed``: 1. Reads ``[project.dependencies]`` straight from pyproject.toml (so we don't trust the venv's stale metadata). 2. Filters by environment markers via ``packaging.requirements.Requirement`` so cross-platform exclusions (``ptyprocess ; sys_platform != 'win32'``) don't false-positive on Windows. 3. Runs ``importlib.metadata.version()`` for each remaining dep inside the *target* venv interpreter (resolved from ``VIRTUAL_ENV``, not ``sys.executable``). 4. If anything is missing, reinstalls the base group with ``--reinstall`` to force re-resolution. If a second probe still reports missing deps, force-installs each one with its pinned spec. 5. Treats final failure as a warning rather than a hard error — a single broken-on-PyPI dep shouldn't block an otherwise-successful update — but the message points at ``hermes update --force`` and names the missing packages so the user knows what's wrong. Tests ----- - ``TestRebuildVenv::test_retries_with_clear_when_dir_already_exists`` — simulates the rmtree-couldn't-delete-it failure mode and asserts the ``--clear`` retry path is taken and succeeds. - ``TestRebuildVenv::test_does_not_retry_when_first_failure_is_not_dir_exists`` — guards against masking real failures (disk full, etc.). - ``test_verify_core_dependencies.py`` — 7 tests covering the happy path, the regression (missing pathspec triggers --reinstall), the per-package fallback when --reinstall doesn't help, the platform- marker filter so Windows doesn't try to install ptyprocess, the missing-pyproject noop, and the VIRTUAL_ENV resolver. Co-authored-by: Kyssta <218078013+kyssta-exe@users.noreply.github.com> --- hermes_cli/main.py | 216 +++++++++++++++++ hermes_cli/managed_uv.py | 35 ++- tests/hermes_cli/test_managed_uv.py | 79 +++++++ .../test_verify_core_dependencies.py | 223 ++++++++++++++++++ 4 files changed, 547 insertions(+), 6 deletions(-) create mode 100644 tests/hermes_cli/test_verify_core_dependencies.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index b7ddc55fe..e500ec82a 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -9065,6 +9065,222 @@ def _install_python_dependencies_with_optional_fallback( f" ⚠ Skipped optional extras that still failed: {', '.join(failed_extras)}" ) + # Belt-and-suspenders: verify every declared core dependency from + # pyproject.toml's [project.dependencies] is actually importable in the + # target venv. uv's incremental resolver has — in the wild — produced + # partial installs where a newly added base dep (e.g. ``pathspec``) + # silently fails to land on top of a half-stale venv, and the only + # symptom is a downstream subprocess crashing with ModuleNotFoundError + # hours later inside ``hermes update``'s desktop-rebuild or skill-sync + # stage. Reinstall with --reinstall to force resolution if anything is + # missing, then re-verify so the failure surfaces here instead of + # downstream. + _verify_core_dependencies_installed(install_cmd_prefix, env=env, group=group) + + +def _verify_core_dependencies_installed( + install_cmd_prefix: list[str], + *, + env: dict[str, str] | None = None, + group: str = "all", +) -> None: + """Check that every base dep from pyproject.toml is importable; if not, retry. + + Reads ``pyproject.toml`` directly (so we don't trust the venv's stale + metadata), filters out deps gated by ``;`` environment markers that don't + apply to this platform, and runs ``importlib.metadata.version()`` in the + venv interpreter for each one. If anything is missing we reinstall the + base group with ``--reinstall`` to force uv to re-resolve, then check + again. We treat the final state as a warning rather than a hard failure + so a single broken-on-PyPI dep can't block an otherwise-successful + update — but the warning makes the partial install visible at the spot + that caused it, instead of hours later in a downstream subprocess. + """ + try: + import tomllib # Python 3.11+ + except ImportError: # pragma: no cover — Python < 3.11 unsupported but be safe + return + + pyproject = PROJECT_ROOT / "pyproject.toml" + if not pyproject.is_file(): + return + + try: + with open(pyproject, "rb") as f: + data = tomllib.load(f) + raw_deps = data.get("project", {}).get("dependencies", []) or [] + except Exception as e: + logger.debug("dep verification: failed to read pyproject.toml: %s", e) + return + + # Parse each "name OP version ; marker" string into (dist_name, marker_obj). + # We use packaging.requirements when available (it ships with pip/uv envs), + # falling back to a naive split that's good enough for the canonical + # ``name==version[; marker]`` style this repo uses. + deps: list[tuple[str, "object | None"]] = [] + try: + from packaging.requirements import Requirement # type: ignore + + for spec in raw_deps: + try: + req = Requirement(spec) + deps.append((req.name, req.marker)) + except Exception: + continue + except Exception: + for spec in raw_deps: + head = spec.split(";", 1)[0] + for op in ("==", ">=", "<=", "~=", ">", "<", "!="): + if op in head: + head = head.split(op, 1)[0] + break + name = head.strip().split("[", 1)[0].strip() + if name: + deps.append((name, None)) + + # Apply environment markers to drop deps that don't apply on this platform + # (e.g. ``ptyprocess ; sys_platform != 'win32'`` is correctly skipped on + # Windows). Without markers we'd false-positive every cross-platform exclusion. + applicable: list[str] = [] + for name, marker in deps: + if marker is None: + applicable.append(name) + continue + try: + if marker.evaluate(): # type: ignore[union-attr] + applicable.append(name) + except Exception: + applicable.append(name) + + if not applicable: + return + + # Run the check inside the venv Python — sys.executable here may be the + # outer Python that drove ``hermes update``, not the venv we just wrote + # to. The uv install_cmd_prefix encodes which environment we targeted + # (either ``[uv, pip]`` with VIRTUAL_ENV in env, or + # ``[sys.executable, -m, pip]`` for the in-process Python); resolve the + # right interpreter for the verification. + venv_python = _resolve_install_target_python(install_cmd_prefix, env) + if venv_python is None: + return + + def _missing_deps() -> list[str]: + check_script = ( + "import importlib.metadata as md, sys\n" + "missing=[]\n" + "for name in sys.argv[1:]:\n" + " try: md.version(name)\n" + " except md.PackageNotFoundError: missing.append(name)\n" + "print('\\n'.join(missing))\n" + ) + try: + result = subprocess.run( + [str(venv_python), "-c", check_script, *applicable], + capture_output=True, + text=True, + check=False, + env=env, + ) + except Exception as e: + logger.debug("dep verification: subprocess failed: %s", e) + return [] + return [line.strip() for line in result.stdout.splitlines() if line.strip()] + + missing = _missing_deps() + if not missing: + return + + print( + f" ⚠ Verification: {len(missing)} declared dep(s) missing after install: " + f"{', '.join(missing[:8])}{'...' if len(missing) > 8 else ''}" + ) + print(" → Reinstalling base group with --reinstall to repair...") + + # Reinstall base group with --reinstall so uv re-resolves from scratch + # against the current pyproject. We don't pass ``[{group}]`` here on + # purpose — the missing dep is in *base* deps; rerunning the full all- + # extras install can cost minutes and trips on whatever optional extra + # was already broken upstream. Base is fast and is what's actually wrong. + repair_args = ["install", "--reinstall", "-e", "."] + try: + _run_install_with_heartbeat(install_cmd_prefix + repair_args, env=env) + except subprocess.CalledProcessError as e: + logger.warning("dep verification: repair install failed: %s", e) + print(" ⚠ Repair install failed; check `hermes update` output above.") + return + + still_missing = _missing_deps() + if not still_missing: + print(" ✓ All declared core dependencies now installed") + return + + # Last-ditch: install each remaining missing dep with its pin directly. + # Useful when uv's resolver thinks the env is satisfied but the on-disk + # package metadata says otherwise (rare but observed). + name_to_spec = {} + for spec in raw_deps: + head = spec.split(";", 1)[0].strip() + bare = head + for op in ("==", ">=", "<=", "~=", ">", "<", "!="): + if op in bare: + bare = bare.split(op, 1)[0] + break + name_to_spec[bare.strip().split("[", 1)[0].strip()] = head + + specs = [name_to_spec.get(n, n) for n in still_missing] + print( + f" → Force-installing remaining missing dep(s): {', '.join(specs)}" + ) + try: + _run_install_with_heartbeat( + install_cmd_prefix + ["install", "--reinstall", *specs], env=env + ) + except subprocess.CalledProcessError as e: + logger.warning("dep verification: per-package repair failed: %s", e) + print( + f" ⚠ Could not install: {', '.join(still_missing)}. " + "Run `hermes update --force` after closing other hermes processes." + ) + return + + final_missing = _missing_deps() + if final_missing: + print( + f" ⚠ Still missing after repair: {', '.join(final_missing)}. " + "Run `hermes update --force` after closing other hermes processes." + ) + else: + print(" ✓ All declared core dependencies now installed") + + +def _resolve_install_target_python( + install_cmd_prefix: list[str], env: dict[str, str] | None +) -> Path | None: + """Figure out which Python interpreter the install just targeted. + + ``_install_python_dependencies_with_optional_fallback`` is called with + either ``[uv, pip]`` (and a ``VIRTUAL_ENV`` env var pointing at the + target venv) or ``[sys.executable, -m, pip]`` (the in-process Python). + The verification step needs the *resulting* environment's Python so + ``importlib.metadata`` queries the right site-packages. + """ + if env and "VIRTUAL_ENV" in env: + venv_root = Path(env["VIRTUAL_ENV"]) + scripts = venv_root / ("Scripts" if _is_windows() else "bin") + candidate = scripts / ("python.exe" if _is_windows() else "python") + if candidate.exists(): + return candidate + + # Fallback: assume install_cmd_prefix[0] is the python interpreter (the + # ``[sys.executable, -m, pip]`` shape). Skip if it looks like ``uv``. + if install_cmd_prefix: + first = Path(install_cmd_prefix[0]) + if first.exists() and "uv" not in first.name.lower(): + return first + + return None + def _is_termux_env(env: dict[str, str] | None = None) -> bool: return _is_termux_startup_environment(env) diff --git a/hermes_cli/managed_uv.py b/hermes_cli/managed_uv.py index b38893f71..31bbbc8b9 100644 --- a/hermes_cli/managed_uv.py +++ b/hermes_cli/managed_uv.py @@ -105,17 +105,40 @@ def rebuild_venv(uv_bin: str, venv_dir: Path, python_version: str = "3.11") -> b old venv may point to a Python without FTS5, so we rebuild it with a fresh interpreter from the current managed uv. Returns ``True`` on success. + + On Windows, ``shutil.rmtree(..., ignore_errors=True)`` can silently leave + the venv directory partially intact when another process is holding an + open handle to a file inside it (typical culprits: a running + ``hermes.exe`` REPL, the gateway, AV scanners). If we don't notice that + and just call ``uv venv``, uv refuses with + ``Caused by: A directory already exists at: venv`` and the *whole + update* falls back to installing on top of the stale venv — which has + historically produced partial installs where a freshly added dependency + (e.g. ``pathspec``) silently fails to land. Retry with ``--clear`` to + force uv past that condition before giving up. """ if venv_dir.exists(): print(f" → Rebuilding venv (old Python may lack FTS5)...") shutil.rmtree(venv_dir, ignore_errors=True) - result = subprocess.run( - [uv_bin, "venv", str(venv_dir), "--python", python_version], - capture_output=True, - text=True, - check=False, - ) + def _run_uv_venv(extra_args: list[str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [uv_bin, "venv", str(venv_dir), "--python", python_version, *extra_args], + capture_output=True, + text=True, + check=False, + ) + + result = _run_uv_venv([]) + + # If uv refused because the directory still exists (rmtree above was + # blocked by an open file handle, common on Windows), retry with + # --clear so uv overwrites it. Match on stderr because uv's exit code + # alone doesn't distinguish "dir exists" from real failures. + if result.returncode != 0 and "already exists" in (result.stderr or "").lower(): + print(" → venv dir not fully removed (likely an open file handle); retrying with --clear...") + result = _run_uv_venv(["--clear"]) + if result.returncode == 0: venv_python = venv_dir / ("Scripts" if platform.system() == "Windows" else "bin") / "python" py_ver = subprocess.run( diff --git a/tests/hermes_cli/test_managed_uv.py b/tests/hermes_cli/test_managed_uv.py index 89f3d8bad..f1394f6ef 100644 --- a/tests/hermes_cli/test_managed_uv.py +++ b/tests/hermes_cli/test_managed_uv.py @@ -145,6 +145,85 @@ class TestRebuildVenv: result = rebuild_venv(uv_bin, venv_dir) assert result is False + def test_retries_with_clear_when_dir_already_exists(self, tmp_path): + """On Windows, rmtree can silently fail when an open handle holds a + file in the venv (running hermes.exe, gateway, AV scanner). uv then + refuses with ``Caused by: A directory already exists at: venv``. + Make sure we don't give up — retry with ``--clear`` to force uv past + the stale directory and rebuild successfully.""" + venv_dir = tmp_path / "venv" + venv_dir.mkdir() + (venv_dir / "stale_open_handle").write_text("rmtree couldn't delete me") + + uv_bin = str(tmp_path / "bin" / "uv") + call_log: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + call_log.append(list(cmd)) + m = MagicMock() + if cmd[1] == "venv" and "--clear" not in cmd: + # First attempt: uv refuses because dir still exists + m.returncode = 1 + m.stderr = ( + "error: Failed to create virtual environment\n" + " Caused by: A directory already exists at: venv\n" + "hint: Use the `--clear` flag or set `UV_VENV_CLEAR=1` to replace the existing directory\n" + ) + m.stdout = "" + return m + if cmd[1] == "venv" and "--clear" in cmd: + # Retry: succeeds. Simulate uv writing the python shim. + m.returncode = 0 + m.stderr = "" + m.stdout = "" + bin_dir = venv_dir / ("Scripts" if os.name == "nt" else "bin") + bin_dir.mkdir(parents=True, exist_ok=True) + python_name = "python.exe" if os.name == "nt" else "python" + (bin_dir / python_name).write_text("#!/bin/sh\necho Python 3.11.0") + return m + if "--version" in cmd: + m.returncode = 0 + m.stdout = "Python 3.11.0" + m.stderr = "" + return m + m.returncode = 0 + return m + + with patch("hermes_cli.managed_uv.subprocess.run", side_effect=fake_run), \ + patch("hermes_cli.managed_uv.shutil.rmtree"): + from hermes_cli.managed_uv import rebuild_venv + result = rebuild_venv(uv_bin, venv_dir) + + assert result is True, "rebuild should succeed after --clear retry" + # We expect exactly two ``uv venv`` calls: one without --clear, one with. + venv_calls = [c for c in call_log if len(c) >= 2 and c[1] == "venv"] + assert len(venv_calls) == 2, f"expected 2 venv calls, got {venv_calls}" + assert "--clear" not in venv_calls[0], "first call should not pass --clear" + assert "--clear" in venv_calls[1], "retry must pass --clear" + + def test_does_not_retry_when_first_failure_is_not_dir_exists(self, tmp_path): + """If uv venv fails for some other reason (e.g. interpreter download + failed, disk full), we should NOT silently retry with --clear — + that would mask a real problem. Just surface the original failure.""" + venv_dir = tmp_path / "venv" + uv_bin = str(tmp_path / "bin" / "uv") + call_log: list[list[str]] = [] + + def fake_run(cmd, **kwargs): + call_log.append(list(cmd)) + m = MagicMock(returncode=1, stderr="error: No space left on device", stdout="") + return m + + with patch("hermes_cli.managed_uv.subprocess.run", side_effect=fake_run), \ + patch("hermes_cli.managed_uv.shutil.rmtree"): + from hermes_cli.managed_uv import rebuild_venv + result = rebuild_venv(uv_bin, venv_dir) + + assert result is False + venv_calls = [c for c in call_log if len(c) >= 2 and c[1] == "venv"] + assert len(venv_calls) == 1, "should not retry on non-dir-exists failures" + assert "--clear" not in venv_calls[0] + # --------------------------------------------------------------------------- # update_managed_uv diff --git a/tests/hermes_cli/test_verify_core_dependencies.py b/tests/hermes_cli/test_verify_core_dependencies.py new file mode 100644 index 000000000..b3d4e3845 --- /dev/null +++ b/tests/hermes_cli/test_verify_core_dependencies.py @@ -0,0 +1,223 @@ +"""Tests for _verify_core_dependencies_installed. + +Regression coverage for the partial-install bug where uv's incremental +resolver silently failed to land ``pathspec`` (and similar newly-added +base deps) during ``hermes update``, leaving the venv in a broken state +that only surfaced hours later when a downstream subprocess imported the +missing module. + +The verification step: + 1. Reads pyproject.toml's [project.dependencies] directly. + 2. Filters by environment markers so cross-platform exclusions don't + false-positive (e.g. ``ptyprocess ; sys_platform != 'win32'`` on Windows). + 3. Probes ``importlib.metadata.version()`` in the venv interpreter. + 4. Reinstalls with --reinstall, then per-package, if anything's missing. +""" + +from __future__ import annotations + +import subprocess +import textwrap +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + + +@pytest.fixture +def temp_pyproject(tmp_path, monkeypatch): + """Point hermes_cli.main.PROJECT_ROOT at a tmp dir with a minimal pyproject. + + The verification helper opens ``PROJECT_ROOT / 'pyproject.toml'`` directly; + redirecting PROJECT_ROOT keeps the test hermetic. + """ + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text(textwrap.dedent("""\ + [project] + name = "fake" + version = "0.0.0" + dependencies = [ + "pathspec==1.1.1", + "pydantic==2.13.4", + "ptyprocess>=0.7.0,<1; sys_platform != 'win32'", + ] + """)) + import hermes_cli.main as main_mod + monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) + return tmp_path + + +@pytest.fixture +def fake_venv_python(tmp_path): + """Create a fake venv python shim path that exists on disk.""" + venv_root = tmp_path / "venv" + scripts = venv_root / "Scripts" + scripts.mkdir(parents=True) + py = scripts / "python.exe" + py.write_text("#!/bin/sh\necho fake python") + return py, venv_root + + +class TestVerifyCoreDependencies: + def test_no_action_when_all_deps_present(self, temp_pyproject, fake_venv_python): + """The happy path: nothing missing, no repair install fires.""" + py, venv_root = fake_venv_python + env = {"VIRTUAL_ENV": str(venv_root)} + + with patch("hermes_cli.main._resolve_install_target_python", return_value=py), \ + patch("hermes_cli.main.subprocess.run") as mock_run, \ + patch("hermes_cli.main._run_install_with_heartbeat") as mock_install: + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") + + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env=env) + + # Probe ran, repair install did not. + assert mock_run.called, "verification probe should have run" + assert not mock_install.called, "repair install should not fire when nothing is missing" + + def test_triggers_reinstall_when_dep_missing(self, temp_pyproject, fake_venv_python): + """The regression: one base dep is missing → trigger --reinstall.""" + py, venv_root = fake_venv_python + env = {"VIRTUAL_ENV": str(venv_root)} + + # First probe reports pathspec missing; after repair, probe is clean. + probe_calls = {"count": 0} + + def fake_subprocess_run(cmd, **kwargs): + # The probe subprocess returns stdout = newline-joined missing names. + probe_calls["count"] += 1 + if probe_calls["count"] == 1: + return MagicMock(returncode=0, stdout="pathspec\n", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + with patch("hermes_cli.main._resolve_install_target_python", return_value=py), \ + patch("hermes_cli.main.subprocess.run", side_effect=fake_subprocess_run), \ + patch("hermes_cli.main._run_install_with_heartbeat") as mock_install: + + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env=env) + + assert mock_install.called, "repair install must fire when a dep is missing" + # First repair must use --reinstall to force re-resolution. + first_repair = mock_install.call_args_list[0] + args = first_repair[0][0] # positional: install_cmd + assert "--reinstall" in args, f"repair install should pass --reinstall, got {args}" + assert "-e" in args and "." in args, "first repair should be base group reinstall" + + def test_falls_back_to_per_package_install_when_reinstall_did_not_help( + self, temp_pyproject, fake_venv_python + ): + """If --reinstall doesn't repair the partial install (uv resolver + thinks the env is satisfied), force-install each missing dep with + its declared pin spec. This is the last-ditch path.""" + py, venv_root = fake_venv_python + env = {"VIRTUAL_ENV": str(venv_root)} + + probe_calls = {"count": 0} + + def fake_subprocess_run(cmd, **kwargs): + probe_calls["count"] += 1 + # 1st probe: pathspec missing + # 2nd probe (after --reinstall): still missing + # 3rd probe (after per-package): clean + if probe_calls["count"] in (1, 2): + return MagicMock(returncode=0, stdout="pathspec\n", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + with patch("hermes_cli.main._resolve_install_target_python", return_value=py), \ + patch("hermes_cli.main.subprocess.run", side_effect=fake_subprocess_run), \ + patch("hermes_cli.main._run_install_with_heartbeat") as mock_install: + + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env=env) + + assert mock_install.call_count >= 2, ( + "expected at least 2 repair installs (reinstall + per-package), " + f"got {mock_install.call_count}" + ) + # Second repair call should pass the pinned spec for the missing dep. + second_repair_args = mock_install.call_args_list[1][0][0] + assert any("pathspec==1.1.1" in str(a) for a in second_repair_args), ( + f"second repair should pin pathspec from pyproject; got {second_repair_args}" + ) + + def test_skips_deps_excluded_by_environment_markers(self, temp_pyproject, fake_venv_python): + """``ptyprocess ; sys_platform != 'win32'`` should NOT be reported as + missing on Windows. Without marker evaluation, the verification step + would false-positive on every cross-platform exclusion and chase its + tail forever trying to install something that can't apply here.""" + py, venv_root = fake_venv_python + env = {"VIRTUAL_ENV": str(venv_root)} + captured_argv: list[list[str]] = [] + + def fake_subprocess_run(cmd, **kwargs): + captured_argv.append(list(cmd)) + return MagicMock(returncode=0, stdout="", stderr="") + + # Force sys.platform to look like Windows so the marker filters + # ptyprocess out. (We need the actual marker.evaluate() to see win32.) + with patch("hermes_cli.main._resolve_install_target_python", return_value=py), \ + patch("hermes_cli.main.subprocess.run", side_effect=fake_subprocess_run), \ + patch("hermes_cli.main._run_install_with_heartbeat"), \ + patch("sys.platform", "win32"): + + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env=env) + + # Find the probe argv — it's the call that passed the dep names. + probe = next( + (argv for argv in captured_argv if any("importlib.metadata" in str(a) for a in argv)), + None, + ) + assert probe is not None, "verification probe should have run" + # The dep names are tacked on after the -c script. + assert "ptyprocess" not in probe, ( + "ptyprocess is gated by sys_platform != 'win32' and must be filtered " + f"out on Windows; full probe argv was: {probe}" + ) + assert "pathspec" in probe, "core deps without markers must be checked" + + def test_no_pyproject_is_noop(self, tmp_path, monkeypatch): + """If pyproject.toml is missing (unusual but possible in some test + envs), the verification step must short-circuit, not crash.""" + import hermes_cli.main as main_mod + monkeypatch.setattr(main_mod, "PROJECT_ROOT", tmp_path) + # No pyproject.toml in tmp_path. + with patch("hermes_cli.main._resolve_install_target_python") as mock_resolve, \ + patch("hermes_cli.main._run_install_with_heartbeat") as mock_install: + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env={}) + assert not mock_resolve.called + assert not mock_install.called + + +class TestResolveInstallTargetPython: + def test_uses_virtual_env_from_environment(self, tmp_path): + """When VIRTUAL_ENV is set, the verification step must probe THAT + venv's interpreter — not the outer Python that drove `hermes update`. + If we probed sys.executable instead, we'd false-positive every dep + the outer interpreter happens to lack.""" + venv_root = tmp_path / "newvenv" + scripts = venv_root / "Scripts" + scripts.mkdir(parents=True) + py = scripts / "python.exe" + py.write_text("fake") + + with patch("hermes_cli.main._is_windows", return_value=True): + from hermes_cli.main import _resolve_install_target_python + result = _resolve_install_target_python( + ["uv", "pip"], env={"VIRTUAL_ENV": str(venv_root)} + ) + assert result == py + + def test_returns_none_when_venv_python_missing(self, tmp_path): + """If the path we'd point at doesn't exist (uv install failed before + the python shim landed), return None so the verification step + cleanly short-circuits instead of crashing on FileNotFoundError.""" + with patch("hermes_cli.main._is_windows", return_value=True): + from hermes_cli.main import _resolve_install_target_python + result = _resolve_install_target_python( + ["uv", "pip"], env={"VIRTUAL_ENV": str(tmp_path / "does_not_exist")} + ) + assert result is None