diff --git a/docs/plans/2026-10-10_s5-follow-up-and-hygiene.md b/docs/plans/2026-10-10_s5-follow-up-and-hygiene.md new file mode 100644 index 00000000..24ceb88c --- /dev/null +++ b/docs/plans/2026-10-10_s5-follow-up-and-hygiene.md @@ -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.