fix(cli): bound doctor spec topology headers and bodies

packages/cli/src/commands/doctor.ts

@@ -6,6 +6,7 @@ import { execSync } from "node:child_process";
 import { resolveDaemonPath } from "../daemon-lifecycle.js";
 import { DaemonClient, ForeignLocalDaemonError, formatDaemonHostForUrl } from "../client.js";
 import { ConfigStore } from "../config-store.js";
+import { fetchWithTimeout, FetchTimeoutError } from "../fetch-with-timeout.js";
 import { buildWritableHomeCheck } from "../system-preflight.js";
 import {
   CMUX_SETTINGS_DISCLOSURE_PATH,
@@ -45,7 +46,7 @@ export interface DoctorDeps {
   /** The caller's effective user id, compared with the one the port's daemon reports. Absent: this process's own;
    *  null: this platform has none. */
   callerUid?: number | null;
-  /** How long the port's /healthz body may take to arrive (default 2000 ms); after that its owner is unknown. */
+  /** Bound for doctor HTTP headers and finite cmux bodies, and separately the owner report (default 2000 ms). */
   ownerReportTimeoutMs?: number;
   /**
    * Build B — path to a rig spec to check against the RUNNING rig of the same name (`--spec`).
@@ -61,6 +62,7 @@ export interface DoctorDeps {
 }
 
 const DEFAULT_PORT = 7433;
+const DOCTOR_HTTP_TIMEOUT_MS = 2000;
 const CMUX_SOCKET_CONTROL_FIX = 'In cmux Settings > Automation, select "Automation mode" for Socket Control Mode, or set automation.socketControlMode to "automation" in ~/.config/cmux/cmux.json (older versions use ~/.config/cmux/settings.json; current cmux reads that legacy file only for keys absent from cmux.json). cmux reloads saved settings automatically; rerun `rig doctor` to re-probe without restarting the daemon.';
 
 /** The port's OpenRig daemon, judged by the OS user it reports. /healthz's pid and euid are the daemon's own report,
@@ -72,7 +74,7 @@ async function listenerOwnerCheck(where: string, base: string, res: { json?: ()
   const env = deps.env ?? process.env;
   const at = `Port ${where} (${base}, OpenRig home ${getOpenRigHome(env)})`;
   const callerUid = deps.callerUid === undefined ? process.geteuid?.() : deps.callerUid ?? undefined;
-  const boundMs = deps.ownerReportTimeoutMs ?? 2000;
+  const boundMs = deps.ownerReportTimeoutMs ?? DOCTOR_HTTP_TIMEOUT_MS;
   let reported: { pid?: unknown; euid?: unknown } = {};
   let late = false;
   let timer: ReturnType<typeof setTimeout> | undefined;
@@ -300,7 +302,8 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
   const daemonHost = config.daemon.host ?? "127.0.0.1";
   const daemonPort = config.daemon.port ?? DEFAULT_PORT;
   const daemonBase = `http://${formatDaemonHostForUrl(daemonHost)}:${daemonPort}`;
-  const fetchFn = deps.fetch ?? globalThis.fetch;
+  const fetchFn = (deps.fetch ?? globalThis.fetch) as typeof fetch;
+  const httpBudget = { timeoutMs: deps.ownerReportTimeoutMs ?? DOCTOR_HTTP_TIMEOUT_MS, timeoutMessage: "Doctor HTTP check timed out" };
   const portCheck = deps.checkPort(daemonPort, daemonHost).then(async (available): Promise<DoctorCheck> => {
     if (available) {
       return { name: "port", status: "pass", message: `Port ${daemonHost}:${daemonPort} available.` };
@@ -308,9 +311,17 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
     // Port in use — check if it's an OpenRig daemon via healthz, and whose.
     const owner = new AbortController();
     try {
-      const res = await fetchFn(`${daemonBase}/healthz`, { signal: owner.signal });
+      const res = await fetchWithTimeout(fetchFn, `${daemonBase}/healthz`, { signal: owner.signal }, httpBudget);
       if (res.ok) return await listenerOwnerCheck(`${daemonHost}:${daemonPort}`, daemonBase, res, () => owner.abort(), deps);
-    } catch { /* not our daemon */ }
+    } catch (error) {
+      if (error instanceof FetchTimeoutError) return {
+        name: "port", status: "warn",
+        message: `Port ${daemonHost}:${daemonPort} is in use; its listener identity is unknown.`,
+        reason: `/healthz response headers did not arrive within ${httpBudget.timeoutMs / 1000} s. A timeout does not establish whether this is an OpenRig daemon or who owns it.`,
+        fix: `Inspect the listener, or give your OpenRig home a free port with: rig config set daemon.port <free-port>, then rig daemon start.`,
+      };
+      // A non-HTTP listener or connection failure remains the existing occupied-port diagnostic.
+    }
     finally { owner.abort(); }
     return {
       name: "port",
@@ -326,7 +337,9 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
   if (shellCmuxPassed) {
     const daemonCmuxCheck = (async (): Promise<DoctorCheck> => {
       try {
-        const healthRes = await fetchFn(`${daemonBase}/healthz`);
+        const healthRes = await fetchWithTimeout(fetchFn, `${daemonBase}/healthz`, {}, {
+          ...httpBudget, consumeResponse: async response => { await response.body?.cancel(); },
+        });
         if (!healthRes.ok) {
           return { name: "cmux_daemon", status: "skipped", message: "Daemon healthz not ok. Skipping daemon cmux check." };
         }
@@ -335,9 +348,14 @@ export function runDoctorChecks(deps: DoctorDeps, providerChecks: DoctorCheck[]
       }
 
       try {
-        const cmuxRes = await fetchFn(`${daemonBase}/api/adapters/cmux/status`);
-        if (cmuxRes.ok && cmuxRes.json) {
-          const data = (await cmuxRes.json()) as { available?: boolean };
+        let data: { available?: boolean } = {};
+        const cmuxRes = await fetchWithTimeout(fetchFn, `${daemonBase}/api/adapters/cmux/status`, {}, {
+          ...httpBudget, consumeResponse: async response => {
+            if (response.ok && response.json) data = await response.json() as { available?: boolean };
+            else await response.body?.cancel();
+          },
+        });
+        if (cmuxRes.ok && data) {
           if (data.available) {
             return { name: "cmux_daemon", status: "pass", message: "Daemon cmux control available." };
           }
@@ -486,15 +504,26 @@ function readTmuxMouseMode(deps: DoctorDeps): "on" | "off" | null {
 export async function readLiveLogicalIds(base: string, rigName: string, ownerClient = new DaemonClient(base)): Promise<string[] | null> {
   try {
     await ownerClient.assertLocalOwner();
-    const rigsRes = await fetch(`${base}/api/rigs`);
+    const budget = { timeoutMs: DOCTOR_HTTP_TIMEOUT_MS, timeoutMessage: "Doctor topology read timed out" };
+    let rigs: Array<{ id?: string; name?: string }> = [];
+    const rigsRes = await fetchWithTimeout(fetch, `${base}/api/rigs`, {}, {
+      ...budget, consumeResponse: async response => {
+        if (response.ok) rigs = await response.json() as typeof rigs;
+        else await response.body?.cancel();
+      },
+    });
     if (!rigsRes.ok) return null;
-    const rigs = (await rigsRes.json()) as Array<{ id?: string; name?: string }>;
     const rig = rigs.find((r) => r.name === rigName);
     if (!rig?.id) return null;
     await ownerClient.assertLocalOwner();
-    const nodesRes = await fetch(`${base}/api/rigs/${rig.id}/nodes`);
+    let nodes: Array<{ logicalId?: string | null }> = [];
+    const nodesRes = await fetchWithTimeout(fetch, `${base}/api/rigs/${rig.id}/nodes`, {}, {
+      ...budget, consumeResponse: async response => {
+        if (response.ok) nodes = await response.json() as typeof nodes;
+        else await response.body?.cancel();
+      },
+    });
     if (!nodesRes.ok) return null;
-    const nodes = (await nodesRes.json()) as Array<{ logicalId?: string | null }>;
     return nodes.map((n) => n.logicalId ?? "");
   } catch (error) {
     if (error instanceof ForeignLocalDaemonError) throw error;

packages/cli/test/doctor.test.ts

@@ -1,7 +1,8 @@
 import { describe, it, expect, vi } from "vitest";
 import { createProgram } from "../src/index.js";
+import { DaemonClient } from "../src/client.js";
 import { Command } from "commander";
-import { doctorCommand, runDoctorChecks, type DoctorDeps } from "../src/commands/doctor.js";
+import { doctorCommand, runDoctorChecks, readLiveLogicalIds, type DoctorDeps } from "../src/commands/doctor.js";
 import { resolveCmuxSettingsPath } from "../src/cmux-config.js";
 
 const CMUX_SETTINGS_PATH = resolveCmuxSettingsPath();
@@ -565,9 +566,13 @@ describe("runDoctorChecks", () => {
 
   // A listener that sends an OK header and never finishes its body must not hold doctor, or setup, before anything prints.
   it("settles a /healthz body that never finishes as owner unknown within its bound, and releases the request", async () => {
-    // The port check's request carries a signal; doctor's other /healthz reads (the cmux check) don't.
+    // Isolate the port owner-body request; conditional cmux requests are covered separately.
     const signals: AbortSignal[] = [];
     const deps = makeDeps({ checkPort: async () => false, callerUid: 1002, ownerReportTimeoutMs: 50,
+      exec: cmd => {
+        if (cmd.startsWith("cmux ")) throw new Error("fixture has no cmux");
+        return makeDeps().exec(cmd);
+      },
       fetch: async (_url: string, init?: { signal?: AbortSignal }) => {
         if (init?.signal) signals.push(init.signal);
         return { ok: true, json: () => new Promise<unknown>(() => {}) };
@@ -775,3 +780,120 @@ describe("rig doctor", () => {
     expect(portArgs).toContain(7433);
   });
 });
+
+
+describe("doctor JSON HTTP deadlines", () => {
+  it.each(["health headers", "cmux headers", "cmux body"] as const)("returns an honest diagnostic when a real listener holds %s", async (held) => {
+    const { createServer } = await import("node:http");
+    const requests: string[] = [];
+    const server = createServer((req, res) => {
+      requests.push(req.url!);
+      if (held === "health headers") return;
+      if (req.url === "/healthz") {
+        res.writeHead(200, { "content-type": "application/json" });
+        res.end(JSON.stringify({ ok: true, pid: 4242, euid: 1002 }));
+      } else if (held === "cmux body") {
+        res.writeHead(200, { "content-type": "application/json" });
+        res.write('{"available":');
+      }
+    });
+    await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
+    const port = (server.address() as { port: number }).port;
+    const log = vi.spyOn(console, "log").mockImplementation(() => {});
+    const priorExitCode = process.exitCode;
+    const program = new Command();
+    program.addCommand(doctorCommand(makeDeps({ checkPort: async () => false, callerUid: 1002,
+      ownerReportTimeoutMs: 100, fetch: (url, init) => globalThis.fetch(url, init),
+      configStore: { resolve: () => ({ ...defaultConfig, daemon: { host: "127.0.0.1", port } }) },
+    })));
+    const task = program.parseAsync(["node", "rig", "doctor", "--json"]);
+    let observationTimer: ReturnType<typeof setTimeout> | undefined;
+    try {
+      const outcome = await Promise.race([task.then(() => "settled"), new Promise<string>(resolve => {
+        observationTimer = setTimeout(() => resolve("still-waiting"), 1500);
+      })]);
+      expect(["darwin", "linux"]).toContain(process.platform);
+      expect(requests).toContain(held === "health headers" ? "/healthz" : "/api/adapters/cmux/status");
+      console.info("[doctor HTTP deadline]", JSON.stringify({ platform: process.platform, held, requests, outcome }));
+      expect(outcome).toBe("settled");
+      const result = JSON.parse(String(log.mock.calls.at(-1)![0]));
+      const portCheck = result.checks.find((c: { name: string }) => c.name === "port");
+      const cmux = result.checks.find((c: { name: string }) => c.name === "cmux_daemon");
+      if (held === "health headers") {
+        expect(portCheck).toMatchObject({ status: "warn", message: expect.stringContaining("unknown") });
+        expect(portCheck.reason).toContain("headers");
+        expect(cmux.status).toBe("skipped");
+      } else {
+        expect(portCheck.status).toBe("pass");
+        expect(cmux.status).toBe("warn");
+      }
+    } finally {
+      clearTimeout(observationTimer);
+      server.closeAllConnections();
+      await new Promise<void>(resolve => server.close(() => resolve()));
+      await Promise.allSettled([task]);
+      log.mockRestore(); process.exitCode = priorExitCode;
+      expect(server.listening).toBe(false);
+    }
+  });
+});
+
+
+describe("doctor --spec topology HTTP deadlines", () => {
+  it.each(["rig headers", "rig body", "nodes headers", "nodes body", "healthy"] as const)(
+    "returns unknown or measured conformance when the topology listener is %s", async held => {
+    const { createServer } = await import("node:http");
+    const requests: string[] = [];
+    const heldPath = held.startsWith("rig") ? "/api/rigs" : "/api/rigs/rig-id/nodes";
+    const server = createServer((req, res) => {
+      requests.push(req.url!);
+      if (held !== "healthy" && req.url === heldPath) {
+        if (held.endsWith("body")) { res.writeHead(200, { "content-type": "application/json" }); res.write("["); }
+        return;
+      }
+      res.writeHead(200, { "content-type": "application/json" });
+      res.end(JSON.stringify(req.url === "/healthz" ? { ok: true, pid: 4242, euid: 1002 }
+        : req.url === "/api/rigs" ? [{ id: "rig-id", name: "fixture" }]
+        : req.url === "/api/rigs/rig-id/nodes" ? [{ logicalId: "dev.worker" }] : { available: true }));
+    });
+    await new Promise<void>(resolve => server.listen(0, "127.0.0.1", resolve));
+    const port = (server.address() as { port: number }).port;
+    const base = `http://127.0.0.1:${port}`;
+    vi.stubEnv("OPENRIG_TRANSPORT_LOCAL_USER_GUARD", "true");
+    vi.stubEnv("OPENRIG_URL", ""); vi.stubEnv("RIGGED_URL", "");
+    const ownerProbe = vi.fn(async () => ({ state: "unknown" as const, reason: "fixture-unconfirmed" }));
+    const ownerClient = new DaemonClient(base, { ownerProbe });
+    const specPath = "/fixture/rig.yaml";
+    const spec = 'version: "0.2"\nname: fixture\npods:\n  - id: dev\n    label: Development\n    members:\n      - id: worker\n';
+    const log = vi.spyOn(console, "log").mockImplementation(() => {});
+    const priorExitCode = process.exitCode;
+    const program = new Command().addCommand(doctorCommand(makeDeps({ callerUid: 1002,
+      checkPort: async () => false, specPath, readFile: path => path === specPath ? spec : null,
+      configStore: { resolve: () => ({ ...defaultConfig, daemon: { host: "127.0.0.1", port } }) },
+      fetch: (url, init) => globalThis.fetch(url, init),
+      fetchLiveLogicalIds: name => readLiveLogicalIds(base, name, ownerClient),
+    })));
+    const task = program.parseAsync(["node", "rig", "doctor", "--json", "--spec", specPath]);
+    let timer: ReturnType<typeof setTimeout> | undefined;
+    try {
+      const outcome = await Promise.race([task.then(() => "settled"), new Promise<string>(resolve => {
+        timer = setTimeout(() => resolve("still-waiting"), 4500);
+      })]);
+      expect(["darwin", "linux"]).toContain(process.platform);
+      expect(requests).toContain(held === "healthy" ? "/api/rigs/rig-id/nodes" : heldPath);
+      expect(ownerProbe).toHaveBeenCalled();
+      console.info("[doctor spec HTTP deadline]", JSON.stringify({ platform: process.platform, held, requests, outcome }));
+      expect(outcome).toBe("settled");
+      const result = JSON.parse(String(log.mock.calls.at(-1)![0]));
+      const check = result.checks.find((c: { name: string }) => c.name === "spec_live_conformance");
+      expect(check.status).toBe(held === "healthy" ? "pass" : "skipped");
+      if (held === "healthy") expect(ownerProbe).toHaveBeenCalledTimes(2);
+    } finally {
+      clearTimeout(timer); server.closeAllConnections();
+      await new Promise<void>(resolve => server.close(() => resolve()));
+      await Promise.allSettled([task]);
+      log.mockRestore(); process.exitCode = priorExitCode; vi.unstubAllEnvs();
+      expect(server.listening).toBe(false);
+    }
+  });
+});