diff --git a/design/test/daemon-discovery.test.ts b/design/test/daemon-discovery.test.ts index 12ed66cba..239ba4214 100644 --- a/design/test/daemon-discovery.test.ts +++ b/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 { + 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 {} + }); +}); diff --git a/design/test/daemon.test.ts b/design/test/daemon.test.ts index f15ee562c..65c5d7a09 100644 --- a/design/test/daemon.test.ts +++ b/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//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//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"); }); });