Conversation
🦋 Changeset detectedLatest commit: a0ef4e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
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:
WalkthroughThe mobile app centralizes KYC status handling through ChangesKYC review and failure flow
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to This change adds KYC review and failure experiences, but users may still be routed to inappropriate onboarding actions after verification, and the new states are not reliably covered by the local flow. Resolve the outstanding routing and validation issues before merging. Sequence Diagram(s)sequenceDiagram
participant User
participant AddFunds
participant useKYC
participant getKYCStatus
participant startKYC
participant GettingStarted
User->>AddFunds: Start bank transfer
AddFunds->>useKYC: Read KYC state
useKYC->>getKYCStatus: Fetch status
getKYCStatus-->>useKYC: Return status code
AddFunds->>startKYC: Begin verification when required
startKYC-->>AddFunds: Return complete or blocked
AddFunds->>GettingStarted: Redirect for review or blocked state
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bcac7d71b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/persona.ts (1)
281-281: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClassify the post-inquiry status before returning
complete.When the refetch returns
"processing"or"bad kyc", this returns"complete".src/components/add-funds/AddFunds.tsxroutes that non-approved complete result to home instead of getting started. Apply the same blocked classification used beforestartKYC()tokyc.Proposed fix
const kyc = await queryClient.fetchQuery<KYCStatus>({ queryKey: ["kyc", "status"], staleTime: 0 }); + const code = "code" in kyc ? kyc.code : undefined; + if (code !== "ok" && code !== "legacy kyc" && code !== "not started" && code !== "no kyc") { + return { status: "blocked", kyc }; + } return { status: "complete", kyc };
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 1ceb862d-9310-47bd-92fd-8108dfc227d4
📒 Files selected for processing (18)
.changeset/six-peas-beg.mdsrc/components/add-funds/AddFunds.tsxsrc/components/card/Card.tsxsrc/components/getting-started/GettingStarted.tsxsrc/components/getting-started/Step.tsxsrc/components/home/GettingStarted.tsxsrc/components/home/Home.tsxsrc/components/send-funds/SendFunds.tsxsrc/components/shared/FundingAlert.tsxsrc/components/shared/InfoAlert.tsxsrc/components/swaps/Swaps.tsxsrc/i18n/es-AR.jsonsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/persona.tssrc/utils/server.tssrc/utils/useKYC.tssrc/utils/useOnboardingSteps.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1291 +/- ##
==========================================
+ Coverage 73.40% 74.87% +1.46%
==========================================
Files 276 279 +3
Lines 13463 13554 +91
Branches 4711 4759 +48
==========================================
+ Hits 9883 10149 +266
+ Misses 3241 3066 -175
Partials 339 339
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/persona.ts (1)
274-281: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClassify the post-inquiry KYC status before returning success. After
startKYC()resolves, the mutation fetches the status but always returnsstatus: "complete". When the API returnsprocessingorbad kyc,GettingStarted.tsxandSwaps.tsxroute the user home instead of to blocked onboarding. Returnblockedunless the fetched code isokorlegacy kyc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 5be8df4f-194e-4a8b-9579-7da075bbcca4
📒 Files selected for processing (10)
.changeset/six-peas-beg.mdsrc/components/card/Card.tsxsrc/components/getting-started/GettingStarted.tsxsrc/components/home/GettingStarted.tsxsrc/components/swaps/Swaps.tsxsrc/i18n/es.jsonsrc/i18n/pt.jsonsrc/utils/server.tssrc/utils/useKYC.tssrc/utils/useOnboardingSteps.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
franm91
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Team
Run ID: 9b5fea01-0d4f-4d71-8997-1655394bc61e
📒 Files selected for processing (2)
src/components/getting-started/GettingStarted.tsxsrc/utils/useKYC.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ef4e95e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,16 @@ | |||
| appId: ${APP_ID ?? "app.exactly"} | |||
There was a problem hiding this comment.
The new assertOnboarding.yaml filename uses camelCase, although the repository’s YAML rules require lowercase kebab-case filenames. Rename it to assert-onboarding.yaml and update the reference in flows/local.yaml so the new flow follows the enforced convention. .agents/rules/yaml.mdL16-L19
Useful? React with 👍 / 👎.
| const { data, error, isFetched, isFetching, refetch } = useQuery<KYCStatus>({ queryKey: ["kyc", "status"], enabled }); | ||
| const code = data && "code" in data ? data.code : undefined; | ||
| const approved = code === "ok" || code === "legacy kyc"; | ||
| const status: KYCState = approved |
There was a problem hiding this comment.
The newly extracted status value is consumed only once, as the status property of the returned object. Inline this conditional at that property instead of adding a single-use binding, as required by the repository’s extraction convention.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
|
|
||
| export default function useOnboardingSteps({ hasKYC, isDeployed }: { hasKYC: boolean; isDeployed: boolean }) { | ||
| export default function useOnboardingSteps({ kyc, isDeployed }: { isDeployed: boolean; kyc: KYCState }) { | ||
| return useMemo(() => { |
There was a problem hiding this comment.
The steps array introduced here is referenced only once by the final spread-and-sort expression. Inline the array at that return site rather than retaining a single-use intermediate binding, in accordance with the repository’s explicit extraction rule.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
| const { t } = useTranslation(); | ||
| const toast = useToastController(); | ||
| const { currentStep, completedSteps } = useOnboardingSteps({ hasKYC, isDeployed }); | ||
| const steps = useOnboardingSteps({ kyc, isDeployed }); |
There was a problem hiding this comment.
The steps binding is used only once to call find, so this new extraction adds an unnecessary layer of indirection. Inline useOnboardingSteps({ kyc, isDeployed }).find(...) into the step initializer to follow the repository’s single-use rule.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
| }; | ||
| } | ||
|
|
||
| export type KYCState = "approved" | "failed" | "pending" | "review"; |
There was a problem hiding this comment.
The new KYCState type uses State, which the repository’s naming guidance explicitly identifies as a meaningless suffix. Rename this type to a precise description of the represented KYC classification and update its consumers so the new abstraction does not establish a prohibited naming pattern.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: f523bf97-214c-44f0-b294-d0848db56fd8
📒 Files selected for processing (2)
.maestro/flows/local.yaml.maestro/subflows/assertOnboarding.yaml
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| - runFlow: | ||
| file: ../subflows/mint.yaml | ||
| env: { asset: ETH, to: "${output.account}", amount: "0.1" } | ||
| - runFlow: ../subflows/assertOnboarding.yaml |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge exactly/exa /tmp/coderabbit-repo-knowledge/exactly-exa-f26ca19b/learnings
Length of output: 25379
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- .maestro/flows/local.yaml ---'
cat -n .maestro/flows/local.yaml
printf '%s\n' '--- candidate subflows and references ---'
find .maestro -type f -name '*.yaml' -print | sort
printf '%s\n' '--- referenced flow definitions ---'
for f in .maestro/subflows/assertOnboarding.yaml .maestro/subflows/verifyIdentity.yaml .maestro/subflows/getAccount.yaml; do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f"
else
echo "MISSING $f"
fi
done
printf '%s\n' '--- references to account and KYC setup ---'
rg -n -i --glob '*.yaml' 'getAccount|verifyIdentity|assertOnboarding|kyc|unverified|approved|review|failed|reset|mock|awaiting-activation|View all steps' .maestroRepository: exactly/exa
Length of output: 8589
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- .maestro/flows/local.yaml ---'
cat -n .maestro/flows/local.yaml
printf '%s\n' '--- direct subflow definitions ---'
for f in .maestro/subflows/assertOnboarding.yaml .maestro/subflows/verifyIdentity.yaml .maestro/subflows/getAccount.yaml; do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f"
else
echo "MISSING $f"
fi
done
printf '%s\n' '--- related Maestro references ---'
rg -n -i --glob '*.yaml' 'getAccount|verifyIdentity|assertOnboarding|kyc|unverified|approved|review|failed|reset|mock|awaiting-activation|View all steps' .maestroRepository: exactly/exa
Length of output: 7678
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- account/KYC helper files ---'
git ls-files | rg '(^|/)(getAccount|approveKYC)\.(js|ts)$|(^|/)dist/|maestro'
printf '%s\n' '--- helper definitions and configuration ---'
for f in .maestro/dist/getAccount.js .maestro/dist/approveKYC.js .maestro/config.yaml; do
if [ -f "$f" ]; then
echo "### $f"
cat -n "$f"
fi
done
printf '%s\n' '--- all matching helper paths ---'
find . -type f \( -name 'getAccount.js' -o -name 'approveKYC.js' -o -name 'getAccount.ts' -o -name 'approveKYC.ts' \) -printRepository: exactly/exa
Length of output: 1628
🏁 Script executed:
#!/bin/bash
set -eu
for f in .maestro/src/script/getAccount.ts .maestro/src/script/approveKYC.ts .maestro/src/server.ts .maestro/src/anvil.ts; do
echo "### $f"
cat -n "$f"
done
printf '%s\n' '--- helper imports and account/KYC endpoints ---'
rg -n -i 'getAccount|approveKYC|credential|kyc|panda|manteca|account' .maestro/src .maestro/package.jsonRepository: exactly/exa
Length of output: 14589
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- local Maestro invocation and environment setup ---'
rg -n -i --glob '!node_modules/**' --glob '!dist/**' 'flows/local|E2E_MNEMONIC|maestro|clearState|anvil|reset.*(db|database|kyc)|persona' package.json pnpm-workspace.yaml .github .maestro README.md docs 2>/dev/null || true
printf '%s\n' '--- files containing local flow setup ---'
git ls-files | rg -i '(^|/)(readme|docker-compose|compose|package\.json|makefile|justfile|.*maestro.*|.*e2e.*|.*test.*)\.(md|yaml|yml|json|ts|js|sh)$' | head -200Repository: exactly/exa
Length of output: 12899
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- README local E2E instructions ---'
sed -n '20,45p' README.md
printf '%s\n' '--- CI setup around E2E ---'
sed -n '55,115p' .github/workflows/test.yaml
printf '%s\n' '--- E2E server setup references ---'
rg -n -i 'reset|seed|database|docker|persona|kyc|mnemonic|e2e' server/script/e2e.ts server/test/e2e.ts server/test/docker-compose.yaml src/utils/e2e.tsRepository: exactly/exa
Length of output: 5741
Establish the remote KYC precondition before this assertion flow.
getAccount.ts derives a deterministic account and does not reset KYC. getAccount.yaml only populates missing output values. Since assertOnboarding.yaml runs before verifyIdentity.yaml, a persisted KYC state can make the assertion fail. Reset/mock KYC or select a known unverified account before invoking assertOnboarding.yaml.
| --- | ||
| - tapOn: View all steps | ||
| - assertVisible: Remaining steps | ||
| - assertVisible: Verify your identity |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy lift
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge exactly/exa /tmp/coderabbit-repo-knowledge/exactly-exa-f26ca19b/learnings /tmp/coderabbit-repo-knowledge/exactly-exa-f26ca19b/conventions
Length of output: 38773
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n .maestro/subflows/assertOnboarding.yaml
printf '%s\n' '--- related Maestro files ---'
find .maestro -maxdepth 3 -type f -print | sort
printf '%s\n' '--- identity/KYC references ---'
rg -n -i --glob '*.yaml' --glob '*.yml' 'verify your identity|identity|kyc|review|failed|failure' .maestroRepository: exactly/exa
Length of output: 2768
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- verify identity subflow ---'
cat -n .maestro/subflows/verifyIdentity.yaml
printf '%s\n' '--- local flow ---'
cat -n .maestro/flows/local.yaml
printf '%s\n' '--- production flow ---'
cat -n .maestro/flows/production.yaml
printf '%s\n' '--- web flow ---'
cat -n .maestro/flows/web.yaml
printf '%s\n' '--- relevant source labels and state names ---'
rg -n -i 'verify your identity|identity verification|kyc|under review|review|failed|rejected|declined' src server .maestro --glob '*.{ts,tsx,yaml,yml,json}'Repository: exactly/exa
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -eu
cat -n .maestro/subflows/verifyIdentity.yaml
cat -n .maestro/flows/local.yaml
cat -n .maestro/flows/production.yaml
cat -n .maestro/flows/web.yaml
rg -n -i 'verify your identity|identity verification|kyc|under review|review|failed|rejected|declined' src server .maestro --glob '*.{ts,tsx,yaml,yml,json}'Repository: exactly/exa
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- onboarding state mapping ---'
cat -n src/utils/useOnboardingSteps.ts
printf '%s\n' '--- onboarding rendering ---'
sed -n '1,190p' src/components/getting-started/GettingStarted.tsx
printf '%s\n' '--- KYC state mapping ---'
cat -n src/utils/useKYC.ts
printf '%s\n' '--- KYC approval helper ---'
cat -n .maestro/src/script/approveKYC.ts
printf '%s\n' '--- swap review/failure rendering ---'
sed -n '460,580p' src/components/swaps/Swaps.tsx
printf '%s\n' '--- changed-file summary ---'
git diff --statRepository: exactly/exa
Length of output: 14497
Cover the review and failed KYC states.
assertOnboarding.yaml checks only the pending Verify your identity state. Add deterministic setup and assertions for IN REVIEW and VERIFICATION FAILED, including their messages and actions. The local flow currently invokes only the approved-state helper, approveKYC.js.
2 participants
Summary by CodeRabbit