From ebed881d46c4d39a7723a0bdbb70b53429f65e26 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 6 Jun 2026 10:50:58 -0700 Subject: [PATCH] fix(cli): quarantine running hermes.exe during update dep-verification repair on Windows (#40409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dependency-verification repair in _verify_core_dependencies_installed ran 'pip install --reinstall -e .' via _run_install_with_heartbeat directly, bypassing the Windows shim-quarantine that the primary install path performs. That reinstall rewrites the entry-point shims, and on Windows the live hermes.exe is the running process — pip can neither delete nor overwrite it. With no quarantine, the shim was left missing and 'hermes' dropped off PATH ('hermes' is not recognized... after update). Extract the rename-out-of-the-way / restore-on-failure logic into a reusable _run_quarantined_install helper and route both the primary editable installs and the --reinstall -e . repair through it. The per-package repair installs only third-party deps (never hermes-agent), so they don't touch the shims and are left untouched. Add a regression test (fails on old code, passes on new). --- hermes_cli/main.py | 57 +++++++++++++++---- .../test_verify_core_dependencies.py | 43 ++++++++++++++ 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 58043229893..38271f64ef7 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -9079,6 +9079,40 @@ def _restore_quarantined_exes(moved: list[tuple[Path, Path]]) -> None: pass +def _run_quarantined_install( + cmd: list[str], + *, + env: dict[str, str] | None = None, + scripts_dir: Path | None = None, +) -> None: + """Run an editable install, quarantining the running ``hermes.exe`` first. + + Any ``pip install -e .`` (or ``--reinstall``) rewrites the entry-point + shims, and on Windows the live ``hermes.exe`` is the running process — + pip can neither delete nor overwrite it, so without quarantine the shim + is left missing and ``hermes`` drops off PATH. This wraps + :func:`_run_install_with_heartbeat` with the same rename-out-of-the-way / + restore-on-failure dance that the primary install path uses, so EVERY + install that touches the shims is protected — including the + verification-repair reinstalls in + :func:`_verify_core_dependencies_installed`, which previously called + ``_run_install_with_heartbeat`` directly and bypassed quarantine. + + Off-Windows (``scripts_dir is None``) this is a thin pass-through. + """ + moved: list[tuple[Path, Path]] = [] + if scripts_dir is not None: + moved = _quarantine_running_hermes_exe(scripts_dir) + try: + _run_install_with_heartbeat(cmd, env=env) + except BaseException: + # Restore shims if pip/uv didn't write replacements (e.g. install + # failed before the entry-points step). Don't swallow the error. + if scripts_dir is not None: + _restore_quarantined_exes(moved) + raise + + def _cleanup_quarantined_exes(scripts_dir: Path | None = None) -> None: """Sweep ``hermes.exe.old.*`` left by prior updates. @@ -9189,17 +9223,9 @@ def _install_python_dependencies_with_optional_fallback( scripts_dir = _venv_scripts_dir() if _is_windows() else None def _install(args: list[str]) -> None: - moved: list[tuple[Path, Path]] = [] - if scripts_dir is not None: - moved = _quarantine_running_hermes_exe(scripts_dir) - try: - _run_install_with_heartbeat(install_cmd_prefix + args, env=env) - except BaseException: - # Restore shims if uv didn't write replacements (e.g. install - # failed before the entry-points step). Don't swallow the error. - if scripts_dir is not None: - _restore_quarantined_exes(moved) - raise + _run_quarantined_install( + install_cmd_prefix + args, env=env, scripts_dir=scripts_dir + ) try: _install(["install", "-e", f".[{group}]"]) @@ -9366,9 +9392,16 @@ def _verify_core_dependencies_installed( # 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. + # + # Quarantine the running ``hermes.exe`` first: ``--reinstall -e .`` + # rewrites the entry-point shims, and on Windows pip can't overwrite the + # live launcher, which would leave ``hermes`` off PATH. + scripts_dir = _venv_scripts_dir() if _is_windows() else None repair_args = ["install", "--reinstall", "-e", "."] try: - _run_install_with_heartbeat(install_cmd_prefix + repair_args, env=env) + _run_quarantined_install( + install_cmd_prefix + repair_args, env=env, scripts_dir=scripts_dir + ) except subprocess.CalledProcessError as e: logger.warning("dep verification: repair install failed: %s", e) print(" ⚠ Repair install failed; check `hermes update` output above.") diff --git a/tests/hermes_cli/test_verify_core_dependencies.py b/tests/hermes_cli/test_verify_core_dependencies.py index b3d4e384592..615b31406e1 100644 --- a/tests/hermes_cli/test_verify_core_dependencies.py +++ b/tests/hermes_cli/test_verify_core_dependencies.py @@ -191,6 +191,49 @@ class TestVerifyCoreDependencies: assert not mock_resolve.called assert not mock_install.called + def test_repair_reinstall_quarantines_running_shim_on_windows( + self, temp_pyproject, fake_venv_python + ): + """Regression: the ``--reinstall -e .`` repair must + quarantine the running ``hermes.exe`` on Windows before installing. + + That reinstall rewrites the editable entry-point shims, and on Windows + pip can't overwrite the live launcher — so without quarantine the shim + is left missing and ``hermes`` drops off PATH. Previously this path + called ``_run_install_with_heartbeat`` directly, bypassing the + quarantine that the primary install path performs. + """ + 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 → triggers --reinstall repair. + # 2nd probe (after repair): clean → stops before per-package path. + if probe_calls["count"] == 1: + return MagicMock(returncode=0, stdout="pathspec\n", stderr="") + return MagicMock(returncode=0, stdout="", stderr="") + + fake_scripts = venv_root / "Scripts" # created by fake_venv_python + + 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._is_windows", return_value=True), \ + patch("hermes_cli.main._venv_scripts_dir", return_value=fake_scripts), \ + patch("hermes_cli.main._run_install_with_heartbeat"), \ + patch("hermes_cli.main._quarantine_running_hermes_exe", return_value=[]) as mock_quar: + + from hermes_cli.main import _verify_core_dependencies_installed + _verify_core_dependencies_installed(["uv", "pip"], env=env) + + assert mock_quar.called, ( + "the --reinstall -e . repair must quarantine the running " + "hermes.exe on Windows" + ) + assert mock_quar.call_args[0][0] == fake_scripts + class TestResolveInstallTargetPython: def test_uses_virtual_env_from_environment(self, tmp_path):