fix(cli): refuse daemon start when a probe is blocked (#2502)

When OS policy prevents a reliable process or port check, daemon startup now reports the uncertainty instead of treating a blocked probe as proof that an address is free. Missing state and preflight cases warn. The auto-start stop is narrowed to the shared PID path for a reused or uninspectable PID. Stop refuses unknown process liveness before probing or signalling and preserves the state file. A denied signal probe yields unknown even when the process state is readable, and a confirmed zombie remains dead. A blocked explicit non-loopback OPENRIG_URL stays on the blocked-status path instead of starting a local daemon (rig up and rig start); loopback targets, including localhost, [::1] and 127.x addresses, keep the existing startup behaviour. Refs #275 and #1650. Contributed by dajiaohuang.
This commit is contained in:
Wu Shuwen authored and GitHub committed 2026-11-06 22:18:20 -08:00
1 parent 126c52ab50
commit 36b0ae6464
10 files changed
+172 -21

No files matched your search

+4 -2
View File
@@ -57,10 +57,12 @@ export function createProcessLiveness(deps: {
readProcessState: (pid: number) => string | null;
}): (pid: number) => ProcessLiveness {
return (pid: number) => {
if (deps.signal(pid) === "missing") return "dead";
const signal = deps.signal(pid);
if (signal === "missing") return "dead";
const state = deps.readProcessState(pid)?.trim();
if (!state) return "unknown";
return state.startsWith("Z") ? "dead" : "alive";
if (state.startsWith("Z")) return "dead";
return signal === "not-permitted" ? "unknown" : "alive";
};
}
+32 -5
View File
@@ -39,6 +39,8 @@ export interface DaemonEventLoopEvidence {
}
export interface DaemonStatus {
/** The selected OPENRIG_URL names a non-loopback endpoint. */
explicitRemote?: boolean;
databaseWarnings?: string[];
/** RULING 1ae863d2 — C3 3-state semantics (canonical: daemon crash-cart-detect.ts, kept in
* lockstep): "stopped" requires POSITIVE evidence (connection refused); a probe TIMEOUT or
@@ -432,9 +434,13 @@ function resolveLifecycleFile(deps: LifecycleDeps, filename: "daemon.json" | "da
* - "openrig" — healthz responded (ok or not) → this is our daemon
* - "not_openrig" — connection refused → PID is alive but not listening on our port
* - "unresponsive" — pid is alive but healthz probe timed out
* - "dead" — PID not alive */
async function checkPid(state: DaemonState, deps: LifecycleDeps): Promise<"openrig" | "not_openrig" | "unresponsive" | "dead"> {
if (!deps.isProcessAlive(state.pid)) return "dead";
* - "dead" — PID not alive
* - { blocked } — the OS refused the probe; identity is unknown */
async function checkPid(state: DaemonState, deps: LifecycleDeps, knownLiveness?: ProcessLiveness): Promise<"openrig" | "not_openrig" | "unresponsive" | "dead" | { blocked: string }> {
const liveness = knownLiveness ?? (deps.processLiveness
? deps.processLiveness(state.pid)
: deps.isProcessAlive(state.pid) ? "alive" : "dead");
if (liveness === "dead") return "dead";
const host = state.host ?? DEFAULT_HOST;
try {
await fetchDaemonProbe(deps, `http://${formatDaemonHostForUrl(host)}:${state.port}/healthz`, HEALTHZ_PROBE_TIMEOUT_MS);
@@ -442,6 +448,8 @@ async function checkPid(state: DaemonState, deps: LifecycleDeps): Promise<"openr
return "openrig";
} catch (err) {
if (err instanceof HealthProbeTimeoutError) return "unresponsive";
const code = connectionErrorCode(err);
if (isBlockedConnectionCode(code)) return { blocked: code };
// Connection refused → PID alive but not our daemon
return "not_openrig";
}
@@ -679,6 +687,10 @@ async function startOwnedDaemon(opts: StartOptions, deps: LifecycleDeps, lock: D
const existing = readState(deps);
if (existing) {
const pidState = await checkPid(existing, deps);
if (typeof pidState === "object") {
const guidance = blockedConnectionGuidance(pidState.blocked);
throw new Error(`${guidance.consequence} ${guidance.action}`);
}
if (pidState === "openrig") {
// Our daemon is running (possibly unhealthy, but alive on our port)
throw new Error(`Daemon already running (pid ${existing.pid} on port ${existing.port})`);
@@ -693,6 +705,10 @@ async function startOwnedDaemon(opts: StartOptions, deps: LifecycleDeps, lock: D
await fetchDaemonProbe(deps, `http://${formatDaemonHostForUrl(probeHost)}:${port}/healthz`, HEALTHZ_PROBE_TIMEOUT_MS);
recoveredRunning = true;
} catch (error) {
const code = connectionErrorCode(error);
if (isBlockedConnectionCode(code)) {
console.warn(`Warning: daemon health probe was blocked (${code}); the port could not be checked and a daemon may already be running.`);
}
// WSL2 can silently drop probes to an unused port. A slow live daemon can
// also time out: do not run a second child's initialization against its DB.
if (error instanceof HealthProbeTimeoutError && await deps.isPortOccupied?.(probeHost, port)) {
@@ -972,7 +988,16 @@ export async function stopDaemon(deps: LifecycleDeps): Promise<"stopped" | "no-t
}
const stateFile = resolveLifecycleFile(deps, "daemon.json");
const pidState = await checkPid(state, deps);
const liveness = deps.processLiveness
? deps.processLiveness(state.pid)
: deps.isProcessAlive(state.pid) ? "alive" : "dead";
if (liveness === "unknown") {
throw new Error(`Cannot stop safely: PID ${state.pid} liveness is unknown; no signal sent, state preserved.`);
}
const pidState = await checkPid(state, deps, liveness);
if (typeof pidState === "object") {
throw new Error(`Cannot stop safely: the identity probe was blocked (${pidState.blocked}); no signal sent, state preserved.`);
}
if (pidState === "not_openrig") {
throw new Error(`Cannot stop safely: PID ${state.pid} is present but identity is not confirmed at ${check}; no signal sent, state preserved.`);
}
@@ -1107,7 +1132,9 @@ export async function getDaemonStatus(
const endpoint = (() => {
try {
const url = new URL(openrigUrl);
return { port: Number(url.port) || DEFAULT_PORT, host: url.hostname || DEFAULT_HOST };
const host = url.hostname || DEFAULT_HOST;
const loopback = host.toLowerCase() === "localhost" || host === "[::1]" || host === "::1" || /^127\.\d+\.\d+\.\d+$/.test(host);
return { port: Number(url.port) || DEFAULT_PORT, host, ...(!loopback ? { explicitRemote: true } : {}) };
} catch {
return {};
}
+3 -2
View File
@@ -72,12 +72,13 @@ export function daemonMayAnswer(status: DaemonStatus): boolean {
/** Whether `rig up` or `rig start` should start a local daemon. Yes for one confirmed down (stopped, or a
* stale record). For an unverified status, as before this check existed, when another home's daemon is
* live (an independent home elsewhere, this one's own endpoint possibly free) or the probe was blocked;
* live (an independent home elsewhere, this one's own endpoint possibly free);
* preflight and the start's own state and listener checks then decide. Not for an explicit HOME-MOVED
* marker, and not for a daemon of this home that is alive but too busy to answer (it gets the request). */
export function shouldAutoStartDaemon(status: DaemonStatus): boolean {
if (status.state === "stopped" || status.state === "stale") return true;
if (status.state !== "unverified") return false;
if (isBlockedConnectionCode(status.probeErrorCode)) return !status.explicitRemote && !status.siblingHint?.moved;
if (status.siblingHint) return !status.siblingHint.moved;
return isBlockedConnectionCode(status.probeErrorCode);
return false;
}
+14 -8
View File
@@ -6,6 +6,7 @@ import type { ConfigStore, RiggedConfig } from "./config-store.js";
import type { DaemonStatus } from "./daemon-lifecycle.js";
import { buildTmuxControlFailure, probeTmuxControlAsync } from "./tmux-health.js";
import { classifyNodeVersion } from "./node-support.js";
import { isBlockedConnectionCode } from "./daemon-reachability.js";
export interface PreflightCheck {
name: string;
@@ -49,22 +50,23 @@ interface WritableHomeCheckDeps {
checkWritable?: (path: string) => void;
}
function checkPort(host: string, port: number): Promise<boolean> {
if (port <= 0) return Promise.resolve(true); // port 0 is always available
function checkPort(host: string, port: number): Promise<"available" | "occupied" | { blocked: string }> {
if (port <= 0) return Promise.resolve("available"); // port 0 is always available
return new Promise((resolve) => {
const socket = new net.Socket();
socket.setTimeout(1000);
socket.on("connect", () => {
socket.destroy();
resolve(false); // port is in use
resolve("occupied"); // port is in use
});
socket.on("error", () => {
socket.on("error", (error) => {
socket.destroy();
resolve(true); // port is available
const code = (error as NodeJS.ErrnoException).code;
resolve(isBlockedConnectionCode(code) ? { blocked: code } : "available");
});
socket.on("timeout", () => {
socket.destroy();
resolve(true); // timeout = nothing listening
resolve("available"); // timeout = nothing listening
});
socket.connect(port, host);
});
@@ -176,12 +178,16 @@ export class SystemPreflight {
(status.host ?? "127.0.0.1") === effectiveHost &&
status.port === effectivePort;
if (daemonOnSameEndpoint) {
if (daemonOnSameEndpoint && isBlockedConnectionCode(status.probeErrorCode)) {
checks.push({ name: "port_available", ok: true, warning: `Port availability could not be checked: the connection was blocked (${status.probeErrorCode}). A daemon may still be running.` });
} else if (daemonOnSameEndpoint) {
// Our daemon is already running on this exact endpoint — skip
checks.push({ name: "port_available", ok: true });
} else {
const available = await checkPort(effectiveHost, effectivePort);
if (available) {
if (typeof available === "object") {
checks.push({ name: "port_available", ok: true, warning: `Port availability could not be checked: the connection was blocked (${available.blocked}). A daemon may still be running.` });
} else if (available === "available") {
checks.push({ name: "port_available", ok: true });
} else {
checks.push({
@@ -64,6 +64,24 @@ describe("auto-start only for a daemon confirmed down", () => {
expect(c.requests.length).toBeGreaterThan(0);
});
it.each(["EPERM", "EACCES"])(`${name}: a blocked health probe reaches the shared start path (%s)`, async (code) => {
lifecycle.status = { state: "unverified", host: "127.0.0.1", port: 7433, probeErrorCode: code };
const c = client();
const cmd = (command as (d: unknown) => import("commander").Command)({ lifecycleDeps: {}, clientFactory: c.factory, preflightExec: async () => "" });
cmd.exitOverride();
await cmd.parseAsync(argv as unknown as string[]).catch(() => {});
expect(lifecycle.startDaemon).toHaveBeenCalledOnce();
});
it.each(["EPERM", "EACCES"])(`${name}: a blocked explicit remote target never starts a local daemon (%s)`, async (code) => {
lifecycle.status = { state: "unverified", host: "remote.example", port: 7433, probeErrorCode: code, explicitRemote: true };
const c = client();
const cmd = (command as (d: unknown) => import("commander").Command)({ lifecycleDeps: {}, clientFactory: c.factory, preflightExec: async () => "" });
cmd.exitOverride();
await cmd.parseAsync(argv as unknown as string[]).catch(() => {});
expect(lifecycle.startDaemon).not.toHaveBeenCalled();
});
}
it("rig up --plan starts nothing: a stopped daemon is daemon_not_running, a blocked probe keeps its own guidance", async () => {
@@ -158,6 +158,23 @@ describe("transport render: blocked is not down", () => {
});
describe("status probe keeps the cause; the precheck guard agrees with the transport render", () => {
it.each([
["http://remote.example:7433", true],
["http://127.0.0.1:7433", false],
["http://localhost:7433", false],
["http://[::1]:7433", false],
])("marks blocked explicit remote targets for %s", async (url, remote) => {
vi.stubEnv("OPENRIG_URL", url);
try {
const status = await getDaemonStatus(lifecycleDeps({ fetch: probeFailing("EPERM") }));
expect(status.state).toBe("unverified");
expect(status.probeErrorCode).toBe("EPERM");
expect(status.explicitRemote === true).toBe(remote);
} finally {
vi.unstubAllEnvs();
}
});
it("records EPERM on every probe path, and never calls a blocked probe 'stopped'", async () => {
process.env.OPENRIG_URL = URL;
try {
@@ -260,7 +277,7 @@ describe("sandboxed liveness: EPERM on kill(pid, 0) is not a dead daemon", () =>
expect(live("sent", "Z")).toBe("dead");
expect(live("sent", null)).toBe("unknown");
expect(live("missing", "S")).toBe("dead");
expect(live("not-permitted", "S")).toBe("alive");
expect(live("not-permitted", "S")).toBe("unknown");
expect(live("not-permitted", "Z")).toBe("dead");
expect(live("not-permitted", null)).toBe("unknown");
});
+52 -1
View File
@@ -3,7 +3,7 @@ import path from "node:path";
import fs from "node:fs";
import os from "node:os";
import { createServer } from "node:net";
import { realDeps } from "../src/commands/daemon.js";
import { createProcessLiveness, realDeps } from "../src/commands/daemon.js";
import { EventEmitter } from "node:events";
import type { ChildProcess } from "node:child_process";
import {
@@ -81,6 +81,57 @@ function writtenState(deps: LifecycleDeps): DaemonState {
}
describe("Daemon Lifecycle", () => {
it.each(["EPERM", "EACCES"])("warns and starts when state is absent and the initial probe is blocked (%s)", async (code) => {
let probes = 0;
const deps = startableDeps({ fetch: vi.fn(async () => { if (probes++ === 0) throw Object.assign(new Error("probe blocked"), { code }); return startupHealth(); }) });
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
try { await startDaemon({ port: 7433 }, deps); expect(deps.spawn).toHaveBeenCalledOnce(); expect(warn).toHaveBeenCalled(); } finally { warn.mockRestore(); }
});
it("does not spawn when recorded process liveness is unknown and its probe is blocked", async () => {
const state: DaemonState = { pid: 999, port: 7433, db: "fixture.db", startedAt: "2026-01-01T00:00:00Z" };
const deps = mockDeps({ exists: vi.fn((p: string) => p === STATE_FILE), readFile: vi.fn((p: string) => p === STATE_FILE ? JSON.stringify(state) : null), isProcessAlive: vi.fn(() => false), processLiveness: vi.fn(() => "unknown"), fetch: vi.fn(async () => { throw Object.assign(new Error("blocked"), { code: "EPERM" }); }) });
await expect(startDaemon({ port: 7433 }, deps)).rejects.toThrow(/blocked the connection/i);
expect(deps.spawn).not.toHaveBeenCalled();
});
it("continues startup when the recorded process is definitely dead", async () => {
const state: DaemonState = { pid: 999, port: 7433, db: "fixture.db", startedAt: "2026-01-01T00:00:00Z" };
const deps = startableDeps({ exists: vi.fn((p: string) => p === STATE_FILE), readFile: vi.fn((p: string) => p === STATE_FILE ? JSON.stringify(state) : null), processLiveness: vi.fn(() => "dead") });
await startDaemon({ port: 7433 }, deps);
expect(deps.spawn).toHaveBeenCalledOnce();
});
it("does not signal or remove state when recorded process liveness is unknown", async () => {
const state: DaemonState = { pid: 999, port: 7433, db: "fixture.db", startedAt: "2026-01-01T00:00:00Z" };
const deps = mockDeps({
exists: vi.fn((p: string) => p === STATE_FILE),
readFile: vi.fn((p: string) => p === STATE_FILE ? JSON.stringify(state) : null),
isProcessAlive: vi.fn(() => false),
processLiveness: createProcessLiveness({ signal: () => "not-permitted", readProcessState: () => "S" }),
fetch: vi.fn(neverFetch),
});
await expect(stopDaemon(deps)).rejects.toThrow(/liveness is unknown.*no signal sent.*state preserved/i);
expect(deps.fetch).not.toHaveBeenCalled();
expect(deps.kill).not.toHaveBeenCalled();
expect(deps.removeFile).not.toHaveBeenCalled();
});
it.each(["EPERM", "EACCES"])("preserves a live recorded PID when its identity probe is blocked (%s)", async (code) => {
const state: DaemonState = { pid: 999, port: 7433, db: "fixture.db", startedAt: "2026-01-01T00:00:00Z" };
const deps = mockDeps({
exists: vi.fn((p: string) => p === STATE_FILE),
readFile: vi.fn((p: string) => p === STATE_FILE ? JSON.stringify(state) : null),
isProcessAlive: vi.fn(() => true),
fetch: vi.fn(async () => { throw Object.assign(new Error("probe blocked"), { code }); }),
});
await expect(startDaemon({ port: 7433 }, deps)).rejects.toThrow(/blocked the connection/i);
expect(deps.spawn).not.toHaveBeenCalled();
await expect(stopDaemon(deps)).rejects.toThrow(/blocked.*no signal sent/i);
expect(deps.kill).not.toHaveBeenCalled();
expect(deps.removeFile).not.toHaveBeenCalled();
});
// A started child whose own endpoint names its PID and reports `hosts`; `probe` scripts each later
// request: "ok" answers, any other value throws with that error code.
function listenerStart(hosts: string[], probe: (url: string) => string) {
+21
View File
@@ -3,6 +3,7 @@ import { mkdtempSync, rmSync, mkdirSync, chmodSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import net from "node:net";
import { EventEmitter } from "node:events";
import { Command } from "commander";
import { SystemPreflight } from "../src/system-preflight.js";
import { ConfigStore } from "../src/config-store.js";
@@ -161,6 +162,26 @@ describe("SystemPreflight", () => {
expect(portCheck!.fix).toContain("rig config set daemon.port");
});
it.each(["EPERM", "EACCES"])("does not say a blocked port probe is available (%s)", async (code) => {
const socket = Object.assign(new EventEmitter(), {
setTimeout: vi.fn(), destroy: vi.fn(),
connect() { queueMicrotask(() => socket.emit("error", Object.assign(new Error("blocked"), { code }))); },
});
const constructor = vi.spyOn(net, "Socket").mockImplementation(() => socket as unknown as net.Socket);
try {
const result = await createPreflight().run({ port: 7433 });
expect(result.ready).toBe(true);
expect(result.checks.find((c) => c.name === "port_available")).toMatchObject({ ok: true, warning: expect.stringContaining(code) });
expect(result.checks.find((c) => c.name === "port_available")!.warning).toContain("could not be checked");
} finally { constructor.mockRestore(); }
});
it("warns when a recorded daemon on the same endpoint has a blocked probe", async () => {
const pf = createPreflight({ daemonStatus: { state: "running", port: 7433, host: "127.0.0.1", probeErrorCode: "EPERM" } });
const result = await pf.run({ port: 7433, host: "127.0.0.1" });
expect(result.checks.find((c) => c.name === "port_available")).toMatchObject({ ok: true, warning: expect.stringContaining("EPERM") });
});
// Test 6
it("port check skipped when daemon running on same host:port", async () => {
const pf = createPreflight({
+5
View File
@@ -1017,6 +1017,10 @@ describe("rig start (OPR.0.3.4.1)", () => {
// never crosses into the spawned daemon env.
describe("S20 — start auto-start: routing env never becomes bind intent", () => {
it("env-sourced OPENRIG_HOST through rig start creates NO bind intent (no OPENRIG_BIND_HOST, routing scrubbed)", async () => {
const { default: net } = await import("node:net");
const { EventEmitter } = await import("node:events");
const socket = Object.assign(new EventEmitter(), { setTimeout: vi.fn(), destroy: vi.fn(), connect() { queueMicrotask(() => socket.emit("error", Object.assign(new Error("refused"), { code: "ECONNREFUSED" }))); } });
const socketSpy = vi.spyOn(net, "Socket").mockImplementation(() => socket as unknown as import("node:net").Socket);
const savedHost = process.env["OPENRIG_HOST"];
process.env["OPENRIG_HOST"] = "100.64.55.66"; // the distinct-value discriminator
let spawnedEnv: Record<string, string> = {};
@@ -1057,6 +1061,7 @@ describe("S20 — start auto-start: routing env never becomes bind intent", () =
expect(spawnedEnv["OPENRIG_BIND_HOST"]).toBeUndefined(); // no intent minted from routing env
expect(spawnedEnv["OPENRIG_HOST"]).toBeUndefined(); // and the routing var never crosses
} finally {
socketSpy.mockRestore();
if (savedHost === undefined) delete process.env["OPENRIG_HOST"];
else process.env["OPENRIG_HOST"] = savedHost;
}
+5 -2
View File
@@ -1192,6 +1192,9 @@ describe("Up CLI", () => {
});
it("S20: env-sourced OPENRIG_HOST creates NO bind intent through up auto-start (routing never crosses)", async () => {
const { default: net } = await import("node:net");
const socket = Object.assign(new EventEmitter(), { setTimeout: vi.fn(), destroy: vi.fn(), connect() { queueMicrotask(() => socket.emit("error", Object.assign(new Error("refused"), { code: "ECONNREFUSED" }))); } });
const socketSpy = vi.spyOn(net, "Socket").mockImplementation(() => socket as unknown as import("node:net").Socket);
const savedHost = process.env["OPENRIG_HOST"];
const savedPort = process.env["OPENRIG_PORT"];
// Use a distinct value so the discriminator can't pass vacuously:
@@ -1207,9 +1210,9 @@ describe("Up CLI", () => {
prog.exitOverride();
prog.addCommand(upCommand({ ...deps, preflightExec: healthyPreflightExec }));
await captureLogs(async () => {
try { await captureLogs(async () => {
await prog.parseAsync(["node", "rig", "up", "/tmp/test.yaml"]);
});
}); } finally { socketSpy.mockRestore(); }
if (savedHost === undefined) delete process.env["OPENRIG_HOST"];
else process.env["OPENRIG_HOST"] = savedHost;