Gift-card rolling expiry (setting-driven, was dead config):
- GetGiftCardExpiryMonths(): single source of truth (business_settings
gift_card_expiry_months, fallback 24) shared by payment handlers and the
CleanupExpiredGiftCards job (was hardcoded 24).
- expiry_date now maintained on ALL 9 gift-card write sites (buy, topup,
transfer, redeem, terminal payment, refund credit, till) so the refund-time
guard at refunds.go actually fires. Schema default 12->24 + migration note;
test-DB seed aligned. Stale "expiry_date IS NULL" test rewritten; new
expired-card-rejected regression test.
Frontend SvelteDate purge (docs' stated convention, wide):
- All 180+ raw `new SvelteDate(...)` uses across routes/components replaced
with parseWallClockDate (backend UTC ISO) or new Date (wall-clock
constructors). SvelteDate imports removed. timeSlots.ts getDayWithOrdinal
fixed. Zero SvelteDate references remain; svelte-check clean.
Strict timezone/DST testing + QA fixes:
- 8 new hermetic boundary tests: clock.DST transitions (both 2026 folds),
closing-hours GMT vs BST, booking date-window midnight, refund-tier
elapsed-time independence, deposit-window UTC-instant, scheduling
LondonDateString midnight, today AT TIME ZONE window + UTC round-trip.
- today.go summary date labels fixed to London wall-clock (were showing the
previous UTC day during BST) + regression test.
- pgx ScanLocation fixed to UTC via AfterConnect (was host-local -> JSON
offsets depended on deployment TZ, contradicting the documented UTC
invariant) + regression test. Registered as a new *Type to avoid a data
race on the shared type map (caught by -race).
Admin Business Settings (setting now functional => legal floor):
- gift_card_expiry_months validation floor raised 1 -> 12 months (CMA/
Consumer Rights Act 2015 unfair-contract-term guidance) in endpoint + UI,
with rolling-expiry semantics shown in both display and edit form.
- 3 new expiry validation tests; 2 pre-existing message assertions updated.
Full suite 25/25 + race clean via run-tests.sh lockfile; svelte-check 0
errors/warnings; production build succeeds.
Money-safety idempotency hardening (I1, wide):
- validate:"max=45" on CreateTerminalPayment/BookingPayment/Refund/Tip/
BuyGiftCard idempotency keys (all feed Square's 45-char /v2/payments,
/v2/refunds, /v2/cards caps); BuyGiftCard corrected from a wrongly-loose
max=64. Till keeps max=64 (its key also feeds the 64-char terminal-checkout
endpoint).
- Explicit 45-char guard in RefundPayment: the one handler that decodes
RefundRequest without running the struct validator, so the tag alone was
inert; a longer key would 400 at Square and be misclassified as a
definitive refund decline.
- New TestIdempotencyKey_OverLength_RejectedAcrossPaymentHandlers covers all
six endpoints (terminal saved-card, booking, tip, gift-card, till, refund).
Stable-sentinel card identity in idempotency keys (C1, wide):
- BookingFlow deposit key now uses the 'new-card' sentinel instead of
embedding the cnon: nonce (matches UserPaymentModal/account). A re-tokenize
after a spent nonce no longer regenerates the key, closing a lost-response
double-charge window.
- TipPayment + UserBookingModal tip keys now include card identity
(selectedCardId || 'new-card'); previously keyed on amount only, so a
same-amount tip on a DIFFERENT card reused the key and deduped a distinct
charge. Resets cleared in every success/close path.
Test isolation (R1): TestRefund_PendingResume_NewKeyAfterModalReopen no
longer t.Parallel — it swaps the package-level SquareClient mid-test and a
concurrent parallel test could observe the swapped instance.
Naming/quality (M1/M2/M4): resolveChargeSource local renamed savedRowID (was
shadowing the cardID *string parameter); BuyGiftCard fallback prefix
"till-" -> "gc-"; saved-card terminal response key "checkout_id" -> "payment_id"
(it holds a DB payment row, not a Square checkout) with matching frontend
fallback. README maintenance-job count corrected 24 -> 25.
Full suite 25/25 + race clean via run-tests.sh lockfile; svelte-check 0
errors/warnings; production build succeeds.
Money-safety idempotency fixes (external review bugs 1-3):
- processChargeGroup: aggregated refund key now hashes the sorted pending-row
set (chargeID-square-agg-<sha256 suffix>) so a changed group can never mark
a new row completed against an old smaller refund; >45-char chargeIDs use a
hashed prefix instead of verbatim truncation (which would collide charges on
Square's global key dedup). Same-set crash-retry keeps Square's dedup.
- CreateTerminalPayment saved_card: two-tier idempotency key — client-supplied
per-attempt UUID preferred (distinct identical charges no longer collapse),
deterministic booking+type+amount+card fallback for no-key retry safety.
PaymentModal sends a per-charge UUID cleared after success.
- ensureRefundKey: legacy NULL-key manual refunds persist a generated key to
the row BEFORE the Square call (race-safe AND idempotency_key IS NULL guard),
so a lost-response retry reuses the key and never double-refunds. Wired into
resumeManualPendingRefund and the sweep's manual-retry loop.
Classification + money-safety hardening:
- till.go/sweep.go: structured square.ErrorCode/IsNotFound are authoritative
when present; message-substring matching only for non-structured errors
(dev mock, client-side status errors). Fixes fragile string-matching driving
sweep retries and gift-card clawbacks.
- SaveCardForUser: ON CONFLICT (user_id, square_card_id) DO NOTHING + re-select
(was a latent UNIQUE-violation 500 on save-card retry).
- CreateBookingPayment: partial payments re-validated against remaining balance
inside the advisory lock (closes concurrent-overpayment race).
- InvalidateSquareCustomerCache on GDPR erasure paths (account.go,
time-blockers.go stale-guest anonymization).
- GetUserGiftCardBalanceAdmin: in-handler admin check (defense-in-depth).
- getCheckoutHTTP: warn on multi-payment checkouts instead of dropping
payments[1:].
- Cash/giftcard terminal branch: removed dead idempotency SELECT, "tip-" ->
"till-" prefix.
- UserPaymentModal: removed vestigial polling state; proper interval cleanup.
- account/+page.svelte: gift-card redeem dialog links /terms.
- nginx CSP: allow *.squarecdn.com and js.squareup.com so the Square Web
Payments SDK + card iframe can tokenize behind the proxy.
Tests: +8 regression tests covering changed-set refund keys, legacy NULL-key
single-refund, saved-card client-key dedup/no-dedup, concurrent partials, and
cache invalidation. Full suite + race detector clean via run-tests.sh lockfile.
Re-review (2 Oracle + security + QA + librarian + context-miner) surfaced fixes, all applied: (1) UserPaymentModal and the account-page gift-card buy now key the cached idempotency key on a stable 'new-card' sentinel instead of the cnon: nonce, so clearing the nonce on a failed charge no longer regenerates the key — a lost-response retry now dedups at Square instead of double-charging (the tip flows already keyed on amount only). (2) The create-with-redeem clawback now logs CRITICAL when the guarded balance reversal is blocked (previously silent), and its transaction DELETE is scoped to this sale instead of deleting every transaction on the card. (3) reconcileStalePaymentAtSquare now treats APPROVED/PENDING as non-terminal (leave pending) instead of definitively failed, matching Square's documented state machine. (4) GetTillCheckoutStatus returns 404 for a sale already swept to failed instead of reporting a live state. (5) The critical-payment scan job skips candidates whose booking was hard-deleted, so one orphan can no longer silence all critical alerts.
HIGH-1: a till sale whose card-machine checkout is provably dead, or whose Square reconcile proves the charge never landed, now claws back the funded gift card atomically with the failed mark (claim-first gating UPDATE serializes against the admin retry; blind-fail and ambiguous/lost-response rows never claw back, and a bare CANCEL_REQUESTED is not treated as proof of non-completion). HIGH-2: a card-machine checkout created at Square but not committed is cancelled on any pre-commit failure. HIGH-3: a cash/on_the_house retry of a pending card sale is reconciled at Square first (COMPLETED rescues + refuses cash; NOT_FOUND/FAILED/CANCELED allows cash; lost-response forces the card-method retry; ambiguous rejects). GetTillCheckoutStatus no longer resurrects a swept-failed sale, and the cash-completion UPDATE checks RowsAffected so the admin is never told to take cash against an already-resolved sale. 23 new tests covering the clawback matrix, the reconcile-or-reject matrix, cancel-on-error, and a till concurrency test.
Till sales and gift-card purchases now use uniqueChargeKey and resolveChargeSource/chargeFailureStatus instead of duplicated inline logic. The saved-card delete path logs the square_card_id through square.TokenPrefix so full ccof references never reach logs.
BuyGiftCard and till online_square charge the cnon: nonce directly (no synthetic card-on-file); BuyGiftCard.IdempotencyKey is now validate-required (empty key previously collided on the UNIQUE constraint). Saved-card branches forward customer_id and lazily provision legacy cards. lockCancellationPayments uses the deliberately-blocking xact lock so a cancellation never silently drops a refund under contention (admins see the full manual refund round-trip).
R1/R4: saved_card branch in CreateTerminalPayment now mirrors CreateTipPayment
- advisory lock (crussell:payment:<bookingID>) serializes concurrent double-clicks
- deterministic key bookingID-sc-type-amount-cardID (<=45 chars) so a lost-response
retry derives the same key and dedups instead of double-charging
- idempotency switch inside the lock: completed -> dedup, pending -> reuse with
pence amount-guard, failed -> clean 409
- success response includes card_brand/card_last4 (frontend already reads them)
R2: add 'failed' case to all four retry switches (tip, booking, gift card, till)
- a swept/definitively-rejected record returns 409 instead of 500-ing on the
idempotency_key UNIQUE constraint
R3: extend SweepStalePendingPayments to till_sales card rows
- sweeps pending till_sales (online_square/in_person_card) past Square's ~24h
key retention, closing the double-charge window for till sales
- swept rows logged with the same CRITICAL manual-reconciliation marker as the
refund sweep
Webhook fail-closed: reject 503 when SQUARE_WEBHOOK_SIGNATURE_KEY unset, 403 on
bad signature (was: skip verification in dev)
Refund status resolution: refunds now resolve by Square status
(COMPLETED/PENDING/FAILED/REJECTED) instead of assuming completed; real error
codes (REFUND_AMOUNT_INVALID, PAYMENT_NOT_REFUNDABLE, REFUND_ALREADY_PENDING)
added to the definitive/processed classification
HTTP client: CreateCard key truncated to <=45 chars, device_options always sent
(env SQUARE_TERMINAL_DEVICE_ID fallback), processing_fee reads amount_money,
ListCards cursor loop, refund keys hashed to <=45 chars
Other fixes: payment/till/gift-card advisory-lock + FOR UPDATE asymmetries,
GetPaymentByID NULL scans, loyalty redemption lock, card upsert on conflict,
mock ccof: prefix parity, IsValidSquareCheckoutID for real Square IDs,
isAdminRequest defense-in-depth on all 6 admin payment handlers, webhook
signature docs, M8/L5 debug markers removed
Docs: README/FC/TM/Overview updated (22 jobs, 20 CRITICAL sites, 23-section
GDPR export, sweep jobs, webhook fail-closed); P11 plan marks remaining items
(sandbox smoke test, M-8 customer_id, saved-card key dedup trade-off) as
deferred with rationale; gap backlog pruned of completed items
Refund system (Round 3 fixes + follow-up + alignment):
- Serialize cancellation refunds against the manual handler via
per-payment advisory locks taken before the prior-refunds read
(pg_advisory_xact_lock, ascending, same crussell:refund: key space)
- Aggregate pending cancellation refunds into ONE Square refund per
charge (stable charge-level -square-agg key); atomic group UPDATE
keeps crash-retry amounts identical for Square key-dedup
- Persist paymentID-square-amount idempotency keys on cancellation
refunds; scheduler reads the stored key (legacy fallback for old rows)
- Add sweep-pending-square-refunds cron (*/5, concurrency 1) with
refund_attempts cap; sweep retries stale manual pending refunds with
each row's own stored idempotency key
- Reconcile at Square (GET /v2/refunds ListPaymentRefunds) before every
terminal failed transition: tri-state result leaves rows pending on
reconcile error instead of false-failing; PAYMENT_ALREADY_REFUNDED
resolves to completed
- Move over-refund guard inside the lock, counting completed + pending
(excluding failed); ErrRefundDeclined distinguishes definitive vs
ambiguous outcomes
- forgiveFees now executes a real full refund (forceFullRefund override)
with admin_forgiven_fees reason threaded to Square
- Surface failed card refunds in the admin notification centre
(refund_failed enum, RETURNING-id pre-pass inserts, NOT EXISTS dedup)
- Dedup double-cancel refund inserts via ON CONFLICT (idempotency_key)
DO NOTHING without consuming refundRemaining
Frontend:
- Remove all raw-PAN card entry: zero card_number/card_cvc/new_card_token
in request bodies; gate new-card entry behind CardEntryUnavailable
notice + newCardDisabled prop across all 8 flows
- Delete hand-rolled CardInput.svelte; keep CardSelection saved-card UI
and CardEntryUnavailable fallback
- Update cancellation-policy page to in-person cash pickup wording
Tests:
- Rewrite the two amount-blind dedup tests to assert real money movement
(single call, aggregated amount, shared refund ID)
- Add coverage: manual refund vs cancellation serialization (concurrent
goroutines), reconcile error vs no-match branches, stale manual retry,
forgive-fees real refund row + reason, double-cancel dedup, mock refund
key dedup, ListPaymentRefunds filtering
- Fix time-dependent booking flakes with fixtures.NextWorkingDayAt
- 25/25 packages pass; -race clean on payments/square/db/jobs/bookings
P0 — float truncation: applied math.Round to all remaining int64(x*100)
sites (till penceAmount, refund over-refund guard, GetAlreadyRefundedAmount,
payment summary conversions). A £1.14 till sale previously charged 113p.
P0 — raw PAN stopped at the API edge:
- Deleted CardNumber/CardExpMonth/CardExpYear/CardCVC from TillSaleRequest
and CardNumber/Expiry/CVC from CreatePaymentMethodRequest. Both now accept
card_token (Square nonce) and return 400 when absent. PAN+CVV no longer
transit the application server (PCI-DSS SAQ-A scope).
- Deleted CreateCardOnFileRaw from the SquareClient interface and all
implementations (MockClient, ProdClient, devProdClient).
- Added idempotency_key column to refunds table (UNIQUE).
P0 — RefundPayment hardened: advisory lock on payment ID (prevents two
concurrent refunds passing the over-refund guard), pending-refund-record-
then-Square pattern (scheduler reprocesses on failure), same-key dedup.
P1 — till sale pending-retry now re-attempts the Square charge instead of
returning the stale 'pending' status (gift card was already funded in the
committed tx — silent money loss otherwise). Sale row reused, not duplicated.
P1 — idempotency key caching in frontend: BuyGiftCard and
UserPaymentModal/BookingFlow now cache the key per amount+card, regenerated
on change and cleared on success — matches the tip-flow pattern so a
lost-response retry dedups instead of double-charging.
P1 — CreateTerminalPayment cash/giftcard INSERTs now persist idempotency_key.
Key is unique per payment (booking+type+amount would wrongly dedup two
legitimate identical payments, e.g. two £50 cash receipts).
P1 — gift-card codes no longer logged (spendable credential; value+recipient
only).
Tests: till pending-retry re-attempt, refund same-key dedup, mock CreatePayment
idempotency dedup, CreatePaymentMethod nonce happy path + raw-PAN rejection,
till online_square card_token required/valid.
CRITICAL — same-amount tip retry silently never charged:
- CreateTipPayment idempotency check now only short-circuits when the
existing record is 'completed'. A 'pending' record (previous Square call
failed) is REUSED and the charge re-attempted with the same key (Square
dedups safely), instead of returning the stale pending record as 200 with
a success toast and no charge.
- Same fix in BuyGiftCard: pending records trigger a re-attempt, not a
false-success response. Unique idempotency_key constraint means the
pending record must be reused, not re-inserted.
- Fixes the savepoint/rollback interaction: the nested tx (savepoint) is
now committed in the reuse path so the deferred rollback doesn't undo the
later status UPDATE on the same connection.
- Regression test: TestTipPayment_RetryPending_ReattemptsCharge verifies a
pending record + same-key retry re-attempts and completes, reusing the
record (count stays 1).
MAJOR — terminal checkout wire contract:
- device_id now sent as checkout.device_options.device_id (Square's required
shape), not a top-level field which Square rejects with 400.
- 'checkout pending' detection now uses typed sentinel ErrCheckoutPending
with errors.Is in both handlers, matching mock and real HTTP client.
MAJOR — exp_month/exp_year omitted from card creation payload when unset
(now *int with omitempty) — Square would 400 on 0/0; expiry comes from the
tokenized source.
Docs:
- README payments/infrastructure sections corrected (Web Payments SDK claim
replaced with accurate P11-backlog note; dev mock parity described)
- Future Work P11 updated to reflect raw-PAN rejection is now enforced in
both mock and prod (new-card flows are a documented dead end)
- Added plans/p11-square-web-payments-sdk.md: full implementation plan +
handoff prompt for the agent picking up P11 (Web Payments SDK nonces)
Money-moving fixes:
- Tip idempotency key regenerates when the tip amount changes after a failed
attempt (all 3 tip flows). Cached key still reused on same-amount retry
(dedup intact) and cleared on success/modal reset. Prevents silent
under-charge when a user retries at a different amount.
- Till replay path returns actual till_sales.status (may be 'pending') instead
of hardcoded 'completed' — no more misreported successful charge.
- BuyerEmail wired for CreateBookingPayment, gift card purchases, and till
sales (saved_card + online_square), matching the tip flow. Email lookup
errors logged, non-fatal.
- Till buyer-email errors now logged (was silently swallowed).
- on_the_house till top-up uses cached getIdempotencyKey() for retry-safe dedup
(was fresh crypto.randomUUID()).
Test/validation fixes:
- Add TestPaymentFromSquare_* unit tests (else-branch + nil card details),
build tag relaxed to 'test' so they run in the standard dev suite.
- Add TestValidateCardInfo table test (7 cases: both/either/neither/empty).
- Add TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed regression test.
- Remove dead mock pre-registration in TestTipPayment_WithSavedCard.
- Correct misleading till regression-test comment.
ESLint cleanup (12 errors -> 0):
- Remove unused loadingCards in tip + pay-tip pages (dead assignments in
loadSavedCards).
- Scoped eslint-disable for {@html} in CardBrandIcon (hardcoded brand SVGs).
- Remove dead confirmSaveDefaultHours + unused rescheduleVersion prop in
WeeklySchedule (and its parent pass-through).
- Replace new Date() with SvelteDate in WeeklySchedule + BusinessHours.
- Fix each-block key in BusinessHours skeleton loader.
- Use void expression for reactivity-tracker reads in effects.
Restore processImage (images.go) and nonDepositPaymentType (handlers.go) with //nolint:unused — used in test files.
Fix 97 tx.Rollback defers to silently discard expected "tx is closed" error after commit.
Frontend: remove 44 unused shadcn-svelte files, 2 dead components, 9 stale npm deps, prune unused exports.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
errcheck: add proper error handling with slog.Error for tx.Rollback, key generation, and s3/dav operations. Add nolint comments for intentionally discarded DB scan errors and HTTP write errors.
unused: remove dead code (svcRow type, processImage, nonDepositPaymentType, generateSecureCode, colorBold, nGreen, nRed)
gosimple S1021: merge var declaration with assignment in manage.go
ineffassign: remove dead assignments in settings.go, till.go, images.go
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Refactor BuyGiftCard to insert pending payment before Square call with VAT applied. Add transaction wrapping to gift card handlers. Remove redundant Content-Type header sets. Migrate all time.Now() to clock.Now().
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Track voucher_type_at_purchase on gift card creation. Apply VAT at gift card purchase (SPV), at redemption (MPV). Use transaction-aware GetCardByIDQuerier for till saved card lookups. Apply VAT to all booking payments (split records, terminal, tip, checkout). Add TotalVATAmount and TotalNetAmount to PaymentSummary responses.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Replace direct *pgxpool.Pool usage with PoolProxy wrapper across the entire backend:
- db.DB renamed to db.Conn (*pgxpool.Pool -> *PoolProxy)
- JWT functions now accept context.Context instead of using context.Background()
- Handler DB calls route through PoolProxy for per-test transaction support
- Fixture/helper/testdb functions accept Querier interface for decoupling
- Query ordering fixed in bookings handlers: COUNT after data query to avoid pgx conn busy
- Time truncation fixed: time.Date instead of Truncate(24*time.Hour) for week start calc
- testmain_test.go files updated with SeedBaseline and NewPoolProxy
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
popertots
