oficina / meet Public

minors: four gaps the whole-branch review named

W-3, W-4, W-5, W-8. Each verified by applying the reviewer's mutation on
a throwaway copy and watching the new test fail.

W-3: §13 asks for "a different-boot 404 after restart", and nothing
compared a boot id across two hubs — new peer ids are not that, and a
hub whose boot came from something stable would pass every gate while
`recoverFromNotFound` lost the one thing that tells a restart from a
lapsed resume window. `window.meet.state()` now carries `boot`, and the
browser `restart` scenario asserts it changed.

W-4: the subject was bound into the hub session key with a reason — the
key then cannot survive the person it belongs to — and nothing tested
it. Not by the reviewer's suggested route: signing out and in again
changes the cookie value too, so the key would move with or without the
subject. The derivation is now `hubSessionKey(salt, subject, cookie)`,
with a test that changing any one of the three alone changes the key,
and that the key is neither the cookie value nor a bare hash of it.

W-5: `Remove`'s refusal to take its own actor as a target has a test —
a member removing itself would end its own participation through the
path that announces `gone` and writes no tombstone. `hubStanding`'s
ended-meeting arm stays as it is, with a comment saying it is belt and
braces and naming the callers that already refuse an ended meeting.

W-8: `refused()`'s 400 arm and `TERMINAL.badlast` are gone. Both callers
pass a probe response, the stream's only 400 is a `last` it could not
parse, and neither probe form sends `last`, so the sentence could not
reach anyone. The attaching stream's 400 stays invisible to EventSource
and ends as a stall, which is what already happened. Copy catalogue row
updated.

Skipped, with the reason recorded in the slice-2 record: W-7 (both fixes
change the wire contract), W-9 (copy, tracked in the catalogue), W-10
and W-11 (recorded, not worth the change today).

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 1 month ago 67d0eadbce13de791a28888f5730e9e75e82bddc parent 4a00329

8 files changed, +149 −13

docs/superpowers/copy/2026-09-10-slice-2-strings.md +1 −1
docs/superpowers/reviews/2026-09-10-slice-2-record.md +9 −2
internal/meet/hub/isolation_test.go +33 −0
internal/meet/participation.go +8 −0
internal/meet/static/room.js +12 −5
internal/meet/stream.go +13 −5
internal/meet/stream_key_test.go +62 −0
test/browser/call.mjs +11 −0

CI passed — run details

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 aba1b1a..08ac311 100644

--- a/docs/superpowers/copy/2026-09-10-slice-2-strings.md

+++ b/docs/superpowers/copy/2026-09-10-slice-2-strings.md

@@ -93,7 +93,7 @@ lines are the holding screen's own wording, reused: noted per row.

| This meeting has ended. | `room.js`, `TERMINAL.ended` | Also the fallback for any reason code `TERMINAL` doesn't recognise. |

| You cannot join this meeting. | `room.js`, `TERMINAL.gone` | Task 8 review: "the fresh-404 sentence must be neutral" — replaces an earlier "This meeting is no longer open." (progress.md Task 8 note; task-8-report.md item 1 of "New user-facing strings"). |

| You have been signed out. | `room.js`, `TERMINAL.signedout` | |

-| This browser could not open the call’s connection. | `room.js`, `TERMINAL.accept` (406) and `TERMINAL.badlast` (400) | Same sentence for both refusals; written as `’` in the source. |

+| This browser could not open the call’s connection. | `room.js`, `TERMINAL.accept` (406) | Written as `’` in the source. `TERMINAL.badlast` carried the same sentence for a 400 and was removed in the fix wave: neither probe form sends `last`, so the 400 it was written for could not reach it (review W-8). |

| Meet cannot reach the call. The room may be full, or you may have been signed out. | `room.js`, `TERMINAL.unreachable` | Reached after `MAX_STALLS` rounds with no `hello` inside `STALL_WINDOW` — a guess covering the two causes the client can't tell apart. |

| This browser cannot make calls. | `room.js`, `TERMINAL.nowebrtc` | |

diff --git a/docs/superpowers/reviews/2026-09-10-slice-2-record.md b/docs/superpowers/reviews/2026-09-10-slice-2-record.md

