fix(session): report daemon health probe failures

.changeset/quiet-daemons-report.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Report the underlying health-probe failure when a reachable session daemon port cannot be verified.

src/session/agent/commands.daemon.test.ts

@@ -87,7 +87,9 @@ describe("resolveDaemonAvailability with a foreign process on the port", () => {
           action: "list",
           output: "json",
         } satisfies SessionCommandInput),
-      ).rejects.toThrow(/already in use/);
+      ).rejects.toThrow(
+        /already in use.*Hunk health probe returned HTTP 404 after \d+ms.*busy Hunk daemon or another process/,
+      );
     } finally {
       server.stop(true);
     }

src/session/agent/commands.ts

@@ -6,9 +6,10 @@ import type {
 import type { SessionLiveCommentSummary, SessionReviewNoteSummary } from "../types";
 import { NO_ACTIVE_SESSIONS_MESSAGE } from "./errors";
 import {
+  describeSessionBrokerHealthProbeFailure,
   ensureSessionBrokerAvailable,
-  isSessionBrokerHealthy,
   isLoopbackPortReachable,
+  probeSessionBrokerHealth,
   readSessionBrokerHealth,
   waitForSessionBrokerShutdown,
 } from "../broker/brokerLauncher";
@@ -151,16 +152,18 @@ async function ensureRequiredAction(action: SessionDaemonAction, selector?: Sess
 
 async function resolveDaemonAvailability(action: SessionCommandInput["action"]) {
   const config = resolveSessionBrokerConfig();
-  const healthy = await isSessionBrokerHealthy(config);
-  if (healthy) {
+  const healthProbe = await probeSessionBrokerHealth(config);
+  if (healthProbe.kind === "healthy") {
     return true;
   }
 
   const portReachable = await isLoopbackPortReachable(config);
   if (portReachable) {
+    const diagnostic = describeSessionBrokerHealthProbeFailure(healthProbe);
     throw new Error(
-      `Hunk session daemon port ${config.host}:${config.port} is already in use by another process. ` +
-        `Stop the conflicting process or set HUNK_MCP_PORT to a different loopback port.`,
+      `Hunk session daemon port ${config.host}:${config.port} is already in use, and the listener's ` +
+        `Hunk health probe ${diagnostic}. The listener may be a busy Hunk daemon or another process. ` +
+        `Retry, stop the conflicting process, or set HUNK_MCP_PORT to a different loopback port.`,
     );
   }
 

src/session/broker/brokerLauncher.test.ts

@@ -1,24 +1,37 @@
 import { afterEach, describe, expect, test } from "bun:test";
 import type { ChildProcess } from "node:child_process";
 import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
-import { tmpdir } from "node:os";
+import { platform, tmpdir } from "node:os";
 import { join } from "node:path";
 import {
+  describeSessionBrokerHealthProbeFailure,
   ensureSessionBrokerAvailable,
   isLoopbackPortReachable,
   parseSessionBrokerHealth,
+  probeSessionBrokerHealth,
+  readSessionBrokerHealth,
   resolveDaemonLaunchCommand,
   resolveSessionBrokerRuntimePaths,
 } from "./brokerLauncher";
 
 const tempDirs: string[] = [];
+const timeoutProbeTest = platform() === "win32" ? test.skip : test;
 const testConfig = {
   host: "127.0.0.1",
   port: 47657,
   httpOrigin: "http://127.0.0.1:47657",
   wsOrigin: "ws://127.0.0.1:47657",
 };
 
+function createTestBrokerConfig(port: number) {
+  return {
+    host: "127.0.0.1",
+    port,
+    httpOrigin: `http://127.0.0.1:${port}`,
+    wsOrigin: `ws://127.0.0.1:${port}`,
+  };
+}
+
 function createRuntimeDir() {
   const dir = mkdtempSync(join(tmpdir(), "hunk-session-daemon-launcher-test-"));
   tempDirs.push(dir);
@@ -53,6 +66,94 @@ describe("session daemon launcher", () => {
       expect(parseSessionBrokerHealth(value)).toBeNull();
     }
   });
+
+  test("retains the health payload for a successful probe", async () => {
+    const server = Bun.serve({
+      hostname: "127.0.0.1",
+      port: 0,
+      fetch: () => Response.json({ ok: true, pid: 123, sessions: 1 }),
+    });
+    const config = createTestBrokerConfig(server.port!);
+
+    try {
+      const result = await probeSessionBrokerHealth(config);
+      expect(result).toMatchObject({
+        kind: "healthy",
+        health: { ok: true, pid: 123, sessions: 1 },
+      });
+    } finally {
+      server.stop(true);
+    }
+  });
+
+  test("retains HTTP status while nullable health readers stay compatible", async () => {
+    const server = Bun.serve({
+      hostname: "127.0.0.1",
+      port: 0,
+      fetch: () => new Response("unavailable", { status: 503 }),
+    });
+    const config = createTestBrokerConfig(server.port!);
+
+    try {
+      await expect(probeSessionBrokerHealth(config)).resolves.toMatchObject({
+        kind: "http-status",
+        status: 503,
+      });
+      await expect(readSessionBrokerHealth(config)).resolves.toBeNull();
+    } finally {
+      server.stop(true);
+    }
+  });
+
+  test("distinguishes invalid JSON from incompatible health payloads", async () => {
+    let response: "json" | "payload" = "json";
+    const server = Bun.serve({
+      hostname: "127.0.0.1",
+      port: 0,
+      fetch: () =>
+        response === "json"
+          ? new Response("not-json", { headers: { "content-type": "application/json" } })
+          : Response.json({ ok: "yes" }),
+    });
+    const config = createTestBrokerConfig(server.port!);
+
+    try {
+      await expect(probeSessionBrokerHealth(config)).resolves.toMatchObject({
+        kind: "invalid-json",
+      });
+      response = "payload";
+      await expect(probeSessionBrokerHealth(config)).resolves.toMatchObject({
+        kind: "invalid-response",
+      });
+    } finally {
+      server.stop(true);
+    }
+  });
+
+  timeoutProbeTest("retains timeout budget and observed probe latency", async () => {
+    const server = Bun.serve({
+      hostname: "127.0.0.1",
+      port: 0,
+      fetch: async () => {
+        await Bun.sleep(100);
+        return Response.json({ ok: true });
+      },
+    });
+    const config = createTestBrokerConfig(server.port!);
+
+    try {
+      const result = await probeSessionBrokerHealth(config, 10);
+      expect(result).toMatchObject({ kind: "timeout", timeoutMs: 10 });
+      if (result.kind !== "timeout") throw new Error("Expected a timeout health probe result.");
+      expect(result.elapsedMs).toBeGreaterThanOrEqual(10);
+      expect(describeSessionBrokerHealthProbeFailure(result)).toMatch(
+        /^timed out after 10ms \(probe elapsed \d+ms\)$/,
+      );
+    } finally {
+      server.stop(true);
+    }
+  });
+
   test("reuses the current script entrypoint when Hunk is running from source or a JS wrapper", () => {
     expect(resolveDaemonLaunchCommand(["bun", "src/main.tsx", "diff"], "/usr/bin/bun")).toEqual({
       command: "/usr/bin/bun",

src/session/broker/brokerLauncher.ts

@@ -322,6 +322,49 @@ export interface SessionBrokerHealth {
   staleSessionTtlMs?: number;
 }
 
+type SessionBrokerHealthProbeResult =
+  | { kind: "healthy"; health: SessionBrokerHealth }
+  | { kind: "http-status"; status: number; elapsedMs: number }
+  | { kind: "invalid-json"; elapsedMs: number }
+  | { kind: "invalid-response"; elapsedMs: number }
+  | { kind: "timeout"; timeoutMs: number; elapsedMs: number }
+  | { kind: "request-error"; message: string; elapsedMs: number };
+
+type SessionBrokerHealthProbeFailure = Exclude<SessionBrokerHealthProbeResult, { kind: "healthy" }>;
+
+/** Round one failed probe duration for stable, human-readable diagnostics. */
+function healthProbeElapsedMs(startedAt: number) {
+  return Math.max(0, Math.round(performance.now() - startedAt));
+}
+
+/** Bound one runtime-generated transport error before it reaches a terminal. */
+function healthProbeErrorMessage(error: unknown) {
+  const raw = error instanceof Error ? error.message || error.name : String(error);
+  return (
+    raw
+      .replace(/[\u0000-\u001f\u007f-\u009f]/g, " ")
+      .replace(/\s+/g, " ")
+      .trim()
+      .slice(0, 240) || "Unknown request error"
+  );
+}
+
+/** Describe one failed health probe for the final user-facing CLI error. */
+export function describeSessionBrokerHealthProbeFailure(failure: SessionBrokerHealthProbeFailure) {
+  switch (failure.kind) {
+    case "http-status":
+      return `returned HTTP ${failure.status} after ${failure.elapsedMs}ms`;
+    case "invalid-json":
+      return `returned invalid JSON after ${failure.elapsedMs}ms`;
+    case "invalid-response":
+      return `returned an incompatible health payload after ${failure.elapsedMs}ms`;
+    case "timeout":
+      return `timed out after ${failure.timeoutMs}ms (probe elapsed ${failure.elapsedMs}ms)`;
+    case "request-error":
+      return `failed after ${failure.elapsedMs}ms (${failure.message})`;
+  }
+}
+
 /** Parse the minimal or legacy-rich health response without trusting cross-process JSON. */
 export function parseSessionBrokerHealth(value: unknown): SessionBrokerHealth | null {
   try {
@@ -376,37 +419,75 @@ export function parseSessionBrokerHealth(value: unknown): SessionBrokerHealth |
   }
 }
 
-/** Read the daemon's health payload when one is reachable on the configured loopback port. */
-export async function readSessionBrokerHealth(
+/** Probe daemon health while retaining the failure evidence needed for a terminal CLI error. */
+export async function probeSessionBrokerHealth(
   config: ResolvedSessionBrokerConfig = resolveSessionBrokerConfig(),
   timeoutMs = 500,
-) {
+): Promise<SessionBrokerHealthProbeResult> {
+  const startedAt = performance.now();
   const controller = new AbortController();
-  const timeout = setTimeout(() => controller.abort(), timeoutMs);
+  let timedOut = false;
+  const timeout = setTimeout(() => {
+    timedOut = true;
+    controller.abort();
+  }, timeoutMs);
   timeout.unref?.();
 
   try {
     const response = await fetch(`${config.httpOrigin}/health`, {
       signal: controller.signal,
     });
     if (!response.ok) {
-      return null;
+      return {
+        kind: "http-status",
+        status: response.status,
+        elapsedMs: healthProbeElapsedMs(startedAt),
+      };
     }
 
-    return parseSessionBrokerHealth(await response.json());
-  } catch {
-    return null;
+    let payload: unknown;
+    try {
+      payload = await response.json();
+    } catch (error) {
+      if (timedOut) throw error;
+      return {
+        kind: "invalid-json",
+        elapsedMs: healthProbeElapsedMs(startedAt),
+      };
+    }
+
+    const health = parseSessionBrokerHealth(payload);
+    return health
+      ? { kind: "healthy", health }
+      : { kind: "invalid-response", elapsedMs: healthProbeElapsedMs(startedAt) };
+  } catch (error) {
+    return timedOut
+      ? { kind: "timeout", timeoutMs, elapsedMs: healthProbeElapsedMs(startedAt) }
+      : {
+          kind: "request-error",
+          message: healthProbeErrorMessage(error),
+          elapsedMs: healthProbeElapsedMs(startedAt),
+        };
   } finally {
     clearTimeout(timeout);
   }
 }
 
+/** Read the daemon's health payload while preserving the nullable compatibility contract. */
+export async function readSessionBrokerHealth(
+  config: ResolvedSessionBrokerConfig = resolveSessionBrokerConfig(),
+  timeoutMs = 500,
+) {
+  const result = await probeSessionBrokerHealth(config, timeoutMs);
+  return result.kind === "healthy" ? result.health : null;
+}
+
 /** Check whether the loopback session broker already answers health probes. */
 export async function isSessionBrokerHealthy(
   config: ResolvedSessionBrokerConfig = resolveSessionBrokerConfig(),
   timeoutMs = 500,
 ) {
-  return (await readSessionBrokerHealth(config, timeoutMs))?.ok === true;
+  return (await probeSessionBrokerHealth(config, timeoutMs)).kind === "healthy";
 }
 
 /** Check whether some local process is already accepting TCP connections on the daemon port. */