Skip to content

Commit 32f24df

Browse files
fix(ramp): skip RESET-action beforeRemove close on HeadlessHost (Cursor Bugbot)
Phase 9.5's chrome-strip commit added a beforeRemove navigation listener on HeadlessHost that fires closeSession({ reason: 'user_dismissed' }) on every beforeRemove event. Cursor Bugbot caught a high-severity bug: useTransakRouting uses navigation.reset() to re-pin HEADLESS_HOST at the base of the stack when moving to VerifyIdentity / BasicInfo / Checkout / KycWebview. The reset action fires beforeRemove on the OLD HEADLESS_HOST instance before re-pinning the new one, but the session is still in flight — closing it here prematurely fires onClose({ reason: 'user_dismissed' }) and breaks the consumer's flow. Resolution: inspect e.data.action.type inside the listener and short-circuit on 'RESET'. Legitimate user-dismissal paths (GO_BACK, POP) still close the session synchronously. Other unmount cases (real stack reset that does NOT re-pin the Host, hot reload) remain caught by useHeadlessSessionDismissal's unmount cleanup, which uses isHeadlessHostStillInNavigator to differentiate. Adds a HeadlessHost.test.tsx case 'does NOT close the session when beforeRemove fires for a RESET action (stack rebuild guard)'. Updates the existing test helper to pass a non-RESET action in the event arg so the listener's new signature stays exercised. Bug report: #30104 (review)
1 parent 3fff472 commit 32f24df

4 files changed

Lines changed: 85 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
### Fixed
1616

1717
- Route swallowed `getUserLimits` infrastructure errors through `failSession` when a headless Ramp session is active, so headless consumers receive an `onError` callback instead of a silent hang (Phase 9.5 Fix A).
18+
- Skip the `beforeRemove` close path on `RESET` actions in the Ramp Headless Host so `useTransakRouting`'s stack rebuilds (re-pinning HEADLESS_HOST when routing to VerifyIdentity / BasicInfo / Checkout / KycWebview) no longer prematurely fire `onClose({ reason: 'user_dismissed' })` (Phase 9.5; Cursor Bugbot finding).
1819

1920
## [7.75.1]
2021

app/components/UI/Ramp/Views/HeadlessHost/HeadlessHost.test.tsx

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,18 @@ const mockUseContinueWithQuoteOptions = jest.fn();
3434
// Holds the most recent 'beforeRemove' listener registered against the
3535
// mocked navigation object. Tests fire it directly to exercise the
3636
// production beforeRemove path without spinning up a real navigator.
37-
let registeredBeforeRemoveListener: (() => void) | null = null;
37+
// The listener now reads `e.data.action.type` so it can ignore RESET
38+
// stack-rebuilds; tests pass an event arg accordingly.
39+
interface BeforeRemoveEvent {
40+
data: { action: { type: string } };
41+
}
42+
let registeredBeforeRemoveListener: ((e?: BeforeRemoveEvent) => void) | null =
43+
null;
44+
// Default event passed when a test wants to simulate a user back/swipe — any
45+
// non-RESET action type triggers the close path.
46+
const DEFAULT_GO_BACK_EVENT: BeforeRemoveEvent = {
47+
data: { action: { type: 'GO_BACK' } },
48+
};
3849

3950
jest.mock('@react-navigation/native', () => {
4051
const actual = jest.requireActual('@react-navigation/native');
@@ -164,7 +175,7 @@ describe('HeadlessHost', () => {
164175
__resetSessionRegistryForTests();
165176
registeredBeforeRemoveListener = null;
166177
mockAddListener.mockImplementation(
167-
(eventName: string, listener: () => void) => {
178+
(eventName: string, listener: (e?: BeforeRemoveEvent) => void) => {
168179
if (eventName === 'beforeRemove') {
169180
registeredBeforeRemoveListener = listener;
170181
}
@@ -455,7 +466,7 @@ describe('HeadlessHost', () => {
455466
expect(typeof registeredBeforeRemoveListener).toBe('function');
456467

457468
// Fire the listener like React Navigation would on a back gesture.
458-
registeredBeforeRemoveListener?.();
469+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
459470

460471
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
461472
expect(callbacks.onClose).toHaveBeenCalledWith({
@@ -464,6 +475,30 @@ describe('HeadlessHost', () => {
464475
expect(getSession(session.id)).toBeUndefined();
465476
});
466477

478+
it('does NOT close the session when beforeRemove fires for a RESET action (stack rebuild guard)', () => {
479+
// Cursor Bugbot — useTransakRouting calls navigation.reset() to
480+
// re-pin HEADLESS_HOST at the base when moving to VerifyIdentity /
481+
// BasicInfo / Checkout / KycWebview. The reset fires beforeRemove on
482+
// the OLD instance, but the session is still in flight; closing it
483+
// here would prematurely fire onClose({ reason: 'user_dismissed' })
484+
// and break the flow.
485+
const quote = buildAggregatorQuote();
486+
const session = seedSession(quote);
487+
const callbacks = session.callbacks;
488+
renderHost({ headlessSessionId: session.id });
489+
expect(typeof registeredBeforeRemoveListener).toBe('function');
490+
491+
registeredBeforeRemoveListener?.({
492+
data: { action: { type: 'RESET' } },
493+
});
494+
495+
expect(callbacks.onClose).not.toHaveBeenCalled();
496+
// The session is still live so useTransakRouting's reset can complete
497+
// and re-pin the new HEADLESS_HOST without losing the consumer's
498+
// callbacks.
499+
expect(getSession(session.id)).toBeDefined();
500+
});
501+
467502
it('fires onClose({ reason: "user_dismissed" }) once when the host unmounts mid-flow without a terminal status', async () => {
468503
mockContinueWithQuote.mockImplementation(
469504
() => new Promise(() => undefined),
@@ -506,7 +541,7 @@ describe('HeadlessHost', () => {
506541
// beforeRemove fires too (React Navigation always fires it on screen
507542
// removal), then unmount cleanup runs. Both find the session gone and
508543
// no-op.
509-
registeredBeforeRemoveListener?.();
544+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
510545
unmount();
511546

512547
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
@@ -526,7 +561,7 @@ describe('HeadlessHost', () => {
526561
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
527562
expect(callbacks.onClose).toHaveBeenCalledWith({ reason: 'unknown' });
528563

529-
registeredBeforeRemoveListener?.();
564+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
530565
unmount();
531566

532567
// Both follow-up paths re-read, see nothing, no-op.
@@ -546,7 +581,7 @@ describe('HeadlessHost', () => {
546581
});
547582

548583
const { unmount } = renderHost({ headlessSessionId: session.id });
549-
registeredBeforeRemoveListener?.();
584+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
550585
unmount();
551586

552587
expect(callbacks.onClose).toHaveBeenCalledTimes(1);

app/components/UI/Ramp/Views/HeadlessHost/HeadlessHost.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,22 @@ function HeadlessHost() {
104104
// are caught by `useHeadlessSessionDismissal`'s unmount cleanup above.
105105
// closeSession is idempotent: Phase 6 success and Phase 7 errors clear
106106
// the session before `beforeRemove` and turn this into a no-op.
107+
//
108+
// Stack-rebuild guard (Cursor Bugbot): `useTransakRouting` calls
109+
// `navigation.reset()` to re-pin HEADLESS_HOST at the base of the stack
110+
// when navigating to VerifyIdentity / BasicInfo / Checkout / KycWebview.
111+
// The reset action fires `beforeRemove` on the OLD HEADLESS_HOST instance
112+
// before re-pinning the new one, but the session is still in flight —
113+
// closing it here would prematurely fire `onClose({ reason: 'user_dismissed' })`
114+
// and break the flow. Skip the close when the action is a RESET; the
115+
// legitimate unmount cases (stack reset that does NOT re-pin the Host,
116+
// hot reload) are caught by `useHeadlessSessionDismissal`'s unmount path
117+
// with `isHeadlessHostStillInNavigator`.
107118
useEffect(() => {
108-
const unsubscribe = navigation.addListener('beforeRemove', () => {
119+
const unsubscribe = navigation.addListener('beforeRemove', (e) => {
120+
if (e.data.action.type === 'RESET') {
121+
return;
122+
}
109123
closeSession(headlessSessionId, { reason: 'user_dismissed' });
110124
});
111125
return unsubscribe;

app/components/UI/Ramp/headless/PLAN.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,34 @@ UB2's existing swallow stays in place — the existing test at [useTransakRoutin
671671
672672
The parallel swallow in [useDepositRouting.ts:137-180](../hooks/useDepositRouting.ts) is deliberately **not** fixed here — Deposit isn't a headless route, and the "ramps work is UB2-only" rule keeps us off the Deposit tree.
673673
674+
#### Fix B — RESET-action guard in HeadlessHost's `beforeRemove` listener (Cursor Bugbot)
675+
676+
The chrome-strip commit added a `beforeRemove` navigation listener on HeadlessHost so the synchronous `closeSession({ reason: 'user_dismissed' })` still fires when the user backs out (no more visible Cancel/Back button to wire). The original implementation fired the close on EVERY `beforeRemove` event.
677+
678+
Cursor Bugbot flagged that `useTransakRouting` uses `navigation.reset()` to re-pin HEADLESS_HOST at the base of the stack whenever it routes to VerifyIdentity / BasicInfo / Checkout / KycWebview. The reset action fires `beforeRemove` on the OLD HEADLESS_HOST instance before re-pinning the new one — but the session is still in flight, so the close fires prematurely with `user_dismissed`, breaking the consumer's flow.
679+
680+
Resolution at [HeadlessHost.tsx:107-118](../Views/HeadlessHost/HeadlessHost.tsx): inspect `e.data.action.type` inside the listener and skip the close on `'RESET'`:
681+
682+
```ts
683+
useEffect(() => {
684+
const unsubscribe = navigation.addListener('beforeRemove', (e) => {
685+
if (e.data.action.type === 'RESET') {
686+
return;
687+
}
688+
closeSession(headlessSessionId, { reason: 'user_dismissed' });
689+
});
690+
return unsubscribe;
691+
}, [navigation, headlessSessionId]);
692+
```
693+
694+
The legitimate unmount paths (real stack reset that does NOT re-pin the Host, hot reload) stay correct because `useHeadlessSessionDismissal`'s unmount cleanup uses `isHeadlessHostStillInNavigator` to differentiate. The two layers are complementary: `beforeRemove` for synchronous user-driven dismissals (GO_BACK, POP), `useHeadlessSessionDismissal` for asynchronous unmount cleanup.
695+
696+
A new test in [HeadlessHost.test.tsx](../Views/HeadlessHost/HeadlessHost.test.tsx) (`'does NOT close the session when beforeRemove fires for a RESET action (stack rebuild guard)'`) fires the listener with `{ data: { action: { type: 'RESET' } } }` and asserts the session stays live.
697+
698+
#### Note on the original "Fix B" deferral
699+
700+
The pasted-plan placeholder "Fix B (`getProviderBuyLimit` belt-and-braces pre-flight)" landed in the Phase 9 PR ([metamask-mobile#30103](https://github.com/MetaMask/metamask-mobile/pull/30103)) as `feat(ramp): pre-quote static bounds check + expose getProviderBuyLimit`, with a more comprehensive implementation than this PR originally drafted (UB2-parity skip on `amount <= 0`, conservative "all candidates reject" rule, `details.source` discriminator, plus a public `getProviderBuyLimit` re-export). Phase 9.5 doesn't duplicate it.
701+
674702
---
675703

676704
## Phase 10 — Implement deferred Phase 5b + playground polish

0 commit comments

Comments
 (0)