Skip to content

test(wallet): second-pass browser regression for card reveal, freeze and controls - #772

Closed
NewWorldVenture wants to merge 4 commits into
mainfrom
claude/card-ui-second-pass-7q6vtf
Closed

NewWorldVenture wants to merge 4 commits into
mainfrom
claude/card-ui-second-pass-7q6vtf

Conversation

@NewWorldVenture

Copy link
Copy Markdown
Owner

This is task JIMMY-SUPPORT-CARDUI-20261001 (owner review 5378108048 on #712): a second-pass synthetic browser regression for the merged card lifecycle repairs #749 (reveal), #751 (pending ownership) and #753 (form ownership).

One new file: tests/e2e/money-card-lifecycle-second-pass.spec.ts (378 lines, 18 tests), based on main 01b2c380d.

  • No source, fixture, configuration, migration, settings, credential or finalaudit.md change.
  • The three merged specs are untouched, and their security assertions and timeouts are unchanged.

Harness

It uses the same harness as those specs:

  • The real CardRevealModal, MoneyCardsView, Button and card-controls run in Chromium with React 19 (tests/e2e/helpers/react-browser.ts).
  • Every server action and Stripe.js call is replaced before module evaluation.
  • The context is offline, and every request is aborted and recorded. afterEach asserts zero requests and zero unhandled rejections; pageerror fails the test.
  • No card is opened or revealed. No key, provider, authentication, database or financial call is made.

Two additions over the merged fixtures:

  • The view can be unmounted, and mounted again with the props the page still holds. That is navigation away and back before a refresh lands.
  • The reveal gates are per call, so two sessions of the same card can be released separately, and a stage can throw instead of answering a structured refusal.

What it drives that the merged specs do not

Card reveal (#749)

  • Reopening the same card. A is reopened while its old prepare or key is pending. The new session finishes first, then the old completion lands. The old session makes no further call and no display. Only the new session's 3 displays exist, and each is destroyed once on close. The fix(wallet): dispose cancelled card reveal displays #749 spec covers this only for a different card.
  • A step that throws. Five cases, one per step: prepare, import, load, nonce, key. Loading ends, the error is shown, and no display is created.
  • A throw after close. A prepare or key that throws after the modal closes is swallowed: no display, no error, no fault.

Freeze (#751)

  • Unmount while pending. A success, refusal or rejection that lands after the view unmounts dispatches exactly once ({ cardId, frozen: true }) and faults nothing.

Controls and physical order (#753)

  • Controls failure after unmount. A refusal or rejection that lands after the view unmounts presents nothing. fix(wallet): retain card form ownership through dismissal #753 covers this only for success.
  • The dialog's own close button. Closing a pending physical order with the dialog's close button, not Cancel, keeps the claim: the reopened dialog's Order stays disabled and a forced click sends nothing. The late success announces nothing, leaves the reopened dialog open, and releases it.

Reported, not blessed

These are reproductions only. No source was edited, and I'm claiming no source ownership. Each test is labelled "today: … (reproduction)" so a change in either direction shows up.

  1. Pending claims do not survive a remount.
    • The claim set is busyRef inside the mounted MoneyCardsView (components/wallet/money-cards-view.tsx, claimPending).
    • Leaving the page and returning before the refresh lands gives a fresh view with no claims and the old props.
    • In that state, the reproduction dispatches a second setCardFrozenAction while the first is pending, and issues a second virtual card for the same child while its first issueCardAction is pending.
    • Each child row always offers Virtual and Physical, and issueCardAction has no one-card-per-child guard, so the second issuance would really happen.
    • Whether a claim should outlive the view is a product decision.
  2. A freeze that lands after unmount is still announced and refreshed ("Card frozen", one refresh).

One note outside these tests. CardRevealModal's effect depends on useTranslations(), which is memoized inside LocaleProvider. Outside a provider, it returns a new function every render, which would restart the reveal session on each render. Every app route renders inside the provider, so this is not a live defect. I found it because a non-memoized mock did exactly that.

Evidence (local, Node 24.21, Chromium)

  • The new spec: 18 out of 18 pass, and 54 out of 54 across three --repeat-each=3 runs.
  • All four card specs together: 87 out of 87 pass.
  • Mutations: 11 restored component mutants, 11 killed by this spec. The cases:
    • the old prepare or key completion adopted;
    • a thrown step left loading or shown nothing;
    • a controls refusal or rejection shown after unmount;
    • the reopened order not pending;
    • a dismissed order announcing success or closing the reopened dialog;
    • two candidate fixes: claims kept across a remount, and freeze presentation silenced after unmount.
  • For comparison, the three merged specs kill 9 of the same 11. The two candidate-fix behaviours are observed only here.
  • Static checks: ESLint on the file is clean, and tsc --noEmit is clean.
  • Scanners: the 18 unit test files that scan tests/e2e pass (197 tests).
  • How it was run locally. @playwright/test 1.61 expects a newer Chromium than this machine has, so I used a scratchpad-only config pointing launchOptions.executablePath at the preinstalled one. Nothing was downloaded, and the repo config is unchanged. Hosted CI's E2E job picks the spec up through testMatch: '**/*.spec.ts'.

Limits

  • Synthetic UI contract only, with stubbed actions and SDK.
  • No real Stripe Issuing Elements, server actions, RLS, provider behaviour or served browser workflow.
  • The remount finding is about the client claim. Server-side idempotency of issuance and freeze is not measured.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS


Generated by Claude Code

…and controls

JIMMY-SUPPORT-CARDUI-20261001. One new spec beside the merged lifecycle
repairs (#749, #751, #753), on the same harness: the real components in
React 19, every server action and Stripe.js call replaced before module
evaluation, the context offline and every request aborted and recorded.

Drives what their specs do not:
- reveal: the same card reopened while its old prepare or key is pending
  (the old completion makes no further call and no display); a thrown
  prepare, import, load, nonce or key ends loading with an error and no
  display; a prepare or key that throws after close faults nothing.
- freeze: an outcome that lands after the view unmounts dispatches once and
  faults nothing.
- controls: a refusal or rejection after the view unmounts has no stale
  presentation.
- physical order: the dialog's own close keeps the pending claim like
  Cancel; its late success neither announces nor closes the reopened dialog.

Reported, not blessed (reproductions, no source change): claims live in
the mounted view, so a remounted view dispatches a second freeze, or issues
a second virtual card, while the first is still pending; and a freeze that
lands after unmount is still announced and refreshed.

18 tests; 11/11 restored component mutants killed (the existing three specs
kill 9/11; the remount and post-unmount freeze behaviours are only observed
here).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
@vercel

vercel Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
bubaly Ignored Ignored Oct 2, 2026 11:45pm UTC

Request Review

@NewWorldVenture NewWorldVenture left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support: received JIMMY-SUPPORT-CARDUI-20261001 evidence and current subscribed destination. Keep this draft test-only while its exact-head CI runs. Next bounded investigation JIMMY-SUPPORT-CARD-RETRY-20261001: trace existing server/provider idempotency and UI remount behavior for the reproduced duplicate physical/virtual issuance and freeze requests, using synthetic tests only. Determine which cases are the same logical user attempt versus a deliberately new order; preserve any legitimate multiple-card behavior. Do not invent a one-card-per-child rule. Report exact failure reproduction, existing product contract, smallest repair options and tests before source changes, especially any choice requiring a new business rule. No real cards/keys/provider calls, credentials, migration or ledger changes. NWV owns finalaudit. The post-unmount freeze announcement is an open UX decision, not automatic permission to suppress completion of an action already committed.

NewWorldVenture pushed a commit that referenced this pull request Oct 1, 2026
…d to the checkpoint

Owner review 5378367970. #772 07602d4 is recorded as author-reported
local evidence (18 new / 54 repeat / 87 combined synthetic browser
cases, 11 mutants), with its hosted CI still running and its two
reported, unfixed residuals. No one-card-per-child policy, no Register B
row, no PASS. #768 promotion/merge awaits the owner's explicit approval.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRUgA6hD6QgzmtpSP6TUmP

Copy link
Copy Markdown
Owner Author

JIMMY-SUPPORT-CARD-RETRY-20261001: investigation report. No source changes.

The evidence is a test-only draft, #773, on claude/card-retry-investigation-7q6vtf at 2526ec2ca (based on main 01b2c380d). It adds tests/money-card-retry-characterization.test.ts with 9 sealed tests:

  • network blocked and attempts counted;
  • the real Stripe SDK and the Supabase client factories forbidden;
  • an in-process provider double with Stripe's documented idempotency rules;
  • an in-memory database with the 00901 uniques.

They drive the real issueCardAction, setCardFrozenAction, lib/stripe/issuing.ts and handleIssuingCardUpdated. This PR (#772) is unchanged at 07602d49c.

1. Existing product contract (what must be preserved)

  • Schema (supabase/migrations/00901_stripe_money.sql):
    • One cardholder per child: UNIQUE (family_id, member_id) (:72).
    • Cards are "virtual + physical", constrained only by UNIQUE (stripe_card_id) (:101), plus a non-unique (family_id, child_wallet_id) index (:103). Any number of cards, of either type, per child.
  • UI (components/wallet/money-cards-view.tsx):
    • Every child row always offers Virtual and Physical.
    • The header counts "N cards".
    • "Issue all" targets only children with no card. That is a bulk convenience, not a limit.
    • No document defines a card-count rule. I have not invented one, and the test P1 pins several cards per child.
  • Keys:
    • Every other Stripe create uses a stable per-resource key: connect-acct-<family> (connect.ts:60), fa-<family> (treasury.ts:38) and cardholder-<member> (issuing.ts:62).
    • Cards alone use card-<wallet>-<Date.now()> (issuing.ts:99). The clock in that key is what lets a deliberate repeat order through. It is also why nothing can tell a retry from a new order.
  • No attempt identity reaches the server. issueCardAction accepts {childWalletId, type, spendLimitCents, spendWindow} and nothing else (actions.ts:119–120). Only the client knows whether a request is a retry.
  • Freeze sends an explicit target, frozen: !card.isFrozen, taken from the props at click time (view :175). setCardFrozen sets the provider status, then the mirror. issuing_card.updated reconciles the mirror from the provider.

2. Exact reproductions

Test What was sent Same attempt or new order? Current result
R1 ×2 The same virtual or physical input, again 1 ms later (a remounted view's re-click; the UI half is reproduced in this PR's spec) Same attempt A second live card. Keys card-wallet-a-T0 and …-T0+1, giving 2 provider cards, 2 mirror rows and 2 card_issued audits. For physical, that means a second shipped card
R4 Provider created the card, then the mirror insert failed The parent's retry is the same attempt The action answers money.couldNotIssueTheCard. The live provider card is listed nowhere, and no cancel or deactivate is sent. The reconciler skips it ("does not mirror", webhook.ts:327). The retry issues a second live card
R2 The same input twice within 1 ms Same attempt The provider replays one card, and the second insert hits 23505 on stripe_card_id. The second caller reads the untranslated That already exists. Try a different value. (errors.ts:56–57), although the card exists
R5 First virtual and physical orders together, for a child with no cardholder New orders, both legitimate One is refused at the cardholder insert (UNIQUE (family_id, member_id), issuing.ts:73) with "That already exists", and no card is created for it. It is reachable in one mounted view, because the claims issue-<child> (:117) and physical-<child> (:375) are independent. Ordering again succeeds
R3 Virtual and physical in the same millisecond New orders They share a key (it carries no type), so the provider refuses the second and the parent sees "could not issue". This is an edge case
P1 Virtual, physical, virtual for child A, then virtual for B, minutes apart New orders All four issue, with one cardholder per child. This is the contract to keep
F1 Freeze, then freeze again (a remount repeats the direction because the props have not refreshed) Same attempt Converges. The provider is inactive and the mirror frozen; the only side effect is two card_frozen audits
F2 Freeze and unfreeze in flight, with the responses arriving in reverse order Different intents The provider ends active but the mirror says frozen, until issuing_card.updated corrects it. This needs a refreshed opposite label while the first request is still outstanding (a second parent or tab). A remount cannot produce it

3. Same attempt vs deliberate new order

  • Same logical attempt (should not issue twice):
    • a re-click after a remount while the original is in flight (R1);
    • a retry after a failed or unknown outcome where the provider did create the card (R4);
    • a duplicate dispatch of an identical order (R2).
  • Deliberate new order (must keep issuing):
    • a click after the previous attempt settled and the list refreshed (P1);
    • a different type (R3, R5);
    • a different child;
    • "Issue all".
  • Cannot be told apart with today's inputs: two parents or tabs placing identical orders independently. De-duplicating that on the server would need a new business rule (for example "one pending order per child and type"). I am not proposing one.

4. Smallest repair options (none implemented)

Option Change Fixes New business rule? Migration or ledger? Owner
A Move the view's pending claims into a module-level registry, keyed as today, so a remounted MoneyCardsView hydrates them and keeps its buttons disabled until the original promise settles. View file only. Mutant VM6 in this PR's spec shows the shape The R1 UI path; F1 duplicates No No money-cards-view.tsx (not the money actions)
B Attempt-scoped key. The client mints an attemptId per order, kept in A's registry and reused only for a retry of the same order after a failed or unknown outcome; a new id after success or any change of child, type or limit. issueCardAction takes an optional validated attemptId, the key becomes card-<wallet>-<type>-<attemptId> (today's key when it is absent), and a 23505 on stripe_card_id for the same card id is treated as already recorded: re-read and return it R1, R2, R3, and R4 for as long as Stripe keeps the key (at least 24 hours). P1 is unchanged No: each attempt still issues exactly one card No Jimmy (money actions plus issuing lib), so it needs coordination
C In ensureCardholder, on a 23505, re-read (family_id, member_id) and return that row if its stripe_cardholder_id equals the one the provider returned (same stable key, so same cardholder). Mutant C1 flips only R5 R5 No No Issuing lib
D Map these 23505s to the action's existing translated fallback. No new i18n keys The R2 and R5 wording (largely moot after B and C) No No Money actions
E Orphan compensation beyond B: cancel vs adopt is a product decision. Naive cancel-on-mirror-failure (mutant O1) is unsafe, because under a replayed key it cancels the already-listed card (R2). Adopting through issuing_card.created would mean creating mirror rows from webhook metadata, which is a trust-boundary decision R4 without a retry Yes (decision) Possibly Coordinator
F A durable server-side attempt ledger Multi-tab and reload cases Depends Yes (migration) Not proposed

Freeze: no server repair recommended.

  • Duplicate requests converge, because each one sends an explicit target.
  • Opposite requests answered out of order self-heal through issuing_card.updated.
  • A compare-and-set ("refuse if the card changed since the view rendered") would be a new rule.
  • The post-unmount announcement stays your open UX decision. None of A–D suppresses the completion or the announcement of an action already committed.

Suggested minimum: A + C. Both are local, need no business rule and no migration. Add B if R4 (a second live or shipped card after a "could not issue") must be closed; it needs Jimmy's agreement because it touches app/(app)/money/actions.ts.

5. Tests and evidence

  • Locally, under Node 24.21:
    • the new file: 9/9 pass;
    • eslint and tsc --noEmit: clean;
    • full vitest suite: 1824 files, 23,937 tests passed plus 2 expected failures (run on the pre-amend commit; the amend touched only this file, which was re-run).
  • Mutants: 9/9 killed, restored afterwards, worktree clean.
    • K1, a stable per-type key (an invented one-card-per-type rule), was killed by the preserve test P1 among others.
    • O1, naive cancel, was killed by R2 and R4.
    • C1, the candidate repair C, was killed by R5 alone.
    • The other six (K2, S1, F1, F2, W1, E1) are listed in the draft's body.
  • How the tests move under each repair:
    • A changes nothing here; it flips this PR's remount reproductions in the browser spec.
    • B flips R1 (when the id is reused), R2, R3 and R4, and must keep P1.
    • C flips R5 only. Under C alone, two orders sent in the same millisecond would still share R3's key; B removes that.
  • Scope kept: no real cards, keys or provider calls; no credentials, migration, ledger or finalaudit changes. I won't push any source change until you pick the options.

Generated by Claude Code

NewWorldVenture pushed a commit that referenced this pull request Oct 1, 2026
…l CI and the A+C execution blocker

Owner review 5379042101. The Last Updated line and section heading said
10:55 UTC but included #772 (10:58), #773 (11:21) and the A+C assignment
(11:23); those additions now carry their own times. Records the terminal
all-four CI for #771/#772/#773 (receipt 5930953854) and Support's A+C
source-edit permission blocker (#712 comment 5930951363). Ownership is
not execution clearance; no workaround, no PASS, R4 unresolved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRUgA6hD6QgzmtpSP6TUmP
NewWorldVenture pushed a commit that referenced this pull request Oct 1, 2026
… CI wording

Owner review 5379767360. #771's exact head CI36863717027 failed only E2E
on the expired fixed-date fixture social-publish-consumers.spec.ts:269
(1 failed / 1,908 passed); recorded as blocked, not a full gate. Same
on #774 af53da9. Fixture-only #775 046bc33 and #774 ecb626b carry
the repair with CI running; #771's green E2E depends on #775. Last
Updated no longer calls #772's hosted CI pending. No PASS, no fixture
copy in this docs-only branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRUgA6hD6QgzmtpSP6TUmP
claude added 2 commits October 2, 2026 19:43
…ard harness

#811 (repair A) makes MoneyCardsView read useAuthenticatedCacheScope()
to key pending claims by signed-in owner. This sealed harness throws on
any unmocked module, so it now mocks that import as "no settled
sign-in", which keeps each mounted view's claims private: exactly the
per-view behaviour these reproductions pin. Against main (where the
view does not import it yet) the mock is unused.

Checked: on this branch 18/18 (plus the social spec 12/12 after the
main merge carrying #775's date fix); against #811's view 18/18.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS

Copy link
Copy Markdown
Owner Author

Update: #772 is now at 143e922e2. Both changes are test-only.

Still a draft with no source change.


Generated by Claude Code

…nd-pass card harness

#811 now keys pending card claims by the actor and family the page was
rendered for (useApp), not the client session scope. The sealed harness
mocks that import as an unknown identity, which keeps each mounted
view's claims private: the per-view behaviour these reproductions pin.
18/18 on main and 18/18 against #811's revised view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS

Copy link
Copy Markdown
Owner Author

Update: #772 is now at ec52e640d, and hosted CI is all green (run 37079062687).


Generated by Claude Code

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