Keep ReconnectApp incremental for signed-in users so other devices don't lose chat messages

src/libs/Middleware/SaveResponseInOnyx.ts

@@ -1,5 +1,7 @@
 import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types';
+import Log from '@libs/Log';
 
+import {reconnectApp} from '@userActions/App';
 import * as OnyxUpdates from '@userActions/OnyxUpdates';
 
 import CONST from '@src/CONST';
@@ -21,6 +23,15 @@ const requestsToIgnoreLastUpdateID = new Set<string>([
     SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES,
 ]);
 
+// These requests already bring the client up to date (or end the session), so they never need a full ReconnectApp to cover a gap in update IDs.
+// This is separate from requestsToIgnoreLastUpdateID because requests like DeleteMoneyRequest skip the OnyxUpdates flow but still move lastUpdateID forward.
+const requestsExemptFromFullReconnect = new Set<string>([
+    WRITE_COMMANDS.OPEN_APP,
+    SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP,
+    SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES,
+    WRITE_COMMANDS.CLOSE_ACCOUNT,
+]);
+
 // A request belongs here when its successData/finallyData is what unblocks authentication, because parking that leaves the client unable to reauthenticate.
 const requestsToApplyWithoutAdvancingLastUpdateID = new Set<string>([READ_COMMANDS.SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN, READ_COMMANDS.SIGN_IN_WITH_SUPPORT_AUTH_TOKEN]);
 
