fix(cli): quote Windows paths with spaces in Claude CLI validation #768
Conversation
When validating Claude CLI on Windows with shell: true (for .cmd/.bat files), paths containing spaces were being split by cmd.exe. For example: `D:\Program Files\nodejs\claude.cmd` was parsed as command `D:\Program` with arguments `Files\nodejs\claude.cmd --version`. This fix adds proper quoting for paths with spaces when using shell: true, preventing cmd.exe from incorrectly splitting the path at space characters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
📝 WalkthroughWalkthroughQuote the Claude CLI executable on Windows when its path contains spaces and use the quoted command for both synchronous and asynchronous validation invocations ( Changes
Sequence Diagram(s)(No sequence diagrams generated — changes are a small CLI invocation adjustment and do not introduce a new multi-component control flow.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)apps/frontend/src/**/*.{ts,tsx,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-01-04T23:59:48.743ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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. Comment |
Summary of ChangesHello @jeremykit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a bug in the Claude CLI validation process on Windows. Previously, paths containing spaces for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug on Windows where Claude CLI validation would fail for paths containing spaces. The fix involves quoting the command path when shell: true is used, which is correct. My review points out that while the synchronous validateClaude function is fixed, its asynchronous counterpart validateClaudeAsync was overlooked and still contains the same bug. It's important to apply the fix there as well for consistent and correct behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/cli-tool-manager.ts (1)
1030-1041: Apply the same quoting fix to the async version.The sync version (
validateClaude) now handles Windows paths with spaces (lines 926-932), but this async version lacks the same fix. WhenvalidateClaudeAsyncis called viadetectClaudeAsync, Windows paths with spaces will still fail.🐛 Proposed fix: Apply consistent quoting logic
private async validateClaudeAsync(claudeCmd: string): Promise<ToolValidation> { try { const needsShell = process.platform === 'win32' && (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); + // When using shell: true on Windows, paths with spaces must be quoted + // to prevent cmd.exe from splitting at spaces + const cmdToRun = needsShell && claudeCmd.includes(' ') + ? `"${claudeCmd}"` + : claudeCmd; + - const { stdout } = await execFileAsync(claudeCmd, ['--version'], { + const { stdout } = await execFileAsync(cmdToRun, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, shell: needsShell, env: await getAugmentedEnvAsync(), });
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/cli-tool-manager.ts:
- Around line 926-932: The code currently wraps claudeCmd in quotes when
needsShell && claudeCmd.includes(' '), which can double-quote an already-quoted
path; update the logic that sets cmdToRun so it only adds surrounding quotes if
claudeCmd is not already enclosed in matching single or double quotes (check
claudeCmd.startsWith('"') && claudeCmd.endsWith('"') or the single-quote
equivalent) and otherwise leave claudeCmd as-is; keep the same execFileSync call
using cmdToRun so behavior is unchanged except preventing double-quoting.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
1 issue(s) must be addressed (1 required, 0 recommended), 1 suggestions
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
Findings Summary
- High: 1 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (2 selected of 2 total)
🟠 [7eea1a85ba75] [HIGH] Missing path quoting in async validateClaudeAsync() method
📁 apps/frontend/src/main/cli-tool-manager.ts:1035
The sync method validateClaude() (lines 928-930) was fixed to quote Windows paths with spaces when using shell: true, but the async counterpart validateClaudeAsync() was NOT updated with the same fix. When claudeCmd contains spaces (e.g., 'D:\Program Files\nodejs\claude.cmd') and needsShell is true, the async method will fail because cmd.exe splits the path at spaces. The async method is used by getToolPathAsync() which is called from the Electron main process.
Suggested fix:
Add the same quoting logic to validateClaudeAsync():
```typescript
private async validateClaudeAsync(claudeCmd: string): Promise<ToolValidation> {
try {
const needsShell = process.platform === 'win32' &&
(claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat'));
// When using shell: true on Windows, paths with spaces must be quoted
// to prevent cmd.exe from splitting at spaces
const cmdToRun = needsShell && claudeCmd.includes(' ')
? `"${claudeCmd}"`
: claudeCmd;
const { stdout } = await execFileAsync(cmdToRun, ['--version'], {
...
});
#### 🔵 [523f33b93260] [LOW] Consider expanding quoting for other cmd.exe special characters
📁 `apps/frontend/src/main/cli-tool-manager.ts:928`
The fix only handles spaces, but Windows cmd.exe also interprets other special characters like &, |, >, <, ^, and % when using shell: true. For example, a path like 'D:\Test&Folder\claude.cmd' would cause cmd.exe to interpret & as a command separator. While this is an edge case for installation paths, expanding the check to include these characters would make the fix more robust.
**Suggested fix:**
Expand the condition to quote paths containing any special cmd.exe characters:
const needsQuoting = needsShell && /[ &|<>^%]/.test(claudeCmd);
const cmdToRun = needsQuoting ? `"${claudeCmd}"` : claudeCmd;
---
*This review was generated by Auto Claude.*
When validating Claude CLI on Windows with shell: true (for .cmd/.bat files), paths containing spaces were being split by cmd.exe. For example: `D:\Program Files\nodejs\claude.cmd` was parsed as command `D:\Program` with arguments `Files\nodejs\claude.cmd --version`. This fix adds proper quoting for paths with spaces when using shell: true, preventing cmd.exe from incorrectly splitting the path at space characters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
jeremykit
force-pushed
the
fix/windows-claude-cli-path-spaces
branch
from
2f1eb24 to
f719ab1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/cli-tool-manager.ts (1)
1030-1058: Apply the same quoting fix to the async method.The async counterpart
validateClaudeAsyncstill has the original bug—it passesclaudeCmddirectly toexecFileAsyncwithout quoting paths that contain spaces. This was flagged in a previous review and you acknowledged it, but the fix has not been applied yet.Windows users with spaces in their Claude CLI path will experience validation failures when the async code path is used.
🐛 Proposed fix to apply the same quoting logic
private async validateClaudeAsync(claudeCmd: string): Promise<ToolValidation> { try { const needsShell = process.platform === 'win32' && (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); + // When using shell: true on Windows, paths with spaces must be quoted + // to prevent cmd.exe from splitting at spaces + const cmdToRun = needsShell && claudeCmd.includes(' ') + ? `"${claudeCmd}"` + : claudeCmd; + - const { stdout } = await execFileAsync(claudeCmd, ['--version'], { + const { stdout } = await execFileAsync(cmdToRun, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, shell: needsShell, env: await getAugmentedEnvAsync(), });
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/cli-tool-manager.ts:
- Around line 926-932: The code quotes claudeCmd when needsShell &&
claudeCmd.includes(' ') to avoid Windows cmd.exe splitting, but it should avoid
double-quoting already-quoted paths; update the cmdToRun assignment (used when
calling execFileSync) to only wrap claudeCmd in quotes if it’s not already
quoted (e.g., check that claudeCmd does not start with a double quote and does
not end with one) while preserving the existing needsShell && includes(' ')
condition so execFileSync continues to receive the corrected cmdToRun.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
No new commits since last review. Previous findings still apply.
Findings (2 selected of 2 total)
🟠 [7eea1a85ba75] [HIGH] Missing path quoting in async validateClaudeAsync() method
📁 apps/frontend/src/main/cli-tool-manager.ts:1035
The sync method validateClaude() (lines 928-930) was fixed to quote Windows paths with spaces when using shell: true, but the async counterpart validateClaudeAsync() was NOT updated with the same fix. When claudeCmd contains spaces (e.g., 'D:\Program Files\nodejs\claude.cmd') and needsShell is true, the async method will fail because cmd.exe splits the path at spaces. The async method is used by getToolPathAsync() which is called from the Electron main process.
Suggested fix:
Add the same quoting logic to validateClaudeAsync():
```typescript
private async validateClaudeAsync(claudeCmd: string): Promise<ToolValidation> {
try {
const needsShell = process.platform === 'win32' &&
(claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat'));
// When using shell: true on Windows, paths with spaces must be quoted
// to prevent cmd.exe from splitting at spaces
const cmdToRun = needsShell && claudeCmd.includes(' ')
? `"${claudeCmd}"`
: claudeCmd;
const { stdout } = await execFileAsync(cmdToRun, ['--version'], {
...
});
#### 🔵 [523f33b93260] [LOW] Consider expanding quoting for other cmd.exe special characters
📁 `apps/frontend/src/main/cli-tool-manager.ts:928`
The fix only handles spaces, but Windows cmd.exe also interprets other special characters like &, |, >, <, ^, and % when using shell: true. For example, a path like 'D:\Test&Folder\claude.cmd' would cause cmd.exe to interpret & as a command separator. While this is an edge case for installation paths, expanding the check to include these characters would make the fix more robust.
**Suggested fix:**
Expand the condition to quote paths containing any special cmd.exe characters:
const needsQuoting = needsShell && /[ &|<>^%]/.test(claudeCmd);
const cmdToRun = needsQuoting ? `"${claudeCmd}"` : claudeCmd;
---
*This review was generated by Auto Claude.*
…om/jeremykit/Auto-Claude into fix/windows-claude-cli-path-spaces
When using shell: true on Windows, paths containing spaces must be quoted to prevent cmd.exe from splitting at spaces. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/cli-tool-manager.ts:
- Around line 926-932: Update the cmdToRun assignment in cli-tool-manager.ts
(the code that builds cmdToRun using claudeCmd and needsShell) to guard against
double-quoting by only wrapping claudeCmd in quotes when it contains spaces,
needsShell is true, and it is not already quoted (e.g., check
!claudeCmd.startsWith('"')). Adjust both the synchronous and asynchronous usages
of cmdToRun so they use the same guarded logic to prevent accidentally adding a
second set of quotes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
apps/frontend/src/main/cli-tool-manager.ts (1)
1035-1041: Async version correctly mirrors sync implementation.The quoting logic here is identical to the synchronous version (lines 926-932), ensuring consistent behavior. The same optional enhancement regarding the double-quoting guard applies here as well.
jeremykit
force-pushed
the
fix/windows-claude-cli-path-spaces
branch
from
c37fae5 to
7184105
Compare
|
@AndyMik90 I've addressed the HIGH priority issue by adding the same quoting fix to I've also synced my branch with the latest Could you please re-review when you have a chance? Thanks! |
…user configuration or future code changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/cli-tool-manager.ts:
- Around line 1035-1041: The async validation path in validateClaudeAsync does
not apply the same Windows-quoting logic as validateClaude(), causing commands
with spaces to be split when needsShell is true; update validateClaudeAsync to
generate cmdToRun using the same condition on needsShell, claudeCmd.includes('
'), and !claudeCmd.startsWith('"') and pass that cmdToRun into execFileAsync
(referencing claudeCmd, needsShell, cmdToRun, execFileAsync, and
validateClaudeAsync) so both validateClaude() and validateClaudeAsync handle
Windows paths with spaces consistently.
- Around line 926-932: Change the quote-detection to ensure the path is fully
quoted before skipping quoting: instead of only checking
!claudeCmd.startsWith('"'), update the condition used to build cmdToRun (the
expression referencing needsShell and claudeCmd) to verify both
claudeCmd.startsWith('"') and claudeCmd.endsWith('"') (or equivalent regex) so
you only skip quoting when the string is already fully wrapped in quotes; keep
the rest of the needsShell && space check and use double quotes for the injected
quoting.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/cli-tool-manager.ts (1)
926-939: Extract quoting logic into a helper function to normalize path handlingThe quoting logic is duplicated (lines 926–939 and 1035–1047) and has a reliability issue: it adds wrapping quotes without first stripping existing ones. When
shell: trueis used on Windows, Node.js passes the command string verbatim to cmd.exe (withwindowsVerbatimArguments = true), and cmd.exe may misinterpret pre-quoted paths due to its own quote-parsing rules.Centralize the logic into a helper like
prepareWindowsShellCommand(command, needsShell)that:
- Strips any existing wrapping quotes
- Re-applies quotes only when
shell: trueand whitespace is presentThis ensures cmd.exe receives a properly formed command line and eliminates duplication.
Proposed refactor
+function prepareWindowsShellCommand(command: string, needsShell: boolean): string { + const trimmed = command.trim(); + const unwrapped = + trimmed.startsWith('"') && trimmed.endsWith('"') + ? trimmed.slice(1, -1) + : trimmed; + + // Only quote when cmd.exe is involved and whitespace exists + return needsShell && /\s/.test(unwrapped) ? `"${unwrapped}"` : unwrapped; +} + private validateClaude(claudeCmd: string): ToolValidation { try { // On Windows, .cmd files need shell: true to execute properly. // SECURITY NOTE: shell: true is safe here because: // 1. claudeCmd comes from internal path detection (user config or known system paths) // 2. Only '--version' is passed as an argument (no user input) // If claudeCmd origin ever changes to accept user input, use escapeShellArgWindows. const needsShell = process.platform === 'win32' && (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); - // When using shell: true on Windows, paths with spaces must be quoted - // to prevent cmd.exe from splitting at spaces - const cmdToRun = needsShell && claudeCmd.includes(' ') && !(claudeCmd.startsWith('"') && claudeCmd.endsWith('"')) - ? `"${claudeCmd}"` - : claudeCmd; + const cmdToRun = prepareWindowsShellCommand(claudeCmd, needsShell); const version = execFileSync(cmdToRun, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, shell: needsShell, env: getAugmentedEnv(), }).trim();private async validateClaudeAsync(claudeCmd: string): Promise<ToolValidation> { try { const needsShell = process.platform === 'win32' && (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); - // When using shell: true on Windows, paths with spaces must be quoted - // to prevent cmd.exe from splitting at spaces - const cmdToRun = needsShell && claudeCmd.includes(' ') && !(claudeCmd.startsWith('"') && claudeCmd.endsWith('"')) - ? `"${claudeCmd}"` - : claudeCmd; + const cmdToRun = prepareWindowsShellCommand(claudeCmd, needsShell); const { stdout } = await execFileAsync(cmdToRun, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, shell: needsShell, env: await getAugmentedEnvAsync(), });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (2)
apps/frontend/src/main/cli-tool-manager.ts (2)
926-938: Quoting fix addressescmd.exepath-splitting on WindowsThe
cmdToRunlogic correctly ensures.cmd/.batpaths with spaces aren’t split whenshell: true, and it’s applied consistently in both sync and async validation.Also applies to: 1035-1047
918-925: Verify security context ofclaudeCmdorigin andshell: trueusageThe review raises a legitimate security concern: if
claudeCmdcan originate from user-configurable settings (userConfig.claudePathin settings.json), then the security comment claiming "no user input" is inaccurate. User configuration is user input. Withshell: trueon Windows, metacharacters like&|<>^\r\ncould alter command behavior.However, the concern requires verification of:
- Whether
claudeCmdis actually sourced fromuserConfig.claudePath- What validation (if any) currently exists for
claudeCmd- Whether the suggested hardening (metacharacter validation) applies at all call sites (lines 926-939, 1032-1047)
At minimum, update the security comment to acknowledge user configuration as a source and add validation if
shell: trueis used with user-sourced paths.
…factor by expression
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/cli-tool-manager.ts:
- Around line 1034-1039: The regex used to detect characters requiring quoting
in the async Claude CLI validation (used in needsQuoting where claudeCmd is
tested before calling execFileAsync) currently checks /[ &|<>^%]/; update it to
include parentheses by changing the character class to include () — i.e., add
'(' and ')' inside the brackets so commands containing parentheses are also
quoted consistently with the sync path.
- Around line 926-931: Update the quoting regex used to decide if claudeCmd
needs quoting: in the code that computes needsQuoting (referencing the
needsQuoting variable and claudeCmd), expand the character class `/[ &|<>^%]/`
to include parentheses so it also detects "(" or ")" in paths (e.g., change to
include `()` inside the brackets) before building cmdToRun and calling
execFileSync.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/cli-tool-manager.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/cli-tool-manager.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/cli-tool-manager.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/cli-tool-manager.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
📚 Learning: 2026-01-04T23:59:48.743Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:48.743Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
Applied to files:
apps/frontend/src/main/cli-tool-manager.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
3 participants
When validating Claude CLI on Windows with shell: true (for .cmd/.bat files), paths containing spaces were being split by cmd.exe. For example:
D:\Program Files\nodejs\claude.cmdwas parsed as commandD:\Programwith argumentsFiles\nodejs\claude.cmd --version.This fix adds proper quoting for paths with spaces when using shell: true, preventing cmd.exe from incorrectly splitting the path at space characters.
🤖 Generated with Claude Code
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: Yes / No
Details:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.