Skip to content

fix(tools): convert Windows native CWD to MSYS format for bash cd - #50596

Closed
Icather wants to merge 2 commits into
NousResearch:mainfrom
Icather:fix/windows-cwd-msys-path-for-bash-cd
Closed

fix(tools): convert Windows native CWD to MSYS format for bash cd#50596
Icather wants to merge 2 commits into
NousResearch:mainfrom
Icather:fix/windows-cwd-msys-path-for-bash-cd

Conversation

@Icather

@Icather Icather commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When Hermes is started from PowerShell or cmd.exe on Windows, os.getcwd() returns a Windows native path (C:\Users\...) which gets stored as the session working directory. Both init_session() and _wrap_command() embed this path directly into bash scripts via cd, but Git Bash cannot parse Windows drive-letter paths.

Impact: Every terminal tool call fails with exit code 126 the first time any command runs. The agent cannot write files, search, or execute shell commands. It wastes tokens diagnosing the failure, may produce incorrect results, and can fail entirely on tasks requiring shell execution. This affects ALL Windows users launching Hermes from non-Msys terminals (PowerShell, cmd.exe).

The existing _msys_to_windows_path() in local.py handles reverse conversion (bash output -> Python Popen cwd). No forward helper existed.

This PR adds _windows_to_msys_path() to BaseEnvironment and applies it in both init_session() and _wrap_command().

Related Issue

Fixes #50594

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/environments/base.py — Added _windows_to_msys_path() static method: C:\Users\x -> /c/Users/x. No-op on non-Windows or already-Msys paths
  • tools/environments/base.pyinit_session(): convert self.cwd via _windows_to_msys_path() before shlex.quote()
  • tools/environments/base.py_wrap_command(): convert cwd via _windows_to_msys_path() before _quote_cwd_for_cd()
  • Added import re and import sys to module imports

How to Test

  1. Open PowerShell (not Git Bash)
  2. cd C:\Users\YourName
  3. hermes
  4. Ask agent: list files in current directory
  5. Command should succeed instead of failing with exit code 126

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix/feature
  • pytest 55 passed (1 pre-existing failure on Windows — POSIX root test)
  • I have added tests for my changes
  • I have tested on my platform: Windows 10, PowerShell

Documentation & Housekeeping

  • N/A — no docs or config keys affected
  • Cross-platform: _windows_to_msys_path() is a no-op on non-Windows

When hermes is started from PowerShell or cmd.exe, os.getcwd()
returns a Windows native path (C:\Users\...) which gets stored as
the session working directory.  Both init_session() and
_wrap_command() embed this path into bash scripts via cd, but
Git Bash cannot parse Windows drive-letter paths.

Add _windows_to_msys_path() to BaseEnvironment:
- C:\Users\x -> /c/Users/x (Git Bash compatible)
- No-op on non-Windows or already-Msys paths

Applied in init_session() and _wrap_command().
Copilot AI review requested due to automatic review settings June 22, 2026 04:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes Windows terminal execution when Hermes is launched from PowerShell/cmd.exe by converting session CWDs from Windows-native drive-letter paths (e.g., C:\Users\...) into Git Bash/MSYS paths (e.g., /c/Users/...) before embedding them into bash cd commands.

Changes:

  • Added BaseEnvironment._windows_to_msys_path() to translate Windows-native paths to MSYS form (no-op off-Windows / already-MSYS).
  • Applied the conversion in BaseEnvironment.init_session() and _wrap_command() so bootstrap and per-command scripts cd reliably in Git Bash.
  • Added re and sys imports to support the conversion helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tools/environments/base.py
Comment thread tools/environments/base.py
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/terminal Terminal execution and process management P1 High — major feature broken, no workaround labels Jun 22, 2026
…indows_to_msys_path

Copilot review follow-up: init_session bootstrap used builtin cd without double-dash; a cwd starting with dash could be parsed as an option. _wrap_command already guards. Added 6 unit tests for _windows_to_msys_path().
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the Windows terminal fix. The premise checks out on current main: BaseEnvironment.init_session() still passes self.cwd directly into bash at tools/environments/base.py:364 / :402, and _wrap_command() still quotes the raw cwd at tools/environments/base.py:475-477; the existing _msys_to_windows_path() in tools/environments/local.py:23-40 only handles the reverse Git-Bash-output → native-Popen direction.

Problems

  • The branch is stale in the same base.py regions. Current main added atomic snapshot replacement using _snap_tmp / $BASHPID at tools/environments/base.py:391-401 and :451-458; keep that race hardening when applying this conversion.
  • The tests cover the helper itself, but not the two call sites. That leaves the regression risk that _windows_to_msys_path() exists but the generated bash still uses the native C:\... path.

Suggested changes

  • Add call-site assertions for init_session() and _wrap_command() with a Windows cwd like C:\Users\NVIDIA, expecting /c/Users/NVIDIA in the emitted builtin cd -- ... lines.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 29, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Superseded by PR #56384, which fixes this at the root: it reorders _find_bash() so Git for Windows always beats the WSL shim on PATH (one canonical MSYS bash), then converts the native cwd C:\Users\x/c/Users/x at both cd sites. This makes the /mnt/c vs /c dialect mismatch impossible rather than translating around it. Thanks for the fix — the cleanest implementation of the same conversion was salvaged with authorship preserved.
#56384

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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.

Windows: session cwd in native format breaks terminal tool

4 participants