-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(windows): add Node.js path detection for claude.cmd validation #1277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The construction of the
Suggested change
|
||||||
| }; | ||||||
|
|
||||||
| let stdout: string; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ import * as path from 'path'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as os from 'os'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { existsSync, readdirSync } from 'fs'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { isWindows, isMacOS, getHomebrewPath, joinPaths, getExecutableExtension } from './index'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { sortNvmVersionDirs } from '../cli-tool-manager'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Resolve Claude CLI executable path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -163,6 +164,65 @@ 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(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const candidates: string[] = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Standard Node.js installer location | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths('C:\\Program Files', 'nodejs'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths('C:\\Program Files (x86)', 'nodejs'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // User-level npm global directory (may contain node.exe with nvm-windows) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths(homeDir, 'AppData', 'Roaming', 'npm'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // NVM for Windows default location | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths(homeDir, 'AppData', 'Roaming', 'nvm'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths('C:\\Program Files', 'nvm'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Scoop installation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths(homeDir, 'scoop', 'apps', 'nodejs', 'current'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Chocolatey installation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| joinPaths('C:\\ProgramData', 'chocolatey', 'bin'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+189
to
+203
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function uses hardcoded paths for
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // For NVM, we need to find the active Node.js version directory | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nvmPath = joinPaths(homeDir, 'AppData', 'Roaming', 'nvm'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic for adding Node.js directories to the path for
.cmdfiles on Windows is duplicated invalidateClaudeAsync(lines 1100-1106). To improve maintainability and reduce code duplication, this logic could be extracted into a private helper function. This function could then be called from bothvalidateClaudeandvalidateClaudeAsync.Here is an example of what that helper function could look like: