fix(update): harden venv rebuild + verify core deps after install
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>
This commit is contained in:
@ -9065,6 +9065,222 @@ def _install_python_dependencies_with_optional_fallback(
|
|||||||
f" ⚠ Skipped optional extras that still failed: {', '.join(failed_extras)}"
|
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:
|
def _is_termux_env(env: dict[str, str] | None = None) -> bool:
|
||||||
return _is_termux_startup_environment(env)
|
return _is_termux_startup_environment(env)
|
||||||
|
|||||||
@ -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
|
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
|
fresh interpreter from the current managed uv. Returns ``True`` on
|
||||||
success.
|
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():
|
if venv_dir.exists():
|
||||||
print(f" → Rebuilding venv (old Python may lack FTS5)...")
|
print(f" → Rebuilding venv (old Python may lack FTS5)...")
|
||||||
shutil.rmtree(venv_dir, ignore_errors=True)
|
shutil.rmtree(venv_dir, ignore_errors=True)
|
||||||
|
|
||||||
result = subprocess.run(
|
def _run_uv_venv(extra_args: list[str]) -> subprocess.CompletedProcess[str]:
|
||||||
[uv_bin, "venv", str(venv_dir), "--python", python_version],
|
return subprocess.run(
|
||||||
capture_output=True,
|
[uv_bin, "venv", str(venv_dir), "--python", python_version, *extra_args],
|
||||||
text=True,
|
capture_output=True,
|
||||||
check=False,
|
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:
|
if result.returncode == 0:
|
||||||
venv_python = venv_dir / ("Scripts" if platform.system() == "Windows" else "bin") / "python"
|
venv_python = venv_dir / ("Scripts" if platform.system() == "Windows" else "bin") / "python"
|
||||||
py_ver = subprocess.run(
|
py_ver = subprocess.run(
|
||||||
|
|||||||
@ -145,6 +145,85 @@ class TestRebuildVenv:
|
|||||||
result = rebuild_venv(uv_bin, venv_dir)
|
result = rebuild_venv(uv_bin, venv_dir)
|
||||||
assert result is False
|
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
|
# update_managed_uv
|
||||||
|
|||||||
223
tests/hermes_cli/test_verify_core_dependencies.py
Normal file
223
tests/hermes_cli/test_verify_core_dependencies.py
Normal file
@ -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
|
||||||
Reference in New Issue
Block a user