fix: handle Windows spawn EINVAL on non-ASCII paths and add helper utilities
This commit is contained in:
parent
995b6ef843
commit
35b23fc784
4 changed files with 269 additions and 0 deletions
99
src/main/utils/childProcess.ts
Normal file
99
src/main/utils/childProcess.ts
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
import { spawn, execFile, exec, SpawnOptions, ExecFileOptions } from 'child_process';
|
||||
import { promisify } from 'util';
|
||||
|
||||
// re-exported helpers used throughout the codebase
|
||||
export const execFileAsync = promisify(execFile);
|
||||
export const execAsync = promisify(exec);
|
||||
|
||||
/**
|
||||
* Returns true if the string contains any non-ASCII character.
|
||||
*/
|
||||
function containsNonAscii(str: string): boolean {
|
||||
return /[^\x00-\x7F]/.test(str);
|
||||
}
|
||||
|
||||
/**
|
||||
* On Windows, creating a process whose *path* contains non-ASCII
|
||||
* characters will often fail with `spawn EINVAL`. Detect that case so
|
||||
* callers can automatically fall back to launching via a shell.
|
||||
*/
|
||||
function needsShell(binaryPath: string): boolean {
|
||||
if (process.platform !== 'win32') return false;
|
||||
if (!binaryPath) return false;
|
||||
return containsNonAscii(binaryPath);
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal quoting for command‑line arguments when building a shell
|
||||
* invocation. We only escape spaces and double quotes since our
|
||||
* callers only ever use simple strings (paths, flags, literals) and
|
||||
* the shell itself will handle most quoting rules.
|
||||
*/
|
||||
function quoteArg(arg: string): string {
|
||||
if (/[^A-Za-z0-9_\-\/.]/.test(arg)) {
|
||||
return `"${arg.replace(/"/g, '\\"')}"`;
|
||||
}
|
||||
return arg;
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute a CLI binary, falling back to running the command through a
|
||||
* shell on Windows if the normal path-based spawn fails. `binaryPath`
|
||||
* may be `null` which causes `claude` (lookup via PATH) to be used.
|
||||
*
|
||||
* The return value matches the shape of Node's `execFile` promise: an
|
||||
* object with `stdout` and `stderr` strings.
|
||||
*/
|
||||
export async function execCli(
|
||||
binaryPath: string | null,
|
||||
args: string[],
|
||||
options: ExecFileOptions = {}
|
||||
): Promise<{ stdout: string; stderr: string }> {
|
||||
const target = binaryPath || 'claude';
|
||||
|
||||
// attempt the normal execFile path first
|
||||
if (!needsShell(target)) {
|
||||
try {
|
||||
const result = await execFileAsync(target, args, options);
|
||||
return { stdout: String(result.stdout), stderr: String(result.stderr) };
|
||||
} catch (err: any) {
|
||||
// fall through to shell fallback only when the error matches the
|
||||
// Windows "invalid argument" problem; otherwise rethrow.
|
||||
if (!(err && err.code === 'EINVAL')) {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// shell fallback (Windows only; others shouldn't reach here)
|
||||
const cmd = [target, ...args].map(quoteArg).join(' ');
|
||||
const shellResult = await execAsync(cmd, options as unknown as import('child_process').ExecOptions);
|
||||
return { stdout: String(shellResult.stdout), stderr: String(shellResult.stderr) };
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn a child process. If the initial `spawn()` call throws
|
||||
* synchronously with EINVAL on Windows, retry using a shell-based
|
||||
* command string. The returned `ChildProcess` is whatever the
|
||||
* underlying call returned; listeners may safely be attached to it.
|
||||
*/
|
||||
export function spawnCli(
|
||||
binaryPath: string,
|
||||
args: string[],
|
||||
options: SpawnOptions = {}
|
||||
) {
|
||||
if (process.platform === 'win32' && needsShell(binaryPath)) {
|
||||
const cmd = [binaryPath, ...args].map(quoteArg).join(' ');
|
||||
return spawn(cmd, { shell: true, ...options });
|
||||
}
|
||||
|
||||
try {
|
||||
return spawn(binaryPath, args, options);
|
||||
} catch (err: any) {
|
||||
if (process.platform === 'win32' && err && err.code === 'EINVAL') {
|
||||
const cmd = [binaryPath, ...args].map(quoteArg).join(' ');
|
||||
return spawn(cmd, { shell: true, ...options });
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
|
@ -105,6 +105,30 @@ describe('CliInstallerService', () => {
|
|||
// Version will be null because execFile is mocked to no-op
|
||||
// and latestVersion will be null because fetch is mocked
|
||||
});
|
||||
|
||||
it('handles spawn EINVAL when binary path contains non-ASCII by falling back', async () => {
|
||||
allowConsoleLogs();
|
||||
const fakePath = 'C:\\Users\\Алексей\\AppData\\Roaming\\npm\\claude.cmd';
|
||||
vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue(fakePath);
|
||||
|
||||
// mock execFile to throw EINVAL first
|
||||
const err: any = new Error('spawn EINVAL');
|
||||
err.code = 'EINVAL';
|
||||
const childProcess = await import('child_process');
|
||||
vi.spyOn(childProcess, 'execFile').mockImplementation((cmd, args, opts, cb) => {
|
||||
cb(err, '', '');
|
||||
return {} as any;
|
||||
});
|
||||
// mock exec to succeed as fallback
|
||||
vi.spyOn(childProcess, 'exec').mockImplementation((cmd, opts, cb) => {
|
||||
cb(null, '2.3.4', '');
|
||||
return {} as any;
|
||||
});
|
||||
|
||||
const status = await service.getStatus();
|
||||
expect(status.installed).toBe(true);
|
||||
expect(status.installedVersion).toBe('2.3.4');
|
||||
});
|
||||
});
|
||||
|
||||
describe('install mutex', () => {
|
||||
|
|
|
|||
38
test/main/services/team/TeamProvisioningService.test.ts
Normal file
38
test/main/services/team/TeamProvisioningService.test.ts
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({
|
||||
ClaudeBinaryResolver: { resolve: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock('@main/utils/childProcess', () => ({
|
||||
spawnCli: vi.fn(),
|
||||
}));
|
||||
|
||||
import { TeamProvisioningService } from '@main/services/team/TeamProvisioningService';
|
||||
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
|
||||
import { spawnCli } from '@main/utils/childProcess';
|
||||
|
||||
function allowConsoleLogs() {
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
}
|
||||
|
||||
describe('TeamProvisioningService', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('warmup', () => {
|
||||
it('does not throw when spawnCli rejects', async () => {
|
||||
allowConsoleLogs();
|
||||
vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('C:\\path\\claude');
|
||||
vi.mocked(spawnCli).mockImplementation(() => {
|
||||
throw new Error('spawn EINVAL');
|
||||
});
|
||||
|
||||
const svc = new TeamProvisioningService();
|
||||
await expect(svc.warmup()).resolves.not.toThrow();
|
||||
expect(spawnCli).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
108
test/main/utils/childProcess.test.ts
Normal file
108
test/main/utils/childProcess.test.ts
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
|
||||
// Mock the entire child_process module so that we can inspect how our helpers
|
||||
// invoke spawn/exec without hitting the real filesystem or spawning anything.
|
||||
vi.mock('child_process', () => ({
|
||||
spawn: vi.fn(),
|
||||
execFile: vi.fn(),
|
||||
exec: vi.fn(),
|
||||
}));
|
||||
|
||||
// Import after the mock call so that the mocked module is returned.
|
||||
import * as child from 'child_process';
|
||||
import { spawnCli, execCli } from '@main/utils/childProcess';
|
||||
|
||||
// Helper to temporarily override process.platform
|
||||
function setPlatform(value: string) {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
value,
|
||||
});
|
||||
}
|
||||
|
||||
// restore platform after tests
|
||||
const originalPlatform = process.platform;
|
||||
|
||||
describe('cli child process helpers', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
setPlatform(originalPlatform);
|
||||
});
|
||||
|
||||
describe('spawnCli', () => {
|
||||
it('calls spawn directly when path is ascii on windows', () => {
|
||||
setPlatform('win32');
|
||||
(child.spawn as unknown as vi.Mock).mockReturnValue({} as any);
|
||||
|
||||
const result = spawnCli('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' });
|
||||
expect(child.spawn).toHaveBeenCalledWith('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' });
|
||||
expect(result).toEqual({} as any);
|
||||
});
|
||||
|
||||
it('falls back to shell when spawn throws EINVAL', () => {
|
||||
setPlatform('win32');
|
||||
const error: any = new Error('spawn EINVAL');
|
||||
error.code = 'EINVAL';
|
||||
const fake = {} as any;
|
||||
const spawnMock = child.spawn as unknown as vi.Mock;
|
||||
spawnMock.mockImplementationOnce(() => {
|
||||
throw error;
|
||||
});
|
||||
spawnMock.mockImplementationOnce(() => fake);
|
||||
|
||||
const result = spawnCli('C:\\Users\\Àëåêñåé\\AppData\\Roaming\\npm\\claude.cmd', ['a', 'b'], {
|
||||
env: { FOO: 'bar' },
|
||||
});
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
const secondArg0 = spawnMock.mock.calls[1][0] as string;
|
||||
expect(secondArg0).toMatch(/claude\.cmd/);
|
||||
expect(spawnMock.mock.calls[1][2]).toMatchObject({ shell: true, env: { FOO: 'bar' } });
|
||||
expect(result).toBe(fake);
|
||||
});
|
||||
|
||||
it('does not use shell when not on windows', () => {
|
||||
setPlatform('linux');
|
||||
(child.spawn as unknown as vi.Mock).mockReturnValue({} as any);
|
||||
const result = spawnCli('/usr/bin/claude', ['--help']);
|
||||
expect(child.spawn).toHaveBeenCalledWith('/usr/bin/claude', ['--help'], {});
|
||||
expect(result).toEqual({} as any);
|
||||
});
|
||||
});
|
||||
|
||||
describe('execCli', () => {
|
||||
it('invokes execFile when path is normal', async () => {
|
||||
setPlatform('win32');
|
||||
const execFileMock = child.execFile as unknown as vi.Mock;
|
||||
execFileMock.mockImplementation((cmd, args, opts, cb) => {
|
||||
cb(null, 'ok', '');
|
||||
return {} as any;
|
||||
});
|
||||
const result = await execCli('C:\\bin\\claude.exe', ['--version']);
|
||||
expect(execFileMock).toHaveBeenCalledWith('C:\\bin\\claude.exe', ['--version'], {}, expect.any(Function));
|
||||
expect(result.stdout).toBe('ok');
|
||||
});
|
||||
|
||||
it('falls back to exec shell when execFile throws EINVAL or path contains non-ascii', async () => {
|
||||
setPlatform('win32');
|
||||
const execFileMock = child.execFile as unknown as vi.Mock;
|
||||
execFileMock.mockImplementation((cmd, args, opts, cb) => {
|
||||
const err: any = new Error('spawn EINVAL');
|
||||
err.code = 'EINVAL';
|
||||
cb(err, '', '');
|
||||
return {} as any;
|
||||
});
|
||||
const execMock = child.exec as unknown as vi.Mock;
|
||||
execMock.mockImplementation((cmd, opts, cb) => {
|
||||
cb(null, '1.2.3', '');
|
||||
return {} as any;
|
||||
});
|
||||
|
||||
const result = await execCli('C:\\Users\\Àëåêñåé\\AppData\\Roaming\\npm\\claude.cmd', ['--version']);
|
||||
expect(execFileMock).toHaveBeenCalled();
|
||||
expect(execMock).toHaveBeenCalled();
|
||||
expect(result.stdout).toBe('1.2.3');
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue