Skip to content

fix(voip): ship blockers for PushKit, licensing, outbound calls, push tokens#7167

Merged
diegolmello merged 4 commits into
feat.voip-lib-newfrom
cursor/b082663a
Apr 20, 2026
Merged

fix(voip): ship blockers for PushKit, licensing, outbound calls, push tokens#7167
diegolmello merged 4 commits into
feat.voip-lib-newfrom
cursor/b082663a

Conversation

@diegolmello

@diegolmello diegolmello commented Apr 16, 2026

Copy link
Copy Markdown
Member

Proposed changes

This change set lands the first slice of the VoIP hardening plan (pre–PR #6918 ship blockers).

  • iOS PushKit: invalid or expired VoIP payloads now report a placeholder call to CallKit and end it immediately, preserving PushKit's entitlement contract. The happy path routes the PushKit completion() through CallKit's reportNewIncomingCall completion handler so PushKit is only marked done after CallKit has been notified (per Apple's iOS 13+ PushKit docs). A shared helper in AppDelegate+Voip.swift removes duplicated CallKit reporting.
  • Enterprise licensing: the teams-voip module check is restored so VoIP stays gated by licensing. Unit tests cover isVoipModuleAvailable for present/absent/empty module lists.
  • Outbound calls (new-media call sheet): startCall is now awaited, failures surface via showErrorAlert, and the action sheet only dismisses on success. In MediaSessionInstance, startCall propagates the SDK's native Promise<void> (no redundant Promise.resolve wrap) and the fire-and-forget caller startCallByRoom attaches a .catch handler so rejections do not leak as unhandled promise rejections.
  • iOS push token registration: registerPushToken no longer short-circuits when the VoIP token is missing on iOS, while still sending both tokens together when VoIP is available.

Issue(s)

How to test or reproduce

  • iOS: exercise PushKit with an expired or malformed VoIP payload; confirm CallKit briefly shows then ends and the app does not skip CallKit reporting. Verify completion() fires only after CallKit accepts the report on the happy path as well.
  • Enterprise: with Redux modules excluding teams-voip, confirm VoIP entry points respect licensing; with teams-voip present, confirm VoIP remains available; with modules empty/cleared, confirm VoIP is unavailable.
  • New media call: start a call from the action sheet; on failure (e.g. mocked rejection), confirm an error alert and that the sheet stays open; on success, confirm the sheet dismisses.
  • startCallByRoom rejections: mock the SDK startCall to reject and confirm the error is logged without becoming an unhandled rejection.
  • iOS push: on a simulator or device without a VoIP token, confirm push.token still registers when APNs token exists; after VoIP token arrives, confirm a follow-up registration includes both fields.
  • Run TZ=UTC yarn test --testPathPattern='enterpriseModules.test|MediaSessionInstance.test'.

Screenshots

N/A (behavioral and platform fixes).

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Split from the VoIP review plan; targets develop as requested. No native project file edits beyond Swift in ios/Libraries/. Review follow-ups applied in this branch: PushKit completion() ordering on the happy path (CodeRabbit), Promise.resolve ceremony removed, startCallByRoom unhandled-rejection guard, and an additional empty-modules regression test.

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling when initiating calls with user-friendly alerts.
    • Fixed VoIP availability check to respect configured modules.
    • Enhanced iOS VoIP push handling for more reliable incoming-call reporting.
    • Optimized iOS push token registration to correctly handle notification and VoIP tokens.
  • Refactor

    • Call-start flow updated to handle async failures more robustly.
  • Tests

    • Added and updated tests to validate VoIP availability and call-start behavior.

@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Make VoIP call initiation asynchronous with try/catch and error alerts; gate VoIP availability on Redux-stored enterpriseModules containing teams-voip; allow partial iOS push-token payloads instead of early-return; centralize iOS PushKit→CallKit reporting and placeholder-call handling.

Changes

Cohort / File(s) Summary
NewMediaCall / UI
app/containers/NewMediaCall/CreateCall.tsx, app/containers/NewMediaCall/CreateCall.test.tsx, app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
handleCall made async and wraps mediaSessionInstance.startCall(...) in try/catch with error-alert on failure. Tests converted to async and flush microtasks after fireEvent.press.
MediaSession / VoIP runtime
app/lib/services/voip/MediaSessionInstance.ts
Changed startCall to async and await underlying instance?.startCall(...); startCallByRoom now attaches a .catch(...) to log start errors. Method signature updated to return Promise<void>.
Enterprise modules check & tests
app/lib/methods/enterpriseModules.ts, app/lib/methods/enterpriseModules.test.ts
isVoipModuleAvailable() now reads reduxStore.getState().enterpriseModules and returns whether it includes teams-voip. Added tests covering empty, non-voip, and voip-present states.
Push token registration
app/lib/services/restApi.ts
Removed iOS-specific early return when token/voipToken missing; builds conditional payload including only present token fields and proceeds to sdk.post('push.token', data).
iOS PushKit / CallKit
ios/Libraries/AppDelegate+Voip.swift
Extracted reportVoipIncomingCallToCallKit(...) helper and introduced reportPlaceholderCallAndEnd(...). Updated pushRegistry(_:didReceiveIncomingPushWith:for:completion:) to use helpers, report placeholder calls on invalid/expired payloads, and defer completion via callback parameter.

Sequence Diagram(s)

sequenceDiagram
    participant UI as NewMediaCall UI
    participant MS as MediaSessionInstance
    participant Core as VoIP Native Instance
    participant Alert as Error Alert

    rect rgba(0,128,0,0.5)
    UI->>MS: call startCall(selectedPeer)
    end
    rect rgba(0,0,255,0.5)
    MS->>Core: await instance.startCall(actor,userId)
    Core-->>MS: success / reject(error)
    end
    alt success
        MS-->>UI: resolves
        UI-->>UI: hideActionSheetRef()
    else failure
        MS-->>UI: throws error
        UI->>Alert: showErrorAlert(message, title)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically references all four major fix areas: PushKit, licensing, outbound calls, and push tokens. It accurately summarizes the main blockers being addressed in this VoIP hardening effort.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@diegolmello diegolmello changed the base branch from develop to feat.voip-lib-new April 16, 2026 20:27
@diegolmello diegolmello marked this pull request as ready for review April 16, 2026 20:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/lib/services/voip/MediaSessionInstance.ts (1)

169-181: ⚠️ Potential issue | 🟠 Major

startCallByRoom leaves startCall's promise unhandled.

Now that startCall returns Promise<void> (line 177), the fire-and-forget invocation at line 173 will surface any rejection (e.g., this.instance?.startCall throwing) as an unhandled promise rejection, and the original caller gets no signal of failure. CreateCall.tsx was updated to await/try-catch, but this path was not.

Attach at least a .catch (or make startCallByRoom async and rethrow/log) so licensing/WebRTC failures from the room-initiated flow are observed.

🐛 Proposed fix
-	public startCallByRoom = (room: TSubscriptionModel | ISubscription) => {
+	public startCallByRoom = (room: TSubscriptionModel | ISubscription) => {
 		useCallStore.getState().setRoomId(room.rid ?? null);
 		const otherUserId = getUidDirectMessage(room);
 		if (otherUserId) {
-			this.startCall(otherUserId, 'user');
+			this.startCall(otherUserId, 'user').catch(error => {
+				console.error('[VoIP] Error starting call from room:', error);
+			});
 		}
 	};

As per coding guidelines: "Use explicit error handling with try/catch blocks for async operations".

🤖 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 169 - 181,
startCallByRoom currently calls the async startCall fire-and-forget which can
produce unhandled rejections; update startCallByRoom to be async and await
this.startCall(...) inside a try/catch (or at minimum attach a .catch) so errors
from this.instance?.startCall are observed—use the same logger used elsewhere
(e.g., console.error or the module's logger) to log the error and optionally
rethrow or propagate a failure signal instead of silently dropping it; reference
the startCallByRoom and startCall methods and the getUidDirectMessage call to
locate where to add the await/try-catch.
🧹 Nitpick comments (2)
app/lib/methods/enterpriseModules.ts (1)

65-68: Extract 'teams-voip' into a named constant for consistency.

The other license identifiers in this file are declared as top-level constants (LICENSE_OMNICHANNEL_MOBILE_ENTERPRISE, LICENSE_LIVECHAT_ENTERPRISE). The same pattern would apply here and remove the magic string duplicated with useMediaCallPermission.ts and useMediaCallPermission usages.

♻️ Proposed refactor
 const LICENSE_OMNICHANNEL_MOBILE_ENTERPRISE = 'omnichannel-mobile-enterprise';
 const LICENSE_LIVECHAT_ENTERPRISE = 'livechat-enterprise';
+const LICENSE_TEAMS_VOIP = 'teams-voip';
@@
 export function isVoipModuleAvailable() {
 	const { enterpriseModules } = reduxStore.getState();
-	return enterpriseModules.includes('teams-voip');
+	return enterpriseModules.includes(LICENSE_TEAMS_VOIP);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/methods/enterpriseModules.ts` around lines 65 - 68, Extract the magic
string 'teams-voip' into a top-level named constant (e.g.,
LICENSE_TEAMS_VOIP_ENTERPRISE) and replace its direct usage in
isVoipModuleAvailable() and any other usages such as in
useMediaCallPermission.ts / useMediaCallPermission so all checks reference the
new constant instead of the literal string; update exports/imports as needed to
keep consistent naming with existing constants
LICENSE_OMNICHANNEL_MOBILE_ENTERPRISE and LICENSE_LIVECHAT_ENTERPRISE.
ios/Libraries/AppDelegate+Voip.swift (1)

51-62: Replace magic reason: 1 with a named CXCallEndedReason value.

RNCallKeep.endCall(withUUID:reason:) accepts the raw value of CXCallEndedReason; 1 corresponds to .failed. Using the named constant (or a file-local constant) avoids ambiguity for future readers and makes intent explicit — this placeholder is being ended because the VoIP payload was unparseable or expired, i.e., a failed call.

♻️ Proposed refactor
+import CallKit
+
 fileprivate let voipAppDelegateLogTag = "RocketChat.AppDelegate+Voip"
@@
-          RNCallKeep.endCall(withUUID: callUUID, reason: 1)
+          RNCallKeep.endCall(withUUID: callUUID, reason: Int32(CXCallEndedReason.failed.rawValue))
           completion()
🤖 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 51 - 62, The call uses a
magic literal reason: 1 when ending the placeholder call in the
reportPlaceholderCallAndEnd closure; replace that magic number with the named
CXCallEndedReason value (e.g., use CXCallEndedReason.failed.rawValue or a
file-local constant like let placeholderEndReason =
CXCallEndedReason.failed.rawValue) and pass that to
RNCallKeep.endCall(withUUID:callUUID, reason: placeholderEndReason) so the
intent is explicit in AppDelegate+Voip.swift and the
RNCallKeep.endCall(withUUID:reason:) call is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ios/Libraries/AppDelegate`+Voip.swift:
- Around line 84-91: The happy-path currently calls completion() immediately
after invoking reportVoipIncomingCallToCallKit, which violates PushKit rules;
instead pass the completion closure into reportVoipIncomingCallToCallKit's
onReportComplete parameter (i.e. replace onReportComplete: {} and the subsequent
completion() call with onReportComplete: { completion() }) so the completion
handler runs only after reportNewIncomingCall's callback completes.

---

Outside diff comments:
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Around line 169-181: startCallByRoom currently calls the async startCall
fire-and-forget which can produce unhandled rejections; update startCallByRoom
to be async and await this.startCall(...) inside a try/catch (or at minimum
attach a .catch) so errors from this.instance?.startCall are observed—use the
same logger used elsewhere (e.g., console.error or the module's logger) to log
the error and optionally rethrow or propagate a failure signal instead of
silently dropping it; reference the startCallByRoom and startCall methods and
the getUidDirectMessage call to locate where to add the await/try-catch.

---

Nitpick comments:
In `@app/lib/methods/enterpriseModules.ts`:
- Around line 65-68: Extract the magic string 'teams-voip' into a top-level
named constant (e.g., LICENSE_TEAMS_VOIP_ENTERPRISE) and replace its direct
usage in isVoipModuleAvailable() and any other usages such as in
useMediaCallPermission.ts / useMediaCallPermission so all checks reference the
new constant instead of the literal string; update exports/imports as needed to
keep consistent naming with existing constants
LICENSE_OMNICHANNEL_MOBILE_ENTERPRISE and LICENSE_LIVECHAT_ENTERPRISE.

In `@ios/Libraries/AppDelegate`+Voip.swift:
- Around line 51-62: The call uses a magic literal reason: 1 when ending the
placeholder call in the reportPlaceholderCallAndEnd closure; replace that magic
number with the named CXCallEndedReason value (e.g., use
CXCallEndedReason.failed.rawValue or a file-local constant like let
placeholderEndReason = CXCallEndedReason.failed.rawValue) and pass that to
RNCallKeep.endCall(withUUID:callUUID, reason: placeholderEndReason) so the
intent is explicit in AppDelegate+Voip.swift and the
RNCallKeep.endCall(withUUID:reason:) call is clear.
🪄 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: 6357a235-b425-450c-8676-a32bbf736daa

📥 Commits

Reviewing files that changed from the base of the PR and between 1e2e6e8 and d922827.

📒 Files selected for processing (6)
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/methods/enterpriseModules.ts
  • app/lib/services/restApi.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • ios/Libraries/AppDelegate+Voip.swift
💤 Files with no reviewable changes (1)
  • app/lib/services/restApi.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/lib/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.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/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.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/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in app/containers/ directory

Files:

  • app/containers/NewMediaCall/CreateCall.tsx
app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/lib/services/voip/MediaSessionInstance.ts
🧠 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/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • ios/Libraries/AppDelegate+Voip.swift
📚 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/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP

Applied to files:

  • app/lib/methods/enterpriseModules.ts
  • app/containers/NewMediaCall/CreateCall.tsx
📚 Learning: 2026-04-07T17:49:25.836Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-07T17:49:25.836Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Use explicit error handling with try/catch blocks for async operations

Applied to files:

  • app/containers/NewMediaCall/CreateCall.tsx
🔇 Additional comments (2)
app/lib/methods/enterpriseModules.test.ts (1)

1-24: LGTM!

Clear, focused unit coverage for both branches of isVoipModuleAvailable(); store setup is properly isolated via beforeEach -> clearEnterpriseModules().

app/containers/NewMediaCall/CreateCall.tsx (1)

18-30: LGTM — proper async error surfacing.

await, try/catch, dismiss-on-success, and localized fallback message are all in order and align with the new Promise<void> signature of startCall.

One optional consideration: e.message coming from the underlying media-signaling/WebRTC layer can be a raw technical string. If that matters for UX, consider always showing I18n.t('VoIP_Call_Issue') and logging e.message separately — but keeping as-is is fine if the PR intent is to surface the underlying reason.

Comment thread ios/Libraries/AppDelegate+Voip.swift Outdated
- AppDelegate+Voip.swift: route PushKit completion() through CallKit onReportComplete on happy path, matching the error path and Apple's PushKit iOS 13+ requirement
- MediaSessionInstance.ts: drop redundant Promise.resolve wrapper around SDK startCall (returns Promise<void>); attach error handler in startCallByRoom to avoid unhandled rejection now that startCall is async
- enterpriseModules.test.ts: add regression-guard case for empty enterpriseModules returning false

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ios/Libraries/AppDelegate+Voip.swift (1)

5-27: Prefer private over fileprivate for the top-level helper.

SwiftLint flags private_over_fileprivate on line 6. Since reportVoipIncomingCallToCallKit is only used within this file, a top-level private declaration has the same effective visibility and is the idiomatic choice.

Proposed change
-fileprivate func reportVoipIncomingCallToCallKit(
+private func reportVoipIncomingCallToCallKit(
🤖 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 5 - 27, The helper
function reportVoipIncomingCallToCallKit is declared fileprivate but is only
used within this file; change its access level to private to satisfy SwiftLint's
private_over_fileprivate rule and follow Swift idioms by replacing fileprivate
with private on the reportVoipIncomingCallToCallKit declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ios/Libraries/AppDelegate`+Voip.swift:
- Around line 5-27: The helper function reportVoipIncomingCallToCallKit is
declared fileprivate but is only used within this file; change its access level
to private to satisfy SwiftLint's private_over_fileprivate rule and follow Swift
idioms by replacing fileprivate with private on the
reportVoipIncomingCallToCallKit declaration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 95fa6087-b208-407b-980f-08217f6a8325

📥 Commits

Reviewing files that changed from the base of the PR and between d922827 and ee26ebd.

📒 Files selected for processing (3)
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • ios/Libraries/AppDelegate+Voip.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/lib/methods/enterpriseModules.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
📜 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). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
🧠 Learnings (2)
📓 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
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/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP
📚 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:

  • ios/Libraries/AppDelegate+Voip.swift
🪛 SwiftLint (0.63.2)
ios/Libraries/AppDelegate+Voip.swift

[Warning] 6-6: Prefer private over fileprivate declarations

(private_over_fileprivate)

🔇 Additional comments (1)
ios/Libraries/AppDelegate+Voip.swift (1)

46-91: LGTM — PushKit completion ordering is now correct on all paths.

Both the unparseable-payload and expired-payload paths now report a placeholder call to CallKit and defer completion() until after reportNewIncomingCall completes (via onReportComplete), and the happy path routes completion() through onReportComplete: { completion() } as well. This satisfies Apple's iOS 13+ PushKit contract (report to CallKit before signaling PushKit completion) and avoids the app-termination risk from the earlier synchronous-completion bug. The extracted reportVoipIncomingCallToCallKit helper also nicely removes the prior duplication.

…e tests

CreateCall.handleCall became async in this PR to surface startCall failures via showErrorAlert. The existing tests fire the press synchronously and assert on hideActionSheetRef/mockHideActionSheet before the awaited microtask resolves. Wrap the press in `await act(async () => { ... })` so the async handler flushes before assertions.
Switch `await act(async () => { fireEvent.press(...) })` to `fireEvent.press(...); await act(() => Promise.resolve())`. The async callback had no await and tripped `require-await`; a non-async arrow returning a resolved promise still flushes pending microtasks so the now-async `handleCall` can resolve before assertions.
@diegolmello diegolmello had a problem deploying to official_android_build April 20, 2026 18:07 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to experimental_android_build April 20, 2026 18:07 — with GitHub Actions Failure
@diegolmello diegolmello temporarily deployed to experimental_ios_build April 20, 2026 18:07 — with GitHub Actions Inactive

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx (1)

416-416: Add explicit return type annotation to async test callback.

The Jest callback at line 416 is async but lacks the explicit Promise<void> return type annotation required by TypeScript guidelines. Similar patterns appear elsewhere in this test file as well.

♻️ Proposed fix
-	it('user peer: press Call → startCall fires newCall → navigates to CallView', async () => {
+	it('user peer: press Call → startCall fires newCall → navigates to CallView', async (): Promise<void> => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx` at line
416, The async Jest test callback for the test named 'user peer: press Call →
startCall fires newCall → navigates to CallView' (the async function passed to
it in VoipCallLifecycle.integration.test.tsx) must include an explicit
Promise<void> return type; update the async arrow function signature to add :
Promise<void>. Also scan the same file for other async Jest callbacks (other
it/describe/test callbacks) and add Promise<void> return annotations to each
async test callback function (e.g., any async () => {...} used in tests) to
satisfy the TypeScript guideline.
app/containers/NewMediaCall/CreateCall.test.tsx (1)

102-102: Add explicit return types to async test callbacks.

Lines 102 and 117 declare async arrow functions without explicit return type annotations. Add Promise<void> to match TypeScript requirements.

♻️ Suggested fix
-	it('should call startCall with user type when user peer is selected and pressed', async () => {
+	it('should call startCall with user type when user peer is selected and pressed', async (): Promise<void> => {
-	it('should call startCall when SIP peer is selected and pressed', async () => {
+	it('should call startCall when SIP peer is selected and pressed', async (): Promise<void> => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/NewMediaCall/CreateCall.test.tsx` at line 102, The async test
callbacks for the two `it(...)` blocks (including the one titled "should call
startCall with user type when user peer is selected and pressed") lack explicit
return types; update each async arrow function to declare a return type of
Promise<void> (e.g., change `async () => { ... }` to `async (): Promise<void> =>
{ ... }`) so the test functions (`it` callbacks) satisfy TypeScript's explicit
return type requirement.
🤖 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/containers/NewMediaCall/CreateCall.test.tsx`:
- Line 102: The async test callbacks for the two `it(...)` blocks (including the
one titled "should call startCall with user type when user peer is selected and
pressed") lack explicit return types; update each async arrow function to
declare a return type of Promise<void> (e.g., change `async () => { ... }` to
`async (): Promise<void> => { ... }`) so the test functions (`it` callbacks)
satisfy TypeScript's explicit return type requirement.

In `@app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx`:
- Line 416: The async Jest test callback for the test named 'user peer: press
Call → startCall fires newCall → navigates to CallView' (the async function
passed to it in VoipCallLifecycle.integration.test.tsx) must include an explicit
Promise<void> return type; update the async arrow function signature to add :
Promise<void>. Also scan the same file for other async Jest callbacks (other
it/describe/test callbacks) and add Promise<void> return annotations to each
async test callback function (e.g., any async () => {...} used in tests) to
satisfy the TypeScript guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8acbd92e-6d64-418a-ab29-bbf4eb224f2b

📥 Commits

Reviewing files that changed from the base of the PR and between ee26ebd and cc8a013.

📒 Files selected for processing (2)
  • app/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
📜 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). (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🧰 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/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
**/*.{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/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in app/containers/ directory

Files:

  • app/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
**/*.{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/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
🧠 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
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/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP
📚 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/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
📚 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/sagas/videoConf.ts|app/lib/methods/videoConf.ts) : Manage video conferencing via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts using server-managed Jitsi integration; do not conflate with VoIP

Applied to files:

  • app/containers/NewMediaCall/CreateCall.test.tsx
  • app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
📚 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/index.tsx : Configure Redux provider, theme, navigation, and notifications in app/index.tsx

Applied to files:

  • app/containers/NewMediaCall/CreateCall.test.tsx
🔇 Additional comments (2)
app/containers/NewMediaCall/CreateCall.test.tsx (1)

2-2: LGTM — the async flush matches the updated call flow.

Importing act and flushing after fireEvent.press makes the assertions observe handleCall after its awaited startCall completes.

Also applies to: 111-111, 126-126

app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx (1)

15-15: LGTM — the integration test now waits for the async call path.

The act import and post-press microtask flush align the assertions with the awaited startCall pipeline.

Also applies to: 432-432

@diegolmello diegolmello merged commit eefbb9b into feat.voip-lib-new Apr 20, 2026
7 of 11 checks passed
@diegolmello diegolmello deleted the cursor/b082663a branch April 20, 2026 18:32
@github-actions

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat Experimental 4.72.0.108576

diegolmello added a commit that referenced this pull request Apr 22, 2026
…/Decline (#7215)

* merge feat.voip-lib

* feat(voip): enhance call handling with UUID mapping and event listeners

* Base call UI

* feat(voip): integrate Zustand for call state management and enhance CallView UI

* feat(voip): add simulateCall function for mock call handling in UI development

* refactor(CallView): update button handlers and improve UI responsiveness

* Add pause-shape-unfilled icon

* Base CallHeader

* toggleFocus

* collapse buttons

* Header components

* Hide header when no call

* Timer

* Add use memo

* Add voice call item on sidebar

* cleanup

* Temp use @rocket.chat/media-signaling from .tgz

* cleanup

* Check module and permissions to enable voip

* Refactor stop method to use optional chaining for media signal listeners

* voip push first test

* Add VoIP call handling with pending call management

- Implemented VoIP push notification handling in index.js, including storing call info for later processing.
- Added CallKeep event handlers for answering and ending calls from a cold start.
- Introduced a new CallIdUUID module to convert call IDs to deterministic UUIDs for compatibility with CallKit.
- Created a pending call store to manage incoming calls when the app is not fully initialized.
- Updated deep linking actions to include VoIP call handling.
- Enhanced MediaSessionInstance to process pending calls and manage call states effectively.

* Remove pending store and create getInitialEvents on app/index

* Attempt to make iOS calls work from cold state

* lint and format

* Patch callkeep ios

* Temp send iOS voip push token on gcm

* Temp fix require cycle

* chore: format code and fix lint issues [skip ci]

* CallIDUUID module on android and voip push

* Add setCallUUID on useCallStore to persist calls accepted on native Android

* remove callkeep from notification

* Android Incoming Call UI POC

* Refactor VoIP handling: Migrate VoIP-related classes to a new package structure, removing deprecated modules and consolidating functionality. Update imports in MainApplication and NotificationIntentHandler to reflect changes. This cleanup enhances code organization and prepares for future VoIP feature enhancements.

* Remove VoipForegroundService

* cleanup and use caller instead of callerName

* Cleanup and make iOS build again

* Refactor VoIP handling: Remove unused event emissions for call answered and declined, switch from SharedPreferences to in-memory storage for pending VoIP call data, and update method signatures for better clarity. This cleanup enhances performance and prepares for future VoIP feature improvements.

* Refactor VoIP handling: Introduce a new VoipPayload class to encapsulate call data, streamline notification processing, and enhance method signatures across the VoIP module. This update improves code clarity and prepares for future feature enhancements.

* Migrate react-native-voip-push-notifications to VoipModule

* Refactor VoIP module: Update package structure by moving VoipTurboPackage to the main package and removing the obsolete NativeVoipSpec class. Adjust imports in MainApplication and VoipModule to reflect these changes, enhancing code organization and maintainability.

* Unify emitters

* Move CallKeep listeners from MediaSessionInstance to getInitialEvents

* Clear callkeep on endcall

* Unify getInitialEvents logic

* getInitialEvents -> MediaCallEvents

* chore: format code and fix lint issues [skip ci]

* feat(Android): Add full screen incoming call (#6977)

* feat: Update call UI (#6990)

* feat: Handle audio routing, e.g., Bluetooth headset vs. internal speaker switching (#6992)

* fix: empty space when not on call (#6993)

* feat: Dialpad (#7000)

* action: organized translations

* feat: start call (#7024)

* chore: format code and fix lint issues

* feat: Pre flight (#7038)

* action: organized translations

* feat: Receive voip push notifications from backend (#7045)

* feat: Refactor media session handling and improve disconnect logic (#7065)

* feat: Control incoming call from native (#7066)

* feat: Voice message blocks (#7057)

* feat: native accept success event (#7068)

* feat(voip): call waiting, busy detection, and videoconf blocking (#7077)

* action: organized translations

* feat(voip): tap-to-hide call controls with animations (#7078)

* feat(voip): navigate to call DM from message button and header (#7082)

* feat(voip): tablet and landscape layout (#7110)

* chore: develop into feat.voip-lib-new (RN 81 + Expo 54 + reanimated 4 + true-sheet + iOS 26) (#7114)

* chore: format code and fix lint issues

* feat(voip): android landscape layout for IncomingCallActivity (#7116)

* Update agents files

* feat(voip): Support a11y (#7106)

* Fix content cutting on iOS on some edge cases

* pods

* Ignore .worktrees on jest

* chore: Merge develop into feat.voip-lib-new (#7129)

* fix(voip): show CallKit UI when call is active in background (#7128)

* chore: Update media-signaling to 0.2.0 (#7153)

* feat(voip): migrate iOS accept/reject from DDP to REST (#7124)

* Fix icons

* feat(voip): migrate Android accept/reject from DDP to REST (#7127)

* test(voip): integration tests for CallView pipeline (#7161)

* feat(voip): display video conf provider as subtitle (#7160)

* fix(voip): CallView button grid and correct landscape/dialpad layouts (#7164)

* fix(voip): prevent stale MMKV cache on Android first-install accept

MMKVKeyManager.initialize ran in MainApplication.onCreate before the JS
engine started and opened the default MMKV file via the Tencent 1.2 JAR
when it was still empty. Tencent caches instances per-ID in a singleton
registry, so that empty-state view was held for the rest of the process.
JS later wrote credentials through react-native-mmkv (MMKV Core 2.0),
which has its own separate registry. When a VoIP push arrived,
Ejson.getMMKV() got the cached empty Tencent instance and reported
"No userId found in MMKV for server". Closing and reopening the app
cleared the cache, which is why only the very first call after install
failed.

Drop the open/verify block — the encryption key is already cached from
SecureKeystore, so no MMKV handle is needed here. The first Tencent
instance is now created inside Ejson.getMMKV() after JS has written,
so it scans the file fresh.

* fix(voip): prevent duplicate ringtone on Android incoming call (#7158)

* fix(voip): set explicit snaps for NewMediaCall bottom sheet (#7165)

* Update app/lib/services/voip/MediaSessionStore.ts

Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>

* fix: make startVoipFork reactive to permissions-changed (#7151)

* fix(android): remove MediaProjectionService from merged manifest (#7190)

* fix(voip): Phone account creation (#7170)

* feat: add Enable Mobile Ringing toggle in user preferences (#7155)

* fix(voip): ship blockers for PushKit, licensing, outbound calls, push tokens (#7167)

* fix(android): Play Store mic discoverability, safer FCM logs, avatar auth via headers (#7171)

* fix(ios): serialize VoipService bridge statics (#7169)

* fix(voip): Android DDP thread safety and VoipPayload bundle parity (#7168)

* chore(voip): dead-code and hygiene sweep (#7174)

* refactor(voip): decouple navigateToCallRoom from Redux and backfill REST/connect tests (#7176)

* test(voip): tighten ringing endCall assertion and add VideoConf VoIP-lock saga coverage (#7177)

* fix(ios): harden VoIP DDP WebSocket client on receive failures and TLS (#7173)

* refactor(voip): MediaCallEvents Redux adapters and resetVoipState (#7178)

* refactor(voip): decouple peer autocomplete from Redux; simplify NewMediaCall (#7175)

* fix(ios): add NS_SWIFT_NAME to Challenge.runChallenge for Swift 6.2 compatibility

Swift 6.2 (Xcode 26.x / macos-26 runner) auto-renames the Objective-C
method runChallenge:didReceiveChallenge:completionHandler: to
run(_:didReceive:completionHandler:) when imported into Swift.

Add NS_SWIFT_NAME to explicitly pin the Swift import name, preventing
the compiler from applying its heuristics. This keeps the existing
Swift call site in DDPClient.swift working without changes.

* fix(ios): cancel old URLSession/webSocketTask before reconnecting in DDPClient.connect (#7197)

* fix(ios): add NSLock to nativeAcceptHandledCallIds and 10s REST timeout to handleNativeAccept (#7198)

* feat(android): create VoipCallService with FOREGROUND_SERVICE_MICROPHONE (#7199)

* fix(android): start VoipCallService on accept, stop on hangup/timeout, install end-call listener (#7200)

* fix(voip): enable DM nav for users with SIP extension (#7203)

* fix(android): handle null VoiceConnection in answerIncomingCall, notify JS (#7201)

* fix(voip): resolve closure capture ordering in handleNativeAccept (#7209)

* fix(android): integrate VoIP modules with SSL-pinned OkHttpClient (#7208)

* fix(push): gate id and voipToken behind server version checks, fix VideoConf caller extra (#7210)

* fix(voip): remove sensitive data from production logs (#7207)

* fix(android): remove isRunning guard + add double-tap guard on Accept/Decline

- VoipCallService: remove if (!isRunning) guard, call startForeground unconditionally
  (idempotent on Android, fixes Android 14+ foreground service requirement)
- IncomingCallActivity: add AtomicBoolean guard on handleAccept/handleDecline
  to prevent double-tap from triggering multiple service starts

---------

Co-authored-by: diegolmello <diegolmello@users.noreply.github.com>
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant