duckworth: federation design r1 review (duckworth-0001), changes B1-B5 on Gizmoduck 6925f78f0

2026-10-20 13:51:09 -05:00
1 parent 99772421ab
commit b402197ff3
@@ -0,0 +1,91 @@
 # Review: Gizmoduck federation design r1 (duckworth-0001)
 Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review, no code run.
 Subject: agents/gizmoduck/work/federation/DESIGN.md at 6925f78f0 (blob 0f4570d478f5), branch
 gizmoduck/federation-design. Code at origin/refactor 9ae8c73ac. Sage's Q175 design read at d55d86bb0.
 Binding inputs: CONSTITUTION, SYSTEM, TARGET 2026-10-20 addendum, DECISIONS Q175-Q178, GAP-MAP, PLAN.
 ## Verdict: changes, B1-B5
 The direction is right. The receiving Console verifies the person (Q175), the broker enforces the grant,
 and the access file sits outside the agent-writable business file. Five gaps stop it from meeting criteria
 1, 2 and 5 as written.
 ## Blockers
 B1. The bus takes the user's identity from the caller. `user-cli.mjs` carries `{ business, user }` and is
 "proven the way tester-cli.mjs is" (DESIGN.md:171-172). The tester proof shows only that some process runs
 the tester CLI. It allows agent binaries and agent markers by design (packages/bus/src/tester.mjs:5-9, :57).
 The `user` field is whatever that process says. Any T3 seat or agent shell on the hub's uid can bind as any
 granted user. That is a forwarded identity, which Q175 rules out. Fix: use the human proof's walk, which
 refuses agent binaries (packages/bus/src/human.mjs:83-88). Bind the user CLI to the Console's tree, the
 way #225 binds human-cli. State the residual risk in the design, as Sage's design section 8 does. Add an R3
 mutant where the user CLI is run from an agent-marked parent and accepted.
 B2. "No new ownership code" (DESIGN.md:180-181) is false. `#owner` returns `'tester'` or `s.human`
 (packages/bus/src/broker.mjs:364-366), which is undefined for a user. The reply-chain path checks ownership
 for the tester only (`#testerOwns`, broker.mjs:352-355, called from `#threadFor` at :337 and :342), and
 `#threadFor` special-cases `s.human` only (:326). A user principal needs explicit handling in all three.
 Without it, a `message.send` with `in_reply_to` can land in another user's thread. Fix: name the changes
 in 3.6. Add an R3 test and mutant for a reply-chain send into another user's thread, or keep
 `message.send` off every level's list and state that.
 B3. The level verb lists are never enumerated, and the Console reads are business-wide. "Read the board"
 for a viewer (DESIGN.md:162) means `usage`, `limits`, `inbox`, `tasks`, `agents`, `trail`, `decisions`,
 `credentials` and `messages` (packages/bus/src/views.mjs:3-13). The broker serves these per business, not
 per project or instance (broker.mjs:2144, :2174, :2195, :2217). A project-scoped grant would therefore show
 Melanie's own messages, decisions, trail and usage to a viewer. Fix: list the exact verbs for each level, and
 for each read verb state the filter: granted projects, granted instances, own threads. Otherwise refuse the
 verb. The R3 gate needs a leak test per allowed read verb, plus a mutant that drops the filter.
 B4. Revoking a grant doesn't reliably end runs (criterion 2).
 - (a) No trigger. The broker re-reads access.json only when a request arrives (DESIGN.md:174-176). Runs end
  only "on a revision where a user loses a grant" (:193-194). A hand edit, which :167 allows, fires nothing
  until the next request. Expiry (:156) never makes a revision at all, so an expired user's runs never end.
 - (b) Ending a claim stops the run only "at its next bus call" (broker.mjs:2273-2275), so a turn already in
  flight keeps working.
 - (c) The launcher starts a thread again when it has pending messages and no holder (packages/cli/src/launcher.mjs:533),
  launched as the business human (:542). Its `startedFor` guard lives in memory and clears on restart. A
  revoked user's queued message can therefore start a fresh run.
 Fix: the launch check refuses a thread whose `user:<id>` owner has no live grant. Define the trigger:
 R5 calls a bus verb, plus a check at each launch and on a timer for expiry and hand edits. Say whether the
 runner gets SIGTERM. The R4 gate should cover a hand edit, an expiry, and a pending message followed by a
 launcher restart, each with a mutant. R4 depends on whatever row provides the trigger, so fix the order.
 B5. On one uid, Melanie's hub isn't isolated from Jason's factory (criterion 5). The design says so
 (DESIGN.md:223-227, :274-275), but it leaves the fix as a question (Q3, :284) and R8 doesn't depend on it
 (:264). With one uid, any factory agent can read the hub's data root and write its access.json. It can also
 run the hub's human-cli and act as the hub owner. Sage's design section 8 states the same limit for the bus
 human proof. Grants don't matter while that holds. Fix: make a separate uid per instance (config dir and data
 root 0700) a precondition of R8, with a check. A process on the factory's uid must be refused by the hub's
 proof (`p.uid !== process.getuid()`, human.mjs:67) and must be unable to read the hub data root. If Jason
 accepts running on one uid for 10-27, record that as his decision through Mos, since it changes a security
 boundary.
 ## Notes (do not block)
 N1. The v1 claims hold. The OIDs are at ca.service.ts:22-24, the guard reads the TLS peer cert at
 federation-auth.guard.ts:93-106, and main.ts:50-53 builds the adapter with no TLS. `requestCert` appears
 only in a guard comment (:36). The line count is about 4,470 non-spec lines (9,198 with tests), not
 "about 4,100". The verdict stands: none of the code is reusable.
 N2. The other cited lines match: config.mjs:19, runtime.mjs:140 and :169, requirements.json:61,
 business.mjs:160, broker.mjs:298-306, 356-362, 367-378, 526-538, 596-609, 763-799, webui bus.mjs:1-9 and
 :27-31, mosaic-config.mjs:112-116, organization.mjs:83, bus-service.sh:200. One error: business.mjs:24
 (DESIGN.md:220) is a comment. `configDir` is at :25-28.
 N3. R1 is marked "depends on none", yet it validates against `console.sso` providers and `federation.peers`.
 Pass both in as arguments so R1 stays pure.
 N4. "Human-only" in R5 is cosmetic while one uid holds (B5). A direct file write bypasses both the CLI and
 the receipt. Say so.
 N5. Run the same file checks on access.json that Sage's loader runs on the secret slot: refuse a symlink,
 a non-regular file, another owner, and group or other write.
 N6. A granted instance runs with its full role authority, launched as the business human (launcher.mjs:542).
 `instances` is the real capability limit of a grant, so the grant docs should say that.
 N7. `console.sso` sits in config.json, so removing a provider takes effect only after a Console restart.
 :195-196 implies it takes effect on the next request.
 N8. Scope: the slice amounts to T1.3's multi-user access, plus `home` and peer labels. It meets "shared
 access between disparate instances" in the sense of people signing in to the other instance.
 "Federation is configurable" is met only through grants and labels. Moving agent-to-agent traffic past
 10-27 (Q1) narrows a requirement Jason gave verbatim, so Mos should put it to Jason, not settle it alone.
 N9. Audience binding (3.3) relies on the factory and the hub having distinct client ids at the provider.
 Add a line to R8's checklist that confirms the two client ids differ.
@@ -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:51:09Z | Duckworth (Opus 5.5) | Gizmoduck federation design r1 review (DESIGN.md blob 0f4570d478f5 at 6925f78f0, code at 9ae8c73ac), static | changes B1-B5 (forwarded user identity, ownership code, business-wide reads, revocation ends no runs reliably, one-uid isolation); notes N1-N9; agents/duckworth/work/federation-r1/REVIEW.md