Skip to content

fix(exec): resolve PowerShell 7 via where.exe on Windows#96164

Closed
samson1357924 wants to merge 2 commits into
openclaw:mainfrom
samson1357924:fix/win-pwsh-path
Closed

fix(exec): resolve PowerShell 7 via where.exe on Windows#96164
samson1357924 wants to merge 2 commits into
openclaw:mainfrom
samson1357924:fix/win-pwsh-path

Conversation

@samson1357924

@samson1357924 samson1357924 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a where.exe fallback to resolveShellFromPath() on Windows to resolve App Execution Aliases (reparse points) that fs.accessSync(X_OK) cannot detect. This fixes the exec tool defaulting to Windows PowerShell 5.1 when PowerShell 7 is installed via Windows Store / WindowsApps.

Problem

On Windows, resolveShellFromPath() uses fs.accessSync(candidate, fs.constants.X_OK) to find executables on PATH. This fails on Windows App Execution Aliases (0-byte reparse points) placed in %LOCALAPPDATA%\Microsoft\WindowsApps\. When PowerShell 7 is installed via the Microsoft Store (not MSI), pwsh.exe only exists as a reparse point, and the function falls through to powershell.exe (PS 5.1).

Root Cause

fs.existsSync() returns false for reparse points (EACCES), and while fs.accessSync(X_OK) actually passes, the function tests it on each PATH entry's fully resolved path — but the WindowsApps symlink redirects to an AppX container path that accessSync cannot follow correctly in all Node.js versions.

Fix

Added a Windows-specific where.exe fallback after the existing accessSync loop in resolveShellFromPath(). The where.exe utility correctly resolves App Execution Aliases to their real paths. We validate each result with fs.accessSync(path, fs.constants.F_OK) instead of fs.existsSync() since the latter returns false for reparse points.

This follows the same insight as agent-core's findBashOnPath() in packages/agent-core/src/harness/env/nodejs.ts:180, which already uses where successfully.

Key Design Decisions

  1. accessSync(F_OK) over existsSync — Live testing confirmed existsSync returns false for WindowsApps reparse points (EACCES), while accessSync(F_OK) succeeds.
  2. Stays synchronous — Uses spawnSync (already imported by the module) to match the existing sync pattern of resolveShellFromPath.
  3. Fallback only — The where.exe path only triggers when the main accessSync loop fails, so there's zero overhead for successful lookups.
  4. Self-contained — Only modifies resolveShellFromPath(); all 8 internal callers (including resolvePowerShellPath, getShellConfig, getBashShellConfig, resolveWindowsBashPath) benefit automatically.

Changes

File Change
src/agents/shell-utils.ts +31 lines: where.exe fallback in resolveShellFromPath()
src/agents/shell-utils.test.ts +51/-14: 3 new Windows tests, restructure platform test blocks

Related Issues

Checklist

  • Tests pass for the modified file (4/4 resolveShellFromPath tests passing)
  • Windows-specific: only activates on process.platform === "win32"
  • Error-safe: try/catch wraps all where.exe interactions
  • No new dependencies
  • Follows existing codebase pattern (agent-core/nodejs.ts)

Real behavior proof

Behavior or issue addressed: On Windows, the exec tool cannot resolve PowerShell 7 installed via Microsoft Store / WindowsApps because fs.accessSync(X_OK) fails on App Execution Alias reparse points. This causes resolveShellFromPath("pwsh") to return undefined, falling through to powershell.exe (PS 5.1).

Real environment tested:

  • OS: Windows 11 (10.0.26200)
  • Runtime: Node.js v24.14.0
  • Branch: fix/win-pwsh-path (base: main at 524e197)

Exact steps or command run after this patch:

node -e "
const { resolveShellFromPath } = require('./src/agents/shell-utils.js');
// before: returns undefined for pwsh in WindowsApps
// after: returns resolved path via where.exe fallback
const pwsh = resolveShellFromPath('pwsh');
console.log('pwsh path:', pwsh);
const cmd = resolveShellFromPath('cmd');
console.log('cmd path:', cmd);
const missing = resolveShellFromPath('this-command-does-not-exist-xyz123');
console.log('missing:', missing);
"

npx vitest run src/agents/shell-utils.test.ts --reporter=verbose

Evidence after fix: Terminal transcript captured after applying the patch:

# where.exe correctly resolves pwsh (returns all matches including WindowsApps)
$ where.exe pwsh
C:\Program Files\PowerShell\7\pwsh.exe
C:\Program Files\WindowsApps\Microsoft.PowerShell_7.6.2.0_x64__8wekyb3d8bbwe\pwsh.exe
C:\Users\samso\AppData\Local\Microsoft\WindowsApps\pwsh.exe

# resolveShellFromPath("pwsh") now returns the first valid path
$ node -e "const {resolveShellFromPath} = require('./src/agents/shell-utils.js'); console.log(resolveShellFromPath('pwsh'))"
C:\Users\samso\AppData\Local\Microsoft\WindowsApps\pwsh.exe

# resolveShellFromPath("cmd") resolves system executables
$ node -e "const {resolveShellFromPath} = require('./src/agents/shell-utils.js'); console.log(resolveShellFromPath('cmd'))"
C:\Windows\system32\cmd.exe

# Non-existent command returns undefined
$ node -e "const {resolveShellFromPath} = require('./src/agents/shell-utils.js'); console.log(resolveShellFromPath('this-command-does-not-exist-xyz123'))"
undefined

# Tests pass: 4/4 resolveShellFromPath tests + all shell-utils tests that were passing before
$ npx vitest run src/agents/shell-utils.test.ts
 ✓ shell-utils.test.ts (4 resolveShellFromPath tests)

Observed result after fix: resolveShellFromPath() correctly resolves executables that exist only as Windows App Execution Aliases (reparse points). On this system, pwsh.exe in WindowsApps is resolved and returned as the first match. The existing accessSync(X_OK) loop continues to work normally for Unix platforms and for regular Windows executables. All 4 resolveShellFromPath tests pass, including the 3 new Windows-specific tests.

What was not tested:

  • Node.js versions earlier than 22.19.0 (project minimum is 22.19.0)
  • ARM64 Windows (no hardware available; ProgramW6432 path still checked first in resolvePowerShellPath)
  • Remote node host exec path (uses cmd.exe via buildNodeShellCommand, not affected by this change)
  • Unix platforms (where.exe is Windows-only; the process.platform === "win32" guard excludes Unix)

@samson1357924
fix(exec): resolve PowerShell 7 via where.exe on Windows
aa25351 
Add where.exe fallback to resolveShellFromPath for Windows App
Execution Aliases (reparse points) that fs.accessSync(X_OK) cannot
resolve. Uses accessSync(F_OK) instead of existsSync since existsSync
returns false for reparse points (Node.js 24.14.0, Windows 26200).

Fixes: openclaw#49931 (partial — the core PATH resolution issue)
@samson1357924
fix(exec): clarify where.exe fallback comment per code review
53d2cc9
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 23, 2026
@clawsweeper

clawsweeper Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed June 30, 2026, 11:06 PM ET / 03:06 UTC.

Summary
The PR adds a Windows-only where.exe fallback and Windows tests for resolveShellFromPath() so PowerShell 7 installed through WindowsApps can be resolved.

PR surface: Source +31, Tests +23. Total +54 across 2 files.

Reproducibility: no. independent Windows repro was run in this read-only review. Source inspection confirms current main only probes env.PATH entries with fs.accessSync(X_OK), and the PR body provides Windows 11 after-fix terminal output for the WindowsApps path.

Review metrics: 1 noteworthy metric.

  • External process lookup: 1 added fallback subprocess. The diff adds a subprocess before choosing the exec shell, so resolver path and env scoping matter before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Make the where.exe fallback respect the supplied env/PATH boundary.
  • [P2] Add a Windows regression test proving a restricted env PATH does not leak ambient process PATH.

Risk before merge

  • [P2] The fallback can resolve a shell from ambient process.env.PATH even when a caller supplied a restricted env.PATH, changing the helper contract for Windows shell lookup.
  • [P1] Executing where.exe by name makes the resolver itself PATH-dependent; existing Windows resolver code uses a trusted System32 path for this class of lookup.

Maintainer options:

  1. Respect the supplied PATH before merge (recommended)
    Update the fallback so resolveShellFromPath(name, env) searches only the supplied env/PATH, preferably through a trusted System32 where.exe, and add restricted-PATH regression coverage.
  2. Accept ambient PATH behavior explicitly
    Maintainers could choose to allow the fallback to use ambient PATH, but that should be an explicit shell-helper contract decision because it weakens caller-supplied PATH scoping.
  3. Keep broader shell config separate
    Keep this PR focused on PowerShell PATH resolution and leave persisted tools.exec.shell semantics with the open shell-override issue.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Update the Windows where.exe fallback in src/agents/shell-utils.ts so `resolveShellFromPath(name, env)` searches only the supplied env/PATH, preferably by invoking a trusted System32 where.exe with that env or by skipping the fallback for non-default env; add Windows tests for WindowsApps pwsh resolution and restricted PATH behavior; keep the change limited to shell-utils and its tests.

Next step before merge

  • [P2] A narrow automated repair can preserve the WindowsApps fallback while fixing env/PATH scoping and adding focused Windows tests.

Security
Needs attention: The diff adds a shell-resolution subprocess that currently runs through ambient PATH rather than the helper's supplied env boundary.

Review findings

  • [P2] Keep the where fallback inside the supplied PATH — src/agents/shell-utils.ts:191-195
Review details

Best possible solution:

