[No QA] Fix lost/duplicated requests in PersistedRequests (Issues 2, 3, 4, 5) by szymonzalarski98 · Pull Request #84622 · Expensify/App

szymonzalarski98 · 2026-03-09T18:03:54Z

Explanation of Change

Sequential Queue Issues — Status Report

Fixed: Issues 2, 3, 4, 5

Issue 3: processNextRequest() and rollbackOngoingRequest() don't persist to disk

File: src/libs/actions/PersistedRequests.ts

Problem: When a request moves from the queue to ongoingRequest, neither the updated queue nor the ongoing request is written to disk. If the app crashes mid-flight, the ongoing request is lost (3a) and the queue on disk is stale (3c).

Fix: Both processNextRequest() and rollbackOngoingRequest() now use Onyx.multiSet() to atomically persist the updated queue and the ongoing request to disk.

Issue 4: Connect callback overwrites in-memory state with stale disk data

File: src/libs/actions/PersistedRequests.ts

Problem: The connectWithoutView callback blindly overwrites the in-memory persistedRequests array with whatever Onyx returns from disk. When multiple Onyx.set() calls resolve out of order, the last callback wins — potentially reverting the in-memory state to a stale snapshot and permanently losing requests.

Fix: After initialization, the callback is a no-op for non-null values: if (isInitialized && val != null) { return; }. In-memory state is authoritative. Onyx.clear() (which sends null) is still allowed through so the module resets properly for tests and app restarts.

Issue 5: Conflict resolver sees stale queue

Fixed automatically by Issue 4. Since in-memory state is no longer overwritten by stale disk callbacks, the conflict resolver in prepareRequest() and push() always sees the complete, up-to-date queue.

Issue 2: Nothing in the write pipeline is awaited

Files: src/libs/actions/PersistedRequests.ts, src/libs/Network/SequentialQueue.ts, src/libs/API/index.ts

Problem: API.write() returned Promise.resolve() immediately. PersistedRequests.save() was void (didn't return the Onyx.set() promise). The caller had zero guarantee that the request was persisted to disk.

Fix:

save() now returns Promise<void> (the Onyx.set() promise)
push() returns a persistencePromise from save() (but calls flush() synchronously to preserve write-before-read ordering)
processRequest() is now async and awaits pushToSequentialQueue()
await API.write() now guarantees the request is persisted to disk

Not Fixed: Issue 1 — Optimistic data is applied before the request is persisted

The Problem

In API.write(), prepareRequest() applies Onyx.update(optimisticData) BEFORE the request is persisted to disk. If the app crashes after the optimistic UI update but before Onyx.set(PERSISTED_REQUESTS) completes, the user sees stale optimistic UI with no backing request to reconcile it.

Why We Can't Fix It Now

The fix requires moving Onyx.update(optimisticData) to AFTER push() in write(). However, conflict resolvers have Onyx side effects inside checkAndFixConflictingRequest.

Specifically, resolveCommentDeletionConflicts in src/libs/actions/RequestConflictUtils.ts:151 calls Onyx.update(rollbackData) — which sets a report action to null — inside the conflict resolver callback.

Current flow (works):

Step	What happens	Onyx batch
1	`prepareRequest()` → conflict resolver → `Onyx.update(rollback)` — sets action to `null`	batch A
2	`prepareRequest()` → `Onyx.update(optimisticData)` — sets `pendingAction: "delete"`	batch A
3	`push()` → conflict resolver again → `Onyx.update(rollback)`	batch B

Steps 1+2 are in the same synchronous tick → Onyx batches them together → null wins → action is deleted. Tests pass.

Flow after moving optimistic data after push (breaks):

Step	What happens	Onyx batch
1	`prepareRequest()` → conflict resolver → `Onyx.update(rollback)` — sets action to `null`	batch A
2	`push()` → `save()` → `Onyx.set(PERSISTED_REQUESTS)`	batch B
3	`push()` → conflict resolver again → `Onyx.update(rollback)`	batch B
4	`write()` → `Onyx.update(optimisticData)` — sets `pendingAction: "delete"`	batch C

Steps 1 and 4 are now in different Onyx batches (separated by push operations). The null from step 1 is consumed in batch A. The optimistic data in step 4 creates the action anew with {pendingAction: "delete"} instead of it staying deleted. 4 ReportTest tests fail.

Plan for Future Fix

The root cause is that resolveCommentDeletionConflicts (and potentially other conflict resolvers) call Onyx.update() as a side effect inside the callback. This couples Onyx state mutations to the conflict resolution logic, making it impossible to reorder the pipeline.

Required refactoring:

Change the ConflictActionData type to include an optional rollbackData field
Modify conflict resolvers to return rollback data instead of calling Onyx.update() directly
Have write() apply rollback data and optimistic data together in a controlled order, after persist

This changes the conflict resolver interface and affects all resolvers — it should be done in a separate PR.

Fixed Issues

$ #80759
PROPOSAL:

Tests

Run npx jest tests/unit/PersistedRequests.ts --no-coverage — all pass, including:
- Issue 3a: verifies PERSISTED_ONGOING_REQUESTS is written to disk after processNextRequest() (via OnyxUtils.get())
- Issue 3c: verifies PERSISTED_REQUESTS on disk matches in-memory after processNextRequest()
- Issue 4: verifies rapid concurrent saves don't lose requests due to out-of-order Onyx.set() resolution
Run npx jest tests/unit/APITest.ts --no-coverage — all pass, including:
- Issue 5: verifies conflict resolver sees complete queue state after out-of-order writes
Run npx jest tests/unit/MiddlewareTest.ts tests/unit/SequentialQueueTest.ts --no-coverage — all pass (no regressions)

Verify that no errors appear in the JS console

Offline tests

Open the app and go to any chat
Go offline
Send a message — it appears optimistically, request is queued
Go back online — the queue starts processing
Force-close the app during processing
Reopen — verify the message was sent exactly once (no duplicates, no lost messages)

QA Steps

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…stedRequests

adhorodyski · 2026-03-10T16:24:52Z

src/libs/actions/PersistedRequests.ts

-        Log.info('[PersistedRequests] hit Onyx connect callback', false, {isValNullish: val == null});
+        Log.info('[PersistedRequests] hit Onyx connect callback', false, {isValNullish: val == null, isInitialized});
+
+        if (isInitialized) {


this is interesting, makes sense to me tbh

…szymonzalarski/investigate-and-fix-sequential-queue-issues-issue-4

…e-and-fix-sequential-queue-issues-issue-4

… make in-memory state authoritative after init

szymonzalarski98 · 2026-03-13T07:43:53Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b867738cf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

src/libs/actions/PersistedRequests.ts

melvin-bot · 2026-03-13T16:41:18Z

@ChavdaSachin Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b867738cf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-13T16:53:02Z

src/libs/actions/PersistedRequests.ts

+        if (isInitialized) {
+            // After initialization, in-memory is authoritative.
+            // Ignore disk callbacks to prevent out-of-order Onyx.set() from
+            // overwriting the correct in-memory state (Bug #80759 Issue 4).
+            return;


Continue syncing queue updates after initialization

Returning early once isInitialized is true makes this connection ignore every later PERSISTED_REQUESTS update, so leader tabs stop seeing writes enqueued by other tabs. Fresh evidence in this commit is that request lifecycle paths now persist queue changes via Onyx.multiSet, while queue execution is still leader-only in src/libs/Network/SequentialQueue.ts (isClientTheLeader() gate), which means non-leader writes can remain unprocessed until reload or leader change because the leader’s in-memory persistedRequests is never updated.

Useful? React with 👍 / 👎.

ChavdaSachin · 2026-03-13T17:24:00Z

I could see the previous PR was reviewed by @mountiny, so do I need to review this?

mountiny · 2026-03-15T23:12:54Z

@ChavdaSachin yes please, can you also do the testing?

…e-and-fix-sequential-queue-issues-issue-4

github-actions · 2026-03-18T11:05:22Z

🚧 @mjasikowski has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-03-18T11:10:10Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #84622.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-46fc401-84e475f-77d09b73609720c3600ee9a5e3eae06352e51080/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-46fc401-84e475f-504ddab03ed9b6fbb0e6cb65555a77d56cfbdb77/index.html

Web 🕸️
https://84622.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-03-18T11:11:04Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2026-03-18T13:29:39Z

🚀 Deployed to staging by https://github.com/mjasikowski in version: 9.3.40-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	failure ❌
🍎 iOS 🍎	success ✅

OSBotify · 2026-03-18T14:05:40Z

🚀 Deployed to staging by https://github.com/mjasikowski in version: 9.3.40-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

The Promise.resolve().then(() => flush(false)) workaround was needed because stale Onyx.set() callbacks could overwrite in-memory persistedRequests during process() (Issues 3c/4 in Expensify#80759). PR Expensify#84622 fixed this at the source: PersistedRequests now treats in-memory state as authoritative after initialization, making the deferral unnecessary. Call flush(false) directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jponikarchuk · 2026-03-18T15:35:25Z

Deploy Blocker #85669 was identified to be related to this PR.

jponikarchuk · 2026-03-18T15:56:58Z

Deploy Blocker #85673 was identified to be related to this PR.

lanitochka17 · 2026-03-18T16:06:40Z

Deploy Blocker #85671 was identified to be related to this PR

szymonzalarski98 · 2026-03-18T16:10:05Z

@jponikarchuk @lanitochka17 on it

szymonzalarski98 · 2026-03-18T16:20:07Z

These 3 deploy blockers are all about file upload/attachment failures, not about the sequential queue persistence pipeline.

The root cause is PR #70740 ("refactor: Improve and simplify attachment and file validation"), merged today, which modifies AttachmentUtils.ts, validateAttachmentFile.ts, FileUtils.ts, and attachment modal components.

Issue #85671 directly links PR #70740 in its "regression testing" field.

Our PR #84622 only touches PersistedRequests.ts, SequentialQueue.ts, and API/index.ts — none of which handle file validation, attachment rendering, or upload UI. Our changes affect when a request is persisted to disk and how Onyx callbacks are handled, not the file handling pipeline.

cc: @jponikarchuk @lanitochka17 @adhorodyski @mountiny

mountiny · 2026-03-18T19:58:30Z

@szymonzalarski98 QA found these regressions using the binary search method. This is the first PR based on the chronological order of being merged that has this regression

mountiny · 2026-03-18T20:02:26Z

so either there is some issue with the builds cc @LukasMod or order, or not sure what is wrong here 😄

lanitochka17 · 2026-03-18T20:59:35Z

Deploy Blocker ##85708 was identified to be related to this PR.

ChavdaSachin · 2026-03-18T22:58:36Z

I reverted the current PR locally and could confirm #85708 is caused by the current PR, haven't tested the other blockers but they are likely to be related to current PR as well based on the similarity(file upload related issues).

My copilot came up with the changes which fixed the issue #85708, but I haven't reviewed the code (gotta go offline now 💤).

szymonzalarski98 · 2026-03-19T07:15:07Z

@ChavdaSachin thank you for letting me know, I'm working on a fix for those issues

jponikarchuk · 2026-03-19T10:57:34Z

Deploy Blocker #85755 was identified to be related to this PR.

szymonzalarski98 · 2026-03-20T09:50:29Z

Hey, here is a PR with fix #85920

Ensure in-memory state is authoritative after initialization in Persi…

2728886

…stedRequests

melvin-bot bot assigned szymonzalarski98 Mar 9, 2026

adhorodyski reviewed Mar 10, 2026

View reviewed changes

adhorodyski approved these changes Mar 10, 2026

View reviewed changes

szymonzalarski98 changed the title ~~Ensure in-memory state is authoritative after initialization in Persi…~~ [No QA] Ensure in-memory state is authoritative after initialization in Persi… Mar 11, 2026

szymonzalarski98 added 3 commits March 11, 2026 07:42

Trigger CI checks

ec5da17

Merge remote-tracking branch 'upstream/main' into callstack-internal/…

dc87c37

…szymonzalarski/investigate-and-fix-sequential-queue-issues-issue-4

Restart CI

8cfba53

szymonzalarski98 force-pushed the callstack-internal/szymonzalarski/investigate-and-fix-sequential-queue-issues-issue-4 branch from 3a608e0 to 8cfba53 Compare March 11, 2026 08:15

szymonzalarski98 added 6 commits March 11, 2026 21:58

Re-run CI checks

745cb1b

Restart CI

944dc31

Merge remote-tracking branch 'upstream/main' into callstack-internal/…

19a90f3

…szymonzalarski/investigate-and-fix-sequential-queue-issues-issue-4

Merge branch 'main' into callstack-internal/szymonzalarski/investigat…

ecf269f

…e-and-fix-sequential-queue-issues-issue-4

Merge branch 'main' into callstack-internal/szymonzalarski/investigat…

3b8101e

…e-and-fix-sequential-queue-issues-issue-4

fix: persist queue and ongoing request to disk in processNextRequest,…

6b86773

… make in-memory state authoritative after init

szymonzalarski98 changed the title ~~[No QA] Ensure in-memory state is authoritative after initialization in Persi…~~ [No QA] Fix lost/duplicated requests in PersistedRequests (Issues 3, 4, 5) Mar 13, 2026

chatgpt-codex-connector bot reviewed Mar 13, 2026

View reviewed changes

src/libs/actions/PersistedRequests.ts Outdated Show resolved Hide resolved

szymonzalarski98 marked this pull request as ready for review March 13, 2026 16:41

szymonzalarski98 requested review from a team as code owners March 13, 2026 16:41

melvin-bot bot requested review from ChavdaSachin and removed request for a team March 13, 2026 16:41

melvin-bot bot requested review from heyjennahay and removed request for a team March 13, 2026 16:41

chatgpt-codex-connector bot reviewed Mar 13, 2026

View reviewed changes

Merge branch 'main' into callstack-internal/szymonzalarski/investigat…

f024caf

…e-and-fix-sequential-queue-issues-issue-4

mjasikowski merged commit 46fc401 into Expensify:main Mar 18, 2026
33 of 38 checks passed

OSBotify mentioned this pull request Mar 18, 2026

Deploy Checklist: New Expensify 2026-03-18 #85646

Closed

100 tasks

adhorodyski mentioned this pull request Mar 18, 2026

HOLD fix network state detection #84760

Open

50 tasks

lanitochka17 mentioned this pull request Mar 18, 2026

[Due for payment 2026-02-26] Investigate and fix sequential queue issues #80759

Open

szymonzalarski98 changed the title ~~[No QA] Fix lost/duplicated requests in PersistedRequests (Issues 3, 4, 5)~~ [No QA] Fix lost/duplicated requests in PersistedRequests (Issues 2, 3, 4, 5) Mar 18, 2026

MelvinBot mentioned this pull request Mar 18, 2026

Tags - Unable to import multi-level tags #85708

Closed

7 tasks

MelvinBot mentioned this pull request Mar 19, 2026

Chat - Custom avatar is not displayed on a group chat #85726

Closed

7 tasks

mountiny mentioned this pull request Mar 19, 2026

[CP Staging] Revert "[No QA] Fix lost/duplicated requests in PersistedRequests (Issues 2, 3, 4, 5)" #85804

Merged

This was referenced Mar 20, 2026

Fixes for sequential queue issues #85898

Closed

Fix lost/duplicated requests in PersistedRequests (Issues 2, 3, 4, 5) + deploy blocker fix #85902

Closed

Fix lost/duplicated requests in PersistedRequests (Issues 2, 3, 4, 5) + deploy blocker fix #85920

Open

VickyStash mentioned this pull request Mar 20, 2026

Reapply Onyx bump to v3.0.46 #85919

Open

52 tasks

Conversation

szymonzalarski98 commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Sequential Queue Issues — Status Report

Fixed: Issues 2, 3, 4, 5

Issue 3: processNextRequest() and rollbackOngoingRequest() don't persist to disk

Issue 4: Connect callback overwrites in-memory state with stale disk data

Issue 5: Conflict resolver sees stale queue

Issue 2: Nothing in the write pipeline is awaited

Not Fixed: Issue 1 — Optimistic data is applied before the request is persisted

The Problem

Why We Can't Fix It Now

Plan for Future Fix

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

adhorodyski Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

szymonzalarski98 commented Mar 13, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

melvin-bot bot commented Mar 13, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

ChavdaSachin commented Mar 13, 2026

Uh oh!

mountiny commented Mar 15, 2026

Uh oh!

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

OSBotify commented Mar 18, 2026

Uh oh!

OSBotify commented Mar 18, 2026

Uh oh!

OSBotify commented Mar 18, 2026

Uh oh!

jponikarchuk commented Mar 18, 2026

Uh oh!

jponikarchuk commented Mar 18, 2026

Uh oh!

lanitochka17 commented Mar 18, 2026

Uh oh!

szymonzalarski98 commented Mar 18, 2026

Uh oh!

szymonzalarski98 commented Mar 18, 2026

Uh oh!

mountiny commented Mar 18, 2026

Uh oh!

mountiny commented Mar 18, 2026

Uh oh!

lanitochka17 commented Mar 18, 2026

Uh oh!

ChavdaSachin commented Mar 18, 2026

Uh oh!

szymonzalarski98 commented Mar 19, 2026

Uh oh!

jponikarchuk commented Mar 19, 2026

Uh oh!

szymonzalarski98 commented Mar 20, 2026

Uh oh!

Reviewers

szymonzalarski98 commented Mar 9, 2026 •

edited

Loading