Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 77 additions & 65 deletions electron/main.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,101 +2,113 @@ const { app, BrowserWindow } = require('electron');
const path = require('path');
const { spawn } = require('child_process');
const http = require('http');
const fs = require('fs');

let mainWindow;
let serverProcess;
let serverHost = '0.0.0.0';
let serverPort = 3000;

try {
const configPath = './src/server-config.json';
if (fs.existsSync(configPath)) {
const config = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
if (config.host) serverHost = config.host;
if (config.frontendPort) serverPort = config.frontendPort;
}
} catch (e) {
console.warn('Failed to load server config:', e);
}
const isDev = !app.isPackaged;

// Prevent multiple instances
const gotLock = app.requestSingleInstanceLock();
if (!gotLock) {
app.quit();
process.exit(0);
}
const DEV_URL = process.env.VITE_DEV_SERVER_URL || 'http://localhost:3000';

// Wait until server is ready
// Wait for server
function waitForServer(url) {
return new Promise((resolve) => {
const check = () => {
http
.get(url, () => resolve())
.on('error', () => setTimeout(check, 500));
};

check();
});
}
Comment on lines 13 to 23

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add timeout to prevent indefinite hang.

The waitForServer function polls indefinitely without a timeout. If the server fails to start (e.g., port already in use, server crash, misconfiguration), the Electron app will hang forever without feedback to the user.

