fix(windows): improve argument escaping and error detection

src/__tests__/windows-escape.test.ts

@@ -0,0 +1,119 @@
+import { escapeArgForWindowsShell } from '../utils/escape.js';
+
+describe('escapeArgForWindowsShell', () => {
+  describe('basic arguments', () => {
+    test('should not modify simple arguments without special chars', () => {
+      expect(escapeArgForWindowsShell('hello')).toBe('hello');
+      expect(escapeArgForWindowsShell('--flag')).toBe('--flag');
+      expect(escapeArgForWindowsShell('-x')).toBe('-x');
+      expect(escapeArgForWindowsShell('filename.txt')).toBe('filename.txt');
+    });
+
+    test('should wrap arguments with spaces in double quotes', () => {
+      expect(escapeArgForWindowsShell('hello world')).toBe('"hello world"');
+      expect(escapeArgForWindowsShell('path with spaces')).toBe(
+        '"path with spaces"'
+      );
+    });
+  });
+
+  describe('percent signs', () => {
+    test('should escape percent signs to prevent env var expansion', () => {
+      expect(escapeArgForWindowsShell('100%')).toBe('100%%');
+      expect(escapeArgForWindowsShell('%PATH%')).toBe('%%PATH%%');
+      expect(escapeArgForWindowsShell('50% off')).toBe('"50%% off"');
+    });
+  });
+
+  describe('newlines', () => {
+    test('should replace newlines with spaces', () => {
+      expect(escapeArgForWindowsShell('line1\nline2')).toBe('"line1 line2"');
+      expect(escapeArgForWindowsShell('line1\r\nline2')).toBe('"line1 line2"');
+      expect(escapeArgForWindowsShell('a\n\n\nb')).toBe('"a b"');
+    });
+
+    test('should handle multiline prompts', () => {
+      const multilinePrompt = 'First line\nSecond line\nThird line';
+      const result = escapeArgForWindowsShell(multilinePrompt);
+      expect(result).toBe('"First line Second line Third line"');
+      expect(result).not.toContain('\n');
+    });
+  });
+
+  describe('quotes', () => {
+    test('should escape quotes with caret when no spaces present', () => {
+      // In practice, -c and model="gpt-5" are separate args in the array
+      // So we test model="gpt-5" alone (no spaces)
+      expect(escapeArgForWindowsShell('model="gpt-5"')).toBe('model=^"gpt-5^"');
+      expect(escapeArgForWindowsShell('key="value"')).toBe('key=^"value^"');
+      expect(escapeArgForWindowsShell('"quoted"')).toBe('^"quoted^"');
+    });
+
+    test('should double quotes when wrapped in outer quotes (has spaces)', () => {
+      // When arg has spaces AND quotes, wrap in quotes and double internal quotes
+      expect(escapeArgForWindowsShell('say "hello"')).toBe('"say ""hello"""');
+      expect(escapeArgForWindowsShell('text with "quotes" inside')).toBe(
+        '"text with ""quotes"" inside"'
+      );
+      // This is a single arg with space - gets wrapped
+      expect(escapeArgForWindowsShell('-c model="gpt-5"')).toBe(
+        '"-c model=""gpt-5"""'
+      );
+    });
+  });
+
+  describe('special shell characters', () => {
+    test('should wrap args with shell metacharacters in quotes', () => {
+      expect(escapeArgForWindowsShell('a&b')).toBe('"a&b"');
+      expect(escapeArgForWindowsShell('a|b')).toBe('"a|b"');
+      expect(escapeArgForWindowsShell('a<b')).toBe('"a<b"');
+      expect(escapeArgForWindowsShell('a>b')).toBe('"a>b"');
+      expect(escapeArgForWindowsShell('a^b')).toBe('"a^b"');
+    });
+  });
+
+  describe('complex real-world cases', () => {
+    test('should handle codex config arguments', () => {
+      // This is a common pattern: -c key="value"
+      const result = escapeArgForWindowsShell('model="gpt-5.2-codex"');
+      expect(result).toBe('model=^"gpt-5.2-codex^"');
+    });
+
+    test('should handle prompts with special characters', () => {
+      const prompt = 'What is 50% of 100?';
+      const result = escapeArgForWindowsShell(prompt);
+      expect(result).toBe('"What is 50%% of 100?"');
+    });
+
+    test('should handle JSON-like arguments', () => {
+      const jsonArg = '{"key":"value"}';
+      const result = escapeArgForWindowsShell(jsonArg);
+      expect(result).toBe('{^"key^":^"value^"}');
+    });
+
+    test('should handle paths with spaces', () => {
+      const path = 'C:\\Program Files\\App\\file.exe';
+      const result = escapeArgForWindowsShell(path);
+      expect(result).toBe('"C:\\Program Files\\App\\file.exe"');
+    });
+
+    test('should handle empty string', () => {
+      expect(escapeArgForWindowsShell('')).toBe('');
+    });
+  });
+
+  describe('combined escaping', () => {
+    test('should handle multiple escape requirements', () => {
+      // Spaces + percent
+      expect(escapeArgForWindowsShell('100% complete')).toBe('"100%% complete"');
+
+      // Newline + spaces
+      expect(escapeArgForWindowsShell('line1\nwith spaces')).toBe(
+        '"line1 with spaces"'
+      );
+
+      // Percent + quotes (no spaces)
+      expect(escapeArgForWindowsShell('rate=100%')).toBe('rate=100%%');
+    });
+  });
+});

