feat: Refactor media session handling and improve disconnect logic

[diegolmello]

Proposed changes

This PR tightens VoIP / media-signaling lifecycle so teardown matches SDK disconnect and server or permission changes.

• connect.disconnect() now calls mediaSessionInstance.reset() after sdk.disconnect(), so VoIP state and listeners are cleared whenever the app disconnects through the connect service.
• Call sites that previously used sdk.disconnect() directly (logout, delete-account flow in login saga, useConnectServer) now use disconnect() from connect so VoIP reset always runs.
• MediaSessionInstance: unsubscribe from mediaSessionStore.onChange on teardown (fixes stacked handlers on re-init), null out listener references after stop, and reset() / stop() call mediaSessionStore.dispose() and reset the call store.
• MediaSessionStore.dispose(): end the signaling session, clear send-signal and WebRTC factory, and notify subscribers.
• selectServer: reset the media session when switching workspace before reconnecting.
• startVoipFork: mediaSessionInstance.reset() when the user does not have VoIP permissions, so prior init is not left active after permission loss.
• Tests: mock MediaSessionInstance in connect / connect.ios tests; add MediaSessionInstance.test.ts for init, signal routing, user switch, reset, and idempotent teardown.

Issue(s)

https://rocketchat.atlassian.net/browse/VMUX-60

How to test or reproduce

1. Log in on a server with VoIP enabled, then log out — confirm no duplicate or stale stream handlers after logging in again (e.g. repeated / unexpected signaling behavior).
2. Switch servers (multi-server) with VoIP — confirm VoIP re-initializes for the new workspace and does not keep state from the previous server.
3. Connect to a new workspace from New Server View (path that disconnects the previous session) — confirm a clean disconnect + reconnect.
4. If applicable: user loses VoIP permissions (or logs into a context without VoIP) after having had VoIP — confirm media session is torn down.
5. Run yarn test (including MediaSessionInstance.test.ts and connect tests).

Screenshots

N/A — behavioral / lifecycle changes, no UI changes.

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

selectServer calls mediaSessionInstance.reset() before connect(), and connect() still begins with disconnect(), which resets VoIP again — redundant but safe and idempotent. This can be simplified later to a single reset path if desired.

Summary by CodeRabbit

• Bug Fixes

  • Improved disconnect and logout flow with proper media session cleanup
  • Enhanced VoIP session lifecycle management when switching servers or logging out
  • Better state handling during account deletion and connection teardown

• Tests

  • Added comprehensive test coverage for media session lifecycle behavior

[diegolmello]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 26504cc4-85c7-4e40-a733-f362777f5798

📥 Commits

Reviewing files that changed from the base of the PR and between 8274083 and 40a4b0d.

📒 Files selected for processing (10)

• app/lib/methods/logout.ts
• app/lib/services/connect.ios.test.ts
• app/lib/services/connect.test.ts
• app/lib/services/connect.ts
• app/lib/services/voip/MediaSessionInstance.test.ts
• app/lib/services/voip/MediaSessionInstance.ts
• app/lib/services/voip/MediaSessionStore.ts
• app/sagas/login.js
• app/sagas/selectServer.ts
• app/views/NewServerView/hooks/useConnectServer.tsx

📜 Recent review details

🔇 Additional comments (14)

app/lib/services/voip/MediaSessionStore.ts (1)

96-105: LGTM!

The dispose() method correctly tears down all state: ends the session if present, nulls out references (sessionInstance, sendSignalFn, _webrtcProcessorFactory), and emits a change event to notify subscribers. The implementation is idempotent and follows the same pattern as makeInstance().

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

34-55: LGTM!

Calling reset() at the start of init() ensures idempotent reinitialization. The onChange subscription is properly tracked via mediaSessionStoreChangeUnsubscribe and will be cleaned up when reset() is called again or on teardown.

---

173-193: LGTM!

The reset() method comprehensively cleans up all resources:

• Unsubscribes from mediaSessionStore.onChange
• Stops the media signal listener
• Unsubscribes from Redux store subscriptions
• Calls mediaSessionStore.dispose() to end the signaling session
• Nulls out the instance reference
• Resets the Zustand call store

The method is idempotent due to null checks and can be safely called multiple times.

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

81-159: LGTM!

Good test coverage for MediaSessionInstance lifecycle:

• Verifies stream listener registration and session creation on init()
• Validates signal routing through sdk.methodCall
• Tests user switch properly calls endSession() on the previous session
• Confirms only one onChange handler remains active after re-init
• Verifies getInstance throws after reset() until init() is called again
• Tests idempotent reset() (no throw on double-call)

