Skip to content

Show Bridge EEA uplift remediation CTA#2080

Merged
kushagrasarathe merged 7 commits into
feat/bridge-rfi-remediation-ui-cleanfrom
feat/bridge-eea-uplift-ui
May 25, 2026
Merged

Show Bridge EEA uplift remediation CTA#2080
kushagrasarathe merged 7 commits into
feat/bridge-rfi-remediation-ui-cleanfrom
feat/bridge-eea-uplift-ui

Conversation

@kushagrasarathe

Copy link
Copy Markdown
Contributor

Summary

  • show EEA uplift as a fixable Bridge remediation requirement
  • use the existing Sumsub remediation path for the applicant action
  • add coverage for approved Bridge users with EEA uplift metadata

Risks

  • UI depends on API metadata carrying questionnaireCluster = eea_uplift
  • post-submit state relies on the existing provider remediation waiting state

QA Guidelines

  • verify affected approved Bridge users see “Provide required details”
  • verify the CTA opens the existing Sumsub remediation flow
  • verify users move to the waiting/provider-review state after completing the action

@vercel

vercel Bot commented May 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
peanut-wallet Ready Ready Preview, Comment May 25, 2026 8:38am

Request Review

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds EEA uplift handling by tagging remediation actions with questionnaireCluster, returning modal/action copy from bridge gate, exposing selectors to read that copy, wiring it into InitiateKycModal across UI, and adding tests for gate and provider-rejection flows.

Changes

EEA Uplift Remediation Support + UI wiring

Layer / File(s) Summary
Provider action type and EEA-specific helpers
src/hooks/useProviderRejectionStatus.ts
Adds questionnaireCluster to ProviderRemediationAction. getActionLabel/title/modal helpers now accept the full action and special-case eea_uplift/eea_tin_reupload, returning EEA-specific labels/messages.
Bridge gate shape and selectors
src/hooks/useBridgeTransferReadiness.ts
BridgeGateAction.fixable_rejection now includes modalTitle and actionLabel; gate creation populates these from bridge rejection. New exported selectors getGateProviderTitle and getGateActionLabel extract that copy.
InitiateKycModal props and copy
src/components/Kyc/InitiateKycModal.tsx
Adds optional providerTitle and providerActionLabel props and uses them to override provider-rejection modal title and CTA text (while preserving loading text).
Wire selectors into pages and lists
src/app/.../add-money/..., src/app/.../withdraw/..., src/components/AddWithdraw/AddWithdrawCountriesList.tsx, src/components/Claim/Link/views/BankFlowManager.view.tsx
Import getGateProviderTitle/getGateActionLabel and pass providerTitle/providerActionLabel into InitiateKycModal alongside the existing providerMessage prop.
Tests: bridge gate and provider-rejection
src/hooks/__tests__/useBridgeTransferReadiness.test.ts, src/hooks/__tests__/useProviderRejectionStatus.test.ts
Extend/use tests to cover modalTitle/actionLabel in fixable gate cases, add selectors tests, and add a provider-rejection test for the EEA uplift flow verifying state, required action, label/handler, and user message.
Self-heal response & Sumsub flow state
src/app/actions/sumsub.ts, src/hooks/useSumsubKycFlow.ts
Add questionnaireCluster/questionnaireVariant to self-heal response typing; track and expose lastSelfHealQuestionnaireCluster in Sumsub flow state and update it on token refresh/resubmission.
Multi-phase action-flow completion
src/hooks/useMultiPhaseKycFlow.ts
Add detection helper for next Bridge questionnaire cluster and implement handleActionFlowComplete to poll fetchUser() and auto-trigger BRIDGE self-heal resubmission for eea_tin_reupload when appropriate.
KYC modal checking phase
src/components/Kyc/KycVerificationInProgressModal.tsx
Add a 'checking' modal phase rendering a non-closable "Checking your details..." ActionModal.
Interfaces update
src/interfaces/interfaces.ts
Remove 'checking' from KycModalPhase union to align the exported type with usages.

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • peanutprotocol/peanut-ui#1943: Main PR’s changes to useProviderRejectionStatus populate fixable_rejection’s modalTitle/actionLabel based on EEA uplift vs EEA TIN reupload, which directly feeds into #1943’s unified useBridgeTransferReadiness/InitiateKycModal gate-driven rejection handling.
  • peanutprotocol/peanut-ui#1952: The main PR’s EEA-specific provider rejection/action-labeling changes are part of (and build on) the same bridge KYC “gate” wiring to InitiateKycModal that the retrieved cherry-pick introduces across the bank-flow pages/components.
  • peanutprotocol/peanut-ui#1905: Both PRs touch the provider-rejection/self-heal UI logic around useProviderRejectionStatus—main PR extends its rejection/action derivation and labeling (e.g., EEA uplift/TIN re-upload) while retrieved PR introduces the core provider rejection state + UI/resubmission behavior—so the changes are directly related at the hook/action-message level.