index 9013ca0..9db9477 100644

--- a/docs/superpowers/reviews/2026-09-10-slice-2-record.md

+++ b/docs/superpowers/reviews/2026-09-10-slice-2-record.md

@@ -131,11 +131,11 @@ with W-1 and W-2 fixed first.

| 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 meettest tests over HTTP, and a `guest` scenario with a second guest left knocking. Every mutation the reviewer lists is now killed. |

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

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

-| W-4 | Dropping the subject from the hub session key is caught by nothing | **Fixed in this wave.** A test signs the same browser out and in again and asserts the old key no longer answers. |

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

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

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

| W-7 | `lost{buffer}` is sent for three endings that are not buffer overruns | **Carried — next fix wave.** See §3. |

-| W-8 | `TERMINAL.badlast` is unreachable | **Fixed in this wave.** The dead arm is gone; the 400 an attaching stream returns is invisible to `EventSource` and is handled as a stall, which is what actually happens. |

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

| W-9 | Three wordings for one move | **Carried — the copy pass.** Already in the tracked copy catalogue. |

| W-10 | The test hook ships to production | **Recorded, not fixed.** See §3. |

| W-11 | Three hub maps only ever grow | **Recorded, not fixed.** See §3. |

@@ -284,6 +284,13 @@ does not use. Run it before every review that touches a page or the

client. Nothing enforces that, which is a known weakness and the reason

this row is here rather than in a comment.

+**It is not perfectly stable.** One run in this fix wave had

+`rosterpair` hang for 120 s and fail, on a tree where it passes in 4.5 s

+alone and passed 18/18 on the run before and the run after. That

+scenario cuts a stream mid-call and waits for a resume, so it has the

+most timing in it of any of them. A single red `rosterpair` is worth

+re-running before it is worth debugging; two in a row is not.

**The race step in CI.** `.amadan/ci.d/50-race` runs `make race` when

the runner has a C compiler and prints a skip line when it does not. The

guard is there because the cloud runner's provisioning

diff --git a/internal/meet/hub/isolation_test.go b/internal/meet/hub/isolation_test.go

index c941e51..7c4e6e6 100644

--- a/internal/meet/hub/isolation_test.go

+++ b/internal/meet/hub/isolation_test.go

@@ -285,3 +285,36 @@ func TestAKnockReachesAdmittedMembersOnly(t *testing.T) {

t.Fatalf("the waiting guest got %v", types(ev))

}

+// Review W-5: `Remove`'s refusal to take its own actor as a target. The

+// guard survived being deleted, and what it stops is worth a test — a

+// member removing itself would end its own participation through the

+// removal path, which announces `gone` and writes no tombstone, so the

+// tab would find itself out of the room with no way to resume and no

+// reason given. Leaving is `leave`, and it is a different ending.

+func TestAMemberCannotRemoveItself(t *testing.T) {

+ now := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC)

+ h := testHub(&now)

+ ctx := context.Background()

+ m, g, _ := guestRoom(t, h)

+ if err := h.Remove(ctx, "r1", "sm", m.Peer, m.Peer); err != ErrUnknownPeer {

+ t.Fatalf("a member removing itself: %v, want ErrUnknownPeer", err)

+ }

+ if p, ok := h.Find("r1", "sm", m.Peer); !ok || !p.Admitted {

+ t.Fatal("the member removed itself")

+ }

+ if ev := drain(m.Sub); len(ev) != 0 {

+ t.Fatalf("the room was told something: %v", types(ev))

+ }

+ // The refusal is the same sentinel an unknown target gets, so it

+ // tells the caller nothing about itself either.

+ if err := h.Remove(ctx, "r1", "sm", m.Peer, PeerID("0123456789abcdef0123456789abcdef")); err != ErrUnknownPeer {

+ t.Fatalf("an unknown target: %v", err)

+ }

+ // And a real other peer is still removable, so the guard is not

+ // simply refusing everything.

+ if err := h.Remove(ctx, "r1", "sm", m.Peer, g.Peer); err != nil {

+ t.Fatalf("removing someone else: %v", err)

+ }

diff --git a/internal/meet/participation.go b/internal/meet/participation.go

index cd94c80..83c6859 100644

