mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-10 03:22:05 +00:00
fix(windows): use PortableGit (not MinGit), fix relaunch os.execvp crash, surface npm errors
Three real bugs from teknium1's first Windows install run:
1. **MinGit has no bash.exe.** MinGit is the minimal-automation Git for Windows
distribution — it ships git.exe but deliberately strips bash and the POSIX
coreutils. Installer logged "Could not locate bash.exe" and Hermes would
fail to run any shell command. Switched to PortableGit — the full Git for
Windows minus the installer UI. PortableGit ships bash.exe at
<root>\bin\bash.exe plus sh, awk, sed, grep, curl, ssh in usr\bin\. ARM64
variant is detected separately (PortableGit-*-arm64.7z.exe). 32-bit falls
back to MinGit-32-bit with a warning (PortableGit is 64-bit only).
PortableGit ships as a 7z self-extractor (56MB vs MinGit's 38MB). We
invoke it with `-o<target> -y` to extract silently — no 7z install needed,
it's self-contained.
Updated tools/environments/local.py::_find_bash candidate order to prefer
the PortableGit layout (<root>\bin\bash.exe) with the MinGit layout
(<root>\usr\bin\bash.exe) as a fallback so existing installs keep working.
2. **os.execvp "Exec format error" on Windows.** Setup wizard's "Launch
hermes chat now? Y" called `os.execvp(["hermes", "chat"])` which on
Windows can only swap to real Win32 .exe files — chokes with OSError(8)
on .cmd batch shims and Python console-script wrappers. Added a
win32 branch in hermes_cli/relaunch.py::relaunch() that uses
subprocess.run + sys.exit — functionally identical (user sees "hermes
exited, then new hermes started") with one extra PID in play. POSIX
path is UNCHANGED — still uses os.execvp for in-place replacement.
Catches OSError in the Windows branch and surfaces a "open a new
terminal so PATH picks up, then re-run hermes" hint instead of a
cryptic traceback.
3. **npm install failures silent on Windows.** The install.ps1 was invoking
`npm install --silent 2>&1 | Out-Null` inside a try/catch. PowerShell's
try/catch does NOT trigger on non-zero process exit codes — only on
unhandled .NET exceptions — so npm failing printed a generic "npm
install failed" with zero information about WHY. The silent pipe ate
the stderr.
Rewrote Install-NodeDeps to:
- Resolve npm.cmd via Get-Command (respects PATHEXT) instead of
relying on bare `npm` name resolution.
- Use Start-Process with -PassThru to capture the actual exit code.
- Redirect stderr to a temp log and surface the first ~800 chars of
the real npm error when install fails, plus the log path for the
full text.
- Fail loudly with the right exit code instead of a misleading success.
- Bail cleanly with a helpful message when npm isn't on PATH at all.
4. **"True" printing to console after Node check.** `Test-Node` returns $true;
installer called it as a bare statement (no assignment, no cast). PowerShell
prints bare return values. Wrapped the call in `[void](Test-Node)`.
## Tests
- Added 3 new tests in tests/hermes_cli/test_relaunch.py covering the
Windows branch: subprocess is called (not execvp), child exit code
propagates, OSError surfaces a helpful message. All 23 tests pass
(20 existing + 3 new).
- 77 Windows-compat tests still pass, POSIX behaviour unchanged.
This commit is contained in:
parent
eeb723fff2
commit
cf9b2df57a
4 changed files with 289 additions and 58 deletions
|
|
@ -152,4 +152,81 @@ class TestRelaunch:
|
|||
with pytest.raises(SystemExit):
|
||||
relaunch_mod.relaunch(["--resume", "abc"])
|
||||
|
||||
assert calls == [("/usr/bin/hermes", ["/usr/bin/hermes", "--resume", "abc"])]
|
||||
assert calls == [("/usr/bin/hermes", ["/usr/bin/hermes", "--resume", "abc"])]
|
||||
|
||||
def test_windows_uses_subprocess_not_execvp(self, monkeypatch):
|
||||
"""On Windows, os.execvp raises OSError "Exec format error" when the
|
||||
target is a .cmd shim or console-script wrapper (both common for
|
||||
hermes). relaunch() must detect win32 and use subprocess.run +
|
||||
sys.exit instead."""
|
||||
monkeypatch.setattr(relaunch_mod.sys, "platform", "win32")
|
||||
monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: r"C:\Users\test\hermes.exe")
|
||||
|
||||
import subprocess as _subprocess
|
||||
|
||||
captured_argv = []
|
||||
|
||||
def fake_subprocess_run(argv, **kwargs):
|
||||
captured_argv.append(list(argv))
|
||||
class _Result:
|
||||
returncode = 0
|
||||
return _Result()
|
||||
|
||||
monkeypatch.setattr(_subprocess, "run", fake_subprocess_run)
|
||||
|
||||
# execvp MUST NOT be called on Windows — route must go through subprocess
|
||||
execvp_calls = []
|
||||
|
||||
def fake_execvp(*args, **kwargs):
|
||||
execvp_calls.append(args)
|
||||
raise AssertionError("os.execvp must not be called on Windows")
|
||||
|
||||
monkeypatch.setattr(relaunch_mod.os, "execvp", fake_execvp)
|
||||
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
relaunch_mod.relaunch(["chat"])
|
||||
|
||||
assert exc_info.value.code == 0
|
||||
assert execvp_calls == []
|
||||
assert captured_argv == [[r"C:\Users\test\hermes.exe", "chat"]]
|
||||
|
||||
def test_windows_propagates_child_exit_code(self, monkeypatch):
|
||||
"""A non-zero exit from the child should flow through to sys.exit."""
|
||||
monkeypatch.setattr(relaunch_mod.sys, "platform", "win32")
|
||||
monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: r"C:\hermes.exe")
|
||||
|
||||
import subprocess as _subprocess
|
||||
|
||||
def fake_run(argv, **kwargs):
|
||||
class _Result:
|
||||
returncode = 42
|
||||
return _Result()
|
||||
|
||||
monkeypatch.setattr(_subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(relaunch_mod.os, "execvp", lambda *a, **kw: None)
|
||||
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
relaunch_mod.relaunch(["chat"])
|
||||
assert exc_info.value.code == 42
|
||||
|
||||
def test_windows_surfaces_oserror_with_help(self, monkeypatch, capsys):
|
||||
"""When subprocess itself raises OSError (file-not-found / bad format),
|
||||
we must NOT let it bubble up as a cryptic traceback — print a
|
||||
user-readable hint and sys.exit(1)."""
|
||||
monkeypatch.setattr(relaunch_mod.sys, "platform", "win32")
|
||||
monkeypatch.setattr(relaunch_mod, "resolve_hermes_bin", lambda: r"C:\missing.exe")
|
||||
|
||||
import subprocess as _subprocess
|
||||
|
||||
def fake_run(argv, **kwargs):
|
||||
raise OSError(2, "No such file or directory")
|
||||
|
||||
monkeypatch.setattr(_subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(relaunch_mod.os, "execvp", lambda *a, **kw: None)
|
||||
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
relaunch_mod.relaunch(["chat"])
|
||||
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()
|
||||
Loading…
Add table
Add a link
Reference in a new issue