mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-03 08:18:23 +00:00
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.
This commit is contained in:
@@ -7,7 +7,11 @@
|
|||||||
import { spawn, ChildProcess } from 'child_process';
|
import { spawn, ChildProcess } from 'child_process';
|
||||||
import { initUI, header, color, fail, warn, info, infoBox, warnBox } from '../../utils/ui';
|
import { initUI, header, color, fail, warn, info, infoBox, warnBox } from '../../utils/ui';
|
||||||
import { getClaudeCliInfo } from '../../utils/claude-detector';
|
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 { isUnifiedMode } from '../../config/unified-config-loader';
|
||||||
import { ProfileMetadata } from '../../types';
|
import { ProfileMetadata } from '../../types';
|
||||||
import {
|
import {
|
||||||
@@ -249,7 +253,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise
|
|||||||
child = spawn(cmdString, {
|
child = spawn(cmdString, {
|
||||||
stdio: 'inherit',
|
stdio: 'inherit',
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
shell: true,
|
shell: getWindowsEscapedCommandShell(),
|
||||||
env: childEnv,
|
env: childEnv,
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ import * as path from 'path';
|
|||||||
import { ProgressIndicator } from '../../utils/progress-indicator';
|
import { ProgressIndicator } from '../../utils/progress-indicator';
|
||||||
import { ok, fail, info, warn } from '../../utils/ui';
|
import { ok, fail, info, warn } from '../../utils/ui';
|
||||||
import { getCcsDir } from '../../utils/config-manager';
|
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 { ensureCLIProxyBinary } from '../binary-manager';
|
||||||
import {
|
import {
|
||||||
generateConfig,
|
generateConfig,
|
||||||
@@ -1316,7 +1316,7 @@ export async function execClaudeWithCLIProxy(
|
|||||||
claude = spawn(cmdString, {
|
claude = spawn(cmdString, {
|
||||||
stdio: 'inherit',
|
stdio: 'inherit',
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
shell: true,
|
shell: getWindowsEscapedCommandShell(),
|
||||||
env: tracedEnv,
|
env: tracedEnv,
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -6,7 +6,11 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { spawn, ChildProcess, SpawnOptions } from 'child_process';
|
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 { getClaudeCliInfo } from './claude-detector';
|
||||||
import { ErrorManager } from './error-manager';
|
import { ErrorManager } from './error-manager';
|
||||||
|
|
||||||
@@ -56,7 +60,7 @@ export function spawnClaude(options: SpawnClaudeOptions = {}): SpawnClaudeResult
|
|||||||
child = spawn(cmdString, {
|
child = spawn(cmdString, {
|
||||||
stdio,
|
stdio,
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
shell: true,
|
shell: getWindowsEscapedCommandShell(),
|
||||||
env: mergedEnv,
|
env: mergedEnv,
|
||||||
cwd,
|
cwd,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
* Cross-platform shell execution utilities for CCS.
|
* 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 { ErrorManager } from './error-manager';
|
||||||
import { getWebSearchHookEnv } from './websearch-manager';
|
import { getWebSearchHookEnv } from './websearch-manager';
|
||||||
import { wireChildProcessSignals } from './signal-forwarder';
|
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
|
* On Windows, prefer ComSpec over a bare `cmd.exe` so escaped wrapper launches
|
||||||
* route through a different quoting path than the one escapeShellArg() expects.
|
* keep the same shell contract without depending on PATH lookup.
|
||||||
*/
|
*/
|
||||||
export function getWindowsEscapedCommandShell(): string {
|
export function getWindowsEscapedCommandShell(): SpawnOptions['shell'] {
|
||||||
return 'cmd.exe';
|
if (process.platform !== 'win32') {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return process.env.ComSpec || process.env.COMSPEC || 'cmd.exe';
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -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 * as childProcess from 'child_process';
|
||||||
import { EventEmitter } from 'events';
|
import { EventEmitter } from 'events';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
|
||||||
const spawnCalls: Array<{
|
import { CodexAdapter } from '../../../src/targets/codex-adapter';
|
||||||
command: string;
|
import { buildCodexBrowserMcpOverrides } from '../../../src/utils/browser-codex-overrides';
|
||||||
args: string[];
|
import * as signalForwarder from '../../../src/utils/signal-forwarder';
|
||||||
options: Record<string, unknown> | undefined;
|
|
||||||
}> = [];
|
|
||||||
const originalPlatform = process.platform;
|
|
||||||
|
|
||||||
function createMockChild(): EventEmitter & {
|
function createMockChild(): EventEmitter & {
|
||||||
stdout: EventEmitter;
|
stdout: EventEmitter;
|
||||||
@@ -46,34 +43,12 @@ function createMockChild(): EventEmitter & {
|
|||||||
return child;
|
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<string, unknown>
|
|
||||||
| 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', () => {
|
describe('codex-adapter exec', () => {
|
||||||
|
const originalPlatform = process.platform;
|
||||||
let tmpDir: string;
|
let tmpDir: string;
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-codex-adapter-exec-'));
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-codex-adapter-exec-'));
|
||||||
spawnCalls.length = 0;
|
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -87,29 +62,45 @@ describe('codex-adapter exec', () => {
|
|||||||
const fakeCodex = path.join(tmpDir, 'codex.cmd');
|
const fakeCodex = path.join(tmpDir, 'codex.cmd');
|
||||||
fs.writeFileSync(fakeCodex, '');
|
fs.writeFileSync(fakeCodex, '');
|
||||||
|
|
||||||
const adapter = new CodexAdapter();
|
const spawnSpy = spyOn(childProcess, 'spawn').mockImplementation(
|
||||||
const binaryInfo = {
|
() => createMockChild() as unknown as ReturnType<typeof childProcess.spawn>
|
||||||
path: fakeCodex,
|
);
|
||||||
needsShell: true,
|
const signalSpy = spyOn(signalForwarder, 'wireChildProcessSignals').mockImplementation(
|
||||||
features: ['config-overrides'],
|
() => undefined
|
||||||
};
|
);
|
||||||
const args = adapter.buildArgs('default', ['--version'], {
|
|
||||||
profileType: 'default',
|
|
||||||
creds: {
|
|
||||||
profile: 'default',
|
|
||||||
baseUrl: '',
|
|
||||||
apiKey: '',
|
|
||||||
runtimeConfigOverrides: buildCodexBrowserMcpOverrides(),
|
|
||||||
},
|
|
||||||
binaryInfo,
|
|
||||||
});
|
|
||||||
|
|
||||||
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);
|
adapter.exec(args, {}, { binaryInfo });
|
||||||
expect(spawnCalls[0]?.options?.shell).toBe('cmd.exe');
|
|
||||||
expect(spawnCalls[0]?.command).toContain(fakeCodex);
|
expect(spawnSpy).toHaveBeenCalledTimes(1);
|
||||||
expect(spawnCalls[0]?.command).toContain('mcp_servers.ccs_browser.args=');
|
const [command, options] = spawnSpy.mock.calls[0] as [
|
||||||
expect(spawnCalls[0]?.command).toContain('@playwright/mcp@0.0.70');
|
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();
|
||||||
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -81,6 +81,56 @@ describe('escapeShellArg', () => {
|
|||||||
const { escapeShellArg } = await import('../../../src/utils/shell-executor');
|
const { escapeShellArg } = await import('../../../src/utils/shell-executor');
|
||||||
expect(escapeShellArg('hello!')).toBe('"hello^^!"');
|
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