From 1e59e7012c050544c0114e41baa401e795965f35 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:38:31 +0200 Subject: [PATCH] fix(server): scope accept-rollback terminal to the acceptor only Code-review catch: the Path-2 accept-rollback previously emitted dm_call_undeliverable { terminal: true } via sendToFederatedCallUsers, which broadcasts to every ringedUserIds entry. In a group DM this would prematurely tear down non-accepting ringees whose own accept / reject / timeout paths should govern their state. Switch to sendToUser(acceptorId) so only the acting user gets the terminal signal. Reorder the clearFederatedCall to happen before the emit so a concurrent end-handler sees a cleared entry (clearFederatedCall is idempotent). Spec updated, test extended to assert the scoping with a two-ringee group-DM fixture. --- .../server/src/ws/events.dmCallRelay.test.ts | 31 ++++++++++++++----- packages/server/src/ws/events.ts | 8 +++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/packages/server/src/ws/events.dmCallRelay.test.ts b/packages/server/src/ws/events.dmCallRelay.test.ts index a1b08450..26547070 100644 --- a/packages/server/src/ws/events.dmCallRelay.test.ts +++ b/packages/server/src/ws/events.dmCallRelay.test.ts @@ -234,13 +234,19 @@ describe('ring-timeout fan-out hook', () => { }); describe('handleDmCallAccept Path-2 relay failure', () => { - it('emits dm_call_undeliverable { phase:"accept", terminal:true } and clears the fedCall when the relay fails', async () => { + it('emits dm_call_undeliverable { phase:"accept", terminal:true } to the acceptor ONLY and clears the fedCall when the relay fails', async () => { const { handleDmCallAcceptForTest } = await importSUT(); const connectionManager = await importManager(); - const fedCall = makeFedCall(); + const fedCall = makeFedCall({ + // Two ringed users — a group DM where only one accepts. Only the acceptor + // should receive the terminal undeliverable; the other ringee must stay + // in ring state so their own timeout/reject path governs teardown. + ringedUserIds: ['acceptor-user', 'other-ringee'], + }); connectionManager.createFederatedCall(fedCall); - const sendToUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers'); + const sendToUserSpy = vi.spyOn(connectionManager, 'sendToUser'); + const sendToCallUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers'); sendCallRelayMock.mockResolvedValue({ ok: false, reason: 'peer_transient_failure', error: 'timeout' }); await handleDmCallAcceptForTest( @@ -250,14 +256,23 @@ describe('handleDmCallAccept Path-2 relay failure', () => { ); expect(connectionManager.getFederatedCall(fedCall.federatedId)).toBeUndefined(); - const calls = sendToUsersSpy.mock.calls.filter(([, ev]) => + + // Terminal undeliverable went to the acceptor only. + const undelivCalls = sendToUserSpy.mock.calls.filter(([, ev]) => (ev as { type: string }).type === 'dm_call_undeliverable', ); - expect(calls).toHaveLength(1); - const undeliverable = calls[0]![1] as { phase: string; terminal: boolean; failures: unknown[] }; + expect(undelivCalls).toHaveLength(1); + expect(undelivCalls[0]![0]).toBe('acceptor-user'); + const undeliverable = undelivCalls[0]![1] as { phase: string; terminal: boolean; failures: unknown[] }; expect(undeliverable.phase).toBe('accept'); expect(undeliverable.terminal).toBe(true); expect(undeliverable.failures).toHaveLength(1); + + // No undeliverable broadcast to all ringed users. + const callUsersCalls = sendToCallUsersSpy.mock.calls.filter(([, ev]) => + (ev as { type: string }).type === 'dm_call_undeliverable', + ); + expect(callUsersCalls).toHaveLength(0); }); it('does not emit undeliverable on relay success', async () => { @@ -266,7 +281,7 @@ describe('handleDmCallAccept Path-2 relay failure', () => { const fedCall = makeFedCall(); connectionManager.createFederatedCall(fedCall); - const sendToUsersSpy = vi.spyOn(connectionManager, 'sendToFederatedCallUsers'); + const sendToUserSpy = vi.spyOn(connectionManager, 'sendToUser'); sendCallRelayMock.mockResolvedValue({ ok: true }); await handleDmCallAcceptForTest( @@ -275,7 +290,7 @@ describe('handleDmCallAccept Path-2 relay failure', () => { {} as never, ); - const undelivCalls = sendToUsersSpy.mock.calls.filter(([, ev]) => + const undelivCalls = sendToUserSpy.mock.calls.filter(([, ev]) => (ev as { type: string }).type === 'dm_call_undeliverable', ); expect(undelivCalls).toHaveLength(0); diff --git a/packages/server/src/ws/events.ts b/packages/server/src/ws/events.ts index 4785d207..61955143 100644 --- a/packages/server/src/ws/events.ts +++ b/packages/server/src/ws/events.ts @@ -1563,7 +1563,12 @@ async function handleDmCallAccept(event: Record, userId: string if (!result.ok) { console.error(`[federation] dm_call_accept relay to ${fedCall.federatedCallHost} failed (${result.reason}): ${result.error}`); const failure = buildFailureFromResult(result, fedCall.federatedCallHost, db); - connectionManager.sendToFederatedCallUsers(fedCall.federatedId, { + // Clear first so a concurrent end-handler sees a cleared entry (idempotent). + connectionManager.clearFederatedCall(fedCall.federatedId); + // Terminal targets ONLY the acceptor — other ringed users (group DM) didn't + // accept and should stay in their ring state; their own dm_call_end / timeout + // paths govern their teardown. + connectionManager.sendToUser(userId, { type: 'dm_call_undeliverable', dmChannelId: fedCall.dmChannelId, federatedCallId: fedCall.federatedId, @@ -1571,7 +1576,6 @@ async function handleDmCallAccept(event: Record, userId: string phase: 'accept', failures: [failure], }); - connectionManager.clearFederatedCall(fedCall.federatedId); } return; }