@@ -44,6 +55,31 @@ const SaveResponseInOnyx: Middleware = <TKey extends OnyxKey>(requestResponse: P
 
         const shouldApplyWithoutAdvancingLastUpdateID = requestsToApplyWithoutAdvancingLastUpdateID.has(request.command);
 
+        // The server only keeps recent update IDs. When a client comes back after a long time, the server no longer knows its
+        // lastUpdateIDAppliedToClient and answers a request with a lastUpdateID but no previousUpdateID. Applying that response moves
+        // the client's lastUpdateID forward past updates it never received, so an incremental ReconnectApp can't recover them.
+        // Run a full ReconnectApp after applying so the client gets everything in the gap.
+        const clientLastUpdateID = OnyxUpdates.getPersistedLastUpdateID();
+        const responseLastUpdateID = Number(response?.lastUpdateID ?? CONST.DEFAULT_NUMBER_ID);
+        const shouldFullReconnectAfterApply =
+            !shouldApplyWithoutAdvancingLastUpdateID &&
+            !requestsExemptFromFullReconnect.has(request.command) &&
+            !!clientLastUpdateID &&
+            responseLastUpdateID > clientLastUpdateID &&
+            !Number(response?.previousUpdateID ?? CONST.DEFAULT_NUMBER_ID);
+
+        if (shouldFullReconnectAfterApply) {
+            Log.info('[SaveResponseInOnyx] Response has a lastUpdateID but no previousUpdateID, running a full ReconnectApp after applying it', false, {
+                command: request.command,
+                clientLastUpdateID,
+                responseLastUpdateID,
+            });
+            return OnyxUpdates.apply(responseToApply).then((appliedResponse) => {
+                reconnectApp();
+                return appliedResponse;
+            });
+        }
+
         if (
             shouldApplyWithoutAdvancingLastUpdateID ||
             requestsToIgnoreLastUpdateID.has(request.command) ||

src/libs/actions/User.ts

@@ -86,7 +86,7 @@ Onyx.connectWithoutView({
     },
 });
 
-// This value is used in the handler for a RECONNECT_APP Pusher event (anonymous users), so it's not possible to use useOnyx().
+// This value is used in the handler for a RECONNECT_APP Pusher event, so it's not possible to use useOnyx().
 let lastUpdateIDAppliedToClient: OnyxEntry<number>;
 Onyx.connectWithoutView({
     key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
@@ -943,14 +943,12 @@ function subscribeToUserEvents(
     // We have an event to reconnect the App. It is triggered when we detect that the user passed updateID
     // is not in the DB
     PusherUtils.subscribeToMultiEvent(Pusher.TYPE.MULTIPLE_EVENT_TYPE.RECONNECT_APP, () => {
-        // Anonymous public-room sessions already have report actions from OpenReport. A full ReconnectApp
-        // uses setCollection and clears them, which flashes the loading skeleton until GetNewerActions
-        // refills. Pass lastUpdateID so reconnect stays incremental for anonymous users (#97847).
-        if (isAnonymousUser()) {
-            reconnectApp(lastUpdateIDAppliedToClient);
-        } else {
-            reconnectApp();
-        }
+        // This event is broadcast on the account-wide channel, so every signed-in device receives it, not only
+        // the one whose updateID is missing. A full ReconnectApp uses setCollection and clears report actions the
+        // device already holds (#97847, #100791). Pass lastUpdateID so the server can reply incrementally. It
+        // still returns full data when it can't serve that range, and we fall back to a full reconnect when the
+        // client has no lastUpdateID yet.
+        reconnectApp(lastUpdateIDAppliedToClient);
         return Promise.resolve();
     });
 }

tests/actions/UserTest.ts

@@ -1180,18 +1180,39 @@ describe('actions/User', () => {
             expect(reconnectAppMock).toHaveBeenCalledWith(lastUpdateID);
         });
 
-        it('calls reconnectApp without updateIDFrom for authenticated users', async () => {
+        it('passes lastUpdateIDAppliedToClient when reconnecting as an authenticated user', async () => {
+            // Given a signed-in device that has already applied updates up to a known ID
+            const lastUpdateID = 42;
+            await Onyx.merge(ONYXKEYS.SESSION, {authTokenType: undefined, accountID: 1, authToken: 'token'});
+            await Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, lastUpdateID);
+            await waitForBatchedUpdates();
+
+            UserActions.subscribeToUserEvents(1, 'test@test.com', () => {}, formatPhoneNumber, undefined);
+            await waitForBatchedUpdates();
+
+            // When the account-wide RECONNECT_APP event arrives (for example, because another device signed in)
+            await PusherUtils.triggerMultiEventHandler(Pusher.TYPE.MULTIPLE_EVENT_TYPE.RECONNECT_APP, []);
+            await waitForBatchedUpdates();
+
+            // Then the reconnect is incremental, so a full ReconnectApp doesn't wipe report actions this device already has
+            expect(reconnectAppMock).toHaveBeenCalledWith(lastUpdateID);
+        });
+
+        it('falls back to a full reconnect when the client has no lastUpdateIDAppliedToClient', async () => {
+            // Given a signed-in device that has not applied any updates yet
             await Onyx.merge(ONYXKEYS.SESSION, {authTokenType: undefined, accountID: 1, authToken: 'token'});
-            await Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, 42);
+            await Onyx.set(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, null);
             await waitForBatchedUpdates();
 
             UserActions.subscribeToUserEvents(1, 'test@test.com', () => {}, formatPhoneNumber, undefined);
             await waitForBatchedUpdates();
 
+            // When the RECONNECT_APP event arrives
             await PusherUtils.triggerMultiEventHandler(Pusher.TYPE.MULTIPLE_EVENT_TYPE.RECONNECT_APP, []);
             await waitForBatchedUpdates();
 
-            expect(reconnectAppMock).toHaveBeenCalledWith();
+            // Then no updateIDFrom is passed, so reconnectApp still does a full reconnect to recover
+            expect(reconnectAppMock).toHaveBeenCalledWith(undefined);
         });
     });
 });

tests/unit/MiddlewareTest.ts

@@ -1,7 +1,8 @@
-import {READ_COMMANDS} from '@libs/API/types';
+import {READ_COMMANDS, SIDE_EFFECT_REQUEST_COMMANDS, WRITE_COMMANDS} from '@libs/API/types';
 import SaveResponseInOnyx from '@libs/Middleware/SaveResponseInOnyx';
 
 import CONST from '@src/CONST';
+import * as App from '@src/libs/actions/App';
 import * as PersistedRequests from '@src/libs/actions/PersistedRequests';
 import type MoveIOUReportToExistingPolicyParams from '@src/libs/API/parameters/MoveIOUReportToExistingPolicyParams';
 import HttpUtils from '@src/libs/HttpUtils';
