mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-13 03:52:00 +00:00
fix(windows): %1 install error, patch CRLF false-negative, SOUL.md BOM
Three bugs from teknium1's successful install + diagnostic chat on Windows:
1. **Start-Process -FilePath npm.cmd fails with "%1 is not a valid Win32
application".** Start-Process bypasses cmd.exe and PATHEXT to call
CreateProcessW directly, which refuses .cmd batch shims. Switched
Install-NodeDeps to use PowerShell's invocation operator (``& $npmExe
install --silent *> $log``) which DOES honour PATHEXT. Extracted a
``_Run-NpmInstall`` helper so the browser + TUI paths share the same
logic. Captures $LASTEXITCODE correctly, still surfaces the real
stderr on failure with a log-file pointer for the full output.
2. **patch tool returns false-negative on Windows due to CRLF round-trip.**
Root cause was upstream of patch: ``subprocess.Popen(..., text=True,
stdin=PIPE)`` on Windows translates ``\\n`` → ``\\r\\n`` when data flows
through the stdin pipe. ``_pipe_stdin()`` was writing the patch's
new_content string through a text-mode pipe, bash then wrote those
CRLF bytes to disk, and patch's post-write verify compared the
on-disk CRLF bytes against the original LF-only string — fail.
Fixed in two places for defense in depth:
- ``_pipe_stdin()`` now writes through ``proc.stdin.buffer`` with
explicit UTF-8 encoding, bypassing Python's newline translation on
every platform. No behaviour change on POSIX (bytes are identical)
but stops the CRLF injection on Windows.
- ``patch_replace``'s post-write verify normalizes CRLF→LF on both
sides before comparing, so even if some future backend still
translates newlines the patch tool won't report a bogus failure.
3. **SOUL.md gets a UTF-8 BOM on Windows PowerShell 5.1.** ``Set-Content
-Encoding UTF8`` on PS5.1 writes UTF-8 WITH a byte-order-mark (changed
in PS7 via ``utf8NoBOM``). Hermes's prompt-injection scanner sees
the BOM (U+FEFF invisible char) and refuses to load the file, so
SOUL.md's persona instructions never get applied.
Fixed by writing the file via ``[System.IO.File]::WriteAllText``
with an explicit ``UTF8Encoding($false)`` — BOM-free on every
PowerShell version.
All POSIX behaviour verified unchanged: 198 tests pass across
test_file_operations, test_local_env_cwd_recovery, test_code_execution,
test_windows_native_support, test_windows_compat.
This commit is contained in:
parent
d52e54170a
commit
8f91d7bfa9
3 changed files with 97 additions and 75 deletions
|
|
@ -903,10 +903,18 @@ function Copy-ConfigTemplates {
|
||||||
Write-Info "~/.hermes/config.yaml already exists, keeping it"
|
Write-Info "~/.hermes/config.yaml already exists, keeping it"
|
||||||
}
|
}
|
||||||
|
|
||||||
# Create SOUL.md if it doesn't exist (global persona file)
|
# Create SOUL.md if it doesn't exist (global persona file).
|
||||||
|
# IMPORTANT: write without a BOM. Windows PowerShell 5.1's
|
||||||
|
# ``Set-Content -Encoding UTF8`` writes UTF-8 WITH a byte-order-mark
|
||||||
|
# (the default PS5 behaviour), and Hermes's prompt-injection scanner
|
||||||
|
# flags the BOM as an invisible unicode character and refuses to
|
||||||
|
# load the file. PS7's ``-Encoding utf8NoBOM`` fixes that but we
|
||||||
|
# don't control which PowerShell version the user has. Go direct
|
||||||
|
# to .NET with an explicit UTF8Encoding($false) — BOM-free on every
|
||||||
|
# PowerShell version.
|
||||||
$soulPath = "$HermesHome\SOUL.md"
|
$soulPath = "$HermesHome\SOUL.md"
|
||||||
if (-not (Test-Path $soulPath)) {
|
if (-not (Test-Path $soulPath)) {
|
||||||
@"
|
$soulContent = @"
|
||||||
# Hermes Agent Persona
|
# Hermes Agent Persona
|
||||||
|
|
||||||
<!--
|
<!--
|
||||||
|
|
@ -922,7 +930,9 @@ Examples:
|
||||||
This file is loaded fresh each message -- no restart needed.
|
This file is loaded fresh each message -- no restart needed.
|
||||||
Delete the contents (or this file) to use the default personality.
|
Delete the contents (or this file) to use the default personality.
|
||||||
-->
|
-->
|
||||||
"@ | Set-Content -Path $soulPath -Encoding UTF8
|
"@
|
||||||
|
$utf8NoBom = New-Object System.Text.UTF8Encoding($false)
|
||||||
|
[System.IO.File]::WriteAllText($soulPath, $soulContent, $utf8NoBom)
|
||||||
Write-Success "Created ~/.hermes/SOUL.md (edit to customize personality)"
|
Write-Success "Created ~/.hermes/SOUL.md (edit to customize personality)"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -964,83 +974,64 @@ function Install-NodeDeps {
|
||||||
}
|
}
|
||||||
$npmExe = $npmCmd.Source
|
$npmExe = $npmCmd.Source
|
||||||
|
|
||||||
Push-Location $InstallDir
|
# Helper: run "npm install" in a given directory and surface the real
|
||||||
|
# error when it fails. Returns $true on success.
|
||||||
if (Test-Path "package.json") {
|
#
|
||||||
Write-Info "Installing Node.js dependencies (browser tools)..."
|
# Implementation note: ``Start-Process -FilePath npm.cmd`` fails with
|
||||||
# Use Start-Process so we can capture the real exit code. PowerShell's
|
# ``%1 is not a valid Win32 application`` on some PowerShell versions
|
||||||
# try/catch doesn't trigger on npm's non-zero exit — only on an unhandled
|
# because Start-Process bypasses cmd.exe / PATHEXT and expects a real
|
||||||
# .NET exception (e.g. npm.cmd missing). Capturing stderr to a file
|
# PE file. The invocation-operator ``& $npmExe`` routes through the
|
||||||
# lets us surface the actual failure reason instead of a generic
|
# PowerShell command pipeline which DOES honour .cmd batch shims, so
|
||||||
# "npm install failed" that hides what went wrong.
|
# it works uniformly for npm.cmd, npx.cmd, and bare .exe files.
|
||||||
$browserLog = "$env:TEMP\hermes-npm-browser-$(Get-Random).log"
|
function _Run-NpmInstall([string]$label, [string]$installDir, [string]$logPath, [string]$npmPath) {
|
||||||
|
Push-Location $installDir
|
||||||
try {
|
try {
|
||||||
$proc = Start-Process -FilePath $npmExe `
|
# Redirect ALL output streams to the log file via 2>&1 and then
|
||||||
-ArgumentList "install", "--silent" `
|
# ``Tee-Object`` / ``Out-File``. Simpler approach: call npm
|
||||||
-NoNewWindow -Wait -PassThru `
|
# with output redirected and inspect $LASTEXITCODE afterwards.
|
||||||
-RedirectStandardOutput "NUL" `
|
& $npmPath install --silent *> $logPath
|
||||||
-RedirectStandardError $browserLog
|
$code = $LASTEXITCODE
|
||||||
if ($proc.ExitCode -eq 0) {
|
if ($code -eq 0) {
|
||||||
Write-Success "Node.js dependencies installed"
|
Write-Success "$label dependencies installed"
|
||||||
Remove-Item -Force $browserLog -ErrorAction SilentlyContinue
|
Remove-Item -Force $logPath -ErrorAction SilentlyContinue
|
||||||
} else {
|
return $true
|
||||||
Write-Warn "npm install failed (browser tools may not work) — exit code $($proc.ExitCode)"
|
|
||||||
if (Test-Path $browserLog) {
|
|
||||||
$errText = (Get-Content $browserLog -Raw -ErrorAction SilentlyContinue)
|
|
||||||
if ($errText) {
|
|
||||||
# Show first ~800 chars — enough to see the real cause.
|
|
||||||
$snippet = if ($errText.Length -gt 800) { $errText.Substring(0, 800) + "..." } else { $errText }
|
|
||||||
Write-Info " npm stderr:"
|
|
||||||
foreach ($line in $snippet -split "`n") {
|
|
||||||
Write-Host " $line" -ForegroundColor DarkGray
|
|
||||||
}
|
|
||||||
Write-Info " Full log: $browserLog"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Write-Info "Run manually later: cd `"$InstallDir`"; npm install"
|
|
||||||
}
|
}
|
||||||
|
Write-Warn "$label npm install failed — exit code $code"
|
||||||
|
if (Test-Path $logPath) {
|
||||||
|
$errText = (Get-Content $logPath -Raw -ErrorAction SilentlyContinue)
|
||||||
|
if ($errText) {
|
||||||
|
$snippet = if ($errText.Length -gt 1200) { $errText.Substring(0, 1200) + "..." } else { $errText }
|
||||||
|
Write-Info " npm output:"
|
||||||
|
foreach ($line in $snippet -split "`n") {
|
||||||
|
Write-Host " $line" -ForegroundColor DarkGray
|
||||||
|
}
|
||||||
|
Write-Info " Full log: $logPath"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Write-Info "Run manually later: cd `"$installDir`"; npm install"
|
||||||
|
return $false
|
||||||
} catch {
|
} catch {
|
||||||
Write-Warn "npm install could not be launched: $_"
|
Write-Warn "$label npm install could not be launched: $_"
|
||||||
|
return $false
|
||||||
|
} finally {
|
||||||
|
Pop-Location
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
# Install TUI dependencies
|
# Browser tools
|
||||||
|
if (Test-Path "$InstallDir\package.json") {
|
||||||
|
Write-Info "Installing Node.js dependencies (browser tools)..."
|
||||||
|
$browserLog = "$env:TEMP\hermes-npm-browser-$(Get-Random).log"
|
||||||
|
[void](_Run-NpmInstall "Browser tools" $InstallDir $browserLog $npmExe)
|
||||||
|
}
|
||||||
|
|
||||||
|
# TUI
|
||||||
$tuiDir = "$InstallDir\ui-tui"
|
$tuiDir = "$InstallDir\ui-tui"
|
||||||
if (Test-Path "$tuiDir\package.json") {
|
if (Test-Path "$tuiDir\package.json") {
|
||||||
Write-Info "Installing TUI dependencies..."
|
Write-Info "Installing TUI dependencies..."
|
||||||
Push-Location $tuiDir
|
|
||||||
$tuiLog = "$env:TEMP\hermes-npm-tui-$(Get-Random).log"
|
$tuiLog = "$env:TEMP\hermes-npm-tui-$(Get-Random).log"
|
||||||
try {
|
[void](_Run-NpmInstall "TUI" $tuiDir $tuiLog $npmExe)
|
||||||
$proc = Start-Process -FilePath $npmExe `
|
|
||||||
-ArgumentList "install", "--silent" `
|
|
||||||
-NoNewWindow -Wait -PassThru `
|
|
||||||
-RedirectStandardOutput "NUL" `
|
|
||||||
-RedirectStandardError $tuiLog
|
|
||||||
if ($proc.ExitCode -eq 0) {
|
|
||||||
Write-Success "TUI dependencies installed"
|
|
||||||
Remove-Item -Force $tuiLog -ErrorAction SilentlyContinue
|
|
||||||
} else {
|
|
||||||
Write-Warn "TUI npm install failed (hermes --tui may not work) — exit code $($proc.ExitCode)"
|
|
||||||
if (Test-Path $tuiLog) {
|
|
||||||
$errText = (Get-Content $tuiLog -Raw -ErrorAction SilentlyContinue)
|
|
||||||
if ($errText) {
|
|
||||||
$snippet = if ($errText.Length -gt 800) { $errText.Substring(0, 800) + "..." } else { $errText }
|
|
||||||
Write-Info " npm stderr:"
|
|
||||||
foreach ($line in $snippet -split "`n") {
|
|
||||||
Write-Host " $line" -ForegroundColor DarkGray
|
|
||||||
}
|
|
||||||
Write-Info " Full log: $tuiLog"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Write-Info "Run manually later: cd `"$tuiDir`"; npm install"
|
|
||||||
}
|
|
||||||
} catch {
|
|
||||||
Write-Warn "TUI npm install could not be launched: $_"
|
|
||||||
}
|
|
||||||
Pop-Location
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Pop-Location
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function Invoke-SetupWizard {
|
function Invoke-SetupWizard {
|
||||||
|
|
|
||||||
|
|
@ -99,12 +99,33 @@ def get_sandbox_dir() -> Path:
|
||||||
|
|
||||||
|
|
||||||
def _pipe_stdin(proc: subprocess.Popen, data: str) -> None:
|
def _pipe_stdin(proc: subprocess.Popen, data: str) -> None:
|
||||||
"""Write *data* to proc.stdin on a daemon thread to avoid pipe-buffer deadlocks."""
|
"""Write *data* to proc.stdin on a daemon thread to avoid pipe-buffer deadlocks.
|
||||||
|
|
||||||
|
On Windows, text-mode stdin (``text=True`` / ``encoding="utf-8"``)
|
||||||
|
translates ``\\n`` → ``\\r\\n`` as the data flows through the pipe —
|
||||||
|
which corrupts every write_file / patch call because the bytes that
|
||||||
|
land on disk include injected carriage returns. The file IS created,
|
||||||
|
but every subsequent byte-count / content compare against the
|
||||||
|
caller's ``\\n``-only string fails.
|
||||||
|
|
||||||
|
Workaround: write through ``proc.stdin.buffer`` (the underlying byte
|
||||||
|
buffer), encoding to UTF-8 ourselves. That bypasses Python's
|
||||||
|
newline translation entirely on every platform. No behaviour change
|
||||||
|
on POSIX — the byte sequence is identical to what text-mode would
|
||||||
|
produce there.
|
||||||
|
"""
|
||||||
|
|
||||||
def _write():
|
def _write():
|
||||||
try:
|
try:
|
||||||
proc.stdin.write(data)
|
# proc.stdin is a TextIOWrapper when text=True was set on the
|
||||||
proc.stdin.close()
|
# Popen. Its ``.buffer`` attribute is the raw BufferedWriter
|
||||||
|
# that bypasses newline translation. When Popen was created
|
||||||
|
# in byte mode, proc.stdin is already a BufferedWriter with
|
||||||
|
# no ``.buffer`` attribute — fall back to .write() directly.
|
||||||
|
raw = data.encode("utf-8") if isinstance(data, str) else data
|
||||||
|
target = getattr(proc.stdin, "buffer", proc.stdin)
|
||||||
|
target.write(raw)
|
||||||
|
target.close()
|
||||||
except (BrokenPipeError, OSError):
|
except (BrokenPipeError, OSError):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -966,11 +966,21 @@ class ShellFileOperations(FileOperations):
|
||||||
verify_result = self._exec(verify_cmd)
|
verify_result = self._exec(verify_cmd)
|
||||||
if verify_result.exit_code != 0:
|
if verify_result.exit_code != 0:
|
||||||
return PatchResult(error=f"Post-write verification failed: could not re-read {path}")
|
return PatchResult(error=f"Post-write verification failed: could not re-read {path}")
|
||||||
if verify_result.stdout != new_content:
|
# Normalize line endings before comparing. On Windows, Python's
|
||||||
|
# default text-mode ``open()`` translates ``\n`` → ``\r\n`` on
|
||||||
|
# write, so the file on disk legitimately holds CRLFs while our
|
||||||
|
# ``new_content`` string has bare LFs. Without this normalization
|
||||||
|
# every patch on Windows returns a bogus "wrote 39, read 42"
|
||||||
|
# false-negative even though the edit landed correctly. POSIX
|
||||||
|
# backends don't translate, so this is a no-op there.
|
||||||
|
_verify_stdout_normalized = verify_result.stdout.replace("\r\n", "\n").replace("\r", "\n")
|
||||||
|
_new_content_normalized = new_content.replace("\r\n", "\n").replace("\r", "\n")
|
||||||
|
if _verify_stdout_normalized != _new_content_normalized:
|
||||||
return PatchResult(error=(
|
return PatchResult(error=(
|
||||||
f"Post-write verification failed for {path}: on-disk content "
|
f"Post-write verification failed for {path}: on-disk content "
|
||||||
f"differs from intended write "
|
f"differs from intended write "
|
||||||
f"(wrote {len(new_content)} chars, read back {len(verify_result.stdout)}). "
|
f"(wrote {len(_new_content_normalized)} chars, read back "
|
||||||
|
f"{len(_verify_stdout_normalized)} chars after normalizing line endings). "
|
||||||
"The patch did not persist. Re-read the file and try again."
|
"The patch did not persist. Re-read the file and try again."
|
||||||
))
|
))
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue