From d17194c6111e592e94c13341f2efc25e358aa325 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 31 Jul 2026 17:17:54 +0200 Subject: [PATCH] fix(desktop): stop dev app surviving its terminal Electron was detached from the desktop runner, allowing it to outlive a stopped workspace terminal with broken output pipes and retain the single-instance lock. Keep Electron owned by the runner and preserve explicit process-group cleanup for detached child trees. --- docs/development.md | 4 +++ .../desktop/scripts/dev-runner-config.mjs | 17 ++++++++++ .../scripts/dev-runner-config.test.mjs | 26 +++++++++++++++ packages/desktop/scripts/dev-runner.mjs | 33 ++++++++++--------- 4 files changed, 65 insertions(+), 15 deletions(-) create mode 100644 packages/desktop/scripts/dev-runner-config.mjs create mode 100644 packages/desktop/scripts/dev-runner-config.test.mjs diff --git a/docs/development.md b/docs/development.md index 2f3ad57f95..e9a894c056 100644 --- a/docs/development.md +++ b/docs/development.md @@ -115,6 +115,10 @@ 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. + 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 new file mode 100644 index 0000000000..8efbd3a033 --- /dev/null +++ b/packages/desktop/scripts/dev-runner-config.mjs @@ -0,0 +1,17 @@ +export function createElectronSpawnOptions({ env, colorEnv, expoDevUrl }) { + return { + // Electron must stay in the runner's process group. Paseo workspace scripts + // own the terminal process group, so detaching Electron lets it survive a + // service stop with broken stdout/stderr pipes and block the next launch. + detached: false, + env: { + ...env, + ...colorEnv, + EXPO_DEV_URL: expoDevUrl, + }, + }; +} + +export function resolveChildKillTarget(pid, detached) { + return detached ? -pid : pid; +} diff --git a/packages/desktop/scripts/dev-runner-config.test.mjs b/packages/desktop/scripts/dev-runner-config.test.mjs new file mode 100644 index 0000000000..813bc6760c --- /dev/null +++ b/packages/desktop/scripts/dev-runner-config.test.mjs @@ -0,0 +1,26 @@ +import { describe, expect, test } from "vitest"; +import { createElectronSpawnOptions, resolveChildKillTarget } from "./dev-runner-config.mjs"; + +describe("desktop dev process ownership", () => { + test("keeps Electron in the runner process group", () => { + const options = createElectronSpawnOptions({ + env: { PATH: "/usr/bin" }, + colorEnv: { FORCE_COLOR: "1" }, + expoDevUrl: "http://localhost:8082", + }); + + expect(options).toMatchObject({ + detached: false, + env: { + PATH: "/usr/bin", + FORCE_COLOR: "1", + EXPO_DEV_URL: "http://localhost:8082", + }, + }); + }); + + test("targets the whole process group for detached child trees", () => { + expect(resolveChildKillTarget(42, true)).toBe(-42); + expect(resolveChildKillTarget(42, false)).toBe(42); + }); +}); diff --git a/packages/desktop/scripts/dev-runner.mjs b/packages/desktop/scripts/dev-runner.mjs index f2b16f8221..0456189643 100644 --- a/packages/desktop/scripts/dev-runner.mjs +++ b/packages/desktop/scripts/dev-runner.mjs @@ -4,6 +4,7 @@ import path from "node:path"; import { spawn } from "node:child_process"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; +import { createElectronSpawnOptions, resolveChildKillTarget } from "./dev-runner-config.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); const desktopDir = path.resolve(scriptDir, ".."); @@ -58,7 +59,11 @@ function spawnChild(name, command, args, options = {}) { ...options, }); - children.set(name, child); + const managedChild = { + process: child, + detached: options.detached === true, + }; + children.set(name, managedChild); prefixStream(name, child.stdout, process.stdout); prefixStream(name, child.stderr, process.stderr); @@ -82,17 +87,13 @@ function spawnChild(name, command, args, options = {}) { return child; } -function killChild(child, signal) { +function killChild({ process: child, detached }, signal) { if (!child.pid || child.killed) { return; } try { - if (child.detached) { - process.kill(-child.pid, signal); - } else { - child.kill(signal); - } + process.kill(resolveChildKillTarget(child.pid, detached), signal); } catch { // The child may have exited between the liveness check and the signal. } @@ -176,12 +177,14 @@ try { } if (!stopping) { - spawnChild("electron", electron, [...electronArgs, desktopDir], { - detached: true, - env: { - ...process.env, - ...colorEnv, - EXPO_DEV_URL: expoDevUrl, - }, - }); + spawnChild( + "electron", + electron, + [...electronArgs, desktopDir], + createElectronSpawnOptions({ + env: process.env, + colorEnv, + expoDevUrl, + }), + ); }