From 946a7ae1873d09563b0e12e759a669e1286f76c2 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 27 Sep 2026 02:25:44 +0100 Subject: [PATCH 1/2] fix: reap the headroom proxy and read zombie pids as dead The crash-restart path never fired after a kill -9 of the daemon, for three compounding reasons, all fixed at their roots. The supervisor spawned headroom detached and unref'd with no exit listener, so nothing ever waited on the child: it lingered as a zombie with the supervisor as its parent. It now keeps every ChildProcess handle and consumes the exit event, which is both the reaping and the crash signal; unref is gone, because a handle taken off the event loop is a child nobody reaps. Even with a listener, the loop slept with Atomics.wait, which blocks the event loop and stops exit events being delivered at all. The SupervisorPorts sleep is now a real timer the loop awaits, so the loop turns between ticks. Crash detection polled liveness with kill(pid, 0), and a zombie answers that as alive. Every liveness check in the coordination layer (the supervisor, the ensure step, session pruning, status, doctor) now uses a zombie-aware predicate: realIsProcessRunning reads ps's state column and treats Z as dead, documented against the signal-0 check a defunct process defeats. The test fakes model existence, zombification, and the exit event as distinct facts, which the old always-alive fakes could not. Also: the deliberate stop path escalates SIGTERM to SIGKILL through a bounded poll, as a pure stopSupervisedProcess with its own tests (this proxy ignores SIGTERM outright); a detected crash clears the daemon fields from state immediately, so status never claims a dead port; the supervisor handles SIGTERM and SIGINT and routes them through an orderly exit, because Node skips exit handlers on unhandled signal death, which orphaned the daemon; and a successor supervisor stops an orphan daemon its predecessor left running before starting its own. Verified live against the real daemon: kill -9 of the proxy restarts it within one tick with the old pid fully reaped, a drift restart serves the new allowlist, and SIGTERM of the supervisor takes the daemon with it. --- docs/architecture.md | 2 + docs/configuration-model.md | 2 +- docs/testing.md | 2 +- src/doctor.test.ts | 10 +- src/doctor.ts | 12 +-- src/headroom/commands.ts | 94 +++++++++++------- src/headroom/ensure.test.ts | 25 ++++- src/headroom/ensure.ts | 9 +- src/headroom/state.ts | 8 +- src/headroom/supervisor.test.ts | 166 +++++++++++++++++++++++++++++++- src/headroom/supervisor.ts | 97 ++++++++++++++++--- src/realPorts.ts | 22 ++++- 12 files changed, 373 insertions(+), 76 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 658f8a7..f8b33d8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -103,6 +103,8 @@ When a launch resolves `headroom` on, the launcher (synchronous end to end, righ The supervisor is the only thing that starts, stops, restarts, or upgrades headroom. Its whole lifecycle (install via `uv tool install` when the binary is missing or its version fails the configured source, crash restarts with bounded exponential backoff and a retry budget, drift restarts deferred until the session registry is empty, idle shutdown) lives in `src/headroom/supervisor.ts` as one pure-ish loop over injected `SupervisorPorts`, so every decision is tested against a fake clock, filesystem, and process table. Coordination between separate OS processes is entirely file-based (state, lock, session files) with pid liveness as the source of truth, which is what lets a synchronous launcher, a detached supervisor, and several concurrent sessions cooperate without any of them holding a socket open to another. +Two facts about process death shape that liveness layer. First, the supervisor keeps every spawned headroom's `ChildProcess` handle and consumes its exit event: that event is both the crash signal and the reaping (a child nobody listens for is a child nobody reaps, and the loop's sleeps run on real timers precisely so the event loop can deliver it). Second, every liveness check anywhere in the coordination layer (crash detection, session pruning, state inspection, the identity lock's holder check) is zombie-aware via `ps`'s state column: a process that exited unreaped still answers `kill(pid, 0)` as alive, but holds no port and will never write state, so it must read as dead. Stops escalate SIGTERM to SIGKILL on a bounded grace, because the proxy has been observed to ignore SIGTERM outright. The supervisor handles SIGTERM and SIGINT explicitly and routes them through an orderly exit (Node skips `exit` handlers on unhandled signal death), so its own death takes its children with it; and a successor supervisor stops any orphan daemon a predecessor left running before starting its own, so nothing squatting on a port escapes supervision. + ### Resolver mechanics For each `~/.claude` entry, walk the cascade to a boolean decision. If the decision is uniform for an entire subtree, symlink that directory in one shot. If a deeper path override splits the decision, materialise that directory as a real local directory instead of a symlink and recurse, repeating the check at each level — only directories with an actual split ever get exploded. A conditional entries key is never eligible for the uniform-symlink shortcut, since its decision can only be evaluated per-file. diff --git a/docs/configuration-model.md b/docs/configuration-model.md index 73bffa3..3abcce1 100644 --- a/docs/configuration-model.md +++ b/docs/configuration-model.md @@ -229,7 +229,7 @@ The ambient-credential guard (below) checks the parent environment and is unaffe `launch.headroom: true` (in a configuration profile, the global config, a directory rule, or a committed `.claude-use.json`, resolved through the same cascade as every other launch flag) or a one-off `CLAUDE_USE_HEADROOM=1 claude` routes the whole session through a local [headroom](https://github.com/ExaDev/headroom) daemon instead of straight to the provider: the child's `ANTHROPIC_BASE_URL` becomes the daemon's loopback address, `HEADROOM_PROXY_URL` names it too, and `ANTHROPIC_CUSTOM_HEADERS` gains `x-headroom-project-id` (the git repository root of the working directory, or the directory itself outside a repository) plus, when a provider is also selected, `x-headroom-base-url` carrying the provider's real upstream so one daemon can serve several providers per request. -claude-use fully orchestrates the daemon; you never start, stop, or upgrade headroom by hand. The first launch that resolves headroom on spawns a detached supervisor (a background copy of the `claude-use` binary running a hidden internal subcommand), which installs headroom with `uv tool install` when the binary is missing or its version does not satisfy the configured source, starts `headroom proxy` on a free loopback port with `HEADROOM_ALLOWED_BASE_URLS` set to every provider's base URL plus `https://api.anthropic.com`, waits for its `/readyz` to answer, and only then records the port where launches can find it. A proxy that crashes is restarted with bounded exponential backoff; after five consecutive failures to become ready the supervisor records the error in its state and gives up, and the next launch fails loudly with the daemon log path rather than silently bypassing headroom. When the allowlist or install source drifts (a provider file changed, the configured source changed), the daemon is restarted only once no session is live, so a running session is never cut off; when no session has been live for `idleShutdownMinutes` (15 by default), the supervisor stops the daemon and exits, freeing its memory. +claude-use fully orchestrates the daemon; you never start, stop, or upgrade headroom by hand. The first launch that resolves headroom on spawns a detached supervisor (a background copy of the `claude-use` binary running a hidden internal subcommand), which installs headroom with `uv tool install` when the binary is missing or its version does not satisfy the configured source, starts `headroom proxy` on a free loopback port with `HEADROOM_ALLOWED_BASE_URLS` set to every provider's base URL plus `https://api.anthropic.com`, waits for its `/readyz` to answer, and only then records the port where launches can find it. A proxy that crashes is restarted with bounded exponential backoff; after five consecutive failures to become ready the supervisor records the error in its state and gives up, and the next launch fails loudly with the daemon log path rather than silently bypassing headroom. Crash detection is driven by the proxy's own exit event (which is also what reaps it), and every pid liveness check in the coordination layer is zombie-aware, because a process that died unreaped still answers `kill(pid, 0)` as alive while holding no port; a deliberate stop escalates SIGTERM to SIGKILL on a bounded timeout, since the proxy does not reliably die on SIGTERM alone. When the allowlist or install source drifts (a provider file changed, the configured source changed), the daemon is restarted only once no session is live, so a running session is never cut off; when no session has been live for `idleShutdownMinutes` (15 by default), the supervisor stops the daemon and exits, freeing its memory. Coordination lives under `~/.claude-use/headroom/`: `state.json` (supervisor pid, daemon pid, port, version, allowlist hash, last error), an exclusive-create start lock so concurrent launches start at most one supervisor, and `sessions/.json` files as the session registry, pruned automatically when a launcher pid is no longer alive. `claude-use headroom status` reports all of it read-only, and `claude-use doctor` includes the daemon in its audit. diff --git a/docs/testing.md b/docs/testing.md index 9c2662f..56fbe9d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -21,7 +21,7 @@ the resolver's cascade and materialisation logic is exactly the kind of thing th `identityManager.ts`, `configProfiles.ts`, `directoryRules.ts`, and `configure.ts` stay thin adapters over the resolver, so most of their correctness rides on the resolver's own test coverage above. The one exception is `listIdentities`, whose own tests cover a deliberate departure from the "throw a validation error and let it propagate" convention: an `identity.json` that is present but unreadable — malformed JSON, or valid JSON this version's `IdentitySchema` rejects — is reported as that one identity's own unreadable entry, so a single bad file never hides every *other* identity from `claude-use identity list` at the moment they most need to be visible. Only those two content-shaped failures are absorbed; a permission error still propagates. A wholly *absent* `identity.json` remains a silent skip rather than a problem, and both it and `doctor`'s own enumeration filter out directories whose name starts with `.`, since `IdentitySchema` requires an identity name to start with a letter or digit and a resync's own `..scratch.`/`..previous.` directories are therefore never identities to report on. `launcher.ts` carries three separately-testable responsibilities of its own that aren't covered by the resolver's purity, and need their own coverage: translating a resolved `Map` into real filesystem side effects (creating/removing symlinks, materialising/collapsing directories, diffing against the farm's prior state, the per-identity lock and atomic-swap behaviour from [Directory rules](configuration-model.md#directory-rules)) against a fake/in-memory filesystem; invoking the real `claude` binary via an injected `spawn` function (argv/env construction, exit-code propagation), never a real subprocess in a unit test; and the ambient-credential guard — given a fake `process.env`, refusing to proceed when any of the six named variables is set and the active identity's `allowAmbientCredential` is unset/false, proceeding when it's true, and proceeding when `CLAUDE_USE_ALLOW_AMBIENT_CREDENTIAL=1` is set for that one call regardless of the identity's own setting. Provider selection gets the same launcher-level coverage against a fake providers map: the child env gaining the provider's base URL, token, static `env` entries, and a cleared `ANTHROPIC_API_KEY`; an unknown provider refusing with the known names listed (exit 1); an unset token environment variable refusing with exit 64; a `--provider` flag beating a cascade-pinned provider; and the guard standing aside for a provider launch (claude-use supplies the child's credential itself) while still refusing an ambient `ANTHROPIC_AUTH_TOKEN` with no provider selected. `providers.test.ts` covers the manager functions (`addProvider`/`listProviders`/`removeProvider`/`loadProvider`) over real temp directories per `identityManager.test.ts`'s convention, plus `resolveProvider` itself as a pure decision over an injected `FsPort`. -Headroom routing keeps the same shape: no real process, port, clock, or HTTP call ever happens in a test. `headroom/ensure.test.ts` drives the launcher-side bring-up against a fake filesystem and scripted supervisor behaviour (first launch spawns the supervisor and waits for its ready state; a healthy daemon is reused without spawning; a dead supervisor pid is replaced; a live start lock held by another launcher is waited on while a dead holder's lock is taken over; a fatal `lastError` refuses immediately; a daemon that never appears times out naming the log path). `headroom/supervisor.test.ts` runs the whole supervisor loop over injected ports with a clock that only advances when the supervisor sleeps: install invoked when the binary is missing or the source changed and skipped when the installed version satisfies the source, the spawn allowlist containing every provider plus `api.anthropic.com`, crash restart while a session is live, the backoff schedule and retry budget ending in a recorded `lastError`, drift restarts deferred until the session registry empties, idle shutdown timing (and its reset while sessions are live), and dead-pid session pruning. `headroom/headers.test.ts` covers the `ANTHROPIC_CUSTOM_HEADERS` merge, `headroom/state.test.ts` the state/registry/allowlist primitives, and `doctor.test.ts` gains the headroom audit cases (never run, malformed state, healthy pids, dead supervisor, a `lastError` warning alongside a running replacement). The launcher's own headroom tests inject a fake `HeadroomPort` to cover the env wiring with and without a provider, the loud refusal when headroom resolved on with no port wired, and that the port is never touched when headroom resolved off. +Headroom routing keeps the same shape: no real process, port, clock, or HTTP call ever happens in a test. `headroom/ensure.test.ts` drives the launcher-side bring-up against a fake filesystem and scripted supervisor behaviour (first launch spawns the supervisor and waits for its ready state; a healthy daemon is reused without spawning; a dead supervisor pid is replaced, including one that is an unreaped zombie signal 0 still reports alive; a live start lock held by another launcher is waited on while a dead holder's lock is taken over; a fatal `lastError` refuses immediately; a daemon that never appears times out naming the log path). `headroom/supervisor.test.ts` runs the whole supervisor loop over injected ports with a clock that only advances when the supervisor sleeps and a process model that keeps signal-0 existence, zombification, and the ChildProcess exit event distinct: install invoked when the binary is missing or the source changed and skipped when the installed version satisfies the source, the spawn allowlist containing every provider plus `api.anthropic.com`, crash restart driven by the exit event while a session is live, a zombie daemon treated as dead and restarted even though existence alone would still report it, the daemon fields cleared from state the moment a crash is detected, an orphan daemon left by a dead predecessor stopped before the successor starts its own, the backoff schedule and retry budget ending in a recorded `lastError`, drift restarts deferred until the session registry empties, idle shutdown timing (and its reset while sessions are live), and session pruning of dead and zombie launcher pids. `stopSupervisedProcess` gets its own table: never escalating when the process exits on SIGTERM, escalating to SIGKILL once the SIGTERM grace expires, reporting `still-running` when even SIGKILL does not clear it, and sending nothing further when the target is already gone. `headroom/headers.test.ts` covers the `ANTHROPIC_CUSTOM_HEADERS` merge, `headroom/state.test.ts` the state/registry/allowlist primitives, and `doctor.test.ts` gains the headroom audit cases (never run, malformed state, healthy pids, dead supervisor, a `lastError` warning alongside a running replacement). The launcher's own headroom tests inject a fake `HeadroomPort` to cover the env wiring with and without a provider, the loud refusal when headroom resolved on with no port wired, and that the port is never touched when headroom resolved off. `check.ts`'s three always-on diagnostics get their own tests too, independent of path/cascade resolution: the ambient-credential check against a fake `process.env` (same fixture as `launcher.ts`'s guard, since they share the same detection logic); the settings-secrets advisory against a fake settings.json with populated `env`/`hooks` fields, confirming it reports counts and key names only, never values; and — since Keychain access is real OS state, not something to fake — a manual/integration-only note that the Keychain-name lookup is exercised against a real `security` call in CI on macOS runners, not unit-tested with a mock. diff --git a/src/doctor.test.ts b/src/doctor.test.ts index 2aa3634..676299c 100644 --- a/src/doctor.test.ts +++ b/src/doctor.test.ts @@ -31,7 +31,7 @@ function baseParams(overrides: Partial = {}): RunDoctorParams { claudeShim: { state: undefined, targetExists: false }, pathResolution: { ownExecutablePath: "/home/u/.local/bin/claude-use", claudeUse: { status: "ok" } }, platform: "linux", - headroom: { state: { path: "/claude-use/headroom/state.json", raw: undefined }, isProcessAlive: () => false }, + headroom: { state: { path: "/claude-use/headroom/state.json", raw: undefined }, isRunning: () => false }, ...overrides, }; } @@ -69,7 +69,7 @@ describe("runDoctor: headroom", () => { }); it("fails on a malformed state.json instead of guessing", () => { - const report = runDoctor(baseParams({ headroom: { state: { path: "/claude-use/headroom/state.json", raw: "{bad" }, isProcessAlive: () => false } })); + const report = runDoctor(baseParams({ headroom: { state: { path: "/claude-use/headroom/state.json", raw: "{bad" }, isRunning: () => false } })); expect(findingsFor(report, "headroom").some((finding) => finding.severity === "fail")).toBe(true); }); @@ -79,7 +79,7 @@ describe("runDoctor: headroom", () => { baseParams({ headroom: { state: { path: "/claude-use/headroom/state.json", raw: JSON.stringify({ supervisorPid: ALIVE_SUPERVISOR_PID, headroomPid: ALIVE_DAEMON_PID, port: HEADROOM_PORT, version: "headroom 0.39.1" }) }, - isProcessAlive: (pid: number) => alive.has(pid), + isRunning: (pid: number) => alive.has(pid), }, }), ); @@ -93,7 +93,7 @@ describe("runDoctor: headroom", () => { baseParams({ headroom: { state: { path: "/claude-use/headroom/state.json", raw: JSON.stringify({ supervisorPid: ALIVE_SUPERVISOR_PID, headroomPid: ALIVE_DAEMON_PID, port: HEADROOM_PORT }) }, - isProcessAlive: () => false, + isRunning: () => false, }, }), ); @@ -111,7 +111,7 @@ describe("runDoctor: headroom", () => { path: "/claude-use/headroom/state.json", raw: JSON.stringify({ supervisorPid: REPLACEMENT_SUPERVISOR_PID, headroomPid: REPLACEMENT_DAEMON_PID, port: HEADROOM_PORT, lastError: "previous crash" }), }, - isProcessAlive: (pid: number) => alive.has(pid), + isRunning: (pid: number) => alive.has(pid), }, }), ); diff --git a/src/doctor.ts b/src/doctor.ts index ed1099d..7e98650 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -26,7 +26,7 @@ import { isIdentityDirectoryName } from "./identityManager"; import { detectAmbientCredential, formatAmbientCredentialGuardMessage } from "./launcher/guard"; import type { RunPort } from "./launcher/ports"; import type { LayoutPaths } from "./paths"; -import { findExecutableInDir, realFsPort, realIsProcessAlive, realOwnExecutablePath, realResolveClaudeBinary, realRunPort } from "./realPorts"; +import { findExecutableInDir, realFsPort, realIsProcessRunning, realOwnExecutablePath, realResolveClaudeBinary, realRunPort } from "./realPorts"; import { lineariseProfile, type ProfileLoader, type ProfileSource } from "./resolve/extends"; import type { DiscoveredClaudeBinary } from "./versionDiscovery"; @@ -132,10 +132,10 @@ export interface RunDoctorParams { readonly run?: RunPort; /** `process.platform` in real use; the Keychain check only ever runs when this is `"darwin"`. */ readonly platform: string; - /** The headroom daemon's state.json plus a liveness predicate for the pids it names. Omit the raw text when the daemon has never run; that is a pass, not a failure. */ + /** The headroom daemon's state.json plus a zombie-aware liveness predicate for the pids it names (a defunct daemon holds no port but still answers signal 0). Omit the raw text when the daemon has never run; that is a pass, not a failure. */ readonly headroom: { readonly state: DoctorFileInput; - readonly isProcessAlive: (pid: number) => boolean; + readonly isRunning: (pid: number) => boolean; }; } @@ -383,8 +383,8 @@ export function runDoctor(params: RunDoctorParams): DoctorReport { push("headroom", "fail", validated.message); } else { const state = validated.data; - const supervisorAlive = state.supervisorPid !== undefined && params.headroom.isProcessAlive(state.supervisorPid); - const headroomAlive = state.headroomPid !== undefined && params.headroom.isProcessAlive(state.headroomPid); + const supervisorAlive = state.supervisorPid !== undefined && params.headroom.isRunning(state.supervisorPid); + const headroomAlive = state.headroomPid !== undefined && params.headroom.isRunning(state.headroomPid); if (state.supervisorPid === undefined) { if (state.lastError === undefined) { push("headroom", "pass", "Headroom daemon is stopped (idle shutdown) with no recorded error."); @@ -603,7 +603,7 @@ export function registerDoctorCommand(program: Command, paths: LayoutPaths): voi }, run: realRunPort, platform: process.platform, - headroom: { state: { path: paths.headroomStateFile, raw: realFsPort.readFileUtf8(paths.headroomStateFile) }, isProcessAlive: realIsProcessAlive }, + headroom: { state: { path: paths.headroomStateFile, raw: realFsPort.readFileUtf8(paths.headroomStateFile) }, isRunning: realIsProcessRunning }, }); for (const line of formatDoctorReport(report)) { diff --git a/src/headroom/commands.ts b/src/headroom/commands.ts index bbec1bc..85631e0 100644 --- a/src/headroom/commands.ts +++ b/src/headroom/commands.ts @@ -1,11 +1,11 @@ import fs from "node:fs"; import net from "node:net"; -import { spawn, spawnSync } from "node:child_process"; +import { spawn, spawnSync, type ChildProcess } from "node:child_process"; import type { Command } from "commander"; import { readGlobalConfig } from "../configProfiles"; import type { LayoutPaths } from "../paths"; -import { realFarmFs, realIsProcessAlive, realSleepSync } from "../realPorts"; +import { realFarmFs, realIsProcessRunning, realSleepSync } from "../realPorts"; import { hashAllowlist, headroomAllowlist, @@ -16,7 +16,7 @@ import { type HeadroomSession, type HeadroomState, } from "./state"; -import { resolveSupervisorConfig, runSupervisor, type SupervisorPorts } from "./supervisor"; +import { resolveSupervisorConfig, runSupervisor, stopSupervisedProcess, type SupervisorPorts } from "./supervisor"; /** One session-registry entry plus whether its launcher pid is still running. */ interface HeadroomSessionStatus extends HeadroomSession { @@ -41,17 +41,17 @@ export interface HeadroomStatus { export function collectHeadroomStatus( fsPort: HeadroomFs, paths: LayoutPaths, - isProcessAlive: (pid: number) => boolean, + isRunning: (pid: number) => boolean, ): HeadroomStatus { const state = readHeadroomState(fsPort, paths.headroomStateFile) ?? {}; const allowlist = headroomAllowlist(readAllProviders(fsPort, paths.providersDir).map((entry) => entry.provider)); return { state, - supervisorAlive: state.supervisorPid !== undefined && isProcessAlive(state.supervisorPid), - headroomAlive: state.headroomPid !== undefined && isProcessAlive(state.headroomPid), + supervisorAlive: state.supervisorPid !== undefined && isRunning(state.supervisorPid), + headroomAlive: state.headroomPid !== undefined && isRunning(state.headroomPid), sessions: listSessions(fsPort, paths.headroomSessionsDir).map((session) => ({ ...session, - alive: isProcessAlive(session.pid), + alive: isRunning(session.pid), })), allowlist, allowlistDrifted: state.allowlistHash !== undefined && state.allowlistHash !== hashAllowlist(allowlist), @@ -115,15 +115,24 @@ async function realFreePort(): Promise { }); } -/** The pid of the headroom process this supervisor currently owns, so the exit hook below never orphans it. */ -let supervisedHeadroomPid: number | undefined; +/** + * The headroom processes this supervisor owns, by pid. Keeping the ChildProcess handles is what reaps the children: consuming the exit event is libuv's cue to waitpid, and a child nobody listens for is a child nobody reaps (the original zombie bug). The entry is removed on exit, so the map also names exactly what the exit hook below must not leave behind. + */ +const ownedHeadroom = new Map(); -/** How long a stopped daemon gets to exit on SIGTERM before the supervisor escalates to SIGKILL: enough to drain an in-flight request, not enough to stall the loop. */ -const STOP_GRACE_MS = 1500; +/** + * Pids whose exit event has arrived: authoritative deadness, known the moment the child dies rather than at the next liveness poll, and regardless of what the not-yet-reaped remains still look like in the process table. + */ +const exitedHeadroom = new Set(); /** Per-attempt timeout on the readiness probe: a loopback request either answers quickly or the attempt has failed. */ const READY_FETCH_TIMEOUT_MS = 2000; +/** Dead by exit event or by the zombie-aware table check, whichever says so first. */ +function headroomPidRunning(pid: number): boolean { + return !exitedHeadroom.has(pid) && realIsProcessRunning(pid); +} + /** The real `SupervisorPorts`: real processes, ports, clock, filesystem, and network. */ function realSupervisorPorts(paths: LayoutPaths): SupervisorPorts { return { @@ -131,8 +140,13 @@ function realSupervisorPorts(paths: LayoutPaths): SupervisorPorts { paths, ownPid: process.pid, now: () => Date.now(), - sleep: realSleepSync, - isProcessAlive: realIsProcessAlive, + // A real timer, not the launcher's blocking Atomics.wait: the supervisor lives on its event loop, and the child's exit event can only be delivered while the loop turns. + sleep: async (ms) => { + await new Promise((resolve) => { + setTimeout(resolve, ms); + }); + }, + isRunning: headroomPidRunning, freePort: realFreePort, spawnHeadroom: (port, allowlist) => { fs.mkdirSync(paths.logsDir, { recursive: true }); @@ -143,31 +157,36 @@ function realSupervisorPorts(paths: LayoutPaths): SupervisorPorts { stdio: ["ignore", logFd, logFd], env: { ...process.env, HEADROOM_ALLOWED_BASE_URLS: allowlist.join(",") }, }); - child.unref(); if (child.pid === undefined) { throw new Error("spawning headroom returned no pid"); } - supervisedHeadroomPid = child.pid; - return child.pid; + const pid = child.pid; + // Deliberately NOT unref'd: this supervisor needs the child's exit event (both the reaping and the crash signal), and a handle taken off the event loop stops delivering it. `detached: true` keeps headroom out of this process's process group so a supervisor crash does not signal it; the exit hook below still kills it on the orderly exit paths. + ownedHeadroom.set(pid, child); + child.once("exit", (code, signal) => { + ownedHeadroom.delete(pid); + exitedHeadroom.add(pid); + fs.appendFileSync(paths.headroomLogPath, `${new Date().toISOString()} claude-use headroom supervisor: headroom exited (${signal ?? `code ${String(code)}`})\n`); + }); + return pid; } finally { fs.closeSync(logFd); } }, stopProcess: (pid) => { - try { - process.kill(pid, "SIGTERM"); - } catch { - // Already dead: the supervisor only ever stops pids it believes are running. - } - // Grace period, then force: a proxy mid-request deserves a chance to drain, but the supervisor must not wait on it forever. - realSleepSync(STOP_GRACE_MS); - try { - process.kill(pid, "SIGKILL"); - } catch { - // It exited after SIGTERM, which is the good outcome. - } - if (supervisedHeadroomPid === pid) { - supervisedHeadroomPid = undefined; + const outcome = stopSupervisedProcess(pid, { + signal: (target, signal) => { + process.kill(target, signal); + }, + isRunning: headroomPidRunning, + waitMs: realSleepSync, + now: () => Date.now(), + }); + if (outcome === "still-running") { + fs.appendFileSync( + paths.headroomLogPath, + `${new Date().toISOString()} claude-use headroom supervisor: pid ${String(pid)} survived SIGKILL within its grace; it is stuck uninterruptibly\n`, + ); } }, ready: async (port) => { @@ -210,7 +229,7 @@ export function registerHeadroomCommand(program: Command, paths: LayoutPaths): v .command("status") .description("Report the headroom daemon's supervisor, process, port, sessions, and last error. Read-only.") .action(() => { - for (const line of formatHeadroomStatus(collectHeadroomStatus(realFarmFs, paths, realIsProcessAlive))) { + for (const line of formatHeadroomStatus(collectHeadroomStatus(realFarmFs, paths, realIsProcessRunning))) { console.log(line); } }); @@ -222,16 +241,23 @@ export function registerHeadroomCommand(program: Command, paths: LayoutPaths): v .action(async () => { const globalConfig = readGlobalConfig(paths); const config = resolveSupervisorConfig(globalConfig?.headroom ?? {}); - // If this supervisor dies without reaching its own shutdown path, take headroom with it rather than leaving an unsupervised daemon behind. + // If this supervisor exits without reaching its own shutdown path (idle or fatal), take every daemon it still owns with it rather than leaving an unsupervised proxy behind. SIGKILL, not the escalating stop: an exit hook is synchronous and already out of time. process.on("exit", () => { - if (supervisedHeadroomPid !== undefined) { + for (const pid of ownedHeadroom.keys()) { try { - process.kill(supervisedHeadroomPid, "SIGTERM"); + process.kill(pid, "SIGKILL"); } catch { // Already gone. } } }); + // Signal death skips `exit` handlers entirely unless the signal itself is handled, so an unhandled SIGTERM would leave the daemon orphaned. Routing both signals through an orderly exit is what makes the hook above run for them. + process.on("SIGTERM", () => { + process.exit(0); + }); + process.on("SIGINT", () => { + process.exit(0); + }); const code = await runSupervisor(config, realSupervisorPorts(paths)); process.exit(code); }); diff --git a/src/headroom/ensure.test.ts b/src/headroom/ensure.test.ts index 46cc829..c85dd92 100644 --- a/src/headroom/ensure.test.ts +++ b/src/headroom/ensure.test.ts @@ -13,6 +13,8 @@ const SLEEPS_BEFORE_RECOVERY = 3; const SLEEPS_BEFORE_OTHER_SUPERVENDOR_READY = 2; const HEADROOM_PID = 501; const PORT = 8123; +/** A supervisor pid that is dead in every test that names it, distinct from the live fake's SUPERVISOR_PID. */ +const DEAD_SUPERVISOR_PID = 999; /** * The fake world `ensureHeadroom` runs against: a fake filesystem, a clock that only advances when the code sleeps, a live-pid set, and a `spawnSupervisor` that records itself and can simulate the freshly spawned supervisor writing a ready state (immediately, or lazily on a later poll via `onSleep`). @@ -22,11 +24,13 @@ function makeWorld(options: { readonly spawnWritesReadyState?: boolean } = {}) { let clock = 0; let onSleep: (() => void) | undefined; const alive = new Set([SUPERVISOR_PID, HEADROOM_PID, process.pid]); + const zombies = new Set(); const spawns: number[] = []; const world = { fs, alive, + zombies, spawns, set onSleep(hook: (() => void) | undefined) { onSleep = hook; @@ -41,7 +45,7 @@ function makeWorld(options: { readonly spawnWritesReadyState?: boolean } = {}) { }, ports: { fs, - isProcessAlive: (pid: number) => alive.has(pid), + isRunning: (pid: number) => alive.has(pid) && !zombies.has(pid), now: () => clock, sleep: (ms: number) => { clock += ms; @@ -82,7 +86,7 @@ describe("ensureHeadroom", () => { it("spawns a replacement supervisor when the recorded one is dead", () => { const world = makeWorld(); writeHeadroomState(world.fs, paths.headroomStateFile, { - supervisorPid: 999, + supervisorPid: DEAD_SUPERVISOR_PID, headroomPid: HEADROOM_PID, port: PORT, }); @@ -91,11 +95,26 @@ describe("ensureHeadroom", () => { expect(world.spawns).toHaveLength(1); }); + it("spawns a replacement supervisor when the recorded one is an unreaped zombie that signal 0 still reports alive", () => { + const world = makeWorld(); + writeHeadroomState(world.fs, paths.headroomStateFile, { + supervisorPid: DEAD_SUPERVISOR_PID, + headroomPid: HEADROOM_PID, + port: PORT, + }); + // The supervisor died without being reaped: it still "exists" in the table, but nothing is running there. + world.alive.add(DEAD_SUPERVISOR_PID); + world.zombies.add(DEAD_SUPERVISOR_PID); + const result = ensureHeadroom({ paths, launcherPid: 51, ports: world.ports }); + expect(result).toEqual({ port: PORT }); + expect(world.spawns).toHaveLength(1); + }); + it("keeps waiting while a live supervisor restarts a dead daemon, and returns once it is back", () => { const world = makeWorld({ spawnWritesReadyState: false }); writeHeadroomState(world.fs, paths.headroomStateFile, { supervisorPid: SUPERVISOR_PID, - headroomPid: 999, + headroomPid: DEAD_SUPERVISOR_PID, port: PORT, }); // The supervisor brings the daemon back partway through the wait. diff --git a/src/headroom/ensure.ts b/src/headroom/ensure.ts index 5418b66..bbe8793 100644 --- a/src/headroom/ensure.ts +++ b/src/headroom/ensure.ts @@ -27,7 +27,8 @@ export class HeadroomStartError extends CliError { /** Every effect the ensure step performs, injected so it runs against fakes in tests. */ export interface EnsureHeadroomPorts { readonly fs: HeadroomFs; - readonly isProcessAlive: (pid: number) => boolean; + /** Zombie-aware liveness: a supervisor or daemon that exited without being reaped still answers signal 0 as alive, but will never serve a request or write state, so it must read as dead here (see `realIsProcessRunning`). */ + readonly isRunning: (pid: number) => boolean; readonly now: () => number; readonly sleep: (ms: number) => void; /** Spawns the detached supervisor process that owns the headroom daemon, returning its pid. */ @@ -52,11 +53,11 @@ export function ensureHeadroom(params: { for (;;) { const state = readHeadroomState(ports.fs, paths.headroomStateFile); - if (state?.supervisorPid !== undefined && ports.isProcessAlive(state.supervisorPid)) { + if (state?.supervisorPid !== undefined && ports.isRunning(state.supervisorPid)) { const daemonUp = state.port !== undefined && state.headroomPid !== undefined && - ports.isProcessAlive(state.headroomPid); + ports.isRunning(state.headroomPid); if (daemonUp && state.port !== undefined) { writeSession(ports.fs, paths.headroomSessionsDir, { pid: params.launcherPid, startedAt: ports.now() }); return { port: state.port }; @@ -78,7 +79,7 @@ export function ensureHeadroom(params: { spawnedSupervisor = true; } else { const lock = readStartLock(ports.fs, paths.headroomLockFile); - if (lock === undefined || !ports.isProcessAlive(lock.pid)) { + if (lock === undefined || !ports.isRunning(lock.pid)) { // A dead holder's lock is litter from a launcher that died before the supervisor it spawned could clear it. ports.fs.removeRecursive(paths.headroomLockFile); continue; diff --git a/src/headroom/state.ts b/src/headroom/state.ts index 9b20a4d..3aeb75c 100644 --- a/src/headroom/state.ts +++ b/src/headroom/state.ts @@ -140,15 +140,17 @@ export function listSessions(fs: HeadroomFs, sessionsDir: string): readonly Head return sessions.sort((a, b) => a.pid - b.pid); } -/** Removes every registry entry whose pid is no longer alive: a launcher that died without releasing its entry must not keep the daemon awake or block a drift restart forever. Returns the pids removed. */ +/** + * Removes every registry entry whose pid is no longer running: a launcher that died without releasing its entry must not keep the daemon awake or block a drift restart forever. The predicate must be zombie-aware (see `realIsProcessRunning`): a launcher that exited but was never reaped still answers signal 0 as alive, which would keep its session registered indefinitely. + */ export function pruneDeadSessions( fs: HeadroomFs, sessionsDir: string, - isProcessAlive: (pid: number) => boolean, + isRunning: (pid: number) => boolean, ): readonly number[] { const removed: number[] = []; for (const session of listSessions(fs, sessionsDir)) { - if (!isProcessAlive(session.pid)) { + if (!isRunning(session.pid)) { removeSession(fs, sessionsDir, session.pid); removed.push(session.pid); } diff --git a/src/headroom/supervisor.test.ts b/src/headroom/supervisor.test.ts index 986ae01..0a66def 100644 --- a/src/headroom/supervisor.test.ts +++ b/src/headroom/supervisor.test.ts @@ -10,8 +10,11 @@ import { HEADROOM_BACKOFF_CAP_MS, HEADROOM_POLL_MS, HEADROOM_START_RETRY_BUDGET, + KILL_GRACE_MS, resolveSupervisorConfig, runSupervisor, + stopSupervisedProcess, + TERM_GRACE_MS, versionSatisfies, type SupervisorPorts, } from "./supervisor"; @@ -20,6 +23,9 @@ import { hashAllowlist, headroomAllowlist, writeHeadroomState, writeSession } fr const paths = buildLayoutPaths("/home/testuser/.claude-use"); const SESSION_PID = 321; +const ZOMBIE_SESSION_PID = 322; +const ORPHAN_DAEMON_PID = 888; +const ORPHAN_DAEMON_PORT = 4321; const OWN_PID = 4242; const TICKS_INSTALL_TEST = 3; const TICKS_SHORT = 2; @@ -37,7 +43,9 @@ const ATTEMPT_FOUR = 4; const DRIFT_DONE_PHASE = 3; /** - * The fake world `runSupervisor` runs against: a fake filesystem seeded with providers, a clock advanced only by the supervisor's own sleeps, a live-pid set, and controllable readiness, install, and version results. `onSleep` lets a test script the outside world (a provider file appearing, a session ending) between ticks. + * The fake world `runSupervisor` runs against: a fake filesystem seeded with providers, a clock advanced only by the supervisor's own sleeps, a modelled process table, and controllable readiness, install, and version results. `onSleep` lets a test script the outside world (a provider file appearing, a session ending) between ticks. + * + * The process model distinguishes exactly what the real one does: `alive` is signal-0-style existence, `zombies` holds pids that exited without being reaped (still "existing", never running), and `kill` models the ChildProcess exit event (death observed and the child reaped, the way the real supervisor's exit listener does). */ function makeWorld(seededProviders: readonly { name: string; baseUrl: string }[] = []) { const fs = createFakeFarmFs({}); @@ -57,6 +65,7 @@ function makeWorld(seededProviders: readonly { name: string; baseUrl: string }[] let version: string | undefined = "headroom 0.39.1"; let installOk = true; const alive = new Set([process.pid]); + const zombies = new Set(); const spawns: { pid: number; port: number; allowlist: readonly string[] }[] = []; const stops: number[] = []; const installs: string[] = []; @@ -72,6 +81,7 @@ async function settled(value: T): Promise { const world = { fs, alive, + zombies, spawns, stops, installs, @@ -90,8 +100,14 @@ const world = { set installOk(value: boolean) { installOk = value; }, + /** The ChildProcess exit event arriving: death observed, child reaped, no zombie remains. */ kill(pid: number): void { alive.delete(pid); + zombies.delete(pid); + }, + /** The observed macOS failure mode: the process died but nothing reaped it, so signal 0 still answers while nothing is running. */ + zombify(pid: number): void { + zombies.add(pid); }, writeSessionFile(pid: number): void { alive.add(pid); @@ -102,14 +118,15 @@ const world = { paths, ownPid: OWN_PID, now: () => clock, - sleep: (ms: number) => { + sleep: async (ms: number) => { clock += ms; sleepDelays.push(ms); if (onSleep !== undefined) { onSleep(); } + await settled(undefined); }, - isProcessAlive: (pid: number) => alive.has(pid), + isRunning: (pid: number) => alive.has(pid) && !zombies.has(pid), freePort: async () => { nextPort += 1; return await settled(nextPort); @@ -117,6 +134,7 @@ const world = { spawnHeadroom: (port: number, allowlist: readonly string[]) => { nextPid += 1; alive.add(nextPid); + zombies.delete(nextPid); spawns.push({ pid: nextPid, port, allowlist: [...allowlist] }); if (autoReady) { readyPorts.add(port); @@ -235,7 +253,7 @@ describe("runSupervisor", () => { expect(String(state.lastError)).toContain("could not install headroom"); }); - it("restarts a daemon that crashed while sessions were live", async () => { + it("restarts a daemon whose exit event reports the crash while sessions were live", async () => { const world = makeWorld(); let crashed = false; world.onSleep = () => { @@ -253,6 +271,68 @@ describe("runSupervisor", () => { expect(world.spawns).toHaveLength(2); }); + it("treats a zombie daemon as dead and restarts it even though a signal-0 existence check would still report it alive", async () => { + const world = makeWorld(); + let zombified = false; + world.onSleep = () => { + if (!zombified && world.spawns.length === 1) { + const first = world.spawns[0]; + if (first !== undefined) { + world.zombify(first.pid); + zombified = true; + } + } + }; + const code = await runSupervisor({ source: config.source, idleShutdownMinutes: IDLE_NEVER_MINUTES }, world.ports, { tickLimit: TICKS_CRASH_RESTART }); + expect(code).toBe(HEADROOM_SUPERVISOR_STILL_RUNNING); + const first = world.spawns[0]; + if (first === undefined) { + throw new Error("expected a first spawn"); + } + // The zombie still "exists" in the modelled table, which is exactly what made the signal-0 check miss it. + expect(world.alive.has(first.pid)).toBe(true); + expect(world.zombies.has(first.pid)).toBe(true); + expect(world.spawns).toHaveLength(2); + }); + + it("clears the daemon fields from state the moment a crash is detected, so status never claims a dead port", async () => { + const world = makeWorld(); + let zombified = false; + world.onSleep = () => { + if (!zombified && world.spawns.length === 1) { + const first = world.spawns[0]; + if (first !== undefined) { + // The replacement never becomes ready, so the run stops in the crashed-and-restarting window where the cleared state is observable. + world.autoReady = false; + world.zombify(first.pid); + zombified = true; + } + } + }; + await runSupervisor({ source: config.source, idleShutdownMinutes: IDLE_NEVER_MINUTES }, world.ports, { tickLimit: TICKS_SHORT }); + const state = JSON.parse(world.fs.readFileUtf8(paths.headroomStateFile) ?? "{}") as Record; + expect(state.port).toBeUndefined(); + expect(state.headroomPid).toBeUndefined(); + expect(state.supervisorPid).toBe(OWN_PID); + expect(world.spawns.length).toBeGreaterThanOrEqual(2); + }); + + it("stops an orphan daemon left behind by a dead predecessor before starting its own", async () => { + const world = makeWorld(); + world.alive.add(ORPHAN_DAEMON_PID); + writeHeadroomState(world.fs, paths.headroomStateFile, { + supervisorPid: 1, + headroomPid: ORPHAN_DAEMON_PID, + port: ORPHAN_DAEMON_PORT, + installedSource: HEADROOM_DEFAULT_SOURCE, + }); + const code = await runSupervisor(config, world.ports, { tickLimit: TICKS_SHORT }); + expect(code).toBe(HEADROOM_SUPERVISOR_STILL_RUNNING); + expect(world.stops).toContain(ORPHAN_DAEMON_PID); + expect(world.spawns).toHaveLength(1); + expect(world.alive.has(ORPHAN_DAEMON_PID)).toBe(false); + }); + it("gives up after the retry budget when the daemon never becomes ready, recording lastError", async () => { const world = makeWorld(); world.autoReady = false; @@ -332,12 +412,15 @@ describe("runSupervisor", () => { expect(world.stops).toHaveLength(0); }); - it("prunes registry entries whose launcher pid has died", async () => { + it("prunes registry entries whose launcher pid has died, including a zombie the signal-0 table still lists", async () => { const world = makeWorld(); world.writeSessionFile(SESSION_PID); world.alive.delete(SESSION_PID); + world.writeSessionFile(ZOMBIE_SESSION_PID); + world.zombify(ZOMBIE_SESSION_PID); await runSupervisor({ source: config.source, idleShutdownMinutes: IDLE_NEVER_MINUTES }, world.ports, { tickLimit: TICKS_INSTALL_TEST }); expect(world.fs.readFileUtf8(`${paths.headroomSessionsDir}/${String(SESSION_PID)}.json`)).toBeUndefined(); + expect(world.fs.readFileUtf8(`${paths.headroomSessionsDir}/${String(ZOMBIE_SESSION_PID)}.json`)).toBeUndefined(); }); it("reinstalls when the configured source changed since the last install", async () => { @@ -350,3 +433,76 @@ describe("runSupervisor", () => { expect(world.installs).toEqual(["headroom==0.39.0"]); }); }); + +describe("stopSupervisedProcess", () => { + const STOP_PID = 900; + + /** How the modelled target responds: exiting on SIGTERM, ignoring SIGTERM until killed, surviving even SIGKILL, or being gone before the first signal. */ + type StopBehaviour = "dies-on-term" | "ignores-term" | "unkillable" | "already-gone"; + + function stopWorld(behaviour: StopBehaviour) { + const signals: NodeJS.Signals[] = []; + let clock = 0; + return { + signals, + elapsed: () => clock, + primitives: { + signal: (pid: number, signal: NodeJS.Signals) => { + signals.push(signal); + if (behaviour === "already-gone") { + throw new Error(`kill(${String(pid)}) failed: no such process`); + } + }, + isRunning: () => { + if (behaviour === "already-gone") { + return false; + } + if (behaviour === "dies-on-term") { + return !signals.includes("SIGTERM"); + } + if (behaviour === "ignores-term") { + return !signals.includes("SIGKILL"); + } + return true; + }, + waitMs: (ms: number) => { + clock += ms; + }, + now: () => clock, + }, + }; + } + + it("never escalates when the process exits on SIGTERM", () => { + const world = stopWorld("dies-on-term"); + expect(stopSupervisedProcess(STOP_PID, world.primitives)).toBe("exited-on-term"); + expect(world.signals).toEqual(["SIGTERM"]); + }); + + it("escalates to SIGKILL once the SIGTERM grace expires, because this proxy ignores SIGTERM", () => { + const world = stopWorld("ignores-term"); + expect(stopSupervisedProcess(STOP_PID, world.primitives)).toBe("killed"); + expect(world.signals).toEqual(["SIGTERM", "SIGKILL"]); + expect(world.elapsed()).toBeGreaterThanOrEqual(TERM_GRACE_MS); + }); + + it("reports still-running when even SIGKILL does not clear the process within its grace", () => { + const world = stopWorld("unkillable"); + expect(stopSupervisedProcess(STOP_PID, world.primitives)).toBe("still-running"); + expect(world.signals).toEqual(["SIGTERM", "SIGKILL"]); + expect(world.elapsed()).toBeGreaterThanOrEqual(TERM_GRACE_MS + KILL_GRACE_MS); + }); + + it("sends nothing further when the process is already gone", () => { + const world = stopWorld("already-gone"); + expect(stopSupervisedProcess(STOP_PID, world.primitives)).toBe("exited-on-term"); + expect(world.signals).toEqual(["SIGTERM"]); + }); + + it("isRunning is consulted with the zombie-aware notion: a defunct target reads as gone", () => { + // The dies-on-term model above exercises the happy path; this pins the contract the real implementation relies on: isRunning false means no escalation, whatever signal-0 would say. + const world = stopWorld("already-gone"); + expect(stopSupervisedProcess(STOP_PID, world.primitives)).toBe("exited-on-term"); + expect(world.signals).not.toContain("SIGKILL"); + }); +}); diff --git a/src/headroom/supervisor.ts b/src/headroom/supervisor.ts index b696447..de0be70 100644 --- a/src/headroom/supervisor.ts +++ b/src/headroom/supervisor.ts @@ -35,14 +35,21 @@ export interface SupervisorPorts { readonly paths: LayoutPaths; readonly ownPid: number; readonly now: () => number; - /** Blocks for `ms`; the real implementation is the same synchronous Atomics.wait sleep the launcher uses. */ - readonly sleep: (ms: number) => void; - readonly isProcessAlive: (pid: number) => boolean; + /** + * Waits `ms`, yielding to the event loop while it does. Deliberately NOT a synchronous Atomics.wait-style sleep: the real supervisor learns of its child's death through the ChildProcess exit event, and a blocking sleep would stop the event loop from ever delivering it (the exact failure that left an unreaped zombie answering liveness checks as alive). + */ + readonly sleep: (ms: number) => Promise; + /** + * Zombie-aware liveness, and for a daemon this supervisor spawned, exit-event-backed: the implementation keeps the spawned ChildProcess handle, consumes its exit event (which is also what reaps it), and reports the pid as not running from that moment. A bare signal-0 check is not enough, because a defunct process still answers it. + */ + readonly isRunning: (pid: number) => boolean; /** Returns a free loopback port. Async because the only reliable way to reserve one is to bind and release a socket. */ readonly freePort: () => Promise; - /** Starts `headroom proxy` bound to `port` with the given allowlist, returning its pid. Output goes to the daemon log. */ + /** + * Starts `headroom proxy` bound to `port` with the given allowlist, returning its pid. Output goes to the daemon log. The implementation must keep the ChildProcess handle and attach an exit listener (detaching the process is fine; unref'ing a child you still need events from is not, since without the listener nothing reaps it and it lingers as a zombie). + */ readonly spawnHeadroom: (port: number, allowlist: readonly string[]) => number; - /** Stops a process the supervisor owns, escalating as needed. */ + /** Stops a process the supervisor owns, escalating SIGTERM to SIGKILL on a bounded timeout (see `stopSupervisedProcess`). */ readonly stopProcess: (pid: number) => void; /** True once `GET /readyz` on the port succeeds. */ readonly ready: (port: number) => Promise; @@ -67,6 +74,62 @@ export const HEADROOM_POLL_MS = 1_000; /** Milliseconds per minute, so the idle shutdown threshold reads as the config field it derives from. */ const MS_PER_MINUTE = 60_000; +/** The process-control primitives the escalating stop needs, injected so the escalation itself is unit-testable against a process that ignores SIGTERM. */ +export interface StopProcessPrimitives { + /** Delivers a signal; throws when the target is already gone. */ + readonly signal: (pid: number, signal: NodeJS.Signals) => void; + /** Zombie-aware liveness: a defunct process is not running. */ + readonly isRunning: (pid: number) => boolean; + /** Synchronous wait between liveness polls; blocking is correct here, since nothing productive can happen until the target is gone or the grace expires. */ + readonly waitMs: (ms: number) => void; + readonly now: () => number; +} + +/** How `stopSupervisedProcess` ended. `still-running` means even SIGKILL did not clear the pid within its grace (an uninterruptible process); callers log it loudly. */ +export type StopOutcome = "exited-on-term" | "killed" | "still-running"; + +/** Poll interval while waiting for a stopped process to disappear: often enough to escalate quickly, rarely enough not to spin. */ +const STOP_POLL_MS = 100; +/** Grace after SIGTERM before escalating to SIGKILL: enough for a proxy to drain an in-flight request, short enough that a restart is not visibly stalled. This proxy's server has been observed to ignore SIGTERM outright, so the escalation is not optional. */ +export const TERM_GRACE_MS = 1500; +/** Grace after SIGKILL before declaring the process unkillable. SIGKILL is immediate at the kernel level; this only bounds the wait for the scheduler to reap it. */ +export const KILL_GRACE_MS = 1500; + +/** + * Stops a process the supervisor owns: SIGTERM, wait up to `TERM_GRACE_MS`; SIGKILL, wait up to `KILL_GRACE_MS`; then report. Liveness during the waits must be zombie-aware: an unreaped target still answers signal 0, which would read as "survived SIGKILL". + */ +export function stopSupervisedProcess(pid: number, primitives: StopProcessPrimitives): StopOutcome { + const send = (signal: NodeJS.Signals): void => { + try { + primitives.signal(pid, signal); + } catch { + // Already gone: the stop's job is done, whichever signal was about to be sent. + } + }; + + send("SIGTERM"); + if (awaitExit(pid, primitives, TERM_GRACE_MS)) { + return "exited-on-term"; + } + send("SIGKILL"); + if (awaitExit(pid, primitives, KILL_GRACE_MS)) { + return "killed"; + } + return "still-running"; +} + +/** Polls `isRunning` until the pid is gone or `budgetMs` elapses. */ +function awaitExit(pid: number, primitives: StopProcessPrimitives, budgetMs: number): boolean { + const deadline = primitives.now() + budgetMs; + while (primitives.isRunning(pid)) { + if (primitives.now() >= deadline) { + return false; + } + primitives.waitMs(STOP_POLL_MS); + } + return true; +} + /** The backoff for attempt `attempt` (1-based): base doubled per failure, capped. */ export function backoffForAttempt(attempt: number): number { return Math.min(HEADROOM_BACKOFF_CAP_MS, HEADROOM_BACKOFF_BASE_MS * 2 ** (attempt - 1)); @@ -144,7 +207,7 @@ export const HEADROOM_SUPERVISOR_STILL_RUNNING = -1; /** * Runs the headroom supervisor loop: install, start, keep alive, restart on crash, restart on drift once no session would be cut off, and shut down after the configured idle period with an empty session registry. Returns the process exit code (0 for an idle shutdown, 1 for a fatal setup failure). * - * Every effect flows through `ports`, so the whole loop is unit-testable with a fake clock, filesystem, and processes. The function is `async` only for the readiness probe; nothing else awaits. + * Every effect flows through `ports`, so the whole loop is unit-testable with a fake clock, filesystem, and processes. The loop awaits its sleeps, which in the real implementation run on a timer: between ticks the event loop turns, the spawned child's exit event is delivered (and the child thereby reaped), and the next tick's liveness check reads the truth. */ export async function runSupervisor( config: HeadroomSupervisorConfig, @@ -160,7 +223,8 @@ export async function runSupervisor( return 1; }; - let installedSource = readHeadroomState(fs, paths.headroomStateFile)?.installedSource; + const previousState = readHeadroomState(fs, paths.headroomStateFile); + let installedSource = previousState?.installedSource; /** * Installs headroom when the binary is missing, its version fails the configured source's specifier, or the configured source changed since the last install this supervisor performed. An absent `installedSource` with a satisfying binary does NOT install: a user's own `uv tool install headroom` is a legitimate install to respect. @@ -191,6 +255,13 @@ export async function runSupervisor( writeHeadroomState(fs, paths.headroomStateFile, { supervisorPid: ports.ownPid, version, installedSource }); ports.log(`claude-use headroom supervisor ${String(ports.ownPid)}: managing headroom on allowlist [${allowlistOf(ports).join(", ")}]`); + const orphanPid = previousState?.headroomPid; + if (orphanPid !== undefined && ports.isRunning(orphanPid)) { + // A predecessor's daemon that outlived it (its supervisor died by a signal nothing could intercept, say) would keep squatting on its port forever: nothing supervises it, nothing idles it out. Take it over before starting our own. + ports.log(`claude-use headroom supervisor: stopping daemon pid ${String(orphanPid)} left behind by the previous supervisor`); + ports.stopProcess(orphanPid); + } + let headroomPid: number | undefined; let runningHash: string | undefined; let consecutiveFailures = 0; @@ -205,14 +276,16 @@ export async function runSupervisor( // Recomputed every tick: provider files can change on disk at any moment, and the allowlist is the daemon's whole security posture. const allowlist = allowlistOf(ports); const allowlistHash = hashAllowlist(allowlist); - pruneDeadSessions(fs, paths.headroomSessionsDir, ports.isProcessAlive); + pruneDeadSessions(fs, paths.headroomSessionsDir, ports.isRunning); - const crashed = headroomPid !== undefined && !ports.isProcessAlive(headroomPid); + const crashed = headroomPid !== undefined && !ports.isRunning(headroomPid); if (headroomPid === undefined || crashed) { if (crashed) { ports.log(`claude-use headroom supervisor: headroom pid ${String(headroomPid)} died`); headroomPid = undefined; runningHash = undefined; + // Clear the daemon fields immediately: until the replacement is ready, state must not claim a serving port for a process that just died, or `headroom status` and waiting launchers read a healthy daemon that no longer exists. + writeHeadroomState(fs, paths.headroomStateFile, { supervisorPid: ports.ownPid, version, installedSource }); } if (consecutiveFailures >= HEADROOM_START_RETRY_BUDGET) { return fail(`headroom failed to become ready ${String(consecutiveFailures)} times in a row; giving up`); @@ -247,7 +320,7 @@ export async function runSupervisor( `claude-use headroom supervisor: headroom did not become ready on port ${String(port)} ` + `(attempt ${String(consecutiveFailures)} of ${String(HEADROOM_START_RETRY_BUDGET)})`, ); - ports.sleep(backoffForAttempt(consecutiveFailures)); + await ports.sleep(backoffForAttempt(consecutiveFailures)); continue; } } else { @@ -279,7 +352,7 @@ export async function runSupervisor( } } - ports.sleep(HEADROOM_POLL_MS); + await ports.sleep(HEADROOM_POLL_MS); } } @@ -298,6 +371,6 @@ async function waitUntilReady(ports: SupervisorPorts, port: number): Promise= deadline) { return false; } - ports.sleep(HEADROOM_POLL_MS); + await ports.sleep(HEADROOM_POLL_MS); } } diff --git a/src/realPorts.ts b/src/realPorts.ts index 3351b38..564feae 100644 --- a/src/realPorts.ts +++ b/src/realPorts.ts @@ -142,7 +142,7 @@ export function realSleepSync(ms: number): void { } /** Whether a process is still running. Signal 0 performs the permission and existence checks without delivering anything; `EPERM` means the process exists but belongs to another user. */ -export function realIsProcessAlive(pid: number): boolean { +function realIsProcessAlive(pid: number): boolean { try { process.kill(pid, 0); return true; @@ -151,6 +151,23 @@ export function realIsProcessAlive(pid: number): boolean { } } +/** + * Whether `pid` is a zombie: a process that has exited but whose parent has not reaped it. Read from `ps`'s process-state column, the one source that distinguishes "exited, awaiting reap" from "running": a defunct process still answers `kill(pid, 0)` (so `realIsProcessAlive` alone cannot see the difference), while `ps -o stat=` reports `Z` for exactly that state. + * + * When `ps` is unavailable (no POSIX userland, i.e. Windows) the answer is "not a zombie", falling back to plain signal-0 semantics rather than guessing every process dead. + */ +function realIsProcessZombie(pid: number): boolean { + const result = spawnSync("ps", ["-o", "stat=", "-p", String(pid)], { encoding: "utf8" }); + return result.status === 0 && result.stdout.trim().startsWith("Z"); +} + +/** + * Whether `pid` is a live, schedulable process: signal-0 alive AND not a zombie. This is the liveness notion every headroom coordination decision must use, because a defunct daemon holds no port and a defunct supervisor will never write state, yet both still "exist" as far as signal 0 is concerned. + */ +export function realIsProcessRunning(pid: number): boolean { + return realIsProcessAlive(pid) && !realIsProcessZombie(pid); +} + /** The real `RunPort`, used for auxiliary commands whose output this process needs to read — git branch detection for `when: { branch }` conditions, and `check.ts`'s macOS Keychain lookup. */ export const realRunPort: RunPort = { run(command, args) { @@ -220,7 +237,8 @@ export function realHeadroomPort(paths: LayoutPaths): HeadroomPort { launcherPid: process.pid, ports: { fs: realFarmFs, - isProcessAlive: realIsProcessAlive, + // Zombie-aware on purpose: a supervisor that died while still a child of this launcher sits unreaped until the launcher itself exits, and a defunct supervisor answering signal 0 as alive would stretch every launch to the full start timeout. + isRunning: realIsProcessRunning, now: () => Date.now(), sleep: realSleepSync, spawnSupervisor: spawnHeadroomSupervisor, From cfc1d318f08e712305fafb7373dee8fee8adb045 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 27 Sep 2026 02:25:47 +0100 Subject: [PATCH 2/2] fix: treat a zombie identity-lock holder as dead The per-identity resync lock judged its holder with kill(pid, 0), which an unreaped launcher still answers as alive. A launcher that died mid-resync with nobody to reap it could therefore hold the lock as a zombie and turn every other session's launch into a lock-busy failure until the zombie was collected. The lock's predicate is now the same zombie-aware realIsProcessRunning the headroom coordination layer uses, renamed from isProcessAlive across the lock, farm, and FarmRuntime wiring to say what it means. --- src/cli.ts | 5 +++-- src/launcher.test.ts | 2 +- src/launcher.ts | 2 +- src/launcher/farm.test.ts | 4 ++-- src/launcher/farm.ts | 6 +++--- src/launcher/lock.test.ts | 26 +++++++++++++------------- src/launcher/lock.ts | 6 +++--- 7 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 52d68a9..12e66c2 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -26,7 +26,7 @@ import { realFarmFs, realFsPort, realHeadroomPort, - realIsProcessAlive, + realIsProcessRunning, realLogPort, realOwnExecutablePath, realProcPort, @@ -103,7 +103,8 @@ function buildFarmRuntime(paths: LayoutPaths): { }).input, now: () => Date.now(), uniqueSuffix: `${String(process.pid)}.${randomUUID()}`, - lock: { pid: process.pid, isProcessAlive: realIsProcessAlive, sleep: realSleepSync }, + // Zombie-aware on purpose: a previous launcher that crashed out of a resync without releasing the lock may sit unreaped, still answering signal 0 as alive, and must read as a dead holder so this launch takes the lock over instead of timing out. + lock: { pid: process.pid, isRunning: realIsProcessRunning, sleep: realSleepSync }, }, ...(selections.identity === undefined ? {} : { directoryIdentity: selections.identity }), ...(selections.configProfile === undefined ? {} : { directoryConfigProfile: selections.configProfile }), diff --git a/src/launcher.test.ts b/src/launcher.test.ts index 0601043..f1968d4 100644 --- a/src/launcher.test.ts +++ b/src/launcher.test.ts @@ -321,7 +321,7 @@ function fakeFarm(fs: FakeFarmFs, cliOverride?: CascadeInput["cliOverride"]): Fa }), now: () => FAKE_NOW_MS, uniqueSuffix: "launcher-test", - lock: { pid: 42, isProcessAlive: () => true, sleep: fakeSleep().sleep, maxAttempts: 2 }, + lock: { pid: 42, isRunning: () => true, sleep: fakeSleep().sleep, maxAttempts: 2 }, }; } diff --git a/src/launcher.ts b/src/launcher.ts index b4a747d..ee28796 100644 --- a/src/launcher.ts +++ b/src/launcher.ts @@ -38,7 +38,7 @@ export interface FarmRuntime { readonly uniqueSuffix: string; readonly lock: { readonly pid: number; - readonly isProcessAlive: (pid: number) => boolean; + readonly isRunning: (pid: number) => boolean; readonly sleep: (ms: number) => void; readonly staleAfterMs?: number; readonly retryDelayMs?: number; diff --git a/src/launcher/farm.test.ts b/src/launcher/farm.test.ts index 58c5470..214a663 100644 --- a/src/launcher/farm.test.ts +++ b/src/launcher/farm.test.ts @@ -40,7 +40,7 @@ function params(fs: FakeFarmFs, overrides: Partial = {}): Resy classification: { defaults: shippedClassification }, now: () => FAKE_NOW_MS, uniqueSuffix: "test", - lock: { pid: 42, isProcessAlive: () => true, sleep: fakeSleep().sleep }, + lock: { pid: 42, isRunning: () => true, sleep: fakeSleep().sleep }, ...overrides, }; } @@ -277,7 +277,7 @@ describe("resyncFarm", () => { params(fs, { uniqueSuffix: "blocked", cascade: cascade({ categories: { history: true } }), - lock: { pid: 42, isProcessAlive: () => true, sleep: fakeSleep().sleep, maxAttempts: 2 }, + lock: { pid: 42, isRunning: () => true, sleep: fakeSleep().sleep, maxAttempts: 2 }, }), ), ).toThrow(IdentityLockBusyError); diff --git a/src/launcher/farm.ts b/src/launcher/farm.ts index 02afcf2..fcd9847 100644 --- a/src/launcher/farm.ts +++ b/src/launcher/farm.ts @@ -459,7 +459,7 @@ export function recoverFarm(params: RecoverFarmParams): RecoveryResult { fs: params.fs, nowMs: params.now, pid: params.lock.pid, - isProcessAlive: params.lock.isProcessAlive, + isRunning: params.lock.isRunning, sleep: params.lock.sleep, ...(params.lock.staleAfterMs === undefined ? {} : { staleAfterMs: params.lock.staleAfterMs }), ...(params.lock.retryDelayMs === undefined ? {} : { retryDelayMs: params.lock.retryDelayMs }), @@ -583,7 +583,7 @@ export interface ResyncFarmParams { readonly uniqueSuffix: string; readonly lock: { readonly pid: number; - readonly isProcessAlive: (pid: number) => boolean; + readonly isRunning: (pid: number) => boolean; readonly sleep: (ms: number) => void; readonly staleAfterMs?: number; readonly retryDelayMs?: number; @@ -660,7 +660,7 @@ export function resyncFarm(params: ResyncFarmParams): ResyncFarmResult { fs: params.fs, nowMs: params.now, pid: params.lock.pid, - isProcessAlive: params.lock.isProcessAlive, + isRunning: params.lock.isRunning, sleep: params.lock.sleep, ...(params.lock.staleAfterMs === undefined ? {} : { staleAfterMs: params.lock.staleAfterMs }), ...(params.lock.retryDelayMs === undefined ? {} : { retryDelayMs: params.lock.retryDelayMs }), diff --git a/src/launcher/lock.test.ts b/src/launcher/lock.test.ts index 405f984..1493e75 100644 --- a/src/launcher/lock.test.ts +++ b/src/launcher/lock.test.ts @@ -37,7 +37,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER, - isProcessAlive: () => true, + isRunning: () => true, sleep: sleeper.sleep, }); @@ -59,7 +59,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER, - isProcessAlive: () => true, + isRunning: () => true, sleep: sleeper.sleep, }); @@ -70,7 +70,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0_PLUS_100_MS, pid: PID_WAITER, - isProcessAlive: () => true, + isRunning: () => true, sleep: sleeper.sleep, maxAttempts: MAX_ATTEMPTS, retryDelayMs: RETRY_DELAY_MS, @@ -86,7 +86,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0_PLUS_200_MS, pid: PID_WAITER, - isProcessAlive: () => true, + isRunning: () => true, sleep: sleeper.sleep, maxAttempts: MAX_ATTEMPTS, }); @@ -101,7 +101,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER_VERBOSE, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, }); @@ -112,7 +112,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_WAITER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, maxAttempts: 1, }), @@ -127,7 +127,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, }); @@ -138,7 +138,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0_PLUS_1_MS, pid: PID_WAITER, - isProcessAlive: (pid) => pid === PID_WAITER, + isRunning: (pid) => pid === PID_WAITER, sleep: sleeper.sleep, maxAttempts: MAX_ATTEMPTS, }); @@ -155,7 +155,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, }); @@ -165,7 +165,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0 + PAST_STALENESS_WINDOW_MS, pid: PID_WAITER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, maxAttempts: MAX_ATTEMPTS, }); @@ -184,7 +184,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_WAITER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, maxAttempts: MAX_ATTEMPTS, }); @@ -200,7 +200,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0, pid: PID_HOLDER, - isProcessAlive: () => true, + isRunning: () => true, sleep: fakeSleep().sleep, }); @@ -211,7 +211,7 @@ describe("acquireIdentityLock", () => { fs, nowMs: () => T0_PLUS_1_MS, pid: PID_WAITER, - isProcessAlive: (pid) => pid === PID_WAITER, + isRunning: (pid) => pid === PID_WAITER, sleep: fakeSleep().sleep, }); first.release(); diff --git a/src/launcher/lock.ts b/src/launcher/lock.ts index ad76790..5026c69 100644 --- a/src/launcher/lock.ts +++ b/src/launcher/lock.ts @@ -50,8 +50,8 @@ export interface AcquireIdentityLockParams { readonly fs: FarmFs; readonly nowMs: () => number; readonly pid: number; - /** Answers whether a process is still running, so a lock left behind by a crash is recognised rather than waited out for the full staleness window. */ - readonly isProcessAlive: (pid: number) => boolean; + /** Answers whether a process is still running, so a lock left behind by a crash is recognised rather than waited out for the full staleness window. Zombie-aware: a holder that exited without being reaped still answers signal 0 as alive, but will never release the lock itself. */ + readonly isRunning: (pid: number) => boolean; /** Blocks for the given number of milliseconds. Synchronous by necessity: the whole launcher is synchronous, right through to `spawnSync`. */ readonly sleep: (ms: number) => void; readonly staleAfterMs?: number; @@ -131,7 +131,7 @@ export function acquireIdentityLock(params: AcquireIdentityLockParams): Identity } lastHolderPid = existing.pid; const expired = params.nowMs() - existing.acquiredAtMs > staleAfterMs; - if (expired || !params.isProcessAlive(existing.pid)) { + if (expired || !params.isRunning(existing.pid)) { params.fs.removeRecursive(lockPath); continue; }