test(design): fill daemon test gaps surfaced by ship review army

Adds 10 net new tests (and removes 1 misleading smoke) for the gaps the
testing specialist flagged at /ship time. Filed as P3 TODOs at ship,
filling now per boil-the-lake.

design/test/daemon-discovery.test.ts (+6 tests, +1 import):
  - "idle daemon (no boards) shuts itself down after IDLE_MS + CHECK_MS"
    Spawn-based, DESIGN_DAEMON_IDLE_MS=2000, CHECK_MS=200. Waits for the
    daemon process to actually exit and asserts the state file is removed.
    Previously only "callable without throwing" was tested.
  - "bare GET polling does NOT prevent idle shutdown"
    Hammers /api/progress every 200ms in a background loop with a done
    board, asserts the daemon still idles out — proves the
    meaningful-activity-only-on-POSTs guard (Codex finding) actually works.
  - "idle with active (non-done) boards triggers extension instead of shutdown"
    Sets DESIGN_DAEMON_EXTENSION_MS=1500 + MAX_EXTENSIONS=2, publishes a
    non-done board, asserts the daemon survives past IDLE_MS (extends),
    then verifies the MAX_EXTENSIONS hard ceiling force-shuts. Both the
    extension counter and the hard ceiling were previously untested.
  - "two parallel ensureDaemon() calls converge on one daemon"
    Fires two ensureDaemon calls in Promise.all against an empty stateFile,
    asserts: both ports match, exactly one spawned=true, exactly one daemon
    alive, no orphaned lock file. The discovery-test file's own docstring
    claimed this test existed; now it actually does.
  - "acquireLock reclaims a lockfile owned by a dead PID"
    Plants a lockfile with PID 999999998, calls acquireLock, asserts the
    returned release fn is non-null and the lock now holds our PID.
  - "acquireLock refuses to reclaim a lockfile owned by an alive PID"
    Uses the test runner's own PID — alive but not the lock's intended
    owner. Asserts acquireLock returns null and leaves the lockfile
    untouched. The unrelated-process-PID-reuse safety guard.

design/test/daemon.test.ts (-2 misleading, +5 new = +3 net):
  - Removed: "bare GET /api/progress does NOT reset meaningful activity"
    (smoke pretending to be behavioral — body comment admitted it couldn't
    verify). Replaced by the spawn-based version in daemon-discovery above.
  - Removed: "idleCheckTick is callable without throwing when there's no idle"
    (collapsed into a single smoke describe that's clearer about its scope).
  - Added: "POST /api/boards rejects invalid JSON body"
  - Added: "POST /api/boards rejects non-object body (e.g. JSON null)"
  - Added: "POST /api/boards: array body falls through to missing-html 400"
    (documents the typeof-array-is-object JS quirk; will surface if we
    ever tighten the type check)
  - Added: "POST /boards/<id>/api/reload rejects invalid JSON body"
  - Added: "POST /boards/<id>/api/reload rejects body missing html field"

Per-file totals after: serve 16, daemon 34, discovery 23, round-trip 4 = 77.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

design/test/daemon-discovery.test.ts

@@ -24,6 +24,7 @@ import {
   shutdownDaemon,
 } from "../src/daemon-client";
 import {
+  acquireLock,
   CMDLINE_MARKER,
   isProcessAlive,
   readStateFile,
@@ -362,3 +363,218 @@ describe("shutdownDaemon + daemonStatus", () => {
     expect(r.stopped).toBe(true);
   });
 });
