[codex] Add native Windows shell support#41
Conversation
📝 WalkthroughWalkthroughAdds configurable shell modes for Bash, PowerShell, and cmd, with native non-Bash execution, PowerShell safeguards, server integration, diagnostics, tests, and updated platform documentation. ChangesConfigurable shell execution
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
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@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
|
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:
|
There was a problem hiding this comment.
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
📒 Files selected for processing (10)
README.mddocs/configuration.mddocs/gotchas.mdpackage.jsonsrc/cli.tssrc/config.test.tssrc/config.tssrc/pi-tools.test.tssrc/pi-tools.tssrc/server.ts
…ell-shell-support # Conflicts: # package.json
|
Updated against current main and addressed the review feedback. Changes in this update:
Validation passed locally:
|
There was a problem hiding this comment.
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
📒 Files selected for processing (4)
package.jsonsrc/cli.tssrc/pi-tools.tssrc/server.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/server.ts
- package.json
- src/cli.ts
| 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); | ||
| } |
There was a problem hiding this comment.
🎯 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,
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_comparison_operators?view=powershell-7.6
- 2: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_operators?view=powershell-7.6
- 3: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_operators?view=powershell-7.5
- 4: https://www.powershelltips.com/powershell-comparison-operators/
- 5: https://www.regular-expressions.info/powershell.html
- 6: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_regular_expressions?view=powershell-7.6
- 7: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_regular_expressions?view=powershell-7.5
- 8: https://learn.microsoft.com/en-us/powershell/scripting/lang-spec/chapter-07?view=powershell-7.5
🏁 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.tsRepository: 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.
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.
DEVSPACE_SHELL=auto|bash|powershell|cmd.cmd.exesupport viaDEVSPACE_SHELL=cmd.DEVSPACE_SHELL=bashand non-Windows defaults.-matchusage with raw Windows path literals, including literals assigned to variables and then passed to-match.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
$_.CommandLinebefore PowerShell received it. Native PowerShell execution fixes that transport issue.A second common failure mode remained: using PowerShell
-matchfor literal Windows path fragments.-matchis regex-based, so path fragments like\profilescan 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 testnpm run typechecknpm run buildgit diff --checkdevspace doctoron Windows confirmedShell mode: autoresolves to native PowerShell.Summary by CodeRabbit
cmd.exe, and Bash-like environments.DEVSPACE_SHELL(auto,bash,powershell,cmd), with Windows defaulting to PowerShell.doctorto display the active shell mode and command.