Skip to content

Conversation

@g1331
Copy link
Contributor

@g1331 g1331 commented Jan 14, 2026

Base Branch

  • This PR targets the develop branch (required for all feature/fix PRs)
  • This PR targets main (hotfix only - maintainers)

Description

Fixes Windows installation scan failures when Claude Code is installed via npm under nvm4w (paths with spaces). The scan now strips wrapping quotes from where output, skips extensionless shim candidates on Windows, and validates .cmd/.bat via cmd.exe with windowsVerbatimArguments. Adds a regression test for quoted .cmd paths + extensionless shims.

Related Issue

Closes #1050

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 📚 Documentation
  • ♻️ Refactor
  • 🧪 Test

Area

  • Frontend
  • Backend
  • Fullstack

Commit Message Format

Follow conventional commits: <type>: <subject>

Types: feat, fix, docs, style, refactor, test, chore

Example: feat: add user authentication system

Checklist

  • I've synced with develop branch
  • I've tested my changes locally
  • I've followed the code principles (SOLID, DRY, KISS)
  • My PR is small and focused (< 400 lines ideally)

CI/Testing Requirements

  • All CI checks pass
  • All existing tests pass
  • New features include test coverage
  • Bug fixes include regression tests

Screenshots

N/A (no UI changes)

Feature Toggle

  • Behind localStorage flag: use_feature_name
  • Behind settings toggle
  • Behind environment variable/config
  • N/A - Feature is complete and ready for all users

Breaking Changes

Breaking: No

Details: N/A

Summary by CodeRabbit

  • Tests

    • Added Windows-specific test suite for Claude Code installations.
  • Bug Fixes

    • Improved path validation, normalization, and deduplication for Claude CLI installations.
    • Enhanced Windows compatibility for installation detection.
    • Strengthened security checks for path handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Adds Windows-specific test suite for Claude Code CLI installation scanning with mocked IPC handlers and file system operations. Updates Claude CLI validation logic to normalize paths, strip quotes, handle Windows executable extensions, deduplicate installations, and fix command execution via cmd.exe with proper verbatim arguments.

Changes

Cohort / File(s) Summary
Test Suite for Windows CLI Scanning
apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts
Introduces Windows-only test suite (skipped on non-Windows) with temporary directory setup, mocked IPC main, child_process mocks, and exec behavior simulation. Validates installation data structure, where command invocation, shim validation skipping, and cmd.exe invocation with windowsVerbatimArguments flag.
Claude CLI Handler Improvements
apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
Adds internal helper to strip surrounding quotes from paths. Centralizes path normalization and unquoting for CLI validation, system PATH discovery, and installation scanning. Implements Windows-aware deduplication using normalized keys, validates only Windows executable extensions (.cmd, .bat, .exe), and enforces proper quoting for cmd.exe invocation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested labels

bug, area/frontend, os/windows, refactor, size/M

Suggested reviewers

  • AndyMik90

Poem

🐰 A rabbit bounces through the Windows CMD,
With quoted paths and .cmd files, oh what joy!
The installations now scan without dread,
No more garbled stderr or paths that mislead,
Deduplication and normalization saved the day! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: hardening Windows Claude Code installations scan to handle paths with spaces and extensionless candidates.
Linked Issues check ✅ Passed The PR directly addresses all requirements from issue #1050: strips wrapping quotes from where output, skips extensionless shim candidates on Windows, validates .cmd/.bat via cmd.exe with windowsVerbatimArguments, and includes regression tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing Windows Claude Code installation scanning; no unrelated modifications detected beyond the stated objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5aecd and e0e42ca.

📒 Files selected for processing (2)
  • apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts
  • apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/frontend/src/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/frontend/src/**/*.{tsx,ts}: Use i18n translation keys for all user-facing text in the frontend. All labels, buttons, messages must use translation keys from react-i18next with namespace:section.key format (e.g., 'navigation:items.githubPRs').
Never use hardcoded strings for UI text in JSX/TSX files. Always use translation keys via useTranslation() hook.

Files:

  • apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts
  • apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
apps/frontend/**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.

Files:

  • apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts
  • apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts
🧬 Code graph analysis (1)
apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts (1)
apps/frontend/src/shared/types/cli.ts (1)
  • ClaudeInstallationInfo (57-66)
🔇 Additional comments (12)
apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts (5)

9-31: LGTM!

The MockIpcMain class provides a clean abstraction for testing IPC handlers. The implementation correctly simulates Electron's ipcMain.handle behavior with proper async invocation support.


39-54: Well-documented mock pattern.

The comment at lines 39-41 clearly explains why the custom promisify implementation is necessary. This ensures the mock maintains the same behavior as Node's execFile when used with promisify().


64-79: LGTM!

The mocks for settings-utils and cli-tool-manager correctly isolate the test to focus on Windows system PATH discovery via the where command.


102-151: Comprehensive mock implementation.

The execFileMock implementation correctly simulates:

  1. where claude returning quoted paths (the bug scenario)
  2. Extensionless shim validation failing with ENOENT
  3. cmd.exe requiring windowsVerbatimArguments: true for paths with spaces

Line 141's check ensures the command line uses the unquoted path, validating the core fix.


163-200: Thorough test assertions.

The test comprehensively validates the fix:

  • Extensionless shims are skipped (negative assertion at lines 178-183)
  • Only the .cmd file is detected as a valid installation
  • cmd.exe is invoked with windowsVerbatimArguments: true
apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts (7)

32-39: LGTM!

The stripWrappingQuotes helper correctly handles:

  • Matching double quotes
  • Matching single quotes
  • Mismatched quotes (no stripping)
  • Whitespace trimming

63-75: Correct Windows cmd.exe invocation pattern.

The combination of:

  • /s flag to strip outer quotes while preserving inner quotes
  • Double-quoted command line format ""${unquotedPath}" --version"
  • windowsVerbatimArguments: true to prevent Node's argument escaping

This correctly handles paths with spaces on Windows.


115-117: Correct Windows path normalization.

The normalized active key uses lowercase on Windows for case-insensitive comparison, matching the seenKey computation in addInstallation. This ensures consistent isActive detection regardless of path casing.


125-128: Essential fix for Windows extensionless shims.

Skipping extensionless candidates (e.g., claude without .cmd/.bat/.exe extension) prevents ENOENT errors when CreateProcess cannot execute these shim files directly on Windows.


130-148: Consistent normalization and deduplication.

The approach of using unquotedPath for filesystem operations and normalizedPath (resolved absolute) for storage and comparison ensures:

  • Actual file operations use the cleaned input path
  • Stored paths are canonical absolute paths
  • Deduplication works correctly with case-insensitive keys on Windows

161-174: LGTM!

The where/which output parsing correctly:

  • Uses utf-8 encoding for consistent string handling
  • Splits on \r?\n to handle both Windows and Unix line endings
  • Filters out empty lines from trailing newlines

219-222: Good fallback behavior.

Automatically marking the first valid installation as active when none is explicitly configured provides a sensible default user experience.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sentry
Copy link

sentry bot commented Jan 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@g1331
Copy link
Contributor Author

g1331 commented Jan 14, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@g1331 g1331 marked this pull request as ready for review January 14, 2026 09:52
@AndyMik90 AndyMik90 self-assigned this Jan 14, 2026
@g1331 g1331 closed this Jan 16, 2026
@g1331
Copy link
Contributor Author

g1331 commented Jan 16, 2026

The latest version of the dev branch no longer has this issue

@g1331 g1331 deleted the fix/windows-claude-installations-scan branch January 16, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: Clicking Claude Code triggers installation scan failures (Found 0 installations)

2 participants