+
+// ─── Real idle-shutdown behavior (spawned daemon, fast clock) ───
+//
+// The lastMeaningfulActivity timestamp is not observable from outside the
+// daemon process, so the only way to prove "bare GETs do not reset the
+// idle timer" is to spawn a real daemon with a short idle window, hit
+// progress polls in a loop, and watch the process exit anyway.
+//
+// These tests aim for ~3-5s real time per test by setting IDLE_MS=2000
+// and CHECK_MS=200. The idle-with-active-boards extension path needs a
+// board in `serving` state to exercise.
+
+describe("daemon idle-shutdown behavior (real process)", () => {
+  // Wait for a child process to exit, with a deadline. Resolves true on
+  // observed exit, false on timeout. Doesn't kill on timeout — caller does.
+  async function waitForExit(pid: number, timeoutMs: number): Promise<boolean> {
+    const deadline = Date.now() + timeoutMs;
+    while (Date.now() < deadline) {
+      if (!isProcessAlive(pid)) return true;
+      await new Promise((r) => setTimeout(r, 100));
+    }
+    return false;
+  }
+
+  test("idle daemon (no boards) shuts itself down after IDLE_MS + CHECK_MS", async () => {
+    const d = await spawnDaemonForTest({
+      stateFile,
+      idleMs: 2_000,
+      checkMs: 200,
+    });
+    // Don't push to activeDaemons; the daemon should self-exit and the
+    // afterEach SIGTERM would race with that. Track manually.
+    try {
+      // No boards published. lastMeaningfulActivity is the startup time.
+      // Wait IDLE_MS + a couple CHECK_MS intervals for the timer to fire.
+      const exited = await waitForExit(d.proc.pid!, 5_000);
+      expect(exited).toBe(true);
+      // State file removed by gracefulShutdown
+      expect(readStateFile(stateFile)).toBeNull();
+    } finally {
+      if (isProcessAlive(d.proc.pid!)) {
+        try { d.proc.kill("SIGKILL"); } catch {}
+      }
+    }
+  }, 10_000);
+
+  test("bare GET polling does NOT prevent idle shutdown (progress polls don't reset idle)", async () => {
+    const d = await spawnDaemonForTest({
+      stateFile,
+      idleMs: 2_000,
+      checkMs: 200,
+    });
+    let polling = true;
+    let pollCount = 0;
+    const boardDir = makeTmpDir("idle-poll");
+    try {
+      const board = await publishBoard({
+        port: d.port,
+        html: makeBoardHtml(boardDir),
+      });
+      // Submit so the board becomes `done` — non-done would trigger the
+      // 1h extension path and keep the daemon alive past IDLE_MS.
+      await fetch(`${board.url}api/feedback`, {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: JSON.stringify({ regenerated: false, preferred: "A" }),
+      });
+      // Hammer /api/progress every 200ms in the background. If bare GETs
+      // reset meaningful activity, the daemon would never idle out.
+      const pollLoop = (async () => {
+        while (polling) {
+          try {
+            await fetch(`${board.url}api/progress`);
+            pollCount += 1;
+          } catch {
+            // daemon went away
+            break;
+          }
+          await new Promise((r) => setTimeout(r, 200));
+        }
+      })();
+
+      const exited = await waitForExit(d.proc.pid!, 6_000);
+      polling = false;
+      await pollLoop;
+
+      expect(exited).toBe(true);
+      // We polled at least a few times before the daemon idled out
+      expect(pollCount).toBeGreaterThan(3);
+      expect(readStateFile(stateFile)).toBeNull();
+    } finally {
+      polling = false;
+      if (isProcessAlive(d.proc.pid!)) {
+        try { d.proc.kill("SIGKILL"); } catch {}
+      }
+      try { fs.rmSync(boardDir, { recursive: true, force: true }); } catch {}
+    }
+  }, 15_000);
+
+  test("idle with active (non-done) boards triggers extension instead of shutdown", async () => {
+    // With non-done boards, the daemon should NOT shut down on the first
+    // idle check after IDLE_MS — it extends. Verify it's still alive past
+    // the would-be-shutdown deadline. The MAX_EXTENSIONS=4 hard ceiling
+    // would take 4 * 1h = 4h to exercise with default extension window,
+    // so we shrink both IDLE and EXTENSION via env to test it in seconds.
+    const d = await spawnDaemonForTest({
+      stateFile,
+      idleMs: 1_500,
+      checkMs: 200,
+      env: {
+        DESIGN_DAEMON_EXTENSION_MS: "1500",
+        DESIGN_DAEMON_MAX_EXTENSIONS: "2",
+      },
+    });
+    const boardDir = makeTmpDir("idle-active");
+    try {
+      await publishBoard({ port: d.port, html: makeBoardHtml(boardDir) });
+      // Daemon has 1 non-done board. After IDLE_MS, idleCheckTick should
+      // extend rather than shut down. So at IDLE_MS + small margin, it's
+      // still alive.
+      await new Promise((r) => setTimeout(r, 2_500));
+      expect(isProcessAlive(d.proc.pid!)).toBe(true);
+      expect(readStateFile(stateFile)).not.toBeNull();
+
+      // After MAX_EXTENSIONS extension windows (2 * 1500ms = 3000ms more),
+      // the hard ceiling kicks in and force-shutdown fires. Total wait:
+      // IDLE_MS(1500) + EXT*MAX(3000) + slack(1000) = ~5500ms. We've already
+      // waited 2500ms, so 4000ms more.
+      const exited = await waitForExit(d.proc.pid!, 5_500);
+      expect(exited).toBe(true);
+      expect(readStateFile(stateFile)).toBeNull();
+    } finally {
+      if (isProcessAlive(d.proc.pid!)) {
+        try { d.proc.kill("SIGKILL"); } catch {}
+      }
+      try { fs.rmSync(boardDir, { recursive: true, force: true }); } catch {}
+    }
+  }, 15_000);
+});
+
+// ─── Concurrent ensureDaemon race (one wins the lock) ───────────
+
+describe("concurrent ensureDaemon race", () => {
+  test("two parallel ensureDaemon() calls converge on one daemon (one spawned, one attached)", async () => {
+    // Fire two ensureDaemon calls in parallel against the same empty
+    // stateFile. The fs.openSync('wx') lock should make exactly one win
+    // the spawn race; the loser waits for the first to write the state
+    // file, then attaches.
+    const [a, b] = await Promise.all([
+      ensureDaemon({ version: "test-version", stateFile, verbose: false }),
+      ensureDaemon({ version: "test-version", stateFile, verbose: false }),
+    ]);
+
+    // Both got the same port (same daemon)
+    expect(a.port).toBe(b.port);
+
+    // Exactly one spawned, one attached
+    const spawnedCount = [a.spawned, b.spawned].filter(Boolean).length;
+    expect(spawnedCount).toBe(1);
+
+    // Exactly one daemon process is alive at that port
+    const state = readStateFile(stateFile);
+    expect(state).not.toBeNull();
+    expect(isProcessAlive(state!.pid)).toBe(true);
+
+    // Lock file cleaned up (the winner released it on exit from the try block)
+    expect(fs.existsSync(resolveLockFilePath(stateFile))).toBe(false);
+
+    // Track for cleanup
+    activeDaemons.push({
+      proc: { pid: state!.pid } as any,
+      port: state!.port,
+      stateFile,
+      stop: async () => {
+        try { process.kill(state!.pid, "SIGTERM"); } catch {}
+      },
+    });
+  }, 15_000);
+});
+
+// ─── Stale-lock reclaim ──────────────────────────────────────────
+
+describe("acquireLock stale-lock reclaim", () => {
+  test("reclaims a lockfile owned by a dead PID and writes our PID", () => {
+    const lockPath = resolveLockFilePath(stateFile);
+    // Plant a lockfile owned by a definitely-dead PID
+    fs.mkdirSync(path.dirname(lockPath), { recursive: true });
+    fs.writeFileSync(lockPath, "999999998\n");
+
+    const release = acquireLock(lockPath);
+    expect(release).not.toBeNull();
+    // Lock file now contains our PID
+    expect(fs.readFileSync(lockPath, "utf-8").trim()).toBe(String(process.pid));
+
+    release!();
+    // Released = lock file gone
+    expect(fs.existsSync(lockPath)).toBe(false);
+  });
+
+  test("refuses to reclaim a lockfile owned by an alive (unrelated) PID", () => {
+    const lockPath = resolveLockFilePath(stateFile);
+    fs.mkdirSync(path.dirname(lockPath), { recursive: true });
+    // Use this test process's own PID — it's alive AND unrelated to a daemon.
+    // acquireLock should refuse and return null without unlinking the lock.
+    fs.writeFileSync(lockPath, `${process.pid}\n`);
+
+    const release = acquireLock(lockPath);
+    expect(release).toBeNull();
+    // Lock file is untouched
+    expect(fs.readFileSync(lockPath, "utf-8").trim()).toBe(String(process.pid));
+
+    // Cleanup
+    try { fs.unlinkSync(lockPath); } catch {}
+  });
+});

