diff --git a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts index 5222690779..9eef26079a 100644 --- a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts +++ b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts @@ -105,6 +105,23 @@ vi.mock('../env-utils', () => ({ } return /\.(cmd|bat)$/i.test(command); }), + getSpawnCommand: vi.fn((command: string) => { + // Mock getSpawnCommand to match actual behavior + const trimmed = command.trim(); + // On Windows, quote .cmd/.bat files + if (process.platform === 'win32' && /\.(cmd|bat)$/i.test(trimmed)) { + // Idempotent - if already quoted, return as-is + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed; + } + return `"${trimmed}"`; + } + // For non-.cmd/.bat files, return trimmed (strip quotes if present) + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed.slice(1, -1); + } + return trimmed; + }), getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({ ...baseOptions, shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32' diff --git a/apps/frontend/src/main/__tests__/env-utils.test.ts b/apps/frontend/src/main/__tests__/env-utils.test.ts index cc0ee17c72..994a334d20 100644 --- a/apps/frontend/src/main/__tests__/env-utils.test.ts +++ b/apps/frontend/src/main/__tests__/env-utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, beforeEach, afterEach } from 'vitest'; -import { shouldUseShell, getSpawnOptions } from '../env-utils'; +import { shouldUseShell, getSpawnOptions, getSpawnCommand } from '../env-utils'; describe('shouldUseShell', () => { const originalPlatform = process.platform; @@ -198,3 +198,181 @@ describe('getSpawnOptions', () => { }); }); }); + +describe('getSpawnCommand', () => { + const originalPlatform = process.platform; + + afterEach(() => { + // Restore original platform after each test + Object.defineProperty(process, 'platform', { + value: originalPlatform, + writable: true, + configurable: true, + }); + }); + + describe('Windows platform', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + }); + + it('should quote .cmd files with spaces', () => { + const cmd = getSpawnCommand('C:\\Users\\First Last\\AppData\\Roaming\\npm\\claude.cmd'); + expect(cmd).toBe('"C:\\Users\\First Last\\AppData\\Roaming\\npm\\claude.cmd"'); + }); + + it('should quote .cmd files without spaces too (idempotent)', () => { + const cmd = getSpawnCommand('C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd'); + expect(cmd).toBe('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"'); + }); + + it('should quote .bat files with spaces', () => { + const cmd = getSpawnCommand('D:\\Program Files (x86)\\scripts\\setup.bat'); + expect(cmd).toBe('"D:\\Program Files (x86)\\scripts\\setup.bat"'); + }); + + it('should NOT quote .exe files', () => { + const cmd = getSpawnCommand('C:\\Program Files\\Git\\cmd\\git.exe'); + expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe'); + }); + + it('should NOT quote extensionless files', () => { + const cmd = getSpawnCommand('D:\\Git\\bin\\bash'); + expect(cmd).toBe('D:\\Git\\bin\\bash'); + }); + + it('should handle uppercase .CMD and .BAT extensions', () => { + expect(getSpawnCommand('D:\\Tools\\CLAUDE.CMD')).toBe('"D:\\Tools\\CLAUDE.CMD"'); + expect(getSpawnCommand('C:\\Scripts\\SETUP.BAT')).toBe('"C:\\Scripts\\SETUP.BAT"'); + }); + + it('should be idempotent - already quoted .cmd files stay quoted', () => { + const cmd = getSpawnCommand('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"'); + expect(cmd).toBe('"C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd"'); + }); + + it('should be idempotent - already quoted .bat files stay quoted', () => { + const cmd = getSpawnCommand('"D:\\Program Files\\scripts\\setup.bat"'); + expect(cmd).toBe('"D:\\Program Files\\scripts\\setup.bat"'); + }); + + it('should be idempotent - double-quoting does not occur', () => { + const once = getSpawnCommand('C:\\Users\\admin\\npm\\claude.cmd'); + const twice = getSpawnCommand(once); + expect(once).toBe(twice); + expect(once).toBe('"C:\\Users\\admin\\npm\\claude.cmd"'); + }); + + it('should trim whitespace before processing', () => { + const cmd = getSpawnCommand(' C:\\Users\\admin\\npm\\claude.cmd '); + expect(cmd).toBe('"C:\\Users\\admin\\npm\\claude.cmd"'); + }); + + it('should handle already-quoted .cmd with spaces', () => { + const cmd = getSpawnCommand('"C:\\Users\\First Last\\npm\\claude.cmd"'); + expect(cmd).toBe('"C:\\Users\\First Last\\npm\\claude.cmd"'); + }); + + it('should strip quotes from .exe files (defensive: no quotes with shell:false)', () => { + const cmd = getSpawnCommand('"C:\\Program Files\\Git\\cmd\\git.exe"'); + expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe'); + }); + + it('should strip quotes from extensionless files (defensive: no quotes with shell:false)', () => { + const cmd = getSpawnCommand('"D:\\Git\\bin\\bash"'); + expect(cmd).toBe('D:\\Git\\bin\\bash'); + }); + + it('should strip quotes and trim whitespace from .exe files', () => { + const cmd = getSpawnCommand(' "C:\\Program Files\\Git\\cmd\\git.exe" '); + expect(cmd).toBe('C:\\Program Files\\Git\\cmd\\git.exe'); + }); + }); + + describe('Non-Windows platforms', () => { + it('should NOT quote commands on macOS', () => { + Object.defineProperty(process, 'platform', { + value: 'darwin', + writable: true, + configurable: true, + }); + expect(getSpawnCommand('/usr/local/bin/claude')).toBe('/usr/local/bin/claude'); + expect(getSpawnCommand('/opt/homebrew/bin/claude.cmd')).toBe('/opt/homebrew/bin/claude.cmd'); + }); + + it('should NOT quote commands on Linux', () => { + Object.defineProperty(process, 'platform', { + value: 'linux', + writable: true, + configurable: true, + }); + expect(getSpawnCommand('/usr/bin/claude')).toBe('/usr/bin/claude'); + expect(getSpawnCommand('/home/user/.local/bin/claude.bat')).toBe('/home/user/.local/bin/claude.bat'); + }); + + it('should trim whitespace on macOS', () => { + Object.defineProperty(process, 'platform', { + value: 'darwin', + writable: true, + configurable: true, + }); + expect(getSpawnCommand(' /usr/local/bin/claude ')).toBe('/usr/local/bin/claude'); + expect(getSpawnCommand('\t/opt/homebrew/bin/claude\t')).toBe('/opt/homebrew/bin/claude'); + }); + + it('should trim whitespace on Linux', () => { + Object.defineProperty(process, 'platform', { + value: 'linux', + writable: true, + configurable: true, + }); + expect(getSpawnCommand(' /usr/bin/claude ')).toBe('/usr/bin/claude'); + expect(getSpawnCommand('\t/home/user/.local/bin/claude\t')).toBe('/home/user/.local/bin/claude'); + }); + }); +}); + +describe('shouldUseShell with quoted paths', () => { + const originalPlatform = process.platform; + + afterEach(() => { + // Restore original platform after each test + Object.defineProperty(process, 'platform', { + value: originalPlatform, + writable: true, + configurable: true, + }); + }); + + describe('Windows platform', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + }); + + it('should detect .cmd files in quoted paths', () => { + expect(shouldUseShell('"C:\\Users\\admin\\npm\\claude.cmd"')).toBe(true); + expect(shouldUseShell('"D:\\Tools\\CLAUDE.CMD"')).toBe(true); + }); + + it('should detect .bat files in quoted paths', () => { + expect(shouldUseShell('"C:\\Scripts\\setup.bat"')).toBe(true); + expect(shouldUseShell('"D:\\Program Files\\script.BAT"')).toBe(true); + }); + + it('should NOT detect .exe files in quoted paths', () => { + expect(shouldUseShell('"C:\\Program Files\\git.exe"')).toBe(false); + }); + + it('should handle whitespace around quoted paths', () => { + expect(shouldUseShell(' "C:\\Users\\admin\\npm\\claude.cmd" ')).toBe(true); + }); + }); +}); diff --git a/apps/frontend/src/main/cli-tool-manager.ts b/apps/frontend/src/main/cli-tool-manager.ts index 3aa514d1e6..cf7013f98e 100644 --- a/apps/frontend/src/main/cli-tool-manager.ts +++ b/apps/frontend/src/main/cli-tool-manager.ts @@ -26,7 +26,7 @@ import path from 'path'; import os from 'os'; import { promisify } from 'util'; import { app } from 'electron'; -import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; +import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync, getSpawnCommand } from './env-utils'; import type { ToolDetectionResult } from '../shared/types'; const execFileAsync = promisify(execFile); @@ -923,7 +923,7 @@ class CLIToolManager { if (needsShell) { // For .cmd/.bat files on Windows, use execSync with quoted path // execFileSync doesn't handle spaces in .cmd paths correctly even with shell:true - const quotedCmd = `"${claudeCmd}"`; + const quotedCmd = getSpawnCommand(claudeCmd); version = execSync(`${quotedCmd} --version`, { encoding: 'utf-8', timeout: 5000, @@ -1039,7 +1039,7 @@ class CLIToolManager { if (needsShell) { // For .cmd/.bat files on Windows, use exec with quoted path - const quotedCmd = `"${claudeCmd}"`; + const quotedCmd = getSpawnCommand(claudeCmd); const result = await execAsync(`${quotedCmd} --version`, { encoding: 'utf-8', timeout: 5000, diff --git a/apps/frontend/src/main/env-utils.ts b/apps/frontend/src/main/env-utils.ts index 8463a2ace4..8d4b88478c 100644 --- a/apps/frontend/src/main/env-utils.ts +++ b/apps/frontend/src/main/env-utils.ts @@ -463,8 +463,12 @@ export function shouldUseShell(command: string): boolean { return false; } + const trimmed = command.trim(); + const unquoted = + trimmed.startsWith('"') && trimmed.endsWith('"') ? trimmed.slice(1, -1) : trimmed; + // Check if command ends with .cmd or .bat (case-insensitive) - return /\.(cmd|bat)$/i.test(command); + return /\.(cmd|bat)$/i.test(unquoted); } /** @@ -473,6 +477,9 @@ export function shouldUseShell(command: string): boolean { * Provides a consistent way to create spawn options that work across platforms. * Handles the shell requirement for Windows .cmd/.bat files automatically. * + * For .cmd/.bat files on Windows, returns options that tell the caller to use + * proper quoting for paths with spaces. + * * @param command - The command path to execute * @param baseOptions - Base spawn options to merge with (optional) * @returns Spawn options with correct shell setting @@ -480,7 +487,7 @@ export function shouldUseShell(command: string): boolean { * @example * ```typescript * const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} }); - * spawn(claudeCmd, ['--version'], opts); + * spawn(getSpawnCommand(claudeCmd), ['--version'], opts); * ``` */ export function getSpawnOptions( @@ -505,3 +512,37 @@ export function getSpawnOptions( shell: shouldUseShell(command), }; } + +/** + * Get the properly quoted command for use with spawn() + * + * For .cmd/.bat files on Windows with shell:true, the command path must be + * quoted to handle paths containing spaces correctly (e.g., C:\Users\OXFAM MONS\...). + * + * @param command - The command path to execute + * @returns The command (quoted if needed for .cmd/.bat files on Windows) + * + * @example + * ```typescript + * const cmd = getSpawnCommand(claudeCmd); // "C:\Users\OXFAM MONS\...\claude.cmd" + * const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} }); + * spawn(cmd, ['--version'], opts); + * ``` + */ +export function getSpawnCommand(command: string): string { + // For .cmd/.bat files on Windows, quote the command to handle spaces + // The shell will parse the quoted path correctly + const trimmed = command.trim(); + if (shouldUseShell(trimmed)) { + // Idempotent if already quoted + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed; + } + return `"${trimmed}"`; + } + // For non-.cmd/.bat files, strip quotes if present (defensive: no double quotes with shell:false) + if (trimmed.startsWith('"') && trimmed.endsWith('"')) { + return trimmed.slice(1, -1); + } + return trimmed; +} diff --git a/apps/frontend/src/main/ipc-handlers/env-handlers.ts b/apps/frontend/src/main/ipc-handlers/env-handlers.ts index 3a9cbe417f..c0d7e2278e 100644 --- a/apps/frontend/src/main/ipc-handlers/env-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/env-handlers.ts @@ -10,7 +10,7 @@ import { projectStore } from '../project-store'; import { parseEnvFile } from './utils'; import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; import { debugError } from '../../shared/utils/debug-logger'; -import { getSpawnOptions } from '../env-utils'; +import { getSpawnOptions, getSpawnCommand } from '../env-utils'; // GitLab environment variable keys const GITLAB_ENV_KEYS = { @@ -603,7 +603,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ try { // Check if Claude CLI is available and authenticated const result = await new Promise((resolve) => { - const proc = spawn(claudeCmd, ['--version'], getSpawnOptions(claudeCmd, { + const proc = spawn(getSpawnCommand(claudeCmd), ['--version'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, })); @@ -623,7 +623,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ if (code === 0) { // Claude CLI is available, check if authenticated // Run a simple command that requires auth - const authCheck = spawn(claudeCmd, ['api', '--help'], getSpawnOptions(claudeCmd, { + const authCheck = spawn(getSpawnCommand(claudeCmd), ['api', '--help'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, })); @@ -692,7 +692,7 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ try { // Run claude setup-token which will open browser for OAuth const result = await new Promise((resolve) => { - const proc = spawn(claudeCmd, ['setup-token'], getSpawnOptions(claudeCmd, { + const proc = spawn(getSpawnCommand(claudeCmd), ['setup-token'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, stdio: 'inherit' // This allows the terminal to handle the interactive auth