Skip to content

fix: make startVoipFork reactive to permissions-changed#7151

Merged
diegolmello merged 12 commits into
feat.voip-lib-newfrom
fix.voip-permissions
Apr 20, 2026
Merged

fix: make startVoipFork reactive to permissions-changed#7151
diegolmello merged 12 commits into
feat.voip-lib-newfrom
fix.voip-permissions

Conversation

@diegolmello

@diegolmello diegolmello commented Apr 14, 2026

Copy link
Copy Markdown
Member

Proposed changes

Refactor startVoipFork to call a plain async checkVoipPermission() that reads state directly via reduxStore.getState(). Add a DDP listener on stream-notify-logged for permissions-changed events so VoIP availability re-checks whenever the server updates user permissions. A guard prevents redundant MediaSessionInstance.init() when an instance already exists. The listener is captured in module scope and stopped on re-subscribe and on logout to avoid leaks and duplicates.

Issue(s)

How to test or reproduce

  1. Log in as a user without the VoIP permission — confirm mediaSessionInstance stays reset.
  2. From admin, grant the VoIP permission to the user's role; verify the mobile client reacts to permissions-changed and initializes VoIP without requiring a reconnect/restart.
  3. Revoke the VoIP permission — verify mediaSessionInstance.reset() is called.
  4. Log out and log in again — verify no duplicate permission listeners accumulate.

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

  • Addressed CodeRabbit review feedback (import syntax fix, lifecycle management for DDP permission listener).
  • Fixed ESLint failure: corrected import path in MediaSessionInstance.ts (../../services/restApi../restApi).
  • Fixed Prettier failure: removed extra blank line before init() method.
  • Replaced silent .catch(() => {}) with .catch(e => log(e)) in stopVoipPermissionListener.
  • Simplified checkVoipPermission: inlined single-use permission role vars, extracted canUseVoip boolean, applied early-return pattern for clarity.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced VOIP system robustness by improving initialization logic, handling edge cases where user data may be unavailable, and preventing duplicate instance creation.
    • Implemented real-time permission change detection to dynamically update VOIP feature availability.
    • Improved media signal transport mechanism for more reliable communication.

Comment thread app/sagas/login.js Outdated
import { mediaSessionInstance } from '../lib/services/voip/MediaSessionInstance';
import { hasPermission } from '../lib/methods/helpers/helpers';
import { mediaSessionStore } from '../lib/services/voip/MediaSessionStore';
import store as reduxStore from '../lib/store/auxStore';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

wrong store

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The pull request refactors VOIP permission checking from a Redux-saga generator approach to an async helper that directly reads Redux state. It adds listener registration for permission-change events with proper lifecycle management during logout and account deletion. Additionally, VOIP signal transport is updated to route through stream-notify-user method calls with subscription handling for media-signal events.

Changes

Cohort / File(s) Summary
VOIP Permission Management
app/sagas/login.js
Replaced Redux-saga permission checks with async helper reading state directly. Added listener registration for permission-change DDP events and lifecycle cleanup via stopVoipPermissionListener() during logout and account deletion. Implemented early-return patterns for defensive VOIP initialization.
VOIP Signal Transport
app/lib/services/voip/MediaSessionInstance.ts
Added signal routing through stream-notify-user method call with userId parameter. Wired real-time subscription handling for media-signal events via SDK stream listener, filtering and processing received signals.

Sequence Diagram

sequenceDiagram
    actor Login as Login Flow
    participant ReduxStore as Redux Store
    participant LoginSaga as Login Saga
    participant VoipHelper as checkVoipPermission
    participant DDP as DDP/SDK Stream
    participant MediaSession as MediaSessionInstance
    
    Login->>LoginSaga: Trigger login saga
    LoginSaga->>VoipHelper: call checkVoipPermission()
    VoipHelper->>ReduxStore: getState() to read userId & permissions
    ReduxStore-->>VoipHelper: Return state
    VoipHelper-->>LoginSaga: Permission check result
    
    alt Permission granted
        LoginSaga->>MediaSession: init(userId)
        MediaSession->>DDP: Setup stream listener for permissions-changed
        DDP-->>MediaSession: Listener registered
    end
    
    par Permission-change monitoring
        DDP->>DDP: Emit permissions-changed event
        DDP->>LoginSaga: Trigger listener
        LoginSaga->>VoipHelper: call checkVoipPermission()
        VoipHelper->>ReduxStore: getState() (re-check permissions)
    end
    
    alt Logout or delete account
        LoginSaga->>DDP: stopVoipPermissionListener()
        DDP-->>LoginSaga: Listener unsubscribed
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: make startVoipFork reactive to permissions-changed' directly and clearly summarizes the main change: adding reactive behavior to the startVoipFork saga to respond to permission-changed events, which is the core objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

