Skip to content

fix(core): prefer pwsh.exe over Windows PowerShell 5.1 (#25859)#25900

Closed
kaluchi wants to merge 5 commits into
google-gemini:mainfrom
kaluchi:fix/windows-prefer-pwsh-25859
Closed

fix(core): prefer pwsh.exe over Windows PowerShell 5.1 (#25859)#25900
kaluchi wants to merge 5 commits into
google-gemini:mainfrom
kaluchi:fix/windows-prefer-pwsh-25859

Conversation

@kaluchi
Copy link
Copy Markdown

@kaluchi kaluchi commented Apr 24, 2026

Summary

Fixes #25859. Related: closes-as-effectively-same #18374; addresses the feature-request spirit of #15493, #2353, #6413.

On Windows, run_shell_command with embedded double quotes (e.g. node -e 'console.log(\"test\")') fails because Windows PowerShell 5.1 silently strips \" during native-command argument passing. PowerShell 7 (pwsh.exe) uses standards-compliant argument passing (\$PSNativeCommandArgumentPassing = 'Windows' by default since 7.3) and does not exhibit this behavior.

This PR makes getShellConfiguration prefer pwsh.exe from PATH over powershell.exe on Windows. Users with PowerShell 7 installed get the fix; users without keep the existing behavior — no regression either way.

Context

Windows PowerShell 5.1 is in legacy maintenance mode. Microsoft itself displays this banner when 5.1 starts:

```
Windows PowerShell
Copyright (C) Microsoft Corporation. All rights reserved.

Install the latest PowerShell for new features and improvements!
https://aka.ms/PSWindows
```

PS 5.1 is .NET Framework-bound, Windows-only, and lags PS 7 on shell behavior. `$PSNativeCommandArgumentPassing` (fixes native-cmd arg passing) and bash-style `&&`/`||` chain operators were added in PS 7 and cannot be backported to 5.1.

Respecting the user's environment. gemini-cli is a developer tool, and developers who work on Windows overwhelmingly install PowerShell 7 — it's the modern, cross-platform, actively-maintained runtime Microsoft recommends. Currently gemini-cli ignores the user's choice and pins Windows invocations to `powershell.exe` (5.1 on any modern Windows) regardless of whether pwsh 7 is installed. This PR follows both Microsoft's own guidance and the user's clear intent: prefer PS 7 when it's on their system, fall back to 5.1 only when it isn't.

Side benefits

PowerShell 7 also addresses other Windows-specific pain reported against gemini-cli:

These aren't explicitly targeted by the fix but improve as a natural consequence for users who have pwsh 7 installed.

Verification

Reproduce directly in each shell — same commands, different results.

Issue 1: embedded double quotes stripped (#25859)

Windows PowerShell 5.1:
```
PS C:\Users\EugenPC> node -e 'console.log("test")'
[eval]:1
console.log(test)
^
ReferenceError: test is not defined
at [eval]:1:13
...
Node.js v22.18.0
```

PowerShell 7.6.1:
```
PS C:\Users\EugenPC> node -e 'console.log("test")'
test
```

Issue 2: `&&` / `||` operators rejected (#20773, #21997, #18022)

Windows PowerShell 5.1:
```
PS C:\Users\EugenPC> echo a && echo b
At line:1 char:8

  • echo a && echo b
  •    ~~
    

The token '&&' is not a valid statement separator in this version.
```

PowerShell 7.6.1:
```
PS C:\Users\EugenPC> echo a && echo b
a
b
```

With this PR, gemini-cli routes through `pwsh.exe` on Windows when it's installed, inheriting the correct behavior for both issues.

Behavior matrix

Environment Before After
Windows + `pwsh.exe` in PATH powershell pwsh (fix)
Windows, no `pwsh.exe` powershell powershell
Windows, explicit `ComSpec` honored honored
Windows sandbox (`windows-native`) cmd.exe /c cmd.exe /c
macOS / Linux bash bash

Only the first row changes behavior.

Tests

  • Unit tests in `shell-utils.test.ts` covering new pwsh preference, path-extension handling, unset `PATH`.
  • New Windows-only integration file (`shellExecutionService.windows.integration.test.ts`, skipped on non-Windows): 5 cases driving real spawns through `ShellExecutionService`, asserting byte-exact preservation of embedded `"`.
  • Full core suite: 0 regressions vs. `main`.

Root cause

PR #11157 (2025-10-16) changed the default Windows shell from `cmd.exe /d /s /c` to `powershell.exe -NoProfile -Command`. Windows PowerShell 5.1 lacks `$PSNativeCommandArgumentPassing` (introduced in PS 7.2 as experimental, default since 7.3) and has a long-standing runtime limitation: `"` inside arguments is dropped when invoking native executables. Fixable only by running through PS 7 — which is what this PR does.

@kaluchi kaluchi requested a review from a team as a code owner April 24, 2026 01:41
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request improves shell command execution on Windows by prioritizing PowerShell 7 (pwsh.exe) over the legacy Windows PowerShell 5.1. This change resolves long-standing issues with argument quoting and improves compatibility with modern shell features like '&&' and '||' operators, while maintaining backward compatibility for users without PowerShell 7 installed.

Highlights

  • PowerShell 7 Preference: Updated the shell configuration to prefer 'pwsh.exe' (PowerShell 7) over 'powershell.exe' (Windows PowerShell 5.1) on Windows when available in the PATH.
  • Resolved Argument Quoting Issue: Addressed the issue where Windows PowerShell 5.1 incorrectly strips embedded double quotes from arguments passed to native executables.
  • Synchronous Executable Resolution: Refactored 'resolveExecutable' to be synchronous, allowing it to be used during shell configuration initialization.
  • New Integration Tests: Added Windows-specific integration tests to verify that shell commands with inline double quotes are executed correctly.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-cli gemini-cli Bot added the priority/p2 Important but can be addressed in a future release. label Apr 24, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the shell configuration to prefer pwsh.exe on Windows to resolve quoting issues and adds integration tests for verification. It also refactors the resolveExecutable utility to be synchronous and updates the test suite to use vi.stubEnv. Feedback indicates that the switch to synchronous I/O blocks the event loop and violates project conventions, identifies a regression in Windows executable resolution for extensionless files, and suggests using asynchronous operations with real-path resolution for better security and performance.

Comment thread packages/core/src/utils/shell-utils.ts
Comment thread packages/core/src/utils/shell-utils.ts
Comment thread packages/core/src/utils/shell-utils.ts Outdated
Comment thread packages/core/src/utils/shell-utils.ts
@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented Apr 24, 2026

/gemini review

@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented Apr 24, 2026

pimg-20260424_203311 For reference — GitHub's own https://github.com/github/copilot-cli ships with pwsh as the default shell on Windows. Same repro from this PR (node -e 'console.log("test")') runs cleanly there, for the same reason this PR fixes it in gemini-cli.

@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented Apr 24, 2026

Note this PR is non-breaking by design: Copilot CLI refuses to run without pwsh 7 (#847) and won't support 5.1 (#411, not planned). Here, 5.1 users keep working exactly as before — only users who already installed pwsh get the fix.

@kaluchi kaluchi force-pushed the fix/windows-prefer-pwsh-25859 branch from 506111a to 650765e Compare April 26, 2026 19:50
Copy link
Copy Markdown

@izambe izambe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good

Comment thread packages/core/src/utils/shell-utils.ts
kaluchi added 3 commits May 6, 2026 08:15
Windows PowerShell 5.1 silently strips embedded double quotes from
arguments when invoking native executables, causing commands such as
`node -e 'console.log("test")'` to fail with `ReferenceError: test is
not defined` when run via `!` in shell mode. PowerShell 7 (pwsh.exe)
uses standards-compliant argument passing and does not exhibit this
behavior.

On Windows, `getShellConfiguration` now prefers `pwsh.exe` on PATH
before falling back to `powershell.exe`. Users with PowerShell 7
installed get the fix automatically; users without it keep the
existing behavior and full cmdlet surface — no regression either way.

The pre-existing `resolveExecutable` helper is converted to sync,
matching the prevailing style in `shell-utils.ts` (the same file
already uses `spawnSync` for the PowerShell AST parser).

Fixes google-gemini#25859
Review feedback from gemini-code-assist caught a regression I
introduced: by skipping the empty extension when the input exe has
no extension, commands like `resolveExecutable('mytool')` on Windows
stopped finding extensionless binaries.

Revert the extension list to match the original async version:
always probe `['.exe', '.cmd', '.bat', '']` on Windows. The
`path.extname` optimization was saving microseconds per call at the
cost of a real correctness bug — not worth it.

Drop the test I added for that optimization.
Match the file's existing convention for Windows path literals
(see line 521: 'C:\Program Files\PowerShell\7\pwsh.exe' and
line 613: path.resolve('C:\Windows\System32')).
@kaluchi kaluchi force-pushed the fix/windows-prefer-pwsh-25859 branch from 650765e to 4fded9b Compare May 6, 2026 05:18
The test uses Windows-style paths (C:\...) that break on Linux/Mac
because path.delimiter splits on ":" — the colon in "C:" fractures
the mocked PATH into two garbage entries.  The real validation runs
on the Windows CI runner where path semantics are native.
@gemini-cli
Copy link
Copy Markdown
Contributor

gemini-cli Bot commented May 8, 2026

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding.

@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented May 8, 2026

@scidomino please help here

@Kevinrob
Copy link
Copy Markdown

Kevinrob commented May 8, 2026

Gemini CLI needs this PR for the Windows users 🤩

@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented May 13, 2026

any updates or resolution here?

@scidomino scidomino reopened this May 13, 2026
@scidomino
Copy link
Copy Markdown
Collaborator

Sorry. Rerunning the tests.

@scidomino
Copy link
Copy Markdown
Collaborator

failed lint.

@gemini-cli gemini-cli Bot added priority/p1 Important and should be addressed in the near term. area/core Issues related to User Interface, OS Support, Core Functionality area/agent Issues related to Core Agent, Tools, Memory, Sub-Agents, Hooks, Agent Quality labels May 13, 2026
Comment thread packages/core/src/utils/shell-utils.ts
@gemini-cli
Copy link
Copy Markdown
Contributor

gemini-cli Bot commented May 14, 2026

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding.

@gemini-cli gemini-cli Bot closed this May 14, 2026
@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented May 14, 2026

Thanks for the review.

The auto-close bot fired again on the PR after you reopened it — same misconfigured policy.

The lint failure from the main-merge is fixed and pushed to the branch (PR head won't show it until reopen). The help wanted label would stop the bot loop.

I resolved the main conflict in favor of the sync resolveExecutable:

a) the alternative has a large transitive blast radius — cascading async through the consumers of getShellConfiguration (4 sync sites in shell-utils itself, plus a React render in SessionSummaryDisplay.tsx);

b) the previous async version was already synchronous in effect — serial for ... of over PATH with sequential await fs.promises.access per entry, no Promise.all;

c) the surrounding execution flow has large synchronous sections — command parsing (parsePowerShellCommandDetails uses spawnSync('powershell.exe', ...)), argument-safety checks, and resolveExecutable itself is called from a synchronous prepare stage in shellExecutionService.

@kaluchi
Copy link
Copy Markdown
Author

kaluchi commented May 14, 2026

@scidomino — the auto-close bot closed the PR again after your reopen (request-review button isn't available while it's closed). Could you reopen and add the 'help wanted' label?

Lint fix and rationale for the sync conflict are in the comment above.

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

Labels

area/agent Issues related to Core Agent, Tools, Memory, Sub-Agents, Hooks, Agent Quality area/core Issues related to User Interface, OS Support, Core Functionality priority/p1 Important and should be addressed in the near term. priority/p2 Important but can be addressed in a future release. status/pr-nudge-sent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: Regression - run_shell_command strips double quotes

5 participants