fix: unblock neobank-demo CI and resolve Cursor findings for #9853

packages/kyc-controller/CHANGELOG.md

@@ -25,4 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Move Money Account wallet registration to `@metamask/ramps-controller`: removes `KycController.registerMoneyAccountWallet`, the `KycService` wallet-registration methods (`getMoonpayCustomerId`, `getWalletRegistrationStatus`, `registerSelfHostedWallet`), the `neobankBaseUrl` service option, and the wallet registration exports (`WalletRegistrationError`, `SelfHostedRegistration`, `MoneyAccountWalletRegistrationResult`, and related types). Wallet ownership signing is a Money Movement (neobank-proxy) concern, so it now lives on `RampsController` / `NeoBankService`. ([#9853](https://github.com/MetaMask/core/pull/9853))
 
+### Fixed
+
+- Clear `moonpayCustomerId` when the active vendor changes, so `getCustomerIdentity()` can no longer report a MoonPay customer id under another vendor. The id is dropped when `initialize` starts a non-MoonPay flow and when `createIronCustomer` switches to Iron. ([#9861](https://github.com/MetaMask/core/pull/9861), [#9853](https://github.com/MetaMask/core/pull/9853))
+- Call `unref()` on the user-status poll timer only when it exists. React Native and browser timers are numbers, so the unconditional call threw when Money status polling started outside Node. ([#9861](https://github.com/MetaMask/core/pull/9861), [#9853](https://github.com/MetaMask/core/pull/9853))
+- Skip the `session_not_in_valid_state` completion write when a `reset()` superseded the SumSub flow, so a late vendor response can no longer force `userStatus` to `completed` (and publish `statusChanged`) on an idle controller. ([#9861](https://github.com/MetaMask/core/pull/9861), [#9853](https://github.com/MetaMask/core/pull/9853))
+
 [Unreleased]: https://github.com/MetaMask/core/

packages/kyc-controller/src/KycController.test.ts

@@ -1251,6 +1251,53 @@ describe('KycController', () => {
         },
       );
     });
+
+    it('drops a MoonPay id when initialize switches to another vendor', async () => {
+      await withController(
+        {
+          options: {
+            state: { moonpayCustomerId: 'cust-1', activeVendor: 'moonpay' },
+          },
+        },
+        async ({ controller }) => {
+          await controller.initialize({ vendor: 'iron' });
+
+          expect(controller.state.moonpayCustomerId).toBeNull();
+          expect(controller.getCustomerIdentity()).toBeNull();
+        },
+      );
+    });
+
+    it('keeps a MoonPay id when initialize stays on MoonPay', async () => {
+      await withController(
+        {
+          options: {
+            state: { moonpayCustomerId: 'cust-1', activeVendor: 'moonpay' },
+          },
+        },
+        async ({ controller }) => {
+          await controller.initialize({ vendor: 'moonpay' });
+
+          expect(controller.state.moonpayCustomerId).toBe('cust-1');
+        },
+      );
+    });
+
+    it('drops a MoonPay id when an Iron customer is created', async () => {
+      await withController(
+        {
+          options: {
+            state: { moonpayCustomerId: 'cust-1', activeVendor: 'moonpay' },
+          },
+        },
+        async ({ controller }) => {
+          await controller.createIronCustomer({ email: 'a@b.co' });
+
+          expect(controller.state.moonpayCustomerId).toBeNull();
+          expect(controller.getCustomerIdentity()).toBeNull();
+        },
+      );
+    });
   });
 
   describe('startSumSub', () => {
@@ -2441,6 +2488,33 @@ describe('KycController', () => {
       );
     });
 
+    it('leaves an already-reset controller idle when SumSub reports a stale session', async () => {
+      await withController(
+        {
+          options: {
+            state: { activeVendor: 'iron', phase: 'submit' },
+          },
+        },
+        async ({ controller, handlers }) => {
+          let rejectSession: (error: Error) => void = () => undefined;
+          handlers.createUkycSession.mockReturnValue(
+            new Promise((_resolve, reject) => {
+              rejectSession = reject;
+            }),
+          );
+
+          const pending = controller.startSumSub();
+          controller.reset();
+          rejectSession(new Error('session_not_in_valid_state'));
+
+          expect(await pending).toStrictEqual({ alreadyCompleted: true });
+          expect(controller.state.userStatus).toBeNull();
+          expect(controller.state.phase).toBe('idle');
+          expect(controller.state.sumsub.status).toBe('idle');
+        },
+      );
+    });
+
     it('keeps phase done when Iron SumSub reports already completed', async () => {
       await withController(
         {

packages/kyc-controller/src/KycController.ts

@@ -633,6 +633,12 @@ export class KycController extends BaseController<
         state.email = params.email;
       }
       state.activeVendor = vendor;
+      // `moonpayCustomerId` is only ever issued by the MoonPay Check / Auth
+      // frames. Leaving it set while the flow switches to another vendor would
+      // make `getCustomerIdentity` report a MoonPay id under the wrong vendor.
+      if (vendor !== 'moonpay') {
+        state.moonpayCustomerId = null;
+      }
       state.activeProduct = params?.product ?? null;
     });
 
@@ -703,6 +709,9 @@ export class KycController extends BaseController<
     this.#applyUpdate((state) => {
       state.email = params.email;
       state.activeVendor = 'iron';
+      // See `initialize`: a MoonPay-issued customer id must not survive a
+      // switch to Iron, or `getCustomerIdentity` reports the wrong vendor.
+      state.moonpayCustomerId = null;
     });
     const generation = this.#generation;
     try {
@@ -1537,6 +1546,12 @@ export class KycController extends BaseController<
     } catch (error) {
       // Applicant already finished KYC — treat as completed for Money toast.
       if (String(error).includes(SESSION_NOT_IN_VALID_STATE)) {
+        // A reset() may have landed while `launch` was in flight; forcing
+        // `completed` (and publishing `statusChanged`) on an idle controller
+        // would resurrect a flow the consumer already tore down.
+        if (this.#generation !== generation) {
+          return { alreadyCompleted: true };
+        }
         this.#applyUserStatus({
           status: 'completed',
           sumsubSessionId: null,
@@ -1670,14 +1685,16 @@ export class KycController extends BaseController<
         tick();
       }, this.#userStatusPollIntervalMs);
       // Allow the process to exit while a pending-status poll is scheduled.
-      this.#userStatusPollTimer.unref();
+      // React Native / browser timers are numbers with no `unref`, hence the
+      // optional call.
+      this.#userStatusPollTimer.unref?.();
     };
     this.#userStatusPollTimer = setTimeout(() => {
       this.#userStatusPollTimer = null;
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       tick();
     }, this.#userStatusPollIntervalMs);
-    this.#userStatusPollTimer.unref();
+    this.#userStatusPollTimer.unref?.();
   }
 
   /**

packages/ramps-controller/CHANGELOG.md

@@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Resolve autoramp / Money Account wallet-registration customer id only via Profile Sync + `NeoBankService:getCustomerByExternalId` (prefer `canonicalProfileId`, else `profileId`). Stop calling `KycController:getCustomerIdentity` from ramps; remove the local `KycControllerGetCustomerIdentityAction` type and drop that action from `RAMPS_CONTROLLER_REQUIRED_CONTROLLER_ACTIONS`. ([#9859](https://github.com/MetaMask/core/pull/9859), [#9853](https://github.com/MetaMask/core/pull/9853))
 - Point `NeoBankService.getAutoramp` at `GET /neobank/autoramps/{id}` (neobank-proxy global `/neobank` prefix) instead of `/api/v2/autoramps/{id}`, so Core matches the proxy that ships. ([#9853](https://github.com/MetaMask/core/pull/9853))
 
+### Fixed
+
+- Keep the local `customerId` / `walletAddress` when a remote autoramp snapshot omits or blanks them. The proxy sends empty identity fields on partial status pushes, and `applyAutorampRemoteStatus` / `mapNeoBankAutorampToRemoteSnapshot` treated those as a clear, wiping valid local values during refresh-on-load and websocket pushes. ([#9861](https://github.com/MetaMask/core/pull/9861), [#9853](https://github.com/MetaMask/core/pull/9853))
+
 ## [20.0.0]
 
 ### Changed

packages/ramps-controller/src/NeoBankService.ts

@@ -179,7 +179,10 @@ export function mapNeoBankAutorampToRemoteSnapshot(
     id: response.id,
     customerId: response.customer_id,
     walletAddress:
-      response.wallet_address ?? response.recipient_account?.address,
+      response.wallet_address !== undefined &&
+      response.wallet_address.length > 0
+        ? response.wallet_address
+        : response.recipient_account?.address,
     status: response.status,
     depositRailsSummary,
   };

packages/ramps-controller/src/RampsController.test.ts

@@ -9305,6 +9305,194 @@ describe('RampsController', () => {
         );
       });
     });
+
+    /**
+     * Registers the User Storage / auth handlers that let the incremental
+     * autoramp pushes run, so tests can drive the remote-write code paths.
+     *
+     * @param rootMessenger - Root messenger of the controller under test.
+     * @param batchSet - Handler for `performBatchSetStorage`.
+     */
+    function registerAutorampSyncHandlers(
+      rootMessenger: RootMessenger,
+      batchSet: jest.Mock,
+    ): void {
+      rootMessenger.registerActionHandler(
+        'UserStorageController:getState',
+        () => ({ isBackupAndSyncEnabled: true }) as never,
+      );
+      rootMessenger.registerActionHandler(
+        'AuthenticationController:isSignedIn',
+        () => true,
+      );
+      rootMessenger.registerActionHandler(
+        'UserStorageController:performGetStorageAllFeatureEntries',
+        async () => [],
+      );
+      rootMessenger.registerActionHandler(
+        'UserStorageController:performBatchSetStorage',
+        batchSet,
+      );
+    }
+
+    /**
+     * Lets floating remote-push promises settle.
+     */
+    async function flushPromises(): Promise<void> {
+      await new Promise((resolve) => setTimeout(resolve, 0));
+    }
+
+    it('updates an existing autoramp when the id is already known', async () => {
+      await withController(({ controller }) => {
+        controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xabc',
+          status: AutorampStatus.Authorized,
+        });
+
+        const updated = controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xdef',
+          status: AutorampStatus.Approved,
+        });
+
+        expect(controller.state.autoramps).toHaveLength(1);
+        expect(updated.walletAddress).toBe('0xdef');
+        expect(updated.status).toBe(AutorampStatus.Approved);
+      });
+    });
+
+    it('ignores removal and notification for unknown autoramp ids', async () => {
+      await withController(({ controller }) => {
+        controller.removeAutoramp('missing');
+        controller.markAutorampAsNotified('missing');
+
+        expect(controller.state.autoramps).toStrictEqual([]);
+      });
+    });
+
+    it('queues a remote delete when a full sync holds the semaphore', async () => {
+      await withController(({ controller }) => {
+        controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xabc',
+          status: AutorampStatus.Authorized,
+        });
+
+        controller.setIsAutorampSyncingInProgress(true);
+        controller.removeAutoramp('ar-1');
+
+        const pending = controller.getPendingRemoteAutorampDeletes();
+        expect(pending.map((account) => account.id)).toStrictEqual(['ar-1']);
+
+        controller.acknowledgePendingRemoteAutorampDeletes([]);
+        expect(controller.getPendingRemoteAutorampDeletes()).toHaveLength(1);
+
+        controller.acknowledgePendingRemoteAutorampDeletes(pending);
+        expect(controller.getPendingRemoteAutorampDeletes()).toStrictEqual([]);
+
+        controller.setIsAutorampSyncingInProgress(false);
+      });
+    });
+
+    it('suppresses remote pushes while applying sync changes locally', async () => {
+      await withController(async ({ controller, rootMessenger }) => {
+        const batchSet = jest.fn().mockResolvedValue(undefined);
+        registerAutorampSyncHandlers(rootMessenger, batchSet);
+
+        controller.setIsApplyingAutorampSyncChanges(true);
+        controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xabc',
+          status: AutorampStatus.Approved,
+        });
+        controller.markAutorampAsNotified('ar-1');
+        controller.removeAutoramp('ar-1');
+        controller.setIsApplyingAutorampSyncChanges(false);
+
+        await flushPromises();
+
+        expect(batchSet).not.toHaveBeenCalled();
+      });
+    });
+
+    it('swallows remote storage failures raised by autoramp mutations', async () => {
+      await withController(async ({ controller, rootMessenger }) => {
+        const batchSet = jest.fn().mockRejectedValue(new Error('storage down'));
+        registerAutorampSyncHandlers(rootMessenger, batchSet);
+
+        controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xabc',
+          status: AutorampStatus.Authorized,
+        });
+        await flushPromises();
+
+        controller.markAutorampAsNotified('ar-1');
+        await flushPromises();
+
+        controller.applyAutorampStatusFromPush({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          status: AutorampStatus.Approved,
+        });
+        await flushPromises();
+
+        controller.removeAutoramp('ar-1');
+        await flushPromises();
+
+        expect(batchSet).toHaveBeenCalled();
+        expect(controller.state.autoramps).toStrictEqual([]);
+      });
+    });
+
+    it('creates an autoramp from a push that carries no wallet address', async () => {
+      await withController(({ controller }) => {
+        const created = controller.applyAutorampStatusFromPush({
+          id: 'ar-new',
+          customerId: 'cust-1',
+          status: AutorampStatus.Approved,
+        });
+
+        expect(created.walletAddress).toBe('');
+        expect(controller.state.autoramps).toHaveLength(1);
+      });
+    });
+
+    it('keeps local identity fields when a remote push omits or blanks them', async () => {
+      await withController(({ controller }) => {
+        controller.addAutoramp({
+          id: 'ar-1',
+          customerId: 'cust-1',
+          walletAddress: '0xabc',
+          status: AutorampStatus.Authorized,
+        });
+
+        const afterOmitted = controller.applyAutorampStatusFromPush({
+          id: 'ar-1',
+          customerId: '',
+          status: AutorampStatus.Approved,
+        });
+
+        expect(afterOmitted.customerId).toBe('cust-1');
+        expect(afterOmitted.walletAddress).toBe('0xabc');
+
+        const afterBlank = controller.applyAutorampStatusFromPush({
+          id: 'ar-1',
+          customerId: '',
+          walletAddress: '',
+          status: AutorampStatus.Approved,
+        });
+
+        expect(afterBlank.customerId).toBe('cust-1');
+        expect(afterBlank.walletAddress).toBe('0xabc');
+      });
+    });
   });
 
   describe('registerMoneyAccountWallet', () => {