Skip to content

feat(wallet-sdk): auth & user slice (step 5)#1166

Open
pmilic021 wants to merge 23 commits into
masterfrom
sdk/auth-slice
Open

feat(wallet-sdk): auth & user slice (step 5)#1166
pmilic021 wants to merge 23 commits into
masterfrom
sdk/auth-slice

Conversation

@pmilic021

@pmilic021 pmilic021 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Step 5 of the no-cache SDK migration: the first runtime Sdk class — AgicashSdk.create(config) with working auth, user, and events namespaces — and the web app's auth + user flows flipped from /temporary / direct Open Secret usage to sdk.*.

Docs: contract spec · implementation plan (Decision Record A1–A13, the 7 accepted behavior deltas, and the deferred list live there).

SDK (@agicash/wallet-sdk)

  • AgicashSdk.create(config) — sync construction; configures Open Secret (module-global, so one instance per process); builds its own Supabase client with an internal, generation-fenced third-party-token getter (A6). init() = memoized single-flight session restore (A3: Breez WASM stays host-side until the first Spark slice).
  • AuthService implements AuthApi — in-memory session snapshot, all auth verbs, and the expiry machinery (A2): guest auto-extend emits auth.session-refreshed (A13), full-account death emits auth.session-expired; restore applies are fenced by a session generation against concurrent verbs; teardown() is terminal so a disposed instance can't re-arm timers.
  • createUserApisdk.user.* with userId implicit from the session; setDefaultAccount reads the account row and writes column-minimally (delta 7); A9 frees WriteUserRepository from the accounts graph.
  • Typed WalletEventEmitter; port-backed guest-account storage (same guestAccount key — existing devices keep their credentials); CSPRNG password generator with a host-override port (A4 — the e2e password mock seam survives with zero e2e changes).
  • Adopts React-agnostic @agicash/opensecret@1.0.0-rc.0; step-5 contract types settled (AuthStorage binds RC-verbatim, AuthUser, A10 param types).
  • Contract quality-of-life: sdk.ts split into per-namespace sdk/ files (each future slice settles types in its own file); AgicashSdk implements Pick<Sdk, …> grows per slice until it collapses to implements Sdk; the logger port is required, with an exported nullLogger for hosts that want silence.

Web

  • features/shared/sdk.client.ts singleton — config assembly, the two-line window.getMockPassword e2e bridge, HMR dispose (awaited by Vite).
  • auth.ts rewritten as thin glue: snapshot-driven authQueryOptions (staleTime pinned to fetch-time refresh expiry — the re-sync fallback for pages where the session-events hook isn't mounted), useAuthActions over sdk.auth, and useHandleSessionEvents (expiry toast, a mount-time catch-up for expiries that fire before the subscription exists, and a hard-reload fallback) sharing useSessionEndCleanup with signOut.
  • User hooks flipped to sdk.user.*; verify-email, the OAuth callback, and the token-claim default-account write (A11) flipped.
  • safeJwtDecode extracted to @agicash/utils and adopted at every stored-token read — also fixing a latent throw in shared/auth.ts on corrupt tokens; setLongTimeout/clearLongTimeout moved to @agicash/utils.
  • The remaining direct reads of Open Secret's storage keys are annotated as temporary leaks pending step 18.

Deliberately unchanged (deferred): ensureUserData stays on /temporary (A1 → step 6), encryption plumbing (step 6), sdk.featureFlags wiring (A12); full list in the plan.

Behavior

Parity with master for every auth flow (email login/signup, guest signup + re-signin, Google OAuth, verify email, convert guest, sign out, session expiry, boot restore) except the 7 documented deltas — notably: guest-extend failure now shows a toast + graceful redirect instead of a hard reload (delta 1), the expiry timer arms wherever a session exists (delta 6), and setDefaultAccount writes are lost-update-proof (delta 7).

Deltas added by the cross-model review round (deliberate improvements over master, all tested):

  • 8 — the fired expiry timer re-checks the stored refresh token and re-arms when it was rotated forward, instead of expiring a live session (was in the slice from the start, previously undeclared).
  • 9 — sign-out's web cleanup runs even when the SDK verb throws (the SDK ends the local session regardless, so master's skip-cleanup-on-throw left a dead session under a live-looking UI).
  • 10 — a guest sign-up whose credential persist fails is undone and rejects, instead of leaving a session that strands (with funds) at first expiry.
  • 11AgicashSdk.create() throws while an undisposed instance exists, making the one-instance-per-process constraint self-enforcing.

