From 822718e687f7680986a5dd8b85aa9fd749fecedf Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 19:35:21 +0000 Subject: [PATCH] fix(call): stop ringing on sibling devices without disturbing the one that answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user is logged in on two devices and receives a call, both ring. When one picks up, the other should stop ringing immediately — but crucially, the device that answered must not lose its WebRTC connection when the silenced device reacts. The "answered elsewhere" state machine (onCallAnswered at the self- pubkey branch) was already in place and covered by a unit test, but the signal it relies on was never actually published: acceptCall wrapped the CallAnswerEvent only for `groupMembers - signer.pubKey` so in a 1:1 call it only reached the caller. Sibling devices subscribed to gift wraps for their own pubkey never saw it and kept ringing until the 60 s local timeout. Fixes: - acceptCall / rejectCall publish an extra gift wrap of the *same* signed answer/reject event addressed to `signer.pubKey`, so sibling devices in IncomingCall observe the echo and transition to Ended(ANSWERED_ELSEWHERE / REJECTED) locally. Neither path publishes any further signaling, so the device that picked up is never disturbed. - onCallRejected: add a self-pubkey early-return mirroring onCallAnswered. Without it, a self-reject echoing back to a device already in Connecting/Connected hit the peer-rejection branches, firing onPeerLeft(signer.pubKey) — which would tell CallController to dispose its own PeerSession and tear down the live audio. - onSignalingEvent: record self-answer call-ids in completedCallIds alongside hangups and rejects, so an out-of-order relay replay delivering the self-answer before the original offer does not make a sibling device start ringing for a call it already knows was answered. Adds multi-device tests covering: the new self-addressed wraps in acceptCall/rejectCall, the end-to-end "two phones, one answers" scenario, the Connected / Connecting self-reject echo guards, and the out-of-order offer-after-self-answer replay case. https://claude.ai/code/session_01QVUnhr79hYqQuXFiEb8puk --- .../amethyst/commons/call/CallManager.kt | 65 ++++-- .../amethyst/commons/call/CallManagerTest.kt | 191 ++++++++++++++++++ 2 files changed, 242 insertions(+), 14 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 117ec5134..3ebd74fc8 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 @@ -386,6 +386,20 @@ class CallManager( Log.d("CallManager") { "acceptCall: publishing answer to ${otherMembers.size} recipients" } val result = factory.createGroupCallAnswer(sdpAnswer, otherMembers, current.callId, signer) result.wraps.forEach { publishEvent(it) } + + // Notify our own other devices so they stop ringing. We wrap the + // *same* signed answer event for our own pubkey and publish it as + // an extra gift wrap. A sibling device in IncomingCall will observe + // the self-answer and transition to Ended(ANSWERED_ELSEWHERE) + // locally (see onCallAnswered). This call is the only place the + // "answered elsewhere" signal is generated — without it, a second + // logged-in device keeps ringing until its 60-second local timeout. + // + // The echo that arrives back at *this* device is ignored in + // Connecting/Connected state by the self-answer guard at the top + // of onCallAnswered, so it cannot disturb the live call. + val selfWrap = EphemeralGiftWrapEvent.create(event = result.msg, recipientPubKey = signer.pubKey) + publishEvent(selfWrap) Log.d("CallManager") { "acceptCall: answer published, now in Connecting state" } // Trigger callee-to-callee mesh connections with peers we discovered @@ -410,6 +424,12 @@ class CallManager( val otherMembers = current.groupMembers - signer.pubKey val result = factory.createGroupReject(otherMembers, current.callId, signer = signer) result.wraps.forEach { publishEvent(it) } + + // Notify our own other devices so they stop ringing. See the + // matching comment in acceptCall — sibling devices in IncomingCall + // pick up this echo and transition to Ended(REJECTED) locally. + val selfWrap = EphemeralGiftWrapEvent.create(event = result.msg, recipientPubKey = signer.pubKey) + publishEvent(selfWrap) } private fun onCallAnswered(event: CallAnswerEvent) { @@ -534,6 +554,23 @@ class CallManager( val callId = event.callId() val rejectingPeer = event.pubKey + // Self-reject: only meaningful as "rejected elsewhere" while we are + // still ringing (IncomingCall). In every other state it is our own + // echo from a relay after a sibling device rejected a call that we + // have already accepted and are now actively in — treating it as a + // peer rejection would drop ourselves from the call (onPeerLeft + // would dispose our own PeerConnection, killing the live audio on + // the device that picked up). Just drop the echo. + if (rejectingPeer == signer.pubKey) { + if (current is CallState.IncomingCall && callId == current.callId) { + Log.d("CallManager") { "onCallRejected: self-reject detected in IncomingCall — rejected elsewhere" } + transitionToEnded(current.callId, current.peerPubKeys(), EndReason.REJECTED) + } else { + Log.d("CallManager") { "onCallRejected: ignoring self-reject echo in ${current::class.simpleName}" } + } + return + } + when (current) { is CallState.Offering -> { if (callId != current.callId) return @@ -565,10 +602,7 @@ class CallManager( is CallState.IncomingCall -> { if (callId != current.callId) return - if (rejectingPeer == signer.pubKey) { - // Own rejection from another device - transitionToEnded(current.callId, current.peerPubKeys(), EndReason.REJECTED) - } else if (rejectingPeer == current.callerPubKey) { + if (rejectingPeer == current.callerPubKey) { // Caller rejected/cancelled the call transitionToEnded(current.callId, current.groupMembers, EndReason.PEER_REJECTED) } else { @@ -860,17 +894,20 @@ class CallManager( // Record call-ids from termination signals so that a later offer // for the same call is recognised as stale (common after app restart - // when relay replays events out of order). - if (event is CallHangupEvent || event is CallRejectEvent) { - val terminatedCallId = - when (event) { - is CallHangupEvent -> event.callId() - is CallRejectEvent -> event.callId() - else -> null - } - if (terminatedCallId != null) { - cappedAdd(completedCallIds, terminatedCallId, MAX_COMPLETED_CALL_IDS) + // when relay replays events out of order). For answers, only + // *self*-answers count as a termination signal: they mean another + // of our devices picked up, so ringing on this device should never + // start for the same call-id. Peer answers to our own offer are + // not terminal — they are what moves us into Connecting. + val terminatedCallId = + when (event) { + is CallHangupEvent -> event.callId() + is CallRejectEvent -> event.callId() + is CallAnswerEvent -> if (event.pubKey == signer.pubKey) event.callId() else null + else -> null } + if (terminatedCallId != null) { + cappedAdd(completedCallIds, terminatedCallId, MAX_COMPLETED_CALL_IDS) } when (event) { 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 6097eb700..463dab25a 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 @@ -1830,4 +1830,195 @@ class CallManagerTest { assertIs(manager.state.value) } + + // ======================================================================== + // Multi-Device: Second Logged-In Device Must Stop Ringing + // ======================================================================== + // When the user is logged in on two phones and Alice calls them, both + // devices ring. When one picks up (or rejects), the other must stop + // ringing without disturbing the device that answered. + // + // Mechanism: acceptCall / rejectCall publish an extra gift wrap of the + // same signed answer/reject event addressed to the user's own pubkey, + // so a sibling device (subscribed to gift wraps for its own pubkey) + // observes the echo, detects a self-answer/self-reject in IncomingCall + // state and transitions to Ended(ANSWERED_ELSEWHERE / REJECTED). + + private fun EphemeralGiftWrapEvent.recipientPubKey(): HexKey? = + tags.firstOrNull { it.size >= 2 && it[0] == "p" }?.get(1) + + @Test + fun acceptCallPublishesAnswerWrappedForSelfSoSiblingDeviceCanStopRinging() = + runTest { + val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + assertIs(manager.state.value) + published.clear() + + manager.acceptCall(sdpAnswer) + + val recipients = published.mapNotNull { it.recipientPubKey() }.toSet() + assertTrue( + alice in recipients, + "Answer must be wrapped for the caller (alice), recipients=$recipients", + ) + assertTrue( + bob in recipients, + "Answer must also be wrapped for the callee's own pubkey (bob) so " + + "sibling devices stop ringing, recipients=$recipients", + ) + } + + @Test + fun rejectCallPublishesRejectWrappedForSelfSoSiblingDeviceCanStopRinging() = + runTest { + val (manager, published) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + assertIs(manager.state.value) + published.clear() + + manager.rejectCall() + + val recipients = published.mapNotNull { it.recipientPubKey() }.toSet() + assertTrue( + alice in recipients, + "Reject must be wrapped for the caller (alice), recipients=$recipients", + ) + assertTrue( + bob in recipients, + "Reject must also be wrapped for the callee's own pubkey (bob) so " + + "sibling devices stop ringing, recipients=$recipients", + ) + } + + /** + * Simulates the full multi-device scenario: + * 1. Alice calls Bob. Bob is logged in on two phones (two separate + * CallManagers backed by the same bobSigner). + * 2. Both phones receive the offer and enter IncomingCall. + * 3. Phone 1 accepts — this publishes an answer gift-wrapped for + * alice AND for bob. + * 4. Phone 2 (subscribed to its own pubkey) receives the self- + * addressed echo and must transition to Ended(ANSWERED_ELSEWHERE). + * 5. Phone 1 must remain in Connecting — the echo it also receives + * must not disturb its in-progress session. + */ + @Test + fun answeringOnDeviceOneStopsDeviceTwoRingingWithoutDisturbingDeviceOne() = + runTest { + val (phone1, published1) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + val (phone2, _) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + val offer = makeOffer(from = alice, to = bob) + phone1.onSignalingEvent(offer) + phone2.onSignalingEvent(offer) + assertIs(phone1.state.value) + assertIs(phone2.state.value) + published1.clear() + + // Phone 1 picks up. It will publish one wrap for alice and one + // wrap for bob (the self-echo). + phone1.acceptCall(sdpAnswer) + assertIs(phone1.state.value) + + // Deliver the self-echo to phone 2. The inner event is what + // another CallManager would see after unwrapping the gift wrap, + // so construct a parallel CallAnswerEvent directly (the wrap + // payload is opaque in the test harness because published1 + // stores EphemeralGiftWrapEvents, not the unwrapped inner). + val selfAnswer = makeAnswer(from = bob, to = alice) + phone2.onSignalingEvent(selfAnswer) + + // Phone 2 stops ringing with ANSWERED_ELSEWHERE. + val phone2State = phone2.state.value + assertIs(phone2State) + assertEquals(EndReason.ANSWERED_ELSEWHERE, phone2State.reason) + + // Phone 1 ALSO receives the self-echo (it is addressed to bob). + // It must stay in Connecting — the live call cannot be torn down + // by its own answer bouncing off the relay. + phone1.onSignalingEvent(selfAnswer) + assertIs(phone1.state.value) + } + + /** + * Regression for the subtle bug where a self-reject echo would trigger + * onPeerLeft(signer.pubKey) on a device that had already accepted, + * disposing its own PeerSession and killing the live call. + */ + @Test + fun selfRejectEchoDoesNotDisturbDeviceAlreadyInCall() = + runTest { + val (manager, _) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + var peerLeftCalled = false + manager.onPeerLeft = { peerLeftCalled = true } + + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + manager.acceptCall(sdpAnswer) + manager.onPeerConnected() + assertIs(manager.state.value) + + // A sibling device rejects the call after we already answered. + // Its reject event is wrapped for us too (multi-device signal). + val siblingReject = makeReject(from = bob, to = alice) + manager.onSignalingEvent(siblingReject) + + // Our live call must survive — the echo must not fire + // onPeerLeft (which would dispose our own PeerSession) and must + // not change state. + assertIs(manager.state.value) + assertTrue( + !peerLeftCalled, + "onPeerLeft must not fire for a self-reject echo — the UI would " + + "tear down our own PeerSession, killing the live call audio.", + ) + } + + /** + * Same guard for the Connecting state: a self-reject arriving during + * the connection handshake must not drop us from our own pending set + * and must not invoke onPeerLeft(self). + */ + @Test + fun selfRejectEchoDuringConnectingDoesNotFireOnPeerLeft() = + runTest { + val (manager, _) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + var peerLeftCalled = false + manager.onPeerLeft = { peerLeftCalled = true } + + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + manager.acceptCall(sdpAnswer) + assertIs(manager.state.value) + + val siblingReject = makeReject(from = bob, to = alice) + manager.onSignalingEvent(siblingReject) + + assertIs(manager.state.value) + assertTrue(!peerLeftCalled, "onPeerLeft must not fire for a self-reject echo during Connecting") + } + + /** + * If the relay replays events out of order — self-answer arriving + * before the original offer — the second device must NOT start ringing + * for a call-id it already knows was answered elsewhere. + */ + @Test + fun offerReplayedAfterSelfAnswerDoesNotStartRinging() = + runTest { + val (manager, _) = createManager(localPubKey = bob, followedKeys = setOf(alice)) + + // Self-answer arrives first (while we're in Idle — nothing to + // do with it except remember the call-id so a later offer for + // it is treated as stale). + manager.onSignalingEvent(makeAnswer(from = bob, to = alice)) + assertIs(manager.state.value) + + // Now the offer finally arrives. We must NOT start ringing. + manager.onSignalingEvent(makeOffer(from = alice, to = bob)) + assertIs(manager.state.value) + } }