feat: multi call handling#2163
Merged
greenfrvr merged 14 commits intofeat/callkit-telecom-integrationfrom Mar 20, 2026
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes refactor the calling SDK to support multiple simultaneous calls, replacing single-call state tracking with per-call management using maps and callId-based lookups. Repositories, services, and notification handling are updated to operate on a per-call basis. The isServiceStarted API is removed across all platforms. Changes
Sequence DiagramsequenceDiagram
participant Client
participant CallService
participant CallRepository
participant NotificationManager
participant TelecomRepository
Note over Client,TelecomRepository: Multi-Call Registration Flow (New)
Client->>CallService: registerCall(callId1, callInfo1)
CallService->>CallRepository: addCall(callId1, call)
CallRepository->>CallRepository: updateCallById(callId1, pendingCall)
Client->>CallService: registerCall(callId2, callInfo2)
CallService->>CallRepository: addCall(callId2, call)
CallRepository->>CallRepository: updateCallById(callId2, pendingCall)
TelecomRepository->>CallRepository: updateCallById(callId1, activeCall)
CallRepository->>NotificationManager: updateCallNotification(callId1, activeCall)
NotificationManager->>NotificationManager: getOrCreateNotificationId(callId1)
NotificationManager->>NotificationManager: postNotification(callId1, notification)
CallService->>CallService: startForegroundSafely(notificationId1, notification)
TelecomRepository->>CallRepository: updateCallById(callId2, activeCall)
CallRepository->>NotificationManager: updateCallNotification(callId2, activeCall)
NotificationManager->>NotificationManager: getOrCreateNotificationId(callId2)
NotificationManager->>NotificationManager: postNotification(callId2, notification)
Note over NotificationManager: Both calls tracked independently
Client->>CallService: endCall(callId1)
CallService->>NotificationManager: cancelNotification(callId1)
NotificationManager->>NotificationManager: repromoteForegroundIfNeeded()
CallRepository->>CallRepository: removeCall(callId1)
Note over Client,TelecomRepository: callId2 remains active
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
|
Contributor
Author
|
@coderabbitai review |
1 similar comment
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
1 similar comment
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.kt (1)
466-483: Clarify: Early return guards against duplicate registration requests.The check on lines 467-468 prevents re-registering a call that's already tracked (including pending calls). This is correct for handling duplicate intents (e.g., push notification retries), but consider adding a brief comment explaining this guards against duplicate registration attempts.
📝 Optional documentation
- // If this specific call is already registered, just notify + // If this call is already tracked (pending or registered), avoid duplicate registration. + // This guards against duplicate push notifications or repeated intents. val existingCall = callRepository.getCall(callInfo.callId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.kt` around lines 466 - 483, Add a short clarifying comment above the duplicate-check block that explains this early return guards against duplicate registration attempts (e.g., retrying incoming intents or push notifications) so pending/already-tracked calls are not re-registered; annotate the block that uses callRepository.getCall(callInfo.callId) and returns after sending the appropriate broadcast via sendBroadcastEvent (CallingxModuleImpl.CALL_REGISTERED_INCOMING_ACTION / CALL_REGISTERED_ACTION) referencing existingCall and callInfo.callId to make intent handling rationale clear.packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/CallNotificationManager.kt (1)
165-175: Minor: Simplify chronometer timing lookup.Line 172 re-reads
notificationsState[callId]?.activeWhenimmediately after setting it on line 170. Since we know the value isnow, this could be simplified:♻️ Optional simplification
if (!state.hasBecameActive) { debugLog(TAG, "[notifications] createNotification: Setting when to current time for $callId") val now = System.currentTimeMillis() notificationsState[callId] = state.copy(activeWhen = now, hasBecameActive = true) + builder.setWhen(now) + } else { + builder.setWhen(state.activeWhen ?: System.currentTimeMillis()) } - builder.setWhen(notificationsState[callId]?.activeWhen ?: System.currentTimeMillis()) builder.setUsesChronometer(true) builder.setShowWhen(true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/CallNotificationManager.kt` around lines 165 - 175, In the call activation branch inside CallNotificationManager (the createNotification logic where call.isActive && optimisticState == OptimisticState.NONE), avoid re-reading notificationsState[callId]?.activeWhen immediately after setting it; when you set notificationsState[callId] = state.copy(activeWhen = now, hasBecameActive = true) use the local now value for builder.setWhen(now) and then call builder.setUsesChronometer(true)/setShowWhen(true) — this removes the redundant lookup and guarantees the exact timestamp is used.packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt (2)
34-45: Only dispatch changed call entries from the collector.This currently replays every registered call on any map update, so a mute/state change on one call retriggers downstream notification/foreground work for all other calls too. Since
previousCallsis already tracked, only emitonCallStateChangedfor added/changedcallIds plus removals.♻️ Minimal diff
calls.collect { currentCalls -> - // Notify about changes per call for ((callId, call) in currentCalls) { - _listener?.onCallStateChanged(callId, call) + if (previousCalls[callId] != call) { + _listener?.onCallStateChanged(callId, call) + } } for ((callId, _) in previousCalls) { if (!currentCalls.containsKey(callId)) { _listener?.onCallStateChanged(callId, Call.None) } } previousCalls = currentCalls }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt` around lines 34 - 45, The collector in calls.collect currently notifies _listener.onCallStateChanged for every entry in currentCalls on any update, causing unrelated calls to be retriggered; change the loop to compare currentCalls against previousCalls and only call _listener.onCallStateChanged for callIds that are new or whose Call value differs, and separately call _listener.onCallStateChanged(callId, Call.None) only for callIds present in previousCalls but missing in currentCalls; update the logic around previousCalls to still assign previousCalls = currentCalls (or a shallow copy) after processing so future diffs work correctly.
46-48: Re-throw coroutine cancellation instead of logging it as a failure.Both collectors catch
Exception, so the normalobserveCallsJob?.cancel()/scope.cancel()paths are logged as errors. HandleCancellationExceptionseparately and only log real failures.♻️ Minimal diff
- } catch (e: Exception) { + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: Exception) { Log.e(TAG, "[repository] setListener: Error collecting call state", e) } @@ - } catch (e: Exception) { + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: Exception) { Log.e(TAG, "[repository] registerCall: Error consuming actions for $callId", e) }Also applies to: 103-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt` around lines 46 - 48, The catch blocks in LegacyCallRepository (inside setListener and the other collector around the observeCallsJob logic) currently catch Exception and log CancellationException as an error; update these to handle coroutine cancellation properly by adding a specific catch for CancellationException that rethrows (or simply rethrowing if caught) before the general catch (e: Exception) so only real failures are logged in Log.e; locate the catch blocks in setListener and the other collector (the ones currently doing Log.e(TAG, "[repository] setListener: Error collecting call state", e) and the similar Log.e at lines ~103-105) and change the error handling to rethrow CancellationException then log other Exceptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/NotificationIntentFactory.kt`:
- Around line 17-19: The current requestCodeFor(callId: String, base: Int)
simply adds base to callId.hashCode(), which risks collisions and PendingIntent
aliasing; replace it with a deterministic composite hash of both base and callId
(for example generate bytes from the string "$base:$callId" and run a stable
hash like SHA-256 or UUID.nameUUIDFromBytes and then convert a slice to a
non-negative Int) so different (base, callId) pairs produce distinct request
codes; update all callers (e.g., NotificationIntentFactory.requestCodeFor and
the places in createNotificationIntent / createActionPendingIntent that use it)
to use the new implementation so PendingIntents are uniquely keyed.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt`:
- Around line 80-105: registerCall creates a Channel called actionSource stored
on Call.Registered and launches a collector (scope.launch ->
actionSource.consumeAsFlow().collect) but removeCall only deletes the call from
the map and leaves that channel/collector alive; modify removeCall to retrieve
the Call.Registered for the given callId and close its actionSource (or cancel a
stored per-call Job) immediately after removing it from _calls so the collector
spawned by processActionLegacy is terminated; ensure any close/cancel is safe
(check channel is not already closed) and that release() still cancels remaining
scope as a fallback.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt`:
- Around line 31-34: The CallActionFlags holder currently exposes mutable
Boolean vars which are not thread-safe even when stored in a ConcurrentHashMap;
replace isSelfAnswered and isSelfDisconnected with thread-safe atomics (e.g.,
java.util.concurrent.atomic.AtomicBoolean) declared as vals inside
CallActionFlags, update all write sites (the action-side locations that set
these flags) to call set(true/false), and update all read/reset sites in the
callback paths to use get() and set(false) accordingly so reads/writes are
atomic and race-free; also ensure CallActionFlags instances remain in the
ConcurrentHashMap as before and adjust imports/usages to compile.
- Around line 78-91: The release() method must acquire the same
registrationMutex used by registerCall() to prevent races where registerCall()
proceeds concurrently and adds an orphaned Telecom call; update release() to
lock registrationMutex around the disconnect loop, clearing _calls, actionFlags,
cancelling observeCallsJob, nulling _listener, and cancelling scope so release
is atomic with respect to registerCall()/callsManager.addCall(); additionally,
in the registerCall()/call-registration path (the region around lines 106-158)
ensure it checks a repository-released flag (set while holding registrationMutex
in release()) or re-checks under the same registrationMutex before calling
callsManager.addCall()/creating per-call CallControlScope to avoid creating
calls after release.
- Around line 132-153: The created action channel (actionSource =
Channel<CallAction>()) is rendezvous/unbuffered, which contradicts the comment
and can cause send() to suspend if actions are sent before the consumer starts;
change the channel instantiation to a buffered channel (e.g., actionSource =
Channel<CallAction>(Channel.UNLIMITED) or Channel.BUFFERED) where the
Call.Registered is created so actions are queued until the call scope starts
processing them (the change should be made where actionSource is assigned before
addCall and relates to Call.Registered, addCall, and actionFlags[callId]).
In `@packages/react-native-sdk/src/utils/push/internal/utils.ts`:
- Around line 134-139: The loop over activeCalls currently awaits
activeCall.leave(...) which, if it throws, prevents the subsequent
callFromPush.join() from running; modify the loop in the activeCalls iteration
(the block using activeCall.leave) to wrap each await activeCall.leave({ reason:
'cancel' }) in its own try-catch, log the error via logger.warn/error including
activeCall.cid, and continue to the next call so that failures to leave one call
do not block calling callFromPush.join() afterwards.
---
Nitpick comments:
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.kt`:
- Around line 466-483: Add a short clarifying comment above the duplicate-check
block that explains this early return guards against duplicate registration
attempts (e.g., retrying incoming intents or push notifications) so
pending/already-tracked calls are not re-registered; annotate the block that
uses callRepository.getCall(callInfo.callId) and returns after sending the
appropriate broadcast via sendBroadcastEvent
(CallingxModuleImpl.CALL_REGISTERED_INCOMING_ACTION / CALL_REGISTERED_ACTION)
referencing existingCall and callInfo.callId to make intent handling rationale
clear.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/CallNotificationManager.kt`:
- Around line 165-175: In the call activation branch inside
CallNotificationManager (the createNotification logic where call.isActive &&
optimisticState == OptimisticState.NONE), avoid re-reading
notificationsState[callId]?.activeWhen immediately after setting it; when you
set notificationsState[callId] = state.copy(activeWhen = now, hasBecameActive =
true) use the local now value for builder.setWhen(now) and then call
builder.setUsesChronometer(true)/setShowWhen(true) — this removes the redundant
lookup and guarantees the exact timestamp is used.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt`:
- Around line 34-45: The collector in calls.collect currently notifies
_listener.onCallStateChanged for every entry in currentCalls on any update,
causing unrelated calls to be retriggered; change the loop to compare
currentCalls against previousCalls and only call _listener.onCallStateChanged
for callIds that are new or whose Call value differs, and separately call
_listener.onCallStateChanged(callId, Call.None) only for callIds present in
previousCalls but missing in currentCalls; update the logic around previousCalls
to still assign previousCalls = currentCalls (or a shallow copy) after
processing so future diffs work correctly.
- Around line 46-48: The catch blocks in LegacyCallRepository (inside
setListener and the other collector around the observeCallsJob logic) currently
catch Exception and log CancellationException as an error; update these to
handle coroutine cancellation properly by adding a specific catch for
CancellationException that rethrows (or simply rethrowing if caught) before the
general catch (e: Exception) so only real failures are logged in Log.e; locate
the catch blocks in setListener and the other collector (the ones currently
doing Log.e(TAG, "[repository] setListener: Error collecting call state", e) and
the similar Log.e at lines ~103-105) and change the error handling to rethrow
CancellationException then log other Exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fefc53ad-9cfd-4ec8-a0a0-2f4cd0a0556d
📒 Files selected for processing (14)
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallRegistrationStore.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModuleImpl.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/model/Call.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/CallNotificationManager.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/notifications/NotificationIntentFactory.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/CallRepository.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.ktpackages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.ktpackages/react-native-callingx/android/src/newarch/java/io/getstream/rn/callingx/CallingxModule.ktpackages/react-native-callingx/android/src/oldarch/java/io/getstream/rn/callingx/CallingxModule.ktpackages/react-native-callingx/ios/Callingx.mmpackages/react-native-callingx/src/spec/NativeCallingx.tspackages/react-native-sdk/src/utils/push/internal/utils.ts
💤 Files with no reviewable changes (4)
- packages/react-native-callingx/android/src/newarch/java/io/getstream/rn/callingx/CallingxModule.kt
- packages/react-native-callingx/ios/Callingx.mm
- packages/react-native-callingx/src/spec/NativeCallingx.ts
- packages/react-native-callingx/android/src/oldarch/java/io/getstream/rn/callingx/CallingxModule.kt
...gx/android/src/main/java/io/getstream/rn/callingx/notifications/NotificationIntentFactory.kt
Show resolved
Hide resolved
...-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt
Show resolved
Hide resolved
...native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt
Show resolved
Hide resolved
...native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt
Show resolved
Hide resolved
...native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt
Show resolved
Hide resolved
# Conflicts: # packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModuleImpl.kt # packages/react-native-callingx/android/src/newarch/java/io/getstream/rn/callingx/CallingxModule.kt # packages/react-native-callingx/android/src/oldarch/java/io/getstream/rn/callingx/CallingxModule.kt # packages/react-native-sdk/src/utils/push/internal/utils.ts
greenfrvr
merged commit 5f6e708
into
feat/callkit-telecom-integration
15 of 19 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Overview
This PR contains multi call support for Android Telecom integration.
Main goal is to be able to manage notification for several simultaneous calls. E.g. if user is in active call they might receive new incoming call. For that new call new notification should be displayed and user should be able to reject/accept new incoming call.
Call repository refactor will allow to implement several simultaneous calls in future when "on hold" functionality will be presented.
screen-20260317-105012-1773740976267.mov
Summary by CodeRabbit
Bug Fixes
Refactor