Fix - Company cards - Broken card feed connection RHP spins forever with no error when the feed has a feed-level error by jmusial · Pull Request #100687 · Expensify/App

jmusial · 2026-09-09T08:38:12Z

Explanation of Change

Route the "log into your bank" CTA through the existing card feed refresh flow.
Stop BankConnection from writing an assign-card step that the broken/refresh connection pages can't render, so the reconnect no longer spins forever
The broken-connection page also now falls back to NotFoundPage for any unrenderable state.

Fixed Issues

PROPOSAL:

Tests

For dev tests - how to get local feed into desired state

const DOMAIN_ID = 'your_domain_id';
const FEED      = 'your_feed_id;
const POLICY_ID = 'your_policy_id;
const KEY       = `sharedNVP_private_domain_member_${DOMAIN_ID}`;
const feedPatch = (patch) => Onyx.merge(KEY, {settings: {oAuthAccountDetails: {[FEED]: patch}}});

globalThis.expire      = () => feedPatch({expiration: Math.floor(Date.now() / 1000) - 60});
globalThis.reconnect   = () => feedPatch({expiration: Math.floor(Date.now() / 1000) + 60 * 60 * 24 * 30});
globalThis.seedError   = () => feedPatch({errors: {[String(Date.now() * 1000)]: 'Could not generate Plaid accessToken'}});
globalThis.clearErrors = () => feedPatch({errors: null});
globalThis.state       = async () => console.log({
    feedErrors: (await Onyx.get('cardFeedErrors'))?.cardFeedErrors?.[`${FEED}#${DOMAIN_ID}`],
    assignCard: await Onyx.get('assignCard'),
});

// mimics importPlaidAccounts failureData
globalThis.failImport = () => Onyx.merge('assignCard', {isRefreshing: null, errors: {[String(Date.now() * 1000)]: 'Could not generate Plaid accessToken'}});

globalThis.addUnassigned = async () => {
    const list = (await Onyx.get(KEY))?.settings?.oAuthAccountDetails?.[FEED]?.accountList ?? [];
    return feedPatch({accountList: [...list, `MOCK - ${Date.now() % 10000}`]});
};


// One unassigned row to click (XXXX5555) and one assigned card.
await feedPatch({accountList: ['XXXX2004', 'XXXX5555']});
await Onyx.merge(`cards_${DOMAIN_ID}_${FEED}`, {
    999900001: {cardID: 999900001, bank: FEED, fundID: DOMAIN_ID, state: 3, cardName: 'XXXX2004', lastFourPAN: '2004'},
});
await Onyx.merge(`lastSelectedFeed_${POLICY_ID}`, `${FEED}#${DOMAIN_ID}`);

// Scenario 1:
await reconnect(); await seedError();

// Scenario 2 
await clearErrors(); await reconnect(); await addUnassigned(); await expire();

// Scenario 3
await failImport());

Scenario 1 - Feed lvl error

Open app, go to a workspace with a direct (Plaid or OAuth) company card feed that shows the "Card feed connection is broken" banner and has no cards with a broken scrape result.
Navigate to Workspace -> Company Cards, click "log into your bank", complete the bank/Plaid login.
Verify that the RHP stays open during login and closes once the reconnect completes, with no indefinite spinner.

Scenario 2 - Expired feed

Open app, go to a workspace with an expired direct company card feed, start assigning a card, reach the Confirmation step, click Assign.
Verify that the bank/Plaid connection step renders (no "Not found" page or spinner), and after login the RHP closes.

Scenario 3 - Failed import

Open app, Company Cards on the feed with error banner
Click "log into your bank"
Verify that RHP "Assign new cards" opens.
Somehow force a failed import (Can be simulated with Scenario 3 snippet)
Verify that the RHP stays open and shows "Couldn't load this feed" with the broken-bank illustration
Verify that Confirm returns to Company Cards.

Offline tests

N/A

QA Steps

Same as tests

PR Author Checklist

Screenshots/Videos

MacOS: Chrome / Safari

Screen.Recording.2026-09-09.at.11.10.30.mov

Screen.Recording.2026-09-09.at.12.03.37.mov

…ith no error when the feed has a feed-level error

codecov · 2026-09-09T08:42:14Z

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/Plaid.ts	`53.84% <100.00%> (+3.84%)`	⬆️
...pace/companyCards/BrokenCardFeedConnectionPage.tsx	`100.00% <100.00%> (+100.00%)`	⬆️
...ace/companyCards/RefreshCardFeedConnectionPage.tsx	`100.00% <100.00%> (ø)`
...kspace/companyCards/addNew/PlaidConnectionStep.tsx	`0.00% <ø> (ø)`
...sTable/WorkspaceCompanyCardsTableHeaderButtons.tsx	`0.00% <0.00%> (ø)`
src/libs/actions/CompanyCards.ts	`37.45% <0.00%> (-0.15%)`	⬇️
...space/companyCards/assignCard/ConfirmationStep.tsx	`82.35% <0.00%> (-0.99%)`	⬇️
...es/workspace/companyCards/BankConnection/index.tsx	`0.00% <0.00%> (ø)`
...space/companyCards/BankConnection/index.native.tsx	`0.00% <0.00%> (ø)`
... and 214 files with indirect coverage changes

jmusial · 2026-09-09T08:55:55Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3775879c1b

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

jmusial · 2026-09-09T09:09:26Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d472a96239

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

jmusial · 2026-09-09T10:04:23Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e2e062d37

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

jmusial · 2026-09-09T10:35:31Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d79654d12

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

jmusial · 2026-09-10T08:25:52Z

@codex review

chatgpt-codex-connector · 2026-09-10T08:31:05Z

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 0b6d804305

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

melvin-bot · 2026-09-10T08:45:35Z

@aimane-chnaif 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: 33fc2462d2

ℹ️ 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-09-10T08:53:26Z

                onyxMethod: Onyx.METHOD.MERGE,
                key: ONYXKEYS.ASSIGN_CARD,
