fix(server): correct process-handler and shutdown lifecycle

src/core/events/bus.ts

@@ -11,6 +11,11 @@ export interface EventBus {
 interface SSEClient {
   controller: ReadableStreamDefaultController
   connectedAt: number
+  // Stored so closeAll() can clear the keepalive deterministically instead of
+  // relying on controller.close() propagating to the stream's cancel()
+  // callback (a Bun ReadableStream implementation detail, not a WhatWG
+  // guarantee).
+  pingInterval?: ReturnType<typeof setInterval> | null
 }
 
 export function createEventBus(): EventBus {
@@ -86,6 +91,7 @@ export function createEventBus(): EventBus {
             }
           }
         }, 3_000)
+        if (client) client.pingInterval = pingInterval
       },
       cancel() {
         if (pingInterval) clearInterval(pingInterval)
@@ -114,7 +120,10 @@ export function createEventBus(): EventBus {
 
   function closeAll(): void {
     for (const c of clients) {
-      try { c.controller.close() } catch { /* ignore */ }
+      // Clear the keepalive explicitly — don't depend on close() triggering
+      // the stream's cancel() callback (Bun impl detail).
+      if (c.pingInterval) clearInterval(c.pingInterval)
+      try { c.controller.close() } catch { /* already-closed controller — safe to ignore during shutdown */ }
     }
     clients.clear()
   }

src/server/AGENTS.md

@@ -15,6 +15,7 @@
 
 ## Key files
 - `index.ts` — THE composition root; 7 named sections (ENV+CONFIG → CORE → NOTIFICATIONS → STATE+BANNER → TRANSPORT → INTEGRATIONS → START → PLUGIN HANDLE); shutdown order is documented here
+- `lifecycle.ts` — process-global error handlers installed exactly once per process (`installGlobalErrorHandlersOnce`) + the re-entrant shutdown guard that also closes SSE clients (`createShutdownGuard`); consumed only by `index.ts`
 - `config.ts` — `loadConfigSafe`, `mergeStoredSettings`, `resolveSources`; config priority: shell env > `~/.opencode-pilot/config.json` > `.env` > defaults
 - `constants.ts` — `PILOT_VERSION`, `DEFAULT_PORT`, all magic numbers; **path is hard-referenced by the release script — do NOT move or rename this file**
 - `config.test.ts` — unit tests for config parsing and merging

src/server/index.ts

@@ -18,6 +18,7 @@ import { opencodeIntegration } from "../integrations/opencode/index"
 import { codexIntegration } from "../integrations/codex/index"
 import { createLogger } from "../infra/logger/index"
 import { PILOT_VERSION, TOAST_DURATION_MS, TOAST_PROMOTION_DURATION_MS, PROMOTION_POLL_INTERVAL_MS } from "./constants"
+import { installGlobalErrorHandlersOnce, createShutdownGuard } from "./lifecycle"
 
 export default {
   id: "opencode-pilot",
@@ -380,48 +381,45 @@ export default {
     // ─── R2: Global error traps ──────────────────────────────────────────
     // Never write to stdout/stderr — the OpenCode TUI renders those as red
     // noise. Write to audit log + logger only.
-    process.on("uncaughtException", (err: Error) => {
-      audit.log("process.uncaughtException", { error: err.message, stack: err.stack })
-      logger.error("Uncaught exception", { error: err.message })
-      eventBus.emit({
-        type: "pilot.error",
-        properties: { kind: "uncaughtException", message: err.message, timestamp: Date.now() },
-      })
-      // Do NOT exit — let the process continue
-    })
-
-    process.on("unhandledRejection", (reason: unknown) => {
-      const message = reason instanceof Error ? reason.message : String(reason)
-      audit.log("process.unhandledRejection", { error: message })
-      logger.error("Unhandled rejection", { error: message })
-      eventBus.emit({
-        type: "pilot.error",
-        properties: { kind: "unhandledRejection", message, timestamp: Date.now() },
-      })
-      // Do NOT exit
-    })
+    //
+    // D1 fix: install EXACTLY ONCE per process regardless of how many
+    // plugin-factory invocations occur in this Bun/Node process. Previously
+    // process.on() was called on every invocation, accumulating N duplicate
+    // handlers → N audit writes per error, MaxListenersExceededWarning, and
+    // a feedback loop where the warning itself got logged as an error.
+    // installGlobalErrorHandlersOnce() uses a module-scoped guard in
+    // lifecycle.ts and is a no-op on every invocation after the first.
+    installGlobalErrorHandlersOnce({ eventBus, audit, logger })
 
     // ─── Graceful shutdown ───────────────────────────────────────────────
     // Shutdown reads `role` at the moment it runs, not at boot — so a
     // passive instance that got promoted correctly tears down the server,
     // tunnel, and state it started. clearState only runs when we're the
     // primary at shutdown; deleting the global state file from a passive
     // instance would blind every other window.
-    async function shutdown(): Promise<void> {
-      if (promotionTimer) {
-        clearInterval(promotionTimer)
-        promotionTimer = null
-      }
-      try { telegram.stop() } catch {}
-      // Spec order: integrations → http → tunnel → notifications.flush → clearState
-      try { await opencode.shutdown() } catch {}
-      try { await codexHandle.shutdown() } catch {}
-      if (role === "primary") {
-        try { server.stop() } catch {}
-        try { tunnel.stop() } catch {}
-        try { await notifications.flush() } catch {}
-        try { clearState(ctx.directory) } catch {}
-      }
+    //
+    // D3 fix: createShutdownGuard() wraps cleanup with a re-entrant guard
+    // (SIGINT then SIGTERM, or double SIGINT, won't double-run cleanup).
+    // D2 fix: the guard also calls eventBus.closeAll() before the body runs
+    // so SSE ping setIntervals don't keep firing after server.stop().
+    const { shutdown } = createShutdownGuard({ eventBus })
+    async function runShutdown(): Promise<void> {
+      return shutdown(async () => {
+        if (promotionTimer) {
+          clearInterval(promotionTimer)
+          promotionTimer = null
+        }
+        try { telegram.stop() } catch {}
+        // Spec order: integrations → http → tunnel → notifications.flush → clearState
+        try { await opencode.shutdown() } catch {}
+        try { await codexHandle.shutdown() } catch {}
+        if (role === "primary") {
+          try { server.stop() } catch {}
+          try { tunnel.stop() } catch {}
+          try { await notifications.flush() } catch {}
+          try { clearState(ctx.directory) } catch {}
+        }
+      })
     }
 
     // ─── Integrations ────────────────────────────────────────────────────
@@ -464,9 +462,19 @@ export default {
     // above. Registering handlers here ensures the closure never captures
     // a variable in the temporal dead zone, even if a SIGINT arrives during
     // the rare window between plugin boot and this line.
-    process.once("SIGINT", () => void shutdown())
-    process.once("SIGTERM", () => void shutdown())
-    process.once("exit", () => void shutdown())
+    //
+    // D4 fix: process.once("exit", ...) was removed. The "exit" event fires
+    // while the event loop is already draining — any awaits inside shutdown()
+    // (opencode.shutdown(), notifications.flush(), etc.) silently never run;
+    // the `void` only suppressed the TS error without fixing anything.
+    // SIGINT and SIGTERM fire BEFORE drain and are sufficient for graceful
+    // shutdown. Note (out-of-scope follow-up): in the multi-instance model,
+    // SIGINT/SIGTERM use process.once, so only the first plugin-factory
+    // invocation's runShutdown() runs on a signal. The second invocation's
+    // cleanup depends on the primary tearing down shared resources. This
+    // design tension is pre-existing and not addressed in this PR.
+    process.once("SIGINT", () => void runShutdown())
+    process.once("SIGTERM", () => void runShutdown())
 
     return {
       event: roleAwareHooks.event,

src/server/lifecycle.ts

@@ -0,0 +1,142 @@
+/**
+ * lifecycle.ts — process-global error handler installation + shutdown guard.
+ *
+ * Extracted from server/index.ts to:
+ *   (D1) install uncaughtException / unhandledRejection EXACTLY ONCE per
+ *        process regardless of how many plugin-factory invocations occur.
+ *   (D3) provide a per-invocation re-entrant shutdown guard so that concurrent
+ *        SIGINT+SIGTERM or multiple signal deliveries do not double-run cleanup.
+ *
+ * Both exports are PURE factory / install functions — no classes, per AGENTS.md.
+ */
+
+import type { EventBus } from "../core/events/bus"
+import { getSharedEventBus } from "../core/events/bus"
+
+// ─── D1 guard — module-scoped, lives for the lifetime of the process ──────────
+//
+// `process.on("uncaughtException", …)` accumulates per call. Every plugin-factory
+// invocation previously added another pair of listeners → N duplicate audit writes
+// and a MaxListenersExceededWarning feedback loop. A module-level boolean ensures
+// the two handlers are installed exactly once no matter how many workspaces/
+// worktrees invoke the factory in the same Node/Bun process.
+//
+// DO NOT use process.once — once removes the listener after the first fire, so a
+// second unhandled rejection would be completely unhandled.
+// DO NOT remove these listeners in per-instance shutdown — they must live for
+// process lifetime so every future unhandled error is still captured.
+let _globalErrorHandlersInstalled = false
+
+/** Deps forwarded to the global error handlers — only the first invocation wins. */
+interface GlobalHandlerDeps {
+  /** The process-wide shared event bus (passed for testability). */
+  eventBus: EventBus
+  /** Audit logger compatible with the AuditLog interface. */
+  audit: { log: (event: string, payload: Record<string, unknown>) => void }
+  /** Structured logger (no console.log — TUI renders stdout as red noise). */
+  logger: { error: (message: string, payload?: Record<string, unknown>) => void }
+}
+
+/**
+ * Install the two process-global error handlers at most once.
+ *
+ * Safe to call on every plugin-factory invocation. On the first call the
+ * handlers are registered; all subsequent calls are no-ops. The handlers
+ * route through `getSharedEventBus()` on EACH invocation so they always use
+ * the live singleton — not a closure over a potentially-stale reference.
+ *
+ * @param deps - passed for testability; the bus obtained via `getSharedEventBus()`
+ *               at call time of each error handler is what's actually used inside
+ *               the callbacks (ensures correctness even if deps differ across invocations).
+ */
+export function installGlobalErrorHandlersOnce(deps: GlobalHandlerDeps): void {
+  if (_globalErrorHandlersInstalled) return
+  _globalErrorHandlersInstalled = true
+
+  // Use the first caller's deps as the stable reference, but inside the
+  // handlers always obtain the shared bus at call time so that bus resets
+  // in tests do not silently break the handler.
+  const { audit, logger } = deps
+
+  process.on("uncaughtException", (err: Error) => {
+    const bus = getSharedEventBus()
+    audit.log("process.uncaughtException", { error: err.message, stack: err.stack ?? "" })
+    logger.error("Uncaught exception", { error: err.message })
+    bus.emit({
+      type: "pilot.error",
+      properties: { kind: "uncaughtException", message: err.message, timestamp: Date.now() },
+    })
+    // Do NOT exit — let the process continue (non-fatal)
+  })
+
+  process.on("unhandledRejection", (reason: unknown) => {
+    const bus = getSharedEventBus()
+    const message = reason instanceof Error ? reason.message : String(reason)
+    audit.log("process.unhandledRejection", { error: message })
+    logger.error("Unhandled rejection", { error: message })
+    bus.emit({
+      type: "pilot.error",
+      properties: { kind: "unhandledRejection", message, timestamp: Date.now() },
+    })
+    // Do NOT exit — non-fatal
+  })
+}
+
+// ─── D3 shutdown guard ────────────────────────────────────────────────────────
+
+interface ShutdownGuardDeps {
+  /** The per-invocation event bus reference (= getSharedEventBus() at call site). */
+  eventBus: EventBus
+}
+
+interface ShutdownGuard {
+  /**
+   * Wraps the caller-supplied async cleanup body with:
+   *   - (D3) re-entrant guard: second invocation is a no-op.
+   *   - (D2) calls `eventBus.closeAll()` before the caller's cleanup runs.
+   *
+   * Usage in server/index.ts:
+   *   const { shutdown } = createShutdownGuard({ eventBus })
+   *   // … build the rest of shutdown logic, then:
+   *   process.once("SIGINT", () => void shutdown(async () => { … your cleanup … }))
+   *
+   * The simpler form `shutdown()` (no argument) is used in tests.
+   */
+  shutdown: (body?: () => Promise<void>) => Promise<void>
+}
+
+/**
+ * Create a per-plugin-invocation re-entrant shutdown guard.
+ *
+ * The returned `shutdown` wrapper guarantees:
+ *   - D2: `eventBus.closeAll()` is called (errors swallowed — best-effort
+ *         cleanup in shutdown path, per AGENTS.md §3 "No silent failures"
+ *         exception: "only acceptable when the error is provably irrelevant
+ *         (e.g., best-effort cleanup in a shutdown path) AND there is a
+ *         comment explaining why").
+ *   - D3: the cleanup body runs at most once; subsequent calls are no-ops.
+ */
+export function createShutdownGuard(deps: ShutdownGuardDeps): ShutdownGuard {
+  let shuttingDown = false
+
+  async function shutdown(body?: () => Promise<void>): Promise<void> {
+    // D3 — re-entrancy guard: SIGINT then SIGTERM (or double-SIGINT) must not
+    // run server.stop(), tunnel.stop(), clearState, etc. twice.
+    if (shuttingDown) return
+    shuttingDown = true
+
+    // D2 — close SSE clients before server.stop() so in-flight streams are
+    // flushed/terminated cleanly. Without this, per-client ping setIntervals
+    // keep firing after Bun's HTTP server has stopped accepting connections.
+    // Swallowed: closeAll() iterates a Set and calls controller.close() per
+    // client — if a client's controller is already closed, close() throws
+    // "ReadableStream is already closed" (Bun/WhatWG). Safe to ignore.
+    try { deps.eventBus.closeAll() } catch { /* best-effort SSE teardown */ }
+
+    if (body) {
+      await body()
+    }
+  }
+
+  return { shutdown }
+}