docs(plans): brief for the S5 follow-up, test hygiene and bus comment cursor rows

Three follow-up rows from the row 40 and row 41 reviews: the Mr flows test and shim cleanup (Dewey), commit.test exit wait and the test-task Docker skip, and persisting the bus comment counts across restarts (after S6). Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-09 22:07:19 -05:00
parent 48f76e7674
commit 5488ff62a8
1 changed files with 129 additions and 0 deletions
@@ -0,0 +1,129 @@
+# S5 follow-up, test hygiene and the bus comment cursor (2026-10-10)
+
+Status: written by Sage, lead. It follows `docs/plans/BRIEF-TEMPLATE.md`
+and amends no section of the slice 1 brief. Three rows, one heading each.
+
+## S5 follow-up: stacked-hold flows test and conversation test cleanup
+
+### Problem
+
+Row 40 (S5, #1522) landed as 08b428ec with two items left for later.
+
+- Mutant Mr survives (Darkwing round 3, note 1). If a hold set inside
+  held text is not drained again, every later terminal input is swallowed,
+  Ctrl-C included. No test fails under that mutant.
+- A conversation `shim.mjs` from Filbert's row 41 round 2 gate kept
+  running after the suite, ignored SIGTERM and was killed by PID (Filbert's
+  row 41 round 3 BUILD-LOG entry).
+
+### Owner and reviewer
+
+Owner: Dewey. Reviewers: Darkwing and Filbert.
+
+### Files owned
+
+- `packages/conversation/tests/flows.test.mjs`
+- `packages/conversation/tests/` helpers that spawn `shim.mjs`, and the
+  shim itself if the fix needs it
+- `packages/conversation/README.md` only if a test changes documented
+  behaviour
+
+### What ships
+
+- A flows test that fails under Mr. Dewey's draft
+  (`~/dewey-scratch/s5/mr-test.patch`) kills Mr and passes on 08b428ec
+  with flows 28/28; start from it.
+- Find why the shim outlives the suite. Either the shim honours SIGTERM,
+  or the test that spawns it waits for its exit and kills it on cleanup.
+  A test proves no shim is left after the suite.
+- Mr run again against the new test, with the mutant output in the packet.
+
+### Out of scope
+
+No change to `terminal.mjs` or `controller.mjs` behaviour unless the new
+test finds a real defect; if it does, stop and report before fixing.
+
+### Gate
+
+Darkwing and Filbert approve on the row's issue. The conversation and
+webui node suites and every `scripts/test-*.sh` green on Sage's gate rerun.
+Mr killed.
+
+## Test hygiene: commit.test exit wait and test-task Docker skip
+
+### Problem
+
+Filbert's row 41 round 1 notes name two test defects outside row 41.
+
+- `packages/queue/tests/commit.test.mjs` (the paused-commit test, near
+  line 172) waits for the child's `exit` event, not `close`. Its stderr
+  may still be unread when the assertion runs.
+- `scripts/test-task.sh` runs the live recall pair (near lines 514-518)
+  without checking that Docker and a model are available. Without them
+  it fails two checks instead of skipping them.
+
+### Owner and reviewer
+
+Owner: unassigned (pick up through `scripts/mosaic queue next`).
+Reviewers: Darkwing and Filbert.
+
+### Files owned
+
+- `packages/queue/tests/commit.test.mjs`
+- `scripts/test-task.sh`
+
+### What ships
+
+- `commit.test.mjs` waits for `close`.
+- `test-task.sh` prints `skip` with a reason for the recall pair when
+  Docker is unreachable, and still fails when Docker is present and the
+  run fails. The skip is never silent and never counts as a pass.
+
+### Out of scope
+
+The recall check's content and any other suite.
+
+### Gate
+
+Darkwing and Filbert approve. `node --test packages/queue/tests/*.test.mjs`
+and every `scripts/test-*.sh` green, with test-task shown once with Docker
+and once without.
+
+## Bus comment cursor across restarts
+
+### Problem
+
+`packages/tasks/src/sync.mjs` keeps `ctx.commentCount` in a `Map` created in
+`packages/tasks/src/adapter.mjs`. The bus persists `task_current` in
+`bus.sqlite`, so a restart reconciles tasks, but the comment counts start
+empty. A comment posted while the bus is stopped never produces an event.
+This came up in the Q14 tracker cutover planning, where the bus stops for
+the move to tasks.woltje.com.
+
+### Owner and reviewer
+
+Owner: unassigned. Reviewers: Darkwing and Filbert. Starts after row 41
+(S6) is done, because S6 changes the bus unit and its restart.
+
+### Files owned
+
+- `packages/tasks/src/sync.mjs`, `packages/tasks/src/adapter.mjs`
+- `packages/bus/src/` where the persisted state lives
+- their tests and READMEs
+
+### What ships
+
+- Comment counts (or a per-task last-seen comment id) persist with
+  `task_current`, so the first poll after a restart emits events for
+  comments posted during the outage.
+- A test stops the bus, posts a comment on the fake tracker, restarts and
+  sees exactly one event for it, with no duplicate on the next poll.
+
+### Out of scope
+
+Event delivery semantics beyond comments, and the tracker client.
+
+### Gate
+
+Darkwing and Filbert approve. The bus and tasks node suites and every
+`scripts/test-*.sh` green on Sage's gate rerun.