Skip to content

Commit 1c5b205

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 4f5ddfb commit 1c5b205

4 files changed

Lines changed: 84 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: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,17 @@ 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+
type BeforeRemoveEvent = { data: { action: { type: string } } };
40+
let registeredBeforeRemoveListener:
41+
| ((e?: BeforeRemoveEvent) => void)
42+
| null = null;
43+
// Default event passed when a test wants to simulate a user back/swipe — any
44+
// non-RESET action type triggers the close path.
45+
const DEFAULT_GO_BACK_EVENT: BeforeRemoveEvent = {
46+
data: { action: { type: 'GO_BACK' } },
47+
};
3848

3949
jest.mock('@react-navigation/native', () => {
4050
const actual = jest.requireActual('@react-navigation/native');
@@ -164,7 +174,7 @@ describe('HeadlessHost', () => {
164174
__resetSessionRegistryForTests();
165175
registeredBeforeRemoveListener = null;
166176
mockAddListener.mockImplementation(
167-
(eventName: string, listener: () => void) => {
177+
(eventName: string, listener: (e?: BeforeRemoveEvent) => void) => {
168178
if (eventName === 'beforeRemove') {
169179
registeredBeforeRemoveListener = listener;
170180
}
@@ -455,7 +465,7 @@ describe('HeadlessHost', () => {
455465
expect(typeof registeredBeforeRemoveListener).toBe('function');
456466

457467
// Fire the listener like React Navigation would on a back gesture.
458-
registeredBeforeRemoveListener?.();
468+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
459469

460470
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
461471
expect(callbacks.onClose).toHaveBeenCalledWith({
@@ -464,6 +474,30 @@ describe('HeadlessHost', () => {
464474
expect(getSession(session.id)).toBeUndefined();
465475
});
466476

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

512546
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
@@ -526,7 +560,7 @@ describe('HeadlessHost', () => {
526560
expect(callbacks.onClose).toHaveBeenCalledTimes(1);
527561
expect(callbacks.onClose).toHaveBeenCalledWith({ reason: 'unknown' });
528562

529-
registeredBeforeRemoveListener?.();
563+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
530564
unmount();
531565

532566
// Both follow-up paths re-read, see nothing, no-op.
@@ -546,7 +580,7 @@ describe('HeadlessHost', () => {
546580
});
547581

548582
const { unmount } = renderHost({ headlessSessionId: session.id });
549-
registeredBeforeRemoveListener?.();
583+
registeredBeforeRemoveListener?.(DEFAULT_GO_BACK_EVENT);
550584
unmount();
551585

552586
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)