fix(sidecar): a sidecar cannot outlive a parent that died before it started

fork/manifest.json

@@ -230,6 +230,7 @@
     "packages/opencode/test/peer/claude/probe-deliver-runtime.mts",
     "packages/opencode/test/peer/claude/sidecar-diagnostic-routing.test.ts",
     "packages/opencode/test/peer/claude/sidecar-server-name.test.ts",
+    "packages/opencode/test/peer/claude/sidecar-startup-race.test.ts",
     "packages/opencode/src/local/model-gallery/search.ts",
     "packages/opencode/src/local/model-gallery/install.ts",
     "packages/opencode/src/local/model-gallery/operations.ts",

openspec/changes/sidecar-orphan-cleanup/proposal.md

@@ -0,0 +1,109 @@
+# Proposal: sidecars that outlive their session
+
+## Problem
+
+A sidecar (`opencode debug claude-sidecar-entry`) is a separate OS process that receives
+inbound peer messages over a unix socket and hands them to the session that spawned it. It
+has its own lifetime, and the code has two mechanisms intended to keep that lifetime tied
+to the session. On 2026-10-03 neither mechanism reclaimed **44 live sidecars** on one
+machine, and they had to be killed by explicit PID.
+
+The 44 were all `bun.exe run <deleted-worktree>/packages/opencode/src/index.ts debug
+claude-sidecar-entry`, with `ppid 1`, ~16 hours old, sleeping, and **holding no session
+database**. Every one was unroutable: `resolveOpencodeSender`
+(`packages/opencode/src/peer/route.ts:203-208`) resolves a claimed address through the
+registry, and none of these had a registry entry, so no peer could address them at all.
+They were pure load — plausibly a contributor to the load spikes seen the same day.
+
+## Why the two existing mechanisms miss them
+
+**The sweep cannot, by design.** `sweepStaleSidecars`
+(`packages/opencode/src/peer/claude/sidecar-registry.ts:98-126`) walks the registry
+directory and removes an entry only when `!isAlive(parsed.pid)`. It removes *files for
+processes that have already exited*. It never signals a live process. An orphaned sidecar
+is alive by definition, so the sweep is structurally incapable of reclaiming one.
+
+**The self-termination guard has a startup blind spot.**
+`packages/opencode/src/peer/claude/sidecar-entry.ts:93-97`:
+
+```ts
+const originalPpid = process.ppid
+const orphanCheck = setInterval(() => {
+  if (process.ppid !== originalPpid) shutdown()
+}, 2_000)
+orphanCheck.unref()
+```
+
+This detects a ppid that *changes after startup*. If the parent dies **before the child
+evaluates `process.ppid`**, the child is already reparented (to pid 1, or the nearest
+subreaper). `originalPpid` then captures the reparented value, the comparison is false
+forever, and the sidecar has no parent to signal it and no sweep that can see it.
+
+There is a second ordering hazard behind the same outcome.
+`packages/opencode/src/peer/claude/sidecar-manager.ts:121` spawns the child, and the
+registration is written afterwards. A parent that dies in that window yields a process that
+is both **unregistered** and **unreclaimable** — the worst combination, because the sweep
+cannot even enumerate it.
+
+## What is already covered, and what is not
+
+`packages/opencode/test/peer/claude/sidecar-e2e.test.ts:222` covers the case the code was
+written for: a parent that dies *after* the sidecar is up. It waits for the registration,
+then exits, and asserts the sidecar self-terminates. Its own comment notes this is the
+case a sweep cannot reclaim.
+
+Neither of the observed failure modes is that case. Both are **startup races**: the parent
+already gone when the sidecar begins running. No test covers either, and the guard's
+structure means the already-orphaned case is not merely untested but undetectable by the
+guard as written.
+
+## The mechanism, confirmed and narrowed
+
+Two peer sessions reproduced the race independently on Bun 1.3.14/macOS. The second
+investigation narrowed the window, and the narrowing matters because it corrects this
+document's original description.
+
+The window is **not** "the parent dies before the child reads `process.ppid`" in the
+abstract. In execution order:
+
+1. `sidecar-entry.ts:37` — `await startSidecar(...)`, which is async and writes the
+   registration from inside `sidecar-server.ts:156`.
+2. `sidecar-entry.ts:93` — `originalPpid = process.ppid`.
+3. `sidecar-entry.ts:94` — the poll interval starts.
+
+So the blind window is the duration of `startSidecar`: from spawn until line 93. A parent
+that dies anywhere inside it leaves `originalPpid` reading as 1, the comparison
+`1 !== 1` false forever, and a sidecar that never self-terminates.
+
+That window has two sub-cases, distinguished by whether the registration reached disk:
+
+- parent dies **before** `writeSidecarRegistration` completes → alive, **unregistered**,
+  invisible to the sweep and unroutable;
+- parent dies **after** it, before line 93 → alive, **registered**, still skipped by the
+  sweep because the sweep only removes registrations whose pid is *dead*.
+
+All 44 observed instances were in the first sub-case (no registration), but both are real
+and one fix covers both: the parent's stdin pipe closes at any point in the sidecar's
+life, so EOF on a real pipe cannot be missed by any of these windows.
+
+## Why this is a change and not a patch
+
+The guard needs to know its spawner was real, not infer it from a ppid read that may already
+be post-reparenting. Options differ in cost and in what they guarantee:
+
+- pass the spawner's pid explicitly via the environment, and treat "spawner pid absent or
+  not alive at startup" as immediate orphanhood;
+- have the parent hold the socket and close it on exit, so the sidecar's accept loop
+  observes EOF;
+- add an explicit reaper that can identify and reclaim a live sidecar with no reachable
+  parent, which requires a registry-independent notion of ownership.
+
+The first is small and closes the window. The third is what would make the sweep able to
+reclaim a leak that already happened on other machines, where no fix to new spawns helps.
+Which of these to do is a decision, not an implementation detail.
+
+## Scope
+
+Process lifetime and cleanup only. Nothing here touches sender identity, lead grants, or
+the review record; `review-on-done` Phase 1b depends on the identity question separately
+and is tracked there.

openspec/changes/sidecar-orphan-cleanup/specs/sidecar-orphan-cleanup/spec.md

@@ -0,0 +1,63 @@
+## ADDED Requirements
+
+### Requirement: a sidecar cannot outlive the session that spawned it
+A sidecar SHALL terminate within a bounded interval of its owning session's process exiting,
+including when the session's process exits before the sidecar has finished starting.
+
+#### Scenario: parent exits during sidecar startup
+- **WHEN** the spawning process exits before the sidecar process has begun executing
+- **THEN** the sidecar terminates within a bounded interval rather than running indefinitely
+
+#### Scenario: parent exits after the sidecar is established
+- **WHEN** the spawning process exits without signalling, after the sidecar is running
+- **THEN** the sidecar self-terminates and removes its own registration
+
+#### Scenario: reparenting is not evidence of orphaning on its own
+- **WHEN** a sidecar is started by an intermediate process that has already exited
+- **THEN** the sidecar SHALL determine liveness from its owning session, not from a single
+  reading of the parent pid taken at startup
+
+### Requirement: every live sidecar is discoverable and reclaimable
+A sidecar that is running SHALL either be attributable to a live session, or be reclaimable by
+an explicit operator action. A running sidecar SHALL NOT exist that is neither.
+
+#### Scenario: orphan with a stale registration
+- **WHEN** a sidecar is running, its registration is present, and the owning session's process
+  is gone
+- **THEN** an operator command can identify and terminate it
+
+#### Scenario: orphan with no registration
+- **WHEN** a sidecar is running and no registration exists for it
+- **THEN** an operator command can still identify it as a candidate, and the condition is
+  reported rather than silently tolerated
+
+### Requirement: registration is established before or with the process
+A sidecar's ownership SHALL be recorded such that a running sidecar is never simultaneously
+unregistered and unreclaimable.
+
+#### Scenario: session exits between spawn and registration
+- **WHEN** the session's process exits between spawning a sidecar and writing its registration
+- **THEN** the resulting sidecar is still reclaimable by an operator command
+
+## MODIFIED Requirements
+
+### Requirement: the boot sweep removes stale registrations without signalling processes
+The startup sweep SHALL continue to remove registration files whose process is no longer alive,
+and SHALL NOT signal any running process. Reclaiming a live process is a separate, explicit
+operator action.
+
+#### Scenario: registration whose process has exited
+- **WHEN** the sweep finds a registration whose pid is not alive
+- **THEN** it removes the registration file and its key file
+
+#### Scenario: registration whose process is alive
+- **WHEN** the sweep finds a registration whose pid is alive
+- **THEN** it leaves the registration untouched, regardless of whether the owning session
+  appears inactive
+
+## Non-goals
+
+These requirements cover process lifetime and cleanup only. Sender identity, lead grants and
+the review record are unchanged here; `review-on-done` tracks identity separately, and its
+Phase 1b depends on the decision recorded in
+`openspec/changes/review-on-done/findings-authenticity.md`.

openspec/changes/sidecar-orphan-cleanup/tasks.md

@@ -0,0 +1,65 @@
+# Tasks
+
+## Phase 0: confirm the mechanism before fixing it
+
+- [x] 0.1 Reproduce the startup race. Spawn a sidecar from a process that exits immediately, so
+      the child is already reparented when it reads `process.ppid`. Confirm it survives past the
+      2s interval. The existing test at `test/peer/claude/sidecar-e2e.test.ts:222` deliberately
+      waits for registration before killing the parent, so it cannot catch this.
+- [x] 0.2 Determine how a running sidecar ends up with no registration: parent exit inside the
+      `Process.spawn` → `writeSidecarRegistration` window (`sidecar-manager.ts:121`), a failed
+      write, or both. Instrument rather than assume.
+- [ ] 0.3 Census the machine for other shapes of leak beyond the 44 observed on 2026-10-03, so the
+      fix covers the real population rather than the one case that was noticed.
+- [ ] 0.4 Decide whether the fix must also reclaim sidecars already leaked elsewhere. A fix that
+      only prevents future leaks leaves existing ones running until someone sweeps by hand.
+
+## Phase 1: make orphaning detectable from the sidecar  (DONE — 7efff559d0)
+
+- [x] 1.0 Exit on stdin EOF from the parent's pipe. The pipe closes at any point in the
+      sidecar's life, so this covers both sub-windows of the blind spot, including the
+      one where `originalPpid` is captured already-reparented.
+- [x] 1.0a Bound `sidecar.stop()` in `shutdown()` with a timer. `shuttingDown` was set
+      before the await, so a stop that never settled left the process alive with every
+      later SIGTERM swallowed.
+- [x] 1.0b The discriminator is `isSocket()`, NOT `isFIFO()`. Measured on Bun 1.3.14:
+      `stdio: ["pipe"]` gives the child a socketpair end (mode 140000, isFIFO false),
+      while `stdio: "ignore"` gives /dev/null (isCharacterDevice true). An isFIFO check
+      silently disables the guard in production — it was caught by the failing test.
+
+
+- [ ] 1.1 Choose the liveness signal: pass the owning session's pid explicitly and treat
+      "absent or not alive at startup" as orphanhood, or have the parent hold the socket so the
+      sidecar observes EOF. Record the choice and its trade-offs in the change.
+- [ ] 1.2 Remove the reliance on a single startup reading of `process.ppid`
+      (`sidecar-entry.ts:93`), which cannot detect a sidecar that was already orphaned.
+- [ ] 1.3 Observed-red test for each scenario in the spec, mutation-checked: revert the fix and
+      confirm the test fails, so a green result is evidence rather than decoration.
+
+## Phase 2: make leaked sidecars reclaimable
+
+- [ ] 2.1 Add an explicit operator command that identifies candidate sidecars: running, no live
+      owning session, and either a stale registration or none at all. It SHALL report before it
+      acts, and SHALL require confirmation.
+- [ ] 2.2 It SHALL refuse any candidate whose owning session is live, even if that session is
+      idle — the mistake that matters is killing a real session's sidecar.
+- [ ] 2.3 Keep the boot sweep unchanged in behaviour: files for dead processes only, never a
+      signal to a running one.
+
+## Phase 3: close the registration window
+
+- [ ] 3.1 Record ownership so a running sidecar is never both unregistered and unreclaimable,
+      whether the session exits between spawn and registration or the write fails.
+- [ ] 3.2 Test the failure path, not just the happy path.
+
+## Notes
+
+- Observed on 2026-10-03: 44 `bun.exe run <deleted-worktree>/…/src/index.ts debug
+  claude-sidecar-entry` processes, `ppid 1`, ~16h old, sleeping, none holding a session database,
+  none present in `~/.claude/sessions/` with `managedBy: "opencode-skein"`. All 44 were
+  unroutable — `resolveOpencodeSender` resolves only through the registry — so they could not
+  receive or be addressed, and were pure load. They were reclaimed by explicit PID after
+  per-process verification.
+- The 15 registered sidecars on the same machine were unaffected and spanned five repositories,
+  so this is a specific hole in the guard rather than a general failure of it.
+- Not in scope: sender identity, lead grants, review gating.

packages/opencode/src/peer/claude/sidecar-entry.ts

@@ -6,8 +6,12 @@
 // peer. Never touches the opencode server's HTTP API or any credential for
 // it — the parent process does the actual session injection, this process's
 // only job is being a real, Claude-compatible peer.
+import { fstatSync } from "fs"
 import { startSidecar } from "./sidecar-server"
 
+/** Upper bound on how long a sidecar may stay alive after it decides to stop. */
+const STOP_TIMEOUT_MS = 5_000
+
 export async function runSidecarEntry() {
   const ownerSessionID = process.env.OPENCODE_SIDECAR_OWNER_SESSION_ID
   if (!ownerSessionID) {
@@ -66,11 +70,37 @@ export async function runSidecarEntry() {
   })
   process.stdin.on("error", () => undefined)
 
+  // Whether stdin is a real pipe from the parent, or /dev/null because the
+  // spawner passed stdio: "ignore". Decided once, before any listener can fire,
+  // so a misdetection cannot kill a live sidecar.
+  //
+  // Not `isFIFO()`. Measured on Bun 1.3.14/macOS: a child spawned with
+  // stdio: ["pipe"] reports isSocket()=true, isFIFO()=false (mode 140000), while
+  // stdio: "ignore" reports isCharacterDevice()=true (mode 20000, /dev/null).
+  // Bun gives the child a socketpair end, not a FIFO, so checking isFIFO()
+  // silently disables this guard in production and only ever matches nothing.
+  const stdinIsPipe = (() => {
+    try {
+      const s = fstatSync(process.stdin.fd ?? 0)
+      return s.isSocket() || s.isFIFO()
+    } catch {
+      return false
+    }
+  })()
+
   let shuttingDown = false
   const shutdown = () => {
     if (shuttingDown) return
     shuttingDown = true
     clearInterval(orphanCheck)
+    // Bound the stop. `sidecar.stop()` closes the listening socket and removes
+    // the registration; if that never settles (a hung close), the original
+    // `.finally(process.exit)` never fires — and because `shuttingDown` is now
+    // true, every later SIGTERM is swallowed and the process is unkillable
+    // except by SIGKILL. So the exit is guaranteed on a timer rather than on
+    // stop() settling.
+    const bail = setTimeout(() => process.exit(0), STOP_TIMEOUT_MS)
+    bail.unref()
     sidecar
       .stop()
       .catch(() => undefined)
@@ -96,6 +126,27 @@ export async function runSidecarEntry() {
   }, 2_000)
   orphanCheck.unref()
 
+  // The ppid guard above has a startup blind spot: it can only see a parent that
+  // dies AFTER this line runs. If the parent is already gone by the time we read
+  // `process.ppid`, the value captured here is the reparented one, the comparison
+  // is false forever, and the sidecar survives indefinitely. Reproduced on
+  // Bun 1.3.14/macOS: SIGKILL the parent before the child's first ppid read and
+  // the child records ppid 1 and never notices.
+  //
+  // The parent's stdin pipe closes when the parent dies, at ANY point in our
+  // lifetime, so EOF on a real pipe is a signal that cannot be missed by a
+  // startup race. Registered after `orphanCheck` exists so `shutdown`'s
+  // `clearInterval(orphanCheck)` cannot hit the temporal dead zone.
+  //
+  // Only when stdin is genuinely a pipe. `sidecar-manager.ts` spawns us with
+  // `stdin: "pipe"`, but tests spawn with `stdio: "ignore"`, which hands us
+  // /dev/null — that reads EOF immediately and would kill a perfectly healthy
+  // sidecar on startup. A FIFO is the discriminator.
+  if (stdinIsPipe) {
+    process.stdin.on("end", shutdown)
+    process.stdin.on("close", shutdown)
+  }
+
   // Never resolve on its own: when run as `debug claude-sidecar-entry`,
   // returning here would let `index.ts`'s `cli.parse()` complete, which
   // unconditionally force-exits the whole process afterward — confirmed