Skip to content

feat(voip): reject incoming calls when user is already busy#7075

Open
diegolmello wants to merge 7 commits intofeat.voip-lib-newfrom
feat.single-call
Open

feat(voip): reject incoming calls when user is already busy#7075
diegolmello wants to merge 7 commits intofeat.voip-lib-newfrom
feat.single-call

Conversation

@diegolmello
Copy link
Copy Markdown
Member

@diegolmello diegolmello commented Mar 30, 2026

Proposed changes

This change implements defense-in-depth so a second incoming VoIP call (or distracting videoconf UI) does not interrupt the user while they are already on a call—whether that call is in Rocket.Chat VoIP, another app (phone, FaceTime, WhatsApp, etc.), or still ringing.

Native (iOS)

  • Detect busy state with CXCallObserver (any non-ended call counts as busy).
  • After PushKit reports the call, reject via rejectBusyCall() when busy; skip stashing initial events for JS when busy so cold-start handling stays consistent.

Native (Android)

  • hasActiveCall() considers the app’s own connections (including ringing/dialing), then TelecomManager.isInCall() only when READ_PHONE_STATE is granted, with try/catch for SecurityException, plus AudioManager mode fallback when Telecom is unavailable.
  • Busy path rejects via DDP without surfacing full incoming UI where applicable.
  • Bugfix: avoids crashing on isInCall() when permission was missing.

JavaScript

  • MediaSessionInstance newCall (callee): if the Zustand call store already has another active callId or a pending native accept, reject via media-signaling and end CallKeep.
  • videoConf saga onDirectCall: early return when VoIP is active or native accept is pending so videoconf does not compete with VoIP (VoIP > videoconf).

Android permission UX

  • Fire-and-forget runtime READ_PHONE_STATE request with i18n rationale (voipPhoneStatePermission), triggered from VoIP entry points (e.g. starting a VoIP call); session flag avoids repeat prompts after deny; degrades silently if denied.

Callers receive a normal rejected signal (same as a manual decline). No call-waiting, no “busy” presence, and no in-app message to the callee that a second call was dropped (per PRD out-of-scope).

Issue(s)

How to test or reproduce

VoIP while on VoIP

  1. User A and B on an active Rocket.Chat VoIP call.
  2. User C calls User A. Expect: second call auto-rejected; A’s current call uninterrupted; C sees rejected behavior.

VoIP while native call / other app

  1. User A on a cellular call, FaceTime, or another VoIP app (where observable).
  2. Incoming Rocket.Chat VoIP to A. Expect: rejected at native layer where detection applies; no duplicate call UI storm.

Android permission

  1. Fresh install or revoke READ_PHONE_STATE.
  2. Start or receive VoIP per wired entry points. Expect: optional permission dialog with rationale strings; no crash when calling isInCall(); fallback still attempts busy detection via app connections + AudioManager where applicable.

JS guard / race

  1. Accept a call from native UI; before JS fully binds, trigger a second incoming call. Expect: second call rejected (store / nativeAcceptedCallId guard).

Videoconf vs VoIP

  1. User A on active VoIP.
  2. Incoming direct videoconf / onDirectCall path. Expect: videoconf notification suppressed; VoIP unchanged.

Regression

  1. End call; next incoming VoIP should ring and behave as before.

Relevant automated tests (if present on the branch): MediaSessionInstance.test.ts, voipPhoneStatePermission.test.ts, voipBlocksIncomingVideoconf.test.ts, plus native behavior verified on device.

Screenshots

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

  • Implements the scope described in the internal PRD “Reject Second Incoming Call When Busy” (native + JS VoIP + videoconf guard, Android permission safety and optional runtime prompt).
  • Stacking: progress.md notes this work on feat.single-call based on feat.voip-lib-new; if the GitHub PR compares to develop, reviewers should expect a larger VoIP footprint than this narrative alone—tighten the PR base or split commits if the team wants a PR that only contains the busy-rejection delta.

Summary by CodeRabbit

  • New Features

    • Incoming VoIP calls are now automatically rejected when you're already in an active call
    • Added phone state permission access to accurately detect ongoing calls and prevent conflicts
  • Improvements

    • Incoming video conference calls are blocked while on an active VoIP call

