psa-systems/lets-chat

Watch

Fork 0

Ring room members when a huddle starts (LC-611) #570

Merged

longjacksonle merged 1 commit from feat/lc611-huddle-ring into main

2026-07-20 22:28:25 +02:00

longjacksonle commented

2026-07-20 21:42:52 +02:00

Owner

Closes LC-611 for the room-wide ring. Targeted invites (AC2) are not in this change - see the bottom.

The problem

Starting a huddle broadcast exactly one ChatEvent::VoiceJoined via broadcast_to_room (ws.rs:2079), which only reaches sockets already subscribed to that room - i.e. people who can see the huddle bar anyway. Everyone else learned nothing. An ongoing group call was something you discovered, not something you were invited to.

What this does

A new ChatEvent::HuddleStarted is addressed to a single recipient (to_user_id) and fanned out over list_room_member_ids, so it reaches members wherever they are in the app. The client drains it into a dismissible banner offering Join, whose link carries ?huddle=1 so voice.js joins on arrival. Join is one click from anywhere, rather than a trip to the room to hunt for the bar.

Both of the ticket's blockers are sidestepped, not solved

This is the part worth reviewing, because it is a deliberate departure from the plan.

The ringing slot is untouched. LC-611 said RingingSlot had to be generalized to N callees. It does not, because the premise does not hold: that slot arbitrates DM glare - two peers inviting each other at once, where exactly one must win - and a huddle has no such contest. Two people starting a huddle in the same room simultaneously both land in the same voice_rooms entry and are, by definition, already in a call together. Generalizing the slot would have meant inventing a loser for a race with no stakes.

It would also have meant editing try_start_ringing, RingingResult, and the TTL, all of which the 1:1 DM path depends on and none of which has test coverage today (LC-614) - while LC-596 AC4 requires that path to keep its current behaviour. The safest change to untested call signaling was no change to it. Nothing in call.js, relay_call_signal, or Hub::ringing is modified.

#lc-call-root is untouched too. A room huddle is better served by a banner than by a modal ring seizing the page, and reusing a shell built around one remote video would have forced the N-peer rework for no benefit here. The ticket's concern about maintaining two ring UIs still stands, but a banner is not a second ring UI - it is a notification.

Structure

The fan-out and its policy live in a new server/src/huddle_ring.rs, not in routes::ws. Who gets rung and when is domain logic, and ws.rs is already very large.

Three guards, each proven load-bearing

Each was deleted in turn; each fails exactly its own test and no others.

Rings once per huddle, not once per joiner - gated on the mesh being empty before the join. The fourth person arriving does not re-notify everyone who already ignored it.
A persistent enclave voice channel never rings - walking into a room that always exists has not summoned anybody. Only the ad-hoc huddle attached to a text room rings.
Muted rooms do not ring, and except_mentions suppresses as well. A huddle starting is not a mention - nobody addressed you by name - so "interrupt me only for mentions" has to mean it. allows_huddle_ring() is a separate predicate from allows_unread_bump() despite matching it today, because if the two ever diverge it should be in the direction of ringing less.

Ignore is client-side only: it drops the banner and remembers the room for that page. Nothing reaches the server, so nobody learns you declined, per the ticket.

The banner text is built with textContent, never innerHTML - room names and display names are user-controlled.

Full suite green (182 test binaries, up from 181), just check clean, en/es Fluent symmetry holds.

Not in this change

Targeted invites (AC2). Ringing the room is the part that removes the discovery problem; inviting named people is a different interaction that probably wants a picker and may want a stronger affordance than a banner. Worth its own PR rather than bolting a member picker onto this one.

AC5's "join via the ring" test. The fan-out and all three policy guards are covered server-side. The click-to-join path is ?huddle=1 in voice.js, which is browser behaviour with no test harness in this repo - there are no JS tests. Flagging rather than pretending it is covered.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5

Closes LC-611 for the room-wide ring. Targeted invites (AC2) are not in this change - see the bottom. ## The problem Starting a huddle broadcast exactly one `ChatEvent::VoiceJoined` via `broadcast_to_room` (`ws.rs:2079`), which only reaches sockets already subscribed to that room - i.e. people who can see the huddle bar anyway. Everyone else learned nothing. An ongoing group call was something you discovered, not something you were invited to. ## What this does A new `ChatEvent::HuddleStarted` is addressed to a single recipient (`to_user_id`) and fanned out over `list_room_member_ids`, so it reaches members wherever they are in the app. The client drains it into a dismissible banner offering **Join**, whose link carries `?huddle=1` so `voice.js` joins on arrival. Join is one click from anywhere, rather than a trip to the room to hunt for the bar. ## Both of the ticket's blockers are sidestepped, not solved This is the part worth reviewing, because it is a deliberate departure from the plan. **The ringing slot is untouched.** LC-611 said `RingingSlot` had to be generalized to N callees. It does not, because the premise does not hold: that slot arbitrates DM *glare* - two peers inviting each other at once, where exactly one must win - and a huddle has no such contest. Two people starting a huddle in the same room simultaneously both land in the same `voice_rooms` entry and are, by definition, already in a call together. Generalizing the slot would have meant inventing a loser for a race with no stakes. It would also have meant editing `try_start_ringing`, `RingingResult`, and the TTL, all of which the 1:1 DM path depends on and none of which has test coverage today (LC-614) - while LC-596 AC4 requires that path to keep its current behaviour. The safest change to untested call signaling was no change to it. Nothing in `call.js`, `relay_call_signal`, or `Hub::ringing` is modified. **`#lc-call-root` is untouched too.** A room huddle is better served by a banner than by a modal ring seizing the page, and reusing a shell built around one remote video would have forced the N-peer rework for no benefit here. The ticket's concern about maintaining two ring UIs still stands, but a banner is not a second ring UI - it is a notification. ## Structure The fan-out and its policy live in a new `server/src/huddle_ring.rs`, not in `routes::ws`. Who gets rung and when is domain logic, and `ws.rs` is already very large. ## Three guards, each proven load-bearing Each was deleted in turn; each fails exactly its own test and no others. 1. **Rings once per huddle, not once per joiner** - gated on the mesh being empty *before* the join. The fourth person arriving does not re-notify everyone who already ignored it. 2. **A persistent enclave voice channel never rings** - walking into a room that always exists has not summoned anybody. Only the ad-hoc huddle attached to a text room rings. 3. **Muted rooms do not ring**, and `except_mentions` suppresses as well. A huddle starting is not a mention - nobody addressed you by name - so "interrupt me only for mentions" has to mean it. `allows_huddle_ring()` is a separate predicate from `allows_unread_bump()` despite matching it today, because if the two ever diverge it should be in the direction of ringing less. Ignore is client-side only: it drops the banner and remembers the room for that page. Nothing reaches the server, so nobody learns you declined, per the ticket. The banner text is built with `textContent`, never `innerHTML` - room names and display names are user-controlled. Full suite green (182 test binaries, up from 181), `just check` clean, en/es Fluent symmetry holds. ## Not in this change **Targeted invites (AC2).** Ringing the room is the part that removes the discovery problem; inviting named people is a different interaction that probably wants a picker and may want a stronger affordance than a banner. Worth its own PR rather than bolting a member picker onto this one. **AC5's "join via the ring" test.** The fan-out and all three policy guards are covered server-side. The click-to-join path is `?huddle=1` in `voice.js`, which is browser behaviour with no test harness in this repo - there are no JS tests. Flagging rather than pretending it is covered. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5

longjacksonle added 1 commit

2026-07-20 21:42:52 +02:00

feat(huddle): ring room members when a huddle starts (LC-611)

check-secrets / Nosey parker (push) Successful in 10s

Details

check-secrets / TruffleHog (push) Successful in 16s

Details

check-secrets / Kingfisher (push) Successful in 17s

Details

check-secrets / Nosey parker (pull_request) Successful in 9s

Details

check-secrets / Kingfisher (pull_request) Successful in 15s

Details

check-secrets / TruffleHog (pull_request) Successful in 15s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Check / clippy + fmt + tests (pull_request) Successful in 5m59s

Details

396bee6552

Starting a huddle broadcast exactly one `ChatEvent::VoiceJoined` over `broadcast_to_room`, which only reaches sockets already subscribed to that room - the people who can see the huddle bar anyway. Members looking anywhere else in the app learned nothing, so an ongoing group call was something you discovered rather than something you were invited to.

A new `HuddleStarted` event is addressed to one recipient (`to_user_id`) and fanned out over `list_room_member_ids`, so it reaches members wherever they are. The client renders a banner offering Join, whose link carries `?huddle=1` so `voice.js` joins on arrival - Join is genuinely one click rather than a trip to the room to find the bar.

This does NOT generalize `Hub::try_start_ringing`, which the ticket assumed it would. That slot exists to arbitrate DM glare: two peers inviting each other at once, where exactly one must win. A huddle has no such contest - two people starting one in the same room simultaneously both land in the same `voice_rooms` entry and are already in a call together. Generalizing it would have meant inventing a loser for a race with no stakes, and would have put the 1:1 call path at risk to do it. That path has no test coverage today (LC-614) and LC-596 AC4 requires it to keep its current behaviour, so the safest change to it was none. Nothing in `call.js`, `relay_call_signal`, or the ringing slot is touched.

For the same reason the single-peer `#lc-call-root` is left alone: a room huddle is better served by a dismissible banner than by a modal ring seizing the page, and reusing a shell built for one remote video would have forced an N-peer rework for no gain here. LC-611's UI blocker is therefore sidestepped rather than solved; a targeted invite (AC2) may still want a stronger affordance, and is not in this change.

The fan-out and its policy live in `huddle_ring.rs` rather than `routes::ws`. Who gets rung and when is domain logic, and `ws.rs` is already large.

Three guards, each with a test proven to fail without it:
- rings once per huddle, not once per joiner, gated on the mesh being empty before the join
- a persistent enclave voice channel never rings: walking into a room that always exists has not summoned anyone
- muted rooms do not ring, and `except_mentions` suppresses too - a huddle starting is not a mention, so "interrupt me only for mentions" has to mean it

Ignore is client-side only. Nothing is sent to the server, so nobody learns you declined, as the ticket requires.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FhDVMViNHqUbwmfe2aXTv5