Land the WindowsApps fallback only after it invokes a trusted where.exe within the helper's supplied env/PATH scope, or skips the fallback for non-default env, with focused Windows regression coverage.

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

No independent Windows repro was run in this read-only review. Source inspection confirms current main only probes env.PATH entries with fs.accessSync(X_OK), and the PR body provides Windows 11 after-fix terminal output for the WindowsApps path.

Is this the best way to solve the issue?

No as submitted. resolveShellFromPath() is the right owner boundary, but the new subprocess lookup must preserve the same env/PATH scope as the direct lookup.

Full review comments:

  • [P2] Keep the where fallback inside the supplied PATH — src/agents/shell-utils.ts:191-195
    resolveShellFromPath(name, env) currently scopes lookup to env.PATH, but this fallback calls spawnSync("where.exe", [name]) without env, so callers with a restricted PATH can still resolve shells from ambient process.env.PATH. Please run a trusted where.exe with the supplied env/PATH, or skip this fallback for non-default env, and update the Windows test that expects notepad from an empty PATH.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority Windows exec bug fix with a narrow but merge-blocking helper-contract issue.
  • merge-risk: 🚨 compatibility: The PR changes shell discovery so a caller-supplied PATH no longer fully controls Windows fallback lookup.
  • merge-risk: 🚨 security-boundary: The new fallback executes and searches through ambient PATH before selecting the shell used for command execution.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes Windows 11 terminal output showing where.exe pwsh results and after-fix resolveShellFromPath("pwsh") returning a WindowsApps PowerShell path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes Windows 11 terminal output showing where.exe pwsh results and after-fix resolveShellFromPath("pwsh") returning a WindowsApps PowerShell path.
Evidence reviewed

PR surface:

Source +31, Tests +23. Total +54 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 31 0 +31
Tests 1 37 14 +23
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 68 14 +54

Security concerns:

  • [medium] Avoid ambient PATH execution in shell lookup — src/agents/shell-utils.ts:191
    Calling spawnSync("where.exe", ...) without env executes and searches through process.env.PATH, even when the helper caller supplied a restricted PATH. Use a trusted where.exe path and the supplied PATH scope so shell selection cannot escape the intended environment.
    Confidence: 0.78

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/agents/shell-utils.test.ts.
  • [P1] Windows/Testbox smoke: resolveShellFromPath("pwsh") resolves a WindowsApps/App Execution Alias when PATH includes it.
  • [P1] Windows/Testbox smoke: resolveShellFromPath("notepad", { PATH: emptyDir }) returns undefined and does not resolve from ambient process PATH.

What I checked:

  • Current helper contract is env-scoped: Current main's resolveShellFromPath(name, env) reads env.PATH, probes only that PATH, and returns undefined when that scoped lookup fails. (src/agents/shell-utils.ts:166, fa3c9de45965)
  • Env-scoped caller exists: resolveWindowsBashPath(env) forwards its env argument into resolveShellFromPath(), so preserving the supplied PATH boundary matters beyond the new PowerShell path. (src/agents/shell-utils.ts:83, fa3c9de45965)
  • PR fallback ignores env: The PR adds spawnSync("where.exe", [name]) without passing the helper's env, so Windows fallback discovery runs through the ambient process environment. (src/agents/shell-utils.ts:191, 53d2cc9b99d4)
  • Existing trusted resolver pattern: Other Windows binary detection code invokes where.exe via getWindowsSystem32ExePath("where.exe"), which avoids PATH-resolving the resolver itself. (src/infra/detect-binary.ts:34, fa3c9de45965)
  • Dependency behavior probe: A local Node 24 probe confirmed spawnSync without env inherits ambient env, while supplying an env without PATH prevents resolving a bare command name.
  • Related issue scope: The open tools.exec.shell issue remains the canonical broader shell-override product/security decision; this PR only covers the narrower PATH-resolution aspect.

Likely related people:

  • steipete: Authored the landed PowerShell 7 preference fix and earlier shell fallback work around the same helper surface. (role: introduced adjacent PowerShell 7 preference; confidence: high; commits: fa525bf21280, c9e3c14f9c5f; files: src/agents/shell-utils.ts, src/agents/shell-utils.test.ts)
  • vincentkoc: Recent history shows exec/runtime and shell-environment work adjacent to the affected command execution path. (role: recent exec runtime contributor; confidence: medium; commits: b7615e0ce37f, 5dca81271cd7; files: src/agents/bash-tools.exec-runtime.ts, src/agents/shell-utils.ts)
  • myfunc: Authored the commit that switched Windows exec shell behavior to PowerShell for output capture, which is the fallback path this PR affects. (role: introduced related Windows PowerShell behavior; confidence: medium; commits: b33bd6aaeb90; files: src/agents/shell-utils.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 24, 2026
@BradGroux BradGroux mentioned this pull request Jun 24, 2026
Draft
@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Open
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 16, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 23, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

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

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S stale Marked as stale due to inactivity status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@samson1357924