Skip to content

Electron packaging workflow#237

Merged
imxade merged 4 commits into
AOSSIE-Org:mainfrom
vinayak302004:electron-packaging-workflow
Mar 3, 2026
Merged

Electron packaging workflow#237
imxade merged 4 commits into
AOSSIE-Org:mainfrom
vinayak302004:electron-packaging-workflow

Conversation

@vinayak302004

@vinayak302004 vinayak302004 commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Addressed Issues:

Fixes #115

Description

Electron Packaging for Rein
Cross Platform

Screenshots/Recordings:

[]

(https://drive.google.com/file/d/1VOWRTHo9XnmoR5teKfiEmc9haILW7WXb/view?usp=sharing)

Screenshot 2026-03-03 122504

Functional Verification

  • Please check off the behaviors verified with this change.

Screen Mirror

  • Screen MIrror works.

Authentication

  • Connection doesn't work without a valid token.

Basic Gestures

  • One-finger tap: Verified as Left Click.

  • Two-finger tap: Verified as Right Click.

  • Click and drag: Verified selection behavior.

  • Pinch to zoom: Verified zoom functionality (if applicable).

Modes & Settings

  • Cursor mode: Cursor moves smoothly and accurately.

  • Scroll mode: Page scrolls as expected.

  • Sensitivity: Verified changes in cursor speed/sensitivity settings.

  • Copy and Paste: Verified both Copy and Paste functionality.

  • Invert Scrolling: Verified scroll direction toggles correctly.

Advanced Input

  • Key combinations: Verified "hold" behavior for modifiers (e.g., Ctrl+C) and held keys are shown in buffer.

  • Keyboard input: Verified Space, Backspace, and Enter keys work correctly.

  • Glide typing: Verified path drawing and text output.

  • Voice input: Verified speech-to-text functionality for full sentences.

  • Backspace doesn't send the previous input.

Any other gesture or input behavior introduced:

  • New Gestures: Verified any other gesture or input behavior introduced in this PR.

Additional Notes:

Checklist

  • [ X ] My PR addresses a single issue, fixes a single bug or makes a single improvement.

  • [ X ] My code follows the project's code style and conventions

  • [ X ] I have performed a self-review of my own code

  • [ X ] I have commented my code, particularly in hard-to-understand areas

  • If applicable, I have made corresponding changes or additions to the documentation

  • If applicable, I have made corresponding changes or additions to tests

  • [ X ] My changes generate no new warnings or errors

  • [ X ] I have joined the and I will share a link to this PR with the project maintainers there

  • [ X ] I have read the

  • [ X ] Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

  • [ X ] Incase of UI change I've added a demo video.

⚠️ AI Notice - Important!
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.

Summary by CodeRabbit

  • New Features

    • Desktop application support: app can run and be packaged as native installers for Windows, macOS, and Linux.
    • Local production server startup integrated so the desktop app loads the shipped frontend.
  • Chores

    • Added automated CI build workflow to produce platform-specific artifacts.
    • Added packaging and distribution scripts/configuration and related dependencies.
    • Vite configured for relative base path and dedicated build output directory.

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds Electron packaging and runtime glue: a new GitHub Actions workflow for multi-OS builds, an Electron main process that spawns and waits for a bundled Nitro server, and build/package configuration and scripts to produce desktop distributables.

Changes

Cohort / File(s) Summary
CI/CD Workflow
.github/workflows/build.yml
New GitHub Actions workflow "Build Electron" with an OS matrix (windows, ubuntu, macos) to install Node, build, rebuild native modules, package the Electron app, and upload artifacts.
Electron Main Process
electron/main.cjs
New main process script enforcing single-instance lock, spawning a packaged Nitro server (app.asar.unpacked/.output/server/index.mjs), polling http://localhost:3000 until ready, creating a hidden BrowserWindow (1200x800) that loads the server URL, and cleaning up the server process on exit.
Package & Build Config
package.json, vite.config.ts
Added main entry, version, Electron-related scripts (electron, electron-dev, dist), dependencies (electron, electron-builder, concurrently, wait-on), Electron Builder config (appId, productName, asarUnpack, extraResources, files, NSIS options), and Vite base: "./" plus build output to .output with emptyOutDir true.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant App as Electron App
    participant ServerProc as Server Process
    participant ServerHTTP as Server (HTTP)
    participant Window as BrowserWindow

    User->>App: Launch application
    App->>App: requestSingleInstanceLock
    alt lock acquired
        App->>ServerProc: spawn node bundled server (PORT=3000)
        loop poll until ready
            App->>ServerHTTP: GET http://127.0.0.1:3000
            alt not ready
                ServerHTTP-->>App: connection error / no response
                App->>App: wait 500ms
            else ready
                ServerHTTP-->>App: 200 OK
                App->>Window: create BrowserWindow (hidden)
                Window->>Window: load http://127.0.0.1:3000
                Window->>User: show UI
            end
        end
    else lock not acquired
        App->>App: exit (another instance running)
    end

    User->>Window: close window
    Window->>App: window-all-closed
    App->>ServerProc: terminate server process
    App->>App: quit (except macOS)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

Typescript Lang

Poem

🐇 I hopped into code with a packaging plan,
Spawned a server and opened a window — tan!
Builds on three platforms, artifacts in store,
A cozy app bundle to run and explore.
Hop, pack, ship — desktop dreams galore! 🚀

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements Electron packaging with cross-platform CI/CD automation [#115], but lacks documentation of testing findings in packaging.md and doesn't verify persistence of authentication/port configuration after restart. Add packaging.md documenting testing results and verify that authentication and port settings persist across application restarts as required by #115.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Electron packaging workflow' is partially related to the changeset, describing the technical approach but not emphasizing the main objective of implementing cross-platform packaging for Rein as stated in issue #115.
Description check ✅ Passed The PR description addresses issue #115 and includes a demo video/screenshot, but most functional verification checkboxes remain unchecked and no documentation (packaging.md) was added as suggested in the linked issue.
Out of Scope Changes check ✅ Passed All changes are in scope: GitHub Actions workflow, Electron main process, package.json configuration, and Vite settings directly support the Electron packaging objective from #115.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/build.yml:
- Around line 7-10: The pull_request path filter currently only includes the
paths array with "src/**" and "public/**" so packaging or workflow changes can
bypass CI; update the pull_request.paths configuration in the same build
workflow to also include patterns for electron/**, package.json (and lockfile),
vite.config.ts, and .github/workflows/** (or a top-level pattern like "*", as
appropriate) so changes to bundling, packaging, or workflows trigger this job;
modify the paths array entry that currently lists "src/**" and "public/**" to
add those additional glob entries.
- Around line 38-50: The workflow currently deletes package-lock.json and uses
forced installs, breaking reproducibility; update the steps named "Clean
node_modules and npm cache (macOS/Linux)" and "Install dependencies
(macOS/Linux)" so they do NOT remove package-lock.json and do NOT use --force,
and change the non-Windows install to use npm ci (like the "Install dependencies
(Windows)" step uses npm ci --include=optional) so all platforms perform
lockfile-driven installs; keep any harmless cache cleanup (npm cache clean) if
desired but ensure installs rely on the lockfile and remove --force flags.

In `@electron/main.cjs`:
- Around line 17-25: The waitForServer function currently retries forever;
change waitForServer to accept an optional timeoutMs (e.g., default 30000) and
implement a bounded wait: start a timer that rejects the returned Promise with a
clear Error once timeoutMs elapses and cancels further retries, and ensure
successful http.get clears that timer and resolves; update any other indefinite
retry logic (the second retry site referenced in the review) to either call the
new waitForServer with a timeout or implement the same timeout+reject behavior,
and make callers of waitForServer handle the rejection path (log an error and
quit the app or surface a failure UI) instead of hanging.
- Around line 79-82: The startup always calls startServer() and opens the local
build instead of respecting VITE_DEV_SERVER_URL; update the app.whenReady() flow
to check process.env.VITE_DEV_SERVER_URL and, if present, skip startServer(),
have createWindow() load that dev URL (instead of default localhost:3000),
otherwise call await startServer() and load the production output; reference
startServer, createWindow and process.env.VITE_DEV_SERVER_URL when making the
conditional so dev mode uses the VITE_DEV_SERVER_URL and production still starts
the server and loads the packaged UI.
- Around line 41-49: The code spawns an external 'node' which fails in packaged
Electron apps; change the spawn call that creates serverProcess (spawn(...,
[serverPath], ...)) to use process.execPath as the executable and add
ELECTRON_RUN_AS_NODE=1 to the spawned env; keep other env merges
(...process.env, HOST, PORT) and preserve stdio/windowsHide options so the
bundled Electron runtime runs the serverPath script instead of relying on a
system Node installation.

In `@package.json`:
- Line 13: The package.json "electron-dev" npm script uses cross-env but
cross-env is not declared in devDependencies; add cross-env to devDependencies
(either run npm install --save-dev cross-env or add "cross-env": "<version>"
under devDependencies in package.json) so the "electron-dev" script can run
successfully; update package.json devDependencies and commit the change.

In `@vite.config.ts`:
- Around line 46-50: The build configuration object (build { outDir: ".output",
emptyOutDir: true }) in vite.config.ts is not formatted to satisfy Biome; run
the project's Biome formatter (or the configured code formatter) on
vite.config.ts, accept the formatter changes so the build object is properly
spaced/indented, and commit the resulting formatted file; specifically ensure
the build block containing the symbols "build", "outDir", and "emptyOutDir"
matches the project's formatting rules before pushing.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c56e381 and 0a4f007.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • .github/workflows/build.yml
  • electron/main.cjs
  • package.json
  • vite.config.ts

Comment thread .github/workflows/build.yml
Comment on lines +38 to +50
- name: Clean node_modules and npm cache (macOS/Linux)
if: runner.os != 'Windows'
run: |
rm -rf node_modules package-lock.json || true
npm cache clean --force

- name: Install dependencies (Windows)
if: runner.os == 'Windows'
run: npm ci --include=optional

- name: Install dependencies (macOS/Linux)
if: runner.os != 'Windows'
run: npm install --include=optional --force

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

Avoid deleting lockfile and forced installs in CI.

Line 41 removes package-lock.json, and Line 50 uses npm install --force. This makes CI dependency resolution non-reproducible and can hide real dependency issues. Use lockfile-driven installs (npm ci) across OSes.

Suggested deterministic install flow
-      - name: Clean node_modules and npm cache (macOS/Linux)
+      - name: Clean node_modules and npm cache (macOS/Linux)
         if: runner.os != 'Windows'
         run: |
-          rm -rf node_modules package-lock.json || true
+          rm -rf node_modules || true
           npm cache clean --force

-      - name: Install dependencies (macOS/Linux)
+      - name: Install dependencies (macOS/Linux)
         if: runner.os != 'Windows'
-        run: npm install --include=optional --force
+        run: npm ci --include=optional
📝 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
- name: Clean node_modules and npm cache (macOS/Linux)
if: runner.os != 'Windows'
run: |
rm -rf node_modules package-lock.json || true
npm cache clean --force
- name: Install dependencies (Windows)
if: runner.os == 'Windows'
run: npm ci --include=optional
- name: Install dependencies (macOS/Linux)
if: runner.os != 'Windows'
run: npm install --include=optional --force
- name: Clean node_modules and npm cache (macOS/Linux)
if: runner.os != 'Windows'
run: |
rm -rf node_modules || true
npm cache clean --force
- name: Install dependencies (Windows)
if: runner.os == 'Windows'
run: npm ci --include=optional
- name: Install dependencies (macOS/Linux)
if: runner.os != 'Windows'
run: npm ci --include=optional
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yml around lines 38 - 50, The workflow currently
deletes package-lock.json and uses forced installs, breaking reproducibility;
update the steps named "Clean node_modules and npm cache (macOS/Linux)" and
"Install dependencies (macOS/Linux)" so they do NOT remove package-lock.json and
do NOT use --force, and change the non-Windows install to use npm ci (like the
"Install dependencies (Windows)" step uses npm ci --include=optional) so all
platforms perform lockfile-driven installs; keep any harmless cache cleanup (npm
cache clean) if desired but ensure installs rely on the lockfile and remove
--force flags.

Comment thread electron/main.cjs
Comment on lines +17 to +25
function waitForServer(url) {
return new Promise((resolve) => {
const check = () => {
http
.get(url, () => resolve())
.on('error', () => setTimeout(check, 500));
};
check();
});

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

Add timeout + failure path to server readiness check.

Lines 17-25 and 51-52 retry forever and never reject. If the server fails to boot, the app hangs before creating a window.

Suggested bounded readiness check
-function waitForServer(url) {
-  return new Promise((resolve) => {
+function waitForServer(url, timeoutMs = 30000) {
+  return new Promise((resolve, reject) => {
+    const start = Date.now();
     const check = () => {
+      if (Date.now() - start > timeoutMs) {
+        return reject(new Error("Server readiness timeout"));
+      }
       http
-        .get(url, () => resolve())
+        .get(url, (res) => {
+          res.resume();
+          resolve();
+        })
         .on('error', () => setTimeout(check, 500));
     };
     check();
   });
 }

As per coding guidelines, "The code adheres to best practices associated with Node.js".

Also applies to: 51-52

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@electron/main.cjs` around lines 17 - 25, The waitForServer function currently
retries forever; change waitForServer to accept an optional timeoutMs (e.g.,
default 30000) and implement a bounded wait: start a timer that rejects the
returned Promise with a clear Error once timeoutMs elapses and cancels further
retries, and ensure successful http.get clears that timer and resolves; update
any other indefinite retry logic (the second retry site referenced in the
review) to either call the new waitForServer with a timeout or implement the
same timeout+reject behavior, and make callers of waitForServer handle the
rejection path (log an error and quit the app or surface a failure UI) instead
of hanging.

Comment thread electron/main.cjs
Comment on lines +41 to +49
serverProcess = spawn('node', [serverPath], {
stdio: 'ignore', // no terminal
windowsHide: true, // hide CMD
env: {
...process.env,
HOST: '127.0.0.1',
PORT: '3000',
},
});

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the actual code at lines 41-49 in electron/main.cjs
echo "=== Lines 41-49 from electron/main.cjs ==="
sed -n '41,49p' electron/main.cjs

echo -e "\n=== Search for spawn calls with 'node' in electron/main.cjs ==="
rg -n "spawn\(['\"]node['\"]" electron/main.cjs

echo -e "\n=== Search for process.execPath usage in electron/main.cjs ==="
rg -n "process\.execPath|ELECTRON_RUN_AS_NODE" electron/main.cjs

echo -e "\n=== Full electron/main.cjs to understand context ==="
cat -n electron/main.cjs

Repository: AOSSIE-Org/Rein

Length of output: 3058


Use the bundled Electron Node.js runtime instead of assuming system node is available.

Line 41 spawns 'node' directly, which will fail when the Electron app is packaged and distributed to end users who do not have Node.js installed. Use process.execPath with ELECTRON_RUN_AS_NODE=1 to run Node.js code within the bundled Electron runtime:

Suggested fix
-    serverProcess = spawn('node', [serverPath], {
+    serverProcess = spawn(process.execPath, [serverPath], {
       stdio: 'ignore',       // no terminal
       windowsHide: true,     // hide CMD
       env: {
         ...process.env,
+        ELECTRON_RUN_AS_NODE: '1',
         HOST: '127.0.0.1',
         PORT: '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
serverProcess = spawn('node', [serverPath], {
stdio: 'ignore', // no terminal
windowsHide: true, // hide CMD
env: {
...process.env,
HOST: '127.0.0.1',
PORT: '3000',
},
});
serverProcess = spawn(process.execPath, [serverPath], {
stdio: 'ignore', // no terminal
windowsHide: true, // hide CMD
env: {
...process.env,
ELECTRON_RUN_AS_NODE: '1',
HOST: '127.0.0.1',
PORT: '3000',
},
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@electron/main.cjs` around lines 41 - 49, The code spawns an external 'node'
which fails in packaged Electron apps; change the spawn call that creates
serverProcess (spawn(..., [serverPath], ...)) to use process.execPath as the
executable and add ELECTRON_RUN_AS_NODE=1 to the spawned env; keep other env
merges (...process.env, HOST, PORT) and preserve stdio/windowsHide options so
the bundled Electron runtime runs the serverPath script instead of relying on a
system Node installation.

Comment thread electron/main.cjs
Comment on lines +79 to +82
app.whenReady().then(async () => {
await startServer();
createWindow();
});

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

Honor VITE_DEV_SERVER_URL to keep electron-dev functional.

Lines 79-82 always call startServer() and then open http://localhost:3000. This ignores the development URL contract and can fail when .output is unavailable during dev.

Suggested dev/prod startup split
 app.whenReady().then(async () => {
-  await startServer();
-  createWindow();
+  const devUrl = process.env.VITE_DEV_SERVER_URL;
+  if (!devUrl) {
+    await startServer();
+  }
+  createWindow(devUrl || '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
app.whenReady().then(async () => {
await startServer();
createWindow();
});
app.whenReady().then(async () => {
const devUrl = process.env.VITE_DEV_SERVER_URL;
if (!devUrl) {
await startServer();
}
createWindow(devUrl || 'http://localhost:3000');
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@electron/main.cjs` around lines 79 - 82, The startup always calls
startServer() and opens the local build instead of respecting
VITE_DEV_SERVER_URL; update the app.whenReady() flow to check
process.env.VITE_DEV_SERVER_URL and, if present, skip startServer(), have
createWindow() load that dev URL (instead of default localhost:3000), otherwise
call await startServer() and load the production output; reference startServer,
createWindow and process.env.VITE_DEV_SERVER_URL when making the conditional so
dev mode uses the VITE_DEV_SERVER_URL and production still starts the server and
loads the packaged UI.

Comment thread package.json
"start": "vite preview --host --open",
"build": "vite build",
"electron": "npx 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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
import json
with open("package.json", "r", encoding="utf-8") as f:
    p = json.load(f)
print("electron-dev script:", p.get("scripts", {}).get("electron-dev"))
print("cross-env in dependencies:", "cross-env" in p.get("dependencies", {}))
print("cross-env in devDependencies:", "cross-env" in p.get("devDependencies", {}))
print("\nAll devDependencies:")
for dep in sorted(p.get("devDependencies", {}).keys()):
    print(f"  {dep}")
PY

Repository: AOSSIE-Org/Rein

Length of output: 651


Add missing cross-env dependency.

The electron-dev script uses cross-env to set environment variables, but the package is not declared in devDependencies. Running npm run electron-dev will fail with a missing command error.

Suggested fix
 	"devDependencies": {
+		"cross-env": "^7.0.3",
 		"@biomejs/biome": "^1.9.4",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 13, The package.json "electron-dev" npm script uses
cross-env but cross-env is not declared in devDependencies; add cross-env to
devDependencies (either run npm install --save-dev cross-env or add "cross-env":
"<version>" under devDependencies in package.json) so the "electron-dev" script
can run successfully; update package.json devDependencies and commit the change.

Comment thread vite.config.ts Outdated

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

♻️ Duplicate comments (2)
.github/workflows/build.yml (2)

8-14: ⚠️ Potential issue | 🟡 Minor

Include lockfile updates in PR path filters.

Lines 8-14 omit package-lock.json, so dependency-resolution changes can bypass this workflow.

Suggested fix
   pull_request:
     paths:
       - "src/**"
       - "public/**"
       - "electron/**"
       - "package.json"
+      - "package-lock.json"
       - "vite.config.ts"
       - ".github/workflows/**"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yml around lines 8 - 14, Update the GitHub Actions
workflow "paths" filter in .github/workflows/build.yml to include lockfile(s) so
dependency updates trigger the workflow; modify the paths list that currently
contains "src/**", "public/**", "electron/**", "package.json", "vite.config.ts",
and ".github/workflows/**" to also include package-lock.json (and any other
lockfiles your repo uses, e.g., yarn.lock or pnpm-lock.yaml) so changes to
lockfiles run the build workflow.

42-54: ⚠️ Potential issue | 🟠 Major

Use lockfile-driven installs on macOS/Linux.

Line 45 deletes package-lock.json, and Line 54 uses npm install --force; this makes CI non-reproducible and can mask dependency issues.

Suggested fix
       - name: Clean node_modules and npm cache (macOS/Linux)
         if: runner.os != 'Windows'
         run: |
-          rm -rf node_modules package-lock.json || true
+          rm -rf node_modules || true
           npm cache clean --force

       - name: Install dependencies (macOS/Linux)
         if: runner.os != 'Windows'
-        run: npm install --include=optional --force
+        run: npm ci --include=optional
#!/bin/bash
set -euo pipefail

# Verify non-deterministic install patterns in workflow
rg -n -C2 'package-lock\.json|npm install --include=optional --force|npm ci --include=optional' .github/workflows/build.yml

As per coding guidelines, "The code adheres to best practices associated with Node.js".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yml around lines 42 - 54, The workflow currently
deletes package-lock.json and runs non-lockfile installs (the "Clean
node_modules and npm cache (macOS/Linux)" and "Install dependencies
(macOS/Linux)" steps), which makes CI non-reproducible; stop removing
package-lock.json, replace the macOS/Linux install step that runs "npm install
--include=optional --force" with a lockfile-driven install using "npm ci
--include=optional" (matching the Windows step), and remove the "--force" flag;
ensure any cache-cleaning still preserves the lockfile so installs are
deterministic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/build.yml:
- Around line 4-6: The workflow currently scopes push triggers to a single
branch by setting push.branches to ["electron-packaging-workflow"]; update the
push trigger in the workflow to remove the branches filter (so it runs on all
pushes) or include the main branch (e.g., add "main") instead of limiting to
"electron-packaging-workflow" so packaging CI runs for merges to main; edit the
push -> branches section in the workflow YAML to either delete the branches key
or add "main" alongside "electron-packaging-workflow".

---

Duplicate comments:
In @.github/workflows/build.yml:
- Around line 8-14: Update the GitHub Actions workflow "paths" filter in
.github/workflows/build.yml to include lockfile(s) so dependency updates trigger
the workflow; modify the paths list that currently contains "src/**",
"public/**", "electron/**", "package.json", "vite.config.ts", and
".github/workflows/**" to also include package-lock.json (and any other
lockfiles your repo uses, e.g., yarn.lock or pnpm-lock.yaml) so changes to
lockfiles run the build workflow.
- Around line 42-54: The workflow currently deletes package-lock.json and runs
non-lockfile installs (the "Clean node_modules and npm cache (macOS/Linux)" and
"Install dependencies (macOS/Linux)" steps), which makes CI non-reproducible;
stop removing package-lock.json, replace the macOS/Linux install step that runs
"npm install --include=optional --force" with a lockfile-driven install using
"npm ci --include=optional" (matching the Windows step), and remove the
"--force" flag; ensure any cache-cleaning still preserves the lockfile so
installs are deterministic.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a4f007 and b6e5009.

📒 Files selected for processing (2)
  • .github/workflows/build.yml
  • vite.config.ts

Comment on lines +4 to +6
push:
branches:
- electron-packaging-workflow

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

Do not scope push builds to a feature branch only.

Line 6 limits push runs to electron-packaging-workflow, so packaging CI will not run on main pushes after merge.

Suggested fix
 on:
   push:
     branches:
+      - main
       - electron-packaging-workflow
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yml around lines 4 - 6, The workflow currently
scopes push triggers to a single branch by setting push.branches to
["electron-packaging-workflow"]; update the push trigger in the workflow to
remove the branches filter (so it runs on all pushes) or include the main branch
(e.g., add "main") instead of limiting to "electron-packaging-workflow" so
packaging CI runs for merges to main; edit the push -> branches section in the
workflow YAML to either delete the branches key or add "main" alongside
"electron-packaging-workflow".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Workflow] Packaging pipelines

2 participants