design/test/daemon.test.ts

@@ -436,33 +436,86 @@ describe("daemon LRU eviction", () => {
 });
 
 // ─── Idle + meaningful activity ──────────────────────────────────
+//
+// The behavioral tests for idle shutdown — actual process exit, bare-GET-
+// doesn't-reset-idle, MAX_EXTENSIONS hard ceiling — live in
+// daemon-discovery.test.ts because they require a real spawned daemon
+// (lastMeaningfulActivity isn't observable in-process). The in-process
+// version of these tests previously was a smoke that the testing specialist
+// correctly flagged as misleading; it was removed.
 
-describe("daemon idle + activity tracking", () => {
-  test("bare GET /api/progress does NOT reset meaningful activity", async () => {
-    const board = await publishTestBoard();
-    // Force the activity timestamp far in the past
-    markMeaningfulActivity(); // reset baseline
-    const beforeGet = Date.now();
-    for (let i = 0; i < 5; i++) {
-      await fetchHandler(req("GET", `/boards/${board.id}/api/progress`));
-    }
-    // If progress polls don't mark activity, the recorded timestamp stays
-    // at-or-before beforeGet. We can't read lastMeaningfulActivity directly,
-    // but we can simulate idle: publish was the last meaningful event, so
-    // overriding the env-driven idle window via DESIGN_DAEMON_IDLE_MS isn't
-    // available in-process. Instead, exercise idleCheckTick after pushing
-    // boards into the done state and confirm the shutdown path is reached
-    // — covered separately. Here we just assert the progress endpoint stays
-    // functional under repeated calls.
-    const r = await fetchHandler(req("GET", `/boards/${board.id}/api/progress`));
-    expect(r.status).toBe(200);
-    expect(((await r.json()) as any).status).toBe("serving");
-  });
-
-  test("idleCheckTick is callable without throwing when there's no idle", () => {
-    // Smoke check: a freshly-touched daemon should never trigger shutdown.
+describe("daemon idle + activity tracking (smoke)", () => {
+  test("idleCheckTick on a freshly-touched daemon does not throw or shut down", () => {
     markMeaningfulActivity();
     expect(() => idleCheckTick()).not.toThrow();
+    // boards map shouldn't have been wiped (no graceful shutdown happened)
+    expect(typeof __testInternals__.boards.size).toBe("number");
+  });
+});
+
+// ─── Malformed body negatives ────────────────────────────────────
+
+describe("daemon malformed body handling", () => {
+  test("POST /api/boards rejects invalid JSON body with 400", async () => {
+    const bad = new Request("http://127.0.0.1:1234/api/boards", {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: "{not json",
+    });
+    const r = await fetchHandler(bad);
+    expect(r.status).toBe(400);
+    const body = (await r.json()) as any;
+    expect(body.error).toContain("Invalid JSON");
+  });
+
+  test("POST /api/boards rejects non-object body (e.g. JSON null) with 400", async () => {
+    // JS quirk: `typeof [] === "object"`, so arrays slip past the
+    // !body || typeof body !== "object" guard and fail at the missing-html
+    // check below. The "Expected JSON object" path only fires for genuinely
+    // non-object values like null, numbers, strings.
+    const bad = new Request("http://127.0.0.1:1234/api/boards", {
+      method: "POST",
+      headers: { "Content-Type": "application/json" },
+      body: "null",
+    });
+    const r = await fetchHandler(bad);
+    expect(r.status).toBe(400);
+    const body = (await r.json()) as any;
+    expect(body.error).toContain("Expected JSON object");
+  });
+
+  test("POST /api/boards: array body falls through to missing-html 400", async () => {
+    // Documents the actual behavior — arrays bypass the type guard but get
+    // caught by the html-field check. If we ever tighten the type check to
+    // reject arrays explicitly, this test will surface the change.
+    const r = await fetchHandler(req("POST", "/api/boards", [1, 2, 3] as any));
+    expect(r.status).toBe(400);
+    const body = (await r.json()) as any;
+    expect(body.error).toContain("Missing 'html'");
+  });
+
+  test("POST /boards/<id>/api/reload rejects invalid JSON body with 400", async () => {
+    const board = await publishTestBoard();
+    const bad = new Request(
+      `http://127.0.0.1:1234/boards/${board.id}/api/reload`,
+      {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },
+        body: "{nope",
+      },
+    );
+    const r = await fetchHandler(bad);
+    expect(r.status).toBe(400);
+  });
+
+  test("POST /boards/<id>/api/reload rejects body missing html field with 400", async () => {
+    const board = await publishTestBoard();
+    const r = await fetchHandler(
+      req("POST", `/boards/${board.id}/api/reload`, { somethingElse: true }),
+    );
+    expect(r.status).toBe(400);
+    const body = (await r.json()) as any;
+    expect(body.error).toContain("HTML file not found");
   });
 });