diff --git a/agents/webby/work/case2-r10/REVIEW.md b/agents/webby/work/case2-r10/REVIEW.md new file mode 100644 index 000000000..a666666c7 --- /dev/null +++ b/agents/webby/work/case2-r10/REVIEW.md @@ -0,0 +1,74 @@ +# Webby review: Case 2 design r10 (#416), two system units + +Webby, 2026-10-20T21:01Z (from `date -u`). Author: Gosalyn (gosalyn-0045). Commit 4281b0064 on +gosalyn/case2-design, DESIGN blob 4a1860e698e0. Diff `git diff 51006710a 4281b0064`, one commit. I +read both unit templates, scripts/webui-service.sh, webui `cli.mjs serve`, scripts/mosaic and decisions 62, +94 and 102 on origin/refactor. Static review. + +## Verdict: CHANGES (B1, B2, B3) + +The fix is the right one. Two units from the existing templates match how the stack runs today, and C2-10b +needs the Console. One copy for both units keeps the human proof working, since the bus accepts only its +own human-cli.mjs path (webui-service.sh:266-273). Making `%h` literal is correct for a system unit. The new +refusal cases are good. + +B1. The Console's boot start is Jason's call, not mine or Mos's, and the design should default to the strict +reading until he rules. Decision 94 (lead-decisions:2016-2023) says the process that starts the Console +holds human authority, and that a start through systemd gives a clean ancestry the proof accepts, "the +detachment gap decision 62 names". Decision 102 lets one named seat, Mos, start it. It calls that a +tradeoff Jason accepted, because "the human proof now rests on Jason's grant to one named seat" (:2253). A +boot-enabled system unit hands that start to PID 1 on every reboot, with no named starter. That extends a +human-authority boundary, so under AGENTS.md it needs an owner decision. Decision 94 also accepts the +local-forgery limit only "for this PoC on dragon-lin under Jason's user ... this decision doesn't carry the +limit there" (:2058-2062). On .54 any process that reaches the Console port can send as Melanie, and +`melanie`'s Hermes processes and sudo are on that host until #423 step 5. Nobody has accepted that limit +for .54. + +Fix: render the Console unit with no `[Install]` section, so a human or Mos starts it at step 9 and after +each boot. That fails closed and is cheap to reverse, because adding `[Install]` later is one re-render. +Put two questions to Jason through Mos: may a boot start stand in for the human's start on .54, and does he +accept the local-forgery limit there? The bus unit can stay boot-enabled, since it holds no human +capability. + +B2. `@PATH@`, `@NODE@` and `@FLAGS@` have no render rule. Both templates set +`Environment=PATH=@PATH@` (mosaic-bus.service.in:15, mosaic-webui.service.in:25). The bus `ExecStart` is +scripts/mosaic, a bash script that runs `exec node` from PATH (scripts/mosaic:22-40). So the bus's node is +whatever PATH the render writes, not `--node`. Row 195 found v22 on .53, and the precheck checks +`node --version` on ``'s shell PATH, not the unit's. Fix: say `@PATH@` is `dirname(--node)` followed +by `/usr/local/bin:/usr/bin:/bin`, as webui-service.sh:246-248 does, and `@NODE@` is `--node`. Say +`@FLAGS@` is the checked serve flags, and refuse any `@[A-Z_]+@` left in either render, not only `%h`. Add +a test that the bus unit's PATH starts with the `--node` directory. + +B3. The hardening lines are never listed. In r9 they lived in the new template that C2-13 owned. r10 drops +that template, and the render "adds ... the hardening lines", but neither the design nor the brief says +which ones. The test "a template with a hardening line removed" has nothing to remove: the templates carry +only `NoNewPrivileges=yes`. Some common lines break sessions. `RestrictNamespaces=`, `PrivateUsers=` or a +`SystemCallFilter=` without the namespace calls stops bwrap, and then no role starts. The bwrap precheck +runs in ``'s shell, outside the unit's sandbox, so it can't catch that. Fix: list the exact lines in +the brief (for example `ProtectHome=yes`, `ProtectSystem=strict` with `ReadWritePaths=`, +`PrivateTmp=yes`, `NoNewPrivileges=yes`), say none of them restricts user namespaces, and add a step 9 +check that one session starts under the bus unit. + +## Non-blocking + +N1. Both templates already end with `[Install]` `WantedBy=default.target` (bus :41-42, webui :39-40). The +render should replace that line with `multi-user.target`, not add a second one. Under B1 it removes the +Console's section. + +N2. The render adds no `After=` or `Wants=`, so at boot the two units start in parallel. That is harmless +today, because serve calls the bus per request (webui cli.mjs, `busCaller`) and starts without it. But the +stated order applies only to a manual step 9. Either add `After=mosaic-bus@.service` to the +Console unit, or say that boot order doesn't matter. + +N3. "The serve flags pass scripts/webui-service.sh's own flag checks" can't be done by calling that +script. `serve_flags` (:146-207) is internal. Its proof-file check compares the owner to `id -u` (:201), +which is root under case2-unit.sh. `render` finds node with `command -v` (:237), not `--node`. And +webui-service.sh isn't in the files owned. Say whether case2-unit.sh copies the checks, with tests, or +whether the row may move `serve_flags` into a sourced file. Also state which serve flags Case 2 uses. + +N4. With a copy, webui-service.sh passes `--checkout ` (:251). Her host has no canonical +checkout, so the queue read runs from the copy, and the queue store refuses that. Say that the Console unit +gets no `--checkout`, and that the queue panel isn't part of Case 2. + +N5. A re-pin must move both units to the same copy, or the human proof fails (webui-service.sh:266-273). +Have case2-unit.sh refuse a render whose two units name different copy ids, and test it. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 8c06cfed3..bb6b3947c 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1390,3 +1390,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T20:49Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r9 review (#416) for Gosalyn (29e85196b, blob 60a2d6cb751e, incl 13a98bfd0 29404f996), static | approve, N1-N3; notes agents/webby/work/case2-r9/REVIEW.md 2026-10-20T20:50Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation review r3 for Herb (herb-0026, N1-N3 delta, BUILD.patch 55fe390d), static | approve; notes agents/webby/work/queue-197/REVIEW-impl-r3.md 2026-10-20T20:55Z | Webby (Opus 5.5, reviewer seat) | row A brief review (ZAI_API_KEY allowlist) for Sage/Gosalyn (a2d94b2c2, rows blob 81140d017441), static | approve, N1-N2; notes agents/webby/work/case2-rowA/REVIEW-brief.md +2026-10-20T21:01Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r10 review (#416) for Gosalyn (4281b0064, blob 4a1860e698e0, two system units), static | changes B1 Console boot start is Jason's call, B2 PATH/NODE/FLAGS render, B3 hardening lines unlisted, N1-N5; notes agents/webby/work/case2-r10/REVIEW.md