-                value: {isRefreshing: null},
+                value: {isRefreshing: null, ...(isRepairingFeed ? {errors: getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage')} : {})},


Scope refresh completion to repair imports

The new isRepairingFeed flag scopes only the error, while every failed import still clears the shared ASSIGN_CARD.isRefreshing flag. For example, useAssignCard starts a non-repair import with the default false; if that request is still pending when the user starts the banner refresh, its failure produces a true → null transition with no error, and RefreshCardFeedConnectionPage closes the RHP before the new reauthentication completes. Only repair imports should update refresh state, ideally tied to the active attempt; the unchanged success path has the same overlapping-request problem.

Useful? React with 👍 / 👎.

Race predates the PR: importPlaidAccounts has cleared isRefreshing unconditionally on both success and failure before. This PR only scopes the new errors write.

jmusial · 2026-09-10T09:03:00Z

@MelvinBot review

MelvinBot · 2026-09-10T09:14:12Z

🤖 Code review

The core diagnosis looks right: the broken/refresh connection pages only render BANK_CONNECTION and PLAID_CONNECTION, so BankConnection writing ASSIGNEE/CONFIRMATION could only ever produce the forever spinner. Routing the CTA through startCardFeedRefresh and clearing stale assignCard state are the right moves. CI is green apart from the two checklist checks.

Five things worth a look before merge.

1. Removing the prevFeedExpiration === undefined guard can close the panel before the user logs in

RefreshCardFeedConnectionPage.tsx:66-71 now treats any prevFeedExpiration !== feedExpiration as reconnect-complete. feedExpiration reads cardFeeds?.[feed]?.expiration, and useCardFeeds returns undefined until allFeeds hydrates. If that hydration lands on a render where isRefreshing is already true, the effect sees undefined → <the old, unchanged expiration> and calls closeRHPFlow() before the bank login ever opens. The page's own !cardFeeds?.[feed] → NotFoundPage guard shows that state is reachable, and the early return does not stop the effect from running.

Suggestion: instead of dropping the guard, gate on "the feed entry has been observed at least once" — e.g. keep a usePrevious(!!cardFeeds?.[feed]) and skip the effect on the false → true transition. That still lets a never-cached expiration count as completion without treating late hydration as one.

2. The expired-feed assign flow now silently discards the in-progress assignment

Both platforms replaced the step write with a plain close (web index.tsx:139-145). Combined with clearAssignCardStepAndData() on BrokenCardFeedConnectionPage unmount, the cardholder, card, name, and start date the admin just entered are all dropped, and the RHP disappears with no explanation. The old code never worked here, so this is still a net fix — but is closing the right end state? Navigating back into the assign flow at the Confirmation step after the reconnect would preserve the work. At minimum the admin needs to know why the panel closed.

3. hasImportError keys off any assignCard.errors, not an import-specific signal

index.tsx:79. This is correct today — importPlaidAccounts is the only writer of that field — but it couples both connection pages to the whole ASSIGN_CARD.errors key, so any future failureData on it turns the reconnect panel into "Couldn't load this feed". A dedicated flag would make the contract explicit. Related: the error message written at Plaid.ts:183 is never read — the page hardcodes common.genericErrorMessage as the description. Either render the stored message or don't store one.

4. Native: the WebView branch is the only one not gated on !hasImportError

index.native.tsx:195. Every sibling got the guard; this one is safe only because import errors are Plaid-only and !isPlaid already excludes it. Add && !hasImportError so it can't drift. Web sidesteps this by using if/return in getContent() — the native render could use the same shape.

5. Card-level broken connections still never reach the bank login — intentional?

For a direct feed where isFeedConnectionBroken is true and the feed is not expired, the effect hits updateBrokenConnection(); close on the first pass, before the login window opens — isFeedConnectionBroken takes precedence over isRefreshing. That predates this PR, and Scenario 1 explicitly scopes itself to feeds with "no cards with a broken scrape result", so I assume it's deliberate. Flagging because this PR is about making that exact CTA work.

Nits

The new tests cover the BrokenCardFeedConnectionPage guard, which is the most mechanical change. The subtle logic — the two close effects in RefreshCardFeedConnectionPage and the hasImportError gate in BankConnection — has no coverage, and that's where a regression would land.
tests/unit/BrokenCardFeedConnectionPageTest.tsx still mocks @hooks/useLocalize, but the page no longer imports it.
WorkspaceCompanyCardsTableHeaderButtons.tsx:76 builds <rbr>…</rbr> by string concatenation. A translation entry that already carries the wrapper, like brokenConnectionError does, would keep the markup out of the component.

I reviewed this statically and did not run the test steps in the app — they need a workspace with an existing direct Plaid/OAuth feed plus the console-injected Onyx state from the PR description, neither of which the automated test account can reach.

Reply with @MelvinBot <your feedback> to dig into any of these.

MelvinBot · 2026-09-13T16:34:16Z

Safe here, and it isn't really a removal — the synchronous navigate is startCardFeedRefresh's existing behaviour on main (CompanyCards.ts:1392), already shipping via WorkspaceCompanyCardsSettingsPage.tsx:115 against the same route, whose page already has the same !currentStep → NotFoundPage guard (RefreshCardFeedConnectionPage.tsx:70). This PR just routes a second caller into a pattern that's already in production.

The wrapper was never added for this route. It originated in Expensify/App#62391 (Plaid feature work, Jun 2025), was put on the reconnect navigate by Expensify/App#67536 (Sep 2025, "Don't open RHP after fixing broken card connection"), then got re-pointed at BROKEN_CARD_FEED_CONNECTION by Expensify/App#78077 — which also deleted the setAssignCardStepAndData seeding next to it. That deletion caused Expensify/App#79291, "fix: broken card feed missing currentStep causing infinite loading", whose body states the bug exactly: "When currentStep is undefined, the default case is hit, showing LoadingPage indefinitely."

So the wrapper's real job is make sure currentStep is written before the destination reads it — which is exactly the invariant startCardFeedRefresh preserves by seeding Onyx itself. The blame anchors you'd hit (4fe61b039, c0186af14) are formatting-only commits from #78167 and #78175.

Two things worth knowing before you sign off:

The safety is ordering-dependent, not guaranteed. Onyx.merge lands one microtask later; Navigation.navigate dispatches synchronously but React flushes in its own microtask. It works out only when no React update was already scheduled in that task. If one was, the page mounts while the merge is still queued — and useOnyx deliberately returns undefined + status: 'loading' in that case, which the guard reads as "not found". The existing useVerifyAccountAndResume branch on the settings page does setPendingAction(null) before resumeAction(), so it schedules a React update first and can hit that window today. Pre-existing, not from this PR.

This PR nudges that ordering. The new clearAssignCardStepAndData() is an Onyx.set, which broadcasts to subscribers synchronously before the merge is queued. No useOnyx(ONYXKEYS.ASSIGN_CARD) subscriber is mounted on the company cards page today, so nothing is notified — but it removes some of the margin.

Rather than adding the wrapper back (it also costs a frame), the durable fix is to stop conflating "loading" with "not found" on both pages:

const [assignCard, assignCardMetadata] = useOnyx(ONYXKEYS.ASSIGN_CARD);
if (isLoadingOnyxValue(assignCardMetadata)) {
    return <LoadingPage title={title} />;
}

TransactionStartDateStep already reads the metadata tuple this way. That closes the window for every call site, including the new BrokenCardFeedConnectionPage.tsx:43 guard.

Why the PR keeps the wrapper in ConfirmationStep but not here

The difference is the destination, not the caller. Before this PR, a pending-merge first render of BrokenCardFeedConnectionPage fell through to LoadingPage — harmless. The PR turns that fall-through into NotFoundPage, so ConfirmationStep.tsx:112 needs the deferral. RefreshCardFeedConnectionPage already had the guard and already navigated synchronously on main.

ConfirmationStep is also a much more hostile ordering context: it subscribes to ASSIGN_CARD itself, sits in an RHP stack with other ASSIGN_CARD subscribers, and submits through PressableWithFeedback, which holds press state in useState — on web onPressOut can fire in the same task before onPress, putting React's flush ahead of the merge. The split is defensible, it's just not documented in the code.

Mechanism, verified against react-native-onyx in node_modules:

Onyx.merge queues OnyxUtils.get(key).then(...); even on a cache hit that defers the cache write and subscriber notification by one microtask.
Onyx.set runs broadcastUpdate in the caller's stack frame, so clearAssignCardStepAndData() is synchronous, and the following merge correctly re-reads the cache and lands on top of {} — no set/merge ordering bug.
useOnyx returns [undefined, {status: 'loading'}] when a component makes its first connection while a merge for that key is pending. That's the trap the guard walks into.
setNavigationActionToMicrotaskQueue is a microtask plus a requestAnimationFrame, so it's unconditionally later than any Onyx merge continuation — that's why it papers over all of this.

Recovery is one microtask later, so in practice it's usually a non-painted commit rather than a visible flash — but it's a real extra NotFoundPage mount, and each one fires a SPAN_NOT_FOUND_PAGE telemetry span.

view run · no recording available

jmusial · 2026-09-15T15:08:03Z

Scenario 2 seems failing to me.

Verify that the bank/Plaid connection step renders (no "Not found" page or spinner), and after login the RHP closes.

I am stuck on spinner

case2.mov

Updated scenario 2 steps to add unassigned card explicitly (may have been disabled depending on other scenarios). Can you please recheck ?

Screen.Recording.2026-09-15.at.17.05.42.mov

jmusial · 2026-09-15T15:55:44Z

@MelvinBot please check fix on regression findings 1, 2 and 4.
On 3 decided to leave as is bc it repeats already preexisting pattern

MelvinBot · 2026-09-15T16:06:14Z

Fix check — findings 1, 2, 4

Verified against 972e185 by static analysis (same caveat as before: no live broken feed here, so nothing is runtime-confirmed).

#	Verdict
1	✅ Fixed — but the borrowed translation key deserves a second look
2	⚠️ Partially fixed — the functional break is gone, the stale banner isn't
4	✅ Fixed, and the approach is better than what I suggested

1. Non-direct feed error copy — fixed

WorkspaceCompanyCardsTableHeaderButtons.tsx:77 now renders personalCard.brokenConnection instead of common.genericErrorMessage, so a commercial/CSV feed with feed-level errors no longer says "try again later" for a condition that won't fix itself. All 11 locales have the key, and the <rbr> wrap is correct since the string carries no <a>.

Two things worth a moment before you merge:

The key is borrowed from the personal-cards namespace. Its only other consumer is PersonalCardDetailsPage.tsx:254. A future copy edit for personal cards would silently change this admin-facing banner. A brokenConnectionErrorNoLink sibling next to brokenConnectionError would decouple them.
The copy is singular-card on a feed-level error. "Your card connection is broken." vs the direct-feed "Card feed connection is broken." — the reader here is an admin looking at a whole commercial feed, not their own card. Since I flagged this one as needing copy sign-off, that's the question to put to them.

2. Plaid import error leaking into the assign form — half fixed

ConfirmationStep.tsx:116 clears the errors before navigating, which closes the harmful half: hasImportError can no longer fire on a stale error, so Assign on an expired feed reaches the bank login instead of "Couldn't load this feed". That was the part that actually blocked the user.

What's still open: Plaid.ts:183 still writes the shared slot and ConfirmationStep.tsx:222 still renders assignCard?.errors ?? cardError. The clear runs only on submit, and only inside the isFeedExpired branch — so the original scenario's first symptom is unchanged: if an abandoned Plaid repair fails while the admin is sitting on Confirmation, a generic banner appears on a form that didn't fail, and it survives a restart because ASSIGN_CARD is persisted. On the non-expired path submit skips the clear entirely, so the banner also rides along through assignWorkspaceCompanyCard.

Reaching it is narrow — useAssignCard clears errors at flow start, so the failure has to land after that — and it's dismissible via the banner's own close. Fine to ship as-is; the dedicated-field option is what would close it fully.

4. Refresh page closing without a reconnect — fixed

Tracking the feed object instead of the scalar (RefreshCardFeedConnectionPage.tsx:47, guard at :67) is a neater fix than the allFeedsResult status gate I proposed — it satisfies both cases with one condition, no extra subscription.

Why it holds for both the late-load and uncached-OAuth cases

Late collection load (the regression): on the render where feeds arrive, usePrevious still returns undefined, so !prevFeed bails and closeRHPFlow() doesn't fire. That's exactly what the dropped prevFeedExpiration === undefined clause covered.
Uncached OAuth feed (why the clause was dropped in d472a96): still works, because the render guard means the page only renders once cardFeeds[feed] exists. So prevFeed is truthy with expiration: undefined, and the first populated expiration still registers as a change. Tracking the object rather than the scalar is what separates "feed absent" from "expiration absent" — that's the insight the scalar version couldn't express.
Hook ordering is safe: usePrevious's internal effect is registered first, but the close effect closes over the render-time prevFeed, not the ref.
No repeat-fire risk from the object dep — prevFeed.expiration catches up on the next render, same as the scalar did.

One narrow residual, strictly better than before: if a reconnect completes before the collection first loads (hard reload mid-reconnect with isRefreshing persisted), the RHP won't auto-close. But the page renders the bank-login step, so re-auth closes it on the next expiration change — an extra step, not the old endless spinner.

The new test at RefreshCardFeedConnectionPageTest.tsx:255 pins the loading→loaded transition directly. Good coverage for the exact hole.

3 — agreed

Leaving it matches RefreshCardFeedConnectionPage.tsx:72, which already ships on main, so the two pages stay consistent. Reasonable call — the loading-vs-not-found conflation is a pre-existing pattern worth fixing in both pages at once, not in this PR.

Not verified: I couldn't run jest, tsc, or eslint in this environment, so the two page suites and typecheck are unconfirmed here — CI covers them.

jmusial · 2026-09-15T16:16:24Z

@aimane-chnaif addressed all above

chatgpt-codex-connector · 2026-09-16T07:01:34Z

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 972e185d2f

ℹ️ 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-16T07:17:48Z

⚠️ Claude's API is currently overloaded (HTTP 529). This is a temporary issue on Anthropic's side. Try again in a few minutes, or check status.anthropic.com.

Run details

github-actions · 2026-09-16T07:52:03Z

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

MelvinBot · 2026-09-16T11:01:25Z

🤖 Regression review of this diff (static analysis — I read the changed files and everything that feeds them; I was not able to execute the Jest suites in this environment, so nothing below is backed by a test run).

Four things look like real regressions or gaps. The first two are the ones I'd fix before merge.

1. The OAuth auto-close guard got weaker and can close the panel mid-login

RefreshCardFeedConnectionPage.tsx:66-71

prevFeedExpiration === undefined guarded every undefined → value expiration transition. !prevFeed only guards the first render where the feed object itself is missing. Once the feed object exists with expiration: undefined — the exact case the new comment says it's targeting — the next Onyx write that populates expiration calls closeRHPFlow() even though the admin never finished the bank login. The reconnect then looks like it succeeded.

The new test only covers cardFeeds === undefined → loaded. It does not cover "feed object present, expiration arrives second", which is where the old guard was doing the work.

2. `BrokenCardFeedConnectionPage` now treats "still loading" as "not found"

BrokenCardFeedConnectionPage.tsx:43

useCardFeeds returns undefined until the Onyx collection hydrates and hands back allFeedsResult as the second tuple element, which this guard ignores (useCardFeeds.tsx:74). This route is in linkingConfig, so a reload or deeplink on it renders "Hmm… it's not here" where LoadingPage used to render. Gating on allFeedsResult.status instead of on cardFeeds alone fixes both this page and the pre-existing one on the refresh page.

3. Reconnecting mid-assign now silently discards the assignment

BankConnection/index.tsx:139-145

Both entry points that reach this branch — useAssignCard.ts:113-118 and ConfirmationStep.tsx:116-121 — now end with the RHP just disappearing after a successful bank login, with the collected cardholder / card / start date wiped by the unmount cleanup. Trading a forever spinner for this is defensible, but the admin gets no message telling them to start over. Worth a design/copy call rather than leaving it silent.

4. The banner's most common trigger still self-closes

BankConnection/index.tsx:134-138 checks isFeedConnectionBroken before the isRefreshing check. For a non-expired direct feed whose cards have broken scrape results, mounting the refresh page still runs updateBrokenConnection() + closeRHPFlow() immediately — the banner clears without anything being reconnected. That's pre-existing, not introduced here, but it is the isFeedConnectionBroken half of getShouldShowBrokenConnectionError, and your Scenario 1 explicitly excludes it ("no cards with a broken scrape result"), so the routing change is untested against the more common path.

Smaller notes

WorkspaceCompanyCardsTableHeaderButtons.tsx:77 puts personalCard.brokenConnection ("Your card connection is broken.") on a workspace company-cards banner for commercial and CSV feeds. It reads as personal-card copy, gives the admin no next step, and none of the test steps cover it.
CompanyCards.ts:1386-1394 does Onyx.set({}) → Onyx.merge(...) → synchronous Navigation.navigate, while the destination hard-returns NotFoundPage on !currentStep. Every other caller of this route defers with setNavigationActionToMicrotaskQueue. Probably fine given the nav transition, but worth one look on device for a Not-found flash on first paint.
In PlaidConnectionStep, importPlaidAccounts only fires inside the plaidConnectedFeed && selectedCountry && plaidConnectedFeedName guard, while currentStep moves to BANK_CONNECTION unconditionally. If that guard misses, isRefreshing is never cleared and BankConnection returns early on isPlaid — a forever spinner this PR doesn't close off.

isRepairingFeed threading looks right to me: PlaidConnectionStep only passes true inside the if (feed) branch, and useImportPlaidAccounts still defaults it to false, so the add-feed flow can't set assignCard.errors. I also confirmed assignWorkspaceCompanyCard never writes errors to ASSIGN_CARD, so the hasImportError comment holds.

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

mountiny

@jmusial I dont think QA can test these flow exactly like that in staging, I wonder what would be the best way to handle the testing

github-actions · 2026-09-17T09:48:21Z

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

OSBotify · 2026-09-17T09:50:20Z

✋ 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-17T09:52:14Z

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

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

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-17T22:21:08Z

🚀 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:34:48Z

🤖 Yes — help site changes are required. Draft PR: Docs updates for E/A#100687 Broken company card feed reconnect flow

One article covers this flow — docs/articles/new-expensify/connect-credit-cards/Fix-a-Broken-Company-Card-Feed-Connection.md — and this PR changed three things it documents, one of which made an existing step wrong.

1. The reconnect link now only exists on direct feeds. WorkspaceCompanyCardsTableHeaderButtons.tsx:77 picks the banner copy by feed type. Direct (OAuth/Plaid) feeds keep brokenConnectionError with the log into your bank link; everything else gets personalCard.brokenConnection — "Your card connection is broken." — with no link. Commercial and spreadsheet feeds still surface the banner whenever the feed itself has errors (getShouldShowBrokenConnectionError.ts:13-15), so the article's old step 4 — "Select the error message under the card feed name and follow the prompts" — no longer works for them.

2. There is a new error screen. BankConnection/index.tsx:204-217 now renders "Couldn't load this feed" with a Got it button when the bank login succeeds but the account import fails. Nothing in the help site mentioned it.

3. The panel's open/close behavior changed. The reconnect stays open through the bank login and closes itself once the reconnect completes, and an assign-card flow that detoured into the reconnect closes rather than resuming — an admin has to assign the card again.

What the docs PR changes

Scoped How to fix a broken company card connection to direct feeds, quoted the full banner message, and replaced "follow the prompts" with selecting log into your bank.
Added What to do when a company card feed can't be reconnected in Expensify for commercial and spreadsheet feeds, with the exact Your card connection is broken. copy and what to do instead.
Added a Couldn't load this feed entry to the troubleshooting list with the exact heading, message, and Got it button.
Documented the panel closing itself, and that an interrupted card assignment isn't saved, under What happens after you fix a company card connection.
Added two FAQ entries: why a feed has no log into your bank link, and why the panel closes mid-assignment.

No new article — all of it belongs to the existing broken-connection workflow, and splitting it would break the one-workflow-per-article rule in HELP_AUTHORING_GUIDELINES.md.

Label verification. Workspaces and Company cards were confirmed against the live UI on web. Strings that need a broken feed to reach were taken verbatim from src/languages/en.ts. Worth flagging: common.buttonConfirm renders as Got it, not "Confirm" as the test steps describe — the docs use Got it.

@jmusial, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

mountiny · 2026-09-18T09:04:30Z

Asked @joekaufmanexpensify if he can help with testing this feature, though he cannot exactly qa the flow mentioned in the test steps

Fix - Company cards - Broken card feed connection RHP spins forever w…

3775879

…ith no error when the feed has a feed-level error

melvin-bot Bot assigned jmusial Sep 9, 2026

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread src/components/Tables/WorkspaceCompanyCardsTable/WorkspaceCompanyCardsTableHeaderButtons.tsx

accomodate for uncached OAuth feed

d472a96

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread src/components/Tables/WorkspaceCompanyCardsTable/WorkspaceCompanyCardsTableHeaderButtons.tsx

jmusial mentioned this pull request Sep 9, 2026

Company cards - Broken card feed connection RHP spins forever with no error when the feed has a feed-level error #100465

Open

2 tasks

accomodate for import error

5e2e062

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread src/libs/actions/Plaid.ts Outdated

Comment thread src/pages/workspace/companyCards/BankConnection/index.tsx Outdated

fix ai review comments

5d79654

chatgpt-codex-connector Bot reviewed Sep 9, 2026

View reviewed changes

Comment thread src/components/Tables/WorkspaceCompanyCardsTable/WorkspaceCompanyCardsTableHeaderButtons.tsx

update banner if not direct feed

0b6d804

Update comments

33fc246

jmusial marked this pull request as ready for review September 10, 2026 08:45

jmusial requested review from a team as code owners September 10, 2026 08:45

melvin-bot Bot requested review from aimane-chnaif and dylanexpensify and removed request for a team September 10, 2026 08:45

chatgpt-codex-connector Bot reviewed Sep 10, 2026

View reviewed changes

Merge branch 'main' into fix/endless-spinner-on-reconnect-card

a345db2

fix Melvins 1 and 2 findings

786b6dd

jmusial dismissed dylanexpensify’s stale review via 786b6dd September 15, 2026 15:48

fix Melvins 4 finding

972e185

aimane-chnaif approved these changes Sep 16, 2026

View reviewed changes

melvin-bot Bot requested a review from mountiny September 16, 2026 06:57

mountiny requested changes Sep 16, 2026

View reviewed changes

Comment thread tests/unit/BrokenCardFeedConnectionPageTest.tsx

Comment thread tests/unit/RefreshCardFeedConnectionPageTest.tsx

This comment has been minimized.

Sign in to view

add given, when, then comments

502374f

jmusial requested a review from mountiny September 16, 2026 13:09

mountiny approved these changes Sep 17, 2026

View reviewed changes

mountiny merged commit 3a2f59d into Expensify:main Sep 17, 2026
36 of 38 checks passed

mountiny added the InternalQA This pull request required internal QA label Sep 17, 2026

OSBotify mentioned this pull request Sep 17, 2026

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

Open

77 tasks

MelvinBot mentioned this pull request Sep 17, 2026

[No QA] Docs updates for E/A#100687 Broken company card feed reconnect flow #101481

Draft

40 tasks

Conversation

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

Uh oh!

Explanation of Change

Fixed Issues

Tests

For dev tests - how to get local feed into desired state

Scenario 1 - Feed lvl error

Scenario 2 - Expired feed

Scenario 3 - Failed import

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

codecov Bot commented Sep 9, 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!

jmusial 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!

jmusial 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!

jmusial 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!

Uh oh!

jmusial 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!

jmusial commented Sep 10, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 10, 2026

Uh oh!

melvin-bot Bot commented Sep 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

jmusial Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

jmusial commented Sep 10, 2026

Uh oh!

MelvinBot commented Sep 10, 2026

Uh oh!

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

Uh oh!

Uh oh!

jmusial commented Sep 15, 2026

Uh oh!

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

Uh oh!

Uh oh!

MelvinBot commented Sep 15, 2026

Fix check — findings 1, 2, 4

1. Non-direct feed error copy — fixed

2. Plaid import error leaking into the assign form — half fixed

jmusial commented Sep 9, 2026 •

edited

Loading

codecov Bot commented Sep 9, 2026 •

edited by github-actions Bot

Loading

MelvinBot commented Sep 13, 2026 •

edited

Loading

jmusial commented Sep 15, 2026 •

edited

Loading

2. `BrokenCardFeedConnectionPage` now treats "still loading" as "not found"