From 3f71b359bdab3c55a9be4e29e7e9eccb562af69b Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 20 Oct 2026 14:40:42 -0500 Subject: [PATCH] row 198: design rev 5, Webby REVIEW-r4 (--board-only parse check, write-counter coalescing) (#1696) --- agents/bushroot/work/queue-198/DESIGN.md | 39 +++++++++++++++++------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/agents/bushroot/work/queue-198/DESIGN.md b/agents/bushroot/work/queue-198/DESIGN.md index be5041bae..b3ee11379 100644 --- a/agents/bushroot/work/queue-198/DESIGN.md +++ b/agents/bushroot/work/queue-198/DESIGN.md @@ -7,6 +7,14 @@ call mos-1019). Owner Bushroot, reviewer Webby, gate owner Sage. Inventory: `age Base: brief commit a31f1a0f8 is on origin `refactor` (head aa1bd209e). This branch rebases on `refactor` after row 192 lands (Sage's sequencing, ruling 4). +## Revision 5 (Webby REVIEW-r4.md, origin/webby/reviews 5a02ee395, on rev 4 9af75dc72; Mos mos-1045) + +- **B1 (r4, the `--tester-port` swap)**: reverted. `parses_flag` is for `--board` only. The `--tester-port` grep at + `stack-release.sh:670` stays. The `--tester-port` test row I added in rev 4 is removed. +- **N1 (r4, stale mark)**: fixed. A write counter guards `index()` coalescing. `markSeen()` and `reply()` start a fresh + scan and never join one. The new `board.test.mjs` case is in the new-tests table, and mutant M15 is in the table. +- **Sequencing (sage-1386)**: row 197 lands before 198. The rebase is noted under Open points. + ## Revision 4 (Webby REVIEW-r3.md, origin/webby/reviews c9d65664a, on rev 3 7e2ead7f3) - **B1** (blocking child calls): fixed. The board's child calls become async. `createBoard` takes an async `exec`, @@ -90,11 +98,13 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar so its board is in-process or null, never 7331. The 7331 fallback applies only to an older copy that parses `--board` and has no recorded flag. That copy starts with its own default, 7331, which is dark, the same as today (Sage ruling 3). - **Parse check (N2).** "Parses it" means the target's `cli.mjs` holds the parser branch, `flag === '--board'`, and + **Parse check (N2, rev 5).** "Parses it" means the target's `cli.mjs` holds the parser branch, `flag === '--board'`, not the flag name. The name also appears in the usage text and in refusal messages, so a plain `grep` would count - a copy that refuses the flag as one that accepts it. The same check replaces the existing `--tester-port` grep at - `stack-release.sh:670`, which has the same gap (its usage line names the flag too). One helper, `parses_flag FILE - FLAG`, greps for `flag === 'FLAG'`. Both call sites use it. + a copy that refuses the flag as one that accepts it. One helper, `parses_flag FILE FLAG`, greps for + `flag === 'FLAG'`. It is used for `--board` only, in `console_for_target` and the rollback path. The existing + `--tester-port` grep at `stack-release.sh:670` stays as it is (Webby REVIEW-r4). Changing it would break the + `stack-release.test.mjs:265` fixtures, which write a comment, not a parser line. No copy names `--tester-port` in a + refusal today, so the gap exists only for `--board`. 9. **The board context uses the writer's default fleet source (N1).** `--fleet` defaults to `~/.mosaic/fleet/agents`, as the writer did (`cli.mjs:14`). The Console reads it and never writes it. Whether the fleet is retired and the default becomes `none` is a later decision. This row keeps the board's contents unchanged. @@ -108,9 +118,11 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar stays for its existing tests. The reply trailer and status mapping are shared pure code. - The default async `exec` is `spawn` with a timeout that kills the child. A timeout or a spawn failure maps to `{ error }`, the same shape the sync path returns. Tests inject `exec`. - - `index()` coalesces concurrent calls. While a scan is in flight, later calls share its promise. The sync path - could not overlap, so this keeps parallel board requests from multiplying tmux calls. This is my choice; the - design has no other constraint on it. + - `index()` coalesces concurrent calls, but only onto a scan that started after the last write (rev 5, Webby N1). + The board keeps a write counter. `markSeen()` and `reply()` bump it, then start a fresh scan and never join one. + `index()` joins an in-flight scan only when that scan started at the current counter value. So an answer never + omits a mark that was set before it was asked for. This coalescing is my choice to limit tmux calls under + parallel reads; the design has no other constraint on it. - `conversation(url)` spawns nothing. It reads files on the event loop, as the writer did. Not changed in this row. ## Open points for Sage and Mos @@ -133,8 +145,12 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar second one rebases on the first. I will not touch the SSO code. - **Cutover.** The new Console code reaches the live Console through a copy and re-pin. The restart is a human step (`console-holds-human-authority`). No unit flag changes (decision 6). The live unit stays up until then. -- **Sequencing (ruling 4).** Sage sequences 198 after 192. I rebase once, onto the tree with 192 landed, and rerun - the board route tests there. +- **Sequencing (ruling 4, sage-1386).** Sage sequences 198 after 192 and after row 197 (Herb). I rebase once, onto the + tree with 192 and 197 landed, and rerun the board route tests there. 197 replaces closed-endpoint ports with dynamic + allocations. Its overlap with this row is `packages/webui/tests/serve.test.mjs:100` and + `packages/webui/tests/preview.test.mjs:208`. My rewrites of those lines (the dead-board 503 change at serve.test.mjs:79 + and :105, and preview.test.mjs:219 in the N3 list) are written against 197's dynamic ports, not the old fixed ones. + I do not edit either overlap line before 197 lands. The manifest goes to review only after the rebase. ## File-by-file @@ -203,6 +219,7 @@ Fixture plan. "Dead board" means a context whose `index()` throws, which replace | `/api/seen` refuses without `--send` | same file | Same shape. No mark file written. | | `/api/reply` runs with `--send` | same file | Positive control: with `send` and the sign-in header, the injected `exec` is called once with the expected argv. | | No Console proxy left | `packages/webui/tests/board-inprocess.test.mjs` | `DEFAULT_BOARD` is not exported. `boardURL` is not exported. Grep check: no `7331` in `packages/webui/src`. | +| markSeen answer includes its own mark (N1, rev 5) | `packages/control-board/tests/board.test.mjs` (new) | Hold a scan in flight with an injected exec that resolves late. Call `index()`, then `markSeen()` before the scan resolves. The `markSeen()` answer shows the mark. A mutant that joins the pre-write scan must fail it. The same check covers `reply()`. | | Board does not block the Console (B1) | same file | Injected async `exec` for the tmux check resolves after 2 s. While `/api/board` is pending, `/healthz` answers within 200 ms. Mutant M14 uses an `exec` that blocks the thread for 2 s with `Atomics.wait`, and this test must kill it. | ## Mutants @@ -224,6 +241,7 @@ must kill it is listed. A mutant that survives means the test is weak. | M10 | The `/api/reply` and `/api/seen` branches move below the `:558` refusal | The positive control "`/api/reply` runs with `--send`" (403 from `:558`) | | M11 | `COPY_PATHS` drops `packages/control-board/src` or `tools/tmux` | Fresh-copy test in `pinned-copy.test.mjs`: `/api/board` must answer 200 | | M12 | `console_for_target` keeps `--board` for a target that does not parse it | `stack-release.test.mjs`: re-pin plan lists the drop; install argv has no `--board`. The target `cli.mjs` names `--board` only in a refusal message and has no parser branch (N2). A grep-based check keeps the flag and this test kills it. | +| M15 | `index()` joins a scan that started before the last write (N1) | the `markSeen` answer case in `board.test.mjs` | | M13 | Rollback drops a recorded `--board` from a target that parses it | `stack-release.test.mjs`: rollback keeps the flag | | M14 | The board's tmux check or reply runs through a blocking call (`spawnSync`, or an `exec` that blocks the thread) | `board-inprocess.test.mjs` B1 case: `/healthz` must answer within 200 ms while `/api/board` is pending | @@ -236,7 +254,6 @@ Mutants M11 to M13 run in the cli suite, which needs Sage's slot. M10 and M14 ru | Fresh-copy Console answers the board | `packages/webui/tests/pinned-copy.test.mjs` (extend) | `stack-copy.sh` builds a copy at a fixed sha in a temp dir. Starts the Console from it on port 0. `GET /api/board` returns 200 JSON. `agent-send.sh` resolves inside the copy. | | Re-pin drops `--board` | `packages/cli/tests/stack-release.test.mjs` (extend) | Pointer records `--board`. Target `cli.mjs` lacks the parser branch, and names `--board` only in a refusal (the N2 fixture). The plan names the drop. The install argv has no `--board`. | | Rollback keeps `--board` | same | Rollback to a copy with `flag === '--board'` in its `cli.mjs` keeps the recorded flag. | -| `--tester-port` check ignores refusals (N2) | same | A target whose `cli.mjs` names `--tester-port` only in a refusal is treated as not supporting it. | ## Test line changes (N3) @@ -261,7 +278,7 @@ Mutant runs use single test files, which run freely. Any suite (control-board, w Webby approves INVENTORY.md (already sent), then this DESIGN.md, then the manifest. Sage's gate runs the control-board and webui suites, the cli suite and every `scripts/test-*.sh`. All must be green, and mutants -M1-M14 killed. +M1-M15 killed. ## Stop points