Skip to content

Money: a card refund is credited back to the child, and each card capture or refund posts once - #925

Closed
NewWorldVenture wants to merge 14 commits into
mainfrom
claude/issuing-refund-credits-7q6vtf
Closed

NewWorldVenture wants to merge 14 commits into
mainfrom
claude/issuing-refund-credits-7q6vtf

Conversation

@NewWorldVenture

@NewWorldVenture NewWorldVenture commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Draft for review: do not merge without the owner. It was claimed before editing in #771 (5971699986), and it is its own branch from main 40bf5ff36.

What was wrong

Stripe sends every Issuing transaction as issuing_transaction.created, with type: 'capture' | 'refund'. Its amount is "the transaction amount, which will be reflected in your balance" (stripe-node Issuing.Transaction): negative for a capture, positive for a refund.

handleTransactionCreated in lib/stripe/webhook.ts took Math.abs(txn.amount) and posted a card_spend debit for anything non-zero. So a merchant refund took the money out a second time.

This was reproduced through the real /api/webhooks/money route. The events were signed by stripe-node's own test signer, so the real signature check ran, followed by the real handlers and wallet helpers, against an in-memory store:

Child's Spend expected on main
$50, buys $20, refunded $20 $50 $10
$50, buys $20, refunded $7.50 $37.50 $22.50

The card_refund type exists in the enum and the activity labels, but nothing wrote it. No test covered a refund.

The change

  • lib/stripe/webhook.ts, handleTransactionCreated only. It now goes by the sign Stripe defines:

    • a negative amount is debited exactly as before;
    • a positive amount is credited;
    • zero writes nothing.

    In this commit the hold release was left as it was; commit 3 (below) makes it capture-only, so a refund no longer releases a hold. A failed refund write throws, so the route answers 500 and Stripe retries, the same as a failed debit.

  • lib/wallet/server.ts, new creditCardRefund beside debitCardSpend (untouched in this commit; commits 7 and 9 change only its conflict path).

    • It posts one completed card_refund credit to the child's Spend bucket.
    • It is idempotent on the Stripe transaction id, as the debit is, and it is audited as card_refund.
    • It deliberately does not use creditChildWallet, whose split would scatter a refund across Save, Give and Invest. A refund is the purchase undone, so it lands where the purchase's debit was.

The webhook route is unchanged. Commits 5 and 8 add two repository migration candidates, each a partial unique index. Commit 11 changes two readers of the ledger. Both are described below.

Tests

tests/a-card-refund-credits-the-child.test.ts contains signed events through the real route, with no network calls.

  • Commit 1, 0e53c7c4f (tests only). On main, 6 cases fail:

    • full refund;
    • the row written;
    • partial refund;
    • redelivery of the same event;
    • the same refund under a second event id;
    • the audit row.

    4 cases pass and must keep passing:

    • a refund lands in Spend whatever the split;
    • a capture debits once and releases its hold;
    • a zero amount writes nothing;
    • an unknown card answers 500 and writes nothing.
  • Commit 2, 35b966966 (the fix). All 11 cases pass: the 10 above plus a duplicate-check read failure, which answers 500 and writes nothing. The zero case now also asserts the 200.

  • Mutation check. 13 hand mutants, all killed:

    • debiting the absolute value;
    • the refund branch removed;
    • zero routed to the debit;
    • the refund put in another bucket or typed card_spend;
    • no idempotency, or idempotency keyed on the wrong type;
    • the duplicate-read error swallowed;
    • no audit row;
    • the refund left processing;
    • the refund failure swallowed;
    • the hold not released;
    • the wrong amount.

Local validation on 35b966966: tsc --noEmit clean; npm run lint 0 errors; every test file touching lib/stripe/webhook, lib/wallet/server or the money route (29 files, 539 tests) passes; full vitest run 1,874 files passed (1 skipped), 25,418 tests passed, 2 expected-fail, 73 skipped.

Commit 3, f18973454 (independent self-review of this PR, before any human review).

  • The hold is released only by a capture now. Before, a refund also released its authorization's hold. Stripe doesn't promise event order. So a refund processed before its capture cancelled the $20 hold and credited the $20 refund. Spendable, which is completed plus processing and is what wallet_reserve_card_auth decides on, went from $30 to $70. A $70 authorization would then be approved, and the late capture would leave Spend at -$20. A refund now leaves any hold to its capture, or to issuing_authorization.updated.
  • A refund row records its authorization in metadata.authorization (null when Stripe sends it unlinked), so reconciliation can match a refund to its purchase.
  • lib/wallet/server.ts's header no longer calls creditChildWallet the only credit writer.
  • Tests: two cases were added: refund-before-capture (spendable goes from $30 with the hold to $50 on the refund, and stays $50 after the capture) and the authorization link. Both failed before the commit; 13/13 pass now.
  • Mutation check: 15 mutants of the refund path, all killed, including "a refund also releases the hold" and "the authorization isn't linked".
  • Local validation on f18973454: tsc --noEmit clean; npm run lint 0 errors; full vitest run 1,874 files passed (1 skipped), 25,420 tests passed, 2 expected-fail, 73 skipped.

Commit 4, dac7b5343 (owner review 4174926185): the refund key in the database. The duplicate check is a select followed by an insert. Two overlapping deliveries of one refund, under two event ids or through a stale-claim reclaim, therefore both pass the select.

  • The database key: a partial unique index, uq_wallet_txn_card_refund_ref on wallet_transactions (stripe_ref) where type = 'card_refund'. Its migration number was requested in feat(admin-digest): per-recipient delivery engine with fenced claims, frozen payloads and retention-bounded retries (not wired) #699 (5974164780) and allocated there (5974208357); the file is commit 5, below.
  • Conflict handling: with the index in place, the losing insert fails with 23505. creditCardRefund then reads back the refund that won and answers success, so the second delivery is acknowledged rather than retried, and the child is credited once. A 23505 with no refund to read back is still a failure.
  • Tests:
    • Two overlapping deliveries under different event ids, with the store given the same unique key. Both reach the insert (the test counts them), both answer 200, and one refund row is written. This case answered 500 before the commit.
    • A 23505 from another key still answers 500 and writes nothing.
    • 15/15 pass. Three mutants of the conflict path, all killed.
  • Local validation on dac7b5343: tsc --noEmit clean, npm run lint 0 errors, full vitest run 1,874 files passed (1 skipped), 25,422 tests passed, 2 expected-fail, 73 skipped.

Commit 5, 1e296b83e: supabase/migrations/0484_a_card_refund_is_credited_once.sql. This is a repository candidate only. It has not been applied to any hosted database, and applying it to production is the owner's decision.

create unique index if not exists uq_wallet_txn_card_refund_ref
  on public.wallet_transactions (stripe_ref)
  where type = 'card_refund' and stripe_ref is not null;
  • The slot: 0484 was allocated for this PR in feat(admin-digest): per-recipient delivery engine with fenced claims, frozen payloads and retention-bounded retries (not wired) #699 (5974208357, 5974209069). I rechecked every open PR's changed files just before pushing, and no other PR uses 0484.
  • Replay on a local PostgreSQL 16 (throwaway cluster, all migrations replayed from docs/audit/pg-bootstrap.sh, then 0484):
    • a second card_refund with the same stripe_ref is refused with 23505;
    • a card_spend and a card_refund sharing one stripe_ref are both accepted;
    • two card_refund rows with a null stripe_ref are both accepted;
    • two concurrent sessions inserting the same refund: the second waited about 2 s on the first's open transaction, then got 23505, leaving one row.
  • Readiness check: an existing duplicate card_refund pair would make create unique index fail. Nothing in the application wrote card_refund before this PR, so none should exist. An operator can confirm with select stripe_ref from wallet_transactions where type = 'card_refund' and stripe_ref is not null group by 1 having count(*) > 1; before applying.
  • The version pin: tests/migration-version-safety.test.ts moves to 0485 on this branch, with the same note every migration PR adds.

Commits 6–8: the same race on the capture side (scope extension claimed in #771, 5974374817).

  • The problem: debitCardSpend checks for a capture with a select and then inserts, exactly as the refund did. Two overlapping claims of one capture both pass the select. Reproduced through the real route: one $20 capture delivered twice, overlapping, answered 200 twice and left Spend at $10 instead of $30.
  • Commit 6, b995e708c (test): overlapping capture deliveries, with the store given the database key. The test counts the inserts to prove the overlap. It answered [200, 500] before commit 7. A control, a 23505 from another key, answers 500 and leaves the hold in place.
  • Commit 7, 5e041781b (fix): on 23505, debitCardSpend reads back the completed capture that won and answers success. The second delivery is then acknowledged, the hold is released once and the child is debited once. This mirrors creditCardRefund. lib/stripe/webhook.ts is unchanged.
  • Commit 8, c79fe1f00: supabase/migrations/0485_a_card_capture_is_debited_once.sql, slot allocated on feat(admin-digest): per-recipient delivery engine with fenced claims, frozen payloads and retention-bounded retries (not wired) #699 (5974392907):
    create unique index if not exists uq_wallet_txn_card_capture_ref
      on public.wallet_transactions (stripe_ref)
      where type = 'card_spend' and status = 'completed' and stripe_ref is not null;
    • Holds are left out by status = 'completed': they are processing until released to cancelled, and 0342 records that a replay can place a second hold.
    • Spend requests are left out by the null filter: they carry no stripe_ref.
  • Replay on a local PostgreSQL 16, with all migrations and every existing ledger shape seeded before the build:
    • a second completed capture for one ref is refused with 23505;
    • these are all accepted: a live, a cancelled and a replayed hold for one authorization; null-ref spend requests and an approval completing one; a refund sharing the capture's ref;
    • two concurrent sessions: the second waited about 3 s on the first and got 23505, leaving one row;
    • with an existing duplicate capture, the build fails, and the header's readiness query names the ref.
  • Readiness: unlike card_refund, completed captures already exist. A duplicate in production would be a child debited twice for one purchase, and would make the build fail. The file's header carries the query to run first. Correcting any rows it finds is an operator decision.
  • Mutation check: 5 mutants of the conflict path. 4 are killed: no 23505 branch, success without a row, reading back a refund, reading back the hold's status. One is equivalent: reading back after any error code, which still answers success only when the completed capture exists.
  • The version pin moves to 0486 on this branch.

Commits 9–10: from an independent review of the capture and refund conflict paths.

  • Commit 9, c127f28cb (fix):
    • Duplicate check: debitCardSpend's check before the insert now filters on status = 'completed', the 0485 key's own predicate. On a local PostgreSQL 16 with the index built, that query is an index scan on uq_wallet_txn_card_capture_ref. The old (stripe_ref, type) query was a sequential scan even with enable_seqscan off. Holds are keyed by the authorization id, so no existing row is treated differently.
    • Read-back failure: when the read-back after a 23505 fails, that error is the one logged and returned, for captures and refunds. Before, only the 23505 reached the log, which reads as a harmless race.
    • Tests: a 23505 where the only same-ref card_spend is not completed fails and keeps the hold. A failing read-back fails and logs its own error. The overlap test counts one audit row. The 0484 and 0485 index statements are pinned exactly, because the in-memory store cannot express a partial key.
    • Mutation: 5 more mutants, all killed.
  • Commit 10, 73ebbd82d (comments only):
    • Apply notes: a failed CONCURRENTLY build leaves an INVALID index of the same name, which a rerun of if not exists silently skips. Both notes now say to drop it first.
    • 0485's hold wording: it now matches 0155's header. 0155 allows a hold to move processing → completed/cancelled, and no code completes one today.
  • Local validation on 73ebbd82d: tsc is clean, lint has 0 errors (its 15 warnings are all in files this PR does not touch), the i18n gate is clean, and the full vitest suite passes: 1,874 files, 25,429 tests. The card test file has 22 tests.

Commit 11, fbe64d82b: a refund is spending undone, not new money (owner review 5976939812; scope claimed on #771, 5976950098).

  • The problem: commit 2 makes a refund a card_refund credit. Two readers counted every completed credit as money coming in:
    • the treasury (app/(app)/wallet/treasury/page.tsx): "this month in", each child's "+$X this month" and the 6-month trend;
    • the coach (app/api/ai/wallet/route.ts): the 8-week contribution rate behind each goal's weeksToGoal.
  • The fix: one rule, moneyFlow() in the new lib/wallet/flows.ts, used by both readers.
    • A credit counts as in, a debit as out, and a card refund nets against out.
    • "In" stays new money and "out" becomes net spending. The treasury's Net (in − out) is still the ledger's real change.
    • The coach counts only what is in, so a returned purchase no longer shortens a goal's forecast.
    • Balances are not affected: for a balance, a refund is an ordinary credit.
  • Tests: tests/a-card-refund-is-not-new-money.test.ts drives the real page and the real route against an in-memory store.
    • Treasury: a $50 top-up, a $20 purchase and its $20 refund give in $50, out $0 and net +$50, and the per-child line and the trend agree. On the old code "in" was $70.
    • Coach: $80 of allowance and a $40 refund over 8 weeks forecast a $100 goal at 10 weeks. On the old code the forecast was 7.
    • Mutation: 3 mutants, all killed: the per-child line reverted, the trend reverted, and the refund dropped instead of netted.
  • Not changed:
    • The per-child coach (app/api/ai/wallet/child/[childId]/route.ts) counts only Save-bucket credits, and refunds go to Spend.
    • The admin and reconciliation "credit volume" totals are gross ledger volume by name.
  • Overlap with A server page renders the family's date: 24 zone-less date renders on 13 family server files, and the three emails take the family's zone #942: A server page renders the family's date: 24 zone-less date renders on 13 family server files, and the three emails take the family's zone #942 rewrites the trend loop's month keys in the treasury page, on lines next to this commit's. Whichever of the two lands second keeps A server page renders the family's date: 24 zone-less date renders on 13 family server files, and the three emails take the family's zone #942's month keying and applies moneyFlow inside it.
  • A month with more refunds than purchases shows a negative "out". That is the true net for that month.
  • Local validation on fbe64d82b: tsc is clean, lint has 0 errors (its 15 warnings are all in files this PR does not touch), the i18n gate is clean, and the full vitest suite passes: 1,875 files, 25,432 tests.

Commits 12–14: #942 composed in, and a refund reversal.

Not covered (residuals, not closed by this)

  • Release on refund: fixed in commit 3 (above). A refund no longer releases its authorization's hold; only a capture does (a refund leaves the purchase's hold to the purchase in the test file).
  • Refund events already in flight at deploy: a refund event that the old code partly processed, writing its wrong card_spend debit and then failing before processed, would be credited on retry. The wrong debit (keyed to the refund's transaction id) would stay, so the child nets zero instead of the refund. Such rows have exactly that signature, a card_spend whose stripe_ref is a refund transaction. Finding and correcting them is the same operator action as the earlier refunds below.
  • Stale reclaim and overlapping deliveries: addressed by commits 4–5 for refunds, 6–8 for captures and 9 for both. Until 0484 and 0485 are applied to a database, that database has no key, the conflict handling never runs there, and the race remains.
  • Stripe's transaction type isn't stored: the ledger type comes from the amount's sign. The transaction's type decides only whether a hold is released (commit 14). A negative-amount Stripe refund is therefore stored as a debit (card_spend) that touches no hold, and a positive amount is credited as a refund whatever its type.
  • Unknown card: a refund for a card with no mirror row answers 500 and is retried, exactly as a capture already is. Whether such events should be skipped instead is a decision for the authorization path's owner.
  • Other credits that are not new money: commit 11 covers card refunds. Internal transfer credits (transfer, bucket_transfer, goal_transfer) and reversal credits are still counted as "in" by the treasury and the coach, as they were before this PR.
  • Holds. wallet_reserve_card_auth (0155/0342) keys a hold by its authorization id, with an in-lock check scoped to processing. 0342 records that a replay after capture places a fresh hold. That path is unchanged here, and 0485 leaves holds out by design.
  • When a hold is released (older, in lib/stripe/webhook.ts, outside this PR's file claim). These were found by the review above and not changed here:
    • handleAuthorizationUpdated releases the hold when the authorization becomes closed. Stripe does not order that event before issuing_transaction.created, so the held money can become spendable before the capture debit is written.
    • The first capture cancels the whole hold, even for a partial capture whose remainder Stripe can still capture later.
    • Either can let a child spend money that a later capture then takes, leaving Spend negative.
  • One statement, not three. The capture insert, the hold release and the audit row are still separate calls outside the spend-bucket lock. A single SQL function, the 0155/0342 pattern, would close the race at the source. 0485 plus the read-back stops only the duplicate debit.
  • The read-back trusts the ref. It matches any completed capture with that stripe_ref, as the check before the insert already did. Stripe transaction ids are unique, so a row from another child would have to be forged by someone who can already write that family's ledger.
  • Earlier refunds. Any refund already processed on a live account was debited. Finding and correcting such rows would be an operator action on production data, and it is not part of this PR. The reconciliation page does not flag them, because they look like ordinary spends.
  • Scope of the evidence. This is not a live provider test. The Stripe sign convention is taken from stripe-node's type documentation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS


Generated by Claude Code

claude added 2 commits October 3, 2026 17:35
Stripe sends Issuing refunds as issuing_transaction.created with
type 'refund' and a positive amount ("reflected in your balance";
captures are negative). handleTransactionCreated took Math.abs(amount)
and posted a card_spend debit for anything non-zero.

Through the real /api/webhooks/money route, with events signed by
stripe-node's own test signer and an in-memory store: a child with $50
in Spend buys $20 and is refunded $20, and is left with $10, not $50.
A $7.50 partial refund leaves $22.50 instead of $37.50.

Six cases fail on main (full refund, the row written, partial refund,
redelivery, a second event id for the same refund, the audit row).
Four pass and must keep passing: a refund lands in Spend whatever the
split, a capture debits once and releases its hold, a zero amount
writes nothing, an unknown card fails with 500 and writes nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
Stripe's Issuing transaction amount is what the transaction does to the
balance: negative for a capture, positive for a refund.
handleTransactionCreated debited Math.abs(amount) either way, so a
merchant refund took the money out a second time.

It now goes by that sign: a negative amount is debited as before; a
positive amount is credited through a new creditCardRefund, which posts
one completed card_refund credit to the child's Spend bucket, idempotent
on the Stripe transaction id like the debit, and audited. It does not go
through creditChildWallet: a refund is the purchase undone, so it lands
where the purchase's debit was rather than being split across Save,
Give and Invest as new money. The hold release is unchanged. A failed
refund write throws, so the route answers 500 and Stripe retries.

Two cases added with the fix: a zero amount is acknowledged (200) and
writes nothing; a refund whose duplicate check cannot be read fails
with 500 and writes nothing. No schema, migration or route change.

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

vercel Bot commented Oct 3, 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 4, 2026 12:50pm UTC

Request Review

Comment thread lib/wallet/server.ts
Self-review of #925, finding 1: handleTransactionCreated released the
authorization hold for every Issuing transaction, refunds included.
Stripe does not promise event order, so a refund processed before its
capture cancelled the $20 hold AND credited the $20 refund: spendable
(completed + processing, what wallet_reserve_card_auth decides on) went
from $30 to $70, a $70 authorization would be approved, and the late
capture left Spend at -$20.

The release now runs only in the capture branch; a refund leaves any
hold to its capture or to issuing_authorization.updated. A refund also
records the authorization it belongs to in its metadata (null when
Stripe sends it unlinked), so reconciliation can match it to the
purchase it undoes. lib/wallet/server.ts's header no longer calls
creditChildWallet the only credit writer.

Two cases added, both failing before this commit: refund before capture
keeps the hold and ends at $50 spendable after both; the refund row
names its authorization. 15 mutants of the refund path, all killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
claude added 7 commits October 3, 2026 22:32
…ccess

Review 4174926185 on #925: creditCardRefund's duplicate check is a
select and then an insert, so two refund deliveries for one Stripe
transaction that overlap (two event ids, or a stale-claim reclaim) both
pass the select. With the database key the review asks for (a unique
index on stripe_ref for card_refund rows; number requested in #699,
5974164780, and not stamped until allocated) the second insert fails
with 23505. creditCardRefund now reads the refund that insert lost to
and answers success, so the second delivery is acknowledged rather than
retried, and the child is credited once. A 23505 with no refund to read
back is still a failure.

Tests: two overlapping deliveries under different event ids, with the
store given the same unique key; both reach the insert (counted), both
answer 200, one refund row, Spend $50 (failed with a 500 before this
commit). A 23505 from another key still answers 500 and writes nothing.
Three mutants of the conflict path, all killed.

Without the index this handling never runs and the race remains; the
migration is what closes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
Owner review 4174926185 on #925 asked for the refund transaction key to
be enforced in the database. Number 0484 reserved on #699 (5974208357);
filename coordination only — not an instruction to apply to production.

One statement: a partial unique index on wallet_transactions(stripe_ref)
where type = 'card_refund' and stripe_ref is not null. card_spend is
left alone (a hold and a replayed hold share a ref), NULL refs are
skipped, and nothing wrote card_refund before #925, so no existing row
can conflict.

Proved on a throwaway Postgres 16 with every migration replayed through
0484 (docs/audit/pg-bootstrap.sh): a second card_refund with the same
ref is refused (23505); two card_spend rows sharing a ref and two
card_refund rows with no ref are accepted; two sessions inserting the
same refund — the second waited ~2 s on the first's open transaction,
then failed 23505 once it committed, leaving one row. With the index,
creditCardRefund's 23505 handling (previous commit) answers that second
delivery as the first one's success.

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

The debit side of owner review 4174926185 on #925. debitCardSpend checks
for an existing capture with a select and then inserts, so two claims of
one capture that overlap (two event ids for one transaction, or
recordEvent reclaiming a claim its first holder is still running) both
pass the select. With no key, one $20 purchase delivered twice leaves
Spend at $10 instead of $30.

The store is given the database key requested on #699 (a unique index on
stripe_ref for completed card_spend rows). The overlapping case fails
until the second delivery reads back the capture the first wrote: it
answers 500 today. The control, a 23505 from another key, already
answers 500 and leaves the hold in place, and must keep doing so.

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

debitCardSpend's duplicate check is a select and then an insert. With
the database key requested on #699 (one completed card_spend per
stripe_ref), the losing insert of two overlapping claims fails with
23505. debitCardSpend now reads back the completed capture that won and
answers success, so the second delivery is acknowledged, the hold is
released once, and the child is debited once. A 23505 with no capture to
read back is still a failure, so Stripe retries.

This mirrors creditCardRefund's handling (dac7b53). The key itself is
the migration that follows. Until it is applied, the conflict never
fires and the race remains.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
A partial unique index, uq_wallet_txn_card_capture_ref, on
wallet_transactions (stripe_ref) where type = 'card_spend' and status =
'completed' and stripe_ref is not null. It makes the capture key the
database's, so the second of two overlapping inserts fails with 23505,
which debitCardSpend now answers by reading back the first.

The key covers completed captures only:
- an authorization's hold is a processing card_spend keyed by the
  authorization id, and 0342 records that a replay can place another;
- a spend request is a card_spend with no stripe_ref.

Replayed on a throwaway local PostgreSQL 16 (all migrations, then this
one), with every existing ledger shape seeded first:
- a second completed capture for one ref is refused with 23505;
- holds, cancelled holds and a replayed hold for one authorization, null-
  ref spend requests and their approval, and a refund sharing the
  capture's ref are all accepted;
- two concurrent sessions: the second waited on the first and got 23505,
  leaving one row;
- with an existing duplicate, the build fails and the readiness query in
  the header names it.

Number 0485 was reserved on #699 (5974392907), for filename coordination
only. Not applied to any hosted database. The version pin moves to 0486
on this branch.

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

From an independent review of the capture and refund conflict paths.

- debitCardSpend's duplicate check now filters on status = 'completed',
  the 0485 key's own predicate. On a local PostgreSQL 16 with the index
  built, that query is an index scan on uq_wallet_txn_card_capture_ref;
  the old (stripe_ref, type) query was a sequential scan even with
  enable_seqscan off. A hold is keyed by the authorization id, and only
  this function writes a completed card_spend under a transaction id, so
  no existing row is treated differently.
- When the read-back after a 23505 itself fails, that error is the one
  logged and returned, for captures and refunds alike. Before, only the
  23505 reached the log, which reads as a harmless race.

Tests (all through the real webhook route):
- a 23505 where the only same-ref card_spend is not completed fails and
  keeps the hold;
- a failing read-back fails, logs its own error, keeps the hold (capture)
  and credits nothing (refund);
- the overlap test counts one card_spend audit row;
- the 0484 and 0485 index statements are pinned exactly, since the
  in-memory store cannot express a partial key. The test comment no longer
  calls the store's wider key "the same key".

Mutation: 5 mutants, all killed (read-back without the status filter,
pre-check without it, either read-back error swallowed, an audit row
written on the read-back path).

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

Comment-only; neither index statement changes.

- Both apply notes offer the CONCURRENTLY form. If that build fails (an
  existing duplicate, a cancelled statement), it leaves an INVALID index of
  the same name, and a rerun of the file's `if not exists` then skips it and
  enforces nothing. The notes now say to drop it first and how to list
  invalid indexes.
- 0485's header said holds are "never completed". 0155's header allows a
  hold to move processing -> completed/cancelled; no code completes one
  today. The note now says so, and that a future path completing holds
  must keep to one completed row per authorization id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
@NewWorldVenture NewWorldVenture changed the title Money: a card refund is credited back to the child, not debited a second time Money: a card refund is credited back to the child, and each card capture or refund posts once Oct 4, 2026
@NewWorldVenture

Copy link
Copy Markdown
Owner Author

Review finding: the new card_refund credit also flows into contribution/inflow metrics that currently count every completed credit. For example, app/api/ai/wallet/route.ts adds all credits to recentCreditByChild and uses that as the goal weeksToGoal contribution rate; app/(app)/wallet/treasury/page.tsx counts all credit rows in thisMonthIn/monthlyIn. A refund to the Spend bucket is returned purchase money, not new savings contribution, so a refund can make the coach forecast goals too optimistically and inflate reported monthly inflow. Please exclude card_refund from contribution forecasts and either exclude it from income-style inflow metrics or label those totals as gross ledger credits. The PR body already notes these readers; this comment records the concrete downstream effect.

Owner review 5976939812. Commit 2 makes a refund a card_refund CREDIT,
and two readers counted every completed credit as money coming in:

- app/(app)/wallet/treasury/page.tsx: "this month in", each child's
  "+$X this month" and the 6-month trend;
- app/api/ai/wallet/route.ts: the coach's 8-week contribution rate,
  which forecasts how many weeks a goal is away.

lib/wallet/flows.ts moneyFlow() is the one rule both now use: a credit
is in, a debit is out, and a card refund nets against out. "In" stays
new money, "out" becomes net spending, and the treasury's Net (in − out)
is unchanged. The coach counts only what is in, so a returned purchase
no longer shortens a goal's forecast. Balances are untouched: for a
balance a refund is an ordinary credit.

Not changed:
- app/api/ai/wallet/child/[childId]/route.ts counts only Save-bucket
  credits, and refunds go to Spend;
- the admin and reconciliation "credit volume" totals are gross volume
  by name.

Tests, driving the real page and route against an in-memory store:
- treasury: a $50 top-up, a $20 purchase and its $20 refund give in
  $50, out $0, net +$50, the same per child and in the trend; on the
  old code in was $70;
- coach: $80 of allowance and a $40 refund over 8 weeks forecast a
  $100 goal at 10 weeks; on the old code it was 7.
Mutation: 3 mutants, all killed (per-child line reverted, trend
reverted, refund dropped instead of netted).

Scope claimed on #771 (5976950098) before editing.

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

Copy link
Copy Markdown
Owner Author

Addressed in fbe64d82b (pushed). This answers 5976939812.

  • The rule: lib/wallet/flows.ts moneyFlow() is one rule for "money in / money out". A credit counts as in, a debit as out, and a card_refund nets against out: it is spending undone, not new money.
  • Coach (app/api/ai/wallet/route.ts): the 8-week contribution rate counts only what is in, so a refund no longer shortens weeksToGoal.
  • Treasury (app/(app)/wallet/treasury/page.tsx): "this month in", each child's "+$X this month" and the 6-month trend all use the same rule.
    • "In" is new money and "out" is net spending.
    • Net (in − out) is still the ledger's real change. That is why I netted refunds rather than only dropping them from "in": dropping them alone would have understated Net by the refund.
  • Tests: they drive the real page and route.
    • Treasury: a $50 top-up, a $20 purchase and its refund show in $50, out $0, net +$50. The old code showed $70 in.
    • Coach: $80 of allowance and a $40 refund over 8 weeks forecast a $100 goal at 10 weeks. The old code forecast 7.
    • Mutation: 3 mutants, all killed.
  • Not changed:
    • The per-child coach counts only Save-bucket credits, and refunds land in Spend.
    • The admin and reconciliation "credit volume" totals are gross volume by name.
  • Still counted as "in", as before this PR: transfer and reversal credits. They are recorded in the body.
  • Local validation on fbe64d82b: the full suite passes, 1,875 files.

Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Composition dependency is now ready: #942138f9eac38ac4a51d22d843b668b41ef74aad264 merged externally at11:57:49 UTC (1e8e8e7), satisfying the condition in your #6995977042630 ownership offer. Support, take the bounded #925 treasury composition in your audit branch after checking current main and active claims. Preserve #942's family-timezone day keys AND #925's moneyFlow/refund accounting; do not replace the whole page or drop either behavior. Current #925 reports mergeable=false, but inspect actual conflict before attributing it.

Run focused two-family/timezone and capture/refund trend regressions plus affected type/lint/tests on the composed head, then fresh CI. Preserve your current card-hold child branch work and coordinate its dependency on this updated parent; no competing writer in the same file. Report exact head and resolved hunks, retain known residuals. This is source/test branch composition only, no PR merge, production migration, live financial operation or deployment.

claude added 2 commits October 4, 2026 12:07
… netting

One conflict, app/(app)/wallet/treasury/page.tsx, in the 6-month trend
loop. #942 (merged as 1e8e8e7) keys each transaction by the family's
month; #925 counts it through moneyFlow, so a card refund nets against
out. The resolution keeps both:

    if (t.status !== 'completed' || monthKeyOf(t.created_at) !== key) continue;
    const flow = moneyFlow(t);
    credits += flow.inCents;
    debits += flow.outCents;

The this-month loop and the per-child lines merged without conflict:
#942's family-zone monthStart with #925's moneyFlow sums.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DXw2nu25BjyRfA6Fg3YiMS
The same ledger rendered for a family in Los Angeles and one in Tokyo,
with "now" fixed at 2026-10-15T12:00Z and a refund at 05:00Z on
1 October:
- Los Angeles (still 30 September): September in $50, out $0, since the
  refund undoes September's $20 purchase; October in $10, out $0.
- Tokyo (already 1 October): September in $50, out $20; October in $10,
  out -$20; net +$30, October's real change.

Pins both halves of the merge. Trend keyed by UTC instead of the
family's zone: 1 failure. Trend without moneyFlow (main's side of the
hunk): 3 failures.

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

Copy link
Copy Markdown
Owner Author

Support: #942 composed into #925 (answers 5979697763). Head 2113c903b, pushed.


Generated by Claude Code

@NewWorldVenture

Copy link
Copy Markdown
Owner Author

Independent money-handling review on exact PR head 2113c90:

lib/stripe/webhook.ts currently selects the capture/refund handling path from the sign of amount. The negative-amount branch posts card_spend and releases the authorization hold without checking transaction type. Stripe documents that an Issuing Transaction with type=refund may have a negative amount for a refund reversal (https://docs.stripe.com/api/issuing/transactions/object). If that reversal arrives before its capture event, this path can release the pending authorization hold prematurely.

The added coverage includes positive refunds and negative captures, but no negative type=refund reversal or out-of-order reversal/capture case. Please classify the event by transaction type as well as amount, and add a regression that pins the expected ledger and hold state for a negative refund reversal arriving before its capture. This is a synthetic code/test finding; no money or production data was touched.

Owner review 5979998503. Stripe documents that an Issuing transaction
of type `refund` can carry a negative amount: a refund reversed.
handleTransactionCreated chose its branch by the amount's sign alone,
so a reversal was debited (right: money goes out) and also released the
linked authorization's hold (wrong). Arriving before its capture, the
reversal freed the held money while the capture was still to come.

The sign still decides which way money moves; the type now decides
whether a hold is touched. Only `type === 'capture'` releases it.

Tests (signed events through the real route):
- a $5 reversal before its $20 capture is debited and leaves the hold
  held (spendable $25 of $50); red before this commit, with the hold
  cancelled;
- the capture then releases the hold, and Spend ends at $25.

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

Copy link
Copy Markdown
Owner Author

Addressed in 356a861b0 (pushed). This answers 5979998503.

  • Fix: the sign of amount still decides which way money moves. The transaction's type now decides whether a hold is touched: only type === 'capture' releases the authorization's hold.
    • A refund reversed (type: 'refund', negative amount) is debited as before.
    • It no longer releases the purchase's hold.
  • Regression tests in tests/a-card-refund-credits-the-child.test.ts, through the real route:
    • A $5 reversal arriving before its $20 capture: one completed $5 card_spend debit, keyed by the reversal's id. The $20 hold stays processing, so spendable is $25 of $50.
      • Red before the commit: the hold was cancelled.
    • The capture then arriving: one $20 debit, the hold cancelled, and Spend and spendable both $25.
  • Local validation on 356a861b0:
    • tsc is clean, lint has 0 errors, and the i18n gate is clean.
    • The full vitest suite passes: 1,895 files.
  • Positive amounts: a positive amount is still credited as a refund, whatever its type, and it never touches a hold. The body's note on sign-based ledger types still holds.
  • The card-hold child branch (docs: ledger checkpoint after #770 — terminal CI, #768 review, status-mark repair #771, 5979360633) is rebased onto this head. It keeps the same rule: only a capture names its authorization to the settle function.

Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Closing again at the owner's direction to close the 85-PR backlog: this PR was in the sweep at 15:51 UTC and was reopened at 16:13 UTC. The branch is kept; nothing is merged or deleted. Please do not reopen without the owner's say-so.


Generated by Claude Code

NewWorldVenture pushed a commit that referenced this pull request Oct 13, 2026
…tes and add up every row

Twenty confirmed money findings (M1-M20), each with a negative-control test
that drives the real page, component, action, route or service.

- M1  family-cfo assessAffordabilityAction applies its page's gates:
      readsHouseholdMoney (guests too), resolveFeatureEntitlement for
      /dashboard/family-cfo, and the aal2 step-up verdict (stepUp path, which
      the form now follows through reportRefusal).
- M2  saveAllowanceRuleAction refuses a NEW rule for a child who already has
      one; the allowance page lists every rule a wallet has (each pausable)
      and says when a child has more than one.
- M3/M5 Budget Planner reads the year's transactions whole
      (readAllByIdAsQuery, ceiling 50,000, failing), from this week's Sunday
      (which can be in December) to the end of the year.
- M4  Expenses reads expense_splits and expense_split_shares whole
      (readAllByIdAsQuery), listing splits newest first in memory.
- M6  handleTransactionCreated branches on Stripe's capture/refund: a refund
      is a card_refund credit to Spend (creditCardRefund), a refund reversal
      is debited, only a capture releases its hold, an unknown type throws.
      Same handler change as draft PR #925, without its migrations.
- M7  /dashboard/money-timeline tells a non-manager whose the money is
      instead of showing Liquid today and the projected low.
- M8  /wallet/babysitters reads and shows payouts only to a parent or adult.
- M9  /api/ai/wallet, /api/ai/wallet/child/[childId] and /api/ai/invest
      answer a guest 403 before any read or model call.
- M10 wallet activation provisions child and teen members only; a guest is
      refused requestAllowanceAction, requestSpendAction and
      placeInvestOrderAction.
- M11 /api/ai/savings judges each budget (canonical, case-insensitive) in its
      own period on the family's calendar, both ends bounded.
- M12 budgetVsActual's totals are the month's: limits as monthly shares,
      against the month's spending in budgeted categories; the AI money line
      says so; the Finances budget bar uses the monthly shares.
- M13 Family CFO's Spending vs budget uses each canonical budget's monthly
      share, matched case-insensitively.
- M14 budgetSpent and the Finances month bound both ends of the period.
- M15 a credit account's balance lowers Total balance on /wallet and Finances.
- M16 AI Finance Insight fails closed on either read and sums total_cents.
- M17 AI spending insights fails closed on the expense_splits read.
- M18 the interactive brief answers 503 on a bills read error.
- M19 the allowance cron reads managers in chunks of families, each paged to
      its end; a failed read fails the run.
- M20 the approve/decline call is bounded to Stripe's window (1.5s, no
      retries); a failed approval releases its hold only when Stripe reports
      the authorization closed; issuing_authorization.created is handled like
      .updated, releasing a hold Stripe declined by its timeout setting.

New pure helpers live in lib/finance/budget-periods.ts (no local calendar
reads, so server paths can use them); three Playwright fixtures list it.
Existing tests that pinned the old behaviour now pin the decided one: a child
is not shown the money timeline (0514 over 0267), a guest may not ask for an
allowance or to spend, and a new allowance for a child with one is refused.

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

The comments cited closed draft #925's migration 0484, a number that now
belongs to a different migration. They now say the unique keys (one
card_refund, and one capture, per stripe_ref) are owner SQL not yet
applied. Comment-only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KP9rt5rVQ9jDMpZp2xBy3K
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