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 edb6af1625..f47a437d94 100644 --- a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts +++ b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts @@ -169,9 +169,35 @@ vi.mock('../utils/windows-paths', () => ({ WINDOWS_GIT_PATHS: {} })); +// Mock platform module - controls isWindows(), isMacOS(), isUnix() in tests +// This is critical because getClaudeDetectionPaths() uses these to generate different paths +const mockIsWindows = vi.fn(() => false); +const mockIsMacOS = vi.fn(() => false); +const mockIsUnix = vi.fn(() => true); + +vi.mock('../platform', async (importOriginal) => { + const path = await import('path'); + const original = await importOriginal(); + + return { + isWindows: () => mockIsWindows(), + isMacOS: () => mockIsMacOS(), + isUnix: () => mockIsUnix(), + joinPaths: (...segments: string[]) => path.join(...segments), + getExecutableExtension: () => mockIsWindows() ? '.exe' : '', + findNodeJsDirectories: vi.fn(() => []), + // Use the actual sortNvmVersionDirs implementation for testing + sortNvmVersionDirs: original.sortNvmVersionDirs + }; +}); + describe('cli-tool-manager - Claude CLI NVM detection', () => { beforeEach(() => { vi.clearAllMocks(); + // Reset platform mocks to default Linux state + mockIsWindows.mockReturnValue(false); + mockIsMacOS.mockReturnValue(false); + mockIsUnix.mockReturnValue(true); // Set default platform to Linux Object.defineProperty(process, 'platform', { value: 'linux', @@ -231,6 +257,10 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { value: 'win32', writable: true }); + // Configure platform mocks for Windows + mockIsWindows.mockReturnValue(true); + mockIsMacOS.mockReturnValue(false); + mockIsUnix.mockReturnValue(false); vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); vi.mocked(existsSync).mockReturnValue(false); @@ -347,6 +377,9 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { describe('Platform-specific path detection', () => { it('should detect Claude CLI in Windows AppData npm global path', () => { + // Configure platform mock for Windows + mockIsWindows.mockReturnValue(true); + mockIsUnix.mockReturnValue(false); Object.defineProperty(process, 'platform', { value: 'win32', writable: true @@ -374,7 +407,10 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { expect(result.source).toBe('system-path'); }); - it('should ignore insecure Windows Claude CLI path from where.exe', () => { + it('should not find Claude CLI when where.exe returns null due to security', () => { + // Configure platform mock for Windows + mockIsWindows.mockReturnValue(true); + mockIsUnix.mockReturnValue(false); Object.defineProperty(process, 'platform', { value: 'win32', writable: true @@ -382,25 +418,19 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); vi.mocked(findExecutable).mockReturnValue(null); - vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( - 'D:\\Tools\\claude.cmd' - ); - vi.mocked(isSecurePath).mockReturnValueOnce(false); + // findWindowsExecutableViaWhere returns null when path is insecure + // (security check happens inside that function, not in cli-tool-manager) + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue(null); - vi.mocked(existsSync).mockImplementation((filePath) => { - const pathStr = String(filePath); - if (pathStr.includes('Tools') && pathStr.includes('claude.cmd')) { - return true; - } - return false; - }); + // No platform paths exist either + vi.mocked(existsSync).mockReturnValue(false); const result = getToolInfo('claude'); expect(result.found).toBe(false); expect(result.source).toBe('fallback'); - expect(execFileSync).not.toHaveBeenCalled(); - expect(isSecurePath).toHaveBeenCalledWith('D:\\Tools\\claude.cmd'); + // findWindowsExecutableViaWhere was called but returned null + expect(findWindowsExecutableViaWhere).toHaveBeenCalled(); }); it('should detect Claude CLI in Unix .local/bin path', () => { @@ -459,6 +489,10 @@ describe('cli-tool-manager - Helper Functions', () => { value: 'win32', writable: true }); + // Configure platform mocks for Windows + mockIsWindows.mockReturnValue(true); + mockIsMacOS.mockReturnValue(false); + mockIsUnix.mockReturnValue(false); const paths = getClaudeDetectionPaths('C:\\Users\\test'); @@ -595,6 +629,10 @@ describe('cli-tool-manager - Helper Functions', () => { describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { beforeEach(() => { vi.clearAllMocks(); + // Configure platform mock for Windows by default in this describe block + mockIsWindows.mockReturnValue(true); + mockIsUnix.mockReturnValue(false); + mockIsMacOS.mockReturnValue(false); Object.defineProperty(process, 'platform', { value: 'win32', writable: true @@ -616,7 +654,7 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' ); - // Mock file system checks + // Mock file system checks - only return true for the where.exe path vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); if (pathStr.includes('nvm4w') && pathStr.includes('claude.cmd')) { @@ -638,6 +676,10 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { }); it('should skip where.exe on non-Windows platforms', () => { + // Configure platform mock for macOS + mockIsWindows.mockReturnValue(false); + mockIsMacOS.mockReturnValue(true); + mockIsUnix.mockReturnValue(true); Object.defineProperty(process, 'platform', { value: 'darwin', writable: true @@ -720,7 +762,14 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' ); - vi.mocked(existsSync).mockReturnValue(true); + // IMPORTANT: Only return true for the where.exe path to avoid platform paths taking priority + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('nvm4w') && pathStr.includes('claude.cmd')) { + return true; + } + return false; + }); vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); const result = getToolInfo('claude'); @@ -755,6 +804,10 @@ describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () => { beforeEach(() => { vi.clearAllMocks(); + // Configure platform mocks for Windows + mockIsWindows.mockReturnValue(true); + mockIsMacOS.mockReturnValue(false); + mockIsUnix.mockReturnValue(false); Object.defineProperty(process, 'platform', { value: 'win32', writable: true diff --git a/apps/frontend/src/main/claude-cli-utils.ts b/apps/frontend/src/main/claude-cli-utils.ts index 49a0c49c71..11219e9930 100644 --- a/apps/frontend/src/main/claude-cli-utils.ts +++ b/apps/frontend/src/main/claude-cli-utils.ts @@ -1,6 +1,7 @@ import path from 'path'; import { getAugmentedEnv, getAugmentedEnvAsync } from './env-utils'; import { getToolPath, getToolPathAsync } from './cli-tool-manager'; +import { findNodeJsDirectories, isWindows, getPathDelimiter } from './platform'; export type ClaudeCliInvocation = { command: string; @@ -12,26 +13,53 @@ function ensureCommandDirInPath(command: string, env: Record): R return env; } - const pathSeparator = process.platform === 'win32' ? ';' : ':'; + const pathSeparator = getPathDelimiter(); const commandDir = path.dirname(command); const currentPath = env.PATH || ''; const pathEntries = currentPath.split(pathSeparator); - const normalizedCommandDir = path.normalize(commandDir); - const hasCommandDir = process.platform === 'win32' - ? pathEntries - .map((entry) => path.normalize(entry).toLowerCase()) - .includes(normalizedCommandDir.toLowerCase()) - : pathEntries - .map((entry) => path.normalize(entry)) - .includes(normalizedCommandDir); - - if (hasCommandDir) { + + // Create a Set for O(1) lookup instead of O(n) array includes + const normalizedPathSet = new Set( + isWindows() + ? pathEntries.map((entry) => path.normalize(entry).toLowerCase()) + : pathEntries.map((entry) => path.normalize(entry)) + ); + + const normalizedCommandDir = isWindows() + ? path.normalize(commandDir).toLowerCase() + : path.normalize(commandDir); + + const hasCommandDir = normalizedPathSet.has(normalizedCommandDir); + + // Collect directories to add + let dirsToAdd = hasCommandDir ? [] : [commandDir]; + + // On Windows, if running claude.cmd, also add Node.js directories to PATH + // This is needed because claude.cmd requires node.exe to execute + if (isWindows() && /\.cmd$/i.test(command)) { + const nodeDirs = findNodeJsDirectories(); + dirsToAdd = [...dirsToAdd, ...nodeDirs]; + } + + if (dirsToAdd.length === 0) { + return env; + } + + // Filter out directories already in PATH using the Set for O(1) lookups + const pathEntriesToAdd = dirsToAdd.filter((dir) => { + const normalizedDir = isWindows() + ? path.normalize(dir).toLowerCase() + : path.normalize(dir); + return !normalizedPathSet.has(normalizedDir); + }); + + if (pathEntriesToAdd.length === 0) { return env; } return { ...env, - PATH: [commandDir, currentPath].filter(Boolean).join(pathSeparator), + PATH: [...pathEntriesToAdd, currentPath].filter(Boolean).join(pathSeparator), }; } diff --git a/apps/frontend/src/main/cli-tool-manager.ts b/apps/frontend/src/main/cli-tool-manager.ts index 442bd438a7..fc91ee238d 100644 --- a/apps/frontend/src/main/cli-tool-manager.ts +++ b/apps/frontend/src/main/cli-tool-manager.ts @@ -27,7 +27,10 @@ import os from 'os'; import { promisify } from 'util'; import { app } from 'electron'; import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; -import { isWindows, isMacOS, isUnix, joinPaths, getExecutableExtension } from './platform'; +import { isWindows, isMacOS, isUnix, joinPaths, getExecutableExtension, findNodeJsDirectories, sortNvmVersionDirs } from './platform'; + +// Re-export sortNvmVersionDirs for backwards compatibility with existing imports +export { sortNvmVersionDirs }; import type { ToolDetectionResult } from '../shared/types'; import { findHomebrewPython as findHomebrewPythonUtil } from './utils/homebrew-python'; @@ -187,46 +190,6 @@ export function getClaudeDetectionPaths(homeDir: string): ClaudeDetectionPaths { return { homebrewPaths, platformPaths, nvmVersionsDir }; } -/** - * Sort NVM version directories by semantic version (newest first). - * - * Filters entries to only include directories starting with 'v' (version directories) - * and sorts them in descending order so the newest Node.js version is checked first. - * - * @param entries - Directory entries from readdir with { name, isDirectory() } - * @returns Array of version directory names sorted newest first - * - * @example - * const entries = [ - * { name: 'v18.0.0', isDirectory: () => true }, - * { name: 'v20.0.0', isDirectory: () => true }, - * { name: '.DS_Store', isDirectory: () => false }, - * ]; - * sortNvmVersionDirs(entries); // ['v20.0.0', 'v18.0.0'] - */ -export function sortNvmVersionDirs( - entries: Array<{ name: string; isDirectory(): boolean }> -): string[] { - // Regex to match valid semver directories: v20.0.0, v18.17.1, etc. - // This prevents NaN from malformed versions (e.g., v20.abc.1) breaking sort - const semverRegex = /^v\d+\.\d+\.\d+$/; - - return entries - .filter((entry) => entry.isDirectory() && semverRegex.test(entry.name)) - .sort((a, b) => { - // Parse version numbers: v20.0.0 -> [20, 0, 0] - const vA = a.name.slice(1).split('.').map(Number); - const vB = b.name.slice(1).split('.').map(Number); - // Compare major, minor, patch in order (descending) - for (let i = 0; i < 3; i++) { - const diff = (vB[i] ?? 0) - (vA[i] ?? 0); - if (diff !== 0) return diff; - } - return 0; - }) - .map((entry) => entry.name); -} - /** * Build a ToolDetectionResult from a validation result. * @@ -266,6 +229,31 @@ export function buildClaudeDetectionResult( }; } +/** + * Get additional paths needed for Claude CLI validation + * + * Consolidates the logic for augmenting PATH when validating Claude CLI. + * On Windows with .cmd files, also adds Node.js directories since claude.cmd + * requires node.exe to execute. + * + * @param unquotedCmd - The unquoted command path being validated + * @param cmdDir - The directory containing the command + * @returns Array of additional paths to prepend to environment PATH + */ +function getAdditionalPathsForValidation(unquotedCmd: string, cmdDir: string): string[] { + let additionalPaths = cmdDir && cmdDir !== '.' ? [cmdDir] : []; + + // On Windows, if validating claude.cmd, also add Node.js directories to PATH + // This is needed because claude.cmd requires node.exe to execute + if (isWindows() && /\.cmd$/i.test(unquotedCmd)) { + const nodeDirs = findNodeJsDirectories(); + additionalPaths = [...additionalPaths, ...nodeDirs]; + console.warn('[Claude CLI] Adding Node.js directories to PATH for .cmd validation:', nodeDirs); + } + + return additionalPaths; +} + /** * Centralized CLI Tool Manager * @@ -942,7 +930,10 @@ class CLIToolManager { const needsShell = shouldUseShell(trimmedCmd); const cmdDir = path.dirname(unquotedCmd); - const env = getAugmentedEnv(cmdDir && cmdDir !== '.' ? [cmdDir] : []); + + // Get additional paths (command dir + Node.js dirs on Windows for .cmd files) + const additionalPaths = getAdditionalPathsForValidation(unquotedCmd, cmdDir); + const env = getAugmentedEnv(additionalPaths); let version: string; @@ -1081,7 +1072,10 @@ class CLIToolManager { const needsShell = shouldUseShell(trimmedCmd); const cmdDir = path.dirname(unquotedCmd); - const env = await getAugmentedEnvAsync(cmdDir && cmdDir !== '.' ? [cmdDir] : []); + + // Get additional paths (command dir + Node.js dirs on Windows for .cmd files) + const additionalPaths = getAdditionalPathsForValidation(unquotedCmd, cmdDir); + const env = await getAugmentedEnvAsync(additionalPaths); let stdout: string; diff --git a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts index 8101350050..f663446c5f 100644 --- a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts @@ -19,6 +19,7 @@ import type { ClaudeCodeVersionInfo, ClaudeInstallationList, ClaudeInstallationI import { getToolInfo, configureTools, sortNvmVersionDirs, getClaudeDetectionPaths, type ExecFileAsyncOptionsWithVerbatim } from '../cli-tool-manager'; import { readSettingsFile, writeSettingsFile } from '../settings-utils'; import { isSecurePath } from '../utils/windows-paths'; +import { findNodeJsDirectories } from '../platform'; import semver from 'semver'; const execFileAsync = promisify(execFile); @@ -45,9 +46,19 @@ async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string // Augment PATH with the CLI directory for proper resolution const cliDir = path.dirname(cliPath); + let pathEntries = [cliDir]; + + // On Windows, if validating claude.cmd, also add Node.js directories to PATH + // This is needed because claude.cmd requires node.exe to execute + if (isWindows && /\.cmd$/i.test(cliPath)) { + const nodeDirs = findNodeJsDirectories(); + pathEntries = [...pathEntries, ...nodeDirs]; + console.log('[Claude CLI] Adding Node.js directories to PATH for .cmd validation:', nodeDirs); + } + const env = { ...process.env, - PATH: cliDir ? `${cliDir}${path.delimiter}${process.env.PATH || ''}` : process.env.PATH, + PATH: pathEntries.filter(Boolean).concat(process.env.PATH || '').join(path.delimiter), }; let stdout: string; diff --git a/apps/frontend/src/main/platform/index.ts b/apps/frontend/src/main/platform/index.ts index ea5c14198c..4c7c7d88ea 100644 --- a/apps/frontend/src/main/platform/index.ts +++ b/apps/frontend/src/main/platform/index.ts @@ -18,7 +18,7 @@ import { spawn, ChildProcess } from 'child_process'; import { OS, ShellType, PathConfig, ShellConfig, BinaryDirectories } from './types'; // Re-export from paths.ts for backward compatibility -export { getWindowsShellPaths } from './paths'; +export { getWindowsShellPaths, findNodeJsDirectories, sortNvmVersionDirs } from './paths'; /** * Get the current operating system @@ -502,3 +502,4 @@ export function killProcessGracefully( forceKillTimer.unref(); } } + diff --git a/apps/frontend/src/main/platform/paths.ts b/apps/frontend/src/main/platform/paths.ts index bf0a6b228b..9f7acd4840 100644 --- a/apps/frontend/src/main/platform/paths.ts +++ b/apps/frontend/src/main/platform/paths.ts @@ -10,6 +10,46 @@ import * as os from 'os'; import { existsSync, readdirSync } from 'fs'; import { isWindows, isMacOS, getHomebrewPath, joinPaths, getExecutableExtension } from './index'; +/** + * Sort NVM version directories by semantic version (newest first) + * + * Filters directory entries to only valid semver directories (e.g., v20.0.0) + * and sorts them in descending order so newer versions are tried first. + * + * @param entries - Directory entries from readdirSync with withFileTypes + * @returns Array of directory names sorted by version (newest first) + * + * @example + * const entries = [ + * { name: 'v18.0.0', isDirectory: () => true }, + * { name: 'v20.0.0', isDirectory: () => true }, + * { name: '.DS_Store', isDirectory: () => false }, + * ]; + * sortNvmVersionDirs(entries); // ['v20.0.0', 'v18.0.0'] + */ +export function sortNvmVersionDirs( + entries: Array<{ name: string; isDirectory(): boolean }> +): string[] { + // Regex to match valid semver directories: v20.0.0, v18.17.1, etc. + // This prevents NaN from malformed versions (e.g., v20.abc.1) breaking sort + const semverRegex = /^v\d+\.\d+\.\d+$/; + + return entries + .filter((entry) => entry.isDirectory() && semverRegex.test(entry.name)) + .sort((a, b) => { + // Parse version numbers: v20.0.0 -> [20, 0, 0] + const vA = a.name.slice(1).split('.').map(Number); + const vB = b.name.slice(1).split('.').map(Number); + // Compare major, minor, patch in order (descending) + for (let i = 0; i < 3; i++) { + const diff = (vB[i] ?? 0) - (vA[i] ?? 0); + if (diff !== 0) return diff; + } + return 0; + }) + .map((entry) => entry.name); +} + /** * Resolve Claude CLI executable path * @@ -163,6 +203,87 @@ export function getNpmExecutablePath(): string { return 'npm'; } +/** + * Find Node.js installation directories on Windows + * + * Returns array of potential Node.js bin directories where node.exe might be installed. + * This is needed because claude.cmd (npm global binary) requires node.exe to be in PATH. + * + * Search priority: + * 1. Program Files\nodejs (standard installer location) + * 2. %APPDATA%\npm (user-level npm globals - may contain node.exe with nvm-windows) + * 3. NVM for Windows installation directories + * 4. Scoop, Chocolatey installations + * + * @returns Array of existing Node.js bin directories + */ +export function findNodeJsDirectories(): string[] { + if (!isWindows()) { + return []; + } + + const homeDir = os.homedir(); + // Use environment variables only - no hardcoded fallbacks + const programFiles = process.env.ProgramFiles; + const programFilesX86 = process.env['ProgramFiles(x86)']; + const appData = process.env.APPDATA; + const programData = process.env.ProgramData; + // NVM for Windows custom installation paths + const nvmHome = process.env.NVM_HOME; + const nvmSymlink = process.env.NVM_SYMLINK; + + const candidates: string[] = []; + + // Standard Node.js installer location (only if env vars are set) + if (programFiles) { + candidates.push(joinPaths(programFiles, 'nodejs')); + candidates.push(joinPaths(programFiles, 'nvm')); + } + if (programFilesX86) { + candidates.push(joinPaths(programFilesX86, 'nodejs')); + } + + // User-level npm global directory (may contain node.exe with nvm-windows) + if (appData) { + candidates.push(joinPaths(appData, 'npm')); + candidates.push(joinPaths(appData, 'nvm')); + } + + // Scoop installation (uses home directory, always available) + candidates.push(joinPaths(homeDir, 'scoop', 'apps', 'nodejs', 'current')); + + // Chocolatey installation + if (programData) { + candidates.push(joinPaths(programData, 'chocolatey', 'bin')); + } + + // NVM for Windows: prefer NVM_SYMLINK (active version) and NVM_HOME (custom install path) + // Fall back to default APPDATA/nvm location if env vars not set + if (nvmSymlink) { + // NVM_SYMLINK points directly to the active Node.js version + candidates.push(nvmSymlink); + } + + // Determine NVM root directory: prefer NVM_HOME, fall back to APPDATA/nvm + const nvmPath = nvmHome || (appData ? joinPaths(appData, 'nvm') : null); + if (nvmPath && existsSync(nvmPath)) { + try { + // Find all version directories (e.g., v20.0.0, v18.17.1) + const entries = readdirSync(nvmPath, { withFileTypes: true }); + const versionNames = sortNvmVersionDirs(entries); + const versionDirs = versionNames.map((name) => joinPaths(nvmPath, name)); + + candidates.push(...versionDirs); + } catch (error) { + // Log error for debugging but don't fail - Node.js detection is non-critical + console.warn('[findNodeJsDirectories] Failed to read NVM directory:', error); + } + } + + // Return only existing directories + return candidates.filter((dir) => existsSync(dir)); +} + /** * Get all Windows shell paths for terminal selection *