fix(coding-agent): one zombie-aware process-liveness probe

packages/coding-agent/.changes/worker-state-one-liveness-probe.md

@@ -0,0 +1 @@
+- Fixed zombie processes being treated as live owners by the daemon supervisor ownership registry, session leases, supervisor launch locks, `daemon ps` process stops, and update-restart liveness checks; all process liveness probes now share the zombie-aware helper.

packages/coding-agent/src/cli/daemon-ps.ts

@@ -19,7 +19,12 @@ import {
 import { defaultDaemonSocketDir, defaultDaemonSocketPath, normalizeSocketPath } from "../modes/daemon/daemon-socket.js";
 import { acquireDaemonShutdownAdmission } from "../modes/daemon/daemon-supervisor-ownership.js";
 import type { DaemonWorkerDescriptor } from "../modes/daemon/daemon-worker-protocol.js";
-import { signalProcessGroupOrProcess } from "../utils/child-process.js";
+import {
+	isProcessAlive,
+	processGroupHasLiveMember,
+	processIdExists,
+	signalProcessGroupIfHeld,
+} from "../utils/child-process.js";
 import { formatDaemonListTable } from "./daemon-ps-format.js";
 import { promptYesNo } from "./daemon-stop-confirm.js";
 
@@ -1061,34 +1066,47 @@ async function stopTrackedProcess(
 	expectedStartId: string | undefined,
 	assertAdmission: () => Promise<void>,
 ): Promise<boolean> {
-	if (!isProcessAlive(pid)) {
+	if (trackedProcessStopped(pid)) {
 		return true;
 	}
-	if (!expectedStartId || getProcessStartId(pid) !== expectedStartId) {
+	if (!expectedStartId || !trackedLeaderIdentityCurrent(pid, expectedStartId)) {
 		return false;
 	}
 	await assertAdmission();
-	if (getProcessStartId(pid) !== expectedStartId) {
+	if (!trackedLeaderIdentityCurrent(pid, expectedStartId)) {
 		return false;
 	}
-	signalProcessGroupOrProcess(pid, "SIGTERM");
+	signalProcessGroupIfHeld(pid, "SIGTERM");
 	let deadline = Date.now() + 500;
-	while (isProcessAlive(pid) && Date.now() < deadline) {
+	while (!trackedProcessStopped(pid) && Date.now() < deadline) {
 		await delay(25);
 	}
-	if (!isProcessAlive(pid)) {
+	if (trackedProcessStopped(pid)) {
 		return true;
 	}
 	await assertAdmission();
-	if (getProcessStartId(pid) !== expectedStartId) {
+	if (!trackedLeaderIdentityCurrent(pid, expectedStartId)) {
 		return false;
 	}
-	signalProcessGroupOrProcess(pid, "SIGKILL");
+	signalProcessGroupIfHeld(pid, "SIGKILL");
 	deadline = Date.now() + 1000;
-	while (isProcessAlive(pid) && Date.now() < deadline) {
+	while (!trackedProcessStopped(pid) && Date.now() < deadline) {
 		await delay(25);
 	}
-	return !isProcessAlive(pid);
+	return trackedProcessStopped(pid);
+}
+
+/** A GROUP stop completes when the leader is gone AND no live member remains; unreaped zombies do not block it. */
+function trackedProcessStopped(pid: number): boolean {
+	return !isProcessAlive(pid) && !processGroupHasLiveMember(pid);
+}
+
+/** Identity gates guard pid reuse, so they apply only while the leader exists; a pgid cannot be reused while members hold it. */
+function trackedLeaderIdentityCurrent(pid: number, expectedStartId: string): boolean {
+	if (!processIdExists(pid)) {
+		return true;
+	}
+	return getProcessStartId(pid) === expectedStartId;
 }
 
 export async function runReap(json: boolean, force: boolean): Promise<void> {
@@ -1220,15 +1238,6 @@ async function forceKillDaemon(pid: number): Promise<void> {
 	}
 }
 
-function isProcessAlive(pid: number): boolean {
-	try {
-		process.kill(pid, 0);
-		return true;
-	} catch (error) {
-		return (error as NodeJS.ErrnoException).code === "EPERM";
-	}
-}
-
 function delay(ms: number): Promise<void> {
 	return new Promise((resolve) => setTimeout(resolve, ms));
 }

packages/coding-agent/src/cli/daemon-update-restart.ts

@@ -14,6 +14,7 @@ import {
 	DAEMON_WORKER_SUPERVISOR_SOCKET_ENV,
 	DAEMON_WORKER_TOKEN_ENV,
 } from "../modes/daemon/daemon-worker-protocol.js";
+import { isProcessAlive } from "../utils/child-process.js";
 import { createCliSubprocessLaunchSpec } from "./subprocess-launch.js";
 
 export const DAEMON_UPDATE_RESTART_COORDINATOR_FLAG = "--internal-update-restart-coordinator";
@@ -354,15 +355,6 @@ async function withCoordinatorRegistryGuard<T>(registryDir: string, action: () =
 	}
 }
 
-function isProcessAlive(pid: number): boolean {
-	try {
-		process.kill(pid, 0);
-	} catch (error) {
-		return (error as NodeJS.ErrnoException).code !== "ESRCH";
-	}
-	return true;
-}
-
 function matchesProcessStartId(identity: DaemonUpdateRestartProcessIdentity): boolean {
 	if (!identity.processStartId) {
 		return true;

packages/coding-agent/src/core/session-lease.ts

@@ -3,6 +3,7 @@ import { createHash, randomUUID } from "node:crypto";
 import { existsSync, mkdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from "node:fs";
 import { basename, dirname, join, resolve } from "node:path";
 import { lockSync } from "proper-lockfile";
+import { isProcessAlive } from "../utils/child-process.js";
 
 export const SESSION_LEASES_ENABLED_ENV = "PRIME_AGENT_INTERNAL_SESSION_LEASES";
 export const SESSION_LEASE_OWNER_ID_ENV = "PRIME_AGENT_INTERNAL_SESSION_LEASE_OWNER_ID";
@@ -101,15 +102,6 @@ function readLeaseOwner(directory: string): SessionLeaseOwner | undefined {
 	}
 }
 
-function isProcessAlive(pid: number): boolean {
-	try {
-		process.kill(pid, 0);
-		return true;
-	} catch (error) {
-		return (error as NodeJS.ErrnoException).code === "EPERM";
-	}
-}
-
 interface ProcessQueryOptions {
 	env?: NodeJS.ProcessEnv;
 }

packages/coding-agent/src/modes/daemon/daemon-mode.ts

@@ -107,6 +107,7 @@ import {
 import { resolveSessionPath } from "../../core/session-resolver.js";
 import type { SessionStats } from "../../core/session-stats.js";
 import { type SideQuestionRun, startSideQuestion } from "../../core/side-question.js";
+import { isProcessAlive } from "../../utils/child-process.js";
 import { killTrackedDetachedChildren } from "../../utils/shell.js";
 import {
 	createAgentConnectionCommands,
@@ -867,7 +868,7 @@ export class AgentDaemon {
 					} catch {
 						// An invalid owner is reclaimed atomically below.
 					}
-					if (ownerPid && this.isProcessAlive(ownerPid)) {
+					if (ownerPid && isProcessAlive(ownerPid)) {
 						return;
 					}
 					const staleDirectory = `${lockDirectory}.stale-${process.pid}-${token}`;
@@ -925,15 +926,6 @@ export class AgentDaemon {
 		}
 	}
 
-	private isProcessAlive(pid: number): boolean {
-		try {
-			process.kill(pid, 0);
-			return true;
-		} catch (error) {
-			return (error as NodeJS.ErrnoException).code === "EPERM";
-		}
-	}
-
 	private cleanupSocketPath(): void {
 		if (!this.ownsSocketPath) {
 			return;

packages/coding-agent/src/modes/daemon/daemon-supervisor-ownership.ts

@@ -14,6 +14,7 @@ import { homedir } from "node:os";
 import { basename, dirname, join, resolve } from "node:path";
 import lockfile from "proper-lockfile";
 import { getProcessStartId } from "../../core/session-lease.js";
+import { isProcessAlive, isZombieProcess, processIdExists } from "../../utils/child-process.js";
 import { defaultDaemonSocketDir, normalizeSocketPath } from "./daemon-socket.js";
 
 const DAEMON_SUPERVISOR_REGISTRY_DIR_ENV = "PRIME_AGENT_INTERNAL_DAEMON_SUPERVISOR_REGISTRY_DIR";
@@ -506,6 +507,34 @@ export async function acquireDaemonSupervisorOwnership(
 	return new DaemonSupervisorOwnership(record, registryDir, ownerDirectory);
 }
 
+// The 250ms fence poll must not spawn `ps` (macOS/BSD zombie check) per tick; existence stays kill(0)-checked every tick.
+const OWNER_ZOMBIE_CONFIRM_INTERVAL_MS = 5000;
+const ownerZombieConfirmations = new Map<number, number>();
+
+function isOwnerProcessAlive(pid: number): boolean {
+	if (!processIdExists(pid)) {
+		ownerZombieConfirmations.delete(pid);
+		return false;
+	}
+	const now = Date.now();
+	const confirmedAt = ownerZombieConfirmations.get(pid);
+	if (confirmedAt !== undefined && now - confirmedAt < OWNER_ZOMBIE_CONFIRM_INTERVAL_MS) {
+		return true;
+	}
+	if (isZombieProcess(pid)) {
+		ownerZombieConfirmations.delete(pid);
+		return false;
+	}
+	// Expired entries belong to owners nothing asserts anymore; dropping them keeps the cache bounded.
+	for (const [staleOwnerPid, staleConfirmedAt] of ownerZombieConfirmations) {
+		if (now - staleConfirmedAt >= OWNER_ZOMBIE_CONFIRM_INTERVAL_MS) {
+			ownerZombieConfirmations.delete(staleOwnerPid);
+		}
+	}
+	ownerZombieConfirmations.set(pid, now);
+	return true;
+}
+
 export async function assertDaemonSupervisorOwnerCurrent(
 	owner: {
 		generation: string;
@@ -526,7 +555,7 @@ export async function assertDaemonSupervisorOwnerCurrent(
 		current.pid !== owner.pid ||
 		current.processStartId !== owner.processStartId ||
 		current.socketPath !== normalizeSocketPath(owner.socketPath) ||
-		!isProcessAlive(current.pid)
+		!isOwnerProcessAlive(current.pid)
 	) {
 		throw new DaemonSupervisorOwnershipLostError(owner.generation, { socketPath: owner.socketPath, registryDir });
 	}
@@ -693,15 +722,6 @@ function matchesExactProcessIdentity(identity: ProcessIdentity): boolean {
 	return identity.processStartId === undefined || getProcessStartId(identity.pid) === identity.processStartId;
 }
 
-function isProcessAlive(pid: number): boolean {
-	try {
-		process.kill(pid, 0);
-	} catch (error) {
-		return (error as NodeJS.ErrnoException).code !== "ESRCH";
-	}
-	return true;
-}
-
 function canonicalizeFilesystemPath(path: string): string {
 	let existingAncestor = resolve(path);
 	const missingSuffix: string[] = [];

packages/coding-agent/src/utils/child-process.ts

@@ -51,6 +51,53 @@ export function isProcessAlive(pid: number): boolean {
 	return processIdExists(pid) && !isZombieProcess(pid);
 }
 
+/** True while the group has any member left, zombies included; a group can outlive its leader. */
+export function processGroupExists(pgid: number): boolean {
+	if (process.platform === "win32") {
+		return false;
+	}
+	try {
+		process.kill(-pgid, 0);
+		return true;
+	} catch (error) {
+		return (error as NodeJS.ErrnoException).code === "EPERM";
+	}
+}
+
+/** True while the group has a RUNNING member; unreaped zombies have exited and must not block a group stop. */
+export function processGroupHasLiveMember(pgid: number): boolean {
+	if (!processGroupExists(pgid)) {
+		return false;
+	}
+	try {
+		const listing = execFileSync("ps", ["-A", "-o", "pgid=", "-o", "stat="], { encoding: "utf8" });
+		for (const line of listing.split("\n")) {
+			const fields = line.trim().split(/\s+/);
+			if (fields.length < 2) continue;
+			if (Number(fields[0]) === pgid && !fields[1]!.startsWith("Z")) {
+				return true;
+			}
+		}
+		return false;
+	} catch {
+		// Unverifiable listing reads alive: callers keep escalating instead of dropping records over live descendants.
+		return true;
+	}
+}
+
+/**
+ * Signal the group only while it is provably still the target: the leader process (even a zombie)
+ * anchors its pgid against reuse; once the leader is gone, a live member must hold the pgid at
+ * signal time, narrowing reuse exposure to the inherent kill() TOCTOU of any single-pid signal.
+ */
+export function signalProcessGroupIfHeld(pgid: number, signal: NodeJS.Signals): boolean {
+	if (!processIdExists(pgid) && !processGroupHasLiveMember(pgid)) {
+		return false;
+	}
+	signalProcessGroupOrProcess(pgid, signal);
+	return true;
+}
+
 export function signalProcessGroupOrProcess(pid: number, signal: NodeJS.Signals): void {
 	try {
 		process.kill(-pid, signal);

packages/coding-agent/test/child-process.test.ts

@@ -1,7 +1,16 @@
 import { type ChildProcess, spawn } from "node:child_process";
 import { EventEmitter } from "node:events";
 import { describe, expect, it } from "vitest";
-import { isProcessAlive, isZombieProcess, waitForChildProcess } from "../src/utils/child-process.js";
+import {
+	isProcessAlive,
+	isZombieProcess,
+	processGroupExists,
+	processGroupHasLiveMember,
+	signalProcessGroupIfHeld,
+	signalProcessGroupOrProcess,
+	waitForChildProcess,
+} from "../src/utils/child-process.js";
+import { spawnZombieProcess } from "./fixtures/zombie-process.js";
 
 describe("waitForChildProcess", () => {
 	it("reports signaled already-exited children as failures", async () => {
@@ -30,32 +39,56 @@ describe("process liveness", () => {
 	});
 
 	it.skipIf(process.platform === "win32")("treats a zombie process as dead", async () => {
-		const parent = spawn(
-			"perl",
-			["-e", '$| = 1; my $pid = fork(); if ($pid) { print "$pid\\n"; sleep 30 } else { exit 0 }'],
-			{ stdio: ["ignore", "pipe", "ignore"] },
-		);
+		const { zombiePid, dispose } = await spawnZombieProcess();
 		try {
-			const zombiePid = await new Promise<number>((resolvePid, rejectPid) => {
-				let output = "";
-				const timer = setTimeout(() => rejectPid(new Error("Timed out waiting for the zombie pid")), 5000);
-				parent.stdout.on("data", (chunk: Buffer) => {
-					output += chunk.toString();
-					const parsed = Number.parseInt(output.trim(), 10);
-					if (Number.isInteger(parsed) && parsed > 0) {
-						clearTimeout(timer);
-						resolvePid(parsed);
-					}
-				});
-			});
-			const deadline = Date.now() + 5000;
-			while (!isZombieProcess(zombiePid) && Date.now() < deadline) {
-				await new Promise((resolveDelay) => setTimeout(resolveDelay, 25));
-			}
 			expect(isZombieProcess(zombiePid)).toBe(true);
 			expect(isProcessAlive(zombiePid)).toBe(false);
 		} finally {
-			parent.kill("SIGKILL");
+			dispose();
+		}
+	});
+
+	it.skipIf(process.platform === "win32")("does not let an unreaped zombie block group-stop completion", async () => {
+		// setpgrp makes the zombie its group's only member: the group exists, but
+		// a stop waiting on it must complete because nothing is left running.
+		const { zombiePid, dispose } = await spawnZombieProcess("setpgrp(0, 0);");
+		try {
+			expect(isZombieProcess(zombiePid)).toBe(true);
+			expect(processGroupExists(zombiePid)).toBe(true);
+			expect(processGroupHasLiveMember(zombiePid)).toBe(false);
+		} finally {
+			dispose();
+		}
+	});
+
+	it.skipIf(process.platform === "win32")("keeps a process group alive after its leader exits", async () => {
+		const childless = spawn("sh", ["-c", "exit 0"], { detached: true, stdio: "ignore" });
+		const childlessExited = new Promise<void>((resolveExit) => childless.once("exit", () => resolveExit()));
+		const leader = spawn("sh", ["-c", "sleep 30 & echo started"], {
+			detached: true,
+			stdio: ["ignore", "pipe", "ignore"],
+		});
+		const leaderExited = new Promise<void>((resolveExit) => leader.once("exit", () => resolveExit()));
+		const pgid = leader.pid!;
+		try {
+			await new Promise<void>((resolveStart, rejectStart) => {
+				const timer = setTimeout(() => rejectStart(new Error("Timed out waiting for the group member")), 5000);
+				leader.stdout?.once("data", () => {
+					clearTimeout(timer);
+					resolveStart();
+				});
+			});
+			await leaderExited;
+			expect(isProcessAlive(pgid)).toBe(false);
+			expect(processGroupExists(pgid)).toBe(true);
+			expect(processGroupHasLiveMember(pgid)).toBe(true);
+			// A held group signals; a fully-gone group refuses (pgid-reuse gate).
+			expect(signalProcessGroupIfHeld(pgid, "SIGKILL")).toBe(true);
+			await childlessExited;
+			expect(processGroupExists(childless.pid!)).toBe(false);
+			expect(signalProcessGroupIfHeld(childless.pid!, "SIGKILL")).toBe(false);
+		} finally {
+			signalProcessGroupOrProcess(pgid, "SIGKILL");
 		}
 	});
 });