From 29032b5a369f080048b7b51e7fa7a74ed1c5a9c3 Mon Sep 17 00:00:00 2001 From: Myx Date: Fri, 12 Jun 2026 01:00:01 +0200 Subject: [PATCH] fix: Bug - Voice states doesn't get cleared for all users on leave Broadcast a cleared voice_state when voice-active sockets drop and reset mute/deafen flags on disconnect or reconnect so stale session state cannot leak to other clients. Co-authored-by: Cursor --- .../voice/voice-mute-state-reset.spec.ts | 127 ++++++++++++++++++ .../handler-voice-disconnect.spec.ts | 105 +++++++++++++++ server/src/websocket/handler.ts | 53 ++++++++ server/src/websocket/index.ts | 4 +- .../users/user-voice-state.rules.spec.ts | 92 +++++++++++++ .../app/store/users/user-voice-state.rules.ts | 51 +++++++ .../store/users/users-status.reducer.spec.ts | 6 +- toju-app/src/app/store/users/users.reducer.ts | 34 ++--- 8 files changed, 443 insertions(+), 29 deletions(-) create mode 100644 e2e/tests/voice/voice-mute-state-reset.spec.ts create mode 100644 server/src/websocket/handler-voice-disconnect.spec.ts create mode 100644 toju-app/src/app/store/users/user-voice-state.rules.spec.ts create mode 100644 toju-app/src/app/store/users/user-voice-state.rules.ts diff --git a/e2e/tests/voice/voice-mute-state-reset.spec.ts b/e2e/tests/voice/voice-mute-state-reset.spec.ts new file mode 100644 index 0000000..ab58f38 --- /dev/null +++ b/e2e/tests/voice/voice-mute-state-reset.spec.ts @@ -0,0 +1,127 @@ +import { test, expect } from '../../fixtures/multi-client'; +import { + MULTI_DEVICE_PASSWORD, + MULTI_DEVICE_VOICE_CHANNEL, + closeClient, + loginSecondDeviceIntoServer, + uniqueMultiDeviceName +} from '../../helpers/multi-device-session'; +import { RegisterPage } from '../../pages/register.page'; +import { ServerSearchPage } from '../../pages/server-search.page'; +import { ChatRoomPage } from '../../pages/chat-room.page'; + +async function waitForVoiceMuteState( + page: import('@playwright/test').Page, + displayName: string, + expectedMuted: boolean, + timeout = 45_000 +): Promise { + await page.waitForFunction( + ({ expectedDisplayName, expectedMuted: muted }) => { + interface VoiceStateShape { isMuted?: boolean } + interface UserShape { displayName: string; voiceState?: VoiceStateShape } + interface ChannelShape { id: string; type: 'text' | 'voice' } + interface RoomShape { channels?: ChannelShape[] } + interface AngularDebugApi { + getComponent: (element: Element) => Record; + } + + const host = document.querySelector('app-rooms-side-panel'); + const debugApi = (window as { ng?: AngularDebugApi }).ng; + + if (!host || !debugApi?.getComponent) { + return false; + } + + const component = debugApi.getComponent(host); + const currentRoom = (component['currentRoom'] as (() => RoomShape | null) | undefined)?.() ?? null; + const voiceChannel = currentRoom?.channels?.find((channel) => channel.type === 'voice'); + + if (!voiceChannel) { + return false; + } + + const roster = (component['voiceUsersInRoom'] as ((roomId: string) => UserShape[]) | undefined)?.(voiceChannel.id) ?? []; + const entry = roster.find((userEntry) => userEntry.displayName === expectedDisplayName); + + return entry?.voiceState?.isMuted === muted; + }, + { expectedDisplayName: displayName, expectedMuted }, + { timeout } + ); +} + +test.describe('Voice mute state reset', () => { + test.describe.configure({ timeout: 300_000, retries: 1 }); + + test('clears stale mute state after abrupt disconnect and voice rejoin', async ({ createClient }) => { + const suffix = uniqueMultiDeviceName('voice-mute-reset'); + const hostCredentials = { + username: `host_${suffix}`, + displayName: 'Voice Host', + password: MULTI_DEVICE_PASSWORD + }; + const guestCredentials = { + username: `guest_${suffix}`, + displayName: 'Voice Guest', + password: MULTI_DEVICE_PASSWORD + }; + const serverName = `Voice Mute Reset ${suffix}`; + + let hostClient = await createClient(); + + const guestClient = await createClient(); + + await test.step('host creates the shared server', async () => { + const registerPage = new RegisterPage(hostClient.page); + + await registerPage.goto(); + await registerPage.register(hostCredentials.username, hostCredentials.displayName, hostCredentials.password); + await expect(hostClient.page).toHaveURL(/\/dashboard/, { timeout: 15_000 }); + + const search = new ServerSearchPage(hostClient.page); + + await search.createServer(serverName, { description: 'Voice mute reset coverage' }); + await expect(hostClient.page).toHaveURL(/\/room\//, { timeout: 15_000 }); + }); + + const hostRoom = new ChatRoomPage(hostClient.page); + + await hostRoom.ensureVoiceChannelExists(MULTI_DEVICE_VOICE_CHANNEL); + + await test.step('guest joins the server', async () => { + const registerPage = new RegisterPage(guestClient.page); + + await registerPage.goto(); + await registerPage.register(guestCredentials.username, guestCredentials.displayName, guestCredentials.password); + await expect(guestClient.page).toHaveURL(/\/dashboard/, { timeout: 15_000 }); + + const search = new ServerSearchPage(guestClient.page); + + await search.joinServerFromSearch(serverName); + await expect(guestClient.page).toHaveURL(/\/room\//, { timeout: 20_000 }); + }); + + await test.step('host joins voice muted and guest observes the muted state', async () => { + await hostRoom.joinVoiceChannel(MULTI_DEVICE_VOICE_CHANNEL); + await expect(hostRoom.voiceControls).toBeVisible({ timeout: 20_000 }); + await hostRoom.muteButton.click(); + + await waitForVoiceMuteState(guestClient.page, hostCredentials.displayName, true); + }); + + await test.step('abrupt host disconnect clears stale mute before rejoin', async () => { + await closeClient(hostClient); + + hostClient = await createClient(); + await loginSecondDeviceIntoServer(hostClient.page, hostCredentials, serverName); + + const reopenedRoom = new ChatRoomPage(hostClient.page); + + await reopenedRoom.joinVoiceChannel(MULTI_DEVICE_VOICE_CHANNEL); + await expect(reopenedRoom.voiceControls).toBeVisible({ timeout: 20_000 }); + + await waitForVoiceMuteState(guestClient.page, hostCredentials.displayName, false); + }); + }); +}); diff --git a/server/src/websocket/handler-voice-disconnect.spec.ts b/server/src/websocket/handler-voice-disconnect.spec.ts new file mode 100644 index 0000000..d1f96cc --- /dev/null +++ b/server/src/websocket/handler-voice-disconnect.spec.ts @@ -0,0 +1,105 @@ +import { + beforeEach, + describe, + expect, + it +} from 'vitest'; +import { WebSocket } from 'ws'; +import { connectedUsers } from './state'; +import { ConnectedUser } from './types'; +import { finalizeVoiceDisconnectForConnection } from './handler'; + +function createMockWs(): WebSocket & { sentMessages: string[] } { + const sent: string[] = []; + const ws = { + readyState: WebSocket.OPEN, + send: (data: string) => { sent.push(data); }, + close: () => {}, + terminate: () => {}, + sentMessages: sent + } as unknown as WebSocket & { sentMessages: string[] }; + + return ws; +} + +function createConnectedUser( + connectionId: string, + overrides: Partial = {} +): ConnectedUser { + const user: ConnectedUser = { + oderId: 'user-1', + ws: createMockWs(), + authenticated: true, + serverIds: new Set(['server-1']), + displayName: 'Alice', + lastPong: Date.now(), + ...overrides + }; + + connectedUsers.set(connectionId, user); + + return user; +} + +function getSentMessages(user: ConnectedUser): string[] { + return (user.ws as WebSocket & { sentMessages: string[] }).sentMessages; +} + +describe('finalizeVoiceDisconnectForConnection', () => { + beforeEach(() => { + connectedUsers.clear(); + }); + + it('broadcasts a cleared voice_state when a voice-active connection is removed', () => { + createConnectedUser('conn-voice', { + voiceActive: true, + voiceStateSnapshot: { + type: 'voice_state', + serverId: 'server-1', + voiceState: { + isConnected: true, + isMuted: true, + isDeafened: false, + roomId: 'voice-1', + serverId: 'server-1' + } + } + }); + + const observer = createConnectedUser('conn-observer', { oderId: 'user-2' }); + + getSentMessages(observer).length = 0; + + finalizeVoiceDisconnectForConnection('conn-voice'); + + const messages = getSentMessages(observer).map((raw) => JSON.parse(raw) as { + type: string; + voiceState?: { isConnected?: boolean; isMuted?: boolean; isDeafened?: boolean }; + }); + const voiceState = messages.find((message) => message.type === 'voice_state'); + + expect(voiceState).toMatchObject({ + type: 'voice_state', + voiceState: { + isConnected: false, + isMuted: false, + isDeafened: false + } + }); + + expect(connectedUsers.get('conn-voice')?.voiceActive).toBe(false); + expect(connectedUsers.get('conn-voice')?.voiceStateSnapshot).toBeUndefined(); + }); + + it('does nothing when the connection was not voice-active', () => { + const observer = createConnectedUser('conn-observer', { oderId: 'user-2' }); + + createConnectedUser('conn-idle'); + + getSentMessages(observer).length = 0; + + finalizeVoiceDisconnectForConnection('conn-idle'); + + expect(getSentMessages(observer)).toHaveLength(0); + }); +}); diff --git a/server/src/websocket/handler.ts b/server/src/websocket/handler.ts index 6bf072d..5a2ffe0 100644 --- a/server/src/websocket/handler.ts +++ b/server/src/websocket/handler.ts @@ -134,6 +134,59 @@ function clearVoiceActiveForOderId(oderId: string, exceptConnectionId?: string): }); } +function readVoiceStateServerId(snapshot: Record | undefined): string | undefined { + if (!snapshot) { + return undefined; + } + + const nestedVoiceState = snapshot['voiceState']; + + if (nestedVoiceState && typeof nestedVoiceState === 'object') { + const nestedServerId = readMessageId((nestedVoiceState as { serverId?: unknown }).serverId); + + if (nestedServerId) { + return nestedServerId; + } + } + + return readMessageId(snapshot['serverId']); +} + +/** Broadcast a cleared voice_state when a voice-active socket disappears without a graceful leave. */ +export function finalizeVoiceDisconnectForConnection(connectionId: string): void { + const user = connectedUsers.get(connectionId); + + if (!user?.authenticated || (!user.voiceActive && !user.voiceStateSnapshot)) { + return; + } + + const serverId = readVoiceStateServerId(user.voiceStateSnapshot) ?? user.viewedServerId; + + if (serverId && user.serverIds.has(serverId)) { + broadcastToServer( + serverId, + { + type: 'voice_state', + serverId, + oderId: user.oderId, + displayName: normalizeDisplayName(user.displayName), + voiceState: { + isConnected: false, + isMuted: false, + isDeafened: false, + isSpeaking: false + } + }, + { excludeConnectionId: connectionId } + ); + } + + user.voiceActive = false; + user.voiceStateSnapshot = undefined; + connectedUsers.set(connectionId, user); + clearVoiceActiveForOderId(user.oderId, connectionId); +} + function sendVoiceStateSnapshotToConnection(user: ConnectedUser, snapshot: Record): void { user.ws.send(JSON.stringify({ type: 'voice_state', diff --git a/server/src/websocket/index.ts b/server/src/websocket/index.ts index aae9b1a..572643e 100644 --- a/server/src/websocket/index.ts +++ b/server/src/websocket/index.ts @@ -11,7 +11,7 @@ import { getServerIdsForOderId, isOderIdConnectedToServer } from './broadcast'; -import { handleWebSocketMessage } from './handler'; +import { handleWebSocketMessage, finalizeVoiceDisconnectForConnection } from './handler'; type IncomingWebSocketMessage = Parameters[1]; @@ -26,6 +26,8 @@ function removeDeadConnection(connectionId: string): void { if (user) { console.log(`Removing dead connection: ${user.displayName ?? 'Unknown'} (${user.oderId})`); + finalizeVoiceDisconnectForConnection(connectionId); + const remainingServerIds = getServerIdsForOderId(user.oderId, connectionId); user.serverIds.forEach((sid) => { diff --git a/toju-app/src/app/store/users/user-voice-state.rules.spec.ts b/toju-app/src/app/store/users/user-voice-state.rules.spec.ts new file mode 100644 index 0000000..3ec3d94 --- /dev/null +++ b/toju-app/src/app/store/users/user-voice-state.rules.spec.ts @@ -0,0 +1,92 @@ +import { + describe, + expect, + it +} from 'vitest'; +import { mergeVoiceStateUpdate } from './user-voice-state.rules'; + +describe('mergeVoiceStateUpdate', () => { + it('clears mute and deafen flags when a user disconnects from voice', () => { + const next = mergeVoiceStateUpdate( + { + isConnected: true, + isMuted: true, + isDeafened: true, + isSpeaking: true, + roomId: 'voice-1', + serverId: 'server-1' + }, + { isConnected: false } + ); + + expect(next).toMatchObject({ + isConnected: false, + isMuted: false, + isDeafened: false, + isSpeaking: false + }); + }); + + it('does not carry stale mute state into a fresh voice connection', () => { + const next = mergeVoiceStateUpdate( + { + isConnected: false, + isMuted: true, + isDeafened: true, + isSpeaking: false, + roomId: undefined, + serverId: undefined + }, + { + isConnected: true, + roomId: 'voice-1', + serverId: 'server-1' + } + ); + + expect(next).toMatchObject({ + isConnected: true, + isMuted: false, + isDeafened: false, + isSpeaking: false, + roomId: 'voice-1', + serverId: 'server-1' + }); + }); + + it('honors an explicit mute flag on reconnect when provided', () => { + const next = mergeVoiceStateUpdate( + { + isConnected: false, + isMuted: true, + isDeafened: false, + isSpeaking: false + }, + { + isConnected: true, + isMuted: true, + roomId: 'voice-1', + serverId: 'server-1' + } + ); + + expect(next.isMuted).toBe(true); + }); + + it('preserves mute toggles during an active voice session', () => { + const next = mergeVoiceStateUpdate( + { + isConnected: true, + isMuted: false, + isDeafened: false, + isSpeaking: false, + roomId: 'voice-1', + serverId: 'server-1' + }, + { isMuted: true } + ); + + expect(next.isMuted).toBe(true); + expect(next.isConnected).toBe(true); + }); +}); diff --git a/toju-app/src/app/store/users/user-voice-state.rules.ts b/toju-app/src/app/store/users/user-voice-state.rules.ts new file mode 100644 index 0000000..d39fa75 --- /dev/null +++ b/toju-app/src/app/store/users/user-voice-state.rules.ts @@ -0,0 +1,51 @@ +import type { VoiceState } from '../../shared-kernel'; + +const DEFAULT_VOICE_STATE: VoiceState = { + isConnected: false, + isMuted: false, + isDeafened: false, + isSpeaking: false +}; + +/** Merge a partial voice-state patch without leaking stale mute/deafen flags across sessions. */ +export function mergeVoiceStateUpdate( + previous: VoiceState | undefined, + update: Partial +): VoiceState { + const prev = previous ?? DEFAULT_VOICE_STATE; + const hasRoomId = Object.prototype.hasOwnProperty.call(update, 'roomId'); + const hasServerId = Object.prototype.hasOwnProperty.call(update, 'serverId'); + const hasClientInstanceId = Object.prototype.hasOwnProperty.call(update, 'clientInstanceId'); + const hasIsMuted = Object.prototype.hasOwnProperty.call(update, 'isMuted'); + const hasIsDeafened = Object.prototype.hasOwnProperty.call(update, 'isDeafened'); + const hasIsSpeaking = Object.prototype.hasOwnProperty.call(update, 'isSpeaking'); + const nextConnected = update.isConnected ?? prev.isConnected; + const isDisconnecting = update.isConnected === false; + const isReconnecting = nextConnected === true && prev.isConnected === false; + const resolveSessionFlag = ( + key: 'isMuted' | 'isDeafened' | 'isSpeaking', + hasExplicit: boolean + ): boolean => { + if (isDisconnecting) { + return false; + } + + if (isReconnecting) { + return hasExplicit ? update[key] === true : false; + } + + return update[key] ?? prev[key]; + }; + + return { + isConnected: nextConnected, + isMuted: resolveSessionFlag('isMuted', hasIsMuted), + isDeafened: resolveSessionFlag('isDeafened', hasIsDeafened), + isSpeaking: resolveSessionFlag('isSpeaking', hasIsSpeaking), + isMutedByAdmin: update.isMutedByAdmin ?? prev.isMutedByAdmin, + volume: update.volume ?? prev.volume, + roomId: hasRoomId ? update.roomId : prev.roomId, + serverId: hasServerId ? update.serverId : prev.serverId, + clientInstanceId: hasClientInstanceId ? update.clientInstanceId : prev.clientInstanceId + }; +} diff --git a/toju-app/src/app/store/users/users-status.reducer.spec.ts b/toju-app/src/app/store/users/users-status.reducer.spec.ts index 6030d80..1aafc29 100644 --- a/toju-app/src/app/store/users/users-status.reducer.spec.ts +++ b/toju-app/src/app/store/users/users-status.reducer.spec.ts @@ -373,8 +373,8 @@ describe('users reducer - status', () => { presenceServerIds: ['s1'], voiceState: { isConnected: true, - isMuted: false, - isDeafened: false, + isMuted: true, + isDeafened: true, isSpeaking: true, roomId: 'voice-1', serverId: 's1' @@ -390,6 +390,8 @@ describe('users reducer - status', () => { expect(state.entities['u6']?.presenceServerIds).toBeUndefined(); expect(state.entities['u6']?.isOnline).toBe(false); expect(state.entities['u6']?.voiceState?.isConnected).toBe(false); + expect(state.entities['u6']?.voiceState?.isMuted).toBe(false); + expect(state.entities['u6']?.voiceState?.isDeafened).toBe(false); expect(state.entities['u6']?.voiceState?.roomId).toBeUndefined(); }); }); diff --git a/toju-app/src/app/store/users/users.reducer.ts b/toju-app/src/app/store/users/users.reducer.ts index 08d15a3..6201ec8 100644 --- a/toju-app/src/app/store/users/users.reducer.ts +++ b/toju-app/src/app/store/users/users.reducer.ts @@ -10,6 +10,7 @@ import { UserStatus } from '../../shared-kernel'; import { UsersActions } from './users.actions'; +import { mergeVoiceStateUpdate } from './user-voice-state.rules'; function normalizePresenceServerIds(serverIds: readonly string[] | undefined): string[] | undefined { if (!Array.isArray(serverIds)) { @@ -148,7 +149,8 @@ function buildDisconnectedVoiceState(user: User): User['voiceState'] { isDeafened: false, isSpeaking: false, roomId: undefined, - serverId: undefined + serverId: undefined, + clientInstanceId: undefined }; } @@ -510,37 +512,17 @@ export const usersReducer = createReducer( state ) ), - on(UsersActions.updateVoiceState, (state, { userId, voiceState }) => { - const prev = state.entities[userId]?.voiceState || { - isConnected: false, - isMuted: false, - isDeafened: false, - isSpeaking: false - }; - const hasRoomId = Object.prototype.hasOwnProperty.call(voiceState, 'roomId'); - const hasServerId = Object.prototype.hasOwnProperty.call(voiceState, 'serverId'); - const hasClientInstanceId = Object.prototype.hasOwnProperty.call(voiceState, 'clientInstanceId'); - - return usersAdapter.updateOne( + on(UsersActions.updateVoiceState, (state, { userId, voiceState }) => + usersAdapter.updateOne( { id: userId, changes: { - voiceState: { - isConnected: voiceState.isConnected ?? prev.isConnected, - isMuted: voiceState.isMuted ?? prev.isMuted, - isDeafened: voiceState.isDeafened ?? prev.isDeafened, - isSpeaking: voiceState.isSpeaking ?? prev.isSpeaking, - isMutedByAdmin: voiceState.isMutedByAdmin ?? prev.isMutedByAdmin, - volume: voiceState.volume ?? prev.volume, - roomId: hasRoomId ? voiceState.roomId : prev.roomId, - serverId: hasServerId ? voiceState.serverId : prev.serverId, - clientInstanceId: hasClientInstanceId ? voiceState.clientInstanceId : prev.clientInstanceId - } + voiceState: mergeVoiceStateUpdate(state.entities[userId]?.voiceState, voiceState) } }, state - ); - }), + ) + ), on(UsersActions.updateScreenShareState, (state, { userId, screenShareState }) => { const prev = state.entities[userId]?.screenShareState || { isSharing: false