fix(kyc): resume Sumsub SDK after PWA reload (TASK-20396)#2316
fix(kyc): resume Sumsub SDK after PWA reload (TASK-20396)#2316kushagrasarathe wants to merge 5 commits into
Conversation
Android evicts backgrounded standalone PWAs from memory — opening the camera/gallery mid-KYC (the exact thing the flow pushes users toward) is the common trigger. On return the page cold-reloads, sumsubToken (useState) resets to null, and the user is dropped out of Sumsub back to the card screen, losing the perceived progress. TASK-20396. Mirror the SDK-open state to the URL (?kyc=true), which survives the app-switcher relaunch. On mount, if the flag is set, re-apply for the card to mint a fresh token for the same in-progress applicant and reopen the SDK where they left off — the same idempotent applyForCard call the token refresh path already uses. Note: this cannot recover the fields on the step the user was actively typing (that lives in Sumsub's iframe, gone with the process) — only the bounce-out to the card screen and the manual re-entry.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe KYC flow now returns explicit initiation outcomes, adds a reload-resume hook keyed by the ChangesSumsub resume and initiation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Code-analysis diffPainscore total: 5853.13 → 5860.23 (+7.1) 🆕 New findings (21)
…and 1 more. ✅ Resolved (19)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/app/`(mobile-ui)/card/page.tsx:
- Around line 418-421: The one-shot KYC resume effect in useEffect is marking
didAttemptKycResumeRef.current too early, before the card gate/loading checks
have settled. Move the ref assignment to after the loading guards for
cardInfo/overview and the existing kycInProgress/sumsubToken checks so resume is
only consumed when prerequisites are ready, and update the effect’s dependency
list to include every value it reads instead of suppressing exhaustive-deps.
- Around line 433-435: The KYC resume flow in the catch block is swallowing
transient failures and immediately clearing the in-progress state, which removes
the retry path. Update the resume logic around the catch in the card page so
failures during reload do not silently reset the KYC flag; instead, preserve the
existing resume state or route the error into applyError so the user can retry.
Use the surrounding KYC resume handler and setKycInProgress/applyError flow to
keep the ?kyc=true state intact on recoverable errors.
🪄 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: 4fad7228-224c-40b9-9129-273f2c1b651b
📒 Files selected for processing (1)
src/app/(mobile-ui)/card/page.tsx
| } catch { | ||
| void setKycInProgress(false) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Don’t silently discard the resume state on transient failure.
A network/auth hiccup during reload currently clears ?kyc=true and gives no user-facing error, so the retry path is lost. Preserve the flag or surface applyError so the user can retry the resume.
Suggested direction
- } catch {
- void setKycInProgress(false)
+ } catch (e) {
+ const message = e instanceof Error ? e.message : 'Failed to resume verification'
+ console.error('[card apply] kyc resume error:', e)
+ setApplyError(message)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch { | |
| void setKycInProgress(false) | |
| } | |
| } catch (e) { | |
| const message = e instanceof Error ? e.message : 'Failed to resume verification' | |
| console.error('[card apply] kyc resume error:', e) | |
| setApplyError(message) | |
| } |
🤖 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/app/`(mobile-ui)/card/page.tsx around lines 433 - 435, The KYC resume
flow in the catch block is swallowing transient failures and immediately
clearing the in-progress state, which removes the retry path. Update the resume
logic around the catch in the card page so failures during reload do not
silently reset the KYC flag; instead, preserve the existing resume state or
route the error into applyError so the user can retry. Use the surrounding KYC
resume handler and setKycInProgress/applyError flow to keep the ?kyc=true state
intact on recoverable errors.
Same PWA-eviction bug as the /card fix, now for the shared multi-phase identity flow (add-money / withdraw / qr-pay / KYC drawer). A backgrounded standalone PWA gets killed by Android; on return the page cold-reloads, `showWrapper` (useState) resets to false, and the SDK is gone. Mirror the SDK-open state to the URL (?kyc=true) via useMultiPhaseKycFlow. On mount, if set, re-initiate to mint a fresh token for the existing applicant and reopen the SDK — with autoStart so it lands straight in the SDK (user already consented) rather than the intro screen. TASK-20396.
kushagrasarathe
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useMultiPhaseKycFlow.ts (1)
51-75: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn failure when Bridge ToS is still required.
Line 55 ignores the second confirmation result, and Lines 66-74 fall through even if
stillNeedsTosremains true after all retries. The caller then runscompleteFlow(), which can close the KYC flow while backend capabilities still require ToS.🐛 Proposed fix
export async function confirmBridgeTosAndAwaitRails(fetchUser: () => Promise<IUserProfile | null>) { - const result = await confirmBridgeTos() + let result = await confirmBridgeTos() if (!result.data?.accepted) { await new Promise((resolve) => setTimeout(resolve, 2000)) - await confirmBridgeTos() + result = await confirmBridgeTos() + if (!result.data?.accepted) { + throw new Error('Bridge ToS was not accepted') + } } @@ for (let i = 0; i < 3; i++) { const updatedUser = await fetchUser() @@ const stillNeedsTos = deriveCapabilityPhaseSignals(updatedUser?.capabilities).needsTos if (!stillNeedsTos) break + if (i === 2) { + throw new Error('Bridge ToS acceptance did not propagate') + } if (i < 2) await new Promise((resolve) => setTimeout(resolve, 2000)) } }🤖 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` around lines 51 - 75, The confirmBridgeTosAndAwaitRails flow can still succeed even when Bridge ToS remains required, so update this function to surface failure instead of falling through. In confirmBridgeTosAndAwaitRails, check the result of the second confirmBridgeTos() call and, if it is still not accepted, stop and return failure or throw. Also, after the retry loop using deriveCapabilityPhaseSignals(updatedUser?.capabilities).needsTos, explicitly fail when stillNeedsTos remains true after all attempts so callers like completeFlow cannot proceed while ToS is still pending.
🤖 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 298-323: The resume path in useMultiPhaseKycFlow can leave the kyc
URL flag stuck when handleInitiateKyc() resolves without reopening Sumsub
because the showWrapper-driven sync effect never runs again after its initial
skip. Update the resume logic so the URL flag is explicitly cleared when the
flow is resolved in-place, using the existing useEffect blocks,
didAttemptResumeRef, kycSyncSkipRef, and setKycInProgress to ensure ?kyc=false
is written even when showWrapper stays false.
- Around line 296-309: The resume path in useMultiPhaseKycFlow leaves
sdkAutoStart stuck on true after calling handleInitiateKyc, which can affect
later manual launches. Reset sdkAutoStart once the resume-triggered SDK reopen
has been handled, or separate the resume flow from the normal initiate flow so
the auto-start flag is only preserved for the intended path. Use the useEffect
that checks didAttemptResumeRef, the setSdkAutoStart state setter, and
handleInitiateKyc to place the reset in the right resume-specific branch.
---
Outside diff comments:
In `@src/hooks/useMultiPhaseKycFlow.ts`:
- Around line 51-75: The confirmBridgeTosAndAwaitRails flow can still succeed
even when Bridge ToS remains required, so update this function to surface
failure instead of falling through. In confirmBridgeTosAndAwaitRails, check the
result of the second confirmBridgeTos() call and, if it is still not accepted,
stop and return failure or throw. Also, after the retry loop using
deriveCapabilityPhaseSignals(updatedUser?.capabilities).needsTos, explicitly
fail when stillNeedsTos remains true after all attempts so callers like
completeFlow cannot proceed while ToS is still pending.
🪄 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: 6c910dd0-3644-422d-bed1-f3e16e3b53cb
📒 Files selected for processing (2)
src/components/Kyc/SumsubKycModals.tsxsrc/hooks/useMultiPhaseKycFlow.ts
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -t f "page.tsx" src/app/\(mobile-ui\)/card || true
echo "---"
cat -n "src/app/(mobile-ui)/card/page.tsx"
echo "=== SumsubKycModals.tsx ==="
cat -n "src/components/Kyc/SumsubKycModals.tsx"
echo "=== useMultiPhaseKycFlow.ts ==="
cat -n "src/hooks/useMultiPhaseKycFlow.ts"Length of output: 62793 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== usages of autoStartSdk ==="
rg -n "autoStartSdk" -C3 --type=ts --type=tsx 2>/dev/null || rg -n "autoStartSdk" -C3
echo "=== useSumsubKycFlow.ts handleInitiateKyc & catch handling ==="
fd -t f "useSumsubKycFlow.ts" src
cat -n src/hooks/useSumsubKycFlow.ts 2>/dev/null | sed -n '1,400p'Length of output: 663 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== usages of autoStartSdk ==="
rg -n "autoStartSdk" -C3 -g '*.ts' -g '*.tsx' || true
echo "=== find useSumsubKycFlow.ts ==="
fd -t f "useSumsubKycFlow.ts"
echo "=== cat useSumsubKycFlow.ts ==="
fd -t f "useSumsubKycFlow.ts" -x cat -n {}Length of output: 443 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== repo root check ==="
ls
echo "=== search for SumsubKycModals usage ==="
rg -n "SumsubKycModals" -C2
echo "=== search useSumsubKycFlow ==="
rg -n "useSumsubKycFlow" -l
echo "=== fd for kyc flow hooks ==="
fd -i "sumsub" -t fLength of output: 1148 🏁 Script executed: #!/bin/bash
set +e
echo "=== search for SumsubKycModals usage ==="
rg -n "SumsubKycModals" -C2 src
echo "exit: $?"
echo "=== search useSumsubKycFlow ==="
rg -n "useSumsubKycFlow" src -l
echo "exit: $?"
echo "=== fd for kyc flow hooks ==="
fd -i "sumsub" -t f srcLength of output: 16776 🏁 Script executed: #!/bin/bash
set -e
cat -n src/hooks/useSumsubKycFlow.tsLength of output: 26463 🏁 Script executed: #!/bin/bash
set -e
ast-grep run --pattern 'autoStart' --lang tsx src/components/Kyc/SumsubKycWrapper.tsx | head -50
echo "---"
rg -n "autoStart" -C3 src/components/Kyc/SumsubKycWrapper.tsxLength of output: 2103
Risk of new bugs / breaking existing functionality
Performance / latencyNegligible. The resume path reuses the same idempotent Code quality / DRYThe "mirror SDK-open state to URL + resume on mount" pattern ( function useKycUrlResumeFlag(isOpen: boolean, onResume: () => void | Promise<void>)that owns the Testing guide for QA
✅ Action performedReview finished.
|
Addresses CodeRabbit feedback on the PWA-reload resume: - Extract the duplicated "mirror SDK-open to URL + resume on mount" logic (card page + useMultiPhaseKycFlow) into a single useSumsubReloadResume hook. One place owns the query param, the skip-first-run ordering, and the error/cleanup contract. - handleInitiateKyc now reports whether the SDK actually opened. The hook's onResume returns that boolean; a resume that doesn't reopen (init error, or a remediation flow like self-heal / start-action / restart-identity that a bare initiate can't reconstruct) clears the ?kyc flag instead of falling back to the standard level or leaving the flag stuck to retry on every future reload. - SumsubKycModals: autoStart uses ?? not || so an explicit autoStartSdk=false isn't overridden by the flow flag.
|
Addressed the review in cdb191b:
Remaining known edge (documented, accepted): Verified: typecheck clean, |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useMultiPhaseKycFlow.ts (1)
51-75: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail the ToS flow when confirmation never propagates.
Line 72 breaks only when
needsTosclears, but Lines 66-74 still resolve after all retries even if Bridge still requires ToS. Downstream callers treat resolution as completion, so the modal can close while terms are still required.🐛 Proposed fix
export async function confirmBridgeTosAndAwaitRails(fetchUser: () => Promise<IUserProfile | null>) { - const result = await confirmBridgeTos() + let result = await confirmBridgeTos() if (!result.data?.accepted) { await new Promise((resolve) => setTimeout(resolve, 2000)) - await confirmBridgeTos() + result = await confirmBridgeTos() + } + + if (!result.data?.accepted) { + throw new Error('Bridge ToS confirmation was not accepted') } @@ for (let i = 0; i < 3; i++) { const updatedUser = await fetchUser() @@ const stillNeedsTos = deriveCapabilityPhaseSignals(updatedUser?.capabilities).needsTos - if (!stillNeedsTos) break + if (!stillNeedsTos) return if (i < 2) await new Promise((resolve) => setTimeout(resolve, 2000)) } + + throw new Error('Bridge ToS confirmation has not propagated yet') }🤖 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` around lines 51 - 75, The confirmBridgeTosAndAwaitRails flow currently resolves even when the ToS confirmation never propagates, so downstream callers can treat a failed confirmation as success. Update confirmBridgeTosAndAwaitRails to track whether deriveCapabilityPhaseSignals(...).needsTos ever clears after the retries, and explicitly throw or reject if it remains true after the loop. Keep the retry/markSubmitted logic intact, but ensure the function only completes successfully when the fresh fetchUser() result shows ToS is no longer required.
🤖 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/app/`(mobile-ui)/card/page.tsx:
- Around line 406-419: The card reload-resume flow restores the Sumsub token but
does not trigger the wrapper to reopen automatically, so the resume path can
still fall back to StartVerificationView. Update the `/card` resume logic around
useSumsubReloadResume and the SumsubKycWrapper props so the resume state also
sets autoStart to true when a fresh token is recovered, matching the existing
token-refresh behavior and ensuring the SDK reopens instead of showing the start
screen.
---
Outside diff comments:
In `@src/hooks/useMultiPhaseKycFlow.ts`:
- Around line 51-75: The confirmBridgeTosAndAwaitRails flow currently resolves
even when the ToS confirmation never propagates, so downstream callers can treat
a failed confirmation as success. Update confirmBridgeTosAndAwaitRails to track
whether deriveCapabilityPhaseSignals(...).needsTos ever clears after the
retries, and explicitly throw or reject if it remains true after the loop. Keep
the retry/markSubmitted logic intact, but ensure the function only completes
successfully when the fresh fetchUser() result shows ToS is no longer required.
🪄 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: d7cc4ad8-4af3-4d53-af00-06d80214999f
📒 Files selected for processing (5)
src/app/(mobile-ui)/card/page.tsxsrc/components/Kyc/SumsubKycModals.tsxsrc/hooks/useMultiPhaseKycFlow.tssrc/hooks/useSumsubKycFlow.tssrc/hooks/useSumsubReloadResume.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Kyc/SumsubKycModals.tsx
| // PWA-reload resume (see useSumsubReloadResume). On a reload mid-Sumsub, | ||
| // re-apply to mint a fresh token for the same in-progress applicant and | ||
| // reopen the SDK — same idempotent call the token-refresh path uses. | ||
| useSumsubReloadResume(sumsubToken !== null, async () => { | ||
| const res = await rainApi.applyForCard({ termsAccepted: false }) | ||
| if ((res.status === 'incomplete' || res.status === 'main-kyc-required') && 'sumsubAccessToken' in res) { | ||
| setSumsubToken(res.sumsubAccessToken) | ||
| posthog.capture(ANALYTICS_EVENTS.CARD_SUMSUB_OPENED) | ||
| return true | ||
| } | ||
| // user advanced past Sumsub while backgrounded — route normally | ||
| advanceFromApplyResponse(res) | ||
| return false | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and inspect their structure first.
git ls-files 'src/**' | rg 'Sumsub|sumsub|card/page.tsx|kyc|reloadResume|useSumsubReloadResume' -n
echo '--- outline card page ---'
ast-grep outline 'src/app/(mobile-ui)/card/page.tsx' --view expanded
echo '--- outline Sumsub wrapper candidates ---'
fd -a 'SumsubKycWrapper*' src || true
fd -a '*KycWrapper*' src || true
fd -a '*sumsub*' src || trueRepository: peanutprotocol/peanut-ui
Length of output: 2156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for wrapper prop definitions/usages and the reload-resume hook.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'SumsubKycWrapper|useSumsubReloadResume|autoStart|termsAccepted|sumsubAccessToken|CARD_SUMSUB_OPENED' src
echo '--- card page snippet context ---'
sed -n '360,460p' 'src/app/(mobile-ui)/card/page.tsx'
echo '--- shared KYC flow candidates ---'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'autoStart=.*SumsubKycWrapper|<SumsubKycWrapper|SumsubKycWrapper\(' srcRepository: peanutprotocol/peanut-ui
Length of output: 12558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SumsubKycWrapper props and autoStart handling ---'
sed -n '1,120p' 'src/components/Kyc/SumsubKycWrapper.tsx'
echo '--- relevant autoStart logic ---'
sed -n '200,340p' 'src/components/Kyc/SumsubKycWrapper.tsx'
echo '--- card page wrapper usage ---'
sed -n '580,650p' 'src/app/(mobile-ui)/card/page.tsx'
echo '--- shared KYC modal wrapper usage ---'
sed -n '1,80p' 'src/components/Kyc/SumsubKycModals.tsx'Repository: peanutprotocol/peanut-ui
Length of output: 14488
Pass autoStart on the card Sumsub resume path. SumsubKycWrapper already supports autoStart; when it’s falsy, it shows StartVerificationView instead of reopening the SDK. The /card reload-resume flow restores sumsubToken but never sets autoStart, so resume still lands on the start screen.
🤖 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/app/`(mobile-ui)/card/page.tsx around lines 406 - 419, The card
reload-resume flow restores the Sumsub token but does not trigger the wrapper to
reopen automatically, so the resume path can still fall back to
StartVerificationView. Update the `/card` resume logic around
useSumsubReloadResume and the SumsubKycWrapper props so the resume state also
sets autoStart to true when a fresh token is recovered, matching the existing
token-refresh behavior and ensuring the SDK reopens instead of showing the start
screen.
|
@coderabbitai re-review in depth. Evaluate:
Also, write a very concise testing guide for QA |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== useSumsubReloadResume.ts ==="
cat -n src/hooks/useSumsubReloadResume.ts
echo "=== card/page.tsx relevant section ==="
rg -n "useSumsubReloadResume|kycInProgress|advanceFromApplyResponse|sumsubToken" -C4 "src/app/(mobile-ui)/card/page.tsx"
echo "=== useMultiPhaseKycFlow.ts full ==="
cat -n src/hooks/useMultiPhaseKycFlow.ts
echo "=== SumsubKycModals.tsx ==="
cat -n src/components/Kyc/SumsubKycModals.tsx
echo "=== useSumsubKycFlow.ts handleInitiateKyc ==="
cat -n src/hooks/useSumsubKycFlow.tsLength of output: 59731 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== SumsubKycWrapper.tsx autoStart usage ==="
rg -n "autoStart" -C6 src/components/Kyc/SumsubKycWrapper.tsx
echo "=== full component outline ==="
ast-grep run --pattern 'export const SumsubKycWrapper = $_ => { $$$ }' --lang tsx src/components/Kyc/SumsubKycWrapper.tsx | head -100Length of output: 11825 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '195,235p' src/components/Kyc/SumsubKycWrapper.tsxLength of output: 1683 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "setSdkAutoStart|sdkAutoStart" -C3 src/hooks/useMultiPhaseKycFlow.ts src/components/Kyc/SumsubKycModals.tsx
echo "=== check callers passing autoStartSdk explicitly ==="
rg -n "autoStartSdk" -C3
echo "=== check SumsubKycModals callers (add-money, withdraw, qr-pay, drawer) ==="
rg -n "SumsubKycModals" -C3 srcLength of output: 2986 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "SumsubKycModals" -C2 --type ts --type tsx src 2>/dev/null || rg -n "SumsubKycModals" -C2 src -g '*.tsx' -g '*.ts'Length of output: 15937
Confirmed fixed from the previous round
New finding:
|
Round-2 review: sdkAutoStart was set true on resume but never reset, so a later genuine reopen in the same session (retry after rejection, self-heal resubmit, start-action, restart-identity) would also skip StartVerificationView. Reset it on a failed resume (SDK never opened) and on the falling edge of showWrapper (resumed session closed). Only the reload-resume open skips the start view now; every other open shows it. Falling-edge reset is race-safe (fires only on true->false), so it never disarms the pending resume open.
|
Addressed round-2 in a82a079.
Result: only the reload-resume open skips
Verified: typecheck clean, |
|
@coderabbitai re-review in depth. Evaluate:
Also, write a very concise testing guide for QA |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== useMultiPhaseKycFlow.ts ==="
cat -n src/hooks/useMultiPhaseKycFlow.ts
echo "=== useSumsubReloadResume.ts ==="
cat -n src/hooks/useSumsubReloadResume.ts
echo "=== card/page.tsx relevant ==="
rg -n "useSumsubReloadResume|kycInProgress|advanceFromApplyResponse|sumsubToken" -C4 "src/app/(mobile-ui)/card/page.tsx"
echo "=== SumsubKycModals.tsx ==="
cat -n src/components/Kyc/SumsubKycModals.tsx
echo "=== useSumsubKycFlow.ts ==="
cat -n src/hooks/useSumsubKycFlow.tsLength of output: 60628
Risk of new bugs / breaking existing functionalityAll three previously-flagged issues are now correctly resolved, and the fix for
Minor (optional) nitThe wrapped Performance / latencyUnchanged from prior rounds — one idempotent re-apply/re-initiate call on cold reload, Code quality / DRYNo concerns remaining. The shared hook + boolean-return contract + edge-triggered disarm is a clean, well-documented state machine, and the reasoning in the commit message matches what's actually in the code. Concise QA guide
[resolve] |
A PWA-reload resume drives handleInitiateKyc and previously fired KYC_INITIATED identically to a fresh start, inflating "initiated" counts in the funnel. Add resumed: true/false to the capture so resumes are distinguishable. Round-3 review nit.
|
Round-3 nit addressed in 836c3a2 — resume-triggered initiations now carry That was the last open item — CodeRabbit's round-3 review confirmed all prior findings resolved with no blocking concerns. PR is ready for human review. |
1 participant
Problem — TASK-20396
Users report that mid-KYC, if they tab out of the PWA (to camera, gallery, or another app) and come back, they're dropped out of Sumsub and lose their progress. Reproduced on Android.
Root cause
The app ships as a standalone PWA (
display: standalone). Android aggressively evicts backgrounded standalone PWAs from memory — and opening the native camera/gallery (which the KYC flow pushes users toward) is a strong trigger. On return the page cold-reloads; there's no bfcache to restore.The "SDK is open" state is
useStatein both KYC integrations, so a cold reload resets it and the SDK vanishes:/card(Rain card): gated bysumsubToken !== null.showWrapperinuseSumsubKycFlow.Fix (approach A — URL-persisted resume)
Mirror the SDK-open state to the URL (
?kyc=true) via nuqs. An app-switcher relaunch restores the last route + query, so the flag survives. On mount, if set, re-acquire a fresh token for the same in-progress applicant and reopen the SDK./card— re-applyForCard({ termsAccepted: false })(the same idempotent call the token-refresh path uses).handleInitiateKyc()inuseMultiPhaseKycFlow, withautoStartso it lands straight in the SDK (user already consented) instead of the intro screen.A sync effect keeps the URL flag in step with the SDK's open state (skipping the first run so the mount-time resume reads the persisted flag first).
What this fixes: the SDK reopening. After the reload, the user is taken straight back into Sumsub on the same applicant, with all previously-submitted steps preserved — instead of being dumped on the underlying page and having to navigate back and restart the flow.
What this does NOT fix: the data the user was actively typing on the current step (e.g. the personal-info / tax-ID form) is still lost. If they had filled fields but not yet advanced to the next step, those fields come back blank.
Why we can't fix that part:
sessionStorage, scoped to the verification session (verification-steps docs): "Sumsub automatically saves the information an applicant enters ... in the browser session storage ... If the session expires before the applicant finishes entering their information, they will need to re-enter their data when they return."sessionStoragesurvives an in-tab reload but is destroyed when the browsing context is torn down — which is exactly what Android does when it kills the backgrounded PWA. That's why submitted steps survive (they're on Sumsub's server) but the mid-typed step does not.localStorageor make the draft durable (confirmed across WebSDK settings + get-started + about docs).static.sumsub.com), so we can't read itssessionStorage, and the WebSDK only emits step-level events (onStepCompleted,onApplicantSubmitted) — there's no field-level/keystroke event to capture and re-push.The only way to fully preserve typed data would be an architectural change: collect the personal-info / tax-ID fields in our own PWA form (persisted in our storage, which survives the kill), push them to Sumsub via the
fixedInfo+/questionnairesAPI before the SDK opens, and enable prefill on the Sumsub level (applicantDataSettings.prefill). That's a separate, larger project touching FE + BE + Sumsub level config — tracked separately, not in this PR.Known limitation (summary)
Recovers the bounce-out + manual re-entry, not the fields on the step the user was actively typing. See the section above for why.
Testing
npm run typecheckclean,npm run buildpasses, prettier clean.jest card137/137;useSumsubKycFlow/ add-money-states / qr-pay-states suites pass (76).Files
src/app/(mobile-ui)/card/page.tsxsrc/hooks/useMultiPhaseKycFlow.tssrc/components/Kyc/SumsubKycModals.tsx