Skip to content

fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) - #7380

Merged
diegolmello merged 4 commits into
developfrom
fix/deeplink-orphan-socket-clobber
Jun 3, 2026
Merged

fix: keep live socket on deeplink login (prevent orphan WebSocket clobber)#7380
diegolmello merged 4 commits into
developfrom
fix/deeplink-orphan-socket-clobber

Conversation

@diegolmello

@diegolmello diegolmello commented Jun 3, 2026

Copy link
Copy Markdown
Member

Proposed changes

Standalone fix extracted from #7311 (commit 29e4f8b).

Opening the app through an auth deeplink (and the call-push path) dispatched loginRequest before the WebSocket reached connected. The SDK login-guard then opened a second, orphaned socket. When the server later idle-timed-out the abandoned socket, its close flipped Redux to disconnected — so the app showed "Waiting for network" while a live, healthy socket still existed.

SDK patch (patches/@rocket.chat+sdk+1.3.3-mobile.patch, ddp driver)

  • onClose now ignores close events from a socket that has already been replaced (compares identity against the current connection), so a zombie socket's late close can't clobber the live connection.
  • open() tears down the previous connection's handlers and closes it before replacing it (mirrors forceReopen's teardown).

deepLinking saga (app/sagas/deepLinking.js)

  • Before dispatching the resume loginRequest (auth-deeplink and call-push paths), gate on the socket being connected: after SERVER.SELECT_SUCCESS, read state.meteor.connected and only take(METEOR.SUCCESS) when still disconnected. This both prevents login on a still-connecting socket (the orphan-socket clobber) and can't hang the splash when the socket connects before SELECT_SUCCESSMETEOR.SUCCESS fires once and never refires, so an unconditional take would wait forever. handleOpen's already-connected fast path skips the wait entirely.

Issue(s)

NATIVE-1232 · related: NATIVE-1123

How to test or reproduce

  1. Cold-start the app via an auth deeplink carrying a resume token (rocketchat://auth?...) for a workspace you are not currently connected to.
  2. Before this fix: a second (orphan) socket opens; when the server idle-times-out the abandoned socket, the app falls to "Waiting for network" even though a live socket exists. After this fix: login waits for connected first, only one socket is ever opened, and no false "Waiting for network" appears.
  3. Repeat via a call-push deeplink — same path, same expectation.
  4. Unit test: TZ=UTC pnpm test --testPathPattern='app/sagas/__tests__/deepLinking.test' → 12/12 pass. Coverage now includes the connect-before-SELECT_SUCCESS ordering race, a gate assertion (no loginRequest until connected), and first-ever coverage for handleClickCallPush.

Screenshots

n/a

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) — app/sagas/__tests__/deepLinking.test.ts
  • I have added necessary documentation (if applicable) — inline comments next to each wait/guard
  • Any dependent changes have been merged and published in downstream modules

Further comments

Cherry-picked from #7311 with git cherry-pick -x; the SDK-patch and saga changes apply cleanly. The handleOpen placement was adapted to develop's hostAlreadyConnected fast path (the wait sits inside the !hostAlreadyConnected branch). The wait was later hardened from an unconditional take(METEOR.SUCCESS) into a state.meteor.connected guard so a socket that connects before SELECT_SUCCESS can't make the saga block on a METEOR.SUCCESS that will never refire.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed cases where login proceeded before the socket was fully connected.
    • Improved reconnection handling with health-check probing and safer socket teardown to avoid stale-close interference and reduce false "Waiting for network" notices.
  • New Features

    • Added support for media-signal and media-calls event types.
  • Tests

    • Strengthened deep-linking tests to assert connection-before-login sequencing and race-order coverage.

…bber)

Auth-deeplink login dispatched loginRequest before the socket reached 'connected', so the SDK login-guard opened a second (orphan) socket. When the server later idle-timed-out the abandoned socket, its close flipped Redux to disconnected and the app showed "Waiting for network" while a live socket still existed.

- SDK (patch): onClose ignores close events from a non-current socket; open() tears down the previous connection before replacing it.

- deepLinking saga: wait for METEOR.SUCCESS ('connected') before dispatching the resume loginRequest (auth-deeplink and call-push paths); update saga test accordingly.

(cherry picked from commit 29e4f8b)
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Make deep-linking wait for the SDK socket to reach METEOR.SUCCESS before starting token-based login, refactor SDK Socket reconnection/teardown (probe/forceReopen, ignore orphan closes), and update deep-linking saga tests to simulate and assert the connected gating.

Changes

Connection State Sequencing for Deep Linking Authentication

Layer / File(s) Summary
SDK socket connection management
patches/@rocket.chat+sdk+1.3.3-mobile.patch
Socket now defensively tears down prior WebSockets, ignores orphan close events by passing the closed connection to onClose, adds probe() (ping/pong, 2s timeout), forceReopen() with synchronous disconnected/close emission and shared _reopenInFlight, and rewrites checkAndReopen() into an async freshness-bucketing flow. DDPDriver subscriptions add media-signal and media-calls.
Saga connection sequencing
app/sagas/deepLinking.js
handleOpen and handleClickCallPush now await types.METEOR.SUCCESS after types.SERVER.SELECT_SUCCESS when a token is present and the host was not already connected, before dispatching loginRequest.
Test updates for connected gating
app/sagas/__tests__/deepLinking.test.ts
Adds a Jest mock for ../../lib/database, imports connectSuccess and navigateToRoom, introduces setupRecordingStore to capture dispatched actions, dispatches connectSuccess()+microtask flush in multiple tests, and adds new tests for handleClickCallPush call-flow and race ordering.

Sequence Diagram

sequenceDiagram
  participant App as Deep Link Handler
  participant Saga as deepLinking saga
  participant Socket as SDK Socket

  App->>Saga: dispatch(SERVER.SELECT_SUCCESS)
  Saga->>Socket: await METEOR.SUCCESS
  Socket-->>Saga: emit METEOR.SUCCESS
  Saga->>App: dispatch(loginRequest)
  App->>Saga: dispatch(loginSuccess)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • OtavioStasiak
  • Rohit3523
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 accurately summarizes the main change: preventing orphan WebSocket from clobbering the live socket during deeplink login.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (2)
  • NATIVE-1232: Request failed with status code 401
  • NATIVE-1123: Request failed with status code 401

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.

Comments in the SDK patch and deepLinking test now describe only the code; removed the 'FIX A'/'Fix B' plan labels.
Comment thread app/sagas/deepLinking.js Outdated
Comment thread app/sagas/deepLinking.js Outdated
Co-authored-by: Diego Mello <diegolmello@gmail.com>

@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)
app/sagas/__tests__/deepLinking.test.ts (1)

205-216: ⚡ Quick win

The added connectSuccess() keeps the saga unblocked but doesn't actually verify the new METEOR.SUCCESS gating.

The saga parks at take(LOGIN.SUCCESS) after put(loginRequest), but take(LOGIN.SUCCESS) is satisfied directly by the test's loginSuccess() dispatch — it doesn't require loginRequest to have fired. So if the take(types.METEOR.SUCCESS) gating were removed from the saga, the connectSuccess() dispatch would become a harmless no-op and every one of these five tests would still pass. The behavior this PR introduces is therefore exercised but not asserted.

Consider adding an assertion that the saga stays parked before METEOR.SUCCESS — e.g. spy on store.dispatch (or assert via state) that loginRequest is not dispatched after selectServerSuccess but before connectSuccess(), then confirm it is dispatched after. That turns these into real regression tests for the orphan-socket fix.

This applies to all five regression tests (lines 208-211, 249-252, 286-289, 320-323, 360-363).

Want me to draft the dispatch spy assertions for one test so you can replicate across the five?

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/sagas/__tests__/deepLinking.test.ts` around lines 205 - 216, The test
currently dispatches selectServerSuccess then connectSuccess and finally
loginSuccess but never asserts that the saga's METEOR.SUCCESS gating actually
prevents loginRequest from being dispatched; update the test around
selectServerSuccess/connectSuccess/loginSuccess to spy on store.dispatch (e.g.,
jest.spyOn(store, 'dispatch') or mock the store dispatch) and assert that
loginRequest is NOT dispatched after selectServerSuccess and BEFORE
connectSuccess, then assert that loginRequest IS dispatched after you dispatch
connectSuccess (and before you call loginSuccess); reference the existing action
creators selectServerSuccess, connectSuccess, loginRequest, and loginSuccess
(and the saga's METEOR.SUCCESS gating) when adding these assertions and
replicate the same pattern for the other four tests mentioned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/sagas/__tests__/deepLinking.test.ts`:
- Around line 205-216: The test currently dispatches selectServerSuccess then
connectSuccess and finally loginSuccess but never asserts that the saga's
METEOR.SUCCESS gating actually prevents loginRequest from being dispatched;
update the test around selectServerSuccess/connectSuccess/loginSuccess to spy on
store.dispatch (e.g., jest.spyOn(store, 'dispatch') or mock the store dispatch)
and assert that loginRequest is NOT dispatched after selectServerSuccess and
BEFORE connectSuccess, then assert that loginRequest IS dispatched after you
dispatch connectSuccess (and before you call loginSuccess); reference the
existing action creators selectServerSuccess, connectSuccess, loginRequest, and
loginSuccess (and the saga's METEOR.SUCCESS gating) when adding these assertions
and replicate the same pattern for the other four tests mentioned.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf90b419-cd7f-42b1-8407-8a8b7687c90c

📥 Commits

Reviewing files that changed from the base of the PR and between acceba1 and 15aec04.

📒 Files selected for processing (2)
  • app/sagas/__tests__/deepLinking.test.ts
  • patches/@rocket.chat+sdk+1.3.3-mobile.patch
🚧 Files skipped from review as they are similar to previous changes (1)
  • patches/@rocket.chat+sdk+1.3.3-mobile.patch
📜 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 (4)
**/*.{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/__tests__/deepLinking.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{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

Use TypeScript with strict mode and baseUrl set to app/ for import resolution

Files:

  • app/sagas/__tests__/deepLinking.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use Prettier with tabs, single quotes, 130 char width, no trailing commas, arrow parens avoid, bracket same line
Use @rocket.chat/eslint-config base with React, React Native, TypeScript, Jest plugins

Files:

  • app/sagas/__tests__/deepLinking.test.ts
app/sagas/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Sagas for side effects should be placed in app/sagas/ directory (init, login, rooms, messages, encryption, deepLinking, videoConf)

Files:

  • app/sagas/__tests__/deepLinking.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:59-90
Timestamp: 2026-05-05T14:41:08.368Z
Learning: In `patches/rocket.chat+sdk+1.3.3-mobile.patch` (`rocket.chat/sdk` DDPDriver), `forceReopen()` intentionally sets `this.lastPing = 0` at teardown start. `Socket.onOpen` resets `this.lastPing = Date.now()` when the DDP connect handshake completes (inside `open()`), so `lastPing` is already fresh by the time `forceReopen()` resolves. Concurrent `checkAndReopen` calls during a reopen are handled by `_reopenInFlight`: the stale bucket routes them back into `forceReopen()`, which short-circuits via the in-flight promise guard — no double teardown. The design is intentionally self-healing and idempotent.
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:79-79
Timestamp: 2026-05-05T21:08:50.423Z
Learning: When reviewing `.patch` files for the `rocket.chat/sdk` package (e.g., `patches/rocket.chat+sdk+1.3.3-mobile.patch`), the sandbox's `node_modules` reflects the **pre-patch** state. Symbols that are pre-existing in the SDK source (such as `userDisconnectCloseCode = 4000` at line 43 of `node_modules/rocket.chat/sdk/lib/drivers/ddp.ts`) will not be found by grep against `node_modules`, causing false-positive "undefined symbol" findings. Always account for this when reviewing patch files against SDK internals.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/lib/services/sdk.ts : SDK for Rocket.Chat WebSocket real-time subscriptions should be in app/lib/services/sdk.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/sagas/**/*.{ts,tsx} : Sagas for side effects should be placed in app/sagas/ directory (init, login, rooms, messages, encryption, deepLinking, videoConf)

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/lib/services/connect.ts : Server connection management should be in app/lib/services/connect.ts

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
🔇 Additional comments (2)
app/sagas/__tests__/deepLinking.test.ts (2)

90-90: LGTM!


414-453: LGTM!

Also applies to: 460-478

The METEOR.SUCCESS take could hang the splash forever if the socket
reached 'connected' before SERVER.SELECT_SUCCESS. Skip the take when
state.meteor.connected is already true, in both handleOpen and
handleClickCallPush.

Add ordering-race and gate-assertion tests for handleOpen, plus
first-ever coverage for handleClickCallPush.

@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)
app/sagas/__tests__/deepLinking.test.ts (1)

563-584: 💤 Low value

Consider making this suite's mock setup self-contained.

This beforeEach resets only a subset of mocks and never pins sdk.current.client.host, so the suite implicitly relies on the prior describe's afterEach (Line 497) leaving host = '' and on the un-reset goRoom/canOpenRoom/waitForNavigationReady mocks. It works under top-to-bottom execution, but becomes order-dependent under .only/filtered runs or future reordering. Explicitly resetting the relevant mocks (or jest.resetAllMocks()) and asserting/setting host here would make the suite independent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/sagas/__tests__/deepLinking.test.ts` around lines 563 - 584, The
beforeEach mock setup is incomplete and leaves the suite order-dependent; update
the beforeEach in this test file to make it self-contained by calling
jest.resetAllMocks() (or explicitly reset mocks for goRoom, canOpenRoom,
waitForNavigationReady) and by explicitly setting or asserting
sdk.current.client.host to the desired value (e.g., '' or a specific host) so
tests don't rely on prior describe teardown; ensure you still reapply the
intended UserPreferences.getString, getServerById, getServerInfo,
navigateToRoom, and database.active.get mocks after the reset so the rest of the
setup remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/sagas/__tests__/deepLinking.test.ts`:
- Around line 563-584: The beforeEach mock setup is incomplete and leaves the
suite order-dependent; update the beforeEach in this test file to make it
self-contained by calling jest.resetAllMocks() (or explicitly reset mocks for
goRoom, canOpenRoom, waitForNavigationReady) and by explicitly setting or
asserting sdk.current.client.host to the desired value (e.g., '' or a specific
host) so tests don't rely on prior describe teardown; ensure you still reapply
the intended UserPreferences.getString, getServerById, getServerInfo,
navigateToRoom, and database.active.get mocks after the reset so the rest of the
setup remains intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 746b4138-c1ec-4597-a179-222617db6e18

📥 Commits

Reviewing files that changed from the base of the PR and between fb39169 and 5e12854.

📒 Files selected for processing (2)
  • app/sagas/__tests__/deepLinking.test.ts
  • app/sagas/deepLinking.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/sagas/deepLinking.js
📜 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 (4)
**/*.{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/__tests__/deepLinking.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{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

Use TypeScript with strict mode and baseUrl set to app/ for import resolution

Files:

  • app/sagas/__tests__/deepLinking.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,jsx}: Use Prettier with tabs, single quotes, 130 char width, no trailing commas, arrow parens avoid, bracket same line
Use @rocket.chat/eslint-config base with React, React Native, TypeScript, Jest plugins

Files:

  • app/sagas/__tests__/deepLinking.test.ts
app/sagas/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Sagas for side effects should be placed in app/sagas/ directory (init, login, rooms, messages, encryption, deepLinking, videoConf)

Files:

  • app/sagas/__tests__/deepLinking.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:59-90
Timestamp: 2026-05-05T14:41:08.368Z
Learning: In `patches/rocket.chat+sdk+1.3.3-mobile.patch` (`rocket.chat/sdk` DDPDriver), `forceReopen()` intentionally sets `this.lastPing = 0` at teardown start. `Socket.onOpen` resets `this.lastPing = Date.now()` when the DDP connect handshake completes (inside `open()`), so `lastPing` is already fresh by the time `forceReopen()` resolves. Concurrent `checkAndReopen` calls during a reopen are handled by `_reopenInFlight`: the stale bucket routes them back into `forceReopen()`, which short-circuits via the in-flight promise guard — no double teardown. The design is intentionally self-healing and idempotent.
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:79-79
Timestamp: 2026-05-05T21:08:50.423Z
Learning: When reviewing `.patch` files for the `rocket.chat/sdk` package (e.g., `patches/rocket.chat+sdk+1.3.3-mobile.patch`), the sandbox's `node_modules` reflects the **pre-patch** state. Symbols that are pre-existing in the SDK source (such as `userDisconnectCloseCode = 4000` at line 43 of `node_modules/rocket.chat/sdk/lib/drivers/ddp.ts`) will not be found by grep against `node_modules`, causing false-positive "undefined symbol" findings. Always account for this when reviewing patch files against SDK internals.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/lib/services/sdk.ts : SDK for Rocket.Chat WebSocket real-time subscriptions should be in app/lib/services/sdk.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/sagas/**/*.{ts,tsx} : Sagas for side effects should be placed in app/sagas/ directory (init, login, rooms, messages, encryption, deepLinking, videoConf)

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/lib/store/**/*.{ts,tsx} : Store configuration should include middleware (saga, app state, internet state) and be placed in app/lib/store/

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/lib/services/connect.ts : Server connection management should be in app/lib/services/connect.ts

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/sagas/videoConf.ts : VideoConf feature should be implemented via Redux actions/reducers/sagas in app/sagas/videoConf.ts and app/lib/methods/videoConf.ts

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/index.tsx : Redux provider, theme, navigation, and notifications setup should be in app/index.tsx

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-05-18T14:40:38.892Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-18T14:40:38.892Z
Learning: Applies to app/reducers/**/*.{ts,tsx} : Reducers should be placed in app/reducers/ directory and manage state shape (app, login, connect, rooms, encryption, etc.)

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.

Applied to files:

  • app/sagas/__tests__/deepLinking.test.ts
🔇 Additional comments (4)
app/sagas/__tests__/deepLinking.test.ts (4)

50-59: LGTM!


134-145: LGTM!


258-306: LGTM!


590-661: LGTM!

@diegolmello
diegolmello merged commit 80989f5 into develop Jun 3, 2026
5 of 8 checks passed
@diegolmello
diegolmello deleted the fix/deeplink-orphan-socket-clobber branch June 3, 2026 15:37
diegolmello added a commit that referenced this pull request Jun 3, 2026
#7311 carried the original cherry-pick base of the deeplink/orphan-socket
fix. The standalone PR #7380 refined it before merging to develop:
- guard the login gate against the connect-before-select race (METEOR.SUCCESS
  may fire before SERVER.SELECT_SUCCESS; the unconditional take would hang)
- added regression tests for that ordering + the call-push path
- clarified the SDK patch comments (FIX A primary/defense)

Bring app/sagas/deepLinking.js, its test, and the SDK patch in line with
develop's merged #7380 version so #7311 stops carrying a divergent copy.
@diegolmello diegolmello mentioned this pull request Jun 3, 2026
7 tasks
diegolmello added a commit that referenced this pull request Jul 31, 2026
diegolmello added a commit that referenced this pull request Jul 31, 2026
…fixes (#7521)

* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)"

This reverts commit 6e98b8b.

* Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"

This reverts commit da389be.

* Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)"

This reverts commit cd6f2a8.

* Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)"

This reverts commit 80989f5.

* Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)"

This reverts commit 2af4bda.

* fix: rewire pendingHangups drain without awaitDdpLoggedIn

* fix: remove 7380 orphan-socket gate tests from deepLinking

* test: cover handleClickCallPush new-server path and pendingHangups login-wait drain

* refactor: extract isLoginReady helper in connect.ts

* fix: restore orphan-socket guard in SDK patch

* test: restore updateMessage concurrency and subscribe coverage

* refactor: address review nits in connect and MediaSessionInstance

* fix(voip): await processSignal before native-accept replay

processSignal returns a promise and mutates the state that
tryAnswerIfNativeAcceptedNotification reads, so both call sites must
sequence on it. Also corrects the isLoginReady comment: close does clear
meteor.connected, but neither it nor ddp.loggedIn survives a silent
background death.
diegolmello added a commit that referenced this pull request Jul 31, 2026
* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)"

This reverts commit 6e98b8b.

* Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"

This reverts commit da389be.

* Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)"

This reverts commit cd6f2a8.

* Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)"

This reverts commit 80989f5.

* Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)"

This reverts commit 2af4bda.

* fix: rewire pendingHangups drain without awaitDdpLoggedIn

* fix: remove 7380 orphan-socket gate tests from deepLinking

* test: cover handleClickCallPush new-server path and pendingHangups login-wait drain

* refactor: extract isLoginReady helper in connect.ts

* fix: restore orphan-socket guard in SDK patch

* test: restore updateMessage concurrency and subscribe coverage

* refactor: address review nits in connect and MediaSessionInstance

* feat(sdk): add reopenNow, liveness probe, and disconnected emit to DDP socket

The SDK send() waits on a 'disconnected' event that nothing ever emitted, so zombie sockets caused in-flight sends to hang forever. Add reopenNow() to force a single shared reconnect and emit 'disconnected' to reject those sends, plus a bounded probe() for gray-zone liveness checks.

Restore ddpSocket.test.ts with coverage for probe, reopenNow, subscription preservation, concurrent reconnect deduplication, and the send() listener-leak fix. Add @rocket.chat/sdk and tiny-events to Jest's transform-ignore exceptions so the SDK's TypeScript source is transformed.

* fix(voip): gate native call accept on socket readiness

After a long suspension the DDP socket can be a zombie: readyState=1 and
connected=true while sends hang and no pong arrives. The native accept path
previously replayed/answered immediately, so the WebRTC setup timed out at the
remote-sdp stage.

Add a single guarded accept helper that every accept path funnels through:
- classify the socket by lastPing age and force reopenNow() when stale;
- wait for login readiness and for the media-signal/media-calls subscriptions
  to be acked on the current socket;
- replay REST state signals and answer only if the call is not already bound;
- on timeout/failure terminate the native call, reset the native accepted id,
  and queue a best-effort hangup.

Expose the socket via sdk.current.ddp and add DDPDriver passthroughs plus a
waitForNotifyUserMediaSubs readiness helper in the SDK patch. Also guard
checkVoipPermission so it does not reset the media session while a call is
active or being accepted.

* fix(sdk): serialize forced reopen against concurrent open, harden probe

* refactor(voip): share socket health classification

Extract the age/ping classification into classifySocketHealth in
waitForLoginReady.ts and make the foreground-saga helper getSocketStaleness
delegate to it. The accept gate imports from the lightweight helper file to
avoid pulling connect.ts (and its heavy deps) into the VoIP unit tests.

* fix(connect): reconnect immediately when foregrounding a stale socket

On foreground after long suspension the DDP socket can be zombie while

redux still reads connected=true. Classify socket freshness via lastPing

and pingInterval: reopen immediately when stale, probe in the gray zone,

and keep the existing checkAndReopen path for healthy/fresh sockets.

Adds an in-flight probe guard so rapid AppState flaps do not stack probes.

* fix(voip): stop aborted accept gates from terminating the call

Aborted gates now return early without running the failure ladder (terminate/endCall).

activeGates cleanup only deletes its own controller so newer gates survive.

Live-signal 'accepted' notifications from the stream listener funnel through acceptNativeCallWithReadiness instead of calling answerCall directly.

SDK waitForNotifyUserMediaSubs now polls up to the timeout for media-signal/media-calls subscriptions to appear after a forced reopen.

Also type the DDP shape in acceptNativeCall and remove "as any" from the AbortSignal fallback.

* test(voip): align call-lifecycle integration tests with gated accept

* fix(voip): await processSignal before native-accept replay

processSignal returns a promise and mutates the state that
tryAnswerIfNativeAcceptedNotification reads, so both call sites must
sequence on it. Also corrects the isLoginReady comment: close does clear
meteor.connected, but neither it nor ddp.loggedIn survives a silent
background death.

* test(voip): tidy timer cleanup and duplicate mock in accept tests

* fix(voip): terminate native call when readiness sequence throws

* fix(sdk): require both media subs and reuse sub id when resubscribing

* test(voip): integration coverage for accept gate ladder and socket probe

* chore(voip): temporary reconnect-latency trace instrumentation, dev-only

Marks app-foreground classification, reopen start, socket connecting/connected,
login success, rooms sync done, and VoIP gate stages. Persists to a file so marks
survive device lock (Metro disconnected), dumps to Metro console after foreground.
Drop before merge.

* fix(voip): classify closed socket as reopen regardless of ping age

Ping-age-only classification labeled a known-dead socket healthy when the
last pong was recent, bypassing the reopen ladder on foreground and in the
native-call accept gate. Short-circuit on the driver connected getter, which
already checks readyState.

* fix(sdk): lower hardcoded ddp ping interval to 10s

DDPDriver overrode the ping interval to 20s, doubling every freshness
threshold derived from it. Also apply the sdk patch via pnpm
patchedDependencies instead of the patch-package postinstall.

* fix(sdk): apply sdk patch via pnpm only

pnpm patchedDependencies already applies the patch at install, so the
patch-package postinstall failed trying to apply it again and broke CI
installs. Move the patch out of patches/ so patch-package never sees it.

* fix(sdk): revert to patch-package, keep 10s ping interval

The pnpm patchedDependencies migration double-applied the sdk patch with
the patch-package postinstall and broke CI installs. patch-package 8 also
cannot author patches under pnpm, so a single mechanism wins: restore
patch-package as the sole applier and hand-add the timeout hunk to its
patch. Verified against pristine with patch-package's own apply engine.

* fix(voip): guard AppState access in reconnect trace

The react-native mock leaves AppState undefined, crashing every suite
that imports the trace module.

* test(voip): match integration ping interval to 10s sdk patch

* fix(voip): always confirm socket health with a round trip

`classifySocketHealth` returned 'healthy' for any ping younger than one
interval, and the accept gate skipped its verification round trip on that
verdict. A young `lastPing` proves nothing: the SDK's `onOpen` sets it
before awaiting the connect reply and `onMessage` refreshes it on any
frame, so a frozen socket can carry a fresh timestamp.

Collapse the classification to 'probe' | 'reopen' and have the accept gate
verify every non-reopen verdict, so a frozen or mid-handshake socket gets
reopened instead of answered into silence.

The foreground ladder follows: a quiet-but-fresh socket now probes rather
than falling through to `checkAndReopen`, and 'fresh' is reachable only
when the SDK lacks the probe/reopen hooks.

Also drop the `connected` half of the `appHasComeBackToForeground` guard.
A real socket close sets `meteor.connected = false`, so that guard
returned early exactly when `reopenNow()` was needed.

* remove reconnectMark

* refactor(connection): extract socket health module with unit suite

* test(connection): add socket health integration suite

* refactor(connection): move foreground socket recovery onto socket health module

* refactor(voip): gate call accept on socket health module

* test(connection): type the socket health integration scaffolding

* refactor(connection): consolidate onAbort into shared helper

Three copies of onAbort (waitForLoginReady, acceptNativeCall, socketHealth)
collapse into app/lib/methods/helpers/onAbort.ts. The helper is
addEventListener-only: React Native's AbortController polyfill
(abort-controller/event-target-shim) always provides addEventListener, so
the legacy onabort fallback was dead code and carried a last-writer-wins
overwrite hazard. Superset behavior wins where the copies diverged: null
signal is a no-op and an already-aborted signal fires the callback
immediately, which closes recoverSocket's blind spot where a pre-aborted
signal never resolved 'abandoned'.

* refactor(connection): fold executeRecovery into shareRecovery

* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)"

This reverts commit 6e98b8b.

* Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"

This reverts commit da389be.

* Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)"

This reverts commit cd6f2a8.

* Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)"

This reverts commit 80989f5.

* Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)"

This reverts commit 2af4bda.

* fix: rewire pendingHangups drain without awaitDdpLoggedIn

* fix: remove 7380 orphan-socket gate tests from deepLinking

* test: cover handleClickCallPush new-server path and pendingHangups login-wait drain

* refactor: extract isLoginReady helper in connect.ts

* fix: restore orphan-socket guard in SDK patch

* test: restore updateMessage concurrency and subscribe coverage

* refactor: address review nits in connect and MediaSessionInstance

* fix(voip): await processSignal before native-accept replay

processSignal returns a promise and mutates the state that
tryAnswerIfNativeAcceptedNotification reads, so both call sites must
sequence on it. Also corrects the isLoginReady comment: close does clear
meteor.connected, but neither it nor ddp.loggedIn survives a silent
background death.
diegolmello added a commit that referenced this pull request Jul 31, 2026
* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)"

This reverts commit 6e98b8b.

* Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"

This reverts commit da389be.

* Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)"

This reverts commit cd6f2a8.

* Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)"

This reverts commit 80989f5.

* Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)"

This reverts commit 2af4bda.

* fix: rewire pendingHangups drain without awaitDdpLoggedIn

* fix: remove 7380 orphan-socket gate tests from deepLinking

* test: cover handleClickCallPush new-server path and pendingHangups login-wait drain

* refactor: extract isLoginReady helper in connect.ts

* fix(sync): derive the message sync cursor from the server clock

`subscription.lastOpen` did double duty as both the `chat.syncMessages` fetch
cursor and the unread-separator anchor, and every writer stamped it from the
DEVICE clock. The server compares that cursor against message `_updatedAt`,
which is a SERVER clock value. Any forward device skew — or simply closing a
room, which stamped `lastOpen = new Date()` regardless of what had been
fetched — pushed the cursor past messages that existed on the server but had
never reached the device. Those messages were then permanently invisible: the
server would never report anything "newer" than the cursor again.

`lastOpen` now means one thing only: the max `_updatedAt` of the server
response actually received for that room.

- add `writeSyncWatermark`, which reads the max `_updatedAt` off the RAW
  payload. It must run before `normalizeMessage`, which invents a device-clock
  `_updatedAt` for rows lacking one. It is deliberately not monotonic, so an
  already-poisoned future cursor can heal.
- write the watermark only where the payload proves coverage: from `updated`
  once the `chat.syncMessages` UPDATED cursor drains, and from the first batch
  of an initial tail load. The jump paths never write it — a ts-ordered
  forward walk never sees the `_updatedAt` of pre-anchor messages.
- delete every device-clock writer: `updateLastOpen`, the `unsubscribe()` call
  that closed a room by advancing the cursor, and the `readMessages`
  `updateLastOpen` flag. Closing a room offline no longer loses messages.
- treat a cursor in the future as absent so a skewed room falls back to a full
  tail load instead of syncing from nothing.
- `loadMissedMessages` no longer accepts a `lastOpen` argument, removing the
  last path by which a caller could inject a device clock.
- move the unread separator to its own RoomView state field `lastSeen`, fed
  from `room.ls`, so it no longer reads a fetch watermark. The DB column keeps
  its `lastOpen` name; no migration.

* fix(state): re-sync the open room on foreground over REST

Backgrounding can outlive the DDP socket without emitting `connected` or
`close`, so `RoomSubscription.handleConnection` never fires and messages sent
during the window never land. `chat.syncMessages` rides REST, so calling
`loadMissedMessages` from the foreground saga delivers the intent of the
reverted socket-probing change with no connection-layer risk: no probing, no
forced reopen, nothing racing the SDK reconnect timer.

`checkAndReopen()` is deleted — it no-ops when connected, which is exactly the
state the foreground gate requires, so it could never heal anything.

Also resets `RoomSubscription.isAlive` in `subscribe`, which was only ever set
in the constructor; a reused instance would immediately unsubscribe itself.

* fix(sync): self-heal a sync cursor that sits ahead of the server

Deriving the cursor from the server clock stops new poisoning, but it cannot
undo it. Rows already in users' databases carry a device-clock `lastOpen`
written by the deleted `updateLastOpen` — closing a room while offline stamped
`new Date()` regardless of what had actually been fetched. Such a cursor sits
ahead of the server's newest `_updatedAt` for the room, so every
`chat.syncMessages` drains an empty UPDATED page, there is no `_updatedAt` to
take as a watermark, and the gap is permanent. The future-skew clamp does not
catch it: the value was device-now when written and is now in the past.

There is deliberately no migration and no backfill, so those cursors heal
themselves on the next open instead. When the UPDATED cursor drains with an
empty payload, compare the subscription's server-supplied `lastMessage._id`
against the local messages table. If the server says a newest message exists
and it was never delivered here, the cursor is provably lying: run a full tail
load, which re-anchors the watermark from a real server response.

The check costs one `getMessageById` on a sync that had nothing to do anyway,
never runs mid-pagination or on a non-empty payload, skips a room with no
`lastMessage`, and recovers via `loadMessagesForRoom` so it cannot recurse.

Also document the timestamp trust boundary in CONTEXT.md: a `_updatedAt` from
a server response is server truth and the only legitimate cursor source, while
the same field on a WatermelonDB row is device-tainted.

* fix(state): request the rooms delta on foreground

`roomsRequest` rode only `LOGIN.SUCCESS`. If the socket died silently while
backgrounded, every `stream-notify-user` update was lost and this delta fetch
is the only thing that heals the rooms list.

* fix(RoomView): re-run init when the subscription row arrives and bound the retry

A notification tap for a room the user was just added to routes through
canOpenRoom, which returns a bare { rid } and creates no subscription row.
RoomView therefore falls into findAndObserveRoom, which throws and installs
observeSubscriptions. When the row later arrives from the rooms sync,
observeSubscriptions only swapped it into state, so init() never re-ran: the
room got no ls-based unread separator and sent no read receipt.

Re-run init() on the transition from "no row" to "row present", guarded by a
one-shot flag plus an in-flight flag so a concurrent or repeated emission
cannot re-enter it.

The init() failure path also re-armed a 300ms timer with no cap, hammering a
room that had no row yet. It is now capped at 5 attempts with exponential
backoff, the previous handle is cleared before re-arming, and the counter
resets once init() succeeds.

* test(RoomView): share one awaiting act flush helper

The nine inline `act(async () => {})` calls tripped require-await and
no-await-in-loop, failing the eslint CI gate. A single helper that awaits a
microtask expresses the same flush honestly instead of suppressing the rules.

* fix(RoomView): lazy state reads in init to heal notification-tap race

* refactor(sync): align cursor terminology and drop dead code

* fix: restore orphan-socket guard in SDK patch

* test: restore updateMessage concurrency and subscribe coverage

* fix(sync): derive the cursor from every fetched batch

An edited older message can postdate batch 1's newest _updatedAt, so a
first-batch-only snapshot could leave the cursor below it and miss later
edits. Snapshot the raw _updatedAt of all batches before updateMessages
mutates rows, and collapse updateLastOpen's max loop to Math.max with
the invalid-date filter kept.

* refactor: address review nits in connect and MediaSessionInstance

* fix(RoomView): dispatch init load on cursor presence, not subscription row

A room with lastOpen resumes via the missed-messages loader; a room
without one pulls history directly, ending the double fetch when a
push-notification open starts before the subscription row arrives.
Also renames the read-receipt local newLastOpen to readReceiptTime so
it no longer masquerades as a cursor.

* fix(sync): throttle self-heal tail load per room

* fix(sync): throttle foreground rooms-delta request

Rapid foreground/background cycles were dispatching roomsRequest() on every
FOREGROUND, hammering the rooms-delta endpoint. Add a 60-second module-level
throttle so cycles inside the window collapse to a single delta fetch, while a
foreground after the window still heals the rooms list over REST.

The 60-second window balances spam protection against missed-rooms risk: the
call exists to recover from a silently dead socket, so a window much longer
would re-open real missed-rooms windows, while the spam scenario is seconds to
a minute.

* test(sagas): pin Date.now past the rooms-delta throttle window

Two foreground tests in state.test.ts ran within the same real-time 60s
window, so the throttle in state.js suppressed the second dispatch and
CI went red. Mock Date.now to jump 2 minutes per read so each test's
foreground is always eligible.

* feat(sdk): add reopenNow, liveness probe, and disconnected emit to DDP socket

The SDK send() waits on a 'disconnected' event that nothing ever emitted, so zombie sockets caused in-flight sends to hang forever. Add reopenNow() to force a single shared reconnect and emit 'disconnected' to reject those sends, plus a bounded probe() for gray-zone liveness checks.

Restore ddpSocket.test.ts with coverage for probe, reopenNow, subscription preservation, concurrent reconnect deduplication, and the send() listener-leak fix. Add @rocket.chat/sdk and tiny-events to Jest's transform-ignore exceptions so the SDK's TypeScript source is transformed.

* fix(voip): gate native call accept on socket readiness

After a long suspension the DDP socket can be a zombie: readyState=1 and
connected=true while sends hang and no pong arrives. The native accept path
previously replayed/answered immediately, so the WebRTC setup timed out at the
remote-sdp stage.

Add a single guarded accept helper that every accept path funnels through:
- classify the socket by lastPing age and force reopenNow() when stale;
- wait for login readiness and for the media-signal/media-calls subscriptions
  to be acked on the current socket;
- replay REST state signals and answer only if the call is not already bound;
- on timeout/failure terminate the native call, reset the native accepted id,
  and queue a best-effort hangup.

Expose the socket via sdk.current.ddp and add DDPDriver passthroughs plus a
waitForNotifyUserMediaSubs readiness helper in the SDK patch. Also guard
checkVoipPermission so it does not reset the media session while a call is
active or being accepted.

* fix(sdk): serialize forced reopen against concurrent open, harden probe

* refactor(voip): share socket health classification

Extract the age/ping classification into classifySocketHealth in
waitForLoginReady.ts and make the foreground-saga helper getSocketStaleness
delegate to it. The accept gate imports from the lightweight helper file to
avoid pulling connect.ts (and its heavy deps) into the VoIP unit tests.

* fix(connect): reconnect immediately when foregrounding a stale socket

On foreground after long suspension the DDP socket can be zombie while

redux still reads connected=true. Classify socket freshness via lastPing

and pingInterval: reopen immediately when stale, probe in the gray zone,

and keep the existing checkAndReopen path for healthy/fresh sockets.

Adds an in-flight probe guard so rapid AppState flaps do not stack probes.

* fix(voip): stop aborted accept gates from terminating the call

Aborted gates now return early without running the failure ladder (terminate/endCall).

activeGates cleanup only deletes its own controller so newer gates survive.

Live-signal 'accepted' notifications from the stream listener funnel through acceptNativeCallWithReadiness instead of calling answerCall directly.

SDK waitForNotifyUserMediaSubs now polls up to the timeout for media-signal/media-calls subscriptions to appear after a forced reopen.

Also type the DDP shape in acceptNativeCall and remove "as any" from the AbortSignal fallback.

* test(voip): align call-lifecycle integration tests with gated accept

* fix(sync): drop the lying-cursor heal

`normalizeCursor` read `subscription.lastMessage._id` and, when that id was
absent from the local `messages` table, declared the persisted cursor a lie and
ran a full tail load behind a 5-minute per-room cooldown.

The predicate cannot detect what it claims to. `app/lib/methods/subscriptions/rooms.ts:190-215`
persists `subscription.lastMessage` as a message row whenever the room is not the
one currently open, so finding that `_id` locally proves nothing about contiguity
between the cursor and it. The rooms-delta back-fills the row while the messages
between the cursor and it are still missing, which means the heal goes quiet in
exactly the back-filled poisoned-cursor case this branch exists to close. The only
case it does fire on is a tombstone `lastMessage` — a deleted newest message that
never resolves locally — which is also the sole reason the cooldown existed.

The reported repros are already covered without it: `load()` discards a cursor that
sits ahead of server time, and a room with no usable cursor falls back to a full
recent-history load. Both are kept and still tested.

* fix(voip): await processSignal before native-accept replay

processSignal returns a promise and mutates the state that
tryAnswerIfNativeAcceptedNotification reads, so both call sites must
sequence on it. Also corrects the isLoginReady comment: close does clear
meteor.connected, but neither it nor ddp.loggedIn survives a silent
background death.

* revert(sync): drop the foreground re-sync chain

Coming back to the foreground resumes the connection through
`checkAndReopen` again, as it did before this branch: no per-room
`chat.syncMessages` re-sync, no extra rooms-delta dispatch, and no
throttle around it.

This PR is scoped to one change only — deriving the message sync cursor
from the server's `_updatedAt` instead of the device clock. The
foreground re-sync is an independent chain: it targets a different
failure (a silently dead socket that drops stream updates), it reaches
into the connection layer, and it collides with the `checkAndReopen`
work owned elsewhere. Restoring `connect.ts` byte-identical to base
keeps that ownership clean in either merge order.

The saga tests that only covered the removed behavior go with it,
including the source guard asserting the saga no longer references
`checkAndReopen`.

* revert(RoomView): drop the init control-flow chain

Opening a room initializes once again: no re-entrancy guard, no bounded
retry with exponential backoff, no `init()` re-run driven by the
subscription-row observer, and no lazy re-reads of room and joined state
after the network await.

That chain targets the notification-tap race — a room opened before its
subscription row exists — which is a separate defect from the one this
PR fixes. This PR is scoped to deriving the sync cursor from the
server's `_updatedAt`, and the `lastOpen`/`lastSeen` column split that
makes the cursor a distinct column from the read receipt. That split
stays: `lastOpen` is the cursor, `lastSeen` anchors the unread
separator, and a room is routed by whether it already holds a cursor.

The RoomView cases covering retry, re-entrancy and row adoption go with
the behavior; the cursor-predicate cases remain.

* fix(sync): snapshot the cursor from the first batch only

The initial tail load can recurse over several pages of history when
hidden system messages consume the visible page. Take the Last Open
snapshot from the first batch only, instead of accumulating the raw
`_updatedAt` of every page.

Accumulating over every batch maxes over a superset of the first batch,
so it can only push the cursor higher — and higher is the unsafe
direction:

    cursor < change._updatedAt  -> server returns the change again  -> wasteful, safe
    cursor > change._updatedAt  -> server stays silent              -> the missing-message bug

The earlier every-batch change argued the opposite and was wrong about
which way the risk points. The asymmetry is structural: the tail load
paginates by creation time while the cursor lives on modification time,
so no maximum taken over a tail load can honestly mean "seen everything
below this". The conservative snapshot is the one that re-fetches.

`updateLastOpen` keeps its rewritten body — a `Math.max` over the
filtered numeric times, invalid dates dropped, empty payload a no-op —
and the write gate is untouched: older pages and gap fills still never
move the cursor.

* docs: describe only the cursor mechanisms that still exist

The Lying Cursor paragraph named a heal that this branch no longer
carries, and the vocabulary around it belonged to the carved
foreground-resync and RoomView-init chains rather than to the cursor
change this PR is scoped to. Replace it with the direction of safety
stated the right way round:

    cursor < change._updatedAt  -> server returns the change again  -> wasteful, safe
    cursor > change._updatedAt  -> server stays silent              -> the missing-message bug

The Timestamp Trust Boundary section and the Last Open / Last Seen /
Server Timestamp / Device Timestamp table stay: they document the
vocabulary the cursor change is written in.

* test(voip): tidy timer cleanup and duplicate mock in accept tests

* fix(voip): terminate native call when readiness sequence throws

* fix(sdk): require both media subs and reuse sub id when resubscribing

* fix(read): let the server own the ls timestamp

readMessages wrote `ls` from the device clock while POSTing
subscriptions.read, so a skewed clock drew the unread separator early or
late until the next subscriptions.get corrected it. The server already
stamps `ls` and the subscription stream delivers it via
createOrUpdateSubscription, so drop the optimistic write and the now
unused `ls` parameter.

* fix(sync): drop loadMissedMessages' cursor guards

getMessages is the sole gate between loadMessagesForRoom and
loadMissedMessages, so the no-cursor fallback and the future-cursor
guard duplicated its job. Cursor writes are server-derived now, so a
skewed device clock can no longer poison lastOpen.

* fix(sync): take the Last Open from every batch and page

Both loaders now accumulate the raw server `_updatedAt` of every batch or
page they walk and write the cursor once, at the end, instead of trusting
a single batch to carry the newest stamp.

`loadMessagesForRoom` snapshots inside `fetchBatch` for every batch, not
only the first: the tail load can recurse over several pages of history
when hidden system messages consume the visible page, and the fetch order
within that load is not part of the contract. Taking the max across all
of them is future proof — if the first batch ever became the oldest one,
a first-batch-only snapshot would silently lower the cursor.

`loadMissedMessages` threads the accumulator through its pagination.
Its pages descend from the newest, so writing from the last page alone
landed the cursor on the oldest page walked. The write still fires only
once the UPDATED cursor has drained, and only on a call that actually
fetched an UPDATED page — a DELETED-only continuation must not write
again. An abort mid-pagination writes no cursor: the run is re-fetched.

`updateLastOpen` is untouched and stays deliberately non-monotonic, so an
older server value can still heal a poisoned cursor. The write gate in
`loadMessagesForRoom` is untouched too: older pages (`latest`) and gap
fills (`loaderItem`) never reach the newest history regardless of fetch
order, so letting them write could only lower the cursor.

* test(voip): integration coverage for accept gate ladder and socket probe

* fix(sync): return early from loadMissedMessages without a subscription

* refactor(sync): name the Server Timestamp carrier and its snapshot

* chore(sync): drop review-flagged comments

* fix(sync): handle CodeRabbit round-3 findings

- catch rejections from the un-awaited pagination continuation
- skip null _updatedAt payload rows so a null-only payload can't write
  an epoch-0 cursor
- annotate the resume-sync fixture helper and assert the persisted
  cursor is sent to chat.syncMessages
- re-point the null-lastOpen resume test: the history fallback was
  removed, RoomView's getMessages owns the initial load

* chore: format code and fix lint issues

* chore(voip): temporary reconnect-latency trace instrumentation, dev-only

Marks app-foreground classification, reopen start, socket connecting/connected,
login success, rooms sync done, and VoIP gate stages. Persists to a file so marks
survive device lock (Metro disconnected), dumps to Metro console after foreground.
Drop before merge.

* fix(voip): classify closed socket as reopen regardless of ping age

Ping-age-only classification labeled a known-dead socket healthy when the
last pong was recent, bypassing the reopen ladder on foreground and in the
native-call accept gate. Short-circuit on the driver connected getter, which
already checks readyState.

* fix(sdk): lower hardcoded ddp ping interval to 10s

DDPDriver overrode the ping interval to 20s, doubling every freshness
threshold derived from it. Also apply the sdk patch via pnpm
patchedDependencies instead of the patch-package postinstall.

* fix(sdk): apply sdk patch via pnpm only

pnpm patchedDependencies already applies the patch at install, so the
patch-package postinstall failed trying to apply it again and broke CI
installs. Move the patch out of patches/ so patch-package never sees it.

* fix(sdk): revert to patch-package, keep 10s ping interval

The pnpm patchedDependencies migration double-applied the sdk patch with
the patch-package postinstall and broke CI installs. patch-package 8 also
cannot author patches under pnpm, so a single mechanism wins: restore
patch-package as the sole applier and hand-add the timeout hunk to its
patch. Verified against pristine with patch-package's own apply engine.

* fix(voip): guard AppState access in reconnect trace

The react-native mock leaves AppState undefined, crashing every suite
that imports the trace module.

* test(voip): match integration ping interval to 10s sdk patch

* fix(voip): always confirm socket health with a round trip

`classifySocketHealth` returned 'healthy' for any ping younger than one
interval, and the accept gate skipped its verification round trip on that
verdict. A young `lastPing` proves nothing: the SDK's `onOpen` sets it
before awaiting the connect reply and `onMessage` refreshes it on any
frame, so a frozen socket can carry a fresh timestamp.

Collapse the classification to 'probe' | 'reopen' and have the accept gate
verify every non-reopen verdict, so a frozen or mid-handshake socket gets
reopened instead of answered into silence.

The foreground ladder follows: a quiet-but-fresh socket now probes rather
than falling through to `checkAndReopen`, and 'fresh' is reachable only
when the SDK lacks the probe/reopen hooks.

Also drop the `connected` half of the `appHasComeBackToForeground` guard.
A real socket close sets `meteor.connected = false`, so that guard
returned early exactly when `reopenNow()` was needed.

* remove reconnectMark

* refactor(connection): extract socket health module with unit suite

* test(connection): add socket health integration suite

* refactor(connection): move foreground socket recovery onto socket health module

* refactor(voip): gate call accept on socket health module

* test(connection): type the socket health integration scaffolding

* refactor(connection): consolidate onAbort into shared helper

Three copies of onAbort (waitForLoginReady, acceptNativeCall, socketHealth)
collapse into app/lib/methods/helpers/onAbort.ts. The helper is
addEventListener-only: React Native's AbortController polyfill
(abort-controller/event-target-shim) always provides addEventListener, so
the legacy onabort fallback was dead code and carried a last-writer-wins
overwrite hazard. Superset behavior wins where the copies diverged: null
signal is a no-op and an already-aborted signal fires the callback
immediately, which closes recoverSocket's blind spot where a pre-aborted
signal never resolved 'abandoned'.

* refactor(connection): fold executeRecovery into shareRecovery

* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)"

This reverts commit 6e98b8b.

* Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"

This reverts commit da389be.

* Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)"

This reverts commit cd6f2a8.

* Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)"

This reverts commit 80989f5.

* Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)"

This reverts commit 2af4bda.

* fix: rewire pendingHangups drain without awaitDdpLoggedIn

* fix: remove 7380 orphan-socket gate tests from deepLinking

* test: cover handleClickCallPush new-server path and pendingHangups login-wait drain

* refactor: extract isLoginReady helper in connect.ts

* fix: restore orphan-socket guard in SDK patch

* test: restore updateMessage concurrency and subscribe coverage

* refactor: address review nits in connect and MediaSessionInstance

* fix(voip): await processSignal before native-accept replay

processSignal returns a promise and mutates the state that
tryAnswerIfNativeAcceptedNotification reads, so both call sites must
sequence on it. Also corrects the isLoginReady comment: close does clear
meteor.connected, but neither it nor ddp.loggedIn survives a silent
background death.

---------

Co-authored-by: diegolmello <diegolmello@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.

2 participants