fix: hook command grandchildren hold stdio pipes and hang triggers forever

.specgit.yaml

@@ -1,17 +1,23 @@
 version: 1
-delivery: accept-builtin-spec
+delivery: hook-command-grandchildren
 context:
   kind: branch
-  branch: fix/506-accept-builtin-spec
+  branch: fix/500-hook-command-grandchildren
 issues:
-  - 506
-  - 507
-  - 508
+  - 500
+  - 501
+  - 502
+  - 503
+  - 504
 issueKinds:
-  - issue: 506
+  - issue: 500
     kind: kind::fix
-  - issue: 507
+  - issue: 501
     kind: kind::fix
-  - issue: 508
-    kind: kind::docs
-pr: 509
+  - issue: 502
+    kind: kind::fix
+  - issue: 503
+    kind: kind::fix
+  - issue: 504
+    kind: kind::fix
+pr: 505

packages/opencode/src/cli/heap.ts

@@ -1,14 +1,50 @@
 import path from "path"
+import * as fs from "fs/promises"
 import { writeHeapSnapshot } from "node:v8"
 import { Flag } from "@opencode-ai/core/flag/flag"
 import { Global } from "@opencode-ai/core/global"
 const MINUTE = 60_000
 const LIMIT = 2 * 1024 * 1024 * 1024
+// Each snapshot is hundreds of MB and RSS storms re-arm; keep only the newest
+// few so repeated snapshots cannot fill the log directory.
+const RETAINED_SNAPSHOTS = 2
 
 let timer: Timer | undefined
 let lock = false
 let armed = true
 
+export function pruneHeapSnapshots(directory: string, keep = RETAINED_SNAPSHOTS) {
+  return fs
+    .readdir(directory, { withFileTypes: true })
+    .then((entries) => {
+      const names = entries
+        .filter((entry) => entry.isFile() && entry.name.startsWith("heap-") && entry.name.endsWith(".heapsnapshot"))
+        .map((entry) => entry.name)
+        // Oldest-first by embedded timestamp, NOT by full name: the layout is
+        // heap-<pid>-<ts>, so a plain lexicographic sort orders snapshots by
+        // pid across runs (pid digit-count changes and wraparound) and pruning
+        // would delete the newest snapshot while keeping stale ones.
+        .sort((a, b) => snapshotTime(a).localeCompare(snapshotTime(b)))
+      return Promise.all(
+        names.slice(0, Math.max(0, names.length - keep)).map((name) =>
+          fs.rm(path.join(directory, name), { force: true }).catch((cause) => {
+            console.warn(`opencode: failed to prune heap snapshot ${name}: ${String(cause)}`)
+          }),
+        ),
+      )
+    })
+    .catch((cause) => {
+      // A missing log directory is the normal first-run state; anything else
+      // is a real prune failure and best-effort cleanup must still surface it.
+      if ((cause as { code?: string }).code === "ENOENT") return
+      console.warn(`opencode: failed to list heap snapshots for pruning: ${String(cause)}`)
+    })
+}
+
+function snapshotTime(name: string) {
+  return name.slice(name.lastIndexOf("-") + 1)
+}
+
 export function start() {
   if (!Flag.OPENCODE_AUTO_HEAP_SNAPSHOT) return
   if (timer) return
@@ -32,6 +68,7 @@ export function start() {
     await Promise.resolve()
       .then(() => writeHeapSnapshot(file))
       .catch(() => {})
+    await pruneHeapSnapshots(Global.Path.log)
 
     lock = false
   }

packages/opencode/src/hook/settings.ts

@@ -1009,6 +1009,12 @@ export function warnUnsupportedFields(
 
 const DEFAULT_TIMEOUT_MS = 60_000 // CC default
 
+// #500: after the child exits (or the spawn timeout SIGTERMs it), stdio EOF is
+// given this grace before the whole process group is SIGKILLed; DRAIN_MS is
+// the post-kill window for final buffered output before resolving partial.
+const KILL_GRACE_MS = 2_000
+const DRAIN_MS = 500
+
 function execShell(
   entry: HookCommand,
   stdinJSON: string,
@@ -1051,12 +1057,16 @@ function execShell(
       }
     }
 
+    // POSIX: run the child as a process-group leader so a hung grandchild
+    // holding the stdio pipes can be signaled as a group (#500). Windows
+    // relies on `taskkill /T` tree kill instead.
     const child = spawn(expandedCommand, [], {
       cwd,
       shell,
       env: { ...process.env, ...extraEnv },
       stdio: ["pipe", "pipe", "pipe"],
       timeout: timeoutMs,
+      detached: process.platform !== "win32",
     })
 
     let stdout = ""
@@ -1078,20 +1088,89 @@ function execShell(
       log.warn("hook stdin write failed", { command, error: String(err) })
     }
 
-    child.on("error", (err) => {
-      log.error("hook command failed to spawn", { command, error: err.message })
-      resolve({ exitCode: null, stdout, stderr, spawnError: err.message })
+    // #500: `close` only fires after BOTH exit and stdio EOF; a grandchild
+    // that inherits the pipes and outlives the shell blocks EOF forever, so
+    // the old `close` waiter never resolved. Exit and stream EOF are now
+    // awaited as independent conditions, with a process-group SIGKILL as the
+    // fallback that guarantees resolution.
+    let exitCode: number | null = null
+    let settled = false
+    const timers = new Set<NodeJS.Timeout>()
+    const arm = (fire: () => void, ms: number) => {
+      const timer = setTimeout(() => {
+        timers.delete(timer)
+        if (!settled) fire()
+      }, ms)
+      timers.add(timer)
+    }
+    const exited = new Promise<void>((resolveExit) => {
+      child.on("exit", (code) => {
+        exitCode = code
+        resolveExit()
+      })
     })
-
-    child.on("close", (code) => {
+    // `end` alone is not enough: on spawn failure the streams can close or
+    // error without ever reaching EOF.
+    const streamSettled = (stream: NodeJS.ReadableStream) =>
+      new Promise<void>((resolveStream) => {
+        stream.on("end", resolveStream)
+        stream.on("close", resolveStream)
+        stream.on("error", resolveStream)
+      })
+    const streamsDone = Promise.all([streamSettled(child.stdout), streamSettled(child.stderr)])
+    const finish = (spawnError?: string) => {
+      if (settled) return
+      settled = true
+      for (const timer of timers) clearTimeout(timer)
+      timers.clear()
+      child.stdout.destroy()
+      child.stderr.destroy()
       log.debug("hook close", {
         command: command.slice(0, 80),
-        exitCode: code,
+        exitCode,
         stdoutLen: stdout.length,
         stderrLen: stderr.length,
       })
-      resolve({ exitCode: code, stdout, stderr })
+      resolve(spawnError === undefined ? { exitCode, stdout, stderr } : { exitCode, stdout, stderr, spawnError })
+    }
+
+    child.on("error", (err) => {
+      log.error("hook command failed to spawn", { command, error: err.message })
+      finish(err.message)
     })
+
+    let killSent = false
+    const killGroup = () => {
+      if (killSent || child.pid === undefined) return
+      killSent = true
+      if (process.platform === "win32") {
+        spawn("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore", windowsHide: true }).on(
+          "error",
+          (err) => log.warn("hook taskkill failed", { command, error: err.message }),
+        )
+        return
+      }
+      try {
+        process.kill(-child.pid, "SIGKILL")
+      } catch (err) {
+        log.warn("hook process-group kill failed", { command, error: String(err) })
+      }
+    }
+    const afterKill = () => {
+      killGroup()
+      const drained = new Promise<void>((resolveDrain) => arm(resolveDrain, DRAIN_MS))
+      void Promise.all([exited, Promise.race([streamsDone, drained])]).then(() => finish())
+    }
+
+    void Promise.all([exited, streamsDone]).then(() => finish())
+
+    // Child exited but pipes are still open (grandchild holds them): kill the
+    // group after the EOF grace, then resolve with whatever was captured.
+    void exited.then(() => arm(afterKill, KILL_GRACE_MS))
+
+    // Child ignored the spawn-timeout SIGTERM and never exited: kill the group
+    // at the absolute deadline, wait for the reap, then resolve.
+    arm(afterKill, timeoutMs + KILL_GRACE_MS)
   })
 }
 

packages/opencode/src/mcp/index.ts

@@ -12,6 +12,7 @@ import { ConfigMCPV1 } from "@opencode-ai/core/v1/config/mcp"
 import { NamedError } from "@opencode-ai/core/util/error"
 import { InstallationVersion } from "@opencode-ai/core/installation/version"
 import { withTimeout } from "@/util/timeout"
+import { Process } from "@/util/process"
 import { FSUtil } from "@opencode-ai/core/fs-util"
 import { McpOAuthProvider, OAUTH_CALLBACK_PATH } from "./oauth-provider"
 import { McpOAuthCallback } from "./oauth-callback"
@@ -398,7 +399,10 @@ export const layer = Layer.effect(
           } satisfies CreateResult
         }).pipe(
           Effect.catchCause((cause) =>
-            Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore, Effect.andThen(Effect.failCause(cause))),
+            Effect.gen(function* () {
+              yield* shutdownClient(mcpClient)
+              return yield* Effect.failCause(cause)
+            }),
           ),
         )
       },
@@ -437,6 +441,19 @@ export const layer = Layer.effect(
       Effect.catch(() => Effect.succeed([] as number[])),
     )
 
+    // Close a client and make sure its whole process tree is reaped. The
+    // descendant snapshot must be taken while the root pid is alive (pgrep
+    // walks parent links); close() shuts the root down gracefully, and
+    // stopTree force-kills whatever survived (ignored SIGTERM, orphaned
+    // grandchildren) and waits for exit.
+    const shutdownClient = Effect.fnUntraced(function* (client: MCPClient) {
+      const pid = client.transport instanceof StdioClientTransport ? client.transport.pid : null
+      const tree = typeof pid === "number" ? [pid, ...(yield* descendants(pid))] : []
+      yield* Effect.tryPromise(() => client.close()).pipe(Effect.ignore)
+      if (tree.length === 0) return
+      yield* Effect.tryPromise(() => Process.stopTree(tree)).pipe(Effect.ignore)
+    })
+
     function watch(s: State, name: string, client: MCPClient, bridge: EffectBridge.Shape, timeout?: number) {
       // mcp-elicitation-notification: handle `elicitation/create` reverse requests.
       // Routes through the Question service (best-effort session via SessionContext),
@@ -536,23 +553,7 @@ export const layer = Layer.effect(
             s.clients = {}
             s.defs = {}
             s.instructions = {}
-            yield* Effect.forEach(
-              clients,
-              (client) =>
-                Effect.gen(function* () {
-                  const pid = client.transport instanceof StdioClientTransport ? client.transport.pid : null
-                  if (typeof pid === "number") {
-                    const pids = yield* descendants(pid)
-                    for (const dpid of pids) {
-                      try {
-                        process.kill(dpid, "SIGTERM")
-                      } catch {}
-                    }
-                  }
-                  yield* Effect.tryPromise(() => client.close()).pipe(Effect.ignore)
-                }),
-              { concurrency: "unbounded" },
-            )
+            yield* Effect.forEach(clients, (client) => shutdownClient(client), { concurrency: "unbounded" })
             pendingOAuthTransports.clear()
           }),
         )
@@ -567,7 +568,7 @@ export const layer = Layer.effect(
       delete s.defs[name]
       delete s.instructions[name]
       if (!client) return Effect.void
-      return Effect.tryPromise(() => client.close()).pipe(Effect.ignore)
+      return shutdownClient(client)
     }
 
     const storeClient = Effect.fnUntraced(function* (
@@ -586,7 +587,7 @@ export const layer = Layer.effect(
       if (instructions) s.instructions[name] = instructions
       else delete s.instructions[name]
       watch(s, name, client, bridge, timeout)
-      if (previous) yield* Effect.tryPromise(() => previous.close()).pipe(Effect.ignore)
+      if (previous) yield* shutdownClient(previous)
       return s.status[name]
     })
 

packages/opencode/src/memory/store.ts

@@ -29,6 +29,10 @@ const METADATA_KEYS = [
 ] as const
 const ITEM_KEYS = ["id", "kind", "content", "rationale", "confirmed_at"] as const
 
+// Keep a few recent generations on disk: a reader holding a just-published
+// manifest must still find its generation after later commits GC older ones.
+const RETAINED_GENERATIONS = 3
+
 const PROHIBITED_CONTENT = [
   /```|`[^`]+`/,
   /(?:^|\s)(?:~\/|\.\.?\/|\/)[^\s]+/,
@@ -236,6 +240,25 @@ export const layer = Layer.effect(
       } satisfies Snapshot
     })
 
+    const gcGenerations = Effect.fnUntraced(function* (projectID: ProjectV2.ID) {
+      const generations = home.generations(projectID)
+      const entries = yield* fs.readDirectoryEntries(generations)
+      const stale = entries
+        .filter((entry) => entry.type === "directory" && !entry.name.startsWith("."))
+        .sort(
+          (a, b) => Number.parseInt(b.name, 10) - Number.parseInt(a.name, 10) || b.name.localeCompare(a.name),
+        )
+        .slice(RETAINED_GENERATIONS)
+        .map((entry) => join(generations, entry.name))
+      // Orphan staging directories are rename leftovers from crashed writes.
+      const staging = entries
+        .filter((entry) => entry.name.startsWith(".") && entry.name.endsWith(".tmp"))
+        .map((entry) => join(generations, entry.name))
+      yield* Effect.forEach([...stale, ...staging], (path) => fs.remove(path, { force: true, recursive: true }), {
+        discard: true,
+      })
+    })
+
     const writeSnapshot = Effect.fnUntraced(function* (
       projectID: ProjectV2.ID,
       revision: number,
@@ -265,6 +288,11 @@ export const layer = Layer.effect(
         )
       }).pipe(Effect.onError(() => fs.remove(staging, { force: true, recursive: true }).pipe(Effect.ignore)))
       yield* fs.remove(home.topics(projectID), { force: true, recursive: true }).pipe(Effect.ignore)
+      // GC is best-effort: the commit has already landed, a cleanup failure
+      // must never fail it.
+      yield* gcGenerations(projectID).pipe(
+        Effect.catchCause((cause) => Effect.logWarning("memory generation GC failed", { cause: cause })),
+      )
     })
 
     const readTopics = Effect.fn("MemoryStore.readTopics")((projectID: ProjectV2.ID) =>

packages/opencode/src/share/share-next.ts

@@ -150,9 +150,16 @@ export const layer = Layer.effect(
     const state: InstanceState.InstanceState<State> = yield* InstanceState.make<State>(
       Effect.fn("ShareNext.state")(function* (_ctx) {
         const cache: State = { queue: new Map(), scope: yield* Scope.make(), shared: new Map() }
+        // EventV2 listeners live in a process-level array; collect their
+        // unsubscribers or every instance remount leaks another batch of
+        // subscribers pinning this closure.
+        const unsubscribers: Array<EventV2.Unsubscribe> = []
 
         yield* Effect.addFinalizer(() =>
-          Scope.close(cache.scope, Exit.void).pipe(
+          // Unsubscribe before closing the scope so no in-flight event lands
+          // in a subscriber whose fork scope is already gone.
+          Effect.forEach(unsubscribers, (unsubscribe) => unsubscribe, { discard: true }).pipe(
+            Effect.andThen(Scope.close(cache.scope, Exit.void)),
             Effect.andThen(
               Effect.sync(() => {
                 cache.queue.clear()
@@ -182,7 +189,7 @@ export const layer = Layer.effect(
                 Effect.logError("share subscriber failed", { type: def.type, cause: cause }),
               ),
             )
-          })
+          }).pipe(Effect.tap((unsubscribe) => Effect.sync(() => unsubscribers.push(unsubscribe))))
 
         yield* watch(Session.Event.Updated, (data) =>
           Effect.gen(function* () {