feat: Electron desktop client for macOS#431
feat: Electron desktop client for macOS#431jorgegonzalez wants to merge 20 commits intoPizzaface:mainfrom
Conversation
Pizzaface
left a comment
Pizzaface
left a comment
There was a problem hiding this comment.
Code Review: feat: Electron desktop client for macOS
👋 Hey @jorgegonzalez — really excited to see this! Ran a local review pass. There are two blocking issues that would prevent packaged builds from working, plus a few important things to clean up before merge.
🚨 P0 — Packaged paths won't resolve with asar enabled
File: packages/desktop/src/main/config.ts:23,31,39
Production paths are currently set to process.resourcesPath/app/... for ui-dist, server-dist, and cli-dist. With electron-builder's default asar packaging, bundled files live under app.asar — not app/. These paths won't resolve in a packaged build, so the desktop app will fail to load the UI, server, and runner on any clean install.
You'll want to use app.getAppPath() or the __dirname-relative approach that accounts for asar, e.g.:
import { app } from 'electron'
const appRoot = app.isPackaged
? path.join(process.resourcesPath, 'app.asar')
: path.join(__dirname, '..', '..')Or alternatively, configure electron-builder to unpack specific dirs with asarUnpack.
🚨 P0 — Cannot assume Bun is installed on end-user machines
Files: packages/desktop/src/main/server-manager.ts:50, packages/desktop/src/main/runner-manager.ts:33
Both managers start child processes via spawn("bun", ...). Packaged Electron apps run on arbitrary macOS installs where Bun will not be present. Server and runner startup will hard-fail on any machine that doesn't already have Bun in /Users/jordan/.pizzapi/bin:/Users/jordan/.nvm/versions/node/v24.13.1/bin:/Users/jordan/.bun/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin.
Options:
- Bundle a Bun binary inside the app and reference it by absolute path
- Ship pre-compiled JS entry points and use
nodeinstead - Check for Bun at startup and surface a helpful error/install prompt if missing
P1 — uncaughtException handler swallows serious errors
File: packages/desktop/src/main/index.ts:21-26
The handler logs non-EPIPE exceptions but doesn't rethrow or call process.exit(). This suppresses Node/Electron's default crash behavior — serious exceptions will be silently swallowed and the app can continue running in a corrupted state. At minimum, non-EPIPE errors should cause a controlled exit.
P1 — Auto-restart race with startup rejection
Files: packages/desktop/src/main/server-manager.ts:71,75, packages/desktop/src/main/index.ts:142-149
When a child exits early, ServerManager auto-restarts internally — but start() still rejects, causing the caller to abort startup and show error UI. This leaves inconsistent state: the error UI is shown while a background restart may still be in flight. Consider either disabling auto-restart during initial startup, or having the auto-restart resolve the promise rather than rejecting.
P1 — Tests use a fixed port and real socket checks (CI-flaky)
File: packages/desktop/tests/server-manager.test.ts:63,81
Tests hardcode port 3001 and rely on real socket availability via isPortFree(). This will be flaky on CI or any dev machine where port 3001 is in use. Use a randomised ephemeral port (e.g. 0) or mock the port check.
P1 — packages/desktop not included in root CI pipelines
Files: package.json:37-38, tsconfig.json:3-10
The root test and typecheck scripts and TS project references don't include packages/desktop. New desktop code won't be caught by the standard quality gates. Worth adding it so CI covers this package going forward.
P2 — any at IPC boundary
File: packages/desktop/src/preload/index.ts:8,18
onServiceStatus callback and event payload are typed as any. Tightening this to a concrete interface would catch mismatches between main/renderer at compile time.
P2 — Runner marked "running" before readiness signal
File: packages/desktop/src/main/index.ts:172-173
The tray/UI is set to healthy immediately after runnerManager.start() resolves, with no actual health check. If the runner exits right after start, the UI shows healthy status briefly before catching up. Consider waiting for a readiness ping.
Note on typecheck/test failures: Both
bun run typecheckandbun testhit pre-existing environment errors (Cannot find package 'handlebars'/Cannot find package 'redis') unrelated to your changes. The P1 about desktop not being in the CI pipelines is a separate issue worth fixing regardless.
jorgegonzalez
force-pushed
the
feat/electron-desktop-client
branch
from
5eb7e9c to
df54537
Compare
…add .js extension to electron-log import
… server fallback, and early-exit detection in health check
jorgegonzalez
force-pushed
the
feat/electron-desktop-client
branch
from
df54537 to
a978156
Compare
P0: Fix asar paths — use app.getAppPath() instead of process.resourcesPath/app/ which doesn't resolve correctly inside app.asar packaging P0: Handle missing Bun — getBunPath() checks well-known install locations (~/.bun/bin, /usr/local/bin, /opt/homebrew/bin), falls back to `which bun`, and throws a descriptive error with install instructions if not found P1: uncaughtException handler now calls process.exit(1) for non-EPIPE errors instead of silently swallowing them P1: Fix auto-restart race during initial startup — disable auto-restart while start() is running so the promise rejects cleanly without a background restart competing with error handling P1: Tests use randomized ephemeral ports instead of hardcoded 3001 P1: Add packages/desktop to root tsconfig references and test script P2: Type IPC boundary — replace `any` with ServiceStatus interface shared between main/preload/renderer via shared/types.ts P2: Runner marked running only after readiness check — start() now polls /api/runners until the daemon registers, with timeout fallback
The desktop package requires electron types which aren't available in the standard CI environment. Revert the root tsconfig/test additions and add test/typecheck scripts to the desktop package.json instead so they can be run independently or in a future desktop-specific CI job.
Adding the desktop workspace caused bun to resolve a separate @types/node@25.3.0 for bun-types, which broke ChildProcess type compatibility in the CLI package. Pin @types/node@22.19.15 via overrides to keep all packages on the same version.
The desktop package's electron dependency tree caused bun to resolve a separate @types/node@25.3.0 for bun-types, breaking ChildProcess types in the CLI package. Desktop is a standalone Electron app — it doesn't need to be in the monorepo workspace. It manages its own deps via a separate bun install. Also removed the unused @pizzapi/protocol dependency and tsconfig reference (desktop doesn't import from protocol).
2 participants
Summary
Adds a native macOS Electron desktop app (
packages/desktop) that wraps PizzaPi into a self-contained experience — relay server, runner daemon, and web UI all launch automatically when you open the app. No terminal required.What's included
Core
packages/desktop— new workspace with Electron 35+, electron-builder, TypeScriptNative OS Features
NotificationAPI)app.setLoginItemSettings()hiddenInsettitle bar style with CSS-webkit-app-region: dragArchitecture
@pizzapi/uiin a BrowserWindowlocalhost:5173), falls back to server-hosted UIdesktopAPIviacontextBridge(version, platform, auto-launch, service status)Dev Workflow
Tests
Design Docs
docs/specs/2026-04-01-electron-desktop-client-design.mddocs/specs/2026-04-01-electron-desktop-client-plan.mdFuture (not in this PR)