fix(frontend): ensure PATH includes system directories when launched (#748)
* fix(frontend): ensure PATH includes system directories when launched from Finder Fixes 'Claude CLI not found' error in Insights panel when Auto-Claude is launched from Finder/Dock on macOS. When Electron apps launch from GUI (not terminal), process.env.PATH is minimal or empty and doesn't include essential system directories. The Claude Agent SDK requires /usr/bin/security to access the macOS Keychain for OAuth tokens. Without this in PATH, SDK initialization fails and Insights falls back to simple mode with 120s timeout. Changes: - env-utils.ts: Ensure /usr/bin, /bin, /usr/sbin, /sbin are always in PATH - Only appends missing paths to respect user's PATH configuration - Applies to both macOS and Linux (platform !== 'win32') Tested by building DMG and launching from Finder - Insights now responds without timeout. * refactor(frontend): address AI review feedback on PATH handling Improves code consistency and empty string handling based on AI review: 1. Extract essential paths to module-level constant - Created ESSENTIAL_SYSTEM_PATHS constant following file's pattern - Consistent with existing COMMON_BIN_PATHS constant - Self-documenting with JSDoc comment 2. Add .filter(Boolean) to second currentPathSet creation - Line 138 now matches line 126's pattern - Ensures consistent empty string filtering throughout function - Addresses @dertuerke's concern about proper falsy value handling These changes improve code maintainability without affecting functionality. The original PATH fix still works correctly - this just makes the code more consistent with project patterns. * refactor(frontend): improve code clarity from second AI review Based on second AI review iteration, made three improvements: 1. Add explicit type annotation to ESSENTIAL_SYSTEM_PATHS - Consistent with adjacent COMMON_BIN_PATHS constant - const ESSENTIAL_SYSTEM_PATHS: string[] = [...] 2. Rename inner variable to avoid shadowing - pathSetForEssentials instead of currentPathSet (inner scope) - Makes it clear this Set checks for missing essentials - Outer currentPathSet (line 137) still has clear purpose 3. Remove unnecessary intermediate variable - Use ESSENTIAL_SYSTEM_PATHS directly instead of essentialPaths alias - Reduces indirection, constant name is already descriptive All changes improve code readability without affecting functionality. * fix: ensure essential paths are always written to env.PATH Previously, env.PATH was only updated when pathsToAdd had items. This caused the fix to fail on minimal systems without Homebrew/npm where pathsToAdd would be empty, leaving env.PATH unset even though currentPath contained the essential system paths. Now we always write currentPath to env.PATH, ensuring essential paths are present even when no additional paths are found. Fixes Auto Claude review finding ce703185936f * fix: apply essential paths logic to async version Applied the same fixes to getAugmentedEnvAsync(): 1. Added essential system paths logic for macOS Keychain access 2. Added .filter(Boolean) to prevent empty string in currentPathSet 3. Removed conditional PATH update to ensure essential paths always written This ensures async code paths (Claude CLI detection, tool validation) also work correctly when app launches from Finder/Dock. Fixes Auto Claude review HIGH severity finding --------- Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
4203341227
commit
4cc9198a3e
@@ -112,6 +112,12 @@ export const COMMON_BIN_PATHS: Record<string, string[]> = {
|
||||
],
|
||||
};
|
||||
|
||||
/**
|
||||
* Essential system directories that must always be in PATH
|
||||
* Required for core system functionality (e.g., /usr/bin/security for Keychain access)
|
||||
*/
|
||||
const ESSENTIAL_SYSTEM_PATHS: string[] = ['/usr/bin', '/bin', '/usr/sbin', '/sbin'];
|
||||
|
||||
/**
|
||||
* Get expanded platform paths for PATH augmentation
|
||||
*
|
||||
@@ -195,9 +201,26 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record<string, stri
|
||||
// Get all candidate paths (platform + additional)
|
||||
const candidatePaths = getExpandedPlatformPaths(additionalPaths);
|
||||
|
||||
// Ensure PATH has essential system directories when launched from Finder/Dock.
|
||||
// When Electron launches from GUI (not terminal), PATH might be empty or minimal.
|
||||
// The Claude Agent SDK needs /usr/bin/security to access macOS Keychain.
|
||||
let currentPath = env.PATH || '';
|
||||
|
||||
// On macOS/Linux, ensure basic system paths are always present
|
||||
if (platform !== 'win32') {
|
||||
const pathSetForEssentials = new Set(currentPath.split(pathSeparator).filter(Boolean));
|
||||
const missingEssentials = ESSENTIAL_SYSTEM_PATHS.filter(p => !pathSetForEssentials.has(p));
|
||||
|
||||
if (missingEssentials.length > 0) {
|
||||
// Append essential paths if missing (append, not prepend, to respect user's PATH)
|
||||
currentPath = currentPath
|
||||
? `${currentPath}${pathSeparator}${missingEssentials.join(pathSeparator)}`
|
||||
: missingEssentials.join(pathSeparator);
|
||||
}
|
||||
}
|
||||
|
||||
// Collect paths to add (only if they exist and aren't already in PATH)
|
||||
const currentPath = env.PATH || '';
|
||||
const currentPathSet = new Set(currentPath.split(pathSeparator));
|
||||
const currentPathSet = new Set(currentPath.split(pathSeparator).filter(Boolean));
|
||||
|
||||
// Check existence synchronously and build existing paths set
|
||||
const existingPaths = new Set(candidatePaths.filter(p => fs.existsSync(p)));
|
||||
@@ -212,9 +235,7 @@ export function getAugmentedEnv(additionalPaths?: string[]): Record<string, stri
|
||||
const pathsToAdd = buildPathsToAdd(candidatePaths, currentPathSet, existingPaths, npmPrefix);
|
||||
|
||||
// Prepend new paths to PATH (prepend so they take priority)
|
||||
if (pathsToAdd.length > 0) {
|
||||
env.PATH = [...pathsToAdd, currentPath].filter(Boolean).join(pathSeparator);
|
||||
}
|
||||
env.PATH = [...pathsToAdd, currentPath].filter(Boolean).join(pathSeparator);
|
||||
|
||||
return env;
|
||||
}
|
||||
@@ -339,9 +360,22 @@ export async function getAugmentedEnvAsync(additionalPaths?: string[]): Promise<
|
||||
// Get all candidate paths (platform + additional)
|
||||
const candidatePaths = getExpandedPlatformPaths(additionalPaths);
|
||||
|
||||
// Ensure essential system paths are present (for macOS Keychain access)
|
||||
let currentPath = env.PATH || '';
|
||||
|
||||
if (platform !== 'win32') {
|
||||
const pathSetForEssentials = new Set(currentPath.split(pathSeparator).filter(Boolean));
|
||||
const missingEssentials = ESSENTIAL_SYSTEM_PATHS.filter(p => !pathSetForEssentials.has(p));
|
||||
|
||||
if (missingEssentials.length > 0) {
|
||||
currentPath = currentPath
|
||||
? `${currentPath}${pathSeparator}${missingEssentials.join(pathSeparator)}`
|
||||
: missingEssentials.join(pathSeparator);
|
||||
}
|
||||
}
|
||||
|
||||
// Collect paths to add (only if they exist and aren't already in PATH)
|
||||
const currentPath = env.PATH || '';
|
||||
const currentPathSet = new Set(currentPath.split(pathSeparator));
|
||||
const currentPathSet = new Set(currentPath.split(pathSeparator).filter(Boolean));
|
||||
|
||||
// Check existence asynchronously in parallel for performance
|
||||
const pathChecks = await Promise.all(
|
||||
@@ -361,9 +395,7 @@ export async function getAugmentedEnvAsync(additionalPaths?: string[]): Promise<
|
||||
const pathsToAdd = buildPathsToAdd(candidatePaths, currentPathSet, existingPaths, npmPrefix);
|
||||
|
||||
// Prepend new paths to PATH (prepend so they take priority)
|
||||
if (pathsToAdd.length > 0) {
|
||||
env.PATH = [...pathsToAdd, currentPath].filter(Boolean).join(pathSeparator);
|
||||
}
|
||||
env.PATH = [...pathsToAdd, currentPath].filter(Boolean).join(pathSeparator);
|
||||
|
||||
return env;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user