Suggested reviewers:

  • Hugo0
  • jjramirezn
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Show Bridge EEA uplift remediation CTA' directly reflects the main objective: displaying EEA uplift as a fixable Bridge remediation requirement with a CTA in the UI.
Description check ✅ Passed The description provides a clear summary of the PR's objectives, acknowledges risks, and includes QA guidelines—all directly related to the changeset of adding EEA uplift remediation UI support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the enhancement New feature or request label May 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/hooks/useMultiPhaseKycFlow.ts (1)

252-252: 💤 Low value

Verify dependency array for handleSdkComplete.

The dependency array includes handleActionFlowComplete which itself depends on fetchUser, handleSelfHealResubmit, lastSelfHealQuestionnaireCluster, and startTracking. React 19 should handle this correctly, but verify that the closure captures the latest values. If stale closures become an issue, consider adding those dependencies directly or using refs for stable function identity.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/useMultiPhaseKycFlow.ts` at line 252, The dependency array for the
effect using originalHandleSdkComplete currently includes
handleActionFlowComplete, but since handleActionFlowComplete closes over
fetchUser, handleSelfHealResubmit, lastSelfHealQuestionnaireCluster, and
startTracking you should ensure the effect sees up-to-date values: either expand
the dependency array to include those underlying symbols (fetchUser,
handleSelfHealResubmit, lastSelfHealQuestionnaireCluster, startTracking) along
with originalHandleSdkComplete, isActionFlow, and regionIntent, or make
handleActionFlowComplete stable (e.g., wrap it in useCallback or store it in a
ref) so the effect's dependency can safely reference the stable function
identity used by handleSdkComplete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hooks/useMultiPhaseKycFlow.ts`:
- Around line 219-235: The polling loop calls fetchUser() without handling
rejections; wrap the await fetchUser() inside a try-catch in
useMultiPhaseKycFlow so network or parsing errors don't throw the whole flow. On
catch, log the error (or report via existing logger), increment a local retry
counter or let the loop continue after awaiting
BRIDGE_ACTION_AUTO_CONTINUE_INTERVAL_MS, and if retries exceed a small threshold
(or elapsed >= BRIDGE_ACTION_AUTO_CONTINUE_TIMEOUT_MS) fall through to the same
"preparing" behavior used elsewhere (e.g., clear modal via
setForceShowModal(false) or break so getBridgeNextQuestionnaireClusterFromUser
and handleSelfHealResubmit logic can proceed safely). Ensure you reference the
existing symbols fetchUser, getBridgeNextQuestionnaireClusterFromUser,
handleSelfHealResubmit, setForceShowModal,
BRIDGE_ACTION_AUTO_CONTINUE_INTERVAL_MS and
BRIDGE_ACTION_AUTO_CONTINUE_TIMEOUT_MS when applying the change.
- Around line 209-240: handleActionFlowComplete currently breaks to the
'preparing' phase silently when handleSelfHealResubmit('BRIDGE') returns false
and also breaks immediately when seeing nextCluster === 'eea_tin_reupload' while
auto-continue is blocked; update handleActionFlowComplete to (1) surface
failures from handleSelfHealResubmit by either setting an error state (e.g.,
setSelfHealError or reuse existing modal state) or at minimum logging the
returned failure so the UI can inform the user before proceeding to
setModalPhase('preparing')/setForceShowModal(true), and (2) when nextCluster ===
'eea_tin_reupload' but canAutoContinueTin is false, await one
BRIDGE_ACTION_AUTO_CONTINUE_INTERVAL_MS (use
wait(BRIDGE_ACTION_AUTO_CONTINUE_INTERVAL_MS)) before breaking to allow API
metadata to settle. Ensure references to handleSelfHealResubmit,
handleActionFlowComplete, setModalPhase, setForceShowModal, startTracking and
the polling logic remain consistent.

