dark/nether-voicebridge

Fork 1

[review] matrix-rtc: LiveKit RoomEvent::Disconnected is swallowed with no recovery (subscriber reaper can wipe real participants) #66

New issue

Closed

opened 2026-07-07 13:52:34 +00:00 by robocub · 1 comment

robocub commented

2026-07-07 13:52:34 +00:00

Collaborator

A dropped LiveKit connection (SFU restart / network blip) is logged and the event task simply breaks — the RTC run loop is never told, so joined/active_session stay set against a dead room with no reconnect or teardown. Worst case (subscriber): the still-armed stale-membership reaper compares live roster against the dead room and empties real humans' m.call.member after the grace. Highest-impact theme in the review.

F7-4 — P2 / LIVE / CONFIRMED

Location: crates/matrix-rtc/src/rtc/session.rs:407 @ 968da57

Defect: handle_room_events silently exits on RoomEvent::Disconnected without notifying its owner: rtc::run keeps joined = true / active_session = Some with a dead LiveKit room — there is no reconnect or teardown trigger, and the reaper tick (rtc/mod.rs:300-309) keeps comparing m.call.member state against the dead room's (now empty) remote_participants, so it starts emptying REAL users' call memberships after the 45s reap grace.

Failure scenario: LiveKit SFU restarts mid-call → the subscriber room fires Disconnected and the event task returns → Matrix→Discord audio is silently dead for the rest of the call (puppets stay, fed nothing), and on the next reaper ticks every live human's m.call.member is classified phantom and emptied, flapping them out of the Element Call roster; recovery requires all humans to leave so call_active drops and a fresh join_livekit runs.

Verifier: Re-derived from code: (1) handle_room_events breaks out on RoomEvent::Disconnected with only a warn log (session.rs:405-408); the task handle is stored as _event_task and never awaited — LiveSession exposes no completion/health signal. (2) rtc::run's loop (rtc/mod.rs:201-311) transitions only on call_active_rx and shutdown; (true,true) is a no-op, so joined stays true and active_session stays Some with a dead room — no reconnect or teardown path exists (the supervisor's "no reconnect yet" arm never fires because run never exits). (3) Puppets are deliberately kept on track-end (session.rs:300-304, "membership owns release"), so Matrix→Discord is silently dead with puppets present. (4) The reaper guard is only active_session.is_some() (mod.rs:302). Verified against livekit rust-sdks v0.7.45: the SDK auto-reconnects before emitting Disconnected, and handle_restarting removes ALL remote participants; so in the reconnect-exhausted path (SFU outage beyond retry budget / expired lk-jwt on restart) remote_participants() is empty at Disconnected time → live_roster_mxids=∅ → classify_stale_memberships reaps every real user's m.call.member after the 45s grace (exact behavior pinned by the existing absent_user_reaped_only_after_grace test for an empty live set). Nuances that don't break the claim: a brief SFU blip that reconnects successfully self-heals (Disconnected never fires — trigger requires reconnect exhaustion); a direct disconnect without a restart phase leaves a frozen stale snapshot instead of an empty one (existing members spared, but new EC joiners get reaped); and in the empty-roster variant the reaper's own emptying flips call_active false → EC clients re-posting membership can accidentally trigger a fresh join, so recovery may be a roster flap rather than strictly "all humans leave" (frozen-snapshot variant matches the claim's recovery exactly). No test or doc comment pins the silent exit as intended. P2 LIVE as claimed.

Regression test sketch: Not required (P2), but the reaper half is locally pinnable: in crates/matrix-rtc/src/matrix.rs tests (classify_stale_memberships module), the empty-live-set reap is already covered; the missing regression is at the session layer — extract the Disconnected handling into a testable seam (e.g. handle_room_events signals its exit via a watch/oneshot that rtc::run selects on) and add a test in rtc/session.rs asserting that feeding RoomEvent::Disconnected into handle_room_events fires the exit signal, plus a mod.rs-level test that the signal clears joined/active_session (stopping the reaper) or triggers rejoin.

F8-1 — P1 / LIVE / UNVERIFIED (needs live)

Location: crates/matrix-rtc/src/rtc/session.rs:405 @ 968da57

Defect: RoomEvent::Disconnected is logged and swallowed (the event task just breaks): rtc/mod.rs's run loop is never told, so joined stays true, the subscriber never reconnects, and the still-armed reaper tick (rtc/mod.rs:300-309) compares live m.call.member state against the dead room's roster and empties REAL participants' memberships after reap_grace.

