Keep the leader's unsent writes when another tab syncs the queue

src/libs/actions/PersistedRequests.ts

@@ -1,3 +1,4 @@
+import {isClientTheLeader} from '@libs/ActiveClientManager';
 import Log from '@libs/Log';
 import sanitizeLogParams from '@libs/sanitizeLogParams';
 
@@ -56,6 +57,30 @@ function onCrossTabRequestsMerged(callbackFunction: () => void) {
     crossTabRequestsCallback = callbackFunction;
 }
 
+function adoptLeaderDeletions(diskRequests: AnyRequest[]) {
+    if (isClientTheLeader()) {
+        return;
+    }
+    const diskIDs = new Set<number>();
+    for (const r of diskRequests) {
+        const requestIndex = getClientRequestIndex(r);
+        if (requestIndex != null) {
+            diskIDs.add(requestIndex);
+        }
+    }
+    const previousLength = persistedRequests.length;
+    persistedRequests = persistedRequests.filter((r) => {
+        const requestIndex = getClientRequestIndex(r);
+        return requestIndex == null || diskIDs.has(requestIndex);
+    });
+    if (persistedRequests.length !== previousLength) {
+        Log.info('[PersistedRequests] Reconciled deletions from leader tab', false, {
+            removedCount: previousLength - persistedRequests.length,
+            remainingCount: persistedRequests.length,
+        });
+    }
+}
+
 // We have opted for connectWithoutView here as this module is strictly non-UI
 Onyx.connectWithoutView({
     key: ONYXKEYS.PERSISTED_REQUESTS,
@@ -89,35 +114,13 @@ Onyx.connectWithoutView({
                     }
                 }
                 persistedRequests = [...persistedRequests, ...newFromOtherTabs];
-                trackOnyxWrite(Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests));
                 crossTabRequestsCallback?.();
                 return;
             }
 
-            // Reconcile deletions from the leader tab. When pendingOnyxWrites > 0,
-            // the callback may be a stale own-write (broadcastUpdate fires synchronously
-            // inside Onyx.set) — skip to preserve Issue 4 protection. When counter is 0,
-            // the callback is from a cross-tab storage event, safe to remove from memory
-            // any requests the leader already processed.
-            if (pendingOnyxWrites === 0) {
-                const diskIDs = new Set<number>();
-                for (const r of val) {
-                    const requestIndex = getClientRequestIndex(r);
-                    if (requestIndex != null) {
-                        diskIDs.add(requestIndex);
-                    }
-                }
-                const previousLength = persistedRequests.length;
-                persistedRequests = persistedRequests.filter((r) => {
-                    const requestIndex = getClientRequestIndex(r);
-                    return requestIndex == null || diskIDs.has(requestIndex);
-                });
-                if (persistedRequests.length !== previousLength) {
-                    Log.info('[PersistedRequests] Reconciled deletions from leader tab', false, {
-                        removedCount: previousLength - persistedRequests.length,
-                        remainingCount: persistedRequests.length,
-                    });
-                }
+            const isCrossTabWrite = pendingOnyxWrites === 0;
+            if (isCrossTabWrite) {
+                adoptLeaderDeletions(val);
             }
             return;
         }

tests/unit/PersistedRequests.ts

@@ -1,3 +1,5 @@
+import {init as initActiveClientManager} from '@libs/ActiveClientManager';
+
 import type {OnyxInput, OnyxKey} from 'react-native-onyx';
 
 import Onyx from 'react-native-onyx';
@@ -10,6 +12,16 @@ import ONYXKEYS from '../../src/ONYXKEYS';
 import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
 import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates';
 
+jest.mock('@libs/ActiveClientManager', () => jest.requireActual<Record<string, unknown>>('../../src/libs/ActiveClientManager/index.ts'));
+
+async function registerAsFollowerTab() {
+    initActiveClientManager();
+    await waitForBatchedUpdates();
+    const activeClients = await OnyxUtils.get(ONYXKEYS.ACTIVE_CLIENTS);
+    await Onyx.set(ONYXKEYS.ACTIVE_CLIENTS, [...(activeClients ?? []), 'newer-tab']);
+    await waitForBatchedUpdates();
+}
+
 const request: Request<'reportMetadata_1' | 'reportMetadata_2'> = {
     command: 'OpenReport',
     successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
@@ -320,6 +332,7 @@ describe('PersistedRequests persistence guarantees', () => {
     });
 
     it('Follower tab should reconcile processed requests from leader via cross-tab callback', async () => {
+        await registerAsFollowerTab();
         PersistedRequests.clear();
         await waitForBatchedUpdates();
         expect(PersistedRequests.getAll()).toHaveLength(0);
@@ -370,4 +383,39 @@ describe('PersistedRequests persistence guarantees', () => {
         expect(PersistedRequests.getAll()).toHaveLength(2);
         expect(PersistedRequests.getAll().map((r) => r.command)).toEqual(['CommandB', 'CommandC']);
     });
+
+    it('Follower tab should not write its stale copy back to storage when it merges a request from another tab', async () => {
+        // Given a follower tab that holds two requests
+        await registerAsFollowerTab();
+        PersistedRequests.clear();
+        await waitForBatchedUpdates();
+        const requestA: Request<'reportMetadata_1' | 'reportMetadata_2'> = {
+            command: 'CommandA',
+            successData: [{key: 'reportMetadata_1', onyxMethod: 'merge', value: {}}],
+            failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}],
+            requestIndex: 40,
+        };
+        const requestB: Request<'reportMetadata_3' | 'reportMetadata_4'> = {
+            command: 'CommandB',
+            successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}],
+            failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}],
+            requestIndex: 41,
+        };
+        const requestC: Request<'reportMetadata_5' | 'reportMetadata_6'> = {
+            command: 'CommandC',
+            successData: [{key: 'reportMetadata_5', onyxMethod: 'merge', value: {}}],
+            failureData: [{key: 'reportMetadata_6', onyxMethod: 'merge', value: {}}],
+            requestIndex: 42,
+        };
+        PersistedRequests.save(requestA);
+        PersistedRequests.save(requestB);
+        await waitForBatchedUpdates();
+
+        // When another tab writes a queue where requestA was already sent and requestC was added
+        await Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, [requestB, requestC]);
+        await waitForBatchedUpdates();
+
+        // Then storage keeps that queue, so the sent requestA is not brought back and sent twice
+        expect(await OnyxUtils.get(ONYXKEYS.PERSISTED_REQUESTS)).toEqual([requestB, requestC]);
+    });
 });

tests/unit/SequentialQueueTest.ts

@@ -484,6 +484,38 @@ describe('SequentialQueue', () => {
             offlineSpy.mockRestore();
         }
     });
+
+    it('keeps and sends a write the leader queued when a follower tab writes back a stale copy of the queue', async () => {
+        // Given a leader with a paused queue holding two requests and a newer approval
+        const processSpy = jest.spyOn(RequestModule, 'processWithMiddleware').mockResolvedValue({jsonCode: CONST.JSON_CODE.SUCCESS});
+        const readNewestAction: AnyRequest = {command: WRITE_COMMANDS.READ_NEWEST_ACTION, requestIndex: 1};
+        const openReport: AnyRequest = {command: WRITE_COMMANDS.OPEN_REPORT, requestIndex: 2};
+        const approveMoneyRequest: AnyRequest = {command: WRITE_COMMANDS.APPROVE_MONEY_REQUEST, requestIndex: 3};
+
+        try {
+            SequentialQueue.pause();
+            await SequentialQueue.push(readNewestAction);
+            await SequentialQueue.push(openReport);
+            await SequentialQueue.push(approveMoneyRequest);
+            await waitForBatchedUpdates();
+
+            // When a follower tab's copy of the queue, saved before the approval, lands in shared storage
+            await Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, [readNewestAction, openReport]);
+            await waitForBatchedUpdates();
+
+            // Then the leader still sends the approval once the queue resumes
+            SequentialQueue.unpause();
+            await SequentialQueue.waitForIdle();
+            await waitForBatchedUpdates();
+            expect(processSpy.mock.calls.map(([sentRequest]) => sentRequest.command)).toEqual([
+                WRITE_COMMANDS.READ_NEWEST_ACTION,
+                WRITE_COMMANDS.OPEN_REPORT,
+                WRITE_COMMANDS.APPROVE_MONEY_REQUEST,
+            ]);
+        } finally {
+            processSpy.mockRestore();
+        }
+    });
 });
 
 describe('SequentialQueue - reconnect coverage collapse', () => {