fix(core): resolve Windows shells installed as app-execution aliases

[Hotragn]

Issue for this PR

Closes #41426

Type of change

• Bug fix

What does this PR do?

On Windows, a configured shell: "pwsh" is silently ignored and commands run under PowerShell 5.1 instead.

When PowerShell 7 is installed from the Store/MSIX package, pwsh on PATH is a Windows app-execution alias: a zero-byte AppExecLink reparse point, not a real executable. CreateProcess resolves it by name, but stat does not see it. Shell.resolve() goes through which(), which stats candidates, so the alias resolves to nothing and select() falls back to win()[0] — Windows PowerShell 5.1 — without reporting that the configured shell was not found.

Measured on Windows 11 with a Store-installed alias:

fs.existsSync(<alias>)   -> false
fs.statSync(<alias>)     -> undefined
spawnSync("winget", ...) -> status 0

When which() finds nothing on Windows for a name we already recognise as a shell, this falls back to where.exe, which does resolve aliases, and returns the bare name so CreateProcess performs the lookup. where.exe distinguishes the two cases cleanly: exit 0 for an installed alias (winget), exit 1 for something genuinely absent (pwsh when not installed, zsh, ksh).

The fallback is deliberately narrow — Windows only, only after which() has already failed, and only for names in the existing shell table — so an unrelated missing binary still resolves to undefined as before.

How did you verify your code works?

• Added a test asserting a shell name that resolves to nothing stays undefined, guarding the case where where.exe might otherwise return a non-shell match.
• bun test test/shell.test.ts in packages/core passes (12/12), and the dependent shell/bash tool suites in packages/opencode still pass.
• Verified the alias behaviour directly on Windows 11 with the numbers above, using winget as a stand-in alias, and confirmed where.exe exits 1 for shells that are actually absent.
• bun typecheck passes.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[Enough1122]

AI code review — automated review for reference; please use your judgment.

• packages/core/src/shell.ts:107-109 — aliased() uses synchronous spawnSync, which blocks the Node event loop for a full process launch (~tens of ms) on every shell resolution that misses which; if resolve() runs per tool call or per session start this adds latency on exactly the machines (Store-installed shells) this PR targets. Memoize the result per shell name (positive and negative lookups) and/or move to async spawn in the caller's Effect pipeline.
• packages/core/src/shell.ts:97 — returning the bare alias name ("pwsh") diverges from resolve()'s existing contract, where which() yields an absolute path. Any consumer that stats, reads, or version-probes the resolved shell will break against an AppExecLink reparse point (they are not readable as regular files), reintroducing a silent-failure path one layer down. Prefer capturing where.exe stdout and returning its first line, so callers get a concrete path while execution still goes through the alias.
• packages/core/src/shell.ts:108 — spawnSync("where.exe", …) can throw outright (where.exe absent on stripped-down images, EPERM under restricted tokens); an exception escaping resolve() turns a fallback decision into a crash. Wrap in try/catch and treat spawn failure as false.
• packages/core/test/shell.test.ts:52 — only the negative branch is tested (ksh not installed ⇒ family fallback); the actual fix — the win32 alias branch — has no coverage because simulating an alias is awkward. A reasonable middle ground: drop a stub .exe into a temp dir prepended to PATH for the child lookup (or inject the aliased predicate) and assert resolve() now returns the configured shell instead of falling back.
• packages/core/src/shell.ts:96 — the meta(shell) gate means a shell name outside the known-families table still returns undefined even when an app-execution alias exists; if intentional (avoid trusting arbitrary names), say so in the comment, since users pointing shell config at an exotic Store app will still hit the silent fallback this PR aims to remove. (nit)

[Hotragn]

Thanks — this was useful, and three of the five landed. I've pushed the changes to the v2 version of this work (#48968), since that is where the branch now targets.

Blocking call on every miss — you were right, and it's worse than "tens of ms". I measured a single where.exe launch at 312ms on this machine. It's also not bounded by resolve()'s cache, because resolve() returns before reaching it whenever a shell is configured:

if (configShell) return select(configShell, options, filter, bin)   // ← returns here
const cached = input.priority === "compat" ? defaultCompatible : defaultConfigured

A configured shell: "pwsh" is exactly the case this PR is about, so every resolution paid the full cost, and list() pays it per candidate. Memoizing at resolve() would therefore have missed it — I've memoized at the lookup instead, caching hits and misses per name, with a reset() following the resolve.reset() convention already in the file. Cached lookups now measure 0ms.

Returning a bare name — agreed, and adopted. aliased() now returns where.exe's first stdout line, so callers get an absolute path as they do from which(). Worth knowing what that path actually behaves like, measured against the winget alias:

probe	result
which("winget")	null
aliased("winget")	C:\Users\…\WindowsApps\winget.exe
fs.existsSync(path)	true
fs.statSync(path)	throws EACCES

So this fixes existence checks and anything doing path arithmetic, but a caller that stats the result still gets an error — just EACCES instead of "not found". executable() uses statSync(..., { throwIfNoEntry: false }), which only swallows ENOENT, so that limitation is worth being explicit about rather than papering over.

spawnSync throwing — I don't think this one holds. It reports a failed launch on .error instead of throwing:

spawnSync("definitely-not-a-real-exe.exe") -> status: undefined, error: ENOENT   (no throw)

So a missing or blocked where.exe already degrades to a miss via the status === 0 check. I've noted that in a comment so the next reader doesn't have to re-derive it. Happy to add a try/catch anyway if you'd rather not rely on that.

Test coverage — fair, fixed. A stub .exe on PATH wouldn't reach the branch, since which() would find it first and return early. Instead I test the detection against a real alias: winget ships as one on stock Windows 11, isn't a shell, and is invisible to which — so it exercises the path without depending on which shells are installed. Three cases added: alias found and absolute, unknown name returns undefined, and a second lookup served from cache. 14/14 in the file.

meta() gate — intentional, now documented. Without it any configured string could resolve to an unrelated Store app sharing the name. I've said so in the comment. An exotic Store shell still falls back, which I'd rather leave to a follow-up that widens the families table deliberately.