From 3a71175236cae47a2950266a0d7888e4f76227c4 Mon Sep 17 00:00:00 2001 From: Alpha Diop <90140491+alphajoop@users.noreply.github.com> Date: Thu, 17 Apr 2025 18:31:19 +0000 Subject: [PATCH] fix: improve Windows compatibility for CLI commands and sandbox (#261) ## Fix Windows compatibility issues (#248) This PR addresses the Windows compatibility issues reported in #248: 1. **Fix sandbox initialization failure on Windows** - Modified `getSandbox()` to return `SandboxType.NONE` on Windows instead of throwing an error - Added a warning log message to inform the user that sandbox is not available on Windows 2. **Fix Unix commands not working on Windows** - Created a new module [platform-commands.ts](cci:7://file:///c:/Users/HP%20840%20G6/workflow/codex/codex-cli/src/utils/agent/platform-commands.ts:0:0-0:0) that automatically adapts Unix commands to their Windows equivalents - Implemented a mapping table for common commands and their options - Integrated this functionality into the command execution process ### Testing Tested on Windows 10 with the following commands: - `ls -R .` (now automatically translates to `dir /s .`) - Other Unix commands like `grep`, `cat`, etc. The CLI no longer crashes when running these commands on Windows. I have read the CLA Document and I hereby sign the CLA --------- Signed-off-by: Alpha Diop --- .../src/utils/agent/handle-exec-command.ts | 8 ++ .../src/utils/agent/platform-commands.ts | 86 +++++++++++++++++++ codex-cli/src/utils/agent/sandbox/raw-exec.ts | 19 +++- 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 codex-cli/src/utils/agent/platform-commands.ts diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 93e8ea52..48f6f954 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -272,7 +272,15 @@ async function getSandbox(runInSandbox: boolean): Promise { return SandboxType.MACOS_SEATBELT; } else if (await isInLinux()) { return SandboxType.NONE; + } else if (process.platform === "win32") { + // On Windows, we don't have a sandbox implementation yet, so we fall back to NONE + // instead of throwing an error, which would crash the application + log( + "WARNING: Sandbox was requested but is not available on Windows. Continuing without sandbox.", + ); + return SandboxType.NONE; } + // For other platforms, still throw an error as before throw new Error("Sandbox was mandated, but no sandbox is available!"); } else { return SandboxType.NONE; diff --git a/codex-cli/src/utils/agent/platform-commands.ts b/codex-cli/src/utils/agent/platform-commands.ts new file mode 100644 index 00000000..7be02c7a --- /dev/null +++ b/codex-cli/src/utils/agent/platform-commands.ts @@ -0,0 +1,86 @@ +/** + * Utility functions for handling platform-specific commands + */ + +import { log, isLoggingEnabled } from "./log.js"; + +/** + * Map of Unix commands to their Windows equivalents + */ +const COMMAND_MAP: Record = { + ls: "dir", + grep: "findstr", + cat: "type", + rm: "del", + cp: "copy", + mv: "move", + touch: "echo.>", + mkdir: "md", +}; + +/** + * Map of common Unix command options to their Windows equivalents + */ +const OPTION_MAP: Record> = { + ls: { + "-l": "/p", + "-a": "/a", + "-R": "/s", + }, + grep: { + "-i": "/i", + "-r": "/s", + }, +}; + +/** + * Adapts a command for the current platform. + * On Windows, this will translate Unix commands to their Windows equivalents. + * On Unix-like systems, this will return the original command. + * + * @param command The command array to adapt + * @returns The adapted command array + */ +export function adaptCommandForPlatform(command: Array): Array { + // If not on Windows, return the original command + if (process.platform !== "win32") { + return command; + } + + // Nothing to adapt if the command is empty + if (command.length === 0) { + return command; + } + + const cmd = command[0]; + + // If cmd is undefined or the command doesn't need adaptation, return it as is + if (!cmd || !COMMAND_MAP[cmd]) { + return command; + } + + if (isLoggingEnabled()) { + log(`Adapting command '${cmd}' for Windows platform`); + } + + // Create a new command array with the adapted command + const adaptedCommand = [...command]; + adaptedCommand[0] = COMMAND_MAP[cmd]; + + // Adapt options if needed + const optionsForCmd = OPTION_MAP[cmd]; + if (optionsForCmd) { + for (let i = 1; i < adaptedCommand.length; i++) { + const option = adaptedCommand[i]; + if (option && optionsForCmd[option]) { + adaptedCommand[i] = optionsForCmd[option]; + } + } + } + + if (isLoggingEnabled()) { + log(`Adapted command: ${adaptedCommand.join(" ")}`); + } + + return adaptedCommand; +} diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts index c0dd7c91..6cfb3047 100644 --- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts +++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts @@ -8,6 +8,7 @@ import type { } from "child_process"; import { log, isLoggingEnabled } from "../log.js"; +import { adaptCommandForPlatform } from "../platform-commands.js"; import { spawn } from "child_process"; import * as os from "os"; @@ -23,7 +24,21 @@ export function exec( _writableRoots: Array, abortSignal?: AbortSignal, ): Promise { - const prog = command[0]; + // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows) + const adaptedCommand = adaptCommandForPlatform(command); + + if ( + isLoggingEnabled() && + JSON.stringify(adaptedCommand) !== JSON.stringify(command) + ) { + log( + `Command adapted for platform: ${command.join( + " ", + )} -> ${adaptedCommand.join(" ")}`, + ); + } + + const prog = adaptedCommand[0]; if (typeof prog !== "string") { return Promise.resolve({ stdout: "", @@ -72,7 +87,7 @@ export function exec( detached: true, }; - const child: ChildProcess = spawn(prog, command.slice(1), fullOptions); + const child: ChildProcess = spawn(prog, adaptedCommand.slice(1), fullOptions); // If an AbortSignal is provided, ensure the spawned process is terminated // when the signal is triggered so that cancellations propagate down to any // long‑running child processes. We default to SIGTERM to give the process a