---

Nitpick comments:
In `@src/hooks/useMultiPhaseKycFlow.ts`:
- Line 252: The dependency array for the effect using originalHandleSdkComplete
currently includes handleActionFlowComplete, but since handleActionFlowComplete
closes over fetchUser, handleSelfHealResubmit, lastSelfHealQuestionnaireCluster,
and startTracking you should ensure the effect sees up-to-date values: either
expand the dependency array to include those underlying symbols (fetchUser,
handleSelfHealResubmit, lastSelfHealQuestionnaireCluster, startTracking) along
with originalHandleSdkComplete, isActionFlow, and regionIntent, or make
handleActionFlowComplete stable (e.g., wrap it in useCallback or store it in a
ref) so the effect's dependency can safely reference the stable function
identity used by handleSdkComplete.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a2a0b488-fb01-4ec7-8169-9dce8f44c837

📥 Commits

Reviewing files that changed from the base of the PR and between b683922 and b67ef97.

📒 Files selected for processing (8)
  • src/app/actions/sumsub.ts
  • src/components/Kyc/KycVerificationInProgressModal.tsx
  • src/hooks/__tests__/useMultiPhaseKycFlow.test.ts
  • src/hooks/__tests__/useProviderRejectionStatus.test.ts
  • src/hooks/useMultiPhaseKycFlow.ts
  • src/hooks/useProviderRejectionStatus.ts
  • src/hooks/useSumsubKycFlow.ts
  • src/interfaces/interfaces.ts

Comment thread src/hooks/useMultiPhaseKycFlow.ts
Comment thread src/hooks/useMultiPhaseKycFlow.ts Outdated
@kushagrasarathe

Copy link
Copy Markdown
Contributor Author

@coderabbitai review in depth.

Evaluate:

  • risk of introducing new bugs and breaking existing functionality
  • performance impact and latency
  • code quality and DRY

Also, write a very concise testing guide for QA

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@kushagrasarathe: Here is the in-depth review.


Risk of New Bugs / Breaking Existing Functionality

🔴 High: Double fetchUser() on self-heal action complete

When a self-heal action SDK completes, two concurrent fetchUser() calls fire immediately:

  1. useSumsubKycFlow.handleSdkComplete – detects selfHealProviderRef.current is set, calls void fetchUser(), and returns early.
  2. useMultiPhaseKycFlow.handleSdkComplete – sees isActionFlow === true, calls void handleActionFlowComplete(), which starts its polling loop with elapsed = 0 and immediately calls fetchUser() again.

These two calls race. The second one is redundant. In practice the results should be the same (idempotent GET), but it's still unnecessary load and can cause subtle race conditions if the server response differs slightly between the two calls.

Fix: Guard the polling loop's first call or have useSumsubKycFlow.handleSdkComplete skip its own fetchUser() when it knows an action-flow continuation is coming.


🟡 Medium: Polling loop executes one extra iteration

// BRIDGE_ACTION_AUTO_CONTINUE_TIMEOUT_MS = 20000, INTERVAL = 2000
for (
    let elapsed = 0;
    elapsed <= BRIDGE_ACTION_AUTO_CONTINUE_TIMEOUT_MS;  // <= not <
    elapsed += BRIDGE_ACTION_AUTO_CONTINUE_INTERVAL_MS
)

With elapsed starting at 0 and ending at 20000 inclusive, this runs 11 iterations instead of the intended 10. The last iteration (elapsed=20000) skips the wait() but still executes fetchUser(). The off-by-one is benign today but worth fixing for correctness.


🟡 Medium: No unmount cleanup in the async polling loop

handleActionFlowComplete is a 20-second async loop with no abort signal or mounted-flag guard. If the component unmounts mid-poll (e.g. user navigates away), React 18 will silently ignore the setModalPhase/setForceShowModal calls, but fetchUser() and handleSelfHealResubmit() will continue firing in the background, consuming resources and potentially triggering unintended side effects.

Fix: Use a ref-based isCancelledRef or an AbortController to exit the loop early:

const cancelled = { current: false }
// at the top of handleActionFlowComplete

// inside the loop:
if (cancelled.current) return

// cleanup (e.g. in a useEffect or via a returned cancel function):
return () => { cancelled.current = true }

🟡 Medium: canAutoContinueTin guard relies on stale closure – fragile but currently correct

handleActionFlowComplete captures lastSelfHealQuestionnaireCluster at the time the useCallback was last created. The dependency array includes it, so after handleSelfHealResubmit sets the state to 'eea_tin_reupload', React re-creates the callback before the next SDK-complete event. This works because the user must interact with Sumsub between the two SDK sessions (giving React time to re-render). However, the correctness is implicit and could silently break if the execution model changes. A comment explaining this invariant would help.


🟢 No regression: 'checking' phase in KycModalPhase

The AI summary incorrectly stated that 'checking' was removed. The actual interfaces.ts line 6 confirms it is present in the union, and KycVerificationInProgressModal correctly renders it. This is consistent throughout. ✓


Performance Impact / Latency

Path Impact
Happy path (EEA uplift → TIN reupload detected on first poll) +1 fetchUser call (~200–500ms added latency), acceptable
Worst case (no cluster returned for 20s) 11 fetchUser calls over 20 seconds while user sees "Checking your details…"
Self-heal action complete (any case) 2 concurrent fetchUser calls fired simultaneously
All other paths No change from pre-PR behavior

The 20-second timeout with a non-dismissible "Checking your details..." modal (no CTA, no close button) is the biggest UX risk. If the API is slow or the cluster never arrives, the user is stuck for 20 seconds with no feedback or escape. Consider adding a fallback CTA ("Skip for now") after a shorter interval, similar to the preparingTimedOut pattern already in the codebase.


Code Quality & DRY

1. Duplicated questionnaireCluster literal union

The same string literal union is defined separately in two files:

  • src/hooks/useProviderRejectionStatus.tsProviderRemediationAction.questionnaireCluster
  • src/app/actions/sumsub.tsSelfHealResubmissionResponse.questionnaireCluster

Adding a new cluster (e.g. 'source_of_address') requires updating both. Extract to a shared type:

// src/interfaces/interfaces.ts or a shared types file
export type BridgeQuestionnaireCluster =
    | 'source_of_funds'
    | 'tax_id'
    | 'address'
    | 'birth_details'
    | 'eea_uplift'
    | 'eea_tin_reupload'

2. Duplicated bridge remediation lookup logic

getBridgeRemediationFromUser in useMultiPhaseKycFlow.ts and getBridgeRemediation in useProviderRejectionStatus.ts both implement the same priority logic (verification metadata wins over rail metadata). The multi-phase version additionally lacks the status validation that the rejection status version performs:

// In getBridgeRemediation (useProviderRejectionStatus.ts) — validated ✓
if (candidate.status !== 'APPROVED' && candidate.status !== 'AWAITING_INPUT' && ...)

// In getBridgeRemediationFromUser (useMultiPhaseKycFlow.ts) — not validated ✗
return isRecord(verificationRemediation) ? verificationRemediation : ...

This means getBridgeNextQuestionnaireClusterFromUser could theoretically return a cluster for an APPROVED remediation. Consider exporting a shared lookup helper.

3. Inconsistent eea_* check pattern

eea_tin_reupload has a named helper isEeaTinReuploadAction(), but eea_uplift is always checked inline:

// inconsistent
if (action?.questionnaireCluster === 'eea_uplift') return 'Provide required details'  // inline
if (isEeaTinReuploadAction(action)) return 'Re-upload TIN'                             // helper

Add isEeaUpliftAction() for symmetry.

4. Double-numbered step comment

In useBridgeTransferReadiness.ts, both "needs_enrollment" and "ready" are labeled // 5. in the gate priority comment block. This is cosmetic but should be // 5. and // 6. respectively.


QA Testing Guide

Prerequisites

  • A Bridge-approved user account with AWAITING_INPUT remediation metadata carrying questionnaireCluster: 'eea_uplift'
  • A staging environment where the API returns EEA uplift metadata

Test Matrix

# Scenario Expected Result
1 Affected Bridge-approved user opens Add Money or Withdraw InitiateKycModal appears with title "We need more details" (or custom providerTitle) and CTA "Provide required details"
2 User clicks "Provide required details" Sumsub SDK opens (applicant action flow); isActionFlow = true
3 User completes Sumsub action "Checking your details…" modal appears; no close button visible
4 API returns eea_tin_reupload as next cluster within 20s Sumsub SDK re-opens automatically with TIN reupload action; "Checking" modal disappears
5 User completes TIN reupload "Checking your details…" reappears; API returns no more clusters; transitions to "Setting up your account" (preparing) → "All set!"
6 API never returns eea_tin_reupload within 20s After 20s, modal transitions from "Checking" to "Setting up your account" and startTracking begins
7 Affected user who has already done TIN reupload (second time) canAutoContinueTin = false; loop exits on finding eea_tin_reupload; goes directly to preparing/tracking
8 User with blocked remediation (TERMINAL) InitiateKycModal shows "Payment setup issue" with "Contact support" CTA; no Sumsub action offered
9 User with AWAITING_PROVIDER remediation Modal shows "Document review in progress" with "Got it" CTA
10 Non-EEA Bridge user with standard BRIDGE_CUSTOMER_FIELDS rejection Existing "Provide details" label and copy unchanged; no regression
11 Countries list page (AddWithdrawCountriesList) providerTitle and providerActionLabel props correctly propagate to InitiateKycModal from getGateProviderTitle/getGateActionLabel
12 User navigates away during "Checking your details…" No JS errors; background polling stops cleanly (note: currently no abort mechanism — may still fire network calls)

@kushagrasarathe

Copy link
Copy Markdown
Contributor Author

@coderabbitai re-review in depth.

Evaluate:

  • risk of introducing new bugs and breaking existing functionality
  • performance impact and latency
  • code quality and DRY

Also, write a very concise testing guide for QA

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@kushagrasarathe Here is the updated in-depth re-review based on the actual code.


Bugs Fixed Since the Previous Review ✅

Issue Status
<= off-by-one in polling loop Fixed — now elapsed < BRIDGE_ACTION_AUTO_CONTINUE_TIMEOUT_MS
Double fetchUser() on action-flow complete Fixed — guarded by if (!isActionFlow) void fetchUser()
Unmount safety in polling loop Partially fixed — isMountedRef.current checked at loop top

🔴 High: isMountedRef not checked after await fetchUser() mid-iteration

handleActionFlowComplete in useMultiPhaseKycFlow.ts checks isMountedRef.current at the top of each iteration, not after each await within the iteration body. If the component unmounts during await fetchUser(), the code continues and calls handleSelfHealResubmit('BRIDGE') and setForceShowModal(false) on an unmounted component:

for (...) {
    if (!isMountedRef.current) return  // ← only checked here

    let updatedUser
    try {
        updatedUser = await fetchUser()  // ← component could unmount here
    } catch (error) { ... }

    // ↓ these run even if unmounted mid-await
    if (nextCluster === 'eea_tin_reupload' && canAutoContinueTin) {
        const started = await handleSelfHealResubmit('BRIDGE')  // fires a network call
        if (started) {
            setForceShowModal(false)  // state update after unmount
            return
        }
    }
    ...
}

Add mid-iteration guards:

updatedUser = await fetchUser()
if (!isMountedRef.current) return  // ← add this

// ...
if (nextCluster === 'eea_tin_reupload' && canAutoContinueTin) {
    const started = await handleSelfHealResubmit('BRIDGE')
    if (!isMountedRef.current) return  // ← add this
    ...
}

🟡 Medium: checking phase has no escape hatch

The KycVerificationInProgressModal.tsx checking phase is fully locked (preventClose, hideModalCloseButton, no CTAs). Unlike the preparing phase — which has preparingTimedOut + a "Go to Home" escape — if anything goes wrong before handleActionFlowComplete reaches setModalPhase('preparing') (e.g. an unhandled exception, or the loop exits via break after a failed handleSelfHealResubmit), the user is permanently stuck:

// this break exits the loop but the code below still runs:
console.warn('[useMultiPhaseKycFlow] failed to start EEA TIN reupload continuation')
break  // ← falls through to setModalPhase('preparing') ✓ — this specific path is fine

// BUT: if handleSelfHealResubmit throws (not just returns false), the try/catch
// is not wrapping this call, so an uncaught rejection reaches the for-loop's catch
// and the flow would continue normally. OK.

The actual risk is if handleActionFlowComplete itself throws before reaching setModalPhase('preparing'). Wrap the entire function body in a try/finally:

const handleActionFlowComplete = useCallback(async () => {
    setModalPhase('checking')
    setForceShowModal(true)
    try {
        // ... polling loop ...
    } finally {
        if (isMountedRef.current) {
            setModalPhase('preparing')
            setForceShowModal(true)
            startTracking()
        }
    }
}, [...])

🟡 Medium: Fragile stale-closure dependency in handleSdkComplete

In useSumsubKycFlow.ts:

const handleSdkComplete = useCallback(() => {
    ...
    if (selfHealProviderRef.current) {
        selfHealProviderRef.current = null
        setIsActionFlow(false)          // ← schedules state update
        if (!isActionFlow) void fetchUser()  // ← reads stale isActionFlow (still true)
        return
    }
    ...
}, [fetchUser, isActionFlow])  // isActionFlow in deps

This works correctly today because React batches synchronous state updates — setIsActionFlow(false) hasn't flushed when !isActionFlow is checked, so the condition is false and fetchUser() is not called (avoiding the double-call with handleActionFlowComplete). However this is an implicit invariant that should be documented since it would silently break if this code were to run in an async context:

// Before: if (!isActionFlow) void fetchUser()
// The condition is intentionally checking the captured (pre-update) value.
// isActionFlow === true when in a self-heal action flow, so fetchUser() is skipped here.
// handleActionFlowComplete() in useMultiPhaseKycFlow handles the polling instead.
if (!isActionFlow) void fetchUser()

🟡 Medium: lastSelfHealQuestionnaireCluster not reset between resubmission attempts

lastSelfHealQuestionnaireCluster is only reset to null in handleInitiateKyc. It is NOT reset in handleSdkComplete or before handleSelfHealResubmit is re-invoked from handleActionFlowComplete. If the first action SDK sets it to 'eea_tin_reupload' and then the second handleSdkComplete fires, canAutoContinueTin will be false (correct). But if the user abandons mid-flow and re-enters without going through handleInitiateKyc, the stale value persists. Low probability but worth a defensive setLastSelfHealQuestionnaireCluster(null) at the start of handleSelfHealResubmit.


Code Quality & DRY

1. Duplicated questionnaireCluster union type (two files)

The identical literal union is defined separately in useProviderRejectionStatus.ts (ProviderRemediationAction.questionnaireCluster) and src/app/actions/sumsub.ts (SelfHealResubmissionResponse.questionnaireCluster). Adding a new cluster requires updating both files.

Fix — extract to interfaces.ts:

export type BridgeQuestionnaireCluster =
    | 'source_of_funds' | 'tax_id' | 'address' | 'birth_details'
    | 'eea_uplift' | 'eea_tin_reupload'

2. Missing isEeaUpliftAction() helper (asymmetric with isEeaTinReuploadAction())

eea_tin_reupload has a named helper used consistently; eea_uplift is always checked inline:

// inconsistent
if (action?.questionnaireCluster === 'eea_uplift') return 'Provide required details'  // inline
if (isEeaTinReuploadAction(action)) return 'Re-upload TIN'                             // helper

