fix: improve CLI tool detection and add Claude CLI path settings (#393)

* fix(changelog): improve CLI tool detection for git and Claude Fixes changelog generation failure with FileNotFoundError when using GitHub issues option to pull commits. Changes: - Replace execSync with execFileSync(getToolPath('git')) in git-integration.ts for cross-platform compatibility and security - Add Claude CLI to centralized CLI Tool Manager with 4-tier detection - Remove 47 lines of duplicate Claude CLI detection from changelog-service.ts - Add dynamic npm prefix detection in env-utils.ts for all npm setups Benefits: - Cross-platform compatibility (no shell injection risk) - Consistent CLI tool detection across codebase - Works with standard npm, nvm, nvm-windows, and custom installations * feat: add Claude CLI path configuration to Settings UI Integrates Claude CLI path configuration into the Settings UI, building on the Claude CLI detection infrastructure from PR #391. Changes: - Add Claude CLI path input field to Settings UI - Expose Claude CLI detection through IPC handlers - Add i18n translations (English/French) for Claude CLI settings - Update type definitions for Claude CLI configuration - Add browser mock for Claude CLI detection This commit combines: - PR #391's comprehensive Claude CLI detection (detectClaude, validateClaude) - PR #392's Settings UI enhancements 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * docs: clarify npm prefix detection is cross-platform - Removed misleading Windows-specific comment on line 60 - Updated comment at call site (line 101) to explicitly state cross-platform support - Clarifies that getNpmGlobalPrefix() works on all platforms (macOS, Linux, Windows) Addresses CodeRabbit feedback * fix: improve npm global prefix detection for cross-platform support - Use npm.cmd on Windows with shell option for proper command resolution - Return prefix/bin on macOS/Linux (where npm globals are actually installed) - Return raw prefix on Windows (correct location for npm globals) - Normalize path and verify existence before returning - Preserve existing encoding, timeout, and error handling Addresses CodeRabbit feedback on platform-specific npm prefix handling --------- Co-authored-by: Joe Slitzker <jslitzker@gmail.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-28 16:30:54 -06:00
parent 68548e33c8
commit aaa83131f4
12 changed files with 249 additions and 63 deletions
@@ -1,9 +1,9 @@
 import { EventEmitter } from 'events';
 import * as path from 'path';
-import * as os from 'os';
 import { existsSync, readFileSync, writeFileSync } from 'fs';
 import { app } from 'electron';
 import { AUTO_BUILD_PATHS, DEFAULT_CHANGELOG_PATH } from '../../shared/constants';
+import { getToolPath } from '../cli-tool-manager';
 import type {
  ChangelogTask,
  TaskSpecContent,
@@ -37,7 +37,7 @@ import { getConfiguredPythonPath } from '../python-env-manager';
 export class ChangelogService extends EventEmitter {
  // Python path will be configured by pythonEnvManager after venv is ready
  private _pythonPath: string | null = null;
-  private claudePath: string = 'claude';
+  private claudePath: string;
  private autoBuildSourcePath: string = '';
  private cachedEnv: Record<string, string> | null = null;
  private debugEnabled: boolean | null = null;
@@ -46,48 +46,9 @@ export class ChangelogService extends EventEmitter {

  constructor() {
    super();
-    this.detectClaudePath();
-    this.debug('ChangelogService initialized');
-  }
-
-  /**
-   * Detect the full path to the claude CLI
-   * Electron apps don't inherit shell PATH, so we need to find it explicitly
-   */
-  private detectClaudePath(): void {
-    const homeDir = os.homedir();
-
-    // Platform-specific possible paths
-    const possiblePaths = process.platform === 'win32'
-      ? [
-          // Windows paths
-          path.join(homeDir, 'AppData', 'Local', 'Programs', 'claude', 'claude.exe'),
-          path.join(homeDir, 'AppData', 'Roaming', 'npm', 'claude.cmd'),
-          path.join(homeDir, '.local', 'bin', 'claude.exe'),
-          'C:\\Program Files\\Claude\\claude.exe',
-          'C:\\Program Files (x86)\\Claude\\claude.exe',
-          // Also check if claude is in system PATH
-          'claude'
-        ]
-      : [
-          // Unix paths (macOS/Linux)
-          '/usr/local/bin/claude',
-          '/opt/homebrew/bin/claude',
-          path.join(homeDir, '.local/bin/claude'),
-          path.join(homeDir, 'bin/claude'),
-          // Also check if claude is in system PATH
-          'claude'
-        ];
-
-    for (const claudePath of possiblePaths) {
-      if (claudePath === 'claude' || existsSync(claudePath)) {
-        this.claudePath = claudePath;
-        this.debug('Claude CLI found at:', claudePath);
-        return;
-      }
-    }
-
-    this.debug('Claude CLI not found in common locations, using default');
+    // Use centralized CLI tool manager for Claude detection
+    this.claudePath = getToolPath('claude');
+    this.debug('ChangelogService initialized with Claude CLI:', this.claudePath);
  }

  /**
@@ -1,4 +1,4 @@
-import { execSync, execFileSync } from 'child_process';
+import { execFileSync } from 'child_process';
 import type {
  GitBranchInfo,
  GitTagInfo,
@@ -89,8 +89,9 @@ export function getBranches(projectPath: string, debugEnabled = false): GitBranc
 export function getTags(projectPath: string, debugEnabled = false): GitTagInfo[] {
  try {
    // Get tags sorted by creation date (newest first)
-    const output = execSync(
-      'git tag -l --sort=-creatordate --format="%(refname:short)|%(creatordate:iso-strict)|%(objectname:short)"',
+    const output = execFileSync(
+      getToolPath('git'),
+      ['tag', '-l', '--sort=-creatordate', '--format=%(refname:short)|%(creatordate:iso-strict)|%(objectname:short)'],
      {
        cwd: projectPath,
        encoding: 'utf-8'
@@ -179,40 +180,40 @@ export function getCommits(
  try {
    // Build the git log command based on options
    const format = '%h|%H|%s|%an|%ae|%aI';
-    let command = `git log --pretty=format:"${format}"`;
+    const args = ['log', `--pretty=format:${format}`];

    // Add merge commit handling
    if (!options.includeMergeCommits) {
-      command += ' --no-merges';
+      args.push('--no-merges');
    }

    // Add range/filters based on type
    switch (options.type) {
      case 'recent':
-        command += ` -n ${options.count || 25}`;
+        args.push('-n', String(options.count || 25));
        break;
      case 'since-date':
        if (options.sinceDate) {
-          command += ` --since="${options.sinceDate}"`;
+          args.push(`--since=${options.sinceDate}`);
        }
        break;
      case 'tag-range':
        if (options.fromTag) {
          const toRef = options.toTag || 'HEAD';
-          command += ` ${options.fromTag}..${toRef}`;
+          args.push(`${options.fromTag}..${toRef}`);
        }
        break;
      case 'since-version':
        // Get all commits since the specified version/tag up to HEAD
        if (options.fromTag) {
-          command += ` ${options.fromTag}..HEAD`;
+          args.push(`${options.fromTag}..HEAD`);
        }
        break;
    }

-    debug(debugEnabled, 'Getting commits with command:', command);
+    debug(debugEnabled, 'Getting commits with args:', args);

-    const output = execSync(command, {
+    const output = execFileSync(getToolPath('git'), args, {
      cwd: projectPath,
      encoding: 'utf-8',
      maxBuffer: 10 * 1024 * 1024 // 10MB buffer for large histories
@@ -236,11 +237,11 @@ export function getBranchDiffCommits(
  try {
    const format = '%h|%H|%s|%an|%ae|%aI';
    // Get commits in compareBranch that are not in baseBranch
-    const command = `git log --pretty=format:"${format}" --no-merges ${options.baseBranch}..${options.compareBranch}`;
+    const args = ['log', `--pretty=format:${format}`, '--no-merges', `${options.baseBranch}..${options.compareBranch}`];

-    debug(debugEnabled, 'Getting branch diff commits with command:', command);
+    debug(debugEnabled, 'Getting branch diff commits with args:', args);

-    const output = execSync(command, {
+    const output = execFileSync(getToolPath('git'), args, {
      cwd: projectPath,
      encoding: 'utf-8',
      maxBuffer: 10 * 1024 * 1024
@@ -1,7 +1,7 @@
 /**
 * CLI Tool Manager
 *
- * Centralized management for CLI tools (Python, Git, GitHub CLI) used throughout
+ * Centralized management for CLI tools (Python, Git, GitHub CLI, Claude CLI) used throughout
 * the application. Provides intelligent multi-level detection with user
 * configuration support.
 *
@@ -23,6 +23,7 @@
 import { execSync, execFileSync } from 'child_process';
 import { existsSync } from 'fs';
 import path from 'path';
+import os from 'os';
 import { app } from 'electron';
 import { findExecutable } from './env-utils';
 import type { ToolDetectionResult } from '../shared/types';
@@ -30,7 +31,7 @@ import type { ToolDetectionResult } from '../shared/types';
 /**
 * Supported CLI tools managed by this system
 */
-export type CLITool = 'python' | 'git' | 'gh';
+export type CLITool = 'python' | 'git' | 'gh' | 'claude';

 /**
 * User configuration for CLI tool paths
@@ -40,6 +41,7 @@ export interface ToolConfig {
  pythonPath?: string;
  gitPath?: string;
  githubCLIPath?: string;
+  claudePath?: string;
 }

 /**
@@ -147,6 +149,8 @@ class CLIToolManager {
        return this.detectGit();
      case 'gh':
        return this.detectGitHubCLI();
+      case 'claude':
+        return this.detectClaude();
      default:
        return {
          found: false,
@@ -440,6 +444,111 @@ class CLIToolManager {
    };
  }

+  /**
+   * Detect Claude CLI with multi-level priority
+   *
+   * Priority order:
+   * 1. User configuration
+   * 2. Homebrew claude (macOS)
+   * 3. System PATH
+   * 4. Windows/macOS/Linux standard locations
+   *
+   * @returns Detection result for Claude CLI
+   */
+  private detectClaude(): ToolDetectionResult {
+    // 1. User configuration
+    if (this.userConfig.claudePath) {
+      const validation = this.validateClaude(this.userConfig.claudePath);
+      if (validation.valid) {
+        return {
+          found: true,
+          path: this.userConfig.claudePath,
+          version: validation.version,
+          source: 'user-config',
+          message: `Using user-configured Claude CLI: ${this.userConfig.claudePath}`,
+        };
+      }
+      console.warn(
+        `[Claude CLI] User-configured path invalid: ${validation.message}`
+      );
+    }
+
+    // 2. Homebrew (macOS)
+    if (process.platform === 'darwin') {
+      const homebrewPaths = [
+        '/opt/homebrew/bin/claude', // Apple Silicon
+        '/usr/local/bin/claude', // Intel Mac
+      ];
+
+      for (const claudePath of homebrewPaths) {
+        if (existsSync(claudePath)) {
+          const validation = this.validateClaude(claudePath);
+          if (validation.valid) {
+            return {
+              found: true,
+              path: claudePath,
+              version: validation.version,
+              source: 'homebrew',
+              message: `Using Homebrew Claude CLI: ${claudePath}`,
+            };
+          }
+        }
+      }
+    }
+
+    // 3. System PATH (augmented)
+    const claudePath = findExecutable('claude');
+    if (claudePath) {
+      const validation = this.validateClaude(claudePath);
+      if (validation.valid) {
+        return {
+          found: true,
+          path: claudePath,
+          version: validation.version,
+          source: 'system-path',
+          message: `Using system Claude CLI: ${claudePath}`,
+        };
+      }
+    }
+
+    // 4. Platform-specific standard locations
+    const homeDir = os.homedir();
+    const platformPaths = process.platform === 'win32'
+      ? [
+          path.join(homeDir, 'AppData', 'Local', 'Programs', 'claude', 'claude.exe'),
+          path.join(homeDir, 'AppData', 'Roaming', 'npm', 'claude.cmd'),
+          path.join(homeDir, '.local', 'bin', 'claude.exe'),
+          'C:\\Program Files\\Claude\\claude.exe',
+          'C:\\Program Files (x86)\\Claude\\claude.exe',
+        ]
+      : [
+          path.join(homeDir, '.local', 'bin', 'claude'),
+          path.join(homeDir, 'bin', 'claude'),
+        ];
+
+    for (const claudePath of platformPaths) {
+      if (existsSync(claudePath)) {
+        const validation = this.validateClaude(claudePath);
+        if (validation.valid) {
+          return {
+            found: true,
+            path: claudePath,
+            version: validation.version,
+            source: 'system-path',
+            message: `Using Claude CLI: ${claudePath}`,
+          };
+        }
+      }
+    }
+
+    // 5. Not found
+    return {
+      found: false,
+      source: 'fallback',
+      message: 'Claude CLI not found. Install from https://claude.ai/download',
+    };
+  }
+
  /**
   * Validate Python version and availability
   *
@@ -557,6 +666,37 @@ class CLIToolManager {
    }
  }

+  /**
+   * Validate Claude CLI availability and version
+   *
+   * @param claudeCmd - The Claude CLI command to validate
+   * @returns Validation result with version information
+   */
+  private validateClaude(claudeCmd: string): ToolValidation {
+    try {
+      const version = execFileSync(claudeCmd, ['--version'], {
+        encoding: 'utf-8',
+        timeout: 5000,
+        windowsHide: true,
+      }).trim();
+
+      // Claude CLI version output format: "claude-code version X.Y.Z" or similar
+      const match = version.match(/(\d+\.\d+\.\d+)/);
+      const versionStr = match ? match[1] : version.split('\n')[0];
+
+      return {
+        valid: true,
+        version: versionStr,
+        message: `Claude CLI ${versionStr} is available`,
+      };
+    } catch (error) {
+      return {
+        valid: false,
+        message: `Failed to validate Claude CLI: ${error instanceof Error ? error.message : String(error)}`,
+      };
+    }
+  }
+
  /**
   * Get bundled Python path for packaged apps
   *
@@ -12,6 +12,49 @@
 import * as os from 'os';
 import * as path from 'path';
 import * as fs from 'fs';
+import { execFileSync } from 'child_process';
+
+/**
+ * Get npm global prefix directory dynamically
+ *
+ * Runs `npm config get prefix` to find where npm globals are installed.
+ * Works with standard npm, nvm-windows, nvm, and custom installations.
+ *
+ * On Windows: returns the prefix directory (e.g., C:\Users\user\AppData\Roaming\npm)
+ * On macOS/Linux: returns prefix/bin (e.g., /usr/local/bin)
+ *
+ * @returns npm global binaries directory, or null if npm not available or path doesn't exist
+ */
+function getNpmGlobalPrefix(): string | null {
+  try {
+    // On Windows, use npm.cmd for proper command resolution
+    const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';
+
+    const rawPrefix = execFileSync(npmCommand, ['config', 'get', 'prefix'], {
+      encoding: 'utf-8',
+      timeout: 3000,
+      windowsHide: true,
+      shell: process.platform === 'win32', // Enable shell on Windows for .cmd resolution
+    }).trim();
+
+    if (!rawPrefix) {
+      return null;
+    }
+
+    // On non-Windows platforms, npm globals are installed in prefix/bin
+    // On Windows, they're installed directly in the prefix directory
+    const binPath = process.platform === 'win32'
+      ? rawPrefix
+      : path.join(rawPrefix, 'bin');
+
+    // Normalize and verify the path exists
+    const normalizedPath = path.normalize(binPath);
+
+    return fs.existsSync(normalizedPath) ? normalizedPath : null;
+  } catch {
+    return null;
+  }
+}

 /**
 * Common binary directories that should be in PATH
@@ -75,6 +118,12 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record<string, stri
    }
  }

+  // Add npm global prefix dynamically (cross-platform: works with standard npm, nvm, nvm-windows)
+  const npmPrefix = getNpmGlobalPrefix();
+  if (npmPrefix && !currentPathSet.has(npmPrefix) && fs.existsSync(npmPrefix)) {
+    pathsToAdd.push(npmPrefix);
+  }
+
  // Add user-requested additional paths
  if (additionalPaths) {
    for (const p of additionalPaths) {
@@ -134,6 +134,7 @@ export function registerSettingsHandlers(
        pythonPath: settings.pythonPath,
        gitPath: settings.gitPath,
        githubCLIPath: settings.githubCLIPath,
+        claudePath: settings.claudePath,
      });

      return { success: true, data: settings as AppSettings };
@@ -159,12 +160,14 @@ export function registerSettingsHandlers(
        if (
          settings.pythonPath !== undefined ||
          settings.gitPath !== undefined ||
-          settings.githubCLIPath !== undefined
+          settings.githubCLIPath !== undefined ||
+          settings.claudePath !== undefined
        ) {
          configureTools({
            pythonPath: newSettings.pythonPath,
            gitPath: newSettings.gitPath,
            githubCLIPath: newSettings.githubCLIPath,
+            claudePath: newSettings.claudePath,
          });
        }

@@ -190,6 +193,7 @@ export function registerSettingsHandlers(
      python: ReturnType<typeof getToolInfo>;
      git: ReturnType<typeof getToolInfo>;
      gh: ReturnType<typeof getToolInfo>;
+      claude: ReturnType<typeof getToolInfo>;
    }>> => {
      try {
        return {
@@ -198,6 +202,7 @@ export function registerSettingsHandlers(
            python: getToolInfo('python'),
            git: getToolInfo('git'),
            gh: getToolInfo('gh'),
+            claude: getToolInfo('claude'),
          },
        };
      } catch (error) {
@@ -18,6 +18,7 @@ export interface SettingsAPI {
    python: ToolDetectionResult;
    git: ToolDetectionResult;
    gh: ToolDetectionResult;
+    claude: ToolDetectionResult;
  }>>;

  // App Info
@@ -42,6 +43,7 @@ export const createSettingsAPI = (): SettingsAPI => ({
    python: ToolDetectionResult;
    git: ToolDetectionResult;
    gh: ToolDetectionResult;
+    claude: ToolDetectionResult;
  }>> =>
    ipcRenderer.invoke(IPC_CHANNELS.SETTINGS_GET_CLI_TOOLS_INFO),

@@ -95,6 +95,7 @@ export function GeneralSettings({ settings, onSettingsChange, section }: General
    python: ToolDetectionResult;
    git: ToolDetectionResult;
    gh: ToolDetectionResult;
+    claude: ToolDetectionResult;
  } | null>(null);
  const [isLoadingTools, setIsLoadingTools] = useState(false);

@@ -104,7 +105,7 @@ export function GeneralSettings({ settings, onSettingsChange, section }: General
      setIsLoadingTools(true);
      window.electronAPI
        .getCliToolsInfo()
-        .then((result: { success: boolean; data?: { python: ToolDetectionResult; git: ToolDetectionResult; gh: ToolDetectionResult } }) => {
+        .then((result: { success: boolean; data?: { python: ToolDetectionResult; git: ToolDetectionResult; gh: ToolDetectionResult; claude: ToolDetectionResult } }) => {
          if (result.success && result.data) {
            setToolsInfo(result.data);
          }
@@ -303,6 +304,24 @@ export function GeneralSettings({ settings, onSettingsChange, section }: General
            />
          )}
        </div>
+        <div className="space-y-3">
+          <Label htmlFor="claudePath" className="text-sm font-medium text-foreground">{t('general.claudePath')}</Label>
+          <p className="text-sm text-muted-foreground">{t('general.claudePathDescription')}</p>
+          <Input
+            id="claudePath"
+            placeholder={t('general.claudePathPlaceholder')}
+            className="w-full max-w-lg"
+            value={settings.claudePath || ''}
+            onChange={(e) => onSettingsChange({ ...settings, claudePath: e.target.value })}
+          />
+          {!settings.claudePath && (
+            <ToolDetectionDisplay
+              info={toolsInfo?.claude || null}
+              isLoading={isLoadingTools}
+              t={t}
+            />
+          )}
+        </div>
        <div className="space-y-3">
          <Label htmlFor="autoBuildPath" className="text-sm font-medium text-foreground">{t('general.autoClaudePath')}</Label>
          <p className="text-sm text-muted-foreground">{t('general.autoClaudePathDescription')}</p>
@@ -18,7 +18,8 @@ export const settingsMock = {
    data: {
      python: { found: false, source: 'fallback' as const, message: 'Not available in browser mode' },
      git: { found: false, source: 'fallback' as const, message: 'Not available in browser mode' },
-      gh: { found: false, source: 'fallback' as const, message: 'Not available in browser mode' }
+      gh: { found: false, source: 'fallback' as const, message: 'Not available in browser mode' },
+      claude: { found: false, source: 'fallback' as const, message: 'Not available in browser mode' }
    }
  }),

@@ -74,6 +74,9 @@
    "githubCLIPath": "GitHub CLI Path",
    "githubCLIPathDescription": "Path to GitHub CLI (gh) executable (leave empty for auto-detection)",
    "githubCLIPathPlaceholder": "gh (default)",
+    "claudePath": "Claude CLI Path",
+    "claudePathDescription": "Path to Claude CLI executable (leave empty for auto-detection)",
+    "claudePathPlaceholder": "claude (default)",
    "detectedPath": "Auto-detected",
    "detectedVersion": "Version",
    "detectedSource": "Source",
@@ -74,6 +74,9 @@
    "githubCLIPath": "Chemin GitHub CLI",
    "githubCLIPathDescription": "Chemin vers l'exécutable GitHub CLI (gh) (laisser vide pour détection automatique)",
    "githubCLIPathPlaceholder": "gh (par défaut)",
+    "claudePath": "Chemin Claude CLI",
+    "claudePathDescription": "Chemin vers l'exécutable Claude CLI (laisser vide pour détection automatique)",
+    "claudePathPlaceholder": "claude (par défaut)",
    "detectedPath": "Détecté automatiquement",
    "detectedVersion": "Version",
    "detectedSource": "Source",
@@ -241,6 +241,7 @@ export interface ElectronAPI {
    python: import('./cli').ToolDetectionResult;
    git: import('./cli').ToolDetectionResult;
    gh: import('./cli').ToolDetectionResult;
+    claude: import('./cli').ToolDetectionResult;
  }>>;

  // Dialog operations
@@ -86,6 +86,7 @@ export interface AppSettings {
  pythonPath?: string;
  gitPath?: string;
  githubCLIPath?: string;
+  claudePath?: string;
  autoBuildPath?: string;
  autoUpdateAutoBuild: boolean;
  autoNameTerminals: boolean;