-
Notifications
You must be signed in to change notification settings - Fork 5
fix: onboarding-flow-drawers #662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughReplaces Drawer-based onboarding, registration, and verification UIs with full-page overlays/modals and explicit step state machines; consolidates conditional branches (loading, preVerified, verificationSuccess, hardware checks, PIN/biometrics) and adds SSE lifecycle handling for verification. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Verification Modal
participant Key as KeyService
participant VerifAPI as Verification API
participant SSE as SSE Stream
User->>UI: Open verification modal
UI->>Key: prepare/ensure keys
Key-->>UI: keys ready
UI->>VerifAPI: submit document/selfie
VerifAPI->>SSE: start SSE updates
SSE-->>UI: stream events (status updates)
UI->>User: render status (loading / success / failure)
User->>UI: Close / Back
UI->>SSE: closeEventStream
SSE-->>SSE: stream closed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
45-75: Clean up the test hardware key after verification.The test key created at line 45 (
hardware-test-${Date.now()}) is never deleted after verification completes. Over time, this could accumulate stale test keys in storage.🧹 Suggested fix: Clean up test key after verification
Add cleanup logic after the hardware check succeeds:
// Hardware works! Clean up test key and proceed console.log("Hardware keys are working"); + // Clean up the test key + try { + await globalState.keyService.deleteKey(testKeyId, "onboarding"); + console.log("Test key cleaned up"); + } catch (cleanupError) { + console.warn("Failed to clean up test key:", cleanupError); + } checkingHardware = false;
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte:
- Around line 341-477: The overlay lacks a way to dismiss it; add a visible
back/close button that calls the component's existing pane-close logic (toggle
the isPaneOpen state or call the same handler used to open/close the pane) so
users can exit onboarding without clicking Next/Continue. Import HugeiconsIcon
from @hugeicons/svelte and render a small icon button in the overlay header
(above the img or in the footer area) that triggers a new onClose function or
directly sets isPaneOpen = false; ensure the button is keyboard accessible and
placed alongside existing actions (handleNext, handlePreVerified,
handleFinalSubmit) so it cleanly closes the overlay and returns users to the
main onboarding page.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: The EID wallet (`infrastructure/eid-wallet`) is a mobile application, so keyboard event handlers (like ESC key handlers) are not needed for dialog/modal components.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (2)
158-164: LGTM!The button attribute restructuring maintains the same functionality while improving code formatting and clarity. The
variantandcallbackattributes are properly configured.Also applies to: 185-189
194-312: LGTM!The full-screen overlay structure properly replaces the Drawer component with appropriate dismissal mechanisms. The back buttons (lines 264-266, 297-299) correctly close the overlay by setting
showDrawer = false, and all conditional branches (PIN confirmation, biometrics setup, completion) have proper navigation controls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (3)
5-6: Remove unused imports.
DrawerandButtonare imported but not used in this file. The drawer-based UI has been replaced with the custom overlay.🧹 Suggested cleanup
-import { ButtonAction, Drawer, InputPin } from "$lib/ui"; -import * as Button from "$lib/ui/Button"; +import { ButtonAction, InputPin } from "$lib/ui";
199-205: Consider resetting state when closing the overlay.The back button closes the overlay without resetting
isBiometricScreenorisBiometricsAdded. If the overlay is reopened (e.g., if the user re-enters PINs), it would display stale state.♻️ Suggested fix to reset state on close
<button - onclick={() => (showDrawer = false)} + onclick={() => { + showDrawer = false; + isBiometricScreen = false; + isBiometricsAdded = false; + }} class="flex items-center gap-2 text-black-500 mb-6 py-2" >
268-279: Apply consistent state reset for all "Back" buttons in the overlay.The footer Back buttons (lines 272-274 and 305-307) should also reset the overlay state for consistency with the suggested fix above.
♻️ Suggested fix
Consider extracting a helper function to ensure consistent behavior:
<script lang="ts"> // Add near other handlers const closeOverlay = () => { showDrawer = false; isBiometricScreen = false; isBiometricsAdded = false; }; </script>Then use
callback={closeOverlay}for all overlay Back buttons.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (4)
134-166: LGTM!The first step PIN entry UI is well-structured with appropriate error handling and navigation.
179-184: Consider whether "Back" should return to the first step.The Back button navigates to
/onboardinginstead of returning to the first PIN entry step. If the intended UX is to let users correct their initial PIN entry, consider resetting tofirstStep = trueinstead.
294-298: Good UX: Biometrics unavailable feedback.Clear messaging when biometrics are unavailable, and the "Set up" button is appropriately disabled.
194-319: Overlay implementation aligns with PR objectives.The conversion from drawer-based UI to full-screen overlay is well-executed. The overlay structure with fixed positioning, scrollable content area, and sticky footer provides a good mobile experience. The conditional rendering for different flow states (PIN success → biometrics setup → completion) is clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Line 402: When closing the Veriff modal (where onclick currently sets
showVeriffModal = false) also reset the verification state by setting verifStep
= 0, clearing status (e.g., status = null or ''), and resetting any document
upload state variables and flags (clear uploaded files, front/back image
variables, progress/error flags, etc.). Replace the inline onclick with or call
a dedicated close handler (e.g., closeVeriffModal) that performs showVeriffModal
= false and the above resets so reopening the modal always starts fresh.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (3)
13-13: Remove unused Drawer import.The
Drawercomponent is no longer used after converting to a full-screen modal. Remove this import to keep the code clean.♻️ Proposed fix
-import Drawer from "$lib/ui/Drawer/Drawer.svelte";
399-407: Add ARIA attributes for modal accessibility.The modal lacks semantic accessibility attributes. Consider adding:
role="dialog"andaria-modal="true"on the container (line 399)aria-label="Close verification"on the back button (line 401)aria-labelledbypointing to a heading element to describe the modal purposeThese attributes help screen readers properly announce the modal and its controls.
♻️ Proposed accessibility improvements
- <div class="fixed inset-0 z-50 bg-white flex flex-col h-full"> + <div + role="dialog" + aria-modal="true" + aria-labelledby="verification-title" + class="fixed inset-0 z-50 bg-white flex flex-col h-full" + > <div class="flex-none px-[5vw] pt-[4svh]"> <button onclick={() => (showVeriffModal = false)} + aria-label="Close verification" class="flex items-center gap-2 text-black-500 py-2" >
399-399: Consider preventing body scroll when modal is open.When the full-screen modal is displayed, the background page can still be scrolled on some devices, which may cause a confusing user experience. Consider adding a body scroll lock when
showVeriffModalis true.Example implementation using Svelte effects:
$effect(() => { if (showVeriffModal) { document.body.style.overflow = 'hidden'; return () => { document.body.style.overflow = ''; }; } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 401-406: The back button is icon-only (uses HugeiconsIcon with
ArrowLeft01Icon and toggles showVeriffModal) and lacks accessible labeling; add
an explicit accessible name by adding an aria-label (e.g., aria-label="Back" or
aria-label="Close verification modal") to the button element or include
visually-hidden text inside the button (e.g., a span with class "sr-only") so
screen readers can announce its purpose while preserving the icon UI.
- Around line 398-476: The modal opened when showVeriffModal is true is missing
ARIA and focus management: add role="dialog" and aria-modal="true" to the modal
container div, add aria-labelledby pointing to a unique id (e.g.,
id="verification-title") on the visible <h3> title(s) inside the content
branches for $status, implement a focus trap on the modal container (use a
Svelte action like use:focusTrap or svelte-focus-trap attached to the same
container) to keep keyboard focus inside while open, and restore focus to the
element that opened the modal when showVeriffModal becomes false (store the
opener element before opening and call .focus() on close); ensure any references
use the existing symbols showVeriffModal, the modal container div, the <h3>
title elements, and the close/back handlers (onclick and handleContinue) so the
changes integrate with the current component.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
13-13: Remove unused Drawer import.The Drawer component is no longer used after the refactor to a full-screen modal. Remove this import to clean up the code.
♻️ Suggested fix
-import Drawer from "$lib/ui/Drawer/Drawer.svelte";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
402-402: > Likely an incorrect or invalid review comment.
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 314-336: The duplicate branch trusts websocketData?.w3id without
robust validation; update the check around websocketData and existingW3id to
verify websocketData came from the expected EventSource and that existingW3id is
a non-empty, well-formed identifier before using it, returning a clear error or
retrying if invalid; if valid, proceed to call axios.get with
PUBLIC_REGISTRY_URL and existingW3id and only then update
globalState.vaultController.profileCreationStatus and
globalState.vaultController.vault (ensure response.data.uri exists before
assigning).
- Line 257: handleContinue is currently a no-op until its real implementation is
assigned inside onMount, which can allow a race if the button is clicked early;
change initialization to either set handleContinue to a guarded wrapper that
awaits a ready flag or throws if called before assignment, or add an
"initialized" reactive boolean set true at the end of onMount and bind the
button's disabled state to it (or conditionally render the button) so
handleContinue cannot be invoked until onMount completes; update references to
handleContinue and the button click handler accordingly.
- Around line 150-177: The EventSource opened in watchEventStream is never
closed, leaking resources; modify watchEventStream to save the EventSource
instance to a module-level variable (e.g., currentEventSource), ensure any
existing currentEventSource is closed before creating a new one, add a
closeEventStream() function that calls currentEventSource.close() and nulls it,
and invoke closeEventStream() both when the modal is closed (replace the current
modal-close handler) and inside Svelte's onDestroy to guarantee cleanup on
navigation/unmount.
- Around line 284-365: The hardcoded setTimeout in handleContinue (setTimeout(()
=> { goto("/register"); }, 10_000)) forces a 10s wait; replace it by observing
globalState.vaultController.profileCreationStatus (and/or websocketData) and
call goto("/register") immediately when profileCreationStatus becomes "success"
(or when the resolved w3id/uri is available), optionally implementing a short
timeout fallback/poll with visible user feedback and an explicit "Continue
anyway" action so users can leave early; remove the fixed 10_000ms delay and
ensure navigation only happens once the condition is met.
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
112-148: Consider notifying users of verification failures.When verification fails (line 134), the error is only logged to console. Users redirected to onboarding (lines 122, 145) receive no explanation of what went wrong, which could create confusion.
🔔 Proposed enhancement with user notification
Consider adding a toast/notification before redirecting:
} catch (error) { console.error("Failed to start verification:", error); + // Show user-friendly error message + globalState?.notificationService?.showError( + "Verification failed to start. Redirecting to setup..." + ); // If verification fails due to key issues or any initialization error, go back to onboarding const errorMessage =
472-478: Clarify the "Back" button label.The button labeled "Back" (line 477) navigates to
/onboarding(line 475), which is not going back in history but rather abandoning the current verification session. This could confuse users who expect it to return to the previous step.📝 Suggested clarification
- Back + CancelOr if you want to be more explicit:
- Back + Exit Verification
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
381-404: Good progressive disclosure of hardware check status.The template appropriately handles three states: checking hardware (spinner), hardware not supported (error message), and hardware supported (action button). This provides clear feedback to users throughout the initialization process.
406-494: Modal structure successfully replaces drawer UI.The full-screen modal implementation addresses the original issue (#649) by replacing the drawer-based UI that had zoom-related layout problems. The modal uses proper semantic HTML with
role="dialog"andaria-modal, maintains the same verification flow logic, and provides a clean full-screen experience.
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
161-180: Wrap JSON.parse in try/catch to prevent unhandled exceptions.If the server sends malformed data,
JSON.parse(e.data)will throw and crash the message handler. Also, closing the EventSource on any error prevents the browser's built-in reconnection mechanism from working for transient network issues.🐛 Proposed fix
eventSource.onmessage = (e) => { - const data = JSON.parse(e.data as string); - if (!data.status) console.log(data); - console.log("STATUS", data); - status.set(data.status); - reason.set(data.reason); - person = data.person; - document = data.document; - websocketData = data; // Store the full websocket data - if (data.status === "resubmission_requested") { - DocFront.set(null); - DocBack.set(null); - SelfiePic.set(null); + try { + const data = JSON.parse(e.data as string); + if (!data.status) console.log(data); + console.log("STATUS", data); + status.set(data.status); + reason.set(data.reason); + person = data.person; + document = data.document; + websocketData = data; + if (data.status === "resubmission_requested") { + DocFront.set(null); + DocBack.set(null); + SelfiePic.set(null); + } + verifStep.set(3); + } catch (parseError) { + console.error("Failed to parse SSE message:", parseError); } - verifStep.set(3); }; + eventSource.onerror = (error) => { console.error("EventSource error:", error); - eventSource?.close(); + // Only close on terminal errors; browser auto-reconnects for transient failures + if (eventSource?.readyState === EventSource.CLOSED) { + eventSource = null; + } };
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 466-472: Replace the awkward heading in the resubmission branch by
changing the text in the {:else if $status === "resubmission_requested"} block
to a grammatically correct sentence like "Your verification failed for the
following reason:" and keep displaying {$reason}; also ensure the UI still
provides navigation when status is "declined" by removing or adjusting the
conditional that currently checks `$status !== "declined"` (so action buttons or
a navigation link are rendered for the "declined" case) — update the template
branches around {$status}, "resubmission_requested", "declined", and {$reason}
to show a clear message and include navigation/actions for declined users.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (1)
411-429: Consider adding focus management for improved accessibility.The modal has good semantic attributes (
role="dialog",aria-modal,aria-label). For better accessibility, consider moving focus to the modal when it opens and returning focus when closed. Since this is a mobile app context (per project learnings, ESC handlers are intentionally omitted), focus trapping may be less critical but programmatic focus management improves screen reader experience.♻️ Optional: Focus management example
<script> let modalRef: HTMLDivElement; $effect(() => { if (showVeriffModal && modalRef) { modalRef.focus(); } }); </script> <!-- In the modal div, add tabindex and bind: --> <div bind:this={modalRef} tabindex="-1" role="dialog" ... >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2025-06-07T04:59:24.520Z
Learnt from: Sahil2004
Repo: MetaState-Prototype-Project/prototype PR: 193
File: platforms/metagram/src/lib/store/store.svelte.ts:0-0
Timestamp: 2025-06-07T04:59:24.520Z
Learning: In the MetaState prototype project, prefer using centralized type definitions from `$lib/types` over importing interfaces from component files for better type organization and to avoid circular dependencies.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (4)
14-16: LGTM!The new imports are correctly added for the modal's back icon and SSE cleanup lifecycle hook.
183-188: LGTM!The cleanup helper properly closes the EventSource and nulls the reference to prevent stale access.
368-371: LGTM!Good addition of
onDestroyto ensure the EventSource connection is properly cleaned up when the component unmounts, preventing resource leaks.
438-444: LGTM!The loading state display provides appropriate feedback during eVault generation, and the subsequent navigation handles the state transition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (2)
277-280: 10-second delay before redirect with mismatched loading message.The
setTimeoutintroduces a 10-second wait before navigating to/register. Whileloadingremainstrue, the spinner message at line 365 says "Generating your eName" which is incorrect for this post-verification phase—the eName has already been generated.Consider either adding a more appropriate message for this state (e.g., "Setting up your profile...") or reducing the delay if it's not strictly necessary.
69-76: Test key is not cleaned up after successful hardware check.The comment at line 69 says "Clean up test key and proceed" but no cleanup code follows. The test key
hardware-test-${Date.now()}is created viaensureKey()and persisted in the key service's contexts, but is never removed after confirming hardware works.The
KeyServiceclass does not currently provide a method to delete individual keys. Consider either:
- Adding a
deleteKey()method toKeyServiceand calling it after successful hardware verification- Reusing a fixed test key ID that gets overwritten on each check instead of generating unique keys
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (3)
14-15: Unused icon imports.
ArrowLeft01IconandHugeiconsIconare imported but neither is used in the template. The Back button at lines 441-449 usesButtonActionwith text instead of an icon.If the icons are no longer needed, remove these imports. If you intended to use an icon for the back navigation, the implementation appears to be missing.
♻️ Suggested cleanup
-import { ArrowLeft01Icon } from "@hugeicons/core-free-icons"; -import { HugeiconsIcon } from "@hugeicons/svelte";
342-344: Consider adding ARIA attributes for accessibility.The full-screen overlay functions as a modal dialog but lacks accessibility attributes. Adding
role="dialog"andaria-modal="true"would improve screen reader support without requiring keyboard event handlers (which should be omitted for mobile per project conventions).♻️ Suggested improvement
{#if isPaneOpen} - <div class="fixed inset-0 z-50 bg-white overflow-y-auto"> + <div class="fixed inset-0 z-50 bg-white overflow-y-auto" role="dialog" aria-modal="true">Based on learnings, ESC-key handlers should be omitted for mobile contexts in this module.
352-358: Consider using Tailwind color class instead of hardcoded hex.The error background uses hardcoded
bg-[#ff3300]. For consistency with the design system, consider using a Tailwind red variant likebg-red-500orbg-red-600.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (3)
437-493: LGTM!The button action logic correctly handles all state combinations: loading, hardware checking, pre-verification flow (both input and success states), hardware error recovery, and default onboarding flow. The conditional variant styling for the disabled state provides good visual feedback.
385-399: LGTM!Input fields are properly bound to state variables with appropriate styling and placeholder text. The transition effect on focus is a nice touch.
345-435: Well-structured overlay content.The conditional rendering cleanly handles all UI states (loading, hardware checking, pre-verification, hardware error, and default). The flex layout with
growon the article andflex-noneon the button container ensures proper spacing. This successfully implements the drawer-to-full-page migration objective from issue #649.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte:
- Line 411: The Shadow component is being passed a color string with a trailing
semicolon ("rgb(142, 82, 255);"); remove the trailing semicolon so the prop is
"rgb(142, 82, 255)" to avoid invalid CSS/color parsing — update the Shadow
invocation (Shadow size={40} color="rgb(142, 82, 255);") to use the corrected
string value.
- Line 370: The Shadow component usage passes an invalid CSS color string with a
trailing semicolon ("rgb(142, 82, 255);"); remove the trailing semicolon in the
color prop on the Shadow component (Shadow size={40} color="rgb(142, 82, 255)");
ensure no other color prop values include stray semicolons so the CSS color is
valid.
- Around line 43-76: The test key is created with a timestamped ID (testKeyId =
`hardware-test-${Date.now()}`) and never deleted, causing orphaned keys; replace
the timestamped ID with a fixed reuseable ID (e.g., "hardware-test") in the
onboarding flow so repeated runs reuse the same test key instead of accumulating
new ones, i.e., update the code that constructs testKeyId before calling
globalState.keyService.ensureKey to use a constant string; alternatively, if you
prefer deletion, add a delete() method to the KeyManager interface and implement
it in HardwareKeyManager and SoftwareKeyManager and expose
KeyService.deleteKey(), then call that to remove the test key after verifying
hardware — choose one approach and apply consistently to ensure no orphaned test
keys remain.
In @infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte:
- Around line 198-204: The back button only sets showDrawer = false and
therefore leaves isBiometricScreen and isBiometricsAdded in stale states; update
the onclick handler for this button (and the other back buttons that currently
only set showDrawer) to also reset isBiometricScreen = false and
isBiometricsAdded = false so the overlay fully resets when closed, ensuring all
handlers that close the drawer (e.g., the other back buttons in the biometric
flow) perform the same three assignments.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
348-350: Consider adding basic accessibility attributes to the overlay.While ESC-key handlers are correctly omitted per mobile context requirements, adding basic ARIA attributes would improve screen reader support.
Suggested improvement
{#if isPaneOpen} - <div class="fixed inset-0 z-50 bg-white overflow-y-auto"> + <div + class="fixed inset-0 z-50 bg-white overflow-y-auto" + role="dialog" + aria-modal="true" + aria-labelledby="overlay-title" + >infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (2)
206-225: Consider extracting repeated icon container to a snippet or component.The icon container with decorative lines pattern is duplicated (lines 206-225 and 232-251). While acceptable for two instances, extracting it could improve maintainability.
Example using Svelte snippet
{#snippet iconBox(icon)} <div class="relative bg-gray w-[72px] h-[72px] rounded-3xl flex justify-center items-center mb-6"> <span class="relative z-1"> <HugeiconsIcon {icon} color="var(--color-primary)" /> </span> <img class="absolute top-0 start-0" src="/images/Line.svg" alt="line" /> <img class="absolute top-0 start-0" src="/images/Line2.svg" alt="line" /> </div> {/snippet} <!-- Usage --> {@render iconBox(CircleLock01Icon)}
193-195: Consider adding ARIA attributes for accessibility.Similar to the onboarding overlay, basic accessibility attributes would benefit screen reader users.
Suggested improvement
{#if showDrawer} - <div class="fixed inset-0 z-50 bg-white overflow-y-auto"> + <div + class="fixed inset-0 z-50 bg-white overflow-y-auto" + role="dialog" + aria-modal="true" + >
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (4)
1-28: LGTM on imports and state declarations.The imports and state variables are well-organized and appropriately typed for the overlay-based onboarding flow.
84-179: LGTM on helper functions and navigation handlers.The key management initialization, context switching, and navigation logic are well-structured with appropriate error handling and user feedback.
428-440: Clear informational content for the default state.The explanatory text effectively communicates the verification flow to users.
443-499: Well-organized button logic with proper state-based rendering.The conditional button rendering correctly handles all flow states (preVerified, verificationSuccess, showHardwareError) with appropriate actions.
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (4)
1-13: LGTM on imports.The imports are appropriate for the PIN setup and biometrics flow, with proper icon library usage.
27-130: LGTM on PIN and biometrics handling.The PIN validation, confirmation flow, and biometrics setup logic are well-implemented with proper error handling and state management. Based on learnings, the PIN overwrite behavior in
setOnboardingPinis intentional.
133-191: Clean PIN entry UI with appropriate validation feedback.The two-step PIN flow with visual feedback for errors is well-structured.
265-316: Well-structured button controls with proper state handling.The conditional rendering correctly handles PIN confirmation, biometrics setup, and completion states. The disabled state and unavailability message for biometrics provide good user feedback.
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
Outdated
Show resolved
Hide resolved
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
340-348: Add accessibility attributes for consistency with the verification modal.The verification modal at
verify/+page.svelteincludesrole="dialog",aria-modal="true", andaria-labelattributes. This overlay should follow the same pattern for accessibility consistency.♻️ Suggested fix
{#if isPaneOpen} - <div class="fixed inset-0 z-50 bg-white overflow-y-auto"> + <div + role="dialog" + aria-modal="true" + aria-label="Onboarding" + class="fixed inset-0 z-50 bg-white overflow-y-auto" + > <div class="min-h-full flex flex-col p-6">infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (3)
177-181: Consider surfacing SSE connection errors to the user.The
onerrorhandler closes the stream but doesn't notify the user or provide recovery options. Users might be unaware that verification status updates have stopped.♻️ Optional enhancement
eventSource.onerror = (error) => { console.error("EventSource error:", error); eventSource?.close(); + // Optionally notify the user + // status.set("connection_error"); };
279-287: Remove or document the commented-out code.The commented-out key initialization calls are deferred to
handleVerification(), but leaving dead code is a maintenance burden. Either remove these lines or add a comment explaining why they're preserved.♻️ Suggested fix - remove commented code
// Initialize key manager and check if default key pair exists - try { - // await initializeKeyManager(); - // await ensureKeyForVerification(); - } catch (error) { - console.error("Failed to initialize keys for verification:", error); - // If key initialization fails, redirect back to onboarding - await goto("/onboarding"); - return; - } + // Key initialization is deferred to handleVerification() when user clicks "I'm ready"
496-498: Consider adding a comment to clarify the purpose of the empty placeholder.The empty
<div>maintains layout consistency, but a brief comment would improve readability.♻️ Suggested improvement
{:else if $verifStep <= 2} + <!-- Placeholder to maintain two-column layout --> <div class="w-full h-full"></div> {/if}infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (2)
21-34: Review the back navigation from PIN_DONE step.Navigating from
PIN_DONEback toREPEATseems inconsistent—the PIN has already been confirmed and saved. Consider whether going back fromPIN_DONEshould either:
- Go to
CREATEand reset both PINs, or- Skip directly to onboarding since the PIN is already set
Current behavior might confuse users who expect to re-enter their PIN but find it already saved.
99-101: Consider clearing the error state when the user starts typing.The
isErrorflag persists even as the user enters a new PIN. Consider clearing it whenpinchanges to provide better UX feedback.♻️ Suggested enhancement
Add an effect to clear the error when the user starts typing:
$effect(() => { if (pin.length > 0 && isError) { isError = false; } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2025-06-07T04:59:24.520Z
Learnt from: Sahil2004
Repo: MetaState-Prototype-Project/prototype PR: 193
File: platforms/metagram/src/lib/store/store.svelte.ts:0-0
Timestamp: 2025-06-07T04:59:24.520Z
Learning: In the MetaState prototype project, prefer using centralized type definitions from `$lib/types` over importing interfaces from component files for better type organization and to avoid circular dependencies.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (12)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (3)
11-11: LGTM!Import change is appropriate for the new overlay-based UI replacing the Drawer component.
41-42: LGTM!The
isFakeflag logic correctly distinguishes between real verification flow (false) and pre-verification demo flow (true).Also applies to: 81-85
435-491: LGTM!The button logic correctly handles all UI states: loading, pre-verified (with/without verification success), hardware error, and default flow. The disabled state for empty inputs is properly implemented.
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (4)
14-16: LGTM!Import additions are appropriate:
HugeiconsIconfor UI controls andonDestroyfor SSE cleanup.
183-188: LGTM!Proper SSE cleanup pattern:
closeEventStreamnullifies after closing, andonDestroyensures cleanup on component unmount, preventing resource leaks.Also applies to: 369-371
411-417: LGTM!The modal includes proper accessibility attributes (
role="dialog",aria-modal="true",aria-label). The fixed positioning with full height layout is appropriate for a verification flow overlay.
469-477: Verify the Back button navigation behavior.When
verifStep > 2, the Back button navigates to/onboarding. This means users who have completed verification but encounter issues (declined, resubmission) lose their progress. Is this the intended UX, or should they stay on the verify page?infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (5)
11-12: LGTM!The
Steptype with discriminated union provides clear state definitions for the PIN/biometrics flow. This is a clean replacement for the previous flag-based state management.
40-56: LGTM!PIN confirmation logic is sound: mismatch resets to CREATE with both PINs cleared, and successful match saves the PIN. Based on learnings,
setOnboardingPinintentionally allows overwriting existing PINs to handle Android's stale file behavior.
71-76: LGTM!The
$effectcorrectly updates the button variant based on PIN length, providing visual feedback for the confirm action during CREATE and REPEAT steps.
156-187: LGTM!The BIOMETRICS step footer correctly provides three actions: Back, Skip (to finish without biometrics), and Set up (with disabled state when unavailable). The unavailability message provides clear user feedback.
58-63: No change needed—biometric flag-setting is appropriate for registration.The
biometricSupportflag is sufficient. The actual biometric authentication occurs in the login flow whenauthenticate()is invoked after checking both the flag and device availability. The registration step correctly records user preference; device-level biometric enrollment happens during the first login attempt through the native API call. No enrollment step is required in registration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (4)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
161-176: Wrap JSON.parse in try-catch to handle malformed SSE data.If the server sends malformed JSON,
JSON.parsewill throw and crash the message handler, potentially leaving the UI in an inconsistent state without user feedback.🛡️ Proposed fix
eventSource.onmessage = (e) => { - const data = JSON.parse(e.data as string); - if (!data.status) console.log(data); - console.log("STATUS", data); + let data; + try { + data = JSON.parse(e.data as string); + } catch (parseError) { + console.error("Failed to parse SSE data:", parseError); + return; + } + console.log("STATUS", data); status.set(data.status);
363-366: Clear the timeout on component unmount to prevent navigation race.If the user navigates away before the 10-second delay completes, the
setTimeoutcallback will still fire and attemptgoto("/register"), potentially causing unexpected navigation.🛡️ Proposed fix
Add a timeout reference and clear it in
onDestroy:+let navigationTimeout: ReturnType<typeof setTimeout> | null = null; + // In handleContinue: - setTimeout(() => { + navigationTimeout = setTimeout(() => { goto("/register"); }, 10_000); // In onDestroy: onDestroy(() => { closeEventStream(); + if (navigationTimeout) { + clearTimeout(navigationTimeout); + } });infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (2)
313-319: Placeholder links for Terms & Privacy Policy.Both links point to
href="/"which navigates to the home/root page rather than actual legal documents. If these are placeholders, consider either updating them to the correct URLs or adding a TODO comment to track this.
249-278: The 10-second delay should account for actual profile creation timing and provide user feedback.The delay is intentional—the vault setter triggers
createUserProfileInEVault()with exponential backoff retry logic (up to 10 retries, max 10 seconds between attempts). However, the fixed 10-second timeout before navigation is insufficient and unreliable:
- Timing risk: Profile creation can take significantly longer (50+ seconds in worst-case retry scenarios) due to exponential backoff across multiple retries, yet navigation happens after just 10 seconds regardless.
- UX issue: Users see a loading state with no explanation for the wait or progress indication.
- Synchronization gap: The vault setter's async profile creation (lines 489–581 in evault.ts) runs in the background without awaiting in the non-Promise code path.
Consider:
- Properly awaiting profile creation before navigating, or polling its status
- Showing a progress message explaining vault/profile setup is in progress
- Implementing an event-driven approach instead of a fixed timeout
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte:
- Around line 434-457: The template contains duplicate id="verification-title"
in the {$status === "approved"} and {$status === "duplicate"} branches which
creates invalid HTML; remove the id attribute from these headings (or from both
occurrences) so only unique IDs remain — since the id isn't referenced elsewhere
the simplest fix is to delete id="verification-title" from the "approved" and
"duplicate" <h3> elements (or remove it entirely from the template).
🧹 Nitpick comments (5)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
13-14: Unused imports detected.
ArrowLeft01IconandHugeiconsIconare imported but not used in the template. These appear to be leftovers from the drawer-based implementation.🧹 Remove unused imports
-import { ArrowLeft01Icon } from "@hugeicons/core-free-icons"; -import { HugeiconsIcon } from "@hugeicons/svelte";
177-181: Consider providing user feedback when SSE connection fails.The error handler closes the stream but doesn't inform the user. If the connection drops during verification, users may wait indefinitely with no status updates.
💡 Suggested improvement
eventSource.onerror = (error) => { console.error("EventSource error:", error); eventSource?.close(); + // Optionally set an error state to inform the user + reason.set("Connection lost. Please retry verification."); + status.set("connection_error"); };infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (3)
81-85: Consider adding a guard check forglobalStateconsistency.Unlike
handleGetStarted(lines 38-40) which checks and initializesglobalStateif needed,handlePreVerifieddirectly accessesglobalState.userControllerwithout a guard. While unlikely to cause issues in practice (since onMount runs before user interaction), adding a consistent guard would improve defensive coding.Suggested improvement
const handlePreVerified = () => { + if (!globalState) { + globalState = getContext<() => GlobalState>("globalState")(); + } globalState.userController.isFake = true; isPaneOpen = true; preVerified = true; };
191-198: Redundant validation conditions.The checks
!verificationIdandverificationId.length === 0are redundant—an empty string is falsy, so!verificationIdalready covers the zero-length case. Same applies todemoName.Simplified validation
if ( - !verificationId || - !demoName || - verificationId.length === 0 || - demoName.length === 0 + !verificationId?.trim() || + !demoName?.trim() ) { return; }
340-348: Consider adding accessibility attributes to the overlay.The full-screen overlay acts as a modal but lacks accessibility attributes. While ESC-key handling should be avoided per mobile context requirements, consider adding ARIA attributes for screen reader users:
Suggested accessibility improvements
{#if isPaneOpen} - <div class="fixed inset-0 z-50 bg-white overflow-y-auto"> + <div + class="fixed inset-0 z-50 bg-white overflow-y-auto" + role="dialog" + aria-modal="true" + aria-labelledby="onboarding-title" + > <div class="min-h-full flex flex-col p-6">Then add an
idto the main heading within the overlay for the aria-labelledby reference.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2025-06-07T04:59:24.520Z
Learnt from: Sahil2004
Repo: MetaState-Prototype-Project/prototype PR: 193
File: platforms/metagram/src/lib/store/store.svelte.ts:0-0
Timestamp: 2025-06-07T04:59:24.520Z
Learning: In the MetaState prototype project, prefer using centralized type definitions from `$lib/types` over importing interfaces from component files for better type organization and to avoid circular dependencies.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelteinfrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (9)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (4)
369-371: LGTM!Proper SSE cleanup on component destruction prevents memory leaks and orphaned connections.
411-418: LGTM!The modal implementation correctly uses ARIA attributes (
role="dialog",aria-modal="true",aria-label) for accessibility. The full-screen overlay approach aligns with the PR objective of replacing drawer-based UI with full-page flows.
466-480: LGTM!The Back button correctly handles both scenarios: closing the modal for early verification steps and navigating to onboarding when verification is complete. The SSE stream is properly closed when the modal is dismissed.
112-148: LGTM!Error handling appropriately catches key initialization failures and redirects to onboarding. The verification flow correctly sequences modal display and SSE connection setup.
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (5)
1-28: LGTM!State declarations using Svelte 5's
$state()runes are correctly structured, and imports align with the component's requirements for the new overlay-based onboarding flow.
87-152: LGTM!Utility functions are well-structured with consistent error handling patterns. The context-based key management (
getKeyContext) properly distinguishes between pre-verification and onboarding flows.
154-173: LGTM!The
handleNextfunction properly initializes keys before navigation, with appropriate error handling and user feedback via the error state timeout.
435-491: LGTM on button state management.The conditional button rendering correctly handles all state combinations (preVerified, verificationSuccess, showHardwareError) with appropriate actions and visual feedback via variant switching.
43-68: Test key is not cleaned up after successful hardware check.The comment at line 66 states "Clean up test key and proceed," but there's no actual cleanup code. Each successful hardware check leaves an orphaned key (
hardware-test-{timestamp}) in the keystore.However, the
KeyServiceAPI does not expose adeleteKey()method to delete individual keys. The available methods arereset()andclear(), which clear all stored keys and contexts—not suitable for selective cleanup. Consider either:
- Implementing a granular
deleteKey(keyId, context)method inKeyServiceto enable cleanup, or- Removing the misleading "Clean up test key" comment if deletion is not feasible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte:
- Around line 49-55: The catch block after calling
globalState?.securityController.setOnboardingPin(pin, repeatPin) leaves the UI
stuck on the REPEAT step with only isError = true; update the error handling to
set a user-visible error message and/or transition the UI back to the CREATE
step so the user can retry: in the catch, inspect the thrown error from
setOnboardingPin and set a new error state (e.g., pinErrorMessage) with a
different message for mismatch vs storage failure, set currentStep to "CREATE"
or expose a retry action, and ensure the template reads that message instead of
relying only on the existing CREATE-only "PINs didn't match" text; reference
setOnboardingPin, currentStep, isError, and the "PIN_DONE"/"REPEAT"/"CREATE"
step values when making the change.
- Around line 58-63: The handleSetupBiometrics function is enabling biometrics
without verifying they work; modify handleSetupBiometrics to call authenticate()
(same method used on the login page), await its result, and only set
globalState.securityController.biometricSupport = true and currentStep =
"ALL_SET" if authenticate() succeeds; on failure, do not flip the flag and
instead handle the error or show the fallback path (e.g., keep currentStep
unchanged or show an error) so biometric support is only enabled when
authentication actually succeeds.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (1)
82-84: Consider logging the caught error for debugging.The error
eis caught but not logged. While biometric availability failures are gracefully handled by defaulting tofalse, logging the error would help diagnose issues on devices where biometrics unexpectedly fail.♻️ Proposed fix
- } catch (e) { + } catch (e) { + console.warn("Biometric status check failed:", e); isBiometricsAvailable = false; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (4)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (4)
1-12: LGTM!The type definition for
Stepis clean, and using a union literal type provides good type safety for the state machine. The imports are appropriate for the Svelte 5 runes pattern.
21-34: LGTM!The
handleBackfunction correctly handles backward navigation through all steps. The fallback to/onboardingon line 32 is a good defensive measure, even though the Back button is hidden whencurrentStep === "CREATE".
88-154: LGTM!The step-driven UI structure is clean and well-organized. The conditional rendering correctly maps each step to its corresponding content, and the error message display logic at line 99 works correctly given that
pinis reset to""on error.
156-238: LGTM!The footer button logic correctly handles all step transitions with appropriate actions. The Back button is correctly hidden on the
CREATEstep (line 190), and the biometrics unavailability feedback (lines 182-186) provides clear user guidance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte:
- Around line 40-59: The optional chaining in handleConfirmRepeat allows the
await to be skipped when globalState or securityController is undefined; change
the flow to explicitly guard and fail fast: before calling setOnboardingPin,
check if globalState and globalState.securityController exist (e.g., if
(!globalState || !globalState.securityController) { isError = true; currentStep
= "CREATE"; pin = ""; repeatPin = ""; return; }), then call await
globalState.securityController.setOnboardingPin(pin, repeatPin); keep the
existing try/catch around that call to handle runtime errors and set
currentStep/isError/pin/repeatPin as you currently do.
- Around line 8-9: The static import of the Tauri biometric plugin at the top
causes SSR failures; remove the top-level import of checkStatus and instead
dynamically import it inside the onMount callback where checkStatus is used (the
onMount block that initializes globalState via getContext and sets
isBiometricsAvailable). Update the onMount to await
import("@tauri-apps/plugin-biometric") and call the returned checkStatus(),
setting isBiometricsAvailable based on its isAvailable value and catching errors
to set it false; alternatively, if you prefer, disable SSR for this route by
adding export const ssr = false in a +layout.ts.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (3)
21-35: ResetisErrorwhen the user starts over; current error copy stays visible while retyping.After a mismatch,
isErrorstaystrueand the “PINs didn't match” message (Line 102-104) will remain visible during the next attempt. Suggest clearingisErroron back/navigation and/or when user starts typing again.One minimal approach
const handleBack = () => { + isError = false; if (currentStep === "REPEAT") { currentStep = "CREATE"; repeatPin = ""; } else if (currentStep === "PIN_DONE") { currentStep = "REPEAT"; } else if (currentStep === "BIOMETRICS") { currentStep = "PIN_DONE"; } else if (currentStep === "ALL_SET") { currentStep = "BIOMETRICS"; } else { goto("/onboarding"); } };- {#if isError}<p class="text-danger mt-4 text-center"> + {#if isError && pin.length === 0}<p class="text-danger mt-4 text-center"> PINs didn't match. Try again. </p>{/if}Also applies to: 95-105
125-134: Treat decorative images as decorative (avoid noisy/non-descriptivealt).The “Line.svg/Line2.svg” assets look decorative;
alt="line"will be announced and isn’t helpful. Prefer empty alt and/oraria-hidden.Proposed fix
<img class="absolute top-0 start-0" src="/images/Line.svg" - alt="line" + alt="" + aria-hidden="true" /> <img class="absolute top-0 start-0" src="/images/Line2.svg" - alt="line" + alt="" + aria-hidden="true" />
36-38: Adddisabledattributes to buttons in CREATE and REPEAT steps based on PIN length.The ButtonAction component properly forwards
disabledto the native<button>element with correct semantics (cursor-not-allowed styling, disabled state). The current implementation relies on visual variant changes (soft → solid) as length indicators rather than hard UI disabling, which allows partial submissions.Add
disabled={pin.length !== 4}to the CREATE step button anddisabled={repeatPin.length !== 4}to the REPEAT step button (lines 205–220).Proposed changes
const handleConfirmFirst = () => { - if (pin.length === 4) currentStep = "REPEAT"; + if (pin.length !== 4) return; + isError = false; + currentStep = "REPEAT"; };{#if currentStep === "CREATE"} <ButtonAction class="w-full" variant={btnVariant} + disabled={pin.length !== 4} callback={handleConfirmFirst} > Confirm </ButtonAction> {:else if currentStep === "REPEAT"} <ButtonAction class="w-full" variant={btnVariant} + disabled={repeatPin.length !== 4} callback={handleConfirmRepeat} > Confirm </ButtonAction> {/if}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (4)
60-63: Minor: Redundant optional chaining.Since
globalStateis already null-checked on line 51, the optional chaining operator on line 62 is redundant. Consider using direct property access for consistency.Suggested fix
- await globalState?.securityController.setOnboardingPin(pin, repeatPin); + await globalState.securityController.setOnboardingPin(pin, repeatPin);
73-78: Consider removing unnecessaryasync.The function is marked
asyncbut contains noawaitexpressions. If biometric setup doesn't require async operations, consider simplifying to a regular function.Suggested fix
-const handleSetupBiometrics = async () => { +const handleSetupBiometrics = () => { if (isBiometricsAvailable && globalState) { globalState.securityController.biometricSupport = true; currentStep = "ALL_SET"; } };
93-100: Consider logging biometric check failures for debugging.The error is silently caught, which is acceptable since unavailable biometrics is a valid state. However, logging the error could help diagnose device-specific issues.
Suggested enhancement
try { isBiometricsAvailable = (await checkStatus()).isAvailable; } catch (e) { + console.warn("Biometric status check failed:", e); isBiometricsAvailable = false; }
113-116: Consider hiding error text once user starts typing.The error message "PINs didn't match" remains visible even after the user starts entering a new PIN, while the
InputPinerror styling clears whenpin.length > 0. This inconsistency could be confusing.Suggested fix for consistent error UX
<InputPin bind:pin isError={isError && pin.length === 0} /> - {#if isError}<p class="text-danger mt-4 text-center"> + {#if isError && pin.length === 0}<p class="text-danger mt-4 text-center"> PINs didn't match. Try again. </p>{/if}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
📚 Learning: 2025-11-14T17:54:47.719Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 437
File: infrastructure/eid-wallet/src/lib/global/controllers/security.ts:138-157
Timestamp: 2025-11-14T17:54:47.719Z
Learning: The `setOnboardingPin` method in `infrastructure/eid-wallet/src/lib/global/controllers/security.ts` is intentionally designed to allow overwriting existing PINs without checking if a PIN already exists. This is a workaround for Android keeping stale files around during app reinstallation or onboarding, which causes errors on some devices. Do not suggest adding guards to prevent PIN overwrites in this method.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
📚 Learning: 2026-01-08T07:01:50.189Z
Learnt from: grv-saini-20
Repo: MetaState-Prototype-Project/prototype PR: 651
File: infrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelte:25-83
Timestamp: 2026-01-08T07:01:50.189Z
Learning: In the EID wallet module (infrastructure/eid-wallet), disable or omit ESC-key handlers for dialog/modal components in mobile contexts. Specifically, for LoggedInDrawer.svelte and similar modals, avoid keyboard dismissal logic unless the platform requires accessibility-compliant behavior; ensure that any modal can still be dismissed via touch/tap interactions or appropriate mobile-native controls.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (6)
infrastructure/eid-wallet/src/routes/(auth)/register/+page.svelte (6)
1-19: LGTM!Clean imports and well-structured state definitions. The
Steptype provides clear, explicit state machine semantics for the onboarding flow.
21-34: LGTM!The backward navigation logic correctly handles all step transitions. The fallback to
/onboardingfrom the initial step is appropriate.
36-38: LGTM!Simple and effective validation before advancing to the repeat step.
80-84: LGTM!Clean guard clause pattern with proper onboarding completion and navigation.
86-91: LGTM!Reactive button variant logic correctly updates based on PIN input for the relevant steps.
171-253: LGTM!The footer implements a well-structured step-aware action layout. The biometrics step correctly handles the three-button layout with Skip functionality, and the disabled state with user feedback message is a good UX pattern.
Description of change
Onboarding UI converted to full page UI.
Issue Number
close #649
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.