# Meet slice 5 — the record Branch `slice/5-drive`, `1c66309..HEAD` (this commit; started on main `1c66309`). Spec: `docs/superpowers/specs/2026-09-07-meet-design.md`, revision 9 (§15.17 carries this slice's wire ruling). Slice 5 built the drive surface: the `agents` table and its store methods, Settings' mint-once/revoke screen, the `/api/v1/meetings` bearer API and its five operations, and the robot badge and principal on the meetings list — four tasks, each with its own review and fix rounds, kept in `.superpowers/sdd/2026-09-11-meet-slice-5-drive/progress.md` and the `task-N-report.md` files. Like slices 2 through 4's, that ledger is gitignored and does not survive the worktree; this file is its substance. --- ## 1. Rulings and decisions From the ledger's pre-flight scan, its rulings, and the tasks that followed: - **`/api/v1` is mounted outside `csrf.Protect`.** **Ruling**, verified against rastrillo's own `csrf.SameOrigin` before Task 3 was written: a state-changing request carrying none of `Sec-Fetch-Site`, `Origin` or `Referer` is refused, and an agent — not a browser — sends none of the three, so a POST from one would 403 before reaching a handler. Safe because the surface honours no cookie: there is no ambient credential a cross-origin page could spend through it, which is the whole of what CSRF defends against. `/api/v1` is its own chi group, registered before the group that carries `csrf.Protect` and every session middleware, so a bearer never reaches `RequireSession` and a cookie never reaches the API. Written into spec §15.17; `TestABearerOnAnHTMLRouteIsIgnored` and `TestAPIRequiresABearerAndIgnoresCookies` prove both directions. - **A missing or malformed bearer is 401; every token-shaped string goes to the store.** **Ruling.** Unknown, revoked and reach-less tokens all answer the identical not-found body — `apiNotFound()`, one map through one writer — so nothing about which of the three it was is ever observable. A malformed bearer is judged on the header alone, never the token's own shape: there is no `mt_` prefix check anywhere on this path, deliberately, which is what keeps a made-up token indistinguishable by status from a real one that was revoked (Task 3's deviation 5, resolving an apparent conflict in the brief in the test's favour). Written into spec §15.17. - **`end`/`links` on an ended meeting is 409 `{"error":"ended"}`; the per-agent limit is 60/min, burst 60, keyed on `actorForAgent` and run after the token resolves.** **Ruling.** Keying the limiter on the raw header first would let 4096 fake tokens (`maxAgentBuckets`) evict every real agent's bucket before one is ever checked to exist. Deliberately no client-address limiter on this surface: behind the platform's relay socket every request the app sees shares one address, so such a limiter would let one flood lock out every real agent along with whatever was flooding — carried to carlos/platform as an edge concern rather than solved here (§3 below). - **`created_by` is the principal for an agent's write, the person's display name for a member's, and the raw actor string if the row behind it no longer resolves.** **Ruling**, `store.creatorLabel` (Task 3, reused by Task 4 rather than duplicated). A label, never an authorization input — nothing is decided on it, and every member already sees every meeting on the list, so naming which agent made one discloses nothing new (`agentAnyByID`'s own doc comment). - **The API ignores cookies entirely; HTML routes ignore bearers.** **Ruling**, proven both directions by Task 3's tests as above. - **Title length is 200 characters counted as runes, on both the form and the API.** Task 3 shipped a byte count (deviation 6: "two surfaces disagreeing about what a 200-character title is seemed worse than the brief's wording", which itself said characters); the Task 3 review's addendum overrode it — a byte count lets the API refuse a title the screen's own copy ("Keep the title under 200 characters.") promises is fine the moment it holds anything outside ASCII. Task 4 folded in the fix: one shared `titleTooLong` (`handlers_meetings.go`) measuring `utf8.RuneCountInString` against `maxTitleLen`, called from both `createMeeting` and `apiCreateMeeting`, with a 200-rune (600-byte) accept and a 201-rune refuse added to `TestAPIBodyRules`. Written into spec §15.17. - **The meeting object is exactly six keys, and three of the five API responses never carry a `"link"` key.** Task 3's review found a surviving mutation (a `link` field added to the GET response would not have been caught) and the addendum asked for the gap closed: `TestAPIListsCreatesLinksAndEnds` now asserts `"link"` absent on the bare GET, on the end response, and on every list row, asserts `"admitted"` absent on a GET taken *after* ending (not just on the end response itself, closing the `m.EndedAt == nil` guard's own blind spot), and pins the list row's key set to the six names on the wire. - **The robot badge is `ui/badge` plus the lucide `bot` icon, never a hand-rolled equivalent.** AGENTS.md: `[x]` conventions are enforced by a vendored component. The icon alone would not satisfy AGENTS.md's "state is never colour alone" rule on its own account — a shape is not a colour, but it is still not text — so the badge's own word ("Agent") is what actually carries the meaning; the icon needs no separate `aria-label` because it sits beside that word. The `bot` icon's own class carries `lucide-bot` alongside this app's usual `icon`, a one-icon deviation from the pattern every other slug follows, recorded so the next reader of `icons.go` does not wonder why (§3 below has the reasoning in full). - **`meetingView.CreatedByPrincipal` is resolved once per distinct actor across a whole list, not once per row.** Task 3's report flagged the N+1 this move would create if the label ever joined `meetingView`; Task 4 closed it with `meetingViewOfCached`, a shared `labels map[string]string` `listMeetings` passes across its whole loop, while `meetingViewOf` (still called by the API's four single-row paths, whose call sites were not touched) passes a throwaway map of its own. The list's separate N+1 on link-presence counts is unrelated and still stands, unaddressed by this slice. --- ## 2. Per-task review dispositions | Task | Built | Review | Fix rounds | Final | |---|---|---|---|---| | 1 — the `agents` table and store | `Agent` model, `0002_agents.sql` (generated, then renamed from `rastrillo`'s own `0002_create_agents.sql`), `newAgentToken`/`hashAgentToken`, `createAgent`/`agentsFor`/`revokeAgent`/`agentByToken`/`agentByID`, `principal`, `actorForAgent`; three tests | Needs fixes: MEDIUM `operatorFor` fell back to a bare-ID `Person` instead of failing closed; MEDIUM `agentByID` was unscoped to its operator; LOW-MED `createAgent` accepted a zero operator, no cap on live agents, no duplicate-name check; LOW test hygiene (tokens printed on failure, ordering/randomness/shape unproven) | Round 1 (`e9539a3`): all fixed — fail-closed lookups via a new `withOperator`/`personByID` path, `agentByID` gained an operator parameter and the owned-row 404 shape, `createAgent` refuses a zero operator and enforces the 20-live cap and the live-duplicate-name check inside its own transaction, new tests for the token's shape, ordering and the fail-closed operator case | Complete. | | 2 — Settings: mint, show once, revoke | `handlers_settings.go`, `settings.html`, `agent_once.html`, four routes inside `requireMember`, `Viewer.Member`, `page`/`ErrorPage` as `*app` methods; nine tests | Needs fixes, minor: no `Cache-Control: no-store` on the mint and list pages; no length guard on a token before rendering it (a 64-hex sanity check); `Autofocus` on a field that did not need it; a double `WriteHeader` on the 422 path; a report count off by one | Round 1 (folded into Task 1's fix round, same commits `e9539a3`/`e8517d1`): no-store added to both once-shown pages, the hex guard added to the list, `Autofocus` dropped, `renderSettings` made to own the status write once, the report corrected | Complete. | | 3 — the API: bearer, reach, five operations | `api.go` (`requireAgent`, `apiJSON`, `apiNotFound`, `refuseAPI`, `apiServerError`, `apiDecode`, `apiRoutes`, the five handlers), `hub.AdmittedCount`, `store.meetingViewOf` (factored out of `listMeetings`), `store.agentAnyByID`/`creatorLabel`/`withOperator`; eight tests, one shipped `t.Skip`ped for Task 4 | **Clean**, 8 mutations run (6 killed; 2 survivors: a `link` key left addable to a GET response, an `admitted` key left readable off an ended meeting's own GET) | Addendum folded into Task 4 rather than a separate round: both survivors closed by new assertions in `TestAPIListsCreatesLinksAndEnds`; title length moved from bytes to runes on both surfaces, one shared helper; the list row's six-key set pinned. §15.17 recorded the csrf mount, the header-only 401 rule, the API's own non-promises (§3 below), and the badge | Complete, addendum closed by Task 4's commit. | | 4 — the badge, the record, the spec note | `meetingView.CreatedByPrincipal`/`ByAgent` (`meetingViewOfCached`), `icons.go`'s `bot`, `index.html`'s badge and "by ``" line, the Task 3 addendum (rune-counted titles, the closed mutations, the pinned key set), spec §15.17, this record | This commit's own work; no separate review round within the task | — | This commit. | **Whole-branch review** (Opus, read-only, scope `1c66309..2f92b0a`): verdict **merge with named fixes** W5-1 through W5-5. All five applied in one fix-wave commit (`.superpowers/sdd/2026-09-11-meet-slice-5-drive/fix-wave-report.md`): W5-1 and W5-2 as new/extended tests in `internal/callstest/api_test.go`, W5-3 in `api.go`, W5-4 in spec §15.17, W5-5 as the line added to §3 below. **Codex round 1** (independent, static, scope `1c66309..bed664b`): verdict **request changes** — two P2 defects and three P3 advisories, S5-1 through S5-5. All five applied in one commit (`.superpowers/sdd/2026-09-11-meet-slice-5-drive/codex-r1-fixes-report.md`): - **S5-1 — P2, malformed and oversized bodies could execute writes. Fixed.** `apiDecode` required `dec.More()` to be false, which is not EOF: `{"title":"x"}]`, `{"title":"x"}}` and a valid object followed by 17 KB of whitespace all created meetings, the last of them swallowing the cap's own `MaxBytesError`. It now decodes one value and requires EOF after it, with a `*http.MaxBytesError` anywhere in the chain answering 413 and everything else 400. The 16 KB cap moved to a group middleware (`capAPIBody`), so the bodyless writes `links` and `end` are capped too; both now read their body to its end and refuse any content (400, or 413 over the cap) before the write. Spec §15.17 carries both rules. Tests: six new `TestAPIBodyRules` cases, each asserting the list afterwards is still empty, and a new `TestAPIBodylessWritesRefuseABody`. - **S5-2 — P2, concurrent endings bypassed the 409. Fixed.** The ended check moved inside both write transactions: `store.endMeeting` returns `(bool, error)` from its conditional update's `RowsAffected` and the API maps false to 409; `store.replaceLink` re-reads `ended_at` inside its transaction and answers the `errMeetingEnded` sentinel, which the API maps to the same 409. The HTML handlers keep their existing answers (end redirects either way, replace answers the not-found page). Tests: `TestASecondEndChangesNothingAndAReplaceAfterEndIsRefused` (store-level, sequential and deterministic) and `TestAPIConcurrentEndsAndReplacesSeeTheEndedRow` (two requests through the real router released together: one 200 and one 409, and no live link on an ended meeting) — the second of which round 2 corrected to a smoke test and replaced as the gate, see S5-6 below. - **S5-3 — P3, the log test did not prove success silence. Fixed.** `TestAPILogsCarryNoToken` now marks the log immediately before each of five successful operations and asserts zero new `meet: api` lines from each; the operator's address joins the secret needles. The slow/failing-query half was **not** taken, and round 2 corrected the reason: the harness does expose `x.db.G` and GORM supports query callbacks, so fault and delay injection need no production change (the seam S5-6's test now uses). The decline stands on the narrower ground — the claim this test makes is about the API surface's own log lines — and database-log secrecy stays untested here, with rastrillo's `ParameterizedQueries: true` as the standing static evidence. - **S5-4 — P3, the badge's text assertion was vacuous. Fixed.** Both rows carry neutral titles ("Quarterly review", "Standup"), and the assertion reads the `rst-badge` span's own inner text rather than searching the row. A replacement and an end over the API are now checked against the rows they write: `meeting_links.created_by` and `meetings.ended_by` both `agent:`, with the member's own meeting's `person:` actor as the control. - **S5-5 — P3, operator-owned queries bypassed the scope helper. Fixed.** The cap count, the duplicate-name check, `agentsFor`, `revokeAgent` and `agentByID` all run through one `agentsOwnedBy` helper over `scope.OwnedBy(g, "operator_person_id", …)`, transactions included. `agentByToken`, `agentAnyByID` and `creatorLabel` stay unscoped, each with a comment saying why: the first has no operator to scope to (the bearer is what names one), and the other two are label lookups for rows the reader already sees, deciding nothing. **Codex round 2** (independent, static, scope `c523a12`): verdict **accept with corrections** — S5-1 through S5-5 confirmed fixed, two P3 advisories about what the fixes *claimed*, S5-6 and S5-7. Both applied in one commit (`.superpowers/sdd/2026-09-11-meet-slice-5-drive/codex-r1-fixes-report.md`, round-2 section): - **S5-6 — P3, the concurrent test did not deterministically expose the race. Fixed.** Closing one channel makes two goroutines runnable; it does not make either read the running meeting before the other writes, so the schedule where the first request finishes before the second reads satisfies every assertion under the old code too. Twenty green runs proved nothing about that. `TestAPIConcurrentEndsAndReplacesSeeTheEndedRow` stays as a smoke test with its claims corrected, and the gate is now `TestAPIAStaleReadCannotWriteOverAnEnd`, which forces the interleaving with a **test-only GORM query callback** registered on the harness's own `x.db.G` (`installMeetingReadStall`, removed again by `t.Cleanup`): it fires after `gorm:query`, only for a request carrying a stall in its context and only for the first `meetings` read — the handler's own `meetingBySqid` — and holds that request there while a second request ends the meeting and commits. No production code gained a hook. Proved by mutation on a throwaway copy under `$TMPDIR`: with the pre-transaction checks restored and the in-transaction ones removed, the held end answers 200 (a second end) and the held replacement mints a link on an ended meeting, failing 5 runs out of 5, both subtests. Spec §15.17 names the gate and demotes the smoke test. - **S5-7 — P3, the universal 413 wording exceeded the contract. Fixed (documentation only), and refined again in round 3.** `MaxBytesReader` limits reading, not receiving, so 413 is reachable only where a handler consumes a body. §15.17 now says the cap is hit when reading runs past it (round 3 sharpened that wording again, S5-9), lists the precedence — 401 (no credential) → 404 (unresolved token or reach) → 429 (budget) → the body rules — and records the two consequences the old wording got wrong: an oversized body that is malformed from its first bytes may answer 400 before the reader reaches the cap, and a GET carrying a body is answered 200 with the body discarded unread. **GET-body enforcement was deliberately not added**: there is no operation where a GET body could reach a write, and capping bytes nobody reads buys nothing. New positive tests (`TestAPICreateAcceptsTrailingWhitespace`) pin the other half: a create ending in a newline, or padded with whitespace, is 201 — the EOF rule is about content, not about the object being the last byte, and it is deliberately not the bodyless rule, where a single space is 400. **Codex round 3** (independent, static, scope `11fb573`): verdict **accept with corrections** — S5-6 and S5-7 confirmed closed, two P3 advisories, S5-8 and S5-9. Both applied in one commit (report: the round-3 section of `.superpowers/sdd/2026-09-11-meet-slice-5-drive/codex-r1-fixes-report.md`): - **S5-8 — P3, a failed assertion could leave the stalled request blocked. Fixed.** In `TestAPIAStaleReadCannotWriteOverAnEnd`, a `t.Fatal` between starting the held request and releasing it — a `waitReached` timeout, or the competing end failing its status assertion — skipped the release and parked that request on the stall channel for the rest of the package run; removing the callback does not unblock one already inside it, and the `<-done` wait had no timeout either. The held request is now a `heldRequest` whose `t.Cleanup` is registered **before** its goroutine starts and which releases and joins it exactly once (`sync.Once` on both the release and the join), with every wait — `waitReached`, the join, the response — bounded at five seconds and failing by name here rather than by the package timeout. Verified by injecting a `t.Fatal` immediately after `waitReached` in a throwaway copy: the test fails in 0.14s instead of hanging. Run 10× under `-race`, green. - **S5-9 — P3, the body table still contradicted the whitespace rule and got the boundary wrong. Fixed.** The 400 row now reads "anything other than JSON whitespace after the first value" rather than "anything at all after the first object", which round 2's whitespace acceptance had made false. The 413 row now reads "reading the body encounters bytes beyond 16,384", and a new paragraph states the inclusive boundary and the two answers on the inside of it: a create padded with JSON whitespace to exactly 16,384 bytes is 201, a bodyless write carrying the same 16,384 bytes is 400 (it takes no body whatever its size), and one byte more is 413 on both. The bodyless paragraph's "reaching the cap is 413" is corrected the same way. Nothing had ever asserted the boundary — every other body test uses 17 KB, comfortably past the edge — so `TestAPIBodyCapBoundary` now pins all four answers, and the meeting being unchanged afterwards. **Codex round 4** (independent, static, scope `4f896c2`): verdict **accept** — no new finding at any severity; S5-8 and S5-9 confirmed closed, S5-1 through S5-7 unchanged under their accepted dispositions; "fit to merge and deploy on this review and the recorded gate results". Four rounds in all: 5, 2, 2, 0 findings. --- ## 3. Carried items, and where they go - **The list's link-presence count is still N+1, unrelated to the label move.** `listMeetings` now shares one `creatorLabel` cache across a whole render (this task), but `meetingViewOfCached` still runs one `MeetingLink` count query per row, exactly as `listMeetings` always has since before this slice. **Destination: unchanged** — a team's meeting list is small (§8.2 does not ask for paging either, for the same reason), and fixing both N+1s at once was judged a larger, unrelated change than this task's brief asked for. Worth a look if a team's list ever grows enough to notice. - **No client-address limiter on the API, and unauthenticated flood control generally.** Recorded as a ruling in §1 above and in spec §15.17: behind the platform's relay socket every request shares one address, so a limiter keyed on it would be worse than none. **Destination: carlos/platform**, the same edge question Task 3's review raised and the addendum confirmed does not belong in this app. - **NUL and lone-surrogate acceptance in titles, on both surfaces.** Titles are unfiltered data once they pass the length and non-empty checks; a NUL byte or an unpaired UTF-16 surrogate (reachable only through a client encoding one, since `encoding/json` itself accepts a lone surrogate escape) is stored and returned as sent. **Destination: informational, not a defect** — recorded in spec §15.17 rather than fixed, on the same reasoning `TestAPIBodyRules`'s `