mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(cli): quarantine running hermes.exe during update dep-verification repair on Windows (#40409)
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).
This commit is contained in:
parent
d4a7bfd3aa
commit
ebed881d46
2 changed files with 88 additions and 12 deletions
|
|
@ -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.")
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue