Skip to content

[codex] Add native Windows shell support#41

Open
yurilopes wants to merge 3 commits into
Waishnav:mainfrom
yurilopes:codex/windows-powershell-shell-support
Open

[codex] Add native Windows shell support#41
yurilopes wants to merge 3 commits into
Waishnav:mainfrom
yurilopes:codex/windows-powershell-shell-support

Conversation

@yurilopes

@yurilopes yurilopes commented Jun 26, 2026

Copy link
Copy Markdown

What changed

This PR improves DevSpace shell execution on Windows while preserving the existing Bash behavior for Linux, macOS, and users who explicitly opt into Bash.

  • Adds DEVSPACE_SHELL=auto|bash|powershell|cmd.
  • Uses native PowerShell by default on Windows instead of routing PowerShell commands through Bash/MSYS.
  • Adds native cmd.exe support via DEVSPACE_SHELL=cmd.
  • Keeps Pi's Bash backend for DEVSPACE_SHELL=bash and non-Windows defaults.
  • Adds a PowerShell guardrail that blocks fragile -match usage with raw Windows path literals, including literals assigned to variables and then passed to -match.
  • Updates devspace doctor, MCP tool guidance, docs, and tests.

Why

When ChatGPT generated PowerShell commands through the previous Bash-backed shell, Bash/MSYS could expand PowerShell syntax such as $_.CommandLine before PowerShell received it. Native PowerShell execution fixes that transport issue.

A second common failure mode remained: using PowerShell -match for literal Windows path fragments. -match is regex-based, so path fragments like \profiles can trigger malformed regex escapes such as \p. The new guardrail blocks the fragile pattern and tells the model to use .Contains(), -like, or [regex]::Escape().

Impact

Windows users can run DevSpace with native PowerShell by default. Existing Bash workflows remain available with DEVSPACE_SHELL=bash, and Linux/macOS behavior stays on the existing Bash backend.

Validation

  • npm test
  • npm run typecheck
  • npm run build
  • git diff --check
  • Manual global install and devspace doctor on Windows confirmed Shell mode: auto resolves to native PowerShell.

Summary by CodeRabbit

  • New Features
    • Added configurable shell execution across platforms, including Windows native PowerShell, cmd.exe, and Bash-like environments.
    • Introduced DEVSPACE_SHELL (auto, bash, powershell, cmd), with Windows defaulting to PowerShell.
  • Bug Fixes
    • Improved shell tool execution with clearer output and more reliable timeout handling.
    • Added protections for fragile PowerShell matching patterns involving literal Windows paths.
  • Documentation
    • Updated platform support, configuration docs, and Windows shell troubleshooting; enhanced doctor to display the active shell mode and command.
  • Tests
    • Added dedicated shell tool tests and updated the test script to run them.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds configurable shell modes for Bash, PowerShell, and cmd, with native non-Bash execution, PowerShell safeguards, server integration, diagnostics, tests, and updated platform documentation.

Changes

Configurable shell execution

Layer / File(s) Summary
Shell mode configuration
src/config.ts, src/config.test.ts
Adds validated shell-mode configuration, defaults DEVSPACE_SHELL to auto, and tests supported and invalid values.
Native shell resolution and execution
src/pi-tools.ts, src/pi-tools.test.ts, package.json
Adds native shell resolution and execution with output aggregation, timeout and error handling, PowerShell fragile-pattern blocking, and cross-platform tests.
Server and diagnostic integration
src/server.ts, src/cli.ts
Passes configured shell modes to the shell tool, updates tool guidance and labels, and reports shell mode and command through doctor.
Platform and shell documentation
README.md, docs/configuration.md, docs/gotchas.md
Documents supported shell modes, Windows defaults, configuration values, diagnostics, and PowerShell path-matching behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant MCPServer
  participant runShellTool
  participant NativeShellProcess
  MCPServer->>runShellTool: pass configured shell mode and command
  runShellTool->>NativeShellProcess: execute resolved native shell
  NativeShellProcess-->>runShellTool: return stdout, stderr, or exit status
  runShellTool-->>MCPServer: return tool response
Loading

Possibly related PRs

Poem

I’m a rabbit with shells in a row,
PowerShell, Bash, and cmd all go.
Commands hop safely, timeouts stay tight,
Doctor sheds light on the setting just right.
Documentation blooms—what a delightful byte!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding native Windows shell support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Waishnav

Waishnav commented Jul 8, 2026

Copy link
Copy Markdown
Owner

@yurilopes i've no idea how openai have supported windows on codex CLI on windows, do it supports through custom powershell tool or smth?

@yurilopes

Copy link
Copy Markdown
Author

@yurilopes i've no idea how openai have supported windows on codex CLI on windows, do it supports through custom powershell tool or smth?

Yes, it does. It supports PowerShell and cmd.exe syntax. PowerShell syntax is the preferred one. I'm a heavy Windows user and have been using the changes I proposed with great results. It's seamless, just working fine with PowerShell.

…ell-shell-support

# Conflicts:
#	package.json
#	src/cli.ts
#	src/config.test.ts
#	src/config.ts
#	src/server.ts
@yurilopes yurilopes marked this pull request as ready for review July 13, 2026 16:38
@yurilopes

yurilopes commented Jul 13, 2026

Copy link
Copy Markdown
Author

Updated this branch against the current main and resolved the merge conflicts. The PR is now ready for review and GitHub reports it as mergeable.

Local validation after conflict resolution:

  • npm test
  • npm run typechec
  • npm run build
  • git diff --check

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli.ts`:
- Around line 688-695: Update checkShellCommand to use the same runnable shell
resolver as the runtime execution path instead of resolveShellCommand, ensuring
Windows diagnostics report the pwsh fallback when powershell.exe is unavailable.
Preserve the existing bash handling and command/argument formatting.

In `@src/pi-tools.ts`:
- Around line 141-171: The quoted-literal regexes in
findUnsafePowerShellRegexLiteral and unsafePowerShellRegexVariableAssignments
can exhibit catastrophic backtracking on malformed backslash-heavy input.
Replace unsafeMatchPattern and assignmentPattern with a linear scan/parser that
finds PowerShell quoted literals, handles escapes and unterminated quotes
without backtracking, and preserves the existing path-literal detection and
variable-assignment behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5580732e-6d21-40f0-947a-a85cd1d6228e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ccefbf and 5a853e0.

📒 Files selected for processing (10)
  • README.md
  • docs/configuration.md
  • docs/gotchas.md
  • package.json
  • src/cli.ts
  • src/config.test.ts
  • src/config.ts
  • src/pi-tools.test.ts
  • src/pi-tools.ts
  • src/server.ts

Comment thread src/cli.ts
Comment thread src/pi-tools.ts
…ell-shell-support

# Conflicts:
#	package.json
@yurilopes

yurilopes commented Jul 14, 2026

Copy link
Copy Markdown
Author

Updated against current main and addressed the review feedback.

Changes in this update:

  • resolved the merge conflict with the latest upstream UI changes.
  • changed doctor to use the same runnable shell resolver as execution, including pwsh fallback.
  • replaced the PowerShell -match guardrail regex parsing with a linear scanner for quoted literals and variables.

Validation passed locally:

  • npm test
  • npm run typecheck
  • npm run build
  • git diff --check

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/pi-tools.ts`:
- Around line 268-275: Update isPowerShellMatchOperatorAt to recognize -match,
-imatch, -cmatch, -notmatch, -inotmatch, and -cnotmatch with token-boundary
validation. Change it to return the matched operator length (or an equivalent
non-match result), and update its caller to use that returned length instead of
hardcoded "-match".length.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4ef110ad-8e10-4b03-b8fc-60f6b7393791

📥 Commits

Reviewing files that changed from the base of the PR and between 5a853e0 and 8348b95.

📒 Files selected for processing (4)
  • package.json
  • src/cli.ts
  • src/pi-tools.ts
  • src/server.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/server.ts
  • package.json
  • src/cli.ts

Comment thread src/pi-tools.ts
Comment on lines +268 to +275
function isPowerShellMatchOperatorAt(command: string, index: number): boolean {
if (command.slice(index, index + "-match".length).toLowerCase() !== "-match") return false;
return isTokenBoundary(command[index - 1]) && isTokenBoundary(command[index + "-match".length]);
}

function isTokenBoundary(char: string | undefined): boolean {
return char === undefined || /\s/.test(char) || "|;&({[,".includes(char);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

PowerShell comparison operators -imatch -cmatch -notmatch regex

💡 Result:

PowerShell uses specific operators to handle regular expressions (regex). The primary regex-based comparison operators are -match, -notmatch, and -replace [1][2][3]. Key Regex Operators: -match: Returns TRUE if the input string matches the specified regex pattern, and FALSE otherwise [4]. When used with a string, it automatically populates the $matches automatic variable with the results of the match [5]. -notmatch: The logical opposite of -match; returns TRUE if the input string does not match the regex pattern [1][4]. -replace: Uses a regex pattern to find text and replace it with a specified substitution string [6][7]. Case Sensitivity: Like other PowerShell operators, regex operators have case-sensitive and case-insensitive variants [8]: Default (Case-Insensitive): -match, -notmatch, -replace [4]. Case-Sensitive: -cmatch, -cnotmatch, -creplace [8]. Explicitly Case-Insensitive (Optional): -imatch, -inotmatch, -ireplace [8]. Important Considerations: Regex vs. Wildcards: Do not confuse these with wildcard operators like -like and -notlike, which use simple wildcards (*,?) rather than full regex patterns [1][2]. $matches Variable: When -match is successful, PowerShell creates an automatic variable named $matches that contains the captured groups from the regex [5]. Handling $ in -replace: Because -replace uses $ for group back-references (e.g., $1, $2, $&) and PowerShell double-quoted strings also use $ for variable expansion, you must be careful with escaping [6][5]. Use single-quoted strings for replacement patterns (e.g., -replace 'regex', '$1') or escape the $ character with a backtick (`) when using double quotes to prevent premature interpolation [6][5][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant source around the reported lines and find all related usages.
sed -n '120,190p' src/pi-tools.ts
printf '\n---\n'
sed -n '240,310p' src/pi-tools.ts
printf '\n---\n'
rg -n "isPowerShellMatchOperatorAt|skipWhitespace\\(command, index \\+|\\-match" src/pi-tools.ts

Repository: Waishnav/devspace

Length of output: 5654


Guardrail should recognize PowerShell regex operator variants. isPowerShellMatchOperatorAt only matches -match, so -imatch, -cmatch, -notmatch, -inotmatch, and -cnotmatch bypass the path guardrail. The caller also hardcodes "-match".length, so the helper needs to return the matched operator length.

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 4: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import {
spawn,
spawnSync,
type ChildProcess,
} from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pi-tools.ts` around lines 268 - 275, Update isPowerShellMatchOperatorAt
to recognize -match, -imatch, -cmatch, -notmatch, -inotmatch, and -cnotmatch
with token-boundary validation. Change it to return the matched operator length
(or an equivalent non-match result), and update its caller to use that returned
length instead of hardcoded "-match".length.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants