[review] discord puppet: rejoin/reconnect gaps (presence desync, echo-exclusion miss, opt-out capture leak) #70

New issue

Open

opened 2026-07-07 13:52:35 +00:00 by robocub · 0 comments

robocub commented

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

Collaborator

Reconnect/runtime-registration edge cases on the Discord side. Note F2-6 is an echo-exclusion risk (constraint #5): songbird's full-reconnect fires DriverReconnect with a NEW SSRC that PuppetSsrcRegistrar (DriverConnect-only) never adds to the exclusion set. Also presence-count desync on runtime-linked channels (F2-3), an ensure_ghost presence race (F2-4), and opt-out audio still written to diagnostic capture WAV before the opt-out gate (F2-5).

F2-6 — P2 / LIVE / UNVERIFIED (needs live)

Location: crates/discord/src/puppet.rs:3338 @ 968da57

Defect: PuppetSsrcRegistrar only handles DriverConnect, but songbird's full-reconnect path fires DriverReconnect carrying a NEW ssrc (vendor/songbird driver/tasks/mod.rs:281), so the puppet's new SSRC is never added to the echo-exclusion set and the stale one is never removed.

Failure scenario: A puppet's voice session does a full reconnect (songbird retries succeed, so no DriverDisconnect/rejoin) and gets a new SSRC: the anchor now processes the puppet's playback frames past the exclude gate (recorded into debug WAVs, counted as 'speaker frames dropped' warn-spam since bots have no sink); meanwhile the stale SSRC stays excluded until leave, so a human later assigned that recycled SSRC by the voice server has their audio silently dropped from Discord->Matrix capture.

Verifier: Code-side chain fully verified, could not be falsified: songbird's non-final WS error path (vendor/songbird/src/driver/tasks/ws.rs:212-224 → tasks/mod.rs:168-205) and mixer UDP-failure FullReconnect both end in a full Connection::new that fires ONLY DriverReconnect (never DriverConnect, and DriverDisconnect only if retries exhaust), with ssrc taken from a fresh Identify→Ready (connection/mod.rs:217). PuppetSsrcRegistrar (puppet.rs:3338) matches only DriverConnect; DriverReconnect is wired log-only to ConnectionLogger (puppet.rs:3161,3172; lib.rs:388-391); the only exclude-set removals (puppet_leave 3213-3222, rejoin_puppet 3264-3270) use current_ssrc, which would still hold the stale value. The capture gate (lib.rs:249) is SSRC-keyed before user attribution, so a stale excluded SSRC recycled to a human silently drops their Discord→Matrix audio, and an unexcluded puppet SSRC pollutes debug WAVs + dropped-frame warn-spam (no Matrix echo loop though — bots have no ghost sink, second-layer defense holds, as the claim itself acknowledges). No test pins DriverReconnect behavior; no doc claims SSRC stability across reconnect — in fact puppet_leave's own comment ("so a recycled SSRC isn't wrongly muted") treats SSRC recycling as a real hazard. HOWEVER the pivotal trigger premise — that Discord's voice server returns a DIFFERENT ssrc when songbird re-Identifies with the SAME session_id on the full-reconnect path — is live Discord behavior undeterminable from code: songbird's resume sub-path provably fires DriverReconnect with the SAME ssrc, so DriverReconnect does not inherently imply a new SSRC; if Discord keeps SSRC session-stable across re-Identify there is no defect (and an invalid session would exhaust retries → DriverDisconnect → the existing rejoin path cleans up the exclude set correctly). The human-muting branch further requires live SSRC recycling within the same anchor session. Hence UNCERTAIN_LIVE, matching the claim's own [LIVE] tag.

Regression test sketch: If live behavior confirms a changed SSRC: no unit test can drive songbird's reconnect machinery, but the handler gap is locally testable — construct PuppetSsrcRegistrar with a populated exclude/current_ssrc (simulating the first DriverConnect), then invoke act() with EventContext::DriverReconnect carrying a different ssrc, and assert exclude contains the new ssrc and not the old, current_ssrc updated. Belongs in a new #[cfg(test)] block near the handler in crates/discord/src/puppet.rs (the existing tests there are allocator-only). Live confirmation alternative: grep prod logs for "songbird DriverReconnect" (ConnectionLogger warn, lib.rs:388) and compare its ssrc field against the preceding "puppet SSRC registered for echo-exclusion" for the same bot/guild.

F2-3 — P1 / LIVE / CONFIRMED

Location: crates/discord/src/puppet.rs:3044 @ 968da57

Defect: puppet_become_anchor's ghost seeding calls ensure_ghost for cached voice members without recording them via note_presence, so for a runtime-registered channel (#45 link flow, no guild_create replay) the user's later leave returns false from note_presence(..., false) and drop_ghost is never called.

Failure scenario: Operator links a new channel at runtime while Discord users are already in voice; a Matrix user joins -> anchor joins and seeds ghosts from ctx.cache (presence set stays empty). A seeded Discord user leaves voice -> voice_state_update's note_presence(bridge, uid, false) removes nothing -> edge not observed -> ghost never retired: a phantom participant stays in the Element Call indefinitely, and the non-empty user_sinks map also makes maybe_release_idle_anchor/release_all keep the anchor bot in voice as if speakers were present.

Verifier: The failure scenario survives step-by-step re-derivation from the code. (1) A #45 runtime-linked channel is created with an empty presence set (puppet.rs:610 via supervisor.rs:216) and the only bulk presence seeder, guild_create (2287-2301), already fired before the channel existed and does not replay. (2) puppet_become_anchor's seeding loop (3044-3046) calls ensure_ghost only; ensure_ghost (2598-2670) populates user_sinks/capture_router but never touches presence, and pre-existing voice members emit no voice_state_update. (3) On the seeded user's leave, voice_state_update (2267) gates drop_ghost behind note_presence(..., false), which returns false because presence.remove misses (830) — so the ghost is never retired and no other retire path applies: a phantom participant persists in the Element Call until rejoin+leave, gateway re-identify, unlink, or restart. (4) The phantom user_sinks entry also makes release_all_locked (1883-1888) keep a capture-only anchor at call-end and makes maybe_release_idle_anchor (1972-1974) refuse release. One overstatement in the claim: the anchor part is bounded — the post-call-end grace timer (release_capture_only_anchor_if_idle, 2033-2040) ignores user_sinks and releases the anchor after anchor_idle_grace — but that does not rescue the P1 core (indefinite phantom ghost). No #[cfg(test)] test pins the current behavior as intended, and doc comments show design intent (guild_create seeding is 'idempotent... via the shared presence set') that the anchor path violates.

Regression test sketch: Belongs in the existing mod tests at the bottom of crates/discord/src/puppet.rs (alongside note_presence_reports_edges_once, using the pool()/channel() helpers). Since puppet_become_anchor needs a live serenity Context, the fix should record presence via the pool (e.g. call pool.note_presence(bridge, uid, true) in the seeding loop, or extract a SharedPool::seed_presence helper the anchor path uses). Test: anchor_seeding_records_presence_so_leave_retires_ghost — arrange: p = pool(1); channel(&p, "A", 1, 10); invoke the extracted seeding/presence-recording path for uid 777 (simulating anchor ghost seeding on a runtime-registered channel with empty presence); assert p.alloc channels["A"].presence contains 777 (or that a subsequent p.note_presence("A", 777, false) returns true — the leave edge is observed so drop_ghost fires). A companion assertion can pin the regression direction: before the fix, note_presence("A", 777, false) returns false while user_sinks still holds 777.

F2-4 — P2 / LIVE / CONFIRMED

Location: crates/discord/src/puppet.rs:2648 @ 968da57

Defect: ensure_ghost's commit re-checks bot_ids and duplicate sinks under the user_sinks write lock but not presence, so a leave processed during the awaited member HTTP lookup (concurrent event tasks / other bots' gateways) mints a ghost for a user who already left.

Failure scenario: User joins the bridged channel and leaves within ~1s: bot A's voice_state_update observes the join edge and awaits guild.member() (member payload absent); meanwhile the leave event (another gateway or serenity's concurrent dispatch) observes the leave edge and runs drop_ghost, which finds no sink and returns. The lookup then completes and inserts the sink -> stale ghost persists in the call (and blocks idle-anchor release) until the user rejoins or the channel is unregistered.

Verifier: The race re-derives exactly from the code in /root/nvb-refactor/crates/discord/src/puppet.rs. (1) Join edge: voice_state_update (line 2263) or guild_create seeding (2294) or anchor seeding (3045) calls note_presence(..., true) — which inserts into the shared presence set and returns the edge — then awaits ensure_ghost. When member is None (explicitly always None on the anchor-seed path at 3045; Option on the gateway paths), ensure_ghost awaits a guild.member() HTTP lookup (2633) with no locks held. (2) Leave during that await: any other pool bot's gateway (each bot is a separate serenity client; every gateway sees every guild voice event) processes the leave, note_presence(..., false) returns true (presence WAS inserted in step 1), drop_ghost (2675) does sinks.remove → None → returns false, releasing nothing. (3) The lookup completes and the commit block (2648-2668) re-checks ONLY bot_ids and a duplicate sink — its own doc comment (2642-2647) enumerates exactly those two races (retract_self_ghost startup race, duplicate ensure_ghost) and never re-checks presence — so it inserts the sink and capture_router.assign() mints a Matrix ghost for a user who already left. (4) Persistence: no reconciliation of user_sinks against presence exists anywhere in the crate (the periodic sweep at 667/1615 only probes puppet voice-connection health), and the stale sink makes maybe_release_idle_anchor (1972: speakers = !user_sinks.is_empty()) permanently refuse to release a capture-only anchor. It self-heals only via the same user's rejoin+re-leave cycle, retract_self_ghost (bots only), or channel unregister. Checks against falsification: no #[cfg(test)] test pins this (tests at 4012+ cover note_presence edge semantics only; ensure_ghost is untested — it needs a serenity Context); no doc comment or CLAUDE.md invariant declares a leave-during-lookup outcome intended; the state is reachable from three real call sites. Severity P2 stands (stale ghost visible in the Element Call + blocked idle-anchor release, i.e. a pool bot leaks until rejoin/unregister). Not directly unit-testable as written — ensure_ghost takes a live Context for the HTTP lookup, so a regression test would first need the commit step extracted into a Context-free function.

F2-5 — P2 / LIVE / CONFIRMED

Location: crates/discord/src/lib.rs:268 @ 968da57

Defect: In demux_tick the debug WAV capture (capture_sink.write_frame) runs before the #63 opt-out gate (line 277-278), so an opted-out user's voice is still recorded to local capture_.wav files despite the 'single Discord->Matrix choke point' consent guarantee.

Failure scenario: Operator enables [discord] capture_wav for diagnostics on a bridge; a user who ran /bridge optout speaks in the channel: their frames are excluded from Matrix forwarding but every 20ms frame is written to disk in capture_.wav — a consent/privacy violation of the #63 opt-out contract.

Verifier: The failure scenario survives re-derivation step by step. In demux_tick (crates/discord/src/lib.rs:268-270), capture_sink.write_frame(*ssrc, pcm) runs for every non-bridge-owned speaking SSRC before the #63 optout gate at lines 277-278; the optout check only gates the Matrix-forward push. The path is reachable: supervisor.rs:198 constructs a real CaptureSink when [bridge.discord] capture_wav = true (or NETHER_DISCORD_CAPTURE_WAV), and CaptureSink::write_frame persists frames to capture_.wav on disk (capture.rs:57-95). So with the diagnostic knob on, an opted-out user's voice is recorded to disk every 20 ms while being dropped from Matrix — exactly as claimed. No test pins this ordering as intended (lib.rs tests only cover playback promotion; the puppet.rs optout test covers ghost retirement/lookup, not capture), and no doc sanctions it: the capture_wav doc (config/src/lib.rs:479-488) describes a debug-only tap, docs/privacy-policy.md promises "Voice audio: not retained at all", and the /bridge optout reply promises the user's voice won't be relayed. The echo-exclusion check already filters BEFORE capture, and ssrc_map+optout locks are already held in the same function, so gating capture on optout is trivially possible — this is an ordering oversight, not a design constraint. One caveat weakening the claim's framing but not its substance: the "single Discord→Matrix choke point" comment only guarantees Matrix crossing (which is preserved), and capture_wav records ALL users when enabled — but the opted-out user is precisely the one who withdrew consent, so P2 stands.

Regression test sketch: Not required at P2, but if added: in a new #[cfg(test)] test in crates/discord/src/lib.rs, build a VoiceReceiver with a real CaptureSink (or a counting test double behind the same Option<Arc>), populate ssrc_map {ssrc→uid}, insert uid into the OptoutSet, feed a synthetic VoiceTick with that SSRC speaking and 1920 decoded samples, call demux_tick, and assert the capture sink received zero frames for that SSRC (while a non-opted-out control SSRC in the same tick is captured).

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.

Reconnect/runtime-registration edge cases on the Discord side. Note F2-6 is an **echo-exclusion** risk (constraint #5): songbird's full-reconnect fires `DriverReconnect` with a NEW SSRC that `PuppetSsrcRegistrar` (DriverConnect-only) never adds to the exclusion set. Also presence-count desync on runtime-linked channels (F2-3), an ensure_ghost presence race (F2-4), and opt-out audio still written to diagnostic capture WAV before the opt-out gate (F2-5). ### F2-6 — P2 / LIVE / UNVERIFIED (needs live) **Location:** `crates/discord/src/puppet.rs:3338` @ `968da57` **Defect:** PuppetSsrcRegistrar only handles DriverConnect, but songbird's full-reconnect path fires DriverReconnect carrying a NEW ssrc (vendor/songbird driver/tasks/mod.rs:281), so the puppet's new SSRC is never added to the echo-exclusion set and the stale one is never removed. **Failure scenario:** A puppet's voice session does a full reconnect (songbird retries succeed, so no DriverDisconnect/rejoin) and gets a new SSRC: the anchor now processes the puppet's playback frames past the exclude gate (recorded into debug WAVs, counted as 'speaker frames dropped' warn-spam since bots have no sink); meanwhile the stale SSRC stays excluded until leave, so a human later assigned that recycled SSRC by the voice server has their audio silently dropped from Discord->Matrix capture. **Verifier:** Code-side chain fully verified, could not be falsified: songbird's non-final WS error path (vendor/songbird/src/driver/tasks/ws.rs:212-224 → tasks/mod.rs:168-205) and mixer UDP-failure FullReconnect both end in a full Connection::new that fires ONLY DriverReconnect (never DriverConnect, and DriverDisconnect only if retries exhaust), with ssrc taken from a fresh Identify→Ready (connection/mod.rs:217). PuppetSsrcRegistrar (puppet.rs:3338) matches only DriverConnect; DriverReconnect is wired log-only to ConnectionLogger (puppet.rs:3161,3172; lib.rs:388-391); the only exclude-set removals (puppet_leave 3213-3222, rejoin_puppet 3264-3270) use current_ssrc, which would still hold the stale value. The capture gate (lib.rs:249) is SSRC-keyed before user attribution, so a stale excluded SSRC recycled to a human silently drops their Discord→Matrix audio, and an unexcluded puppet SSRC pollutes debug WAVs + dropped-frame warn-spam (no Matrix echo loop though — bots have no ghost sink, second-layer defense holds, as the claim itself acknowledges). No test pins DriverReconnect behavior; no doc claims SSRC stability across reconnect — in fact puppet_leave's own comment ("so a recycled SSRC isn't wrongly muted") treats SSRC recycling as a real hazard. HOWEVER the pivotal trigger premise — that Discord's voice server returns a DIFFERENT ssrc when songbird re-Identifies with the SAME session_id on the full-reconnect path — is live Discord behavior undeterminable from code: songbird's resume sub-path provably fires DriverReconnect with the SAME ssrc, so DriverReconnect does not inherently imply a new SSRC; if Discord keeps SSRC session-stable across re-Identify there is no defect (and an invalid session would exhaust retries → DriverDisconnect → the existing rejoin path cleans up the exclude set correctly). The human-muting branch further requires live SSRC recycling within the same anchor session. Hence UNCERTAIN_LIVE, matching the claim's own [LIVE] tag. **Regression test sketch:** If live behavior confirms a changed SSRC: no unit test can drive songbird's reconnect machinery, but the handler gap is locally testable — construct PuppetSsrcRegistrar with a populated exclude/current_ssrc (simulating the first DriverConnect), then invoke act() with EventContext::DriverReconnect carrying a different ssrc, and assert exclude contains the new ssrc and not the old, current_ssrc updated. Belongs in a new #[cfg(test)] block near the handler in crates/discord/src/puppet.rs (the existing tests there are allocator-only). Live confirmation alternative: grep prod logs for "songbird DriverReconnect" (ConnectionLogger warn, lib.rs:388) and compare its ssrc field against the preceding "puppet SSRC registered for echo-exclusion" for the same bot/guild. ### F2-3 — P1 / LIVE / CONFIRMED **Location:** `crates/discord/src/puppet.rs:3044` @ `968da57` **Defect:** puppet_become_anchor's ghost seeding calls ensure_ghost for cached voice members without recording them via note_presence, so for a runtime-registered channel (#45 link flow, no guild_create replay) the user's later leave returns false from note_presence(..., false) and drop_ghost is never called. **Failure scenario:** Operator links a new channel at runtime while Discord users are already in voice; a Matrix user joins -> anchor joins and seeds ghosts from ctx.cache (presence set stays empty). A seeded Discord user leaves voice -> voice_state_update's note_presence(bridge, uid, false) removes nothing -> edge not observed -> ghost never retired: a phantom participant stays in the Element Call indefinitely, and the non-empty user_sinks map also makes maybe_release_idle_anchor/release_all keep the anchor bot in voice as if speakers were present. **Verifier:** The failure scenario survives step-by-step re-derivation from the code. (1) A #45 runtime-linked channel is created with an empty presence set (puppet.rs:610 via supervisor.rs:216) and the only bulk presence seeder, guild_create (2287-2301), already fired before the channel existed and does not replay. (2) puppet_become_anchor's seeding loop (3044-3046) calls ensure_ghost only; ensure_ghost (2598-2670) populates user_sinks/capture_router but never touches presence, and pre-existing voice members emit no voice_state_update. (3) On the seeded user's leave, voice_state_update (2267) gates drop_ghost behind note_presence(..., false), which returns false because presence.remove misses (830) — so the ghost is never retired and no other retire path applies: a phantom participant persists in the Element Call until rejoin+leave, gateway re-identify, unlink, or restart. (4) The phantom user_sinks entry also makes release_all_locked (1883-1888) keep a capture-only anchor at call-end and makes maybe_release_idle_anchor (1972-1974) refuse release. One overstatement in the claim: the anchor part is bounded — the post-call-end grace timer (release_capture_only_anchor_if_idle, 2033-2040) ignores user_sinks and releases the anchor after anchor_idle_grace — but that does not rescue the P1 core (indefinite phantom ghost). No #[cfg(test)] test pins the current behavior as intended, and doc comments show design intent (guild_create seeding is 'idempotent... via the shared presence set') that the anchor path violates. **Regression test sketch:** Belongs in the existing `mod tests` at the bottom of crates/discord/src/puppet.rs (alongside note_presence_reports_edges_once, using the pool()/channel() helpers). Since puppet_become_anchor needs a live serenity Context, the fix should record presence via the pool (e.g. call pool.note_presence(bridge, uid, true) in the seeding loop, or extract a SharedPool::seed_presence helper the anchor path uses). Test: `anchor_seeding_records_presence_so_leave_retires_ghost` — arrange: p = pool(1); channel(&p, "A", 1, 10); invoke the extracted seeding/presence-recording path for uid 777 (simulating anchor ghost seeding on a runtime-registered channel with empty presence); assert p.alloc channels["A"].presence contains 777 (or that a subsequent p.note_presence("A", 777, false) returns true — the leave edge is observed so drop_ghost fires). A companion assertion can pin the regression direction: before the fix, note_presence("A", 777, false) returns false while user_sinks still holds 777. ### F2-4 — P2 / LIVE / CONFIRMED **Location:** `crates/discord/src/puppet.rs:2648` @ `968da57` **Defect:** ensure_ghost's commit re-checks bot_ids and duplicate sinks under the user_sinks write lock but not presence, so a leave processed during the awaited member HTTP lookup (concurrent event tasks / other bots' gateways) mints a ghost for a user who already left. **Failure scenario:** User joins the bridged channel and leaves within ~1s: bot A's voice_state_update observes the join edge and awaits guild.member() (member payload absent); meanwhile the leave event (another gateway or serenity's concurrent dispatch) observes the leave edge and runs drop_ghost, which finds no sink and returns. The lookup then completes and inserts the sink -> stale ghost persists in the call (and blocks idle-anchor release) until the user rejoins or the channel is unregistered. **Verifier:** The race re-derives exactly from the code in /root/nvb-refactor/crates/discord/src/puppet.rs. (1) Join edge: voice_state_update (line 2263) or guild_create seeding (2294) or anchor seeding (3045) calls note_presence(..., true) — which inserts into the shared presence set and returns the edge — then awaits ensure_ghost. When `member` is None (explicitly always None on the anchor-seed path at 3045; Option on the gateway paths), ensure_ghost awaits a guild.member() HTTP lookup (2633) with no locks held. (2) Leave during that await: any other pool bot's gateway (each bot is a separate serenity client; every gateway sees every guild voice event) processes the leave, note_presence(..., false) returns true (presence WAS inserted in step 1), drop_ghost (2675) does sinks.remove → None → returns false, releasing nothing. (3) The lookup completes and the commit block (2648-2668) re-checks ONLY bot_ids and a duplicate sink — its own doc comment (2642-2647) enumerates exactly those two races (retract_self_ghost startup race, duplicate ensure_ghost) and never re-checks presence — so it inserts the sink and capture_router.assign() mints a Matrix ghost for a user who already left. (4) Persistence: no reconciliation of user_sinks against presence exists anywhere in the crate (the periodic sweep at 667/1615 only probes puppet voice-connection health), and the stale sink makes maybe_release_idle_anchor (1972: `speakers = !user_sinks.is_empty()`) permanently refuse to release a capture-only anchor. It self-heals only via the same user's rejoin+re-leave cycle, retract_self_ghost (bots only), or channel unregister. Checks against falsification: no #[cfg(test)] test pins this (tests at 4012+ cover note_presence edge semantics only; ensure_ghost is untested — it needs a serenity Context); no doc comment or CLAUDE.md invariant declares a leave-during-lookup outcome intended; the state is reachable from three real call sites. Severity P2 stands (stale ghost visible in the Element Call + blocked idle-anchor release, i.e. a pool bot leaks until rejoin/unregister). Not directly unit-testable as written — ensure_ghost takes a live Context for the HTTP lookup, so a regression test would first need the commit step extracted into a Context-free function. ### F2-5 — P2 / LIVE / CONFIRMED **Location:** `crates/discord/src/lib.rs:268` @ `968da57` **Defect:** In demux_tick the debug WAV capture (capture_sink.write_frame) runs before the #63 opt-out gate (line 277-278), so an opted-out user's voice is still recorded to local capture_<ssrc>.wav files despite the 'single Discord->Matrix choke point' consent guarantee. **Failure scenario:** Operator enables [discord] capture_wav for diagnostics on a bridge; a user who ran /bridge optout speaks in the channel: their frames are excluded from Matrix forwarding but every 20ms frame is written to disk in capture_<ssrc>.wav — a consent/privacy violation of the #63 opt-out contract. **Verifier:** The failure scenario survives re-derivation step by step. In demux_tick (crates/discord/src/lib.rs:268-270), capture_sink.write_frame(*ssrc, pcm) runs for every non-bridge-owned speaking SSRC before the #63 optout gate at lines 277-278; the optout check only gates the Matrix-forward push. The path is reachable: supervisor.rs:198 constructs a real CaptureSink when `[bridge.discord] capture_wav = true` (or NETHER_DISCORD_CAPTURE_WAV), and CaptureSink::write_frame persists frames to capture_<ssrc>.wav on disk (capture.rs:57-95). So with the diagnostic knob on, an opted-out user's voice is recorded to disk every 20 ms while being dropped from Matrix — exactly as claimed. No test pins this ordering as intended (lib.rs tests only cover playback promotion; the puppet.rs optout test covers ghost retirement/lookup, not capture), and no doc sanctions it: the capture_wav doc (config/src/lib.rs:479-488) describes a debug-only tap, docs/privacy-policy.md promises "Voice audio: not retained at all", and the /bridge optout reply promises the user's voice won't be relayed. The echo-exclusion check already filters BEFORE capture, and ssrc_map+optout locks are already held in the same function, so gating capture on optout is trivially possible — this is an ordering oversight, not a design constraint. One caveat weakening the claim's framing but not its substance: the "single Discord→Matrix choke point" comment only guarantees Matrix crossing (which is preserved), and capture_wav records ALL users when enabled — but the opted-out user is precisely the one who withdrew consent, so P2 stands. **Regression test sketch:** Not required at P2, but if added: in a new #[cfg(test)] test in crates/discord/src/lib.rs, build a VoiceReceiver with a real CaptureSink (or a counting test double behind the same Option<Arc<CaptureSink>>), populate ssrc_map {ssrc→uid}, insert uid into the OptoutSet, feed a synthetic VoiceTick with that SSRC speaking and 1920 decoded samples, call demux_tick, and assert the capture sink received zero frames for that SSRC (while a non-opted-out control SSRC in the same tick is captured). --- *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

2026-07-19 22:12:58 +00:00

e2e nightly RED 8 days — real multi-channel/encrypted/recovery audio failure cluster (NOT contention); env-vs-code TBD #124