fix(terminal): prefer Scoop MSYS bash over WSL bash on Windows - #35675
fix(terminal): prefer Scoop MSYS bash over WSL bash on Windows#35675sunnirvana wants to merge 1 commit into
Conversation
On Windows hosts with WSL and Scoop Git installed side-by-side,
shutil.which('bash') may return /usr/bin/bash (Linux ELF from WSL)
which cannot be executed by Python running on the Windows side.
Fixes three related issues:
- _find_bash() now checks Scoop-managed Git before shutil.which
- base.py skips redundant cd for Windows cwd when using WSL bash
- file_operations.py bypasses bash cwd for Windows absolute paths
Health-check job fails with: cd: /c/Users/.../hermes: No such file or directory
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Overview
Fixes three related Windows terminal issues: Scoop Git bash priority (avoids picking WSL's Linux ELF bash), skipping redundant builtin cd for Windows cwd in WSL bash sessions, and bypassing bash cwd entirely for Windows absolute paths in write_file.
Looks Good
_find_bash(): Scoop Git check beforeshutil.which— correct prioritybase.py: Detects Windows cwd via[A-Za-z]:[/\\]regex, skips cd that would fail in WSL bashfile_operations.py: Native Python I/O for Windows absolute paths — avoids bash cwd entirely, with full write + lint + LSP pipeline- Good edge case handling throughout
- Well-commented rationale at each change point
Caveats
- PR description template is mostly unfilled (missing issue number, test steps)
- No tests for the new Windows I/O path in file_operations
- However, this is a platform-specific fix on a code path that previously didn't work at all — the approach is sound
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the Scoop + WSL lookup case. The Scoop-specific gap is still present: current tools/environments/local.py:596 reaches shutil.which("bash") without a Scoop Git candidate, so WSL can still win when it appears earlier on PATH.
Problems
- The
base.pyandfile_operations.pyportions are stale against current main. Current code converts native cwd values throughLocalEnvironment._quote_cwd_for_cd(tools/environments/local.py:1022) and rewrites file-operation paths before using the atomic write pipeline (tools/file_operations.py:960,tools/file_operations.py:977). The PR's directopen(..., "w")path should not be transplanted over that pipeline. - The PR has no regression coverage for the Scoop-before-WSL ordering.
Suggested changes
- Salvage the Scoop candidate narrowly before
_find_bash()falls back toshutil.which("bash"), preserving the current path conversion and write mechanisms. - Add a mocked Windows test where Scoop Git exists and
shutil.which("bash")resolves to WSL, asserting Scoop wins.
Automated hermes-sweeper review.
| _scoop_git = os.path.join(_userprofile, "scoop", "apps", "git", "current", "bin", "bash.exe") | ||
| if os.path.isfile(_scoop_git): | ||
| return _scoop_git | ||
|
|
There was a problem hiding this comment.
Please add a regression test for this priority: mock the Scoop bash.exe path as existing while shutil.which("bash") returns the WSL launcher, then assert _find_bash() selects Scoop.
On Windows hosts with WSL and Scoop Git installed side-by-side, shutil.which('bash') may return /usr/bin/bash (Linux ELF from WSL) which cannot be executed by Python running on the Windows side.
Fixes three related issues:
Health-check job fails with: cd: /c/Users/.../hermes: No such file or directory
What does this PR do?
Related Issue
Fixes #
Type of Change
Changes Made
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs