diff --git a/agents/webby/work/case2-r9/REVIEW.md b/agents/webby/work/case2-r9/REVIEW.md new file mode 100644 index 000000000..572d4b8ad --- /dev/null +++ b/agents/webby/work/case2-r9/REVIEW.md @@ -0,0 +1,61 @@ +# Webby review: Case 2 design r9 (#416) + +Webby, 2026-10-20T20:48Z (from `date -u`). Author: Gosalyn (gosalyn-0035). Commit 29e85196b on +gosalyn/case2-design, DESIGN blob 60a2d6cb751e. Round 8: case2-r8/REVIEW.md, approve on eab6e1a3a. + +Scope: `git diff eab6e1a3a 29e85196b` on DESIGN.md and docs/plans/2026-10-20_case2-rows.md, including +13a98bfd0 and 29404f996 as asked. Row A's and row B's briefs (f325657c7) are not part of this round. I +read their citations only where the design shares them. This was a static review. I ran nothing. + +## Verdict: APPROVE + +Model path (section 9). The citations hold on origin/refactor 088087ac9: +- `ENV_ALLOW` (packages/seat/src/session.mjs:83-88) has no `ZAI_API_KEY`, so today the key is dropped. +- pi-ai reads `zai` from `ZAI_API_KEY` (env-api-keys.js:89), and `glm-5.3` is a zai model id (see N1 for + the file path). +- pi's harness sets `countsAccount` to false (pi/index.mjs:42), so the account check at launcher.mjs:296-308 + never runs for pi. `preflight()` (:26) checks only the binary. `credentialStatus` (launcher.mjs:343) is + metadata passed into the launch object (:385), not a refusal. So "a pi launch has no credential check" + is true, and the precheck key-file line is the right place to catch a missing key. +- `PI_PROVIDER` comes from `session.provider` (pi/index.mjs:57). + +2d. The key file is the path the bus unit already loads (mosaic-bus.service.in:24), and the `%h` in that +line resolves to /srv/mosaic because `` is mosaic with that home (DESIGN 1010-1012). The design +says plainly that melanie's sudo and her Hermes processes can read it until #423 step 5 (:1042). On +Gosalyn's naming question, I'd keep `claude-session.env`. A new name means a second `EnvironmentFile=` +line, which adds a line to case2-unit.sh's fixed set and departs from the bus unit. The cost of keeping +it is a misleading name. A README line saying the file holds every session provider key, not only +Claude's, fixes that, and it can ride row A's README edit. + +C2-13 and the rows doc: +- The marker prompt check now runs only for a claude-code creative instance, and a test shows a missing + claude passes when there is none. That's right, since Bean and Chet run pi. +- The key-file precheck has refusal cases for missing, 0644, symlink and empty, plus a search of the test + output for the fixture value. +- `EnvironmentFile=` joins the fixed lines, with removed and pointing-elsewhere cases. +- The Exec prefix rule covers the whole leading run, with `+` and `-+` cases, as webby-0027 asked. +- 13a98bfd0, `--profile factory|library`. `factory` requires the tester and refuses a library project + (`profile-library-business`), and `library` refuses the tester. Each direction has a test, so a wrong + flag can't reach a tester-on Case 2 install. +- 13a98bfd0, node >= 24 and bwrap prechecks. Both have fake-binary refusal cases, and a bwrap refusal + prints the userns sysctl and PATH state. +- 29404f996, the copy id. The rule matches `copy_root` (scripts/stack-copy.sh:20): 40 lowercase hex and + `scripts/mosaic` present. The tests cover short, uppercase, `/`, `..` and missing. + +C2-20. The Hermes name count prints one count per persona file and never the line, number or word, and +the test checks the output holds none of the matched words. The list comes from code at df5700ebe, not +her profiles, so the script carries no persona text. + +## Non-blocking + +N1. The model data file is `dist/providers/data/zai.json`, not `data/zai.json` (DESIGN:1023, and row A's +brief). The id `glm-5.3` is there. Fix the path when the file is next touched. + +N2. `copy_root` reads `COPIES` as `${XDG_DATA_HOME:-$HOME/.local/share}/mosaic-stack/copies` +(stack-copy.sh:6), while case2-unit.sh fixes `/.local/share/mosaic-stack/copies`. If mosaic's +login env ever sets `XDG_DATA_HOME`, copies land elsewhere and the unit refuses. That fails closed, so it +doesn't block. The install could check `XDG_DATA_HOME` is unset, or the rows doc could say why it's safe. + +N3. "Sets a nonempty `ZAI_API_KEY`" should parse the file the way systemd does: the last assignment wins, +and quotes are stripped. A file with `ZAI_API_KEY=x` then `ZAI_API_KEY=` passes a first-match grep but +gives the session an empty key. Add that case to the refusal list. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 583388a10..9ab4c1568 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1387,3 +1387,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T20:24:21Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation packet review for Herb (BUILD.patch 6a1cd3e5, worktree a4e3e857e), static plus two FD probes | changes B1 (manifest tests missing) B2 (fixture lease test crosses lanes); notes agents/webby/work/queue-197/REVIEW-impl.md 2026-10-20T20:31:25Z | Webby (Opus 5.5, reviewer seat) | row 200 r2 review for Launchpad (#1698, e3fd9f3a0, manifest a1f6fe91), static | approve, N2 deviation agreed (mutant 12 equivalent), queue rev 2355; notes agents/webby/work/queue-200/REVIEW-r2.md 2026-10-20T20:45:28Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation review r2 for Herb (herb-0023, BUILD.patch 0b3871bd, SOURCE.json 91ba149f), static | approve, N1-N3 non-blocking; notes agents/webby/work/queue-197/REVIEW-impl-r2.md +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