Skip to content

feat(wallet-sdk): transactions slice (step 8) - #1175

Merged
jbojcic1 merged 4 commits into
masterfrom
sdk/transactions-slice
Aug 13, 2026
Merged

jbojcic1 merged 4 commits into
masterfrom
sdk/transactions-slice

Conversation

@ditto-agent

Copy link
Copy Markdown
Contributor

Summary

Step 8 of the 19-step no-cache wallet-SDK extraction (spec: docs/superpowers/specs/2026-06-24-wallet-sdk-no-cache-production-design.md; plan committed in this PR: docs/superpowers/plans/2026-08-12-wallet-sdk-transactions-slice.md). This wires the transactions namespace of the SDK contract and flips the web transactions feature from @agicash/wallet-sdk/temporary imports to sdk.transactions.*.

What changed

  • NEW packages/wallet-sdk/domain/transactions/transactions-api.ts — createTransactionsApi(deps): session-fenced wrapper over the existing TransactionRepository (accounts-style async repository construction via await keys.getEncryption(); signal captured before the await, re-checked after each await; abortSignal threaded into every repository call). Methods: get, list, countPendingAck, acknowledge.
  • NEW packages/wallet-sdk/domain/transactions/transactions-api.test.ts — 12 tests: per-method delegation/userId injection plus the session-fence matrix (NoSessionError, SessionEndedError after dispose with zero repository calls, SessionEndedError when the session ends inside the async repository factory, SessionEndedError mid-read).
  • domain/sdk/transactions.ts — the contract's Transaction = Omit<DomainTransaction, 'userId'> projection is deleted and the domain Transaction union is used directly. list gained JSDoc documenting the RPC semantics (DRAFT/FAILED excluded, ordering, default pageSize 25, nextCursor === null on the last page).
  • domain/transactions/transaction.ts — userId deleted from BaseTransactionSchema; toTransaction stops mapping data.user_id.
  • domain/transactions/transaction-repository.ts — Cursor normalized to the non-null keyset tuple (| null moved to use sites); the page-end rule folded into list (nextCursor is null when the page is short).
  • domain/sdk/sdk.ts — the throwing transactions getter replaced with a readonly field wired via createTransactionsApi({ db, getSession: getLiveSession, keys }).
  • domain/sdk/events.ts — Transaction import flipped from the contract file to the domain entity (contacts precedent).
  • apps/web-wallet/.../transaction-hooks.ts — useTransaction, useTransactions, useHasTransactionsPendingAck, useAcknowledgeTransaction now call sdk.transactions.*; useUser dependency dropped (session is SDK-internal); query keys, staleTime, refetch options, retry counts unchanged; the hook's manual page-end rule removed (now SDK-side); Cursor + NotFoundError imports moved from /temporary to the public root.
  • packages/wallet-sdk/temporary.ts — 28 dead transaction re-exports pruned (enum schemas, transaction schemas, all per-variant details schemas/parsers, details types, parser, parser input/shape types, and post-flip Cursor). AgicashDbTransaction + TransactionRepository stay (live realtime consumers).

Design decisions

  • userId deleted from the domain entity: nothing reads transaction.userId; every other userId in the domain is an input parameter; ownership = session + RLS.
  • Omit-collapse fix: Omit over a discriminated union collapses it to a flat object type and breaks narrowing (e.g. transaction-list.tsx narrows type === 'CASHU_LIGHTNING' && direction === 'SEND' to reach details.destinationDetails), so the contract uses the domain Transaction union directly — same resolution as the contacts slice.
  • Cursor normalization: the repository Cursor type is the non-null keyset tuple; nullability lives at use sites.
  • Page-end rule folded into the repository list: headless consumers get true end-of-list semantics (nextCursor === null) instead of each caller re-deriving it.
  • No events emitted: transaction.created/transaction.updated stay type-only until the step-18 realtime feed.
  • Realtime echo stays on /temporary: useTransactionChangeHandlers + useTransactionRepository (needs instance toTransaction decryption) and useReverseTransaction (unmigrated send domain) are intentionally unchanged until steps 14/18.

Verification

  • bun run fix:all exit 0; bun run typecheck exit 0 (8 packages).
  • Unit tests: wallet-sdk 155 pass (12 new), web-wallet 38 pass, all other package suites pass.
  • Browser smoke on the local stack: transaction history renders through sdk.transactions.list (empty state + real row); full receive money-path on a test mint created a real transaction end-to-end; the detail page rendered it through sdk.transactions.get; auto-acknowledge ran through sdk.transactions.acknowledge; pending-ack badge query through sdk.transactions.countPendingAck; zero console errors.
  • An independent adversarial review of the full diff was commissioned (marketplace) — outcome recorded in the PR conversation.

Delegation note

Implementation, tests, web flip, the temporary.ts prune, this description, and the adversarial review were produced as paid maxplayer marketplace jobs and integrated + verified locally by the orchestrating agent.

🤖 Generated with Claude Code

ditto-agent and others added 2 commits August 12, 2026 16:47
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s and the web flip

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

vercel Bot commented Aug 12, 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 Aug 12, 2026 3:45pm

Request Review

@supabase

supabase Bot commented Aug 12, 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 ↗︎.

…erage

Review follow-up: the page-end rule moved into TransactionRepository.list
without a direct test, and acknowledge lacked a mid-call abort test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ditto-agent
ditto-agent force-pushed the sdk/transactions-slice branch from 7639966 to abc84f8 Compare August 12, 2026 15:17
@ditto-agent

Copy link
Copy Markdown
Contributor Author

Marketplace adversarial review (maxplayer job 5c7bc43b…, seller milibot)

Verdict: READY — 0 Critical, 0 Important, 2 Minor, 2 Nit. Full report below, then the orchestrator's triage.

Triage

  • Minor 1 (get throws no NoSessionError) — kept as-is: the same id-scoped-get asymmetry exists on purpose in the contacts and accounts namespaces (RLS scopes the read; no userId needed). Harmonizing all three is a separate decision.
  • Minor 2 (moved pagination rule untested) — fixed in abc84f8: transaction-repository.test.ts covers full page / pending-first cursor / short page / empty page.
  • Nit 3 (repository factory runs before the abort check) — kept: matches the accounts fence order exactly; the invariant (no repository call) holds either way.
  • Nit 4 (no mid-write fence test for acknowledge) — fixed in abc84f8.

Review: Step 8 — wire sdk.transactions, migrate the web transaction hooks

Verdict

READY

I found no Critical and no Important defects. I verified each invariant against the
diff and against the base source of transaction-repository.ts, transaction-hooks.ts,
index.ts, domain/sdk/index.ts, sdk.client.ts, and lib/error.ts. The findings
below are two Minor items and two Nits. None of them blocks the merge.

Invariant check summary

  1. Pagination parity — holds. The base repository destructures pageSize = 25,
    so the new transactions.length === pageSize check uses the effective page size.
    The web passes pageSize: 25 through the api unchanged. The composed result is
    identical to the old two-layer rule for all four cases:

    • Full page with more rows: old gave a cursor (repo cursor, length 25); new gives a cursor.
    • Exact-full last page: both give a cursor; the next fetch returns an empty page and
      both then give null. The extra empty fetch exists in both versions.
    • Short page: old repo gave a cursor and the web hook replaced it with null; the new
      repo gives null directly.
    • Empty page: lastTransaction is undefined in both versions, so both give null.

    The query keys, staleTime, refetchOnWindowFocus, refetchOnReconnect, retry: 1
    (useTransactions), the NotFoundError retry guard (useTransaction), and the
    { transactions, nextCursor } page shape are unchanged. The web hook is the only
    caller of TransactionRepository.list, so the moved rule changes no other consumer.

  2. Session fence — holds, with one asymmetry (Finding 1). Every method captures
    keys.sessionSignal() before await getRepository(), re-checks after that await and
    before the repository call, threads the signal into the repository, and re-checks
    after the repository call. A session switch during getRepository() cannot leak: the
    captured signal belongs to the old session and is aborted, so the check before the
    repository call throws SessionEndedError.

  3. userId deletion — holds. I searched the web app and the SDK for reads of
    transaction.userId and found none. toTransaction is the only
    TransactionSchema.parse site. The realtime echo path (useTransactionChangeHandlers)
    reads acknowledgmentStatus and payload.previous_acknowledgment_status only.

  4. Public Transaction type — holds. The contract file now only imports
    Transaction; it no longer exports it. index.ts line 60 exports the domain union
    explicitly, and an explicit export beats export *, so the collapse cannot recur
    from the contract file.

  5. Deliberately unchanged items — confirmed unchanged. Not flagged.

  6. temporary.ts deletions — safe. No file outside packages/wallet-sdk imports the
    deleted names. AgicashDbTransaction and TransactionRepository stay exported.
    Cursor reaches the root through domain/sdk/transactions.ts → export * from './transactions' in domain/sdk/index.ts → export * from './domain/sdk' in
    index.ts, so the new web import compiles.

Findings

Critical

None.

Important

None.

Minor

1. get omits the session guard that the other three methods have

  • File: packages/wallet-sdk/domain/transactions/transactions-api.ts, the get method
    (lines ~31–42).
  • list, countPendingAck, and acknowledge call requireUserId() first and throw
    NoSessionError when no session exists. get skips this guard because the repository
    get(id) takes no userId. As a result, get has no code path that throws
    NoSessionError.
  • Failure scenario: a caller invokes sdk.transactions.get(id) before login, or after a
    logout that did not come from dispose. The caller does not receive NoSessionError.
    Instead the call falls through to keys.sessionSignal() / keys.getEncryption(), and
    the error type depends on the internal state of SessionKeys. Callers that branch on
    NoSessionError (for example a redirect-to-login handler) miss this case for get
    only. No data leaks: the repository needs session-derived encryption keys, so the call
    cannot decrypt another user's rows.
  • Related test gap: the test list claims "NoSessionError per method". The production
    get contains no NoSessionError path, so a passing get test for this must get the
    error from a mock (sessionSignal or the createRepository seam), or the test does
    not cover get. Either way the test does not prove the production behavior.
  • Suggested fix: call requireUserId() (or a requireSession() variant that discards
    the id) at the top of get, to match the other three methods. Alternatively, document
    the asymmetry in the TransactionsApi contract.

2. The moved pagination rule has no direct test

  • File: packages/wallet-sdk/domain/transactions/transaction-repository.ts, list
    (lines ~88–100 after the change).
  • No transaction-repository.test.ts exists. The new api tests mock the repository and
    assert a verbatim page passthrough. The web hook now returns the result verbatim. So
    after the move, no test in either package exercises the
    lastTransaction && transactions.length === pageSize rule or the four page-shape
    cases from invariant 1.
  • Failure scenario: a later change breaks the condition — for example, a change to the
    destructured pageSize default, or a change of === to a length check against a
    constant. If the cursor becomes always null, infinite scroll stops after one page
    of 25 rows. If the cursor becomes always non-null on a short page, the observer loop
    issues one extra empty fetch per scroll end. No test catches either regression.
  • Suggested fix: add repository list tests (mock the RPC) for the four cases: full
    page with more rows, exact-full last page, short page, empty page.

Nit

3. After dispose, the repository factory still runs before the abort check

  • File: packages/wallet-sdk/domain/transactions/transactions-api.ts, all four methods.
  • Each method awaits getRepository() before the first signal.aborted check. When the
    signal is already aborted at entry (post-dispose call), the default path relies on
    the self-fencing getEncryption() rejection, and the createRepository test seam runs
    its factory for nothing. No repository call is issued either way, so the invariant
    holds.
  • Suggested fix: add if (signal.aborted) throw new SessionEndedError() directly after
    the sessionSignal() capture. This also makes the post-dispose error type
    independent of the getEncryption() rejection type.

4. Session-end tests do not cover countPendingAck and acknowledge mid-read

  • File: packages/wallet-sdk/domain/transactions/transactions-api.test.ts.
  • The mid-read SessionEndedError tests cover list and get only. The other two
    methods share the same code shape, so the risk is low, but acknowledge is the one
    method with a write, and its post-write fence is the one a future edit is most likely
    to remove as "pointless after success".
  • Suggested fix: add the mid-call abort test for acknowledge (and optionally
    countPendingAck).

Delivered via a maxplayer contribution-mode job (seller forked the repo
and read the context itself; no source pasting).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jbojcic1
jbojcic1 merged commit 5eecf47 into master Aug 13, 2026
5 checks passed
@jbojcic1
jbojcic1 deleted the sdk/transactions-slice branch August 13, 2026 10:34

This branch was successfully deployed

1 active deployment
Preview — bdd298fe Deployed Aug 12, 2026 by vercel[bot]
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