app/lib/services/connect.ts (1)

410-414: LGTM!

Centralizing mediaSessionInstance.reset() in disconnect() ensures all callers (logout, account deletion, server switching) consistently clean up VoIP state. The order is correct: disconnect SDK first, then reset media session.

app/views/NewServerView/hooks/useConnectServer.tsx (1)

4-4: LGTM!

Switching from sdk.disconnect() to the shared disconnect() import ensures VoIP media session cleanup happens when clearing the previous workspace before connecting to a new server.

Also applies to: 26-28

app/sagas/selectServer.ts (1)

154-154: LGTM!

Calling mediaSessionInstance.reset() directly here is appropriate. The saga manages its own connection flow and will call connect() later (which internally calls disconnect()). This ensures VoIP state from the previous workspace is cleared early in the server selection flow, before loading user credentials for the new workspace.

app/lib/services/connect.test.ts (1)

3-6: LGTM!

The mock prevents real VoIP initialization during determineAuthType tests. Mocking only reset and init is sufficient since those are the only methods potentially invoked through the module's imports.

app/lib/services/connect.ios.test.ts (1)

3-6: LGTM!

Consistent mock setup with connect.test.ts, ensuring iOS-specific determineAuthType tests don't trigger real VoIP initialization.

app/lib/methods/logout.ts (2)

9-9: LGTM!

The import aligns with the PR objective to centralize disconnect logic through connect.disconnect(), which now handles VoIP teardown via mediaSessionInstance.reset().

---

114-116: No changes needed.

The disconnect() function is not asynchronous—it returns an Action type and does not return a Promise. Therefore, await is not appropriate and the current code is correct.

			> Likely an incorrect or invalid review comment.

app/sagas/login.js (3)

35-35: LGTM!

The import aligns with the PR objective to use the centralized disconnect() function that handles VoIP teardown.

---

228-243: LGTM! Good defensive cleanup for permission loss scenarios.

The new else branch ensures that when VoIP is unavailable or the user lacks required permissions, any previously initialized media session is properly reset. This prevents stale VoIP state from persisting across permission changes or module availability changes.

---

416-421: No changes needed — disconnect() is synchronous.

The disconnect() function at app/lib/services/connect.ts:410 is synchronous and does not return a Promise. It immediately calls sdk.disconnect(), resets mediaSessionInstance, and returns. Since it completes synchronously, calling it directly without yield is correct and matches the behavior intended for synchronous operations.

			> Likely an incorrect or invalid review comment.

---

Walkthrough

The changes introduce proper cleanup for VoIP media sessions during workspace switching. A new reset() method is added to MediaSessionInstance that tears down VoIP resources. The disconnect() function now invokes this reset. Multiple integration points are updated to ensure media session cleanup happens at logout, server selection, and account deletion.

Changes

Cohort / File(s)	Summary
VoIP Media Session Lifecycle app/lib/services/voip/MediaSessionInstance.ts, app/lib/services/voip/MediaSessionInstance.test.ts, app/lib/services/voip/MediaSessionStore.ts	Refactored media session cleanup: stop() is replaced with a public reset() method that properly disposes stores and clears references. MediaSessionStore.dispose() added to terminate sessions and nullify resources. Comprehensive test suite added to verify initialization, re-initialization with different users, and reset semantics.
Disconnect Service & Tests app/lib/services/connect.ts, app/lib/services/connect.test.ts, app/lib/services/connect.ios.test.ts	Enhanced disconnect() to call mediaSessionInstance.reset() after SDK disconnect before returning result. Jest mocks added for MediaSessionInstance in iOS and general test files to isolate unit tests from real media session behavior.
Workspace Transition Integration app/lib/methods/logout.ts, app/sagas/login.js, app/sagas/selectServer.ts, app/views/NewServerView/hooks/useConnectServer.tsx	Updated all call sites to use extracted disconnect() function instead of sdk.disconnect() method. Added explicit mediaSessionInstance.reset() calls in login saga's VoIP fork and server selection flow to ensure cleanup during workspace transitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

type: feature, type: bug

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: refactoring media session handling and improving disconnect logic by adding mediaSessionInstance.reset() calls and replacing direct sdk.disconnect() calls with a dedicated disconnect function.
Linked Issues check	✅ Passed	The PR comprehensively addresses VMUX-60's requirement to prevent cross-workspace state leakage by implementing deterministic teardown and reinitialization of media sessions across all disconnect paths (logout, delete-account, server switching, permission loss).
Out of Scope Changes check	✅ Passed	All changes directly support the core objective of preventing VoIP/media-signaling state leakage across workspaces; no unrelated modifications detected in the changeset.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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