From ec6e3988178d1d82badd93319999f84fc8a8e613 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 13 Oct 2026 12:23:41 -0500 Subject: [PATCH] docs: post-retest follow-up briefs, queue add gate owner, row 88 and row 100 notes Co-Authored-By: Claude Opus 5.5 --- .../2026-10-13_post-retest-follow-ups.md | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 docs/plans/2026-10-13_post-retest-follow-ups.md diff --git a/docs/plans/2026-10-13_post-retest-follow-ups.md b/docs/plans/2026-10-13_post-retest-follow-ups.md new file mode 100644 index 000000000..7741f0144 --- /dev/null +++ b/docs/plans/2026-10-13_post-retest-follow-ups.md @@ -0,0 +1,201 @@ +# Follow-ups queued behind Jason's PM re-test + +Three rows filed on 2026-10-13. Each stays `queued` until Mos confirms +Jason's PM re-test is finished (Mos's ruling, 2026-10-13). Sage moves a +row to `briefed` only after that. + +## Queue add names its gate owner + +### Problem + +`queue add` defaults `gateOwner` to `jason` when the caller passes none +(`packages/queue/src/queue.mjs`, the `add` case in apply). Sage added +row 100 at rev 942 without `--gate-owner`. The row got a Jason gate +that nobody meant and whose gate text holds nothing for Jason. After +both approvals, `move 100 done` refused, so the row went to +`waiting-on-jason`. That state's only exit is `done`, and when Sage +makes that move it needs evidence of Jason's approval. A slip put a +step in front of Jason that he never agreed to (Mos, Vikunja #257). + +Non-privileged seats can't set `gateOwner` on add at all (the same +case refuses it). For them the Jason default is the fail-closed choice, +and it stays. + +### Owner and reviewer + +Owner: Researcher. Reviewer: Filbert (decision 130, not a sandbox row). + +### Files owned + +- `packages/queue/src/queue.mjs`, the `add` validation and apply only +- `packages/queue/src/cli.mjs`, the `add` usage text only +- `packages/queue/tests/`, one existing test file or a new one +- `packages/queue/README.md`, the `add` paragraph only +- `agents/researcher/work/queue-/` + +### What ships + +- A privileged actor (`jason`, `sage`) adding a row without + `--gate-owner` is refused with a message that names the flag. The + refusal comes before the log entry is written. +- A non-privileged add is unchanged: it can't name a gate owner, and the + row's gate owner is `jason`. +- Replay of the existing log is unchanged. Every entry already in + `docs/plans/queue.json` replays to the same rows, and + `queue verify --current` passes on the current file. If the check + needs a semantics bump to tell new entries from old, say so in the + build notes. +- Tests: a privileged add without the flag is refused and writes no + entry; with it, the row carries the named owner; a non-privileged add + still gets `jason`; a log holding an old privileged add with + `gateOwner: null` still replays. +- The usage text and README say the flag is required on a privileged + add, and why. + +Each new check is shown to fail on a mutant that removes it. + +Suites: the queue node suite and `scripts/test-queue.sh`, then every +`scripts/test-*.sh`. + +### Out of scope + +Any new exit from `waiting-on-jason`. Changing row 100 or any other +existing row. Any other verb. + +### Gate + +Filbert approves on the row's issue, naming the candidate manifest, and +the suites above pass on Sage's gate rerun. + +## Row 88 follow-up: field table header, line length and bidi escapes + +### Problem + +Row 88 (82414385, #1582) landed with Filbert's approval (28214) and +four non-blocking notes: + +- N1. The header comment in `packages/tasks/src/fields.mjs` says the + test checks the prose against the table, but not how. +- N2. `packages/tasks/tests/fields.test.mjs` line 13 is 146 characters. +- N3. Three files carry literal bidi control characters in source where + an escape would do: `packages/webui/src/public/bus.js` line 13 (the + UNSEEN pattern), `packages/webui/tests/bus.test.mjs` line 20 and + `packages/conversation/tests/flows.test.mjs` line 495. +- N4. `inertCheck` in `packages/webui/tests/bus-browser.test.mjs` + refuses U+202A-U+202E and U+2066-U+2069 in shown text but not U+200E + and U+200F. + +### Owner and reviewer + +Owner: Researcher, row 88's owner. Reviewer: Filbert. + +### Files owned + +- `packages/tasks/src/fields.mjs`, the header comment only +- `packages/tasks/tests/fields.test.mjs`, line 13 only +- `packages/webui/src/public/bus.js`, the UNSEEN pattern only +- `packages/webui/tests/bus.test.mjs` +- `packages/conversation/tests/flows.test.mjs`, line 495 only +- `packages/webui/tests/bus-browser.test.mjs`, `inertCheck` only +- `agents/researcher/work/queue-/` + +### What ships + +- N1: one sentence in the header naming the check the test makes. +- N2: line 13 wrapped. No other reflow. +- N3: each literal replaced by its `\u` escape, with the same code + points matched before and after. The build notes show each file's + bidi count going from its current value to zero, and the tests that + cover each pattern still pass. +- N4: U+200E and U+200F added to the `inertCheck` pattern, shown to + fail when a test injects either. + +Suites: the tasks, webui and conversation node suites and every +`scripts/test-*.sh`. + +### Out of scope + +Any behaviour change in `bus.js` beyond the escape. Restarting the +Console. + +### Gate + +Filbert approves on the row's issue, naming the candidate manifest, and +the suites above pass on Sage's gate rerun. The `public/` change needs +Mos's go before landing (decision 123), and the decision 109 notices go +to Mos before and after. + +## Row 100 follow-up: git kill reach, close bound and the last refusal tests + +### Problem + +Row 100 (664199f7, #1594) landed with approvals from Sage (28225) and +Dewey (28226). Their notes and Darkwing's build notes +(`agents/darkwing/work/queue-100/r1/build.md`) leave these gaps: + +- `packages/seat/src/checkout.mjs` `git()` and the runner-side git in + `packages/harness/src/git-tools.mjs` still time out with spawn's SIGKILL, so the stale lock row + 100 fixed in `working.mjs` can still happen there. +- Every timed-out git kill reaches only the direct child, not its + process group. A hook or helper git spawns can outlive it. +- `close()` in `git-socket.mjs` waits on the told sockets with no time + limit (Sage d, Dewey d). +- `send()` relies on EPIPE to classify a peer that closes with bytes + queued. The post-loop errored arm has no test; a FIFO as the file + makes it deterministic (Dewey a, Sage b, Dewey e). +- The 30 second idle refusal has no test (Dewey b). +- `have`'s within-remote date order isn't asserted; the test stays + under HAVE_MAX (Dewey c). "Newest first" holds within each clone, not + across clones, and the comment doesn't say so (Sage a). +- `startHost`'s single-host check on `host.json` isn't atomic. +- `roles/reviewer.md` doesn't point at the `nothing-to-fetch` text in + `packages/cli/README.md` (Dewey g). + +### Owner and reviewer + +Darkwing builds it. Dewey and Sage review. + +### Files owned + +- `packages/seat/src/checkout.mjs`, `packages/seat/src/working.mjs` and + their tests, `packages/seat/README.md` +- `packages/harness/src/git-tools.mjs` (its git spawn and the `have` + comment only) and its tests +- `packages/cli/src/git-socket.mjs`, `packages/cli/src/host.mjs` + (the `startHost` check only), their tests, `packages/cli/README.md` +- `roles/reviewer.md`, one pointer line only +- `agents/darkwing/work/queue-/` + +### What ships + +- `checkout.mjs` and `git-tools.mjs` use the same SIGTERM, grace, + SIGKILL sequence as `working.mjs`, each with a test. +- Every host git runs in its own process group, and the timeout signals + the group. A test shows a child git spawns doesn't outlive the kill. +- `close()` caps its wait on told sockets at a stated constant, then + destroys them. A test with a peer that never reads shows the cap. +- A FIFO test reaches `send()`'s post-loop errored arm. The comment + states the EPIPE assumption. +- A test for the idle refusal. +- A `have` test with more working refs than HAVE_MAX and known commit + dates that asserts the order. The comment says the order is per + clone, or the loop sorts across clones; say which in the build notes. +- `startHost` takes `host.json` atomically (exclusive create or a lock), + with a test that two hosts racing leave one running. +- `roles/reviewer.md` points at the `nothing-to-fetch` text. + +Each new check is shown to fail on a mutant that removes it. + +Suites: every node suite and every `scripts/test-*.sh`. + +### Out of scope + +Any broker or schema change. Tying a push to its task's project or +assignee. gc of refused pushes. + +### Gate + +Dewey and Sage approve on the row's issue, naming the candidate +manifest, and the suites above pass on Sage's gate rerun. Decision 109 +notices go to Mos before and after landing. The cli and seat side is +live only at a bus restart, and that needs Mos's go.