oficina / meet Public

Clone
git clone https://amadan.net/oficina/meet

Plain git — no account needed to clone.

Download

Download this file

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.Skipped 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 <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. |

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:<id>, 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 <script> case already establishes: a title is never rendered as anything but an escaped JSON string or HTML text, so nothing downstream is at risk from what it contains.
  • §8.3, the MCP adapter. Deferred by the spec itself ("until a consumer asks by name"), not touched this slice. Destination: unchanged — no consumer has asked yet.
  • *The per-agent limiter shares no budget with the signalling limiter, and is keyed only on the agent, not the agent and the operation. Noted in Task 3's report as a concern, not acted on: sixty writes a minute is as available to an agent as sixty reads. Destination: watched, not fixed this slice* — nothing in the spec asks for a finer-grained budget, and a single 60/min ceiling is the one §8's Global Constraints actually specifies.
  • admitted is per-process. Read from the in-process hub exactly as every other count the hub answers is (§4.2); on a multi-host fleet it would be that host's count, not the meeting's. Destination: unchanged, consistent with everything else the hub answers and already noted for slices 2 through 4.
  • apiServerError logs the raw driver error. (Task 3's review item 6, which never reached a destination until the whole-branch review named it W5-5.) It is the one API log line whose content is not a fixed vocabulary. GORM renders placeholders, not bound arguments, so a token hash cannot appear in it; no change was asked for. Destination: informational, watch on driver/logging changes — the line to re-examine if the driver or the logging setup ever changes in a way that could put a bound value into an error's text.
  • The whole-branch review. whole-branch-review.md's brief was written into the ledger before Task 4 started and has not been run as of this commit — its own six questions (trust end to end, the five operations against §8.2, coherence with earlier slices, the three gates, that every carried item above has a destination, and what blocks merge versus deploy) are for whoever runs it next. Destination: its own entry in this file, the way slice 4's Codex rounds were appended to that slice's record after the fact — not run or fabricated here ahead of the result.

---

4. Deploying this slice

Canary, 2026-09-11. slice2-meet.bds.oncarlos.com took bed664b onto its slice-4 database and logged migrate applied=[meet/0002_agents] skipped=4, then 4f896c2 with applied=[] skipped=5; both boots logged meet: relay configured regions=1. Probed on the platform: GET /api/v1/meetings with no bearer is 401 {"error":"unauthorized"}, a made-up bearer on a made-up meeting is 404, /settings redirects to sign-in. The carlos deploy wait can time out before the instance hands over (it reported the old version for a minute and then served the new one); check /api/version before retrying.

One migration, 0002_agents.sql, applied at boot. Schema (migrations.go) embeds internal/calls/migrations/*.sql in filename order and migrate.Apply runs whatever a given database has not yet seen, once, on boot — never re-run, never reversed. This slice adds exactly one file to that directory, generated by rastrillo migration generate after Agent was added to models.go: one CREATE TABLE agents, a unique index on token_hash, an index on operator_person_id, nothing else. rastrillo migration check ("models and migrations agree") passes on every gate run in this slice's ledger, including this commit's.

The filename was renamed before it ever shipped, and that is safe. rastrillo migration generate first wrote 0002_create_agents.sql; Task 1 renamed it to 0002_agents.sql to match the plan before committing (Task 1's report, deviation 1). migrate.FromFS derives each migration's identity from its filename, so the two names are, to the migration runner, two different migrations — a developer database that had already applied the pre-rename 0002_create_agents would fail at boot, seeing 0002_agents as a new, unapplied migration racing to create a table that already exists. This is safe precisely because no such database exists: the rename happened in the same commit as the migration's first appearance on this branch, 0002_create_agents was never pushed, deployed, or merged under that name, and git log for this repository has no commit carrying it. Anyone resuming this worktree from a stash or an old checkout predating 533b629 should discard rather than reuse a database file from that window, on the same reasoning.

Every deploy still drops every live call, exactly as slice 2's record already states (spec §15.11 — the hub is per process and dies with it) and slices 3 and 4 did not change: this slice adds nothing to the hub's own durability, and the drive surface itself never touches a live call — its five operations are metadata (list, create, replace a link, end) and none of them joins a room (§8.2, "no operation joins a call", ruling 11). An agent's own in-flight request at deploy time either completes against the old process or is refused by the load balancer the way any other request would be; nothing about this slice changes that.

A revoked or reach-lost agent takes effect at its very next request, with no deploy or restart needed. §8.1's reach re-derivation runs inside requireAgent on every call, so removing an operator from the allowlist or revoking their agent through Settings is live the moment the write commits — TestAPIReachIsTheOperatorsStandingOnEveryRequest is the test-level proof, and it needs no server restart to pass.

---

5. The gates, and how to run them

| Gate | Command | Notes | |---|---|---| | 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. | | Race | CGO_ENABLED=1 go test -race ./internal/... | Run through every task's fix rounds this slice; green throughout, including this commit's. | | 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. |

Why a slice that added a JSON API and a database table needed no new browser scenario. §8.4 already draws this line: the meeting API is a plain JSON surface a native client would use directly, with nothing in it that assumes a browser, so it is proven by internal/callstest's Go HTTP tests (api_test.go) rather than by driving Chromium. The one place this slice's work is visible inside a browser — the robot badge and principal on index.html — is server-rendered HTML with no script involvement at all (AGENTS.md: screens work with JavaScript disabled), so TestAPIWritesAreMarkedAsTheAgents (a Go test using the existing browser-cookie harness to sign in and read the rendered page, not a real browser) is the right level to prove it at, and make browser's own 31 scenarios needed nothing added.