Failure scenario: LiveKit SFU container restarts (or a network blip drops the room) mid-call with 3 humans present → Matrix→Discord audio goes silent with no recovery attempt; ~45 s later the reaper sees an empty live_roster_mxids() and wipes all 3 humans' m.call.member (visible roster disruption in Element Call); the bridge only recovers via the accidental call-active flap that wipe causes, churning every puppet.

Verifier: verifier unavailable

F8-4 — P2 / LIVE / UNVERIFIED (needs live)

Location: crates/matrix-rtc/src/rtc/ghost.rs:1185 @ 968da57

Defect: A ghost's LiveKit RoomEvent::Disconnected is logged, the drain task breaks and closes the room, but run_ghost/pump_audio are never signalled — the ghost keeps 'publishing' into a dead source (capture_frame errors rate-limited to a warn every 50) with no reconnect, and its m.call.member stays posted.

Failure scenario: SFU drops one ghost's connection mid-call → that Discord speaker is silently inaudible in Element Call (while still shown as a call member) until the speaker leaves and rejoins the voice channel or the bridge restarts.

Verifier: verifier unavailable

Filed from an automated multi-agent code review of master @ 968da57 (8 scoped finders + adversarial verification). Every finding below was re-derived from the code by an independent verifier. Fixes for the confirmed local-verifiable defects (allocator anchor panic F1-1/F1-2/F1-3/F2-1/F2-2, capture-sink panic F2-7, config env-fallback F5-4, opt-out RMW race F5-2) already landed on branch refactor/review-and-split.

A dropped LiveKit connection (SFU restart / network blip) is logged and the event task simply breaks — the RTC run loop is never told, so `joined`/`active_session` stay set against a dead room with no reconnect or teardown. Worst case (subscriber): the still-armed stale-membership reaper compares live roster against the dead room and empties **real humans'** `m.call.member` after the grace. Highest-impact theme in the review. ### F7-4 — P2 / LIVE / CONFIRMED **Location:** `crates/matrix-rtc/src/rtc/session.rs:407` @ `968da57` **Defect:** handle_room_events silently exits on RoomEvent::Disconnected without notifying its owner: rtc::run keeps `joined = true` / `active_session = Some` with a dead LiveKit room — there is no reconnect or teardown trigger, and the reaper tick (rtc/mod.rs:300-309) keeps comparing m.call.member state against the dead room's (now empty) remote_participants, so it starts emptying REAL users' call memberships after the 45s reap grace. **Failure scenario:** LiveKit SFU restarts mid-call → the subscriber room fires Disconnected and the event task returns → Matrix→Discord audio is silently dead for the rest of the call (puppets stay, fed nothing), and on the next reaper ticks every live human's m.call.member is classified phantom and emptied, flapping them out of the Element Call roster; recovery requires all humans to leave so call_active drops and a fresh join_livekit runs. **Verifier:** Re-derived from code: (1) handle_room_events breaks out on RoomEvent::Disconnected with only a warn log (session.rs:405-408); the task handle is stored as `_event_task` and never awaited — LiveSession exposes no completion/health signal. (2) rtc::run's loop (rtc/mod.rs:201-311) transitions only on call_active_rx and shutdown; (true,true) is a no-op, so `joined` stays true and `active_session` stays Some with a dead room — no reconnect or teardown path exists (the supervisor's "no reconnect yet" arm never fires because run never exits). (3) Puppets are deliberately kept on track-end (session.rs:300-304, "membership owns release"), so Matrix→Discord is silently dead with puppets present. (4) The reaper guard is only `active_session.is_some()` (mod.rs:302). Verified against livekit rust-sdks v0.7.45: the SDK auto-reconnects before emitting Disconnected, and handle_restarting removes ALL remote participants; so in the reconnect-exhausted path (SFU outage beyond retry budget / expired lk-jwt on restart) remote_participants() is empty at Disconnected time → live_roster_mxids=∅ → classify_stale_memberships reaps every real user's m.call.member after the 45s grace (exact behavior pinned by the existing absent_user_reaped_only_after_grace test for an empty live set). Nuances that don't break the claim: a brief SFU blip that reconnects successfully self-heals (Disconnected never fires — trigger requires reconnect exhaustion); a direct disconnect without a restart phase leaves a frozen stale snapshot instead of an empty one (existing members spared, but new EC joiners get reaped); and in the empty-roster variant the reaper's own emptying flips call_active false → EC clients re-posting membership can accidentally trigger a fresh join, so recovery may be a roster flap rather than strictly "all humans leave" (frozen-snapshot variant matches the claim's recovery exactly). No test or doc comment pins the silent exit as intended. P2 LIVE as claimed. **Regression test sketch:** Not required (P2), but the reaper half is locally pinnable: in crates/matrix-rtc/src/matrix.rs tests (classify_stale_memberships module), the empty-live-set reap is already covered; the missing regression is at the session layer — extract the Disconnected handling into a testable seam (e.g. handle_room_events signals its exit via a watch/oneshot that rtc::run selects on) and add a test in rtc/session.rs asserting that feeding RoomEvent::Disconnected into handle_room_events fires the exit signal, plus a mod.rs-level test that the signal clears joined/active_session (stopping the reaper) or triggers rejoin. ### F8-1 — P1 / LIVE / UNVERIFIED (needs live) **Location:** `crates/matrix-rtc/src/rtc/session.rs:405` @ `968da57` **Defect:** RoomEvent::Disconnected is logged and swallowed (the event task just breaks): rtc/mod.rs's run loop is never told, so `joined` stays true, the subscriber never reconnects, and the still-armed reaper tick (rtc/mod.rs:300-309) compares live m.call.member state against the dead room's roster and empties REAL participants' memberships after reap_grace. **Failure scenario:** LiveKit SFU container restarts (or a network blip drops the room) mid-call with 3 humans present → Matrix→Discord audio goes silent with no recovery attempt; ~45 s later the reaper sees an empty live_roster_mxids() and wipes all 3 humans' m.call.member (visible roster disruption in Element Call); the bridge only recovers via the accidental call-active flap that wipe causes, churning every puppet. **Verifier:** verifier unavailable ### F8-4 — P2 / LIVE / UNVERIFIED (needs live) **Location:** `crates/matrix-rtc/src/rtc/ghost.rs:1185` @ `968da57` **Defect:** A ghost's LiveKit RoomEvent::Disconnected is logged, the drain task breaks and closes the room, but run_ghost/pump_audio are never signalled — the ghost keeps 'publishing' into a dead source (capture_frame errors rate-limited to a warn every 50) with no reconnect, and its m.call.member stays posted. **Failure scenario:** SFU drops one ghost's connection mid-call → that Discord speaker is silently inaudible in Element Call (while still shown as a call member) until the speaker leaves and rejoins the voice channel or the bridge restarts. **Verifier:** verifier unavailable --- *Filed from an automated multi-agent code review of `master` @ `968da57` (8 scoped finders + adversarial verification). Every finding below was re-derived from the code by an independent verifier. Fixes for the confirmed local-verifiable defects (allocator anchor panic F1-1/F1-2/F1-3/F2-1/F2-2, capture-sink panic F2-7, config env-fallback F5-4, opt-out RMW race F5-2) already landed on branch `refactor/review-and-split`.*

