Recover the Pusher apply chain after a rejection, behind a watermark continuity guard by adhorodyski · Pull Request #100594 · Expensify/App

adhorodyski · 2026-09-08T12:22:36Z

Explanation of Change

Three changes:

Recover the Pusher apply chain after a rejection, so the next update still runs.
Hold lastUpdateIDAppliedToClient behind an update whose apply failed, released only when a response proves it covered that range.
Heal the Airship chain the same way.

Before this, one rejected apply left the shared chain rejected for the session, so every later update failed before running. In production logs for 2026-09-01 to 09-07, about 6 users a day (0.011%, 1 in 9,300) hit it. Of the 43 affected that week, 13 went hours without live updates and 12 needed a reload.

Recovery still waits for the next update, so an idle client stays behind until one arrives or it reconnects.

This replaces the safety net that #99774 pinned in OnyxUpdatesPusherChainFailureTest.ts: a stalled chain no longer stops a follower update from applying, so that test now asserts the watermark held behind the failed update is what keeps the missing range recoverable, which is the same protection without leaving the client dark.

Fixed Issues

$ #100174
PROPOSAL: N/A

Tests

This is a resilience fix for a failure that cannot be triggered from the UI, so the behaviour it restores is covered by unit tests in tests/unit/OnyxUpdatesTest.ts and tests/unit/OnyxUpdateManagerTest.ts, which run on CI. They cover:

a Pusher update applies normally after an earlier one rejected, so the chain is no longer poisoned
an Airship update applies normally after an earlier one rejected
the failed apply still rejects, so its caller can report it
the watermark stays behind the failed update, and doesClientNeedToBeUpdated still reports the client as behind
the watermark stays behind it even after a later WRITE flushes, so the gap fetch still starts below it
a catch-up fetch covering the failed range lets the watermark advance again
a full reconnect covering the failed range lets the watermark advance again
an OpenApp covering the failed range lets the watermark advance again
a redelivery of the failed update itself releases the hold, while a response that only ends on its ID does not
a failure already covered by a completed reconnect is not held back again
the next update after a failure fetches a range that covers the failed update
a failed apply still finishes the Android background task, and is reported rather than left unhandled
two Pusher applies enqueued in the same tick still run in enqueue order

Smoke test that the normal path did not regress:

Sign in on two devices with the same account and open the same chat on both.
Send several messages from device A and verify they appear on device B in order, with no duplicates.
Verify no [OnyxUpdateManagerError] lines and no unhandled promise rejections appear in the JS console.

Verify that no errors appear in the JS console

Offline tests

No offline-specific behaviour changes. The paths touched only run while connected, when Pusher updates or API responses are being applied. Going offline and back online still triggers the usual reconnect and catch-up flow; this PR only affects what happens when an apply of those updates fails.

Go offline, send a message, verify it appears with the offline (pending) styling.
Go back online and verify the message sends and the chat is up to date, with no duplicated or missing messages.

QA Steps

Sign in on two devices (or two browsers) with the same account and open the same chat on both.
Send several messages from device A in quick succession.
Verify all of them appear on device B, in order, with no duplicates and none stuck pending.
Reload device B and verify every message is still there, in order.
Verify no errors appear in the JS console.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

No UI surface: this changes how Onyx update application recovers from a failure. Nothing visual to capture.

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…mark continuity guard A rejected Pusher apply left the shared pusherEventsPromise rejected for the rest of the session, so every later Pusher update failed before its handler ran. Heal the chain in a catch while still handing the rejection to the caller. Healing alone would be data loss, since nothing checks that update IDs are contiguous before advancing the watermark, so also refuse to advance past an update whose apply failed until a catch-up response re-delivers that range. Finally, key recovery off the failure itself in applyOnyxUpdatesReliably: an update with previousUpdateID 0 never reaches the gap check, so a failed apply there would otherwise wait for an incidental gap.

# Conflicts: # tests/unit/OnyxUpdateManagerTest.ts

Inject apply failures with real handlers registered via subscribeToMultiEvent instead of spying on PusherUtils.triggerMultiEventHandler, prove same-tick ordering through merged Onyx state instead of a mock call log, split failure propagation from chain recovery, and drop the SequentialQueue.isPaused assertion.

adhorodyski · 2026-09-08T14:11:08Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94f6a82e5c

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

A full ReconnectApp and OpenApp both re-download the whole state, so they cover a failed update range at least as well as an incremental catch-up. Clearing the latch only on catch-up left the watermark pinned forever on a client whose recovery traffic is full reloads, turning every later update into a false gap.

# Conflicts: # src/libs/actions/OnyxUpdates.ts # src/libs/actions/applyOnyxUpdatesReliably.ts # tests/unit/OnyxUpdatesTest.ts

adhorodyski · 2026-09-09T13:40:49Z

@MelvinBot please review this PR

melvin-bot · 2026-09-09T13:42:01Z

@nyomanjyotisa 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: a1f0ca0edb

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

MelvinBot · 2026-09-09T13:50:56Z

The three-part split is the right shape, and rewriting OnyxUpdatesPusherChainFailureTest to assert the watermark hold instead of the poisoned chain is the correct way to re-state that invariant. One issue I'd block on: the hold in part 2 can be silently defeated, so the safety net the rewritten test claims to protect isn't actually there.

🔴 The watermark hold is defeated by `lastUpdateIDPendingWriteFlush`

The new guard's early return at OnyxUpdates.ts:211-216 skips the lastUpdateIDPendingWriteFlush reset at OnyxUpdates.ts:227-229, but that marker was already raised before the apply at OnyxUpdates.ts:261. Since getEffectiveLastUpdateID() is a flat max over it, one WRITE landing after the failure pushes the effective watermark past the failed range and keeps it there.

Concretely: Pusher update 20 fails → watermark held at 10, lastFailedUpdateID = 20. A WRITE response with lastUpdateID: 30 arrives → lastUpdateIDPendingWriteFlush = 30, its flush succeeds, the guard early-returns, and nothing ever clears the 30. From then on:

handleMissingOnyxUpdates computes the catch-up range from 30, so GetMissingOnyxMessages(30, …) skips 11–20.
That response is a catch-up with lastUpdateID >= 20, so OnyxUpdates.ts:207-209 clears lastFailedUpdateID and the watermark jumps past a range that was never refetched — no alert, no gap left.

The release condition only checks the recovering response's upper bound; nothing checks where its range started. Two suggestions, either of which closes it:

Clear lastUpdateIDPendingWriteFlush in the early-return branch, exactly as it already does for lastUpdateIDPendingPusherApply on line 212. That matches the policy the file already states at OnyxUpdates.ts:236-238 — err toward a redundant refetch rather than masking a gap.
Make the release provable instead of assumed: request?.data?.updateIDFrom is on the catch-up and incremental-reconnect requests, so also require Number(request.data.updateIDFrom) <= lastFailedUpdateID. Full ReconnectApp and OpenApp re-download everything, so they still need no lower-bound check.

🟡 `lastFailedUpdateID` survives a successful re-apply of the failed update

If the failed update is later re-delivered and applies (Number(lastUpdateID) === lastFailedUpdateID), neither branch runs: the watermark correctly advances to 20, but lastFailedUpdateID stays 20. The next update is then blocked from advancing the watermark, which forces a SequentialQueue.pause() plus a redundant GetMissingOnyxMessages before it self-heals — and logs a misleading Not advancing past an update whose apply failed. Worth adding Number(lastUpdateID) === lastFailedUpdateID to the clear condition on line 207.

🟡 The new test leaves the queue paused if it fails

OnyxUpdateManagerTest.ts:765 unpauses inline after the assertion. The file's afterEach only does jest.restoreAllMocks(), and resetDeferralLogicVariables passes shouldUnpauseSequentialQueue: false — so if that expect ever fails, SequentialQueue stays paused for the rest of the file and the failure cascades. Move it to afterEach or a finally.

While you're there: the test asserts getMissingOnyxUpdates was called once but not with what. Since the whole point of part 3 is fetching the missing range from a previousUpdateID: 0 update, asserting the arguments would lock in the behaviour that matters.

Things I checked that look correct

Chain ordering is preserved. pusherEventsPromise = applyPromise.catch(() => {}) still serializes each apply behind the previous one; the log-only .then on line 112 is overwritten by line 122 in the same call, so same-tick enqueues keep their order. The new applies same-tick Pusher updates in enqueue order test covers this.
The returned promise still rejects, so applyOnyxUpdatesReliably's new .catch sees the failure while the chain itself recovers. Good separation.
Transient out-of-order state is inherent, not a defect. Applying 30 before 20 means Onyx briefly holds state that never existed on the server, repaired when the refetch reapplies 11–30 in order. Same as any reconnect, and the PR reasons about it explicitly.
lastFailedUpdateID resets on sign-out via the val === undefined branch on line 50, and Onyx.clear() in the tests' beforeEach means it doesn't leak between the new OnyxUpdatesTest cases.
applyOnyxUpdatesReliably no longer rejects to its callers. For subscribeToPushNotifications.ts:116-117 that's an improvement — finishBackgroundProcessing() now runs after recovery instead of being skipped. Just flagging it as an intentional behaviour change, since the one case it papers over is a non-leader client, where handleMissingOnyxUpdates returns early and the caller sees success with no recovery attempted.

