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.
The pending-retry amount guard compared pence via int64(pounds*100),
which truncates instead of rounding. For non-exact pound values (e.g.
£1.14 stored as the float64 1.1399999999999999) the truncation yields
113 != 114, falsely rejecting a legitimate same-amount retry with 400.
Since the frontend reuses the idempotency key on same-amount retries,
every retry was rejected, permanently stranding the pending record
(and any orphaned Square charge) with no recovery path.
Fix: compare in pence via math.Round — the existing pattern already
used in refunds.go — so non-exact values round to the true pence.
Also applied the same correction to the sibling lossy conversions:
- CreateTipPayment / CreateBookingPayment completed-dedup responses
(would have reported 113p for a 114p payment)
- BuyGiftCard retry amount guard (latent: £10/£20/£50 are float-exact
so it never bit, but the identical trap is now closed)
Tests:
- TestTipPayment_RetryPending_NonExactAmountSucceeds: 114p pending
record + same-amount retry completes and charges 114p (failed on the
old truncation with 400)
- TestTipPayment_RetryPending_AmountMismatchRejected: a same-key retry
at a different amount is still rejected with 400 and the pending
record is left untouched
Verified: full backend suite green (25/25 packages, 0 failures),
-race clean on handlers/payments, go build + go vet clean.
N1 (HIGH) — BuyGiftCard concurrent same-key retry could double-issue gift
cards (2× value for 1 charge). Added pg_advisory_lock on the idempotency key
(mirroring the tip pattern) acquired before the idempotency check, so
concurrent same-key retries serialize and only one executes gift-card
creation.
N2 — Amount-equality guards in both reuse branches (CreateTipPayment and
BuyGiftCard). A same-key retry with a different amount now returns 400
instead of silently mutating the pending record's books/VAT/refund caps.
N3 — test coverage:
- TestBuyGiftCard_RetryPending_ReattemptsCharge: pending record + same-key
retry re-attempts, reuses the record (count=1), completes, and issues the
gift card exactly once.
- TestCreateCheckoutHTTP_DeviceOptionsWireShape: httptest.Server asserts
device_id is under checkout.device_options (not top-level). Extracted
createCheckoutHTTPWithClient for injectable base URL.
- MockClient.CreatePayment now dedups on idempotency key (paymentByKey map),
matching real Square behaviour.
N4 — Corrected the savepoint comments in handlers.go and giftcards.go: the
savepoint only exists in the test harness; in production db.Conn.Begin is a
plain tx and the status UPDATE runs on a separate pooled connection. Commit
is a harmless no-op in prod but required in tests.
Bonus bug fixed: CheckIdempotencyByKey scanned NULL booking_id/gift_card_id
(gift-card purchases) into plain string, failing with 'cannot scan NULL'.
Now uses sql.NullString.
Docs: Technical Manual.md:53 and Feature Catalog.md (2.1, 2.5) corrected —
no longer claim Web Payments SDK is live; new-card entry is documented as
pending P11, saved-card flow works via ccof tokens, dev mock rejects raw PANs.
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)
Previously the dev MockClient was more permissive than production:
- MockClient.CreateCardOnFileRaw processed raw PANs and stored mock cards,
while ProdClient and devProdClient both block raw PANs. A dev testing the
raw-card flow saw it succeed, masking a production failure.
- MockClient.CreateCardOnFile accepted raw PANs as source_id via an
isAllDigits branch. Real Square only accepts cnon:xxx/ccof:xxx tokens.
Now the mock behaves identically to production:
- CreateCardOnFileRaw returns the same PCI error as ProdClient
- CreateCardOnFile validates source_id is token-like (cnon:/ccof:) and
rejects raw PANs
- Removed dead isAllDigits helper
Tests updated to assert the parity behavior:
- TestDevClient_CreateCardOnFileRaw_Rejected_ProdParity (table-driven,
replaces 5 brand-specific raw-PAN tests)
- TestDevClient_CreateCardOnFile_RejectsRawPAN (replaces RawNumber)
- TestCreatePaymentMethod_HappyPath / SecondCardNotDefault now expect
500 instead of 200, documenting the prod block
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.
Admin no longer receives a notification when someone buys a gift card for a friend. The recipient email and gift code are logged for future SMTP delivery instead.
New test files cover previously untested paths across DAV, validators,
S3, Square, mw, bookings, user, and payments packages.
Includes mock fix: HoldCheckouts flag on MockClient allows tests to
pause auto-complete goroutine for testing PENDING checkout states.
Coverage: 50.4% → 65.0% (+14.6pp)
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>
The phase 3 payment query used ORDER BY created_at DESC LIMIT 1.
Within a single transaction, NOW() returns the same timestamp for all
inserts, making the ordering non-deterministic when other parallel tests
insert payment rows with the same timestamp. Fix by parsing the
response's payment ID and querying by it directly.
Also revert eslint.config.js back to error-level rules for all categories
(removing the previous 'warn' overrides).
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>
Add GetVATConfig for reading VAT settings via any db.Querier, ApplyVATToBookingPayment for applying VAT to payment records, and ApplyVATToTillSale for SPV gift card till sales. Errors are logged without blocking payment flow.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Simplify payment handlers by using bookings.total_amount computed column instead of inline UNION sub-queries calculating price totals from booking_services and booking_custom_services.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Migrate all test files from SetupTestDB/db.DB pattern to per-test transactions:
- Replace SetupTestDB(t) with SetupTestTx(t) for context + transaction
- Replace db.DB.Query/QueryRow/Exec with tx.Query/QueryRow/Exec
- Replace context.Background() with context from SetupTestTx
- Replace defer rows.Close() pattern with explicit rows.Close()
- Add testdb.SeedBaseline(pool) to all TestMain functions
- Wire db.Conn = db.NewPoolProxy(pool) in all TestMain functions
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>
Add referral discount to payment preview for users with unused referral discounts.
- Check referral_discounts table for unused discounts in calculateDiscountPreview
- Wrap campaign discount application in a proper database transaction
- Apply eligibility guard for 2nd+ payments to prevent duplicate discount application
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Migrate all test files from resetTestData(t) to testutils.SetupTestDB(t) for isolated per-package test databases.
- Add new feature tests: name history assertions, referral discount preview,
time blockers, email validation, GDPR export, loyalty manual redemption
- Update existing tests to use batch queries and SetupTestDB
- Remove test_helpers.go resetTestData infrastructure
- Add comprehensive user profile tests (442 new lines)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Add CreateTestDatabase function for parallel isolated test databases per package.
- Add CreateTestDatabase() for isolated test DBs (parallel-safe)
- Move all TestMain functions to per-package testmain_test.go files
- Remove old TestMain from handlers_test.go, jwt_test.go, main_test.go
- Add JWT init guard in main.go to skip when -test.* flags detected
- Update testdb.go with admin DSN and proper cleanup
- Rename test database to crussell_test_db for consistency
- Replace testdb.NewPool + testdb.Migrate pattern with CreateTestDatabase
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Extract LoyaltyStampCost and add LoyaltyDiscountPercent constant. Add first-payment guard to ApplyLoyaltyRedemption — reject if a non-discount payment already exists on the booking.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
popertots
