Skip to content

fix(windows): put Git Bash coreutils on PATH for the non-login fallback - #64042

Merged
OutThisLife merged 1 commit into
mainfrom
bb/fix-windows-nonlogin-coreutils-path
Jul 14, 2026
Merged

fix(windows): put Git Bash coreutils on PATH for the non-login fallback#64042
OutThisLife merged 1 commit into
mainfrom
bb/fix-windows-nonlogin-coreutils-path

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

What does this PR do?

Completes #63955. That PR made Hermes survive a broken Git Bash login shell (the classic Windows Directory \drivers\etc does not exist on bash -l) by falling back to a non-login bash -c. But a non-login shell never sources /etc/profile, so it never gets …\Git\usr\bin on PATH — and that directory holds every coreutil the file/terminal tools shell out to (cat, mktemp, mv, wc, head, stat, chmod, mkdir, find, …).

Net effect on an affected box: the fallback stopped Hermes from crashing, but then silently failed every write:

Tool write_file returned error: {"bytes_written": 0, "dirs_created": false,
  "error": "Failed to write file: ", "resolved_path": "C:\\Users\\...\\readme.txt"}

The error string is empty because the failure text went to a missing binary's stderr, and terminal commands returned exit 127 (command not found). So write_file → terminal echo/cat fallback both failed and nothing landed on disk.

Fix

Derive Git Bash's binary dirs (mingw64/bin, usr/bin, bin, …) from the already-resolved bash.exe and prepend them to the subprocess PATH on Windows, in /etc/profile precedence order so coreutils win over the same-named Windows System32 tools (find.exe, sort.exe) inside the shell.

  • No-op off Windows (_git_bash_bin_dirs() returns []).
  • No-op when the login snapshot is healthy — the snapshot re-exports the full login PATH inside the shell, so this only affects the broken-login fallback path.
  • Fixes the whole class (every broken-login-bash Windows user), not one machine.

Adds _git_bash_bin_dirs() (derivation, cached) + _prepend_git_bash_dirs() (PATH merge), wired into _make_run_env.

How to test

scripts/run_tests.sh tests/tools/test_local_env_windows_msys.py — 40 passed. New TestGitBashCoreutilsOnPath covers:

  • bin-dir derivation for PortableGit (<root>\bin\bash.exe) and MinGit (<root>\usr\bin\bash.exe) layouts
  • exclusion of non-existent dirs; [] off Windows / when bash is unresolvable
  • _make_run_env prepends coreutils before System32; idempotent; POSIX no-op

Checklist

  • Conventional Commits
  • Only changes related to this fix
  • Added tests
  • Cross-platform considered — Windows-only behavior, no-op elsewhere; validated on macOS host (derivation returns [], PATH unchanged)

#63955 made Hermes survive a broken `bash -l` (Ainz's `Directory
\drivers\etc does not exist`) by falling back to non-login `bash -c`.
But a non-login shell never sources /etc/profile, so it never gets
`…\usr\bin` on PATH — and that dir holds every coreutil the file/terminal
tools shell out to (cat, mktemp, mv, wc, head, stat, chmod, mkdir, find).
Result: `write_file` returned bytes_written:0 with an EMPTY error (the
failure text went to a missing binary's stderr) and terminal commands
exited 127. The survive-broken-login-bash fix was only half-done: it
stopped crashing but silently failed every write.

Derive Git Bash's bin dirs (mingw64/bin, usr/bin, bin, …) from the
resolved bash.exe and prepend them to the subprocess PATH on Windows, in
/etc/profile precedence order so coreutils win over same-named System32
tools (find.exe, sort.exe) inside the shell. No-op off Windows and when a
login snapshot is healthy (the snapshot re-exports the full PATH inside
the shell), so this only bites on the broken-login fallback path.

Adds _git_bash_bin_dirs() (derivation, cached) + _prepend_git_bash_dirs()
(PATH merge), plus regression tests for PortableGit/MinGit layouts and
the run-env injection ordering.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Comment

30 PRs reviewed (batch: 64044-63999). See aggregate summary at PR 64044.


Reviewed by Hermes Agent

@alt-glitch alt-glitch added type/bug Something isn't working backend/local Local shell execution tool/terminal Terminal execution and process management platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists labels Jul 14, 2026
@OutThisLife
OutThisLife merged commit b663d50 into main Jul 14, 2026
31 checks passed
@OutThisLife
OutThisLife deleted the bb/fix-windows-nonlogin-coreutils-path branch July 14, 2026 00:38
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…s-nonlogin-coreutils-path

fix(windows): put Git Bash coreutils on PATH for the non-login fallback
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…s-nonlogin-coreutils-path

fix(windows): put Git Bash coreutils on PATH for the non-login fallback
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
…s-nonlogin-coreutils-path

fix(windows): put Git Bash coreutils on PATH for the non-login fallback
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…s-nonlogin-coreutils-path

fix(windows): put Git Bash coreutils on PATH for the non-login fallback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/local Local shell execution P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants