Fix: prevent back to auth on eID Wallet #666
Conversation
📝 WalkthroughWalkthroughAdds a new Svelte auth layout at Changes
Sequence Diagram(s)sequenceDiagram
participant AuthLayout as (auth)/+layout.svelte
participant GlobalState as GlobalState (vaultController, securityController)
participant Router as goto()/history
AuthLayout->>GlobalState: get globalState from context
AuthLayout->>GlobalState: await vaultController.vault
alt vault exists
AuthLayout->>Router: goto("/main", replaceState: true)
else vault missing
AuthLayout->>GlobalState: check isOnboardingComplete and securityController.pinHash
alt onboarding complete & pinHash present and not /login
AuthLayout->>Router: goto("/login", replaceState: true)
else
AuthLayout-->>AuthLayout: render children (allow auth flow)
end
end
AuthLayout-->>AuthLayout: set isChecking = false / handle errors
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 |
|
Connected to Huly®: METAS-532 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte:
- Around line 11-17: getContext is being called inside onMount which causes
"Function called outside component initialization"; move the getContext<() =>
GlobalState>("globalState") call out of onMount to run synchronously during
component initialization (top-level of the <script>), assign its return to the
globalState variable (preserving the GlobalState type assertion), and keep
onMount only for async/DOM-dependent work; also add a null/undefined check right
after the top-level getContext call and log/return early if globalState is
missing.
In @platforms/group-charter-manager/.browserslistrc:
- Around line 1-8: The top-of-file comment claims "iOS 16+ Safari support" but
the actual targets are "iOS >= 15" and "Safari >= 15"; either update the
human-readable comment to reflect "iOS 15+ / Safari 15+" or change the target
lines to "iOS >= 16" and "Safari >= 16" if iOS 16+ was intended—modify the
comment text or the two target entries (the "iOS >= 15" and "Safari >= 15"
lines) so the comment and configuration match.
In @platforms/group-charter-manager/next-env.d.ts:
- Around line 1-6: The manual reference to the auto-generated routes file should
be removed: delete the line containing /// <reference
path="./.next/types/routes.d.ts" /> from next-env.d.ts so the file remains
auto-generated and won't be overwritten on build; alternatively, if you intend
to keep that reference, enable typed routes by setting typedRoutes: true in your
NextConfig object in next.config.ts (e.g., add typedRoutes: true) so the
generated routes file is intentionally used.
🧹 Nitpick comments (4)
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (2)
17-39: Consider handling localStorage unavailability.The
localStorage.getItemcall on line 30 could throw in environments where localStorage is unavailable (e.g., private browsing in some browsers, SSR edge cases). While this is a client component, wrapping the localStorage access in a try-catch would make it more resilient.Suggested improvement
// Check if this message has been dismissed if (response.data.status === "maintenance") { - const dismissed = localStorage.getItem(DISMISSED_KEY); - setIsDismissed(dismissed === response.data.message); + try { + const dismissed = localStorage.getItem(DISMISSED_KEY); + setIsDismissed(dismissed === response.data.message); + } catch { + // localStorage unavailable, banner will show + } }
41-46: Same localStorage resilience consideration for dismissBanner.The
localStorage.setItemcall could also throw in restricted environments.Suggested improvement
const dismissBanner = () => { if (motd?.message) { - localStorage.setItem(DISMISSED_KEY, motd.message); - setIsDismissed(true); + try { + localStorage.setItem(DISMISSED_KEY, motd.message); + } catch { + // localStorage unavailable, dismiss won't persist + } + setIsDismissed(true); // Still dismiss for this session } };infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte (2)
29-40: Use SvelteKit's$pagestore instead ofwindow.location.pathname.Accessing
window.location.pathnamedirectly bypasses SvelteKit's routing awareness. Use the$pagestore for consistency with SvelteKit's navigation system.Suggested improvement
<script lang="ts"> import { goto } from "$app/navigation"; +import { page } from "$app/stores"; import type { GlobalState } from "$lib/global"; import { getContext, onMount } from "svelte"; +import { get } from "svelte/store"; ... if (globalState.isOnboardingComplete) { const pinHash = await globalState.securityController.pinHash; - const isAlreadyAtLogin = window.location.pathname === "/login"; + const isAlreadyAtLogin = get(page).url.pathname === "/login"; if (pinHash && !isAlreadyAtLogin) {
49-53: Rendering logic has an unreachable state.When
vaultExistsis true, the user is redirected beforeisCheckingbecomes false (line 25-26 returns early). The{:else if !vaultExists}condition is always true when reached, making it effectively just{:else}. This isn't a bug, but the condition is redundant.Simplified version
{#if isChecking} <div class="h-screen w-screen bg-background"></div> -{:else if !vaultExists} +{:else} {@render children()} {/if}However, keeping the explicit check provides a safety net if the redirect fails, so this is a minor nitpick.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelteplatforms/group-charter-manager/.browserslistrcplatforms/group-charter-manager/next-env.d.tsplatforms/group-charter-manager/next.config.tsplatforms/group-charter-manager/src/components/MaintenanceBanner.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 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)/+layout.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)/+layout.svelte
🔇 Additional comments (1)
platforms/group-charter-manager/next.config.ts (1)
7-12: LGTM!Using
transpilePackagesis the correct approach to ensure these packages are transpiled to browser-compatible code. This should resolve Safari 16 compatibility issues withmarkedand@milkdown/preset-gfm.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte:
- Around line 15-18: The guard currently logs and returns when globalState is
undefined but still lets isChecking become false (in the finally block), so
children render and fail; add a failure flag (e.g., hasError or authFailed) and
set it true inside the if (!globalState) branch, ensure the finally still sets
isChecking = false but change the template to render children only when
!isChecking && !hasError && globalState; update any usages of
isChecking/globalState to check hasError (symbols: globalState, isChecking,
hasError/authFailed) so children are prevented from rendering when the guard
failed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 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)/+layout.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)/+layout.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 (3)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte (3)
1-10: LGTM on imports and state initialization.The Svelte 5 runes usage and context-based global state retrieval are appropriate for this guard layout.
50-54: Template logic correctly handles guard states.The conditional rendering prevents auth content from flashing while redirecting authenticated users. The
{:else if !vaultExists}guard is intentional and correct.
20-41: Auth guard logic correctly implements the PR objectives.The use of
replaceState: trueproperly prevents back-navigation loops, and the/loginpath check avoids redirect cycles. BothvaultController.vaultandsecurityController.pinHashare getters returning Promises (via.then()chain and async method respectively), so theawaitstatements are correct and necessary.
Bekiboo
force-pushed
the
fix/prevent-back-to-auth
branch
from
b50caa9 to
e24dc74
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte:
- Around line 45-48: The catch block in the auth layout guard logs errors but
does not mark guardFailed, so when an exception occurs guardFailed remains false
and the UI may render children; update the catch block in the auth layout guard
to set guardFailed = true when an error is caught (in addition to
console.error), ensuring the evaluation of vaultExists/isChecking/guardFailed
fails closed and the error UI is shown rather than rendering auth children.
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte (1)
53-65: Consider: Rendering state whenvaultExistsis true.When
vaultExistsbecomestrueand the redirect to/mainis triggered, none of the three conditions match afterisCheckingbecomesfalse. This results in rendering nothing briefly while navigation completes.This is likely imperceptible due to the fast redirect, but for completeness you could keep showing the placeholder during the redirect:
♻️ Optional: Show placeholder during redirect
-{#if isChecking} +{#if isChecking || vaultExists} <div class="h-screen w-screen bg-background"></div> {:else if guardFailed}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 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)/+layout.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)/+layout.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 (1)
infrastructure/eid-wallet/src/routes/(auth)/+layout.svelte (1)
1-12: LGTM!Imports and state declarations are well-structured using Svelte 5 runes syntax. The
getContextcall at the module level is the correct pattern.
Description of change
Implements a centralized auth guard in (auth)/+layout.svelte. This prevents authenticated users from using the OS back button to return to login/onboarding screens.
Issue Number
Closes #665
Type of change
How the change has been tested
Tested on virtual device (pixel 7)
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.