mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-12 03:42:08 +00:00
fix(windows): prefer npm.cmd over npm.ps1, skip .py argv0 in relaunch
Two fixes from teknium1's next install run:
1. **npm install: "npm.ps1 cannot be loaded because running scripts is
disabled on this system."** Get-Command's default PATHEXT ordering
picked up ``npm.ps1`` (the PowerShell shim) ahead of ``npm.cmd`` (the
batch shim). Most Windows users have PowerShell's execution policy
set to Restricted or RemoteSigned, which blocks unsigned ``.ps1``
files. ``npm.cmd`` has no such restriction and works universally.
Install-NodeDeps now detects when Get-Command returned npm.ps1, looks
for a sibling npm.cmd in the same directory, and prefers it. Prints
an info line so the user sees why. Emits a warning + hint if only
npm.ps1 is available.
2. **"Launch hermes chat now? Y" crashes with "%1 is not a valid Win32
application" on Windows installs.** The setup wizard calls
``relaunch(["chat"])``; ``resolve_hermes_bin()`` returned
``sys.argv[0]`` which was ``...\\hermes_cli\\main.py`` (because hermes
was launched via ``python -m hermes_cli.main`` during setup).
On Windows, ``os.access(script.py, os.X_OK)`` returns True because
PATHEXT lists ``.py`` when the Python launcher is registered — but
``subprocess.run([script.py, ...])`` can't actually execute a ``.py``
directly. CreateProcessW needs a real PE file.
Fixed ``resolve_hermes_bin`` to reject ``.py``/``.pyc`` argv0 values
on Windows specifically. Falls through to ``shutil.which("hermes")``
(hermes.exe in the venv Scripts dir) or, as a final fallback, lets
build_relaunch_argv build ``[sys.executable, "-m", "hermes_cli.main"]``
which is bulletproof. POSIX behaviour unchanged — ``.py`` argv0 with
a shebang + chmod+x is still a valid exec target there.
3 new tests cover the Windows paths: .py argv0 + hermes.exe on PATH →
returns hermes.exe; .py argv0 + no PATH → returns None (caller uses
python -m); POSIX + executable .py → still accepted.
26 relaunch tests pass, no POSIX regressions.
This commit is contained in:
parent
21efeb51bb
commit
a2efad6bea
3 changed files with 94 additions and 6 deletions
|
|
@ -229,4 +229,58 @@ class TestRelaunch:
|
|||
assert exc_info.value.code == 1
|
||||
err = capsys.readouterr().err
|
||||
assert "relaunch failed" in err
|
||||
assert "open a new terminal" in err.lower() or "path" in err.lower()
|
||||
assert "open a new terminal" in err.lower() or "path" in err.lower()
|
||||
|
||||
|
||||
class TestResolveHermesBinWindowsPyGuard:
|
||||
"""On Windows, resolve_hermes_bin MUST NOT return a .py path.
|
||||
os.access(x, os.X_OK) returns True for .py files on Windows because
|
||||
PATHEXT includes .py when the Python launcher is installed — but
|
||||
subprocess.run can't actually exec a .py directly, so the relaunch
|
||||
would fail with the cryptic "%1 is not a valid Win32 application" error.
|
||||
"""
|
||||
|
||||
def test_windows_rejects_py_argv0_falls_through_to_path(self, monkeypatch, tmp_path):
|
||||
"""On Windows, if sys.argv[0] is a .py file, we must skip the
|
||||
argv[0] fast-path and fall through to PATH / python -m."""
|
||||
# Build a fake .py script that "passes" the isfile + X_OK checks.
|
||||
script = tmp_path / "main.py"
|
||||
script.write_text("# stub")
|
||||
|
||||
monkeypatch.setattr(relaunch_mod.sys, "platform", "win32")
|
||||
monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"])
|
||||
# Force PATH lookup to return a hermes.exe so the test doesn't
|
||||
# exercise the None-fallback path (that's a separate test).
|
||||
monkeypatch.setattr(
|
||||
relaunch_mod.shutil, "which",
|
||||
lambda name: r"C:\venv\Scripts\hermes.exe" if name == "hermes" else None,
|
||||
)
|
||||
|
||||
bin_path = relaunch_mod.resolve_hermes_bin()
|
||||
# Must NOT be the .py — must be the hermes.exe PATH entry.
|
||||
assert bin_path == r"C:\venv\Scripts\hermes.exe"
|
||||
|
||||
def test_posix_still_accepts_py_argv0(self, monkeypatch, tmp_path):
|
||||
"""POSIX behaviour unchanged: argv[0] pointing at an executable
|
||||
script (including .py with a shebang + chmod +x) is fine to return
|
||||
because POSIX exec can route through the shebang line."""
|
||||
if sys.platform == "win32":
|
||||
pytest.skip("POSIX semantics")
|
||||
script = tmp_path / "hermes"
|
||||
script.write_text("#!/usr/bin/env python3\n")
|
||||
script.chmod(0o755)
|
||||
monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"])
|
||||
assert relaunch_mod.resolve_hermes_bin() == str(script)
|
||||
|
||||
def test_windows_py_argv0_with_no_hermes_on_path_returns_none(self, monkeypatch, tmp_path):
|
||||
"""Bulletproof fallback: if argv0 is .py on Windows AND hermes.exe
|
||||
isn't on PATH, return None so the caller falls back to
|
||||
python -m hermes_cli.main."""
|
||||
script = tmp_path / "main.py"
|
||||
script.write_text("# stub")
|
||||
|
||||
monkeypatch.setattr(relaunch_mod.sys, "platform", "win32")
|
||||
monkeypatch.setattr(relaunch_mod.sys, "argv", [str(script), "chat"])
|
||||
monkeypatch.setattr(relaunch_mod.shutil, "which", lambda name: None)
|
||||
|
||||
assert relaunch_mod.resolve_hermes_bin() is None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue