| 1 | # Meet slice 5 — the record |
| 2 | |
| 3 | Branch `slice/5-drive`, `1c66309..HEAD` (this commit; started on main |
| 4 | `1c66309`). Spec: `docs/superpowers/specs/2026-09-07-meet-design.md`, |
| 5 | revision 9 (§15.17 carries this slice's wire ruling). |
| 6 | |
| 7 | Slice 5 built the drive surface: the `agents` table and its store |
| 8 | methods, Settings' mint-once/revoke screen, the `/api/v1/meetings` |
| 9 | bearer API and its five operations, and the robot badge and principal |
| 10 | on the meetings list — four tasks, each with its own review and fix |
| 11 | rounds, kept in |
| 12 | `.superpowers/sdd/2026-09-11-meet-slice-5-drive/progress.md` and the |
| 13 | `task-N-report.md` files. Like slices 2 through 4's, that ledger is |
| 14 | gitignored and does not survive the worktree; this file is its |
| 15 | substance. |
| 16 | |
| 17 | --- |
| 18 | |
| 19 | ## 1. Rulings and decisions |
| 20 | |
| 21 | From the ledger's pre-flight scan, its rulings, and the tasks that |
| 22 | followed: |
| 23 | |
| 24 | - **`/api/v1` is mounted outside `csrf.Protect`.** **Ruling**, verified |
| 25 | against rastrillo's own `csrf.SameOrigin` before Task 3 was written: |
| 26 | a state-changing request carrying none of `Sec-Fetch-Site`, `Origin` |
| 27 | or `Referer` is refused, and an agent — not a browser — sends none of |
| 28 | the three, so a POST from one would 403 before reaching a handler. |
| 29 | Safe because the surface honours no cookie: there is no ambient |
| 30 | credential a cross-origin page could spend through it, which is the |
| 31 | whole of what CSRF defends against. `/api/v1` is its own chi group, |
| 32 | registered before the group that carries `csrf.Protect` and every |
| 33 | session middleware, so a bearer never reaches `RequireSession` and a |
| 34 | cookie never reaches the API. Written into spec §15.17; |
| 35 | `TestABearerOnAnHTMLRouteIsIgnored` and |
| 36 | `TestAPIRequiresABearerAndIgnoresCookies` prove both directions. |
| 37 | - **A missing or malformed bearer is 401; every token-shaped string |
| 38 | goes to the store.** **Ruling.** Unknown, revoked and reach-less |
| 39 | tokens all answer the identical not-found body — `apiNotFound()`, |
| 40 | one map through one writer — so nothing about which of the three it |
| 41 | was is ever observable. A malformed bearer is judged on the header |
| 42 | alone, never the token's own shape: there is no `mt_` prefix check |
| 43 | anywhere on this path, deliberately, which is what keeps a made-up |
| 44 | token indistinguishable by status from a real one that was revoked |
| 45 | (Task 3's deviation 5, resolving an apparent conflict in the brief in |
| 46 | the test's favour). Written into spec §15.17. |
| 47 | - **`end`/`links` on an ended meeting is 409 `{"error":"ended"}`; the |
| 48 | per-agent limit is 60/min, burst 60, keyed on `actorForAgent` and run |
| 49 | after the token resolves.** **Ruling.** Keying the limiter on the raw |
| 50 | header first would let 4096 fake tokens (`maxAgentBuckets`) evict |
| 51 | every real agent's bucket before one is ever checked to exist. |
| 52 | Deliberately no client-address limiter on this surface: behind the |
| 53 | platform's relay socket every request the app sees shares one |
| 54 | address, so such a limiter would let one flood lock out every real |
| 55 | agent along with whatever was flooding — carried to carlos/platform |
| 56 | as an edge concern rather than solved here (§3 below). |
| 57 | - **`created_by` is the principal for an agent's write, the person's |
| 58 | display name for a member's, and the raw actor string if the row |
| 59 | behind it no longer resolves.** **Ruling**, `store.creatorLabel` |
| 60 | (Task 3, reused by Task 4 rather than duplicated). A label, never an |
| 61 | authorization input — nothing is decided on it, and every member |
| 62 | already sees every meeting on the list, so naming which agent made |
| 63 | one discloses nothing new (`agentAnyByID`'s own doc comment). |
| 64 | - **The API ignores cookies entirely; HTML routes ignore bearers.** |
| 65 | **Ruling**, proven both directions by Task 3's tests as above. |
| 66 | - **Title length is 200 characters counted as runes, on both the form |
| 67 | and the API.** Task 3 shipped a byte count (deviation 6: "two |
| 68 | surfaces disagreeing about what a 200-character title is seemed |
| 69 | worse than the brief's wording", which itself said characters); the |
| 70 | Task 3 review's addendum overrode it — a byte count lets the API |
| 71 | refuse a title the screen's own copy ("Keep the title under 200 |
| 72 | characters.") promises is fine the moment it holds anything outside |
| 73 | ASCII. Task 4 folded in the fix: one shared `titleTooLong` |
| 74 | (`handlers_meetings.go`) measuring `utf8.RuneCountInString` against |
| 75 | `maxTitleLen`, called from both `createMeeting` and |
| 76 | `apiCreateMeeting`, with a 200-rune (600-byte) accept and a 201-rune |
| 77 | refuse added to `TestAPIBodyRules`. Written into spec §15.17. |
| 78 | - **The meeting object is exactly six keys, and three of the five API |
| 79 | responses never carry a `"link"` key.** Task 3's review found a |
| 80 | surviving mutation (a `link` field added to the GET response would |
| 81 | not have been caught) and the addendum asked for the gap closed: |
| 82 | `TestAPIListsCreatesLinksAndEnds` now asserts `"link"` absent on the |
| 83 | bare GET, on the end response, and on every list row, asserts |
| 84 | `"admitted"` absent on a GET taken *after* ending (not just on the |
| 85 | end response itself, closing the `m.EndedAt == nil` guard's own |
| 86 | blind spot), and pins the list row's key set to the six names on the |
| 87 | wire. |
| 88 | - **The robot badge is `ui/badge` plus the lucide `bot` icon, never a |
| 89 | hand-rolled equivalent.** AGENTS.md: `[x]` conventions are enforced |
| 90 | by a vendored component. The icon alone would not satisfy AGENTS.md's |
| 91 | "state is never colour alone" rule on its own account — a shape is |
| 92 | not a colour, but it is still not text — so the badge's own word |
| 93 | ("Agent") is what actually carries the meaning; the icon needs no |
| 94 | separate `aria-label` because it sits beside that word. The `bot` |
| 95 | icon's own class carries `lucide-bot` alongside this app's usual |
| 96 | `icon`, a one-icon deviation from the pattern every other slug |
| 97 | follows, recorded so the next reader of `icons.go` does not wonder |
| 98 | why (§3 below has the reasoning in full). |
| 99 | - **`meetingView.CreatedByPrincipal` is resolved once per distinct |
| 100 | actor across a whole list, not once per row.** Task 3's report |
| 101 | flagged the N+1 this move would create if the label ever joined |
| 102 | `meetingView`; Task 4 closed it with `meetingViewOfCached`, a shared |
| 103 | `labels map[string]string` `listMeetings` passes across its whole |
| 104 | loop, while `meetingViewOf` (still called by the API's four |
| 105 | single-row paths, whose call sites were not touched) passes a |
| 106 | throwaway map of its own. The list's separate N+1 on link-presence |
| 107 | counts is unrelated and still stands, unaddressed by this slice. |
| 108 | |
| 109 | --- |
| 110 | |
| 111 | ## 2. Per-task review dispositions |
| 112 | |
| 113 | | Task | Built | Review | Fix rounds | Final | |
| 114 | |---|---|---|---|---| |
| 115 | | 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. | |
| 116 | | 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. | |
| 117 | | 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. | |
| 118 | | 4 — the badge, the record, the spec note | `meetingView.CreatedByPrincipal`/`ByAgent` (`meetingViewOfCached`), `icons.go`'s `bot`, `index.html`'s badge and "by `<principal>`" 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. | |
| 119 | |
| 120 | **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. |
| 121 | |
| 122 | **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`): |
| 123 | |
| 124 | - **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`. |
| 125 | - **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. |
| 126 | - **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. |
| 127 | - **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:<id>`, with the member's own meeting's `person:` actor as the control. |
| 128 | - **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. |
| 129 | |
| 130 | **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): |
| 131 | |
| 132 | - **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. |
| 133 | - **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. |
| 134 | |
| 135 | **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`): |
| 136 | |
| 137 | - **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. |
| 138 | - **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. |
| 139 | |
| 140 | **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. |
| 141 | |
| 142 | --- |
| 143 | |
| 144 | ## 3. Carried items, and where they go |
| 145 | |
| 146 | - **The list's link-presence count is still N+1, unrelated to the |
| 147 | label move.** `listMeetings` now shares one `creatorLabel` cache |
| 148 | across a whole render (this task), but `meetingViewOfCached` still |
| 149 | runs one `MeetingLink` count query per row, exactly as `listMeetings` |
| 150 | always has since before this slice. **Destination: unchanged** — a |
| 151 | team's meeting list is small (§8.2 does not ask for paging either, |
| 152 | for the same reason), and fixing both N+1s at once was judged a |
| 153 | larger, unrelated change than this task's brief asked for. Worth a |
| 154 | look if a team's list ever grows enough to notice. |
| 155 | - **No client-address limiter on the API, and unauthenticated flood |
| 156 | control generally.** Recorded as a ruling in §1 above and in spec |
| 157 | §15.17: behind the platform's relay socket every request shares one |
| 158 | address, so a limiter keyed on it would be worse than none. |
| 159 | **Destination: carlos/platform**, the same edge question Task 3's |
| 160 | review raised and the addendum confirmed does not belong in this |
| 161 | app. |
| 162 | - **NUL and lone-surrogate acceptance in titles, on both surfaces.** |
| 163 | Titles are unfiltered data once they pass the length and non-empty |
| 164 | checks; a NUL byte or an unpaired UTF-16 surrogate (reachable only |
| 165 | through a client encoding one, since `encoding/json` itself accepts |
| 166 | a lone surrogate escape) is stored and returned as sent. |
| 167 | **Destination: informational, not a defect** — recorded in spec |
| 168 | §15.17 rather than fixed, on the same reasoning `TestAPIBodyRules`'s |
| 169 | `<script>` case already establishes: a title is never rendered as |
| 170 | anything but an escaped JSON string or HTML text, so nothing |
| 171 | downstream is at risk from what it contains. |
| 172 | - **§8.3, the MCP adapter.** Deferred by the spec itself ("until a |
| 173 | consumer asks by name"), not touched this slice. **Destination:** |
| 174 | unchanged — no consumer has asked yet. |
| 175 | - **The per-agent limiter shares no budget with the signalling |
| 176 | limiter, and is keyed only on the agent, not the agent *and* the |
| 177 | operation.** Noted in Task 3's report as a concern, not acted on: |
| 178 | sixty writes a minute is as available to an agent as sixty reads. |
| 179 | **Destination: watched, not fixed this slice** — nothing in the spec |
| 180 | asks for a finer-grained budget, and a single 60/min ceiling is the |
| 181 | one §8's Global Constraints actually specifies. |
| 182 | - **`admitted` is per-process.** Read from the in-process hub exactly |
| 183 | as every other count the hub answers is (§4.2); on a multi-host |
| 184 | fleet it would be that host's count, not the meeting's. |
| 185 | **Destination: unchanged**, consistent with everything else the hub |
| 186 | answers and already noted for slices 2 through 4. |
| 187 | - **`apiServerError` logs the raw driver error.** (Task 3's review item |
| 188 | 6, which never reached a destination until the whole-branch review |
| 189 | named it W5-5.) It is the one API log line whose content is not a |
| 190 | fixed vocabulary. GORM renders placeholders, not bound arguments, so |
| 191 | a token hash cannot appear in it; no change was asked for. |
| 192 | **Destination: informational, watch on driver/logging changes** — the |
| 193 | line to re-examine if the driver or the logging setup ever changes in |
| 194 | a way that could put a bound value into an error's text. |
| 195 | - **The whole-branch review.** `whole-branch-review.md`'s brief was |
| 196 | written into the ledger before Task 4 started and has not been run |
| 197 | as of this commit — its own six questions (trust end to end, the |
| 198 | five operations against §8.2, coherence with earlier slices, the |
| 199 | three gates, that every carried item above has a destination, and |
| 200 | what blocks merge versus deploy) are for whoever runs it next. |
| 201 | **Destination: its own entry in this file**, the way slice 4's |
| 202 | Codex rounds were appended to that slice's record after the fact — |
| 203 | not run or fabricated here ahead of the result. |
| 204 | |
| 205 | --- |
| 206 | |
| 207 | ## 4. Deploying this slice |
| 208 | |
| 209 | **Canary, 2026-09-11.** `slice2-meet.bds.oncarlos.com` took `bed664b` |
| 210 | onto its slice-4 database and logged `migrate applied=[meet/0002_agents] |
| 211 | skipped=4`, then `4f896c2` with `applied=[] skipped=5`; both boots |
| 212 | logged `meet: relay configured regions=1`. Probed on the platform: |
| 213 | `GET /api/v1/meetings` with no bearer is `401 {"error":"unauthorized"}`, |
| 214 | a made-up bearer on a made-up meeting is 404, `/settings` redirects to |
| 215 | sign-in. The `carlos deploy` wait can time out before the instance |
| 216 | hands over (it reported the old version for a minute and then served |
| 217 | the new one); check `/api/version` before retrying. |
| 218 | |
| 219 | **One migration, `0002_agents.sql`, applied at boot.** `Schema` |
| 220 | (`migrations.go`) embeds `internal/calls/migrations/*.sql` in filename |
| 221 | order and `migrate.Apply` runs whatever a given database has not yet |
| 222 | seen, once, on boot — never re-run, never reversed. This slice adds |
| 223 | exactly one file to that directory, generated by `rastrillo migration |
| 224 | generate` after `Agent` was added to `models.go`: one `CREATE TABLE |
| 225 | agents`, a unique index on `token_hash`, an index on |
| 226 | `operator_person_id`, nothing else. `rastrillo migration check` |
| 227 | ("models and migrations agree") passes on every gate run in this |
| 228 | slice's ledger, including this commit's. |
| 229 | |
| 230 | **The filename was renamed before it ever shipped, and that is safe.** |
| 231 | `rastrillo migration generate` first wrote `0002_create_agents.sql`; |
| 232 | Task 1 renamed it to `0002_agents.sql` to match the plan before |
| 233 | committing (Task 1's report, deviation 1). `migrate.FromFS` derives |
| 234 | each migration's identity from its filename, so the two names are, to |
| 235 | the migration runner, two different migrations — **a developer |
| 236 | database that had already applied the pre-rename |
| 237 | `0002_create_agents` would fail at boot**, seeing `0002_agents` as a |
| 238 | new, unapplied migration racing to create a table that already exists. |
| 239 | This is safe precisely because no such database exists: the rename |
| 240 | happened in the same commit as the migration's first appearance on |
| 241 | this branch, `0002_create_agents` was never pushed, deployed, or |
| 242 | merged under that name, and `git log` for this repository has no |
| 243 | commit carrying it. Anyone resuming this worktree from a stash or an |
| 244 | old checkout predating `533b629` should discard rather than reuse a |
| 245 | database file from that window, on the same reasoning. |
| 246 | |
| 247 | **Every deploy still drops every live call**, exactly as slice 2's |
| 248 | record already states (spec §15.11 — the hub is per process and dies |
| 249 | with it) and slices 3 and 4 did not change: this slice adds nothing to |
| 250 | the hub's own durability, and the drive surface itself never touches a |
| 251 | live call — its five operations are metadata (list, create, replace a |
| 252 | link, end) and none of them joins a room (§8.2, "no operation joins a |
| 253 | call", ruling 11). An agent's own in-flight request at deploy time |
| 254 | either completes against the old process or is refused by the load |
| 255 | balancer the way any other request would be; nothing about this slice |
| 256 | changes that. |
| 257 | |
| 258 | **A revoked or reach-lost agent takes effect at its very next |
| 259 | request, with no deploy or restart needed.** §8.1's reach re-derivation |
| 260 | runs inside `requireAgent` on every call, so removing an operator from |
| 261 | the allowlist or revoking their agent through Settings is live the |
| 262 | moment the write commits — `TestAPIReachIsTheOperatorsStandingOnEveryRequest` |
| 263 | is the test-level proof, and it needs no server restart to pass. |
| 264 | |
| 265 | --- |
| 266 | |
| 267 | ## 5. The gates, and how to run them |
| 268 | |
| 269 | | Gate | Command | Notes | |
| 270 | |---|---|---| |
| 271 | | The one gate | `GOCACHE=/home/paulca/.cache/tmp/claude-1001/gocache CGO_ENABLED=0 make ci` | vet, gofmt, `go test ./...`, `rastrillo migration check` — unchanged from slice 4. Green on this commit; `internal/callstest` alone (the package Tasks 2-4 mostly touch) runs in the twenty-to-thirty-second range each pass. | |
| 272 | | Race | `CGO_ENABLED=1 go test -race ./internal/...` | Run through every task's fix rounds this slice; green throughout, including this commit's. | |
| 273 | | Browser | `make browser` | **Unchanged by this slice** — no file under `internal/calls/hub`'s signalling path, `room.js`, or any `test/browser/*.test.mjs` was touched; the drive surface is a second, independent HTTP surface beside the one the browser gate exercises. Still needs the sandbox off (Chromium gathers zero ICE candidates and `getDisplayMedia` produces nothing crossing the connection inside the bash sandbox, as every slice's record from 2 onward already notes). Run once on this task's own final head, in the foreground: `go vet -tags browser` clean, 232 node tests across the five `test/browser/*.test.mjs` files, and **`TestCallInARealBrowser` 31 of 31, 0 skipped, 522.71s** — no scenario lost to the environment's own media-stall flake this run. Run again on the shipping head `4f896c2` after the four Codex rounds (which touched `api.go`, `store.go`, `handlers_meetings.go` and the API tests, none of the browser path): **31 of 31, 0 skipped, 522.67s**, no flake. | |
| 274 | |
| 275 | **Why a slice that added a JSON API and a database table needed no new |
| 276 | browser scenario.** §8.4 already draws this line: the meeting API is a |
| 277 | plain JSON surface a native client would use directly, with nothing in |
| 278 | it that assumes a browser, so it is proven by `internal/callstest`'s Go |
| 279 | HTTP tests (`api_test.go`) rather than by driving Chromium. The one |
| 280 | place this slice's work is *visible* inside a browser — the robot |
| 281 | badge and principal on `index.html` — is server-rendered HTML with no |
| 282 | script involvement at all (AGENTS.md: screens work with JavaScript |
| 283 | disabled), so `TestAPIWritesAreMarkedAsTheAgents` (a Go test using the |
| 284 | existing browser-cookie harness to sign in and read the rendered page, |
| 285 | not a real browser) is the right level to prove it at, and `make |
| 286 | browser`'s own 31 scenarios needed nothing added. |
| 287 | |
amadan