oficina / meet Public

task 10 fix round 1: two gates that could pass on nothing

Important 1 — the slow-subscriber test depended on the host's sysctls.
Its flood had to outrun whatever the kernel's send buffer would absorb
on the reader's behalf, which is tcp_wmem's ceiling: about 1.8 MiB here
and 8 MiB elsewhere, so on a more generous box the server's write never
blocks, the hub's buffer never fills, and the test fails on a correct
implementation. The harness's httptest.Server now accepts through a
listener that pins SO_SNDBUF on every connection, so the arithmetic is a
constant this repo owns: 16 KiB out, 8 KiB in at the client, at most one
60 KiB event in the socket, sixty-four in the buffer, one in the
handler's hand. The flood drops from 160 to 100 and the test asserts a
ceiling of 72 events rather than merely "fewer than the flood" — which
is what makes the pin load-bearing: without it the victim reads 95.

Important 2 — the replay scenario's success condition was its starting
state. B was already stable and connected, and A's frames keep rising
because the cut is on the SSE stream and not on the media, so a dispatch
that never reached restartIce passed. It now asserts, while A is still
cut, that B reaches signaling "have-local-offer" toward A — an offer out
that nobody can answer. Dispatched at the Play control instead of Retry,
the scenario now fails there.

Minor 4 — a new `rosterpair` scenario finishes §13's gap clause: B resets
the pair while A is cut across the whole of it, so A's `pair` is never
delivered and is not replayed either (a `pair` is only replayed above the
roster's own generation). A learns it from the roster on its resume, and
its own reset count stays at zero, which is what separates this from the
gap scenario.

Minor 5 — the copy catalogue's terminal-links rows described the
behaviour before this task's part C: `replaced` now carries the holding
screen behind its takeover link, and the browser gate locates that link
by href rather than assuming `#status a` is a single element. Three rows
corrected, including "Try again"'s, which now says why it points at the
room rather than the holding screen.

Minor 6 — wentElsewhere compares origin as well as pathname.

Minor 7 — the Makefile's browser comment listed nine scenarios by name;
it now points at the `scenarios` slice, which is the list that has to be
right.

Cosmetic: the concurrent-admissions comment said five members where
three of five open participations are the actors; allowNow's comment
named the tick goroutine, where in tests it is the request handlers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CzvnuKhLzudvq7jZ7SvVt2
Paul Campbell pushed by paul@keymail.dev cfc0dc0711d78829ec77d5bc15d6af3465ec87fe parent 14a972b
9 files changed, +192 −29
  • Makefile +3 −2
  • docs/superpowers/copy/2026-09-10-slice-2-strings.md +3 −3
  • internal/meet/hub/admission_test.go +5 −3
  • internal/meet/standing.go +6 −4
  • internal/meet/static/room.js +6 −1
  • internal/meettest/browser_test.go +1 −0
  • internal/meettest/harness_test.go +46 −1
  • internal/meettest/stream_test.go +24 −15
  • test/browser/call.mjs +98 −0

CI passed — run details

diff --git a/Makefile b/Makefile
index 96c6c9e..4cd3ddd 100644
--- a/Makefile
+++ b/Makefile
@@ -88,8 +88,9 @@ migration-check:
ci: vet fmt-check test migration-check
# The browser gate: a real call between two real Chromium contexts, with
-# fake camera and microphone. Nine scenarios — devices, call, renewal,
-# guest, removed, restart, takeover, crosssite, scriptless.
+# fake camera and microphone. The scenarios, and the order they run in,
+# are the `scenarios` slice in internal/meettest/browser_test.go — that
+# list is the one that has to be right, so it is not copied here.
#
# Deliberately NOT part of `ci`. It needs node and `npm install`, and the
# one gate the team runs on every push must not depend on a toolchain the
diff --git a/docs/superpowers/copy/2026-09-10-slice-2-strings.md b/docs/superpowers/copy/2026-09-10-slice-2-strings.md
index c9f7b34..aba1b1a 100644
--- a/docs/superpowers/copy/2026-09-10-slice-2-strings.md
+++ b/docs/superpowers/copy/2026-09-10-slice-2-strings.md
@@ -86,7 +86,7 @@ lines are the holding screen's own wording, reused: noted per row.
| String | Where | Note |
| --- | --- | --- |
-| This meeting is open in another tab. | `room.js`, `TERMINAL.replaced` | The one reason with no "Back to the holding screen" link appended — it already has its own real next step (the takeover link, below), and the browser gate reads `#status a` as a single element (task-8-report.md, Deviations item 1). |
+| This meeting is open in another tab. | `room.js`, `TERMINAL.replaced` | Carries two links: the takeover, which is its real next step, and "Back to the holding screen" behind it (task-10-report.md, part C). The browser gate locates the takeover by href rather than reading `#status a` as a single element. |
| You no longer have access to this meeting. | `room.js`, `TERMINAL.standing` | |
| Your invitation to this meeting has expired. | `room.js`, `TERMINAL.expired` | |
| Someone removed you from this meeting. | `room.js`, `TERMINAL.removed` | |
@@ -103,8 +103,8 @@ lines are the holding screen's own wording, reused: noted per row.
| --- | --- | --- |
| Sign in again | `room.js`, `wayOut()`, link to `/signin` | Shown only for `signedout`, and it is the *only* link on that screen — no "Back to the holding screen" is appended, because that would just bounce the person straight back to sign-in (task-8-report.md, Deviations item 1). |
| Use this tab instead | `room.js`, `wayOut()`, link to `/m/{sqid}/takeover` | Shown only for `replaced`. Same wording as the takeover confirm page's own submit button (see below). |
-| Try again | `room.js`, `wayOut()`, link to `/m/{sqid}` | Shown only for `unreachable`, alongside "Back to the holding screen". |
-| Back to the holding screen | `room.js`, `wayOut()`, link to `/m/{sqid}/join` | Shown for every terminal reason except `replaced` and `signedout` — the rule is that no terminal screen is a dead end. |
+| Try again | `room.js`, `wayOut()`, link to `/m/{sqid}` | Shown only for `unreachable`, first, with "Back to the holding screen" behind it. The room, not the holding screen: the guess this sentence makes is "the room may be full", and the answer to that is to try the room again. |
+| Back to the holding screen | `room.js`, `wayOut()`, link to `/m/{sqid}/join` | Shown for every terminal reason except `signedout` — the rule is that no terminal screen is a dead end, and `replaced` is no longer the exception (task-10-report.md, part C). `signedout` still is: the holding screen would only bounce that person straight back to sign-in. |
### Tile captions
diff --git a/internal/meet/hub/admission_test.go b/internal/meet/hub/admission_test.go
index ce914ce..d1f7a8a 100644
--- a/internal/meet/hub/admission_test.go
+++ b/internal/meet/hub/admission_test.go
@@ -527,9 +527,11 @@ func TestConcurrentAdmissionsFillTheLastSlotExactlyOnce(t *testing.T) {
h := testHub(&now)
ctx := context.Background()
- // Three admitted members: three of the four mesh slots taken, one
- // free. Each admits from its own session, so five goroutines are
- // five real callers and not one caller five times.
+ // Five member participations open, of which the first three join and
+ // are admitted: three of the four mesh slots taken, one free. Only
+ // those three may admit, so the five goroutines below use them
+ // round-robin — five real callers through three sessions, rather
+ // than one caller five times.
actors := make([]struct {
session string
peer PeerID
diff --git a/internal/meet/standing.go b/internal/meet/standing.go
index 23758f8..6723ed7 100644
--- a/internal/meet/standing.go
+++ b/internal/meet/standing.go
@@ -65,10 +65,12 @@ func (s Standing) String() string {
// allowNow is the team as it stands right now. It is read through an
// atomic rather than straight off a.cfg because TestHandle.SetAllowlist
-// swaps it while the tick goroutine and live handlers are reading it —
-// a member dropped from the team mid-call is what spec §2.4's tick has
-// to notice, and there is no other way to make that happen in a test
-// without rewriting an environment variable and restarting the process.
+// swaps it while it is being read — by request handlers on their own
+// goroutines, which is what the race detector actually catches here, and
+// by the hub's tick when a test drives one. A member dropped from the
+// team mid-call is what spec §2.4's tick has to notice, and there is no
+// other way to make that happen in a test without rewriting an
+// environment variable and restarting the process.
// Production never calls the setter, so this is one atomic load on a
// path that already does a database read.
func (a *app) allowNow() allowlist {
diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js
index 9b6644f..1307d48 100644
--- a/internal/meet/static/room.js
+++ b/internal/meet/static/room.js
@@ -444,10 +444,15 @@
return { status: res.status, data: data || {}, signedOut, retryAfter: res.headers.get('Retry-After') };
}
+ // Origin as well as pathname: a redirect off this origin is somewhere
+ // else however familiar the path looks, and a sign-in page served from
+ // an identity host would otherwise read as success.
function wentElsewhere(res, asked) {
if (!res.redirected) return false;
try {
- return new URL(res.url).pathname !== new URL(asked, location.origin).pathname;
+ const got = new URL(res.url);
+ const want = new URL(asked, location.origin);
+ return got.origin !== want.origin || got.pathname !== want.pathname;
} catch {
// A URL neither of them can parse is not one this browser asked
// for: treat it the way the old check did.
diff --git a/internal/meettest/browser_test.go b/internal/meettest/browser_test.go
index 6786d1c..6ba58a5 100644
--- a/internal/meettest/browser_test.go
+++ b/internal/meettest/browser_test.go
@@ -80,6 +80,7 @@ var scenarios = []string{
"departure",
"replay",
"gap",
+ "rosterpair",
"restart",
"takeover",
"crosssite",
diff --git a/internal/meettest/harness_test.go b/internal/meettest/harness_test.go
index 68db65f..3d7fd87 100644
--- a/internal/meettest/harness_test.go
+++ b/internal/meettest/harness_test.go
@@ -11,6 +11,7 @@ import (
"database/sql"
"io"
"log/slog"
+ "net"
"net/http"
"net/http/httptest"
"net/url"
@@ -18,6 +19,7 @@ import (
"regexp"
"strings"
"sync"
+ "syscall"
"testing"
"time"
@@ -57,7 +59,20 @@ type harness struct {
// testOrigin, exactly as a browser on the app's own origin would.
func (x *harness) server() *httptest.Server {
if x.srv == nil {
- x.srv = httptest.NewServer(x.h)
+ srv := httptest.NewUnstartedServer(x.h)
+ // Every accepted connection gets a small, fixed send buffer. It
+ // costs the other tests nothing — every event they read is a few
+ // hundred bytes — and it is what makes "the client stopped
+ // reading" mean something to the server: with the kernel's own
+ // defaults, an unread socket absorbs however much tcp_wmem's
+ // ceiling allows (about 1.8 MiB on this box, 8 MiB on another),
+ // so the write never blocks, the hub's sixty-four-event buffer
+ // never fills behind it, and a test of §4.4's slow subscriber
+ // would pass or fail on the host's sysctls rather than on the
+ // code. See TestASlowClientIsLostOnTheBuffer...
+ srv.Listener = smallSendBuffer{srv.Listener}
+ srv.Start()
+ x.srv = srv
// Registered before the shutdown, so it runs after it: Close
// waits for outstanding requests, and a live stream ends when
// the app's context is cancelled. A stream's own cleanup is
@@ -68,6 +83,36 @@ func (x *harness) server() *httptest.Server {
return x.srv
}
+// serverSendBuffer is SO_SNDBUF on every accepted connection. Linux
+// doubles what it is given and stops autotuning once it is set, so the
+// server can hold roughly 16 KiB of unread response — a quarter of one
+// maximum-sized signalling payload.
+const serverSendBuffer = 8 << 10
+
+type smallSendBuffer struct{ net.Listener }
+
+func (l smallSendBuffer) Accept() (net.Conn, error) {
+ c, err := l.Listener.Accept()
+ if err != nil {
+ return nil, err
+ }
+ tc, ok := c.(*net.TCPConn)
+ if !ok {
+ return c, nil
+ }
+ raw, err := tc.SyscallConn()
+ if err != nil {
+ return c, nil
+ }
+ // Best effort: a kernel that refuses the option leaves the
+ // connection working, and the one test that depends on the size
+ // says how many events got through when it fails.
+ _ = raw.Control(func(fd uintptr) {
+ _ = syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_SNDBUF, serverSendBuffer)
+ })
+ return c, nil
+}
+
func (x *harness) sqlDB() *sql.DB { s, _ := x.db.G.DB(); return s }
func (x *harness) advance(d time.Duration) {
diff --git a/internal/meettest/stream_test.go b/internal/meettest/stream_test.go
index c983ce8..7005ef9 100644
--- a/internal/meettest/stream_test.go
+++ b/internal/meettest/stream_test.go
@@ -383,21 +383,21 @@ func TestASlowClientIsLostOnTheBufferAndResumesTheSameParticipation(t *testing.T
t.Fatalf("the sender never saw the victim admitted (got %q); nothing below would reach it", ev)
}
- // The arithmetic, because it is what makes this deterministic rather
- // than lucky. Two things absorb events before the hub's own buffer
- // starts filling: the socket, and the handler's one in hand. The
- // socket's share is bounded by the kernel's send buffer — about
- // 1.8 MiB on a default Linux (tcp_wmem's 4 MiB ceiling, minus skb
- // overhead), which at 60 KiB an event is roughly thirty of them —
- // and the receive side is pinned small by rawStream's own dial. The
- // buffer is sixty-four. So a flood well past ninety-five is past
- // every one of those, and the count below leaves room for a kernel
- // twice as generous as this one.
+ // The arithmetic, and it is the host's sysctls that it deliberately
+ // does not depend on. Two things absorb events before the hub's own
+ // buffer starts filling: the socket, and the handler's one in hand.
+ // Both ends of the socket are pinned — the server's SO_SNDBUF by the
+ // harness's own listener (about 16 KiB once Linux has doubled it),
+ // the client's SO_RCVBUF by rawStream's dial (about 8 KiB) — so at
+ // 60 KiB an event the socket holds at most one, whatever tcp_wmem's
+ // ceiling happens to be on this machine. The hub's buffer is
+ // sixty-four, and the handler holds one more. So the overflow is at
+ // about sixty-six, and a hundred is comfortably past it.
//
// The limiter is thirty a second with a burst of sixty on the
// harness's frozen clock, so the burst is spent in batches with a
// refill between them rather than being throttled halfway through.
- const flood = 160
+ const flood = 100
payload := strings.Repeat("x", 60*1024)
for i := 1; i <= flood; i++ {
if i%50 == 1 && i > 1 {
@@ -415,7 +415,9 @@ func TestASlowClientIsLostOnTheBufferAndResumesTheSameParticipation(t *testing.T
for {
name, data, ok := vs.next(30 * time.Second)
if !ok {
- t.Fatalf("the stream ended without lost{buffer} after %d events", seen)
+ t.Fatalf("the stream ended without lost{buffer} after %d of %d events; "+
+ "if that is close to %d the socket absorbed the whole flood and the "+
+ "harness's SO_SNDBUF did not take", seen, flood, flood)
}
seen++
if name != "lost" {
@@ -430,9 +432,16 @@ func TestASlowClientIsLostOnTheBufferAndResumesTheSameParticipation(t *testing.T
}
// Events were actually lost, which is the whole claim: a stream that
// delivered every one of them and then said lost{buffer} would be
- // something else entirely.
- if seen > flood {
- t.Fatalf("the victim read %d of %d events and lost none of them", seen, flood)
+ // something else entirely. The bound is the arithmetic above and not
+ // merely "fewer than the flood": sixty-six is what the buffer, the
+ // handler's one in hand and a socket that can hold at most one event
+ // add up to, so a number well past it means the socket absorbed the
+ // flood and the harness's pinned send buffer did not take.
+ const ceiling = 72
+ if seen > ceiling {
+ t.Fatalf("the victim read %d of %d events before it was declared lost; "+
+ "the buffer is sixty-four and the socket should hold at most one 60 KiB event, "+
+ "so anything past %d means the send buffer was not pinned", seen, flood, ceiling)
}
// The participation is not the subscriber: it is still there, and a
diff --git a/test/browser/call.mjs b/test/browser/call.mjs
index 42d706c..7ffcb15 100644
--- a/test/browser/call.mjs
+++ b/test/browser/call.mjs
@@ -704,6 +704,21 @@ const run = {
// reveals it on a failed connection, so it is dispatched rather than
// clicked: this scenario is about the signalling, not the control.
await pageB.locator('#tiles figure:not(.meet-tile-self) figcaption button').first().dispatchEvent('click');
+
+ // Asserted while A is still cut, and this is the assertion that
+ // makes the scenario mean anything: B has an offer out that A cannot
+ // answer, so its signalingState leaves `stable`. Without this, the
+ // ending state below is the starting state — B stable and connected,
+ // A's frames still rising because the cut is on the SSE stream and
+ // not on the media — and a dispatch that never reached restartIce
+ // would pass.
+ const offered = await until(async () => {
+ const b = await state(pageB);
+ return b.signaling[pair.a.peer] === 'have-local-offer' ? b : false;
+ }, 5000);
+ check(offered, 'B has signalling out that A cannot answer while it is cut — ' +
+ (await diagnose(pageB, 'member B')));
+
await sleep(5000);
cut.on = false;
@@ -801,6 +816,89 @@ const run = {
await ctxB.close();
},
+ // The rest of §13's gap clause: "including when one side's `pair` was
+ // cut and it learns the generation from the roster on its own resume".
+ //
+ // The reset comes from B, and A is cut across the whole of it, so A's
+ // `pair` is never delivered. It is not replayed either: a `pair` is
+ // only replayed when it carries a generation *above* the roster's, and
+ // the roster carries the current one — so the roster is the only place
+ // A can learn it (hub/relay.go's own note on that branch). A's
+ // reconcile path is what has to notice: `ensurePC` with a generation
+ // above the local one rebuilds.
+ //
+ // That A never asked is the assertion that separates this from the gap
+ // scenario: its own `reset` count stays at zero throughout.
+ async rosterpair() {
+ const ctxA = await newContext('member A', { cookie: cookieA });
+ const ctxB = await newContext('member B', { cookie: cookieB });
+ const cut = await cuttableStream(ctxA);
+ let resets = 0;
+ await ctxA.route('**/signal', async (route) => {
+ if ((route.request().postData() || '').includes('"type":"reset"')) resets += 1;
+ await route.continue();
+ });
+ const pageA = await openRoom(ctxA);
+ const pageB = await openRoom(ctxB);
+ const pair = await assertPair(pageA, pageB, 'member A', 'member B');
+ if (!pair) {
+ await ctxA.close();
+ await ctxB.close();
+ return;
+ }
+
+ cut.on = true;
+ check(await biteCut(cut), "A's attaching stream is cut (" + cut.aborted + ' aborted)');
+
+ // B resets the pair from its own page, so the request carries B's
+ // session and peer and nothing on A's side is involved.
+ const reset = await pageB.evaluate(async ({ room, to }) => {
+ const res = await fetch('/m/' + room + '/signal', {
+ method: 'POST',
+ credentials: 'same-origin',
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({ peer: window.meet.state().peer, type: 'reset', to }),
+ });
+ return res.status;
+ }, { room: sqid, to: pair.a.peer });
+ check(reset === 200, 'B reset the pair while A was cut (' + reset + ')');
+
+ // B rebuilt on its own `pair`, which it did receive: its connection
+ // to A leaves `connected` while A knows nothing about it.
+ const bRebuilt = await until(async () => {
+ const b = await state(pageB);
+ return b.connection[pair.a.peer] !== 'connected' || b.signaling[pair.a.peer] !== 'stable' ? b : false;
+ }, 15000);
+ check(bRebuilt, 'B rebuilt its side at the new generation — ' + (await diagnose(pageB, 'member B')));
+
+ cut.on = false;
+
+ const rebuilt = await until(async () => {
+ const a = await state(pageA);
+ const b = await state(pageB);
+ return a.peer === pair.a.peer && !a.terminal &&
+ a.peers.includes(pair.b.peer) && b.peers.includes(pair.a.peer) &&
+ a.signaling[pair.b.peer] === 'stable' && a.connection[pair.b.peer] === 'connected' &&
+ b.signaling[pair.a.peer] === 'stable' && b.connection[pair.a.peer] === 'connected' ? { a, b } : false;
+ }, 45000);
+ // Connecting at all is the proof that A converged on B's generation:
+ // the hub refuses signalling at any other one with 409, so a side
+ // still at the old generation never gets an offer through.
+ check(rebuilt, 'A learned the new generation from the roster alone and the pair connected again — ' +
+ (await diagnose(pageA, 'member A')) + ' ' + (await diagnose(pageB, 'member B')));
+ check(resets === 0, 'A never asked for a reset of its own (' + resets + ')');
+
+ const before = (await state(pageA)).frames[pair.b.peer] || 0;
+ const rising = await until(async () => ((await state(pageA)).frames[pair.b.peer] || 0) > before, 25000);
+ check(rising, 'A is decoding video over the rebuilt connection (' + before + ' -> ' +
+ ((await state(pageA)).frames[pair.b.peer] || 0) + ')');
+
+ await leaveRoom(pageA);
+ await leaveRoom(pageB);
+ await ctxA.close();
+ await ctxB.close();
+ },
+
// The process goes away and comes back. Both browsers see the stream
// die, find a boot they do not know, and rejoin (§4.5).
async restart() {