feat(voip): display caller avatar on CallKit incoming call screen (iOS)#7166
Open
diegolmello wants to merge 2 commits intofeat.voip-lib-newfrom
Open
feat(voip): display caller avatar on CallKit incoming call screen (iOS)#7166diegolmello wants to merge 2 commits intofeat.voip-lib-newfrom
diegolmello wants to merge 2 commits intofeat.voip-lib-newfrom
Conversation
Uses a temporary CNContact to surface the caller's avatar in the native
CallKit UI. Apple provides no CXCallUpdate.callerImage API; the only
supported path is Contacts-based lookup: create a synthetic contact with
a unique phone number, then call reportCall(with:updated:) to switch the
handle type so CallKit resolves the photo automatically.
Key pieces:
- CallerContactManager.swift: downloads avatar (5s timeout, 1 MB cap,
400×400 resize), creates a temporary CNContact tagged "rc-voip-temp-{id}",
and exposes removeContact/removeTemporaryContacts for cleanup.
- AppDelegate+Voip.swift: after PushKit delivery, asynchronously creates
the contact and updates the live CallKit call handle.
- VoipService.swift: adds isCallTracked() guard (race condition safety) and
removeContact() at all four call-exit paths.
- AppDelegate.swift: sweeps orphaned contacts on app launch.
- MMKV opt-in gate: feature is off by default; native reads
VOIP_CALLER_PHOTO_KEY before attempting contact creation.
- requestContactsAccess TurboModule method (iOS real, Android stub).
- Extended react-native-callkeep patch to expose sharedCXProvider.
- i18n keys for the future user-preference toggle.
Contributor
WalkthroughA feature to display caller photos on incoming VoIP calls is being implemented across iOS and Android. The change adds contact management utilities on iOS, localization strings, feature flags, and native module methods for both platforms. Android returns "unavailable" while iOS implements full temporary contact creation with avatar downloading and management. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant PKPush as PKPushRegistry
participant VoipSvc as VoipService
participant Delegate as AppDelegate+Voip
participant MMKV as MMKV
participant CCMgr as CallerContactManager
participant CNStore as Contacts Store
participant CallKit as CallKit/RNCallKeep
PKPush->>Delegate: pushRegistry didReceiveIncomingPushWith payload
Delegate->>VoipSvc: reportNewIncomingCall()
VoipSvc->>CallKit: RNCallKeep.reportNewIncomingCall()
Delegate->>MMKV: Read VOIP_CALLER_PHOTO_KEY flag
alt Feature Enabled & avatarUrl present
Delegate->>CCMgr: createTemporaryContact(name, username, avatarUrl, callId)
CCMgr->>CCMgr: Verify Contacts permission
CCMgr->>CCMgr: Download avatar (5s timeout)
CCMgr->>CCMgr: Resize to 400×400 PNG
CCMgr->>CNStore: Create & save CNMutableContact
CCMgr->>VoipSvc: Check isCallTracked(callUUID)
alt Call still active
CCMgr->>CallKit: Get sharedCXProvider
CCMgr->>CallKit: reportCall(with phoneNumber, callerName)
else Call ended
CCMgr->>CNStore: Remove temporary contact
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
🚥 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 |
![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 (1)
app/lib/native/NativeVoip.ts (1)
38-43: Model permission statuses as a shared enum instead of raw strings.
Promise<string>plus the hardcoded'unavailable'fallback leaves this contract open-ended and easy to drift across JS, iOS, and Android. Narrow it to a shared status enum and reuse that in the fallback/native implementations.As per coding guidelines "Use enums for sets of related constants rather than magic strings or numbers".♻️ Suggested shape
+export enum ContactsAccessStatus { + Authorized = 'authorized', + Denied = 'denied', + Unavailable = 'unavailable' +} + export interface Spec extends TurboModule { @@ - requestContactsAccess(): Promise<string>; + requestContactsAccess(): Promise<ContactsAccessStatus>; @@ - requestContactsAccess: () => Promise.resolve('unavailable'), + requestContactsAccess: () => Promise.resolve(ContactsAccessStatus.Unavailable),Also applies to: 68-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/native/NativeVoip.ts` around lines 38 - 43, Change the requestContactsAccess contract from Promise<string> to a shared enum type (e.g., PermissionStatus) and replace all raw strings with that enum; specifically, define a PermissionStatus enum (values like GRANTED, DENIED, UNAVAILABLE, PROMPT) and update the requestContactsAccess signature to Promise<PermissionStatus>, update the JS fallback that returns 'unavailable' to return PermissionStatus.UNAVAILABLE, and update the iOS/Android native implementations to return the enum values instead of raw strings. Also find any other permission-returning methods (the similar declaration around the other permission at the same area) and migrate them to use the same PermissionStatus enum so all platforms share the same type.
🤖 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/i18n/locales/en.json`:
- Around line 842-843: Update the two i18n entries to use singular "photo"
consistently and clarify the permission requirement: change the value for
"Show_caller_photo_on_incoming_calls" to use "photo" (already singular) and
update "Show_caller_photo_on_incoming_calls_description" to a clearer sentence
such as "Display the caller's photo on the incoming call screen. Requires access
to the device's Contacts." so it explicitly references the Contacts permission
and uses singular form "caller’s photo."
In `@ios/Libraries/AppDelegate`+Voip.swift:
- Around line 68-85: The closure passed to
CallerContactManager.createTemporaryContact runs on contactQueue and is
performing UI/CallKit-related work off the main thread; wrap the work that reads
VoipService.isCallTracked(callUUID), calls
CallerContactManager.removeContact(forCallId:), builds the CXCallUpdate
(callUpdate) and calls RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID,
updated: callUpdate) in a DispatchQueue.main.async block so the tracked-call
check and CallKit update happen on the main thread to avoid races with
VoipService.trackIncomingCall() and handleObservedCallChanged().
In `@ios/Libraries/CallerContactManager.swift`:
- Around line 146-187: downloadAvatar currently uses URLSession.shared.dataTask
which loads the entire body into memory before applying the 1 MB cap; change the
implementation to stream and enforce the size limit as the bytes arrive (or
cancel early based on expectedContentLength) by switching to a URLSession with a
delegate (implement URLSessionDataDelegate) or using a dataTask with a delegate
callback: in the delegate's urlSession(_:dataTask:didReceive:), accumulate bytes
into a mutable Data buffer, cancel the task and call completion(nil) if
buffer.count exceeds 1_048_576; also implement
urlSession(_:dataTask:didReceive:completionHandler:) to inspect
response.expectedContentLength and cancel early if it exceeds the cap, then on
urlSession(_:task:didCompleteWithError:) convert the accumulated Data to
UIImage, call resizeIfNeeded(image, maxDimension: 400) and
completion(resized.pngData()) or completion(nil) on errors.
- Around line 31-59: The callback from downloadAvatar treats a nil image as
success and still creates a CNMutableContact; change the downloadAvatar
completion handler in the createTemporaryContact flow so that if imageData is
nil you immediately call completion(nil) (and do not enqueue contactQueue work),
otherwise proceed to the contactQueue.async block that builds CNMutableContact,
sets imageData, creates CNSaveRequest and executes contactStore.execute; ensure
no contact creation or completion(number) happens when imageData is nil.
- Around line 94-121: The current removeTemporaryContacts() routine only filters
by organizationName (orgTag) and may delete unrelated contacts; update the
deletion guard to require the rc-voip-temp- marker as well: in
removeTemporaryContacts() after confirming contact.organizationName == orgTag
(and before deleting), check that the contact contains the "rc-voip-temp-"
marker (e.g., in a predictable field you use for temp contacts such as
givenName, note, nickname, or a custom suffix in organizationName) and only call
saveRequest.delete(mutable) when that marker is present; keep the existing
predicate and keysToFetch but add the extra marker check in the loop so only
temp contacts created by this feature are removed.
In `@ios/RocketChatRN/Info.plist`:
- Around line 76-77: Update the NSContactsUsageDescription entry in Info.plist
to disclose that the app may create and later delete temporary/synthetic
contacts in the user’s address book (not just read/display caller photos);
locate the NSContactsUsageDescription key in Info.plist and replace the current
string with a concise message that mentions both displaying caller photos and
the creation/removal of temporary contacts so the permission prompt accurately
reflects the behavior.
---
Nitpick comments:
In `@app/lib/native/NativeVoip.ts`:
- Around line 38-43: Change the requestContactsAccess contract from
Promise<string> to a shared enum type (e.g., PermissionStatus) and replace all
raw strings with that enum; specifically, define a PermissionStatus enum (values
like GRANTED, DENIED, UNAVAILABLE, PROMPT) and update the requestContactsAccess
signature to Promise<PermissionStatus>, update the JS fallback that returns
'unavailable' to return PermissionStatus.UNAVAILABLE, and update the iOS/Android
native implementations to return the enum values instead of raw strings. Also
find any other permission-returning methods (the similar declaration around the
other permission at the same area) and migrate them to use the same
PermissionStatus enum so all platforms share the same type.
🪄 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: eb3c5f13-6e0c-459e-a58a-3d6a6fa24216
📒 Files selected for processing (12)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/i18n/locales/en.jsonapp/lib/constants/keys.tsapp/lib/native/NativeVoip.tsios/AppDelegate.swiftios/Libraries/AppDelegate+Voip.swiftios/Libraries/CallerContactManager.swiftios/Libraries/VoipModule.mmios/Libraries/VoipService.swiftios/RocketChatRN.xcodeproj/project.pbxprojios/RocketChatRN/Info.plistpatches/react-native-callkeep+4.3.16.patch
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line
Files:
app/lib/constants/keys.tsapp/lib/native/NativeVoip.tsapp/i18n/locales/en.json
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint with
@rocket.chat/eslint-configbase configuration including React, React Native, TypeScript, and Jest plugins
Files:
app/lib/constants/keys.tsapp/lib/native/NativeVoip.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/constants/keys.tsapp/lib/native/NativeVoip.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/constants/keys.tsapp/lib/native/NativeVoip.ts
app/i18n/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place internationalization configuration and locales in app/i18n/ with support for 40+ languages and RTL
Files:
app/i18n/locales/en.json
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate
Applied to files:
app/lib/constants/keys.tsios/Libraries/VoipModule.mmandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.ktapp/lib/native/NativeVoip.tsios/Libraries/AppDelegate+Voip.swiftios/Libraries/VoipService.swiftpatches/react-native-callkeep+4.3.16.patch
📚 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-05T06:06:19.755Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:19.755Z
Learning: In the Rocket.Chat React Native iOS app, `WCSession` (WatchConnectivity) is activated with its delegate in `ios/RocketChat Watch App/Loaders/WatchSession.swift`, not in `RCTWatchModule.mm`. Since `WCSession.defaultSession` is a singleton, activating it once in `WatchSession.swift` is sufficient; `RCTWatchModule.mm` does not need to re-activate or re-set the delegate.
Applied to files:
ios/AppDelegate.swiftios/RocketChatRN.xcodeproj/project.pbxproj
🔇 Additional comments (7)
app/lib/constants/keys.ts (1)
28-28: Looks good.The new key follows the existing constant pattern and keeps the feature flag easy to reference across JS and native code.
patches/react-native-callkeep+4.3.16.patch (2)
48-50: Good minimal iOS patch surface.The
_delayedEventsnil guard andsharedCXProvideraccessor keep the CallKit change narrowly scoped to the pieces this feature actually needs.Also applies to: 59-60
9-27: Overload removal is safe; the 3-argument variants are unused in this codebase.The app calls neither
displayIncomingCall()norstartCall()directly. All RNCallKeep usage is limited toendCall(),setCurrentCallActive(), andsetAvailable(). The removed overloads were convenience wrappers that delegated to the 5-argument versions, which remain untouched. The iOS additions (sharedCXProvideraccessor and nil guard for_delayedEvents) are orthogonal and correct.ios/AppDelegate.swift (1)
31-31: Reasonable place for orphan cleanup.Running the sweep during launch matches the temp-contact lifecycle and helps avoid stale synthetic contacts surviving crashes or force-quits.
android/app/src/main/java/chat/rocket/reactnative/voip/VoipModule.kt (1)
144-147: LGTM.Returning
"unavailable"here keeps the Android native behavior aligned with the shared JS contract.ios/Libraries/VoipModule.mm (1)
16-16: Nice cross-platform bridge hook.Delegating the permission request through
VoipServicekeeps the TurboModule surface aligned with the native implementations without adding extra event plumbing.Also applies to: 121-125
ios/Libraries/VoipService.swift (1)
260-265: Nice cleanup coverage across the native exit paths.Removing the temporary contact on timeout, reject, accept completion, and CallKit-end closes most of the leak surface for these synthetic Contacts entries.
Also applies to: 427-430, 528-530, 613-617
Comment on lines
+842
to
+843
| "Show_caller_photo_on_incoming_calls": "Show caller photo on incoming calls", | ||
| "Show_caller_photo_on_incoming_calls_description": "Display caller photos on the incoming call screen. Requires access to Contacts.", |
Contributor
There was a problem hiding this comment.
Fix singular/plural inconsistency and improve description clarity.
There's an inconsistency in the wording:
- Line 842 uses singular "photo"
- Line 843 uses plural "photos"
Since the feature displays one caller's photo per incoming call, use the singular form consistently. Additionally, the description could be more precise about the permission requirement.
✏️ Proposed fix for consistency and clarity
"Show_caller_photo_on_incoming_calls": "Show caller photo on incoming calls",
-"Show_caller_photo_on_incoming_calls_description": "Display caller photos on the incoming call screen. Requires access to Contacts.",
+"Show_caller_photo_on_incoming_calls_description": "Displays the caller's photo on the incoming call screen. Requires Contacts permission.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "Show_caller_photo_on_incoming_calls": "Show caller photo on incoming calls", | |
| "Show_caller_photo_on_incoming_calls_description": "Display caller photos on the incoming call screen. Requires access to Contacts.", | |
| "Show_caller_photo_on_incoming_calls": "Show caller photo on incoming calls", | |
| "Show_caller_photo_on_incoming_calls_description": "Displays the caller's photo on the incoming call screen. Requires Contacts permission.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/i18n/locales/en.json` around lines 842 - 843, Update the two i18n entries
to use singular "photo" consistently and clarify the permission requirement:
change the value for "Show_caller_photo_on_incoming_calls" to use "photo"
(already singular) and update "Show_caller_photo_on_incoming_calls_description"
to a clearer sentence such as "Display the caller's photo on the incoming call
screen. Requires access to the device's Contacts." so it explicitly references
the Contacts permission and uses singular form "caller’s photo."
Comment on lines
+68
to
+85
| CallerContactManager.createTemporaryContact( | ||
| callerName: caller, | ||
| username: voipPayload.username, | ||
| avatarUrl: avatarUrl, | ||
| callId: callId | ||
| ) { syntheticNumber in | ||
| guard let syntheticNumber else { return } | ||
|
|
||
| // Verify the call is still active before updating CallKit | ||
| guard VoipService.isCallTracked(callUUID) else { | ||
| CallerContactManager.removeContact(forCallId: callId) | ||
| return | ||
| } | ||
|
|
||
| let callUpdate = CXCallUpdate() | ||
| callUpdate.remoteHandle = CXHandle(type: .phoneNumber, value: syntheticNumber) | ||
| callUpdate.localizedCallerName = caller | ||
| RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID, updated: callUpdate) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "CallerContactManager completion queue:"
nl -ba ios/Libraries/CallerContactManager.swift | sed -n '32,58p'
echo
echo "AppDelegate avatar-update callback:"
nl -ba ios/Libraries/AppDelegate+Voip.swift | sed -n '68,86p'
echo
echo "VoipService tracked-call mutations:"
nl -ba ios/Libraries/VoipService.swift | sed -n '573,605p'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the actual files
echo "=== Locating files ==="
fd -e swift | grep -E "(AppDelegate|CallerContactManager|VoipService)" | head -20
# Get file sizes
echo ""
echo "=== File sizes ==="
wc -l ios/Libraries/AppDelegate+Voip.swift ios/Libraries/CallerContactManager.swift ios/Libraries/VoipService.swift 2>/dev/null || echo "Files not found, searching..."
# Search for files with pattern matching
echo ""
echo "=== Search for VoipService files ==="
fd "VoipService" ios/Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full AppDelegate+Voip.swift ==="
cat -n ios/Libraries/AppDelegate+Voip.swift
echo ""
echo "=== CallerContactManager.swift createTemporaryContact method ==="
grep -n "createTemporaryContact" -A 30 ios/Libraries/CallerContactManager.swift | head -50
echo ""
echo "=== VoipService observedIncomingCalls mutations ==="
grep -n "observedIncomingCalls" ios/Libraries/VoipService.swift | head -20Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 7017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== VoipService isCallTracked method ==="
grep -n "isCallTracked" -B 2 -A 3 ios/Libraries/VoipService.swift | head -20
echo ""
echo "=== VoipService trackIncomingCall method ==="
grep -n "trackIncomingCall" -B 2 -A 10 ios/Libraries/VoipService.swift | head -30
echo ""
echo "=== VoipService clearTrackedIncomingCall method ==="
grep -n "clearTrackedIncomingCall" -B 2 -A 10 ios/Libraries/VoipService.swift | head -30
echo ""
echo "=== VoipService handleObservedCallChanged method ==="
grep -n "handleObservedCallChanged" -B 2 -A 15 ios/Libraries/VoipService.swift | head -40Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 4235
Run the post-download CallKit update on the main queue.
createTemporaryContact completes on contactQueue, so lines 74-85 currently read tracked-call state and report the CallKit update off-main. That races with the main-thread mutations in VoipService.trackIncomingCall() and handleObservedCallChanged(), risking a stale read or updating a call that has already transitioned.
🛠️ Proposed fix
CallerContactManager.createTemporaryContact(
callerName: caller,
username: voipPayload.username,
avatarUrl: avatarUrl,
callId: callId
) { syntheticNumber in
guard let syntheticNumber else { return }
-
- // Verify the call is still active before updating CallKit
- guard VoipService.isCallTracked(callUUID) else {
- CallerContactManager.removeContact(forCallId: callId)
- return
- }
-
- let callUpdate = CXCallUpdate()
- callUpdate.remoteHandle = CXHandle(type: .phoneNumber, value: syntheticNumber)
- callUpdate.localizedCallerName = caller
- RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID, updated: callUpdate)
+ DispatchQueue.main.async {
+ guard VoipService.isCallTracked(callUUID) else {
+ CallerContactManager.removeContact(forCallId: callId)
+ return
+ }
+
+ let callUpdate = CXCallUpdate()
+ callUpdate.remoteHandle = CXHandle(type: .phoneNumber, value: syntheticNumber)
+ callUpdate.localizedCallerName = caller
+ RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID, updated: callUpdate)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CallerContactManager.createTemporaryContact( | |
| callerName: caller, | |
| username: voipPayload.username, | |
| avatarUrl: avatarUrl, | |
| callId: callId | |
| ) { syntheticNumber in | |
| guard let syntheticNumber else { return } | |
| // Verify the call is still active before updating CallKit | |
| guard VoipService.isCallTracked(callUUID) else { | |
| CallerContactManager.removeContact(forCallId: callId) | |
| return | |
| } | |
| let callUpdate = CXCallUpdate() | |
| callUpdate.remoteHandle = CXHandle(type: .phoneNumber, value: syntheticNumber) | |
| callUpdate.localizedCallerName = caller | |
| RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID, updated: callUpdate) | |
| CallerContactManager.createTemporaryContact( | |
| callerName: caller, | |
| username: voipPayload.username, | |
| avatarUrl: avatarUrl, | |
| callId: callId | |
| ) { syntheticNumber in | |
| guard let syntheticNumber else { return } | |
| DispatchQueue.main.async { | |
| guard VoipService.isCallTracked(callUUID) else { | |
| CallerContactManager.removeContact(forCallId: callId) | |
| return | |
| } | |
| let callUpdate = CXCallUpdate() | |
| callUpdate.remoteHandle = CXHandle(type: .phoneNumber, value: syntheticNumber) | |
| callUpdate.localizedCallerName = caller | |
| RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID, updated: callUpdate) | |
| } | |
| } |
🤖 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 68 - 85, The closure
passed to CallerContactManager.createTemporaryContact runs on contactQueue and
is performing UI/CallKit-related work off the main thread; wrap the work that
reads VoipService.isCallTracked(callUUID), calls
CallerContactManager.removeContact(forCallId:), builds the CXCallUpdate
(callUpdate) and calls RNCallKeep.sharedCXProvider()?.reportCall(with: callUUID,
updated: callUpdate) in a DispatchQueue.main.async block so the tracked-call
check and CallKit update happen on the main thread to avoid races with
VoipService.trackIncomingCall() and handleObservedCallChanged().
Comment on lines
+31
to
+59
| // Download avatar first, then write to Contacts on the serial queue. | ||
| downloadAvatar(from: avatarUrl) { imageData in | ||
| contactQueue.async { | ||
| do { | ||
| let contact = CNMutableContact() | ||
| contact.givenName = callerName | ||
| contact.organizationName = orgTag | ||
| contact.note = "rc-voip-temp-\(callId)" | ||
| contact.imageData = imageData | ||
|
|
||
| let phoneNumber = CNLabeledValue( | ||
| label: CNLabelPhoneNumberMain, | ||
| value: CNPhoneNumber(stringValue: number) | ||
| ) | ||
| contact.phoneNumbers = [phoneNumber] | ||
|
|
||
| let saveRequest = CNSaveRequest() | ||
| saveRequest.add(contact, toContainerWithIdentifier: nil) | ||
| try contactStore.execute(saveRequest) | ||
|
|
||
| completion(number) | ||
| } catch { | ||
| #if DEBUG | ||
| print("RocketChat.CallerContactManager createTemporaryContact error: \(error)") | ||
| #endif | ||
| completion(nil) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Treat avatar download failure as a hard failure for contact creation.
downloadAvatar returns nil on network/parse errors, but this block still saves a temporary contact and completes with number. That means failed downloads still switch CallKit to the synthetic handle and leave behind a contact with no image.
🛠️ Proposed fix
let number = syntheticPhoneNumber(for: username)
// Download avatar first, then write to Contacts on the serial queue.
downloadAvatar(from: avatarUrl) { imageData in
+ guard let imageData else {
+ completion(nil)
+ return
+ }
+
contactQueue.async {
do {
let contact = CNMutableContact()
contact.givenName = callerName
contact.organizationName = orgTag
contact.note = "rc-voip-temp-\(callId)"
contact.imageData = imageData🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Libraries/CallerContactManager.swift` around lines 31 - 59, The callback
from downloadAvatar treats a nil image as success and still creates a
CNMutableContact; change the downloadAvatar completion handler in the
createTemporaryContact flow so that if imageData is nil you immediately call
completion(nil) (and do not enqueue contactQueue work), otherwise proceed to the
contactQueue.async block that builds CNMutableContact, sets imageData, creates
CNSaveRequest and executes contactStore.execute; ensure no contact creation or
completion(number) happens when imageData is nil.
Comment on lines
+94
to
+121
| /// Removes ALL temporary contacts created by this manager. Call on app launch as an orphan sweep. | ||
| static func removeTemporaryContacts() { | ||
| contactQueue.async { | ||
| do { | ||
| let keysToFetch: [CNKeyDescriptor] = [ | ||
| CNContactOrganizationNameKey as CNKeyDescriptor | ||
| ] | ||
| let predicate = CNContact.predicateForContacts(matchingName: orgTag) | ||
| let candidates = try contactStore.unifiedContacts(matching: predicate, keysToFetch: keysToFetch) | ||
|
|
||
| let saveRequest = CNSaveRequest() | ||
| var found = false | ||
| for contact in candidates { | ||
| guard contact.organizationName == orgTag else { continue } | ||
| guard let mutable = contact.mutableCopy() as? CNMutableContact else { continue } | ||
| saveRequest.delete(mutable) | ||
| found = true | ||
| } | ||
|
|
||
| if found { | ||
| try contactStore.execute(saveRequest) | ||
| } | ||
| } catch { | ||
| #if DEBUG | ||
| print("RocketChat.CallerContactManager removeTemporaryContacts error: \(error)") | ||
| #endif | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t orphan-sweep by organizationName alone.
On app launch this will delete any user contact whose organization happens to be Rocket.Chat VoIP, not just the temporary contacts created by this feature. The sweep should require the rc-voip-temp- marker as well.
🛠️ Proposed fix
let keysToFetch: [CNKeyDescriptor] = [
- CNContactOrganizationNameKey as CNKeyDescriptor
+ CNContactOrganizationNameKey as CNKeyDescriptor,
+ CNContactNoteKey as CNKeyDescriptor
]
let predicate = CNContact.predicateForContacts(matchingName: orgTag)
let candidates = try contactStore.unifiedContacts(matching: predicate, keysToFetch: keysToFetch)
let saveRequest = CNSaveRequest()
var found = false
for contact in candidates {
- guard contact.organizationName == orgTag else { continue }
+ guard contact.organizationName == orgTag,
+ contact.note.hasPrefix("rc-voip-temp-") else { continue }
guard let mutable = contact.mutableCopy() as? CNMutableContact else { continue }
saveRequest.delete(mutable)
found = true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Removes ALL temporary contacts created by this manager. Call on app launch as an orphan sweep. | |
| static func removeTemporaryContacts() { | |
| contactQueue.async { | |
| do { | |
| let keysToFetch: [CNKeyDescriptor] = [ | |
| CNContactOrganizationNameKey as CNKeyDescriptor | |
| ] | |
| let predicate = CNContact.predicateForContacts(matchingName: orgTag) | |
| let candidates = try contactStore.unifiedContacts(matching: predicate, keysToFetch: keysToFetch) | |
| let saveRequest = CNSaveRequest() | |
| var found = false | |
| for contact in candidates { | |
| guard contact.organizationName == orgTag else { continue } | |
| guard let mutable = contact.mutableCopy() as? CNMutableContact else { continue } | |
| saveRequest.delete(mutable) | |
| found = true | |
| } | |
| if found { | |
| try contactStore.execute(saveRequest) | |
| } | |
| } catch { | |
| #if DEBUG | |
| print("RocketChat.CallerContactManager removeTemporaryContacts error: \(error)") | |
| #endif | |
| } | |
| } | |
| /// Removes ALL temporary contacts created by this manager. Call on app launch as an orphan sweep. | |
| static func removeTemporaryContacts() { | |
| contactQueue.async { | |
| do { | |
| let keysToFetch: [CNKeyDescriptor] = [ | |
| CNContactOrganizationNameKey as CNKeyDescriptor, | |
| CNContactNoteKey as CNKeyDescriptor | |
| ] | |
| let predicate = CNContact.predicateForContacts(matchingName: orgTag) | |
| let candidates = try contactStore.unifiedContacts(matching: predicate, keysToFetch: keysToFetch) | |
| let saveRequest = CNSaveRequest() | |
| var found = false | |
| for contact in candidates { | |
| guard contact.organizationName == orgTag, | |
| contact.note.hasPrefix("rc-voip-temp-") else { continue } | |
| guard let mutable = contact.mutableCopy() as? CNMutableContact else { continue } | |
| saveRequest.delete(mutable) | |
| found = true | |
| } | |
| if found { | |
| try contactStore.execute(saveRequest) | |
| } | |
| } catch { | |
| `#if` DEBUG | |
| print("RocketChat.CallerContactManager removeTemporaryContacts error: \(error)") | |
| `#endif` | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Libraries/CallerContactManager.swift` around lines 94 - 121, The current
removeTemporaryContacts() routine only filters by organizationName (orgTag) and
may delete unrelated contacts; update the deletion guard to require the
rc-voip-temp- marker as well: in removeTemporaryContacts() after confirming
contact.organizationName == orgTag (and before deleting), check that the contact
contains the "rc-voip-temp-" marker (e.g., in a predictable field you use for
temp contacts such as givenName, note, nickname, or a custom suffix in
organizationName) and only call saveRequest.delete(mutable) when that marker is
present; keep the existing predicate and keysToFetch but add the extra marker
check in the loop so only temp contacts created by this feature are removed.
Comment on lines
+146
to
+187
| /// Downloads avatar data from a URL with a 5-second timeout and a 1 MB size cap. | ||
| /// Resizes to at most 400×400 before returning PNG data. Returns nil on any failure. | ||
| private static func downloadAvatar(from urlString: String, completion: @escaping (Data?) -> Void) { | ||
| guard let url = URL(string: urlString) else { | ||
| completion(nil) | ||
| return | ||
| } | ||
|
|
||
| var request = URLRequest(url: url, cachePolicy: .useProtocolCachePolicy, timeoutInterval: 5) | ||
| request.httpMethod = "GET" | ||
|
|
||
| let task = URLSession.shared.dataTask(with: request) { data, response, error in | ||
| if let error = error { | ||
| #if DEBUG | ||
| print("RocketChat.CallerContactManager avatar download error: \(error)") | ||
| #endif | ||
| completion(nil) | ||
| return | ||
| } | ||
|
|
||
| guard | ||
| let httpResponse = response as? HTTPURLResponse, | ||
| httpResponse.statusCode == 200, | ||
| let data = data, | ||
| data.count <= 1_048_576 | ||
| else { | ||
| #if DEBUG | ||
| print("RocketChat.CallerContactManager avatar download: invalid response or data too large") | ||
| #endif | ||
| completion(nil) | ||
| return | ||
| } | ||
|
|
||
| guard let image = UIImage(data: data) else { | ||
| completion(nil) | ||
| return | ||
| } | ||
|
|
||
| let resized = resizeIfNeeded(image, maxDimension: 400) | ||
| completion(resized.pngData()) | ||
| } | ||
| task.resume() |
Contributor
There was a problem hiding this comment.
The 1 MB cap is enforced too late.
Line 170 rejects oversized avatars only after dataTask has already delivered the full body into memory, so large responses can still exceed the stated transfer/memory budget. If the 1 MB limit is part of the contract for this feature, enforce it while streaming or cancel early from expectedContentLength before reading the body.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Libraries/CallerContactManager.swift` around lines 146 - 187,
downloadAvatar currently uses URLSession.shared.dataTask which loads the entire
body into memory before applying the 1 MB cap; change the implementation to
stream and enforce the size limit as the bytes arrive (or cancel early based on
expectedContentLength) by switching to a URLSession with a delegate (implement
URLSessionDataDelegate) or using a dataTask with a delegate callback: in the
delegate's urlSession(_:dataTask:didReceive:), accumulate bytes into a mutable
Data buffer, cancel the task and call completion(nil) if buffer.count exceeds
1_048_576; also implement urlSession(_:dataTask:didReceive:completionHandler:)
to inspect response.expectedContentLength and cancel early if it exceeds the
cap, then on urlSession(_:task:didCompleteWithError:) convert the accumulated
Data to UIImage, call resizeIfNeeded(image, maxDimension: 400) and
completion(resized.pngData()) or completion(nil) on errors.
Comment on lines
+76
to
+77
| <key>NSContactsUsageDescription</key> | ||
| <string>Rocket.Chat uses Contacts to display caller photos on incoming call screens.</string> |
Contributor
There was a problem hiding this comment.
Disclose the temporary contact writes in the permission prompt.
Line 77 only says the app uses Contacts to display caller photos, but this flow also creates and later deletes synthetic contacts in the user's address book. That is a materially different privacy disclosure and may confuse users or cause review friction.
📝 Suggested wording
<key>NSContactsUsageDescription</key>
-<string>Rocket.Chat uses Contacts to display caller photos on incoming call screens.</string>
+<string>Rocket.Chat temporarily creates and removes contacts so iOS can show caller photos on incoming call screens.</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <key>NSContactsUsageDescription</key> | |
| <string>Rocket.Chat uses Contacts to display caller photos on incoming call screens.</string> | |
| <key>NSContactsUsageDescription</key> | |
| <string>Rocket.Chat temporarily creates and removes contacts so iOS can show caller photos on incoming call screens.</string> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/RocketChatRN/Info.plist` around lines 76 - 77, Update the
NSContactsUsageDescription entry in Info.plist to disclose that the app may
create and later delete temporary/synthetic contacts in the user’s address book
(not just read/display caller photos); locate the NSContactsUsageDescription key
in Info.plist and replace the current string with a concise message that
mentions both displaying caller photos and the creation/removal of temporary
contacts so the permission prompt accurately reflects the behavior.
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
1 participant
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.
Proposed changes
Adds caller avatar support to the native iOS CallKit incoming call screen for VoIP calls.
The problem: Apple's
CXCallUpdatehas nocallerImageproperty. The only supported path for showing a photo in CallKit is Contacts-based lookup — CallKit automatically resolves the avatar when the call handle matches a contact's phone number.The approach: A new
CallerContactManager.swiftclass creates a temporaryCNContactfor each incoming call:reportNewIncomingCallfires immediately with a generic handle (required by Apple within the PushKit deadline)CNContacttaggedrc-voip-temp-{callId}with a unique synthetic phone numberRNCallKeep.sharedCXProvider().reportCall(with:updated:)to switch the handle type to.phoneNumber— CallKit then resolves the photo from Contacts automaticallyOpt-in gate: Feature is disabled by default. The native side reads
VOIP_CALLER_PHOTO_KEYfrom MMKV before attempting contact creation. The RN infrastructure (requestContactsAccessTurboModule method + i18n keys) is ready for a follow-up preference toggle inUserPreferencesView.Files changed
ios/Libraries/CallerContactManager.swiftios/Libraries/AppDelegate+Voip.swiftios/Libraries/VoipService.swiftisCallTracked()guard +removeContact()at all 4 exit paths +requestContactsAccess()ios/AppDelegate.swiftremoveTemporaryContacts()orphan sweep on launchios/Libraries/VoipModule.mmrequestContactsAccessbridge methodios/RocketChatRN/Info.plistNSContactsUsageDescriptionios/RocketChatRN.xcodeproj/project.pbxprojCallerContactManager.swiftin both targetspatches/react-native-callkeep+4.3.16.patch+sharedCXProviderclass method fromRNCallKeepapp/lib/native/NativeVoip.tsrequestContactsAccess(): Promise<string>spec + Android stubapp/lib/constants/keys.tsVOIP_CALLER_PHOTO_KEYconstantapp/i18n/locales/en.jsonIssue(s)
https://rocketchat.atlassian.net/browse/VMUX-80
How to test or reproduce
Prerequisites: Physical iOS device (Simulator does not support PushKit/CallKit), two Rocket.Chat accounts with VoIP enabled.
yarn iosor Xcode)VOIP_CALLER_PHOTO_KEY = "true"in MMKV (or temporarily hardcodecallerPhotoEnabled = trueinAppDelegate+Voip.swift)Regression: Full VoIP call flow should work normally when feature is disabled (default) — accept, reject, timeout, background/foreground transitions.
Screenshots
Requires physical device — simulator does not support PushKit.
Types of changes
Checklist
Further comments
Why temporary contacts instead of a direct image API?
Apple confirmed that
CXCallUpdateintentionally has no image property. The Contacts-based path (CNContactStore) is the only supported mechanism documented in WWDC sessions on CallKit customization. This approach is used by WhatsApp, Telegram, and other major VoIP apps.Cleanup strategy — why multiple sites?
CallKit calls can end via 5 different paths on iOS (caller hangs up, callee rejects, callee accepts then ends, timeout, app restart). Each path calls
CallerContactManager.removeContact(forCallId:). The app-launch sweep catches any contacts that survived a crash or force-quit.Follow-up work needed (not in this PR):
UserPreferencesView(all infrastructure is ready:VOIP_CALLER_PHOTO_KEY,requestContactsAccess, i18n strings)reportCall(with:updated:)(Step 0 spike from the plan)Summary by CodeRabbit