Known issue (accepted deliberately — review finding 1): the expiry handler races concurrent session transitions — a sign-out landing while the guest auto-extension's re-sign-in is in flight is silently undone, and an in-flight handler survives teardown() (HMR). Inherited from master's useHandleSessionExpiry, not a regression. It was fixed with a generation fence (1e6a6272) and then reverted (68b41e2e) by owner decision: per-await fencing is too subtle to maintain. The planned fix is structural — serialize all session transitions through a single command lane (single-writer). The two parked it.todo tests in auth-service.test.ts are the spec that refactor must satisfy; a KNOWN ISSUE note marks the handler in auth-service.ts.

Verification

  • Adversarial review loop: round 1 = 3 independent fresh-context reviewers (2 general + a flow-by-flow parity audit) → 5 findings confirmed and fixed; round 2 = 2 fresh reviewers → 0 findings, fixes verified sound.
  • Cross-model fleet review (the consolidated comment below) → all 17 findings triaged: 10 fixed with 6 new regression tests (each verified to fail against the pre-fix code), 1 acknowledged as the known issue above (fix reverted in favor of the planned command-lane serialization; its 2 race tests parked as it.todo specs), 1 declined to keep master's generator verbatim (the guest-password modulo skew — ~1 in 5.7×10⁷ per character, cryptographically negligible), 1 recorded as deltas 8–11 above, 1 deferred to the typed-error pass (steps 17/19), 3 record-only confirmations — incl. verifying against the OpenSecret 1.0.0 source that signOut clears its local keys unconditionally and swallows the network logout failure.
  • 184 unit tests across the workspace (72 in the SDK, + 2 parked todo) and the production build (client + server + prerender) pass.
  • Browser smoke: guest signup → session restore on reload → sign-out keeps guestAccount → guest re-signup lands the same account → username/default-account/currency edits persist → the Google button redirects to accounts.google.com with state.sessionId → exactly two Supabase token exchanges (web + SDK clients, per A6) and wallet.users reads via the SDK client; no new console errors.
  • Master-session upgrade check: a session created on master (opensecret 0.1.0) restores on this branch.
  • e2e (signup/login/verify): fails locally, but identically on master (control run) — a local mock/env issue, not a regression; CI is the arbiter.

🤖 Generated with Claude Code

pmilic021 and others added 22 commits July 9, 2026 19:49
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hUser, user params)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ssword generator

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chinery

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ken getter

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…h; add createUserApi

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nts namespaces

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ret config into it

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…codable refresh tokens

A restore whose fetchUser resolved after a concurrent auth verb (or session
end) could clobber the newer session with the stale result; a session
generation counter now gates the apply. A present-but-undecodable refresh
token now restores anonymous instead of establishing a session the expiry
machinery can neither time out nor refresh.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dd a reload fallback

The SDK arms its expiry timer during init(), so an expiry can fire before
Wallet's event subscription mounts and emit to no subscribers; the hook now
detects the already-dead SDK session on mount and runs the expiry handling.
The handler also gets a hard window.location.reload() fallback so an
unexpected failure mid-reset can't strand the user on a blank page.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each migration slice now settles its types in its own sdk/<namespace>.ts
instead of growing one contract file (19 placeholders across steps 6-16
remain to land); cross-cutting pieces (SdkConfig, Sdk, Logger) stay in
sdk/index.ts. Pure move — the exported surface is unchanged, and existing
'./sdk' import specifiers resolve to the directory index as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ick<Sdk>

The compiler now checks the implemented subset (auth, user, events, init,
dispose) against the Sdk contract; each slice adds its namespace to the
Pick until it collapses to the full Sdk. Unimplemented namespaces stay
absent rather than stubbed, so consuming them early remains a compile
error instead of a runtime crash.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A host that wants no diagnostics now passes the exported no-op nullLogger
instead of omitting the field, so silence is an explicit choice. Internal
components take a required Logger too — forgetting to forward one is now a
compile error rather than a silently diagnostics-less component.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…chinery

A fetchUser continuation resolving after teardown (restore or verb) could
re-arm the expiry timer on a disposed instance, resurrecting the zombie
timer the HMR dispose hook exists to prevent. A disposed flag now blocks
re-arming and a late-firing timer callback; verbs and the session snapshot
are unaffected — disposal is not logout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Vite awaits a promise returned from hot.dispose, so the old SDK's teardown
is guaranteed to complete before the replacement module constructs the new
instance — which matters once dispose() gains async teardown work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nOut and the expiry handler

Both ran the same order-sensitive sequence (flags reset, auth invalidation,
navigate/revalidate, Sentry clear, queryClient.clear) with the ordering
rationale documented on only one of them; useSessionEndCleanup now owns the
sequence and its invariants, while each caller keeps its own edges (the
sdk.auth.signOut() call and redirect target vs the toast and reload
fallback).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fail-soft decode idiom (a corrupt STORED token must degrade to null,
never throw) was duplicated in the SDK's auth service and the web's auth
glue, and missing from shared/auth.ts's isLoggedIn — where a corrupt
refresh token would throw through the DB client's token getter into every
query; it now reads as logged out. Tokens freshly minted by a server keep
decoding with jwtDecode directly so a malformed one fails loudly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ry leak

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@supabase

supabase Bot commented Jul 10, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project hrebgkfhjpkbxpztqqke because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agicash Ready Ready Preview, Comment Jul 11, 2026 2:24pm

Request Review

@orveth

orveth commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Automated cross-model review — 4 lanes, consolidated

Scope: PR head b4ba28e3 vs master (31eca250), 22 commits / 59 files. Method: four independent lanes as requested — Claude (Fable) general review · GPT‑5.5 (codex, deep) general review · GPT‑5.5 (codex, deep) master-parity review · Claude (Opus) adversarial session-lifecycle pass — plus orchestrator verification: every finding below was re-derived or checked against the code (and master / OpenSecret‑SDK source where relevant) before inclusion. Additive to the in-branch review loop documented in the PR body.

Verdict: high-quality slice; the concurrency machinery is sound where applied, the headless constraint holds everywhere, the contract split is lossless, the web glue is parity-or-better, and all four lanes independently confirmed the 7 declared behavior deltas are honest. One HIGH-class race (all three model lanes converged on it independently), two MEDs, and a batch of LOWs — plus two inherited #1164 contract defects surfaced by the review that deserve a fix while this surface is warm.


1. HIGH — sign-out can be silently undone by an in-flight guest auto-extend

packages/wallet-sdk/domain/user/auth-service.ts:295-343 (+ :200-212)

handleSessionExpiry checks disposed/isLoggedIn only at entry (:297) and its refreshSessionSnapshotapplySessionFromServer() call (:202) passes no expectedGeneration — the SDK's only self-initiated session writer is the one transition the new generation fence doesn't cover.

Interleaving: guest timer fires → signUpGuest() in flight (two RTTs) → user signs out (endSession bumps the generation and disarms, but a fired callback can't be cancelled, and guest creds survive sign-out by design) → os.signInGuest writes fresh tokens (verified against OpenSecret‑SDK 1.0.0 source: it always re-stores) → the unfenced apply sets the session logged-in, re-arms the timer, and emits auth.session-refreshed → the web's own invalidation flips the UI back to authenticated. On a shared device: money access after a believed sign-out. The window widens on a backgrounded tab whose throttled timer fires just as the user returns to sign out.

Inherited, not a regression — master's useHandleSessionExpiry (old auth.ts:377-408) has the same unfenced shape. But the machinery now lives in one place and the branch built sessionGeneration for exactly this, so it's a one-place fix now: capture the generation at entry, re-check after every await (gating the re-signin itself, not just the apply — signInGuest rewrites tokens, so a fenced apply alone still resurrects on next boot), thread expectedGeneration through the extension's apply, and guard the final endSession()+emit. Re-checking this.disposed at the same points also closes a smaller gap: an in-flight handler survives teardown() (entry-only check), so an HMR dispose can let the old instance clear tokens the new instance just restored (sdk.client.ts:52-57; dev-only today). No test covers either race — worth pinning both.

2. MED — public receive projections expose Cashu proof material (inherited from #1164)

packages/wallet-sdk/sdk/receive.ts:7-9

  • CashuReceiveSwap = Omit<Domain…, 'userId'> — the domain type carries top-level tokenProofs (domain/receive/cashu-receive-swap.ts:25), so the public type and the cashu-receive-swap.created/updated event types expose spendable proofs.
  • CashuReceiveQuote = Omit<Domain…, 'userId'> over a discriminated union — TS Omit collapses the union to its shared keys, so the CASHU_TOKEN variant's fields (including its tokenProofs) are silently mangled rather than deliberately projected.

Both lines are byte-identical on master — born in #1164, where the projection sweep stripped proofs from the send entities but wrongly concluded the receive entities carried none. Nothing at runtime serves these objects yet (receive params/returns are still step‑9/12 placeholders), so this is a contract-level defect today — which is exactly why it's cheap to fix now, before hosts or the step‑12 implementation build against the leaking shape. Fix: omit tokenProofs explicitly, and give CashuReceiveQuote per-variant public projections instead of a union Omit; note the runtime-strip convention at the projection boundary for the implementing slices. Sweep result across all namespaces: send.ts clean (proofs/inputProofs/proofsToSend omitted), accounts.ts clean (keysetCounters/proofs/wallet omitted), contacts/transactions clean — receive is the only dirty namespace.

3. MED — setDefaultAccount reads the session twice across an await (undeclared)

packages/wallet-sdk/domain/user/user-api.ts:60-68

The only verb that derives the user from the live session twice: getAccountRef validates account ownership under requireUserId() №1 (:35), then the update calls requireUserId() №2 (:65) after the await. A sign-out + sign-in-as-B between them writes A's accountId onto B's user row — and the schema has no same-user constraint on the default-account FKs, so it persists (RLS still hides A's account data from B: broken default pointer, not fund exposure). Master used one captured user/account for the whole write. Fix: const userId = requireUserId() once at entry, thread it through (getAccountRef takes it as a param). The parity lane's suggestion of a DB-level accounts.user_id = users.id constraint/RPC is good defense-in-depth.

4. MED — failed network sign-out skips all web cleanup and wedges the button

apps/web-wallet/app/features/user/auth.ts:227-233, :287-297

sdk.auth.signOut() ends the local session in a finally and rethrows; the web's await sdk.auth.signOut(); await endSessionCleanup(…) has no finally, and useSignOut never reaches setLoading(false). On a flaky network: SDK session dead, but query caches / Sentry identity / hint cookie stay live, no navigation, spinner stuck. Master also skipped cleanup on a failed osSignOut() — but master left you coherently logged in; the branch creates a new incoherent middle state (dead session under a live-looking wallet). Fix: run endSessionCleanup (and setLoading(false)) in finally — the SDK guarantees the local session is already gone.

5. MED‑LOW — guest auto-extend during init() loses auth.session-refreshed (undeclared)

apps/web-wallet/app/features/user/auth.ts:310-335

The mount-time catch-up compensates for a missed auth.session-expired but not a missed refresh: a near-expiry guest can extend during sdk.init() before the subscription exists, leaving the auth query + session-hint cookie on the old expiry (one later cold-load /home bounce; same self-heal as declared delta 2's residual — staleTime pinned to fetch-time expiry). Both codex lanes flagged it independently. Fix: after subscribing, resync when getSession().isLoggedIn and the stored expiry moved — or have the SDK replay the last refresh on subscribe.

LOW / notes

  • Reload fallback has no once-guard (auth.ts:313-318): endSessionCleanup().catch(() => reload()) can loop, but only under a compound partial outage (restore succeeds + cleanup persistently rejects + tokens survive the expiry path's os.signOut). A sessionStorage one-shot flag is cheap insurance.
  • Auth verbs resolve success when the post-verb fetchUser fails (auth-service.ts:200-212): deliberate parity per the code comment; still means correct credentials + no toast + anonymous bounce. Suggest a typed error when the step‑17/19 SdkError pass lands.
  • Restore-memo clobber (auth-service.ts:84-88): a slow-failing restore clears restorePromise unconditionally and its endSession bump can fence a newer restore's apply out → anonymous boot despite valid tokens (self-heals on next invalidation). Clear only when the memo is still your own promise.
  • Double fetchUser after same-lifetime sign-out→sign-in: endSession clears the restore memo and the auth queryFn always awaits init() → every re-login pays a redundant restore round-trip. Early-return from doRestore when already logged in.
  • New-guest credential persist failure (auth-service.ts:136-142): if guestAccountStorage.store rejects after os.signUpGuest created the account, a later extend/restore mints a fresh guest identity — prior guest wallet stranded. Narrow (browser setItem rarely fails) but money-adjacent for future non-browser hosts.
  • Guest password modulo bias (lib/password.ts:27-28): % charset.length bias is ~1 in 5.7e7 per char — negligible at 32 chars and verbatim from master's generator; rejection sampling is a 3-line fix for the sole credential of funded guest accounts.
  • Emitter iterates the live Set (lib/events.ts:30-36): mid-emit (un)subscribe changes the current dispatch; snapshot […handlers] first. (Throwing-subscriber isolation is correctly handled.)
  • In-flight Supabase token after sign-out (db/supabase-session.ts:56-73): an already-awaiting caller still receives the old session's token; cache and cross-session serving are correctly fenced, master behaved the same, and it's the same user's own token — note-only. Strictness option: re-check the generation before returning.
  • create() doesn't enforce the one-instance-per-process invariant (agicash-sdk.ts:82-84): declared in the PR body; a throw-on-second-create would make the module-global OpenSecret constraint self-enforcing.
  • Expiry timer re-arms when the stored refresh token moved (auth-service.ts:300-306): undeclared but strictly better than master (whose render-captured timer could sign out a session OpenSecret had just rotated); tested. Suggest adding it to the delta list for the record.
  • Restore no longer validates the access token (auth-service.ts:91-104): master's decode-throw on a corrupt access token booted anonymous; the branch restores via the (validated) refresh token — strictly better, adjacent to the PR body's declared safeJwtDecode fix. For the record only.
  • Expiry fallback no longer clears token storage (parity lane): likely moot — OpenSecret‑SDK 1.0.0's signOut clears local keys unconditionally per its source; flagging only so the assumption is on the record. Worth a maintainer confirm since it's your library.

Confirmed clean (checked, no findings)

Headless-safety (zero react/@tanstack/DOM/storage references in packages/wallet-sdk outside doc comments) · restore single-flight + generation fencing (tested) · Supabase token getter's cross-user invariant (tested; sign-out-mid-fetch pinned) · sign-out memo-clear ordering + repopulation fencing · guest storage verbatim incl. legacy key · contract split lossless (export-name diff vs base: zero losses; entrypoint resolution checked) · web glue verb ordering/invalidation/Sentry/hint-cookie/staleTime units vs master · declared deltas 1–7 each verified against master's actual behavior.

Test gaps worth closing with the fix: the finding‑1 race (sign-out during in-flight extend), teardown-during-in-flight-handler, and a setDefaultAccount session-switch guard.


Fleet: fable-general · codex-deep-general · codex-deep-parity · opus-lifecycle, orchestrated by keeper:sdk-auth (forge). Reviews ran against isolated detached worktrees at b4ba28e3; codex chunked 59 files into 4 parts per run (cross-chunk invariants covered by the orchestrator's sweep).

pmilic021 added a commit that referenced this pull request Jul 11, 2026
…transitions

A fired expiry timer can't be cancelled, so its handler raced every other
session transition. Worst case: a sign-out landing while the guest
auto-extension's re-sign-in was in flight got silently undone — the
re-sign-in wrote fresh tokens over the sign-out's clear and the unfenced
snapshot apply flipped the session back to logged-in (money access after a
believed sign-out on a shared device). An in-flight handler also survived
teardown(), letting a disposed instance clear tokens its successor had just
restored.

The handler now captures the session generation at entry and re-checks it
plus the disposed flag after every await: a raced re-sign-in's tokens are
cleared again instead of resurrecting the session, the extension's apply is
generation-fenced, and the death path (sign-out, end, expired event) only
runs while the handler still owns the session it started from.

Finding 1 (HIGH) of the cross-model review on PR #1166. Both regression
tests fail against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pmilic021 added a commit that referenced this pull request Jul 11, 2026
Three LOW findings from the PR #1166 cross-model review:

- A slow-failing restore cleared the restore memo unconditionally and its
  endSession bumped the generation, fencing out a newer restore's apply —
  an anonymous boot despite valid tokens. The rejection now un-memoizes
  only its own memo, and the failure path leaves the session alone when
  another transition owns it.
- A restore after a same-lifetime sign-out/sign-in repeated the verb's user
  fetch; doRestore now returns early when a verb already established the
  session.
- A guest sign-up whose credential persist failed left a live session whose
  account would strand (with any funds) at its first expiry. The sign-up is
  now undone and the verb rejects, so the retry lands on a recoverable
  account.

All three regression tests fail against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pmilic021 added a commit that referenced this pull request Jul 11, 2026
…nd collapsing variants

The receive domain entities are intersections over variant unions, and the
contract projected them with a bare `Omit`. TypeScript collapses each union
to its shared keys under `keyof`, so the projections simultaneously leaked
the swap's top-level `tokenProofs` (spendable Cashu proof material, also via
the receive events' payload types) and silently dropped every variant-only
field (`tokenReceiveData`, `failureReason`, keyset fields) along with
discriminant narrowing.

The domain schemas now name their variant unions once (feeding both the
schema and an exported variant type), and the projections omit base keys
only, re-apply the variant unions, and strip proof material at both levels —
top-level `tokenProofs` and the melt data's nested `tokenProofs`. The
implementing slices (steps 9/11/12) must strip the same fields at runtime.

Inherited from #1164 (the projection sweep missed the receive entities);
finding 2 of the cross-model review on PR #1166.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pmilic021 added a commit that referenced this pull request Jul 11, 2026
The verb derived the user from the live session twice — once inside the
account-ownership read and again, after that await, for the row update. A
session switch between the two wrote the previous user's account id onto the
next user's row (the schema has no same-user constraint on the default
account FKs, so the broken pointer persists; RLS still hides the other
user's data). The session is now captured once at verb entry and threaded
through, so the validating user and the written user cannot diverge.

Finding 3 of the cross-model review on PR #1166. The regression test fails
against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pmilic021 added a commit that referenced this pull request Jul 11, 2026
…lose two expiry-path gaps

Three findings from the PR #1166 cross-model review:

- A throwing sdk.auth.signOut() skipped the whole web cleanup while the SDK
  had already ended the local session (its own finally), leaving query
  caches, the Sentry identity, and the hint cookie serving a dead session —
  with the sign-out button wedged in its loading state. The cleanup and the
  loading reset now run in finally.
- A guest auto-extension during init() fires auth.session-refreshed before
  the events hook subscribes, so the auth query and session-hint cookie
  stayed pinned to the old expiry. The hook now re-syncs on mount when the
  stored refresh-token expiry moved past what the query captured.
- The expiry hard-fallback reload had no guard; it is now throttled through
  a sessionStorage timestamp so a persistently failing cleanup can't
  reload-loop the tab.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pmilic021 added a commit that referenced this pull request Jul 11, 2026
- generateRandomPassword now rejection-samples instead of a bare modulo, so
  every charset character is exactly equally likely — this string is the
  sole credential of funded guest accounts.
- WalletEventEmitter.emit dispatches over a snapshot, so a handler that
  (un)subscribes mid-emit can't alter the current dispatch.
- AgicashSdk.create() now throws while an undisposed instance exists,
  making the one-instance-per-process constraint (module-global Open Secret
  state) self-enforcing; dispose() releases the slot, so the HMR
  dispose-then-recreate cycle still works.

LOW findings of the cross-model review on PR #1166; each regression test
fails against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drops the rejection sampling added in 7d3bc1c (review LOW finding),
returning the generator to master's algorithm verbatim. The modulo skew is
~1 in 5.7e7 per character over a 32-character password — cryptographically
negligible — and master parity is preferred for the slice. The redraw test
goes with it; the length sanity test stays.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

revert(wallet-sdk): unfence the expiry handler, park its races as a known issue

Reverts the generation fencing of handleSessionExpiry from 1e6a627 (review
finding 1). The per-await fence pattern is too subtle to maintain — the
plan is to fix the race class structurally by serializing all session
transitions through a single command lane (single-writer) instead.

The handler returns to the shape inherited from master's expiry hook, with
a KNOWN ISSUE note: a sign-out landing while the guest re-sign-in is in
flight is silently undone, and an in-flight handler survives teardown()
(HMR). The two race regression tests stay as it.todo — they are the spec
the command-lane refactor must satisfy.

Kept from the reverted commit: the signInGuestAccount extraction (the
guest-persist undo lives there) and the test fake's real-SDK sign-out
fidelity (clears token keys), which later tests rely on.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(wallet-sdk): tighten three lib-level invariants from the review

- generateRandomPassword now rejection-samples instead of a bare modulo, so
  every charset character is exactly equally likely — this string is the
  sole credential of funded guest accounts.
- WalletEventEmitter.emit dispatches over a snapshot, so a handler that
  (un)subscribes mid-emit can't alter the current dispatch.
- AgicashSdk.create() now throws while an undisposed instance exists,
  making the one-instance-per-process constraint (module-global Open Secret
  state) self-enforcing; dispose() releases the slot, so the HMR
  dispose-then-recreate cycle still works.

LOW findings of the cross-model review on PR #1166; each regression test
fails against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(web-wallet): make the session-end web cleanup unconditional and close two expiry-path gaps

Three findings from the PR #1166 cross-model review:

- A throwing sdk.auth.signOut() skipped the whole web cleanup while the SDK
  had already ended the local session (its own finally), leaving query
  caches, the Sentry identity, and the hint cookie serving a dead session —
  with the sign-out button wedged in its loading state. The cleanup and the
  loading reset now run in finally.
- A guest auto-extension during init() fires auth.session-refreshed before
  the events hook subscribes, so the auth query and session-hint cookie
  stayed pinned to the old expiry. The hook now re-syncs on mount when the
  stored refresh-token expiry moved past what the query captured.
- The expiry hard-fallback reload had no guard; it is now throttled through
  a sessionStorage timestamp so a persistently failing cleanup can't
  reload-loop the tab.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(wallet-sdk): read the session once per setDefaultAccount verb

The verb derived the user from the live session twice — once inside the
account-ownership read and again, after that await, for the row update. A
session switch between the two wrote the previous user's account id onto the
next user's row (the schema has no same-user constraint on the default
account FKs, so the broken pointer persists; RLS still hides the other
user's data). The session is now captured once at verb entry and threaded
through, so the validating user and the written user cannot diverge.

Finding 3 of the cross-model review on PR #1166. The regression test fails
against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(wallet-sdk): stop the public receive projections leaking proofs and collapsing variants

The receive domain entities are intersections over variant unions, and the
contract projected them with a bare `Omit`. TypeScript collapses each union
to its shared keys under `keyof`, so the projections simultaneously leaked
the swap's top-level `tokenProofs` (spendable Cashu proof material, also via
the receive events' payload types) and silently dropped every variant-only
field (`tokenReceiveData`, `failureReason`, keyset fields) along with
discriminant narrowing.

The domain schemas now name their variant unions once (feeding both the
schema and an exported variant type), and the projections omit base keys
only, re-apply the variant unions, and strip proof material at both levels —
top-level `tokenProofs` and the melt data's nested `tokenProofs`. The
implementing slices (steps 9/11/12) must strip the same fields at runtime.

Inherited from #1164 (the projection sweep missed the receive entities);
finding 2 of the cross-model review on PR #1166.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(wallet-sdk): harden the restore memo and guest sign-up edge paths

Three LOW findings from the PR #1166 cross-model review:

- A slow-failing restore cleared the restore memo unconditionally and its
  endSession bumped the generation, fencing out a newer restore's apply —
  an anonymous boot despite valid tokens. The rejection now un-memoizes
  only its own memo, and the failure path leaves the session alone when
  another transition owns it.
- A restore after a same-lifetime sign-out/sign-in repeated the verb's user
  fetch; doRestore now returns early when a verb already established the
  session.
- A guest sign-up whose credential persist failed left a live session whose
  account would strand (with any funds) at its first expiry. The sign-up is
  now undone and the verb rejects, so the retry lands on a recoverable
  account.

All three regression tests fail against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix(wallet-sdk): fence the expiry handler against concurrent session transitions

A fired expiry timer can't be cancelled, so its handler raced every other
session transition. Worst case: a sign-out landing while the guest
auto-extension's re-sign-in was in flight got silently undone — the
re-sign-in wrote fresh tokens over the sign-out's clear and the unfenced
snapshot apply flipped the session back to logged-in (money access after a
believed sign-out on a shared device). An in-flight handler also survived
teardown(), letting a disposed instance clear tokens its successor had just
restored.

The handler now captures the session generation at entry and re-checks it
plus the disposed flag after every await: a raced re-sign-in's tokens are
cleared again instead of resurrecting the session, the extension's apply is
generation-fenced, and the death path (sign-out, end, expired event) only
runs while the handler still owns the session it started from.

Finding 1 (HIGH) of the cross-model review on PR #1166. Both regression
tests fail against the previous code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants