fix(desktop): bind teardown to retained process authority (clean landing)

apps/desktop/electron/backend-child.ts

@@ -1,103 +1,225 @@
 /**
- * backend-child.ts
+ * Fail-closed lifecycle control for Desktop-owned backend execution scopes.
  *
- * Windows-aware teardown for the desktop's managed backend child process.
- *
- * Node's `child.kill()` only signals the direct child. On Windows a backend
- * that spawned its own grandchildren (a `hermes` REPL, a pty terminal
- * session, the gateway) survives a plain SIGTERM and keeps files (e.g. the
- * venv shim) locked. So on Windows we tree-kill via `forceKillProcessTree`.
- *
- * On POSIX the backend IS spawned into its own session/process-group
- * (start_new_session=True), so `child.kill('SIGTERM')` would only reach the
- * backend and orphan its MCP grandchildren (the leak in #serve-orphans). We
- * signal the whole group via `process.kill(-pid, ...)` instead, falling back
- * to the direct child if the group send fails.
- *
- * Extracted into its own dependency-free module (no electron import) so the
- * tree-kill / group-kill branching can be asserted directly with a fake child
- * object and spy kill functions, instead of grepping main.ts source text for
- * the function body.
+ * A numeric PID is observation, never destructive authority. Electron signals
+ * only the retained ChildProcess object. Platform authority installed before
+ * `hermes_cli.main` expands that retained-root signal to the complete owned
+ * scope: a Windows Job Object or a POSIX session supervisor.
  */
 
