mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-14 13:22:37 +00:00
fix(desktop): shut down cleanly with workspace terminals (#3268)
Workspace terminal shutdown only signaled an npm wrapper, leaving Electron alive with broken output pipes. Make the signal-handling runner the terminal-owned process and route external Electron termination through the existing quit lifecycle.
This commit is contained in:
+5
-3
@@ -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:
|
||||
|
||||
@@ -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"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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<void> {
|
||||
}
|
||||
|
||||
describe("quit-lifecycle", () => {
|
||||
it("turns external termination signals into one Electron quit", () => {
|
||||
const listeners = new Map<NodeJS.Signals, () => 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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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") {
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user