Repository navigation
fix: deeplink issue - #337
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughRefactors scan-qr page to show an in-drawer persistent success UI for signing/blind voting, adds a confirm handler, and aligns camera control. Introduces a global deep-link event bridge in layout, dispatching/storing payloads and routing to scan-qr, with listeners and cleanup. Adds guards to prevent duplicate deep-link processing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant OS as OS/Browser
participant Layout as +layout.svelte
participant Session as sessionStorage
participant Scan as scan-qr/+page.svelte
OS->>Layout: Deep link arrives (auth/sign/reveal data)
Note over Layout: Dispatch CustomEvent "deepLinkReceived"
Layout->>Session: Save deepLinkData (backup)
alt Current route != /scan-qr
Layout->>OS: navigate(/scan-qr)
end
Layout-->>Scan: Event "deepLinkReceived" (if on page)
Scan->>Scan: Guard against duplicate/in-flight\nprocessing
Scan->>Scan: Process deep link payload
Scan->>Scan: startScan or cancelScan per flow
sequenceDiagram
autonumber
participant Scan as scan-qr/+page.svelte
participant Camera as Camera
participant Drawer as Signing Drawer
participant Nav as Router
Scan->>Camera: startScan()
Camera-->>Scan: QR data → sign/blind-vote
Scan->>Scan: Perform sign / submit blind vote
Scan->>Camera: cancelScan()
Scan->>Drawer: showSigningSuccess = true
Note over Drawer: Success screen shown with “Okay”
User->>Drawer: Click Okay
Drawer->>Scan: handleSuccessOkay()
Scan->>Scan: Reset state
Scan->>Nav: navigate(/main)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
6 tasks done
coodos
added a commit
that referenced
this pull request
Nov 4, 2025
This was referenced Nov 6, 2025
Merged
Merged
This was referenced Dec 9, 2025
Closed
6 tasks done
6 tasks done
Sahil2004
added a commit
that referenced
this pull request
Sep 17, 2026
globalDeepLinkHandler is registered for deepLinkReceived, and on the /scan-qr branch it dispatched a new deepLinkReceived. dispatchEvent is synchronous, so the handler re-entered itself and recursed until the stack overflowed, roughly 3270 frames in. The surrounding try/catch swallowed the RangeError, so it failed silently while /scan-qr's own handler ran thousands of times. The re-dispatch was never needed: scanLogic.ts registers its own deepLinkReceived listener, so an already-mounted /scan-qr receives the original event directly. Both are on the same window and event name, and grep confirms these are the only two listeners. It only triggered when a second deep link arrived while the consent screen was already open, which is why it survived since #337. Extract the handler as handleDeepLinkEvent() so it can be tested: it now stores the payload in every case and navigates only when /scan-qr is not the current route. Storing on the /scan-qr branch also covers a route that is mid-navigation and has not mounted its listener yet. Mutation-tested: restoring the self-dispatch fails the new test with the RangeError itself, not a proxy for it.
Bekiboo
added a commit
that referenced
this pull request
Sep 22, 2026
* fix(eid-wallet): show the consent screen when a deep link races cold-start auth
Opening a w3ds:// login link on a cold start could drop the user on /main
with the Approve/Decline screen never shown. It reproduced when biometric
authentication succeeded quickly.
Showing the consent screen needs two independent things to finish, in an
order nobody controls: the URL arriving (the layout imports the deep-link
plugin asynchronously) and the user authenticating. The layout decided
which had happened by asking whether window.location.pathname was an
authenticated route. On a cold start that path is "/" for the splash no
matter how the race went, so a user who had ALREADY authenticated was
still classified as logged out. The payload was parked for a screen that
had finished running, and nothing ever collected it.
Make it a rendezvous where whoever finishes LAST does the routing, and
record authentication explicitly instead of inferring it from the URL:
- the layout parks the payload if the user is not authenticated yet, and
routes to /scan-qr if they are
- every authentication path funnels through continueAfterSuccessfulAuth,
which records the fact before any await, then collects a parked payload
and routes to /scan-qr rather than /main
Either ordering now reaches the consent screen, because both sides test
the same explicitly-recorded fact.
Also here, because they follow from the above:
- the splash no longer diverts a deep-link launch to /login. It is where
biometrics are prompted, so diverting downgraded a returning user to
the PIN pad for the flow most likely to be used in a hurry.
- the splash's async onMount gets liveness checks. Unmounting does not
cancel a continuation parked on an await, so it could wake after the
consent drawer opened and navigate away from it.
- logout clears the authenticated flag. goto("/") is an SPA navigation
and leaves sessionStorage intact, so without this a later deep link
would skip the authentication gate entirely.
The three near-identical 105-line checkAuth blocks in the layout (auth,
sign, reveal) collapse into one routeDeepLink function, which is most of
the 376 deleted lines.
* refactor(eid-wallet): move deep-link storage into lib/stores
Splits the deep-link flow along the same seam the other stores use
(see personalBinding: "the store is just state, actual writes live in
lib/utils"). lib/stores/deepLink.ts owns the keys and the sessionStorage
access; lib/utils/deepLinkFlow.ts keeps the routing decisions and is now
storage-agnostic.
Promotion moved into the store as a raw string copy. Doing it in the
logic layer meant JSON.parse followed by re-stringify, which made that
layer interpret a payload it has no business reading and would corrupt
anything JSON does not round-trip exactly.
No behaviour change. The four mutations still fail the suite: auth check
always false (4 tests), promotion never collecting (2), logout not
clearing (1), and recording auth as a no-op (4).
* refactor(eid-wallet): collapse the deep-link payload to one slot
There were two slots, pendingDeepLink and deepLinkData, and a promote
step that copied between them. The copy carried no information: the
payload was byte-identical on both sides, and the only difference was a
label meaning "the user may act on this now" — which is already answered
by the authenticated flag that every consumer checks anyway.
The duplication leaked outward. /scan-qr read one key, fell back to the
other, then had to remember to clear both; the layout wrote whichever it
guessed was right; /login read the pending key directly to decide whether
to show its banner. Any of those forgetting a key was a silent bug.
Now: store the payload, ask isWalletAuthenticated() for permission. The
layout stores unconditionally and only routes when authenticated, and
continueAfterSuccessfulAuth asks hasDeepLink() once it has recorded the
authentication.
All deep-link storage access now goes through lib/stores/deepLink.ts;
no route touches sessionStorage for this flow directly.
Five mutations fail the suite: auth check always false (4 tests), payload
never stored (6), logout not clearing auth (1), recording auth as a no-op
(4), and the consent screen's clear doing nothing (2).
* refactor(eid-wallet): drop the deep-link pass-through layer
utils/deepLinkFlow.ts had seven exports, and six were one-line forwards
to lib/stores/deepLink.ts: markWalletAuthenticated called setAuthenticated,
storeDeepLink called setPayload, and so on. The only one doing anything was
resetAuthSession, clearing two keys. A layer that renames its callees is a
second vocabulary for the same concepts, not an abstraction.
Merge it into the store, which now carries the rendezvous documentation
alongside the state it describes. Names lose the prefixes that only existed
to avoid collisions between the two layers: isWalletAuthenticated ->
isAuthenticated, peekDeepLinkPayload -> peekDeepLink, clearDeepLinkFlow ->
clearDeepLink. The spec moves next to the module it covers.
The split was worth having when the logic layer held real decisions
(shouldRedirectToLogin, ownership claims, replay windows). Those are gone,
so the seam has nothing left on one side of it.
Five mutations still fail the suite: auth check always false (4 tests),
payload never stored (6), logout not clearing auth (1), recording auth as a
no-op (4), and the consent screen's clear doing nothing (2).
* docs(eid-wallet): move the deep-link architecture out of the source
The rendezvous explanation, the storage-choice rationale and the history
of the pathname-inference bug were living in a header comment on
lib/stores/deepLink.ts, with fragments repeated in the layout and
postLogin. Prose that describes a flow spanning four files does not
belong to any one of them, and duplicating it guarantees the copies drift.
Move it to docs/architecture/deepLink.md and leave each site a pointer.
Comments that explain a local decision stay put: why markAuthenticated
must precede any await, why resetAuthSession clears on logout.
The doc also records what the code cannot say for itself: that the (app)
guard checks enrolment rather than authentication, that PIN change and
passphrase rotation are untraced, and that the Ok confirmation card after
a deep-link login is still broken.
* refactor(eid-wallet): name the deep-link auth accessors for their consumer
markAuthenticated / isAuthenticated / resetAuthSession read like the
app-wide authentication gate when imported elsewhere, which they are not:
the (app) route guard checks the vault (enrolment), and nothing but the
deep-link flow reads this flag.
Rename to markAuthenticatedForDeepLink, isAuthenticatedForDeepLink and
resetDeepLinkAuthSession so a call site says which flow it belongs to.
The suffix describes the consumer, not the scope — the underlying fact is
the session's authentication state. Noted at the declaration and in the
architecture doc so the names do not imply a deep-link-only concept that
someone later duplicates for another flow.
* refactor(eid-wallet): move session auth onto a SessionController
"Has the user authenticated this run" was a pair of functions in
lib/stores/deepLink.ts, which read as a deep-link concept. It is not: it
is the session's state, and the deep-link flow is only its first reader.
Add GlobalState.sessionController alongside the other controllers. Logout
now clears it automatically, because GlobalState.reset() already calls
clear() on each controller — settings no longer needs its own call for
the flag, only for the parked payload.
SessionController deliberately takes no Store, unlike its siblings. The
flag must NOT survive the app being killed, or a deep link on a cold
start would inherit a previous run's login and skip the prompt. It must
survive the WEBVIEW being rebuilt, which is a different event: Android
can reload the webview while the app is backgrounded by openUrl, and the
approve path does a document navigation to the platform's redirect.
Neither is a new run, and the flow cannot re-prompt mid-handoff, so an
in-memory field would strand the user. sessionStorage is exactly that
lifetime.
A test pins it: a fresh SessionController reading the same storage is
what a rebuilt webview sees. Swapping the implementation to an in-memory
field fails that test and the logout test.
lib/stores/deepLink.ts is now payload-only.
* chore(eid-wallet): drop a comment restating what logout already shows
The line said reset() clears the authenticated flag via sessionController,
which is visible at the reset() call directly above it. What is not
obvious stays: why the parked payload needs a separate clear.
* chore(eid-wallet): drop the splash comment about not diverting to /login
It explained the absence of a redirect that no longer exists in the file,
so it described history rather than the code. The deep-link routing rules
are in docs/architecture/deepLink.md.
* chore(eid-wallet): drop the pointer to where auth state lives
A module saying what it does not contain is noise; the file exports four
payload functions and nothing about authentication.
* fix(eid-wallet): prompt biometrics only on the splash
/login had its own authenticate() call, suppressed by a
biometricAttemptedOnSplash handshake flag. The suppression was racy: the
splash wrote the flag only after two awaits resolved biometricAvailable,
while /login read it behind a globalState poll of up to five seconds.
Anything routing to /login inside that window found no flag and prompted
a second time, over a half-painted PIN pad, with a second post-auth
routine able to consume the same deep-link payload.
Delete the prompt from /login rather than coordinate it. The screen is
the PIN fallback by definition: the splash routes here only once the
prompt was declined, failed, or was unavailable.
With it go the flag, the authOpts block and the biometric imports. The
splash is now the only file importing authenticate() from
@tauri-apps/plugin-biometric; every other importer takes checkStatus for
availability. A single prompt site is structural, so there is no longer a
flag to get wrong.
Trade-off: cancelling the prompt leaves the user on the PIN pad with no
way to retry biometrics without relaunching. That is what a fallback
screen means, and it matches the behaviour deep-link launches already had.
* test(eid-wallet): exercise the real routing code, not a copy of it
The deep-link spec reimplemented both halves of the rendezvous as local
helpers and asserted against those. The tests therefore described the
design rather than the app, and would have kept passing if the shipped
routing had been deleted. The earlier mutation runs hid this because
every mutation happened to target the two modules the spec did import.
Extract routeDeepLink() from +layout.svelte into lib/utils so it can be
imported, and have the spec call it and continueAfterSuccessfulAuth()
directly with goto() mocked, asserting on where the code navigated.
The routing logic itself is moved unchanged.
Add a test for the ordering that markAuthenticated() must precede the
vault await: a deep link arriving mid-login has to observe the user as
authenticated, otherwise the layout parks the payload for a screen that
has already finished, which is the original bug.
Mutation-tested: dropping the auth gate, never routing, not storing the
payload, always routing to /main, and marking authentication after the
await each fail the suite. The last four of those survived beforehand.
* docs(eid-wallet): correct four comments that outlived the code
Each described a shape the code had before a later commit changed it:
- deepLink.md said reset() clears both keys. It clears the sign-in flag
via SessionController.clear(); performLogout() clears the payload.
- session.ts said the controller "takes the Store like its siblings".
It takes no arguments and reads sessionStorage directly, which is the
point the rest of that comment argues for.
- deepLink.spec.ts named resetDeepLinkAuthSession(), removed when session
handling moved onto SessionController.
- postLogin.ts said /login runs a fallback biometric prompt. The splash
is the only biometric prompt now.
Comments only; no behaviour change.
* fix(eid-wallet): stop the layout dispatching the event it listens for
globalDeepLinkHandler is registered for deepLinkReceived, and on the
/scan-qr branch it dispatched a new deepLinkReceived. dispatchEvent is
synchronous, so the handler re-entered itself and recursed until the
stack overflowed, roughly 3270 frames in. The surrounding try/catch
swallowed the RangeError, so it failed silently while /scan-qr's own
handler ran thousands of times.
The re-dispatch was never needed: scanLogic.ts registers its own
deepLinkReceived listener, so an already-mounted /scan-qr receives the
original event directly. Both are on the same window and event name,
and grep confirms these are the only two listeners.
It only triggered when a second deep link arrived while the consent
screen was already open, which is why it survived since #337.
Extract the handler as handleDeepLinkEvent() so it can be tested: it now
stores the payload in every case and navigates only when /scan-qr is not
the current route. Storing on the /scan-qr branch also covers a route
that is mid-navigation and has not mounted its listener yet.
Mutation-tested: restoring the self-dispatch fails the new test with the
RangeError itself, not a proxy for it.
* fix(eid-wallet): treat finishing onboarding as being signed in
A deep link that arrived during onboarding did nothing: the user landed
on /main and the consent screen only surfaced later, whenever something
next mounted /scan-qr. A regression from this branch.
The old gate asked whether a vault existed. Onboarding persists the
vault immediately before routing, so that check passed. The rendezvous
gate asks whether the user signed in this session, which only the splash
and /login recorded, so a user who had just created their identity was
classified as logged out. That is the same mistake the branch set out to
fix, asking a question the cold-start state cannot answer, in a new
place.
Creating or restoring an identity is proving it. Add completeOnboarding()
beside continueAfterSuccessfulAuth(), marking the session and honouring a
waiting deep link the same way, and route all four onboarding and
recovery exits through it. Those exits each repeated the same trio of
calls; isOnboardingComplete now has one writer.
Mutation-tested: dropping markAuthenticated() from the helper, or making
it ignore a waiting payload, each fail the new tests.
* chore(eid-wallet): bump version to 1.1.1
Android versionCode 28 -> 30.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(eid-wallet): put node/pnpm on PATH for the Xcode build phase
Xcode launched from the Dock runs script phases with a minimal PATH, so
pnpm from a version manager is not found. The generated phase only sourced
nvm; cover mise, volta and the Homebrew/local prefixes too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(eid-wallet): bump Android versionCode to 31
* chore(eid-wallet): set iOS build number to 1.1.1.1
* fix(eid-wallet): guard each sessionStorage operation, not just access
The typeof guard in SessionController and deepLink.ts only proves the
storage object is reachable. Reaching it can succeed while every
operation on it throws: quota exhausted, or a private mode where setItem
always throws. Each call site then failed in a way its own comment says
it avoids.
markAuthenticated() is called from the splash inside the try that wraps
the biometric prompt, whose catch means "biometrics failed" and routes
to /login. A throw there sent a user who had just authenticated back to
the PIN screen. isAuthenticated backs the layout's deep-link gate, where
a throw drops the payload. clear() runs inside the single try block in
GlobalState.reset(), so a throw abandoned the rest of logout.
storeDeepLink/peekDeepLink/clearDeepLink had the same gap: the first two
run inside the deep-link callback where the file's own comment says a
throw is invisible to the user, and clearDeepLink runs in performLogout
outside any try, ahead of the navigation that ends the session.
Wrap each operation and keep the documented fallback: writes are
best-effort, reads degrade to "nothing stored", deletes never reject.
Mutation-tested: removing any one of the six guards fails the suite.
* Do not store a deep-link payload that cannot be serialised
JSON.stringify returns undefined for undefined, a function or a symbol.
setItem coerces that to the literal string "undefined", so hasDeepLink()
reports a payload waiting that the consent screen then fails to parse,
routing the user to /scan-qr to look at nothing. A deepLinkReceived event
dispatched without a detail reaches storeDeepLink() exactly that way.
Skip the write instead, leaving any previously stored payload intact.
* Dispatch the deep-link event only to an already-mounted consent screen
The root layout registers its own deepLinkReceived listener, and that
handler navigates to /scan-qr. Dispatching the event from routeDeepLink()
when the user is somewhere else therefore woke the layout handler as well
as running routeDeepLink's own goto(), entering the route twice for a
single link. Off /scan-qr the stored payload is what the mount reads, so
the event delivers nothing the navigation does not.
Dispatch only when /scan-qr is already the current route, where it has a
live listener and no mount is coming, and navigate otherwise.
* chore(eid-wallet): sync Cargo and iOS spec versions to 1.1.1
The 1.1.1 bump updated package.json and tauri.conf.json but left the Rust
crate and the xcodegen spec on 1.0.1. Cosmetic for the built artifacts,
which take their version from tauri.conf.json, but the spec would revert
the iOS bundle version if the project were regenerated.
* fix(eid-wallet): report the real app version instead of hardcoded strings
Authentication sent appVersion "0.4.0" from two separate literals, and the
Settings header showed "App Version 1.0.0". All three were stale.
Auth now reads getVersion() once and uses it on both the POST and deeplink
paths. Settings cannot use it: the subtitle is captured synchronously at
layout init to avoid a flash mid-transition, and getVersion() is async, so
it reads __APP_VERSION__ instead. Both resolve to the tauri.conf.json
version, so the value shown and the value sent cannot drift apart.
---------
Co-authored-by: Bekiboo <julien.connault@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit