fix(shell): resolve pwsh.exe/.cmd/.bat on PATH for Windows shell discovery#111500
fix(shell): resolve pwsh.exe/.cmd/.bat on PATH for Windows shell discovery#111500labsclaw wants to merge 1 commit into
Conversation
…overy
On Windows, PowerShell 7 installed via the Microsoft Store / App Execution
Alias ships as pwsh.exe. resolvePowerShellPath() step 3 called
resolveShellFromPath("pwsh"), which only probed a bare "pwsh" with no
extension. fs.accessSync therefore missed the real binary and discovery
fell through to Windows PowerShell 5.1.
Changes:
- Export resolveShellFromPath for external use
- Add .exe probing on Windows (win32 only) for pwsh discovery
- Add tests for resolveWindowsBashPath, resolveShellFromPath,
resolveShellFromWhich, resolvePowerShellPath
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close this PR: GitHub reports no changed files against the current base branch. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. So I’m closing this here because the remaining work is already tracked in the canonical issue. Review detailsDo we have a high-confidence way to reproduce the issue? Yes, at source level: on Windows, place only Is this the best way to solve the issue? Yes. Probing only Security review: Security review cleared: The stated change is confined to local executable discovery and tests; it adds no dependency, workflow, permission, download, secret, or package-resolution surface. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 1ddb6f1bbb42. |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(shell): resolve pwsh.exe/.cmd/.bat on PATH for Windows shell discovery This is item 1/1 in the current shard. Shard 19/22. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
What Problem This Solves
On Windows, PowerShell 7 installed via the Microsoft Store / App Execution
Alias ships as
pwsh.exe(most portable installs too).resolvePowerShellPath()step 3 called
resolveShellFromPath("pwsh"), which only probed a barepwshwith no extension.
fs.accessSynctherefore missed the real binary anddiscovery fell through to Windows PowerShell 5.1.
This means OpenClaw on Windows with PS7 installed via Microsoft Store would
silently use the older PS5.1 instead, missing features like
&&support andmodern cmdlets.
What Changed
src/agents/shell-utils.ts: ExportresolveShellFromPath, add.exeprobing on Windows (win32 only)src/agents/shell-utils.test.ts: Add tests forresolveWindowsBashPath,resolveShellFromPath,resolveShellFromWhich,resolvePowerShellPathThe
.exeextension is the only extension added..cmd/.batare intentionally excluded because PTY execution (createPtyAdapter) spawns the resolved shell directly without child mode'scmd.exewrapping, so apwsh.cmdselected here could fail in PTY mode..exeis directly executable in both child and PTY modes. Non-Windows behavior is unchanged.Evidence
Before fix (Windows with pwsh.exe on PATH via Microsoft Store)
resolveShellFromPath("pwsh")returnsundefinedbecausefs.accessSync("C:\...\pwsh", X_OK)fails — the file ispwsh.exe, notpwsh.After fix
resolveShellFromPath("pwsh")now also probespwsh.exevia theexecutableExtensionsloop, finds the real binary, and returns it.Test coverage (Windows-only, guarded by
isWin)resolveShellFromPath: findsname.exewhen barenameis absent; prefers bare match when both exist; resolves pwsh on PATH soresolvePowerShellPathprefers it over 5.1resolvePowerShellPath: prefers PS7 in ProgramFiles, prefers ProgramW6432, finds pwsh on PATH, falls back to PS 5.1resolveWindowsBashPath: finds Git Bash under ProgramFilesresolveShellFromWhich: useswhichas fallback on non-WindowsCI results
Supersedes
This PR supersedes the original #104086 which had unresolvable merge conflicts
due to being based on an older merge base (4633 commits behind main).