@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: 2

🤖 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/sagas/login.js`:
- Around line 271-280: The DDP listener registered in startVoipFork via
sdk.current.onStreamData is never unsubscribed, causing memory leaks and stale
callbacks; change startVoipFork to capture the returned unsubscribe function
from sdk.current.onStreamData, wrap the subscription in a try/finally (or use
takeLatest-style lifecycle), and call the unsubscribe in the finally block (or
when the saga is cancelled/logged out) so the listener is removed when the saga
ends; locate startVoipFork and sdk.current.onStreamData in login.js to implement
this cleanup.
- Line 45: The import line using "import store as reduxStore from
'../lib/store/auxStore';" is invalid; update the import to match auxStore's
export style—if auxStore exports a named symbol "store" use "import { store as
reduxStore } from '../lib/store/auxStore';" otherwise if it exports a default
use "import reduxStore from '../lib/store/auxStore';" and ensure all usages of
reduxStore in this file remain unchanged.
🪄 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: 42a6c3f0-493d-4f85-90cb-15ce2fd3e3f0

📥 Commits

Reviewing files that changed from the base of the PR and between 2143aba and 1f2ac4f.

📒 Files selected for processing (1)
  • app/sagas/login.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/sagas/login.js
**/*.{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/sagas/login.js
**/*.{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/sagas/login.js
🧠 Learnings (5)
📓 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
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/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state
📚 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/sagas/login.js
📚 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/sagas/login.js
📚 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/**/*.{ts,tsx} : Place Redux sagas in app/sagas/ directory with separate files for init, login, rooms, messages, encryption, deepLinking, and videoConf side effects

Applied to files:

  • app/sagas/login.js
📚 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/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state

Applied to files:

  • app/sagas/login.js
🪛 Biome (2.4.10)
app/sagas/login.js

[error] 45-45: expected from but instead found as

(parse)


[error] 45-45: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 45-45: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)


[error] 45-45: Expected a semicolon or an implicit semicolon after a statement, but found none

(parse)

🔇 Additional comments (1)
app/sagas/login.js (1)

247-269: LGTM!

The async permission check logic is well-structured with proper error handling. The guard at lines 259-261 is a sensible optimization to avoid redundant re-initialization when an instance already exists.

Comment thread app/sagas/login.js Outdated

@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/sagas/login.js (1)

440-441: ⚠️ Potential issue | 🟡 Minor

Missing VoIP listener cleanup on account deletion.

handleDeleteAccount should also call stopVoipPermissionListener() for consistency with handleLogout. Without this, the DDP listener may persist and attempt to invoke checkVoipPermission() after the account is deleted.

🛡️ Proposed fix
 const handleDeleteAccount = function* handleDeleteAccount() {
+	stopVoipPermissionListener();
 	yield put(encryptionStop());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/sagas/login.js` around lines 440 - 441, handleDeleteAccount currently
