From 3a5c118d3565bfed5af7e03bb0fa9bf66e6f48d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 16:30:35 +0000 Subject: [PATCH] fix: end call when the last connected peer hangs up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scenario: Alice calls {bob, carol} as a group. Bob answers, Bob and Alice are talking, Carol never joins. Before Carol's 30 s watchdog fires, Alice hangs up. Bob's state had peerPubKeys={alice} and pendingPeerPubKeys={carol}. The old onPeerHangup handler only ended the call when BOTH sets became empty, so Bob stayed in Connected alone staring at a black screen until Carol's watchdog finally dropped her — and even then the state machine didn't terminate because "no peers, no pending" wasn't true at the exact same step. Change the termination rule in onPeerHangup (both Connected and Connecting states) to end the call as soon as connectedRemaining becomes empty, regardless of how many pending peers are still tracked. Without a single connected peer the call has no one to talk to and can't meaningfully continue. Also publish a CallHangup to any peers in the pending set that WE personally invited (peersInvitedByUs ∩ pendingPeerPubKeys) so their devices stop ringing. Peers we did not invite (e.g. group members the caller originally included) are the caller's responsibility — the caller's own group hangup already reaches them — so we leave them alone. Adds three regression tests covering: - Connected state: caller hangs up while one pending member remains - Connecting state: same, before onPeerConnected fires - Callee had invited someone mid-call: call ends AND Bob notifies his invitee to stop ringing --- .../amethyst/commons/call/CallManager.kt | 49 ++++++++- .../amethyst/commons/call/CallManagerTest.kt | 102 ++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt index 2f958704e..117ec5134 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt @@ -719,7 +719,7 @@ class CallManager( result.wraps.forEach { publishEvent(it) } } - private fun onPeerHangup(event: CallHangupEvent) { + private suspend fun onPeerHangup(event: CallHangupEvent) { val current = _state.value val callId = event.callId() ?: return val leavingPeer = event.pubKey @@ -732,8 +732,17 @@ class CallManager( cancelPeerTimeout(leavingPeer) val connectedRemaining = current.peerPubKeys - leavingPeer val pendingRemaining = current.pendingPeerPubKeys - leavingPeer - if (connectedRemaining.isEmpty() && pendingRemaining.isEmpty()) { - Log.d("CallManager") { "onPeerHangup: last peer left, ending call" } + if (connectedRemaining.isEmpty()) { + // No connected peers left. Pending members we never + // heard from can't sustain the call on their own, so + // terminate — otherwise the caller (the only other + // participant we were actually talking to) has hung up + // and we'd be left staring at a black screen waiting + // for someone who may never join. + Log.d("CallManager") { + "onPeerHangup: last connected peer left, ending call (pendingRemaining=${pendingRemaining.size})" + } + publishHangupToInvitedPendingPeers(callId, pendingRemaining) transitionToEnded(callId, current.allPeerPubKeys, EndReason.PEER_HANGUP) } else { Log.d("CallManager") { "onPeerHangup: ${leavingPeer.take(8)} left, remaining=${connectedRemaining.map { it.take(8) }}, pending=${pendingRemaining.map { it.take(8) }}" } @@ -751,8 +760,14 @@ class CallManager( cancelPeerTimeout(leavingPeer) val connectedRemaining = current.peerPubKeys - leavingPeer val pendingRemaining = current.pendingPeerPubKeys - leavingPeer - if (connectedRemaining.isEmpty() && pendingRemaining.isEmpty()) { - Log.d("CallManager") { "onPeerHangup: last peer left during connecting, ending call" } + if (connectedRemaining.isEmpty()) { + // Same rule as Connected: without a single connected + // peer the call can't keep running. Pending members + // are dropped regardless of how many are left. + Log.d("CallManager") { + "onPeerHangup: last connected peer left during connecting, ending call (pendingRemaining=${pendingRemaining.size})" + } + publishHangupToInvitedPendingPeers(callId, pendingRemaining) transitionToEnded(callId, current.peerPubKeys + current.pendingPeerPubKeys, EndReason.PEER_HANGUP) } else { Log.d("CallManager") { "onPeerHangup: ${leavingPeer.take(8)} left during connecting, remaining=${connectedRemaining.map { it.take(8) }}" } @@ -797,6 +812,30 @@ class CallManager( } } + /** + * When a call is ending because its last connected peer left, publish + * a CallHangup to every *pending* peer we had personally invited so + * their devices stop ringing. Peers we did NOT invite (e.g. callee-side + * group members) are the caller's responsibility and are left alone. + * + * Must be called while still inside the CallManager state mutex and + * BEFORE [transitionToEnded], which clears [peersInvitedByUs]. + */ + private suspend fun publishHangupToInvitedPendingPeers( + callId: String, + pendingPeers: Set, + ) { + val invitedPending = peersInvitedByUs.intersect(pendingPeers) + if (invitedPending.isEmpty()) return + Log.d("CallManager") { + "publishHangupToInvitedPendingPeers: notifying ${invitedPending.size} invited pending peers to stop ringing" + } + for (peer in invitedPending) { + val result = factory.createHangup(peer, callId, signer = signer) + publishEvent(result.wrap) + } + } + suspend fun onSignalingEvent(event: Event) { if (isEventTooOld(event)) { Log.d("CallManager") { "Discarding old event kind=${event.kind} age=${TimeUtils.now() - event.createdAt}s" } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/call/CallManagerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/call/CallManagerTest.kt index f9da7138a..6097eb700 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/call/CallManagerTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/call/CallManagerTest.kt @@ -1577,6 +1577,108 @@ class CallManagerTest { ) } + /** + * Regression: Alice calls {bob, carol} as a group, Bob accepts and + * talks to Alice, Carol never joins. Alice hangs up before Carol's + * watchdog timer fires. Bob must end the call immediately — NOT stay + * in a Connected state alone waiting for Carol. + */ + @Test + fun callerHangupEndsCalleeCallEvenWhenPendingPeersRemain() = + runTest { + val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol)) + + // Alice calls {bob, carol}. Bob accepts and reaches Connected. + manager.onSignalingEvent(makeGroupOffer(from = alice, members = setOf(bob, carol))) + manager.acceptCall(sdpAnswer) + manager.onPeerConnected() + val connected = manager.state.value + assertIs(connected) + assertTrue(alice in connected.peerPubKeys, "Alice must be the sole connected peer") + assertTrue(carol in connected.pendingPeerPubKeys, "Carol must still be pending") + published.clear() + + // Alice hangs up well before Carol's 30 s watchdog fires. + manager.onSignalingEvent(makeHangup(from = alice, to = bob)) + + // The call must terminate immediately — Bob has nobody left + // to talk to, so staying in Connected alone is wrong. + val after = manager.state.value + assertIs(after) + assertEquals(EndReason.PEER_HANGUP, after.reason) + + // Bob must not publish any hangup: Carol's ringing is Alice's + // responsibility (Alice already broadcast her hangup to Carol + // as part of her own group hangup). + assertTrue( + published.isEmpty(), + "Callee must not publish anything when ending due to caller hangup", + ) + } + + /** + * Regression: when Bob is alone in a Connecting state (haven't reached + * Connected yet) and the caller hangs up, the call must end rather + * than lingering in Connecting with an empty peerPubKeys until the + * watchdog fires. + */ + @Test + fun callerHangupEndsCalleeCallInConnectingEvenWhenPendingPeersRemain() = + runTest { + val (manager, _) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol)) + + manager.onSignalingEvent(makeGroupOffer(from = alice, members = setOf(bob, carol))) + manager.acceptCall(sdpAnswer) + // Intentionally NOT calling onPeerConnected — we want to stay in Connecting. + assertIs(manager.state.value) + + manager.onSignalingEvent(makeHangup(from = alice, to = bob)) + + val after = manager.state.value + assertIs(after) + assertEquals(EndReason.PEER_HANGUP, after.reason) + } + + /** + * Regression: if Bob is in a Connected group call with Alice and has + * personally invited Dave mid-call, and then Alice hangs up, the call + * ends AND Bob publishes a hangup to Dave (who is still ringing on + * Bob's invitation) so Dave's device stops ringing. + */ + @Test + fun callerHangupEndsCallAndNotifiesInvitedPendingPeers() = + runTest { + val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice, carol)) + + // Alice ↔ Bob Connected (1-1). + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + manager.acceptCall(sdpAnswer) + manager.onPeerConnected() + assertIs(manager.state.value) + + // Bob invites Carol mid-call — Bob becomes Carol's inviter. + manager.invitePeer(carol, "bob-to-carol-sdp") + val afterInvite = manager.state.value + assertIs(afterInvite) + assertTrue(carol in afterInvite.pendingPeerPubKeys) + published.clear() + + // Alice hangs up before Carol accepts Bob's invite. + manager.onSignalingEvent(makeHangup(from = alice, to = bob)) + + val after = manager.state.value + assertIs(after) + assertEquals(EndReason.PEER_HANGUP, after.reason) + + // Bob must publish a hangup to Carol (his invitee) so her + // device stops ringing. + assertEquals( + 1, + published.size, + "Must publish exactly one hangup to the invited-but-pending peer", + ) + } + /** * When a group offer is made, each callee gets its own timer. If NEITHER * answers, both timers fire in turn and the whole call ends with