webby: Case 2 design r1 review (#416), changes B1-B12, and session line

2026-10-20 13:57:45 -05:00
1 parent 0599b119f4
commit f9d218ab1e
@@ -0,0 +1,173 @@
 # Webby review: Case 2 design r1 (#416)
 Webby, 2026-10-20 (from `date -u`). Independent reviewer, replacing Dewey (Mos, mos-0980).
 Subject: `agents/gosalyn/work/case2/DESIGN.md`, blob fba567f71dc1, at e997f3b76 and on
 origin/gosalyn/case2-design 88222f3f3 (same blob). Author: Gosalyn. Static review only. I ran no
 tests and changed no product code.
 Code base: `git archive 5d019d876`. `git diff --stat 5d019d876 9ae8c73ac -- packages adapters roles scripts`
 is empty, so every line below also holds at origin/refactor 9ae8c73ac. Paths are relative to `packages/`
 unless they start with `adapters/` or `roles/`.
 ## Verdict: CHANGES
 Twelve blockers. Section 6's no-schema claim holds. Section 5's write-side guardrails hold. The
 message-side guardrails have two channels the design misses (B3, B4), and the smallest slice can't
 run as written (B1, B2).
 ## Citations checked
 These hold as cited: git.mjs:8 (`protocol.allow=never` in SAFE), git.mjs:44 (any `remote.*` key
 refuses), git.mjs:55-56; project index.mjs:12, :31-84, :97, :195-234, :209, :248-261, :256-258;
 paths.mjs:7, :14-15, :34-48 (`absolute(project.root)` at :41 throws on a missing root);
 business.mjs:186-187; gate.mjs:130-167 (typed by name at :134, built-ins at :137-138);
 claude-gate.mjs:13; tools.mjs:6-7, :95-102, :186, :211-220, :271-280; vocabulary.mjs:92-97, :131;
 role.mjs:19-28, :55-66; broker.mjs:290-291, :642-650, :1378-1430, :1434-1435, :1867, :2331-2347,
 :2374-2399; schema.sql:240-250, :296-306, :310; runner.mjs:446-456; seat session.mjs:59-64;
 adapters/claude/adapter.sh:47-53; launcher.mjs:135, :236-238, :353; webui bus.js:885, :919.
 One miss: the `ls-tree` and `cat-file` reads that 1.3 cites at read.mjs:67-86 are in `blob()` at
 read.mjs:19-26 (N1).
 ## Section 6: "no schema or trigger change"
 It holds. `threads` has no definition column or check (schema.sql:240-250).
 `message_threads_match` (:296-299) and `delivery_same_thread` (:300-306) check one thread's
 instance, and a room is one thread per instance, so each insert in `room.send` passes as written.
 `thread_one_per_run` allows one held thread per run, which matches one session per agent. The only
 constraint the design must name is `UNIQUE (project, name)` (:249). `room.open` names each thread
 after its instance, so an existing thread with that name refuses (N9).
 The single-instance rule is code, not schema: broker.mjs:291, :2332-2334, launcher.mjs:135 and
 :236-237. Changing it is C2-09's job, which is why B1 matters.
 ## Jason's three guardrails
 | Guardrail | Enforced in code? |
 |---|---|
 | Agents never write prose for the user | Into the library: yes. Gate (gate.mjs:137-138), typed tools (no `write_record` without `record.write`, tools.mjs:211) and human-only verbs make an agent write impossible. In messages: only the form is bounded (M5), and two channels skip M5 (B3, B4). The 400-character limit is stated in 5.1, but the stated residual is too small (B5) |
 | Agents always refer to the source material | Structural once B6 and B7 are fixed. The quote-in-lines check at a reachable commit is real verification |
 | Tool use is guarded | Yes: gate, typed tool list, broker re-check (tools.mjs:6-7) and `#checkMessaging` (broker.mjs:1867). N6 closes a role-file gap |
 ## Blockers
 B1. The smallest slice gives Melanie no way to chat with a creative agent. `#threaded` is true only
 for definition `assistant` (broker.mjs:291). `thread.create` refuses `thread-instance-ambiguous`
 with more than one (:2334). The launcher refuses `thread-not-allowed` for any other definition
 (launcher.mjs:135, :237). C2-08 adds the `THREAD_DEFINITIONS` constant, but C2-09 is the row that
 makes the broker and launcher read it and adds `instance` to `thread.create`, and C2-09 is outside
 the slice (DESIGN 9). The Console's thread creation must pass `instance` too. Fix: put C2-09's
 broker, launcher and `thread.create` parts in the slice. The Console chat must pick the instance.
 `room.*` can stay out.
 B2. In the slice, Melanie reads raw JSON and can't see sync failures. Replies are JSON bodies (5.2),
 and only C2-10 renders them. 7.2 step 2 says "The Console shows them with file and line", but the
 library pages are C2-11b, outside the slice. A bad Obsidian push silently stops agents seeing new
 work. Fix: the slice needs a minimal cited-item renderer in the existing chat view and a sync status
 line (last valid commit, problems, `unpushed`).
 B3. `raise_decision` is a second prose channel to the human. Every agent gets it with free-text
 `question` and option `text` (tools.mjs:221, comment at :5-6). An uncited raise goes to the human.
 M3's "no write tool exists" is true, but M5 covers only `message.send`. Fix: either refuse
 `decision.raise` from creative definitions in the broker and drop the tool from their typed set, or
 put the raise text through the same guard. Test both paths.
 B4. The Console conversation view shows the model's raw text, including text M5 refused.
 webui serve.mjs:559-569 proxies `/api/conversation` to the control board, and the reader emits
 assistant text blocks (conversation/src/events.mjs:58, :82). A refused prose answer stays in the
 session file and is readable. Fix: leave creative sessions out of the catalogue on a library
 instance, or show stored messages only. Test that a creative session isn't listed.
 B5. The stated limit understates what M5 lets through. A reply holds 20 items of 400 characters,
 which is 8,000 characters. An `idea` item's text is free, and any real quote of any line satisfies
 the cite. That is a full scene per reply, not "a one-sentence suggestion" (5.1). Jason wrote
 "NEVER", and Q176 says the system enforces it. Fix: add a total-characters cap per reply and a lower
 cap for `idea` items (for example 200 characters and at most 5 items). State the residual with
 those numbers. Mos takes the residual to Jason or Melanie for acceptance, because it is their rule.
 B6. A `correction` isn't bound to the source. M5 limits the edit distance between `from` and `to`,
 but nothing requires `from` to be text in the cited lines. An agent can invent both. Fix: `from`
 must equal one cite's `quote`, after the same whitespace normalization, inside that cite's lines.
 Test that an invented `from` is refused.
 B7. `citation-unfetched` has nothing to check against. The web trail stores the URL
 (broker.mjs:1521) and a summary of status, bytes and redirects (bus/src/web.mjs:46), not the page
 text. "The quote occurs in the fetched text" needs that text. Also, Claude's built-in WebSearch and
 WebFetch are on when `policy.webBuiltin` is set (gate.mjs:28-32), and those fetches never reach the
 broker. Fix: say where fetched text is kept (bounded, per run, or a new table, and if a table then
 say so). M1 must refuse the built-in web source for creative definitions, so `research` uses only
 the broker's tools.
 B8. Remote content is a new untrusted input, and the S-rules don't check tree entries. Today only
 the bus writes record repos. Under 7.2, Obsidian or anyone with push to the Gitea repo writes too.
 A fast-forward checks out whatever the commit holds. No rule refuses symlinks (120000), gitlinks
 (160000), executables or nested `.gitattributes` and `.gitmodules`. Separately, S1 refuses the
 `.gitignore` that C2-12 commits, and Obsidian puts new notes and `.trash/` at the vault root by
 default. Fix: add a rule that every entry is a 100644 blob or a tree, and refuse those names at any
 depth. Allow `.gitignore` at the top. Commit Obsidian's new-file location and trash settings in
 `app.json`. Test each case.
 B9. The fail-closed state in 7.2 strands Melanie's writes. If remote main is ahead and invalid, the
 bus stays on the old commit (step 2). The next bus write commits on that base. The push can't
 fast-forward, so the commit is `unpushed`, then `library-diverged`, with no merge until C2-17.
 S5 makes this likely: Obsidian users create `[[stub]]` links to notes that don't exist yet, so one
 dangling link in the playground blocks the whole sync. Fix: refuse library writes while remote main
 is ahead and invalid (`library-remote-invalid`). Apply S5 to canon only, or allow unresolved links
 in `playground/`. Test both.
 B10. The 7.2 gate can't catch the obvious mutant. Tests use the file protocol through a test-only
 argument, so the https path, `GIT_ASKPASS` and the protocol override are never run. A mutant that
 sets `protocol.allow=always` passes them all. `git()` passes only `gitEnv()` plus the identity
 argument (git.mjs:10-12, seat checkout.mjs:79-89), so sync needs its own env entry for the helper.
 `GIT_TERMINAL_PROMPT=0` (checkout.mjs:85) doesn't stop askpass, which is fine. Fix: one function
 builds the production fetch and push argv and env. A test asserts that the argv holds SAFE, then
 exactly `-c protocol.https.allow=always`, and no other `protocol.*` key. Another asserts that
 `GIT_ASKPASS` names a helper outside the repo that reads the slot file. Keep the file-protocol
 switch out of any path that a business file or a bus verb can reach.
 B11. An optional `root` breaks more than the overlap loop. `ensureCheckouts` clones every entry's
 root (seat checkout.mjs:310, :319). With no root it calls `clone(project, undefined, ...)`, so "a
 project with no root gives agents no clone" (2.1) is false today. `projectFilePath(entry.root)`
 (business/src/project.mjs:9-10) throws on undefined, and it is called at business/src/cli.mjs:72,
 :78, :118, resolve.mjs:44 and instance-settings.mjs:142. Fix: C2-04 lists these consumers, each
 skips a library project explicitly, and each has a test.
 B12. Scope and crossovers can also come through front-matter fields, and in the slice a hybrid
 rests on convention. S2 and S3 speak of links only. `extends:`, `sources`, and `exclude` and `pin`
 paths are references too, and a canon book entity with `extends: playground/...` would bring
 playground text into canon lookups labelled canon. `library_manuscript` has no `scope`, so it must
 label or refuse a playground book. In the slice there is no crossover dialog (C2-11b), so a hybrid
 is a hand-written `universe.json` pushed from Obsidian, and S6 doesn't require `sanctioned`. Fix:
 put every path field under S2 and S3. Make S6 require `sanctioned`. State that in the slice a hybrid
 is Melanie's hand edit, and that only her account and the bus account can write to the Gitea repo.
 ## Non-blocking
 N1. Cite read.mjs:19-26 for the blob reads, not :67-86.
 N2. `END_PROTECTED` is `[THREAD_DEFINITION]` (vocabulary.mjs:135). Say whether creative
 definitions become end-protected when `THREAD_DEFINITIONS` replaces it.
 N3. The runner sends `(the session gave no answer)` and `The turn failed: ...` (runner.mjs:449).
 The guard will refuse both, so Melanie never learns a turn failed. Add a fixed turn-failed notice,
 and test that any notice other than the fixed bodies is refused.
 N4. Promotion moves several files. `commitFile` commits one path (index.mjs:219). C2-02 needs a
 multi-path commit.
 N5. Ids allow capitals (paths.mjs:7). Two universes that differ only by case collide on a
 case-insensitive file system. Enforce lowercase in `checkLibrary`.
 N6. A role with no `messaging` key may message anyone (role.mjs:50). M1 must require `messaging`
 for creative definitions.
 N7. C2-05b's sync-before-read needs C2-03, which its row doesn't list. C2-03 reads the remote URL
 that C2-04 adds to the business file, so either pass the URL in or order C2-04 first.
 N8. C2-06 must run the guard through both the runner's RESULT path and `send_message`, with a
 non-creative sender in a library business. Otherwise a mutant keyed on `CREATIVE_DEFINITIONS`
 passes, against 5.2's "role choice doesn't matter".
 N9. Name the refusal when `room.open` meets an existing thread with an instance's name
 (schema.sql:249).
 ## Rows and the date
 The rows are small and most gates name the right refusal. B6, B10 and N8 are the gates that miss
 their obvious mutant. The listed slice (C2-01 to C2-08, C2-12, C2-13) does not let Melanie start.
 She can write in Obsidian, but she can't reach an agent (B1) or read its answers (B2). With
 C2-09's broker part and a minimal renderer added, it can be done by 10-27 only if T1 row 1 (row
 190) lands by about 10-24, as section 9 says. Reaching the Console from her machine also needs SSO
 (Q175).
@@ -1361,3 +1361,4 @@ are never rewritten or removed; corrections are new entries.
 2026-10-20T18:34:53Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design review r2 (DESIGN.md blob 3eb718b5e131 at 1a268249e), static | changes: C1 config.json console key fails old loaders (mosaic-config.mjs:113 rejects unknown keys; key add inside phase 2 window, rollback removes it, loader test), C2 app.js signs out only on opaqueredirect so 401 never shows sign-in; notes on tests 15/7/9a, Q-B 200-HTML classification, scratch Console flags; Q-E keep iss/sub escaped and capped; verdict sent to Sage, no Gitea/queue record
 2026-10-20T18:40:20Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design review r3 (DESIGN.md blob a152fd58e18e at 817e8f51b), static | approve on condition D1 (slot file checks stay out of shared loader, shape-only there) as a line edit; D2 notes: slot root from configDir, scratch dataRoot, name discovery GETs for Mos; C1/C2 and notes verified; Mos C1 sequence confirm still open; no Gitea/queue record
 2026-10-20T18:41:20Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design D1/D2 check (DESIGN.md blob eba861f5d2bb at d55d86bb0), static git diff 817e8f51b..d55d86bb0 | ack: D1 and D2 match; design approved; Mos C1 sequence confirm still open; no Gitea/queue record
 2026-10-20T18:57:45Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r1 review for Gosalyn (#416, DESIGN.md blob fba567f71dc1 at e997f3b76 / 88222f3f3), static | changes: B1-B12, N1-N9 in agents/webby/work/case2-r1/REVIEW.md; section 6 no-schema claim holds