Add isEeaUpliftAction(action?: ProviderRemediationAction) for symmetry.

3. eea_uplift modal title is generic, not EEA-specific

getActionModalTitle for eea_uplift falls through to the BRIDGE_CUSTOMER_FIELDS case ('We need more details'), while getActionLabel correctly returns 'Provide required details'. The result is a generic title paired with a specific CTA — a minor inconsistency. Consider adding an eea_uplift branch in getActionModalTitle to return something like 'We need additional details'.

4. getBridgeRemediationFromUser lacks status validation

getBridgeRemediationFromUser (in useMultiPhaseKycFlow.ts) returns any record without validating the status field. The parallel getBridgeRemediation in useProviderRejectionStatus.ts guards against unrecognized status values. getBridgeNextQuestionnaireClusterFromUser checks status !== 'AWAITING_INPUT' downstream, so the practical risk is low, but the inconsistency could be confusing.


Concise QA Testing Guide

# Scenario Expected
1 Approved Bridge user with AWAITING_INPUT + questionnaireCluster: eea_uplift opens Add Money / Withdraw InitiateKycModal shows title "We need more details", CTA "Provide required details"
2 User taps "Provide required details" Sumsub SDK opens; isActionFlow = true
3 User completes Sumsub action flow "Checking your details..." modal appears; no close button or CTA visible
4 API returns eea_tin_reupload as next cluster within 20 s Sumsub SDK re-opens automatically with TIN reupload; checking modal disappears
5 User completes TIN reupload "Checking your details..." reappears → "Setting up your account" → "All set!"
6 API never returns eea_tin_reupload within 20 s After 20 s, transitions to "Setting up your account" and startTracking begins
7 User who has already done TIN reupload (second session) canAutoContinueTin = false; loop detects eea_tin_reupload cluster but skips auto-continue; goes to preparing/tracking
8 User navigates away during "Checking your details..." No JS errors in console; background polling eventually stops (note: network calls may still fire until timeout)
9 TERMINAL remediation status "Payment setup issue" title, "Contact support" CTA; no Sumsub action
10 AWAITING_PROVIDER status "Document review in progress" / "Got it" CTA
11 Standard Bridge BRIDGE_CUSTOMER_FIELDS rejection (no EEA cluster) Existing "Provide details" label; no regression
12 AddWithdrawCountriesList, add-money page, withdraw page, BankFlowManager providerTitle and providerActionLabel all propagate correctly to InitiateKycModal

@kushagrasarathe

Copy link
Copy Markdown
Contributor Author

@coderabbitai re-review in depth.

Evaluate:

  • risk of introducing new bugs and breaking existing functionality
  • performance impact and latency
  • code quality and DRY

Also, write a very concise testing guide for QA

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@kushagrasarathe Here is the third-pass in-depth review, based on reading the current code.


Issues Fixed Since the Previous Review ✅

Previous finding Status
isMountedRef not checked after await fetchUser() or await handleSelfHealResubmit() mid-iteration ✅ Fixed — both guards are now present
checking phase has no escape hatch if the function exits early ✅ Effectively fixed — outer try/catch + unconditional tail (setModalPhase('preparing') … startTracking()) ensures the loop always lands in preparing, even on unexpected throws
Stale-closure invariant undocumented in handleSdkComplete ✅ Fixed — the comment now reads "this intentionally reads the captured pre-update value. action flows are polled by useMultiPhaseKycFlow."
lastSelfHealQuestionnaireCluster not reset before resubmission ✅ Already present — setLastSelfHealQuestionnaireCluster(null) is called at the top of both handleInitiateKyc and handleSelfHealResubmit
Off-by-one <= in polling loop ✅ Fixed — now <
Double fetchUser() on action-flow complete ✅ Fixed — guarded by if (!isActionFlow)
Step-comment numbering off by one (// 5. × 2) ✅ Fixed — now 5 = needs_enrollment, 6 = ready

The high- and medium-severity bugs from the last two reviews are all resolved. The remaining items are code-quality / DRY only.


Remaining Issues

🟡 Low: Duplicated questionnaireCluster literal union (two files)

ProviderRemediationAction.questionnaireCluster in useProviderRejectionStatus.ts and SelfHealResubmissionResponse.questionnaireCluster in src/app/actions/sumsub.ts are identical. Adding a new cluster requires updating both.

Suggested fix — extract to interfaces.ts:

export type BridgeQuestionnaireCluster =
    | 'source_of_funds'
    | 'tax_id'
    | 'address'
    | 'birth_details'
    | 'eea_uplift'
    | 'eea_tin_reupload'

Then reference BridgeQuestionnaireCluster in both interfaces.


🟡 Low: Asymmetric eea_uplift check — missing isEeaUpliftAction() helper

eea_tin_reupload has a named helper isEeaTinReuploadAction() used consistently across all four helper functions, while eea_uplift is always checked inline. This asymmetry makes the code harder to scan and refactor.

// current — inconsistent
if (action?.questionnaireCluster === 'eea_uplift') ...  // inline (getActionLabel, getBridgeActionMessage)
if (isEeaTinReuploadAction(action)) ...                 // helper (all four functions)

Suggested addition (mirrors the existing pattern):

function isEeaUpliftAction(action?: ProviderRemediationAction) {
    return action?.questionnaireCluster === 'eea_uplift'
}

🟡 Low: eea_uplift modal title falls through to generic copy

getActionModalTitle has a specific branch for eea_tin_reupload ('Re-upload your TIN') but no branch for eea_uplift, so it falls through to the BRIDGE_CUSTOMER_FIELDS case and returns 'We need more details'. The CTA label is correctly specific ('Provide required details'), creating a mismatch in specificity. A minimal fix:

function getActionModalTitle(action?: ProviderRemediationAction) {
    if (isEeaUpliftAction(action)) return 'We need additional details'   // ← add
    if (isEeaTinReuploadAction(action)) return 'Re-upload your TIN'
    ...
}

🟢 Low / Informational: getBridgeRemediationFromUser skips status validation

getBridgeRemediationFromUser in useMultiPhaseKycFlow.ts returns any Record without validating the status field, unlike getBridgeRemediation in useProviderRejectionStatus.ts. In practice the risk is low because getBridgeNextQuestionnaireClusterFromUser checks status !== 'AWAITING_INPUT' immediately after, but the inconsistency could confuse future maintainers.


Performance — No Change

All polling paths are the same as analysed previously. The happy-path latency (+1 fetchUser call), 20 s worst-case and the non-dismissible checking modal remain unchanged. No new regressions.


Concise QA Testing Guide

# Scenario Expected
1 Approved Bridge user with AWAITING_INPUT + questionnaireCluster: eea_uplift opens Add Money / Withdraw InitiateKycModal shows title "We need more details", CTA "Provide required details"
2 User taps CTA Sumsub SDK opens; isActionFlow = true
3 User completes Sumsub action "Checking your details..." appears; no close button or CTA visible
4 API returns eea_tin_reupload within 20 s Sumsub re-opens automatically for TIN reupload; checking modal disappears
5 User completes TIN reupload "Checking…" → "Setting up your account" → "All set!"
6 API never returns eea_tin_reupload within 20 s After 20 s → "Setting up your account"; startTracking begins
7 Second session — TIN already submitted (canAutoContinueTin = false) Loop detects eea_tin_reupload cluster, skips auto-continue, goes to preparing
8 User navigates away during "Checking…" No JS errors; network calls eventually stop after timeout
9 TERMINAL remediation "Payment setup issue" + "Contact support" CTA
10 AWAITING_PROVIDER remediation "Document review in progress" + "Got it" CTA
11 Non-EEA Bridge BRIDGE_CUSTOMER_FIELDS rejection Existing "Provide details" label; no regression
12 All four entry-points (AddWithdrawCountriesList, add-money page, withdraw page, BankFlowManager) providerTitle + providerActionLabel propagate correctly to InitiateKycModal

@kushagrasarathe kushagrasarathe merged commit 1ccf830 into feat/bridge-rfi-remediation-ui-clean May 25, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant