diff --git a/agents/webby/work/case2-r2/REVIEW.md b/agents/webby/work/case2-r2/REVIEW.md new file mode 100644 index 000000000..acd2e9559 --- /dev/null +++ b/agents/webby/work/case2-r2/REVIEW.md @@ -0,0 +1,85 @@ +# Webby review: Case 2 design r2 (#416, creative writing hub) + +Webby, 2026-10-20T19:28Z (from `date -u`). Independent reviewer. Author: Gosalyn. +Subject: `agents/gosalyn/work/case2/DESIGN.md` at c1913fb07 on gosalyn/case2-design, blob +05a79cbca624, sha256 bda7a717...4055 (verified). Code read at 9ae8c73ac, where the citations point. +Static review. I ran nothing. I read the whole file. The r1 items I don't name below read as fixed. + +## Verdict: CHANGES + +Four blockers. Three are in the persona path, which is new in r2. The data model rewrite onto her +real repo is the right call, and the 1.3 claim mostly holds (see "The three questions"). + +## Blockers + +B1. A top-level playground has no universe. 1.1 (:169-170) suggests "one at the top", but 1.2 +(:191) gives a file the universe whose `books` list holds its book folder. A file under a top-level +prefix has no book folder, so `library_find` (universe required) can't return it, and the cite +check's one-universe rule (:464) has nothing to compare. Fix: put `playground` inside each universe +entry, or refuse a prefix that isn't inside a book folder. Also give playground the `.md` condition +canon has (:187). As written, a `.canvas` under a playground prefix is playground and citable. + +B2. Where `persona.text` lives, and who can read it. Instance entries take inline `vars` only +(business.mjs:89 closes the keys). Nothing lets an entry point at a file, so "a 0600 file that the +business file entry takes" (:665, :722-723) has no mechanism. Inline, the text is read by more than +the design says: +- The tester's `instances` read is allowed (broker.mjs:103) and returns every instance's vars + whole (instance-settings.mjs:58-63). That breaks "No agent sees the persona text" (:724) on any + host with `bus.tester` on (vars.mjs, `bus.tester`). +- Every organization write logs old and new values of each changed path in + `businesses/.history//changes.jsonl` (organization.mjs:70-80, :201-204). +Fix: say the text sits inline in the business file, or add a file-reference var with its own +checks. Then either refuse persona keys in the tester's reads, or make C2-13's precheck refuse +`bus.tester` on ``. State the mode of the business file and `.history`. Test it in C2-08. + +B3. Name the places `THREAD_DEFINITION` must stay `assistant`. Section 4 (:404-406) replaces it +"wherever it means may hold a thread" and keeps only tools.mjs:186. launch-settings.mjs:140, :186 +and :191 also use it. They let `organization.set`'s `assistants` key set an assistant's whole vars, +which the chief of staff holds (`ORGANIZATION_KEYS`, broker.mjs:145, :1266-1271, Q169). Read as "may hold a thread", that +lets the chief of staff rewrite Bean's `persona.text` and model. Fix: list those three lines as +staying `assistant`, and add a C2-08 test that `assistants` with `bean` refuses +`organization-not-assistant`. +The same change has a second effect the design doesn't state. With `END_PROTECTED` set to +`THREAD_DEFINITIONS` (:407-408), `instances.set` refuses to add or change a creative instance +(instance-settings.mjs:23, :109), and `definitions()` marks all six not accepted (:52). So her model +choice for Bean, a persona edit and adding a specialist later are all hand edits of the business +file. 4 (:394) and 10 step 7 read as if she can pick specialists later. Say how that happens. + +B4. Name the persona block's exact place. `prompt()` is the contract (bundle.mjs:177), then +`## This session` (:179) with the session lines. The last of those says "end each turn with the +answer itself" (:188), then the end_run and review-loop lines follow. "After the contract" can +mean before `## This session` or at the very end, and the C2-08 snapshot depends on which. I'd put +it between the contract and `## This session`, so the broker's facts come after the persona. Fence +it the way skills are (`SKILL_END`, :107, :170), and refuse persona text that holds the end marker +or a line starting `## `. Hermes text is 8,000 characters that nobody here has read, so it can't be +allowed to forge a section. The snapshot then pins the order: contract, persona, session. + +## The three questions + +1.3, reporting is enough. For the agent-side guarantee, yes. Scope comes from the path alone, and +the cite check (:463) refuses a playground cite on `fact`, `finding` and `correction` whatever her +files contain. None of R1-R7 changes a scope. Holding back her head would add nothing to that +check. Two limits belong in the text (N1, N2). + +M9. It holds today. Every organization setter builds its document from the file and changes only +its own keys (broker.mjs:1261-1269, instance-settings.mjs:93-121, launch-settings.mjs:182-191), so +no verb can change `projects` now. M9 guards against a future setter. Place it where that setter +can't skip it (N3). + +Persona order. See B4. + +## Non-blocking + +N1. `library_get` takes only `path` (:346), so its `links_out` and `links_in` are the one place a +lookup can name a file in another universe or an unmapped file. Say both lists drop those targets +(R6, R2), and add a C2-05a test. +N2. Scope is the current mapping applied to the path. A note she moves into a book folder is canon +on her next push, and a mapping edit re-scopes old commits for the cite check. Both are right, and +both are hers. Put the first in the setup sheet (C2-12). +N3. Put M9 in `organizationWriter.write` (organization.mjs:177), comparing `canonicalJson` of old +and new `projects`, hybrid entries included. Since no verb can build the change, the C2-04 test +calls `writer.write` directly with a changed playground prefix, and one with a changed hybrid. +N4. Obsidian resolves a shortest-path link across the whole vault, which is `~/src` with her other +repos in it. The reader sees one repo. A base name unique in the books repo but not in the vault +can resolve to a different file than Obsidian shows her. Count those in the survey's link-form +counts if the survey can see it, or note it in the setup sheet. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index d6a48217f..ac3d6e3ba 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1366,3 +1366,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T19:20:19Z | Webby (Opus 5.5, reviewer seat) | row 197 design review for Herb (#1695, DESIGN.md sha256 64a429a3 at 85a6eb0a9), static | changes: B1 (temp root under /tmp hides sandboxed run dirs), N1-N5 in agents/webby/work/queue-197/REVIEW.md 2026-10-20T19:23:14Z | Webby (Opus 5.5, reviewer seat) | row 198 inventory and design review for Bushroot (#1696, INVENTORY 76b630ce at 004f133bd, DESIGN c2ea3df5 at c8fe23a37), static | inventory approved; design changes B1-B3 (copy lacks board code, re-pin carries --board, POST placement vs serve.mjs:558), N1-N5 in agents/webby/work/queue-198/REVIEW.md 2026-10-20T19:24:03Z | Webby (Opus 5.5, reviewer seat) | row 197 design r2 re-review for Herb (#1695, DESIGN.md sha256 732ee0e2 at 6ad25d745), static | approve: B1 fixed, N1-N5 in; one manifest note in agents/webby/work/queue-197/REVIEW-r2.md +2026-10-20T19:29:11Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r2 review for Gosalyn (#416, DESIGN.md sha256 bda7a717 at c1913fb07), static | changes: B1 top-level playground has no universe, B2 persona.text storage and tester read, B3 THREAD_DEFINITION in launch-settings must stay assistant, B4 persona block position and fence; N1-N4; notes agents/webby/work/case2-r2/REVIEW.md