fix: notify subscribers when evicting unrelated keys during write retry

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: elirangoshen

lib/OnyxUtils.ts

@@ -802,7 +802,13 @@ function reportStorageQuota(error?: Error): Promise<void> {
  * - Non-retriable errors: logs an alert and resolves without retrying
  * - Other errors: retries the operation
  */
-function retryOperation<TMethod extends RetriableOnyxOperation>(error: Error, onyxMethod: TMethod, defaultParams: Parameters<TMethod>[0], retryAttempt: number | undefined): Promise<void> {
+function retryOperation<TMethod extends RetriableOnyxOperation>(
+    error: Error,
+    onyxMethod: TMethod,
+    defaultParams: Parameters<TMethod>[0],
+    retryAttempt: number | undefined,
+    inFlightKeys?: Set<OnyxKey>,
+): Promise<void> {
     const currentRetryAttempt = retryAttempt ?? 0;
     const nextRetryAttempt = currentRetryAttempt + 1;
 
@@ -847,10 +853,12 @@ function retryOperation<TMethod extends RetriableOnyxOperation>(error: Error, on
     Logger.logInfo(`Out of storage. Evicting least recently accessed key (${keyForRemoval}) and retrying. Error: ${error}`);
     reportStorageQuota(error);
 
-    // skipNotify=true: retry's orchestrator skips keysChanged on retryAttempt > 0, so we
-    // must not let remove() fire keyChanged(undefined) — cache.set on retry restores the value.
+    // Only suppress keyChanged(undefined) when the evicted key is part of the in-flight
+    // write — then cache.set on retry will restore it. For unrelated keys, eviction is a
+    // genuine loss and subscribers must see the removed state.
+    const willBeRestored = inFlightKeys?.has(keyForRemoval) ?? false;
     // @ts-expect-error No overload matches this call.
-    return remove(keyForRemoval, undefined, true).then(() => onyxMethod(defaultParams, nextRetryAttempt));
+    return remove(keyForRemoval, undefined, willBeRestored).then(() => onyxMethod(defaultParams, nextRetryAttempt));
 }
 
 /**
@@ -1425,8 +1433,10 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
         return !OnyxKeys.isRamOnlyKey(key);
     });
 
+    const inFlightKeys = new Set<OnyxKey>(keyValuePairsToSet.map(([key]) => key));
+
     return Storage.multiSet(keyValuePairsToStore)
-        .catch((error) => OnyxUtils.retryOperation(error, multiSetWithRetry, newData, retryAttempt))
+        .catch((error) => OnyxUtils.retryOperation(error, multiSetWithRetry, newData, retryAttempt, inFlightKeys))
         .then(() => {
             OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData);
         });
@@ -1502,8 +1512,10 @@ function setCollectionWithRetry<TKey extends CollectionKeyBase>({collectionKey,
             return;
         }
 
+        const inFlightKeys = new Set<OnyxKey>(keyValuePairs.map(([key]) => key));
+
         return Storage.multiSet(keyValuePairs)
-            .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt))
+            .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt, inFlightKeys))
             .then(() => {
                 OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection);
             });
@@ -1649,13 +1661,16 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
                     promises.push(Storage.multiSet(keyValuePairsForNewCollection));
                 }
 
+                const inFlightKeys = new Set<OnyxKey>(Object.keys(finalMergedCollection));
+
                 return Promise.all(promises)
                     .catch((error) =>
                         retryOperation(
                             error,
                             mergeCollectionWithPatches,
                             {collectionKey, collection: resultCollection as OnyxMergeCollectionInput<TKey>, mergeReplaceNullPatches, isProcessingCollectionUpdate},
                             retryAttempt,
+                            inFlightKeys,
                         ),
                     )
                     .then(() => {
@@ -1723,8 +1738,10 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
             return;
         }
 
+        const inFlightKeys = new Set<OnyxKey>(keyValuePairs.map(([key]) => key));
+
         return Storage.multiSet(keyValuePairs)
-            .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt))
+            .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt, inFlightKeys))
             .then(() => {
                 sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection);
             });

tests/unit/onyxUtilsTest.ts

@@ -1181,6 +1181,63 @@ describe('OnyxUtils', () => {
             expect(logInfoSpy).toHaveBeenCalledWith(`Out of storage. Evicting least recently accessed key (${key1}) and retrying. Error: ${diskFullError}`);
             expect(logInfoSpy).toHaveBeenCalledWith(`Storage Quota Check -- bytesUsed: 0 bytesRemaining: Infinity. Original error: ${diskFullError}`);
         });
+
+        it('multiSet — eviction of an UNRELATED key still notifies its subscribers (codex regression guard)', async () => {
+            const evictableKey = `${ONYXKEYS.COLLECTION.TEST_KEY}1`;
+            const writeKey = `${ONYXKEYS.COLLECTION.TEST_KEY}2`;
+
+            // Seed the evictable key first so it becomes the LRU evictable. The subsequent multiSet
+            // writes a DIFFERENT key, so the evicted key is unrelated to the in-flight write.
+            await LocalOnyx.set(evictableKey, {value: 'will-be-evicted'});
+            expect(LocalOnyxCache.getKeyForEviction()).toBe(evictableKey);
+
+            const subscriberCalls: unknown[] = [];
+            LocalOnyx.connect({
+                key: evictableKey,
+                callback: (value) => subscriberCalls.push(value),
+            });
+            await waitForPromisesToResolve();
+            subscriberCalls.length = 0;
+
+            // Storage.multiSet rejects once with disk-full, then succeeds on retry.
+            LocalStorageMock.multiSet = jest.fn(LocalStorageMock.multiSet).mockRejectedValueOnce(diskFullError).mockImplementation(LocalStorageMock.multiSet);
+
+            await LocalOnyx.multiSet({[writeKey]: {value: 'new'}});
+
+            // evictableKey was the LRU evictable, so retryOperation evicted it. It's not in the
+            // in-flight write's keys, so the retry's cache.set won't restore it — subscribers MUST
+            // see keyChanged(undefined) so they reflect the genuine removal (not stale value).
+            expect(LocalOnyxCache.hasCacheForKey(evictableKey)).toBe(false);
+            expect(subscriberCalls.at(-1)).toBeUndefined();
+        });
+
+        it('multiSet — eviction of an IN-FLIGHT key does not strand its subscriber', async () => {
+            const memberKey = `${ONYXKEYS.COLLECTION.TEST_KEY}1`;
+
+            // Seed memberKey so it becomes the LRU evictable. The multiSet below writes to the SAME
+            // key, so eviction picks an in-flight key.
+            await LocalOnyx.set(memberKey, {value: 'original'});
+            expect(LocalOnyxCache.getKeyForEviction()).toBe(memberKey);
+
+            const subscriberCalls: unknown[] = [];
+            LocalOnyx.connect({
+                key: memberKey,
+                callback: (value) => subscriberCalls.push(value),
+            });
+            await waitForPromisesToResolve();
+            subscriberCalls.length = 0;
+
+            LocalStorageMock.multiSet = jest.fn(LocalStorageMock.multiSet).mockRejectedValueOnce(diskFullError).mockImplementation(LocalStorageMock.multiSet);
+
+            await LocalOnyx.multiSet({[memberKey]: {value: 'updated'}});
+
+            // The in-flight key was evicted then restored by the retry's cache.set. Subscriber's
+            // last value must be the new value, never a transient undefined from the eviction.
+            expect(LocalOnyxCache.get(memberKey)).toEqual({value: 'updated'});
+            expect(subscriberCalls.at(-1)).toEqual({value: 'updated'});
+            // Subscriber should never have seen undefined in the middle of the eviction-retry cycle.
+            expect(subscriberCalls).not.toContain(undefined);
+        });
     });
 
     describe('afterInit', () => {