Repository navigation
Refactor Codex command execution to support WSL integration - #385
Rana-Faraz wants to merge 3 commits into
Conversation
- Introduced `resolveCodexSpawnConfig` to handle command and argument resolution for Codex in WSL environments. - Updated `runCodexCommand` to utilize WSL when necessary, improving compatibility on Windows. - Enhanced terminal management to use `wsl.exe` for WSL workspace terminals. - Refactored shell command normalization and candidate resolution to accommodate platform-specific behavior. - Added tests to verify WSL fallback functionality and command execution across different platforms.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Refactor Codex command execution to support WSL integration
| @@ -223,8 +244,8 @@ const makeCodexTextGeneration = Effect.gen(function* () { | |||
| "-", | |||
There was a problem hiding this comment.
🔴 Critical Layers/CodexTextGeneration.ts:244
The fallback Windows execution path sets shell: false unconditionally, but shell: true is required on Windows to spawn npm-installed CLI tools (shims like codex.cmd). When wslLaunch is null (non-WSL path), spawn("codex", { shell: false }) fails with ENOENT because the .cmd extension cannot be resolved without a shell.
Also found in 2 other location(s)
apps/server/src/codexAppServerManager.ts:590
The
spawncall now unconditionally usesshell: false, replacing the previous logicshell: process.platform === "win32". This causes a regression for standard Windows usage (non-WSL) where thecodexbinary is a script (e.g.,codex.cmdorcodex.batinstalled via npm) rather than a direct.exe. Withoutshell: true,spawnon Windows will fail to execute these scripts when relying on PATH resolution. The code should conditionally setshellto true for the native Windows fallback path, while keeping it false for the WSL execution path.
apps/server/src/provider/Layers/ProviderHealth.ts:184
The
runCommandfunction hardcodesshell: false, removing the Windows-specific logic (shell: process.platform === "win32") that was present in the previousrunCodexCommandimplementation. On Windows, executing CLI tools installed via npm or other package managers typically requires a shell (or explicitly appending.cmd) because they are wrapper scripts/batch files, not direct executables. This regression will likely cause the nativecodexhealth check to fail withENOENTon Windows, incorrectly forcing a fallback to WSL or failing entirely if WSL is unavailable.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/git/Layers/CodexTextGeneration.ts around line 244:
The fallback Windows execution path sets `shell: false` unconditionally, but `shell: true` is required on Windows to spawn npm-installed CLI tools (shims like `codex.cmd`). When `wslLaunch` is null (non-WSL path), `spawn("codex", { shell: false })` fails with `ENOENT` because the `.cmd` extension cannot be resolved without a shell.
Evidence trail:
1. apps/server/src/git/Layers/CodexTextGeneration.ts lines 228-249: shows `ChildProcess.make(wslLaunch?.command ?? "codex", ..., { shell: false })` - when wslLaunch is null, uses "codex" with shell:false
2. apps/server/src/wsl.ts lines 164-169: `resolveWorkspaceCommandLaunch` returns null when `parseWslWorkspacePath` returns null
3. apps/server/src/wsl.test.ts line 31: `parseWslWorkspacePath("C:\\repo", "win32")` returns null - confirms regular Windows paths don't return WSL config
4. https://nodejs.org/api/child_process.html - "On Windows, however, `.bat` and `.cmd` files are not executable on their own without a terminal, and therefore cannot be launched using `child_process.execFile()`. When running on Windows, `.bat` and `.cmd` files can be invoked using `child_process.spawn()` with the `shell` option set"
5. apps/server/src/codexAppServerManager.ts line 591: same pattern with native spawn() and shell: false
Also found in 2 other location(s):
- apps/server/src/codexAppServerManager.ts:590 -- The `spawn` call now unconditionally uses `shell: false`, replacing the previous logic `shell: process.platform === "win32"`. This causes a regression for standard Windows usage (non-WSL) where the `codex` binary is a script (e.g., `codex.cmd` or `codex.bat` installed via npm) rather than a direct `.exe`. Without `shell: true`, `spawn` on Windows will fail to execute these scripts when relying on PATH resolution. The code should conditionally set `shell` to true for the native Windows fallback path, while keeping it false for the WSL execution path.
- apps/server/src/provider/Layers/ProviderHealth.ts:184 -- The `runCommand` function hardcodes `shell: false`, removing the Windows-specific logic (`shell: process.platform === "win32"`) that was present in the previous `runCodexCommand` implementation. On Windows, executing CLI tools installed via npm or other package managers typically requires a shell (or explicitly appending `.cmd`) because they are wrapper scripts/batch files, not direct executables. This regression will likely cause the native `codex` health check to fail with `ENOENT` on Windows, incorrectly forcing a fallback to WSL or failing entirely if WSL is unavailable.
- Removed the `publish_cli` job from the release workflow to streamline the process. - Introduced new functions for building WSL command execution arguments, improving command resolution and execution in WSL environments. - Added tests for the new WSL command execution logic to ensure proper functionality across different scenarios.
resolveCodexSpawnConfigto handle command and argument resolution for Codex in WSL environments.runCodexCommandto utilize WSL when necessary, improving compatibility on Windows.wsl.exefor WSL workspace terminals.Note
Refactor server WSL launch flow and modify
CodexAppServerManager.startSession,git.Layers.CodexTextGeneration.makeCodexTextGeneration, and terminal runtime to execute Codex and shells viawsl.exefor WSL workspacesAdd WSL-aware command and shell launch utilities, switch Codex CLI and app-server spawns to
wsl.exewhen the workspace is a WSL path, translate paths to Linux formats, and remove thepublish_clijob from the release workflow.📍Where to Start
Start with the WSL utilities in wsl.ts and then follow their use in
resolveCodexSpawnConfigwithin codexAppServerManager.ts and terminal candidate resolution in Manager.ts.Macroscope summarized dbd62bf.