What I did and didn't verify

This is a code review against a1f0ca0. I did not run the tests locally or drive the app — the fix has no UI surface by design, and the smoke test in the PR body needs two devices on one account, which a single session can't do. CI's Jest run is the real signal for the eight new unit tests.

Next Steps: Reply with @MelvinBot <your feedback> to refine this review.

A second review pass found four ways the hold added in this PR could be walked over or held too long: - Every catch-up range is computed from getEffectiveLastUpdateID, so a WRITE landing after the failure carried it past the failed range and the recovery fetch skipped the hole. Report only the persisted watermark while a failure is outstanding, which also stops a redelivery of the failed update itself from looking old and being discarded. - Releasing the hold only checked where the recovering response ended. Require proof it covered the range: a catch-up must have started at or below the failure, and any other response must carry a previousUpdateID at or below our watermark, which makes its range contiguous over the hole. - Refuse the failed ID itself, not just IDs above it, so a response that merely ends on it cannot pull the watermark onto the hole. - shouldAdvanceLastUpdateID is captured before the apply is awaited, so a failure whose range a reconnect had already covered was recorded anyway, costing a false gap and a redundant refetch. Re-check the live watermark. Also heal airshipEventsPromise the same way as the Pusher chain: the linked issue asks for both, and one rejected Airship apply otherwise stops every later push for the session. The unpause at the end of the OnyxUpdateManager test was dead code: finalizeUpdatesAndResumeQueue already resumes the queue before the awaited promise settles.

GetMissingOnyxMessages is exclusive of updateIDFrom, so a response whose updateIDFrom equals the failed update carries only the updates after it and can never re-deliver the one that failed. Release the latch only when the bound sits strictly below it. Add coverage that passes a real data.updateIDFrom: one test pinning the hold at the boundary, one control pinning the legitimate release one ID lower. The existing release test passed data: {}, so the bound was never exercised.

The hold was a single number and the catch path took a Math.min, so a second failure at a higher ID was not overwritten, it was never recorded: with 20 and 40 failing the guard read 20, the catch-up that legitimately covered 20 cleared it, and a later response walked the watermark past the still-missing 40 with no gap left to report. Keep the set of unrecovered failures and express the release as an explicit covered range (coveredFrom, lastUpdateID], so a response releases only the failures inside the range it provably re-delivered and every other one stays held. A full reconnect and OpenApp cover from 0, a catch-up covers from its updateIDFrom, a response chained onto the watermark covers from its previousUpdateID, and a response that proves nothing releases nothing. That makes this hole and the off-by-one bound from the last round unrepresentable instead of separately guarded, and leaves the release semantics those rounds converged on. Bound the set at CONST.NETWORK.MAX_TRACKED_FAILED_UPDATE_IDS, alongside the stall guard and watchdog constants it interacts with, by forgetting the highest failures into a remembered band: min and max of what the cap dropped, so a response releases that band only when it covers all of it. Below the cap the watermark advances as far as a response genuinely covers; at the cap it holds until one does, which is what the stall guard's escalation then provokes. failedUpdateCount and hasUntrackedFailedUpdateIDs join the existing failure alert to make the cap observable, and a previousUpdateID of the string "0" no longer counts as covering from 0, which now matters because that value releases everything. Document the hold, its release, the cap and the escalation it leans on under Pausing & Data-Gap Sync, which described both neighbouring mechanisms without it. Add the two-failure sequence, the descending and three-failure orderings, a control that a partial recovery still advances the watermark as far as it is genuinely covered, the straddling case where a catch-up starting above a lower failure must leave that failure held with its own control, the bound with an assertion that fails if it forgets the lowest rather than the highest, and the string zero.

