feat: Control incoming call from native#7066
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughNative VoIP accept flow was refactored across Android and iOS: accepts are deduplicated, heads‑up accepts routed via VoipNotification, native accept failures emit a Changes
Sequence DiagramsequenceDiagram
participant CallKit as CallKit/Telecom
participant Native as Native VoIP Module
participant Notif as VoipNotification / VoipService
participant DDP as DDP Connection
participant JS as JS (MediaSession / DeepLinking)
participant UI as VoIP UI
Note over CallKit,UI: User or heads‑up accept triggers native accept
CallKit->>Native: Answer/Accept event
Native->>Notif: handleNativeAccept(payload)
Notif->>Notif: cancel timeouts & dedupe (per-call)
Notif->>Notif: resolve identity (userId, deviceId)
Notif->>DDP: send or queue "stream-notify-user" accept signal
DDP-->>Notif: ack or queued
Notif->>Native: onAnswer / VoiceConnection.onAnswer()
Notif->>UI: dismiss VoIP UI (ACTION_DISMISS)
alt DDP accept failure
Notif->>JS: emit VoipAcceptFailed (initial events with voipAcceptFailed)
JS->>JS: handleVoipAcceptFailed -> reset state, end CallKit call, navigate, show toast
else DDP accept success
Notif->>JS: emit initial events for JS recovery
JS->>JS: normal incoming‑call flow / navigation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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. 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 |
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 171-178: The heads-up accept path causes duplicate JS emissions
because VoipModule.storeInitialEvents(payload) is invoked in
prepareMainActivityForIncomingVoip and again in handleAcceptAction's finish
callback; fix by ensuring storeInitialEvents is called only once—either remove
the call from prepareMainActivityForIncomingVoip when invoked from the heads-up
branch, or add a boolean flag to handleAcceptAction (e.g.,
skipStoreInitialEvents) and propagate it to the finish() callback so callers
(the heads-up branch) can request skipping the second store; update
handleAcceptAction's signature and all callers (including IncomingCallActivity
accept path) accordingly so behavior remains unchanged for non-heads-up flows.
In `@app/lib/services/voip/MediaCallEvents.ts`:
- Around line 35-44: Re-enable the RNCallKeep 'answerCall' listener in
MediaCallEvents.ts so the JS fallback still invokes
mediaSessionInstance.answerCall(callUUID) when native accept fails; restore the
RNCallKeep.addEventListener('answerCall', ({ callUUID }) => {
mediaSessionInstance.answerCall(callUUID);
NativeVoipModule.clearInitialEvents(); RNCallKeep.clearInitialEvents(); })
subscription and ensure it is added to subscriptions (so it can be cleaned up),
keeping existing duplicate-avoidance logic that prevents double-answering when
VoipService.handleNativeAccept(...) succeeds.
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Around line 74-78: Don't call NativeVoipModule.stopNativeDDPClient()
immediately in the this.instance?.on('registered', ...) handler because it can
clear queued native accepts; instead defer stopping the native DDP client until
queued accepts are flushed. Update the 'registered' callback in
MediaSessionInstance.ts to only call stopNativeDDPClient after verifying there
are no pending native accepts (e.g. by calling a new or existing
NativeVoipModule.hasQueuedAccepts()/flushQueuedAccepts() or by listening for a
NativeVoipModule 'queuedAcceptsFlushed' event), or after a safe confirmation
(timeout + check of activeCalls) so pending accept calls are not dropped. Ensure
references to NativeVoipModule.stopNativeDDPClient and the 'registered' handler
are where this logic is implemented.
In `@app/lib/services/voip/MediaSessionStore.ts`:
- Around line 56-58: The code logs a persistent device identifier
(mobileDeviceId) obtained via getUniqueIdSync in MediaSessionStore.ts which
leaks a cross-session tracking ID; remove the console.log that prints
mobileDeviceId and ensure no other code paths output this value (search for
getUniqueIdSync and mobileDeviceId in MediaSessionStore and related functions),
and if debugging is needed replace with a non-identifying marker (e.g.,
hashed/truncated or a boolean) emitted via a secure logger only when explicitly
enabled.
In `@ios/Libraries/VoipService.swift`:
- Around line 414-417: The code inserts payload.callId into
nativeAcceptHandledCallIds before doing network work, but never removes it on
failure, which prevents retries; update the accept flow (where
nativeAcceptHandledCallIds.insert(payload.callId) is used) to remove the callId
from nativeAcceptHandledCallIds on every failure path (e.g., missing client,
parameter build failure, unsuccessful callback/response) before returning or
throwing, and ensure this cleanup runs in all early-return branches in the
accept handling block (also apply the same removal to the other accept block
around lines 421-454) so the dedupe guard is only retained on successful
completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5532036-b541-4919-8c00-f3387ddbfc37
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
.cursor/skills/agent-skillsandroid/app/src/main/java/chat/rocket/reactnative/MainActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.ktapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/services/voip/MediaSessionStore.tsios/Libraries/AppDelegate+Voip.swiftios/Libraries/VoipService.swiftpackage.jsonpackages/rocket.chat-media-signaling-0.1.1.tgzpackages/rocket.chat-media-signaling-0.1.3.tgz
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
🔇 Additional comments (12)
.cursor/skills/agent-skills (1)
1-1: Verify this submodule update is intentional.This submodule pointer update appears unrelated to the VoIP native accept feature described in the PR objectives. Please confirm whether this change should be included in this PR or if it was accidentally staged.
android/app/src/main/java/chat/rocket/reactnative/voip/IncomingCallActivity.kt (1)
280-287: LGTM!The refactoring to delegate accept handling to
VoipNotification.handleAcceptActionwith async completion viaACTION_DISMISSbroadcast is well-structured. The activity correctly waits for the broadcast before finishing, and the receiver is properly registered/unregistered in lifecycle methods.android/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.kt (1)
24-27: LGTM!Clean refactoring that consolidates VoIP intent handling in
VoipNotification. The delegation pattern with early return maintains clear control flow.android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (9)
53-71: LGTM!The new constants and data structures are well-documented. The KDoc on
ACTION_VOIP_ACCEPT_HEADS_UPclearly explains the Android 12+ trampoline restriction, andSUPPORTED_VOIP_FEATUREShas a helpful sync reminder comment.
207-263: LGTM!The async accept handling with completion callback is well-structured:
- Telecom call is only answered on DDP success
- On failure, app opens for JS recovery (good fallback)
stopDDPClientInternalonly called on success, preserving the client for potential JS retry
265-286: LGTM!Both helpers are well-implemented:
launchMainActivityForVoipcorrectly addsFLAG_ACTIVITY_NEW_TASKwhen context is not an ActivityanswerIncomingCalluses type-safe pattern matching with appropriate logging for edge cases
353-361: LGTM!Good rename from
flushPendingRejectSignalIfNeededtoflushPendingQueuedSignalsIfNeededto reflect that it now handles both accept and reject signals.
363-408: LGTM!The accept signal functions follow the same pattern as their reject counterparts, with the appropriate addition of completion callbacks needed for async accept handling.
433-460: LGTM!The signal params are correctly structured:
- Accept includes
supportedFeatures: ["audio"]per PR requirements- Both use consistent identity resolution
- Channel format
"${ids.userId}/media-calls"aligns with the DDP stream pattern
540-544: LGTM!Updated call site correctly reflects the renamed function.
750-774: LGTM!Correct handling of Android 12+ notification trampoline restrictions by using
PendingIntent.getActivitydirectly. The version-specificFLAG_IMMUTABLEhandling is also correct.
410-431: The KDoc is accurate. The code correctly usesSettings.Secure.ANDROID_ID, which matches the implementation ofgetUniqueIdSync()from react-native-device-info—both retrieve the same Android system value via identical API calls.
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
Show resolved
Hide resolved
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/lib/services/voip/MediaSessionInstance.ts (1)
67-69: Update stale comment to match behavior.The comment says “accepted from another device,” but current condition handles same-device contract match and answers locally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/services/voip/MediaSessionInstance.ts` around lines 67 - 69, The comment above the conditional in MediaSessionInstance.ts is outdated: update the comment to reflect that when signal.type === 'notification', signal.notification === 'accepted' and signal.signedContractId === getUniqueIdSync(), the instance should answer the call locally (same-device contract match) rather than handling an acceptance from another device; locate the condition checking signal.type, signal.notification, signal.signedContractId and getUniqueIdSync() and replace the comment to state that the call is accepted on the same device and answerCall(signal.callId) is invoked locally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Line 65: The console.log in MediaSessionInstance that prints the entire signal
object (e.g., "console.log('🤙 [VoIP] Processed signal:', signal)") should be
replaced to avoid leaking sensitive VoIP payloads: change logging to only emit
minimal safe fields (for example signal.type, signal.callId, signal.state) and
wrap any verbose/full-payload logging behind a dev-only guard (e.g., if
(__DEV__) { /* verbose logging */ }) so production never logs the full object;
update every similar occurrence (including the other console.log of signal) in
the class to follow this pattern.
- Around line 68-69: The fire-and-forget call to this.answerCall(signal.callId)
inside MediaSessionInstance when signal.type==='notification' can produce
unhandled promise rejections if answerCall (or underlying mainCall.accept())
throws; update the code to handle the promise—either make the enclosing handler
async and await this.answerCall(...) inside a try/catch, or append a .catch(...)
to this.answerCall(...) that logs or handles the error appropriately (include
context like signal.callId and signal.signedContractId). Ensure the chosen
handler uses the MediaSessionInstance logger/error reporting so failures are
surfaced instead of becoming unhandled.
In `@app/sagas/deepLinking.js`:
- Around line 99-128: In handleVoipAcceptFailed remove the redundant yield
put(appStart({ root: RootEnum.ROOT_INSIDE })) since navigate({ params:
navigateParams }) already triggers appStart; locate the appStart call inside the
handleVoipAcceptFailed generator and delete it, leaving the navigate invocation
to perform the final appStart, and run/adjust any tests that assumed two
dispatches to confirm behavior remains correct.
In `@ios/Libraries/VoipService.swift`:
- Around line 423-427: The native dedupe set nativeAcceptHandledCallIds is never
cleared for successful accepts; update handleNativeAccept (and related accept
paths at the other instances) to remove payload.callId from
nativeAcceptHandledCallIds once the accept is confirmed successful—either after
the send completes successfully or in the hasConnected/hasEnded callbacks that
indicate a completed connection lifecycle—so that observedIncomingCall being
cleared earlier doesn't prevent cleanup; reference nativeAcceptHandledCallIds,
handleNativeAccept, observedIncomingCall, hasConnected, and hasEnded and ensure
the removal happens in the success/connected path as well as existing
failure/timeout paths.
---
Nitpick comments:
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Around line 67-69: The comment above the conditional in
MediaSessionInstance.ts is outdated: update the comment to reflect that when
signal.type === 'notification', signal.notification === 'accepted' and
signal.signedContractId === getUniqueIdSync(), the instance should answer the
call locally (same-device contract match) rather than handling an acceptance
from another device; locate the condition checking signal.type,
signal.notification, signal.signedContractId and getUniqueIdSync() and replace
the comment to state that the call is accepted on the same device and
answerCall(signal.callId) is invoked locally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6295391a-bb94-438d-bced-ceaf79c44962
📒 Files selected for processing (14)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.ktapp/actions/actionsTypes.tsapp/actions/deepLinking.tsapp/definitions/Voip.tsapp/i18n/locales/en.jsonapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/MediaSessionInstance.tsapp/sagas/deepLinking.jsios/Libraries/AppDelegate+Voip.swiftios/Libraries/VoipModule.mmios/Libraries/VoipPayload.swiftios/Libraries/VoipService.swift
✅ Files skipped from review due to trivial changes (3)
- app/actions/actionsTypes.ts
- app/definitions/Voip.ts
- app/i18n/locales/en.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/sagas/deepLinking.js
📚 Learning: 2026-03-05T06:06:12.277Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:12.277Z
Learning: Do not re-activate or reset the WCSession singleton in iOS Objective-C/Swift bridge modules. Ensure WCSession is activated and its delegate is set in a single, central place (e.g., ios/RocketChat Watch App/Loaders/WatchSession.swift) and avoid duplicating activation or delegate assignment in other iOS bridge files like ios/RCTWatchModule.mm. If WCSession is already activated via the central loader, relying on WCSession.defaultSession is sufficient and maintains a single session lifecycle.
Applied to files:
ios/Libraries/VoipModule.mm
🪛 SwiftLint (0.63.2)
ios/Libraries/AppDelegate+Voip.swift
[Warning] 3-3: Prefer private over fileprivate declarations
(private_over_fileprivate)
🔇 Additional comments (15)
app/lib/services/voip/MediaSessionInstance.ts (1)
73-75: Good change: registered handler no longer eagerly stops native DDP.This avoids the previous cold-start race where queued native accepts could be dropped before flush.
ios/Libraries/VoipService.swift (1)
431-450: This path still does not actually “rely on JS”.All native-accept failure branches funnel into
finishAccept(false), which ends the CallKit call and emitsVoipAcceptFailed. With the iOSanswerCallfallback removed on the JS side, that turns a user answer into a hard failure instead of a JS handoff.#!/bin/bash # Verify there is no remaining JS-side CallKeep answer fallback on iOS. rg -nC2 "RNCallKeep\\.addEventListener\\(['\"]answerCall['\"]|mediaSessionInstance\\.answerCall\\(|VoipAcceptFailed" app/lib/services/voip rg -nC2 "voipAcceptFailed|VoIP_Call_Issue|endCall" app/sagas/deepLinking.jsAlso applies to: 475-500
android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt (1)
64-77: This issue has been resolved. ThetoBundle()method now correctly serializes thevoipAcceptFailedflag (lines 91–93), andfromBundle()deserializes it properly (lines 197–198). Bundle round-tripping is now complete and symmetric.app/sagas/deepLinking.js (5)
1-3: LGTM!The new imports for
InteractionManager,RNCallKeep,I18n,showToast, anduseCallStoreare appropriate for the VoIP accept failure handling functionality.Also applies to: 23-23, 30-30
50-62: LGTM!The
waitForNavigationfunction correctly handles both immediate resolution when navigation is ready and deferred resolution via emitter listener.
130-136: LGTM!The
completeDeepLinkNavigationwrapper cleanly routes VoIP accept failures to the dedicated handler while preserving the normal navigation flow for other cases.
191-196: LGTM!Correctly handles the
voipAcceptFailedcase when no host is present, providing an early exit to the dedicated failure handler before falling back to normal navigation.
231-231: LGTM!The integration of
completeDeepLinkNavigationat all navigation completion points ensures consistent VoIP failure handling across same-server, cross-server with existing user, failed server lookup, and new server with token scenarios.Also applies to: 239-239, 248-251, 266-266
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (7)
53-61: LGTM!The new
ACTION_VOIP_ACCEPT_HEADS_UPconstant is well-documented, explaining the Android 12+ trampoline restriction and the intent flow. TheVoipMediaCallIdentitydata class andSUPPORTED_VOIP_FEATURESconstant provide clean encapsulation for the DDP signaling parameters.Also applies to: 67-71
159-187: LGTM — past review concern addressed.The duplicate
storeInitialEventsissue from the previous review has been resolved. In the heads-up accept path (line 174),prepareMainActivityForIncomingVoipis called withstorePayloadForJs = false, preventing the first store. The single store now occurs inhandleAcceptAction'sfinish(ddpSuccess)callback on success (line 233).
193-211: LGTM!
prepareMainActivityForIncomingVoipcleanly handles notification/timeout cleanup, conditional payload storage, and keyguard dismissal for Android O_MR1+.
222-266: LGTM!The
handleAcceptActionfunction properly handles both immediate (logged-in) and queued (pre-login) DDP accept signaling with a unifiedfinishcallback. The callback correctly:
- Stops DDP client unconditionally
- On success: answers the telecom call and stores events for JS
- On failure: disconnects the call and stores failure state for JS recovery
- Sends dismiss broadcast and optionally launches MainActivity
436-463: LGTM!The
buildAcceptSignalParamsandbuildRejectSignalParamsfunctions correctly construct the DDP signaling payloads with appropriate fields. Accept includessupportedFeatures, while reject omits it as expected.
753-777: LGTM!The accept
PendingIntentcorrectly usesPendingIntent.getActivityto work around Android 12's notification trampoline restrictions. TheFLAG_IMMUTABLEflag is properly applied on API 31+ as required, with the fallback to mutable on older versions.
413-434: ANDROID_ID implementation is correct and matchesgetUniqueIdSync().The code correctly uses
Settings.Secure.getString(context.contentResolver, Settings.Secure.ANDROID_ID), which is the same underlying call used byreact-native-device-info'sgetUniqueIdSync()on Android. ANDROID_ID retrieval viaSettings.Secureis consistent across all supported API levels (24–35); no API-level-specific behavior differences exist for this particular call.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/sagas/deepLinking.js (1)
95-98: Doc comment mentions "dialog" but only toast is shown.The comment at line 97 states "toast/dialog per a11y", but the implementation (line 124) only calls
showToast. If accessibility requires a dialog in certain contexts, consider adding that logic or updating the comment to reflect the current behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/sagas/deepLinking.js` around lines 95 - 98, The doc comment in deepLinking.js claims "toast/dialog per a11y" but the implementation only calls showToast (see the saga handling native VoIP accept failure where showToast is invoked); update the comment to accurately reflect current behavior (e.g., "toast shown for user feedback; no dialog is displayed") or, if you intend to support an a11y dialog, implement that conditional dialog display alongside showToast in the same handler instead of changing the text—refer to the saga function that resets call state, ends CallKit session, and calls showToast to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/sagas/deepLinking.js`:
- Around line 95-98: The doc comment in deepLinking.js claims "toast/dialog per
a11y" but the implementation only calls showToast (see the saga handling native
VoIP accept failure where showToast is invoked); update the comment to
accurately reflect current behavior (e.g., "toast shown for user feedback; no
dialog is displayed") or, if you intend to support an a11y dialog, implement
that conditional dialog display alongside showToast in the same handler instead
of changing the text—refer to the saga function that resets call state, ends
CallKit session, and calls showToast to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 042b1cee-6e57-4c07-9662-56de3d40aa78
📒 Files selected for processing (2)
app/actions/deepLinking.tsapp/sagas/deepLinking.js
🚧 Files skipped from review as they are similar to previous changes (1)
- app/actions/deepLinking.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/sagas/deepLinking.js
🔇 Additional comments (6)
app/sagas/deepLinking.js (6)
1-3: LGTM!New imports are appropriate and all are utilized by the new VoIP failure handling logic.
109-115: DuplicateappStartdispatch persists.Line 109 dispatches
appStart({ root: RootEnum.ROOT_INSIDE }), and then line 115 callsnavigate(), which unconditionally dispatches the same action at the end (line 92). This causes redundant state updates.Remove the explicit
appStartat line 109, sincenavigatealready handles it.
[raise_minor_issue, duplicate_comment]Suggested fix
yield call(waitForNavigation); - yield put(appStart({ root: RootEnum.ROOT_INSIDE })); - const navigateParams = {
130-136: LGTM!Good abstraction that centralizes the VoIP failure vs. normal navigation decision, reducing duplication across
handleOpenbranches.
191-194: LGTM!Correctly handles VoIP failure when the deep link lacks a host, ensuring the user receives feedback even without full server context.
231-231: LGTM!Consistent use of
completeDeepLinkNavigationacross all successful deep link resolution paths.
248-251: LGTM!Correctly prioritizes VoIP failure handling over generic fallback when the server info lookup fails, ensuring the user still receives appropriate feedback.
diegolmello
force-pushed
the
feat.voip-registered-event
branch
from
d49ddea to
d3776e9
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/Libraries/AppDelegate+Voip.swift (1)
27-37:⚠️ Potential issue | 🟠 MajorAvoid logging full VoIP payload contents (PII/compliance risk).
At Line 27 and Line 37, dumping
payloadDict/voipPayloadcan leak sensitive call metadata into logs. Log only minimal non-sensitive context (e.g.,callIdand payload keys).Proposed redaction-focused change
- print("[\(voipAppDelegateLogTag)] Failed to parse incoming VoIP payload: \(payloadDict)") + let payloadKeys = payloadDict.keys.sorted() + print("[\(voipAppDelegateLogTag)] Failed to parse incoming VoIP payload. keys=\(payloadKeys)") ... - print("[\(voipAppDelegateLogTag)] Skipping expired or invalid VoIP payload for callId: \(callId): \(voipPayload)") + print("[\(voipAppDelegateLogTag)] Skipping expired or invalid VoIP payload for callId: \(callId)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Libraries/AppDelegate`+Voip.swift around lines 27 - 37, The prints currently logging full payloadDict and voipPayload (see voipAppDelegateLogTag, payloadDict, voipPayload) may leak PII; change those debug prints to only emit non-sensitive context such as callId and a list of payload keys (or a redaction placeholder) instead of dumping the whole payload object, and ensure the isExpired() guard log uses callId (and not voipPayload) when reporting skipped/expired payloads.
🧹 Nitpick comments (2)
ios/Libraries/AppDelegate+Voip.swift (1)
3-3: Useprivateinstead offileprivatefor the log tag.For file-scope constants,
privateis the idiomatic choice and behaviorally equivalent tofileprivate. This aligns with the SwiftLint ruleprivate_over_fileprivate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Libraries/AppDelegate`+Voip.swift at line 3, The constant voipAppDelegateLogTag is declared fileprivate but should be private per Swift style; change the declaration of voipAppDelegateLogTag from fileprivate to private in AppDelegate+Voip (replace "fileprivate let voipAppDelegateLogTag" with "private let voipAppDelegateLogTag") to satisfy the private_over_fileprivate SwiftLint rule and keep the same visibility.android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)
677-678: Null check on non-nullable String parameters is redundant.
callIdandcallerare declared asString(notString?) inVoipPayload, so they cannot be null. The.isNullOrEmpty()check works but the null part is unnecessary.Proposed simplification
- if (callId.isNullOrEmpty() || caller.isNullOrEmpty()) { + if (callId.isEmpty() || caller.isEmpty()) { Log.e(TAG, "Cannot register call with TelecomManager: callId is null or empty") return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt` around lines 677 - 678, The null checks on callId and caller are redundant because VoipPayload declares them as non-nullable Strings; update the validation in VoipNotification (where the check occurs) to only test emptiness (e.g., use callId.isEmpty() || caller.isEmpty() or isBlank() if you want to treat whitespace as empty) and remove the isNullOrEmpty() calls to keep the check correct and idiomatic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ios/Libraries/AppDelegate`+Voip.swift:
- Around line 27-37: The prints currently logging full payloadDict and
voipPayload (see voipAppDelegateLogTag, payloadDict, voipPayload) may leak PII;
change those debug prints to only emit non-sensitive context such as callId and
a list of payload keys (or a redaction placeholder) instead of dumping the whole
payload object, and ensure the isExpired() guard log uses callId (and not
voipPayload) when reporting skipped/expired payloads.
---
Nitpick comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 677-678: The null checks on callId and caller are redundant
because VoipPayload declares them as non-nullable Strings; update the validation
in VoipNotification (where the check occurs) to only test emptiness (e.g., use
callId.isEmpty() || caller.isEmpty() or isBlank() if you want to treat
whitespace as empty) and remove the isNullOrEmpty() calls to keep the check
correct and idiomatic.
In `@ios/Libraries/AppDelegate`+Voip.swift:
- Line 3: The constant voipAppDelegateLogTag is declared fileprivate but should
be private per Swift style; change the declaration of voipAppDelegateLogTag from
fileprivate to private in AppDelegate+Voip (replace "fileprivate let
voipAppDelegateLogTag" with "private let voipAppDelegateLogTag") to satisfy the
private_over_fileprivate SwiftLint rule and keep the same visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d2acd91-cfe4-4a45-a8b0-a5402d537151
📒 Files selected for processing (15)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.ktapp/actions/actionsTypes.tsapp/actions/deepLinking.tsapp/definitions/Voip.tsapp/i18n/locales/en.jsonapp/index.tsxapp/lib/services/voip/MediaCallEvents.tsapp/lib/services/voip/MediaSessionInstance.tsapp/sagas/deepLinking.jsios/Libraries/AppDelegate+Voip.swiftios/Libraries/VoipModule.mmios/Libraries/VoipPayload.swiftios/Libraries/VoipService.swift
✅ Files skipped from review due to trivial changes (4)
- app/definitions/Voip.ts
- app/index.tsx
- app/i18n/locales/en.json
- app/actions/actionsTypes.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- android/app/src/main/java/chat/rocket/reactnative/voip/VoipPayload.kt
- app/actions/deepLinking.ts
- ios/Libraries/VoipPayload.swift
- android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt
- ios/Libraries/VoipService.swift
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-05T06:06:12.277Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:12.277Z
Learning: Do not re-activate or reset the WCSession singleton in iOS Objective-C/Swift bridge modules. Ensure WCSession is activated and its delegate is set in a single, central place (e.g., ios/RocketChat Watch App/Loaders/WatchSession.swift) and avoid duplicating activation or delegate assignment in other iOS bridge files like ios/RCTWatchModule.mm. If WCSession is already activated via the central loader, relying on WCSession.defaultSession is sufficient and maintains a single session lifecycle.
Applied to files:
ios/Libraries/VoipModule.mm
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/sagas/deepLinking.js
🪛 SwiftLint (0.63.2)
ios/Libraries/AppDelegate+Voip.swift
[Warning] 3-3: Prefer private over fileprivate declarations
(private_over_fileprivate)
🔇 Additional comments (15)
app/lib/services/voip/MediaSessionInstance.ts (3)
65-77: Avoid logging full VoIP signal/call payloads in production.These logs include entire signaling/call objects, which may expose sensitive call metadata and create high-volume logs. Log only minimal fields (e.g.,
type,callId, state) and gate verbose logs behind__DEV__.Proposed fix
- console.log('🤙 [VoIP] Processed signal:', signal); + if (__DEV__) { + console.log('🤙 [VoIP] Processed signal:', { type: signal?.type, callId: signal?.callId }); + }
83-84: Gate verbose call data logging behind__DEV__.Logging the full
callobject on every state change could expose sensitive data and create excessive log volume in production.Proposed fix
call.emitter.on('stateChange', oldState => { console.log(`📊 ${oldState} → ${call.state}`); - console.log('🤙 [VoIP] New call data:', call); + if (__DEV__) { + console.log('🤙 [VoIP] New call data:', { callId: call.callId, role: call.role, state: call.state }); + } });
68-72: LGTM! Proper error handling for answerCall.The
.catch()handler addresses the previous concern about unhandled promise rejections. ThesignedContractIdcheck correctly identifies accepts from this device using the synchronousgetUniqueIdSync().ios/Libraries/VoipModule.mm (2)
37-52: LGTM! VoipAcceptFailed event support added correctly.The new event follows the established pattern for
VoipPushTokenRegistered. The observer registration instartObservingand thesendEventWrapperusage for delayed event queuing are consistent with the existing implementation.
111-119: Good fix for TurboModule listener lifecycle.Calling
[super addListener:]and[super removeListeners:]ensuresRCTEventEmitterproperly tracks listener state. The previous empty implementations would have preventedstartObserving/stopObservingfrom being called, breaking event delivery to JS.android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (3)
159-187: LGTM! Clean intent routing for VoIP accepts.The
handleMainActivityVoipIntentfunction properly handles the heads-up accept flow by:
- Checking for the specific action before processing
- Resetting the intent action to prevent re-processing
- Using
storePayloadForJs=falseto avoid duplicate stash (addresses past review)- Returning
trueto signal the intent was consumed
222-266: Async DDP accept with proper failure recovery.The
handleAcceptActiondesign is solid:
- Cancels timeout first to prevent races
- Uses a
finish()callback to ensure cleanup happens regardless of DDP outcome- Stores failure state for JS recovery when DDP fails
- Correctly disconnects Telecom connection on failure (line 236)
753-777: LGTM! Correct PendingIntent for Android 12+ trampoline restrictions.Using
PendingIntent.getActivitywithFLAG_IMMUTABLEfor API 31+ is the correct approach to avoid the notification trampoline restrictions. The comment at lines 753-755 clearly explains the rationale.app/sagas/deepLinking.js (3)
95-126: LGTM! Clean VoIP accept failure recovery flow.The
handleVoipAcceptFailedsaga properly:
- Resets call state before accessing params (callId comes from params, not store)
- Guards
RNCallKeep.endCallwith a null check- Waits for navigation readiness before proceeding
- Uses
InteractionManager.runAfterInteractionsto ensure the toast appears after UI settles- Catches and logs errors without crashing the app
128-134: LGTM! Unified deep-link routing through completeDeepLinkNavigation.This helper cleanly separates the VoIP failure path from normal navigation while ensuring both flows go through the same navigation infrastructure.
189-192: Good edge case handling for voipAcceptFailed without host.The early returns ensure the VoIP failure flow is handled even when the deep link lacks a host or when server info retrieval fails, preventing the app from getting stuck.
Also applies to: 246-249
app/lib/services/voip/MediaCallEvents.ts (4)
17-39: LGTM! Clean deduplication for accept failure events.The
lastHandledVoipAcceptFailureCallIdpattern prevents double-dispatch when both the initial-events stash and the live event fire for the same failed accept. The early returns for missingvoipAcceptFailedflag and duplicatecallIdare correct.
95-101: VoipAcceptFailed listener correctly applies to both platforms.This listener is intentionally outside the
if (isIOS)block since both iOS (viaNativeEventEmitter) and Android (viaDeviceEventEmitter) emit this event. ThevoipAcceptFailed: truespread ensures the flag is set even if native omitted it.
122-128: LGTM! Correct early return to prevent appInit() race.Returning
truewhenvoipAcceptFailedis detected ensures the caller (app/index.tsx:156-160) skipsappInit(), preventing a race between the deep-linking saga and the normal app initialization path.
45-106: The iOS VoipAcceptFailed path does not attempt to answer the call via JS.When native iOS accept fails before DDP completes, the
VoipAcceptFailedlistener (line 96) only dispatchesdeepLinkingOpento show the failure UI—it does not callmediaSessionInstance.answerCallas a fallback. Unlike Android's explicit fallback mechanism (which stores the failure and allows the VoipPushInitialEvents listener to handle recovery), iOS has no corresponding answerCall attempt when the native accept fails. The call is abandoned after the failure notification is sent. Verify whether this behavior is intentional or ifmediaSessionInstance.answerCallshould be invoked in theVoipAcceptFailedhandler to recover from native accept failures.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ios/Libraries/VoipService.swift (1)
439-442:⚠️ Potential issue | 🟡 MinorSuccessful native accepts never release their dedupe entry.
When
finishAccept(true)is called (line 441-442), thecallIdremains innativeAcceptHandledCallIds. ThehasEndedcallback at lines 615-627 won't clear it becauseobservedIncomingCallis set tonilat line 610 beforehandleNativeAcceptreturns, so the guard at line 604 fails.This leaves stale entries in the set until process restart. While bounded in practice (0-1 concurrent calls), it's inconsistent with the documented lifecycle and could cause issues if the same
callIdis reused across app restarts without process termination.🔧 Proposed fix: Clear dedupe on successful accept
let finishAccept: (Bool) -> Void = { success in stopDDPClientInternal() if success { storeInitialEvents(payload) + clearNativeAcceptDedupe(for: payload.callId) } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Libraries/VoipService.swift` around lines 439 - 442, The successful native accept path never clears the dedupe entry, leaving callId in nativeAcceptHandledCallIds; update the finishAccept closure (the success branch in finishAccept) to explicitly remove the current callId from nativeAcceptHandledCallIds (or invoke the same cleanup the hasEnded callback uses) when success is true so that handleNativeAccept/hasEnded/observedIncomingCall lifecycle invariants are preserved and stale entries aren't left behind.
🧹 Nitpick comments (2)
ios/Libraries/VoipService.swift (1)
603-612: Potential race:observedIncomingCallcleared before asynchandleNativeAcceptcompletes.Line 610 sets
observedIncomingCall = nilsynchronously, then callshandleNativeAcceptwhich performs async DDP work. IfcallObserver(_:callChanged:)fires again for the same call (e.g.,hasEndedshortly afterhasConnected), the guard at line 604 will fail and the end event won't be processed.This is likely acceptable given the dedupe guard, but worth noting that any cleanup logic in the
hasEndedbranch (lines 615-627) won't run for calls that connect and end quickly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Libraries/VoipService.swift` around lines 603 - 612, The code clears observedIncomingCall immediately before invoking handleNativeAccept, which can race with later callChanged events (e.g., hasEnded) and skip cleanup; modify handleObservedCall/handleNativeAccept so observedIncomingCall is cleared only after the async accept work finishes: either make handleNativeAccept return/accept a completion (or be async) and move observedIncomingCall = nil into that completion, or have handleNativeAccept set observedIncomingCall = nil when its async DDP work completes; update callers accordingly (symbols: handleObservedCall(_:) , observedIncomingCall, handleNativeAccept(payload:)).android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)
69-72: Verify sync with MediaSessionStore when updating VOIP features.The Kotlin
SUPPORTED_VOIP_FEATURESmirrors the JSMediaSessionStore.tsfeatures array (line 69:features: ['audio']). Both are currently in sync, but any feature additions on the JS side require manual updates to the Kotlin constant to maintain consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt` around lines 69 - 72, The Kotlin constant SUPPORTED_VOIP_FEATURES in VoipNotification.kt is a hard-coded mirror of the JS MediaSessionStore.features array and can drift when JS features change; update SUPPORTED_VOIP_FEATURES whenever you add or remove features in MediaSessionStore.ts (e.g., add the same string entries such as "audio" or any new feature names), and consider adding a clear comment referencing MediaSessionStore.ts to prevent future desyncs (locate SUPPORTED_VOIP_FEATURES inside the VoipNotification class and the features array in MediaSessionStore.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ios/Libraries/VoipService.swift`:
- Around line 439-442: The successful native accept path never clears the dedupe
entry, leaving callId in nativeAcceptHandledCallIds; update the finishAccept
closure (the success branch in finishAccept) to explicitly remove the current
callId from nativeAcceptHandledCallIds (or invoke the same cleanup the hasEnded
callback uses) when success is true so that
handleNativeAccept/hasEnded/observedIncomingCall lifecycle invariants are
preserved and stale entries aren't left behind.
---
Nitpick comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 69-72: The Kotlin constant SUPPORTED_VOIP_FEATURES in
VoipNotification.kt is a hard-coded mirror of the JS MediaSessionStore.features
array and can drift when JS features change; update SUPPORTED_VOIP_FEATURES
whenever you add or remove features in MediaSessionStore.ts (e.g., add the same
string entries such as "audio" or any new feature names), and consider adding a
clear comment referencing MediaSessionStore.ts to prevent future desyncs (locate
SUPPORTED_VOIP_FEATURES inside the VoipNotification class and the features array
in MediaSessionStore.ts).
In `@ios/Libraries/VoipService.swift`:
- Around line 603-612: The code clears observedIncomingCall immediately before
invoking handleNativeAccept, which can race with later callChanged events (e.g.,
hasEnded) and skip cleanup; modify handleObservedCall/handleNativeAccept so
observedIncomingCall is cleared only after the async accept work finishes:
either make handleNativeAccept return/accept a completion (or be async) and move
observedIncomingCall = nil into that completion, or have handleNativeAccept set
observedIncomingCall = nil when its async DDP work completes; update callers
accordingly (symbols: handleObservedCall(_:) , observedIncomingCall,
handleNativeAccept(payload:)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce292e8a-a033-4cee-b03c-f54710c79694
📒 Files selected for processing (3)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.ktapp/lib/services/voip/MediaCallEvents.tsios/Libraries/VoipService.swift
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (12)
app/lib/services/voip/MediaCallEvents.ts (4)
65-70: Intentional removal of iOSanswerCalllistener is now documented.The comment at lines 66-69 explains that
VoipService.swifthandles accepts natively viahandleNativeAccept()before JS runs, and JS only reads storedinitialEventsData. This addresses the previous review concern about missing JS fallback — the native path now handles all accept scenarios including failure recovery viaVoipAcceptFailedevent.
22-39: LGTM! Well-structured dedupe logic for accept failures.The deduplication guard correctly prevents duplicate handling when native emits and stash replay occur for the same failed accept. Setting
lastHandledVoipAcceptFailureCallIdbefore dispatching ensures atomicity within the single-threaded JS runtime.
102-108: Minor: RedundantvoipAcceptFailed: trueoverride.Line 105 spreads
dataand then forcesvoipAcceptFailed: true. If native already sets this flag, the override is redundant. However, this is a safe defensive pattern ensuring the helper always receives the correct flag regardless of native payload shape.
119-135: Cold-start accept failure handling looks correct.The early return at line 134 prevents racing
appInit()with the deep-linking saga. The validation at line 129 ensures bothcallIdandhostare present before dispatching, avoiding malformed deep links.android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (5)
172-179: Heads-up accept path correctly avoids duplicatestoreInitialEvents.Line 175 passes
storePayloadForJs = falsetoprepareMainActivityForIncomingVoip, and line 247 inhandleAcceptAction'sfinish(true)branch stores the payload. This addresses the previous review concern about duplicate emissions.
225-262: LGTM! Well-designed async accept flow with proper timeout handling.The
AtomicBooleanguard ensuresfinish()executes at most once, preventing race conditions between the DDP callback and the 10-second timeout. ThetimeoutHandler.removeCallbacks(timeoutRunnable)at line 243 cleans up properly when DDP completes first.
427-448: Device ID resolution usesSettings.Secure.ANDROID_IDcorrectly.The comment at line 428-429 correctly notes this must match JS
getUniqueIdSync()from react-native-device-info. The null checks at lines 436 and 442 handle edge cases where credentials or device ID are unavailable.
767-791: Accept action usesPendingIntent.getActivityto comply with Android 12+ restrictions.The comment at lines 767-769 explains why
getActivityis used instead ofgetBroadcast— Android 12+ blocks starting activities from notificationBroadcastReceivertrampolines. TheFLAG_IMMUTABLEis correctly applied only on API 31+.
296-303: Null connection handling inanswerIncomingCallis appropriate.The
whenexpression handlesVoiceConnection,null, and other connection types gracefully with logging. This defensive approach prevents crashes if the connection state is unexpected.ios/Libraries/VoipService.swift (3)
45-58: Excellent documentation of the dedupe guard lifecycle.The detailed comment explaining when entries are added and removed helps maintainability. The memory bound note ("0-1 concurrent VoIP calls") is accurate for this use case.
399-428: LGTM! Unified parameter builder simplifies accept/reject logic.The
buildMediaCallAnswerParamsfunction cleanly handles both cases with theVoipMediaCallAnswerKindenum, conditionally addingsupportedFeaturesonly for accepts.
459-479: Accept failure notification payload construction is thorough.The
acceptFailedUserInfodictionary includes all necessary fields for JS recovery. The optional handling ofavatarUrlandcreatedAtat lines 469-474 is correct.
diegolmello
force-pushed
the
feat.voip-registered-event
branch
from
d0aede0 to
47dc4a5
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
1 participant
Proposed changes
VoipAcceptFailedwhen the React instance is active; clear native accept dedupe on failure and other terminal paths (timeout, DDP hangup, CallKit ended before accept, etc.).VoipServicenative accept over DDP (parity with Android);NotificationCenter→VoipModuleforVoipAcceptFailed;VoipPayloadsupportsvoipAcceptFailed.VoipNotificationaccept completion refactored (storeAcceptFailureForJs, heads-up path usesprepareMainActivityForIncomingVoip(..., storePayloadForJs = false));VoipModule/VoipPayloadsupport failure stash + event.VOIP_CALL/voipCallOpen; usedeepLinkingOpenwith optionalvoipAcceptFailed; extendhandleOpenwithcompleteDeepLinkNavigationand inlinehandleVoipAcceptFailed(reset store,RNCallKeep.endCall,appStart(ROOT_INSIDE), reusenavigatewithdirect/${username}whenusernameis set, thenshowToastforVoIP_Call_Issue— toast vs dialog per accessibility preference).VoipAcceptFailedlisteners, guard on Android initial events so failed accepts never callanswerCall, dedupe bycallId; cold-start handling viagetInitialEventswithvoipAcceptFailed.stopNativeDDPClientonregisteredand related dead code.VoIP_Call_Issuefor failure copy.AppDelegate+Voipfileprivate log tag.Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-56
How to test or reproduce
Incoming VoIP (happy path)
Native accept failure → in-app recovery
username— Force native DDP accept failure (e.g. network/credentials) with validhost+username. Expect system call UI ended; inside app root; navigation viadirect/<username>(may create/open DM percanOpenRoom); then toast or single-string dialog if accessibility alert type is DIALOG.username— Same but no username. Expect inside root and toast/dialog only (no forced DM path).host→ open withvoipAcceptFailed. Expect failure handler + toast, no stuck splash.voipAcceptFailedafter server switch +LOGIN.SUCCESS. Expect failure handler after auth, not stuck outside.getServerInfofails —voipAcceptFailedwith host that fails server info. Expect failure handler (not only silent fallback).host—voipAcceptFailedwithout host. Expect handler runs without crash; toast shown.handleVoipAcceptFailed, not invite-only stuck state.callId. Expect a single user-visible failure flow (one toast / one navigation burst).Native → JS signaling
voipAcceptFailedconsumed;deepLinkingOpen+ recovery + toast.VoipAcceptFailed(iOS/Android) handled; noanswerCallon failure payloads on Android initial-events path.Deep linking
DEEP_LINKING.OPEN/deepLinkingOpenwithhost+callId(novoipAcceptFailed). No dependency on removedVOIP_CALL.voipAcceptFailed: trueroutes throughcompleteDeepLinkNavigation→handleVoipAcceptFailed.Accessibility
Alertwith the sameVoIP_Call_Issuestring.Native DDP / dedupe (spot-check)
Not in scope for this PR: mid-call abnormal drop (future work).
Automated
yarn testScreenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Chores