Round 2 Loop A fresh money/security/dup-mod review. 23 findings fixed:
MONEY:
- CRITICAL: B1 duplicate auto-refund gains an attempt cap (b1_attempts col, cap 3) —
a rejected auto-refund no longer re-replays the expired key every sweep run
(which minted a stacking unauthorized charge each time); FAILED-webhook
demotion respects the cap; never re-replay a key whose B1 refund failed
- HIGH: A6 deposit_covered_by_discount skip path now APPLIES the eligible
campaign discount rows immediately (capped) instead of skipping with no
discount recorded — no more promised-discount-not-recorded overcharge
- MEDIUM: 2FA code burned by the SAVE gate is re-issued on failed
new-card+save_card charges (re-issue guard now covers req.SaveCard)
- LOW: GetBookingPaymentSummary excludes tip rows from paidAmount (remaining
now matches the authoritative tip-excluded balance)
SECURITY:
- MEDIUM: unacknowledged CRITICAL admin-notification flood capped (global cap
on critical_payment_log + refresh_token_reuse rows)
- MEDIUM: 2FA reissue no longer bypasses the mint cooldown (Check no longer
clears LastMintAt on gate-verify; cleared on terminal charge success)
- MEDIUM: twofa.StateFor map-saturation returns a shared permanently-locked
state instead of a fresh 5-guess budget per request
- MEDIUM: ProgressiveRateLimit rejects 429 past maxProgressiveSleepDelayMs
instead of sleeping unboundedly; login bcrypt concurrency semaphore added
- LOW: loginInProgress 409->429; webhook key-set/URL-unset startup check;
email-verification per-user attempt counter
DUP/MOD:
- formatCurrency single source (frontend format.ts, 7 files consolidated);
SquareRefundStatusToLocal single source (errors.go, all sites); admin
audit-log helper dedup; SCA retry model unified (proactive on all 6
surfaces); buyDailyTotal/daily-cap mirror via backend; lock TTL from
backend; generateUUID at all card-form sites; magic numbers named
(defaultPostgresHost, epsilon, fee constants); admin CASH + gift-card
terminal charges now audited; DAV_SKIP_INIT documented in manuals
Verified: 26/26 dev + 24/24 prod (GO_TESTING=1, the CI condition), both vet
tags, frontend tests+build, env-docs 42/42.
Loop A fresh money/security/dup-mod review of the whole payments overhaul. 28 consolidated findings fixed:
MONEY:
- HIGH-1: B12 overflow guard now uses the discounted obligation — a pre-start deposit can never mint an unintended tip; the discount is never truncated to £0 when the customer pays the discounted deposit
- HIGH-2: discounted-deposit pending-reuse retry compares pendingStoredAmountPence vs chargeAmount (the actual Square amount), not req.Amount — no more permanent amount_mismatch 400 on lost-response retries
- MEDIUM-3: sweep rescue now carves overflow as a tip record + runs completion side-effects (was booking overflow as service revenue, skipping completion)
- MEDIUM-4 (shared w/ security): admin_audit_log.admin_id made nullable + anonymize_user/delete_guest_user NULL it + scrub details.card_last4 — 2fa_fallback_charge PII no longer survives account deletion
- MEDIUM-5: till gift-card payment now passes the £5,000/day admin cap (giftcard_limits)
- LOW-6: expired gift-card balance surfaced as expired/zero in GetUserGiftCardBalance
SECURITY:
- 2FA single-use consume made atomic at verify time for all 5 saved-card gates (fresh charges consume; pending-reuse retries don't); deferred consumption removed
- reissueTwoFACodeAfterFailedCharge routed through the fail-closed issuance gate (pepper check, cooldown) + fresh-only semantics (only when a code was actually consumed)
- family-alive cache invalidated on the stale-family cleanup DELETE (no 30s warm window after expiry)
- frontend 503-retry no longer reuses a consumed 2FA code — aligns with backend re-issue
DUP/MOD:
- reissue helper single-sourced (5 call sites), squareRefundStatusToLocal (10 inline switches), writeChargeSnapshot (7 sites, immutability guard on gift-card/till), postChargeRecheck (3+1 sites), scanIdempotencySlot (2), applyVATToChargeRecord (3 patterns), user_saved_cards upsert (2), BuyGiftCard pending INSERT via service
- till completed-dedup now re-validates paymentHasLiveRefund (aligns with booking/tip/gift-card)
- frontend 402 idempotency-key regeneration added to PaymentModal (aligns with other CIT surfaces)
- PAYMENT_METHOD_SAVED_CARD constant standardised ('saved_card' everywhere)
- admin audit coverage added for AdminRefundBooking + gift-card buy/top-up
- audit-helper cross-package dedup (user/twofa.go now calls payments' exported insert)
Verified: 26/26 dev + 24/24 prod packages, both vet tags, frontend tests + build, gitleaks clean.
Loop B restart (money/security/dup-mod adversarial) fixes:
- CRITICAL: CreateTerminalPayment rejects payment_type='tip' (mirrors CreateBookingPayment) — a tip-typed admin charge no longer records the FULL amount as a tip and double-collects (all is-paid computations exclude tip rows)
- HIGH: tip refunds can no longer re-open booking capacity — refunded_total subqueries filter payment_type <> 'tip' (service.go) and RefundPayment rejects tip rows
- MEDIUM: loyalty-stamp farming closed — stamp award once-per-booking via loyalty_stamp_awarded_at column (init-script.sql) + existing same-day guard
- MEDIUM: CreateTipPayment/CreateBookingPayment 2FA gates moved AFTER the idempotency completed-dedup (code consumed only on new money paths; terminal path already correct) — lost-response retries return the completed payment instead of 400
- MEDIUM: replayRescueLowerBoundSkew widened to 5m (DB-clock-skew stranded originals now rescued)
- MEDIUM-1: verifyFamilyAlive DB amplification reduced via 30s bounded family-alive cache; admin route group rate-limited
- MEDIUM-3: admin saved-card charges now write admin_audit_log (handlers.go helper + till); [2FA] log line decoupled from user identity
- LOW-1: logout scoped to the presented token's family (no cross-session kill)
- LOW-2: refresh-reuse grace widened for same-IP replays
- LOW-4: squareEnvironmentMismatch enforced for empty env
- LOW-5: uuid.ts hard-fails on Math.random fallback (crypto.randomUUID)
- Cash/giftcard tip-enabled overflow mirrors the card-terminal carve
26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41.
Restart of Loop A (fresh review -> fix -> verify) findings from commit 5e967fa:
- B1: sweep auto-refund treats Square PENDING refunds as NON-terminal (row stays pending, no gift-card clawback, refunds row inserted for payments AND till_sales, re-polls the deterministic sweepdup- key); Square-less pre-pass exempts square_refund_id IS NOT NULL rows
- M4: terminal tip carve accounts for pending campaign discounts (headroom = total - pending - paid) so explicit tips aren't absorbed as service revenue; no-tip case stays a single record
- max_redemptions TOCTOU closed with atomic conditional UPDATE ... RETURNING; exhausted-at-apply surfaces campaign_fully_redeemed
- 2FA: verification code is single-use on the saved-card gate (VerifyForUser consume=true, interactive flows unaffected); new POST /api/user/2fa/code mints a fresh code for enabled users (RequireAuth + RequireNonGuest + mint cooldown + per-user limiter)
- Refresh tokens: family_id + used_at columns; reuse of an already-rotated token revokes the ENTIRE family and inserts a refresh_token_reuse admin alert; rotation mints descendants in the same family
- Frontend: 2FA code input + Request-a-new-code on all saved-card surfaces; admin modal keys code input to customer 2FA + 403 self-heal; tip-display note for pending discounts; 76 frontend tests
- Verified: all 26 backend packages pass, frontend build+tests green, env-docs 41/41
Rate limiting (backend):
- RateLimit/ProgressiveRateLimit now derive the per-client key from
CF-Connecting-IP, then chi's GetClientIP (the X-Real-IP value nginx sets at
main.go:323), then RemoteAddr. Previously only CF-Connecting-IP/RemoteAddr
were used, so behind the Docker nginx every client shared ONE bucket per
limiter — 10 logins/min site-wide blocked all users (the reported
'Error: Rate limit exceeded' after seeding was the login 10/min bucket
tripped by the seed's 11 logins, all keyed 127.0.0.1 in dev).
- Real implementation is now //go:build !dev || test; new
mw/ratelimit_dev.go (//go:build dev && !test) is a no-op passthrough, so
'go run -tags dev' (the dev harness) never rate-limits dev/seeding traffic,
while production and tests (-tags test,dev) keep the real limiter. The docs
(Technical Manual) already claimed dev no-op behaviour — the code now
matches. NewProgressiveRateLimiter is provided in the no-op build because
tag-free ratelimit_shared.go:104 initializes the global at package init.
Admin 2FA management (backend):
- users.two_factor_last_used_at TIMESTAMPTZ column (init-script, fresh-DB).
- AdminUserDetail now returns twoFactorEnabled/twoFactorMethod/
twoFactorLastUsedAt.
- New POST /api/admin/users/{id}/2fa/remove (admin-only): clears all 5 2FA
columns + drops the user's in-memory attempt/lockout state — an admin
recovery path when a user loses 2FA access.
- two_factor_last_used_at updated on every successful 2FA verification.
Account page (/account):
- 2FA section moved under the Notifications heading, visible to all roles;
Email/SMS toggles (Notifications styling) acting as a radio group with
'none' state; Apply button only when the selection differs from saved;
unselecting shows a payment-rules warning dialog; the dev-comment
'2FA is optional right now (REQUIRE_2FA is off)' and the 'Dev code:' debug
line are removed.
- Cards tab hidden from admin role.
Admin modals:
- User Details modal: new 'Two-Factor Authentication' section above Patch
Tests showing Enabled/Disabled, method, last-used timestamp, and a Remove
2FA button with a confirmation dialog (POST to the admin endpoint, refetch
on success).
- Booking Details modal: the customer's name now links to their User Details
modal (optional openUserModal prop threaded through admin/+page and
today/+page; other call sites unaffected).
Take Payment + /today:
- PaymentModal shows pre-tip (netTotal) and post-tip (totalWithTip) totals
with a tip-amount delta row only when a tip is selected; zero-tip flow
unchanged.
- The /today Payment button is hidden unless the booking is in_progress or
completed, matching the backend gate (was shown for confirmed/pending
bookings, producing the 'Booking must be in_progress or completed' error).
Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok
incl. new admin 2FA tests + mw tests), go build ./... and -tags dev both
compile, go vet clean, svelte-check 0 errors 0 warnings, env-docs gate OK,
docker compose config valid.
Sixth fresh-eyes review pass (5 agents: goal, QA, code-quality, security,
context-mining). QA FAILED the deposit-required new-card flow; the P0 root
cause was backend + frontend, now fixed. All 20 packages green.
P0 money-safety:
- Deposit-required bookings now actually charge the deposit on new-card
payment. Two-part fix: (1) CreateBookingHandler re-reads the
trigger-maintained total_amount/total_duration_minutes from the DB after the
booking_services insert (the INSERT..RETURNING row predates the recalc
trigger, so TotalAmount serialized as 0 and DepositPaid computed TRUE on an
unpaid booking — the frontend gate trusted deposit_paid:true, never charged,
and confirmed the booking with zero payment rows); (2) BookingFlow.svelte
gates the confirmation view on depositPaid and guards against re-creating a
booking on retry. Regression test
TestBookings_Create_DepositPaidFalseOnUnpaidBooking.
Payments (idempotency + money):
- deriveBookingPaymentIdempotencyKey: no-client-key fallback now advances a
sequence for repeatable types (partial) and rotates past refunded completed
rows, so refund-then-repay and equal-amount partials diverge onto distinct
keys; an un-refunded completed row keeps its key (double-charge protection
holds). Dedup hits on refunded rows now 409, never stale success.
- chargeFailureStatus default is 503 (ambiguous), never 402; table test.
- Flaky TestBookingPayment_FullPayment_SplitsIntoDepositAndBalance fixed
(ORDER BY payment_type).
- resolveChargeSource: orphaned card-on-file disabled via DeleteCardOnFile
when SaveCardForUser fails (best-effort, redacted log); retry path preserved.
Square client:
- Dev builds HARD-FAIL (panic) on SQUARE_ENVIRONMENT=production without
SQUARE_ALLOW_REAL_API=1; sandbox routes with a loud banner.
- Mock fault-injection FailAfterCommit (commit-then-5xx) exercises the exact
lost-response same-key retry; SimulateCardTokenUsed; 45-char idempotency-key
cap parity; SquareEnvironment/SquareLocationID shared env helpers used by
the sweep (env contract no longer comment-only).
- listRefunds truncation now errors (money-sensitive reconcile retries
instead of over-refunding); getCardsOnFile truncation loudly logged.
Webhooks + 2FA:
- square-environment header checked fail-closed (403) when configured env is
production/sandbox; dispatch DB work bounded by 30s timeout contexts.
- 2FA codes HMAC-SHA256 pepper'd (TWO_FACTOR_PEPPER) with legacy-hash
migration + upgrade-on-verify; disable-flow mint cooldown (1/min, 429) caps
the brute-force loop; in-lockout records never LRU-evicted.
Repo hygiene:
- env-docs CI gate green again (FRONTEND_ORIGIN + SQUARE_ALLOW_REAL_API +
TWO_FACTOR_PEPPER documented; Vite DEV built-in allowlisted).
- Dead square_deposits schema dropped; obsidian/README/legal-page drift fixed
(consumeradvice.scot signposting, CORS allowlist, p11 R3/P13, T1).
- 2FA disable residual documented; P6 email/SMS delivery and P12 sandbox
smoke test remain the pre-go-live gates.
Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok),
go build ./... + -tags dev, go vet clean, svelte-check 0 errors, env-docs
gate OK, live deposit-required flow re-verified end-to-end (deposit £11
charged, square_payment_id recorded).
Second fresh-eyes review pass (7 agents: goal, security, code-quality,
context-mining, webhooks+2FA, client+mock+sweep, refunds/giftcards/handlers).
Money-safety core verified sound (identical-body replay byte-lossless, clawback
gated on definitive proof, no double-charge window). This round fixes the
issues the fresh pass surfaced:
2FA:
- Setup now DELIVERS the code via the [2FA] server log in ALL modes (was:
nothing in enforced mode -> production 2FA was an unbreakable dead-end and
saved-card charges were permanently 403). Enforced mode still withholds the
code from the API response; the log line is the fake delivery channel until
email/SMS lands (P6).
- Disabling 2FA now requires a fresh verification code when enforcement is ON
(previously ignored the code -> a password-only attacker could lift the gate).
Shares the 5-attempt lockout and timing-safe compare. Dev bypass retained.
- REQUIRE_2FA parsing normalized (false/0/off/no, case-insensitive);
startup warning extended to the empty-env/mock-client/enforced-2FA confusion.
GDPR:
- anonymize_user() SQL now scrubs two_factor_* columns + staff notes, so the
idle-account batch cleanup (CleanupIdleAccounts) is erasure-clean, not just
the user-initiated delete path.
Webhooks:
- dispute.created for an untracked Square payment now raises a
critical_payment_log admin notification (chargeback the app can't reconcile
is never silent). Reason strings truncated on rune boundaries (valid UTF-8).
Stale at-most-once comment corrected; revertTillSaleGiftCardFunding
duplication noted.
Sweep/mock parity:
- Mock CreatePayment dedup is now source-aware (IDEMPOTENCY_KEY_REUSED on
source mismatch) matching ReplayPaymentByKey and real Square.
- COMPLETED-but-never-polled terminal till-sale checkouts are now recorded by
the sweep (previously only booking checkouts were; till charges were
invisible until the 24h blind-fail WARN).
- Legacy snapshot-less minimal-body replay, SQUARE_LOCATION_ID drift, and
in-memory-mock-restart limitations documented.
Docs:
- Webhook path corrected everywhere (/webhooks/square, not /api/webhooks/square
- a deployer following the old path would 404 and silently lose all webhook
reconciliation).
- 2FA enforcement semantics + code-delivery mechanism documented accurately
(fail-closed default; log-delivery channel; disable re-verification).
- README/User Manual note the 2FA requirement on online saved-card payments.
Tests: 2,151 (up from 2,142). Backend 26/27 packages green (crussell/db fails
only in this environment: local postgres doesn't offer scram-sha-256 for the
test role; package is byte-identical to HEAD and untouched here). Frontend
builds; svelte-check 0 errors.
Follow-up to the comprehensive payment-system review. Fixes the issues the
review found in the initial integration, plus the rough edges it introduced.
Money-safety:
- Replay-by-key now replays the FULL original request verbatim from a stored
square_request_snapshot, so a retained idempotency key returns the original
payment instead of IDEMPOTENCY_KEY_REUSED (previously the row sat pending
forever). IDEMPOTENCY_KEY_REUSED remains ambiguous (never proof of no charge).
- Dev mock mirrors real Square for unknown-key replays: ccof: saved-card
sources are charged and rescued; spent cnon: nonces surface
ErrReplayKeyNotRetained. (Fixes dev/prod parity divergence.)
- Webhook dedup row committed AFTER dispatch (at-least-once); FAILED till sales
claw back gift-card funding; event-type strings match Square's real catalog.
- Expired-gift-card cancellation refunds set creditFailed (never a phantom
'completed' refund); cancellation refunds lock all payment rows ascending.
- Sweep never rescue-completes a gift-card purchase without delivering the card.
- Tip no-client-key fallback is a deterministic count-based key under the
booking advisory lock (retry-safe, distinct tips don't collapse).
- M-cap subtracts completed refunds, clamped to [0, total].
2FA (PSD2 SCA stand-in) for online saved-card payments:
- Full feature: status/setup/verify/disable endpoints, gating helper wired into
all 7 saved-card charge paths (incl. BuyGiftCard + admin saved-card), account
admin-tab settings UI, frontend gating across all payment surfaces.
- Enforcement is FAIL-CLOSED: on unless REQUIRE_2FA=false or an explicit
mock/dev SQUARE_ENVIRONMENT; startup warning when off in a non-dev env.
- Verify is brute-force hardened (5-attempt lockout, timing-safe compare);
plaintext codes only logged when enforcement is off (dev).
- GDPR: anonymize_user also scrubs 2FA columns and staff notes.
Infra/docs:
- nginx: /api/ response cache removed (cross-user disclosure); port 80
redirects to HTTPS (localhost/RFC1918 exempt, end-anchored regexes); HSTS;
separate webhook rate-limit zone.
- Schema: users 2FA columns; payments/till_sales square_source_id +
square_request_snapshot.
- Legal docs: gift-card cooling-off, international-transfers section, tips
policy; Gap Backlog P3 webhooks marked done; stale counts/wording corrected.
- Flaky test race fixed (t.Parallel + global mock mutation); suite 26/26
packages green, 2,142 tests, svelte-check clean.
Tip double-count (root cause of £33.75 vs £28.75 display):
- Remove mock's fixed +500p auto-tip when AllowTipping is true (square_dev.go) —
real Square only enables a terminal prompt, it never adds a tip to the amount
- Set AllowTipping=false in CreateTerminalPayment: the frontend already embeds
the tip in the amount, so the terminal must not prompt for a second tip
- M4 tip split now derives the tip as charged amount minus remaining booking
value ('after 100% is tips'), not from Square's TipAmount field
- Success screens divide paymentResult.amount by 100 (pence -> pounds) in both
PaymentModal and UserPaymentModal
Fully-paid bookings auto-complete:
- Extract ApplyBookingCompletionSideEffects into payments package (shared by
admin progress endpoint and payment paths; avoids circular import)
- Add bookingIsFullyPaid + completeFullyPaidBooking: when completed non-tip
payments reach 100% of the booking total, an active booking transitions to
'completed' so it leaves the admin Current Appointment view
- Wired into CreateBookingPayment (inside tx) and GetCheckoutStatus (terminal,
after commit); completion side-effects (loyalty, campaigns, deposits_required)
fire identically to the manual progress endpoint
- Add /admin/bookings/{id}/refund route (AdminRefundBooking)
Discount-refund hardening:
- RefundPayment explicitly rejects discount/on_the_house payments (was relying
on the incidental NULL-square_payment_id guard)
- Hide the Refund button for discount/on_the_house payments in EditBookingModal
- Cancel-refund estimate in BookingModal also excludes on_the_house
- Cancellation refund loop + GetBookingPaymentInfo + GetBookingRefundableAmountCents
exclude payment_type='tip' from refundable totals
Tip flow (start-time guard) fixes tests:
- Tip tests updated to use past-dated bookings (tips now require booking started)
Tests:
- m4_tip_refund_redesign_test.go (tip split, refund exclusion, admin refund cap)
- m5_fully_paid_completion_test.go (online + terminal full-payment completion,
partial stays active, tip excluded, cancelled stays cancelled)
- Full suite passes with -race (25 packages)
The project is pre-launch with no production DB — all dev starts from a fresh
volume recreated from init-scripts/init-script.sql. All ALTER statements are
removed and their effects expressed directly in the CREATE statements:
name_history now lives after bookings with its booking_id FK inlined (no
ALTER TABLE ADD CONSTRAINT), the critical_payment_log enum value is in the
CREATE TYPE (no ALTER TYPE ADD VALUE), and voucher_type_at_purchase is in the
CREATE TABLE gift_cards (no ADD COLUMN).
gift_card_expired_balances.account_id is now nullable: CleanupExpiredGiftCards
inserts NULL for unredeemed gift cards (bought for non-account-holders, never
claimed) — a NOT NULL column made that expiry fail at runtime. The three tests
that papered over this with ALTER TABLE DROP NOT NULL now rely on the schema
directly.
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.
Adds critical_payment_log to the admin_notification_reason enum (fresh installs + ALTER TYPE for existing deploys); corrects the README's false cash-with-change claim; updates Gap Backlog T14 with the scan job stopgap; documents the till flow's clawback/cash-reconciliation model in the Technical Manual; records the frontend's re-tokenize-on-failure design in P11; and marks the P14 customer_id assumption VERIFIED (Square runtime enforces it per its SDK maintainer; only the OpenAPI schema stays ambiguous, so the P12 sandbox test remains the definitive live check).
square_webhook_events is now CREATE TABLE IF NOT EXISTS, registered as the 24th maintenance job (sweep-square-webhook-events, daily 2:30am) pruning rows older than 90 days, and testdb gives a clear docker compose hint when the admin DB connection fails.
anonymize_user, delete_guest_user, and AnonymizeStaleGuestAccounts now NULL square_card_id and square_customer_id on user_saved_cards (7-year retained_until soft-delete kept for financial records). Make square_card_id nullable in the schema. GDPR export refunds join fixed to include gift-card-purchase refunds. Add scrub assertions to the GDPR and stale-guest test suites.
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.
Add new table for staging future default hours changes with an upsert guard (at most one pending change). Extend admin_notification_reason enum with 'default_hours_changed'.
Add CancelReservationHandler (DELETE /api/bookings/reserve) to release authenticated user's active reservation. Register route in main.go. Add background goroutine for periodic reservation cleanup using CleanupOldReservations. Add idx_time_blockers_created_at index and extend anon cleanup to cover edit_request reservations in init-script.sql.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add total_duration_minutes, total_amount, end_time computed columns to bookings table with PL/pgSQL triggers for auto-recalculation. Add indexes for new columns (end_time, status+end_time, notes trigram) and existing join columns (service_id, custom_service_id). Add trigram indexes for user name and service name search.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add generate_*_id() functions for consistent short ID generation across all tables.
- Convert user_social_logins, verification_codes, custom_services,
time_blockers, forgiven_no_shows, user_referrals from BIGSERIAL to CHAR(12)
- Add name_history, referral_discounts tables
- Add dedicated generate_*_id() functions for each table
- Add FK from name_history.booking_id to bookings(id)
- Add referral_discounts index on user_id
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Added generic formatAndPreserveCursor() helper on frontend to track
and restore selection caret position during dynamic input sanitization
- Applied to all card inputs, gift card code inputs, and expiry inputs
- Added Luhn validation (isValidLuhn) for saved cards and gift cards
- Rebuilt payments test DB and got 100% green tests
Fix anonymize_user() setting exp_month/exp_year to NULL on NOT NULL columns — use placeholder values (1/2000) matching pattern for other NOT NULL columns. Extend export_all_user_data() with 9 new sections: saved_cards, refunds, social_logins, loyalty_redemptions, booking_discounts, edit_requests, affiliate_payouts, verification_codes, forgiven_no_shows. Add total_price to booking objects (sums COALESCE override prices). LEFT JOIN loyalty_redemptions to bookings for booking_date/booking_services. LEFT JOIN booking_discounts to discount_campaigns for campaign_name. Simplify referrals to referrer_name/referred_name using u.fn.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Backend now stores AVIF, WebP, JPEG, and optional JXL variants for both full-size and thumbnail images. Database schema extended with 7 new columns. Image validation supports AVIF and JXL magic bytes. Comprehensive test coverage for all format URL fields and magic byte detection. Legacy single-URL images remain backward-compatible.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
popertotsandSisyphus