fix: resolve PowerShell as default shell on Windows when CLAUDE_CODE_USE_POWERSHELL_TOOL=true

[Keker8841]

Summary

When users set CLAUDE_CODE_USE_POWERSHELL_TOOL=true to enable the PowerShell tool on Windows, also resolve PowerShell as the default shell for ! commands and bash-mode.

Previously, users had to separately configure defaultShell: powershell in settings.json even after enabling the env var. This change makes the env var a single opt-in switch that fully activates PowerShell on Windows.

Change

src/utils/shell/resolveDefaultShell.ts — added Windows + env var check in the fallback chain:

settings.defaultShell -> (Windows + CLAUDE_CODE_USE_POWERSHELL_TOOL) -> 'bash'

Safety

• No change for existing users unless they explicitly set the env var. Without CLAUDE_CODE_USE_POWERSHELL_TOOL=true, behavior is identical.
• Follows the same pattern already used elsewhere (isEnvTruthy, getPlatform).
• Non-Windows platforms unaffected.

[kevincodex1]

can we rename this env var to OPEN_CLAUDE_USE_POWERSHELL_TOOL ? and also kindly add this too .env.example with proper comments

[kevincodex1]

LGTM! Thank you

[Vasanthdev2004]

Thanks for the PR. The goal makes sense, but I found one compatibility blocker on the current head.

Review scope: Targeted review of the PowerShell env-var/default-shell behavior.

Verdict: Needs changes

Blocking issue:

1. The PR title/body describe making CLAUDE_CODE_USE_POWERSHELL_TOOL=true also select PowerShell as the default shell, but the implementation replaces the existing env var with OPENCLAUDE_USE_POWERSHELL_TOOL only. That means existing Windows users who already set CLAUDE_CODE_USE_POWERSHELL_TOOL=1 would lose PowerShell tool/default-shell behavior after this change unless they rename their env var. This is especially risky because the current code already uses the legacy env var in isPowerShellToolEnabled().

Please keep OPENCLAUDE_USE_POWERSHELL_TOOL as the preferred name, but preserve CLAUDE_CODE_USE_POWERSHELL_TOOL as a backward-compatible fallback, ideally with precedence like:

• explicit OPENCLAUDE_USE_POWERSHELL_TOOL wins
• otherwise fall back to CLAUDE_CODE_USE_POWERSHELL_TOOL

Also please add a focused test covering:

• Windows + preferred env var => PowerShell enabled/default
• Windows + legacy env var only => still PowerShell enabled/default
• settings.defaultShell still overrides both
• non-Windows remains unchanged

Non-blocking note:

• The .env.example docs can recommend only the OpenClaude name, but the runtime should not silently strand existing users using the legacy name.

[kevincodex1]

LGTM

[techbrewboss]

Thanks for the compatibility fallback here. The implementation direction looks reasonable, but the new test suite currently fails on non-Windows runners because the Windows cases do not mock getPlatform() to return windows.

In src/utils/shell/shellToolUtils.test.ts, the tests under isPowerShellToolEnabled (Windows) and resolveDefaultShell (Windows) assert Windows behavior while running against the real platform. On my macOS checkout, the Windows truthy-env cases return false/bash because getPlatform() is macos; Linux CI should behave the same way. Please mock ../platform.js to windows for those sections, or otherwise isolate these cases so they are platform-independent.

Failing command:

bun test src/utils/shell/shellToolUtils.test.ts

Result: 14 pass, 6 fail. The failing assertions are around lines 72, 78, 95, 127, 137, and 148.

[jatmn]

I reviewed the latest PR head (53196d7875f0e61271e3820c9341cc123b8c9123) and do not have a new blocking finding to request fixes for.

The focused shell test suite now passes locally on the latest branch.

Requested fixes

No additional fixes from me.

techbrewboss requested changes

Verified against techbrewboss's review request: the latest PR head now mocks ../platform.js to return windows in the Windows cases under both isPowerShellToolEnabled (Windows) and resolveDefaultShell (Windows).

The requested failing command now passes locally:

bun test src/utils/shell/shellToolUtils.test.ts

Result: 20 pass, 0 fail.

Notes from validation