…ous advance The set of tracked failure IDs was the machinery a per-ID release needs. Replace it with the single number this file already had, now named for what it has to hold: highestFailedUpdateID, the highest lastUpdateID whose apply rejected while sitting above the persisted watermark. The release asks one question instead of testing whether each failed ID falls inside this response's range: did the response re-deliver everything from the persisted watermark up? A full reconnect and OpenApp do. A catch-up does when its updateIDFrom is at or below the watermark. Anything else does only on a previousUpdateID above zero and at or below the watermark, so the string "0" still proves nothing. While the hold is set, a response that cannot answer yes has its own onyxData applied but leaves the watermark exactly where it was. That contiguity is what makes a set unnecessary. Every advance during a hold starts at or below the watermark, so the watermark can never step over a lower failure, and one response that reaches the highest failure has necessarily re-delivered the others. It also keeps recovery live without a reconnect: while the hold is set getEffectiveLastUpdateID() reports the persisted watermark, so the catch-up that the held watermark provokes always declares an updateIDFrom that releases it. The catch path takes a maximum where it took a minimum. The direction is load-bearing: a minimum is cleared by the very response that recovers the first failure, and the next unchained response then walks the watermark past the one still missing, which is the bug being fixed. Retains-every-failure covers it from both arrival orders. Removing the per-ID release removes what the bound existed to protect. The Set, MAX_TRACKED_FAILED_UPDATE_IDS, the saturation bound, the drop-the-highest rule, the remembered untracked band and the five helpers are gone, one number cannot grow, and the failure alert carries highestFailedUpdateID in place of the two cap fields. Reword the hold under Pausing & Data-Gap Sync for one number and the contiguity rule. Drop the two tests that pinned the cap and the drop rule, which cannot be tested once they do not exist, and turn the test that pinned a release above the watermark into one that pins the hold there and the release from it. The reviewer's four regression tests pass with no edit to them.

…oc's release list Three corrections from the buddy review of the one-number rewrite. None of them asks for a product-code change; the guard already behaves correctly in every case, and each of these closes a boundary the suite let move silently. The release compares the response's lastUpdateID to the held failure with >=, and the whole suite passed when that was mutated to a strict >. Every existing catch-up test used a range head above the failure, so the equality case went untested — and it is the production shape, since GetMissingOnyxMessages asks for a range ending at the update that triggered the fetch. Pin it: a catch-up whose lastUpdateID is exactly the failed update must clear the hold, not just carry the watermark onto it, which is what the following unchained response proves. A catch-up whose updateIDFrom sits strictly between the watermark and the failure was the retuned test's old value and nothing replaced it, so only the two ends were pinned. A fetch from above the watermark proves nothing about the range between, so that case holds the watermark: pin it at 19 against a watermark of 10 and a failure at 40. Many outstanding failures were released by one contiguous response only in a test that died with the cap. Restore that scenario without the cap: 105 failures, an unchained response refused, one catch-up covering through the highest released, normal flow back. Reword the release list in the doc: exclusivity is no longer why a catch-up is refused, the watermark bound is, and the chained branch requires a previousUpdateID above zero, so a response at a zero watermark is not treated as chained.

…p helper Measured before cutting: of the 44 tests this branch adds to OnyxUpdatesTest, 39 fail with the product code reverted to the merge base, so the suite is testing this change rather than decorating it. Two of the five that pass on merge-base code are the pins that must stay anyway. The four regression tests are byte-identical and untouched, and both mutation pins still bite after this: the faithful minimum still fails three reviewer tests, and the release boundary moving from >= to > still fails its one test. Cut one test. A catch-up starting between the watermark and the failure holds the watermark, which the same file already pins three times over: at the failed ID, below the watermark, and from above the watermark via a staged WRITE. Removing it leaves the mutation that loosens the watermark bound caught by those same three. Its content was an interior value of a comparison, not a case a caller can tell apart. Shrink another. It looped 105 failures because the bound it was written against was 100, and that constant no longer exists; the count was an implementation artifact and the refusal half duplicated the three-failure test beside it. It now uses the same three failures and asserts only what nothing else does: one contiguous response covering through the highest releases every one of them, and the unchained response after it advances, so the hold is genuinely gone rather than overtaken. Replace two hand-built catch-up response literals with the helper the file already has. The two boundary tests the last review round asked for were constructing, inline and identically, what catchUpResponse returns; their assertions are unchanged. Test surface added by the branch: 817 lines, now 786. No product code touched.

nyomanjyotisa

LGTM!

mountiny

Thanks, changes look good to me

github-actions · 2026-09-17T17:13:55Z

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

OSBotify · 2026-09-17T17:16:08Z

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

github-actions · 2026-09-17T17:18:26Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-26d4fc3-9b6d912-b754c7694e3f483e3e2fa37642871e62bf70542d/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-26d4fc3-9b6d912-d60230030923073f6412fd51a11ab7c47372043f/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-17T22:20:58Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.82-0 🚀

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

MelvinBot · 2026-09-17T22:26:28Z

🤖 No help site changes are required for this PR, so I did not create a draft docs PR.

Why: every change is internal client plumbing for how Onyx updates recover after a failed apply. Nothing in the diff adds, removes, or renames a user-facing feature, tab, setting, or button, and no copy strings changed.

What I checked

Changed files — three source files, six test files, plus cspell.json and a contributing guide:

src/libs/actions/OnyxUpdates.ts — recovers the Pusher and Airship apply chains after a rejection and holds the lastUpdateIDAppliedToClient watermark behind a failed update
src/libs/actions/User.ts:842 — adds a .catch() that logs the failure instead of leaving it unhandled
src/libs/Notification/PushNotification/subscribeToPushNotifications.ts — finishes the Android background task on a failed apply
contributingGuides/SEQUENTIAL_QUEUE.md — contributor documentation, not help site content

Evidence that nothing is user-facing:

No src/languages/*.ts file is touched, and the diff contains no translate( or Localize additions — so no user-visible copy changed.
The PR body itself states there is no UI surface and no screenshots to capture. The behavior it restores is only observable as messages arriving live instead of the client silently going stale.
I searched docs/articles/ for onyx, pusher, airship, and real-time/live updates. The only two hits are Expensify-Card-Notifications.md:8 and Workday.md:17, both unrelated uses of the phrase "Real-Time Updates" about card notifications and Workday syncing. Neither describes the update-application behavior this PR changes.

The user-visible effect is that a client which previously went dark after one rejected update now recovers on the next update. That is a reliability improvement to existing documented behavior, not a change to it. Help site articles describe what the product does, not how the client recovers from an internal failure, so there is nothing to update.

Since no docs PR was needed, there is no linked help site PR to review or mark Ready for review. If you disagree and want an article updated anyway, tell me which one and what it should say, and I'll open the draft PR with the HelpDot label and assign it to you.

view run

OSBotify · 2026-09-18T18:07:21Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.86-0 🚀

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

MelvinBot · 2026-09-18T18:10:53Z

🤖 No help site changes are required. I did not create a draft PR.

This PR changes how the client recovers internally when an Onyx update fails to apply. It adds no feature, setting, button, tab, or user-visible behavior, so nothing in docs/articles describes it.

What I checked

Changed files, by category:

File	Type
`src/libs/actions/OnyxUpdates.ts`	Internal sync logic
`src/libs/Notification/PushNotification/subscribeToPushNotifications.ts`	Internal sync logic
`src/libs/actions/User.ts`	Internal sync logic
`contributingGuides/SEQUENTIAL_QUEUE.md`	Contributor guide, already updated in this PR
`cspell.json`	Tooling
5 test files	Tests

The one documentation file touched is a contributor guide, not a help site article. The help site covers what customers do in the product; this PR covers how the client heals a rejected update chain behind the scenes.

Help site search: no article under docs/articles references Pusher, Onyx, or update watermarks. The user-facing outcome — messages arrive live and in order — is already the documented normal behavior, so no article text becomes wrong or incomplete because of this fix.

Since there are no help site changes, there is no linked docs PR to review. If you think a customer-facing article should call out something here, reply with @MelvinBot and the article you have in mind and I will draft it.

view run

OSBotify · 2026-09-18T19:24:06Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.89-0 🚀

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

MelvinBot · 2026-09-18T19:28:19Z

🤖 Re-checked: still no help site changes are required. I did not create a draft PR.

Nothing has changed since my previous answer — same merge commit (26d4fc3), same 10 files. This PR changes how the client recovers internally when an Onyx update fails to apply. It adds no feature, setting, button, tab, or user-visible behavior, so nothing in docs/articles describes it.

The only documentation file it touches is contributingGuides/SEQUENTIAL_QUEUE.md, a contributor guide rather than a help site article, and this PR already updates it. No article under docs/articles references Pusher, Onyx, or update watermarks, and the customer-facing outcome — messages arrive live and in order — is already the documented normal behavior.

Since there are no help site changes, there is no linked docs PR to review.

Heads up: this is the third time this request has run on this PR, once per staging deploy comment. The answer will stay the same while the diff is unchanged, so you can ignore further repeats.

view run

OSBotify · 2026-09-18T19:59:20Z

🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.89-0 🚀

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

Bundle Size Analysis (Sentry):

Android

melvin-bot Bot assigned adhorodyski Sep 8, 2026

adhorodyski force-pushed the 2938-pusher-recover branch from 96fedf0 to 0917dbb Compare September 8, 2026 12:38

adhorodyski added 3 commits September 8, 2026 15:11

Merge remote-tracking branch 'exfy/main' into 2938-pusher-recover

0a5864c

# Conflicts: # tests/unit/OnyxUpdateManagerTest.ts

fix: satisfy oxfmt and eslint in Pusher recovery tests

a4e5ead

chatgpt-codex-connector Bot reviewed Sep 8, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdates.ts Resolved

adhorodyski added 5 commits September 9, 2026 13:57

Merge remote-tracking branch 'exfy/main' into 2938-pusher-recover

21d8e2d

# Conflicts: # src/libs/actions/OnyxUpdates.ts # src/libs/actions/applyOnyxUpdatesReliably.ts # tests/unit/OnyxUpdatesTest.ts

fix: decomment, spellcheck and typecheck fixes

4f64f90

fix: use a real Report field in the pusher ordering test

0ac8de6

Merge remote-tracking branch 'exfy/main' into 2938-pusher-recover

a1f0ca0

adhorodyski marked this pull request as ready for review September 9, 2026 13:41

adhorodyski requested review from a team as code owners September 9, 2026 13:41

melvin-bot Bot requested review from flaviadefaria and nyomanjyotisa and removed request for a team September 9, 2026 13:41

melvin-bot Bot requested review from a team and removed request for a team September 9, 2026 13:42

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdates.ts Resolved

nyomanjyotisa reviewed Sep 10, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdates.ts Resolved

adhorodyski added 3 commits September 10, 2026 13:14

refactor: decomment the review-round changes

3ada522

test: write the Pusher recovery tests in given/when/then

b010c7b

nyomanjyotisa reviewed Sep 12, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdates.ts Resolved

adhorodyski requested a review from nyomanjyotisa September 14, 2026 18:00

nyomanjyotisa reviewed Sep 15, 2026

View reviewed changes

Comment thread src/libs/actions/OnyxUpdates.ts Resolved

adhorodyski added 4 commits September 15, 2026 16:15

adhorodyski requested a review from nyomanjyotisa September 15, 2026 17:56

Merge branch 'Expensify:main' into 2938-pusher-recover

664f208

nyomanjyotisa approved these changes Sep 16, 2026

View reviewed changes

melvin-bot Bot requested a review from mountiny September 16, 2026 23:41

mountiny approved these changes Sep 17, 2026

View reviewed changes

mountiny merged commit 26d4fc3 into Expensify:main Sep 17, 2026
35 of 37 checks passed

OSBotify mentioned this pull request Sep 17, 2026

Deploy Checklist: New Expensify 2026-09-17 #101475

Closed

82 tasks done

adhorodyski deleted the 2938-pusher-recover branch September 18, 2026 08:52

MelvinBot mentioned this pull request Sep 18, 2026

[$175] "Please enter different email error" displayed when navigate back to Work email modal #101576

Open

4 of 7 tasks

melvin-bot Bot mentioned this pull request Sep 18, 2026

[Due for payment 2026-09-25] A single rejected Pusher apply poisons pusherEventsPromise, stopping all Pusher Onyx updates for the session #100174

Closed

This was referenced Sep 21, 2026

fix: ensure pusher events are processed in the same order they were received #31144

Merged

[HOLD for payment 2023-11-29] [$1000] Removed reactions on offline mode re appear on the receiver account and there is no way to remove them #23679

Closed

Conversation

adhorodyski commented Sep 8, 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 Sep 8, 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 Sep 9, 2026

Uh oh!

melvin-bot Bot commented Sep 9, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

MelvinBot commented Sep 9, 2026

🔴 The watermark hold is defeated by lastUpdateIDPendingWriteFlush

🟡 lastFailedUpdateID survives a successful re-apply of the failed update

🟡 The new test leaves the queue paused if it fails

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nyomanjyotisa left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 17, 2026

Uh oh!

github-actions Bot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 17, 2026

Uh oh!

MelvinBot commented Sep 17, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

MelvinBot commented Sep 18, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

MelvinBot commented Sep 18, 2026

Uh oh!

OSBotify commented Sep 18, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adhorodyski commented Sep 8, 2026 •

edited

Loading

🔴 The watermark hold is defeated by `lastUpdateIDPendingWriteFlush`

🟡 `lastFailedUpdateID` survives a successful re-apply of the failed update