Conversation
🦋 Changeset detectedLatest commit: cf2b0b2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds an "invalid legal id" recovery flow: server can return inquiry/session tokens on that error; client caches tokens, routes users to a resumed KYC flow that reuses tokens via startMantecaKYC, clears the cache on completion/error, and introduces pending-state handling, UI text/translation updates, and a small navigation tweak. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Client App
participant Server
participant Cache as QueryCache
participant KYC as Manteca KYC
User->>Client: Start ramp onboarding
Client->>Server: startRampOnboarding()
alt Server returns success
Server-->>Client: success response
Client->>Client: navigate to /add-funds/status?pending=true
else Server returns invalid legal id
Server-->>Client: { code: "invalid legal id", inquiryId, sessionToken }
Client->>Cache: set ["ramp","invalid-legal-id"] = {inquiryId, sessionToken}
Client->>Client: navigate to /add-funds/kyc
Client->>Cache: get ["ramp","invalid-legal-id"]
Client->>KYC: startMantecaKYC(tokens={inquiryId, sessionToken})
KYC->>User: present resumed KYC flow
User-->>KYC: completes verification
KYC-->>Client: result = "complete"
Client->>Cache: delete ["ramp","invalid-legal-id"]
Client->>Client: navigate to /add-funds/status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #769 +/- ##
==========================================
- Coverage 68.46% 68.32% -0.15%
==========================================
Files 207 207
Lines 7059 7030 -29
Branches 2214 2200 -14
==========================================
- Hits 4833 4803 -30
- Misses 2029 2034 +5
+ Partials 197 193 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const { code } = await response.json(); | ||
| throw new APIError(response.status, code); | ||
| const body = await response.json(); | ||
| if (body.code === "invalid legal id") { |
There was a problem hiding this comment.
🚩 Hardcoded error code string bypasses shared constant
The comparison body.code === "invalid legal id" uses a hardcoded string literal rather than referencing the MantecaErrorCodes.INVALID_LEGAL_ID constant that the server uses (server/api/ramp.ts:211). If the server-side constant value ever changes, this client-side check would silently stop matching, causing the "invalid legal id" flow to be treated as a generic API error instead.
The @exactly/common package is already used for shared constants (e.g., AUTH_EXPIRY, domain, Credential). The error code string could be moved there to ensure consistency between client and server.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
The error code string is part of the HTTP API contract, not shared business logic — if the server value changes, the frontend safely falls back to the generic APIError path, and the server's own integration tests (which hardcode the same string) would break first.
| const { data: providers, isFetching } = useQuery({ | ||
| queryKey: ["ramp", "providers", countryCode], | ||
| queryFn: () => getRampProviders(countryCode), | ||
| enabled: isPending && timedOut && !!countryCode, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (providers?.manteca.status === "ACTIVE" && currency) { | ||
| router.replace({ pathname: "/add-funds/ramp", params: { currency } }); | ||
| } | ||
| }, [providers, currency, router]); | ||
|
|
||
| const ready = !isPending || (timedOut && !isFetching); |
There was a problem hiding this comment.
🚩 Providers query only polls once — no retry if status isn't ACTIVE yet
The pending flow in Status.tsx waits 5 seconds (setTimeout at line 33), then fetches providers exactly once (no refetchInterval). If the manteca status isn't ACTIVE after that single fetch, the ready variable becomes true (line 50: timedOut && !isFetching), and the user sees the "Close" button with the message "We're verifying your information. You'll be able to add funds soon."
This appears to be a deliberate design choice — the page acts as a notification that verification is in progress, not as a real-time polling screen. The user closes and returns later. However, if the server typically processes onboarding within 10-30 seconds, adding a refetchInterval (e.g., poll every 5 seconds for up to 60 seconds) could improve the user experience by auto-redirecting to the ramp page once the status becomes ACTIVE.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is deliberate. getRampProviders is an expensive backend query, and polling would risk leaving the user waiting indefinitely if a non-timing error occurs. if the status is already ACTIVE, we auto-redirect; otherwise, the user closes and returns later.
Summary of ChangesHello @franm91, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the user experience for the Know Your Customer (KYC) onboarding process, particularly when issues arise with a user's identification. It introduces a dedicated flow for invalid legal IDs, allowing users to re-engage with the verification process with specific guidance. Additionally, it enhances the feedback provided during the initial onboarding by introducing a pending state, ensuring users are informed about the ongoing verification without being stuck on an unresponsive screen. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/add-funds/KYC.tsx`:
- Line 38: The call to startMantecaKYC uses a redundant nullish-coalescing to
undefined; remove the "?? undefined" and pass invalidLegalId directly
(invalidLegalId is already typed as { inquiryId: string; sessionToken: string }
| undefined from queryClient.getQueryData). Update the invocation of
startMantecaKYC(invalidLegalId) accordingly to simplify the code.
In `@src/utils/completeOnboarding.ts`:
- Around line 10-17: The cached Persona tokens stored via
queryClient.setQueryData(["ramp","invalid-legal-id"]) can become stale if the
user abandons the KYC screen; update the flow so this cache is not left
indefinitely: either attach an expiry/gc marker when setting the data (e.g.,
store a timestamp or gcTime alongside inquiryId/sessionToken) and have KYC.tsx
check and reject expired entries before calling startMantecaKYC, or ensure the
KYC cancel/error and route-change handlers call
queryClient.removeQueries(["ramp","invalid-legal-id"]) to clear the entry;
modify the code that sets the cache in completeOnboarding (the
queryClient.setQueryData call) and the KYC cancel/error handlers (the place that
currently removes the entry on success) so stale tokens are evicted
automatically or explicitly removed on abandon.
In `@src/utils/persona.ts`:
- Around line 135-139: The current promise reuse in startMantecaKYC can return
an in-flight promise that was created with a different tokens value; update the
logic that checks the shared current promise so it also compares the incoming
tokens argument to the tokens used to create the existing promise (or store a
small object {promise, tokens} instead of just promise) and only reuse
current.promise when the tokens match; otherwise create a new promise that
awaits import("persona") and tokens ?? getKYCTokens(...), then set current to
the new {promise, tokens} before returning. Ensure references to current,
startMantecaKYC, and getKYCTokens are updated accordingly.
| if ("inquiryId" in result) { | ||
| queryClient.setQueryData(["ramp", "invalid-legal-id"], { | ||
| inquiryId: result.inquiryId, | ||
| sessionToken: result.sessionToken, | ||
| }); | ||
| router.replace({ pathname: "/add-funds/kyc", params: { currency } }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Stale cached tokens if user abandons the KYC screen.
The ["ramp", "invalid-legal-id"] cache entry is only removed on successful KYC completion (KYC.tsx Line 44). If the user navigates away (e.g., presses back or the app backgrounds), the tokens remain cached indefinitely. On a later visit to KYC.tsx, expired Persona tokens would be passed to startMantecaKYC, likely producing an opaque error.
Consider also clearing the cache on cancel/error, or setting a gcTime on this query data so stale tokens are automatically evicted.
Proposed fix in KYC.tsx mutationFn
async mutationFn() {
if (!currency) return;
const result = await startMantecaKYC(invalidLegalId ?? undefined);
- if (result.status === "cancel") return;
+ if (result.status === "cancel") {
+ queryClient.removeQueries({ queryKey: ["ramp", "invalid-legal-id"] });
+ return;
+ }
if (result.status !== "complete") {
+ queryClient.removeQueries({ queryKey: ["ramp", "invalid-legal-id"] });
router.replace({ pathname: "/add-funds/status", params: { status: "error", currency } });
return;
}🤖 Prompt for AI Agents
In `@src/utils/completeOnboarding.ts` around lines 10 - 17, The cached Persona
tokens stored via queryClient.setQueryData(["ramp","invalid-legal-id"]) can
become stale if the user abandons the KYC screen; update the flow so this cache
is not left indefinitely: either attach an expiry/gc marker when setting the
data (e.g., store a timestamp or gcTime alongside inquiryId/sessionToken) and
have KYC.tsx check and reject expired entries before calling startMantecaKYC, or
ensure the KYC cancel/error and route-change handlers call
queryClient.removeQueries(["ramp","invalid-legal-id"]) to clear the entry;
modify the code that sets the cache in completeOnboarding (the
queryClient.setQueryData call) and the KYC cancel/error handlers (the place that
currently removes the entry on success) so stale tokens are evicted
automatically or explicitly removed on abandon.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new user flow to handle cases where a user's legal ID is invalid during the KYC process. The changes are well-structured, modifying the API client to handle the specific error case, updating the onboarding logic to redirect the user, and adjusting the KYC screen to present the user with context-specific information for re-verification. A significant improvement opportunity has been identified in the Status.tsx component, where the current implementation for checking the verification status could lead to a confusing user experience. My review includes a suggestion to implement polling for a more robust and user-friendly status update mechanism, which remains valid as it does not contradict any provided rules.
| const { data: providers, isFetching } = useQuery({ | ||
| queryKey: ["ramp", "providers", countryCode], | ||
| queryFn: () => getRampProviders(countryCode), | ||
| enabled: isPending && timedOut && !!countryCode, | ||
| }); |
There was a problem hiding this comment.
The current implementation to fetch provider status will only run once after the initial 5-second timeout. If the backend verification is not complete by then, the user will be shown a "Close" button and will not receive further updates, potentially leading to a confusing experience where they are not aware the process is still running. To provide a better user experience and automatically update the status, you should implement polling.
| const { data: providers, isFetching } = useQuery({ | |
| queryKey: ["ramp", "providers", countryCode], | |
| queryFn: () => getRampProviders(countryCode), | |
| enabled: isPending && timedOut && !!countryCode, | |
| }); | |
| const { data: providers, isFetching } = useQuery({ | |
| queryKey: ["ramp", "providers", countryCode], | |
| queryFn: () => getRampProviders(countryCode), | |
| enabled: isPending && timedOut && !!countryCode, | |
| refetchInterval: (query) => (query.state.data?.manteca.status === "ACTIVE" ? false : 5000), | |
| }); |
There was a problem hiding this comment.
good catch but in this case is deliberate. getRampProviders is an expensive backend query, and polling would risk leaving the user waiting indefinitely if a non-timing error occurs. if the status is already ACTIVE, we auto-redirect; otherwise, the user closes and returns later.
franm91
force-pushed
the
manteca
branch
2 times, most recently
from
2c02d17 to
7d2a037
Compare
| } | ||
| queryClient.removeQueries({ queryKey: ["ramp", "invalid-legal-id"] }); | ||
| await completeOnboarding(router, currency); |
There was a problem hiding this comment.
🚩 Potential loop between KYC and completeOnboarding if server keeps returning invalid-legal-id
After a successful Persona inquiry in the invalid-legal-id flow (result.status === "complete"), completeOnboarding is called again (line 46), which calls startRampOnboarding. If the server still returns "invalid legal id" (e.g., the user submitted the same invalid document), completeOnboarding sets new query data and navigates back to KYC, creating a cycle: KYC → Persona complete → completeOnboarding → invalid legal id → KYC → ...
This isn't an infinite loop in the strict sense (it requires user interaction at each step — the user must click the button and complete the Persona inquiry each time), but it could be a confusing UX if the server-side validation keeps rejecting the document. There's no maximum retry counter or different error handling after repeated failures.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is intentional, the only way to validate the updated document is by calling startRampOnboarding again, since manteca performs the legal ID validation server-side during onboarding. Each iteration requires explicit user action
There is no client-side way to pre-validate whether the new document will be accepted.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/add-funds/Status.tsx`:
- Around line 37-42: The useQuery call for the ["user","country"] key (data
assigned to countryCode) lacks a queryFn and is enabled by default, which can
cause runtime fetch errors or unexpected fetches in React Query v5; change this
to read cached value instead of attempting a fetch by either using
queryClient.getQueryData(["user","country"]) where countryCode is needed or make
the useQuery explicitly non-fetching (add enabled: false) and read its data from
the cache, and ensure the providers query (queryKey
["ramp","providers",countryCode], queryFn getRampProviders, enabled: isPending
&& timedOut && !!countryCode) still uses the cache-backed countryCode so
auto-navigation behavior remains correct if getKYCStatus(..., true) has
previously populated the ["user","country"] cache.
In `@src/utils/persona.ts`:
- Around line 141-145: The Promise.all currently awaits getRedirectURI() inline
which forces evaluate it before import("persona") can start; hoist the redirect
URI resolution into its own promise/variable before the Promise.all so
import("persona") and the getKYCTokens(...) invocation run in parallel — e.g.,
call getRedirectURI() (or create a redirectUriPromise) first, then use
Promise.all([{ import("persona") }, tokens ?? getKYCTokens("manteca", await
redirectUriPromise)]) to ensure import("persona") and token retrieval execute
concurrently; update the code around the Platform.OS === "web" block and the
references to import("persona") and getKYCTokens to use the hoisted
promise/variable.
9afdb94 to
5a9335f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/server.ts`:
- Around line 306-314: The startRampOnboarding response currently returns two
shapes causing brittle discrimination by `"inquiryId" in result`; update
startRampOnboarding to return a tagged union (e.g., success object with a
distinct tag like { status: "ok", data: ... } or include a unique discriminator
field on the error path like { code, inquiryId, sessionToken }) so consumers can
reliably discriminate by the tag/`code` field; then update completeOnboarding.ts
to check the explicit discriminator (e.g., `result.status === "ok"` or `'code'
in result`) instead of testing for `inquiryId`. Reference startRampOnboarding
and completeOnboarding.ts when applying the change and ensure types/signatures
are updated accordingly.
| const invalidLegalId = queryClient.getQueryData<{ inquiryId: string; sessionToken: string }>([ | ||
| "ramp", | ||
| "invalid-legal-id", | ||
| ]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reading query cache outside a hook won't trigger re-renders.
queryClient.getQueryData is a snapshot read — if ["ramp", "invalid-legal-id"] changes while this component is mounted, the UI won't update. This is currently safe because the value is only set before navigating here and cleared inside the mutation, but it's worth noting for future maintainability. If re-render reactivity is ever needed, switch to useQueryClient().getQueryData inside the mutation or useQuery with enabled: false.
| useEffect(() => { | ||
| if (isPending && providers?.manteca.status === "ACTIVE" && currency) { | ||
| router.replace({ pathname: "/add-funds/ramp", params: { currency } }); | ||
| } | ||
| }, [isPending, providers, currency, router]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Auto-navigation effect: if manteca status is not ACTIVE, user sees Close after timeout with no retry.
When providers?.manteca.status is not "ACTIVE" (e.g., still "ONBOARDING" or "PENDING"), the effect doesn't navigate, and once timedOut && !isFetching the user only sees the "Close" button. There's no polling or retry mechanism — the user must manually close and re-enter the flow.
This may be intentional (the screen says "We're verifying your information"), but if the expectation is that manteca status transitions to ACTIVE within seconds, consider adding refetchInterval to the providers query so it polls until active or a max attempt is reached.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Localization