Commit Graph

2 Commits

Author SHA1 Message Date
Garry Tan 6b037c55bf
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>
2026-05-25 20:18:06 -07:00
Garry Tan e3c235ae5c
feat(design): daemon-client with lock + identity-verified spawn
design/src/daemon-client.ts implements the CLI side of the daemon lifecycle:
ensureDaemon() (the spawn-or-attach decision), publishBoard(), and the
$D daemon stop|status helpers.

Modeled on browse/src/cli.ts:317-415 — same health-check-first attach,
same fs.openSync('wx') lock, same re-read-state-INSIDE-the-lock guard
against two CLIs both deciding "no daemon, spawn." Two design-specific
safety properties added beyond browse:

1. verifyIdentity before any SIGTERM/SIGKILL. Reads the running process's
   cmdline (/proc/PID/cmdline on Linux, `ps -p PID -o command=` on macOS)
   and only signals if it contains CMDLINE_MARKER ("gstack-design-daemon",
   passed as argv at spawn time). Prevents a stale state file from
   causing us to kill an unrelated process that inherited the PID.

2. Refuse-kill-with-active-boards on version mismatch. Browse silently
   restarts; here in-memory board history would vanish, so the client
   prints a user-actionable WARNING and exit 1 instead. Users explicitly
   `$D daemon stop` to override.

Spawn uses Node child_process.spawn (NOT Bun.spawn().unref) because of
the macOS session-detach quirks browse already discovered. Stdio is
redirected to ~/.gstack/design-daemon-startup.log, which the client
tails into stderr if waitForHealthOrError times out — no more silent
"daemon failed for some unknowable reason."

daemon-state.ts gains DESIGN_DAEMON_STATE_FILE env override so tests
can point both client and spawned daemon at a per-test path without a
shared cwd.

design/test/daemon-discovery.test.ts: 17 tests, all green in ~8s. Covers:
spawn-fresh, attach-existing, stale-state-file (pid dead), PID-reuse
safety (uses the test runner's own PID as the bait — verifyIdentity
catches the cmdline mismatch, daemon not signaled), version-mismatch
with/without active boards (the active-boards case runs a subprocess
and asserts exit 1 + WARNING in stderr), publishBoard 200 + 409,
shutdownDaemon refuse/force/unresponsive paths, daemonStatus.

The daemon-discovery suite is split out of daemon.test.ts because each
real spawn costs ~200ms; the in-process daemon.test.ts (30 tests, 70ms)
covers the same handler logic without the spawn overhead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-25 15:26:07 -07:00