diff --git a/agents/duckworth/work/row193/REVIEW-r4.md b/agents/duckworth/work/row193/REVIEW-r4.md new file mode 100644 index 000000000..dac2dcd62 --- /dev/null +++ b/agents/duckworth/work/row193/REVIEW-r4.md @@ -0,0 +1,18 @@ +# Review: Morgana row 193 design r4 (duckworth-0014) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review, no code run. +Subject: agents/morgana/work/queue-193/DESIGN.md, diff 76f4605e1..edc794933 (one commit, morgana-0009). +DESIGN.md at edc794933 has sha256 c18d27b0…b613a, checked. Against duckworth-0011 (REVIEW-r3.md, 912b1d325). + +## Verdict: approve + +## r3 codes + +- B3 resolved (5.7 table and sentence). The `project.files` and `project.file` row is in, marked + governance, visible. `message.receive` and `message.read` are on the `messages` row. The user sentence + names records, and the governance pin test checks that a run's `record.write` shows in the owner's + `project.file`. +- N5 done. The gated-raise paragraph cites FED r4.5 (acd5f1f42) and drops the conditional. It matches what + I approved in duckworth-0012. + +FED R6's notice should use this sentence (duckworth-0012 N21). diff --git a/agents/duckworth/work/row199/REVIEW.md b/agents/duckworth/work/row199/REVIEW.md new file mode 100644 index 000000000..a3db2248c --- /dev/null +++ b/agents/duckworth/work/row199/REVIEW.md @@ -0,0 +1,69 @@ +# Review: Megavolt row 199 design r1, per-agent credentials and remote push (duckworth-0013) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review, no code run. +Subject: agents/megavolt/work/queue-199/DESIGN.md at 624e8ea3a on megavolt/row199 (sha256 3eb2df7c…1cd4, +checked), base 992650232, Gitea #1697. Code read at 992650232. + +## Verdict: changes, B1 and B2 + +The shape is right. The path is derived from the cap, never named in a request. No `gitea` or `vikunja` +token enters a sandbox. The destination ref is built by the host. `publish` is no-force. E1 to E10 match the +code where I checked them. The launcher and its git socket run in the bus process (cli.mjs:504-506), so +`useFor` from `publish` doesn't move a token across processes. Two things block. + +## Blockers + +B1. A `store` reference with no file stops the whole bus at start. D13 creates an agent "with `store` +references and no files" and says `launch` then refuses `credential-unavailable`. `revoke` (section 6) +unlinks the file and leaves the reference. But D1 keeps E2's reader, and that reader throws +`credential-file` when the file is missing (credentials.mjs:38-43). It runs inside `startBroker`'s try +(runtime.mjs:89), so the bus refuses to start. The fault doesn't show at the verb. It shows at the next +restart, for every agent of the business. Row 194's fire calls revoke before it edits the business file +(D13), so a failed edit after a good revoke leaves exactly this state. +Fix: say what a `store` reference with no file means. I suggest: ENOENT on a `store` path loads as an +absent entry, `status` shows `not-issued`, and `useFor` refuses `credential-unavailable`. Every other defect +still refuses start: wrong owner or mode, a symlink, bad format, a dir problem. `{file}` and `{env}` stay as +they are. T1 and T5 gain a bus restart after create-without-issue and after revoke. Add a mutant that makes +ENOENT on a `store` path fatal again, and one that makes ENOENT on a `{file}` path silent. + +B2. D9 step 5 doesn't bind the token to the remote's host. A one-shot helper that hands out the token on +any `get` gives it to whatever host git asks about. Git follows a redirect on the first request +(`http.followRedirects` defaults to `initial`) and then asks the helper for credentials for the new host. +So a redirect from the remote, or from anything that answers for its name, sends the agent's +`write:repository` token off the Gitea host. Separately, "E8's safe env" can't push to https at all: +working.mjs:95 sets `protocol.allow=never` with only `file` allowed. +Fix, in D9 step 5: +- `publish` sets `protocol.https.allow=always` and nothing else beyond SAFE. +- It sets `http.followRedirects=false`. +- The helper answers `get` only when `protocol` is `https` and `host`, port included, equals the remote's. + It answers nothing for `store` and `erase`. +- Step 2's host check compares the port too. +- T3 gains a fake remote that 302s to a second 127.0.0.1 port, where no basic auth may arrive. +- Add a mutant whose helper answers any host. + +## Notes (do not block) + +N1. Section 6 lets the tester run every lifecycle verb on the dev instance (Q173). Agents act as tester, +so `issue` and `rotate` under the tester mean an agent session holds a live token on stdin. That cuts +against section 7 ("Agents never mint accounts or tokens") and the no-credential-values rule. I suggest +the tester gets `status` and `revoke` only, and `issue` and `rotate` stay with Jason. This is input to +Q-M3. +N2. The `issue` flag has to follow the kind. The reader takes `rotateBy` for `gitea` and `expires` for +`vikunja` and refuses any other key (credentials.mjs:24-33). Say `--rotate-by` is gitea-only and +`--expires` is vikunja-only. +N3. The work ref is per task, `work/vikunja--` (working.mjs:110-114), not per agent. "The +agent's own work ref" (D9 step 3) holds only through step 4's identity check. Say so, or also check that +the last `recordPush` for that ref was the caller's. +N4. `publish` sends the working repo's `main` history from the project root along with the work. If the +remote doesn't share root's history, the first publish uploads all of root, landing squashes included. Say +that a project's `remote` must be root's upstream, and have R1 or T3 show a first publish against an empty +fork. +N5. E5 cites `git-socket.mjs:398-407`. It is `packages/cli/src/git-socket.mjs`. Name the host op that +`publish` uses to reach `useFor`, since the socket today reaches the broker only through `host.op`. + +## Q-M inputs for Mos + +- Q-M1: O1 (forks). Gitea enforces it, and no per-agent rule can drift. O2 depends on probe P1, which hasn't run. +- Q-M2: no objection. It's a reviewed `roles/` change, with nothing automatic. +- Q-M3: human-only, plus N1's narrowing for the tester. +- Q-M4: agreed. Per-agent model tokens need the proxy row. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index b2778c931..6b0a53aba 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1372,3 +1372,5 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T19:20Z | Duckworth (Opus 5.5) | federation design r4.2-r4.4 re-review (duckworth-0010), Gizmoduck 73677b87d..b67bc6de9 | changes B9 (user-thread decision route :1683); B8, N16-N19 resolved; note N20 2026-10-20T19:22Z | Duckworth (Opus 5.5) | row 193 design r3 re-review (duckworth-0011), Morgana 19c015741..76f4605e1 | changes B3 (5.7 table omits project.file reads), B1 B2 N1-N4 resolved 2026-10-20T19:22Z | Duckworth (Opus 5.5) | federation design r4.5 re-review (duckworth-0012), Gizmoduck b67bc6de9..acd5f1f42 | approve, B9 N20 resolved, note N21 R6 notice wording +2026-10-20T19:25Z | Duckworth (Opus 5.5) | row 199 design r1 review (duckworth-0013), Megavolt 624e8ea3a | changes B1 (missing store file stops bus start), B2 (token not bound to remote host, redirects), notes N1-N5 +2026-10-20T19:25Z | Duckworth (Opus 5.5) | row 193 design r4 re-review (duckworth-0014), Morgana 76f4605e1..edc794933 | approve, B3 and N5 resolved