Roll back optimistic new-chat expense on money-request failure by MelvinBot · Pull Request #96573 · Expensify/App

MelvinBot · 2026-07-21T04:48:02Z

Explanation of Change

When an expense is created for a brand-new user (a chat that doesn't exist yet) and the request fails on the server (e.g. the "Simulate failing network request" toggle), the app optimistically created a chat report, an IOU report, a transaction, the report actions and a transaction thread. Two symptoms were reported:

Infinite loading chat — the new chat's metadata stayed isOptimisticReport: true (only the success path reset it to false), so fetchReport short-circuited, never called OpenReport, and the loading skeleton spun forever.
Failed expense still visible — the IOU report / expense shell survived with its error, and dismissing the error left the chat and IOU report shells orphaned.

This implements option 2 from the proposal: keep the failed expense visible (with its red-brick-road error) until the user dismisses it, and clean everything up on dismiss.

buildOnyxDataForMoneyRequest keeps the standard error-merge failureData (so the failed expense stays visible and retryable), and additionally resets the newly-created chat and IOU report metadata to isOptimisticReport: false — mirroring the success path. This unblocks fetchReport/OpenReport so the chat is no longer stuck on an infinite loading skeleton.
dismissError (in ReportActionItem) now detects a failed brand-new-chat expense (report.errorFields.createChat) and, instead of only clearing errors, navigates the user out of the chat and calls deleteReport, which cascades to the linked IOU report and the transaction thread(s). Nothing orphaned is left behind, matching the issue's Expected Result.
deleteReport now also clears the report's REPORT_METADATA (e.g. isOptimisticReport) so a deleted report leaves nothing behind.

Expenses added to an existing chat or IOU report are untouched: they keep the standard error-merge behavior so they remain retryable and no server-backed data is deleted. Because failureData only runs on a genuine server failure (offline requests are queued, not failed), offline behavior is unchanged.

Unit tests in BuildOnyxDataForMoneyRequestTest cover the new-chat failure path (error-merge kept, metadata reset to non-optimistic, nothing SET to null) and the existing-chat path (standard error-merge, no metadata reset). The RequestMoneyTest RedBrickRoad test continues to assert the failed expense stays visible until dismissed.

Fixed Issues

$ #93542
PROPOSAL: #93542 (comment)

Tests

Go to Inbox → global create → Create expense → enter any amount.
On the confirm page, select a user you have never chatted with before.
Turn on "Simulate failing network request".
Create the expense (report creation fails).
Turn off "Simulate failing network request".
Search for that user / open the chat.
Verify the chat is NOT stuck in an infinite loading state — it loads and shows the failed expense with a red-brick-road error.
Dismiss the error on the failed expense.
Verify the failed expense AND the chat are removed (you are navigated out of the chat and it no longer exists / lingers).
Regression check: create an expense to an existing chat with the network sim on, verify the failed expense still shows a red-brick-road error and can be dismissed/retried (existing behavior unchanged).

Verify that no errors appear in the JS console

Offline tests

Creating an expense while offline queues the request (it does not fail), so the optimistic chat/expense persist as before. The rollback only applies to a genuine server failure once back online.

QA Steps

Same as Tests.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Co-authored-by: daledah <daledah@users.noreply.github.com>

daledah · 2026-07-21T04:59:15Z

@MelvinBot please use option 2 in this proposal

…e failure Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-07-21T05:10:52Z

Fixed the failing test (job 2) check.

What failed: tests/actions/IOU/RequestMoneyTest.ts › correctly implements RedBrickRoad error handling — expect(Object.values(reportActionsForIOUReport ?? {}).length).toBe(2) got 0.

Why: That test exercises exactly the scenario this PR changes — a brand-new chat (report: {reportID: ''}) + a brand-new IOU report. It was asserting the old behavior: after a server failure the optimistic IOU report/actions/transaction survive with red-brick-road errors, and only get removed once the user clears the errors. This PR intentionally makes that case a full rollback (failureData now SETs those entities to null), so the post-failure assertions no longer matched.

Fix: Updated the post-failure assertions in that test to expect the rollback end-state instead of surviving-with-errors — the IOU report actions, transaction-thread actions, and transaction are now expected to be gone immediately after failure. The later assertions (all reports/actions/transactions removed) were already correct and are unchanged. See tests/actions/IOU/RequestMoneyTest.ts.

⚠️ Heads up for review: this is the canonical RBR test, and the change means it now asserts that new-chat expense failures leave nothing behind (no red-brick-road retry/dismiss path) for this scenario — consistent with the PR's approved proposal. Please confirm that's the intended behavior when reviewing.

Verified locally: npm test tests/actions/IOU/RequestMoneyTest.ts (27/27 pass), ./scripts/lint.sh on the file, and typecheck-tsgo all pass.

The other failing check, PR Author Checklist / checklist, is the human checklist gate and isn't something I can resolve with a code change.

MelvinBot · 2026-07-21T05:20:31Z

All the code checks are now passing. The real failure — test (job 2) (RequestMoneyTest › correctly implements RedBrickRoad error handling) — was fixed by my earlier push (commit 721cc53), and test, typecheck, ESLint, Oxfmt, and the rest are green.

The only check still red is PR Author Checklist / checklist. That's the human PR Author Checklist gate — it stays red until the author checks off the checklist items in the PR description, so there's nothing for me to fix in code there.

…n dismiss Instead of rolling back every optimistic entity in failureData, keep the standard error-merge behavior and reset the new chat/IOU report metadata to non-optimistic so the chat is no longer stuck on an infinite loading skeleton. Dismissing the failed expense now fully removes the orphaned chat and IOU report shells (deleteReport cascades to the linked IOU report and threads and now also clears report metadata). Co-authored-by: daledah <daledah@users.noreply.github.com>

…nse' into claude-rollbackFailedNewChatExpense Co-authored-by: daledah <daledah@users.noreply.github.com>

…ys visible) Option 2 keeps the standard error-merge behavior on failure, so the failed IOU action, transaction thread actions and transaction remain with red-brick-road errors until dismissed. Reverts the option-1 rollback expectations added earlier so the test matches the current behavior. Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot · 2026-07-21T05:24:24Z

