diff --git a/docs/development.md b/docs/development.md index bd573b5b96..244db268ee 100644 --- a/docs/development.md +++ b/docs/development.md @@ -131,9 +131,11 @@ desktop-only environment inherited by terminals opened inside Paseo from couplin a new worktree instance to the parent desktop instance's profile or single-instance lock. -Electron remains in the desktop dev runner's process group. Closing or stopping the -workspace-script terminal must terminate Electron with Metro; detaching Electron -leaves an orphan holding the worktree's single-instance lock and broken output pipes. +The desktop workspace script `exec`s the dev runner so the terminal owns the runner +PID. Terminal shutdown reaches the runner as `SIGHUP`; the runner stops Metro and +asks Electron to quit through its normal app lifecycle. Do not add an npm wrapper or +detach Electron: either change leaves an orphan holding the worktree's single-instance +lock and broken output pipes. With desktop dev running, verify the real BrowserWindow, titlebar clearance, fullscreen transition, and 751-pixel settings split with: diff --git a/packages/desktop/scripts/dev-runner-config.mjs b/packages/desktop/scripts/dev-runner-config.mjs index 8efbd3a033..c855b3e5d1 100644 --- a/packages/desktop/scripts/dev-runner-config.mjs +++ b/packages/desktop/scripts/dev-runner-config.mjs @@ -15,3 +15,9 @@ export function createElectronSpawnOptions({ env, colorEnv, expoDevUrl }) { export function resolveChildKillTarget(pid, detached) { return detached ? -pid : pid; } + +export function registerDevRunnerShutdownSignals({ signalSource, stop }) { + for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"]) { + signalSource.on(signal, () => stop("SIGTERM")); + } +} diff --git a/packages/desktop/scripts/dev-runner-config.test.mjs b/packages/desktop/scripts/dev-runner-config.test.mjs index 813bc6760c..ca34fe8c16 100644 --- a/packages/desktop/scripts/dev-runner-config.test.mjs +++ b/packages/desktop/scripts/dev-runner-config.test.mjs @@ -1,7 +1,23 @@ import { describe, expect, test } from "vitest"; -import { createElectronSpawnOptions, resolveChildKillTarget } from "./dev-runner-config.mjs"; +import { readFileSync } from "node:fs"; +import { + createElectronSpawnOptions, + registerDevRunnerShutdownSignals, + resolveChildKillTarget, +} from "./dev-runner-config.mjs"; describe("desktop dev process ownership", () => { + test("makes the signal-handling runner the workspace terminal process", () => { + const paseoConfig = JSON.parse( + readFileSync(new URL("../../../paseo.json", import.meta.url), "utf8"), + ); + const devScript = readFileSync(new URL("./dev.sh", import.meta.url), "utf8"); + + expect(paseoConfig.scripts.desktop.command).toContain("exec ./packages/desktop/scripts/dev.sh"); + expect(devScript).toContain('npm --prefix "$DESKTOP_DIR" run build:main'); + expect(devScript).toContain('exec node "$SCRIPT_DIR/dev-runner.mjs"'); + }); + test("keeps Electron in the runner process group", () => { const options = createElectronSpawnOptions({ env: { PATH: "/usr/bin" }, @@ -23,4 +39,24 @@ describe("desktop dev process ownership", () => { expect(resolveChildKillTarget(42, true)).toBe(-42); expect(resolveChildKillTarget(42, false)).toBe(42); }); + + test("stops children when the owning terminal hangs up", () => { + const listeners = new Map(); + const receivedSignals = []; + + registerDevRunnerShutdownSignals({ + signalSource: { + on(signal, listener) { + listeners.set(signal, listener); + }, + }, + stop(signal) { + receivedSignals.push(signal); + }, + }); + + expect(Array.from(listeners.keys())).toEqual(["SIGHUP", "SIGINT", "SIGTERM"]); + listeners.get("SIGHUP")(); + expect(receivedSignals).toEqual(["SIGTERM"]); + }); }); diff --git a/packages/desktop/scripts/dev-runner.mjs b/packages/desktop/scripts/dev-runner.mjs index c653f31b39..5e0344c0d1 100644 --- a/packages/desktop/scripts/dev-runner.mjs +++ b/packages/desktop/scripts/dev-runner.mjs @@ -4,7 +4,11 @@ import path from "node:path"; import { execFileSync, spawn } from "node:child_process"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; -import { createElectronSpawnOptions, resolveChildKillTarget } from "./dev-runner-config.mjs"; +import { + createElectronSpawnOptions, + registerDevRunnerShutdownSignals, + resolveChildKillTarget, +} from "./dev-runner-config.mjs"; import { resolveDevElectronArgs } from "./dev-runner-args.mjs"; @@ -159,8 +163,7 @@ function canConnect(port, host) { }); } -process.on("SIGINT", () => stopAll("SIGTERM")); -process.on("SIGTERM", () => stopAll("SIGTERM")); +registerDevRunnerShutdownSignals({ signalSource: process, stop: stopAll }); spawnChild("metro", "npx", ["expo", "start", "--port", String(expoPort)], { cwd: appDir, diff --git a/packages/desktop/scripts/dev.sh b/packages/desktop/scripts/dev.sh index f00625d873..90d5ffa095 100755 --- a/packages/desktop/scripts/dev.sh +++ b/packages/desktop/scripts/dev.sh @@ -31,7 +31,7 @@ export PASEO_DAEMON_ENDPOINT="$DAEMON_ENDPOINT" export PASEO_CORS_ORIGINS="${PASEO_CORS_ORIGINS:-*}" -npm run build:main +npm --prefix "$DESKTOP_DIR" run build:main echo "══════════════════════════════════════════════════════" echo " Paseo Desktop Dev" diff --git a/packages/desktop/src/daemon/quit-lifecycle.test.ts b/packages/desktop/src/daemon/quit-lifecycle.test.ts index 5dd1e66806..9985817d23 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.test.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_DESKTOP_SETTINGS } from "../settings/desktop-settings"; import { createQuitLifecycle, + registerExternalQuitSignals, shouldStopDesktopManagedDaemonOnQuit, stopDesktopManagedDaemonOnQuitIfNeeded, } from "./quit-lifecycle"; @@ -29,6 +30,25 @@ function waitForQuitLifecycle(): Promise { } describe("quit-lifecycle", () => { + it("turns external termination signals into one Electron quit", () => { + const listeners = new Map void>(); + const quits: string[] = []; + + registerExternalQuitSignals({ + signals: { + on: (signal, listener) => { + listeners.set(signal, listener); + }, + }, + quit: () => quits.push("quit"), + }); + + expect(Array.from(listeners.keys())).toEqual(["SIGHUP", "SIGINT", "SIGTERM"]); + listeners.get("SIGTERM")?.(); + listeners.get("SIGHUP")?.(); + expect(quits).toEqual(["quit"]); + }); + it("stops by default and only keeps running when keepRunningAfterQuit is enabled", () => { expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_STOP_ON_QUIT)).toBe(true); expect(shouldStopDesktopManagedDaemonOnQuit(SETTINGS_KEEP_RUNNING)).toBe(false); diff --git a/packages/desktop/src/daemon/quit-lifecycle.ts b/packages/desktop/src/daemon/quit-lifecycle.ts index 1998985f56..bd1847ce6b 100644 --- a/packages/desktop/src/daemon/quit-lifecycle.ts +++ b/packages/desktop/src/daemon/quit-lifecycle.ts @@ -14,6 +14,10 @@ interface BeforeQuitApp { exit(code: number): void; } +interface ExternalQuitSignalSource { + on(signal: NodeJS.Signals, listener: () => void): unknown; +} + interface QuitLifecycle { handleBeforeQuit(event: BeforeQuitEvent): void; handleBeforeQuitForUpdate(): void; @@ -31,6 +35,23 @@ export interface StopOnQuitDeps { showShutdownFeedback: () => void; } +export function registerExternalQuitSignals({ + signals, + quit, +}: { + signals: ExternalQuitSignalSource; + quit: () => void; +}): void { + let quitRequested = false; + for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"] satisfies NodeJS.Signals[]) { + signals.on(signal, () => { + if (quitRequested) return; + quitRequested = true; + quit(); + }); + } +} + export function shouldStopDesktopManagedDaemonOnQuit(settings: QuitLifecycleSettings): boolean { return !settings.daemon.keepRunningAfterQuit; } diff --git a/packages/desktop/src/main.ts b/packages/desktop/src/main.ts index 36b66783ad..cb0ae0aa5a 100644 --- a/packages/desktop/src/main.ts +++ b/packages/desktop/src/main.ts @@ -83,6 +83,7 @@ import { } from "./daemon/daemon-manager.js"; import { createQuitLifecycle, + registerExternalQuitSignals, stopDesktopManagedDaemonOnQuitIfNeeded, } from "./daemon/quit-lifecycle.js"; import { runDesktopStartup } from "./desktop-startup.js"; @@ -1081,6 +1082,7 @@ const quitLifecycle = createQuitLifecycle({ // electron-updater forwards this event through Electron's built-in autoUpdater. electronAutoUpdater.on("before-quit-for-update", quitLifecycle.handleBeforeQuitForUpdate); app.on("before-quit", quitLifecycle.handleBeforeQuit); +registerExternalQuitSignals({ signals: process, quit: () => app.quit() }); app.on("window-all-closed", () => { if (process.platform !== "darwin") { diff --git a/paseo.json b/paseo.json index 076d1f2c58..4e7b0e8116 100644 --- a/paseo.json +++ b/paseo.json @@ -19,7 +19,7 @@ }, "desktop": { "type": "service", - "command": "PASEO_DEV_MANAGED_HOME=1 PASEO_DEV_ROOT=\"${PASEO_WORKTREE_PATH:-$PWD}\" PASEO_HOME=\"${PASEO_WORKTREE_PATH:-$PWD}/.dev/paseo-home\" PASEO_LISTEN=0.0.0.0:${PASEO_SERVICE_DAEMON_PORT:-6768} PASEO_DEV_DAEMON_ENDPOINT=localhost:${PASEO_SERVICE_DAEMON_PORT:-6768} EXPO_PORT=${PASEO_PORT:-} npm run dev --workspace=@getpaseo/desktop" + "command": "PASEO_DEV_MANAGED_HOME=1 PASEO_DEV_ROOT=\"${PASEO_WORKTREE_PATH:-$PWD}\" PASEO_HOME=\"${PASEO_WORKTREE_PATH:-$PWD}/.dev/paseo-home\" PASEO_LISTEN=0.0.0.0:${PASEO_SERVICE_DAEMON_PORT:-6768} PASEO_DEV_DAEMON_ENDPOINT=localhost:${PASEO_SERVICE_DAEMON_PORT:-6768} EXPO_PORT=${PASEO_PORT:-} exec ./packages/desktop/scripts/dev.sh" }, "ios-simulator": { "type": "service",