Repository navigation
fix(kyc): route Mexico bank enrollment through the NA intent (TASK-22333) - #3036
Conversation
…333) Mexico is tagged region 'latam' for the region picker, but its bank rail (SPEI) is a Bridge rail. Every bank-flow unlock CTA derived the KYC intent from that picker region and sent LATAM + targetCountry=MX; the BE rejects MX for the Manteca path and the UI collapsed the typed rejection into the 'contact support' dead-end. 8 approved Mexico users have no SPEI rail. Add getBankRegionIntent (MX -> NA, else the region intent) and route all six bank-flow call sites through it — add-money bank, withdraw bank, the countries list and the bank claim — so the fix lives in one place instead of the one page the report named (regression of hotfix #2163 coverage). The region picker keeps getRegionIntent: it works off the clicked region, not a country.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR adds country-aware bank KYC intent resolution. Bank flows now use rail jurisdiction data, so Mexico routes to ChangesBank KYC routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Mexico bank enrollment now uses the North America KYC intent while remaining in the LATAM picker category. Saved-account claims select a resolved account country without replacing an existing selection when metadata is unavailable; the covered bank flows are ready to merge. Sequence Diagram(s)sequenceDiagram
participant SavedAccountsView
participant BankFlowManager
participant getCountryFromAccount
participant ClaimBankFlow
participant handleInitiateKyc
SavedAccountsView->>BankFlowManager: select saved account
BankFlowManager->>getCountryFromAccount: resolve account country
getCountryFromAccount-->>BankFlowManager: country or no result
BankFlowManager->>ClaimBankFlow: setSelectedCountry(country)
BankFlowManager->>handleInitiateKyc: initiate with bank intent
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 7762.74 → 7763.96 (+1.22) 🆕 New findings (21)
…and 1 more. ✅ Resolved (21)
…and 1 more. |
Reuse the existing 'Bridge serves this country' predicate instead of a fifth hard-coded MX literal — the rule is 'latam picker country whose bank rail is Bridge', and the predicate is already the withdraw page's routing guard.
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Mexico bank-flow intent exception is correctly scoped and matches the backend Bridge enrollment contract; no actionable defects found.
Checked clean
- All six bank-flow call sites now use getBankRegionIntent, while the region-picker flow correctly retains getRegionIntent.
- Country-catalog blast-radius analysis confirms Mexico is the only current LATAM country whose intent changes through the Bridge-supported-country predicate.
- The sibling API contract maps NA to bridge-requirements and enrolls ACH_US, SEPA_EU, SPEI_MX, and FASTER_PAYMENTS_GB; the frontend also drops MX from the Manteca-only targetCountry field.
- The live Lexicon does not define Mexico/SPEI routing; canonical product country and network sources confirm Mexico SPEI deposits and withdrawals use Bridge.
- All completed exact-head CI checks are green, including unit, typecheck, eslint, format, e2e, CodeQL, and deploy preview.
- Focused local Jest execution was unavailable because the detached worktree has no Jest binary; exact-head unit CI passed.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 0a596dcc4dba · Context: repo, product · Took 8m
…ver the saved-account claim path Review fixes (/code-review medium on #3036): - getBankRegionIntent now reads RAIL_COUNTRY_TO_REGION_PATH — the table the pending-rail badges already use — instead of a Bridge-membership predicate that would flip AR/BR/CO to NA the day BRA/COL land in the Bridge alpha3 map. One source of truth for 'which region tile does this rail belong to'. - BankFlowManager: only the country list set selectedCountry, so a saved account (CLABE) reached the unlock CTA with a rest-of-world intent. Set it from the account — the account is the destination. - withdraw page: one country lookup for intent and target country. - page test asserts intent + crossRegion only; the country arg is dropped by useSumsubKycFlow for non-Manteca countries, so it must not be pinned.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The Mexico bank-flow intent fix is coherent across all six call sites, the saved-account claim path now supplies its destination country, and no reachable regression was found.
Findings
- MAJOR · src/components/Claim/Link/views/BankFlowManager.view.tsx:529 · [claude-opus] Saved-account claim path mutates shared flow state with no test
setSelectedCountry(getCountryFromAccount(account) ?? null)was added to the saved-accounts click handler in the bank claim flow — a money-moving path (it ends in a Bridge offramp) that also writes shared state on ClaimBankFlowContext, which the unlock CTA (line 625), the DynamicBankAccountForm key/country props (lines 566-568) and the guest-claim country code (line 410) all read. No test exercises it: there is no test file for BankFlowManager anywhere (grep -rln BankFlowManager src --include=*.test.tsxis empty), and claim-states.test.tsx stubs ClaimBankFlowContext to{ setFlowStep, flowStep: null }so it never sees selectedCountry. The commit message for c58b73c claims this PR covers 'the saved-account claim path', but the two tests added cover regions.utils and the add-money page only.
Exact untested case, and the reason it matters: getCountryFromAccount returning undefined is a known production state — withdraw-states.test.tsx GROUP 6 documents it as a Sentry regression (6 users/14d) for an account with empty countryName/countryCode. On the reachable path SavedAccountsList -> 'select new method' -> BankCountryList (sets selectedCountry, e.g. DE) -> BankDetailsForm -> back -> SavedAccountsList -> click a saved account whose country cannot be resolved, the new line overwrites a good DE with null, so the unlock CTA now sends ROW instead of EU; the BE answers unsupported-region (initiate-kyc.ts:419) and the user gets a terminal 'not available in your region' where they previously got the Bridge uplift.
Fix: add a BankFlowManager (or a ClaimBankFlowContext-level) test asserting (a) clicking a saved MX CLABE account sets selectedCountry to the MX CountryData and the unlock CTA calls handleInitiateKyc with 'NA', and (b) an account whose country cannot be resolved does not clobber an already-selected country — i.e. guard the write with const resolved = getCountryFromAccount(account); if (resolved) setSelectedCountry(resolved).
Checked clean
- Verified the detached checkout, supplied merge base, trusted author, and main target at the exact requested head.
- Reviewed all eight changed files and traced each of the six bank-flow KYC call sites through the shared intent helper.
- Traced MX -> NA through useSumsubKycFlow target-country filtering and the current backend Bridge intent and rail-enrollment contracts.
- Checked saved-account country derivation and the transition into bank-claim confirmation; canonical MX/MEX account metadata resolves to Mexico before the unlock modal.
- Exact-head unit, e2e, typecheck, eslint, format, CodeQL, and required aggregate CI checks passed; the Deploy Preview check was still in progress.
- Ran correctness, security, adversarial, and slop passes; no exact failure scenario survived the existing guards and tests.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: c58b73c453a0 · Context: repo, api sibling · Took 8m
… picked country Third-reviewer finding on #3036: setSelectedCountry(getCountryFromAccount(account) ?? null) overwrote a country the user already chose when the saved account's metadata has no resolvable country (empty countryCode/countryName — a known prod state), turning an EU uplift into a rest-of-world dead end. Guard the write, and add the first BankFlowManager test: saved MX CLABE → selectedCountry = Mexico; unresolvable account → no write; unlock CTA → NA intent.
|
/chip review |
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the supplied head. Mexico bank enrollment now consistently selects the Bridge/NA intent, and the saved-account claim mutation is covered by focused regression tests.
Findings
- MINOR · src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx:606 · [claude-opus] Withdraw bank unlock CTA has no test for the Mexico NA intent
The same TASK-22333 fix was applied to three call sites, but only two got page-level coverage. src/app/(mobile-ui)/add-money/[country]/bank/page.tsx got 'mexico needs-enrollment unlock sends the NA intent, not LATAM', and the claim path got BankFlowManager.savedAccount.test.tsx. The withdraw bank page's unlock CTA (line 606) is the money-out direction of the identical bug and has no test: src/app/(mobile-ui)/withdraw/tests/withdraw-states.test.tsx never renders this page's KYC path (no match for handleInitiateKyc, needs-enrollment, or regions.utils in it).
This is not a pure rename. handleInitiateKyc writes regionIntent onto the user's Sumsub verification record, which peanut-api-ts freezes at approval and uses to drive rail auto-enrollment (src/kyc/rails.ts autoEnrollUserRails via REGION_RAIL_MAP) — shared state that determines which bank rails the user ends up with. For /withdraw/mexico/bank the value sent changes from LATAM to NA.
Risk is low because the mapping itself is unit-tested in regions.utils.test.ts and only the one-line wiring is uncovered — hence minor, not major.
Fix: add a case to withdraw-states.test.tsx mirroring the add-money one — render the bank page with country 'mexico' and a needs-enrollment gate, click through to the KYC modal's verify button, and assert handleInitiateKyc receives ['NA', undefined, true].
Checked clean
- P1 is fixed: the new saved-account suite directly checks the selected-country mutation, the unresolved-country guard, and the Mexico-to-NA unlock path.
- All six bank-flow initiation sites now use the rail-jurisdiction helper; Mexico changes from LATAM to NA while other supported countries retain their previous intent.
- The frontend drops Mexico from targetCountry but preserves the NA intent, matching the sibling API's Bridge-intent enrollment path and all-four-Bridge-rails contract.
- Product truth confirms Mexico SPEI is a live Bridge bank rail and the Bridge cohort receives SPEI alongside the other Bridge rails; the Lexicon has no conflicting route definition.
- Correctness, security, adversarial, and slop passes found no additional reachable defect in the changed lines.
- Exact-head unit, E2E, typecheck, eslint, format, CodeQL, and analyze checks completed successfully; the detached worktree has no local Jest binary, so no duplicate local run was possible.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: e7501719b894 · Context: repo, product, sibling-api · Took 16m
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
Clean at the supplied head. Mexico bank enrollment now consistently selects the Bridge/NA intent, and the saved-account claim mutation is covered by focused regression tests.
Checked clean
- P1 is fixed: the saved-account suite directly checks the selected-country mutation, the unresolved-country guard, and the Mexico-to-NA unlock path.
- All six bank-flow initiation sites use the rail-jurisdiction helper; Mexico changes from LATAM to NA while other supported countries retain their prior intent.
- The frontend drops Mexico from targetCountry but preserves the NA intent, matching the sibling API's Bridge workflow and four-rail enrollment contract.
- Product truth confirms that Mexico SPEI is a live Bridge bank rail; the Lexicon contains no conflicting route definition.
- Correctness, security, adversarial, and slop passes found no additional reachable defect in the changed behavior.
- All exact-head checks completed successfully, including unit, E2E, typecheck, eslint, format, CodeQL, analyze, and deployment preview; the detached worktree has no local Jest binary.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: e7501719b894 · Context: repo, product, sibling-api · Took 7m
Summary
A newly approved Mexico user who taps "Add money → bank" reaches "contact support" instead of the SPEI deposit flow. No bank customer, SPEI/MXN rail, intent or entry is created. 8 approved Mexico users have no SPEI rail today (1 approved in the last 7 days). $0 moved or lost — the flow is blocked before any transfer.
Root cause. Mexico is
region: 'latam'in the country table (the region picker bucket), but its bank rail (SPEI) is a Bridge rail. Every bank-flow unlock CTA derived the KYC intent from that picker region viagetRegionIntent(country.region)and sentLATAM + targetCountry=MX.initiate-kyc.tsintentionally rejects MX for the Manteca (LATAM) path, and the UI collapsed that typed rejection into the support dead-end. This is a coverage regression of hotfix #2163, which fixed the'STANDARD'literal at the same call sites but kept region as the source of the intent.Fix. One helper,
getBankRegionIntent(country)inregions.utils.ts: the intent comes from the rail-jurisdiction tableRAIL_COUNTRY_TO_REGION_PATH(the one the pending-rail badges already use —MX → north-america), falling back to the picker region for countries with no rail entry. Output is identical to before for every country except Mexico. All six bank-flow call sites route through it (add-money bank page, withdraw bank page,AddWithdrawCountriesList×2,BankFlowManager×2) — the task names one page, but the same derivation is copied at every bank flow, so fixing only the reported one would have left withdraw / claim-to-bank / countries-list still dead for Mexico.UnlockedRegionskeepsgetRegionIntent: it works off the clicked picker region, not a country.The bank-claim flow had a second gap on the same line: only the country list sets
selectedCountry, so a saved account (CLABE) reached the unlock CTA with no country and a rest-of-world intent.onAccountClicknow sets it from the account (getCountryFromAccount) — the account is the destination. The write is guarded: an account whose country cannot be resolved (emptycountryCode/countryName, a known prod state) leaves an already-picked country alone instead of resetting it to rest-of-world.On the BE,
NAis a Bridge intent:isBridgeIntent('NA')→ bridge-requirements level, andautoEnrollUserRails(userId, 'STANDARD')enrollsACH_US, SEPA_EU, SPEI_MX, FASTER_PAYMENTS_GB, so the SPEI rail is created after enrollment. ThetargetCountryarg is dropped byuseSumsubKycFlowfor non-Manteca countries before the request, so the BE never sees MX — no BE change needed.Cohort check (prod, read-only, 2026-09-08). Exactly 8 approved
sumsub_geo = MXusers have noSPEI_MXrail. Stored region intent: 1 × LATAM (the 09-06 report), 7 × null; none NA, none holds any Bridge rail — so every one of them takes the cross-region / Bridge-enrollment-recovery path with the new intent. The "stored NA + live Bridge rail + no SPEI" state that would loop silently does not exist in the cohort.Task
TASK-22333 — https://app.notion.com/p/3d3838117579811c8861ea26ee1b0b95
Risks / breaking changes
main(P2 production issue, user-blocking) → creates main→dev back-merge debt.country.id === 'MX'changes behaviour (LATAM → NA). Every other country resolves exactly as before (covered by the helper's unit test).NAvsEUfor Mexico is a wash on the BE (both → bridge-requirements + the same Bridge rail set);NAis what the US bank flow sends and what the task specifies.Design notes / accepted trade-offs
regiontonorth-americain the country table?regionis the picker bucket (Mexico sits under the LATAM tile in the UI and inderiveRegionAccess), so the row stayslatam; the bank intent is a rail fact and now comes from the rail table.initiate-kyc.tsstill routes onregionIntent === 'LATAM'alone and would mis-route an older client that sends LATAM for a Mexico user; a boundary coercion there needs the FE to stop stripping non-Manteca countries first. Filed as a follow-up in the readiness report.regions.utils.test.ts, the add-money page and the bank claim have page-level tests, and the withdraw wiring is the same one-line helper call; a 40-mock harness for that one assertion is not worth it.handleInitiateKyc(intent, undefined, needs-enrollment, country.id)call; ahandleInitiateBankKyc(country, gate)would collapse them. Out of this PR's blast radius.QA
regions.utils.test.ts:getBankRegionIntentMX → NA; AR/US/DE/ROW/unknown/undefined unchanged.add-money-states.test.tsxGROUP 5: Mexico +needs-enrollmentgate → Continue → Verify callshandleInitiateKycwith('NA', undefined, true, …). The country fixture now carriesregion: 'latam'so the test reproduces the real routing (pre-fix it sendsLATAM).BankFlowManager.savedAccount.test.tsx(new, first test for that view): saved MX CLABE account →selectedCountry= Mexico; an account with no resolvable country does not clobber an already-picked country; unlock CTA from the saved account →handleInitiateKyc('NA', undefined, true, …).AddWithdrawCountriesList.test.tsxmock updated to the new helper name./add-money/mexico/bank→ Continue → KYC modal → Verify → Sumsub opens onbridge-requirements, and after completionuser_railsgainsSPEI_MX.Screenshots: N/A (no visible change — the same modal opens; only the intent it sends differs).
Summary by CodeRabbit