fix(ramps,kyc): fix VBA hydration for foreign and already-consented KYC sessions - #10337
Open
georgeweiler wants to merge 4 commits into
Open
georgeweiler wants to merge 4 commits into
georgeweiler wants to merge 4 commits into
Conversation
…dration A KYC session persisted from a previous identity (e.g. a new wallet created over an existing install) makes the backend reject the session-scoped disclaimer calls with an owner mismatch (surfaced as HTTP 502). `hydrateVbaOnboarding` previously let that rejection propagate, dead-ending the user on the recoverable-error stage. When the session came from persisted controller state, discard it via the new `KycController:clearState` messenger action and restart onboarding at the email step, which recreates a session for the current identity. Errors on a freshly-fetched (current-user) session still propagate as genuine failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
… handling Rework the VBA hydration recovery based on real device testing: - ramps-controller: replace the reactive "discard on any session-scoped error" approach with a proactive ownership check. A persisted session is only discarded when its `externalUserId` does not match the signed-in profile (`AuthenticationController:getSessionProfile`). This avoids wiping a valid, in-progress session when the backend returns an opaque 502 for a transient failure or an already-completed session. Backend-fetched sessions are already user-scoped and are not ownership-checked; if the profile id can't be resolved, the session is kept. - kyc-controller: `hasCompletedSessionDisclaimers` short-circuits to true when the session's `consentStatus` is already `given`. After consents are recorded idOS returns 409 on a re-fetch (surfaced as a 502), which previously failed hydration for a completed session and bounced the user to the email step or the error screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
georgeweiler
changed the title
roz0n
previously approved these changes
Sep 22, 2026
…scard-stale-session Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/kyc-controller/CHANGELOG.md # packages/ramps-controller/CHANGELOG.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b9b222a. Configure here.
| return true; | ||
| } | ||
| return session.externalUserId === canonicalId; | ||
| } |
There was a problem hiding this comment.
Profile lookup throw fails hydration
Medium Severity
#isVbaSessionOwnedByCurrentProfile is documented to keep the session when the current identity cannot be resolved, but AuthenticationController:getSessionProfile throwing is uncaught. That rejection escapes hydrateVbaOnboarding and can still route a valid persisted session to the recoverable-error screen.
Reviewed by Cursor Bugbot for commit b9b222a. Configure here.
4 tasks done
pull Bot
pushed a commit
to Reality2byte/core
that referenced
this pull request
Sep 23, 2026
…en stage (MetaMask#10354) ## Explanation `RampsController:hydrateVbaOnboarding` currently returns a linear `VbaOnboardingStage` and persists it. That encodes product funnel order in Core, so a design change such as putting vendor terms before email requires a controller release. This PR keeps Core responsible for backend onboarding facts and money-account activation while removing screen and route concepts: - `hydrateVbaOnboarding` returns a `VbaOnboardingSnapshot` containing `sessionExists`, disclaimer completion flags, `kycStatus`, `finalStatus`, and `activation`. - The persisted `vbaOnboardingStage` state field and `VbaOnboardingStage` enum are removed. - After `finalStatus === 'approved'`, wallet registration and autoramp creation still run as a coalesced operation. - Activation failures return `activation: 'retryable_failure'` instead of turning hydration into a fatal error. - A persisted KYC session owned by a previous identity is cleared and represented as an empty snapshot. Mobile owns the funnel in MetaMask/metamask-mobile#36522. Its current sequence is MoonPay Terms 1 → Iron email → session Terms 2 → SumSub. Entry and resume points hydrate this snapshot, while successful local actions advance optimistically. ## References - Replaces the stage contract introduced by MetaMask#10278 - Mobile consumer: MetaMask/metamask-mobile#36522 - Overlaps the foreign-session discard in MetaMask#10337; that PR will need to rebase onto this snapshot return type if both land ## Testing - Updated `RampsController` unit coverage for every snapshot branch, including empty, pending, approved, rejected, retryable activation failure, and foreign-session reset states. - Verified the locally built package in MetaMask Mobile through a dist overlay; Mobile resolves `hydrateVbaOnboarding` as `Promise<VbaOnboardingSnapshot>` with no remaining stage API references. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Entering the VBA onboarding flow could dead-end the user on the recoverable-error ("couldn't continue") screen. Root-caused by instrumenting Mobile + the KYC API against a real device; there are two distinct backend conditions behind it, both handled here.
1. Foreign persisted session →
ramps-controllerRampsController.hydrateVbaOnboardingreads the persisted KYC session (KycController:refreshSessionStatus) and calls session-scoped endpoints with it. When that session was created by a previous identity (e.g. a new wallet created over an existing install), itsexternalUserIdno longer matches the caller, so the backend rejects every session-scoped call as an owner mismatch (surfaced as an opaque 502). The rejection propagated and routed the user to the error screen.Fix: verify ownership proactively — compare the persisted session's
externalUserIdto the signed-in profile's canonical id (AuthenticationController:getSessionProfile) before using it. On a mismatch, clear it via the newKycController:clearStateaction and restart at the email step. This is deliberately not reactive error-catching: the API returns the same 502 for owner mismatch, transient relay failures, and already-completed consents, so discarding on any 502 would destroy valid sessions. A backend-fetched session is already user-scoped (not ownership-checked); if the profile id can't be resolved, the session is kept.2. Already-consented session →
kyc-controllerOnce consents are recorded, idOS returns 409 ("already consented") on a re-fetch of the session disclaimers, which the API surfaces as a 502.
hasCompletedSessionDisclaimersthen failed for a completed session, bouncing the user back to email or the error screen (e.g. tapping "check status" on the pending screen, or reopening the app).Fix:
hasCompletedSessionDisclaimersshort-circuits totruewhen the session'sconsentStatusis alreadygiven, skipping the redundant fetch (this was already aTODOshortcut in the code).References
/sessions/:id/disclaimers502 is an intentional owner-mismatch guard, not an idOS outage; the follow-on 502 after completing SumSub is an idOS 409 ("already consented").Changelog
See
packages/ramps-controller/CHANGELOG.mdandpackages/kyc-controller/CHANGELOG.md.Checklist
Note
Medium Risk
Touches VBA/KYC onboarding routing and adds a breaking messenger requirement, but changes are narrow fixes with conservative fallbacks when profile resolution fails.
Overview
Fixes VBA onboarding hydration dead-ends caused by two backend error paths that surface as 502s during
hydrateVbaOnboarding.In kyc-controller,
hasCompletedSessionDisclaimersnow returnstrueimmediately when persisted sessionconsentStatusis alreadygiven, skipping a redundant disclaimer fetch that idOS rejects with 409 after consents are recorded.In ramps-controller, hydration validates ownership of persisted KYC sessions only: it compares the cached session’s
externalUserIdto the signed-in profile fromAuthenticationController:getSessionProfile. On mismatch it callsKycController:clearStateand routes back to email OTP instead of continuing with a stale session from a prior identity. Backend-fetched sessions are not ownership-checked; if the profile id cannot be resolved, the session is kept to avoid discarding valid state on transient failures.Breaking:
RampsControllerMessengermust now delegateKycController:clearState(structural type in ramps, no new package dependency).Reviewed by Cursor Bugbot for commit b9b222a. Bugbot is set up for automated code reviews on this repo. Configure here.