src/errors.ts

@@ -32,7 +32,29 @@ export class ValidationError extends Error {
 
 export function handleError(error: unknown, context: string): string {
   if (error instanceof Error) {
-    return `Error in ${context}: ${error.message}`;
+    const messages: string[] = [error.message];
+
+    // Traverse the cause chain to get root cause details
+    let current: unknown = error.cause;
+    while (current) {
+      if (current instanceof Error) {
+        messages.push(current.message);
+        current = current.cause;
+      } else if (typeof current === 'string') {
+        messages.push(current);
+        break;
+      } else {
+        messages.push(String(current));
+        break;
+      }
+    }
+
+    // Deduplicate consecutive identical messages
+    const uniqueMessages = messages.filter(
+      (msg, idx) => idx === 0 || msg !== messages[idx - 1]
+    );
+
+    return `Error in ${context}: ${uniqueMessages.join(' - Caused by: ')}`;
   }
   return `Error in ${context}: ${String(error)}`;
 }

src/utils/command.ts

@@ -3,20 +3,7 @@ import { Buffer } from 'node:buffer';
 import chalk from 'chalk';
 import { CommandExecutionError } from '../errors.js';
 import { type CommandResult } from '../types.js';
-
-/**
- * Escape argument for Windows shell (cmd.exe)
- */
-function escapeArgForWindows(arg: string): string {
-  // Escape percent signs to prevent environment variable expansion
-  let escaped = arg.replace(/%/g, '%%');
-  // If arg contains spaces or special chars, wrap in double quotes
-  if (/[\s"&|<>^%]/.test(arg)) {
-    // Escape internal double quotes using CMD-style doubling
-    escaped = `"${escaped.replace(/"/g, '""')}"`;
-  }
-  return escaped;
-}
+import { escapeArgForWindowsShell } from './escape.js';
 
 const isWindows = process.platform === 'win32';
 
@@ -34,12 +21,14 @@ export async function executeCommand(
   args: string[] = []
 ): Promise<CommandResult> {
   return new Promise((resolve, reject) => {
-    // Escape args for Windows shell
-    const escapedArgs = isWindows ? args.map(escapeArgForWindows) : args;
+    // Escape args for Windows shell to prevent injection
+    // Note: file is not escaped - it should be a simple command name (e.g., "codex", "node")
+    const escapedArgs = isWindows ? args.map(escapeArgForWindowsShell) : args;
 
     console.error(chalk.blue('Executing:'), file, escapedArgs.join(' '));
 
     const child = spawn(file, escapedArgs, {
+      // Use shell on Windows to resolve .cmd/.bat executables (e.g., codex.cmd)
       shell: isWindows,
       env: process.env,
       stdio: ['pipe', 'pipe', 'pipe'],
@@ -118,18 +107,19 @@ export async function executeCommand(
  * Execute a command with streaming output support.
  * Calls onProgress callback with each chunk of output for real-time feedback.
  *
- * Note: Unlike executeCommand, this function treats stderr output as success
- * because tools like codex write their primary output to stderr. This is
- * intentional for streaming use cases where we want to capture all output.
+ * Note: This function accepts exit code 0 with any output (stdout or stderr),
+ * since tools like codex write their primary output to stderr. However, non-zero
+ * exit codes are only accepted if there's stdout output, to avoid masking errors.
  */
 export async function executeCommandStreaming(
   file: string,
   args: string[] = [],
   options: StreamingCommandOptions = {}
 ): Promise<CommandResult> {
   return new Promise((resolve, reject) => {
-    // Escape args for Windows shell
-    const escapedArgs = isWindows ? args.map(escapeArgForWindows) : args;
+    // Escape args for Windows shell to prevent injection
+    // Note: file is not escaped - it should be a simple command name (e.g., "codex", "node")
+    const escapedArgs = isWindows ? args.map(escapeArgForWindowsShell) : args;
 
     console.error(
       chalk.blue('Executing (streaming):'),
@@ -138,7 +128,8 @@ export async function executeCommandStreaming(
     );
 
     const child = spawn(file, escapedArgs, {
-      shell: isWindows, // Use shell on Windows to inherit PATH correctly
+      // Use shell on Windows to resolve .cmd/.bat executables (e.g., codex.cmd)
+      shell: isWindows,
       env: process.env,
       stdio: ['pipe', 'pipe', 'pipe'],
     });
@@ -202,11 +193,13 @@ export async function executeCommandStreaming(
         }
       }
 
-      if (code === 0 || stdout || stderr) {
-        // Success or we have output (treat as success like the original)
-        if (code !== 0 && (stdout || stderr)) {
+      // Accept exit code 0, or non-zero with stdout output.
+      // Do NOT accept non-zero with only stderr - this would mask ENOENT errors
+      // when shell: true is used (command not found produces stderr, not 'error' event)
+      if (code === 0 || stdout) {
+        if (code !== 0 && stdout) {
           console.error(
-            chalk.yellow('Command failed but produced output, using output')
+            chalk.yellow('Command failed but produced stdout, using output')
           );
         }
         resolve({ stdout, stderr });
@@ -215,7 +208,7 @@ export async function executeCommandStreaming(
           new CommandExecutionError(
             [file, ...args].join(' '),
             `Command exited with code ${code}`,
-            new Error(`Exit code: ${code}`)
+            new Error(stderr || `Exit code: ${code}`)
           )
         );
       }

src/utils/escape.ts

@@ -0,0 +1,35 @@
+/**
+ * Escape an argument for Windows shell (cmd.exe) to prevent injection attacks.
+ * Note: This is only used for arguments, not for the command itself.
+ * The command (file) should be a simple executable name without special characters.
+ *
+ * Handles:
+ * - Newlines: replaced with spaces (cmd.exe treats \n as command separator)
+ * - Percent signs: escaped as %% to prevent env var expansion
+ * - Arguments with spaces/special chars: wrapped in double quotes
+ * - Internal quotes: doubled ("") per cmd.exe convention
+ *
+ * @param arg - The argument to escape
+ * @returns The escaped argument safe for cmd.exe
+ */
+export function escapeArgForWindowsShell(arg: string): string {
+  // Replace newlines with spaces - cmd.exe interprets \n as command separator
+  let escaped = arg.replace(/[\r\n]+/g, ' ');
+
+  // Escape percent signs to prevent environment variable expansion
+  escaped = escaped.replace(/%/g, '%%');
+
+  // Check if we need to wrap in quotes (has spaces or special shell chars)
+  // but not if it's already a simple flag like --flag or -x
+  const needsQuotes = /[\s&|<>^]/.test(escaped);
+
+  if (needsQuotes) {
+    // Escape internal double quotes using CMD-style doubling
+    escaped = `"${escaped.replace(/"/g, '""')}"`;
+  } else if (escaped.includes('"')) {
+    // Has quotes but no spaces - escape quotes with caret for cmd.exe
+    escaped = escaped.replace(/"/g, '^"');
+  }
+
+  return escaped;
+}