robocub referenced this issue from a commit

2026-07-07 18:16:30 +00:00

fix(matrix-rtc): signal subscriber LiveKit room death instead of swallowing it (#66)

robocub referenced this issue from a commit

2026-07-07 18:16:30 +00:00

fix(matrix-rtc): subscriber disconnect recovery — rejoin + reaper guard + peer-key replay (#66)

robocub referenced this issue from a commit

2026-07-07 18:16:30 +00:00

fix(matrix-rtc): ghost LiveKit reconnect loop on room death (#66)

robocub referenced this issue

2026-07-10 15:22:23 +00:00

Subscriber inbound-frame decryptor wedges permanently when frames arrive before the peer key (encrypted-room Matrix→Discord silence) #73

robocub referenced this issue from a commit

2026-07-10 15:34:43 +00:00

fix(matrix-rtc): signal subscriber LiveKit room death instead of swallowing it (#66)

robocub referenced this issue from a commit

2026-07-10 15:34:43 +00:00

fix(matrix-rtc): subscriber disconnect recovery — rejoin + reaper guard + peer-key replay (#66)

robocub referenced this issue from a commit

2026-07-10 15:34:43 +00:00

fix(matrix-rtc): ghost LiveKit reconnect loop on room death (#66)

robocub commented

2026-07-10 15:35:15 +00:00

Author

Collaborator

Shipped on master 05a8740 (3 commits, rebased over #38's 389a53d — testkit call site updated for the new connect_and_publish signature).

What changed (all crates/matrix-rtc/src/rtc/):

session.rs: LiveSession gains a dedicated disconnected token, fired when handle_room_events ends (terminal Disconnected or event-stream close). Reconnecting/Reconnected now logged INFO.
mod.rs: run() selects on that token → tears down (keeping puppet routes) + rejoins via the idempotent join_livekit; clears reaper grace clocks; keeps membership posted (no retract+repost flap). Reap arm skips while the disconnect signal is pending; a membership_lingering flag + guarded (false,false) arm retract a membership if the call ends mid-recovery. New keys::PeerKeyCache records every delivered peer key and replays it into the fresh provider on rejoin (closes the post-rejoin decrypt gap without roster flicker).
ghost.rs: drain loop extracted to drain_room_events (fires a per-attempt disconnected token); pump_audio returns PumpExit; run_ghost reconnect loop (backoff 2→30s, drops stale buffered frames) republishes on room death.

Verified: workspace cargo test + clippy -D warnings green (matrix-rtc 81 tests, incl. 8 new).

Live-proven end-to-end 2026-07-10 on staging (unencrypted Light Voice, both directions live; SFU severed via host-side iptables REJECT to the SFU IP only — Matrix/Cloudflare untouched, no other participant affected):

Transient (iptables DROP, 2 min): SDK auto-reconnect, connection lost — SDK auto-reconnecting INFO fired, no false teardown — recovery is correctly gated on terminal Disconnected.
Terminal (iptables REJECT): subscriber LiveKit room disconnected — tearing down for rejoin → join_livekit retry; ghost ghost LiveKit room disconnected — reconnecting → backoff loop.
Reaper spared the live human — zero real-user m.call.member reaps across the whole outage (the P1 harm this issue was filed for).
On unblock: subscriber resubscribed to @dark + first frame, ghost reconnected — resuming publish; audio confirmed both ways by the operator, 0 errors.

Note: the encrypted-room path couldn't be used to validate because of a pre-existing, unrelated receive-side decryptor wedge discovered during this test (frames-before-key → permanent silence), now filed as #73. #66's recovery logic is media-E2EE-agnostic and its peer-key replay is unit-tested, so #66 is not blocked by #73.

Unreleased — rides in the next tag. Out of scope (left open): GhostPool supervision of self-exited ghost tasks (#67).

Shipped on master `05a8740` (3 commits, rebased over #38's `389a53d` — testkit call site updated for the new `connect_and_publish` signature). **What changed** (all `crates/matrix-rtc/src/rtc/`): - `session.rs`: `LiveSession` gains a dedicated `disconnected` token, fired when `handle_room_events` ends (terminal `Disconnected` or event-stream close). `Reconnecting`/`Reconnected` now logged INFO. - `mod.rs`: `run()` selects on that token → tears down (keeping puppet routes) + rejoins via the idempotent `join_livekit`; clears reaper grace clocks; keeps membership posted (no retract+repost flap). Reap arm skips while the disconnect signal is pending; a `membership_lingering` flag + guarded `(false,false)` arm retract a membership if the call ends mid-recovery. New `keys::PeerKeyCache` records every delivered peer key and replays it into the fresh provider on rejoin (closes the post-rejoin decrypt gap without roster flicker). - `ghost.rs`: drain loop extracted to `drain_room_events` (fires a per-attempt `disconnected` token); `pump_audio` returns `PumpExit`; `run_ghost` reconnect loop (backoff 2→30s, drops stale buffered frames) republishes on room death. **Verified:** workspace `cargo test` + clippy `-D warnings` green (matrix-rtc 81 tests, incl. 8 new). **Live-proven end-to-end 2026-07-10 on staging** (unencrypted Light Voice, both directions live; SFU severed via host-side `iptables REJECT` to the SFU IP only — Matrix/Cloudflare untouched, no other participant affected): - **Transient (iptables DROP, 2 min):** SDK auto-reconnect, `connection lost — SDK auto-reconnecting` INFO fired, **no** false teardown — recovery is correctly gated on *terminal* Disconnected. - **Terminal (iptables REJECT):** subscriber `LiveKit room disconnected — tearing down for rejoin` → `join_livekit` retry; ghost `ghost LiveKit room disconnected — reconnecting` → backoff loop. - **Reaper spared the live human** — zero real-user `m.call.member` reaps across the whole outage (the P1 harm this issue was filed for). - **On unblock:** subscriber resubscribed to `@dark` + `first frame`, ghost `reconnected — resuming publish`; audio confirmed both ways by the operator, 0 errors. Note: the encrypted-room path couldn't be used to validate because of a **pre-existing, unrelated receive-side decryptor wedge** discovered during this test (frames-before-key → permanent silence), now filed as #73. #66's recovery logic is media-E2EE-agnostic and its peer-key replay is unit-tested, so #66 is not blocked by #73. Unreleased — rides in the next tag. Out of scope (left open): GhostPool supervision of self-exited ghost tasks (#67).

robocub closed this issue

2026-07-10 15:35:15 +00:00

robocub referenced this issue from a commit

2026-07-10 16:21:35 +00:00

fix(matrix-rtc): heal inbound decryptor wedged by frames-before-key (#73)