[No QA] Claim the sequential queue read gate for deferred writes by Abdukhamid000 · Pull Request #99815 · Expensify/App

Abdukhamid000 · 2026-08-29T06:23:08Z

cc @JakubKorytko - writeWhenReady is yours.

Explanation of Change

API.write() closes the sequential queue's read gate the moment it is called, so a READ firing right after it parks behind the write. writeWhenReady() did not: while the write waits on its barrier the queue is empty, so waitForWrites() resolves straight away and a screen that refetches the same data can read back pre-write server state. That is #99805.

Deferred writes now hold a gate of their own, and waitForIdle() waits on it before it looks at the queue - so a READ cannot overtake a write that has not been pushed yet. It is a separate gate rather than the queue's own, because the queue opens its gate whenever it finds itself empty, which it is for the whole deferral.

While offline the gate is ignored: the queue cannot run, and push()/flush() open theirs for the same reason. The wait re-checks on every network change, so going offline or coming back mid-barrier both end up in the right state.

On by default, since forgetting to opt in would cost a silent stale read while forgetting to opt out only costs latency. Pass {shouldClaimReadGate: false} to opt out.

Fixed Issues

$ #99805
PROPOSAL: N/A - root-cause follow-up to the revert in #99814

Tests

N/A - nothing calls writeWhenReady in production on this branch, so there is no flow to click through. The behaviour is covered by tests/unit/SequentialQueueReadGateTest.ts and tests/unit/APIWriteWhenReadyTest.ts.

Verify that no errors appear in the JS console

Offline steps

N/A

QA steps

N/A

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

N/A

Android: mWeb Chrome

N/A

iOS: Native

N/A

iOS: mWeb Safari

N/A

MacOS: Chrome / Safari

N/A

MacOS: Desktop

N/A

`API.write()` claims the read gate synchronously: `push()` calls `setIsReadyPromisePending()` before its first await, so a READ firing on the next line parks behind the write. `writeWhenReady()` never did, so deferring a write silently dropped that ordering - the queue stayed empty for the length of the deferral, and a destination screen fetching the same data raced ahead and repopulated itself from pre-write server state. `claimReadGateForDeferredWrite()` claims the gate on behalf of a write that is not on the queue yet. `push()` adopts the claim when the write lands, so the handover leaves no gap, and the queue drain resolves it. `flush()` also had to learn about it: its empty-queue branch resolved the gate unconditionally, which during a deferral means "the write hasn't been pushed yet", not "nothing is coming". The follower branch still resolves, since a tab that never processes the queue would otherwise park READs forever. No-op while offline, matching `push()` and `flush()`, neither of which parks READs behind a queue that isn't running.

melvin-bot · 2026-08-29T06:23:16Z

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

codecov · 2026-08-29T06:26:46Z

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines	Coverage Δ
src/libs/API/writeWhenReady.ts	`98.66% <100.00%> (+0.03%)`	⬆️
src/libs/Network/SequentialQueue.ts	`90.97% <100.00%> (+0.97%)`	⬆️
... and 10 files with indirect coverage changes

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7214f06594

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

github-actions · 2026-08-29T06:28:45Z

+            if (!settleOnce()) {
+                return;
+            }
+            // A later write may have opened a new gate since; resolving that one would let READs through


❌ CONSISTENCY-16 (docs)

Comments should read as plain, natural sentences. This comment joins two independent clauses with a semicolon, which the style guide asks you to write as two separate sentences instead.

Split the clauses into two sentences:

// A later write may have opened a new gate since. Resolving that one would let READs through // while somebody else's write is still pending. if (isReadyPromise !== claimedPromise) { return; }

Reviewed at: 7214f06 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

github-actions · 2026-08-29T06:28:47Z

    waitForIdle: jest.fn(() => Promise.resolve()),
    // Called by the network layer on init; stub so advancing fake timers doesn't hit a missing export.
    flush: jest.fn(),
+    // The claim writeWhenReady settles once its write reaches (or fails to reach) the queue; the real


❌ CONSISTENCY-16 (docs)

Comments should read as plain, natural sentences. This comment uses a semicolon to join two independent sentences; the style guide asks you to write them as two separate sentences.

Split into two sentences:

// The claim writeWhenReady settles once its write reaches (or fails to reach) the queue. The real // gate is covered in SequentialQueueReadGateTest. claimReadGateForDeferredWrite: jest.fn(() => ({handOff: jest.fn(), release: jest.fn()})),

Reviewed at: 7214f06 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Nothing imports the type - consumers use the inferred return type of claimReadGateForDeferredWrite - and exporting it added a knip finding.

JakubKorytko

apart from the code, please make the EoC at least a half shorter, more plain english and simple explanations, its hard to read. Do not add "jest", "typecheck" and "eslint" as a part of tests - there are CI checks so no value in them being mentioned in the section. This seems No QA so add [No QA] to the title, and if no tests are available apart from automated just mark N/A on tests, offline tests and qa tests - no need for QA to read so much text just to learn there is nothing for them to test

JakubKorytko · 2026-08-31T09:02:48Z

+            if (isReadyPromise !== claimedPromise) {
+                return;
+            }
+            releaseClaimedPromise?.();


two deferred writes share a single gate, because setIsReadyPromisePending() no-ops the second time. Shouldn't this also check "am I the last claim?" so the gate isn't lost for the other ones? Something like bailing unless deferredWriteGateClaims === 0 after settleOnce()

JakubKorytko · 2026-08-31T09:02:48Z

+ * flush(), which resolves it: neither parks READs behind a queue that isn't running.
+ */
+function claimReadGateForDeferredWrite(): DeferredWriteReadGateClaim {
+    if (isOfflineNetwork()) {


this only checks offline once, at claim time. If we're offline then and come back online while the barrier is still waiting, the write never claims the gate at all, so a READ in that window doesn't wait for it. push() re-checks at push time, this doesn't

JakubKorytko · 2026-08-31T09:02:48Z


+        // Claimed synchronously, before the barrier is even built: a READ firing on the next line has to
+        // see the gate already pending, exactly as it would had `write()` queued the request right here.
+        const readGateClaim = claimReadGateForDeferredWrite();


every caller now holds the gate, and it's global: API.read and API.paginate all park on the same waitForIdle(), not scoped to the written key. On the opt-out question, I'd keep it on by default and add the escape hatch instead of making it opt-in. Forgetting to opt out only costs some latency, forgetting to opt in costs a silent stale-read race

JakubKorytko · 2026-08-31T09:02:48Z

+
+        // Then both refer to one gate, so releasing it once is enough - the handover leaves no second
+        // gate for a READ to be stranded behind
+        first.release();


this locks in the early release: second is still unsettled here, so its write hasn't been pushed, but the assert wants the gate open. If release() only resolves on the last claim, this flips to false

JakubKorytko · 2026-08-31T09:02:49Z

+        claim.release();
+    });
+
+    it('ignores a release once a later write has opened a new gate', async () => {


this doesn't reach the stale-promise guard it's named after. stale.release() already settled up in the Given block, so the second call returns early on hasSettled and the isReadyPromise !== claimedPromise check never runs. It goes green on idempotency instead

JakubKorytko · 2026-08-31T09:13:46Z

+    /** The write is on its way to `push()`: the queue owns the gate from here, so stop holding `flush()` open. */
+    handOff: () => void;
+
+    /** The write never reached the queue (offline, or `write()` threw): stop holding `flush()` open and resolve the gate. */


same as above

JakubKorytko · 2026-08-31T09:16:32Z

+/**
+ * Claims the read gate for a write that has not been pushed yet, so READs consulting waitForIdle()
+ * park behind a deferred write exactly as they park behind a queued one.
+ *
+ * Without this, deferring a write silently drops the read-after-write ordering that write() gives
+ * every caller for free: the queue is empty for as long as the write waits, so a refetch of the same
+ * data resolves immediately and can come back with pre-write server state.
+ *
+ * push() marks the gate pending again once the write lands - idempotently, so it adopts this claim
+ * rather than opening a second one - and the queue drain resolves it. The handover therefore has no
+ * gap for a READ to slip through.
+ *
+ * No-op while offline, matching push(), which returns before claiming the gate in that state, and
+ * flush(), which resolves it: neither parks READs behind a queue that isn't running.
+ */


does it really need to be that long? seems like over-explaining, two first paragraphs could be easily a one sentence imo

JakubKorytko · 2026-08-31T09:16:57Z

        // resolver deleted the only request without pushing a replacement). Resolve here so READs
        // parked on waitForIdle() don't hang until unrelated queue activity releases them.
-        resolveIsReadyPromise?.();
+        //


Suggested change

//

JakubKorytko · 2026-08-31T09:18:51Z

+        // Unless a deferred write is holding the gate: an empty queue then means its write hasn't been
+        // pushed *yet*, not that nothing is coming, and resolving would let through the very READs that
+        // write is meant to be ordered before. Its own claim resolves the gate if it never reaches the queue.
+        // Only this branch is guarded - the follower branch below must still resolve, since a tab that
+        // never processes the queue would otherwise park READs forever.


why we explain branch below in this comment? The comment itself is also making an assumption that "an empty queue then means its write hasn't been pushed yet, not that nothing is coming" - I guess it's true but when reading I wonder how come we are sure of it

JakubKorytko · 2026-08-31T09:37:04Z

+// `waitForIdle()` hands back a module-level promise, so every test here shares one gate. Each releases what
+// it claims, and this lives apart from SequentialQueueTest so no pushed request has moved the gate first.


this comment sounds like it is above the gate but I believe offlineSpy is not the gate?

JakubKorytko · 2026-08-31T09:41:55Z

@codex review

JakubKorytko · 2026-08-31T09:42:15Z

@MelvinBot review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37bf90d766

ℹ️ 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-08-31T09:48:43Z

Review

The approach is right, but the gate is only opened by the claim — nothing keeps it open. Two paths reopen it while the write is still deferred, so the ordering guarantee doesn't hold in the cases that matter. Details below, then an answer to your design question, JakubKorytko.

1. An unrelated write draining mid-deferral reopens the gate 🔴

src/libs/Network/SequentialQueue.ts:617-622

flush()'s empty-queue branch is guarded, but the drain-completion resolve isn't. Any ordinary API.write() during the deferral adopts the same gate (setIsReadyPromisePending() is idempotent), and when it finishes, process().finally calls resolveIsReadyPromise?.() while deferredWriteGateClaims is still 1. From that point the deferred write has no gate at all.

Trace

writeWhenReady claims → isReadyPromise = P1, resolveIsReadyPromise = R1, claims = 1.
Unrelated API.write(X) → push() line 807 no-ops (already pending, adopts P1) → flush(false) → process().
X completes, hasRemainingRequests === false → R1() fires. P1 resolved, isReadyPromisePending = false, claims still 1.
A READ fires 1s later → waitForIdle() hands back the resolved P1 → runs immediately, ahead of the deferred write. That is [Due for payment 2026-09-15] Spend - Expenses are not shown in the table after splitting on group-by:from view #99805 again; it only needs one other write in the ~2-5s window.

Fix: apply the same deferredWriteGateClaims === 0 guard to the drain resolve. Or stop piggybacking on isReadyPromise entirely and have waitForIdle() return Promise.all([isReadyPromise, deferredClaimsPromise]) — that removes the adopt/hand-off subtlety in one go and there's then no third site to forget.

2. `release()` opens the gate while other claims are outstanding 🔴

src/libs/Network/SequentialQueue.ts:130-140

Two concurrent writeWhenReady calls share one gate. If the first goes offline and releases, settleOnce() drops the count to 1 — but release() never consults the count and resolves anyway, so the second write is still deferred with no gate. The counter is only read by flush().

 release: () => {
     if (!settleOnce()) {
         return;
     }
+    // Somebody else's deferred write is still holding this gate.
+    if (deferredWriteGateClaims > 0) {
+        return;
+    }
     if (isReadyPromise !== claimedPromise) {

The test adopts an existing claim rather than opening a second gate asserts the current behavior as correct (first.release() → READ proceeds while second is outstanding), so it needs to flip with the fix.

3. A rejected `write()` strands the gate 🟡

src/libs/API/writeWhenReady.ts:198-222

The catch only sees a synchronous throw, and write() is async — a rejection out of push() takes the handOff() branch instead, so nothing resolves the gate the claim opened. It self-heals on the next flush() with an empty queue (the count is back to 0 by then), but plain write() has no equivalent case, since it opens the gate only after it's past the throwing code. Settling from the rejection path too would close it.

4. `resetQueue()` can drive the counter negative 🟡

src/libs/Network/SequentialQueue.ts:872

It zeroes the counter while live claims still have hasSettled === false. A later release() takes it to -1, and deferredWriteGateClaims === 0 in flush() is never true again for the rest of that test file — READs silently stop being released. Test-only, but a quiet trap for the next person. Math.max(0, …) in settleOnce, or a generation token on the claim.

On the design question

I'd make it opt-in rather than safe-by-default. SAFETY_TIMEOUT_MS is 5 * MAX_TRANSITION_DURATION_MS = 5s, and the claim parks every API.read plus read-type makeRequestWithSideEffects app-wide — not just reads touching the write's data. A barrier that never settles means five seconds with no reads anywhere in the app, and the caller who deferred the write gets no signal that they did that. The "a barrier must not await a READ of its own or it deadlocks" footgun you documented is the same over-broad coupling surfacing. An option ({claimReadGate: true}), or a cap on the claim much shorter than the safety timeout, keeps the fix available for the callers that need it without making every future writeWhenReady a global read barrier.

Otherwise

The stale-promise guard in release(), the idempotent adopt in push(), and the offline no-op mirroring push()/flush() all look right, and the tests read well — the flush()-mid-deferral one in particular is the kind of guard that would have been easy to skip.

How I reviewed

Read-only review of 37bf90d against f0092b3. Findings come from reading the queue's gate lifecycle end to end (setIsReadyPromisePending → push → flush → process().finally → waitForIdle); I did not execute the suite locally. There's no user-facing flow to exercise, since nothing on this branch calls writeWhenReady in production. CI is green on test (jobs 1-8), typecheck, ESLint, spellcheck; the only red checks are checklist and Check independent approval.

view run

JakubKorytko · 2026-08-31T09:52:20Z

I'd make it opt-in rather than safe-by-default

actually, that is something that I thought at first since this is the first flow that has such problems (at least reported, though submit-expense used it for a long time), but assumed safe-by-default is better. Perhaps we indeed should go with opt-in

ikevin127 · 2026-08-31T21:44:24Z

Awaiting for Jakub's review comments to be addressed before moving forward with C+ review ⏳

Piggybacking on isReadyPromise meant every queue path that resolves it could reopen the gate mid-deferral, so each one needed its own guard. waitForIdle() now waits out the deferred claims first and reads isReadyPromise after, which closes the drain-completion and multi-claim holes without guarding flush() at all. The network is checked while waiting rather than once at claim time, so going offline releases parked READs and coming back re-parks them. Add {claimReadGate: false} to opt out.

Abdukhamid000 · 2026-08-31T23:23:25Z

Restructured rather than patched: deferred writes hold their own gate now, and waitForIdle() waits it out before reading isReadyPromise. That drops the drain-completion hole, the last-claim check and the stale-promise guard in one go — flush() is untouched again.

Offline is checked while waiting instead of once at claim time, so both directions work: going offline releases parked READs, coming back re-parks them.

You were right on both tests. The adopt one asserted the early release as correct, and the stale one never reached the guard it was named after. Both replaced.

Went with default-on + {claimReadGate: false}, following your inline comment rather than the later message — say the word if you'd rather have opt-in.

EoC halved, [No QA] added, tests/offline/QA are N/A.

Also went back over the comments themselves and cut them down - the long docblocks are gone.

Abdukhamid000 · 2026-08-31T23:31:16Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 942db7103e

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

JakubKorytko

Reads a lot better, a few small ones inline, nothing blocking. @Abdukhamid000 please like or respond briefly to every previous review comment if you have addressed it. I assumed they were all addressed. Also, you don't need to use AI to create messages in response to my review 😅 A simple "Done, re-review please" + a like on review comments or an answer to any comment you haven't addressed, explaining why, is enough. I don't expect perfect English or super long explanations, just for the change to be safe

JakubKorytko · 2026-09-01T08:08:38Z

+            } finally {
+                // `push()` ran synchronously inside `write()`: the queue holds the gate now, or we were
+                // offline and nothing was queued.
+                settleReadGateClaim();


I think this only releases the gate at the right time because write() happens to call push() synchronously before its first await. If write()/processRequest() ever grows an early await before reaching push() (a feature flag check, a conflict lookup, anything), this finally block runs first and opens the gate before the write is actually on the queue. A READ could then slip in ahead of a write that looks queued but isn't yet. Nothing here enforces that ordering, it is just implied by the comment above.

JakubKorytko · 2026-09-01T08:08:38Z

+        // Wake on a network change too, so going offline releases READs and coming back re-parks them.
+        let unsubscribeFromNetworkState = () => {};
+        const networkStateChanged = new Promise<void>((resolve) => {
+            unsubscribeFromNetworkState = subscribeToNetworkState(resolve);


waitForIdle() runs for every API.read/paginate call, so while a deferred write is pending each read that fires gets its own promise plus its own network-state subscriber here. If several reads happen during one deferral window that's several subscribers all doing the same wait, not one shared wait. Could this be a single module-level promise/subscription that all waiters share instead of one per caller?

JakubKorytko · 2026-09-01T08:08:38Z

    onWriteStarted?: () => void;
+
+    /** Whether READs wait for this write. Defaults to `true`. See the read-gate caveat on `writeWhenReady`. */
+    claimReadGate?: boolean;


Suggested change

claimReadGate?: boolean;

shouldClaimReadGate?: boolean;

so it reads as a flag at the call site

JakubKorytko · 2026-09-01T08:08:38Z

+ * Sole writer of the count, so deferredWritesLanded is pending exactly while claims are outstanding -
+ * resolving it early would spin waitForIdle(). Floored: resetQueue() can zero the count mid-claim.
+ */
+function setDeferredWriteClaims(count: number) {


Every other state change in this file (pause, unpause, flush, push) logs through Log.info so you can trace what happened from device logs. The new claim counter and the waitForIdle wait loop around it log nothing, so a stuck READ from a lost claim would be invisible in the logs

Abdukhamid000 · 2026-09-02T04:31:30Z

@ikevin127 Done, all 5 addressed — re-review please.

Two notes where I did a bit more than asked:

On the mis-named throw test I did both: renamed it to rejects, and added the settleClaim assertion to rejects the returned promise when the write throws synchronously so execute()'s catch actually has coverage.
resetQueue() now unsubscribes the listener rather than just dropping the promise.

ikevin127

🟢 LGTM

cc @JakubKorytko if you wanna sign-off before merge

ikevin127 · 2026-09-02T04:50:53Z

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 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
- 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
- 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 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.

ikevin127

🟢 LGTM

cc @JakubKorytko if you wanna sign-off before merge

JakubKorytko

LGTM, one small NAB: the description still says {claimReadGate: false} and the option is shouldClaimReadGate now

Abdukhamid000 · 2026-09-02T09:22:30Z

Good catch, description updated to {shouldClaimReadGate: false}.

rlinoz · 2026-09-02T14:23:50Z

Please merge main, the failing tests are fixed I think.

…read-gate

Abdukhamid000 · 2026-09-03T01:19:20Z

@rlinoz done

rlinoz · 2026-09-03T14:51:25Z

+    // Drop the listener too, not just the promise, so a suite that parks a READ doesn't leave a subscriber
+    // behind on every reset.


Suggested change

// Drop the listener too, not just the promise, so a suite that parks a READ doesn't leave a subscriber

// behind on every reset.

rlinoz · 2026-09-03T15:20:48Z

    /** Fires only after `write()` has been called and returned without throwing. A throwing handler is logged, not thrown. */
    onWriteStarted?: () => void;
+
+    /** Whether READs wait for this write. Defaults to `true`. See the read-gate caveat on `writeWhenReady`. */


Suggested change

/** Whether READs wait for this write. Defaults to `true`. See the read-gate caveat on `writeWhenReady`. */

/** Whether READs wait for this write. Defaults to `true`.*/

rlinoz · 2026-09-03T15:21:35Z

+                // reached synchronously inside write(). Holding the gate a moment longer is safe. Opening
+                // it early is not.


Suggested change

// reached synchronously inside write(). Holding the gate a moment longer is safe. Opening

// it early is not.

// reached synchronously inside write().

rlinoz · 2026-09-03T15:24:54Z

+ *
+ * Not only READs wait here: the auth-token swaps in `Delegate.connect`/`disconnect`, the account merge in
+ * `Session`, and the post-sign-in `openApp` all gate on this too, so a deferred write delays them by up to
+ * `SAFETY_TIMEOUT_MS` as well. That wait is intended - swapping the token out from under a pending write is
+ * exactly what it prevents - but the `shouldClaimReadGate` opt-out lives on the writer, so a caller adding a


Suggested change

*

* Not only READs wait here: the auth-token swaps in `Delegate.connect`/`disconnect`, the account merge in

* `Session`, and the post-sign-in `openApp` all gate on this too, so a deferred write delays them by up to

* `SAFETY_TIMEOUT_MS` as well. That wait is intended - swapping the token out from under a pending write is

* exactly what it prevents - but the `shouldClaimReadGate` opt-out lives on the writer, so a caller adding a

rlinoz · 2026-09-03T15:27:25Z

+async function waitForIdle(): Promise<unknown> {
+    while (deferredWriteClaims > 0 && !isOfflineNetwork()) {
+        Log.info('[SequentialQueue] READ is waiting on a deferred write', false, {claims: deferredWriteClaims});
+        // The waits are deliberately sequential, not parallel. Each one re-checks the claim count and the


Suggested change

// The waits are deliberately sequential, not parallel. Each one re-checks the claim count and the

// The waits are deliberately sequential, each one re-checks the claim count and the

rlinoz · 2026-09-03T15:27:43Z

+        await Promise.race([deferredWritesLanded, whenNetworkStateChanges()]);
+    }
+
+    // Read after the wait, not before: the deferred write's push() has re-closed the gate by now.


Suggested change

// Read after the wait, not before: the deferred write's push() has re-closed the gate by now.

// Read after the wait, the deferred write's push() has re-closed the gate by now.

Abdukhamid000 · 2026-09-03T16:04:41Z

Done, all six applied in dc4caa6. @rlinoz ready for re-review.

rlinoz · 2026-09-03T16:18:54Z

hmm are the lint errors related?

…read-gate

github-actions · 2026-09-04T10:55:34Z

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

OSBotify · 2026-09-04T10:57:55Z

✋ 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-04T10:59:36Z

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

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-eef921d-1c13481-ede698d26d774d5b43680dac00667730b94e8279/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-eef921d-1c13481-ca9834eb2028ad74772a17a64f9edbcb8cea0f0f/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-07T11:03:10Z

🚀 Deployed to staging by https://github.com/rlinoz in version: 9.4.73-0 🚀

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

MelvinBot · 2026-09-07T11:05:44Z

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

This change is internal request-queue plumbing. It adds a read gate that deferred writes hold so a READ can't overtake a writeWhenReady() write that hasn't been pushed yet. All four changed files are library code and unit tests:

src/libs/API/writeWhenReady.ts — new shouldClaimReadGate option (developer API, defaults to true)
src/libs/Network/SequentialQueue.ts — claimReadGateForDeferredWrite() and the waitForIdle() wait
tests/unit/APIWriteWhenReadyTest.ts and tests/unit/SequentialQueueReadGateTest.ts — new unit tests

Why this doesn't reach the help site

App/docs/articles documents customer-facing product behavior — workspace settings, features, tab and button names, billing, and integrations. This PR changes none of those:

No UI, screens, labels, buttons, or copy changed.
No feature was added, renamed, or removed, so nothing in the help site's naming conventions applies.
The only new surface is a TypeScript option on an internal function. Developer APIs are documented in code comments and contributingGuides, not on the help site.
The offline behavior change (the gate is ignored while offline) is invisible to users — it preserves existing behavior rather than altering what someone sees.
Nothing in App/docs references the sequential queue, writeWhenReady, or read gates, so there is no existing article to correct.

The PR is also labeled [No QA], and the author notes nothing calls writeWhenReady in production on this branch — there is no user-visible flow to describe.

@Abdukhamid000 — there is no help site PR to link, so there is nothing to mark Ready for review. If you disagree and think a specific article needs updating, tell me which one and I'll draft the change.

view run · view recording

OSBotify · 2026-09-08T19:02:41Z

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.73-3 🚀

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

Bundle Size Analysis (Sentry):

Android

Abdukhamid000 requested review from a team as code owners August 29, 2026 06:23

melvin-bot Bot assigned Abdukhamid000 Aug 29, 2026

melvin-bot Bot requested review from heyjennahay and ikevin127 and removed request for a team August 29, 2026 06:23

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

Comment thread src/libs/Network/SequentialQueue.ts Outdated

github-actions Bot reviewed Aug 29, 2026

View reviewed changes

Keep DeferredWriteReadGateClaim module-local

37bf90d

Nothing imports the type - consumers use the inferred return type of claimReadGateForDeferredWrite - and exporting it added a knip finding.

JakubKorytko suggested changes Aug 31, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread src/libs/Network/SequentialQueue.ts Outdated

Abdukhamid000 changed the title ~~Claim the sequential queue read gate for deferred writes~~ [No QA] Claim the sequential queue read gate for deferred writes Aug 31, 2026

Pin the drain-completion path with its own regression test

1538b9f

chatgpt-codex-connector Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread src/libs/Network/SequentialQueue.ts

Trim the read-gate comments

ed3d9a0

Abdukhamid000 requested a review from JakubKorytko August 31, 2026 23:46

JakubKorytko approved these changes Sep 1, 2026

View reviewed changes

ikevin127 approved these changes Sep 2, 2026

View reviewed changes

melvin-bot Bot requested a review from rlinoz September 2, 2026 04:38

ikevin127 approved these changes Sep 2, 2026

View reviewed changes

JakubKorytko approved these changes Sep 2, 2026

View reviewed changes

Abdukhamid000 added 2 commits September 2, 2026 21:51

Merge remote-tracking branch 'upstream/main' into fix/writeWhenReady-…

d6a25cb

…read-gate

Merge remote-tracking branch 'upstream/main' into fix/writeWhenReady-…

c340f36

…read-gate

rlinoz requested changes Sep 3, 2026

View reviewed changes

Trim read-gate comments per review

dc4caa6

Merge remote-tracking branch 'upstream/main' into fix/writeWhenReady-…

da37107

…read-gate

Abdukhamid000 requested a review from rlinoz September 4, 2026 03:54

rlinoz approved these changes Sep 4, 2026

View reviewed changes

rlinoz merged commit eef921d into Expensify:main Sep 4, 2026
34 of 35 checks passed

OSBotify mentioned this pull request Sep 7, 2026

Deploy Checklist: New Expensify 2026-09-07 #100479

Closed

JakubKorytko mentioned this pull request Sep 8, 2026

[Performance] Move Search and submit-expense onto write-readiness barriers and delete deferredLayoutWrite #100169

Open

38 tasks

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

[Due for payment 2026-09-15] Spend - Expenses are not shown in the table after splitting on group-by:from view #99805

Closed

7 tasks

ikevin127 mentioned this pull request Sep 10, 2026

[$250] [INP] Improve SplitExpense-SaveButton interaction #97802

Open

Abdukhamid000 mentioned this pull request Sep 11, 2026

perf: defer the split-save API.write behind the destination layout (re-land #98012) #100976

Open

39 tasks

		// `waitForIdle()` hands back a module-level promise, so every test here shares one gate. Each releases what
		// it claims, and this lives apart from SequentialQueueTest so no pushed request has moved the gate first.

		// Drop the listener too, not just the promise, so a suite that parks a READ doesn't leave a subscriber
		// behind on every reset.

	/** Whether READs wait for this write. Defaults to `true`. See the read-gate caveat on `writeWhenReady`. */
	/** Whether READs wait for this write. Defaults to `true`.*/

		// reached synchronously inside write(). Holding the gate a moment longer is safe. Opening
		// it early is not.

	// The waits are deliberately sequential, not parallel. Each one re-checks the claim count and the
	// The waits are deliberately sequential, each one re-checks the claim count and the

	// Read after the wait, not before: the deferred write's push() has re-closed the gate by now.
	// Read after the wait, the deferred write's push() has re-closed the gate by now.

Conversation

Abdukhamid000 commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline steps

QA steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Aug 29, 2026

Uh oh!

codecov Bot commented Aug 29, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot Aug 29, 2026

Choose a reason for hiding this comment

❌ CONSISTENCY-16 (docs)

Uh oh!

github-actions Bot Aug 29, 2026

Choose a reason for hiding this comment

❌ CONSISTENCY-16 (docs)

Uh oh!

JakubKorytko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JakubKorytko commented Aug 31, 2026

Uh oh!

JakubKorytko commented Aug 31, 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 Aug 31, 2026

Review

1. An unrelated write draining mid-deferral reopens the gate 🔴

2. release() opens the gate while other claims are outstanding 🔴

3. A rejected write() strands the gate 🟡

4. resetQueue() can drive the counter negative 🟡

On the design question

Otherwise

Uh oh!

JakubKorytko commented Aug 31, 2026

Uh oh!

ikevin127 commented Aug 31, 2026

Uh oh!

Abdukhamid000 commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Abdukhamid000 commented Aug 31, 2026

Uh oh!

Abdukhamid000 commented Aug 29, 2026 •

edited

Loading

codecov Bot commented Aug 29, 2026 •

edited by github-actions Bot

Loading

2. `release()` opens the gate while other claims are outstanding 🔴

3. A rejected `write()` strands the gate 🟡

4. `resetQueue()` can drive the counter negative 🟡

Abdukhamid000 commented Aug 31, 2026 •

edited

Loading

Abdukhamid000 commented Sep 2, 2026 •

edited

Loading