amadan
docs: slice 3 record and spec notes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
2 files changed,
+292
−0
docs/superpowers/reviews/2026-09-10-slice-3-record.md+196 −0docs/superpowers/specs/2026-09-07-meet-design.md+96 −0
CI passed — run details
diff --git a/docs/superpowers/reviews/2026-09-10-slice-3-record.md b/docs/superpowers/reviews/2026-09-10-slice-3-record.md| new file mode 100644 |
| index 0000000..a0f7695 |
| --- /dev/null |
| +++ b/docs/superpowers/reviews/2026-09-10-slice-3-record.md |
| @@ -0,0 +1,196 @@ |
| +# Meet slice 3 — the record |
| + |
| +Branch `slice/3-relay`, `0690a7a..67f2292` (started on main `2215f26`). |
| +Spec: `docs/superpowers/specs/2026-09-07-meet-design.md`, revision 9 |
| +(§15.13 and §15.14 carry this slice's two wire/behaviour rulings). |
| + |
| +Slice 3 built the platform relay: `CARLOS_RELAY` parsing and minting |
| +against the platform's own vector, `GET /m/{sqid}/ice`, and the |
| +browser client's renewal before the credential's deadline — six tasks, |
| +each with its own review and fix rounds, kept in |
| +`.superpowers/sdd/2026-09-10-meet-slice-3-relay/progress.md` and the |
| +`task-N-report.md` files. Like slice 2'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 and the tasks that followed it: |
| + |
| +- **`/ice` carries the peer as `?peer=`**, the way the stream's own |
| + resume already does, not a body. **Ruling.** A native client needs |
| + no body on a GET, and `/ice` has nothing else a body would carry. |
| + Cost if wrong: none observable. Written into spec §15.13. |
| +- **A member's `/ice` re-derives standing and, when it is gone, ends |
| + the participation** — mutating, exactly like `/signal`'s own |
| + §15.12 correction — while a guest never consults standing at all |
| + (ruling 14). Cost if wrong: one extra standing read per `/ice` call, |
| + which is rare — once per two and a half hours per call, on the |
| + client's own schedule. Written into spec §15.13. |
| +- **The client's fetch-failure retry stays at one minute.** The gate |
| + proves the warning appears and the call stays connected on a failed |
| + fetch; it does not prove the one-minute recovery, which would cost |
| + the gate roughly seventy seconds per scenario for no more assurance. |
| +- **The relay is enabled on the app after the edge deploy**, verified |
| + by `carlos relay status` and the instance's own startup line; no test |
| + in this branch touches a real relay (§13, and §4 below). |
| +- **The renewal schedule is the earlier of a ratio estimate and a |
| + clock estimate**, never the server's `renewAt` read as a bare |
| + instant: `(expiresAt − renewAt) × 5` survives clock skew because it |
| + is built from two of the server's own instants, while |
| + `renewAt − fetchedAt` catches the truncation the ratio reading can |
| + mishandle at short lives. Found during Task 4's fix round 2, after |
| + round 1's schedule amplified the server's own second-truncation at |
| + short test lives; written into spec §15.14. |
| +- **`memberStandingGone` returns `(bool, error)`**, not the brief's bare |
| + `error` — `hub.Authorize` mirrors `Relay`'s own |
| + `gone, err := …; if err != nil { return err }; if gone { return |
| + ErrNotYours }` shape. An implementation decision the spec has no |
| + opinion about (Task 3). |
| +- **The development header's TTL test uses 12s/10s, not the brief's 2s |
| + example** — at two seconds, `life*5/6`'s fractional remainder can |
| + round to the same whole second as `life` once both are truncated by |
| + `.Unix()`, intermittently making `renewAt == expiresAt`. Any TTL that |
| + is a multiple of six seconds keeps the margin clear of that rounding |
| + (Task 3; the same fact governs the browser gate's own TTL choice, |
| + §4 below). |
| +- **`icerenewal` uses an 18-second TTL, not the brief's 12s** — twelve |
| + leaves two seconds between the renewal starting and the deadline that |
| + prints the expired sentence, which this scenario asserts stays away; |
| + eighteen leaves three, and the run measures the margin directly so a |
| + change to either end is caught (Task 5). `icefail` keeps 12s, where |
| + the deadline arriving *is* the point. |
| +- **Recovery from a failed `/ice` fetch is left unproved in the browser |
| + gate**, exactly as the ledger's own ruling allows: `ICE_RETRY` is a |
| + fixed minute by design, and watching it clear costs roughly seventy |
| + seconds for no more assurance than the warning appearing already |
| + gives. |
| + |
| +The rest of each task's own decisions — the suffix validation regexes, |
| +the JSON golden-shape pins, `atomic.Value`'s nil-but-typed slice, the |
| +`--host-resolver-rules` Chromium flag that keeps the platform's relay |
| +hostname off the network entirely — are in the task reports themselves |
| +and are not repeated here; §2 gives each task's disposition, not its |
| +full diff. |
| + |
| +--- |
| + |
| +## 2. Per-task review dispositions |
| + |
| +| Task | Built | Review | Fix rounds | Final | |
| +|---|---|---|---|---| |
| +| 1 — `internal/meet/relay` | `Parse`, `Region`, `Mint`, `Credential`, `ICEServer`; the platform vector test | Needs fixes: HIGH — `Parse` echoed the raw region field in its errors; MEDIUM — no redacting `String()` on `Region`; MEDIUM — the no-echo test didn't prove its own name; six LOW items (`[]`/`null` unification, an empty URL reaching `RTCIceServer`, an undocumented expiry precondition, unpinned no-mutation/JSON-shape/suffix-regex/base64-alphabet/TURN-case-insensitivity behaviour) | Round 1 `991352e` (combined with Task 2): all ten fixed. Re-review: three test-only pins remained, folded into Task 3's fix round. | Complete; pins verified in Task 3's fix round (`b5c29b3`, items 9–12). | |
| +| 2 — `Config.Relay`, `resolve`, the atomic floor read | `app.relay atomic.Value`, `relayRegions()`/`relayConfigured()`, `TestHandle.SetRelay`, the room-page `Floor` value wired to it | Needs fixes (production code judged right): the Task-1-inherited echo at `config.go:146/148`; a test that printed the key on failure; the with-relay build path and its one INFO line untested; the floor sentence's "relay configured ⇒ absent" direction untested (a hardcoded `Floor` was passing `make ci`); the refusal test satisfied by `relay.Parse`'s text rather than `config.go`'s own phrase; a false doc comment on the since-deleted `RelayConfigured` field; `SetRelay` unproven before Task 3 became its first caller; malformed cases exercising only the outer error | Round 1 `62853ed` (combined with Task 1): all eight fixed, including deleting the dead field once `grep` confirmed nothing else read it. | Complete. | |
| +| 3 — `GET /m/{sqid}/ice`, `hub.Authorize` | The wire shape, the §2.5 authority check, the development TTL header | Needs fixes: MEDIUM — `/ice` read the store before authority and was unlimited (no `LiveForSession` gate, no limiter); MEDIUM — the logging test passed even with a credential logged on success; MEDIUM — the dev header had no ceiling; several test gaps (ended-meeting refusal, `validPeerID`/`validSqid` load-bearing against a newline, byte-equality against a real `/signal` 404, a malformed sqid); five LOW items (the 500 arm's `path`, an overstated doc comment, log level, an unlogged meeting refusal, a duplicated test helper, an unpinned JSON entry); the guest half of the standing test not discriminating | Round 1 `b5c29b3`: all fixed except four items carried forward — see §3. Re-review: clean. | Complete. Carried: the sqid guard on `/ice` (unproved); the shared-bucket pin; the log-test comment; the family-wide `path` nit. `N4` (`Retry-After` on `/ice` 429) was folded into Task 4's fix round instead of carried. | |
| +| 4 — `room.js`, the ICE list and renewal | `fetchIce`, `scheduleRenewal`, `renewAll`/`renewOne`, `proveRenewed`, the must-succeed-by watchdog, the `data-relay` floor sentence | Needs fixes: HIGH — the watchdog's exemption compared against the wrong stamp and was unreachable; HIGH — a stale selected pair could answer for a fresh one in Chromium; MEDIUM — manual Retry erased the note and did nothing on a failed fetch; MEDIUM — the warning showed even on the floor, with no relay to fail; MEDIUM — the schedule trusted the client's clock, not a duration; six LOW items | Round 1 `9fed86c`: nine of eleven items fixed outright, two (the 429 loop, the schedule) added or partly addressed. Re-review after round 1: two half-closed — MEDIUM A, the ratio/clock amplification of the server's second-truncation at short lives (a 2s life could silently never renew; a 10s life landed the deadline 2s late); LOW B, `failedSince` stamped once per outage rather than per attempt, letting a visibility catch-up jump the retry queue — plus three nits. | Round 2 `67f2292`: A and B fixed (earlier-of-ratio-and-clock scheduling per spec §15.14; `lastTryAt` stamped every attempt); the nits fixed; and, not asked for but needed, the stale-roster-after-await defect Task 5 traced (see below) and an `adoptServers` guard against restarting a negotiation already in flight. Still owed: a Go tripwire pinning `iceRenewAfter == iceLife*5/6` against the client's own ratio arithmetic drifting from it silently, and the residual gate flake's diagnostics — see §3. | |
| +| 5 — the browser gate: `ice`, `icerenewal`, `icefail` | The three scenarios, `state().ice`/`state().renewals`, the credential-never-in-console check, the Chromium flag keeping the relay hostname off the network | No separate review pass: the task's own result was 23 passed, 3 skipped, with the skip's cause fully diagnosed and reproduced rather than papered over — `applyRoster` reconciled the roster it captured *before* awaiting `/ice`, so a `peer` event landing during that fetch was closed by the stale roster and its §4.6 counters dropped, discarding the answer that followed as an already-seen replay. The same defect, reached through a different door, explained slice 2's `staletarget` flake. | Fixed as part of Task 4's fix round 2 (`67f2292`), which also fixed a second door onto the same symptom (a superseded roster applied at all) and reordered `accept`'s generation check ahead of its cursor check. | Complete; the three scenarios un-skipped, `make browser` 26/26. | |
| +| 6 — this record and spec §15.13/§15.14 | Documentation only | — | — | This commit. | |
| + |
| +--- |
| + |
| +## 3. Carried items, and where they go |
| + |
| +- **The sqid guard on `/ice` is unproved.** Task 3's own peer-newline |
| + test (`TestIceNeverLogsAMalformedPeerVerbatim`) has no sqid |
| + equivalent: nothing proves an oversized or newline-bearing sqid never |
| + reaches the limiter key or the log the way an oversized peer is |
| + proven not to. **Destination: the next fix wave**, alongside whatever |
| + review of the whole branch runs next — mirror the peer test with a |
| + sqid-shaped input. |
| +- **`/signal` and `/ice` sharing one rate-limiter bucket is |
| + implemented but not pinned.** `TestIceRateLimitsOneSessionsBurst` |
| + proves `/ice` alone is rate-limited; nothing spends the budget on one |
| + endpoint and shows the other refused because of it. **Destination: |
| + the next fix wave** — a test that exhausts the bucket through |
| + `/signal` and then gets 429 from `/ice` in the same session and room, |
| + or the reverse. |
| +- **The log-test comment says authority where the arm is unbound.** |
| + `ice.go`'s refusal logging distinguishes an `outcome=unbound` refusal |
| + (no live participation at all, checked first, before the store or the |
| + limiter) from an `outcome=authority` refusal (`hub.Authorize`'s own |
| + four sentinel errors, checked last); a comment near the log-content |
| + test names the wrong one. **Destination: the next fix wave** — a |
| + one-line comment fix, found while writing this record and not worth |
| + a separate commit on its own. |
| +- **The family-wide `path` nit.** Every no-session-subject 500 across |
| + the handler family (`auth.go`, `stream.go`, `signal.go`, `ice.go`, |
| + `handlers_room.go`'s four arms) logs `r.URL.Path` on an arm that |
| + should be unreachable if the framework's own session middleware is |
| + doing its job — an unbounded, attacker-shaped value on a path that |
| + exists only for a bug elsewhere. Not `/ice`-specific; carried under |
| + its own name because fixing it in `/ice` alone would leave six other |
| + call sites inconsistent. **Destination: a family-wide pass**, not |
| + this slice's fix wave — touching all seven call sites at once is a |
| + different-shaped commit from anything slice 3 built. |
| +- **The Go tripwire `iceRenewAfter == iceLife*5/6`.** Task 4's fix |
| + round 2 moved the client's own schedule to the earlier of a ratio and |
| + a clock estimate (spec §15.14) specifically so the client's |
| + arithmetic stays correct even if the server's numbers drift — but |
| + nothing on the Go side pins that `ice.go`'s `iceRenewAfter` constant |
| + is still five sixths of `iceLife`, which is the assumption the |
| + ratio's own "renewal at five sixths of the life" shape depends on. |
| + Named as owed in Task 4's own fix-round-2 report. **Destination: the |
| + next fix wave**, in `internal/meet/ice_test.go` — a one-line |
| + assertion, not a behaviour change. |
| +- **The residual gate flake, `connectionState: "new"`.** Roughly one |
| + full `make browser` run in three loses one scenario — a different one |
| + each time — always at the first pairing, always with the same |
| + signature on both browsers: descriptions exchanged and `stable`, but |
| + `connectionState: "new"` — no ICE transport ever left the new state, |
| + meaning gathering and checking never started. This is not the |
| + stale-roster defect (whose signature was `peers: []`); it matches the |
| + Makefile's own documented environment symptom, hitting one scenario |
| + at a time rather than every scenario in the run. Three things were |
| + tried against it in Task 4's fix round 2 and none moved it: the |
| + `accept`-ordering fix (kept regardless — it closes a real hole), a |
| + negotiation nudge for a lost `negotiationneeded` event (reverted — no |
| + evidence it was the cause), and re-running affected scenarios in |
| + isolation, where all of them pass every time. **Destination: the next |
| + diagnostics pass**, in order of cost — run the gate on CI hardware and |
| + see whether the rate changes; add a check to `call.mjs` that reports |
| + `getStats`'s own candidate counts when a pair fails to connect, to |
| + settle "gathered nothing" against "gathered and could not pair" in |
| + one run; only then look at the client again. |
| + |
| +--- |
| + |
| +## 4. Deploying this slice |
| + |
| +Enabling the relay is not free the way slice 2's ordinary deploys are. |
| +**`carlos relay enable --app meet` re-renders the app's config and |
| +restarts the instance**, which drops every live call exactly as an |
| +ordinary deploy does (spec §15.11 — the hub is per process and dies |
| +with it). So: **enable the relay in the same deploy window as the edge |
| +deploy**, not as a separate step days later that catches a room full of |
| +people by surprise a second time. |
| + |
| +Verification is two commands, not one: `carlos relay status --app |
| +meet` confirms the platform's own side, and the instance's own startup |
| +line — `meet: relay configured regions=N` — confirms the app actually |
| +parsed what the platform handed it, at the region count expected. Both |
| +matter; the platform reporting a relay attached and the app logging |
| +zero regions would be the config having reached the process malformed, |
| +caught only outside development (§15.13's startup refusal) or silently |
| +accepted as no relay if the value were simply empty. |
| + |
| +**No test on this branch touches a real relay.** Every gate — `relay`'s |
| +own vector test, the config tests, `/ice`'s tests, and the three |
| +browser scenarios — proves the contract against fixtures and a fake |
| +`/ice`. The platform relay itself is verified only by the two commands |
| +above, after the deploy that enables it. |
| + |
| +--- |
| + |
| +## 5. The gates, and how to run them |
| + |
| +| Gate | Command | Notes | |
| +|---|---|---| |
| +| The one gate | `CGO_ENABLED=0 make ci` | vet, gofmt, tests, migration check — unchanged from slice 2. | |
| +| Race | `CGO_ENABLED=1 go test -race ./internal/...` | Run repeatedly through this slice's fix rounds; green throughout. | |
| +| Browser | `make browser` | 26 scenarios, three of them (`ice`, `icerenewal`, `icefail`) new this slice. **Needs the sandbox off** — inside the Claude Code bash sandbox Chromium gathers zero ICE candidates and every call fails, as slice 2's record already noted. **A development TTL header must be a multiple of six seconds** — `X-Meet-Test-Ice-TTL`'s life is split `renewAt = life*5/6`, `.Unix()`-truncated on both ends, and only a multiple of six keeps the margin clear of that truncation (a twelve-second life reads exactly 10 and 12; a ten-second life reads 8 and 10, where the same arithmetic renews at the very moment the credential dies). **The run takes about 5.5 minutes** — 317.95 s measured over ten full runs in Task 4's fix round 2, five of the ten and the last two clean at that mark, against 287 s in Task 5's own run with the three ICE scenarios skipped. | |
| + |
| +`make browser` is still deliberately outside `make ci`, for the reason |
| +slice 2's record already gives. The residual flake in §3 above is why a |
| +red run here is worth a second run before it is worth debugging, exactly |
| +as slice 2's `rosterpair` flake was. |
diff --git a/docs/superpowers/specs/2026-09-07-meet-design.md b/docs/superpowers/specs/2026-09-07-meet-design.md| index 022e7c8..9e2ff51 100644 |
| --- a/docs/superpowers/specs/2026-09-07-meet-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-meet-design.md |
| @@ -1486,3 +1486,99 @@ limiter's thirty requests a second per session and room, which was |
| already the ceiling on this endpoint, and by the four streams per |
| person of §4.4. A guest costs nothing extra: the kind is checked first |
| and a guest never reaches the read. |
| + |
| +### 15.13 §5.2 — the `/ice` wire shape, and its authority |
| + |
| +**Decision.** `GET /m/{sqid}/ice?peer=<id>` answers `200 |
| +{"iceServers": [...], "expiresAt": <unix seconds>, "renewAt": <unix |
| +seconds>}`. `iceServers` is §5.2's own list shape (`{urls, username?, |
| +credential?}`); a bare `stun`/`stuns` entry carries neither key. With |
| +no relay configured, the answer is `{"iceServers": [], "expiresAt": 0, |
| +"renewAt": 0}` — the floor, not a refusal. Every failure is the |
| +identical `404 {"error":"not-found","boot":…}` `/signal` itself |
| +answers, byte for byte: a malformed sqid, a malformed peer, an unknown |
| +room, an unknown or unbound peer, a peer belonging to another session, |
| +a waiting guest, a member who has lost standing, or a meeting that has |
| +ended. |
| + |
| +**The peer is a query parameter.** `?peer=` carries the id the way the |
| +stream's own resume already does, not a body. **Ruling.** A native |
| +client needs no body on a GET, and `/ice` has nothing else a body would |
| +carry. |
| + |
| +**Authority, before the store is ever read.** `/ice` runs |
| +`LiveForSession` — the same gate `/signal` runs before its own limiter |
| +— refusing the identical 404 for a session with no live participation |
| +in this exact room, before either the store or the rate limiter is |
| +touched. It then spends the shared session+room limiter bucket |
| +`/signal` already spends: a burst against either endpoint from one |
| +session drains the one budget it can ever spend, never two. Only past |
| +both gates does `/ice` read the meeting row — to catch a meeting the |
| +hub does not yet know has ended — and then call `hub.Authorize`, the |
| +§2.5 last-row check. |
| + |
| +**The development-only header.** `X-Meet-Test-Ice-TTL`, honoured only |
| +under `-env development`, shortens the credential's life for the gate. |
| +It is clamped both ways, to between 2 seconds and 3 hours, so a header |
| +can neither stall the schedule at a zero life nor mint a life past the |
| +platform's own four-hour ceiling. |
| + |
| +**Standing, re-derived exactly as `/signal` does.** §15.12 corrected |
| +the last row of §2.5's table: a participation whose kind is `member` |
| +re-derives standing on every request, and one whose standing is gone |
| +is ended with `lost {reason: "standing"}` and answered the same 404 an |
| +unknown peer gets. `/ice` is one of those requests, exactly as |
| +`/signal` is. A guest never consults standing at all (ruling 14) — |
| +`/ice` does not reopen the door §15.12 closed for a guest; it stands |
| +behind the same gate `Relay` and `Reset` do. |
| + |
| +### 15.14 §5.3 — the renewal schedule is skew-free, and its two verdicts are not one |
| + |
| +**Decision.** The client never schedules a renewal on the server's |
| +clock alone: it takes the earlier of two readings, both measured from |
| +the moment of the fetch that produced them. The *ratio* reading is |
| +`(expiresAt − renewAt) × 5` — the margin the server left itself, |
| +multiplied out from the contract's own shape (renewal at five sixths |
| +of the life, so the margin is a sixth of it and the delay is five of |
| +them) — built from two of the server's own instants subtracted from |
| +each other, so it survives any skew between the two clocks entirely. |
| +The *clock* reading is `renewAt − fetchedAt`: this browser's own clock |
| +against the server's stated instant, exact when the clocks agree and |
| +wrong only in the direction that costs nothing — a browser running |
| +ahead renews early, and one running behind gets a longer answer, which |
| +the ratio reading then caps. The earlier of the two wins, with a floor |
| +under the delay so a server that has truncated its numbers down to no |
| +margin at all cannot turn the schedule into a spin. The must-succeed-by |
| +deadline is read the same way, at six of the margin rather than five. |
| + |
| +**Why not `renewAt` read as a point on the server's clock.** The |
| +server sends whole seconds, and truncation eats the margin at short |
| +lives: a twelve-second life gives 10 and 12 — margin 2, and the ratio's |
| +arithmetic is exact — while a ten-second life gives 8 and 10, where the |
| +same arithmetic would schedule the renewal at the very moment the |
| +credential dies. Reading `renewAt` as an instant on the server's own |
| +clock, rather than a duration counted from this browser's own fetch, |
| +would additionally put a client whose clock is minutes off the |
| +server's on the wrong schedule outright — early enough to be harmless, |
| +or late enough to treat a credential as still good after it has |
| +expired. Neither truncation nor skew is acceptable on its own reading, |
| +so the client keeps both and takes whichever is shorter. |
| + |
| +**The three renewal outcomes.** |
| + |
| +- **Renewed.** The exchange has completed, `signalingState` is |
| + `stable` with the new answer applied, and `getStats()` reports a |
| + selected pair, `succeeded`, carrying the restart's own |
| + `usernameFragment` — §5.3 step 2's own proof. Nothing is said to the |
| + person: the tile carries no sentence at all, and the call continues |
| + on the new credential. |
| +- **Expired** — "The relay credential expired for this connection." |
| + The old credential's own deadline, the must-succeed-by, has passed |
| + with nothing proven, because the fetch itself kept failing. This is |
| + the one honest signal the browser has, since it cannot see which |
| + credential the relay allocation is actually using. |
| +- **Stuck** — "Could not renew this connection through the relay." A |
| + new credential is in hand and three ICE restarts have run against |
| + it, none of them proven within the window. The credential is fine; |
| + the path is not — saying "expired" here would be a sentence about the |
| + wrong thing. |