Check permission before TelecomManager.isInCall on API 26+, catch
SecurityException, and use AudioManager MODE_IN_COMMUNICATION fallback
on all API levels when Telecom is unavailable or denied.

Made-with: Cursor
Add requestPhoneStatePermission() with session-scoped prompt, i18n rationale,
and Jest coverage. English strings only per issue scope.

Made-with: Cursor
Call requestPhoneStatePermission() at the start of MediaSessionInstance.startCall()
so Android can use TelecomManager for busy detection. Videoconf is unchanged.

Tests: extend MediaSessionInstance.test with startCall permission assertion.
Made-with: Cursor
…VoIP

Align native busy detection with iOS (non-ended calls): reject a second
incoming push while the first is still ringing or dialing, not only
active/hold.

Made-with: Cursor
When the user is already in a call, still satisfy PushKit by reporting
to CallKit then rejecting, but do not store the payload for
getInitialEvents. Keeps DDP listener/timeout so the reject signal can
be sent. Clear matching initial events in rejectBusyCall as a safeguard.

Made-with: Cursor
Added a new function to determine if incoming videoconference calls should be blocked based on active VoIP calls or pending native accept states. Updated the videoConf saga to utilize this new logic. Additionally, created unit tests for the new functionality to ensure correct behavior in various scenarios.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

Walkthrough

This change implements busy-call rejection for incoming VoIP calls on both iOS and Android platforms. When a user is already in an active call, incoming calls are automatically rejected with server-side signaling rather than being presented to the user.

Changes

Cohort / File(s) Summary
Android VoIP Call Rejection
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
Added rejectBusyCall() public function and private helpers hasActiveCall() and hasSystemLevelActiveCallIndicators() to detect active calls via VoiceConnectionService state and system-level indicators. Incoming call handler now conditionally rejects when user is busy.
iOS VoIP Call Rejection
ios/Libraries/VoipService.swift, ios/Libraries/AppDelegate+Voip.swift
Added hasActiveCall() and rejectBusyCall() methods to VoipService. Modified prepareIncomingCall() to gate storeInitialEvents based on storeEventsForJs flag. AppDelegate now checks active call state before handling incoming push and conditionally rejects busy calls.
React Native Phone State Permission
app/lib/methods/voipPhoneStatePermission.ts, app/lib/methods/voipPhoneStatePermission.test.ts
New module providing requestPhoneStatePermission() for Android-only, session-scoped permission requests. Uses i18n localization for dialog labels and includes test coverage validating request behavior and single-prompt-per-session pattern.
Call State Management & Guards
app/lib/services/voip/MediaSessionInstance.ts, app/lib/services/voip/MediaSessionInstance.test.ts, app/lib/services/voip/voipBlocksIncomingVideoconf.ts, app/lib/services/voip/voipBlocksIncomingVideoconf.test.ts
Added voipBlocksIncomingVideoconf() function to prevent direct videoconf handling during active VoIP calls. Enhanced MediaSessionInstance to request phone state permission on startCall() and reject incoming calls when user is already on a different call. Includes comprehensive test coverage for busy-call guard logic.
Saga Integration
app/sagas/videoConf.ts
Added early-return guard in onDirectCall saga to skip incoming direct-call processing when voipBlocksIncomingVideoconf() returns true.
Localization
app/i18n/locales/en.json
Added two new i18n entries: Phone_state_permission_title and Phone_state_permission_message for phone state permission UI text.

Sequence Diagrams

sequenceDiagram
    participant iOS as iOS OS
    participant App as App (AppDelegate)
    participant VoipService as VoipService
    participant Server as DDP Server
    participant User as User/CallKit

    iOS->>App: Incoming PushKit VoIP notification
    App->>VoipService: Check hasActiveCall()
    VoipService->>VoipService: Observe CallKit state
    VoipService-->>App: true (user busy)
    App->>VoipService: prepareIncomingCall(payload, storeEventsForJs: false)
    Note over VoipService: Skip storing events<br/>Schedule timeout<br/>Start DDP listener
    App->>User: reportNewIncomingCall() to CallKit
    App->>VoipService: rejectBusyCall(payload)
    VoipService->>VoipService: Cancel timeout<br/>Clear tracking state<br/>RNCallKeep.endCall()
    VoipService->>Server: Send/queue DDP reject signal
    Server-->>VoipService: Ack
    Note over User: Call appears briefly<br/>then ends in CallKit
