fix: the audit's remaining findings, and doctor learns whether jobs detach

README.md

@@ -77,23 +77,33 @@ pin routes, add an endpoint, or change a default.
 `configure --yes` detects installed harnesses, writes `config.yaml` into the
 tool's own state directory (`~/.harness-dispatch/`, or `HARNESS_DISPATCH_STATE_DIR`)
 — unless a `config.yaml` already exists in the current directory or
-`HARNESS_DISPATCH_CONFIG` is set, in which case that file is the target — and then
-offers to register this server with each MCP client it finds (Claude Code,
-Cursor) — showing you what it would write, and what is already there, before
-changing anything. `--no-clients` skips the offer and prints a snippet to paste
-instead; `harness-dispatch connect` does the same registration later on its own,
-and `connect --remove` undoes it. Without
-`--yes` configure previews and writes nothing. Re-running it regenerates a file it
-wrote and you have not edited, so installing a harness later is just `configure --yes`
-again; a file you have changed is refused without `--force`, and because such a file
-lists its own routes, even `--force` regenerates it from the file rather than from a
-fresh detection (it says so; add `detect: true` to the file to merge new harnesses). `doctor` then checks the whole chain:
-binary, config load, harness detection, auth and billing classification, route
-readiness, and for a Codex route asks `codex login status` whether the CLI is
-logged in (the other harnesses have no equivalent this tool has verified, so
-their login state is not checked). `--live` goes further and routes one tiny real prompt, so you see a
-completion before wiring anything into your agent. The live probe never touches paid or
-unknown-billing routes unless you pass `--allow-paid`.
+`HARNESS_DISPATCH_CONFIG` is set, in which case that file is the target.
+
+Without `--yes` it previews and writes nothing.
+
+**Registering with your MCP clients.** After writing, `configure` offers to
+register this server with each client it finds (Claude Code, Cursor), showing
+what it would write and what is already there before changing anything.
+`--no-clients` skips the offer and prints a snippet to paste instead.
+`harness-dispatch connect` does the same registration later on its own, and
+`connect --remove` undoes it.
+
+**Re-running it.** A file `configure` wrote and you have not edited is
+regenerated, so installing a harness later is just `configure --yes` again. A
+file you have changed is refused without `--force` — and because such a file
+lists its own routes, even `--force` regenerates it from the file rather than
+from a fresh detection. It says so when that happens; add `detect: true` to the
+file to merge newly installed harnesses in.
+
+**What `doctor` checks.** The whole chain: binary, config load, harness
+detection, auth and billing classification, route readiness, whether
+`dist/job-runner.js` is present (without it jobs run in-process and the
+concurrency cap does not apply), and for a Codex route it asks `codex login
+status` whether the CLI is logged in. The other harnesses have no equivalent
+this tool has verified, so their login state is not checked. `--live` goes
+further and routes one tiny real prompt through an eligible route, so you see a
+completion before wiring anything into your agent — that one spends quota, and
+it never touches paid or unknown-billing routes unless you pass `--allow-paid`.
 
 Your Claude Code / Codex / Cursor subscriptions run by default with no opt-in;
 `configure` tells you if anything is blocked and why.

src/bin.ts

@@ -20,6 +20,7 @@ import { codexLoginState } from "./dispatchers/shared/harness-login.js";
 import { clientConfigLocations, inspectClientEntries } from "./mcp-clients.js";
 import { buildDispatchers } from "./mcp/dispatcher-factory.js";
 import { startMcpServer } from "./mcp/server.js";
+import { resolveRunnerPath } from "./jobs.js";
 import { initObservability } from "./observability/index.js";
 import { QuotaCache } from "./quota.js";
 import { Router } from "./router.js";
@@ -838,6 +839,25 @@ async function cmdDoctor(
       // asked the same question twice to fill two fields.
       ...stateDirWritable(),
     },
+    {
+      // Whether dispatches will actually be detached.
+      //
+      // `resolveRunnerPath()` returning undefined is not an error — it is the
+      // signal to run the job IN-PROCESS, which is right for an unbuilt
+      // checkout and wrong everywhere else: the concurrency cap is enforced by
+      // the supervisor pool, so in-process mode silently removes the bound
+      // that exists because of a measured OOM. It prints one line on stderr at
+      // dispatch time and nothing checked it, so "am I actually capped?" had
+      // no answer. An audit noticed; this is that answer.
+      name: "job-runner",
+      ok: resolveRunnerPath() !== undefined,
+      detail:
+        resolveRunnerPath() !== undefined
+          ? "found; jobs run detached and the concurrency cap applies"
+          : "dist/job-runner.js not found — jobs will run IN-PROCESS, which " +
+            "removes the max_concurrent_runs cap and does not survive a server " +
+            "restart. Run `npm run build`, or reinstall the package.",
+    },
     {
       name: "http-auth",
       ok: true,
@@ -1391,8 +1411,18 @@ if (isThisFile(entrypoint)) {
       // reliable way to tell "bug" from "bad input" by class here. Only a
       // non-Error throw (a genuine programming error) keeps its stack.
       if (err instanceof UsageError || err instanceof Error) {
-        process.stderr.write(`harness-dispatch: ${err.message}
-`);
+        // `--json` is a promise about the SHAPE of this command's output, and
+        // it was kept only on the success path: a bad --config made
+        // `doctor --json` print a sentence, so anything parsing the output got
+        // a parse error instead of the reason. The message is the same; only
+        // the envelope follows what was asked for. Errors still go to stderr,
+        // so a caller reading stdout for results is unaffected either way.
+        const wantsJson = process.argv.slice(2).includes("--json");
+        process.stderr.write(
+          wantsJson
+            ? `${JSON.stringify({ ok: false, error: err.message }, null, 2)}\n`
+            : `harness-dispatch: ${err.message}\n`,
+        );
         process.exit(1);
       }
       throw err;

src/jobs.ts

@@ -7,7 +7,7 @@
  * consumer imports from here, so the split is invisible outside src/jobs/.
  */
 
-export { executeJobDir, runJob } from "./jobs/run.js";
+export { executeJobDir, resolveRunnerPath, runJob } from "./jobs/run.js";
 export {
   activeCapacity,
   claimJobDir,

src/jobs/start.ts

@@ -5,6 +5,7 @@
  */
 
 import { mkdir, writeFile } from "node:fs/promises";
+import { pruneDeadWorkspaceLocks } from "../workspace-lock.js";
 import path from "node:path";
 import { resolveWorkingDir, validateWorkingDir, workingDirWarning } from "../working-dir.js";
 import { buildContextPreamble } from "./context.js";
@@ -54,6 +55,12 @@ export async function startAsyncJobTracked(deps: JobDeps, input: StartJobInput):
   if (configError !== undefined) throw new Error(configError);
 
   await pruneStaleJobs();
+  // Same maintenance moment, same reason: a lock whose holder is gone is only
+  // reclaimed when something contends for that exact directory, so a
+  // dispatched-once workspace leaves its file behind forever. Best effort, and
+  // deliberately not awaited-into-failure — a sweep must never block the job
+  // that was actually asked for.
+  await pruneDeadWorkspaceLocks().catch(() => undefined);
   const jobId = newJobId();
   const root = jobsRoot();
   const jobDir = path.join(root, jobId);

src/status.ts

@@ -505,6 +505,21 @@ export function buildUsage(status: HarnessDispatchStatus): HarnessDispatchUsage
 
 export function renderUsageText(usage: HarnessDispatchUsage): string {
   const lines: string[] = ["harness-dispatch usage", ""];
+  if (usage.routes.length === 0) {
+    // A bare header and nothing else reads as "this command is broken", which
+    // is the one thing it does not mean. Carried as an open item across three
+    // releases because it looked cosmetic; it is the first thing a user with
+    // no routes sees, and it told them nothing about why or what to do.
+    lines.push(
+      "No routes configured.",
+      "",
+      "Install a harness CLI (claude, codex, cursor-agent, agy) and it is picked",
+      "up automatically, or add an `endpoints:` entry to config.yaml — those need",
+      "no CLI. `harness-dispatch doctor` says which of the two applies here.",
+      "",
+    );
+    return lines.join("\n");
+  }
   for (const route of usage.routes) {
     const mark = route.available && route.enabled ? "ok" : "off";
     const quota =

src/workspace-lock.ts

@@ -30,6 +30,7 @@
 
 import { createHash } from "node:crypto";
 import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs";
+import { readdir, rm } from "node:fs/promises";
 import path from "node:path";
 
 import { stateRoot } from "./state-dir.js";
@@ -277,3 +278,44 @@ export async function acquireWorkspaceLock(
     if (inProcessLocks.get(key) === current) inProcessLocks.delete(key);
   };
 }
+
+/**
+ * Delete lock files whose holder is definitely gone.
+ *
+ * A dead lock IS reclaimed correctly — but only when something contends for
+ * that same working directory. Nothing contends for a path you dispatched
+ * against once and moved on from, so its file stays forever: one per distinct
+ * workspace, indefinitely. Found by a resource audit, which measured a
+ * seven-day-old lock still sitting there.
+ *
+ * Tiny individually (a couple of hundred bytes), and the point is the count
+ * rather than the size — this directory is walked when locks are examined, so
+ * an unbounded file count is a cost paid later.
+ *
+ * Deliberately reuses `isDead`, so a lock is swept under exactly the same rule
+ * that lets a waiter steal it. A second, looser rule here would be a way for
+ * this to delete a lock the acquire path still considers live.
+ */
+export async function pruneDeadWorkspaceLocks(): Promise<void> {
+  const dir = path.join(stateRoot(), "workspace-locks");
+  let entries: string[];
+  try {
+    entries = await readdir(dir);
+  } catch {
+    return; // No lock directory yet: nothing to sweep.
+  }
+  for (const entry of entries) {
+    if (!entry.endsWith(".json")) continue;
+    const file = path.join(dir, entry);
+    try {
+      const record = readRecord(file);
+      // An unreadable record is not evidence of a live holder — `readRecord`
+      // returns undefined for a corrupt or half-written file, and the acquire
+      // path already treats that as absent.
+      if (record !== undefined && !isDead(record)) continue;
+      await rm(file, { force: true });
+    } catch {
+      // Vanished mid-sweep, or another process got there first.
+    }
+  }
+}

tests/bin-entrypoint.test.ts

@@ -52,3 +52,37 @@ describe("the built command runs when invoked through a link (as npm installs it
     },
   );
 });
+
+describe("--json is a promise about the shape of the output, including on failure", () => {
+  /**
+   * `--json` was honoured only on the success path. A bad `--config` made
+   * `doctor --json` print a sentence, so anything parsing the output got a
+   * parse error rather than the reason — carried as an open acceptance item
+   * across three releases because it reads as cosmetic. It is not: the whole
+   * point of the flag is that a program, not a person, is reading.
+   */
+  it.skipIf(!existsSync(bin))("reports a bad --config as JSON when --json is given", async () => {
+    const dir = mkdtempSync(path.join(tmpdir(), "hd-json-err-"));
+    cleanup.push(dir);
+    // A directory, which is a real mistake people make and cannot be a config.
+    const err = await run(process.execPath, [bin, "doctor", "--json", "--config", dir]).catch(
+      (e: { stderr?: string; stdout?: string }) => e,
+    );
+    const text = String(err.stderr ?? "");
+    const parsed = JSON.parse(text) as { ok: boolean; error: string };
+    expect(parsed.ok).toBe(false);
+    expect(parsed.error).toContain("is a directory");
+  });
+
+  it.skipIf(!existsSync(bin))("still reports it as a plain line without --json", async () => {
+    // The default must not become JSON for a person reading a terminal.
+    const dir = mkdtempSync(path.join(tmpdir(), "hd-txt-err-"));
+    cleanup.push(dir);
+    const err = await run(process.execPath, [bin, "doctor", "--config", dir]).catch(
+      (e: { stderr?: string }) => e,
+    );
+    const text = String(err.stderr ?? "");
+    expect(text).toContain("harness-dispatch: ");
+    expect(() => JSON.parse(text)).toThrow();
+  });
+});

tests/workspace-lock.test.ts

@@ -259,3 +259,45 @@ function readdirSyncSafeName(workingDir: string): string {
   const { createHash } = require("node:crypto") as typeof import("node:crypto");
   return `${createHash("sha256").update(key).digest("hex").slice(0, 16)}.json`;
 }
+
+describe("pruneDeadWorkspaceLocks", () => {
+  /**
+   * A dead lock IS reclaimed — but only when something contends for that same
+   * working directory. Nothing contends for a path dispatched against once and
+   * left alone, so its file stayed forever: one per distinct workspace. A
+   * resource audit measured a seven-day-old lock still sitting there.
+   */
+  it("removes a lock whose holder is gone and keeps a live one", async () => {
+    const { pruneDeadWorkspaceLocks } = await import("../src/workspace-lock.js");
+    await fs.mkdir(lockDir(), { recursive: true });
+
+    const dead = path.join(lockDir(), "dead000000000000.json");
+    await fs.writeFile(
+      dead,
+      JSON.stringify({ pid: 0, key: "/gone", beatMs: Date.now() - 10 * 60_000 }),
+      "utf8",
+    );
+    // This process is alive and its beat is now, so it must survive.
+    const live = path.join(lockDir(), "live000000000000.json");
+    await fs.writeFile(
+      live,
+      JSON.stringify({ pid: process.pid, key: "/here", beatMs: Date.now() }),
+      "utf8",
+    );
+    // Unreadable: the acquire path already treats this as absent, so the
+    // sweep must not be more cautious than the rule that steals it.
+    const corrupt = path.join(lockDir(), "corrupt00000000.json");
+    await fs.writeFile(corrupt, "{not json", "utf8");
+
+    await pruneDeadWorkspaceLocks();
+
+    expect(existsSync(dead), "a dead lock was kept").toBe(false);
+    expect(existsSync(corrupt), "an unreadable lock was kept").toBe(false);
+    expect(existsSync(live), "a LIVE lock was deleted").toBe(true);
+  });
+
+  it("does nothing when there is no lock directory yet", async () => {
+    const { pruneDeadWorkspaceLocks } = await import("../src/workspace-lock.js");
+    await expect(pruneDeadWorkspaceLocks()).resolves.toBeUndefined();
+  });
+});