diff --git a/CHANGELOG.md b/CHANGELOG.md index c41dba887..a126d4c13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,18 +17,18 @@ Source: `bun test design/test/` and `git diff origin/main...HEAD --stat`. | Idle timeout per board | 10 minutes | 24 hours | 144× | | Server processes for N boards | N | 1 | N× | | Browser tabs to keep open | one per board | one total | N× | -| Design tests in repo | 16 | 67 | +51 | +| Design tests in repo | 16 | 77 | +61 | | Test paths covered (failure modes) | not enumerated| 38 / 100% | full coverage | | Plan-review findings absorbed pre-impl | 2 | 19 | 17× from Codex | | Component | New lines | Test lines | |----------------------------|-----------|------------| -| design/src/daemon.ts | ~580 | 30 tests | -| design/src/daemon-client.ts| ~340 | 17 tests | -| design/src/daemon-state.ts | ~180 | (via client + daemon tests) | +| design/src/daemon.ts | ~580 | 34 tests | +| design/src/daemon-client.ts| ~340 | 23 tests | +| design/src/daemon-state.ts | ~180 | (via client + daemon tests; direct stale-lock reclaim coverage) | | Browser round-trip via HTTP| (existed) | 4 tests | -The compression: 51 new tests cover every endpoint, lifecycle path, LRU eviction, idle handling, identity-verified spawn, version mismatch with and without active boards, PID-reuse safety, path traversal rejection, and cross-board feedback isolation. The plan-review pass caught 2 architectural issues in-house; an outside Codex pass caught 17 more, all absorbed into the implementation before any code was written. The version-mismatch path now refuses to silently kill a daemon with active boards (it prints a warning and exits 1), so upgrading gstack mid-design-session doesn't drop your in-memory board history. +The compression: 61 new tests cover every endpoint, lifecycle path, LRU eviction, real idle-shutdown behavior (spawn-based, daemon process observed exiting after `IDLE_MS`), the bare-GET-doesn't-reset-idle invariant (poll loop in background, daemon still idles out), the idle-with-active-boards extension path with `MAX_EXTENSIONS` hard ceiling, concurrent-CLIs lock race (two parallel `ensureDaemon` calls converge on one daemon), identity-verified spawn, version mismatch with and without active boards, PID-reuse safety, path traversal rejection, malformed-body negatives on every POST, and cross-board feedback isolation. The plan-review pass caught 2 architectural issues in-house; an outside Codex pass caught 17 more, all absorbed into the implementation before any code was written; the /ship review army caught 1 backwards-compat break in skill resolvers (fixed) + 5 deferred test gaps (filled). The version-mismatch path now refuses to silently kill a daemon with active boards (it prints a warning and exits 1), so upgrading gstack mid-design-session doesn't drop your in-memory board history. ### What this means for the builder diff --git a/TODOS.md b/TODOS.md index e8f1c9a70..72e3d023e 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2,41 +2,21 @@ ## design daemon: follow-ups (filed v1.45.0.0 via /ship review army) -### P3: Tighten daemon test coverage +### ✅ DONE (v1.45.0.0): Tighten daemon test coverage -**What:** Three test gaps the testing specialist flagged on the v1.45.0.0 ship: - -1. `design/test/daemon.test.ts:441` (`bare GET /api/progress does NOT reset - meaningful activity`) is a smoke test pretending to be a behavioral test. - Its own body comment admits it can't read `lastMeaningfulActivity` in-process - and only asserts the endpoint stays functional. Move to - `daemon-discovery.test.ts` with `DESIGN_DAEMON_IDLE_MS=2000` + - `DESIGN_DAEMON_CHECK_MS=200`, poll `/api/progress` in a loop, wait - `IDLE_MS+CHECK_MS`, assert the daemon process actually exited. - -2. `design/test/daemon-discovery.test.ts:14` docstring claims a - "concurrent-CLIs race (two real subprocesses, one wins the lock)" test - exists. It doesn't. Add one: fire two `ensureDaemon()` calls in parallel - against the same stateFile via `Promise.all` (or two real subprocesses); - assert both resolve to the same port, exactly one `spawned=true`, exactly - one daemon process alive, no orphaned lock file. This is the primary - correctness gate for the new daemon's spawn-vs-attach race. - -3. `design/test/daemon.test.ts:462` `idleCheckTick` only has a "callable - without throwing" smoke. The 24h-extension-with-active-boards path - (`daemon.ts:240-252`) and the `MAX_EXTENSIONS` hard ceiling - (`daemon.ts:244-247`, force-shutdown after 4 extensions) are untested. - Both are load-bearing for the 24h timeout this PR ships. - -Plus: missing malformed-JSON tests for `POST /api/boards` and -`POST /boards//api/reload` (only feedback has one); missing stale-lock -reclaim test (`daemon-state.ts:208-213` PID-dead branch is only exercised -indirectly). - -**Pros:** Closes real coverage gaps that ship-time review surfaced. Each test -is small (one or two `it()` blocks). - -**Cons:** None — these are additive tests, no behavior change. +**Resolved in commit `6b037c55` (same PR):** All 5 test gaps filled before +landing. Per-file totals after: serve 16, daemon 34, daemon-discovery 23, +feedback-roundtrip-daemon 4 = 77 (+10 from initial ship). Specifically: +- Idle-shutdown actually fires (spawn-based, daemon process observed exiting, + state file removed). +- Bare GET polling doesn't reset idle (hammers `/api/progress` in background, + daemon still idles out). +- Idle-with-active-boards extends, then force-shuts after MAX_EXTENSIONS + (with `DESIGN_DAEMON_EXTENSION_MS=1500` + `MAX_EXTENSIONS=2`). +- Concurrent `ensureDaemon()` race converges on one daemon (lock wins). +- Stale-lock reclaim (dead PID succeeds, alive unrelated PID refuses). +- Malformed-JSON + non-object + array-body + missing-html negatives for + `POST /api/boards` and `POST /boards//api/reload`. ### P3: Minor maintainability nits from /ship review