Keep existing payments working when new sales are off

TODO.md

@@ -456,6 +456,74 @@ they are. Nothing further planned here.
 
 ---
 
+## Payment-processing review follow-ups (from PR #1692)
+
+Both items describe behaviour that predates the payment-processing split (the
+code was moved verbatim from the old `payment-processing.ts` monolith). They are
+recorded here because the split PR was a pure reorganisation — changing this
+behaviour there would be out of scope — and CodeRabbit flagged them as worth a
+look.
+
+- **Per-item DB reads not batched** (`src/features/api/payment-processing/items.ts`
+  `validateAllItems`, and `package-pricing.ts` `loadPackagePricingByGroup`).
+  `validateAllItems` calls `getListingWithCount` once per item in a loop, and
+  `loadPackagePricingByGroup` makes two sequential round-trips per group. Under
+  the edge subrequest budget these accumulate for larger orders. Fix direction:
+  add/use a batched `getListingsWithCount(ids)` for all order listing ids at once
+  and group the package-pricing loads, preserving the existing validation and
+  fail-closed behaviour. See the "Respect the subrequest budget" guidance in
+  AGENTS.md.
+
+## Payment aggregate — safety behaviour (PR 1)
+
+New sales and existing payments are now resolved by different questions:
+`getActivePaymentProvider()` / `isPaymentsEnabled()` gate new checkouts;
+`getPaymentProviderForExistingPayments()` resolves refunds, replayed
+callbacks, and completion. When sales are off, the existing-payment path
+falls back to the last activated provider. A site already on `none`
+recovers when exactly one provider has stored credentials; when multiple
+do, the operator must choose the provider in a recovery form that keeps new
+sales off. `setPaymentProviderNone` reads the
+current provider via an atomic INSERT ... SELECT subquery so a concurrent
+activation cannot land between the read and the write.
+
+The seven accepted safety rules are recorded as acceptance constraints in
+[`docs/payment-aggregate-acceptance.md`](docs/payment-aggregate-acceptance.md).
+
+- **Track the provider each charge was captured with.** `main` stores only the
+  opaque `payment_reference` per processed payment, not which provider captured
+  it. So after an operator switches providers (Stripe → Square) and then selects
+  "none", the last-active fallback resolves every payment through Square, and
+  an older Stripe charge cannot be refunded or reconciled against the provider
+  that captured it. This predates PR 1 and is future aggregate work. Fix
+  direction: store the provider type on each `processed_payments` row at capture
+  time and dispatch existing-payment work from that per-charge provider instead
+  of one global fallback. Referenced from
+  `docs/payment-aggregate-acceptance.md` rule 2.
+
+- **Split payment-provider persistence out of `src/shared/db/settings.ts`.**
+  Review of PR 1 correctly noted that the settings assembly is already over the
+  preferred 400-line size and now also owns provider activation, recovery,
+  credential-state preservation, and cache synchronization. The clean starting
+  point is `src/shared/db/settings/payment-provider.ts`, moving the provider
+  getters and `settings.update` methods together with mirror tests under
+  `test/shared/db/settings/payment-provider/`. This is deferred because that
+  extraction would take PR 1 beyond its strict 800-line source-change limit.
+
+- **Split provider credential routes out of
+  `src/features/admin/settings-helpers.ts`.** The generic helper now also owns
+  `ProviderCredentialsConfig`, `persistProviderCredentials`, and
+  `defineProviderCredentialsRoute`. Move that block to a focused admin settings
+  module and move its mirror tests from
+  `test/features/admin/settings-helpers/provider-credentials.test.ts` with it.
+  This is deferred because doing the move in PR 1 would break the same strict
+  800-line source-change limit.
+
+- **Split `src/features/api/webhooks.ts` below 400 lines.** Move the payment
+  callback and webhook processing paths into focused modules. This predates PR
+  1 and is deferred because the split would exceed its strict source-change
+  limit.
+
 ## Request performance: consolidate AsyncLocalStorage scopes
 
 `src/features/app/request.ts` enters eleven nested request scopes for locale, client
@@ -1745,6 +1813,102 @@ Note that the id that made this reachable in the first place is now checked at
 the insert (`insertedRowId` in `src/shared/db/client.ts`), so this is about the
 answer given for a failure that should no longer happen — not a live fault.
 
+## Record a foreign-currency charge in the money history without pretending it is ours
+
+*Origin: Codex review on PR #2021, which sent a charge taken in the wrong
+currency down the existing mismatch-and-refund path.*
+
+The money history holds one currency — the site's. When a charge arrives in a
+different one, `classify.ts` sends it through the ordinary mismatch flow, and
+`storeRefundPlaceholder` in
+`src/features/api/payment-processing/store-refund.ts` writes
+`session.amountTotal` straight into that history. The number is right but the
+currency is not, so a 1,000 yen charge on a pounds site is filed as £10. The
+refund itself is unaffected — that goes back through the provider in the
+currency it was taken — but the operator's cash history reads wrong, and if the
+refund fails it names the wrong amount as still held.
+
+Two ways out: give the money history a currency of its own so a foreign charge
+can be filed honestly, or keep these charges out of it and record them
+somewhere that does not claim a site-currency total.
+
+Why it is not fixed in that PR: either way changes what the money history can
+hold — a stored currency per entry, plus every reader and every total that
+today assumes one currency. That is a change to the accounting store, well
+past a PR about reading provider money safely, and the wrong thing to bolt on
+without deciding which of the two shapes we want.
+
+## Record a rejected-and-refunded payment session as finished
+
+*Origin: Codex review on PR #2021, which added the automatic refund for a paid
+charge the payment boundary cannot read.*
+
+When a provider callback meets a `malformed_charge` rejection and refunds it,
+nothing durable is written down. No reservation, no processed-payment row, no
+ledger entry says "this session was refused and the money went back". The
+webhook simply acknowledges and moves on
+(`refundRejectedCharge` in `src/features/api/payment-processing/refunds.ts`, and
+its callers in `src/features/api/webhooks.ts` and
+`src/features/api/payment-processing/classify.ts`).
+
+The reviewer's concern is that a later delivery of the same session — a webhook
+redelivery, or the buyer opening the success page — could read it in a
+well-formed shape, still see it as paid, find no record of it, and make a real
+ticket for money that was already returned.
+
+Why it is not being fixed in that PR: every rejection reason is a fixed
+property of the provider's own stored record (an amount that is not a whole
+number of minor units, a missing or malformed currency, a SumUp amount more
+precise than its currency allows). None of those can turn well-formed on a
+later read, so the double-book needs a provider changing a completed session's
+money — which no provider does. The refund itself is already safe to repeat:
+`tryRefund` treats a provider's "already fully refunded" answer as success.
+
+If it is taken on: carry the session id in `SessionRejection`
+(`src/shared/payment/validated-session.ts`), and finish the session through the
+same state machine a normal payment uses — `reserveSession` /
+`processed-payments` in `src/shared/db/processed-payments.ts` — with a terminal
+"refused and refunded" outcome, so a later delivery of that session id short
+circuits the way an already-processed payment does.
+
+## Tell a buyer when their money was taken and not (yet) given back
+
+*Origin: Codex review on PR #2021, which added the "your money has been sent
+back" page for a charge the payment boundary refused and refunded.*
+
+That page is only shown when the refund actually went through. Three other
+outcomes still fall back to "Payment session not found", and in each of them the
+buyer really was charged:
+
+- A `blank_reference` rejection: the provider says paid but gave no reference,
+  so no automatic refund is possible at all. They should be asked to get in
+  touch.
+- A `malformed_charge` rejection that is paid but whose reference is unusable —
+  the same situation, reached a different way (`refundable` is
+  `paid && isResourceId(...)`, and the `paid` half is discarded today).
+- A refund the provider refused (`settled: false`, answered 503). Careful here:
+  a 503 only asks the *webhook* to be delivered again. The redirect and cancel
+  paths have no retry behind them — they re-attempt only if that person happens
+  to reload the page. So this outcome must not be described to the buyer as
+  being in hand until an unresolved refund is actually written down somewhere
+  and owned by something that will retry it. Recording that state is part of
+  this job, not a follow-up to it, and it overlaps with the durable terminal
+  outcome described in the section above.
+
+A fourth case should keep the generic message: a rejection whose price proof
+does not verify may belong to another site sharing the provider account, and we
+must not tell someone else's buyer anything about their payment.
+
+What it needs: `SessionRejection` carries whether the charge was paid (see
+`malformedChargeRejection` in `src/shared/payment/validated-session.ts`, which
+computes `refundable` from it and drops it), `RejectionOutcome` in
+`src/features/api/payment-processing/refunds.ts` grows a third state for
+"captured, not returned", and `answerRejectedSession` picks between two new
+catalog entries beside `payment.error.refunded` in
+`src/locales/en/payment.json`.
+
+Not done in that PR only because it was at its agreed `src/` line budget; there
+is nothing hard about it.
 ## Record equivalent mutants by something that survives an edit
 
 *Origin: two breakages on PR #2025, both caught by review rather than by any

docs/payment-aggregate-acceptance.md

@@ -0,0 +1,88 @@
+# Payment aggregate — accepted safety rules
+
+These seven rules are the safety contract the payment aggregate must satisfy.
+Some describe behaviour not yet on `main` (owner-review flow, queued owner
+email, aggregate activation) — future aggregate work. They are recorded here
+so earlier work does not paint the aggregate into a corner.
+
+Each rule names the current behaviour where one exists. Nothing here is
+implemented ahead of its time; this document is the contract, not scaffolding.
+
+## The separation this PR establishes
+
+- **New sales** — `getActivePaymentProvider()` / `isPaymentsEnabled()`. Null
+  when the provider is saved as "none".
+- **Existing payments** — `getPaymentProviderForExistingPayments()`. Refunds,
+  reconciliation, replayed callbacks, and completion use this. When new sales
+  are off, it falls back to the last activated provider (whose credentials stay
+  stored), so payments captured by that provider stay refundable.
+
+The rules below assume that separation holds.
+
+> **Sales-off recovery.** A site may have no remembered provider. The resolver
+> uses the sole provider with stored credentials when that choice is clear. If
+> several providers have credentials, the settings page requires the operator
+> to choose which one took the existing payments. Saving that choice records
+> the provider and keeps new sales off. The system never guesses.
+
+## 1. Failed checkout plus captured money becomes owner review with complete-or-refund choices
+
+A captured charge whose booking cannot be honoured is surfaced for the
+operator to either complete or refund.
+
+*On `main` today:* refunded automatically (`storeRefundedBooking` /
+`refundAndFail`). The "complete-or-refund" choice is future aggregate work;
+the current path must keep refunding safely so the aggregate can later
+replace the automatic refund with a review.
+
+## 2. Completed refunds count immediately while overlapping refunds are blocked until provider totals catch up
+
+A refund that succeeds is recorded at once. A second attempt for the same
+charge is treated as done once the provider's own refund total confirms it.
+
+*On `main` today:* `tryRefund` treats a payment the provider reports as
+already refunded as success, and each charge carries a `provider_refunded_at`
+marker so a later attempt skips the provider call. This must stay.
+
+> **Known gap (provider switching).** `main` does not record which provider
+> captured a charge — only the opaque `payment_reference`. So after an operator
+> switches providers (Stripe → Square) and then selects "none", the last-active
+> fallback resolves every existing payment through Square, and an older Stripe
+> charge cannot be refunded. This predates this PR. Per-charge provider
+> tracking is future aggregate work; recorded in TODO.md.
+
+## 3. Multiple captured charges require owner review
+
+A buyer with more than one captured charge must have the set surfaced for
+the operator to decide, not auto-resolved.
+
+*On `main` today:* merged attendees carry several references, handled as a
+batch without review. Future aggregate work.
+
+## 4. Queued owner email uses the current business address but stored body/buyer facts
+
+The recipient address is read at send time. The body and buyer facts come from
+when the case was raised, so the message describes what happened then.
+
+*On `main` today:* there is no queued owner-email path; notifications are sent
+inline. This is future aggregate work.
+
+## 5. Malformed legacy records migrate without invented facts into owner review
+
+A malformed record is carried forward only as far as it can be honestly read
+and lands in owner review. `main` copies what is provably there and invents
+nothing. This must stay.
+
+## 6. A buyer with a paid booking under review sees "payment received" and gets a stable reload
+
+A buyer must not be offered a way to pay again. A reload must return the same
+state. `main` replays as success and never re-processes (`replaySessionFromLedger`).
+The "under review" state is future aggregate work.
+
+## 7. Aggregate activation waits for complete owner case pages/actions
+
+The aggregate is only switched on once every owner-case page and action it
+depends on exists. No half-enabled state.
+
+*On `main` today:* there is no aggregate. Future work must clear this gate
+before it takes over any payment path.

scripts/mutation/equivalent-mutants/features.txt

@@ -132,7 +132,7 @@ src/shared/runtime.ts:68:18  ?? → ||   # os is string|undefined and falls back
 src/shared/runtime.ts:70:43  ?? → ||   # typescriptVersion is string|undefined and falls back to "", so both operators agree
 src/shared/runtime.ts:71:38  ?? → ||   # userAgent is string|undefined and falls back to "", so both operators agree
 src/shared/square.ts:312:21  ?: → consequent only   # the buyer_phone_number ternary spreads {} when the phone is absent; the consequent-only mutant always spreads { buyer_phone_number: undefined }, but JSON.stringify omits undefined values, so the serialized request body is identical in both cases
-src/shared/square.ts:795:37  ?? → ||   # locations is an array or undefined; arrays are truthy and undefined takes [] under both operators
+src/shared/square.ts:799:37  ?? → ||   # locations is an array or undefined; arrays are truthy and undefined takes [] under both operators
 src/ui/client/dom.ts:13:19  = → +=   # createElement returns a new button with className "", so both assignments produce the supplied class string
 
 # Paid-payment processing: private defaults are always supplied by their only
@@ -152,6 +152,11 @@ src/features/api/payment-processing/package-pricing.ts:253:19  false → true
 src/features/api/payment-processing/package-pricing.ts:270:60  ?? → ||   # intent.allocations is an array or undefined, and arrays (including []) are truthy
 src/features/api/payment-processing/package-pricing.ts:306:51  ?? → ||   # the allocated quantity is non-negative and the fallback is 0, so both operators agree for 0 and undefined
 
+# Payment success replay only loads a listing fallback while thankYouUrl is the
+# empty string, so assignment and append produce the same string for every path.
+src/features/api/webhooks.ts:177:16  = → +=   # the enclosing !thankYouUrl guard narrows the string to "" before this assignment, and "" + fallback equals fallback
+src/ui/templates/admin/settings/payment.tsx:88:51  ?? → ||   # both operands are PaymentProviderType|null; every provider id is non-empty and truthy, while null selects the same fallback under either operator
+
 # Folded API booking: equivalents for schema-validated inputs and unused paths.
 src/features/api/folded-booking.ts:87:42  ?? → ||   # childrenByParentId is Map<number, TicketListing[]>; get returns an array (always truthy, even []) or undefined, so ?? and || agree
 src/features/api/folded-booking.ts:118:31  ?? → ||   # qtyByChild accumulates ApiQuantitySchema values (positive integers >= 1) or is unset; it is never 0 or NaN for validated inputs, so ?? 0 and || 0 agree
@@ -163,6 +168,7 @@ src/features/api/folded-booking.ts:381:17  1 → 0   # base.dayCount is read onl
 # placeholder and a throw-only label that can never throw.
 src/features/admin/questions/answers.ts:88:45  0 → 1   # the inserted sortOrder is a placeholder: insertScopedOrderedRow runs order.append in the same transaction, and append sets sort_order from MAX over siblings that explicitly exclude the row itself, then overwrites it unconditionally before commit — the placeholder value is never read by anything
 src/features/admin/questions/forms.ts:32:9  Display as → ""   # requireChoiceOptions reads its label only in its two throw paths: an empty options list, or a value failing an invalid-rule. QUESTION_DISPLAY_TYPES is a non-empty picklist constant and requireChoiceOptions carries no invalid rule, so neither throw is reachable and the label is never read
+src/features/admin/settings-page.ts:132:56  0 → 1   # existingPaymentProviderState resolves one configured provider directly; recoveryChoices is therefore either empty or has at least two providers, so both comparisons agree
 
 # Cart package resolution (features/public/groups.ts): map lookups whose key is
 # guaranteed present, and a nullable-object fallback.