feat(wallet-sdk): accounts slice (step 6)

[pmilic021]

Step 6 of the wallet-SDK migration (docs/superpowers/specs/2026-06-24-wallet-sdk-no-cache-production-design.md): wrap the accounts domain in AccountsApi and flip the web's accounts data layer onto sdk.accounts.*. Plan with the full Decision Record (B1–B7, maintainer-resolved) and gate record: docs/superpowers/plans/2026-07-13-wallet-sdk-accounts-slice.md.

Stacked on sdk/auth-slice (#1166). After #1166 merges: rebase onto master, retarget, re-verify (two-green-PRs rule).

What lands

• Session key plumbing (session-keys.ts): memoized, generation-fenced getters over Open Secret (encryption keypair, cashu seed, spark mnemonic, locking xpub, spark identity pubkey), cleared in onSessionEnded — a different user can never be served the previous user's keys.
• sdk.accounts (get/list/cashu.add) over one shared domain→projection mapper; AddCashuAccountParams pinned ({ name, mintUrl, currency, purpose }); AgicashSdk Pick grows 'accounts'.
• B1 (maintainer ruling): projection-typed, runtime-fat cache. The web ['accounts'] cache holds projection-typed objects fetched via sdk.accounts.list(); hidden domain fields ride along at runtime until step 18 (type-level strip now, physical then). The shared mapper is the only cache entry point (queryFn, realtime, add onSuccess, ensure seed, claim upserts). Reality-class: sdk.accounts.* is TYPE-honest / RUNTIME-fat-until-18, intended and time-boxed; assumes web-internal consumers only during the window.
• /temporary bridge v2: internal-repo accessor + toDomainAccount() checked cast (throws MissingDomainFieldsError naming missing fields — never a bare cast) + mapper re-export. All carry step-18 removal notes.
• sdk.user.ensure(): the _protected.tsx bootstrap ported verbatim (key derivation, default accounts, Zod-aware retry, session-fenced memo); returns { user, accounts } projection-typed. Timestamp params are replayed acceptance times from pending-terms storage (maintainer-confirmed intent, JSDoc'd). Web glue keeps master's structure exactly: short-circuit, prefetch warm-up, conditional seeding.
• Web flip (48 files): root Account/CashuAccount/SparkAccount/ExtendedAccount are now the projections (domain types import from /temporary); display reads .balance off the cache; money flows stay domain end-to-end through sanctioned unwrap seams.

Unwrap seams (every hidden-field read passes toDomainAccount())

1. account-hooks: the four getter hooks unwrap internally; useDefaultAccount/useAccountOrDefault return domain.
2. buy-input / send-input / receive-input: unwrap at the selector option build; account-selector renders domain via getAccountBalance (master verbatim).
3. send-provider getAccounts; routes _protected.buy.checkout / receive.cashu / receive.spark / transfer.$destinationAccountId / send; claim route.
4. account-proofs, transaction-additional-details, use-track-spark-account-balances (spark listener) at their seam.
5. receive-cashu-token-hooks: unwraps for getSourceAndDestinationAccounts; maps the domain source account back through the shared mapper for display items.

Declared (questions and transitional items, not silent changes)

• accounts.get(id) is not session-gated (master's repository posture — RLS scopes it); list/add gate on the session for userId, user.* gates every verb. Parity kept; consistency call open to review.
• withRetry/delay ported into the SDK lib; web copies remain for unmigrated consumers (dies with their slices).
• account-repository-hooks relocated to features/receive/ (not deleted): its only consumers are unmigrated receive repos constructing AccountRepository synchronously; reshaping them is step 8–16 scope. account-service-hooks deleted.
• One parity FLAG: session-keys.ts ports master's hardcoded MAINNET for the spark identity pubkey (master's own TODO).
• sparkDebugLog stays a named /temporary exception (maintainer ruling); dies at 18 with its call site.

Verification

• fix:all clean · workspace typecheck green (all 9 packages incl. web) · tests exit 0: wallet-sdk 96 (25 new: projection completeness type+runtime, mapper single-entry, checked-cast thin-object throw, ensure memo/retry, key fencing across session end), web 36, money 14, bolt11 9, ecies 18, cashu 35. Chain re-run independently at the keeper gate on the frozen tree.
• Browser smoke (build box): boots clean, landing/signup render with zero console errors, guest flow drives to ToS and initiates registration; blocked there by the dev enclave's server-side 403 for this box's origin (environmental — master fails identically here). Post-auth surfaces want a maintainer-env smoke; the ensure/mapper/fencing paths are unit-covered.

🤖 Generated with Claude Code

[vercel]

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

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

[supabase]

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 ↗︎.

[orveth]

Automated cross-model review — accounts slice (step 6)

Scope: step-6 slice diff 937e23a9..1dba0430 (64 files: SDK accounts namespace + key plumbing + 48-file web import sweep). Method: four independent lanes — Claude (Opus) general + Claude (Opus) adversarial + GPT‑5.5 (codex) general + GPT‑5.5 (codex) master-parity — plus orchestrator verification: every finding below was re-derived against the code (and master / the frozen tree) before inclusion. Additive to the maintainer's own review.

Verdict: well-built, faithful, well-tested slice. The projection-vs-domain split is applied consistently at every flipped call site, the fat-cache integrity design holds under adversarial attack (the toDomainAccount() checked cast genuinely throws on a thin object; the shared mapper is the only cache-entry point; key getters are generation-fenced against cross-user leaks; balance math is unchanged), the 48-file web sweep is coherent with no dangling /temporary imports, and the root-type-shadow deletion is sound. One MED, five LOW. No HIGH. The test suite is independently green on the frozen tree (re-run, not gate-trusted: bun install / typecheck / 96 SDK / 36 web, all exit 0, uncapped).

---

MED — sdk.user.ensure()'s result-memo can replay a stale account snapshot into the ['accounts'] cache (undeclared parity regression)

packages/wallet-sdk/domain/user/user-api.ts:148-149,200,207 + apps/web-wallet/app/routes/_protected.tsx:55-70

ensure() memoizes { user, accounts } per session (ensureMemo, set at :200, returned on hit at :149) and is only invalidated on session end (reset() at :207) — never by a later account add / balance change. The _protected seed fires on !user || hasUserChanged, where user is the TanStack ['user'] cache; the memo-hit keys on the SDK's ensureMemo. These are different layers and can desync: if TanStack ['user'] is lost while the SDK session (and its memo) survive for the same user, the seed's !user branch calls ensure(), gets the stale memoized snapshot, and writes it into ['accounts'] — which has staleTime: Infinity, so it stays stale.

• Consequence: newly-added accounts / updated balances vanish from the UI until some mutation or invalidation touches ['accounts']. Money-display staleness (not a fund-safety bug — spend paths unwrap to domain/DB truth).
• Why it's a regression: master seeded ['accounts'] only from a fresh writeUserRepository.upsert, so its re-seed was always current DB truth. The SDK-level memo introduces staleness master didn't have. Not in the plan's Accepted-deltas list → undeclared.
• Same root cause also drops terms params: on a memo-hit, ensure({ termsAcceptedAt }) returns the cached snapshot without persisting the new terms (latent today — the web passes terms once; JSDoc advertises the short-circuit without noting params are dropped).
• Reachability is narrow — needs TanStack ['user'] eviction while the SDK session/memo survive (the exact trigger, e.g. gcTime GC vs a partial cache clear, isn't pinned; a full reload resets both and is safe). But staleTime: Infinity means once stale it stays stale.
• Fix (any one): drop ensure()'s result-level memo and keep the short-circuit in _protected; or return a fromMemo/didUpsert signal and seed accounts only on a fresh upsert; or version-merge sdk.accounts.list() when seeding instead of a wholesale setQueryData replace (this also closes the unconditional-seed concern).

---

LOW findings

1. accounts.get(id) skips the session guard (domain/accounts/accounts-api.ts:52) — list() and add() both requireUserId(); get does not, so a logged-out call fails in key derivation rather than with a clean NoSessionError. RLS (session-token client) masks any cross-user read and this matches master's trust boundary, so no security impact — an API-consistency + error-quality gap. Fix: add requireUserId() to get (fail-fast + match siblings), or document it as intentionally session-agnostic.
2. config.spark.network accepted but not threaded / MAINNET hardcode (session-keys.ts:112-115) — verified faithful master-parity (master hardcoded 'MAINNET' and derived the identity via network.toLowerCase() → identical key); the code carries an explicit FLAG comment documenting the deliberate port. No wrong-network risk. Informational: wire config.spark.network when non-mainnet support lands, or the comment suffices as the deferral record.
3. toDomainAccount() guards key-presence, not value-definedness (domain/accounts/account-projection.ts:61-63) — field in account lets { wallet: undefined } past the loud-throw guard. Not reachable today (every sanctioned entry sets real values). Hardening: check account[field] == null.
4. Dead defaultAccounts export left behind (apps/web-wallet/app/features/user/user-hooks.tsx:97) — the list was copied into the SDK (user-api.ts:24) and its _protected consumer removed, orphaning the web original (zero remaining consumers; the file isn't otherwise in the diff). Delete it (and its now-unused isDevelopmentMode).
5. Non-transitive sort comparator (domain/user/user-service.ts:38) — getExtendedAccounts uses (_, b) => (b.isDefault ? 1 : -1), which floats defaults but scrambles the relative order of non-defaults. Harmless for current callers (they re-sort / .find) but now a public SDK export. Pre-existing logic relocated verbatim. Fix: (a, b) => Number(b.isDefault) - Number(a.isDefault).

Verified clean (multi-lane, hand-traced)

Shared mapper single-entry (every ['accounts'] write routes through it) · toDomainAccount genuine throwing assertion, tested with thin objects · session-key generation-fencing (in-flight-at-reset interleaving tested — no stale key served to a second user) · balance math (empty proofs → zero, Money.add, no raw arithmetic) · SDK bridge wiring + getInternalAccountRepository lifecycle · with-retry/delay · index.ts/temporary.ts export split · the receive-cashu-token and claim money paths (projection→domain→projection round-trip) · all pure import-swap .tsx. Test gaps noted (not findings): no ensure-with-changed-params test, no get-without-session test.

Lanes: opus-general · opus-adversarial · codex-deep-general · codex-deep-parity, orchestrated by keeper:sdk-auth. Reviews ran against isolated worktrees at 1dba0430; codex chunked 64 files into 2 parts per run (cross-chunk invariants swept by hand).

[orveth]

MED addressed in 0eda60b — sdk.user.ensure()'s result-memo is removed entirely: the upsert runs on every call with the given params, and the web's own getUserFromCache + hasUserChanged short-circuit (unchanged, master-verbatim) is the only gating layer — master's exact architecture. This closes both symptoms (stale-snapshot seed under TanStack user-cache eviction; terms params dropped on a memo hit) and makes the seed's freshness by-construction rather than mount-ordering-dependent. createUserApi returns UserApi directly again; the onSessionEnded reset wiring is gone with the memo. Tests: the three memo tests are replaced by always-upserts + params-carried-every-call regression tests (wallet-sdk 95 pass / 2 todo; web 36; full chain re-run green at the keeper gate).

The five LOW items are held for the maintainer's disposition, per the review's framing — none are folded into this fix commit.

[orveth]

Re-review — MED fix (0eda60b5)

Targeted re-review of the fix diff 1dba0430..0eda60b5 (4 SDK files, +44/−81, web byte-identical). Verified against the diff, not the gate.

The MED is closed. The fix is a pure removal that reverts to master's architecture — ensure()'s result-memo is deleted, so the host's own short-circuit is the only caching layer:

• Memo structurally gone — ensureMemo, the memo-hit early-return, the = result assignment, the hasUserChanged helper (its only consumer), and createUserApi's reset are all removed; the factory now returns UserApi directly. No state remains that can go stale, so the stale-seed class is eliminated at the root rather than patched.
• ensure() is now always-fresh and honors params every call — killing both MED symptoms (the stale-account seed and the dropped terms params on a memo-hit) in one stroke.
• Session-end fencing intact — onSessionEnded drops only the now-vestigial resetUserSession(); clearSparkWallets() / clearAgicashMintAuthToken() / keys.reset() are untouched (no cross-user-key regression).
• Web byte-identical — the _protected short-circuit is unchanged, so call frequency matches master (no redundant-ensure perf delta) and the seed is now always a fresh upsert.
• JSDoc corrected — from "Memoized per session… short-circuit" to "Idempotent… the host owns caching."
• Regression guards added — one test asserts the second ensure() returns a fresh result (no memo replay), one asserts terms params reach the upsert on every call. Both MED symptoms now covered.

Suite: author gate CHAIN_EXIT=0 (wallet-sdk 95 pass/2 todo, web 36); not independently re-run this round — the fix is a strict removal + added tests, and the base tree's suite was independently verified green in the prior round.

The five LOW findings from the first pass are untouched here (correctly held for maintainer disposition, not folded into this commit). Clean fix.

Re-review by keeper:sdk-auth.

[orveth]

Post-review addition (maintainer-requested): the web now sources its key queries from the SDK's session keys instead of deriving its own copies — e0ea372 (getInternalSessionKeys() on the /temporary bridge, mirroring the repo accessor: module-scoped, dispose-cleared, step-18 removal note) + b5fb64f (useEncryption becomes one ['encryption'] suspense query over the SDK's getEncryption(); the private-key/public-key queries and their hooks are deleted — verified reader-less; seedQueryOptions/sparkMnemonicQueryOptions keep their keys but delegate their queryFns; the middleware warms shrink to encryption + seed + mnemonic).

Net: the ~5 duplicate enclave derivations per session bootstrap drop to zero; one derivation authority; raw private-key bytes leave the query cache as a side effect.

One declared deviation from the discussed sketch (smaller, same direction): xpubQueryOptions is KEPT untouched — its getCashuCryptography.getXpub() consumer feeds the cashu receive-quote path, so only its middleware warm was dropped. Failure surfacing is preserved because ensure() derives the same locking xpub SDK-side in the middleware; the residual cost is one lazy xpub derivation at first receive-quote, retired when receive migrates (steps 9–12).

Verified at the keeper gate: full chain re-run on the frozen tree (fix:all clean, 9/9 packages typecheck, tests exit 0 — wallet-sdk 99, web 36).

[orveth]

On balance / domain-vs-projection returns (deferred here from jbojcic's #1166 review of sdk/accounts.ts — this PR is that contract's implementation, so the answer belongs here):

Why it currently is the way it is. A cashu account's balance is derived state — sumProofs over its proofs — so the domain type deliberately doesn't store it: a stored field would be a second source of truth that can drift from the proofs (master computes it at render via getAccountBalance for the same reason; spark differs because its balance comes from the wallet service, not from local data). The Omit<> projection implements the contract ruling in force when this slice was planned (#1164: wallet handles / proofs / keysetCounters don't cross the public surface), which this PR's decision record (B1) turned into the type-level-strip-until-step-18 arrangement — the shared mapper attaches balance at the single point where account objects enter the web cache, preserving the one-computation-point property.

What changes under the expose-full-domain-types direction (the direction #1166 has now taken for the receive/send equivalents): the projection rationale collapses, and accounts can align cleanly — declare balance on the domain CashuAccount and attach it in AccountRepository.toAccount, which is already the single constructor every account object flows through (list, realtime row mapping, add, ensure) — the anti-drift property survives intact, one layer deeper. sdk.accounts.* and ensure then return domain types directly, and this PR's projection machinery deletes wholesale: the projection types, the shared mapper, the toDomainAccount checked cast, the unwrap seams in the web hooks, and the type-honest/runtime-fat transitional record — a net-negative, contained diff (deletions dominate; the web's domain-type imports flip back to the root).

The call is yours + petar's, not mine — it reopens B1, which petar ruled explicitly for this slice. Two coherent end-states:

• Align now, pre-merge on this branch: cheapest total cost if the full-domain-types direction is contract-wide — this PR becomes the first slice fully on it, and nobody migrates these files twice.
• Merge as-ruled, align later with the spec-doc update: keeps the ruled design; costs a second pass over the same surface when accounts follows.

Sized and ready to execute either way on your word.

[orveth]

Rebased onto the domain-types contract (d5d2f18 landing in #1166: domain Account/CashuAccount/SparkAccount exposed directly, no balance field — the direction decided for the question deferred here). The base also restructured (sdk/ → domain/sdk/, 16 commits), so the branch was rebuilt as 8 clean commits rather than replaying 17 through conflicts; the pre-rebase state is preserved at sdk/accounts-slice-pre-domain.

What changed vs the previous head:

• The projection machinery is gone (mapper, checked cast, unwrap seams, projection types) — the cache holds domain accounts, balance reads are getAccountBalance(account) exactly like master, and getExtendedAccounts/isDefaultAccount keep their clean domain signatures (the generic hack died with the projections).
• AddCashuAccountParams pinned in the new contract file: { name, mintUrl, currency, purpose }.
• Everything operational carried over intact: sdk.accounts (get/list/cashu.add over the instance's repository), sdk.user.ensure() (merged with the base's split write repositories + withRetry around derivation and upsert), session-key plumbing + fencing (reset rides the base's session-teardown clear list), both /temporary accessors, the web re-sourcing (list/get/add/realtime/ensure glue), withRetry/delay in @agicash/utils, and the key-query delegation round (xpubQueryOptions untouched — live getXpub money-path consumer).

Verified at the keeper gate on the rebuilt head: fix:all clean, all packages typecheck, tests exit 0 (wallet-sdk 105, web 36); repo-wide grep confirms zero projection references remain. Independent delta review running.

[orveth]

Rebased onto master following the #1166 merge and retargeted the PR (base: sdk/auth-slice → master).

• Clean replay of the same 8 commits: d9a1fa26 → 0a34e6c9 (no conflicts, no content changes)
• Verification re-run on the rebased tree: lint + typecheck + tests green (wallet-sdk 105/105, web-wallet 36/36)

Next: sweeping all #1166 review comments for reasoning applicable to this slice, per @petar; will report findings here.

[orveth]

#1166 review-comment sweep — applicability to this PR

Per @petar's ask: enumerated all #1166 commentary (55/55 review threads, 192 inline comments, 3 issue comments — pagination exhausted, counts cross-checked against the REST comment total) and audited this PR's full diff against every ruling that generalizes beyond the auth slice.

Applied (2 new commits)

91282973 — AbortController fence for the session key memos. session-keys.ts used a hand-rolled generation counter to keep an in-flight derivation from populating the cache after reset() — the same pattern #1166 replaced in auth-service with an AbortController session scope ("scope.aborted answers 'what is this for' at the call site"). Now mirrors auth-service: the keys hold a scope controller, reset() aborts and replaces it, an in-flight derivation captures the signal at start and neither caches nor releases the in-flight slot once aborted. Behavior identical; all existing tests (including the in-flight-derivation-at-reset test) pass unmodified.

db746ef8 — comment cleanup per review conventions. Comments only, no code: removed prior-code/branch narration ("master derived these…", "preserves master's…", "ported as-is"), kept the load-bearing facts (retry re-derives only failed keys; the _protected warms feed unmigrated repositories; network fixed to mainnet, not yet wired to config.spark.network); "the web" → "the host" in comments this branch added; dropped grammar-metaphor wording.

Verification on the pushed head: lint + typecheck clean, wallet-sdk 105/105, web-wallet 36/36.

Already satisfied (verified against the diff, no change needed)

• Domain types direct, no projections — zero projection/balance-field remnants tree-wide; consumers sum proofs via getAccountBalance.
• domain/sdk/ taxonomy + full-Sdk implementation — accounts fills the throwing NotImplementedError getter; no Pick-widening.
• setDefaultAccount(userId, account) full-entity contract — untouched; isDefaultAccount/getExtendedAccounts operate on the passed entity, no id-plus-refetch.
• Repository split seam — ensure() consumes the base's UpsertUserRepository(db, accountRepository) with one repository construction path shared by the namespace, the /temporary accessor, and ensure; the web no longer constructs it.
• In-band results, not events — ensure() returns { user, accounts } and the host seeds caches from the return value.
• Primitives over call-site patches — withRetry/delay promoted to @agicash/utils; no call-site papering.

Held as questions (parity vs. ruling — your call, no code changed)

1. storageDir ?? './.spark-data' at the SDK composition root (sdk.ts:135). The value is master parity and the config field is documented optional-with-default; the feat(wallet-sdk): auth & user slice (step 5) #1166 generateToken ruling ("why even have the default?") could be read to require the host to pass it. Fine as is, or make it required?
2. Mainnet hardcode in session-keys.ts ignores the available config.spark.network (carried master parity, flagged in-code). Port-as-is stands, or wire it now?
3. DisposedError posture: user/accounts verbs don't throw DisposedError after dispose() (they're session-gated only) — consistent with the base's own user namespace, so flagging as a shared posture question, not a divergence. Relatedly, the /temporary accessors throw a raw Error rather than a typed SdkError.

[jbojcic1]

get rid of this comment