From 9af75dc729fe1aa5c404a81b0c3bb0097283078a Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 20 Oct 2026 14:37:49 -0500 Subject: [PATCH] row 198: design rev 4, Webby REVIEW-r3 B1 async board calls, N1 fallback copy, N2 parse check (#1696) --- agents/bushroot/work/queue-198/DESIGN.md | 54 +++++++++++++++++++----- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/agents/bushroot/work/queue-198/DESIGN.md b/agents/bushroot/work/queue-198/DESIGN.md index d9524eadb..be5041bae 100644 --- a/agents/bushroot/work/queue-198/DESIGN.md +++ b/agents/bushroot/work/queue-198/DESIGN.md @@ -7,6 +7,15 @@ 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 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`, + and `index()`, `markSeen()` and `reply()` return promises. The sync `scan` and `replyToRow` stay for the CLI and + their existing tests. See decision 10, the new B1 test and mutant M14. +- **N1** (fallback copy): fixed. Decision 8 now says which copy gets the 7331 fallback. +- **N2** (flag check matched a refusal): fixed. The parse check matches the parser branch, not the flag name. See + decision 8. M12's target now names `--board` only in a refusal. + ## Revision 3 (Sage rulings sage-1372, on rev 2 80fd45521) - Ruling 1: `/api/seen` goes under `--send`, the same as `/api/reply`. Mos agrees (mos-1023 item 5). Both POSTs sit @@ -42,7 +51,7 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar 1. **The Console calls the board's read code in-process.** A new module `packages/control-board/src/board.mjs` holds the route logic that `createServer` runs today: rescan, mark seen, reply and conversation read. It - imports no `node:http` and opens no socket. + imports no `node:http` and opens no socket. Its methods are async (decision 10). 2. **No listener anywhere in the board package.** `createServer` and `startServer` are deleted from `serve.mjs`. `serve.mjs` keeps its pure exports (`replyToRow`, `isLoopbackHost`, `foreignRequest`, `conversationResponse`, `REFUSAL_STATUS`, the reply constants). That keeps `packages/conversation/tests/flows.test.mjs` @@ -77,11 +86,32 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar carries one, such as an older pointer or a rollback target. Rule: keep `--board` only when the target's `packages/webui/src/cli.mjs` parses it. Otherwise drop it and report the drop in the plan. That is the same shape as `console_for_target` and `--tester-port` (`:667`, `:687-690`). Rollback to a copy that parses `--board` keeps - the recorded flag. A copy that does not parse it gets the flag dropped, and falls back to its own default, 7331. - That default is dark, the same as today (Sage ruling 3). + the recorded flag. A copy that does not parse it (a new copy) gets the flag dropped and starts with no board URL, + 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 + 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. 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. +10. **The board's child calls are async (B1).** The Console runs the board in its own process. A blocking child call + would stall every route, the event stream, `/healthz` and the tester listener, which shares the process + (webui `cli.mjs:91`). Rule: no `spawnSync` on any path that `createBoard` reaches. + - `scan.mjs` gains `tmuxInspectAsync` (same args, same 5 s timeout) and `scanAsync`. The sync `tmuxInspect` and + `scan` stay for the CLI. The parse of the tmux output is one pure function that both paths call, so they cannot + drift. + - `serve.mjs` gains `replyToRowAsync` (same 15 s timeout, same env, same result shapes). The sync `replyToRow` + 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. + - `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 @@ -112,8 +142,9 @@ Rev 1 was c8fe23a37. Rev 2 answered B1-B3 and N1-N5 (80fd45521). Changes are mar | File | Change | |---|---| -| `packages/control-board/src/board.mjs` | **New.** `createBoard({ specs, boardDir, seatsDir, discordDataRoot, isAlive, now, agentSend, exec, conversationReader })`. Returns `{ index(), markSeen(body), reply(body), conversation(url) }`. Each method is the body of a route that `createServer` runs now. No `node:http`, no listener. | -| `packages/control-board/src/serve.mjs` | Delete `createServer` (lines 229-310), `startServer` (312-322), `page` loading (`loadPage`, 224-227), and the `node:http` import. Keep the pure exports. Remove the `7331` default. | +| `packages/control-board/src/board.mjs` | **New.** `createBoard({ specs, boardDir, seatsDir, discordDataRoot, isAlive, now, agentSend, exec, conversationReader })`. Returns `{ index(), markSeen(body), reply(body), conversation(url) }`. `index`, `markSeen` and `reply` return promises and use the async paths (decision 10). Each method is the body of a route that `createServer` runs now. No `node:http`, no listener. | +| `packages/control-board/src/scan.mjs` | Adds `tmuxInspectAsync` and `scanAsync`, and one shared pure parse. Sync `tmuxInspect` and `scan` stay for the CLI. | +| `packages/control-board/src/serve.mjs` | Delete `createServer` (lines 229-310), `startServer` (312-322), `page` loading (`loadPage`, 224-227), and the `node:http` import. Keep the pure exports and the sync `replyToRow`. Add `replyToRowAsync` (decision 10). Remove the `7331` default. | | `packages/control-board/src/cli.mjs` | `serve` exits 2 with `refused: the control board no longer serves HTTP. Use the Console: node packages/webui/src/cli.mjs serve`. The `--port` and `--host` flags are removed. `scan` is unchanged. Usage text is updated. | | `packages/control-board/src/page.html` | Delete. The Console serves its own `index.html`. | | `packages/control-board/README.md` | Remove the `serve` section (lines ~107, 127), the port default and the start instructions. Keep the reply-convention text (lines 144-151). It names no start command. | @@ -172,6 +203,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`. | +| 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 @@ -191,18 +223,20 @@ must kill it is listed. A mutant that survives means the test is weak. | M9 | A README or script again tells people to run `control-board serve` | `retired.test.mjs` (3) | | 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` | +| 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. | | 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 | -Mutants M11 to M13 run in the cli suite, which needs Sage's slot. M10 runs in one webui test file. +Mutants M11 to M13 run in the cli suite, which needs Sage's slot. M10 and M14 run in one webui test file each. ## Test additions (B1, B2) | Test | File | Assertion | |---|---|---| | 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 it. The plan names the drop. The install argv has no `--board`. | -| Rollback keeps `--board` | same | Rollback to a copy that parses `--board` keeps the recorded flag. | +| 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) @@ -227,7 +261,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-M13 killed. +M1-M14 killed. ## Stop points