| 1 | # Meet slice 2 — the record |
| 2 | |
| 3 | Branch `slice/2-room`, `77227de..` through the whole-branch fix wave. |
| 4 | Spec: `docs/superpowers/specs/2026-09-07-meet-design.md`, revision 9 |
| 5 | (§15 is this slice's departures from revision 8). |
| 6 | |
| 7 | Slice 2 built the room: the hub, `POST /signal`, the SSE stream, the |
| 8 | holding screen, the browser client, and a Playwright gate that makes a |
| 9 | real call between two real Chromium contexts. |
| 10 | |
| 11 | This file exists because everything below was, until now, written down |
| 12 | only in `.superpowers/`, which is gitignored. The reasoning did not |
| 13 | merge and did not survive the worktree. What follows is that ledger's |
| 14 | substance: the decisions, why each one was taken, what the reviews |
| 15 | found, and what is still open with the slice it belongs to. |
| 16 | |
| 17 | --- |
| 18 | |
| 19 | ## 1. Rulings and decisions |
| 20 | |
| 21 | Eleven of these changed what the spec says and are written into §15 of |
| 22 | the spec itself, with their reasoning. They are listed here by name |
| 23 | only, so this file stays the index and the spec stays the contract: |
| 24 | |
| 25 | §15.1 the hub session key · §15.2 the `Accept` gate and its 406 · |
| 26 | §15.3 the `?probe=1` resume probe · §15.4 the scoped 410 · |
| 27 | §15.5 `/signal`'s refusals before the limiter · §15.6 the narrowed |
| 28 | room-page row · §15.7 the resume window enforced without waiting for the |
| 29 | tick · §15.8 `leave` is not idempotent · §15.9 the stale-gen 409 carries |
| 30 | no gen · §15.10 the four client-only terminal reasons · §15.12 the |
| 31 | member's standing re-derived on every request (Codex round 1, §2a). |
| 32 | |
| 33 | The rest are implementation decisions the spec has no opinion about. |
| 34 | |
| 35 | **The hub** |
| 36 | |
| 37 | - A hub method that dereferences `h.rooms[sqid]` must nil-check the |
| 38 | room and return `ErrUnknownPeer`. Cost if wrong: a nil-map panic on a |
| 39 | bad sqid. |
| 40 | - A second resume of an already-attached peer replaces the attachment, |
| 41 | and the old stream is told `lost {reason: "replaced"}`. No tombstone is |
| 42 | written, so the participation itself continues. Two tabs of one |
| 43 | browser are the one-peer-per-session rule, working as intended. |
| 44 | - `Kind` is written in exactly one place (`setKindLocked`) and |
| 45 | `Admitted` in exactly one (`admitLocked`), because a kind change owes |
| 46 | every admitted peer a fresh roster and a second writer would be a |
| 47 | second place to forget it. |
| 48 | - `StandingFor` is never called under the hub lock. Every caller drops |
| 49 | the lock, reads, retakes it, and **re-finds** the participation, |
| 50 | because it may have ended while the read was in flight. |
| 51 | - An admitted guest is never ended by a grant expiring (ruling 14): the |
| 52 | participation is the authority once someone is in. `standingLostLocked` |
| 53 | is the one place that decides what a loss of standing costs, because |
| 54 | what it costs depends on the kind held. |
| 55 | - `Admit` settles the actor's authority completely — found, standing |
| 56 | applied, admitted, member — before it looks at the target at all. |
| 57 | The other order let any joined participation learn from the shape of |
| 58 | the refusal whether a candidate peer id existed in the room. |
| 59 | - Headstone rooms are never reaped and `h.rooms` is never deleted from. |
| 60 | Any future reaper must wait out `TombstoneLife`; the note sits beside |
| 61 | `End`. |
| 62 | - `gap` is the highest evicted `seq` per sender above `last[from]` — a |
| 63 | deviation from the Task 3 brief, which asked for a simpler test. |
| 64 | |
| 65 | **The signal endpoint** |
| 66 | |
| 67 | - The rate limiter is keyed on session + room, never on anything the |
| 68 | request body claims, and the key is built before the body is read. |
| 69 | Keyed on the claimed peer id, one session could drain another's bucket |
| 70 | (peer ids are published in rosters) or evade its own by rotating ids. |
| 71 | - `peer` is 32 lowercase hex or 400, and `type` is logged only from a |
| 72 | fixed list. Attacker-chosen text never becomes a map key or a log |
| 73 | argument. |
| 74 | - Hub errors other than the four sentinels are logged and answered 500. |
| 75 | `ErrNotYours` and `ErrUnknownPeer` share one byte-identical 404 body |
| 76 | including the boot id. |
| 77 | |
| 78 | **The pages** |
| 79 | |
| 80 | - The removal confirm page checks fresh member standing *and* an |
| 81 | admitted member participation (`hub.PeerNameFor`), and names the |
| 82 | person, not the meeting. |
| 83 | - After a removal the confirm tab goes to `/m/{sqid}/join` — inert, |
| 84 | opens no stream — not back to the room. Both confirm pages' Cancel and |
| 85 | back links go there too. |
| 86 | - The takeover confirm page 303s to `/join` when this session holds |
| 87 | nothing to take over, and re-checks standing before `hub.Takeover`. |
| 88 | - Toggle buttons keep a fixed accessible name (`aria-label` Microphone / |
| 89 | Camera) with `aria-pressed`, plus a child span carrying the state text |
| 90 | — "On"/"Muted", "On"/"Off" — marked `aria-hidden`. Never colour alone, |
| 91 | and never a label that changes under the person's cursor. |
| 92 | - The three device pickers use `ui/field-select`. AGENTS.md's `[x]` |
| 93 | Fields rule is hard: `devices.js` fills the vendored component rather |
| 94 | than hand-rolling an equivalent. |
| 95 | - Load-time `getUserMedia` on the holding screen stands, with a |
| 96 | `visibilityState` guard and a `visibilitychange` retry: it is a page |
| 97 | the person navigated to on purpose. |
| 98 | |
| 99 | **The client** |
| 100 | |
| 101 | - Every terminal state carries a way out: a link to `/m/{sqid}/join`. |
| 102 | - A stream that dies moments after its `hello` has proved nothing: the |
| 103 | backoff is cleared only by a stream that stayed open five seconds. |
| 104 | - The stall ceiling is gated on time as well as count — eight stalls |
| 105 | *and* sixty seconds since the last `hello` — so a room that is merely |
| 106 | slow is not declared unreachable. |
| 107 | - `terminal()` clears `admitted` and keeps `peer`: the peer id names |
| 108 | what this tab held, which is worth having in a diagnosis, and nothing |
| 109 | reconnects with it once `stopped` is set. |
| 110 | |
| 111 | **The gates** |
| 112 | |
| 113 | - The browser gate reports a client failure as `t.Skip` carrying the |
| 114 | exact text, never by deleting or loosening a scenario. |
| 115 | - The buffer-overrun scenario pins `SO_SNDBUF` through a custom |
| 116 | listener: without it the gate depended on the machine's `tcp_wmem`. |
| 117 | - The renewal scenario is production-shaped — a six-second TTL through |
| 118 | the development-only `X-Meet-Test-TTL` header, with a floor of four — |
| 119 | rather than lowering the client's `HEALTHY_STREAM`. |
| 120 | |
| 121 | --- |
| 122 | |
| 123 | ## 2. Whole-branch review findings, and what happened to them |
| 124 | |
| 125 | Whole-branch review at `cfc0dc0`: 53 files, 13,338 insertions, all three |
| 126 | gates green, 48 mutations applied to a throwaway copy — 40 killed, 8 |
| 127 | survived, every survivor a test gap rather than a bug. Verdict: merge |
| 128 | with W-1 and W-2 fixed first. |
| 129 | |
| 130 | | # | Finding | Disposition | |
| 131 | |---|---|---| |
| 132 | | W-1 | The five guest-isolation audience filters (§4.3, ruling 9) are gated by nothing: each could be mutated away with the whole suite green, including two §13 names as slice-2 gates and one the browser `guest` scenario was written to catch | **Fixed in this wave.** Five hub tests, three callstest tests over HTTP, and a `guest` scenario with a second guest left knocking. Every mutation the reviewer lists is now killed. | |
| 133 | | W-2 | The branch's whole record lives in a gitignored directory, and the spec was never revised past 8 | **Fixed in this wave.** Spec revision 9 (§15) carries the ten departures with their reasons; this file carries the rest. | |
| 134 | | W-3 | Nothing proves the boot id changes across a restart | **Fixed in this wave.** The browser `restart` scenario compares `state().boot` before and after. | |
| 135 | | W-4 | Dropping the subject from the hub session key is caught by nothing | **Fixed in this wave**, differently from the suggestion. The reviewer proposed signing out and in again over HTTP; that does not gate the claim, because re-signing-in changes the cookie value too and the key would move with or without the subject. The derivation is now a function of its own (`hubSessionKey`) with a test that changing the subject alone, the salt alone or the cookie alone each changes the key, and that the key is neither the cookie value nor a bare hash of it. | |
| 136 | | W-5 | Two guards with no test: `Remove`'s self-target check, `hubStanding`'s ended-meeting early return | **Fixed in this wave.** The first has a test; the second is belt-and-braces behind callers that check `EndedAt` first, and its comment now says so. | |
| 137 | | W-6 | `-race` cannot be run the way the repo is configured, on the first slice with real concurrency | **Fixed in this wave**, with a caveat: `make race` exists and AGENTS.md names it as the one exception to the cgo-free rule. The CI step is present but guarded — see §4. | |
| 138 | | W-7 | `lost{buffer}` is sent for three endings that are not buffer overruns | **Carried — next fix wave.** See §3. | |
| 139 | | W-8 | `TERMINAL.badlast` is unreachable | **Fixed in this wave.** `refused()`'s 400 arm and `TERMINAL.badlast` are gone, with a comment saying why they could not fire: both callers pass a probe response, the stream's only 400 is a `last` it could not parse, and neither probe form sends `last`. The attaching stream's 400 stays invisible to `EventSource` and ends as a stall, which is what already happened. The copy catalogue row is updated. | |
| 140 | | W-9 | Three wordings for one move | **Carried — the copy pass.** Already in the tracked copy catalogue. | |
| 141 | | W-10 | The test hook ships to production | **Recorded, not fixed.** See §3. | |
| 142 | | W-11 | Three hub maps only ever grow | **Recorded, not fixed.** See §3. | |
| 143 | | W-12 | The tick has no per-pass deadline and does serial database reads | **Fixed in this wave.** Each pass runs under a deadline of `tickEvery`, with a test that a read which never returns cannot stall the pass. | |
| 144 | |
| 145 | Mutation survivors the reviewer reported, and where each is killed now: |
| 146 | |
| 147 | | Predicate | Killed by | |
| 148 | |---|---| |
| 149 | | `rosterFor`'s member-only waiting queue | hub, callstest, browser | |
| 150 | | `rosterFor`'s admitted-only mesh | hub, callstest | |
| 151 | | `admitLocked`'s admitted-only `peer` fan-out | hub, callstest, browser | |
| 152 | | `refreshRostersLocked`'s admitted-only refresh | hub | |
| 153 | | `knock`'s admitted-members audience | hub, callstest, browser | |
| 154 | |
| 155 | --- |
| 156 | |
| 157 | ## 2a. Codex round 1, and what happened to it |
| 158 | |
| 159 | An independent review by Codex, judged at `cfc0dc0` and applied on top |
| 160 | of the whole-branch fix wave (`67d0ead`). One P1, six P2s, two P3s; |
| 161 | verdict: request changes. Every one is fixed. Line numbers in the |
| 162 | original moved by a few lines between `cfc0dc0` and the wave that |
| 163 | applied these. |
| 164 | |
| 165 | | # | Finding | Disposition | Commit | Test | |
| 166 | |---|---|---|---|---| |
| 167 | | S2-1 (P1) | A removed member keeps relay, reset and resume authority until the tick: `Relay`, `Reset`, `CanResume` and `Resume` check the participation and never re-derive standing, which contradicts §2.5's own paragraph | **Fixed.** The paragraph wins over the table row, and the row is corrected in spec §15.12. All four re-derive standing for a member-kind participation, outside the hub lock, re-find under it, and apply through `standingLostLocked` — so the admitted guest's ruling-14 exemption is untouched. The member is ended with `lost{standing}` and answered 404, byte-identical to an unknown peer's. `CanResume` answers without ending: a probe changes nothing (§15.3). | `3c8c6ac` | `TestAMemberWithNoStandingIsRefusedByTheNextRequestNotTheNextTick` (relay, reset, resume, probe, each before any tick, each checking the other stream sees `gone`) and `TestAnAdmittedGuestWithNoStandingKeepsRelayingResettingAndResuming` | |
| 168 | | S2-2 (P2) | A participation past its resume window is refused a resume (§15.7) but still 409s the fresh stream that refusal sends the browser to, until the tick — a false "open in another tab" | **Fixed.** `Open` ends a lapsed participation under the lock and takes its place, on the tick's own terms (`lost` unobservable, departure announced); `LiveForSession` and `LiveKindForSession` no longer count one, so the holding screen's notice and the peerless probe agree with it. Probes stay non-mutating. | `3c8c6ac` | `TestAResumePastItsWindowIsRefusedAndAFreshStreamTakesItsPlace` (renamed from `...BeforeTheTickRemovesIt`) and `TestAFreshStreamAfterALapsedResumeSucceedsBeforeAnyTick` over HTTP | |
| 169 | | S2-3 (P2) | `recoverFromNotFound` reads a signal 404 as this participation's ending, when it can equally mean the target left: the client closed a live call, then met 409 from its own participation and showed the takeover screen | **Fixed.** The sender's own participation is probed first: 204 keeps the call, drops that target's queue and resets only if the roster still lists it; 410 is terminal with the tombstone's reason; 404 is the fresh-entry ladder. | `5e8db6c` | browser `staletarget`: A's stream is cut, B leaves, A posts to B, A is answered 404 and stays in the call with its own peer id and no takeover screen, then reconciles from the roster | |
| 170 | | S2-4 (P2) | `Sub.terminate`'s eviction receive can block on a channel the reader drained, with `h.mu` held — the hub's one lock, against every room in the process | **Fixed.** The receive is non-blocking and the final send is retried once; two attempts is the ceiling. | `3c8c6ac` | `TestTerminateNeverBlocksOnAChannelTheReaderDrained` — a reader draining concurrently, a per-round deadline, and run under `-race` | |
| 171 | | S2-5 (P2) | `requestReset` discards the POST result, so a 429 or a 5xx loses the reset silently — and after an outbound deadline that reset is the only thing that can rebuild the pair | **Fixed.** Bounded retry inside the same thirty seconds, with `Retry-After`, the 429/5xx backoff, the signed-out arm and the 404 ladder. | `5e8db6c` | browser `resetretry`: every POST refused until the deadline fires, then the reset it asks for refused once with 429 — A asks again and the pair comes back | |
| 172 | | S2-6 (P2) | The outbound queue's thirty seconds is not enforced across 429s or a hanging fetch, and sleeps are not bounded by it | **Fixed.** An absolute per-message deadline covering requests, backoff and 429s, with an `AbortController` on the fetch at the deadline. | `5e8db6c` | browser `senddeadline` (continuous 429s) and `sendhang` (a route that never settles) | |
| 173 | | S2-7 (P2) | Perfect negotiation has no `isSettingRemoteAnswerPending`, so an offer arriving while an answer is being applied reads as glare and the impolite side discards it | **Fixed.** The W3C guard exactly, factored into `offerCollision(entry, signalingState)`. | `5e8db6c` | `test/browser/negotiation.test.mjs` lifts that function out of the shipped source and checks the truth table under `node --test`, wired into `make browser`; the `gap` and `replay` scenarios stay green | |
| 174 | | S2-8 (P3) | `join` and the pump process a 200 before checking `signedOut`, so a followed redirect to sign-in reads as success | **Fixed.** `signedOut` is checked first in both. | `5e8db6c` | the browser `signedout` scenario gains a second phase: `/signal` answers 303 to `/signin`, fetch follows it into a 200 HTML page, and the room ends as signed out | |
| 175 | | S2-9 (P3) | Two browser assertions pass vacuously: `playfallback` checks only that the button disappears, `devices` only that `srcObject` is truthy | **Fixed.** `playfallback` asserts the tile is paused before the gesture and that frames and the element's clock both advance after it — and the rig now emulates a blocking autoplay policy properly, pausing the element rather than only rejecting the promise, without which the assertion was true before the gesture too. `devices` asserts `videoWidth > 0` and a rising `currentTime`. | commit below | the two scenarios themselves | |
| 176 | |
| 177 | Every test above was run against the unfixed code and seen to fail: |
| 178 | S2-1's four sub-cases with `memberStandingGone` stubbed to "not gone"; |
| 179 | S2-2's with `lapsedLocked` disabled, and again with only `Open`'s use of |
| 180 | it disabled; S2-4's a few thousand rounds in, with and without `-race`; |
| 181 | S2-3, S2-5, S2-6 and S2-8's against `room.js` at `67d0ead`; S2-7's |
| 182 | against the old one-line decision; `playfallback`'s against a Play |
| 183 | handler that hides the control without calling `startPlaying` (the |
| 184 | mutation Codex names), which the old assertion still passed; `devices`' |
| 185 | against a preview handed a stream with its video track removed, which |
| 186 | the old truthy-`srcObject` check also still passed. |
| 187 | |
| 188 | --- |
| 189 | |
| 190 | ## 2b. Codex round 2, and what happened to it |
| 191 | |
| 192 | The same reviewer, judged at `3eec748` — the round-1 wave — against spec |
| 193 | revision 9. Six of the nine round-1 findings are closed. Three came back |
| 194 | as new P2s because the fix was right and incomplete: a probe that still |
| 195 | wrote, a recovery that recursed, and a recovery that acted on an answer |
| 196 | that proved nothing. Two P3 test advisories. No new P1. |
| 197 | |
| 198 | | # | Finding | Disposition | Commit | Test | |
| 199 | |---|---|---|---|---| |
| 200 | | S2-10 (P2) | `memberStandingGone(..., end=false)` honoured the flag only when standing was *absent*; with standing present it wrote the kind, so a probe could demote a member to guest — and broadcast a fresh roster to the whole room while doing it | **Fixed.** A probe writes nothing whatever the answer is. The transition is real and is applied by the next mutating request, or by the tick. | `de597c6` | `TestAProbeNeverWritesAKindNorBroadcastsARoster`: a member taken off the team while holding a live grant, probed — kind unchanged and both subscribers silent — then one relay, which demotes and refreshes both rosters | |
| 201 | | S2-11 (P2) | A signal 404 whose probe answered 204 asked for a reset to the stale target; that reset was answered 404 too, probed, and asked again — each round with a fresh thirty-second budget | **Fixed.** A target the hub has 404'd is marked stale: `send` drops for it, only the first round may ask for a reset, a reset's own target-404 never asks for another, one reset per target is in flight at a time, and the recovery's retries share the caller's budget. The roster clears the mark and `reconcile` rebuilds the connection if the peer is really there. | `dc614c9` | browser `staletarget` counts every POST A makes while its roster is stale and bounds it: 1 on the fixed client, **450** on the round-1 client | |
| 202 | | S2-12 (P2) | A probe answering 429, 5xx or nothing fell through to the fresh-entry ladder — S2-3's failure, reached by another door | **Fixed.** Only the probe's own 404 enters afresh; a transient answer is retried on the stream backoff inside the shared budget, and a recovery that never gets an answer leaves the participation alone. | `dc614c9` | browser `staletarget` refuses the recovery's probe 503 before letting it through: A keeps its peer id and never sees the takeover screen | |
| 203 | | S2-13 (P3) | `senddeadline` and `sendhang` accepted anything under sixty seconds, and `sendhang` never showed the request had hung or that the page aborted it | **Fixed.** Both measure from the first intercepted message and require 25–35 s. `sendhang` asserts the hung count and watches `requestfailed` for the cancellation. | commit below | the two scenarios: 30018 ms and 30238 ms on this machine, one cancelled request | |
| 204 | | S2-14 (P3) | `TestTheTickDeadlineIsTheTickInterval` asserted a constant equalled itself | **Fixed.** The scheduling path is `tickLoop(ctx, every)` with the pass behind a seam that is nil in production; the test runs the real loop in milliseconds and records the deadline it hands out. | `de597c6` | `TestTheSchedulingPathGivesEachPassTheIntervalAsItsDeadline` | |
| 205 | | Negotiation note | The six decision cases would stay green with the handler's flag assignment removed | **Fixed.** `negotiation.test.mjs` also drives `onAnswer` then `onOffer` through a fake connection whose `setRemoteDescription` can be held open. | `dc614c9` | the seventh case fails with the assignment removed | |
| 206 | |
| 207 | Every test above was run against the unfixed code and seen to fail: |
| 208 | S2-10's with the kind write restored ("the probe wrote the kind: guest"); |
| 209 | S2-14's with the loop passing three times its interval; S2-12's and |
| 210 | S2-11's against `room.js` at `3eec748` — the first ended terminal with |
| 211 | `peer: null`, and the second, with the 503 phase disabled so the cycle |
| 212 | could run at all, reached 450 POSTs in thirteen seconds. The seventh |
| 213 | negotiation case fails on "the answer handler must raise the flag before |
| 214 | it awaits". |
| 215 | |
| 216 | The two round-2 client findings share one shape, and it is worth naming |
| 217 | because it will come round again: **a refusal is evidence about exactly |
| 218 | one thing.** A 404 from `/signal` is evidence about the target, not |
| 219 | about us; a 503 from a probe is evidence about neither. Both bugs were a |
| 220 | client treating an answer as if it settled more than it did. |
| 221 | |
| 222 | --- |
| 223 | |
| 224 | ## 2c. Codex round 3, and what happened to it |
| 225 | |
| 226 | Judged at `f1a0f9c`. S2-10, S2-13, S2-14 and the negotiation note are |
| 227 | closed. S2-11 and S2-12 came back half-closed as one P2 — the shared |
| 228 | budget was named in the commit message and not actually shared — and one |
| 229 | P3 on a test predicate. No new P1. |
| 230 | |
| 231 | | # | Finding | Disposition | Commit | Test | |
| 232 | |---|---|---|---|---| |
| 233 | | S2-15 (P2) | The recovery's "one budget" was three: the pump passed no deadline (a fresh thirty seconds), a successful diagnosis asked for a reset with another, and `probe()` had no cancellation, so the deadline was checked only between attempts and a hanging probe held its target's pump or reset marked in flight indefinitely | **Fixed.** The message's deadline goes from the queue head through `recoverFromNotFound` and into any reset it initiates; `probe(path, deadline)` bounds the fetch **and** the body read with an `AbortController`; the remaining time is checked before every attempt; and a recovery that runs out marks the target stale and releases it, leaving the next roster to settle it. The resume and fresh-entry probes are bounded too — a hanging probe there would have ended this browser's reconnecting. | `6bd2f00` | browser `probehang`: the 404 is answered twenty seconds late and the probe that follows never answers. The target is let go at **30236 ms** from the first intercepted signal — a restarted clock lands near 50000 — and A stays in the call | |
| 234 | | S2-16 (P3) | `staletarget` could call the recovery complete before the stream came back: `s.peers` is local `pcs` keys, and `staleTarget` closes that connection while the stream is still cut | **Fixed.** Reconciliation now requires a stream that actually reattached (the route's own attach counter) and a roster that actually arrived — `state()` gained a roster counter for it — before the final POST bound is checked. | commit below | the strengthened `staletarget`: attaches 1→2, rosters 1→2 | |
| 235 | | Negotiation note | Case seven could not tell whose cleanup it watched, because `onOffer` clears the pending-answer flag too | **Fixed.** An eighth case drives an answer with no offer behind it. | `6bd2f00` | `the answer handler clears its own flag` | |
| 236 | | `git diff --check` | A blank line at the end of the copy catalogue | **Fixed.** | commit below | `git diff --check` is clean | |
| 237 | |
| 238 | Verified failing first: `probehang` against `room.js` at `f1a0f9c` — |
| 239 | the target was never let go, and the measurement read 70056 ms at the |
| 240 | scenario's own ceiling; the strengthened `staletarget` predicate with |
| 241 | the stream deliberately left cut — attaches 1→1, rosters 1→1, never |
| 242 | true, where the round-2 predicate was satisfied the moment |
| 243 | `staleTarget` closed the connection locally; the eighth negotiation |
| 244 | case fails with the answer handler's flag assignment removed. |
| 245 | |
| 246 | Also recorded from this round, not changed: the targeted-404 assumption |
| 247 | behind `stale` is exact for the path the client actually walks — an |
| 248 | admitted sender addressing a previously admitted, different peer — and |
| 249 | the reviewer confirmed the server's other 404s (unadmitted sender, |
| 250 | unadmitted target, self-target) are not reachable from it. A stale |
| 251 | generation is a 409, not a 404. Slice 3's `/ice` and slice 4's chat |
| 252 | should re-check that list before reusing `stale`. |
| 253 | |
| 254 | --- |
| 255 | |
| 256 | ## 2d. Codex round 4, and what happened to it |
| 257 | |
| 258 | Judged at `2f7f63f`. S2-16 closed. S2-15 half-closed through one P2, and |
| 259 | it is the same lesson a third time: the budget was threaded correctly |
| 260 | and then leaked out of the one exit nobody was watching. |
| 261 | |
| 262 | | # | Finding | Disposition | Commit | Test | |
| 263 | |---|---|---|---|---| |
| 264 | | S2-17 (P2) | An aborted probe or a transient refusal that exhausted `waitWithin` returned from `recoverFromNotFound` without releasing the target. The queue survived, the pump's `finally` restarted it, its expired head called `unreachable`, and that opened a reset with a fresh thirty seconds — measured at 31000 for the offer and 61000 for the reset, with zero stale-target calls. Separately `resetLoop` checked the participation before every attempt but never the clock, so one POST could go out past the deadline meant to cover it | **Fixed.** Both loops round back to a single check at the top, which is now the only place a spent budget is answered, and it answers by releasing the target — queue dropped before the pump is released, connection closed, mark set, next roster settles it. `resetLoop` gained the same check. `createPC` clears the mark, because building a connection to a peer means the server has just named it. | `50953f4` | a pure node test — the recovery lifted out of the shipped source, given fifty milliseconds and a probe that never answers, must release its target once and ask for nothing — and `probehang`, which now asserts the offer was the *only* thing A posted, that it is still the only thing ten seconds later, and that A holds the target marked stale | |
| 265 | |
| 266 | Verified failing first: the node test with the exhaustion `return` |
| 267 | restored — "the target must be released exactly once"; `probehang` |
| 268 | against `room.js` at `2f7f63f` — **2** POSTs where the fixed client |
| 269 | makes 1, and no stale mark at all. The second POST is the fingerprint of |
| 270 | the finding: it is the reset that ran on a fresh thirty seconds while |
| 271 | the scenario's earlier assertions were all passing. |
| 272 | |
| 273 | `state()` gained the stale set for this, which is the third thing the |
| 274 | gate has needed the client to say about itself (`boot`, `rosters`, |
| 275 | `stale`). Each is a fact no DOM shows and no HTTP answer reveals: a |
| 276 | client that has *stopped* asking for something looks exactly like one |
| 277 | that never had to. |
| 278 | |
| 279 | --- |
| 280 | |
| 281 | ## 3. Carried items, and where they go |
| 282 | |
| 283 | **Next fix wave** |
| 284 | |
| 285 | - **W-7, `lost{buffer}` for three endings that are not buffer |
| 286 | overruns** (`admission.go`'s `Leave`, `hub.go`'s unjoined timeout and |
| 287 | lapsed resume window). `buffer` is the one reason the wire contract |
| 288 | defines as *not* terminal — "followed by a resume". Today this is |
| 289 | harmless because `room.js` sets `stopped` and closes the stream before |
| 290 | the event can be read, but the safety lives entirely in the client, and |
| 291 | §8's drive surface will have other callers. A peer whose join keeps |
| 292 | failing can loop: unjoined timeout → `lost{buffer}` → resume → 404 same |
| 293 | boot → fresh stream → join → thirty seconds → repeat, and the stall |
| 294 | counter never fires because each round gets a `hello`. **Not fixed here |
| 295 | because either fix changes the wire contract** — a new terminal reason, |
| 296 | or an ending that sends no event at all — and this wave took no |
| 297 | wire-contract changes. Fix it with the client change in the same |
| 298 | commit, and re-run the browser gate. |
| 299 | - **W-10, the test hook in the release binary.** `window.meet.state()` |
| 300 | is defined unconditionally and `TestHandle` compiles into the release |
| 301 | build (unreachable without `NewForTest`, which `main` never calls). |
| 302 | Neither exposes anything secret, and the CSP keeps third-party script |
| 303 | off the page. Confining either one costs more than it buys today: |
| 304 | build-tagging `TestHandle` means `make ci` grows a `-tags` argument, |
| 305 | and gating `window.meet` on the environment means the browser gate |
| 306 | tests a page production does not serve. |
| 307 | - **W-11, three hub maps that only grow.** `h.rooms` is never deleted |
| 308 | from (declared; `End`'s comment explains why a reaper is dangerous). |
| 309 | `h.bySession[sqid]` keeps an empty inner map per room forever, and |
| 310 | `h.byPerson` a zero entry per person forever. All three are bounded by |
| 311 | real meetings and real people — an outsider cannot mint a room id that |
| 312 | reaches `Open`, because standing is checked first — so this is a slow |
| 313 | leak on a long-lived process, not an attack surface. It is also |
| 314 | invisible while every deploy empties the hub (§4). Fix it with, or |
| 315 | after, whatever ends that. |
| 316 | - **A just-added member gets 404 from the removal confirm page until the |
| 317 | next tick.** Recorded in spec §15.11. |
| 318 | - **Watch: the limiter now sits behind a gate that takes the hub lock.** |
| 319 | Revisit if hub-lock hold time grows. Recorded in spec §15.5. |
| 320 | |
| 321 | **The copy pass** |
| 322 | |
| 323 | - W-9's three wordings for one move: "Use this tab instead" (holding |
| 324 | notice and `room.js`'s `wayOut`), "Use this tab" (takeover confirm |
| 325 | submit), and "This meeting is already open in another tab" versus |
| 326 | "This meeting is open in another tab." |
| 327 | - The holding notice's "already open in another tab" is untrue during |
| 328 | the resume window; "You are already in this meeting from this browser." |
| 329 | is the candidate. |
| 330 | - "Remove `<name>` from the call?" reads oddly for a guest who is not in |
| 331 | the call yet. |
| 332 | |
| 333 | All three are in `docs/superpowers/copy/2026-09-10-slice-2-strings.md`. |
| 334 | |
| 335 | **Slice 3 — relay and `/ice`** |
| 336 | |
| 337 | - `createPC` passes `{ iceServers: [] }` (`room.js`), with the floor |
| 338 | notice keyed on `cfg.RelayConfigured`. Those are the two places to |
| 339 | change. |
| 340 | - `GET /m/{sqid}/ice` is §2.5's "participation, admitted" row — the same |
| 341 | authority as a non-`join` signal. `hub.Find` plus `p.Admitted` is the |
| 342 | whole check; do not consult standing (ruling 14). |
| 343 | - Renewal (§5.3) needs a timer that survives a resume. The browser gate |
| 344 | already has a `renewal` scenario for the *stream* TTL using the |
| 345 | development-only `X-Meet-Test-TTL` header; copy that shape for the |
| 346 | credential rather than inventing a second one. |
| 347 | - ICE restart already has a client path: `restartIce()` with |
| 348 | `requestReset` as the fallback, and the `replay` scenario asserts |
| 349 | `have-local-offer` to prove it fires. |
| 350 | - W-12 is now bounded, but `/ice` adds a per-request credential mint on |
| 351 | a path that already does a database read. Watch the tick's deadline. |
| 352 | |
| 353 | **Slice 4 — chat and screen share** |
| 354 | |
| 355 | - `chat` and `share` are already named in `messages.go` and refused with |
| 356 | 400 in `signal.go`. Removing that arm and adding the fan-out is the |
| 357 | whole server change; `broadcastLocked(r, m, admittedPeers)` is the |
| 358 | audience. |
| 359 | - §4.4's 2 KB chat cap has no code yet. It must be checked against the |
| 360 | decoded text, not the body, and it must not read the text for anything |
| 361 | else — the captured-log test extends to *accepted* chat in slice 4, |
| 362 | which is the first time an accepted message has content worth |
| 363 | protecting. |
| 364 | - W-1 is fixed, so chat's "arrives at the admitted context and not at a |
| 365 | waiting one" is now two lines: extend |
| 366 | `internal/calls/hub/isolation_test.go` and |
| 367 | `internal/callstest/isolation_test.go`, which exist for exactly this. |
| 368 | - Screen share adds a second video track per peer. `tile()` keys on peer |
| 369 | id and `renderTiles` rebuilds from `pcs`, so the tile model needs a |
| 370 | track dimension **before** the share arrives, not after. |
| 371 | |
| 372 | --- |
| 373 | |
| 374 | ## 4. Deploying this slice |
| 375 | |
| 376 | **Every deploy drops every live call.** The hub is per process and dies |
| 377 | with it. This is spec'd (§4.2, §4.5) and not solved: slice 2 is the |
| 378 | first release where it has a user-visible cost. |
| 379 | |
| 380 | What a deploy does to people who are in a meeting: |
| 381 | |
| 382 | - every browser's resume is refused with a 404 carrying the new boot id; |
| 383 | - each one closes every peer connection, then joins afresh; |
| 384 | - **members** re-enter at once and negotiate again — a few seconds of |
| 385 | black tiles; |
| 386 | - **guests go back to waiting** and must be admitted again by a member. |
| 387 | |
| 388 | So: **deploy outside meeting hours** until slice 3, or until a drain |
| 389 | exists. There is no partial mitigation worth the words — the participants |
| 390 | are in memory, and the memory goes. |
| 391 | |
| 392 | Nothing else about a deploy is unusual. Migrations apply at boot, once |
| 393 | each; `GET /healthz` and `GET /api/version` are answered by the framework |
| 394 | outside app middleware. |
| 395 | |
| 396 | --- |
| 397 | |
| 398 | ## 5. The gates, and how to run them |
| 399 | |
| 400 | | Gate | Command | Notes | |
| 401 | |---|---|---| |
| 402 | | The one gate | `CGO_ENABLED=0 make ci` | vet, gofmt, tests, migration check. Cgo-free, as AGENTS.md requires. | |
| 403 | | Race | `CGO_ENABLED=1 go test -race ./internal/...`, or `make race` | **Needs `CGO_ENABLED=1`**: `-race` requires cgo, and the repo is cgo-free everywhere else. This builds a test binary, never a release, which is why it is the one exception. | |
| 404 | | Browser | `make browser` | ~300 s, 23 scenarios, plus one node test (`test/browser/negotiation.test.mjs`, §5.1's glare decision and the handler race it turns on, no browser needed). Needs node, a network fetch of Playwright and Chromium, and **UDP** — inside the Claude Code bash sandbox Chromium gathers zero ICE candidates and every call fails with frames stuck at 0, so **run this target with the sandbox off**. | |
| 405 | |
| 406 | `make browser` is deliberately not part of `make ci`: the one gate the |
| 407 | team runs on every push must not depend on a toolchain the app itself |
| 408 | does not use. Run it before every review that touches a page or the |
| 409 | client. Nothing enforces that, which is a known weakness and the reason |
| 410 | this row is here rather than in a comment. |
| 411 | |
| 412 | **It is not perfectly stable.** One run in this fix wave had |
| 413 | `rosterpair` hang for 120 s and fail, on a tree where it passes in 4.5 s |
| 414 | alone and passed 18/18 on the run before and the run after. That |
| 415 | scenario cuts a stream mid-call and waits for a resume, so it has the |
| 416 | most timing in it of any of them. A single red `rosterpair` is worth |
| 417 | re-running before it is worth debugging; two in a row is not. |
| 418 | |
| 419 | **The race step in CI.** `.amadan/ci.d/50-race` runs `make race` when |
| 420 | the runner has a C compiler and prints a skip line when it does not. The |
| 421 | guard is there because the cloud runner's provisioning |
| 422 | (`amadan/docs/ci-cloud-runner.md`) installs the Go toolchain and Node and |
| 423 | never names a C compiler, and a step that fails on every push would be |
| 424 | worse than no step. **When somebody confirms `cc` on the runner, delete |
| 425 | the guard** — a race gate that can silently skip is half a gate. |
| 426 | |
amadan