• OPENCLAUDE_USE_POWERSHELL_TOOL is documented in .env.example.
• CLAUDE_CODE_USE_POWERSHELL_TOOL remains supported as a backward-compatible fallback.
• The preferred OpenClaude env var takes precedence over the legacy env var.
• The Windows test sections now mock getPlatform() as windows, addressing techbrewboss's existing platform-specific test concern.

Commands run

bun test src/utils/shell/shellToolUtils.test.ts
bun test src/utils/shell/shellToolUtils.test.ts src/utils/processUserInput/processBashCommand.test.tsx
bun run build

All three commands passed locally.

LGTM

[techbrewboss]

LGTM. The latest commit fixes my previous blocker: the Windows-specific shell tests now mock ../platform.js to return windows, so they are no longer dependent on the runner OS.

I reran the focused validation in an isolated worktree:

bun test ./src/utils/shell/shellToolUtils.test.ts ./src/utils/processUserInput/processBashCommand.test.tsx
bun run build

The shell suite passed with 20 pass, 0 fail, and the build completed successfully.

[Vasanthdev2004]

Targeted re-review of current head 53196d7.

Verdict: Needs changes

The previous compatibility concern is fixed: OPENCLAUDE_USE_POWERSHELL_TOOL is preferred, CLAUDE_CODE_USE_POWERSHELL_TOOL remains a legacy fallback, and the helper is used by both tool enablement and default-shell resolution.

Blocking issue:

1. The focused test suite added by this PR is still not deterministic locally. Running bun test src/utils/shell/shellToolUtils.test.ts src/utils/processUserInput/processBashCommand.test.tsx fails in resolveDefaultShell (Windows) > returns bash by default on Windows without env var with:

Cannot find module './commands/fork/index.js' from 'src/commands.ts'

That specific test imports resolveDefaultShell without first mocking ../settings/settings.js, so the real settings module pulls in the broader command graph and hits the feature-gated missing commands/fork require path. The later resolveDefaultShell tests already mock settings before import, so the fix should be small: make the default-shell test use the same settings mock/isolation pattern before importing resolveDefaultShell.

What I checked:

• Current head: 53196d7
• The legacy/preferred env fallback implementation in shellToolUtils.ts and resolveDefaultShell.ts
• Tip suppression behavior for both env var names
• Local validation: bun run build passed, but the focused test command above fails as described.

Happy to re-review once the test import path is made deterministic.

[gnanam1990]

Thanks for the iteration here — the legacy/preferred env var precedence (OPENCLAUDE_USE_POWERSHELL_TOOL wins, CLAUDE_CODE_USE_POWERSHELL_TOOL falls back) and the getPlatform() mocking in the Windows test sections both look right. techbrewboss and jatmn already verified the latest head and approved.

On Vasanthdev2004's most recent CHANGES_REQUESTED: I cannot reproduce the Cannot find module './commands/fork/index.js' failure they cited. The path they ran with — src/utils/processUserInput/processBashCommand.test.tsx — does not exist in this branch (only processBashCommand.tsx), so bun test silently no-ops on the missing path and only runs the shell suite. The shell suite is deterministic on its own.

Non-blocking:

• .env.example documents the OpenClaude name only, with a comment about legacy compatibility — appropriate.
• Tip suppression covers both names.

Verified locally: gh pr checkout 1043; bun test src/utils/shell/shellToolUtils.test.ts → 20 pass, 0 fail; combined run with the (non-existent) processBashCommand path matches Vasanthdev's command and still passes 20/20 for me.

[Vasanthdev2004]

Targeted re-review of current head a88664f.

Verdict: Approve-ready

The previous blocker I raised is fixed on the latest head. The default resolveDefaultShell test now mocks settings.js before importing resolveDefaultShell, so it no longer pulls the broader command graph and no longer hits the missing feature-gated commands/fork require path.

What I checked:

• Preferred env var remains OPENCLAUDE_USE_POWERSHELL_TOOL.
• Legacy CLAUDE_CODE_USE_POWERSHELL_TOOL still works as a fallback.
• Preferred OpenClaude env var wins when both are set.
• Tip suppression checks both env var names.
• Windows-specific tests mock platform as windows.

Local validation:

• bun test src/utils/shell/shellToolUtils.test.ts passed: 20 pass, 0 fail.
• bun run build passed.

I do not see a remaining blocker from my side.