-export interface StopBackendChildDeps {
-  /** Defaults to the real platform check; injectable for tests. */
-  isWindows?: boolean
-  /** Windows tree-kill implementation (real: taskkill /T /F via execFileSync). */
-  forceKillProcessTree: (pid: number) => void
-  /**
-   * POSIX group-signal implementation. Real: process.kill(-pgid, signal).
-   * Injectable so the negative-pid group send is asserted in tests without a
-   * live process group. Defaults to process.kill.
-   */
-  killGroup?: (pgid: number, signal: string) => void
+export const STOP_REQUESTED = 'StopRequested' as const
+export const STOP_EXITED = 'Exited' as const
+export const STOP_ALREADY_EXITED = 'AlreadyExited' as const
+export const STOP_NO_AUTHORITY = 'NoAuthority' as const
+export const STOP_PERMISSION_DENIED = 'PermissionDenied' as const
+export const STOP_TIMED_OUT = 'TimedOut' as const
+
+export type BackendStopKind =
+  | typeof STOP_REQUESTED
+  | typeof STOP_EXITED
+  | typeof STOP_ALREADY_EXITED
+  | typeof STOP_NO_AUTHORITY
+  | typeof STOP_PERMISSION_DENIED
+  | typeof STOP_TIMED_OUT
+
+export interface BackendStopResult {
+  readonly kind: BackendStopKind
+  readonly pid?: number | null
+  readonly exitCode?: number | null
+  readonly signalCode?: string | null
+  readonly detail?: string
 }
 
-export interface StopBackendTreesForUpdateDeps {
-  /** Synchronous Windows taskkill /T /F implementation. */
-  forceKillProcessTree: (pid: number) => void
-  /** Clears and stops the desktop's pooled backends. */
-  stopAllPoolBackends: () => void
+export class BackendStopError extends Error {
+  readonly result: BackendStopResult
+
+  constructor(operation: string, result: BackendStopResult) {
+    super(`${operation} failed with ${result.kind}${result.detail ? `: ${result.detail}` : ''}`)
+    this.name = 'BackendStopError'
+    this.result = result
+  }
 }
 
 export interface BackendProcessRoot {
   pid?: number | null
+  exitCode?: null | number
+  signalCode?: null | string
 }
 
 export interface KillableChild extends BackendProcessRoot {
   killed?: boolean
-  kill: (signal: string) => void
+  kill: (signal?: NodeJS.Signals | number | null) => unknown
+  once?: (event: 'exit', listener: (code: number | null, signal: string | null) => void) => unknown
+  removeListener?: (event: 'exit', listener: (...args: any[]) => void) => unknown
 }
 
-/**
- * Stop a managed child process, choosing the right strategy for the platform.
- * No-ops silently if `child` is falsy, already killed, or the kill attempt
- * throws (the process may already be gone) -- mirrors the original inline
- * best-effort semantics in main.ts.
- */
-export function stopBackendChild(child: KillableChild | null | undefined, deps: StopBackendChildDeps) {
-  if (!child || child.killed) {
-    return
-  }
+export interface StopBackendTreesForUpdateDeps {
+  /** Stops pooled backends through their retained ChildProcess owners. */
+  stopAllPoolBackends: () => Promise<void> | void
+}
 
-  const isWindows = deps.isWindows ?? process.platform === 'win32'
-  const killGroup = deps.killGroup ?? ((pgid: number, signal: string) => process.kill(pgid, signal))
+function snapshot(child: KillableChild | null | undefined): Omit<BackendStopResult, 'kind'> {
+  return child
+    ? {
+        pid: child.pid,
+        exitCode: child.exitCode,
+        signalCode: child.signalCode
+      }
+    : {}
+}
+
+/** Missing lifecycle fields mean no authority, never a legacy fallback. */
+export function isLiveProcessRoot(root: BackendProcessRoot | null | undefined): boolean {
+  return Boolean(
+    root &&
+      Number.isInteger(root.pid) &&
+      (root.pid as number) > 0 &&
+      root.exitCode === null &&
+      root.signalCode === null
+  )
+}
+
+function signalRetainedChild(
+  child: KillableChild | null | undefined,
+  signal: NodeJS.Signals
+): BackendStopResult {
+  if (!child) {
+    return { kind: STOP_ALREADY_EXITED }
+  }
+  if (typeof child.kill !== 'function') {
+    return { kind: STOP_NO_AUTHORITY, ...snapshot(child) }
+  }
+  if (child.exitCode != null || child.signalCode != null) {
+    return { kind: STOP_ALREADY_EXITED, ...snapshot(child) }
+  }
+  if (!isLiveProcessRoot(child)) {
+    return { kind: STOP_NO_AUTHORITY, ...snapshot(child) }
+  }
 
   try {
-    if (isWindows && Number.isInteger(child.pid)) {
-      deps.forceKillProcessTree(child.pid as number)
-    } else if (Number.isInteger(child.pid)) {
-      // POSIX: pgid == pid (start_new_session). Signal the whole group so MCP
-      // grandchildren die too; fall back to the direct child on failure.
-      try {
-        killGroup(-(child.pid as number), 'SIGTERM')
-      } catch {
-        child.kill('SIGTERM')
+    if (child.kill(signal) === false) {
+      return {
+        kind: STOP_PERMISSION_DENIED,
+        detail: `retained ChildProcess refused ${signal}`,
+        ...snapshot(child)
       }
-    } else {
-      child.kill('SIGTERM')
     }
-  } catch {
-    // Already gone.
+    // Signal submission is not exit observation. The retained owner remains
+    // live until its exit event or populated exit fields prove otherwise.
+    return {
+      kind: STOP_REQUESTED,
+      detail: `submitted ${signal} to retained owner`,
+      ...snapshot(child)
+    }
+  } catch (error) {
+    return {
+      kind: STOP_PERMISSION_DENIED,
+      detail: error instanceof Error ? error.message : String(error),
+      ...snapshot(child)
+    }
+  }
+}
+
+function requireStopSubmission(operation: string, result: BackendStopResult): BackendStopResult {
+  if (result.kind === STOP_NO_AUTHORITY || result.kind === STOP_PERMISSION_DENIED) {
+    // Existing lifecycle call sites may ignore a compatibility return value,
+    // but they can no longer silently discard a hard authority failure.
+    throw new BackendStopError(operation, result)
   }
+  return result
+}
+
+/** Graceful stop through the retained owner only. */
+export function requestBackendGracefulStop(
+  child: KillableChild | null | undefined
+): BackendStopResult {
+  return signalRetainedChild(child, 'SIGTERM')
 }
 
 /**
- * Stop every backend tree owned by a Windows Desktop update hand-off.
- *
- * Tree-kill the primary root while its PID is still live, then delegate pool
- * teardown to the existing routine that tree-kills each pooled root exactly
- * once before mutating its registry. In particular, do not signal the primary
- * first: if that root exits before taskkill /T runs, Windows can no longer
- * enumerate its MCP grandchildren and they survive with the venv locked.
+ * Forced stop through the same retained owner. POSIX uses SIGUSR2 as the
+ * supervisor's non-PID force command; Windows uses SIGKILL, whose root exit
+ * closes the generation-bound Job and reaps descendants.
  */
-export function stopBackendTreesForUpdate(
-  primary: BackendProcessRoot | null | undefined,
-  deps: StopBackendTreesForUpdateDeps
-): void {
-  if (primary && Number.isInteger(primary.pid)) {
-    deps.forceKillProcessTree(primary.pid as number)
+export function requestBackendForceStop(
+  child: KillableChild | null | undefined,
+  platform = process.platform
+): BackendStopResult {
+  const signal: NodeJS.Signals = platform === 'win32' ? 'SIGKILL' : 'SIGUSR2'
+  return signalRetainedChild(child, signal)
+}
+
+/** Compatibility entry point that preserves typed outcomes and fails loudly. */
+export function stopBackendChild(
+  child: KillableChild | null | undefined
+): BackendStopResult {
+  return requireStopSubmission('graceful backend stop', requestBackendGracefulStop(child))
+}
+
+/** Compatibility entry point that preserves typed outcomes and fails loudly. */
+export function forceStopBackendChild(
+  child: KillableChild | null | undefined,
+  platform = process.platform
+): BackendStopResult {
+  return requireStopSubmission('forced backend stop', requestBackendForceStop(child, platform))
+}
+
+function waitForExit(
+  child: KillableChild | null | undefined,
+  timeoutMs: number
+): Promise<BackendStopResult> {
+  if (!child) {
+    return Promise.resolve({ kind: STOP_ALREADY_EXITED })
+  }
+  if (typeof child.once !== 'function') {
+    return Promise.resolve({ kind: STOP_NO_AUTHORITY, ...snapshot(child) })
+  }
+  if (!isLiveProcessRoot(child)) {
+    return Promise.resolve({ kind: STOP_ALREADY_EXITED, ...snapshot(child) })
+  }
+
+  return new Promise(resolve => {
+    const onExit = (code: number | null, signal: string | null) => {
+      clearTimeout(timer)
+      child.exitCode = code
+      child.signalCode = signal
+      resolve({ kind: STOP_EXITED, ...snapshot(child) })
+    }
+    const timer = setTimeout(() => {
+      child.removeListener?.('exit', onExit)
+      resolve({ kind: STOP_TIMED_OUT, ...snapshot(child) })
+    }, Math.max(0, Math.min(Math.trunc(timeoutMs), 120_000)))
+    child.once?.('exit', onExit)
+  })
+}
+
+/** Graceful -> bounded wait -> force -> terminal confirmation. */
+export async function stopBackendChildAndWait(
+  child: KillableChild | null | undefined,
+  options: { gracefulTimeoutMs?: number; forceTimeoutMs?: number; platform?: NodeJS.Platform } = {}
+): Promise<BackendStopResult> {
+  const graceful = requestBackendGracefulStop(child)
+  if (graceful.kind !== STOP_REQUESTED) {
+    return graceful
   }
 
-  deps.stopAllPoolBackends()
+  const gracefulExit = await waitForExit(child, options.gracefulTimeoutMs ?? 5_000)
+  if (gracefulExit.kind !== STOP_TIMED_OUT) {
+    return gracefulExit
+  }
+
+  const forced = requestBackendForceStop(child, options.platform ?? process.platform)
+  if (forced.kind !== STOP_REQUESTED) {
+    return forced
+  }
+  return waitForExit(child, options.forceTimeoutMs ?? 2_000)
+}
+
+export async function stopBackendTreesForUpdate(
+  primary: KillableChild | null | undefined,
+  deps: StopBackendTreesForUpdateDeps
+): Promise<BackendStopResult> {
+  const primaryResult = stopBackendChild(primary)
+  await deps.stopAllPoolBackends()
+  return primaryResult
 }

apps/desktop/electron/backend-env.test.ts

@@ -118,7 +118,10 @@ test('buildDesktopBackendEnv extends PYTHONPATH and backend PATH together', () =
     pathModule: path.posix
   })
 
-  assert.equal(env.PYTHONPATH, '/repo/hermes-agent:/existing/pythonpath')
+  assert.equal(
+    env.PYTHONPATH,
+    '/repo/hermes-agent/hermes_cli/desktop_bootstrap:/repo/hermes-agent:/existing/pythonpath'
+  )
   assert.ok(
     env.PATH.startsWith(
       '/Users/test/.hermes/node/bin:/Users/test/.hermes/node:/Users/test/.hermes/hermes-agent/venv/bin:'