fix: coalesce redundant GetMissingOnyxMessages while a fetch is in flight by adhorodyski · Pull Request #93948 · Expensify/App

adhorodyski · 2026-06-18T15:49:32Z

Explanation of Change

When the app detects a gap in server update IDs it fetches the missing ones via GetMissingOnyxMessages (GMOM). The gap-branch guard in OnyxUpdateManager checked areDeferredUpdatesQueued and isFetchingForPendingUpdates to decide whether a fetch was already running.

Mid-fetch, the deferred queue drains while the GMOM request is still in flight. A concurrent Pusher update arriving in that window saw an empty queue and started a second parallel fetch. Under a burst this stampeded. Production saw 28 GMOM requests in 9 seconds, which saturated the main thread and caused dead clicks. The same slip also overwrote the in-flight query promise and attached a second finalize callback, so SequentialQueue resumed twice and could send requests against half-applied data.

getMissingOnyxUpdatesQueryPromise() is the handle on the in-flight fetch, and the reconnect branch already treats it as authoritative. The gap-branch guard now includes it, so the branch stands down when a fetch is already in flight. The concurrent update is not lost: it is enqueued just before the guard, so the running fetch picks it up in order. The guard comment states the invariant it enforces rather than naming a single signal.

The first reconnect had the same kind of drop. Before a client has ever applied a reliable update, the gap handler runs a full reconnectApp through finalReconnectAppAfterActivatingReliableUpdates instead of a GMOM fetch. That promise did not drain the deferred queue, so an update that arrived while the reconnect was in flight stayed in the queue until finalizeUpdatesAndResumeQueue cleared it without applying it. The reconnect promise now chains validateAndApplyDeferredUpdates, the same way the gap-fetch and pending-fetch paths do, so the queued update is applied in order.

Fixed Issues

$ #92541
PROPOSAL:

Tests

Check out this branch and run npm run test -- tests/unit/OnyxUpdateManagerTest.ts.
Verify all 14 tests pass, including the four regression tests that fail on main and pass here:
- should not re-fetch missing updates when a duplicate arrives while a fetch is already in flight
- should resume the SequentialQueue exactly once when a duplicate arrives while a fetch is already in flight
- should defer and later fetch a brand-new update that arrives while a fetch is already in flight
- should apply an update that arrives mid-reconnect instead of dropping it when the queue is finalized
Sign in with a high-traffic account, trigger a burst of incoming updates (e.g. open a busy chat while another client posts), and verify the app stays responsive, with no freezes or dead clicks, and data stays in sync.
Verify that no errors appear in the JS console.

Verify that no errors appear in the JS console

Offline tests

Go offline, then come back online with pending updates on the server.
Verify the client fetches missing updates in order, applies them, and resumes the SequentialQueue once, with no duplicate GMOM requests and no stuck/paused queue.

QA Steps

Sign in with a high-traffic account.
Generate a burst of incoming updates (e.g. have another user post rapidly in a shared chat, or reconnect after being offline).
Verify the app stays responsive during the burst, with no freezes or dead clicks.
Verify reports and chats stay in sync with the server after the burst settles.
Verify that no errors appear in the JS console.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

No UI changes.

Android: mWeb Chrome

No UI changes.

iOS: Native

No UI changes.

iOS: mWeb Safari

No UI changes.

MacOS: Chrome / Safari

No UI changes.

…mpede OnyxUpdateManager's gap branch guards on `areDeferredUpdatesQueued || isFetchingForPendingUpdates` but never consults `getMissingOnyxUpdatesQueryPromise()`. When a concurrent update arrives in the window where the deferred queue has been drained but the fetch is still in flight, the guard leaks and fires a redundant GetMissingOnyxMessages. This test injects a duplicate mid-cycle via the beforeApplyUpdates hook and asserts the duplicate is coalesced (2 fetches, not 3). It fails on current HEAD and will pass once the gap-branch guard also checks the query promise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Complements the duplicate-injection regression with the range-coverage half of the guarantee: a brand-new, higher update injected in the drained-but-in-flight window (via beforeApplyUpdates) must not fire its own parallel GetMissingOnyxMessages, yet must not be dropped either. Asserts the fetches stay sequential and in order (1-2, 3-4, 5-6) with no overlapping leak, and that update7 is actually applied (client reaches 7) via the post-fetch recursion. Fails on current HEAD; passes once the gap-branch guard also consults getMissingOnyxUpdatesQueryPromise(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The in-flight stampede has a second symptom beyond the redundant fetch: the leaked call overwrites the in-flight query promise and attaches a second finalize, so finalizeUpdatesAndResumeQueue runs twice and the SequentialQueue is resumed twice. The first resume can fire while the other fetch is still applying updates, an ordering hazard invisible to a fetch-count assertion. Spies SequentialQueue.unpause and asserts exactly one resume, draining past the first finalize via waitForBatchedUpdates so a leaked second finalize has settled before counting. afterEach restoreAllMocks keeps the spy from leaking into other tests even when an assertion throws. Fails on current HEAD (resumes twice); passes once the gap-branch guard also consults getMissingOnyxUpdatesQueryPromise(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The guard stands down on more than a queued deferred-update list; the comment now states the actual invariant it enforces (a fetch is already in progress) instead of naming only one of the signals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The gap branch decided whether a missing-updates fetch was already running by checking only `areDeferredUpdatesQueued`/`isFetchingForPendingUpdates`. Mid-fetch, the deferred queue is drained while the GetMissingOnyxMessages request is still in flight; a concurrent Pusher update arriving in that window saw an empty queue and started a redundant parallel fetch. Under a burst this stampeded (28 fetches in 9s in production), saturating the main thread and causing dead clicks. It also overwrote the in-flight query promise and attached a second finalize, resuming the SequentialQueue twice. The reliable signal — `getMissingOnyxUpdatesQueryPromise()`, the handle on the in-flight fetch — was already authoritative in the adjacent reconnect branch. Add it to the guard so the gap branch stands down when a fetch is in flight. The new update is not lost: it is enqueued just above, so the running fetch sweeps it up in order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

adhorodyski · 2026-06-18T15:54:29Z

@codex review

melvin-bot · 2026-06-18T15:58:50Z

@mkhutornyi 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]

mkhutornyi · 2026-06-18T16:01:36Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

android.mov

Android: mWeb Chrome

iOS: HybridApp

ios.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

web.mov

web2.mov

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c35e7a430b

ℹ️ 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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c35e7a430b

ℹ️ 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".

mountiny · 2026-06-18T16:17:12Z

@MelvinBot Review

MelvinBot · 2026-06-18T16:20:45Z

🤖 Review — the core fix is correct and well-targeted, with strong regression coverage. One edge case (matching Codex's P2) is worth confirming before merge.

The fix is sound

Adding getMissingOnyxUpdatesQueryPromise() to the gap-branch guard (OnyxUpdateManager/index.ts:178) is the right signal — it's the same authoritative handle the reconnect branch already guards on (index.ts:157). For both the gap-fetch and pending-fetch paths the in-flight promise chains validateAndApplyDeferredUpdates (index.ts:191), which drains the deferred queue — so coalescing the concurrent update (enqueued just above the guard) is safe: the running fetch sweeps it up in order. The three regression tests cover the redundant fetch, the double unpause, and the brand-new-update-deferral cases well.

One edge case to confirm (agree with Codex P2)

The guard now coalesces against any in-flight promise, but not every in-flight promise drains the deferred queue. The first-time reconnect promise — finalReconnectAppAfterActivatingReliableUpdates() (App.ts:529-541) — does not chain validateAndApplyDeferredUpdates; it just resolves and finalizeUpdatesAndResumeQueue clears the queue (index.ts:69-77).

So in the narrow window where the reconnect response has advanced lastUpdateIDAppliedToClient (making lastUpdateIDFromClient truthy, so a new update reaches the gap branch rather than the reconnect branch) but the reconnect promise hasn't yet been cleared, the new guard would enqueue that update and return false — and the finalize then clears it without fetching. On main that same update starts its own GMOM and is applied.

Why this is likely low-severity, not a blocker

The first-time-reconnect path (!lastUpdateIDFromClient) is rare and transient.
The window is microtask-narrow (it requires a Pusher update to land between the reconnect response being applied and finalize running).
A dropped update is self-healing: the next server update detects the previousUpdateID gap and refetches the missing range, so data converges — there may just be a brief stale window.

Suggestion: either confirm this is acceptable (recovered via gap detection), or tighten the guard so it only coalesces against promises that drain the queue (e.g. gate on isFetchingForPendingUpdates || areDeferredUpdatesQueued || <a flag set only for the GMOM/pending fetch paths> rather than the raw reconnect promise). A regression test injecting an update mid-reconnect would lock in whichever behavior you choose.

This is an automated review — treat it as a second opinion, not a gate. Reply with @MelvinBot <feedback> to refine.

mkhutornyi

There's one outstanding comment.
Otherwise looks good

…dates are not dropped Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

adhorodyski · 2026-06-19T11:56:15Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 293075d811

ℹ️ 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".

adhorodyski · 2026-06-19T12:12:46Z

let's wait as I validate the most recent review comment.

update: done ✅

github-actions · 2026-06-19T12:36:39Z

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

github-actions · 2026-06-19T12:42:14Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-00123cb073f-912d4a2-6d21570bb2b3be915a666592fb4724c5e3d5ccee/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-00123cb-912d4a2-507c968bfd5d1dc206641d79e7323ffb5dabc127/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-06-19T15:52:16Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.16-1 🚀

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

Bundle Size Analysis (Sentry):

Android
iOS

MelvinBot · 2026-06-19T15:53:40Z

🤖 Help site review: no changes required.

This PR is a purely internal reliability fix to the Onyx update-sync mechanism (OnyxUpdateManager). It coalesces redundant GetMissingOnyxMessages (GMOM) requests while a fetch is in flight and ensures updates that arrive mid-reconnect are applied in order. The only files touched are src/libs/actions/OnyxUpdateManager/index.ts and a unit test — there are no UI changes, no new or renamed features, tabs, settings, or buttons, and no change to any user-facing workflow.

Help site articles under App/docs/articles document customer-facing product behavior, none of which is affected by this change. I reviewed the diff against the help authoring guidelines and found nothing that requires a docs update, so I did not open a draft PR.

@adhorodyski, if you believe a customer-facing behavior changed that I missed, reply with the specifics and I'll draft the help site update.

OSBotify · 2026-06-22T23:04:53Z

🚀 Deployed to production by https://github.com/blimpich in version: 9.4.16-5 🚀

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

adhorodyski and others added 6 commits June 18, 2026 14:32

Merge branch 'Expensify:main' into reconnect-patterns-mode-2

7364d1b

melvin-bot Bot assigned adhorodyski Jun 18, 2026

adhorodyski marked this pull request as ready for review June 18, 2026 15:58

adhorodyski requested review from a team as code owners June 18, 2026 15:58

melvin-bot Bot requested review from mkhutornyi and trjExpensify and removed request for a team June 18, 2026 15:58

melvin-bot Bot removed the request for review from a team June 18, 2026 15:58

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdateManager/index.ts

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdateManager/index.ts

mkhutornyi reviewed Jun 18, 2026

View reviewed changes

fix: drain deferred queue on first-time reconnect so mid-reconnect up…

293075d

…dates are not dropped Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Jun 19, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdateManager/index.ts

mountiny approved these changes Jun 19, 2026

View reviewed changes

mountiny merged commit 00123cb into Expensify:main Jun 19, 2026
40 checks passed

mkhutornyi approved these changes Jun 19, 2026

View reviewed changes

melvin-bot Bot requested a review from blimpich June 19, 2026 12:36

melvin-bot Bot requested a review from mountiny June 19, 2026 12:36

github-actions Bot mentioned this pull request Jun 19, 2026

Investigate workflow job failing on main: test / test (job 6) #94045

Closed

adhorodyski deleted the reconnect-patterns-mode-2 branch June 19, 2026 12:54

OSBotify mentioned this pull request Jun 19, 2026

Deploy Checklist: New Expensify 2026-06-19 #94062

Closed

48 tasks

adhorodyski mentioned this pull request Jun 22, 2026

Collapse in-flight reconnects by coverage to close the SequentialQueue dedup gap #94210

Merged

43 tasks

melvin-bot Bot mentioned this pull request Jun 22, 2026

[Due for payment 2026-07-03] [Due for payment 2026-06-29] [Performance] App hangs when OpenApp / ReconnectApp / GetMissingOnyxMessages fire in rapid stampedes #92541

Open

4 tasks

elirangoshen mentioned this pull request Jun 30, 2026

fix: back off GetMissingOnyxMessages once it stops advancing the client #94974

Open

43 tasks

adhorodyski mentioned this pull request Jul 7, 2026

fix: escalate to an incremental ReconnectApp when the missing-updates fetch stops advancing the client #95477

Open

41 tasks

mountiny mentioned this pull request Jul 7, 2026

fix: escalate to an incremental ReconnectApp when the missing-updates fetch stops advancing the client #95486

Open

Uh oh!

Conversation

adhorodyski commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

adhorodyski commented Jun 18, 2026

Uh oh!

melvin-bot Bot commented Jun 18, 2026

Uh oh!

mkhutornyi commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

mountiny commented Jun 18, 2026

Uh oh!

MelvinBot commented Jun 18, 2026

The fix is sound

One edge case to confirm (agree with Codex P2)

Uh oh!

mkhutornyi left a comment

Choose a reason for hiding this comment

Uh oh!

adhorodyski commented Jun 19, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

adhorodyski commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

github-actions Bot commented Jun 19, 2026

Uh oh!

OSBotify commented Jun 19, 2026

Uh oh!

MelvinBot commented Jun 19, 2026

Uh oh!

OSBotify commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adhorodyski commented Jun 18, 2026 •

edited

Loading

mkhutornyi commented Jun 18, 2026 •

edited

Loading

adhorodyski commented Jun 19, 2026 •

edited

Loading