diff --git a/agents/webby/work/case2-r3/REVIEW.md b/agents/webby/work/case2-r3/REVIEW.md new file mode 100644 index 000000000..86d25e929 --- /dev/null +++ b/agents/webby/work/case2-r3/REVIEW.md @@ -0,0 +1,79 @@ +# Webby review: Case 2 design r3 (Vikunja #416) + +Webby, 2026-10-20T19:43Z (from `date -u`). Author: Gosalyn. Subject: `agents/gosalyn/work/case2/DESIGN.md` at +2958acb81 on gosalyn/case2-design, blob 60bc7d682e0d, sha256 f368c27d...c325 (verified). Read as the diff from +c1913fb07 (r2, my REVIEW at 162ce46db), with the code at 9ae8c73ac. Static review. I ran nothing. + +## Verdict: CHANGES + +My r2 B1, B2 and B4 are fixed, and so are N1 to N4. B3 is fixed for the three paths it names, but it misses a +fourth (B1 below). + +- r2 B1. Playground prefixes now sit inside a universe and must start with one of its book folders, or the + file refuses with `library-playground-outside-book`. A `.canvas` under a prefix is unmapped. 1.2's universe + rule then holds for playground files. +- r2 B2. The text moves to `/personas//.md`, and the vars hold only the name and + the digest. I checked the modes against the code. `durable` (organization.mjs:59-61) and `log` (:138-141) + open with 0600 and O_NOFOLLOW. `log` refuses a group or other bit. `directory` (:39, :45) makes 0700 and + refuses a loose one. The `instances` read (instance-settings.mjs:61-63) copies vars whole, so it now carries + only the digest. `previous.json` (organization.mjs:197-198) holds the old business file, which has no text + either. +- r2 B3. launch-settings.mjs:140, :186 and :191 stay `THREAD_DEFINITION`. `organization.set`'s `instances` key + refuses a creative instance for every actor. `instances.set` gets the human-only creative branch. +- r2 B4. The fence follows `SKILL_END` (bundle.mjs:107, :170). The block sits between `contract.trimEnd()` + (:177) and "## This session" (:179). The snapshot pins that order. +- The correction is right. broker.mjs:147-149 says the chief of staff or a human-resolved decision ends an + `END_PROTECTED` instance. + +## Blocker + +B1. `routes.set` lets the tester change a creative instance's harness, model and thinking. It is in +`TESTER_ORGANIZATION_VERBS` (broker.mjs:103), and the gate at :1259 lets a tester through. `setRoutes` +(launch-settings.mjs:240-272) has no protected-definition check. It replaces the route vars of any named +instance, then writes at :277. So the design's 4 is wrong in two places: +- DESIGN.md:474-493 lists every path to a creative instance and leaves this one out. +- DESIGN.md:493 says that with `bus.tester` on, the tester "could change no creative instance". + +M10 (DESIGN.md:563) has no test for it. `bus.tester` is off on her business and C2-13 refuses it, so this is +not reachable there today. But the design claims the rule holds even if testing were on, and on any library +business that turns testing on, it doesn't. + +Fix: in C2-08, have `setRoutes` refuse a `CREATIVE_DEFINITIONS` instance from actor `tester` with +`organization-creative-human-only`, as the `instances.set` branch does. The human keeps `routes.set`, which is +the Console's routing table. Add the path to the list at 474, fix 493, and add the case to M10's tests: the +tester's `routes.set` on `bean` refuses, the file is unchanged, and the human's still works. + +## Your two questions + +Keying the creative branch on actor `"tester"` is enough. broker.mjs:1272 sets the actor to `"tester"` only +for a tester session. The business load check refuses a human id of `tester` (broker.mjs:245) and an instance +of that name (:248). instance-settings.mjs:86 refuses one too. So the string can't name the human, and the +writer is the only place that knows the definition, because it reads the document. A broker-side check would +have to read the file a second time to learn the definition. The one thing to add: make C2-08's tester case go +through the broker with a bound tester session, not just `set(..., "tester")`, so a change to the actor line at +:1272 fails the test. The same goes for B1's case. + +The `# `/`## ` refusal is safe, because it fails closed, but it may refuse her real personas. The fence alone +stops a section escape, because nothing after `` belongs to the persona. The heading rule +guards against a model reading a `## ` line inside the fence as a new section, which is reasonable. I don't +know what her Hermes persona files look like. If they use Markdown headings, C2-20 stops at the fence check +(DESIGN.md row C2-20) and the design names no next step. See N2. + +## Non-blocking + +N1. The chief of staff can change which model a creative instance runs without touching the instance. +`organization.set` takes `models` (broker.mjs:145, ORGANIZATION_KEYS), and launch-settings.mjs:169 replaces +the whole `launch.models` table. If `bean`'s `model` var is a key the chief of staff rewrites, Bean runs the new +model. That matches what 4 promises, since the instance and the persona stay unchanged, but mos-1033's "only +the human" reads wider. Either say in 4 that the models table is shared and the chief of staff may retune it, +or refuse a `models` change to a key a creative instance names. Mos's call, so I'd put it to him. +N2. For C2-20, count the lines starting `# ` or `## ` in each persona and print the count with the byte count, +before Jason's go. Then state the remedy in 9.1. That could be Melanie or Jason editing the text, or a recorded +rewrite to `### `. Any rewrite needs her say, because the text is hers. +N3. "Only Bean's or Chet's own session sees it" (DESIGN.md:866) needs a qualifier. `buildBundle` writes +prompt.md, persona included, once per run into the run directory at 0600 (bundle.mjs:241-244, :265). Run +directories are write-once and stay until `prune`. The claude adapter also passes prompt.md as one argument +(bundle.mjs:104-105), and that argument is visible in `/proc//cmdline` to other local users. No verb reads +either copy, and on her VM the only other user is `melanie`, whose personas they are. So this doesn't change the +design's safety. The text should name both copies, and the C2-13 precheck should check the run root's mode +with the rest. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 02b45eeba..516ac0660 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1374,3 +1374,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T19:38:46Z | Webby (Opus 5.5, reviewer seat) | row 194 design r4 re-review for Honker (#1692, DESIGN.md 824ef884 and PRD-1.7-DRAFT.md 379cc318 at 7d0788236), static | approve: HIRE_ALLOWED allowlist checked against role files, fire order A-B-C repeatable; N1 launch should refuse a listed instance mid-fire, N2 hire-time check should require role version 2; notes agents/webby/work/queue-194/REVIEW-r4.md 2026-10-20T19:39:47Z | Webby (Opus 5.5, reviewer seat) | row 198 design rev 4 re-review for Bushroot (#1696, DESIGN.md 5ef6436f at 9af75dc72), static | changes: B1 and N1 fixed, --board parse check right; new B1 parses_flag swap at stack-release.sh:670 breaks the tester-port fixtures (stack-release.test.mjs:265, T37), keep it to --board; N1 index coalescing can return a scan from before markSeen's write; notes agents/webby/work/queue-198/REVIEW-r4.md 2026-10-20T19:40:29Z | Webby (Opus 5.5, reviewer seat) | row 197 manifest r2 re-review for Herb (#1695, MANIFEST.md 2a1a3ad0 at bdc9cd56a), static | approve: literal Board :9 URLs kept, leases only at the close-rebind probes (preview.test:208, serve.test:100), T1 :9 preflight; P3 row 198 order per sage-1384; notes agents/webby/work/queue-197/REVIEW-manifest-r2.md +2026-10-20T19:43:52Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r3 for Gosalyn (#416, DESIGN.md sha256 f368c27d at 2958acb81), static | CHANGES B1 (routes.set lets the tester change a creative instance's route vars), N1-N3; notes agents/webby/work/case2-r3/REVIEW.md