amadan

oficina / meet Public

hub: a grant expiring never ends an admitted guest, and a kind change is always announced

Review round 1. Three corrections to the admission paths:

A guest's grant expiring is not an ending (spec 2.4, ruling 14), but
Join ended one before it reached the idempotency check, so an admitted
guest re-posting join after the two-hour mark lost the room. Admit and
Remove had the same hole for their actor. All three now go through
standingLostLocked, which ends a member, ends anyone not yet admitted
with the reason their state earns, and leaves an admitted guest alone.

A peer the tick promoted to member while the room was full is promised
a retry with join, but Join returned early for anyone already joined,
so the retry was silence. It now attempts admission and sends full
again on refusal.

A kind change re-derived on a request was written straight onto the
participation, so the roster refresh it owes admitted peers never went
out, and the tick could never see the change afterwards. Every
re-derivation now writes through setKindLocked, which refreshes the
room's rosters when an admitted peer's kind actually moved.

Also: ErrMeshFull separates the mesh cap of four from Open's
sixteen-participation ErrRoomFull; a waiting entry in a roster carries
its real kind; End no longer mints a headstone for a room nobody
opened; admitLocked no longer tells a newly admitted member its own
knock has gone; and TypeEnded says in a comment why nothing sends it.
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 da19288a4e244813940ea7300a512f4b092c8e83 parent a419e4b
4 files changed, +253 −56
  • internal/meet/hub/admission.go +94 −33
  • internal/meet/hub/admission_test.go +127 −0
  • internal/meet/hub/hub.go +22 −18
  • internal/meet/hub/messages.go +10 −5
diff --git a/internal/meet/hub/admission.go b/internal/meet/hub/admission.go
index 644db6b..ee870fa 100644
--- a/internal/meet/hub/admission.go
+++ b/internal/meet/hub/admission.go
@@ -40,7 +40,9 @@ func (h *Hub) rosterFor(r *room, viewer *Participation) *Roster {
if viewer.Kind == KindMember {
for _, p := range r.peers {
if p.Joined && !p.Admitted {
- ro.Waiting = append(ro.Waiting, RosterPeer{Peer: p.Peer, Name: p.Name, Kind: "guest"})
+ // Usually a guest, but a peer the tick promoted while the
+ // room was full waits on as a member (spec §2.4).
+ ro.Waiting = append(ro.Waiting, RosterPeer{Peer: p.Peer, Name: p.Name, Kind: p.Kind.String()})
}
}
}
@@ -68,9 +70,7 @@ func (h *Hub) admitLocked(r *room, p *Participation) bool {
p.Admitted = true
p.Joined = true
p.JoinedAt = h.cfg.Now()
- if p.sub != nil {
- p.sub.send(Message{Type: TypeRoster, Roster: h.rosterFor(r, p)})
- }
+ h.sendLocked(p, Message{Type: TypeRoster, Roster: h.rosterFor(r, p)})
for _, q := range r.peers {
if q.Admitted && q.Peer != p.Peer && q.sub != nil {
// The one-entry roster carries this recipient's own pair
@@ -81,7 +81,12 @@ func (h *Hub) admitLocked(r *room, p *Participation) bool {
}
}
if wasWaiting {
- h.broadcastLocked(r, Message{Type: TypeKnockGone, From: p.Peer}, admittedMembers)
+ // Not to the newcomer: a peer the tick promoted to member is
+ // admitted as one, and would otherwise be told its own knock had
+ // gone, a moment after the roster that already said so.
+ h.broadcastLocked(r, Message{Type: TypeKnockGone, From: p.Peer}, func(q *Participation) bool {
+ return q.Peer != p.Peer && admittedMembers(q)
+ })
}
return true
}
@@ -96,6 +101,39 @@ func (h *Hub) refreshRostersLocked(r *room) {
}
}
+// setKindLocked records a re-derived kind. A kind that actually changed
+// on an admitted peer owes every admitted peer a fresh roster, whether
+// the change was seen on the tick or on a request (spec §2.4), so every
+// re-derivation writes Kind through here and nowhere else.
+func (h *Hub) setKindLocked(r *room, p *Participation, kind Kind) {
+ if p.Kind == kind {
+ return
+ }
+ p.Kind = kind
+ if p.Admitted {
+ h.refreshRostersLocked(r)
+ }
+}
+
+// standingLostLocked applies a loss of standing the way spec §2.4 asks.
+// A member loses the room; so does anyone not yet admitted, a guest with
+// the reason their grant earns. An **admitted** guest is untouched: a
+// participation is never ended by a grant expiring, because the
+// participation is the authority now (ruling 14). It reports whether the
+// participation ended, and announces the departure when it did.
+func (h *Hub) standingLostLocked(r *room, p *Participation) bool {
+ if p.Kind != KindMember && p.Admitted {
+ return false
+ }
+ reason := ReasonExpired
+ if p.Kind == KindMember {
+ reason = ReasonStanding
+ }
+ wasAdmitted, wasWaiting := h.endLocked(r, p, reason)
+ h.announceDepartureLocked(r, p, wasAdmitted, wasWaiting)
+ return true
+}
+
// Join is the entry, so standing is re-derived now (spec §2.5).
func (h *Hub) Join(ctx context.Context, sqid, session string, peer PeerID) error {
kind, ok, err := h.standing(ctx, sqid, session, peer)
@@ -110,30 +148,41 @@ func (h *Hub) Join(ctx context.Context, sqid, session string, peer PeerID) error
}
r := h.rooms[sqid]
if !ok {
- // A member who lost standing is dropped by the next thing they
- // do, not the next tick (spec §2.5), and the room is told in the
- // shape their state earns.
- wasAdmitted, wasWaiting := h.endLocked(r, p, ReasonStanding)
- h.announceDepartureLocked(r, p, wasAdmitted, wasWaiting)
- return ErrNotYours
+ // Standing is checked before the kind is written, because what a
+ // loss costs depends on the kind the peer held. A member is
+ // dropped by the next thing they do, not the next tick (§2.5);
+ // an admitted guest whose grant lapsed keeps everything.
+ if h.standingLostLocked(r, p) {
+ return ErrNotYours
+ }
+ return nil
}
- p.Kind = kind
- if p.Joined {
+ h.setKindLocked(r, p, kind)
+ if p.Joined && p.Admitted {
return nil // idempotent
}
+ if p.Joined {
+ // Waiting. A guest waits on; a peer the tick promoted to member
+ // while the room was full may retry here, which is the retry
+ // §2.4 promises them.
+ if kind != KindMember {
+ return nil
+ }
+ if !h.admitLocked(r, p) {
+ h.sendLocked(p, Message{Type: TypeFull})
+ return ErrMeshFull
+ }
+ return nil
+ }
if kind == KindMember {
if !h.admitLocked(r, p) {
- if p.sub != nil {
- p.sub.send(Message{Type: TypeFull})
- }
- return ErrRoomFull
+ h.sendLocked(p, Message{Type: TypeFull})
+ return ErrMeshFull
}
return nil
}
if waitingCountLocked(r) >= MaxWaiting {
- if p.sub != nil {
- p.sub.send(Message{Type: TypeFull})
- }
+ h.sendLocked(p, Message{Type: TypeFull})
return ErrTooManyWaiting
}
p.Joined = true
@@ -192,11 +241,14 @@ func (h *Hub) Admit(ctx context.Context, sqid, session string, actor, target Pee
}
r := h.rooms[sqid]
if !actorOK {
- wasAdmitted, wasWaiting := h.endLocked(r, a, ReasonStanding)
- h.announceDepartureLocked(r, a, wasAdmitted, wasWaiting)
- return ErrNotYours
+ if h.standingLostLocked(r, a) {
+ return ErrNotYours
+ }
+ // An admitted guest whose grant lapsed keeps the room but never
+ // gains the authority to let anyone else in.
+ return ErrNotMember
}
- a.Kind = actorKind
+ h.setKindLocked(r, a, actorKind)
if !a.Admitted {
return ErrNotAdmitted
}
@@ -212,11 +264,9 @@ func (h *Hub) Admit(ctx context.Context, sqid, session string, actor, target Pee
h.announceDepartureLocked(r, tp, wasAdmitted, wasWaiting)
return nil
}
- tp.Kind = targetKind
+ h.setKindLocked(r, tp, targetKind)
if !h.admitLocked(r, tp) {
- if a.sub != nil {
- a.sub.send(Message{Type: TypeFull})
- }
+ h.sendLocked(a, Message{Type: TypeFull})
}
return nil
}
@@ -250,11 +300,17 @@ func (h *Hub) Remove(ctx context.Context, sqid, session string, actor, target Pe
h.mu.Lock()
defer h.mu.Unlock()
a, found := h.findLocked(sqid, session, actor)
- if !found || !ok {
+ if !found {
return ErrUnknownPeer
}
r := h.rooms[sqid]
- a.Kind = kind
+ if !ok {
+ if h.standingLostLocked(r, a) {
+ return ErrNotYours
+ }
+ return ErrNotMember // an admitted guest, grant lapsed: still not a member
+ }
+ h.setKindLocked(r, a, kind)
if !a.Admitted {
return ErrNotAdmitted
}
@@ -295,12 +351,17 @@ func (h *Hub) Takeover(sqid, session string) (PeerID, error) {
}
// End is the meeting ending: lost{ended} to every stream, every
-// participation ended, and the room kept only as a headstone so a later
-// Open on an ended meeting is refused (spec §2.4).
+// participation ended, and the room kept only as a headstone so a stream
+// racing the ending is refused (spec §2.4). A room nobody opened needs
+// no headstone: the handler has already read ended_at, and every later
+// entry reads it too.
func (h *Hub) End(sqid string) {
h.mu.Lock()
defer h.mu.Unlock()
- r := h.roomLocked(sqid)
+ r, ok := h.rooms[sqid]
+ if !ok {
+ return
+ }
for _, p := range r.peers {
h.endLocked(r, p, ReasonEnded)
}
diff --git a/internal/meet/hub/admission_test.go b/internal/meet/hub/admission_test.go
index f52ff75..1cc0b11 100644
--- a/internal/meet/hub/admission_test.go
+++ b/internal/meet/hub/admission_test.go
@@ -229,6 +229,133 @@ func TestTickDropsAMemberWhoLostStandingAndPromotesAWaitingGuest(t *testing.T) {
}
}
+// An admitted guest is never ended by their grant expiring: the
+// participation is the authority now (spec §2.4, ruling 14).
+func TestJoinKeepsAnAdmittedGuestWhoseGrantExpired(t *testing.T) {
+ now := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC)
+ authority := map[int64]bool{100: true}
+ h := New(Config{Now: func() time.Time { return now }, StandingFor: func(ctx context.Context, room string, id int64) (Kind, bool, error) {
+ if id < 100 {
+ return KindMember, true, nil
+ }
+ return KindGuest, authority[id], nil
+ }})
+ ctx := context.Background()
+ m, _ := h.Open("r1", "sm", 1, "Mia", KindMember)
+ h.Join(ctx, "r1", "sm", m.Peer)
+ g, _ := h.Open("r1", "sg", 100, "Gus", KindGuest)
+ h.Join(ctx, "r1", "sg", g.Peer)
+ h.Admit(ctx, "r1", "sm", m.Peer, g.Peer)
+ drain(m.Sub)
+ drain(g.Sub)
+ now = now.Add(2 * time.Hour)
+ authority[100] = false // the two-hour grant lapsed mid-call
+ if err := h.Join(ctx, "r1", "sg", g.Peer); err != nil {
+ t.Fatalf("admitted guest re-posting join after expiry: %v", err)
+ }
+ if p, found := h.Find("r1", "sg", g.Peer); !found || !p.Admitted {
+ t.Fatal("an admitted guest was ended by their grant expiring")
+ }
+ if ev := types(drain(m.Sub)); len(ev) != 0 {
+ t.Fatalf("the room was told of a departure that did not happen: %v", ev)
+ }
+ if ev := types(drain(g.Sub)); len(ev) != 0 {
+ t.Fatalf("the admitted guest got %v", ev)
+ }
+ h.Tick(ctx) // and the tick agrees with the request path
+ if p, found := h.Find("r1", "sg", g.Peer); !found || !p.Admitted {
+ t.Fatal("the tick ended an admitted guest whose grant expired")
+ }
+}
+
+// A guest the tick promoted to member while the room was full waits on
+// as a member and may retry with join (spec §2.4).
+func TestAWaitingMemberMayRetryJoin(t *testing.T) {
+ now := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC)
+ kinds := map[int64]Kind{100: KindGuest}
+ h := New(Config{Now: func() time.Time { return now }, StandingFor: func(ctx context.Context, room string, id int64) (Kind, bool, error) {
+ if id < 100 {
+ return KindMember, true, nil
+ }
+ return kinds[id], true, nil
+ }})
+ ctx := context.Background()
+ var members []OpenResult
+ for i := 0; i < MaxAdmitted; i++ {
+ o, _ := h.Open("r1", "m"+string(rune('a'+i)), int64(1+i), "M", KindMember)
+ members = append(members, o)
+ h.Join(ctx, "r1", "m"+string(rune('a'+i)), o.Peer)
+ }
+ g, _ := h.Open("r1", "sg", 100, "Gus", KindGuest)
+ h.Join(ctx, "r1", "sg", g.Peer)
+ kinds[100] = KindMember // promoted while the room is full
+ h.Tick(ctx)
+ p, _ := h.Find("r1", "sg", g.Peer)
+ if p.Admitted || p.Kind != KindMember {
+ t.Fatalf("promoted guest: admitted=%v kind=%v, want waiting as a member", p.Admitted, p.Kind)
+ }
+ if ev := types(drain(g.Sub)); len(ev) != 1 || ev[0] != TypeFull {
+ t.Fatalf("promotion into a full room got %v, want full", ev)
+ }
+ // Retrying while the cap is still held earns another full, not silence.
+ if err := h.Join(ctx, "r1", "sg", g.Peer); err != ErrMeshFull {
+ t.Fatalf("retry into a full room: %v", err)
+ }
+ if ev := types(drain(g.Sub)); len(ev) != 1 || ev[0] != TypeFull {
+ t.Fatalf("retry got %v, want full", ev)
+ }
+ // A slot frees and the same retry admits.
+ h.Leave("r1", "ma", members[0].Peer)
+ if err := h.Join(ctx, "r1", "sg", g.Peer); err != nil {
+ t.Fatal(err)
+ }
+ if p, _ := h.Find("r1", "sg", g.Peer); !p.Admitted {
+ t.Fatal("the retry did not admit the waiting member")
+ }
+ if ev := types(drain(g.Sub)); len(ev) != 1 || ev[0] != TypeRoster {
+ t.Fatalf("admitted retry got %v, want roster", ev)
+ }
+}
+
+// A kind change is announced whichever path notices it: the request one
+// re-derives standing too, and the tick will never see this change again
+// (spec §2.4, §2.5).
+func TestAKindChangeOnTheRequestPathRefreshesRosters(t *testing.T) {
+ now := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC)
+ kinds := map[int64]Kind{1: KindMember, 2: KindMember}
+ h := New(Config{Now: func() time.Time { return now }, StandingFor: func(ctx context.Context, room string, id int64) (Kind, bool, error) {
+ return kinds[id], true, nil
+ }})
+ ctx := context.Background()
+ a, _ := h.Open("r1", "sa", 1, "Ann", KindMember)
+ h.Join(ctx, "r1", "sa", a.Peer)
+ b, _ := h.Open("r1", "sb", 2, "Bob", KindMember)
+ h.Join(ctx, "r1", "sb", b.Peer)
+ drain(a.Sub)
+ drain(b.Sub)
+ kinds[2] = KindGuest // removed from the allowlist, live grant, no tick yet
+ if err := h.Join(ctx, "r1", "sb", b.Peer); err != nil {
+ t.Fatal(err)
+ }
+ ev := drain(a.Sub)
+ if len(ev) != 1 || ev[0].Type != TypeRoster {
+ t.Fatalf("the other admitted peer got %v, want one roster", types(ev))
+ }
+ got := ""
+ for _, rp := range ev[0].Roster.Peers {
+ if rp.Peer == b.Peer {
+ got = rp.Kind
+ }
+ }
+ if got != "guest" {
+ t.Fatalf("Ann's roster shows Bob as %q, want guest", got)
+ }
+ // The demoted peer keeps the room but loses the authority to admit.
+ if err := h.Admit(ctx, "r1", "sb", b.Peer, a.Peer); err != ErrNotMember {
+ t.Fatalf("demoted member admitting: %v", err)
+ }
+}
+
func TestJoinDropsAndAnnouncesAMemberWhoLostStanding(t *testing.T) {
now := time.Date(2026, 9, 10, 9, 0, 0, 0, time.UTC)
standing := map[int64]bool{1: true, 2: true}
diff --git a/internal/meet/hub/hub.go b/internal/meet/hub/hub.go
index 5a7328d..11f7adb 100644
--- a/internal/meet/hub/hub.go
+++ b/internal/meet/hub/hub.go
@@ -44,7 +44,13 @@ const (
)
var (
- ErrRoomFull = errors.New("hub: room holds sixteen participations")
+ // ErrRoomFull is the sixteen-participation cap of spec §4.2, which a
+ // handler answers with 503 before anything is allocated.
+ ErrRoomFull = errors.New("hub: room holds sixteen participations")
+ // ErrMeshFull is the mesh cap of four (spec §4.3), which is not a
+ // failed request: the peer has been sent full and may retry with
+ // join, so a handler answers 2xx and lets the event carry the news.
+ ErrMeshFull = errors.New("hub: four already admitted")
ErrSessionHasPeer = errors.New("hub: this session already has a peer in the room")
ErrPersonStreams = errors.New("hub: this person has four streams open")
ErrUnknownPeer = errors.New("hub: no such peer")
@@ -335,29 +341,19 @@ func (h *Hub) Tick(ctx context.Context) {
continue // it ended while standing was being read
}
switch {
- case !c.ok && p.Kind == KindMember:
- // A member who lost standing loses the room, admitted or not.
- wasAdmitted, wasWaiting := h.endLocked(r, p, ReasonStanding)
- h.announceDepartureLocked(r, p, wasAdmitted, wasWaiting)
- case !c.ok && !p.Admitted:
- // A waiting guest's grant lapsed before anyone admitted them.
- wasAdmitted, wasWaiting := h.endLocked(r, p, ReasonExpired)
- h.announceDepartureLocked(r, p, wasAdmitted, wasWaiting)
case !c.ok:
- // An admitted guest whose grant expired: nothing. The
- // participation is the authority now (ruling 14).
+ // A member loses the room, and so does a waiting guest whose
+ // grant lapsed; an admitted guest is untouched (ruling 14).
+ h.standingLostLocked(r, p)
case p.Admitted:
- if p.Kind != c.kind {
- p.Kind = c.kind
- h.refreshRostersLocked(r)
- }
+ h.setKindLocked(r, p, c.kind) // a change refreshes the rosters
default:
// Waiting. A promotion to member is admission under the cap,
// exactly as an admit would be; a refusal leaves them waiting
// as a member, free to retry with join.
- p.Kind = c.kind
- if c.kind == KindMember && !h.admitLocked(r, p) && p.sub != nil {
- p.sub.send(Message{Type: TypeFull})
+ h.setKindLocked(r, p, c.kind)
+ if c.kind == KindMember && !h.admitLocked(r, p) {
+ h.sendLocked(p, Message{Type: TypeFull})
}
}
}
@@ -381,6 +377,14 @@ type audience func(*Participation) bool
func admittedPeers(p *Participation) bool { return p.Admitted }
func admittedMembers(p *Participation) bool { return p.Admitted && p.Kind == KindMember }
+// sendLocked is one event to one participation, which is a no-op while
+// its stream is detached and waiting to resume.
+func (h *Hub) sendLocked(p *Participation, m Message) {
+ if p.sub != nil {
+ p.sub.send(m)
+ }
+}
+
func (h *Hub) broadcastLocked(r *room, m Message, who audience) {
for _, q := range r.peers {
if who(q) && q.sub != nil {
diff --git a/internal/meet/hub/messages.go b/internal/meet/hub/messages.go
index ec731f9..4141824 100644
--- a/internal/meet/hub/messages.go
+++ b/internal/meet/hub/messages.go
@@ -58,11 +58,16 @@ const (
TypeKnock = "knock"
TypeKnockGone = "knock-gone"
TypeFull = "full"
- TypeEnded = "ended"
- TypeLost = "lost"
- TypePair = "pair"
- TypeGap = "gap"
- TypeState = "state"
+ // TypeEnded names the ending in §4.3's table, but nothing sends it:
+ // a meeting that ends closes every stream, and a closing stream
+ // carries its reason in lost, so the browser reads the ending as
+ // lost{reason: "ended"} (§4.5). The name is kept because the spec
+ // uses it.
+ TypeEnded = "ended"
+ TypeLost = "lost"
+ TypePair = "pair"
+ TypeGap = "gap"
+ TypeState = "state"
)
// Message is the one envelope, built by the server. Payload is opaque.