From ccdd0b6e8e4185ec2b01186cfc69e28f171d0906 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 19 Apr 2026 14:27:27 -0400 Subject: [PATCH] fix(windows): align escaped wrapper shell handling Use ComSpec-aware shell selection for escaped wrapper launches. Apply it to the remaining Windows Claude launch paths. Rewrite the Codex exec regression test to use scoped spies so the full suite stays isolated. --- src/auth/commands/create-command.ts | 8 +- src/cliproxy/executor/index.ts | 4 +- src/utils/claude-spawner.ts | 8 +- src/utils/shell-executor.ts | 16 ++-- tests/unit/targets/codex-adapter-exec.test.ts | 95 +++++++++---------- tests/unit/utils/shell-executor.test.ts | 50 ++++++++++ 6 files changed, 117 insertions(+), 64 deletions(-) diff --git a/src/auth/commands/create-command.ts b/src/auth/commands/create-command.ts index ef59fb60..c70b2c6c 100644 --- a/src/auth/commands/create-command.ts +++ b/src/auth/commands/create-command.ts @@ -7,7 +7,11 @@ import { spawn, ChildProcess } from 'child_process'; import { initUI, header, color, fail, warn, info, infoBox, warnBox } from '../../utils/ui'; import { getClaudeCliInfo } from '../../utils/claude-detector'; -import { escapeShellArg, stripClaudeCodeEnv } from '../../utils/shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripClaudeCodeEnv, +} from '../../utils/shell-executor'; import { isUnifiedMode } from '../../config/unified-config-loader'; import { ProfileMetadata } from '../../types'; import { @@ -249,7 +253,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise child = spawn(cmdString, { stdio: 'inherit', windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: childEnv, }); } else { diff --git a/src/cliproxy/executor/index.ts b/src/cliproxy/executor/index.ts index d02a12f5..3f6d1d11 100644 --- a/src/cliproxy/executor/index.ts +++ b/src/cliproxy/executor/index.ts @@ -17,7 +17,7 @@ import * as path from 'path'; import { ProgressIndicator } from '../../utils/progress-indicator'; import { ok, fail, info, warn } from '../../utils/ui'; import { getCcsDir } from '../../utils/config-manager'; -import { escapeShellArg } from '../../utils/shell-executor'; +import { escapeShellArg, getWindowsEscapedCommandShell } from '../../utils/shell-executor'; import { ensureCLIProxyBinary } from '../binary-manager'; import { generateConfig, @@ -1316,7 +1316,7 @@ export async function execClaudeWithCLIProxy( claude = spawn(cmdString, { stdio: 'inherit', windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: tracedEnv, }); } else { diff --git a/src/utils/claude-spawner.ts b/src/utils/claude-spawner.ts index 9f82ff8b..67f9a527 100644 --- a/src/utils/claude-spawner.ts +++ b/src/utils/claude-spawner.ts @@ -6,7 +6,11 @@ */ import { spawn, ChildProcess, SpawnOptions } from 'child_process'; -import { escapeShellArg, stripClaudeCodeEnv } from './shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripClaudeCodeEnv, +} from './shell-executor'; import { getClaudeCliInfo } from './claude-detector'; import { ErrorManager } from './error-manager'; @@ -56,7 +60,7 @@ export function spawnClaude(options: SpawnClaudeOptions = {}): SpawnClaudeResult child = spawn(cmdString, { stdio, windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: mergedEnv, cwd, }); diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 191c004a..b6f26ab2 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -4,7 +4,7 @@ * Cross-platform shell execution utilities for CCS. */ -import { spawn, spawnSync, ChildProcess } from 'child_process'; +import { spawn, spawnSync, ChildProcess, type SpawnOptions } from 'child_process'; import { ErrorManager } from './error-manager'; import { getWebSearchHookEnv } from './websearch-manager'; import { wireChildProcessSignals } from './signal-forwarder'; @@ -108,13 +108,17 @@ export function escapeShellArg(arg: string): string { } /** - * Return the Windows shell that matches escapeShellArg() quoting semantics. + * Return the shell that matches escapeShellArg() quoting semantics. * - * `shell: true` is not strict enough for npm `.cmd` wrappers because Node may - * route through a different quoting path than the one escapeShellArg() expects. + * On Windows, prefer ComSpec over a bare `cmd.exe` so escaped wrapper launches + * keep the same shell contract without depending on PATH lookup. */ -export function getWindowsEscapedCommandShell(): string { - return 'cmd.exe'; +export function getWindowsEscapedCommandShell(): SpawnOptions['shell'] { + if (process.platform !== 'win32') { + return true; + } + + return process.env.ComSpec || process.env.COMSPEC || 'cmd.exe'; } /** diff --git a/tests/unit/targets/codex-adapter-exec.test.ts b/tests/unit/targets/codex-adapter-exec.test.ts index cf8017c8..d5166e03 100644 --- a/tests/unit/targets/codex-adapter-exec.test.ts +++ b/tests/unit/targets/codex-adapter-exec.test.ts @@ -1,16 +1,13 @@ -import { afterEach, beforeEach, describe, expect, it, mock } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, it, spyOn } from 'bun:test'; import * as childProcess from 'child_process'; import { EventEmitter } from 'events'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; -const spawnCalls: Array<{ - command: string; - args: string[]; - options: Record | undefined; -}> = []; -const originalPlatform = process.platform; +import { CodexAdapter } from '../../../src/targets/codex-adapter'; +import { buildCodexBrowserMcpOverrides } from '../../../src/utils/browser-codex-overrides'; +import * as signalForwarder from '../../../src/utils/signal-forwarder'; function createMockChild(): EventEmitter & { stdout: EventEmitter; @@ -46,34 +43,12 @@ function createMockChild(): EventEmitter & { return child; } -mock.module('child_process', () => ({ - ...childProcess, - spawn: (...spawnArgs: unknown[]) => { - const command = String(spawnArgs[0] ?? ''); - const maybeArgs = spawnArgs[1]; - const args = Array.isArray(maybeArgs) ? (maybeArgs as string[]) : []; - const options = (Array.isArray(maybeArgs) ? spawnArgs[2] : spawnArgs[1]) as - | Record - | undefined; - - spawnCalls.push({ command, args, options }); - return createMockChild(); - }, -})); - -mock.module('../../../src/utils/signal-forwarder', () => ({ - wireChildProcessSignals: () => {}, -})); - -import { CodexAdapter } from '../../../src/targets/codex-adapter'; -import { buildCodexBrowserMcpOverrides } from '../../../src/utils/browser-codex-overrides'; - describe('codex-adapter exec', () => { + const originalPlatform = process.platform; let tmpDir: string; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-codex-adapter-exec-')); - spawnCalls.length = 0; }); afterEach(() => { @@ -87,29 +62,45 @@ describe('codex-adapter exec', () => { const fakeCodex = path.join(tmpDir, 'codex.cmd'); fs.writeFileSync(fakeCodex, ''); - const adapter = new CodexAdapter(); - const binaryInfo = { - path: fakeCodex, - needsShell: true, - features: ['config-overrides'], - }; - const args = adapter.buildArgs('default', ['--version'], { - profileType: 'default', - creds: { - profile: 'default', - baseUrl: '', - apiKey: '', - runtimeConfigOverrides: buildCodexBrowserMcpOverrides(), - }, - binaryInfo, - }); + const spawnSpy = spyOn(childProcess, 'spawn').mockImplementation( + () => createMockChild() as unknown as ReturnType + ); + const signalSpy = spyOn(signalForwarder, 'wireChildProcessSignals').mockImplementation( + () => undefined + ); - adapter.exec(args, {}, { binaryInfo }); + try { + const adapter = new CodexAdapter(); + const binaryInfo = { + path: fakeCodex, + needsShell: true, + features: ['config-overrides'], + }; + const args = adapter.buildArgs('default', ['--version'], { + profileType: 'default', + creds: { + profile: 'default', + baseUrl: '', + apiKey: '', + runtimeConfigOverrides: buildCodexBrowserMcpOverrides(), + }, + binaryInfo, + }); - expect(spawnCalls).toHaveLength(1); - expect(spawnCalls[0]?.options?.shell).toBe('cmd.exe'); - expect(spawnCalls[0]?.command).toContain(fakeCodex); - expect(spawnCalls[0]?.command).toContain('mcp_servers.ccs_browser.args='); - expect(spawnCalls[0]?.command).toContain('@playwright/mcp@0.0.70'); + adapter.exec(args, {}, { binaryInfo }); + + expect(spawnSpy).toHaveBeenCalledTimes(1); + const [command, options] = spawnSpy.mock.calls[0] as [ + string, + Record | undefined, + ]; + expect(options?.shell).toBe('cmd.exe'); + expect(command).toContain(fakeCodex); + expect(command).toContain('mcp_servers.ccs_browser.args='); + expect(command).toContain('@playwright/mcp@0.0.70'); + } finally { + spawnSpy.mockRestore(); + signalSpy.mockRestore(); + } }); }); diff --git a/tests/unit/utils/shell-executor.test.ts b/tests/unit/utils/shell-executor.test.ts index 78d7e274..f3b7e600 100644 --- a/tests/unit/utils/shell-executor.test.ts +++ b/tests/unit/utils/shell-executor.test.ts @@ -81,6 +81,56 @@ describe('escapeShellArg', () => { const { escapeShellArg } = await import('../../../src/utils/shell-executor'); expect(escapeShellArg('hello!')).toBe('"hello^^!"'); }); + + it('prefers ComSpec when resolving the escaped command shell', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + + try { + process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe'; + delete process.env.COMSPEC; + const { getWindowsEscapedCommandShell } = await import( + '../../../src/utils/shell-executor' + ); + expect(getWindowsEscapedCommandShell()).toBe('C:\\Windows\\System32\\cmd.exe'); + } finally { + if (originalComSpec === undefined) delete process.env.ComSpec; + else process.env.ComSpec = originalComSpec; + if (originalCOMSPEC === undefined) delete process.env.COMSPEC; + else process.env.COMSPEC = originalCOMSPEC; + } + }); + + it('falls back to cmd.exe when ComSpec is unavailable', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + + try { + delete process.env.ComSpec; + delete process.env.COMSPEC; + const { getWindowsEscapedCommandShell } = await import( + '../../../src/utils/shell-executor' + ); + expect(getWindowsEscapedCommandShell()).toBe('cmd.exe'); + } finally { + if (originalComSpec === undefined) delete process.env.ComSpec; + else process.env.ComSpec = originalComSpec; + if (originalCOMSPEC === undefined) delete process.env.COMSPEC; + else process.env.COMSPEC = originalCOMSPEC; + } + }); + }); +}); + +describe('getWindowsEscapedCommandShell', () => { + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('returns shell=true outside Windows if called defensively', async () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); + expect(getWindowsEscapedCommandShell()).toBe(true); }); });