mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-01-30 06:12:03 +00:00
fix: address additional code review feedback
- Add path.normalize() for Windows mixed separator handling - Add validation to check Node executable exists after finding it - Improve error dialog with specific troubleshooting advice for Node.js related errors vs general errors - Include source info in validation error message 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -134,6 +134,12 @@ async function startServer(): Promise<void> {
|
||||
logger: (msg) => console.log(`[Electron] ${msg}`),
|
||||
});
|
||||
const command = nodeResult.nodePath;
|
||||
|
||||
// Validate that the found Node executable actually exists
|
||||
if (command !== 'node' && !fs.existsSync(command)) {
|
||||
throw new Error(`Node.js executable not found at: ${command} (source: ${nodeResult.source})`);
|
||||
}
|
||||
|
||||
let args: string[];
|
||||
let serverPath: string;
|
||||
|
||||
@@ -338,9 +344,15 @@ app.whenReady().then(async () => {
|
||||
createWindow();
|
||||
} catch (error) {
|
||||
console.error('[Electron] Failed to start:', error);
|
||||
const errorMessage = (error as Error).message;
|
||||
const isNodeError = errorMessage.includes('Node.js');
|
||||
dialog.showErrorBox(
|
||||
'Automaker Failed to Start',
|
||||
`The application failed to start.\n\n${(error as Error).message}\n\nPlease ensure Node.js is installed and accessible.`
|
||||
`The application failed to start.\n\n${errorMessage}\n\n${
|
||||
isNodeError
|
||||
? 'Please install Node.js from https://nodejs.org or via a package manager (Homebrew, nvm, fnm).'
|
||||
: 'Please check the application logs for more details.'
|
||||
}`
|
||||
);
|
||||
app.quit();
|
||||
}
|
||||
|
||||
@@ -334,8 +334,10 @@ export function buildEnhancedPath(nodePath: string, currentPath: string = ''): s
|
||||
|
||||
// Don't add if already present or if it's just '.'
|
||||
// Use path segment matching to avoid false positives (e.g., /opt/node vs /opt/node-v18)
|
||||
const pathSegments = currentPath.split(path.delimiter);
|
||||
if (nodeDir === '.' || pathSegments.includes(nodeDir)) {
|
||||
// Normalize paths for comparison to handle mixed separators on Windows
|
||||
const normalizedNodeDir = path.normalize(nodeDir);
|
||||
const pathSegments = currentPath.split(path.delimiter).map((s) => path.normalize(s));
|
||||
if (normalizedNodeDir === '.' || pathSegments.includes(normalizedNodeDir)) {
|
||||
return currentPath;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user