Loading
sequenceDiagram
    participant Android as Android OS
    participant App as App (onMessageReceived)
    participant VoipNotif as VoipNotification
    participant VoiceService as VoiceConnectionService
    participant Server as DDP Server
    participant User as User/CallKit

    Android->>App: Incoming VoIP notification
    App->>VoipNotif: Check hasActiveCall()
    VoipNotif->>VoiceService: Query currentConnections state
    alt Active call detected
        VoiceService-->>VoipNotif: true (ringing/active/holding)
        VoipNotif-->>App: true
        App->>VoipNotif: rejectBusyCall(context, payload)
        VoipNotif->>VoipNotif: Cancel timeout<br/>Start end-listener
        VoipNotif->>Server: Send/queue DDP reject
    else No active call
        VoiceService-->>VoipNotif: Check AudioManager mode
        VoipNotif-->>App: false
        App->>App: showIncomingCall(payload)
        Note over User: Call presented to user
    end
Loading
sequenceDiagram
    participant JS as JavaScript Layer
    participant MediaSession as MediaSessionInstance
    participant PermModule as voipPhoneStatePermission
    participant Android as Android OS
    participant CallStore as useCallStore
    participant VoipService as VoipService

    JS->>MediaSession: startCall(userId, actor)
    MediaSession->>PermModule: requestPhoneStatePermission()
    PermModule->>Android: PermissionsAndroid.request(READ_PHONE_STATE)
    Note over PermModule: Once per session only
    Android-->>PermModule: User response
    PermModule-->>MediaSession: Permission requested
    
    MediaSession->>VoipService: Create newCall handler
    
    alt Incoming call arrives
        VoipService->>MediaSession: on('newCall', callee)
        MediaSession->>CallStore: getState()
        CallStore-->>MediaSession: current call info
        alt Different call already active
            MediaSession->>MediaSession: call.reject()
            MediaSession->>Android: RNCallKeep.endCall()
            Note over MediaSession: Reject busy incoming
        else No active call
            MediaSession->>MediaSession: Register stateChange listener
            Note over MediaSession: Accept incoming call
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Suggested labels

type: feature

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% 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 accurately and concisely summarizes the main change: implementing automatic rejection of incoming VoIP calls when the user is already on a call.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
app/lib/methods/voipPhoneStatePermission.ts (1)

17-21: Consider handling or explicitly ignoring the returned Promise.

PermissionsAndroid.request() returns a Promise that is currently not awaited or caught. While the fire-and-forget design is intentional per the PR objectives, an unhandled rejection could trigger warnings in some environments.

🔧 Optional: Explicitly suppress errors
-	PermissionsAndroid.request(PermissionsAndroid.PERMISSIONS.READ_PHONE_STATE, {
+	PermissionsAndroid.request(PermissionsAndroid.PERMISSIONS.READ_PHONE_STATE, {
 		buttonPositive: 'Ok',
 		message: i18n.t('Phone_state_permission_message'),
 		title: i18n.t('Phone_state_permission_title')
-	});
+	}).catch(() => {
+		// Fire-and-forget: permission denial is acceptable
+	});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/methods/voipPhoneStatePermission.ts` around lines 17 - 21, The call
to PermissionsAndroid.request(...) currently returns a Promise that is neither
awaited nor handled; explicitly consume it to avoid unhandled-rejection warnings
by appending a deliberate handler (for example, using the void operator or
adding .catch(() => {}) to PermissionsAndroid.request) so the intent is clear
and errors are suppressed; update the call site where PermissionsAndroid.request
is invoked (the invocation passing i18n.t('Phone_state_permission_message') and
i18n.t('Phone_state_permission_title')) to explicitly handle or ignore the
returned Promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 537-545: The rejectBusyCall path should not call
startListeningForCallEnd() because that function tears down the singleton
ddpClient and will steal the native listener for another ringing call; remove
the startListeningForCallEnd(context, payload) call from rejectBusyCall and
instead either 1) implement a new helper (e.g.,
startPassiveCallEndWatcherWithoutTearDown or startNonDestructiveCallEndListener)
that only registers a listener for this call without tearing down ddpClient and
call that here, or 2) skip starting any listener at all and rely on existing
listeners for the active call; ensure cancelTimeout(payload.callId) remains and
keep the existing sendRejectSignal/queueRejectSignal logic in rejectBusyCall.
- Around line 510-529: The hasSystemLevelActiveCallIndicators function misses
detecting cellular calls when READ_PHONE_STATE is denied because it only checks
AudioManager.MODE_IN_COMMUNICATION; update the AudioManager fallback (in
hasSystemLevelActiveCallIndicators) to also treat AudioManager.MODE_IN_CALL as
an active call by checking audio?.mode against both MODE_IN_COMMUNICATION and
MODE_IN_CALL and return true if either matches so incoming VoIP calls won’t
interrupt cellular calls.

In `@app/lib/methods/voipPhoneStatePermission.test.ts`:
- Line 11: The ESLint error comes from using the forbidden import() type
annotation in the jest.requireActual call; remove the generic type argument from
the spread expression (replace "jest.requireActual<typeof
import('./helpers')>('./helpers')" with simply "jest.requireActual('./helpers')"
or, if you prefer explicit typing, use "jest.requireActual('./helpers') as
Record<string, unknown>" ), and apply the same change for the equivalent usages
at the other two places (the other jest.requireActual spreads at lines that
reference './helpers') so the type-import syntax is no longer used.

In `@ios/Libraries/VoipService.swift`:
- Around line 145-154: prepareIncomingCall currently always calls
startListeningForCallEnd even when storeEventsForJs is false, and that helper
tears down the shared ddpClient/observedIncomingCall singleton causing a busy
second call to steal the listener; change prepareIncomingCall so it only calls
startListeningForCallEnd when storeEventsForJs is true (i.e., keep the busy path
from re-arming/tearing down the shared listener), retaining
scheduleIncomingCallTimeout(for:) for the busy path and still calling
storeInitialEvents(payload) only when storeEventsForJs is true to avoid
impacting ddpClient/observedIncomingCall for existing calls.

---

Nitpick comments:
In `@app/lib/methods/voipPhoneStatePermission.ts`:
- Around line 17-21: The call to PermissionsAndroid.request(...) currently
returns a Promise that is neither awaited nor handled; explicitly consume it to
avoid unhandled-rejection warnings by appending a deliberate handler (for
example, using the void operator or adding .catch(() => {}) to
PermissionsAndroid.request) so the intent is clear and errors are suppressed;
update the call site where PermissionsAndroid.request is invoked (the invocation
passing i18n.t('Phone_state_permission_message') and
i18n.t('Phone_state_permission_title')) to explicitly handle or ignore the
returned Promise.
🪄 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: 380ab9b4-a908-4c35-895f-62ca099f3a25

📥 Commits

Reviewing files that changed from the base of the PR and between 81dba28 and 8fc00f5.

📒 Files selected for processing (11)
  • android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
  • app/i18n/locales/en.json
  • app/lib/methods/voipPhoneStatePermission.test.ts
  • app/lib/methods/voipPhoneStatePermission.ts
  • app/lib/services/voip/MediaSessionInstance.test.ts
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/voipBlocksIncomingVideoconf.test.ts
  • app/lib/services/voip/voipBlocksIncomingVideoconf.ts
  • app/sagas/videoConf.ts
  • ios/Libraries/AppDelegate+Voip.swift
  • ios/Libraries/VoipService.swift
📜 Review details
🧰 Additional context used
🪛 ESLint
app/lib/methods/voipPhoneStatePermission.test.ts

[error] 11-11: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)


[error] 29-29: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)


[error] 51-51: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)

🪛 GitHub Actions: Format Code with Prettier
app/lib/methods/voipPhoneStatePermission.test.ts

[error] 11-11: ESLint error: import() type annotations are forbidden. (typescript-eslint/consistent-type-imports)

🪛 GitHub Check: format
app/lib/methods/voipPhoneStatePermission.test.ts

[failure] 51-51:
import() type annotations are forbidden


[failure] 29-29:
import() type annotations are forbidden


[failure] 11-11:
import() type annotations are forbidden

🔇 Additional comments (10)
app/lib/services/voip/voipBlocksIncomingVideoconf.ts (1)

1-7: LGTM!

Clean utility function that correctly checks both active VoIP call state and pending native accept. The logic aligns with the store's state shape where both fields are initialized as null and must be explicitly set.

app/i18n/locales/en.json (1)

665-666: LGTM!

Clear, user-friendly permission rationale text that explains the purpose without being overly technical. The message appropriately conveys the benefit to users.

app/lib/services/voip/voipBlocksIncomingVideoconf.test.ts (1)

1-42: LGTM!

Good test coverage for all three logical branches of the function. The mock setup correctly isolates the store dependency.

app/sagas/videoConf.ts (1)

50-52: LGTM!

Clean early-return guard that prioritizes VoIP calls over incoming videoconf. The placement at the function start ensures the check happens before any other processing or state mutations.

app/lib/services/voip/MediaSessionInstance.ts (2)

88-98: LGTM!

Well-implemented busy guard that correctly handles both active calls and pending native accepts. The nullish coalescing chain existingCall?.callId ?? nativeAcceptedCallId ?? null properly covers the time window where native accept is set but the call object isn't yet bound. Rejecting and ending CallKeep ensures clean state on both the signaling and native layers.


152-156: LGTM!

Good placement of the phone state permission request at the start of outgoing calls. This aligns with the PR's session-scoped, fire-and-forget permission flow.

ios/Libraries/AppDelegate+Voip.swift (1)

43-67: LGTM!

Correct implementation of iOS busy-call handling that respects Apple's PushKit requirement to always report incoming calls to CallKit. The sequence is proper:

  1. Check busy state before reporting
  2. Conditionally store events for JS (skip when busy)
  3. Always report to CallKit (required)
  4. Reject if busy after reporting

The storeEventsForJs: !isBusy parameter prevents the busy-rejected call from surfacing in JS initial events.

android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)

490-504: Nice catch including ringing/dialing in own-app busy detection.

This closes the second-incoming-while-ringing gap and brings Android in line with the iOS behavior.

ios/Libraries/VoipService.swift (1)

138-143: Using CXCallObserver here makes sense.

Treating any non-ended CallKit call as busy matches the PR requirement and covers system as well as third-party calls.

app/lib/services/voip/MediaSessionInstance.test.ts (1)

52-80: Nice coverage around the new busy guard.

The added harness exercises the reject/endCall path, the nativeAcceptedCallId exceptions, and the fire-and-forget permission request without needing the full signaling stack.

Also applies to: 128-161, 368-474

Comment on lines +510 to +529
private fun hasSystemLevelActiveCallIndicators(context: Context): Boolean {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
val granted = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_PHONE_STATE) ==
PackageManager.PERMISSION_GRANTED
if (granted) {
val telecom = context.getSystemService(Context.TELECOM_SERVICE) as? TelecomManager
try {
if (telecom?.isInCall == true) {
return true
}
} catch (e: SecurityException) {
Log.w(TAG, "TelecomManager.isInCall not allowed", e)
}
}
}
val audio = context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager
if (audio?.mode == AudioManager.MODE_IN_COMMUNICATION) {
return true
}
return false
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt | head -550 | tail -100

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 1927


🏁 Script executed:

rg "hasActiveCall|hasSystemLevelActiveCallIndicators" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt -B 5 -A 15

Repository: RocketChat/Rocket.Chat.ReactNative

Length of output: 2958


🌐 Web query:

Android AudioManager MODE_IN_CALL vs MODE_IN_COMMUNICATION when used

💡 Result:

  • MODE_IN_CALL: Use only for a real telephony (PSTN/cellular) call handled by the system dialer/telephony stack. The Android framework explicitly notes this mode “should only be used by the telephony application” because it can make radio-layer call audio feed into the platform mixer, and it’s restricted (only selectable by the main telephony app with MODIFY_PHONE_STATE). [1]

  • MODE_IN_COMMUNICATION: Use for app-to-app communication like VoIP / audio-video chat (e.g., WebRTC, SIP). It indicates “an audio/video chat or VoIP call is established” and is the appropriate mode for third-party calling apps to get “call-like” routing/processing behavior. [1]

  • Practical permission/behavior difference:

    • If you try to set MODE_IN_CALL as a normal app, the system will reject it without MODIFY_PHONE_STATE. [2]
    • While the device is already in MODE_IN_CALL or MODE_IN_COMMUNICATION, the system also prevents other apps from changing the mode unless they’re privileged (or the current “mode owner”), to avoid apps disrupting an ongoing call. [2]

Sources:
[1] AOSP AudioManager.java (mode definitions + setMode() guidance/restriction note) (android.googlesource.com)
[2] AOSP AudioService.java (setMode permission enforcement / call-active change restrictions) (android.googlesource.com)

Citations:


Add MODE_IN_CALL check to detect cellular calls when READ_PHONE_STATE is denied.

When READ_PHONE_STATE permission is denied or unavailable, the fallback to AudioManager only checks MODE_IN_COMMUNICATION. However, system cellular calls are marked as MODE_IN_CALL, which means hasActiveCall() returns false and an incoming Rocket.Chat call can interrupt an active cellular call. Include MODE_IN_CALL in the AudioManager check to detect all active calls regardless of permission state.

Suggested fix
             val audio = context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager
-            if (audio?.mode == AudioManager.MODE_IN_COMMUNICATION) {
+            if (
+                audio?.mode == AudioManager.MODE_IN_COMMUNICATION ||
+                audio?.mode == AudioManager.MODE_IN_CALL
+            ) {
                 return 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
private fun hasSystemLevelActiveCallIndicators(context: Context): Boolean {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
val granted = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_PHONE_STATE) ==
PackageManager.PERMISSION_GRANTED
if (granted) {
val telecom = context.getSystemService(Context.TELECOM_SERVICE) as? TelecomManager
try {
if (telecom?.isInCall == true) {
return true
}
} catch (e: SecurityException) {
Log.w(TAG, "TelecomManager.isInCall not allowed", e)
}
}
}
val audio = context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager
if (audio?.mode == AudioManager.MODE_IN_COMMUNICATION) {
return true
}
return false
private fun hasSystemLevelActiveCallIndicators(context: Context): Boolean {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
val granted = ContextCompat.checkSelfPermission(context, Manifest.permission.READ_PHONE_STATE) ==
PackageManager.PERMISSION_GRANTED
if (granted) {
val telecom = context.getSystemService(Context.TELECOM_SERVICE) as? TelecomManager
try {
if (telecom?.isInCall == true) {
return true
}
} catch (e: SecurityException) {
Log.w(TAG, "TelecomManager.isInCall not allowed", e)
}
}
}
val audio = context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager
if (
audio?.mode == AudioManager.MODE_IN_COMMUNICATION ||
audio?.mode == AudioManager.MODE_IN_CALL
) {
return true
}
return false
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`
around lines 510 - 529, The hasSystemLevelActiveCallIndicators function misses
detecting cellular calls when READ_PHONE_STATE is denied because it only checks
AudioManager.MODE_IN_COMMUNICATION; update the AudioManager fallback (in
hasSystemLevelActiveCallIndicators) to also treat AudioManager.MODE_IN_CALL as
an active call by checking audio?.mode against both MODE_IN_COMMUNICATION and
MODE_IN_CALL and return true if either matches so incoming VoIP calls won’t
interrupt cellular calls.

Comment on lines +537 to +545
fun rejectBusyCall(context: Context, payload: VoipPayload) {
Log.d(TAG, "Rejected busy call ${payload.callId} — user already on a call")
cancelTimeout(payload.callId)
startListeningForCallEnd(context, payload)
if (isDdpLoggedIn) {
sendRejectSignal(context, payload)
} else {
queueRejectSignal(context, payload)
}
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.

⚠️ Potential issue | 🟠 Major

Don't reuse startListeningForCallEnd() to reject the busy call.

startListeningForCallEnd() starts by tearing down the singleton ddpClient. If call A is still ringing and call B is auto-rejected as busy, B steals A's native listener, so hangup/accept updates for the first call are no longer observed until timeout.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`
around lines 537 - 545, The rejectBusyCall path should not call
startListeningForCallEnd() because that function tears down the singleton
ddpClient and will steal the native listener for another ringing call; remove
the startListeningForCallEnd(context, payload) call from rejectBusyCall and
instead either 1) implement a new helper (e.g.,
startPassiveCallEndWatcherWithoutTearDown or startNonDestructiveCallEndListener)
that only registers a listener for this call without tearing down ddpClient and
call that here, or 2) skip starting any listener at all and rely on existing
listeners for the active call; ensure cancelTimeout(payload.callId) remains and
keep the existing sendRejectSignal/queueRejectSignal logic in rejectBusyCall.

it('does not call PermissionsAndroid.request when not on Android', () => {
jest.resetModules();
jest.doMock('./helpers', () => ({
...jest.requireActual<typeof import('./helpers')>('./helpers'),
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.

⚠️ Potential issue | 🟡 Minor

Fix ESLint error: import() type annotations are forbidden.

The pipeline is failing due to @typescript-eslint/consistent-type-imports violations at lines 11, 29, and 51. The import() type annotation syntax is not allowed.

🔧 Proposed fix using `Record`

Since the spread is only used to preserve other exports, you can simplify by removing the type annotation entirely:

-		jest.doMock('./helpers', () => ({
-			...jest.requireActual<typeof import('./helpers')>('./helpers'),
-			isAndroid: false
-		}));
+		jest.doMock('./helpers', () => ({
+			...jest.requireActual('./helpers'),
+			isAndroid: false
+		}));

Apply the same change at lines 29 and 51.

📝 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
...jest.requireActual<typeof import('./helpers')>('./helpers'),
...jest.requireActual('./helpers'),
🧰 Tools
🪛 ESLint

[error] 11-11: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)

🪛 GitHub Actions: Format Code with Prettier

[error] 11-11: ESLint error: import() type annotations are forbidden. (typescript-eslint/consistent-type-imports)

🪛 GitHub Check: format

[failure] 11-11:
import() type annotations are forbidden

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/lib/methods/voipPhoneStatePermission.test.ts` at line 11, The ESLint
error comes from using the forbidden import() type annotation in the
jest.requireActual call; remove the generic type argument from the spread
expression (replace "jest.requireActual<typeof
import('./helpers')>('./helpers')" with simply "jest.requireActual('./helpers')"
or, if you prefer explicit typing, use "jest.requireActual('./helpers') as
Record<string, unknown>" ), and apply the same change for the equivalent usages
at the other two places (the other jest.requireActual spreads at lines that
reference './helpers') so the type-import syntax is no longer used.

Comment on lines +145 to 154
/// Prepares DDP listener and timeout for an incoming VoIP push. When `storeEventsForJs` is false
/// (e.g. user is already on a call and we will `rejectBusyCall` immediately), skip stashing payload
/// for `getInitialEvents` so JS does not treat an auto-rejected call as a real incoming ring.
public static func prepareIncomingCall(_ payload: VoipPayload, storeEventsForJs: Bool = true) {
if storeEventsForJs {
storeInitialEvents(payload)
}
scheduleIncomingCallTimeout(for: payload)
startListeningForCallEnd(payload: payload)
}
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.

⚠️ Potential issue | 🟠 Major

Don't arm the shared incoming-call listener for the busy path.

When storeEventsForJs is false, this still calls startListeningForCallEnd(payload), and that helper first tears down the existing singleton ddpClient/observedIncomingCall. If another Rocket.Chat call is already ringing, the busy second call steals that listener and the first call stops receiving native hangup/accept updates until timeout.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ios/Libraries/VoipService.swift` around lines 145 - 154, prepareIncomingCall
currently always calls startListeningForCallEnd even when storeEventsForJs is
false, and that helper tears down the shared ddpClient/observedIncomingCall
singleton causing a busy second call to steal the listener; change
prepareIncomingCall so it only calls startListeningForCallEnd when
storeEventsForJs is true (i.e., keep the busy path from re-arming/tearing down
the shared listener), retaining scheduleIncomingCallTimeout(for:) for the busy
path and still calling storeInitialEvents(payload) only when storeEventsForJs is
true to avoid impacting ddpClient/observedIncomingCall for existing calls.

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