--- a/internal/meet/participation.go

+++ b/internal/meet/participation.go

@@ -17,6 +17,14 @@ import (

// with no error, which is what a lapsed grant answers too: the tick

// then applies §2.4's transitions rather than treating it as a failure

// to retry on the next tick.

+//

+// That ended-meeting arm is belt and braces, and deliberately so

+// (review W-5): every path that reaches a room already refuses an ended

+// meeting earlier — roomAccess on the pages, and the stream handler

+// which reads ended_at before it attaches, because End no longer writes

+// a headstone for a room that was never opened. Removing it here would

+// break nothing today and would make this function's answer depend on

+// its callers all continuing to check first.

func (a *app) hubStanding(ctx context.Context, sqid string, personID int64) (hub.Kind, bool, error) {

m, ok, err := a.store.meetingBySqid(ctx, sqid)

if err != nil {

diff --git a/internal/meet/static/room.js b/internal/meet/static/room.js

index 1307d48..424f02f 100644

--- a/internal/meet/static/room.js

+++ b/internal/meet/static/room.js

@@ -344,6 +344,13 @@

// refused is the two answers that mean "stop, and say why", wherever

// they turn up: a session that has ended, and a request the stream

// endpoint will not accept in the shape this client sends it.

+ //

+ // There is no arm for 400 (review W-8). Both callers pass a probe

+ // response, the stream's only 400 is a `last` it could not parse, and

+ // neither probe form sends `last` — so the arm that used to be here

+ // could not fire. The attaching stream's 400 is invisible to

+ // EventSource anyway: it arrives as an error with no status, and the

+ // stall ladder is what ends it.

function refused(res) {

if (res.signedOut) {

terminal('signedout');

@@ -353,10 +360,6 @@

terminal('accept');

return true;

}

- if (res.status === 400) {

- terminal('badlast');

- return true;

- }

return false;

}

@@ -1057,7 +1060,6 @@

gone: 'You cannot join this meeting.',

signedout: 'You have been signed out.',

accept: 'This browser could not open the call\u2019s connection.',

- badlast: 'This browser could not open the call\u2019s connection.',

unreachable: 'Meet cannot reach the call. The room may be full, or you may have been signed out.',

nowebrtc: 'This browser cannot make calls.',

};

@@ -1463,6 +1465,11 @@

}

return {

peer: state.peer,

+ // The boot id of the process this stream last said hello from.

+ // A restart changes it, and that is the only thing that tells a

+ // browser a 404 on resume is a restart rather than a lapsed

+ // window (§4.5) — so the gate has to be able to read it.

+ boot: state.boot,

admitted: state.admitted,

terminal: state.terminal,

peers: Array.from(pcs.keys()),

diff --git a/internal/meet/stream.go b/internal/meet/stream.go

index f7c8de3..4dae231 100644

--- a/internal/meet/stream.go

+++ b/internal/meet/stream.go

@@ -51,11 +51,19 @@ func (a *app) sessionKey(r *http.Request) (string, bool) {

if err != nil || c.Value == "" {

return "", false

}

- // The subject is bound in as well as the cookie: the key then

- // cannot survive the person it belongs to, whatever a cookie value

- // might come to be reused for.

- sum := sha256.Sum256([]byte(a.hubSalt + "|" + s.Subject + "|" + c.Value))

- return hex.EncodeToString(sum[:]), true

+ return hubSessionKey(a.hubSalt, s.Subject, c.Value), true

+// hubSessionKey is the derivation itself, kept apart from the request so

+// its three properties can be gated (review W-4): the subject is bound

+// in as well as the cookie, so the key cannot survive the person it

+// belongs to whatever a cookie value might come to be reused for; the

+// per-boot salt means it is meaningless across a restart; and the

+// result is neither the session token nor the hash rastrillo stores, so

+// a hub key is not a credential.

+func hubSessionKey(salt, subject, cookie string) string {

+ sum := sha256.Sum256([]byte(salt + "|" + subject + "|" + cookie))

+ return hex.EncodeToString(sum[:])

}

// acceptsEventStream is ruling 15. The attaching stream is only ever

diff --git a/internal/meet/stream_key_test.go b/internal/meet/stream_key_test.go

new file mode 100644

index 0000000..92710f0

--- /dev/null

+++ b/internal/meet/stream_key_test.go

@@ -0,0 +1,62 @@

+package meet

+import (

+ "crypto/sha256"

+ "encoding/hex"

+ "testing"

+// Review W-4: the subject was bound into the hub session key in the

+// Task 4 fix round with a reason — "the key then cannot survive the

+// person it belongs to" — and nothing tested it. Dropping the subject

+// left the whole suite green.

+//

+// A sign-out-and-in test over HTTP does not gate this: re-signing-in

+// hands the browser a new cookie value, so the key would change with or

+// without the subject in the hash. What the claim is actually about is

+// the same cookie value under a different person, which is why this is

+// a test of the derivation rather than of a request.

+func TestTheHubSessionKeyBindsTheSubjectTheSaltAndTheCookie(t *testing.T) {

+ const (

+ salt = "boot-salt"

+ who = "person@example.com"

+ cookie = "a-session-cookie-value"

+ )

+ base := hubSessionKey(salt, who, cookie)

+ // Two tabs of one browser: the same key, which is what one peer per

+ // room and its takeover need (§4.2).

+ if again := hubSessionKey(salt, who, cookie); again != base {

+ t.Fatal("the same session did not derive the same key")

+ }

+ for _, c := range []struct {

+ what string

+ salt, who, cookie string

+ why string

+ }{

+ {"subject", salt, "someone.else@example.com", cookie,

+ "a cookie value reused under another person would inherit the room"},

+ {"salt", "another-boot", who, cookie,

+ "the key would still name a participation from before a restart"},

+ {"cookie", salt, who, "another-session-cookie",

+ "two browsers of one person would collide, and §4.4 allows four streams per person"},

+ } {

+ if got := hubSessionKey(c.salt, c.who, c.cookie); got == base {

+ t.Fatalf("changing the %s did not change the key: %s", c.what, c.why)

+ }

+ // And the key is not a credential: it is neither the cookie value

+ // nor a bare hash of it, so a hub key on its own reaches nothing.

+ if base == cookie {

+ t.Fatal("the key is the session cookie value")

+ }

+ bare := sha256.Sum256([]byte(cookie))

+ if base == hex.EncodeToString(bare[:]) {

+ t.Fatal("the key is an unsalted hash of the session cookie value")

+ }

+ if len(base) != 64 {

+ t.Fatalf("the key is %d characters, want 64 hex", len(base))

+ }

diff --git a/test/browser/call.mjs b/test/browser/call.mjs

index 6543d30..f0c2df4 100644

--- a/test/browser/call.mjs

+++ b/test/browser/call.mjs

@@ -193,6 +193,7 @@ function state(page) {

const s = window.meet.state();

return {

peer: s.peer,

+ boot: s.boot,

admitted: s.admitted,

terminal: s.terminal,

peers: s.peers,

@@ -1007,6 +1008,8 @@ const run = {

await ctxB.close();

return;

}

+ const bootBefore = (await state(pageA)).boot;

+ check(!!bootBefore, 'the browser knows which process it is talking to (boot ' + bootBefore + ')');

// §13 asks that both contexts have closed their old connections

// *before* rejoining, and that is a moment rather than a state: the

@@ -1036,6 +1039,14 @@ const run = {

check(back, 'both browsers took a new participation after the restart — ' +

(await diagnose(pageA, 'member A')) + ' ' + (await diagnose(pageB, 'member B')));

+ // §13: "a different-boot 404 after restart" (W-3). New peer ids are

+ // not that: a hub whose boot came from something stable would issue

+ // new ids too, and `recoverFromNotFound` would lose the one thing

+ // that tells a restart from a lapsed resume window (§4.5).

+ const bootAfter = back ? back.a.boot : (await state(pageA)).boot;

+ check(!!bootAfter && bootAfter !== bootBefore,

+ 'the restarted process announced a different boot id (' + bootBefore + ' then ' + bootAfter + ')');

for (const [page, label] of [[pageA, 'member A'], [pageB, 'member B']]) {

const watched = await page.evaluate(() => {

clearInterval(window.__watch);