fix(install): keep venv Scripts dir off the user PATH (#83797)

hermes_cli/uninstall.py

@@ -316,7 +316,8 @@ def uninstall_gateway_service():
 #      don't live in ~/.bashrc — they're in the Windows registry at
 #      HKCU\Environment.
 #   2. Prepends to User-scope ``PATH`` (same registry location) entries
-#      like ``%LOCALAPPDATA%\hermes\git\cmd``, ``%LOCALAPPDATA%\hermes\git\bin``,
+#      like ``%LOCALAPPDATA%\hermes\bin`` (Hermes .cmd shims; #83797),
+#      ``%LOCALAPPDATA%\hermes\git\cmd``, ``%LOCALAPPDATA%\hermes\git\bin``,
 #      ``%LOCALAPPDATA%\hermes\git\usr\bin``, ``%LOCALAPPDATA%\hermes\node``.
 #      Again not in any rc file — only accessible via the registry or the
 #      .NET [Environment] API.
@@ -340,8 +341,9 @@ def _hermes_path_markers(hermes_home: Path) -> list[str]:
     """Path-entry substrings that identify Hermes-owned User-PATH entries."""
     root = str(hermes_home).rstrip("\\/")
     # Match on prefix so sub-entries (git\cmd, git\bin, git\usr\bin, node, etc.)
-    # all get swept.  Also match the bare hermes-agent install dir.
-    markers = [root + "\\hermes-agent", root + "\\git", root + "\\node", root + "\\venv"]
+    # all get swept.  Also match the bare hermes-agent install dir and the
+    # Hermes-owned bin dir (hermes\bin) that ships .cmd shims (#83797).
+    markers = [root + "\\hermes-agent", root + "\\bin", root + "\\git", root + "\\node", root + "\\venv"]
     # Also match if HERMES_HOME was customised to somewhere else — find-and-nuke
     # any entry whose path component contains "hermes".  We don't want to catch
     # unrelated entries like "chermes-foo" or "ephermeral", so we look for

hermes_cli/update_cmd.py

@@ -2507,6 +2507,44 @@ def _ensure_fhs_path_guard() -> None:
     if wrote_any:
         print("    (reload your shell or run 'source ~/.bashrc' to pick it up)")
 
+def _ensure_windows_acp_shim() -> None:
+    """Self-heal the ``hermes-acp`` command on Windows.
+
+    install.ps1 ships ``hermes``/``hermes-acp`` as .cmd shims in
+    ``%LOCALAPPDATA%\\hermes\\bin`` and deliberately keeps the venv Scripts
+    dir off the user PATH (it contains python.exe/pip.exe — see #83797).
+    ``hermes update`` does not re-run install.ps1, so re-ensure the acp shim
+    here for installs that predate the shim layout (their venv Scripts entry
+    may still be on the user PATH, or may already have been migrated).
+
+    The target path is hardcoded to the standard layout, so installs with a
+    custom install dir (``target.is_file()`` false) degrade silently: no
+    shim is written and the existing ``hermes-acp`` resolution keeps
+    working the way the custom install configured it.
+    """
+    local_appdata = os.environ.get("LOCALAPPDATA", "")
+    if not local_appdata:
+        return
+    bin_dir = Path(local_appdata) / "hermes" / "bin"
+    target = (
+        Path(local_appdata) / "hermes" / "hermes-agent" / "venv" / "Scripts"
+        / "hermes-acp.exe"
+    )
+    if not target.is_file():
+        return
+    shim = bin_dir / "hermes-acp.cmd"
+    # Text-mode write_text/read_text translate newlines on Windows, which
+    # would double the CRLF; go through bytes so the shim is verbatim.
+    encoding = "mbcs" if os.name == "nt" else "utf-8"
+    payload = ("@echo off\r\n@\"{}\" %*\r\n".format(target)).encode(encoding)
+    try:
+        bin_dir.mkdir(parents=True, exist_ok=True)
+        if not shim.exists() or shim.read_bytes() != payload:
+            shim.write_bytes(payload)
+    except (OSError, UnicodeError):
+        return
+
+
 def _ensure_acp_launcher() -> None:
     """Self-heal: install a ``hermes-acp`` launcher next to the ``hermes`` one.
 
@@ -2522,12 +2560,16 @@ def _ensure_acp_launcher() -> None:
     (venv wrapper, FHS symlink, pipx/pip console script) without having to
     reconstruct interpreter/entrypoint paths.
 
-    No-op on Windows (install.ps1 puts ``venv\\Scripts`` on the user PATH, so
-    ``hermes-acp.exe`` already resolves) and wherever a ``hermes-acp`` is
-    already present next to the ``hermes`` command.  Unwritable directories
+    On Windows, ``hermes-acp.exe`` resolves because install.ps1 puts a
+    forwarding .cmd shim in ``%LOCALAPPDATA%\\hermes\\bin`` (which is on the
+    user PATH) — the venv Scripts dir is intentionally not on that PATH since
+    it also hosts python.exe (#83797). ``_ensure_windows_acp_shim`` keeps the
+    shim in place across updates; everywhere else this function installs a
+    shell shim next to the ``hermes`` command.  Unwritable directories
     (e.g. ``/usr/local/bin`` as non-root) are skipped silently.  Idempotent.
     """
     if _m().sys.platform == "win32":
+        _ensure_windows_acp_shim()
         return
     for bin_dir in (Path.home() / ".local" / "bin", Path("/usr/local/bin")):
         hermes_cmd = bin_dir / "hermes"

scripts/ci/test_install_ps1_hermes_shim_path.ps1

@@ -0,0 +1,229 @@
+# Behavioral test for install.ps1's User-PATH shim migration
+# (Update-UserPathForHermes): drops the legacy venv\Scripts entry that
+# hijacked the user's `python` command (#83797) and ensures the Hermes bin
+# (shim) dir is present.
+#
+# Run:  pwsh -NoProfile -File scripts/ci/test_install_ps1_hermes_shim_path.ps1
+#
+# Not wired into the default CI lane — the Linux runners have no PowerShell
+# host. It runs on any machine with pwsh (including via nixpkgs#powershell),
+# and on a Windows runner if one is ever added.
+#
+# Same harness style as test_install_ps1_path_migration.ps1: parses
+# install.ps1, lifts the real Update-UserPathForHermes body out of the AST,
+# and rewrites *only* the two registry calls into an in-memory store so the
+# actual shipped logic — split, drop-legacy, ensure-shim-dir, change-
+# detection — executes for real.
+
+Set-StrictMode -Version Latest
+$ErrorActionPreference = 'Stop'
+
+$installPs1 = Join-Path $PSScriptRoot '..' 'install.ps1' | Resolve-Path
+$ast = [System.Management.Automation.Language.Parser]::ParseFile(
+    $installPs1, [ref]$null, [ref]$null)
+
+$fn = $ast.Find({
+    param($n)
+    $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
+    $n.Name -eq 'Update-UserPathForHermes'
+}, $true)
+
+if (-not $fn) {
+    throw "Update-UserPathForHermes not found in $installPs1"
+}
+
+# Swap the two registry calls for the in-memory store. Both must match, or the
+# function has changed shape and this harness is no longer exercising it.
+$definition = $fn.Extent.Text
+$reads = ([regex]'\[Environment\]::GetEnvironmentVariable\("Path", "User"\)').Matches($definition).Count
+$writes = ([regex]'\[Environment\]::SetEnvironmentVariable\("Path", ([^,]+), "User"\)').Matches($definition).Count
+if ($reads -ne 1 -or $writes -ne 1) {
+    throw "expected exactly one User PATH read and one write in the function body; found $reads read(s), $writes write(s). Update this harness."
+}
+
+$definition = $definition -replace `
+    '\[Environment\]::GetEnvironmentVariable\("Path", "User"\)', '$script:FakeUserPath'
+$definition = $definition -replace `
+    '\[Environment\]::SetEnvironmentVariable\("Path", ([^,]+), "User"\)', '$script:FakeUserPath = $1; $script:FakeWrites++'
+
+Invoke-Expression $definition
+
+$BIN = 'C:\Users\me\AppData\Local\hermes\bin'
+$VENV_SCRIPTS = 'C:\Users\me\AppData\Local\hermes\hermes-agent\venv\Scripts'
+$script:Failures = 0
+
+function Invoke-Migration {
+    param([string]$Start, [string]$ShimDir = $BIN, [string]$Legacy = $VENV_SCRIPTS)
+    $script:FakeUserPath = $Start
+    $script:FakeWrites = 0
+    Update-UserPathForHermes -ShimDir $ShimDir -LegacyVenvScripts $Legacy
+}
+
+function Assert-Equal {
+    param($Expected, $Actual, [string]$Name)
+    if ($Expected -ceq $Actual) {
+        Write-Host "  PASS  $Name"
+    } else {
+        Write-Host "  FAIL  $Name"
+        Write-Host "        expected: [$Expected]"
+        Write-Host "        actual:   [$Actual]"
+        $script:Failures++
+    }
+}
+
+Write-Host "install.ps1 Update-UserPathForHermes"
+
+# The regression this function exists for: installs made by the pre-2026-08
+# installer, which prepended the venv Scripts dir (host of python.exe/pip.exe)
+# to the persisted User PATH, hijacking `python` in every new shell.
+Invoke-Migration "$VENV_SCRIPTS;C:\Program Files\nodejs;C:\Users\me\bin"
+Assert-Equal "$BIN;C:\Program Files\nodejs;C:\Users\me\bin" $script:FakeUserPath `
+    'upgrade from venv-Scripts installer: venv\Scripts dropped, shim dir prepended'
+Assert-Equal 0 (@($script:FakeUserPath -split ';' | Where-Object { $_ -eq $VENV_SCRIPTS }).Count) `
+    'upgrade: venv\Scripts fully removed'
+Assert-Equal 1 (@($script:FakeUserPath -split ';' | Where-Object { $_ -eq $BIN }).Count) `
+    'upgrade: shim dir present exactly once'
+Assert-Equal 1 $script:FakeWrites 'upgrade: persists exactly once'
+
+# A shim dir already at the tail (e.g. from an earlier partial install) stays
+# put; only the stale venv\Scripts entry goes away.
+Invoke-Migration "C:\Program Files\nodejs;$VENV_SCRIPTS;$BIN"
+Assert-Equal "C:\Program Files\nodejs;$BIN" $script:FakeUserPath `
+    'existing shim dir keeps its position, venv\Scripts removed'
+Assert-Equal 1 $script:FakeWrites 'existing shim dir: persists exactly once'
+
+Invoke-Migration "$BIN;C:\Program Files\nodejs"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath 'already correct: unchanged'
+Assert-Equal 0 $script:FakeWrites 'already correct: no registry write'
+
+Invoke-Migration "C:\Program Files\nodejs"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath 'fresh install: shim dir prepended'
+
+# Empty segments are legal in a real User PATH (a trailing ';' is common) and
+# the installer's other PATH code preserves them. Migration must not quietly
+# rewrite parts of PATH it was not asked to touch.
+Invoke-Migration "C:\Program Files\nodejs;;C:\Users\me\bin;"
+Assert-Equal "$BIN;C:\Program Files\nodejs;;C:\Users\me\bin;" $script:FakeUserPath `
+    'empty segments are preserved'
+
+# Windows paths are case-insensitive.
+Invoke-Migration "c:\users\me\appdata\local\hermes\hermes-agent\venv\scripts;C:\Program Files\nodejs"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath `
+    'legacy entry in different case is removed, not duplicated'
+
+# A trailing backslash on the registry entry must not defeat the removal.
+Invoke-Migration "$VENV_SCRIPTS\;C:\Program Files\nodejs"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath `
+    'trailing backslash variant is removed'
+
+# Duplicate legacy entries all collapse.
+Invoke-Migration "$VENV_SCRIPTS;C:\Program Files\nodejs;$VENV_SCRIPTS"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath 'duplicates collapse'
+
+# A trailing backslash on the shim dir must not defeat membership detection:
+# the function normalizes it, so no duplicate entry is prepended.
+Invoke-Migration "$BIN;C:\Program Files\nodejs" "$BIN\"
+Assert-Equal "$BIN;C:\Program Files\nodejs" $script:FakeUserPath `
+    'trailing backslash on shim dir: no duplicate entry'
+Assert-Equal 0 $script:FakeWrites 'trailing backslash on shim dir: no write'
+
+Invoke-Migration ""
+Assert-Equal $BIN $script:FakeUserPath 'empty User PATH'
+
+# Empty LegacyVenvScripts (no-venv layout never calls this, but the guard
+# must not filter unrelated entries when it is empty).
+Invoke-Migration "C:\Program Files\nodejs;;" -Legacy ""
+Assert-Equal "$BIN;C:\Program Files\nodejs;;" $script:FakeUserPath `
+    'empty legacy arg: no filtering, empty segments kept'
+
+Invoke-Migration "C:\Program Files\nodejs" "" ""
+Assert-Equal "C:\Program Files\nodejs" $script:FakeUserPath 'empty ShimDir is a no-op'
+Assert-Equal 0 $script:FakeWrites 'empty ShimDir does not write'
+
+# --- New-HermesShims (file-system behavior) --------------------------------
+#
+# Unlike Update-UserPathForHermes this function has no registry calls, so it
+# is tested against real temp dirs.
+
+$fn2 = $ast.Find({
+    param($n)
+    $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
+    $n.Name -eq 'New-HermesShims'
+}, $true)
+
+if (-not $fn2) {
+    throw "New-HermesShims not found in $installPs1"
+}
+Invoke-Expression $fn2.Extent.Text
+
+$shimRoot = Join-Path $env:TEMP ("hermes-shim-test-" + [guid]::NewGuid().ToString('N'))
+$fakeVenv = Join-Path $shimRoot 'venv\Scripts'
+$fakeBin = Join-Path $shimRoot 'bin'
+New-Item -ItemType Directory -Path $fakeVenv -Force | Out-Null
+
+# Seed the venv with the two console scripts the installer ships.
+Set-Content -LiteralPath (Join-Path $fakeVenv 'hermes.exe') -Value 'dummy' -Encoding ASCII
+Set-Content -LiteralPath (Join-Path $fakeVenv 'hermes-acp.exe') -Value 'dummy' -Encoding ASCII
+
+function Assert-Shim {
+    param([string]$Name, [string]$Expected, [string]$TestName)
+    $shim = Join-Path $fakeBin "$Name.cmd"
+    $actual = Get-Content -LiteralPath $shim -Raw
+    if ($actual -ceq $Expected) {
+        Write-Host "  PASS  $TestName"
+    } else {
+        Write-Host "  FAIL  $TestName"
+        Write-Host "        expected: [$Expected]"
+        Write-Host "        actual:   [$actual]"
+        $script:Failures++
+    }
+}
+
+$expectedHermes = "@echo off`r`n@`"$(Join-Path $fakeVenv 'hermes.exe')`" %*`r`n"
+$expectedAcp = "@echo off`r`n@`"$(Join-Path $fakeVenv 'hermes-acp.exe')`" %*`r`n"
+
+New-HermesShims -ShimDir $fakeBin -VenvScripts $fakeVenv
+Assert-Shim 'hermes' $expectedHermes 'shim created with quoted absolute target'
+Assert-Shim 'hermes-acp' $expectedAcp 'acp shim created with quoted absolute target'
+
+# Idempotent: unchanged content must not rewrite (mtime stays put).
+$before = (Get-Item (Join-Path $fakeBin 'hermes.cmd')).LastWriteTimeUtc
+Start-Sleep -Milliseconds 50
+New-HermesShims -ShimDir $fakeBin -VenvScripts $fakeVenv
+$after = (Get-Item (Join-Path $fakeBin 'hermes.cmd')).LastWriteTimeUtc
+if ($before -eq $after) {
+    Write-Host '  PASS  idempotent: no rewrite when content unchanged'
+} else {
+    Write-Host '  FAIL  idempotent: shim rewritten'
+    $script:Failures++
+}
+
+# A missing console script is skipped, not stubbed.
+$sparseVenv = Join-Path $shimRoot 'sparse\Scripts'
+$sparseBin = Join-Path $shimRoot 'sparse-bin'
+New-Item -ItemType Directory -Path $sparseVenv -Force | Out-Null
+Set-Content -LiteralPath (Join-Path $sparseVenv 'hermes.exe') -Value 'dummy' -Encoding ASCII
+New-HermesShims -ShimDir $sparseBin -VenvScripts $sparseVenv
+if ((Test-Path (Join-Path $sparseBin 'hermes.cmd')) -and
+    -not (Test-Path (Join-Path $sparseBin 'hermes-acp.cmd'))) {
+    Write-Host '  PASS  missing console script is skipped, not stubbed'
+} else {
+    Write-Host '  FAIL  missing console script handling'
+    $script:Failures++
+}
+
+# Empty args are a no-op.
+New-HermesShims -ShimDir '' -VenvScripts $fakeVenv
+New-HermesShims -ShimDir $fakeBin -VenvScripts ''
+Write-Host '  PASS  empty args are a no-op'
+
+Remove-Item -Recurse -Force $shimRoot -ErrorAction SilentlyContinue
+
+if ($script:Failures -gt 0) {
+    Write-Host ""
+    Write-Host "$script:Failures assertion(s) failed"
+    exit 1
+}
+
+Write-Host ""
+Write-Host "all assertions passed"