Skip to content

fix: prevent _update_cwd from overwriting self.cwd with POSIX paths on Windows#17342

Closed
hypen-o wants to merge 1 commit into
NousResearch:mainfrom
hypen-o:fix/windows-update-cwd
Closed

fix: prevent _update_cwd from overwriting self.cwd with POSIX paths on Windows#17342
hypen-o wants to merge 1 commit into
NousResearch:mainfrom
hypen-o:fix/windows-update-cwd

Conversation

@hypen-o

@hypen-o hypen-o commented Apr 29, 2026

Copy link
Copy Markdown

What

Prevent _update_cwd() from overwriting self.cwd with Git Bash POSIX paths on Windows.

 tools/environments/local.py | 9 +++++++++
 1 file changed, 9 insertions(+)

Why

On Windows, _update_cwd() reads a temp file written by Git Bash's pwd -P command.
Git Bash always returns POSIX-style paths (/d/users/hermes), which are not valid for subprocess.Popen(cwd=...):

Before this fix:
  Terminal command 1 → works (uses correct initial cwd)
  _update_cwd() → sets self.cwd = "/d/users/hermes" (POSIX)
  Terminal command 2 → WinError 267 (invalid cwd)

After this fix:
  Terminal command 1 → works
  _update_cwd() → strips marker from stdout, preserves original cwd
  Terminal command 2 → works

Root cause chain

  1. LocalEnvironment.init_session() sets self.cwd correctly (Windows path from config or detection)
  2. After each command, _update_cwd() reads Git Bash's pwd -P output → POSIX path (/d/...)
  3. POSIX path stored into self.cwd → next subprocess.Popen(cwd=...) fails

Fix

On Windows, _update_cwd() still calls _extract_cwd_from_output() to strip the CWD marker from stdout (keeping output clean), but restores the original self.cwd instead of using the POSIX path from the temp file.

The _IS_WINDOWS constant already exists at local.py:12 for other platform guards.

Verification

  • Windows 10/11 with Git Bash in non-standard path (e.g. D:\...\Git\bin\bash.exe)
  • TERMINAL_CWD set in .env
  • Multiple consecutive terminal() calls succeed (previously the second call failed)

Alternatives considered

Approach Decision
Convert POSIX → Windows path in _update_cwd() Rejected — fragile regex/mount-point mapping, breaks with MSYS2 mount changes
Skip _update_cwd() entirely on Windows Chosen — simpler, correct, self.cwd is already valid

@hypen-o
fix: prevent _update_cwd from overwriting self.cwd with POSIX paths o…
55c1036 
…n Windows

On Windows with Git Bash, _update_cwd() reads the output of 'pwd -P'
which returns POSIX-style paths (e.g. /c/Users/msn). These paths are
invalid for subprocess.Popen(cwd=...), causing WinError 267.

This fix adds a Windows guard: on Windows, _update_cwd() still strips
the CWD marker from stdout (for clean output) but preserves the
original self.cwd (which was set by the TERMINAL_CWD config or
the initial Windows path detection).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists backend/local Local shell execution comp/tools Tool registry, model_tools, toolsets labels Apr 29, 2026
@alt-glitch

alt-glitch commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #16628 — same fix for _update_cwd() overwriting self.cwd with Git Bash POSIX path on Windows. Also related to #14644.

@alt-glitch

alt-glitch commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #16628

@briandevans briandevans mentioned this pull request Apr 29, 2026
Closed
3 tasks
@teknium1

teknium1 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: this Windows Git Bash cwd issue is already fixed on current main.

Evidence:

  • The prior discussion here correctly identified this as a likely duplicate of fix(windows): prevent _update_cwd() from overwriting self.cwd with Git Bash POSIX path #16628, covering _update_cwd() overwriting self.cwd with Git Bash POSIX/MSYS paths.
  • Main now has tools/environments/local.py support for _msys_to_windows_path() and uses it from LocalEnvironment._update_cwd() before assigning self.cwd, so /c/... / /d/... marker values are normalized to native Windows paths before they can reach subprocess.Popen(cwd=...).
  • LocalEnvironment._extract_cwd_from_output() still strips the marker from stdout, then validates the normalized cwd and rolls back to the previous cwd if the marker points at a stale/non-existent path.
  • Regression coverage exists in tests/tools/test_local_env_windows_msys.py for marker-file MSYS paths being stored in native form and stale MSYS markers not clobbering cwd.
  • The implementation landed in 4aec25bc4411edb4563292cadbd02c365c846286 and is contained in v2026.5.16.

@teknium1 teknium1 closed this Jun 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

backend/local Local shell execution comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hypen-o @alt-glitch @teknium1