Skip to content

feat: add Ubuntu/Linux .deb build support#44

Open
davebulaval wants to merge 9 commits into
Charlie85270:mainfrom
davebulaval:feat/ubuntu-linux-build
Open

feat: add Ubuntu/Linux .deb build support#44
davebulaval wants to merge 9 commits into
Charlie85270:mainfrom
davebulaval:feat/ubuntu-linux-build

Conversation

@davebulaval

@davebulaval davebulaval commented Mar 28, 2026

Copy link
Copy Markdown

Summary

  • Add .deb package build for Ubuntu/Linux (x64) via electron-builder
  • Add GitHub Actions workflow (build-linux.yml) triggered on tags and workflow_dispatch
  • CI includes smoke tests: package size check (>50MB), dpkg-deb metadata validation, binary presence verification

What's included

  • package.json: Linux electron-builder config (deb target), electron:build:linux / electron:pack:linux scripts, added description and author fields
  • electron/main.ts: EPIPE error handler for stdout/stderr on Linux (crashes when launched from .desktop entries with no TTY)
  • .github/workflows/build-linux.yml: Full CI pipeline — build, smoke test, upload artifact, publish to GitHub Release on tags
  • build/linux/after-install.sh: Post-install script that sets SUID bit on chrome-sandbox (required for Electron sandbox)
  • Desktop entry: --no-sandbox flag, Development category, StartupNotify=true

Tested on

  • Ubuntu 24.04 (x86_64)
  • dpkg -i install + launch from /opt/Dorothy/dorothy
  • Launch from desktop entry ✅
  • All pages load (agents, settings, memory, vault, kanban, etc.) ✅
  • API server starts on 127.0.0.1:31415

Test plan

  • CI build passes on GitHub Actions
  • .deb artifact is downloadable from workflow run
  • Install with sudo dpkg -i Dorothy-*.deb on Ubuntu 22.04+
  • App launches from application menu and terminal
  • Auto-updater gracefully handles missing latest-linux.yml (expected until first Linux release)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Official Linux (Ubuntu/Debian) build pipeline that produces and uploads .deb artifacts for tag-triggered releases and manual runs.
  • Improvements
    • Native .deb packaging with desktop metadata for smoother installation and launching on Linux.
    • Built-in safeguards to ignore common broken-pipe stream errors on Linux.
    • Added install/remove hooks to ensure correct sandbox permissions for Chromium/Electron.
  • Chores
    • Centralized Electron build preparation and packaging scripts; added package metadata.

davebulaval and others added 3 commits March 27, 2026 11:59
Add electron-builder Linux configuration with AppImage and deb targets,
npm scripts for Linux builds, and a GitHub Actions workflow that builds
on ubuntu-latest and uploads artifacts to releases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…PIPE)

- Remove AppImage target, keep .deb only for Ubuntu/Linux
- Add CI smoke tests: size check, dpkg-deb metadata, binary presence
- Fix EPIPE errors on stdout/stderr for desktop entry launches
- Add --no-sandbox flag to .desktop file executable args
- Add description and author fields to package.json
- Wire after-install script via fpm for chrome-sandbox SUID

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 28, 2026

Copy link
Copy Markdown

@davebulaval is attempting to deploy a commit to the Charlie Rabiller's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Linux packaging and CI: a GitHub Actions workflow builds, validates, and uploads Debian .deb artifacts; package.json gains Linux build scripts and deb metadata; helper scripts manage chrome-sandbox permissions; main process ignores Linux EPIPE on stdout/stderr.

Changes

Cohort / File(s) Summary
CI: GitHub Actions
​.github/workflows/build-linux.yml
New workflow triggered on v* tag pushes and workflow_dispatch; sets up Node 20, installs deps and libarchive-tools, runs npm run electron:build:linux, validates release/*.deb (dpkg info, contains opt/Dorothy/dorothy, non-zero size), uploads artifacts, and publishes .deb to GitHub Release for tag runs.
Packaging & Scripts
package.json, scripts/electron-prepare.sh
Added description and author; introduced electron:prepare, electron:build:linux, electron:pack:linux; simplified mac build/pack scripts to use electron:prepare; added build.linux and build.deb metadata (category, icon, deb target, maintainer, fpm hooks, runtime depends, artifactName template).
Installer helpers
build/linux/after-install.sh, build/linux/before-remove.sh
New post-install script sets owner root:root and mode 4755 on /opt/Dorothy/chrome-sandbox (idempotent). New pre-remove script resets permissions to 0755 if the sandbox file exists.
Runtime error handling
electron/main.ts
Linux-only: registers error listeners on process.stdout and process.stderr to ignore EPIPE errors and re-throw other NodeJS.ErrnoException.

Sequence Diagram(s)

sequenceDiagram
    participant Tag as Git Tag / Manual Trigger
    participant GH as GitHub Actions
    participant Runner as Ubuntu Runner
    participant NPM as npm / electron-builder
    participant Art as Artifact Storage
    participant Rel as GitHub Release

    Tag->>GH: push tag / workflow_dispatch
    GH->>Runner: start `build-linux` job (ubuntu-latest)
    Runner->>NPM: checkout, setup Node.js 20, npm ci
    Runner->>Runner: apt-get install libarchive-tools
    Runner->>NPM: run `npm run electron:build:linux` (electron-builder)
    NPM-->>Runner: produce `release/*.deb`
    Runner->>Runner: validate .deb files (dpkg-deb info, contains `opt/Dorothy/dorothy`, non-zero size)
    Runner->>Art: upload `.deb` artifacts
    alt refs/tags/v*
        Runner->>Rel: upload `.deb` to GitHub Release
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through builds and signed each bit,

Debs tucked snug, the sandbox fits,
EPIPE sneezed and I said "not today",
Artifacts packed and sent away,
Dorothy dances, ready to transmit.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add Ubuntu/Linux .deb build support' directly and accurately summarizes the main change: adding Linux .deb packaging support with associated build infrastructure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@davebulaval

Copy link
Copy Markdown
Author

Just to be clear, I never built (before working with Claude Code) a .deb for Ubuntu. So it is 100% Claude Code code generation, but I have tested the app locally, and it works.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
package.json (1)

19-22: Consolidate duplicated Electron build pipelines.

The four scripts duplicate a long command chain; this is easy to drift and hard to maintain. Extract the shared prep/build steps into one script and pass platform flags.

♻️ Suggested script consolidation
"scripts": {
-  "electron:build": "bash -c '...; electron-builder --mac'",
-  "electron:build:linux": "bash -c '...; electron-builder --linux'",
-  "electron:pack": "tsc ... && electron-builder --dir --mac",
-  "electron:pack:linux": "tsc ... && electron-builder --dir --linux"
+  "electron:prepare": "bash scripts/electron-prepare.sh",
+  "electron:build": "npm run electron:prepare && electron-builder --mac",
+  "electron:build:linux": "npm run electron:prepare && electron-builder --linux",
+  "electron:pack": "npm run electron:prepare && electron-builder --dir --mac",
+  "electron:pack:linux": "npm run electron:prepare && electron-builder --dir --linux"
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 19 - 22, The four npm scripts ("electron:build",
"electron:build:linux", "electron:pack", "electron:pack:linux") duplicate the
same prep and subproject build chain; extract that common sequence into a single
script (e.g., "electron:prepare" or "electron:build:common") that performs the
mv/backups/trap, NEXT/TSC build steps and the looped npm install/npm run build
for mcp-* packages, and then have the four platform-specific scripts simply set
platform flags/environment (ELECTRON_BUILD or a PLATFORM var) and call the
shared script before invoking electron-builder with the appropriate platform
arguments. Update references to the unique script names ("electron:prepare" and
the original four) and ensure the backup/trap logic and tsc invocation remain in
the shared script so behavior is unchanged.
🤖 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-linux.yml:
- Around line 1-13: The workflow fails in orgs with hardened token settings
because the release upload step using softprops/action-gh-release@v2 requires
write access to repo contents but no explicit permissions are declared; add an
explicit permissions block to the workflow (top-level) granting contents: write
so the build-linux job and the release upload step can write release assets
(reference: job name build-linux and action softprops/action-gh-release@v2).
- Around line 44-48: The current DEB handling uses DEB=$(ls release/*.deb) and
then stat "$DEB", which breaks if multiple .deb files exist; update the
validation block to iterate over release/*.deb (or explicitly select a single
file e.g., the first or largest) and perform the existence check and stat on
each candidate rather than on a concatenated string. Specifically change the
logic around the DEB variable and DEB_SIZE calculation so you loop (for deb in
release/*.deb) or use an array expansion to set DEB to a single path before
calling stat, and emit an error/exit if any checked .deb is missing or below the
50MB threshold; touch references to DEB and DEB_SIZE in that block so the script
works with multiple artifacts.

In `@package.json`:
- Around line 124-135: Remove the global "--no-sandbox" from the
linux.executableArgs in package.json and instead implement the SUID setup script
referenced as build/linux/after-install.sh to set the chrome-sandbox binary
owner to root and the setuid bit (e.g., chown root:root chrome-sandbox && chmod
4755 chrome-sandbox) as part of the package post-install; alternatively, if you
must keep a flag for dev builds, gate executableArgs so it only applies in
development (not production) and wire a postinstall hook in package.json that
invokes build/linux/after-install.sh; ensure the script exists, is executable,
idempotent, and documents why it sets SUID on chrome-sandbox.

---

Nitpick comments:
In `@package.json`:
- Around line 19-22: The four npm scripts ("electron:build",
"electron:build:linux", "electron:pack", "electron:pack:linux") duplicate the
same prep and subproject build chain; extract that common sequence into a single
script (e.g., "electron:prepare" or "electron:build:common") that performs the
mv/backups/trap, NEXT/TSC build steps and the looped npm install/npm run build
for mcp-* packages, and then have the four platform-specific scripts simply set
platform flags/environment (ELECTRON_BUILD or a PLATFORM var) and call the
shared script before invoking electron-builder with the appropriate platform
arguments. Update references to the unique script names ("electron:prepare" and
the original four) and ensure the backup/trap logic and tsc invocation remain in
the shared script so behavior is unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bc56f5e-cd56-4da7-b3a8-df6075e98a01

📥 Commits

Reviewing files that changed from the base of the PR and between 902b4ff and 319cd0a.

📒 Files selected for processing (3)
  • .github/workflows/build-linux.yml
  • electron/main.ts
  • package.json

Comment thread .github/workflows/build-linux.yml
Comment thread .github/workflows/build-linux.yml Outdated
Comment thread package.json
…andbox

- Add explicit `permissions: contents: write` to workflow for hardened orgs
- Use bash array loop for .deb validation (handles multiple artifacts)
- Remove `--no-sandbox` from desktop entry; rely on SUID chrome-sandbox
  set by after-install.sh (chown root + chmod 4755) per Electron guidance
- Make after-install.sh idempotent with chown root:root

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@JiwaniZakir JiwaniZakir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The optional chaining in electron/main.ts (process.stdout?.on?.('error', ...)) is unnecessary — process.stdout and process.stderr are always defined in Node.js, so this reads as defensive code without purpose and may mislead future readers about their nullability.

The 50MB size threshold in the Verify build artifacts step is a magic number that will cause false-positive failures if the app grows naturally or shrinks due to dependency changes. A more robust check would be to verify the .deb contains a non-zero-size binary rather than asserting an absolute byte threshold.

The after-install.sh script sets SUID on chrome-sandbox, but there's no corresponding before-remove.sh to clean up that SUID root binary on uninstall — leaving a world-executable SUID binary behind on uninstall is a minor security hygiene issue worth addressing.

The electron:build and electron:build:linux scripts in package.json are nearly identical 200-character bash one-liners duplicated across both entries, differing only in the final --mac/--linux flag. Extracting the shared MCP build steps into a separate script (e.g., build/scripts/build-mcps.sh) would make both commands dramatically easier to maintain and less error-prone when new MCP modules are added.

…p on uninstall

- Remove unnecessary optional chaining on process.stdout/stderr (always
  defined in Node.js)
- Replace magic 50MB size threshold with non-zero binary size check
  in CI smoke tests
- Add before-remove.sh to strip SUID bit from chrome-sandbox on
  package uninstall (security hygiene)
- Wire before-remove.sh into deb fpm config

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@davebulaval

Copy link
Copy Markdown
Author

@JiwaniZakir Thanks for the thorough review! All points addressed in 81697a9:

1. Optional chaining on process.stdout/process.stderr — Fixed. Removed ?.on?.() since both are always defined in Node.js. Now uses plain .on().

2. Magic 50MB threshold — Fixed. Replaced the hardcoded byte threshold with a check that the dorothy binary inside the .deb exists and has a non-zero size. This won't false-positive as the app grows or shrinks.

3. SUID cleanup on uninstall — Fixed. Added build/linux/before-remove.sh that strips the SUID bit (chmod 0755) from chrome-sandbox before package removal, so no SUID root binary is left behind. Wired into package.json via --before-remove in fpm config.

4. Script duplication (nitpick) — Not addressed in this PR. Extracting a shared electron:prepare script would also require refactoring the existing macOS build scripts, which is out of scope here. Happy to do it as a follow-up PR if desired.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
package.json (1)

19-22: Consider extracting common build steps to reduce duplication.

The electron:build, electron:build:linux, electron:pack, and electron:pack:linux scripts share nearly identical logic for moving API files, building sub-packages, and running traps. This duplication makes maintenance harder—any change to the build process must be applied in multiple places.

♻️ Suggested approach

Extract the common build logic into a shell script:

# scripts/build-subpackages.sh
#!/bin/bash
set -e
for pkg in mcp-orchestrator mcp-telegram mcp-kanban mcp-vault mcp-socialdata mcp-x mcp-world; do
  cd "$pkg" && npm install && npm run build && cd ..
done

Then simplify the npm scripts:

-"electron:build": "bash -c 'set -e; mv src/app/api src/app/_api_backup; mv src/app/icon.tsx src/app/_icon_backup.tsx 2>/dev/null || true; trap \"mv src/app/_api_backup src/app/api 2>/dev/null; mv src/app/_icon_backup.tsx src/app/icon.tsx 2>/dev/null\" EXIT; ELECTRON_BUILD=1 next build; tsc -p electron/tsconfig.json; cd mcp-orchestrator && npm install && npm run build && cd ..; cd mcp-telegram && npm install && npm run build && cd ..; cd mcp-kanban && npm install && npm run build && cd ..; cd mcp-vault && npm install && npm run build && cd ..; cd mcp-socialdata && npm install && npm run build && cd ..; cd mcp-x && npm install && npm run build && cd ..; cd mcp-world && npm install && npm run build && cd ..; electron-builder --mac'",
+"electron:build": "bash scripts/electron-build.sh --mac",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 19 - 22, The package.json scripts
"electron:build", "electron:build:linux", "electron:pack", and
"electron:pack:linux" duplicate the same subpackage build and file-move logic;
extract the repeated steps into a single executable script (e.g.
scripts/build-subpackages.sh) that loops over the packages (mcp-orchestrator,
mcp-telegram, mcp-kanban, mcp-vault, mcp-socialdata, mcp-x, mcp-world) and runs
npm install && npm run build for each, preserve the existing API/icon backup and
trap logic and the ELECTRON_BUILD/next/tsc/electron-builder steps, then update
the four npm script entries to call the new script (and keep platform-specific
electron-builder flags) to remove duplication while keeping behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@package.json`:
- Around line 19-22: The package.json scripts "electron:build",
"electron:build:linux", "electron:pack", and "electron:pack:linux" duplicate the
same subpackage build and file-move logic; extract the repeated steps into a
single executable script (e.g. scripts/build-subpackages.sh) that loops over the
packages (mcp-orchestrator, mcp-telegram, mcp-kanban, mcp-vault, mcp-socialdata,
mcp-x, mcp-world) and runs npm install && npm run build for each, preserve the
existing API/icon backup and trap logic and the
ELECTRON_BUILD/next/tsc/electron-builder steps, then update the four npm script
entries to call the new script (and keep platform-specific electron-builder
flags) to remove duplication while keeping behavior identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1ddfe0d5-f00e-4d7a-8c4e-5238c7ce38e7

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad0824 and 81697a9.

📒 Files selected for processing (4)
  • .github/workflows/build-linux.yml
  • build/linux/before-remove.sh
  • electron/main.ts
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/build-linux.yml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
package.json (1)

20-22: Extract a shared Electron prepare/build command to avoid drift.

These scripts duplicate a long command chain; one future edit can easily diverge behavior between mac/linux build/pack.

Refactor sketch
  "scripts": {
+   "electron:prepare": "tsc -p electron/tsconfig.json && cd mcp-orchestrator && npm install && npm run build && cd .. && cd mcp-telegram && npm install && npm run build && cd .. && cd mcp-kanban && npm install && npm run build && cd .. && cd mcp-vault && npm install && npm run build && cd .. && cd mcp-socialdata && npm install && npm run build && cd .. && cd mcp-x && npm install && npm run build && cd .. && cd mcp-world && npm install && npm run build && cd ..",
-   "electron:pack": "... && electron-builder --dir --mac",
-   "electron:pack:linux": "... && electron-builder --dir --linux"
+   "electron:pack": "npm run electron:prepare && electron-builder --dir --mac",
+   "electron:pack:linux": "npm run electron:prepare && electron-builder --dir --linux"
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 20 - 22, The three npm scripts
electron:build:linux, electron:pack, and electron:pack:linux duplicate a long
prepare/build command; create a new shared script (e.g., "electron:prepare" or
"electron:build:prepare") that contains the common steps (the mv/trap backups,
ELECTRON_BUILD=1 next build, tsc -p electron/tsconfig.json, and the repeated cd
... && npm install && npm run build for each mcp-*), then update
electron:build:linux, electron:pack, and electron:pack:linux to call npm run
electron:prepare (or the chosen name) and append only the platform-specific
final electron-builder invocation (and any platform flags like --linux or
--mac); ensure you preserve the exact backup/trap semantics and order so
behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@package.json`:
- Around line 20-22: The three npm scripts electron:build:linux, electron:pack,
and electron:pack:linux duplicate a long prepare/build command; create a new
shared script (e.g., "electron:prepare" or "electron:build:prepare") that
contains the common steps (the mv/trap backups, ELECTRON_BUILD=1 next build, tsc
-p electron/tsconfig.json, and the repeated cd ... && npm install && npm run
build for each mcp-*), then update electron:build:linux, electron:pack, and
electron:pack:linux to call npm run electron:prepare (or the chosen name) and
append only the platform-specific final electron-builder invocation (and any
platform flags like --linux or --mac); ensure you preserve the exact backup/trap
semantics and order so behavior remains identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: feabeb8e-bcb6-4bdf-a9c1-83016d7bf406

📥 Commits

Reviewing files that changed from the base of the PR and between 81697a9 and 3976303.

📒 Files selected for processing (2)
  • electron/main.ts
  • package.json
✅ Files skipped from review due to trivial changes (1)
  • electron/main.ts

@JiwaniZakir

Copy link
Copy Markdown

The SUID setup in after-install.sh is the right call, but worth noting that if the package is installed on a system where /opt is mounted nosuid, the chmod 4755 on chrome-sandbox will silently have no effect — the sandbox will fail at runtime with a cryptic permission error. Consider adding a post-install check that warns the user if the SUID bit didn't actually stick. Also, the build/linux/after-install.sh being in .gitignore is a problem that should be addressed at the repo level rather than relying on reviewers to know it was committed in a prior commit — either remove it from .gitignore or add a !build/linux/ exception so the file is visible in future diffs.

…repare.sh

The 4 electron build/pack scripts duplicated ~200 chars of identical
logic (API backup, next build, tsc, MCP sub-package builds). Extract
into a shared script and simplify each npm script to a one-liner.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
scripts/electron-prepare.sh (1)

12-12: Ensure next command is available in PATH.

The script invokes next build directly. If run outside of an npm script context (e.g., via bash scripts/electron-prepare.sh directly), next may not be in PATH. Consider using npx next build or ./node_modules/.bin/next build for robustness.

♻️ Suggested change
 # Build Next.js static export
-ELECTRON_BUILD=1 next build
+ELECTRON_BUILD=1 npx next build
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/electron-prepare.sh` at line 12, The script currently runs the
Next.js build with the bare command "next build" (line containing
ELECTRON_BUILD=1 next build), which can fail if "next" isn't in PATH; update
that invocation to use a project-local resolver such as "npx next build" or
"./node_modules/.bin/next build" so the script works when run directly (e.g.,
bash scripts/electron-prepare.sh) and preserves the ELECTRON_BUILD=1 environment
variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@package.json`:
- Around line 138-148: The deb packaging depends array in package.json currently
lists libgtk-3-0 and libatspi2.0-0 which will fail on Ubuntu 24.04; update the
"depends" array to replace "libgtk-3-0" with "libgtk-3-0t64" and "libatspi2.0-0"
with "libatspi2.0-0t64" (or alternatively implement separate build targets that
produce distinct .deb artifacts for Ubuntu 22.04 and 24.04 with the appropriate
dependency sets) so that the depends list in package.json matches the t64 time_t
transition requirements.

In `@scripts/electron-prepare.sh`:
- Around line 7-9: The trap uses relative paths so if the script changes
directories (e.g., after cd "$pkg" in the MCP build loop) the cleanup mv
commands can run from the wrong cwd and silently fail; modify the script to
compute and use absolute paths (or capture and use pushd/popd) for the backup
files referenced in the trap and in the mv calls — specifically update the mv
commands that move src/app/api and src/app/icon.tsx and the trap restore
commands so they reference a project-root variable (e.g., ROOT) or use
pushd/popd around cd "$pkg" to guarantee the trap always moves
ROOT/src/app/_api_backup and ROOT/src/app/_icon_backup.tsx back to
ROOT/src/app/api and ROOT/src/app/icon.tsx even if the script exits from a
different working directory.

---

Nitpick comments:
In `@scripts/electron-prepare.sh`:
- Line 12: The script currently runs the Next.js build with the bare command
"next build" (line containing ELECTRON_BUILD=1 next build), which can fail if
"next" isn't in PATH; update that invocation to use a project-local resolver
such as "npx next build" or "./node_modules/.bin/next build" so the script works
when run directly (e.g., bash scripts/electron-prepare.sh) and preserves the
ELECTRON_BUILD=1 environment variable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0cf6e4a5-9f71-45c4-afe9-919e16eb44ae

📥 Commits

Reviewing files that changed from the base of the PR and between 3976303 and c257ae2.

📒 Files selected for processing (2)
  • package.json
  • scripts/electron-prepare.sh

Comment thread package.json
Comment thread scripts/electron-prepare.sh Outdated
@davebulaval

Copy link
Copy Markdown
Author

Addressed the repeated nitpick about script duplication in c257ae2:

  • Created scripts/electron-prepare.sh that contains all shared build steps (API backup/restore with trap, next build, tsc, MCP sub-package loop)
  • Simplified all 4 electron scripts to one-liners:
    "electron:prepare": "bash scripts/electron-prepare.sh",
    "electron:build": "npm run electron:prepare && electron-builder --mac",
    "electron:build:linux": "npm run electron:prepare && electron-builder --linux",
    "electron:pack": "npm run electron:prepare && electron-builder --dir --mac",
    "electron:pack:linux": "npm run electron:prepare && electron-builder --dir --linux"
    
  • Verified the Linux build still produces a valid .deb with the refactored scripts.

…tignore

- Add post-install check: warn if SUID bit didn't stick (nosuid mount)
- Change .gitignore from `/build` to `/build/*` + `!/build/linux/` so
  Linux packaging scripts are tracked without git add -f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@davebulaval

Copy link
Copy Markdown
Author

@JiwaniZakir Good catches, both addressed in 06031f3:

1. nosuid mount warningafter-install.sh now checks if the SUID bit actually stuck after chmod 4755 using [ ! -u ... ]. If the filesystem is mounted nosuid, the user gets a clear warning with the --no-sandbox workaround:

WARNING: Failed to set SUID bit on /opt/Dorothy/chrome-sandbox.
If /opt is mounted with 'nosuid', the Electron sandbox will not work.
You may need to run Dorothy with --no-sandbox as a workaround.

2. build/ in .gitignore — Changed /build to /build/* with !/build/linux/ exception. The Linux packaging scripts (after-install.sh, before-remove.sh) are now properly tracked without needing git add -f. The rest of /build (Next.js output) remains ignored.

@JiwaniZakir

Copy link
Copy Markdown

The after-install.sh setting the SUID bit on chrome-sandbox and simultaneously using --no-sandbox in the desktop entry is contradictory — pick one approach. If the SUID sandbox is properly configured, the --no-sandbox flag shouldn't be needed and is better avoided since it reduces process isolation. Also worth noting: the EPIPE handler in electron/main.ts should guard against swallowing other process.stdout/process.stderr errors unintentionally — checking err.code === 'EPIPE' before ignoring would be safer.

@davebulaval

Copy link
Copy Markdown
Author

@JiwaniZakir Both points are already addressed in earlier commits:

  1. --no-sandbox was removed in 0ad0824 — it's no longer in the desktop entry or anywhere in package.json. We rely solely on the SUID sandbox via after-install.sh. The --no-sandbox workaround is only mentioned in the warning message if the SUID bit fails to stick (nosuid mount).

  2. The EPIPE handler already guards against swallowing other errors — the code checks err.code === 'EPIPE' before returning, and re-throws everything else:

    process.stdout.on('error', (err: NodeJS.ErrnoException) => {
      if (err.code === 'EPIPE') return;
      throw err;
    });

    This was the pattern from the start, then refined in 81697a9 to drop unnecessary optional chaining.

…4.04

- Use REPO_ROOT absolute paths in trap/cleanup so API route restoration
  works even if script exits from a subdirectory (cd into mcp-* packages)
- Use pushd/popd instead of cd for MCP sub-package builds
- Use npx for next build to ensure it's found outside npm script context
- Add t64 alternatives for libgtk-3-0 and libatspi2.0-0 in deb depends
  (Ubuntu 24.04 Y2038 transition, electron-builder#9539)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@davebulaval

Copy link
Copy Markdown
Author

Replying to the two remaining CodeRabbit comments from review 4040787862 (both fixed in 34ebb1b):


Re: Trap uses relative paths (Critical — scripts/electron-prepare.sh)

Fixed:

  • Compute REPO_ROOT via dirname "$0"/.. and use absolute paths in all mv commands and the cleanup trap, so restoration works regardless of cwd.
  • Replaced cd "$pkg" && ... && cd .. with pushd/popd to avoid leaving the working directory on failure.
  • Switched next build to npx next build per the nitpick about PATH availability.

Re: Update deb dependencies for Ubuntu 24.04 t64 transition (Major — package.json)

Fixed:

  • Added t64 alternatives for the two affected packages: libgtk-3-0 | libgtk-3-0t64 and libatspi2.0-0 | libatspi2.0-0t64.
  • The other packages (libnotify4, libnss3, libxss1, libxtst6, libuuid1, libsecret-1-0) were not affected by the t64 transition and keep their original names.
  • Uses dpkg alternatives syntax so the .deb installs on both Ubuntu 22.04 and 24.04. Workaround for electron-builder#9539.

@JiwaniZakir

Copy link
Copy Markdown

The fixes look good — removing the optional chaining on process.stdout/process.stderr is the right call since those streams are guaranteed to exist in Node.js. The named constant for the size threshold also makes the smoke test much more maintainable. Marking my review comments as resolved.

@davebulaval

Copy link
Copy Markdown
Author

Any update on the merge timeframe?

@Charlie85270

Copy link
Copy Markdown
Owner

Hey @davebulaval, thanks a lot for your contribution!
I'm currently OOO but I'll have a look to it next week.

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.

3 participants