fix(windows): resolve Claude CLI detection race condition on GUI launch #837
Conversation
|
|
📝 WalkthroughWalkthroughCentralized user-configured path retrieval and per-tool validation were added to CLIToolManager; getToolInfo now prefers validated user paths before detection. getNpmGlobalPrefixAsync suppresses a Windows-specific expected error message and resets its cache promise in a finally block. Changes
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
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 @VDT-91, 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 addresses critical issues affecting the initial detection of the Claude CLI and the retrieval of the npm global prefix, particularly on Windows systems when the application is launched via the GUI. By resolving a race condition in tool detection and intelligently suppressing an expected error related to 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
|
![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 effectively addresses a race condition with Claude CLI detection on Windows GUI launch by prioritizing user-configured paths. It also smartly suppresses an expected error message related to nvm-windows. The changes are well-targeted. My main feedback is on cli-tool-manager.ts, where the new logic introduces a redundant check. I've suggested a refactoring to improve efficiency and maintainability by separating user-configuration checks from system-wide tool detection.
| } | ||
| console.warn(`[CLI Tools] User-configured ${tool} path invalid: ${validation.message}`); | ||
| } | ||
| return this.detectToolPath(tool); |
There was a problem hiding this comment.
This fallback call to this.detectToolPath(tool) introduces a redundant check. The logic within getToolInfo already handles checking the user-configured path. However, detectToolPath(tool) delegates to methods like detectPython() or detectClaude(), which also perform the same user-configured path check at the beginning. This leads to inefficiency, as the validation (which can involve spawning a process) might run twice if the user-configured path is invalid.
To improve efficiency and maintainability, consider refactoring to avoid this. One approach could be to separate the system detection logic from the user-config check within the detect<Tool>() methods. For example, you could have a detectPythonFromSystem() which getToolInfo could call here, while the original detectPython() (called by getToolPath) would still perform both checks.
|
I have read the CLA Document and I hereby sign the CLA |
![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
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/env-utils.ts:
- Around line 335-344: Replace the brittle English-text check around
isExpectedWindowsError with a robust detection: before attempting the npm call,
check for the npm executable on Windows (e.g., look for "npm.cmd" in PATH) and
treat a missing executable as the expected Windows case; additionally treat
OS-level missing-command errors (e.g., error.code === 'ENOENT') as the expected
case on all locales, and only call console.warn when neither the executable is
present nor the error.code indicates a missing command; update the logic that
uses process.platform, isExpectedWindowsError, error and console.warn
accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.
🧬 Code graph analysis (2)
apps/frontend/src/main/cli-tool-manager.ts (1)
apps/frontend/src/shared/types/cli.ts (1)
ToolDetectionResult(12-25)
apps/frontend/src/main/env-utils.ts (1)
apps/frontend/src/renderer/components/settings/utils/hookProxyFactory.ts (1)
error(18-18)
⏰ 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)
🔇 Additional comments (4)
apps/frontend/src/main/env-utils.ts (1)
347-349: LGTM: Proper cache cleanup in finally block.The finally block correctly resets
npmGlobalPrefixCachePromiseto null after the async operation completes, ensuring proper cache state management for subsequent calls.apps/frontend/src/main/cli-tool-manager.ts (3)
1629-1648: LGTM: Clean helper for user-configured paths.The helper method consolidates user configuration lookup logic, improving maintainability and reducing code duplication in
getToolInfo().
1650-1670: LGTM: Type-based validation helper.The helper method provides a clean abstraction for validating tools by type, making the code more maintainable.
1681-1699: getToolInfo() intentionally bypasses cache for fresh diagnostics—no action required, but usage is acceptable given infrequent calls.The method does spawn subprocesses on every call (via
execFileSyncinvalidateToolByType), and the 4 sequential calls insettings-handlers.ts(lines 263-266) do execute 4 version checks. However, this is by design:getToolInfo()exists for diagnostics purposes and explicitly prioritizes correctness over caching. The session cache exists inCLIToolManagerbut is reserved forgetToolPath()andgetToolPathAsync(), which are used in performance-sensitive paths like agent startup.Actual usage shows limited call frequency: the Settings UI calls it once per handler invocation, and UI components call it during render but not in tight loops. The performance trade-off is reasonable for a diagnostic method that needs to reflect current system state without stale data.
When Auto Claude starts from Windows GUI (not terminal), two issues occurred: 1. Race condition: version check runs before settings load, ignoring user config 2. npm.cmd failure: with nvm-windows, node isn't in PATH for GUI-launched apps Changes: - Update getUserConfiguredPath() to read directly from settings.json on disk as a fallback when this.userConfig is empty (before configureTools is called) - Add validateToolByType() helper to validate paths by tool type - Update getToolInfo() to check user config FIRST before fresh detection - Suppress expected "node is not recognized" error on Windows with nvm Root cause: getToolInfo() was called by CLAUDE_CODE_CHECK_VERSION IPC before SETTINGS_GET loaded settings and called configureTools(). The original fix checked this.userConfig which was still empty. Now we read settings.json directly from disk as a fallback, ensuring the user's configured claudePath is found even before the settings IPC roundtrip completes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
VDT-91
force-pushed
the
fix/windows-claude-cli-detection-race
branch
from
7ebc0e3 to
2a45e07
Compare
Update: Fixed the actual root causeThe initial fix checked Timeline of the race condition:The fix:Updated // First check in-memory config (set by configureTools)
const inMemoryPath = this.userConfig[settingsKey];
if (inMemoryPath) {
return inMemoryPath;
}
// Fallback: read directly from settings file on disk
const savedSettings = readSettingsFile();
if (savedSettings) {
const diskPath = savedSettings[settingsKey];
if (typeof diskPath === 'string') {
return diskPath;
}
}Verified working:Claude CLI is now detected immediately on the first version check, no refresh needed. |
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 1641-1676: getUserConfiguredPath currently falls back to
readSettingsFile on every call causing repeated synchronous disk I/O; add a
diskSettingsCache property (e.g., diskSettingsCache: Record<string, unknown> |
undefined | null) and use it to store the result of readSettingsFile on first
access (treating exceptions as undefined) and then consult that cache instead of
re-reading the file; also reset diskSettingsCache to null inside configure
(alongside this.userConfig assignment and cache.clear) so updates invalidate the
cached disk settings; reference getUserConfiguredPath, readSettingsFile,
diskSettingsCache, and configure when making the changes.
In @apps/frontend/src/main/env-utils.ts:
- Around line 335-344: The Windows "command not found" detection in env-utils.ts
uses error.message.includes('is not recognized') which fails on localized
systems; update the isExpectedWindowsError logic to check error.code ===
'ENOENT' (while still ensuring process.platform === 'win32' and error is an
Error) so the child_process command-not-found case is detected reliably across
locales and the conditional that suppresses the console.warn uses that new
check.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
apps/frontend/src/main/cli-tool-manager.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.tsapps/frontend/src/main/env-utils.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.
🧬 Code graph analysis (1)
apps/frontend/src/main/cli-tool-manager.ts (2)
apps/frontend/src/main/settings-utils.ts (1)
readSettingsFile(29-43)apps/frontend/src/shared/types/cli.ts (1)
ToolDetectionResult(12-25)
⏰ 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 (4)
apps/frontend/src/main/env-utils.ts (1)
347-349: LGTM! Proper cache cleanup.The finally block correctly resets the cache promise regardless of whether the operation succeeds or fails, ensuring future attempts can retry if needed.
apps/frontend/src/main/cli-tool-manager.ts (3)
30-30: LGTM! Necessary import for race condition fix.The import of
readSettingsFileenables reading user configuration directly from disk, which is essential for fixing the race condition on Windows GUI launch.
1685-1698: LGTM! Clean abstraction for tool validation.This method effectively centralizes validation logic across all tool types, improving code maintainability and reducing duplication.
1709-1727: Excellent fix for the race condition!The updated logic correctly prioritizes user-configured paths by checking both in-memory and on-disk settings before falling back to fresh detection. This ensures Claude CLI is detected correctly on Windows GUI launch even when
CLAUDE_CODE_CHECK_VERSIONIPC runs before settings load.Based on learnings, the changes successfully address the core issue described in the PR objectives.
| private getUserConfiguredPath(tool: CLITool): string | undefined { | ||
| // Map tool to settings key | ||
| const settingsKey = { | ||
| python: 'pythonPath', | ||
| git: 'gitPath', | ||
| gh: 'githubCLIPath', | ||
| claude: 'claudePath', | ||
| }[tool] as 'pythonPath' | 'gitPath' | 'githubCLIPath' | 'claudePath' | undefined; | ||
|
|
||
| if (!settingsKey) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // First check in-memory config (set by configureTools) | ||
| const inMemoryPath = this.userConfig[settingsKey]; | ||
| if (inMemoryPath) { | ||
| return inMemoryPath; | ||
| } | ||
|
|
||
| // Fallback: read directly from settings file on disk | ||
| // This handles the race condition where getToolInfo is called before | ||
| // settings are loaded via SETTINGS_GET → configureTools | ||
| try { | ||
| const savedSettings = readSettingsFile(); | ||
| if (savedSettings) { | ||
| const diskPath = savedSettings[settingsKey]; | ||
| if (typeof diskPath === 'string' && diskPath.length > 0) { | ||
| return diskPath; | ||
| } | ||
| } | ||
| } catch { | ||
| // Ignore read errors - will fall back to detection | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good fix for race condition, but consider caching disk reads.
The fallback to readSettingsFile() effectively fixes the race condition. However, this method is called on every getToolInfo() invocation when in-memory config is empty, causing repeated synchronous disk reads.
Consider caching the disk-read settings to avoid repeated I/O:
private diskSettingsCache: Record<string, unknown> | undefined | null = null;
private getUserConfiguredPath(tool: CLITool): string | undefined {
const settingsKey = {
python: 'pythonPath',
git: 'gitPath',
gh: 'githubCLIPath',
claude: 'claudePath',
}[tool] as 'pythonPath' | 'gitPath' | 'githubCLIPath' | 'claudePath' | undefined;
if (!settingsKey) {
return undefined;
}
// First check in-memory config
const inMemoryPath = this.userConfig[settingsKey];
if (inMemoryPath) {
return inMemoryPath;
}
// Cache disk settings on first read
if (this.diskSettingsCache === null) {
try {
this.diskSettingsCache = readSettingsFile();
} catch {
this.diskSettingsCache = undefined;
}
}
// Use cached disk settings
if (this.diskSettingsCache) {
const diskPath = this.diskSettingsCache[settingsKey];
if (typeof diskPath === 'string' && diskPath.length > 0) {
return diskPath;
}
}
return undefined;
}Also clear the cache in configure():
configure(config: ToolConfig): void {
this.userConfig = config;
this.diskSettingsCache = null; // Reset disk cache
this.cache.clear();
console.warn('[CLI Tools] Configuration updated, cache cleared');
}🤖 Prompt for AI Agents
In @apps/frontend/src/main/cli-tool-manager.ts around lines 1641 - 1676,
getUserConfiguredPath currently falls back to readSettingsFile on every call
causing repeated synchronous disk I/O; add a diskSettingsCache property (e.g.,
diskSettingsCache: Record<string, unknown> | undefined | null) and use it to
store the result of readSettingsFile on first access (treating exceptions as
undefined) and then consult that cache instead of re-reading the file; also
reset diskSettingsCache to null inside configure (alongside this.userConfig
assignment and cache.clear) so updates invalidate the cached disk settings;
reference getUserConfiguredPath, readSettingsFile, diskSettingsCache, and
configure when making the changes.
| // On Windows with nvm-windows, node may not be in PATH when launched from GUI | ||
| // This is expected behavior, not an error worth logging | ||
| const isExpectedWindowsError = | ||
| process.platform === 'win32' && | ||
| error instanceof Error && | ||
| error.message.includes('is not recognized'); | ||
|
|
||
| if (!isExpectedWindowsError) { | ||
| console.warn(`[env-utils] Failed to get npm global prefix: ${error}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the env-utils.ts file around the suspect code
cat -n apps/frontend/src/main/env-utils.ts | sed -n '320,350p'Repository: AndyMik90/Auto-Claude
Length of output: 1285
🌐 Web query:
Node.js child_process execFile error codes for command not found on Windows
💡 Result:
Short answer:
- execFile/ spawn when the executable isn't found emits an Error with code 'ENOENT' (on Windows you'll often see errno -4058). [1][3]
- If you run via a shell (exec or spawn with shell) and cmd.exe reports "command not found", the shell normally returns exit code 9009. [2]
Sources:
[1] Node.js child_process docs (Windows .bat/.cmd and ENOENT behavior).
[2] Explanation of cmd.exe exit code 9009 = "command not found".
[3] Examples/posts showing Error with errno -4058 / code 'ENOENT' on Windows.
🌐 Web query:
Windows ENOENT error code command not recognized versus command not found Node.js
💡 Result:
Short answer
- Node's "Error: spawn ENOENT" means the child process couldn't be spawned because the executable file couldn't be found (or the shell path is wrong). Node reports ENOENT; the human-facing shell message differs by OS: on Windows cmd.exe prints "'xyz' is not recognized ...", while POSIX shells print "command not found". (stackoverflow.com)
Why you see different text
- The underlying failure is the same (executable not found / spawn failed → ENOENT). The shell or OS prints different wording: Windows shows the "not recognized" message, bash/sh show "command not found". Node surfaces ENOENT from its child_process layer. (2ality.com)
Common causes
- PATH (or PATHEXT on Windows) doesn't include the directory with the executable.
- You passed a non‑existent command name to spawn/exec (or used shell options incorrectly).
- In WSL/WSL→Windows interop the Windows PATH/mounts may be unavailable so cmd.exe can't be found.
- Using spawn without shell when the command is a shell built‑in or a script requiring an extension (.cmd/.bat/.exe). (stackoverflow.com)
Quick debugging / fixes
- Inspect the PATH Node uses: console.log(process.env.PATH) before spawn. (stackoverflow.com)
- Verify the executable exists: on Linux/mac run which ; on Windows run where . If not found, install or add its folder to PATH. (stackoverflow.com)
- On Windows, ensure PATHEXT and that you call the correct file (node.exe, npm.cmd). Use the full path if needed (e.g., "C:\Program Files\nodejs\node.exe"). (bobbyhadz.com)
- If you need a shell (to run shell built‑ins, redirection, or shell syntax) set { shell: true } in spawn/exec. If you rely on cmd.exe from WSL, ensure /mnt/c/Windows/System32 is in PATH. (2ality.com)
- Enable debugging: set NODE_DEBUG=child_process to see more spawn diagnostics or wrap spawn to log arguments. (stackoverflow.com)
If you want, tell me the exact spawn/exec call and the full error text and I’ll point to the precise fix.
Citations:
- 1: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js?utm_source=openai
- 2: https://2ality.com/2022/07/nodejs-child-process.html?utm_source=openai
- 3: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js?utm_source=openai
- 4: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js?utm_source=openai
- 5: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js?utm_source=openai
- 6: https://bobbyhadz.com/blog/node-is-not-recognized-as-internal-or-external-command?utm_source=openai
- 7: https://2ality.com/2022/07/nodejs-child-process.html?utm_source=openai
- 8: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js?utm_source=openai
🏁 Script executed:
# Look at the execFileAsync call and surrounding context
cat -n apps/frontend/src/main/env-utils.ts | sed -n '300,335p'Repository: AndyMik90/Auto-Claude
Length of output: 1505
🏁 Script executed:
# Search for the execFileAsync function definition or import
rg -n 'execFileAsync' apps/frontend/src/main/env-utils.ts -B5 -A2Repository: AndyMik90/Auto-Claude
Length of output: 652
Use error.code instead of message string to detect Windows "command not found" errors.
The error detection uses error.message.includes('is not recognized'), which will fail on non-English Windows installations. Windows error messages are localized (e.g., German: "wurde nicht als interner", French: "n'est pas reconnu"), so this check won't match for non-English users, causing spurious warning logs.
Instead, check the error code directly: error.code === 'ENOENT'. Node.js child_process reports command-not-found errors with the ENOENT code on all platforms regardless of locale. For example:
const isExpectedWindowsError =
process.platform === 'win32' &&
error instanceof Error &&
error.code === 'ENOENT';
This approach is robust, locale-independent, and more reliable than message matching.
🤖 Prompt for AI Agents
In @apps/frontend/src/main/env-utils.ts around lines 335 - 344, The Windows
"command not found" detection in env-utils.ts uses error.message.includes('is
not recognized') which fails on localized systems; update the
isExpectedWindowsError logic to check error.code === 'ENOENT' (while still
ensuring process.platform === 'win32' and error is an Error) so the
child_process command-not-found case is detected reliably across locales and the
conditional that suppresses the console.warn uses that new check.
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 (4 selected of 4 total)
🔵 [239621b37f42] [LOW] [UNRESOLVED] Windows error detection may miss ENOENT spawn failures
📁 apps/frontend/src/main/env-utils.ts:337
The isExpectedWindowsError check only looks for 'is not recognized' in error message. On Windows, when npm.cmd is not found, Node.js can throw ENOENT errors with different message formats. This could cause unexpected console.warn logs for normal Windows configurations.
Resolution note: const isExpectedWindowsError =
process.platform === 'win32' &&
error instanceof Error &&
error.message.includes('is not recognized');
Suggested fix:
Expand error check:
const isExpectedWindowsError =
process.platform === 'win32' &&
error instanceof Error &&
(error.message.includes('is not recognized') ||
(error as NodeJS.ErrnoException).code === 'ENOENT');
🟡 [NEW-001] [MEDIUM] Synchronous file read in getToolInfo may block main process
📁 apps/frontend/src/main/cli-tool-manager.ts:1664
The new getUserConfiguredPath method calls readSettingsFile() which uses synchronous readFileSync. When getToolInfo is called (e.g., from Settings UI), this synchronous disk read can block the Electron main process event loop. This is inconsistent with the file's async alternatives pattern.
Suggested fix:
Consider creating an async version getUserConfiguredPathAsync that uses fs.promises.readFile, consistent with existing async patterns (getToolPathAsync, detectToolPathAsync).
🟡 [NEW-002] [MEDIUM] Tool validation executes synchronously after config read
📁 apps/frontend/src/main/cli-tool-manager.ts:1714
When a user-configured path is found, validateToolByType() executes execFileSync to validate the tool. This synchronous subprocess execution can block for up to 5 seconds. Combined with synchronous file read, this creates potential for UI freezing in getToolInfo(). The existing async methods (validateClaudeAsync, etc.) are not used.
Suggested fix:
Consider adding getToolInfoAsync method that uses async validation, following existing pattern where each sync method has an async counterpart.
🔵 [NEW-003] [LOW] Type assertion could use exhaustive checking
📁 apps/frontend/src/main/cli-tool-manager.ts:1648
The type assertion includes undefined but relies on runtime check rather than TypeScript exhaustive checking. If a new CLITool is added without updating the mapping, it would silently fall through. Using a typed const object would cause compile-time errors.
Suggested fix:
Use: const TOOL_TO_SETTINGS_KEY: Record<CLITool, keyof ToolConfig | undefined> = {...} as const;
This review was generated by Auto Claude.
5 participants
Summary
Problem
When Auto Claude starts from Windows GUI (Start Menu, desktop shortcut) - not terminal:
CLAUDE_CODE_CHECK_VERSIONIPC called before user settings load, sogetToolInfo('claude')did fresh detection ignoring user confignodeisn't in PATH when launched from GUI, causingnpm.cmd config get prefixto fail with "node is not recognized"Solution
Change 1: Make
getToolInfo()respect user configurationgetUserConfiguredPath()helper to extract user-configured pathsvalidateToolByType()helper to validate paths by tool typegetToolInfo()to check user config FIRST before fresh detectionChange 2: Suppress expected Windows error
getNpmGlobalPrefixAsync()to detect expected "is not recognized" error on WindowsFiles Changed
apps/frontend/src/main/cli-tool-manager.tsgetToolInfo()apps/frontend/src/main/env-utils.tsTest Plan
npm startfrom terminal🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.