From 679e9cd2943dddf59f50486c265a7ab110ad0f19 Mon Sep 17 00:00:00 2001 From: emozilla Date: Sat, 22 Aug 2026 13:38:56 -0400 Subject: [PATCH] fix(windows): stage hermes launchers in the managed binary dir, not the git checkout The installer staged the hermes/hermes-acp launcher copies at hermes-agent\bin -- inside the git working tree -- and put that dir on the user PATH (#84452). The update command's pre-pull autostash (git stash push --include-untracked) swept those untracked, unignored copies off disk, and once the desktop updater stopped re-applying stashes (--keep-stash, 5dd221d442) nothing restored them: `hermes` stopped resolving in every new terminal on every desktop-updated install. Move the canonical launcher home to the managed binary dir (%LOCALAPPDATA%\hermes\bin, next to the managed uv) -- outside the checkout, where no git operation can ever touch it. The dir is per-machine and shared by every profile, so all anchoring uses get_default_hermes_root(), never HERMES_HOME (which points inside profiles\ under `hermes -p`). The copy design also had a second latent break: managed-uv rebuilds create relocatable venvs, and a relocatable venv's exe trampoline resolves relative to its own location -- a copy outside venv\Scripts dies with 'uv trampoline failed to canonicalize script path'. Launcher form now depends on the venv (lockstep in install.ps1 and _install_repair.py): exe copy for normal venvs, a .cmd delegator invoking the in-venv exe by absolute path for relocatable ones. Either form counts as present, so pre-rebuild exe copies are left alone. Delivery to the existing fleet, per cohort: - already-broken installs cannot run the CLI, so an import-time heal in hermes_cli.main (ensure_windows_bin_launchers) re-stages missing launchers when the desktop app spawns its backend -- the one channel that still reaches them. Gates fail toward inaction: canonical dir only for the managed clone, legacy hermes-agent\bin only while the user PATH still resolves through it (some pre-managed-uv installs have no hermes\bin PATH entry; the legacy re-stage is what fixes those). Staging-name + os.replace keeps concurrent process starts from tearing a launcher; the helper never raises. - healthy old-layout installs migrate in the update tail (migrate_windows_bin_path): stage canonical launchers, verify them BEFORE touching the registry, prepend hermes\bin to the user PATH, strip the legacy entries (hermes-agent\bin and venv\Scripts, #83797), preserving REG_EXPAND_SZ and raw %VARS%. The legacy dir's files stay on purpose -- configs holding absolute launcher paths keep working; only the sweepable PATH resolution route goes. - fresh installs get the new layout from install.ps1 directly. /bin/ is gitignored so the one update that DELIVERS this fix cannot sweep pre-migration launchers a final time under the old rules; the gitignore line, the legacy re-stage branch, and the update-tail call are transition machinery with a named expiry once the fleet has migrated. Also rewrites _ensure_acp_launcher's stale Windows paragraph to match (raw docstring fixes its invalid \S escape) and updates the Windows native docs to the new layout, with a docs<->installer parity test. --- .gitignore | 1 + hermes_cli/_install_repair.py | 335 +++++++++++++++++ hermes_cli/main.py | 25 ++ hermes_cli/update_cmd.py | 30 +- scripts/install.ps1 | 65 +++- .../test_ensure_windows_bin_launchers.py | 342 ++++++++++++++++++ tests/hermes_cli/test_windows_native_docs.py | 17 +- website/docs/user-guide/windows-native.md | 12 +- 8 files changed, 791 insertions(+), 36 deletions(-) create mode 100644 tests/hermes_cli/test_ensure_windows_bin_launchers.py diff --git a/.gitignore b/.gitignore index ba347a4a7f..fb0f257095 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /venv/ /venv.old/ /venv.stale.runtime-*/ +/bin/ /.hermes-runtime/ /_pycache/ *.pyc* diff --git a/hermes_cli/_install_repair.py b/hermes_cli/_install_repair.py index 76e78adb98..05da09b42f 100644 --- a/hermes_cli/_install_repair.py +++ b/hermes_cli/_install_repair.py @@ -117,6 +117,341 @@ def _venv_scripts_dir(root: Path) -> Path | None: return scripts if scripts.is_dir() else None +#: Launcher command names install.ps1's Set-PathVariable exposes from the +#: managed binary dir (the default Hermes root's ``bin``, next to uv.exe) +#: on the user PATH. Keep in lockstep with the launcher list in +#: scripts/install.ps1. +_WINDOWS_BIN_LAUNCHERS = ("hermes", "hermes-acp") + + +def _venv_is_relocatable(venv_dir: Path) -> bool: + """True when the venv's pyvenv.cfg declares ``relocatable = true``. + + uv writes the flag; ``hermes_cli.managed_uv`` builds its replacement + venvs with ``--relocatable`` (they are constructed aside and swapped + into place). A relocatable venv's console-script trampolines embed a + RELATIVE interpreter reference, so a COPY of one placed outside + ``venv\\Scripts`` fails at run time with ``uv trampoline failed to + canonicalize script path``. Non-relocatable venvs (fresh installs) + embed the absolute interpreter path and their trampolines survive + copying. This flag decides which launcher form a PATH dir gets. + """ + try: + cfg = (Path(venv_dir) / "pyvenv.cfg").read_text( + encoding="utf-8", errors="replace" + ) + except OSError: + return False + for line in cfg.splitlines(): + key, _, value = line.partition("=") + if key.strip().lower() == "relocatable" and value.strip().lower() == "true": + return True + return False + + +def _normalize_windows_path(value) -> str: + """Windows path equality key: backslashes, no trailing separator, lowered. + + Lowercase via ``.lower()`` (what ``ntpath.normcase`` does) rather than + ``os.path.normcase`` — that is an identity function on POSIX, and this + comparison must behave Windows-correct even when tests exercise the + Windows branch from another host (same rationale as + ``venv_bin_dir(windows=...)``). + """ + return str(value).replace("/", "\\").rstrip("\\").lower() + + +def _windows_user_path_entries() -> list[str]: + """User PATH entries from the registry — the value install.ps1 writes. + + Falls back to the process PATH when the registry is unreadable. Only + called on Windows. + """ + try: + import winreg + + with winreg.OpenKey(winreg.HKEY_CURRENT_USER, "Environment") as key: + raw, _kind = winreg.QueryValueEx(key, "Path") + value = os.path.expandvars(str(raw)) + except (OSError, ImportError): + value = os.environ.get("PATH", "") + return [entry for entry in value.split(";") if entry.strip()] + + +def ensure_windows_bin_launchers( + root, + *, + windows: bool | None = None, + user_path_entries: list[str] | None = None, +) -> list[str]: + """Re-stage the Windows ``hermes`` launchers when they vanish. + + On Windows, ``hermes`` resolves through launchers derived from the venv + console scripts — never ``venv\\Scripts`` itself on PATH, which would + shadow the user's ``python`` (#83797). The canonical launcher home is + the managed binary dir — the default Hermes root's ``bin`` + (``%LOCALAPPDATA%\\hermes\\bin``, next to the managed uv) — which lives + OUTSIDE the git checkout so no git operation can ever touch it. It is + a per-machine dir shared by every profile: ``get_hermes_home()`` would + point inside ``profiles\\`` under ``hermes -p``, so the anchor + here is :func:`hermes_constants.get_default_hermes_root`. + + Earlier installer versions staged them at ``\\bin`` instead — + inside the git working tree — where ``hermes update``'s pre-update + autostash (``git stash push --include-untracked``) swept them off disk; + once the desktop updater stopped re-applying stashes (``--keep-stash``) + nothing restored them and ``hermes`` stopped resolving in every new + terminal. That legacy location is re-staged too, during the transition, + for installs whose user PATH still resolves through it. + + The launcher FORM depends on the venv (see :func:`_venv_is_relocatable`): + a normal venv's exe trampoline embeds an absolute interpreter path and + survives copying, so it is copied as ``.exe``; a relocatable + venv's trampoline resolves relative to its own location and a copy + dies with ``uv trampoline failed to canonicalize script path``, so a + ``.cmd`` delegator invoking the in-venv exe by absolute path is + written instead. A name counts as present when EITHER form exists — + exe copies staged before a venv rebuild keep working (they embed the + swapped-in-place venv's absolute path) and are left alone. + + Two targets, two gates, both failing toward inaction: + + - canonical managed binary dir: only when *root* is the managed clone + (``root.parent == get_default_hermes_root()``), so source checkouts + elsewhere never gain launchers; + - legacy ``\\bin``: only when that dir is on the user PATH + (registry value, process PATH as fallback), i.e. the install opted + into the old layout and still resolves through it. + + Writes go through a staging name + ``os.replace`` so concurrent process + starts cannot tear a launcher. Never raises; returns the restored paths. + + *windows* and *user_path_entries* are injectable for tests, same pattern + as ``hermes_constants.venv_bin_dir``. + """ + if windows is None: + windows = _is_windows() + if not windows: + return [] + + root = Path(root) + + # Per-machine anchor: the DEFAULT Hermes root, not get_hermes_home() — + # under ``hermes -p `` that returns ``profiles\\``, which + # would fail the managed-clone gate below and silently skip the heal + # for profile users. The launcher dir serves the whole machine. + from hermes_constants import get_default_hermes_root + + try: + home = Path(get_default_hermes_root()) + except Exception: + return [] + + def _launcher_present(target: Path, name: str) -> bool: + return (target / f"{name}.exe").exists() or (target / f"{name}.cmd").exists() + + targets: list[Path] = [] + + # Canonical target — gate on the managed-clone shape. This runs at + # every hermes_cli.main process start (right after the profile + # override), so the healthy path must stay at a couple of stat calls. + if _normalize_windows_path(root.parent) == _normalize_windows_path(home): + canonical = home / "bin" + if any(not _launcher_present(canonical, name) for name in _WINDOWS_BIN_LAUNCHERS): + targets.append(canonical) + + # Legacy transition target — the pre-migration in-checkout dir. Only + # re-staged while the user PATH still points at it (consent), compared + # as normalized literal strings: the installer wrote the long literal + # path, and realpath'ing arbitrary PATH entries could hang on dead + # network shares. An entry stored some other way (8.3 short path, + # subst drive) misses the re-stage, which fails safe: no-op. + legacy = root / "bin" + if any(not _launcher_present(legacy, name) for name in _WINDOWS_BIN_LAUNCHERS): + if user_path_entries is None: + user_path_entries = _windows_user_path_entries() + configured = {_normalize_windows_path(entry) for entry in user_path_entries} + if _normalize_windows_path(legacy) in configured: + targets.append(legacy) + + if not targets: + return [] + + from hermes_constants import project_venv_dir, venv_bin_dir + + venv_dir = project_venv_dir(root) + if venv_dir is None: + return [] + scripts_dir = venv_bin_dir(venv_dir, windows=windows) + sources = [ + (name, scripts_dir / f"{name}.exe") + for name in _WINDOWS_BIN_LAUNCHERS + if (scripts_dir / f"{name}.exe").is_file() + ] + if not sources: + return [] + relocatable = _venv_is_relocatable(venv_dir) + + import shutil + + restored: list[str] = [] + for target in targets: + try: + target.mkdir(parents=True, exist_ok=True) + except OSError: + continue + for name, source in sources: + if _launcher_present(target, name): + continue + final = target / (f"{name}.cmd" if relocatable else f"{name}.exe") + staging = target / f"{final.name}.heal.{os.getpid()}" + try: + if relocatable: + staging.write_text( + "@echo off\r\n" f'"{source}" %*\r\n', encoding="ascii" + ) + else: + shutil.copy2(source, staging) + os.replace(staging, final) + restored.append(str(final)) + except OSError: + with contextlib.suppress(OSError): + staging.unlink() + if restored: + # Guarded like everything else in this never-raises helper: a + # closed/broken stderr must not turn a successful heal into a crash. + with contextlib.suppress(OSError, ValueError): + print( + " ✓ Restored hermes launcher(s): " + ", ".join(restored), + file=sys.stderr, + ) + return restored + + +def _read_user_path_raw() -> tuple[list[str], int]: + """Raw (unexpanded) user PATH entries + registry value type. + + Raw so a rewrite preserves ``%VARS%`` exactly as the user stored them + (same discipline as ``hermes_cli.uninstall``). Only called on Windows. + """ + import winreg + + with winreg.OpenKey(winreg.HKEY_CURRENT_USER, "Environment") as key: + try: + raw, kind = winreg.QueryValueEx(key, "Path") + except FileNotFoundError: + return [], winreg.REG_EXPAND_SZ + return [entry for entry in str(raw).split(";") if entry], int(kind) + + +def _write_user_path_raw(entries: list[str], kind: int) -> None: + """Write the user PATH back, preserving the registry value type.""" + import winreg + + with winreg.OpenKey( + winreg.HKEY_CURRENT_USER, "Environment", 0, winreg.KEY_READ | winreg.KEY_WRITE + ) as key: + winreg.SetValueEx(key, "Path", 0, kind, ";".join(entries)) + + +def migrate_windows_bin_path( + root, + *, + windows: bool | None = None, + read_user_path=None, + write_user_path=None, +) -> bool: + """One-time PATH migration to the ``HERMES_HOME\\bin`` launcher layout. + + Runs from the ``hermes update`` tail (and mirrors what install.ps1's + Set-PathVariable does on fresh installs/repairs, which never reach + existing installs — updates don't run install.ps1): + + 1. stage the launcher copies into the managed binary dir (via + :func:`ensure_windows_bin_launchers`); + 2. verify both launchers are present there — otherwise STOP, leaving + the user PATH untouched (never strip a working entry before its + replacement is proven); + 3. ensure the managed binary dir is on the user PATH (prepend); + 4. strip the legacy entries: ``\\bin`` (in-checkout launcher dir + the update autostash could sweep) and ``\\venv\\Scripts`` + (shadowed the user's ``python``, #83797). + + The legacy ``\\bin`` FILES are deliberately left in place: editor + and ACP configs that captured absolute launcher paths keep working + (the launchers run fine from there — only PATH resolution through a + dir git could sweep was the bug), and the dir is git-ignored so it + cannot dirty the tree. + + Registry writes preserve the stored value type and raw ``%VARS%``. + Never raises; returns True when the canonical layout is in place. + + *read_user_path*/*write_user_path* are injectable for tests. + """ + if windows is None: + windows = _is_windows() + if not windows: + return False + + root = Path(root) + + # Same per-machine anchor as ensure_windows_bin_launchers (see there). + from hermes_constants import get_default_hermes_root + + try: + home = Path(get_default_hermes_root()) + except Exception: + return False + if _normalize_windows_path(root.parent) != _normalize_windows_path(home): + return False # not the managed clone — nothing to migrate + + ensure_windows_bin_launchers(root, windows=windows, user_path_entries=[]) + + home_bin = home / "bin" + if any( + not ((home_bin / f"{name}.exe").is_file() or (home_bin / f"{name}.cmd").is_file()) + for name in _WINDOWS_BIN_LAUNCHERS + ): + return False # staging incomplete — leave the PATH alone + + if read_user_path is None: + read_user_path = _read_user_path_raw + if write_user_path is None: + write_user_path = _write_user_path_raw + + try: + entries, kind = read_user_path() + except (OSError, ImportError): + return False + + legacy_keys = { + _normalize_windows_path(root / "bin"), + _normalize_windows_path(root / "venv" / "Scripts"), + } + home_bin_key = _normalize_windows_path(home_bin) + + def _entry_key(entry: str) -> str: + return _normalize_windows_path(os.path.expandvars(entry)) + + kept = [e for e in entries if _entry_key(e) not in legacy_keys] + have_home_bin = any(_entry_key(e) == home_bin_key for e in kept) + if not have_home_bin: + kept = [str(home_bin)] + kept + + if kept != entries: + try: + write_user_path(kept, kind) + except (OSError, ImportError): + return False + with contextlib.suppress(OSError, ValueError): + print( + f" ✓ hermes launchers now resolve from {home_bin} " + "(legacy PATH entries removed)", + file=sys.stderr, + ) + return True + + def _load_console_script_names(root: Path) -> list[str]: """``[project.scripts]`` names from pyproject.toml (tomllib, 3.11+).""" try: diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 9f5e1ec934..4582b31e02 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -693,6 +693,31 @@ def _apply_profile_override() -> None: _apply_profile_override() +# Windows launcher self-heal — the ``hermes`` command users run is a COPY of +# the venv console script, staged into the managed binary dir (the default +# Hermes root's ``bin``, next to the managed uv) by install.ps1. That dir +# lives OUTSIDE the git checkout precisely because an earlier layout staged +# the copies at ``\bin``, where ``hermes update``'s autostash +# (``git stash push --include-untracked``) swept them off disk; with the +# desktop updater's ``--keep-stash`` nothing restored them and ``hermes`` +# stopped resolving in every new terminal (venv\Scripts itself must stay off +# PATH — it shadows the user's ``python``, #83797). Re-staging at process +# start reaches already-broken installs through the one channel that still +# works there: the desktop app spawning its backend via +# ``python -m hermes_cli.main``. Costs a few stat calls when healthy; gates +# fail toward inaction so source checkouts are untouched. Sits AFTER the +# profile override on purpose — no hermes module may be imported before +# profiles resolve. The launcher dir itself is per-machine (the helper +# anchors on the DEFAULT root, not HERMES_HOME), so profile sessions heal +# the same shared dir. +if sys.platform == "win32": + try: + from hermes_cli import _install_repair as _install_repair_mod + + _install_repair_mod.ensure_windows_bin_launchers(_bootstrap_root) + except Exception: + pass + # Load .env from ~/.hermes/.env first, then project root as dev fallback. # User-managed env files should override stale shell exports on restart. from hermes_cli.config import get_hermes_home diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index f41a37fb67..4fd762c70d 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -3181,7 +3181,7 @@ def _ensure_fhs_path_guard() -> None: print(" (reload your shell or run 'source ~/.bashrc' to pick it up)") def _ensure_acp_launcher() -> None: - """Self-heal: install a ``hermes-acp`` launcher next to the ``hermes`` one. + r"""Self-heal: install a ``hermes-acp`` launcher next to the ``hermes`` one. Mirrors the launcher block in ``scripts/install.sh`` so existing installs gain the ACP command on ``hermes update`` without a reinstall. ACP hosts @@ -3197,10 +3197,12 @@ def _ensure_acp_launcher() -> None: No-op on Windows (install.ps1 copies ``hermes.exe`` + ``hermes-acp.exe`` into ``$InstallDir\bin`` and puts THAT on the user PATH — never the whole - ``venv\Scripts`` dir, which would shadow the user's ``python`` (#83797) — - so ``hermes-acp.exe`` already resolves) and wherever a ``hermes-acp`` is - already present next to the ``hermes`` command. Unwritable directories - (e.g. ``/usr/local/bin`` as non-root) are skipped silently. Idempotent. + ``venv\Scripts`` dir, which would shadow the user's ``python`` (#83797); + when those copies go missing, ``hermes_cli._install_repair. + ensure_windows_bin_launchers`` re-stages them) and wherever a + ``hermes-acp`` is already present next to the ``hermes`` command. + Unwritable directories (e.g. ``/usr/local/bin`` as non-root) are skipped + silently. Idempotent. """ if _m().sys.platform == "win32": return @@ -6486,6 +6488,24 @@ def _cmd_update_impl(args, gateway_mode: bool): except Exception as e: logger.debug("hermes-acp launcher self-heal failed: %s", e) + # Migrate the Windows hermes launchers to the managed binary dir + # (the default Hermes root's bin, next to the managed uv) and repair + # them if they are missing. Earlier layouts put them inside the git + # checkout (hermes-agent\bin) or put venv\Scripts itself on PATH; the + # in-checkout copies were swept by this command's own pre-update + # autostash (git stash push --include-untracked) and, with + # --keep-stash (the desktop updater), never restored — `hermes` + # stopped resolving in every new terminal. Updates never run + # install.ps1, so this tail call is how existing installs reach the + # new layout. No-op on POSIX and on source checkouts (root is not + # the managed clone under the default Hermes root). + try: + from hermes_cli._install_repair import migrate_windows_bin_path + + migrate_windows_bin_path(_m().PROJECT_ROOT) + except Exception as e: + logger.debug("Windows bin launcher migration failed: %s", e) + # Refresh the cua-driver binary used by the Computer Use toolset. # The upstream installer is gated on supported platforms and on the # binary already being on PATH, so this is a no-op for users who diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 38c2012a97..48dd10e8ad 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -2987,31 +2987,60 @@ function Set-PathVariable { # venv\Scripts directory. venv\Scripts contains python.exe / # pythonw.exe / pip.exe, and putting it on the user PATH silently # hijacks the `python` command in every terminal on the machine - # (#83797): unrelated projects start resolving python to Hermes' - # runtime interpreter. A dedicated bin dir with copies of the - # launcher exes keeps `hermes` globally available without - # shadowing anything. (Launcher exes embed the venv interpreter - # path, so they work from any location and survive updates.) - $hermesBin = "$InstallDir\bin" + # (#83797). And never a directory inside the git checkout: + # `hermes update`'s autostash (git stash push --include-untracked) + # deletes untracked files from the working tree, which silently + # removed the launchers an earlier installer staged under + # hermes-agent\bin. $HermesHome\bin is the managed binary dir + # (shared with the managed uv), outside the checkout, where no git + # operation can ever touch it. (Launcher exes embed the venv + # interpreter path, so they work from any location and survive + # updates.) + $hermesBin = "$HermesHome\bin" New-Item -ItemType Directory -Force -Path $hermesBin | Out-Null - foreach ($launcher in @("hermes.exe", "hermes-acp.exe")) { - $src = "$InstallDir\venv\Scripts\$launcher" - if (Test-Path $src) { - Copy-Item -Force $src "$hermesBin\$launcher" + # Launcher form depends on the venv (keep in lockstep with + # hermes_cli/_install_repair.py): a normal venv's exe trampoline + # embeds an absolute interpreter path and survives copying; a + # relocatable venv's trampoline (managed_uv rebuilds use + # --relocatable) resolves relative to its own location, and a copy + # dies with 'uv trampoline failed to canonicalize script path' -- + # those get a .cmd delegator invoking the in-venv exe instead. + $pyvenvCfg = "$InstallDir\venv\pyvenv.cfg" + $venvRelocatable = $false + if (Test-Path $pyvenvCfg) { + $venvRelocatable = [bool](Select-String -Path $pyvenvCfg -Pattern '^\s*relocatable\s*=\s*true\s*$' -Quiet) + } + foreach ($launcher in @("hermes", "hermes-acp")) { + $src = "$InstallDir\venv\Scripts\$launcher.exe" + if (-not (Test-Path $src)) { continue } + if ($venvRelocatable) { + Remove-Item "$hermesBin\$launcher.exe" -Force -ErrorAction SilentlyContinue + Set-Content -Path "$hermesBin\$launcher.cmd" -Value "@echo off`r`n`"$src`" %*" -Encoding Ascii + } else { + Remove-Item "$hermesBin\$launcher.cmd" -Force -ErrorAction SilentlyContinue + Copy-Item -Force $src "$hermesBin\$launcher.exe" } } } $currentPath = [Environment]::GetEnvironmentVariable("Path", "User") - # Migrate installs that got venv\Scripts onto PATH from earlier - # installer versions -- remove it so the python shadowing stops. - $legacyBin = "$InstallDir\venv\Scripts" - if ((-not $NoVenv) -and $currentPath -like "*$legacyBin*") { - $cleaned = ($currentPath -split ';' | Where-Object { $_ -and $_ -ne $legacyBin }) -join ';' - [Environment]::SetEnvironmentVariable("Path", $cleaned, "User") - $currentPath = $cleaned - Write-Info "Removed legacy venv\Scripts from user PATH (kept hermes via $hermesBin)" + # Migrate older layouts off the user PATH: + # venv\Scripts -- shadowed the user's python (#83797) + # hermes-agent\bin -- lived inside the git checkout, where the update + # autostash could sweep the launchers off disk + # The hermes-agent\bin FILES are left in place on purpose: editor/ACP + # configs that captured absolute launcher paths keep working, and the + # dir is git-ignored so it cannot dirty the checkout. + if (-not $NoVenv) { + $legacyEntries = @("$InstallDir\venv\Scripts", "$InstallDir\bin") + $items = @(($currentPath -split ';') | Where-Object { $_ }) + $cleaned = @($items | Where-Object { $legacyEntries -notcontains $_ }) + if ($cleaned.Count -ne $items.Count) { + $currentPath = $cleaned -join ";" + [Environment]::SetEnvironmentVariable("Path", $currentPath, "User") + Write-Info "Removed legacy launcher entries from user PATH (kept hermes via $hermesBin)" + } } if ($currentPath -notlike "*$hermesBin*") { diff --git a/tests/hermes_cli/test_ensure_windows_bin_launchers.py b/tests/hermes_cli/test_ensure_windows_bin_launchers.py new file mode 100644 index 0000000000..e2001cbf7d --- /dev/null +++ b/tests/hermes_cli/test_ensure_windows_bin_launchers.py @@ -0,0 +1,342 @@ +"""``hermes`` must survive git operations on the checkout (launcher layout). + +The Windows ``hermes`` command is a launcher derived from the venv console +script. Its canonical home is the managed binary dir ``HERMES_HOME\\bin`` — +OUTSIDE the git checkout — because the earlier in-checkout home +(``hermes-agent\\bin``) was swept by ``hermes update``'s autostash +(``git stash push --include-untracked``) and, with the desktop updater's +``--keep-stash``, never restored: ``hermes`` stopped resolving in every new +terminal (``venv\\Scripts`` itself must stay off PATH — it shadows the +user's ``python``, #83797). + +``ensure_windows_bin_launchers`` re-stages missing launchers (canonical dir +always for the managed clone; legacy dir only while the user PATH still +points at it), choosing the form by venv kind: exe copy for normal venvs, +``.cmd`` delegator for relocatable venvs whose exe trampolines die when +copied out of ``venv\\Scripts``. ``migrate_windows_bin_path`` moves an +existing install's PATH to the canonical layout from the ``hermes update`` +tail. Platform verdict, PATH values, and registry I/O are injected +parameters (same pattern as ``hermes_constants.venv_bin_dir``), so these +tests are host-independent input→output checks, not host fakes. +""" + +from pathlib import Path + +import pytest + +from hermes_cli._install_repair import ( + _WINDOWS_BIN_LAUNCHERS, + _normalize_windows_path, + ensure_windows_bin_launchers, + migrate_windows_bin_path, +) + + +def _make_managed(tmp_path, monkeypatch, *, relocatable: bool = False): + """Fake managed layout: HERMES_HOME/hermes-agent/venv/Scripts + launchers.""" + home = tmp_path / "hermes" + root = home / "hermes-agent" + scripts = root / "venv" / "Scripts" + scripts.mkdir(parents=True) + for name in _WINDOWS_BIN_LAUNCHERS: + (scripts / f"{name}.exe").write_bytes(b"MZ console script: " + name.encode()) + cfg = "home = X\nversion_info = 3.11.15\n" + if relocatable: + cfg += "relocatable = true\n" + (root / "venv" / "pyvenv.cfg").write_text(cfg, encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(home)) + return home, root + + +@pytest.fixture +def managed_install(tmp_path, monkeypatch): + return _make_managed(tmp_path, monkeypatch) + + +def test_managed_clone_heals_canonical_home_bin(managed_install): + home, root = managed_install + + restored = ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) + + assert len(restored) == len(_WINDOWS_BIN_LAUNCHERS) + for name in _WINDOWS_BIN_LAUNCHERS: + assert (home / "bin" / f"{name}.exe").read_bytes() == ( + root / "venv" / "Scripts" / f"{name}.exe" + ).read_bytes() + + +def test_relocatable_venv_gets_cmd_delegators_not_exe_copies(tmp_path, monkeypatch): + """A copied relocatable-venv trampoline dies ('uv trampoline failed to + canonicalize script path') — the heal must emit .cmd delegators.""" + home, root = _make_managed(tmp_path, monkeypatch, relocatable=True) + + restored = ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) + + assert {Path(p).suffix for p in restored} == {".cmd"} + for name in _WINDOWS_BIN_LAUNCHERS: + body = (home / "bin" / f"{name}.cmd").read_text(encoding="ascii") + # Delegates to the in-venv exe by absolute path, forwarding args. + assert str(root / "venv" / "Scripts" / f"{name}.exe") in body + assert "%*" in body + assert not (home / "bin" / f"{name}.exe").exists() + + +def test_existing_exe_counts_as_present_for_relocatable_venv(tmp_path, monkeypatch): + """Exe copies staged before a venv rebuild embed the swapped-in-place + venv's absolute path and keep working — never replaced with .cmd.""" + home, root = _make_managed(tmp_path, monkeypatch, relocatable=True) + (home / "bin").mkdir() + for name in _WINDOWS_BIN_LAUNCHERS: + (home / "bin" / f"{name}.exe").write_bytes(b"pre-rebuild copy") + + assert ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) == [] + for name in _WINDOWS_BIN_LAUNCHERS: + assert (home / "bin" / f"{name}.exe").read_bytes() == b"pre-rebuild copy" + assert not (home / "bin" / f"{name}.cmd").exists() + + +def test_healthy_canonical_layout_is_a_noop(managed_install): + home, root = managed_install + (home / "bin").mkdir() + for name in _WINDOWS_BIN_LAUNCHERS: + (home / "bin" / f"{name}.exe").write_bytes(b"present") + + assert ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) == [] + + +def test_legacy_bin_restaged_only_while_on_user_path(managed_install): + home, root = managed_install + legacy = root / "bin" + + restored = ensure_windows_bin_launchers( + root, windows=True, user_path_entries=[str(legacy)] + ) + + stems = {Path(p).stem for p in restored} + assert set(_WINDOWS_BIN_LAUNCHERS) <= stems + for name in _WINDOWS_BIN_LAUNCHERS: + assert (legacy / f"{name}.exe").is_file() # legacy consent honored + assert (home / "bin" / f"{name}.exe").is_file() # canonical healed too + + +def test_legacy_bin_not_restaged_without_path_consent(managed_install): + home, root = managed_install + + ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) + + assert not (root / "bin").exists() + + +def test_source_checkout_untouched(tmp_path, monkeypatch): + """A checkout NOT under HERMES_HOME gains nothing anywhere.""" + home = tmp_path / "hermes-home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + root = tmp_path / "src" / "hermes-agent" + scripts = root / "venv" / "Scripts" + scripts.mkdir(parents=True) + for name in _WINDOWS_BIN_LAUNCHERS: + (scripts / f"{name}.exe").write_bytes(b"MZ") + + assert ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) == [] + assert not (home / "bin").exists() + assert not (root / "bin").exists() + + +def test_noop_on_posix(managed_install): + home, root = managed_install + + assert ensure_windows_bin_launchers(root, windows=False) == [] + assert not (home / "bin").exists() + + +def test_profile_session_still_heals_the_shared_bin(tmp_path, monkeypatch): + """Under ``hermes -p `` HERMES_HOME points inside profiles/; + the launcher dir is per-machine, so the heal must anchor on the default + root and fire anyway — a habitual profile user gets the same repair.""" + home = tmp_path / "hermes" + root = home / "hermes-agent" + scripts = root / "venv" / "Scripts" + scripts.mkdir(parents=True) + for name in _WINDOWS_BIN_LAUNCHERS: + (scripts / f"{name}.exe").write_bytes(b"MZ") + (root / "venv" / "pyvenv.cfg").write_text("home = X\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(home / "profiles" / "work")) + + restored = ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) + + assert len(restored) == len(_WINDOWS_BIN_LAUNCHERS) + for name in _WINDOWS_BIN_LAUNCHERS: + assert (home / "bin" / f"{name}.exe").is_file() + assert not (home / "profiles" / "work" / "bin").exists() + + +def test_noop_when_console_scripts_missing(tmp_path, monkeypatch): + """A venv mid-repair has no console scripts — nothing to copy, no error.""" + home = tmp_path / "hermes" + root = home / "hermes-agent" + (root / "venv" / "Scripts").mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(home)) + + assert ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) == [] + + +def test_no_staging_litter_left_behind(managed_install): + home, root = managed_install + + ensure_windows_bin_launchers(root, windows=True, user_path_entries=[]) + + leftovers = [p.name for p in (home / "bin").iterdir() if ".heal." in p.name] + assert leftovers == [] + + +# --------------------------------------------------------------------------- +# migrate_windows_bin_path — the `hermes update` tail migration +# --------------------------------------------------------------------------- + + +def _fake_registry(initial: list[str]): + """In-memory user-PATH store standing in for the HKCU registry value.""" + state = {"entries": list(initial), "kind": 2, "writes": 0} + + def read(): + return list(state["entries"]), state["kind"] + + def write(entries, kind): + state["entries"] = list(entries) + state["kind"] = kind + state["writes"] += 1 + + return state, read, write + + +def test_migration_moves_path_to_home_bin_and_strips_legacy(managed_install): + home, root = managed_install + legacy_bin = str(root / "bin") + legacy_scripts = str(root / "venv" / "Scripts") + state, read, write = _fake_registry( + [legacy_bin, legacy_scripts, r"C:\Windows\system32"] + ) + (root / "bin").mkdir() + (root / "bin" / "hermes.exe").write_bytes(b"legacy copy") + + ok = migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + + assert ok + keys = [_normalize_windows_path(e) for e in state["entries"]] + assert _normalize_windows_path(home / "bin") in keys + assert _normalize_windows_path(legacy_bin) not in keys + assert _normalize_windows_path(legacy_scripts) not in keys + assert _normalize_windows_path(r"C:\Windows\system32") in keys # untouched + for name in _WINDOWS_BIN_LAUNCHERS: + assert (home / "bin" / f"{name}.exe").is_file() + # Legacy FILES stay: editor/ACP configs holding absolute launcher paths + # keep working. Only the PATH entry (the sweepable resolution route) goes. + assert (root / "bin" / "hermes.exe").read_bytes() == b"legacy copy" + + +def test_migration_works_for_relocatable_venv(tmp_path, monkeypatch): + home, root = _make_managed(tmp_path, monkeypatch, relocatable=True) + state, read, write = _fake_registry([str(root / "bin")]) + + ok = migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + + assert ok + for name in _WINDOWS_BIN_LAUNCHERS: + assert (home / "bin" / f"{name}.cmd").is_file() + keys = [_normalize_windows_path(e) for e in state["entries"]] + assert _normalize_windows_path(home / "bin") in keys + + +def test_migration_is_idempotent(managed_install): + home, root = managed_install + state, read, write = _fake_registry([str(home / "bin"), r"C:\Windows\system32"]) + + assert migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + first_entries = list(state["entries"]) + first_writes = state["writes"] + + assert migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + assert state["entries"] == first_entries + assert state["writes"] == first_writes # no redundant registry write + + +def test_migration_never_strips_path_when_staging_fails(tmp_path, monkeypatch): + """No venv sources → launchers can't stage → PATH must stay untouched.""" + home = tmp_path / "hermes" + root = home / "hermes-agent" + (root / "venv" / "Scripts").mkdir(parents=True) # no launcher exes inside + monkeypatch.setenv("HERMES_HOME", str(home)) + legacy_bin = str(root / "bin") + state, read, write = _fake_registry([legacy_bin]) + + ok = migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + + assert not ok + assert state["entries"] == [legacy_bin] # working entry preserved + assert state["writes"] == 0 + + +def test_migration_skips_source_checkouts(tmp_path, monkeypatch): + home = tmp_path / "hermes-home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + root = tmp_path / "src" / "hermes-agent" + scripts = root / "venv" / "Scripts" + scripts.mkdir(parents=True) + for name in _WINDOWS_BIN_LAUNCHERS: + (scripts / f"{name}.exe").write_bytes(b"MZ") + state, read, write = _fake_registry([r"C:\Windows\system32"]) + + assert not migrate_windows_bin_path( + root, windows=True, read_user_path=read, write_user_path=write + ) + assert state["writes"] == 0 + + +def test_migration_noop_on_posix(managed_install): + home, root = managed_install + + assert not migrate_windows_bin_path(root, windows=False) + + +def test_normalize_windows_path_equivalences(): + assert ( + _normalize_windows_path(r"C:\Users\Me\AppData\Local\hermes\bin") + == _normalize_windows_path("c:/users/me/appdata/local/HERMES/BIN/") + ) + + +def test_repo_gitignores_the_legacy_bin_dir(): + """Transition safety: legacy in-checkout launchers must not be stash-swept. + + Until every install has migrated, pre-migration checkouts still carry + launchers at ``/bin``. ``hermes update`` autostashes with + ``git stash push --include-untracked``; anything untracked and NOT + ignored inside the checkout gets swept off disk. Exercises git's real + ignore machinery rather than reading .gitignore text. + """ + import subprocess + + repo_root = Path(__file__).resolve().parents[2] + if not (repo_root / ".git").exists(): + pytest.skip("not running from a git checkout") + + result = subprocess.run( + ["git", "-C", str(repo_root), "check-ignore", "-q", "bin/hermes.exe"], + capture_output=True, + ) + assert result.returncode == 0, ( + "bin/hermes.exe is not gitignored — hermes update's autostash " + "(--include-untracked) would sweep pre-migration launchers off disk" + ) diff --git a/tests/hermes_cli/test_windows_native_docs.py b/tests/hermes_cli/test_windows_native_docs.py index 6e53ff7a0f..2594ff8d48 100644 --- a/tests/hermes_cli/test_windows_native_docs.py +++ b/tests/hermes_cli/test_windows_native_docs.py @@ -5,15 +5,18 @@ def test_windows_native_install_path_docs_match_installer() -> None: doc = Path("website/docs/user-guide/windows-native.md").read_text() install = Path("scripts/install.ps1").read_text() - # The launchers live in a dedicated bin/ dir on PATH — NOT the whole - # venv\Scripts (which would shadow the user's python, #83797). - assert "%LOCALAPPDATA%\\hermes\\hermes-agent\\bin" in doc + # The launchers live in the managed binary dir OUTSIDE the git checkout + # (HERMES_HOME\bin, next to the managed uv) — NOT the whole venv\Scripts + # (which would shadow the user's python, #83797) and NOT a dir inside + # the checkout (which `hermes update`'s autostash swept off disk). + assert "%LOCALAPPDATA%\\hermes\\bin" in doc assert ( "Get-Command hermes # should print " - "C:\\Users\\\\AppData\\Local\\hermes\\hermes-agent\\bin\\hermes.exe" + "C:\\Users\\\\AppData\\Local\\hermes\\bin\\hermes.exe" ) in doc - # Installer exposes $InstallDir\bin, and must copy the launchers into it. - assert '$hermesBin = "$InstallDir\\bin"' in install + # Installer exposes $HermesHome\bin, and must copy the launchers into it. + assert '$hermesBin = "$HermesHome\\bin"' in install assert "hermes.exe" in install and "hermes-acp.exe" in install - # Guard against a regression back to putting venv\Scripts on PATH. + # Guard against regressions to either legacy layout. assert '$hermesBin = "$InstallDir\\venv\\Scripts"' not in install + assert '$hermesBin = "$InstallDir\\bin"' not in install diff --git a/website/docs/user-guide/windows-native.md b/website/docs/user-guide/windows-native.md index 4e0a48c0b0..f703dfe286 100644 --- a/website/docs/user-guide/windows-native.md +++ b/website/docs/user-guide/windows-native.md @@ -75,7 +75,7 @@ Top-to-bottom, in order: 6. **Tiered `uv pip install`** — tries `.[all]` first, falls back to progressively smaller sets (`[messaging,dashboard,ext]` → `[messaging]` → `.`) if a `git+https` dep flakes on rate-limited GitHub. Prevents "single flake drops you to a bare install" failure mode. 7. **Auto-installs messaging SDKs** keyed off `.env` — if `TELEGRAM_BOT_TOKEN` / `DISCORD_BOT_TOKEN` / `SLACK_BOT_TOKEN` / `SLACK_APP_TOKEN` / `WHATSAPP_ENABLED` are present, runs `python -m ensurepip --upgrade` and targeted `pip install` calls so each platform's SDK is actually importable. 8. **Sets `HERMES_GIT_BASH_PATH`** to the resolved `bash.exe` so Hermes finds it deterministically in fresh shells. -9. **Adds `%LOCALAPPDATA%\hermes\hermes-agent\bin` to User PATH and sets `HERMES_HOME=%LOCALAPPDATA%\hermes`** — exposes the `hermes` command (and points it at your data dir) after you open a new terminal. Only the `hermes.exe` / `hermes-acp.exe` launchers are copied into this `bin` directory; the full `venv\Scripts` is deliberately **not** placed on PATH so Hermes never shadows your own `python` command. +9. **Adds `%LOCALAPPDATA%\hermes\bin` to User PATH and sets `HERMES_HOME=%LOCALAPPDATA%\hermes`** — exposes the `hermes` command (and points it at your data dir) after you open a new terminal. Only the `hermes.exe` / `hermes-acp.exe` launchers are copied into this `bin` directory; the full `venv\Scripts` is deliberately **not** placed on PATH so Hermes never shadows your own `python` command. 10. **Runs `hermes setup`** — the normal first-run wizard (model, provider, toolsets). Skip with `-SkipSetup`. :::tip Skip provider hunting on Windows @@ -202,10 +202,10 @@ Services require admin rights to install and tie the gateway's lifecycle to mach | Path | Contents | |---|---| -| `%LOCALAPPDATA%\hermes\hermes-agent\` | Git checkout + venv. The `bin\hermes.exe` launcher (copied from `venv\Scripts\hermes.exe`) is the command added to User PATH. Safe to `Remove-Item -Recurse` and reinstall. | +| `%LOCALAPPDATA%\hermes\hermes-agent\` | Git checkout + venv. Safe to `Remove-Item -Recurse` and reinstall. | | `%LOCALAPPDATA%\hermes\git\` | PortableGit (only if the installer provisioned it). | | `%LOCALAPPDATA%\hermes\node\` | Portable Node.js (only if the installer provisioned it). | -| `%LOCALAPPDATA%\hermes\bin\` | Hermes's managed `uv.exe` (the Python manager it uses for updates). | +| `%LOCALAPPDATA%\hermes\bin\` | The `hermes` / `hermes-acp` launchers and Hermes's managed `uv.exe` (the Python manager it uses for updates). | | `%LOCALAPPDATA%\hermes\` (root) | Your config, auth, skills, sessions, logs (`config.yaml`, `.env`, `skills\`, `sessions\`, `logs\`, …). **Survives reinstalls.** | On native Windows the installer sets `HERMES_HOME=%LOCALAPPDATA%\hermes`, so your data and the disposable install live under the **same** `%LOCALAPPDATA%\hermes` root: the install/runtime is the `hermes-agent\`, `git\`, `node\`, and `bin\` subdirectories, while your data files sit directly in `%LOCALAPPDATA%\hermes`. Reinstalling only replaces the `hermes-agent\` checkout, so your data survives — but because the two share a root, **don't** `Remove-Item -Recurse %LOCALAPPDATA%\hermes` if you want to keep your data; delete the `hermes-agent\` subdirectory instead. Your data directory is identical in shape to a Linux `~/.hermes`, so you can mirror it between machines. @@ -224,12 +224,12 @@ The browser tool uses `agent-browser` (a Node helper) to drive Chromium. On Wind ### PATH after install -The installer adds `%LOCALAPPDATA%\hermes\hermes-agent\bin` to your **User PATH** via `[Environment]::SetEnvironmentVariable`. Existing terminals don't pick this up — open a new PowerShell window (or Windows Terminal tab) after installation. Close-and-reopen, don't `$env:PATH += …` by hand unless you know what you're doing. +The installer adds `%LOCALAPPDATA%\hermes\bin` to your **User PATH** via `[Environment]::SetEnvironmentVariable`. Existing terminals don't pick this up — open a new PowerShell window (or Windows Terminal tab) after installation. Close-and-reopen, don't `$env:PATH += …` by hand unless you know what you're doing. Verify: ```powershell -Get-Command hermes # should print C:\Users\\AppData\Local\hermes\hermes-agent\bin\hermes.exe +Get-Command hermes # should print C:\Users\\AppData\Local\hermes\bin\hermes.exe hermes --version ``` @@ -288,7 +288,7 @@ Consequence: any codepath that said "check if this PID is alive" via `os.kill(pi ## Common pitfalls **`hermes: command not found` right after install.** -Open a new PowerShell window. The installer added `%LOCALAPPDATA%\hermes\hermes-agent\bin` to User PATH, but existing shells need to be restarted to pick it up. In the meantime you can run `& "$env:LOCALAPPDATA\hermes\hermes-agent\bin\hermes.exe"`. +Open a new PowerShell window. The installer added `%LOCALAPPDATA%\hermes\bin` to User PATH, but existing shells need to be restarted to pick it up. In the meantime you can run `& "$env:LOCALAPPDATA\hermes\bin\hermes.exe"`. **`WinError 193: %1 is not a valid Win32 application` when running a tool.** You hit a shebang-script invocation that bypassed the `.cmd` shim. Hermes resolves commands through `shutil.which(cmd, path=local_bin)` so PATHEXT picks up `.CMD` — if you're invoking the tool via a hardcoded path instead, switch to the `.cmd` variant (e.g., `npx.cmd`, not `npx`).