mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 14:16:43 +00:00
Merge pull request #1043 from kaitranntt/kai/fix/windows-codex-wrapper-quoting
fix(targets): use cmd.exe for escaped Windows wrapper launches
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -9,7 +9,12 @@ import { spawn, ChildProcess } from 'child_process';
|
||||
import { TargetAdapter, TargetBinaryInfo, TargetCredentials, TargetType } from './target-adapter';
|
||||
import { detectClaudeCli, getClaudeCliInfo } from '../utils/claude-detector';
|
||||
import type { ProfileType } from '../types/profile';
|
||||
import { escapeShellArg, stripAnthropicEnv, stripClaudeCodeEnv } from '../utils/shell-executor';
|
||||
import {
|
||||
escapeShellArg,
|
||||
getWindowsEscapedCommandShell,
|
||||
stripAnthropicEnv,
|
||||
stripClaudeCodeEnv,
|
||||
} from '../utils/shell-executor';
|
||||
import { ErrorManager } from '../utils/error-manager';
|
||||
import { getWebSearchHookEnv } from '../utils/websearch-manager';
|
||||
import { appendBrowserToolArgs } from '../utils/browser';
|
||||
@@ -111,7 +116,7 @@ export class ClaudeAdapter implements TargetAdapter {
|
||||
child = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env,
|
||||
});
|
||||
} else {
|
||||
|
||||
@@ -4,7 +4,12 @@ import type { ProfileType } from '../types/profile';
|
||||
import { runCleanup } from '../errors';
|
||||
import { expandPath } from '../utils/helpers';
|
||||
import { wireChildProcessSignals } from '../utils/signal-forwarder';
|
||||
import { escapeShellArg, stripAnthropicEnv, stripCodexSessionEnv } from '../utils/shell-executor';
|
||||
import {
|
||||
escapeShellArg,
|
||||
getWindowsEscapedCommandShell,
|
||||
stripAnthropicEnv,
|
||||
stripCodexSessionEnv,
|
||||
} from '../utils/shell-executor';
|
||||
import type {
|
||||
TargetAdapter,
|
||||
TargetBinaryInfo,
|
||||
@@ -316,7 +321,7 @@ export class CodexAdapter implements TargetAdapter {
|
||||
child = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env: launchEnv,
|
||||
});
|
||||
} else {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import * as fs from 'fs';
|
||||
import * as childProcess from 'child_process';
|
||||
import { expandPath } from '../utils/helpers';
|
||||
import { escapeShellArg } from '../utils/shell-executor';
|
||||
import { escapeShellArg, getWindowsEscapedCommandShell } from '../utils/shell-executor';
|
||||
import type { TargetBinaryInfo } from './target-adapter';
|
||||
|
||||
const CODEX_CONFIG_OVERRIDE_FEATURE = 'config-overrides';
|
||||
@@ -58,7 +58,7 @@ function runCodexProbe(codexPath: string, args: string[]): string | undefined {
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
timeout: 5000,
|
||||
windowsHide: true,
|
||||
shell: 'cmd.exe',
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
});
|
||||
return result.status === 0 ? result.stdout : undefined;
|
||||
}
|
||||
|
||||
@@ -12,7 +12,11 @@ import { getDroidBinaryInfo, detectDroidCli, checkDroidVersion } from './droid-d
|
||||
import type { ProfileType } from '../types/profile';
|
||||
import { upsertCcsModel } from './droid-config-manager';
|
||||
import { resolveDroidProvider } from './droid-provider';
|
||||
import { escapeShellArg, stripAnthropicEnv } from '../utils/shell-executor';
|
||||
import {
|
||||
escapeShellArg,
|
||||
getWindowsEscapedCommandShell,
|
||||
stripAnthropicEnv,
|
||||
} from '../utils/shell-executor';
|
||||
import { wireChildProcessSignals } from '../utils/signal-forwarder';
|
||||
import { runCleanup } from '../errors';
|
||||
|
||||
@@ -134,7 +138,7 @@ export class DroidAdapter implements TargetAdapter {
|
||||
child = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env,
|
||||
});
|
||||
} else {
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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';
|
||||
@@ -107,6 +107,20 @@ export function escapeShellArg(arg: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the shell that matches escapeShellArg() quoting semantics.
|
||||
*
|
||||
* 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(): SpawnOptions['shell'] {
|
||||
if (process.platform !== 'win32') {
|
||||
return true;
|
||||
}
|
||||
|
||||
return process.env.ComSpec || process.env.COMSPEC || 'cmd.exe';
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute Claude CLI with unified spawn logic
|
||||
*/
|
||||
@@ -182,7 +196,7 @@ export function execClaude(
|
||||
child = spawn(cmdString, {
|
||||
stdio: 'inherit',
|
||||
windowsHide: true,
|
||||
shell: true,
|
||||
shell: getWindowsEscapedCommandShell(),
|
||||
env,
|
||||
});
|
||||
} else {
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
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';
|
||||
|
||||
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;
|
||||
stderr: EventEmitter;
|
||||
exitCode: number | null;
|
||||
killed: boolean;
|
||||
pid: number;
|
||||
unref: () => EventEmitter;
|
||||
kill: () => boolean;
|
||||
} {
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
stdout: EventEmitter;
|
||||
stderr: EventEmitter;
|
||||
exitCode: number | null;
|
||||
killed: boolean;
|
||||
pid: number;
|
||||
unref: () => EventEmitter;
|
||||
kill: () => boolean;
|
||||
};
|
||||
|
||||
child.stdout = new EventEmitter();
|
||||
child.stderr = new EventEmitter();
|
||||
child.exitCode = null;
|
||||
child.killed = false;
|
||||
child.pid = process.pid;
|
||||
child.unref = () => child;
|
||||
child.kill = () => {
|
||||
child.killed = true;
|
||||
child.exitCode = 1;
|
||||
return true;
|
||||
};
|
||||
|
||||
return child;
|
||||
}
|
||||
|
||||
describe('codex-adapter exec', () => {
|
||||
const originalPlatform = process.platform;
|
||||
let tmpDir: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-codex-adapter-exec-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(process, 'platform', { value: originalPlatform });
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('launches Windows cmd wrappers via cmd.exe when runtime overrides include browser MCP args', () => {
|
||||
Object.defineProperty(process, 'platform', { value: 'win32' });
|
||||
|
||||
const fakeCodex = path.join(tmpDir, 'codex.cmd');
|
||||
fs.writeFileSync(fakeCodex, '');
|
||||
|
||||
const spawnSpy = spyOn(childProcess, 'spawn').mockImplementation(
|
||||
() => createMockChild() as unknown as ReturnType<typeof childProcess.spawn>
|
||||
);
|
||||
const signalSpy = spyOn(signalForwarder, 'wireChildProcessSignals').mockImplementation(
|
||||
() => undefined
|
||||
);
|
||||
|
||||
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,
|
||||
});
|
||||
|
||||
adapter.exec(args, {}, { binaryInfo });
|
||||
|
||||
expect(spawnSpy).toHaveBeenCalledTimes(1);
|
||||
const [command, options] = spawnSpy.mock.calls[0] as [
|
||||
string,
|
||||
Record<string, unknown> | 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();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -266,6 +266,7 @@ describe('CLAUDECODE environment stripping', () => {
|
||||
expect(spawnCalls.length).toBeGreaterThan(0);
|
||||
const env = spawnCalls[0].options?.env as NodeJS.ProcessEnv;
|
||||
expect(Object.keys(env).map((k) => k.toUpperCase())).not.toContain('CLAUDECODE');
|
||||
expect(spawnCalls[0].options?.shell).toBe('cmd.exe');
|
||||
});
|
||||
|
||||
it('execClaude sets DISABLE_AUTOUPDATER=1 when preferences.auto_update is false', () => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user