only yields put(encryptionStop()) and omits cleaning up the VoIP listener;
update the generator function handleDeleteAccount to also call
stopVoipPermissionListener() (same placement used in handleLogout) so the DDP
listener is unsubscribed and won't later invoke checkVoipPermission() after
account deletion, and ensure stopVoipPermissionListener is imported/available in
this module.
🤖 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/sagas/login.js`:
- Around line 273-278: The stopVoipPermissionListener function currently
swallows errors via an empty .catch(() => {}); update this to handle and log
errors instead: when voipPermissionListener.then(listener => listener.stop())
rejects, catch the error and call the project's logger (or console.error if no
logger is available) with a clear message including the error and context (e.g.,
"Failed to stop VoIP permission listener"). Ensure voipPermissionListener is
still set to null after handling the error so cleanup proceeds reliably.

---

Outside diff comments:
In `@app/sagas/login.js`:
- Around line 440-441: handleDeleteAccount currently only yields
put(encryptionStop()) and omits cleaning up the VoIP listener; update the
generator function handleDeleteAccount to also call stopVoipPermissionListener()
(same placement used in handleLogout) so the DDP listener is unsubscribed and
won't later invoke checkVoipPermission() after account deletion, and ensure
stopVoipPermissionListener is imported/available in this module.
🪄 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: 3b8447d8-3a29-4516-ade6-ac19e1585429

📥 Commits

Reviewing files that changed from the base of the PR and between 1f2ac4f and 718ab08.

📒 Files selected for processing (1)
  • app/sagas/login.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/sagas/login.js
**/*.{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/sagas/login.js
**/*.{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/sagas/login.js
🧠 Learnings (7)
📓 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/sagas/login.js
📚 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/sagas/login.js
📚 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/**/*.{ts,tsx} : Place Redux sagas in app/sagas/ directory with separate files for init, login, rooms, messages, encryption, deepLinking, and videoConf side effects

Applied to files:

  • app/sagas/login.js
📚 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/store/**/*.{ts,tsx} : Configure Redux store in app/lib/store/ with middleware for saga, app state, and internet state

Applied to files:

  • app/sagas/login.js
📚 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/reducers/**/*.{ts,tsx} : Place Redux reducers in app/reducers/ directory with separate files for app, login, connect, rooms, encryption, and other state slices

Applied to files:

  • app/sagas/login.js
📚 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/selectors/**/*.{ts,tsx} : Place Redux selectors in app/selectors/ directory and memoize them using reselect

Applied to files:

  • app/sagas/login.js
🔇 Additional comments (3)
app/sagas/login.js (3)

44-45: LGTM!

The import syntax is now correct. The named export is properly aliased using { store as reduxStore }.


247-269: LGTM!

The refactored checkVoipPermission function correctly:

  • Uses async/await with proper try/catch error handling
  • Guards against redundant mediaSessionInstance.init() calls when an instance already exists
  • Properly resets the session when permissions are not granted

364-364: LGTM!

Good placement of stopVoipPermissionListener() at the start of logout to ensure cleanup happens before other teardown operations.

Comment thread app/sagas/login.js

@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: 4

Caution

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

⚠️ Outside diff range comments (1)
app/sagas/login.js (1)

247-269: ⚠️ Potential issue | 🟠 Major

mediaSessionInstance.init(userId) is not awaited; errors escape the try/catch.

init is async and performs IO (applyRestStateSignals, mediaSessionStore.setWebRTCProcessorFactory, stream subscribe). Calling it without await here means:

  • Rejections become unhandled promise rejections (not caught by the surrounding try/catch).
  • A rapid permissions-changed sequence can interleave multiple in-flight init calls with the getCurrentInstance() guard racing against them.
♻️ Proposed fix
-			if (mediaSessionStore.getCurrentInstance()) {
-				return;
-			}
-			mediaSessionInstance.init(userId);
+			if (mediaSessionStore.getCurrentInstance()) {
+				return;
+			}
+			await mediaSessionInstance.init(userId);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/sagas/login.js` around lines 247 - 269, The call to
mediaSessionInstance.init(userId) in checkVoipPermission is not awaited, so
initialization errors escape the try/catch and concurrent permission changes can
race multiple inits; update checkVoipPermission to await
mediaSessionInstance.init(userId) and keep the existing
mediaSessionStore.getCurrentInstance() guard around init to avoid races (e.g.,
re-check getCurrentInstance() after awaiting or use a short-circuit that awaits
init only if there is no current instance), ensuring any rejection is caught by
the surrounding try/catch and preventing overlapping in-flight initializations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/lib/services/voip/MediaSessionInstance.ts`:
- Line 69: Remove the stray blank line immediately before the init method in the
MediaSessionInstance class so the method declaration is directly adjacent to the
previous member; this fixes the Prettier/ESLint formatting failure by
eliminating the extra newline before init in MediaSessionInstance.
- Line 19: The import in MediaSessionInstance.ts uses a useless path segment;
update the import that brings in mediaCallsStateSignals to use the correct
relative path from this file by changing the module specifier
'../../services/restApi' to '../restApi' so ESLint path-resolution passes and
mediaCallsStateSignals continues to be imported from restApi.

In `@app/sagas/login.js`:
- Around line 280-290: The DDP listener is registered outside the saga lifecycle
in startVoipFork which bypasses saga cancellation; either ensure
stopVoipPermissionListener() is also called from other teardown flows like
handleDeleteAccount (and any forced-disconnect handlers) or move the
onStreamData subscription into a dedicated saga that yields take/takeEvery and
uses cancelled() to call stopVoipPermissionListener() and avoid dangling
listeners — update startVoipFork/handleLoginSuccess to fork that managed saga
(or add stopVoipPermissionListener() calls to handleDeleteAccount and
forced-disconnect paths) so checkVoipPermission is never invoked after session
disposal.
- Around line 271-278: The stopVoipPermissionListener assumes
voipPermissionListener is a Promise (uses .then) but sdk.onStreamData() can
return a plain object; update stopVoipPermissionListener to handle both
thenables and direct listener objects: if voipPermissionListener has a .then
function await/then it to get the listener and call listener.stop(), otherwise
call voipPermissionListener.stop() directly; always catch and log any errors
(use console.error or the app logger) instead of swallowing them, and finally
set voipPermissionListener = null; ensure this logic is applied to the symbol
voipPermissionListener and function stopVoipPermissionListener and aligns with
how sdk.onStreamData() is consumed (as in room.ts).

---

Outside diff comments:
In `@app/sagas/login.js`:
- Around line 247-269: The call to mediaSessionInstance.init(userId) in
checkVoipPermission is not awaited, so initialization errors escape the
try/catch and concurrent permission changes can race multiple inits; update
checkVoipPermission to await mediaSessionInstance.init(userId) and keep the
existing mediaSessionStore.getCurrentInstance() guard around init to avoid races
(e.g., re-check getCurrentInstance() after awaiting or use a short-circuit that
awaits init only if there is no current instance), ensuring any rejection is
caught by the surrounding try/catch and preventing overlapping in-flight
initializations.
🪄 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: a5b1f349-69bc-4bdc-ad90-afec8a7678f7

📥 Commits

Reviewing files that changed from the base of the PR and between 718ab08 and 2dd22b8.

📒 Files selected for processing (4)
  • app/lib/services/voip/MediaSessionInstance.ts
  • app/lib/services/voip/MediaSessionStore.ts
  • app/sagas/login.js
  • packages/rocket.chat-media-signaling-0.2.0-rc.0.tgz
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: format
🧰 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/services/voip/MediaSessionStore.ts
  • app/sagas/login.js
  • 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/services/voip/MediaSessionStore.ts
  • app/sagas/login.js
  • 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/services/voip/MediaSessionStore.ts
  • app/lib/services/voip/MediaSessionInstance.ts
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/MediaSessionStore.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/services/voip/MediaSessionStore.ts
  • app/sagas/login.js
  • app/lib/services/voip/MediaSessionInstance.ts
🪛 ESLint
app/lib/services/voip/MediaSessionInstance.ts

[error] 19-19: Useless path segments for "../../services/restApi", should be "../restApi"

(import/no-useless-path-segments)


[error] 69-70: Delete

(prettier/prettier)

🪛 GitHub Check: ESLint and Test / run-eslint-and-test
app/lib/services/voip/MediaSessionInstance.ts

[failure] 19-19:
Useless path segments for "../../services/restApi", should be "../restApi"


[failure] 69-69:
Delete

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

70-71: LGTM!

Disabling requestInitialStateSignals aligns with the new flow where applyRestStateSignals() is invoked explicitly in MediaSessionInstance.init() after REST bootstrap.

Comment thread app/lib/services/voip/MediaSessionInstance.ts Outdated
Comment thread app/lib/services/voip/MediaSessionInstance.ts Outdated
Comment thread app/sagas/login.js
Comment thread app/sagas/login.js
diegolmello and others added 11 commits April 20, 2026 11:09
…ry fields

Iteration 2 fixes: wrap REST block in null guard, replace field accesses with
as any casts since role/muted/held/contact/setMuted/setHeld were removed
from IClientMediaCall in 0.2.0-rc.0 library.
Refactor startVoipFork to call plain async checkVoipPermission().
Add DDP listener on stream-notify-logged for permissions-changed.
- correct named import syntax for reduxStore
- clean up DDP permission listener on re-subscribe and logout
…ission

- Fix import path in MediaSessionInstance.ts: '../../services/restApi' → '../restApi' (ESLint failure)
- Remove extra blank line before init() method (Prettier failure)
- Log listener stop errors instead of swallowing: .catch(() => {}) → .catch(e => log(e))
- Simplify checkVoipPermission: inline single-use permission vars, named canUseVoip bool, early-return pattern
onStreamData returns { stop: () => void } synchronously (same pattern as
connect.ts and MediaSessionInstance.ts). Remove the invalid .then().catch()
chain and call .stop() directly.

Also stop the listener in handleDeleteAccount to prevent leaks on account
deletion paths that bypass handleLogout.
…essionConfig

Property no longer exists in the updated @rocket.chat/media-signaling type after
feat.voip-lib-new rebased onto the newer package version.
@diegolmello diegolmello merged commit 0e6f6ae into feat.voip-lib-new Apr 20, 2026
5 of 6 checks passed
@diegolmello diegolmello deleted the fix.voip-permissions branch April 20, 2026 14:17
@diegolmello diegolmello had a problem deploying to experimental_ios_build April 20, 2026 14:20 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to official_android_build April 20, 2026 14:20 — with GitHub Actions Failure
@diegolmello diegolmello had a problem deploying to experimental_android_build April 20, 2026 14:20 — with GitHub Actions Failure
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