fix(desktop): reap CLI process trees on quit (#1166)

app/electron/cli.test.ts

@@ -447,10 +447,11 @@ describe('graceful kill (SIGTERM, then SIGKILL after the grace)', () => {
       `const fs = require('node:fs');
        fs.writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));
        process.on('SIGTERM', () => fs.appendFileSync(${JSON.stringify(signalFile)}, 'TERM'));
+       process.stdout.write('ready');
        setInterval(() => {}, 1000);`,
     )
 
-    await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' })
+    await expect(spawnCli(['status'], { timeoutMs: 1_500 })).rejects.toMatchObject({ kind: 'timeout' })
     // SIGTERM arrives with the rejection; the child survives it and is SIGKILLed
     // only after KILL_GRACE_MS (5s).
     await waitFor(() => readMaybe(signalFile) === 'TERM')
@@ -496,10 +497,11 @@ describe('graceful kill (SIGTERM, then SIGKILL after the grace)', () => {
       'handles-sigterm.js',
       `const fs = require('node:fs');
        process.on('SIGTERM', () => { fs.writeFileSync(${JSON.stringify(cleanupFile)}, 'released'); process.exit(0); });
+       process.stdout.write('ready');
        setInterval(() => {}, 1000);`,
     )
     const began = Date.now()
-    await expect(spawnCli(['status'], { timeoutMs: 300 })).rejects.toMatchObject({ kind: 'timeout' })
+    await expect(spawnCli(['status'], { timeoutMs: 1_500 })).rejects.toMatchObject({ kind: 'timeout' })
     await waitFor(() => readMaybe(cleanupFile) === 'released')
     expect(Date.now() - began).toBeLessThan(5_000) // never waited out the grace
   })
@@ -1131,6 +1133,56 @@ describe('resident serve single-flight', () => {
     expect(readMaybe(oneShotsFile)).toBe('')
   })
 
+  it('gracefully releases the resident lock, then force-reaps its process tree', async () => {
+    if (process.platform === 'win32') return
+    const pidFile = join(dir, 'resident-shutdown-pid')
+    const grandchildPidFile = join(dir, 'provider-child-pid')
+    const termFile = join(dir, 'resident-shutdown-term')
+    const grandchildTermFile = join(dir, 'provider-child-term')
+    const hydrationLock = join(dir, 'hydrating.lock')
+    const grandchildBody = `
+      const fs = require('node:fs');
+      fs.writeFileSync(${JSON.stringify(grandchildPidFile)}, String(process.pid));
+      process.on('SIGTERM', () => fs.appendFileSync(${JSON.stringify(grandchildTermFile)}, 'TERM'));
+      setInterval(() => {}, 1000);
+    `
+    fakeBin(
+      'stubborn-resident-shutdown.js',
+      `const fs = require('node:fs'); const readline = require('node:readline'); const { spawn } = require('node:child_process');
+       if (process.argv[2] === 'serve') {
+         fs.writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));
+         fs.writeFileSync(${JSON.stringify(hydrationLock)}, 'owned');
+         spawn(process.execPath, ['-e', ${JSON.stringify(grandchildBody)}], { stdio: 'ignore' });
+         process.on('SIGTERM', () => {
+           fs.writeFileSync(${JSON.stringify(termFile)}, 'TERM');
+           try { fs.unlinkSync(${JSON.stringify(hydrationLock)}); } catch {}
+           setTimeout(() => process.exit(0), 50);
+         });
+         readline.createInterface({ input: process.stdin });
+         setInterval(() => {}, 1000);
+       } else { process.stdout.write('{}'); }`,
+    )
+    startServe()
+    await waitFor(() => readMaybe(pidFile).length > 0 && readMaybe(grandchildPidFile).length > 0)
+    const pids = [Number(readMaybe(pidFile)), Number(readMaybe(grandchildPidFile))]
+
+    try {
+      const startedAt = performance.now()
+      await shutdownAll()
+      const elapsedMs = performance.now() - startedAt
+
+      expect(elapsedMs).toBeLessThan(1_500)
+      expect(readMaybe(termFile)).toBe('TERM')
+      expect(readMaybe(grandchildTermFile)).toBe('TERM')
+      expect(readMaybe(hydrationLock)).toBe('')
+      for (const pid of pids) expect(() => process.kill(pid, 0)).toThrow()
+    } finally {
+      for (const pid of pids) {
+        try { process.kill(pid, 'SIGKILL') } catch { /* already gone */ }
+      }
+    }
+  })
+
   it('keeps the warm resident child after a successful export', async () => {
     const files = fakeResidentBin()
     startServe()

app/electron/cli.ts

@@ -72,6 +72,11 @@ const MAX_RUNTIME_MS = 15 * 60_000
 // dead pid — self-healing, but via the stale-takeover path rather than a clean
 // release. Neither signal publishes a partial parse; nothing does.
 const KILL_GRACE_MS = 5_000
+// Quit has a stricter user-visible budget than ordinary request timeouts. Give
+// the CLI enough time to catch SIGTERM and release its cache locks, then reap
+// every remaining process-group member before Electron exits.
+export const SHUTDOWN_TERM_GRACE_MS = 750
+const SHUTDOWN_FORCE_WAIT_MS = 250
 /** Wire marker for CLI scan-progress lines (src/parser.ts: PROGRESS_LINE_PREFIX). */
 export const PROGRESS_LINE_PREFIX = 'CODEBURN_PROGRESS '
 // A runaway CLI (or a compromised binary) must not exhaust main-process memory.
@@ -97,6 +102,36 @@ let running = 0
 const interactiveQueue: SlotWaiter[] = []
 const backgroundQueue: SlotWaiter[] = []
 let shuttingDown = false
+let shutdownPromise: Promise<void> | null = null
+
+function ownsProcessGroup(): boolean {
+  return platform() !== 'win32'
+}
+
+/** Signal the CLI and every subprocess it created. POSIX children are spawned
+ * as process-group leaders; Windows keeps the direct-child behavior here and
+ * relies on its existing orphan-reap path after a crash. */
+function signalOwnedTree(child: ChildProcess, signal: NodeJS.Signals): boolean {
+  if (ownsProcessGroup() && child.pid) {
+    try { process.kill(-child.pid, signal); return true } catch { /* already gone or no group */ }
+  }
+  try { return child.kill(signal) } catch { return false }
+}
+
+function ownedTreeAlive(child: ChildProcess): boolean {
+  if (ownsProcessGroup() && child.pid) {
+    try { process.kill(-child.pid, 0); return true }
+    catch (error) { return (error as NodeJS.ErrnoException).code === 'EPERM' }
+  }
+  return child.exitCode === null && child.signalCode === null
+}
+
+async function waitForOwnedTreesToExit(children: ChildProcess[], timeoutMs: number): Promise<void> {
+  const deadline = Date.now() + timeoutMs
+  while (children.some(ownedTreeAlive) && Date.now() < deadline) {
+    await new Promise(resolve => setTimeout(resolve, 10))
+  }
+}
 
 /** Grant free slots to queued waiters, interactive first, up to the cap. */
 function pumpSlots(): void {
@@ -122,15 +157,12 @@ function releaseSlot(): void {
   pumpSlots()
 }
 
-/** Reap every child and cancel anything still queued for a slot. */
-function reapAll(): void {
-  serveClient?.destroy()
+/** Detach every owned child and cancel anything still queued for a slot. */
+function takeAllChildren(): ChildProcess[] {
+  const children = new Set(activeChildren)
+  const resident = serveClient?.destroy()
+  if (resident) children.add(resident)
   serveClient = null
-  // Deliberately harder than every other kill path: quit has a 1.5s flush budget,
-  // shorter than the SIGTERM grace, so waiting one out would just wedge the quit.
-  // A lock left behind here still self-heals via the next parse's stale-pid
-  // takeover; a quit that hangs does not.
-  for (const child of activeChildren) child.kill('SIGKILL')
   activeChildren.clear()
   // A queued waiter has no child to reap, so releaseSlot never fires for it;
   // reject it explicitly so its caller settles instead of hanging past quit.
@@ -139,19 +171,34 @@ function reapAll(): void {
   backgroundQueue.length = 0
   running = 0
   for (const waiter of waiting) waiter.reject(new CliError('nonzero', 'codeburn cancelled'))
+  return [...children]
 }
 
 /** Test/dev cleanup that permits a later fresh start in this same process. */
 export function killAll(): void {
   shuttingDown = false
-  reapAll()
+  shutdownPromise = null
+  for (const child of takeAllChildren()) signalOwnedTree(child, 'SIGKILL')
 }
 
 /** Terminal app shutdown: reap current work and reject any IPC race that arrives
  * while Electron is still flushing telemetry before the final quit pass. */
-export function shutdownAll(): void {
+export function shutdownAll(): Promise<void> {
   shuttingDown = true
-  reapAll()
+  if (shutdownPromise) return shutdownPromise
+  const children = takeAllChildren()
+  for (const child of children) signalOwnedTree(child, 'SIGTERM')
+  shutdownPromise = (async () => {
+    await waitForOwnedTreesToExit(children, SHUTDOWN_TERM_GRACE_MS)
+    // Signal every still-live group, not only direct children. A cooperative
+    // CLI can exit before a stubborn provider subprocess; the group remains
+    // addressable by the original leader pid and must still be reaped.
+    for (const child of children) {
+      if (ownedTreeAlive(child)) signalOwnedTree(child, 'SIGKILL')
+    }
+    await waitForOwnedTreesToExit(children, SHUTDOWN_FORCE_WAIT_MS)
+  })()
+  return shutdownPromise
 }
 
 // Homebrew + common Node version managers, mirroring mac/CodeburnCLI.swift so a
@@ -359,9 +406,9 @@ function killGracefully(child: ChildProcess): void {
   activeChildren.add(child)
   let grace: NodeJS.Timeout | undefined
   const settle = () => { activeChildren.delete(child); if (grace) clearTimeout(grace) }
-  child.once('exit', settle)
-  try { child.kill('SIGTERM') } catch { settle(); return }
-  grace = setTimeout(() => { try { child.kill('SIGKILL') } catch { /* already gone */ } settle() }, KILL_GRACE_MS)
+  child.once('exit', () => { if (!ownedTreeAlive(child)) settle() })
+  if (!signalOwnedTree(child, 'SIGTERM')) { settle(); return }
+  grace = setTimeout(() => { signalOwnedTree(child, 'SIGKILL'); settle() }, KILL_GRACE_MS)
   grace.unref?.()
 }
 
@@ -373,7 +420,7 @@ function withoutProgressLines(stderr: string): string {
 
 function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?: (chunk: string) => void): Promise<unknown> {
   return new Promise<unknown>((resolve, reject) => {
-    const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env })
+    const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env, detached: ownsProcessGroup() })
     activeChildren.add(child)
     let stdout = ''
     let stderr = ''
@@ -413,7 +460,7 @@ function runCli(spec: SpawnSpec, cmdLabel: string, timeoutMs: number, onStderr?:
       total += n
       if (total > MAX_OUTPUT_BYTES) {
         finish(() => {
-          child.kill('SIGKILL')
+          signalOwnedTree(child, 'SIGKILL')
           reject(new CliError('too-large', `codeburn ${cmdLabel} produced more than ${MAX_OUTPUT_BYTES} bytes`))
         })
       }
@@ -527,10 +574,10 @@ class ServeClient {
 
   start(): void {
     if (this.child || this.disabled() || this.destroyed) return
-    const child = spawn(this.spec.bin, [...this.spec.args], { shell: false, stdio: ['pipe', 'pipe', 'ignore'], env: this.spec.env })
+    const child = spawn(this.spec.bin, [...this.spec.args], { shell: false, stdio: ['pipe', 'pipe', 'ignore'], env: this.spec.env, detached: ownsProcessGroup() })
     this.child = child
     if (this.pidFile && child.pid) {
-      const record = JSON.stringify({ pid: child.pid, cmd: [this.spec.bin, ...this.spec.args].join(' ') })
+      const record = JSON.stringify({ pid: child.pid, cmd: [this.spec.bin, ...this.spec.args].join(' '), processGroup: ownsProcessGroup() })
       try { writeFileSync(this.pidFile, record) } catch { /* reaping is best-effort */ }
     }
     child.stdout!.setEncoding('utf8')
@@ -629,7 +676,7 @@ class ServeClient {
       waiter.reject(error)
     }
     this.pending.clear()
-    child.kill('SIGKILL')
+    signalOwnedTree(child, 'SIGKILL')
   }
 
   private onDeath(child: ReturnType<typeof spawn>, countsTowardBudget = true): void {
@@ -713,13 +760,13 @@ class ServeClient {
     })
   }
 
-  destroy(): void {
+  destroy(): ChildProcess | null {
     this.destroyed = true
     this.deaths = SERVE_MAX_RESTARTS
     const child = this.child
-    if (!child) return
+    if (!child) return null
     this.onDeath(child, false)
-    killGracefully(child)
+    return child
   }
 }
 
@@ -795,7 +842,7 @@ export function serveCommandMatches(recorded: string, observed: string | null):
 }
 
 export function reapOrphanServe(pidFile: string): void {
-  let record: { pid?: unknown; cmd?: unknown }
+  let record: { pid?: unknown; cmd?: unknown; processGroup?: unknown }
   try { record = JSON.parse(readFileSync(pidFile, 'utf-8')) } catch { return }
   try { unlinkSync(pidFile) } catch { /* stale file is harmless */ }
   const pid = record.pid
@@ -803,6 +850,9 @@ export function reapOrphanServe(pidFile: string): void {
   if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 1 || pid === process.pid) return
   if (typeof cmd !== 'string' || !cmd) return
   if (!serveCommandMatches(cmd, processCommandLine(pid))) return
+  if (record.processGroup === true && platform() !== 'win32') {
+    try { process.kill(-pid, 'SIGTERM'); return } catch { /* fall back for a pre-group child */ }
+  }
   try { process.kill(pid, 'SIGTERM') } catch { /* already gone */ }
 }
 
@@ -925,7 +975,7 @@ export function spawnCliAction(args: string[], opts: { timeoutMs?: number } = {}
 // no long silent stretch for a watchdog to misread.
 function runAction(spec: SpawnSpec, args: string[], timeoutMs: number): Promise<ActionResult> {
   return new Promise<ActionResult>(resolve => {
-    const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env })
+    const child = spawn(spec.bin, spec.args, { shell: false, stdio: ['ignore', 'pipe', 'pipe'], env: spec.env, detached: ownsProcessGroup() })
     activeChildren.add(child)
     let stdout = ''
     let stderr = ''
@@ -947,7 +997,7 @@ function runAction(spec: SpawnSpec, args: string[], timeoutMs: number): Promise<
     }
 
     const timer = setTimeout(() => {
-      child.kill('SIGKILL')
+      signalOwnedTree(child, 'SIGKILL')
       finish({ ok: false, stdout, stderr: `codeburn ${args[0] ?? ''} timed out after ${timeoutMs}ms`, code: null })
     }, timeoutMs)
 

app/electron/main.test.ts

@@ -379,6 +379,24 @@ describe('createBeforeQuitHandler', () => {
     }
   })
 
+  it('waits for asynchronous child cleanup before the final exit', async () => {
+    let releaseCleanup: (() => void) | undefined
+    const cleanup = new Promise<void>(resolve => { releaseCleanup = resolve })
+    const quit = vi.fn()
+    const handler = createBeforeQuitHandler({
+      getTelemetry: () => null,
+      killAll: () => cleanup,
+      quit,
+    })
+
+    handler({ preventDefault: vi.fn() })
+    await Promise.resolve()
+    expect(quit).not.toHaveBeenCalled()
+
+    releaseCleanup?.()
+    await vi.waitFor(() => expect(quit).toHaveBeenCalledOnce())
+  })
+
   it('still flushes and quits when trackClose throws synchronously', async () => {
     const trackClose = vi.fn(() => { throw new Error('track close failed') })
     const flush = vi.fn(async () => true)

app/electron/main.ts

@@ -18,7 +18,7 @@ type QuitTelemetry = Pick<Telemetry, 'trackClose' | 'flush'>
 type BeforeQuitEvent = { preventDefault: () => void }
 type BeforeQuitDeps = {
   getTelemetry: () => QuitTelemetry | null
-  killAll: () => void
+  killAll: () => void | Promise<void>
   quit: () => void
   timeoutMs?: number
 }
@@ -40,7 +40,8 @@ export function createBeforeQuitHandler(deps: BeforeQuitDeps): (event: BeforeQui
     void (async () => {
       let timer: ReturnType<typeof setTimeout> | undefined
       try {
-        try { deps.killAll() } catch { /* child cleanup must not wedge quit */ }
+        let childCleanup: Promise<unknown> = Promise.resolve()
+        try { childCleanup = Promise.resolve(deps.killAll()).catch(() => undefined) } catch { /* child cleanup must not wedge quit */ }
 
         let telemetry: QuitTelemetry | null = null
         try { telemetry = deps.getTelemetry() } catch { /* telemetry lookup is best-effort */ }
@@ -57,7 +58,10 @@ export function createBeforeQuitHandler(deps: BeforeQuitDeps): (event: BeforeQui
         const timeout = new Promise<void>(resolve => {
           timer = setTimeout(resolve, deps.timeoutMs ?? QUIT_FLUSH_TIMEOUT_MS)
         })
-        await Promise.race([flush.catch(() => false), timeout])
+        await Promise.race([
+          Promise.all([flush.catch(() => false), childCleanup]),
+          timeout,
+        ])
       } finally {
         if (timer !== undefined) clearTimeout(timer)
         allowQuit = true