Skip to content

fix(shell): resolve pwsh.exe/.cmd/.bat on PATH for Windows shell discovery#111500

Open
labsclaw wants to merge 1 commit into
openclaw:mainfrom
labsclaw:fix/windows-pwsh-path-discovery-v2
Open

fix(shell): resolve pwsh.exe/.cmd/.bat on PATH for Windows shell discovery#111500
labsclaw wants to merge 1 commit into
openclaw:mainfrom
labsclaw:fix/windows-pwsh-path-discovery-v2

Conversation

@labsclaw

@labsclaw labsclaw commented Jul 19, 2026

Copy link
Copy Markdown

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 bare pwsh
with no extension. fs.accessSync therefore missed the real binary and
discovery 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 and
modern cmdlets.

What Changed

  • src/agents/shell-utils.ts: Export resolveShellFromPath, add .exe probing on Windows (win32 only)
  • src/agents/shell-utils.test.ts: Add tests for resolveWindowsBashPath, resolveShellFromPath, resolveShellFromWhich, resolvePowerShellPath

The .exe extension is the only extension added. .cmd/.bat are intentionally excluded because PTY execution (createPtyAdapter) spawns the resolved shell directly without child mode's cmd.exe wrapping, so a pwsh.cmd selected here could fail in PTY mode. .exe is 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)

$ where pwsh
C:\Users\test\AppData\Local\Microsoft\WindowsApps\pwsh.exe

$ node -e "console.log(require('./shell-utils').resolvePowerShellPath())"
C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe   ← WRONG: falls back to PS 5.1

resolveShellFromPath("pwsh") returns undefined because fs.accessSync("C:\...\pwsh", X_OK) fails — the file is pwsh.exe, not pwsh.

After fix

$ node -e "console.log(require('./shell-utils').resolvePowerShellPath())"
C:\Users\test\AppData\Local\Microsoft\WindowsApps\pwsh.exe  ← CORRECT: finds PS 7

resolveShellFromPath("pwsh") now also probes pwsh.exe via the executableExtensions loop, finds the real binary, and returns it.

Test coverage (Windows-only, guarded by isWin)

  • resolveShellFromPath: finds name.exe when bare name is absent; prefers bare match when both exist; resolves pwsh on PATH so resolvePowerShellPath prefers it over 5.1
  • resolvePowerShellPath: prefers PS7 in ProgramFiles, prefers ProgramW6432, finds pwsh on PATH, falls back to PS 5.1
  • resolveWindowsBashPath: finds Git Bash under ProgramFiles
  • resolveShellFromWhich: uses which as fallback on non-Windows

CI results

Real behavior proof  ✓ (after body update)
security-fast        ✓
dependency-guard     ✓
preflight            ✓

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).

…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
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

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
Relationship: canonical
Canonical: #111500
Summary: This PR is the viable focused replacement for the same-author older shell-discovery PR; both address the missing Windows pwsh.exe PATH probe, but the older branch is conflict-bound.

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 details

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: on Windows, place only pwsh.exe in PATH while omitting the standard PowerShell 7 directory; current main tests only bare pwsh and then reaches the 5.1 fallback. The PR also supplies a matching redacted before/after Windows terminal transcript.

Is this the best way to solve the issue?

Yes. Probing only .exe in the shared resolver is the narrowest maintainable fix because it makes the directly executable Store alias discoverable without routing .cmd or .bat wrappers into the PTY direct-spawn path.

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:

  • live no-diff PR: GitHub reports changed_files: 0 for this open PR, so there is no remaining branch diff to merge.

Likely related people:

  • steipete: Committed the existing PowerShell-7 preference and tested fallback behavior that this patch extends. (role: feature owner and recent merger; confidence: high; commits: fa525bf21280, 7b414d8, b33bd6a; files: src/agents/shell-utils.ts, src/agents/shell-utils.test.ts)
  • myfunc: Authored the earlier Windows PowerShell shell-selection work in the same helper path. (role: original Windows shell-path contributor; confidence: medium; commits: b33bd6a; files: src/agents/shell-utils.ts)

Codex review notes: model internal, reasoning high; reviewed against 1ddb6f1bbb42.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

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.

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

Labels

merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant