Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/coding-agent/.changes/eng-5302.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Added `doctor` detection of daemons that lost their supervisor ownership record (status `ownership-lost`); `doctor --fix` now restarts only the affected daemon on the same socket and agent dir, preserving sessions (declining daemons whose state lives under a different agent dir), and ownership and socket-lock errors name the cause and the incumbent owner.
19 changes: 16 additions & 3 deletions packages/coding-agent/src/cli/daemon-ps-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,21 @@ export function formatDaemonListTable(daemons: readonly DaemonInfo[]): string {
uptime: formatUptime(daemon.uptimeSeconds),
}));
const table = formatTable(["socket", "pid", "version", "status", "sessions", "uptime"], rows, formatDaemonCell);
return daemons.some((daemon) => daemon.isDefault)
? `${table}\n\n${chalk.dim("* default background service")}`
: table;
const footers: string[] = [];
if (daemons.some((daemon) => daemon.isDefault)) {
footers.push(chalk.dim("* default background service"));
}
for (const daemon of daemons) {
if (daemon.status === "ownership-lost") {
footers.push(
chalk.dim(
`! ${daemon.socketPath}: supervisor lost its ownership record — ` +
'run "prime-agent doctor --fix" to restart it (sessions are preserved)',
),
);
}
}
return footers.length > 0 ? `${table}\n\n${footers.join("\n")}` : table;
}

function formatDaemonCell(_row: DaemonRow, column: keyof DaemonRow, value: string): string {
Expand All @@ -38,6 +50,7 @@ function colorStatus(status: DaemonStatus, value: string): string {
return chalk.green(value);
case "stale":
return chalk.yellow(value);
case "ownership-lost":
case "unreachable":
return chalk.red(value);
case "orphan-file":
Expand Down
222 changes: 211 additions & 11 deletions packages/coding-agent/src/cli/daemon-ps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ import {
type DaemonRuntimeIdentity,
} from "../modes/daemon/daemon-protocol.js";
import { defaultDaemonSocketDir, defaultDaemonSocketPath, normalizeSocketPath } from "../modes/daemon/daemon-socket.js";
import { acquireDaemonShutdownAdmission } from "../modes/daemon/daemon-supervisor-ownership.js";
import { supervisorStateDirMatches } from "../modes/daemon/daemon-supervisor.js";
import {
acquireDaemonShutdownAdmission,
assertDaemonSupervisorOwnerCurrent,
waitForDaemonStartupFence,
} 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 { ensureInteractiveDaemonRunning } from "./daemon-launch.js";
import { formatDaemonListTable } from "./daemon-ps-format.js";
import { promptYesNo } from "./daemon-stop-confirm.js";

Expand All @@ -36,7 +42,7 @@ import { promptYesNo } from "./daemon-stop-confirm.js";
* older build (a new protocol command would not).
*/

export type DaemonStatus = "current" | "stale" | "unreachable" | "orphan-file";
export type DaemonStatus = "current" | "stale" | "ownership-lost" | "unreachable" | "orphan-file";

export interface DiscoveredDaemonProcess {
pid: number;
Expand All @@ -63,8 +69,9 @@ export interface DaemonInfo {
const STATUS_ORDER: Record<DaemonStatus, number> = {
current: 0,
stale: 1,
unreachable: 2,
"orphan-file": 3,
"ownership-lost": 2,
unreachable: 3,
"orphan-file": 4,
};
const SHUTDOWN_QUIET_PERIOD_MS = 1000;
const SHUTDOWN_CONVERGENCE_TIMEOUT_MS = 10_000;
Expand Down Expand Up @@ -233,12 +240,13 @@ function scanSocketDir(): string[] {
return sockets;
}

interface ProbeResult {
export interface ProbeResult {
version?: string;
protocolVersion?: number;
schemaId?: string;
runtime?: DaemonRuntimeIdentity;
sessionCount?: number;
supervisorGeneration?: string;
supervisorPid?: number;
supervisorProcessStartId?: string;
reachable: boolean;
Expand All @@ -257,6 +265,7 @@ async function probeDaemon(socketPath: string): Promise<ProbeResult> {
let protocolVersion: number | undefined;
let schemaId: string | undefined;
let runtime: DaemonRuntimeIdentity | undefined;
let supervisorGeneration: string | undefined;
let supervisorPid: number | undefined;
let supervisorProcessStartId: string | undefined;
let greeted = false;
Expand All @@ -266,6 +275,7 @@ async function probeDaemon(socketPath: string): Promise<ProbeResult> {
protocolVersion = hello.protocol.version;
schemaId = hello.schemaId;
runtime = hello.runtime;
supervisorGeneration = hello.supervisorGeneration;
supervisorPid = hello.supervisorPid;
supervisorProcessStartId = hello.supervisorProcessStartId;
greeted = true;
Expand All @@ -290,6 +300,7 @@ async function probeDaemon(socketPath: string): Promise<ProbeResult> {
schemaId,
runtime,
sessionCount,
supervisorGeneration,
supervisorPid,
supervisorProcessStartId,
reachable: true,
Expand Down Expand Up @@ -333,6 +344,48 @@ export function verifyHelloSupervisorPid(
return pid;
}

/**
* Cross-checks the probed hello identity against the on-disk ownership
* registry; no daemon commands (a wedged supervisor cannot serve them). Race-
* free without age heuristics: the record is written before listen() and
* released only after the socket closes, and the final pid+startId liveness
* check rejects daemons that exited or were replaced since the probe.
*/
export async function detectDaemonOwnershipLost(
socketPath: string,
probe: ProbeResult,
registryDir?: string,
): Promise<boolean> {
if (!probe.reachable || !probe.supervisorGeneration) {
return false;
}
const pid = verifyHelloSupervisorPid(probe.supervisorPid, probe.supervisorProcessStartId);
if (pid === undefined) {
return false;
}
try {
await assertDaemonSupervisorOwnerCurrent(
{
generation: probe.supervisorGeneration,
pid,
...(probe.supervisorProcessStartId ? { processStartId: probe.supervisorProcessStartId } : {}),
socketPath,
},
undefined,
registryDir,
);
return false;
} catch (error) {
// This result gates a kill: an unexpected registry error must never
// classify a healthy daemon as lost.
if ((error as { code?: unknown }).code !== "supervisor_generation_stale") {
return false;
}
// Flag only a live process: a stopping daemon releases its record late.
return verifyHelloSupervisorPid(pid, probe.supervisorProcessStartId) !== undefined;
}
}

/** Discover every daemon on the machine and probe each for version + session count. */
export async function discoverDaemons(): Promise<DaemonInfo[]> {
const processBySocket = new Map<string, DiscoveredDaemonProcess>();
Expand All @@ -359,11 +412,17 @@ export async function discoverDaemons(): Promise<DaemonInfo[]> {
const probe = await probeDaemon(socketPath);
const pid = proc?.pid ?? verifyHelloSupervisorPid(probe.supervisorPid, probe.supervisorProcessStartId);
const hasTrackedWorkers = workerSockets.has(socketPath);
const status: DaemonStatus = probe.reachable
let status: DaemonStatus = probe.reachable
? classifyReachable(probe)
: proc || hasTrackedWorkers
? "unreachable"
: "orphan-file";
// Stale daemons wedge the same way — an upgraded CLI probing a pre-upgrade
// supervisor is the incident-report case, so version mismatch must not
// mask ownership loss.
if ((status === "current" || status === "stale") && (await detectDaemonOwnershipLost(socketPath, probe))) {
status = "ownership-lost";
}
Comment thread
cursor[bot] marked this conversation as resolved.
return {
socketPath,
pid,
Expand Down Expand Up @@ -412,6 +471,7 @@ export async function runPs(json: boolean): Promise<void> {
export type ReapAction =
| { kind: "remove-file"; daemon: DaemonInfo }
| { kind: "kill"; daemon: DaemonInfo }
| { kind: "restart"; daemon: DaemonInfo }
| { kind: "shutdown"; daemon: DaemonInfo }
| { kind: "skip"; daemon: DaemonInfo; reason: string };

Expand Down Expand Up @@ -445,6 +505,10 @@ export function planReap(daemons: readonly DaemonInfo[], force: boolean): ReapAc
if (daemon.status === "orphan-file") {
return { kind: "remove-file", daemon };
}
// Wedged supervisors reject every command; only a restart repairs them.
if (daemon.status === "ownership-lost") {
return { kind: "restart", daemon };
}
if (daemon.isDefault) {
return { kind: "skip", daemon, reason: "default background service" };
}
Expand Down Expand Up @@ -489,7 +553,8 @@ const SHUTDOWN_ALL_ACTION_ORDER: Record<ReapAction["kind"], number> = {
shutdown: 0,
"remove-file": 1,
kill: 2,
skip: 3,
restart: 3,
skip: 4,
};

export type ShutdownConfirmationPlan = "none" | "prompt" | "json-error" | "tty-error";
Expand Down Expand Up @@ -1067,6 +1132,111 @@ async function stopTrackedProcess(
return !isProcessAlive(pid);
}

export interface RepairHooks {
probe: (socketPath: string) => Promise<ProbeResult>;
detectLost: (socketPath: string, probe: ProbeResult) => Promise<boolean>;
ownsSupervisorState: (socketPath: string, supervisorGeneration: string) => boolean;
acquireAdmission: () => Promise<{ assertOrRenew: () => Promise<void>; release: () => Promise<void> }>;
killDaemon: (pid: number, expectedProcessStartId: string | undefined) => Promise<boolean>;
waitStartupFence: (socketPath: string) => Promise<void>;
ensureDaemonRunning: (socketPath: string) => Promise<void>;
}

const defaultRepairHooks: RepairHooks = {
probe: probeDaemon,
detectLost: detectDaemonOwnershipLost,
ownsSupervisorState: (socketPath, supervisorGeneration) =>
supervisorStateDirMatches(getAgentDir(), socketPath, supervisorGeneration),
acquireAdmission: acquireDaemonShutdownAdmission,
killDaemon: forceKillDaemon,
waitStartupFence: waitForDaemonStartupFence,
ensureDaemonRunning: ensureInteractiveDaemonRunning,
};

/**
* Kill the wedged supervisor and relaunch on the same socket, mirroring the
* update-restart coordinator's handoff: shutdown admission is held across
* kill, socket cleanup, and fence wait (no concurrent launch can bind the
* socket mid-stop), then released before relaunch since a successor cannot
* acquire ownership under it — if a client autostart wins that relaunch race,
* ensureDaemonRunning converges on the same healthy outcome. Repair first
* proves the daemon's state dir lives under this process's agent dir and
* declines otherwise: the relaunch inherits this environment, so a foreign
* agent dir's workers could not be re-adopted.
*/
export async function repairOwnershipLostDaemon(
daemon: DaemonInfo,
hooks: RepairHooks = defaultRepairHooks,
): Promise<ReapOutcome> {
const { socketPath } = daemon;
// A replaced daemon must never be killed for its predecessor.
const probe = await hooks.probe(socketPath);
if (!probe.reachable) {
return { skipped: "no longer reachable; not restarting" };
}
if (!(await hooks.detectLost(socketPath, probe))) {
return { skipped: "no longer ownership-lost; not restarting" };
}
if (verifyHelloSupervisorPid(probe.supervisorPid, probe.supervisorProcessStartId) === undefined) {
return { skipped: "ownership-lost but no verified pid to restart" };
}
if (!probe.supervisorGeneration || !hooks.ownsSupervisorState(socketPath, probe.supervisorGeneration)) {
return {
skipped:
"daemon belongs to a different agent dir; not restarting " +
`(rerun "${APP_NAME} doctor --fix" with that daemon's agent dir configured)`,
};
}
const admission = await hooks.acquireAdmission();
let pid: number | undefined;
let killedPid: number | undefined;
try {
// Acquiring the admission may have waited; only a pid verified from a
// fresh hello under it is ever signaled.
const recheck = await hooks.probe(socketPath);
pid =
recheck.reachable && recheck.supervisorGeneration === probe.supervisorGeneration
? verifyHelloSupervisorPid(recheck.supervisorPid, recheck.supervisorProcessStartId)
: undefined;
if (pid === undefined || !(await hooks.detectLost(socketPath, recheck))) {
return { skipped: "no longer ownership-lost; not restarting" };
}
let killed: boolean;
try {
await admission.assertOrRenew();
killed = await hooks.killDaemon(pid, recheck.supervisorProcessStartId);
} catch (error) {
return { skipped: `could not stop wedged supervisor (pid ${pid}): ${String(error)}` };
}
if (!killed) {
return { skipped: `wedged supervisor (pid ${pid}) did not exit after SIGKILL; not touching its socket` };
}
killedPid = pid;
removeSocketFile(socketPath);
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
await hooks.waitStartupFence(socketPath);
await admission.assertOrRenew();
} catch (error) {
return { skipped: relaunchFailureReason(killedPid, error) };
} finally {
await admission.release();
}
try {
await hooks.ensureDaemonRunning(socketPath);
} catch (error) {
return { skipped: relaunchFailureReason(killedPid, error) };
}
return {
reaped: `restarted background service after ownership loss (killed pid ${pid}, relaunched on same socket)`,
};
}

function relaunchFailureReason(killedPid: number | undefined, error: unknown): string {
return killedPid === undefined
? `could not repair ownership-lost supervisor: ${String(error)}`
: `killed wedged supervisor (pid ${killedPid}) but relaunch failed: ${String(error)}; ` +
"run any prime-agent command to autostart it";
}

export async function runReap(json: boolean, force: boolean): Promise<void> {
const daemons = await discoverDaemons();
const reaped: Array<{ socketPath: string; action: string }> = [];
Expand Down Expand Up @@ -1106,6 +1276,9 @@ export async function runReap(json: boolean, force: boolean): Promise<void> {
}
break;
}
case "restart":
apply(await repairOwnershipLostDaemon(action.daemon), socketPath, reaped, skipped);
break;
case "shutdown":
apply(await reapReachableDaemon(socketPath, pid), socketPath, reaped, skipped);
break;
Expand All @@ -1128,7 +1301,7 @@ export async function runReap(json: boolean, force: boolean): Promise<void> {
}
}

type ReapOutcome = { reaped: string } | { skipped: string };
export type ReapOutcome = { reaped: string } | { skipped: string };

function apply(
outcome: ReapOutcome,
Expand Down Expand Up @@ -1180,20 +1353,47 @@ function killDaemon(pid: number): void {
}
}

async function forceKillDaemon(pid: number): Promise<void> {
/**
* SIGTERM, then SIGKILL; resolves true only once the process is confirmed gone.
* With an expected start id, each signal is fenced against PID reuse: a pid
* whose process identity changed since verification is treated as exited.
*/
async function forceKillDaemon(pid: number, expectedProcessStartId?: string): Promise<boolean> {
// Only a PROVEN identity change counts as exited: an unreadable start id
// (transient ps failure) must neither signal a possibly-recycled pid nor
// report the supervisor gone while it may still hold the socket.
const identityChanged = () => {
if (expectedProcessStartId === undefined) return false;
const observed = getProcessStartId(pid);
return observed !== undefined && observed !== expectedProcessStartId;
};
if (identityChanged()) {
return true;
}
killDaemon(pid);
const deadline = Date.now() + 1000;
let deadline = Date.now() + 1000;
while (Date.now() < deadline) {
if (!isProcessAlive(pid)) {
return;
return true;
}
await delay(50);
}
if (identityChanged()) {
return true;
}
try {
process.kill(pid, "SIGKILL");
} catch {
// Process already exited between the liveness check and the kill.
}
deadline = Date.now() + 2000;
while (isProcessAlive(pid) && Date.now() < deadline) {
if (identityChanged()) {
return true;
}
await delay(50);
}
return !isProcessAlive(pid) || identityChanged();
}

function isProcessAlive(pid: number): boolean {
Expand Down
Loading
Loading