@@ -120,6 +121,75 @@ describe('Middleware', () => {
             expect(await getOnyxValue(ONYXKEYS.ONYX_UPDATES_FROM_SERVER)).toBeUndefined();
             expect(await getOnyxValue(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT)).toBe(clientLastUpdateID);
         });
+
+        describe('when the server no longer knows the client lastUpdateID', () => {
+            beforeEach(() => {
+                jest.spyOn(App, 'reconnectApp').mockImplementation(() => {});
+            });
+
+            afterEach(() => {
+                jest.restoreAllMocks();
+            });
+
+            test.each([
+                ['AddComment', 'AddComment'],
+                // DeleteMoneyRequest skips the OnyxUpdates flow but still moves lastUpdateID forward, so it can leave the same gap
+                ['DeleteMoneyRequest', WRITE_COMMANDS.DELETE_MONEY_REQUEST],
+            ])('applies the response and runs a full ReconnectApp to cover the gap for %s', async (_case, command) => {
+                // Given a device that was backgrounded for a long time, so the server no longer has its lastUpdateIDAppliedToClient
+                await Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, 100);
+                await waitForBatchedUpdates();
+
+                Request.addMiddleware(SaveResponseInOnyx);
+
+                // When a write returns a lastUpdateID but no previousUpdateID
+                jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({jsonCode: 200, lastUpdateID: 5001});
+                const result = await Request.processWithMiddleware({
+                    command,
+                    data: {apiRequestType: CONST.API_REQUEST_TYPE.WRITE},
+                    successData: [{onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.IS_LOADING_APP, value: false}],
+                });
+                await waitForBatchedUpdates();
+
+                // Then the response is applied without pausing the queue
+                expect(result?.shouldPauseQueue).toBeUndefined();
+                expect(await getOnyxValue(ONYXKEYS.ONYX_UPDATES_FROM_SERVER)).toBeUndefined();
+
+                // And a full ReconnectApp runs, because an incremental one would start after the skipped updates
+                expect(App.reconnectApp).toHaveBeenCalledTimes(1);
+                expect(App.reconnectApp).toHaveBeenCalledWith();
+            });
+
+            test.each([
+                ['the response has a previousUpdateID', 100, 'AddComment', {lastUpdateID: 101, previousUpdateID: 100}],
+                ['the client has no lastUpdateID yet', undefined, 'AddComment', {lastUpdateID: 5001}],
+                ['the response is not newer than the client', 100, 'AddComment', {lastUpdateID: 100}],
+                ['the request is a ReconnectApp', 100, SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP, {lastUpdateID: 5001}],
+                ['the request is an OpenApp', 100, WRITE_COMMANDS.OPEN_APP, {lastUpdateID: 5001}],
+                ['the request is a GetMissingOnyxMessages', 100, SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES, {lastUpdateID: 5001}],
+                ['the request is a CloseAccount', 100, WRITE_COMMANDS.CLOSE_ACCOUNT, {lastUpdateID: 5001}],
+            ])('does not run a full ReconnectApp when %s', async (_case, clientLastUpdateID, command, updateIDs) => {
+                // Given a client whose lastUpdateID is either still known to the server or has nothing to lose
+                if (clientLastUpdateID) {
+                    await Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, clientLastUpdateID);
+                }
+                await waitForBatchedUpdates();
+
+                Request.addMiddleware(SaveResponseInOnyx);
+                jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({jsonCode: 200, ...updateIDs});
+
+                // When the response comes back
+                await Request.processWithMiddleware({
+                    command,
+                    data: {apiRequestType: CONST.API_REQUEST_TYPE.WRITE},
+                    successData: [{onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.IS_LOADING_APP, value: false}],
+                });
+                await waitForBatchedUpdates();
+
+                // Then no extra full ReconnectApp runs, since no updates were skipped or the request already returns full data
+                expect(App.reconnectApp).not.toHaveBeenCalled();
+            });
+        });
     });
 
     describe('HandleUnusedOptimisticID', () => {