feat(kyc-controller): generalize UKYC vendor APIs and add status polling

[georgeweiler]

Summary

• Parameterizes Universal KYC vendor HTTP (fetchDisclaimers / checkKycRequired / createVendorCustomer) so identity vendors share one client surface instead of Iron-branded public methods (createIronCustomer, fetchIronDisclaimers, etc.).
• Adds a consents-path flow for non-MoonPay vendors (initialize({ vendor: 'iron' }) → empty-shell customer → consents → SumSub, skipping MoonPay Check/Auth), plus refreshKycStatus / statusChanged and getCustomerIdentity.
• This is PR-A of the KYC / Money split (TRAM-3862). It is a from-main alternative to stacking on #9852; that PR should not merge as-is.

Test plan

• yarn workspace @metamask/kyc-controller run test (100% coverage)
• yarn workspace @metamask/kyc-controller run changelog:validate
• yarn workspace @metamask/kyc-controller run messenger-action-types:check
• Confirm MoonPay ramps/card callers still work with default vendor: 'moonpay' (no createIron* on the public surface)
• Confirm Money can drive the consents path via initialize({ vendor: 'iron', product: 'money' }) and refreshKycStatus

Made with Cursor

[cursor]

Cursor Bugbot has reviewed your changes and found 3 potential issues.

^{❌ 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 276fd29. Configure here.}

[georgeweiler]

@cursoragent review this PR

[cursor]

Now I'll provide a comprehensive review of this PR. Let me examine the Bugbot findings and conduct my own analysis.

Code Review: KYC Controller Generic UKYC Vendor APIs

Summary of Changes

This PR generalizes the KYC controller to support multiple identity vendors (currently MoonPay and Iron) by:

• Param
  eterizing vendor APIs (fetchDisclaimers, checkKycRequired, createVendorCustomer)
• Adding a consents-flow path for non-MoonPay vendors
• Adding refreshKycStatus() and statusChanged event
• Adding getCustomerIdentity() method
• Introducing termsAcceptedVendor state to track which vendor's terms were accepted

Analysis of Bugbot Issues

Issue #1: Cross-vendor terms reuse ✅ VALID HIGH SEVERITY

The root cause is a state migration problem. Terms accepted before this PR have termsAcceptedAt and acceptedDisclaimerIds set, but termsAcceptedVendor is null. The fallback logic in #hasTermsForVendor treats null as 'moonpay':

#hasTermsForVendor(vendor: KycVendor): boolean {
  return (this.state.termsAcceptedVendor ?? 'moonpay') === vendor;
}

When users upgrade and call initialize({ vendor: 'moonpay' }), old MoonPay terms are reused without revalidation, even though disclaimers may have changed.

Fix: Clear terms if termsAcceptedVendor is null:

#hasTermsForVendor(vendor: KycVendor): boolean {
  if (this.state.termsAcceptedVendor === null) {
    return false; // Force reacceptance for pre-migration state
  }
  return this.state.termsAcceptedVendor === vendor;
}

Issue #2: Polling restarts after reset ⚠️ PARTIALLY VALID

The concern is that refreshKycStatus() called after reset() can restart polling even though the controller is idle. While the generation guards prevent issues during concurrent operations, the intended behavior after reset() is unclear:

• userStatus is persisted (for Money toast across sessions)
• Calling refreshKycStatus() after reset() will fetch status and restart polling if pending

This seems like a design question rather than a bug. If refreshKycStatus() is meant to be independent of the main KYC flow, current behavior is fine. If not, add a phase check.

Issue #3: Zero-arg session start crash ❌ INVALID

The code uses optional chaining correctly (params?.email, params?.product, params?.sumsubTncSigned, params?.idosTncSigned). No crash will occur with zero args. Test at line 336 confirms this works.

Additional Issues Found

1. Race condition in vendor customer creation (Medium)

In initialize() lines 713-729, after creating a vendor customer for consents-flow vendors, the code checks generation before continuing. However, if reset() is called during the customer creation, the generation check at line 719 returns early, but the next check at line 723 also checks generation before calling #fail().

The issue: if customer creation fails AND reset happens during the failure, the error won't be recorded. While not critical (reset clears error anyway), it's inconsistent with the pattern elsewhere.

2. Hardcoded T&C flags in initialize() (Low)

Lines 737-740:

await this.#startConsentsSession({
  sumsubTncSigned: true,
  idosTncSigned: true,
});

When initialize() auto-starts a consents session (because terms were already accepted), it hardcodes both T&C2 flags to true. This assumes the previously-persisted acceptedDisclaimerIds imply Sumsub/idOS consent, but those flags weren't captured during the original acceptance. This could cause issues if the vendor's T&C2 requirements change.

Recommendation: Persist sumsubTncSigned and idosTncSigned alongside acceptedDisclaimerIds, or require re-acceptance when these fields aren't available.

3. Inconsistent vendor field naming (Low)

KycService.createUkycSession() uses vendorId (line 713) while other methods use vendor. Consider standardizing to vendor throughout for consistency.

Positive Aspects

✅ Excellent generation-based async guard pattern prevents state corruption
✅ Comprehensive test coverage (100%)
✅ Clear separation between MoonPay and consents flows
✅ Good documentation in comments and changelog
✅ Optional fetch parameter is a nice DX improvement

Recommendations

Must Fix (before merge):

1. Fix Issue Mock endpoints during tests #1 (cross-vendor terms reuse) by treating null vendor as invalid

Should Fix:
2. Clarify/document behavior of refreshKycStatus() after reset(), or add phase guard
3. Persist/validate T&C2 flags for consents flow resumption

Nice to Have:
4. Standardize vendor vs vendorId parameter naming
5. Add integration test covering vendor-switching scenarios

Test Plan Verification

• ✅ Unit tests pass with 100% coverage
• ⚠️ Manual testing needed: "Confirm MoonPay ramps/card callers still work" - critical for production
• ⚠️ Manual testing needed: "Confirm Money can drive consents path" - blocks Money feature

Would you like me to implement fixes for the identified issues?

  

[Copilot]

Pull request overview

This PR expands @metamask/kyc-controller to support multiple UKYC identity vendors (MoonPay + Iron) behind a unified service surface, adds a non-MoonPay “consents path” flow, and introduces user-keyed KYC status polling for Money UI surfaces.

Changes:

• Generalize vendor HTTP endpoints in KycService (vendor-parameterized disclaimers/KYC-required plus new customer/consents/status endpoints).
• Add non-MoonPay (iron) controller flow (customer → disclaimers → consents → SumSub) and expose refreshKycStatus + statusChanged + getCustomerIdentity.
• Extend shared types/docs/changelog for money, iron, and simplified /kyc/status user status.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/kyc-controller/src/types.ts	Adds money product, iron vendor, customer identity type, and simplified user-status types.
packages/kyc-controller/src/KycService.ts	Vendor-parameterized endpoints, optional injected fetch, consents + status APIs, improved HttpError detail.
packages/kyc-controller/src/KycService.test.ts	Adds coverage for new vendor APIs, 204 handling, error-detail parsing, and optional fetch behavior.
packages/kyc-controller/src/KycService-method-action-types.ts	Exposes new service methods via messenger action types.
packages/kyc-controller/src/KycController.ts	Implements consents-path vendor flow, vendor-scoped terms acceptance, status polling + events, identity accessor.
packages/kyc-controller/src/KycController.test.ts	Adds extensive tests for iron flow, vendor-scoped terms behavior, status polling, and identity behavior.
packages/kyc-controller/src/KycController-method-action-types.ts	Exposes new controller methods via messenger action types.
packages/kyc-controller/src/index.ts	Exports new public types and action types.
packages/kyc-controller/CHANGELOG.md	Documents new vendor-generalization, iron flow, and status polling additions.
packages/kyc-controller/ARCHITECTURE.md	Updates architecture docs to reflect vendor parameterization and consents-path flow.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.