⏱️ Add configurable timeout with clear error
-function waitForServer(url) {
+function waitForServer(url, timeoutMs = 30000) {
   return new Promise((resolve, reject) => {
+    const startTime = Date.now();
     const check = () => {
+      if (Date.now() - startTime > timeoutMs) {
+        return reject(new Error(`Server at ${url} did not start within ${timeoutMs}ms`));
+      }
       http
         .get(url, () => resolve())
         .on('error', () => setTimeout(check, 500));
     };
 
     check();
   });
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 13 - 23, The waitForServer function currently
polls forever; update waitForServer to accept an optional timeoutMs (default
e.g. 30_000) and return a Promise that rejects with a clear Error if the server
hasn't responded within that timeout. Implement a timer (setTimeout) that
rejects after timeoutMs and ensure you clear that timer and any pending retries
when http.get succeeds (resolve) or when rejecting; keep the existing retry
logic (check and setTimeout(check, 500)) but stop retries on reject/resolve.
Reference waitForServer and the inner http.get/.on('error') handler so you
replace the infinite polling with a timeout-aware implementation that provides a
descriptive error (including the URL and timeout) when it fails.


// Start Nitro server (production)
function startServer() {
return new Promise((resolve) => {
const serverPath = path.join(
process.resourcesPath,
'app.asar.unpacked',
'.output',
'server',
'index.mjs'
);

console.log("Starting server from:", serverPath);

serverProcess = spawn('node', [serverPath], {
stdio: 'ignore', // no terminal
windowsHide: true, // hide CMD
env: {
...process.env,
HOST: serverHost,
PORT: serverPort.toString(),
},
});

waitForServer(`http://localhost:${serverPort}`).then(resolve);
// Start production Nitro server
async function startProductionServer() {
const serverPath = path.join(
process.resourcesPath,
'app.asar.unpacked',
'.output',
'server',
'index.mjs'
);
Comment on lines +27 to +33

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that electron-builder config correctly unpacks .output for the production server path

# Check that .output is in asarUnpack array
jq -e '.build.asarUnpack | any(. == ".output/**/*")' package.json

if [ $? -eq 0 ]; then
  echo "✓ .output is configured for ASAR unpacking"
else
  echo "✗ WARNING: .output is not in asarUnpack - production server path will fail"
fi

# Also check extraResources
jq -e '.build.extraResources | any(.from == ".output")' package.json

if [ $? -eq 0 ]; then
  echo "✓ .output is in extraResources"
else
  echo "✗ .output is not in extraResources"
fi

Repository: AOSSIE-Org/Rein

Length of output: 143


Server path matches current electron-builder ASAR-unpack config for .output

package.json configures build.asarUnpack to include .output/**/* and also ships .output via build.extraResources, which makes process.resourcesPath/app.asar.unpacked/.output/server/index.mjs in electron/main.cjs consistent with the current packaging setup.
Optional: add a fallback/guard for non-ASAR or layout changes (e.g., process.resourcesPath/.output/... when app.asar.unpacked is absent).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 27 - 33, The current construction of
serverPath always assumes an ASAR-unpacked layout (serverPath built from
process.resourcesPath + 'app.asar.unpacked' + '.output/server/index.mjs');
update it to check for the presence of the ASAR-unpacked path and fall back to
the non-ASAR layout (e.g., process.resourcesPath + '.output/server/index.mjs')
when app.asar.unpacked is absent or the file doesn't exist. Modify the code that
sets serverPath (referencing serverPath and process.resourcesPath and the
'app.asar.unpacked' segment) to attempt the unpacked path first and then a
fallback path, using a synchronous existence check or try/catch to select the
correct path at runtime.


console.log('Starting production server:', serverPath);

serverProcess = spawn('node', [serverPath], {
stdio: 'inherit',
windowsHide: true,
env: {
...process.env,
HOST: '0.0.0.0',
PORT: '3000',
},
Comment on lines +40 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Hardcoded HOST and PORT create maintenance risk.

The production server spawn hardcodes HOST: '0.0.0.0' and PORT: '3000', while the actual server configuration is defined in src/server-config.json. This duplication creates a maintenance risk: if the port or host in server-config.json is changed, the Electron app will attempt to connect to the wrong endpoint and fail to load.

♻️ Load configuration from server-config.json
+const serverConfig = require('../src/server-config.json');
+
 async function startProductionServer() {
   const serverPath = path.join(
     process.resourcesPath,
     'app.asar.unpacked',
     '.output',
     'server',
     'index.mjs'
   );
 
   console.log('Starting production server:', serverPath);
 
   serverProcess = spawn('node', [serverPath], {
     stdio: 'inherit',
     windowsHide: true,
     env: {
       ...process.env,
-      HOST: '0.0.0.0',
-      PORT: '3000',
+      HOST: serverConfig.host,
+      PORT: String(serverConfig.frontendPort),
     },
   });
 
-  await waitForServer('http://localhost:3000');
+  await waitForServer(`http://localhost:${serverConfig.frontendPort}`);
 }

Then update line 79 as well:

-      await mainWindow.loadURL("http://localhost:3000");
+      await mainWindow.loadURL(`http://localhost:${serverConfig.frontendPort}`);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
env: {
...process.env,
HOST: '0.0.0.0',
PORT: '3000',
},
const serverConfig = require('../src/server-config.json');
async function startProductionServer() {
const serverPath = path.join(
process.resourcesPath,
'app.asar.unpacked',
'.output',
'server',
'index.mjs'
);
console.log('Starting production server:', serverPath);
serverProcess = spawn('node', [serverPath], {
stdio: 'inherit',
windowsHide: true,
env: {
...process.env,
HOST: serverConfig.host,
PORT: String(serverConfig.frontendPort),
},
});
await waitForServer(`http://localhost:${serverConfig.frontendPort}`);
}
Suggested change
env: {
...process.env,
HOST: '0.0.0.0',
PORT: '3000',
},
await mainWindow.loadURL(`http://localhost:${serverConfig.frontendPort}`);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 40 - 44, Replace the hardcoded env values
HOST and PORT in the spawned process env (the object that currently spreads
...process.env and sets HOST: '0.0.0.0' and PORT: '3000') so they are read from
the shared server configuration file (src/server-config.json) instead;
load/require the JSON (or import) at top of the module, pull host and port
values (e.g., config.host and config.port) and use those to set the env.HOST and
env.PORT before spawning the production server, and apply the same replacement
for the other occurrence referenced near line 79 so both spawn sites use the
single source of truth config.

});
}

// Create window
function createWindow() {
if (mainWindow) return;
await waitForServer('http://localhost:3000');
}
Comment on lines +26 to +48

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error handling for production server spawn failures.

The startProductionServer function spawns the Node.js server process but does not handle spawn errors or server crashes. If the server executable is missing, Node.js is unavailable, or the server exits immediately due to an error, the function will either hang (waiting for a server that never starts) or silently fail, leaving the user with a non-functional app.

🛡️ Add spawn error handling and process monitoring
 async function startProductionServer() {
   const serverPath = path.join(
     process.resourcesPath,
     'app.asar.unpacked',
     '.output',
     'server',
     'index.mjs'
   );
 
   console.log('Starting production server:', serverPath);
 
   serverProcess = spawn('node', [serverPath], {
     stdio: 'inherit',
     windowsHide: true,
     env: {
       ...process.env,
       HOST: '0.0.0.0',
       PORT: '3000',
     },
   });
+
+  serverProcess.on('error', (err) => {
+    console.error('Failed to start server process:', err);
+    throw err;
+  });
+
+  serverProcess.on('exit', (code, signal) => {
+    if (code !== null && code !== 0) {
+      console.error(`Server process exited with code ${code}`);
+    }
+    if (signal) {
+      console.error(`Server process killed by signal ${signal}`);
+    }
+  });
 
   await waitForServer('http://localhost:3000');
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function startProductionServer() {
const serverPath = path.join(
process.resourcesPath,
'app.asar.unpacked',
'.output',
'server',
'index.mjs'
);
console.log('Starting production server:', serverPath);
serverProcess = spawn('node', [serverPath], {
stdio: 'inherit',
windowsHide: true,
env: {
...process.env,
HOST: '0.0.0.0',
PORT: '3000',
},
});
}
// Create window
function createWindow() {
if (mainWindow) return;
await waitForServer('http://localhost:3000');
}
async function startProductionServer() {
const serverPath = path.join(
process.resourcesPath,
'app.asar.unpacked',
'.output',
'server',
'index.mjs'
);
console.log('Starting production server:', serverPath);
serverProcess = spawn('node', [serverPath], {
stdio: 'inherit',
windowsHide: true,
env: {
...process.env,
HOST: '0.0.0.0',
PORT: '3000',
},
});
serverProcess.on('error', (err) => {
console.error('Failed to start server process:', err);
throw err;
});
serverProcess.on('exit', (code, signal) => {
if (code !== null && code !== 0) {
console.error(`Server process exited with code ${code}`);
}
if (signal) {
console.error(`Server process killed by signal ${signal}`);
}
});
await waitForServer('http://localhost:3000');
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 26 - 48, startProductionServer currently
spawns the server but has no error/exit handling or cleanup, so add robust spawn
failure monitoring: when calling spawn in startProductionServer attach 'error'
and 'exit' listeners on serverProcess to log the error/exit code and
reject/throw so the caller can handle it, and ensure you kill/cleanup the
process on failure; also wrap the spawn+waitForServer sequence so if
waitForServer times out or rejects you terminate serverProcess and propagate the
error. Reference: startProductionServer, serverProcess, spawn, and
waitForServer.


// Create Electron window
async function createWindow() {
mainWindow = new BrowserWindow({
width: 1200,
height: 800,
show: false,
});

mainWindow.loadURL(`http://localhost:${serverPort}`);
// IMPORTANT
show: true,

// Show when ready
mainWindow.once('ready-to-show', () => {
mainWindow.show();
webPreferences: {
nodeIntegration: true,
contextIsolation: false,
},
Comment on lines +59 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Critical security risk: nodeIntegration enabled with contextIsolation disabled.

Enabling nodeIntegration: true and disabling contextIsolation: false allows the renderer process (which loads web content) to directly access Node.js APIs and Electron internals. This creates a severe security vulnerability: any XSS attack or malicious code execution in the web content can gain full system access. If the app loads any external content, user-generated content, or has any XSS vulnerability, an attacker can execute arbitrary code with full Node.js privileges.

Recommended mitigation:

  • Set nodeIntegration: false and contextIsolation: true (secure defaults)
  • If you need Node.js functionality in the renderer, use a preload script with contextBridge to expose only the specific APIs needed
  • Review Electron's security guidelines: https://www.electronjs.org/docs/latest/tutorial/security
🔒 Secure webPreferences configuration
     webPreferences: {
-      nodeIntegration: true,
-      contextIsolation: false,
+      nodeIntegration: false,
+      contextIsolation: true,
+      preload: path.join(__dirname, 'preload.cjs'), // Create preload script if needed
     },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 59 - 62, The current BrowserWindow
webPreferences dangerously enable nodeIntegration and disable contextIsolation
(webPreferences: { nodeIntegration: true, contextIsolation: false }), creating a
critical security risk; change these to safe defaults (nodeIntegration: false,
contextIsolation: true), add a preload script (pass preload path in
webPreferences.preload) and move any required Node APIs into a limited
contextBridge API inside that preload so renderer access is explicit and
minimal; update any renderer code that relied on global Node APIs to use the new
exposed preload methods instead.

});

// Debug only if needed
mainWindow.webContents.on('did-fail-load', (e, code, desc) => {
console.log("LOAD FAILED:", code, desc);
mainWindow.webContents.openDevTools();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Remove automatic DevTools in production builds.

Opening DevTools automatically (mainWindow.webContents.openDevTools()) is useful for development but should not be enabled in production builds. This exposes internal application details to end users and creates a poor user experience.

🔧 Conditionally open DevTools only in dev mode
-  mainWindow.webContents.openDevTools();
+  if (isDev) {
+    mainWindow.webContents.openDevTools();
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` at line 65, The call to
mainWindow.webContents.openDevTools() should only run in development; update the
code around where mainWindow is created so that openDevTools() is invoked
conditionally (e.g., guard it with a development check such as
process.env.NODE_ENV === 'development' or an isDev/isElectronDev flag or using
electron-is-dev) and ensure no DevTools are opened when running production
builds; locate the call to mainWindow.webContents.openDevTools() and wrap it in
that dev-only conditional (or replace it with a helper like isDev &&
mainWindow.webContents.openDevTools()).


try {
if (isDev) {
console.log("Loading DEV URL:", DEV_URL);

await waitForServer(DEV_URL);

await mainWindow.loadURL(DEV_URL);
} else {
console.log("Loading PROD URL");

await startProductionServer();

await mainWindow.loadURL("http://localhost:3000");
}

console.log("WINDOW LOADED");
} catch (err) {
console.error("LOAD ERROR:", err);
}
Comment on lines +67 to +85

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Improve error handling with user-facing feedback.

The current error handling logs to console but provides no feedback to the user when the window fails to load. If the server fails to start or the URL fails to load, the user sees a blank window with no indication of what went wrong.

📢 Show error dialog on load failure
+  const { dialog } = require('electron');
+
   try {
     if (isDev) {
       console.log("Loading DEV URL:", DEV_URL);
 
       await waitForServer(DEV_URL);
 
       await mainWindow.loadURL(DEV_URL);
     } else {
       console.log("Loading PROD URL");
 
       await startProductionServer();
 
       await mainWindow.loadURL("http://localhost:3000");
     }
 
     console.log("WINDOW LOADED");
   } catch (err) {
     console.error("LOAD ERROR:", err);
+    dialog.showErrorBox(
+      'Failed to Start Application',
+      `The application failed to start: ${err.message}\n\nPlease check the logs for details.`
+    );
+    app.quit();
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/main.cjs` around lines 67 - 85, The catch block for the window load
sequence only logs errors to console, leaving users with a blank window; update
the error handling in the try/catch around waitForServer/startProductionServer
and mainWindow.loadURL to show a user-facing error dialog and/or load a local
error HTML page: use Electron's dialog (e.g., dialog.showErrorBox or
dialog.showMessageBox) to display a clear message containing the error and a
friendly title, and as a fallback call mainWindow.loadFile or mainWindow.loadURL
to display an embedded error page; reference the existing symbols mainWindow,
waitForServer, startProductionServer, DEV_URL and mainWindow.loadURL when
implementing the dialog and fallback load.


mainWindow.on("closed", () => {
mainWindow = null;
});

mainWindow.webContents.on(
"did-fail-load",
(_, code, desc) => {
console.log("FAILED LOAD:", code, desc);
}
);

mainWindow.webContents.on(
"render-process-gone",
(_, details) => {
console.log("RENDER GONE:", details);
}
);
}

// App start
app.whenReady().then(async () => {
await startServer();
createWindow();
});
app.whenReady().then(createWindow);

// Cleanup
app.on('window-all-closed', () => {
if (serverProcess) serverProcess.kill();
if (process.platform !== 'darwin') app.quit();

if (process.platform !== 'darwin') {
app.quit();
}
});
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"start": "vite preview --host --open",
"build": "vite build",
"electron": "npx electron .",
"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",
"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && cross-env VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Script synchronization assumes Vite dev server URL matches hardcoded value.

The electron-dev script waits for http://localhost:3000, then sets VITE_DEV_SERVER_URL=http://localhost:3000 when launching Electron. However, if the Vite dev server is configured to run on a different port (e.g., via vite.config.ts or command-line flags), the script will wait for the wrong URL and potentially hang or connect to an incorrect server.

This is currently correct based on src/server-config.json (frontendPort: 3000), but creates a maintenance coupling between the script and the config file.

Consider reading the port from server-config.json dynamically, or at minimum, add a comment documenting the port dependency:

📝 Document port dependency
-		"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && cross-env VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",
+		"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && cross-env VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",
+		"_comment_electron-dev": "Port 3000 must match frontendPort in src/server-config.json",

Or for a more robust solution, use a small helper script to read the port:

"electron-dev": "node scripts/run-electron-dev.js"
// scripts/run-electron-dev.js
const { spawn } = require('child_process');
const config = require('../src/server-config.json');
const port = config.frontendPort;
const url = `http://localhost:${port}`;

spawn('concurrently', [
  '"vite"',
  `"wait-on ${url} && cross-env VITE_DEV_SERVER_URL=${url} electron ."`
], { stdio: 'inherit', shell: true });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && cross-env VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",
"electron-dev": "concurrently \"vite\" \"wait-on http://localhost:3000 && cross-env VITE_DEV_SERVER_URL=http://localhost:3000 electron .\"",
"_comment_electron-dev": "Port 3000 must match frontendPort in src/server-config.json",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 13, The electron-dev npm script hardcodes port 3000
which couples it to src/server-config.json; update the script to read the
frontend port dynamically (or at minimum document the dependency). Replace the
current "electron-dev" entry with a tiny Node helper (e.g.,
scripts/run-electron-dev.js) that requires ../src/server-config.json, builds the
URL from config.frontendPort, and spawns the concurrently command using that URL
(or add a clear comment by the electron-dev script referencing
src/server-config.json frontendPort and noting the required port if you prefer
the simpler change). Ensure the helper is referenced from package.json as
"electron-dev": "node scripts/run-electron-dev.js" and that the helper uses the
built URL for both wait-on and VITE_DEV_SERVER_URL.

"dist": "electron-builder",
"preview": "vite preview",
"test": "vitest run",
Expand Down Expand Up @@ -50,6 +50,7 @@
"@vitejs/plugin-react": "^5.2.0",
"autoprefixer": "^10.4.23",
"concurrently": "^9.2.1",
"cross-env": "^10.1.0",
"daisyui": "^5.5.14",
"electron": "^40.6.1",
"electron-builder": "^26.8.1",
Expand Down
Loading