[No QA] Make the monthly automatic-submit eta deterministic on the offset day by MelvinBot · Pull Request #100145 · Expensify/App

MelvinBot · 2026-09-02T10:48:33Z

Explanation of Change

NextStepUtilsTest › scheduled submit enabled › monthly on the 2nd fails on main on the 2nd of every month, blocking CI for every open PR that day. It failed on every PR run on 2026-09-02.

The test derived its expected value from a different helper than production uses, and the two disagreed about whether "the next Nth of the month" includes today:

Production builds setDate(new Date(), autoReportingOffset) and rolls forward a month only when isPast(etaDateTime). On the 2nd, setDate(now, 2) is today at the current time of day, so it returns today.
The test expectation used DateUtils.getNextNthOfMonth(2), whose day < nth check excludes today, so it returned next month.

They only diverge when today === nth, which is why this surfaced exactly one day per month. On that day it was a near-deterministic failure rather than a coin flip: measured over 200,000 iterations on the 2nd, production returned today 99.96% of the time, so the old assertion failed ~99.97% of runs.

The expectation no longer comes from a live new Date(). The case is now three tests with the clock pinned via jest.setSystemTime, covering each position relative to the offset day — before it, on it, and after it. The on-the-day case is the one that used to only run once a month; it is now exercised on every CI run.

DateUtils.getNextNthOfMonth is removed. That test was its only caller anywhere in the repo, and its off-by-one at the boundary is what encoded the disagreement in the first place — leaving it in place leaves the same trap for the next caller.

This PR is test-only — src/libs/NextStepUtils.ts is unchanged from main. An earlier revision also changed isPast(etaDateTime) to isPast(endOfDay(etaDateTime)). That was dropped after review: it is not load-bearing for the flake (all 48 tests pass with NextStepUtils.ts byte-identical to main), and the pinned-clock tests cannot distinguish the two implementations, since under a frozen clock etaDateTime === now exactly and both isPast(now) and isPast(endOfDay(now)) are false.

For the record, there is a real sub-millisecond race on the offset day — setDate preserves time-of-day, so isPast only returns true if the millisecond ticks over between new Date() and Date.now() (measured: 0.038% of the time). It is deliberately left alone here because it has no user-visible effect: the monthly eta renders through CONST.DATE.ORDINAL_DAY_OF_MONTH ('do') as "on the 2nd of each month", so the month and year of eta.dateTime are never displayed, and the offset picker caps at 28 so addMonths never clamps the ordinal. Worth a separate PR if we want it tightened.

Fixed Issues

$ #100134
PROPOSAL:

Tests

// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validate your change worked.

Verify that no errors appear in the JS console

Offline tests

No production code changes, so offline behaviour is unchanged.

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."
// The human co-author must fill out the QA steps before marking this PR as "ready for review".

Verify that no errors appear in the JS console

PR Author Checklist

Automated verification (MelvinBot)

Local checks run against this branch, all passing:

Check	Result
`npm test -- tests/unit/NextStepUtilsTest.ts`	✅ 48 passed — run on 2026-09-02, the day the old test failed, with `NextStepUtils.ts` identical to `main`
`npm test -- tests/unit/DateUtilsTest.ts`	✅ 93 passed (covers the removed helper's file)
`npm run typecheck`	✅ passed
`npx eslint` on changed files	✅ 0 errors (3 pre-existing grandfathered warnings in `DateUtils.ts`, untouched lines)

The first row is the load-bearing one: the previous test failed on this exact calendar day, and the replacement passes on it while still asserting the on-the-day behaviour explicitly.

Screenshots/Videos

No UI changes — this PR only touches tests and removes an unused helper.

Co-authored-by: Alberto Gonzalez-Cela <Gonals@users.noreply.github.com>

MelvinBot · 2026-09-02T11:03:55Z

🤖 Android test steps verification results: ✅ pass

Verified on standalone Android NewDot: Monthly scheduled submit can be configured with a specific day; the open expense report's next-step banner correctly reflects the configured day (20th and then 2nd/today) with no errors or crashes. One caveat worth your attention — the WAITING_FOR_AUTOMATIC_SUBMIT copy renders only the ordinal day ("2nd"), never a month or year, so the UI cannot distinguish "today's 2nd" from "next month's 2nd"; that difference lives in the nextStep.eta.dateTime value this PR changes, and the unit tests are what actually pin it.

Step	Status
VERIFY 1: Workspace Workflows settings allow enabling delay submissions / scheduled submit with Monthly frequency and a specific day of month In a newly created workspace's Workflows page, toggled on "Choose a custom schedule for submitting expenses", opened Frequency, selected "Monthly", then set "Date of month" to "20th" via the offset picker; Workflows page then showed "Frequency: Monthly".	✅
VERIFY 2: With Monthly offset set to the 20th (later in month), an open expense report shows next-step text with the 20th of the current month Created a draft/open expense report (1 expense, category assigned) in the workspace. The report's next-step banner read exactly: "Waiting for your expenses to automatically submit on the 20th of each month." Since today is the 2nd, the 20th falls later in the current month with no rollover, matching expectation.	✅
VERIFY 3: With Monthly offset set to today's day (2nd), the next-step message resolves to today, not next month Changed the workspace's Monthly offset to the 2nd (today, 2026-09-02). After a short refresh delay, the same report's next-step banner updated to: "Waiting for your expenses to automatically submit on the 2nd of each month." — correctly reflecting today's day rather than erroring or going blank. Note: this message template only ever renders the ordinal day-of-month, never a full date, so the rendered string is identical whether the underlying eta resolves to today or to next month's 2nd. The distinction this PR fixes lives in that underlying date value, not in the rendered copy — so this is the extent to which the fix is observable from the UI.	✅
VERIFY 4: No JS runtime errors, red-box errors, or crashes during the flow Throughout workspace creation, Workflows configuration (toggle, frequency, offset day changes), expense/report creation, category assignment, and report navigation, the app remained responsive with normal snapshots at every step; no red-box screens, error dialogs, or unexpected crashes/restarts occurred.	✅

The PR's Tests section is still the unfilled template (only "Verify that no errors appear in the JS console"), so the steps above were derived from the behavior change in src/libs/NextStepUtils.ts. Worth filling in real steps before review.

view run · view recording

MelvinBot · 2026-09-02T11:21:17Z

🤖 Web test steps verification results: ✅ pass

Verified on dev NewDot web: Monthly scheduled submit with a specific day-of-month can be configured in Workflows settings, and the open report's next-step message correctly reflects the configured day for both a future day (20th) and today's day (2nd, 2026-09-02) with no errors. The PR's specific fix — ensuring the offset day resolves to today rather than racing the clock — is not visually distinguishable in the rendered message text, since it only ever shows the ordinal day and never the month or year; that was instead confirmed by reading the code diff and its deterministic unit tests.

Step	Status
VERIFY 1: Workspace Workflows settings allow enabling Delay Submissions / scheduled submit with Monthly frequency and a specific day of the month Created a new workspace, opened Workspaces > Workflows > Submissions. Toggled the custom-schedule switch on, opened Frequency, selected "Monthly", then set "Date of month" to "20th" via the picker, and saved. The Workflows page then showed "Frequency: Monthly" persisted.	✅
VERIFY 2: With Monthly scheduled submit set to a later day (20th), the open expense report's next-step message shows the 20th of the current month Created an expense (categorized it to clear the "fix issues" block) with Monthly/20th configured. The report's next-step banner read: "Waiting for your expenses to automatically submit on the 20th of each month." Since 2026-09-02 is before the 20th, this correctly resolves within the current month (September).	✅
VERIFY 3: With Monthly scheduled submit set to today's day (2nd), the next-step message reflects today's date rather than skipping to next month Changed Workflows > Submissions > Frequency > Date of month to "2nd" (today, per the 2026-09-02 clock) and saved; Workflows page showed "Frequency: Monthly" persisted. The report's next-step banner then read: "Waiting for your expenses to automatically submit on the 2nd of each month" with no errors, confirming the offset-day flow completes without incident. Note: the rendered message format (ordinal only, no month/year) is identical whether the internal eta.dateTime resolves to Sept 2 or Oct 2, so the specific bug this PR fixes is not visually provable from the UI text alone. Confirmed instead by reading the diff: it now compares isPast(endOfDay(etaDateTime)) instead of isPast(etaDateTime), which deterministically keeps today's date valid until midnight, and is covered by the new pinned-clock unit tests.	✅
VERIFY 4: No JS console errors during the flow The web harness has no direct console-log capture command, so as the closest available signal a network dump of the session's HTTP traffic during the entire flow (workspace creation, Workflows changes including SetWorkspaceAutoReportingOffset, expense creation, categorization, and report navigation) showed all requests returning status=200 with no failed calls, and no error banners or toasts appeared in any screenshot. Treat this as indirect evidence rather than a true console check.	✅

The PR's Tests section is still the unfilled template (only "Verify that no errors appear in the JS console"), so the steps above were derived from the behavior change in src/libs/NextStepUtils.ts:210-221. Worth filling in real steps before review.

view run · no recording available

melvin-bot · 2026-09-02T11:56:53Z

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

Gonals · 2026-09-02T11:57:54Z

@MelvinBot, shouldn't this just fix the flaky test?

Reviewer feedback: this should just fix the flaky test. Verified that the endOfDay change was not load-bearing for the flake -- all 48 tests in NextStepUtilsTest pass with NextStepUtils.ts byte-identical to main. The three pinned-clock tests cannot distinguish the two implementations: under a frozen clock setDate preserves time-of-day, so etaDateTime equals now exactly, and both isPast(now) and isPast(endOfDay(now)) are false. NextStepUtils.ts is now unchanged from main. Co-authored-by: Alberto Gonzalez-Cela <Gonals@users.noreply.github.com>

MelvinBot · 2026-09-02T12:13:11Z

You're right. I've dropped the production change — src/libs/NextStepUtils.ts is now byte-identical to main, and this PR is test-only (pushed in dba210d).

I checked whether the endOfDay change was load-bearing before removing it, and it wasn't, on two counts:

It doesn't fix the flake. All 48 tests in NextStepUtilsTest pass with NextStepUtils.ts reverted to main. What actually fixes the flake is pinning the clock and hardcoding the expectations instead of deriving them from getNextNthOfMonth.

The new tests couldn't have caught it anyway. Under jest.setSystemTime, setDate preserves time-of-day, so etaDateTime === now exactly — making isPast(now) and isPast(endOfDay(now)) both false. Sweeping all 24 hours of the offset day, the two implementations differ in 0 of 24. So I was shipping an untested production change alongside the fix, which is the worst version of this.

On the race itself — it's real, but immaterial

My original description called it a "sub-millisecond coin flip." That was wrong in two ways, and I've corrected the PR body.

It isn't a coin flip. isPast only returns true if the millisecond ticks over between new Date() and Date.now(). Measured over 200k real-clock iterations on the 2nd: 76 true / 199,924 false (0.038%). So production returns today ~99.96% of the time. That also means the old test wasn't intermittently flaky on the 2nd — it failed ~99.97% of runs that day. Near-deterministic, once a month.

More importantly, even when it flips, nothing user-visible changes. The monthly eta renders through CONST.DATE.ORDINAL_DAY_OF_MONTH ('do') at src/libs/NextStepUtils.ts:83-89, producing "on the 2nd of each month" — the month and year of eta.dateTime are never displayed, in any of the 12 locales. And WorkspaceAutoReportingMonthlyOffsetPage.tsx:30 caps the picker at 28, so addMonths never clamps the ordinal either. The only consumers of eta.dateTime are that formatter, a debug-mode type check, and the tests.

Happy to tighten the race in a separate PR if you want it, but it's cosmetic at best.

I kept the DateUtils.getNextNthOfMonth removal — the test was its only caller anywhere in the repo, so it's dead code now, and its day < nth boundary is what encoded the disagreement. Say the word if you'd rather I restore it and keep this to just the test file.

view run

MelvinBot · 2026-09-02T12:20:10Z

There's nothing to fix here — no check is failing because of the code.

Every automated check on the current head passes, including all 8 Jest shards, ESLint, typecheck, Oxfmt, knip, spellcheck, Bun tests, Storybook, React Compiler, both native builds, and all three Reassure perf jobs.

The single red check is Check independent approval, and it fails for a review-process reason, not a code reason:

Insufficient independent peer review
  independentApprovers: []
##[error]Expensify/App#100145 does not have enough independent Expensify employee approvals.

It needs an approving review from an Expensify employee who isn't the author or an assignee. No commit can satisfy that, so I haven't pushed anything.

One thing that does still need a human: the Tests and QA Steps sections of the PR body have // TODO placeholders that I'm not allowed to fill in on your behalf. Those will need your input before the PR goes ready for review.

view run

Gonals · 2026-09-02T12:29:05Z

@MelvinBot, you are still removing getNextNthOfMonth. Why is that? Is is not used anywhere else?

Gonals · 2026-09-02T12:29:50Z

Nevermind, already fixed in a different PR

MelvinBot · 2026-09-02T12:33:07Z

Correct — it isn't used anywhere else. On main there are exactly three occurrences in the whole repo, and one of them is the test line this PR rewrites:

Occurrence	Location
Definition	`src/libs/DateUtils.ts:1284`
Export entry	`src/libs/DateUtils.ts:1382`
Only caller	`tests/unit/NextStepUtilsTest.ts:290`

Once that test stops deriving its expectation from the helper, the export has zero callers. That's the whole reason for the removal — it's a helper that only ever existed to serve one test assertion, and it encoded the off-by-one (day < nth excludes today) that disagreed with production and caused the monthly failure.

That said, keeping it costs nothing if you'd rather not widen the diff. It's a judgement call, not a requirement — say the word and I'll restore it.

How I verified

git grep across the full origin/main tree (not just src/) for getNextNthOfMonth, plus a search for case variants and for dynamic access via DateUtils[...], which returned nothing. So there's no string-keyed or computed call site that a plain identifier search would miss.

Scope caveat: this covers the Expensify/App repo only. src/libs/DateUtils.ts is App-internal and isn't published as a package, so I don't believe there's an external consumer — but I didn't search other repos.

One thing worth a second look before you merge

The latest commit (dba210d) drops the production endOfDay change and keeps only the test fix. That makes the new is today when today is the 2nd test pass because the clock is frozen, not because the underlying race is gone.

isPast is literally +toDate(date) < Date.now(), and production still reads the clock twice:

let etaDateTime = setDate(new Date(), policy?.autoReportingOffset); // reads clock at T0
if (isPast(etaDateTime)) {                                          // reads clock again at T1

On the offset day setDate is a no-op, so this compares T0 < T1. Under jest.setSystemTime the clock is frozen, so T0 === T1 and it's deterministically false → today. Under a real clock the two reads are microseconds apart, so it's false whenever they land in the same millisecond and true when they straddle a boundary — in which case a user on the 2nd sees next month's 2nd.

So the test is now deterministic, but it pins the frozen-clock outcome rather than proving production is race-free. The endOfDay change is what actually removed the double clock read. Two honest options:

Restore endOfDay — fixes the real race, at the cost of a production behavior change.
Keep it test-only — perfectly defensible if you want a minimal CI unblock, but the rare wrong-date-on-the-offset-day bug stays, and the test won't catch it.

I don't have a strong opinion on which, and I couldn't execute the suite here to confirm the pass — the above is read from the date-fns source and the fake-timer semantics, not from a run. Your call.

view run · view recording

MelvinBot · 2026-09-02T14:57:20Z

Gonals — this wasn't fixed in a different PR. main is still unfixed, so closing this leaves the flake in place.

Checked against main at bded4c8 (merged 14:37 UTC today):

On `main` today	Status
`src/libs/DateUtils.ts:1284` — `getNextNthOfMonth` definition, still `day < nth`	still there
`src/libs/DateUtils.ts:1382` — export entry	still there
`tests/unit/NextStepUtilsTest.ts:290` — `format(DateUtils.getNextNthOfMonth(2), 'yyyy-MM-dd')`	still there
Any fake timers in that test file	none

No merged PR touches either file, and no other open PR does either — the only open-PR hits for getNextNthOfMonth are unrelated feature PRs where I posted this same "unrelated failure" analysis.

I also confirmed this branch still does the job. Applying just its two files on top of another branch and running the suite today, on the 2nd:

PASS tests/unit/NextStepUtilsTest.ts (27.162 s)
Tests: 48 passed, 48 total

The branch claude-nextStepMonthlyEtaFlakyTest is intact at dba210d, is test-only as you asked, and every check was green except Check independent approval.

Could you reopen this? I don't have permission to reopen it myself. Without it the suite goes green tomorrow on its own and then breaks CI again on 2026-10-02, per #100134.

view run

Make the monthly automatic-submit eta deterministic on the offset day

42e553a

Co-authored-by: Alberto Gonzalez-Cela <Gonals@users.noreply.github.com>

MelvinBot assigned Gonals Sep 2, 2026

melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Sep 2, 2026

MelvinBot requested a review from a team September 2, 2026 10:48

This was referenced Sep 2, 2026

NextStepUtilsTest 'monthly on the 2nd' fails on the 2nd of every month, blocking all PR CI #100134

Closed

Align distance rates toggle switches with the page's right inset #100143

Merged

Gonals changed the title ~~Make the monthly automatic-submit eta deterministic on the offset day~~ [No QA] Make the monthly automatic-submit eta deterministic on the offset day Sep 2, 2026

Gonals marked this pull request as ready for review September 2, 2026 11:56

Gonals requested a review from a team as a code owner September 2, 2026 11:56

melvin-bot Bot requested review from Pujan92 and heyjennahay and removed request for a team September 2, 2026 11:56

melvin-bot Bot removed the request for review from a team September 2, 2026 11:56

Gonals marked this pull request as draft September 2, 2026 11:57

Gonals closed this Sep 2, 2026

This was referenced Sep 2, 2026

Always show the Settings option in the categories More menu #100119

Closed

Filter task search rows by live report status instead of the stale snapshot #100069

Open

Fix bulk Pay/Mark as Paid disappearing for whole-report selections #94075

Open

Navigation Menu

Uh oh!

[No QA] Make the monthly automatic-submit eta deterministic on the offset day - #100145

[No QA] Make the monthly automatic-submit eta deterministic on the offset day#100145
MelvinBot wants to merge 2 commits into
mainfrom
claude-nextStepMonthlyEtaFlakyTest

MelvinBot commented Sep 2, 2026 •

edited

Loading

Uh oh!

MelvinBot commented Sep 2, 2026 •

edited

Loading

Uh oh!

MelvinBot commented Sep 2, 2026 •

edited

Loading

Uh oh!

melvin-bot Bot commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026 •

edited

Loading

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MelvinBot commented Sep 2, 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

Automated verification (MelvinBot)

Screenshots/Videos

Uh oh!

MelvinBot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MelvinBot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melvin-bot Bot commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

Gonals commented Sep 2, 2026

Uh oh!

MelvinBot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

One thing worth a second look before you merge

Uh oh!

MelvinBot commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MelvinBot commented Sep 2, 2026 •

edited

Loading

MelvinBot commented Sep 2, 2026 •

edited

Loading

MelvinBot commented Sep 2, 2026 •

edited

Loading

MelvinBot commented Sep 2, 2026 •

edited

Loading