fix: EEA-uplift deposit/withdraw errors + Bridge re-verification pending modal#2345
Open
kushagrasarathe wants to merge 8 commits into
Open
fix: EEA-uplift deposit/withdraw errors + Bridge re-verification pending modal#2345kushagrasarathe wants to merge 8 commits into
kushagrasarathe wants to merge 8 commits into
Conversation
EEA-uplift users whose docs are under Bridge review were retrying deposits
with zero feedback: the onramp hook threw a hardcoded message without
reading the backend's { error } body, the page's catch read the hook's
error state before it flushed (always null on first attempt), and the
waiting-on-provider gate dead-ended the Continue button with no message.
Parse the error body, show the caught message directly, and tell
under-review users their verification is being processed.
Addresses code-review findings on the silent-failure fix: - only surface the backend body for 4xx (client errors); 5xx bodies carry raw internal messages the global handler doesn't sanitize, so those fall back to the generic string (no internal-detail leak). - make useCreateOnramp throw-only: its React error state had no reader after the page stopped destructuring it, and was the same stale-closure trap the page fix works around. The thrown Error carries the reason. - reuse gate.userMessage for the waiting-on-provider copy instead of hardcoding, matching every other gate consumer. - extract GENERIC_ONRAMP_ERROR so the fallback copy lives in one place. - keep the distinct 'Unknown error' analytics label for non-Error throws.
…diation The eea_uplift_started event fired only on the advisory modal's 'Complete now' CTA, so it missed (a) users who open the uplift modal and abandon, and (b) the entire urgent post-cliff cohort, whose remediation is now blocking (fixable-rejection → KYC modal) rather than a future-dated advisory. Fire at modal-open instead, across both remediation paths and both channels (deposit + withdraw): - blocking/urgent: gate.reason.code === 'eea_uplift*' (BE sets it to the questionnaire cluster). - advisory/upcoming: requirementKey in the eea_uplift set (sof_individual_ primary_purpose, has_foreign_tax_registration, place_of_birth_missing, nationalities) — derived from prod remediation data. Event now carries source + urgent + effective_date so urgent vs upcoming (dob-type) split in PostHog. PoA / gov-id-expired co-occur on the cliff but are excluded (separate remediation) to avoid over-firing on generic doc rejections for non-EEA users.
Addresses code-review findings on the eea_uplift funnel:
- phantom completions: the blocking path arms the started latch, but the
InitiateKycModal onClose/onContactSupport did not reset it (only the
advisory/Sumsub path did). A later unrelated KYC approval would fire a
false eea_uplift_completed. Reset the latch on modal dismiss.
- re-fire: trackStarted is now idempotent per armed attempt, so repeat
Continue clicks in one attempt don't inflate the started count.
- DRY + convention: collapse the four duplicated trigger-construction sites
into upliftTriggerFrom{Gate,Advisory} helpers and move UpliftStartTrigger
out of the hook into eea-uplift.utils (per the no-types-in-hooks rule).
When a user completes an EEA uplift and then tries to on/offramp via Bridge while Bridge is still re-reviewing, the deposit page showed only an inline message and the withdraw page silently did nothing. Show a dedicated pending/in-progress modal instead, driven by the waiting-on-provider gate. To reflect proper live status from Bridge, re-arm the existing submission- window poller (markSubmitted) when the modal opens — the 4s user refetch picks up Bridge's latest verification status and the modal auto-dismisses the moment the gate leaves waiting-on-provider. No new backend endpoint: the capability pipeline already mirrors Bridge status. Also closes the withdraw-side silent no-op noted in the deposit fix.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Jul 3, 2026
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds waiting-on-provider modal handling for add-money and withdraw bank flows, centralizes EEA uplift trigger derivation and tracking, and changes onramp creation to throw user-facing errors instead of exposing hook error state. ChangesBank flow gating, uplift tracking, and onramp errors
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Code-analysis diffPainscore total: 5872.71 → 5890.67 (+17.96) 🆕 New findings (36)
…and 16 more. ✅ Resolved (31)
…and 11 more. 📈 Painscore deltas (top movers)
|
Contributor
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/utils/eea-uplift.utils.ts (1)
48-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer discriminant narrowing over unsafe cast.
(gate as { reason?: { code?: string } })bypassesGateState's discriminated union entirely. If the realreasonshape changes or moves to a differently-named field, this compiles silently and returns wrong results at runtime instead of failing at compile time.♻️ Suggested narrowing via the `kind` discriminant
export function upliftTriggerFromGate(gate: GateState): UpliftStartTrigger | null { - const code = (gate as { reason?: { code?: string } }).reason?.code - if (!code?.startsWith('eea_uplift')) return null - return { requirementKey: code, source: 'blocking' } + if (gate.kind !== 'fixable-rejection') return null + const code = gate.reason?.code + if (!code?.startsWith('eea_uplift')) return null + return { requirementKey: code, source: 'blocking' } }This assumes
reasononly exists on thefixable-rejectionvariant ofGateState(consistent with the tests at lines 28-31 and the doc comment). Please confirm against the actualGateStatedefinition incapability-gate.ts.🤖 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/utils/eea-uplift.utils.ts` around lines 48 - 52, Replace the unsafe cast in upliftTriggerFromGate with proper discriminant narrowing on GateState, using the kind field to access the fixable-rejection variant before reading reason.code. Keep the existing return shape, but remove the `(gate as { reason?: { code?: string } })` assertion so the compiler enforces the real GateState union. Please verify the exact variant shape against GateState in capability-gate.ts and only read reason from the matching branch.src/hooks/useCreateOnramp.ts (1)
26-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLoosen
Promise<any>return type.Flagged by ESLint (
@typescript-eslint/no-explicit-any). The success path already returns a well-shaped payload (transferId,depositInstructions, …) documented inIOnrampDatainOnrampFlowContext.tsx. TypingcreateOnrampasPromise<IOnrampData>(or a dedicated response type) would give callers compile-time safety instead ofany.♻️ Proposed fix
+import { type IOnrampData } from '`@/context/OnrampFlowContext`' + export interface UseCreateOnrampReturn { - createOnramp: (params: CreateOnrampParams) => Promise<any> + createOnramp: (params: CreateOnrampParams) => Promise<IOnrampData> isLoading: boolean }🤖 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/useCreateOnramp.ts` around lines 26 - 29, The `UseCreateOnrampReturn.createOnramp` signature is too loose because it returns `Promise<any>`, which triggers the ESLint `no-explicit-any` rule and hides the actual response shape. Update the return type in `UseCreateOnrampReturn` to use the existing `IOnrampData` shape from `OnrampFlowContext` (or a dedicated response type if needed), and make sure the `useCreateOnramp` implementation and its callers are aligned with that typed payload so the success path is strongly typed.Source: Linters/SAST tools
🤖 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)/add-money/[country]/bank/page.tsx:
- Around line 74-75: Remove the unused isRiskAccepted state from the bank
add-money page, since it is never read or passed to OnrampConfirmationModal and
is causing the lint/CI failure. Update the related handlers in the same
component, including handleWarningConfirm and handleWarningCancel, to drop the
setIsRiskAccepted(false) calls unless you wire the state into actual UI logic
such as the Continue button disabled state or the modal props.
In `@src/app/`(mobile-ui)/withdraw/[country]/bank/page.tsx:
- Line 131: Reset the pending modal latch when the provider gate stops being
waiting-on-provider so it cannot reopen from stale state. Update the
`showPendingModal` handling in `BankWithdrawPage` (the component that owns the
`useState` and the `gate.kind` auto-hide logic) to clear
`setShowPendingModal(false)` when the gate transitions away from
`waiting-on-provider`, not only when `onClose` runs. Keep the existing auto-hide
behavior, but ensure the state is explicitly reset as part of the gate-change
effect/branch that watches `gate.kind`.
---
Nitpick comments:
In `@src/hooks/useCreateOnramp.ts`:
- Around line 26-29: The `UseCreateOnrampReturn.createOnramp` signature is too
loose because it returns `Promise<any>`, which triggers the ESLint
`no-explicit-any` rule and hides the actual response shape. Update the return
type in `UseCreateOnrampReturn` to use the existing `IOnrampData` shape from
`OnrampFlowContext` (or a dedicated response type if needed), and make sure the
`useCreateOnramp` implementation and its callers are aligned with that typed
payload so the success path is strongly typed.
In `@src/utils/eea-uplift.utils.ts`:
- Around line 48-52: Replace the unsafe cast in upliftTriggerFromGate with
proper discriminant narrowing on GateState, using the kind field to access the
fixable-rejection variant before reading reason.code. Keep the existing return
shape, but remove the `(gate as { reason?: { code?: string } })` assertion so
the compiler enforces the real GateState union. Please verify the exact variant
shape against GateState in capability-gate.ts and only read reason from the
matching branch.
🪄 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: 1335424a-437e-468f-8b49-70f1f82d91b8
📒 Files selected for processing (10)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/add-money/__tests__/add-money-states.test.tsxsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/components/Kyc/KycReverificationPendingModal.tsxsrc/hooks/__tests__/useCreateOnramp.test.tssrc/hooks/useCreateOnramp.tssrc/hooks/useEeaUpliftFunnel.test.tssrc/hooks/useEeaUpliftFunnel.tssrc/utils/eea-uplift.utils.test.tssrc/utils/eea-uplift.utils.ts
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Addresses code-review on the reverification pending modal: - FEATURE-BREAKING: a single 30s submission window lapsed mid-review (waiting-on-provider rails are requires-info, not pending, so the poller self-terminated) — the modal never auto-dismissed even after Bridge approved. Extract useWaitingOnProviderModal, which re-arms the window on a 20s interval while open so the ~4s user poll runs for the whole wait. - stale-flag reopen: drop the request flag when the gate resolves, so a transient re-flip to waiting-on-provider can't reopen the modal on its own. - surface gate.userMessage (bridge-specific review copy) in the modal instead of always the generic text. - DRY: the shared hook collapses the duplicated deposit/withdraw wiring. - drop the redundant 'clock' as IconName cast.
Write-only (declared + set, never read) — the cherry-picked handlers re-added the setters onto state main had already reduced to a bare declaration. No behavior change.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 2345 --repo peanutprotocol/peanut-ui --json number,title,baseRefName,headRefName,commits -q '.'
echo "---DIFF FILES---"
gh pr diff 2345 --repo peanutprotocol/peanut-ui > /tmp/pr.diff
wc -l /tmp/pr.diff
echo "---useCreateOnramp---"
sed -n '1,200p' /tmp/pr.diff | grep -n "diff --git"Length of output: 8316 🏁 Script executed: #!/bin/bash
set -euo pipefail
cat /tmp/pr.diff | grep -n "^diff --git"Length of output: 1488 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,182p' /tmp/pr.diff
echo "=====WITHDRAW====="
sed -n '289,405p' /tmp/pr.diff
echo "=====MODAL====="
sed -n '406,461p' /tmp/pr.diff
echo "=====useWaitingOnProviderModal====="
sed -n '938,993p' /tmp/pr.diff
echo "=====useWaitingOnProviderModal TEST====="
sed -n '557,641p' /tmp/pr.diff
echo "=====useCreateOnramp====="
sed -n '642,715p' /tmp/pr.diff
echo "=====useEeaUpliftFunnel====="
sed -n '838,937p' /tmp/pr.diff
echo "=====eea-uplift.utils====="
sed -n '1058,1129p' /tmp/pr.diffLength of output: 36493 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -uu "capability-gate" -e ts src
echo "---"
sed -n '1,150p' src/utils/capability-gate.ts 2>/dev/null | head -200Length of output: 7565 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "CapabilityReason" -r src/types/capabilities.ts | head -20
echo "---"
sed -n '1,60p' src/types/capabilities.ts | grep -n "interface CapabilityReason" -A 10Length of output: 902 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd "useSubmissionWindow" src
echo "---"
cat src/hooks/useSubmissionWindow.ts 2>/dev/null
echo "---UserAutoRefresh---"
fd "useUserAutoRefresh" src
cat src/hooks/useUserAutoRefresh.ts 2>/dev/null | head -80Length of output: 7052 🏁 Script executed: #!/bin/bash
set -euo pipefail
grep -n "useEeaUpliftFunnel\|trackUpliftStarted\|resetUpliftFunnel\|trackUpliftCompleted" "src/app/(mobile-ui)/add-money/[country]/bank/page.tsx" "src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx"Length of output: 2702 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd "useAdvisoryPreempt" src
cat src/hooks/useAdvisoryPreempt.ts 2>/dev/nullLength of output: 3335 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '90,100p' "src/app/(mobile-ui)/add-money/[country]/bank/page.tsx"
echo "---"
grep -n "onKycApproved\|handleSelfHealResubmit\|useMultiPhaseKycFlow" -r src/hooks/useMultiPhaseKycFlow.ts | head -30Length of output: 1168 Good — confirmed the flow is shared and consistent. I now have enough to write the review.
Risk of new bugs / breaking existing functionality — Low
const message = gate.kind === 'waiting-on-provider' ? (gate.userMessage ?? undefined) : undefinedinstead of the cast. Same idea applies to Performance impact / latency — Negligible
Code quality & DRY — Good, above bar for a hotfix
Concise QA testing guide1. Deposit — 4xx error surfacing
2. Deposit — 5xx error surfacing
3. Bridge re-verification pending modal (deposit)
4. Bridge re-verification pending modal (withdraw)
5. Analytics —
6. Regression check
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useWaitingOnProviderModal.ts (2)
23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary type assertion where discriminant narrowing would work.
gate.kind === 'waiting-on-provider'is a proper discriminant onGateState, and per thecapability-gate.tsdefinitionuserMessageis required (not optional) on that variant. Casting to{ userMessage?: string | null }bypasses that safety instead of narrowing directly, and would silently swallow a typo/refactor mistake if the field were ever renamed.♻️ Proposed fix
- const isOpen = requested && isWaiting - const message = isOpen ? ((gate as { userMessage?: string | null }).userMessage ?? undefined) : undefined + const isOpen = requested && isWaiting + const message = isOpen && gate.kind === 'waiting-on-provider' ? (gate.userMessage ?? undefined) : undefined🤖 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/useWaitingOnProviderModal.ts` around lines 23 - 27, The `useWaitingOnProviderModal` hook is using an unnecessary type assertion on `gate` when reading `userMessage`; use the `gate.kind === 'waiting-on-provider'` discriminant to narrow `GateState` directly and access `gate.userMessage` without casting. Update the `message` assignment in `useWaitingOnProviderModal` so it relies on the narrowed waiting-on-provider branch rather than `{ userMessage?: string | null }`, preserving the type safety defined by `GateState` and `capability-gate.ts`.
5-7: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTie the re-arm cadence to the submission window.
SUBMISSION_WINDOW_MSis private inuseSubmissionWindow.ts, so this file can’t import it as proposed; export the shared constant or deriveREARM_INTERVAL_MSfrom a shared helper to keep the modal poller from drifting out of sync.🤖 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/useWaitingOnProviderModal.ts` around lines 5 - 7, The re-arm cadence in useWaitingOnProviderModal is hard-coded and can drift from the submission window; move SUBMISSION_WINDOW_MS to a shared export or add a shared helper and derive REARM_INTERVAL_MS from it instead of duplicating the value. Update the useWaitingOnProviderModal and useSubmissionWindow symbols so both depend on the same source of truth, keeping the modal poller aligned with the submission window.
🤖 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.
Nitpick comments:
In `@src/hooks/useWaitingOnProviderModal.ts`:
- Around line 23-27: The `useWaitingOnProviderModal` hook is using an
unnecessary type assertion on `gate` when reading `userMessage`; use the
`gate.kind === 'waiting-on-provider'` discriminant to narrow `GateState`
directly and access `gate.userMessage` without casting. Update the `message`
assignment in `useWaitingOnProviderModal` so it relies on the narrowed
waiting-on-provider branch rather than `{ userMessage?: string | null }`,
preserving the type safety defined by `GateState` and `capability-gate.ts`.
- Around line 5-7: The re-arm cadence in useWaitingOnProviderModal is hard-coded
and can drift from the submission window; move SUBMISSION_WINDOW_MS to a shared
export or add a shared helper and derive REARM_INTERVAL_MS from it instead of
duplicating the value. Update the useWaitingOnProviderModal and
useSubmissionWindow symbols so both depend on the same source of truth, keeping
the modal poller aligned with the submission window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c248cc9-bc23-4e49-a5d3-47522e11db3b
📒 Files selected for processing (5)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/components/Kyc/KycReverificationPendingModal.tsxsrc/hooks/__tests__/useWaitingOnProviderModal.test.tssrc/hooks/useWaitingOnProviderModal.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Kyc/KycReverificationPendingModal.tsx
- src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
Replace the ad-hoc gate casts in useWaitingOnProviderModal (userMessage) and upliftTriggerFromGate (reason.code) with discriminated-union narrowing, so a future rename of reason.code / userMessage fails the build instead of silently returning undefined. No behavior change.
Contributor
Author
|
Thanks for the thorough pass. Addressed the one actionable item — the ad-hoc type assertions. Both now narrow on the
So a future rename of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets main (hotfix — the affected users are live). Bundles three related pieces for the EEA-uplift Bridge flow; supersedes #2329 and #2332.
1. Surface deposit errors instead of failing silently (was #2329)
EEA users whose Bridge KYC was under review created deposits that failed with zero feedback. Fixed:
useCreateOnrampnow parses the backend error body — but only for 4xx (deliberate, user-safe messages); 5xx bodies are suppressed to a generic string so internal detail can't leak.2. Bridge re-verification pending modal (new — the core ask)
When a user completes an EEA uplift and then tries to on/offramp while Bridge is still re-reviewing, they hit the
waiting-on-providergate. Before: deposit showed a bare inline line, withdraw did nothing (dead button). Now:KycReverificationPendingModal("We're reviewing your details…") on both deposit + withdraw.markSubmitted), so the 4s user refetch pulls Bridge's live status and the modal auto-dismisses the moment the gate clears. No new BE endpoint — the capability pipeline already mirrors Bridge status.3. EEA-uplift funnel fires on modal-open, all remediation (was #2332)
eea_uplift_startedfired only on the advisory CTA — missing abandoners and the entire urgent post-cliff (blocking) cohort. Now fires at modal-open across both remediation paths + both channels, carryingsource/urgent/effective_dateto split urgent vs upcoming in PostHog. Detection:reason.codeeea_uplift*(blocking) + a requirement-key set (advisory), derived from prod data.Risks
startedcounts rise (now include abandoners) — intended; funnel dedups by person.QA
npm test110 suites / 1693 green · typecheck · build · prettier. New tests:useCreateOnramp(incl. 5xx leak-guard),eea-uplift.utils, rewrittenuseEeaUpliftFunnel(dedup/latch), and the deposit waiting-on-provider→modal assertion.