⚠️ MelvinBot CI Fix Loop Detected

MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR.

Next steps: A human should review the recent commits and resolve the conflicting CI errors manually.

MelvinBot · 2026-07-21T05:25:34Z

🤖 Switched the implementation to option 2 from the proposal and pushed to this PR.

What changed vs. the previous (option 1) approach:

MoneyRequestBuilder.ts — reverted the "SET everything to null" rollback. failureData now keeps the standard error-merge (the failed expense stays visible/retryable) and resets the new chat + IOU report metadata to isOptimisticReport: false, mirroring the success path. This unblocks fetchReport/OpenReport so the chat is no longer stuck on an infinite loading skeleton.
ReportActionItem.tsx — dismissError now detects a failed brand-new-chat expense (report.errorFields.createChat), navigates the user out of the chat, and calls deleteReport, which cascades to the linked IOU report and transaction thread(s). Nothing orphaned is left behind on dismiss.
DeleteReport.ts — also clears REPORT_METADATA when a report is deleted (it previously left isOptimisticReport behind).

Expenses added to an existing chat/IOU report are untouched (standard error-merge, still retryable). The PR description and test steps were updated to match option 2 (chat loads with the failed expense → dismiss removes the expense and the chat).

Tests updated & run locally:

BuildOnyxDataForMoneyRequestTest rewritten for option 2 — asserts error-merge is kept, metadata is reset to non-optimistic, and nothing is SET to null on failure. ✅ 28/28
RequestMoneyTest RedBrickRoad assertions restored to expect the failed expense staying visible until dismissed. ✅
npm run typecheck, lint, and the React Compiler compliance check for the touched TSX all pass. ✅

Note: I couldn't record on-device screenshots/videos in this run — please add the platform recordings before marking the PR ready for review.

daledah · 2026-07-21T15:49:51Z

@MelvinBot please open this PR

melvin-bot · 2026-07-21T16:02:49Z

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

MelvinBot · 2026-07-21T16:02:49Z

🤖 Done — this PR is now marked Ready for review.

codecov · 2026-07-21T16:02:59Z

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines	Coverage Δ
src/libs/actions/IOU/MoneyRequestBuilder.ts	`92.37% <100.00%> (+0.06%)`	⬆️
src/libs/actions/Report/DeleteReport.ts	`95.83% <ø> (ø)`
src/pages/inbox/report/ReportActionItem.tsx	`89.71% <33.33%> (-0.99%)`	⬇️
... and 9 files with indirect coverage changes

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 471b2686ba

ℹ️ About Codex in GitHub

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

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

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

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

chatgpt-codex-connector · 2026-07-21T16:08:09Z

+        // them entirely (see #93542) rather than only clearing the error, so navigate the user out of the now-deleted
+        // chat and delete it — deleteReport cascades to the linked IOU report and the transaction thread(s).
+        if (reportID && report?.errorFields?.createChat) {
+            Navigation.goBack(undefined, {afterTransition: () => deleteReport(reportID, true)});


Delete the failed chat, not the current IOU report

When the user opens the failed expense report and dismisses the IOU action error, reportID is the IOU report ID because the failure data attaches the action errors under REPORT_ACTIONS<iouReportID> and errorFields.createChat to that IOU report. Calling deleteReport(reportID, true) from this context deletes only the IOU report and its transaction thread; deleteReport does not follow chatReportID back to the brand-new DM, so the chat shell/preview remains orphaned. Use the parent chat report ID for this rollback path, or otherwise distinguish whether the current report is the chat or the expense report.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-21T16:08:09Z

+                onyxMethod: Onyx.METHOD.MERGE,
+                key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${chat.report?.reportID}`,
+                value: {
+                    isOptimisticReport: false,


Keep created-row dismiss deleting failed new chats

For a failed brand-new chat, this flips the chat metadata to non-optimistic, but the created/welcome row close handler still calls clearCreateChatError, which deletes the report only when REPORT_METADATA.isOptimisticReport is true; once this failure merge runs, that handler only clears errorFields.createChat. If the user closes the red error on the created row first, the optimistic chat and IOU shell are left behind, and the expense close path will no longer see report.errorFields.createChat to clean them up.

Useful? React with 👍 / 👎.

daledah · 2026-07-22T09:05:52Z

@MelvinBot could you please check all comments above?

Roll back optimistic new-chat expense on money-request failure

37720ee

Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot assigned daledah Jul 21, 2026

MelvinBot requested a review from a team July 21, 2026 04:48

MelvinBot mentioned this pull request Jul 21, 2026

[$250] [Exploratory] Expense- Infinite loading and failed report appear after toggle off network fail #93542

Open

7 tasks

Update RedBrickRoad test to expect rollback for brand-new-chat expens…

721cc53

…e failure Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot and others added 3 commits July 21, 2026 05:21

Merge remote-tracking branch 'origin/claude-rollbackFailedNewChatExpe…

9ad4345

…nse' into claude-rollbackFailedNewChatExpense Co-authored-by: daledah <daledah@users.noreply.github.com>

MelvinBot marked this pull request as ready for review July 21, 2026 16:02

MelvinBot requested a review from a team as a code owner July 21, 2026 16:02

melvin-bot Bot requested review from daledah and heyjennahay and removed request for a team July 21, 2026 16:02

melvin-bot Bot removed the request for review from a team July 21, 2026 16:02

chatgpt-codex-connector Bot reviewed Jul 21, 2026

View reviewed changes

Uh oh!

Conversation

MelvinBot commented Jul 21, 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!

daledah commented Jul 21, 2026

Uh oh!

MelvinBot commented Jul 21, 2026

Uh oh!

MelvinBot commented Jul 21, 2026

Uh oh!

MelvinBot commented Jul 21, 2026

Uh oh!

MelvinBot commented Jul 21, 2026

Uh oh!

daledah commented Jul 21, 2026

Uh oh!

melvin-bot Bot commented Jul 21, 2026

Uh oh!

MelvinBot commented Jul 21, 2026

Uh oh!

codecov Bot commented Jul 21, 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!

chatgpt-codex-connector Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

daledah commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MelvinBot commented Jul 21, 2026 •

edited

Loading

codecov Bot commented Jul 21, 2026 •

edited by github-actions Bot

Loading