Commit Graph
27 Commits
Author SHA1 Message Date
popertots 9bb812669e fix: fresh-review round — 2FA deliverability, disable re-verification, GDPR batch scrub, dispute alerting, docs accuracy
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.
2026-08-22 00:34:49 +01:00
popertots e9b0f0f2a7 fix: review-loop hardening — identical-body replay, 2FA gates, webhook at-least-once, GDPR scrub
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.
2026-08-22 00:34:49 +01:00
popertots 4b28e93710 fix: tip double-count, fully-paid auto-completion, discount-refund hardening
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)
2026-08-22 00:34:49 +01:00
popertots 01ac211408 Square client and dev mock: replay-by-key reconcile, refund classification, mock parity
ReplayPaymentByKey (POST /v2/payments re-issue with the same idempotency key
and a synthetic probe source token that can never process a real charge):
Square returns the ORIGINAL payment for a retained key and definitively
rejects an unknown/expired one, so the stale-pending sweep can rescue
lost-response charges without ever issuing a second payment. ErrReplayKeyNotRetained
marks a probe rejection as proof the charge never happened.

Refund classification: zero-amount refunds are now rejected (Square requires
amount_money) instead of lenient full-refund; REFUND_ALREADY_PENDING is
classified as already-processed to match the real contract.

Dev mock parity: SquarePayID == payment ID (was fabricated 'sqp_' prefix),
ForceCheckoutState for IN_PROGRESS/CANCEL_REQUESTED terminal states,
replay-by-key support, aligned refund error codes.
2026-08-22 00:34:49 +01:00
popertots a8d54f1e2a Fix review findings: aggregated-refund/saved-card/legacy-refund idempotency keys, structured Square error classification, CSP for Square SDK
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.
2026-08-22 00:34:49 +01:00
popertots 8a3a7ec062 Mirror Square's customer_id requirement in the dev mock card-creation gate
Square rejects POST /v2/cards without card.customer_id at runtime (confirmed by Square's SDK maintainer); the production client omits an empty id via omitempty and every caller provisions a customer first, so the mock now enforces the same structured 400 INVALID_REQUEST_ERROR to keep sandbox/dev parity with the gate the production code depends on.
2026-08-22 00:34:49 +01:00
popertots f0099714ff Harden Square HTTP client and dev mock: DeleteCustomer, response limits, token redaction
Adds SquareClient.DeleteCustomer for GDPR erasure (DELETE /v2/customers/{id}, NOT_FOUND as no-op), bounds doJSON response reads to 1 MiB with rune-safe 500-byte error snippets, adds validCardID guard to the disable-card URL, makes paymentFromSquare card fields consistent when the card ID is empty, redacts ccof/cnon tokens in all log paths, and fixes the idempotency-key-length comment (45 chars for payments/refunds/cards, 64 only for terminal checkouts).
2026-08-22 00:34:49 +01:00
popertots 12af3af3b3 Harden Square HTTP client and dev mock: status codes, token validation, deadline wire format
Add StatusCode/Category/Field to squareAPIError and an IsNotFound helper so 400/401/404/429/5xx are distinguishable structurally instead of by substring. Validate cnon:/ccof: token prefixes in createPayment/createCardOnFile (PCI parity with the mock). Reject ccof charges without customer_id in the mock so dev parity catches the production bug. Emit Deadline as the RFC 3339 duration (PT5M) and correct the deprecated-comment.
2026-08-22 00:34:49 +01:00
popertots 54a5b1024e Implement full Square payment review fixes + frontend polish
Implement every finding from the deep payment review (P0-P2, minors,
nitpicks), then close the post-implementation re-review items, then
align card-form typography and roll out the Square trust badge.

Backend - Square API alignment:
- tip_settings.allow_tipping nested under device_options (was top-level:
  terminal tips were silently lost in prod)
- CreateCardOnFile now accepts customerID and sends card.customer_id;
  saved-card (ccof:) charges forward square_customer_id as CustomerID
- New SquareClient methods GetPayment, CreateCustomer, CancelCheckout
- SCA verification_token accepted + forwarded in all charge paths
- ExpMonth/ExpYear -> *int; URL-path id validation; CancelCheckout
  NOT_FOUND-only no-op (dropped unverified NOOP); exported ErrorCode/
  ErrorDetail helpers; mock rejects raw PANs, RList locks, redacts
  emails, ForceRefundPending hook

Backend - money safety:
- sweepManualPendingSquareRefunds reconciles rows WITH square_refund_id
  instead of stranding them forever
- SweepStalePendingPayments reconciles at Square before failing (tri-state:
  leave pending on transport error, rescue completed, fail definitively)
- GetCheckoutStatus cancellation-recheck; terminal CANCELED resolution;
  SweepStaleTerminalCheckouts covers terminal_checkouts table
- till gift-card clawback on definitive failure incl. retry path +
  INSUFFICIENT_FUNDS/ADDRESS_VERIFICATION_FAILURE/TRANSACTION_LIMIT
- cross-user saved-card collision fixed (UNIQUE(user_id,square_card_id))
- customer provisioning (lazy, save-only); one-off/guest mint no customer
- discount preview/apply unified in discounts.go (global-milestone visible
  in preview, N+1 eliminated, redemption counter preserved on failures)
- webhook event_id dedup; refund loop dedup; stale comment fixes
- test-isolation t.Cleanup on committed sweep tests

Frontend:
- SCA tokenizeWithVerification across all charge flows (amount as
  major-units decimal), 5-min token-expiry re-tokenize, verification_token
  in request bodies
- PaymentModal synchronous double-click + zero/negative-amount guards
- till online-card UI wired to /api/admin/till/sale
- policyPopover generalised; new /privacy-policy route; consent checkbox
  copy + Square privacy link
- Square card iframe styled to app typography (Inter 14px, oklch tokens);
  mock form md:text-sm parity
- 'Secure payment powered by Square' badge on all 8 card-payment flows

Schema/docs: terminal_checkouts + square_customer_id + per-user card
constraint in init-script.sql; README migrations; P14 plan + backlog +
Technical Manual updated.

Includes 39 modified/new test files; full backend suite (25 pkgs),
-race on payments+square, and frontend build are green.
2026-08-22 00:34:49 +01:00
popertots 7439fa86c1 Fix payment review round 3: saved-card idempotency, stale-pending sweep, webhook fail-closed
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
2026-08-22 00:34:49 +01:00
popertots 16240d67e3 Fix Square card linkage: reference_id instead of customer_id (no Square customer provisioning)
The app does not provision Square customers, so sending the local user ID as
customer_id in Create Card was rejected with CUSTOMER_NOT_FOUND, and filtering
List Cards by it returned nothing. reference_id is Square's free-form client
reference — max 128 chars, no uniqueness constraint — and is echoed in both
Create and List responses.

- Create Card payload: reference_id = local user ID (customer_id absent)
- List Cards: native ?reference_id=<userID> filter (no limit/customer_id,
  no client-side filter, no cursor handling needed)
- Mock parity: CreateCardOnFile stores ReferenceID; GetCardsOnFile unchanged
- Regression guards: TestCreateCardOnFileHTTP_IdempotencyKey asserts
  reference_id=user_1 and customer_id ABSENT; new
  TestGetCardsOnFileHTTP_ReferenceIDFilter asserts the query shape
2026-08-22 00:34:49 +01:00
popertots ae8735ba2f Close refund system and gate raw-PAN card entry
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
2026-08-22 00:34:49 +01:00
popertots 54f6bf3c1a Fix P0/P1 review findings: truncation, raw-PAN API edge, refund lock, till pending-retry, idempotency keys
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.
2026-08-22 00:34:49 +01:00
popertots 4f5dd5c426 Fix review findings: BuyGiftCard concurrency lock, amount guards, NULL scan, mock dedup, docs
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.
2026-08-22 00:34:49 +01:00
popertots 3db8b54923 Fix silent tip/gift-card money loss on pending retry; terminal checkout wire; sentinel error; docs
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)
2026-08-22 00:34:49 +01:00
popertots bbb55dae82 Match dev mock to production: reject raw PAN card creation (PCI-DSS parity)
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
2026-08-22 00:34:49 +01:00
popertots 2459ddc919 Fix review findings: expiry bug (all 8 files), idempotency keys, card_expiry/card_cvc removal, URL encoding, BuyerEmail logging, ValidateCardInfo, saved-card test, future work doc
Backend:
- Fix refund idempotency key: clock.Now() → deterministic (pr.ID + amount)
- Fix ValidateCardInfo: enforce mutual exclusivity, handle empty strings symmetrically
- Fix paymentFromSquare brand fallback (remove dead SourceType fallback)
- Fix URL encoding: PathEscape → QueryEscape for customer_id query param
- Fix BuyerEmail: log warning on DB error instead of silent discard
- Fix idempotency key in createCardOnFileHTTP: time.Now() → deterministic hex hash
- Add BuyerEmail to CreateTipPayment Square request
- Move realBaseURL from shared file to square_dev.go (only used in dev)
- Add TestTipPayment_WithSavedCard test (card_id path coverage)
- Fix AMEX brand in mock (AMEX → AMERICAN_EXPRESS, fix test)

Frontend:
- Fix off-by-month expiry bug in ALL 8 files using year-month arithmetic
  (parseExpiryParts returns 1-indexed, SvelteDate expects 0-indexed)
  Files: tip/+page, pay-tip/[id], UserBookingModal, UserPaymentModal,
  BookingFlow, account/+page (add card + buy gift card sections)
- Remove card_expiry/card_cvc from tip request bodies (backend has no fields)

Docs:
- Mark P9 (placeholder tokens) as completed, add P11 (Square Web Payments SDK)
- Mark T13 (rune arithmetic) as completed
2026-08-22 00:34:49 +01:00
popertots 4abcb324c9 Square payment integration: real HTTP client, tip flow rewrite, card UI/validation overhaul
Backend:
- Create square_http_client.go: real Square REST API client (Payments, Terminal Checkouts, Refunds, Cards, Locations) with proper JSON types, auth, error handling
- Update ProdClient in square.go to delegate to shared HTTP functions
- Wire devProdClient in square_dev.go to also make real HTTP calls for sandbox/prod env
- Rewrite CreateTipPayment handler: accept card_id OR new_card_token (+save_card), advisory lock, idempotency check, max amount validation
- Add ValidateCardInfo, bump ValidateAmount max to £10,000
- Fix mock CreateCardOnFile to detect brand/last4 from raw card numbers
- Fix mock RefundPayment to index by SquarePayID and accept unknown payment IDs
- Remove dead types (ProcessingFee, sqAddress), add Deadline parity
- Fix AMEX brand inconsistency (AMEX -> AMERICAN_EXPRESS)
- Pre-existing fix: remove unused context import in giftcards.go

Frontend:
- CardInput.svelte: add onfieldblur/onfieldinput callbacks for blur-based validation
- CardBrandIcon.svelte: brand SVGs for VISA, MC, AMEX, Discover, Diners, JCB, Square Gift Card, UnionPay, Interac, EFTPOS
- tip/+page, pay-tip/[id], UserBookingModal tip: saved card list + CardInput + Luhn/expiry/CVC validation + blur-based errors + no-saved-cards edge case
- UserPaymentModal, BookingFlow: card validation parity (blur-based, all-valid check)
- account page: replace text brand badges with CardBrandIcon
- Fix handleCustomTip bug (state mutations outside if block)
- Remove dead pageState variable
- Add tip modal scroll (max-h-[90vh] overflow-y-auto)
- Submit button disabled on !isCardValid

Tests:
- 30 square package tests (+new: CreateCardOnFile raw number path, detectCardInfo variants)
- 5 tip handler tests (HappyPath, NoPriorPayment, WrongOwner, MultipleTips, TxFailure)
- All +-race clean, refund tests fixed
2026-08-22 00:34:49 +01:00
popertots 05c5d73d30 fix: add ShouldFail to Square MockClient for testing payment error paths 2026-07-11 16:07:05 +01:00
popertots 609a099677 fix: data races in Square mock and scheduler hostname tests
CI / Env docs check (push) Successful in 12s
CI / Docker compose check (push) Successful in 14s
CI / Nginx config check (push) Successful in 17s
CI / Frontend major deps (push) Failing after 23s
CI / Frontend deps check (push) Successful in 28s
CI / Secrets scan (push) Successful in 37s
CI / Go build (push) Successful in 37s
CI / Frontend build (push) Successful in 1m6s
CI / Knip (push) Successful in 34s
CI / Frontend a11y check (push) Successful in 1m36s
CI / Go vet (prod) (push) Successful in 1m30s
CI / go mod tidy (push) Successful in 44s
CI / Go vet (dev) (push) Successful in 2m9s
CI / Frontend QC (audit) (push) Successful in 45s
CI / Go vulnerabilities (push) Successful in 1m24s
CI / Staticcheck (prod) (push) Successful in 2m55s
CI / Staticcheck (dev) (push) Successful in 3m28s
CI / golangci-lint (push) Successful in 3m29s
CI / Frontend QC (typecheck) (push) Successful in 1m26s
CI / Security scan (prod) (push) Successful in 4m22s
CI / Security scan (dev) (push) Successful in 4m23s
CI / Frontend QC (lint) (push) Successful in 1m58s
CI / Svelte strict check (push) Successful in 57s
CI / Tests (prod) (push) Successful in 4m15s
CI / Tests (dev) (push) Successful in 4m17s
CI / Race (prod) (push) Successful in 8m2s
CI / Race (dev) (push) Successful in 8m17s
- square_dev.go: return a copy of CheckoutResult to the caller
  instead of sharing the map-stored pointer, preventing the goroutine
  from racing with the caller on Status field reads
- scheduler_test.go: remove t.Parallel from TestInitHostname_* tests
  which modify a shared package-level hostname variable
2026-07-10 19:10:31 +01:00
popertots 3029fd5179 test: add coverage tests across backend + fix mock for PENDING checkout support
CI / Nginx config check (push) Successful in 13s
CI / Env docs check (push) Successful in 15s
CI / Docker compose check (push) Successful in 15s
CI / Frontend major deps (push) Failing after 24s
CI / Frontend deps check (push) Successful in 30s
CI / Secrets scan (push) Successful in 38s
CI / Go build (push) Successful in 39s
CI / Frontend build (push) Successful in 1m3s
CI / Knip (push) Successful in 45s
CI / Go vet (prod) (push) Failing after 1m42s
CI / Frontend a11y check (push) Successful in 2m34s
CI / Go vet (dev) (push) Successful in 2m29s
CI / Staticcheck (prod) (push) Failing after 2m38s
CI / go mod tidy (push) Successful in 1m3s
CI / Staticcheck (dev) (push) Successful in 2m55s
CI / Frontend QC (audit) (push) Successful in 51s
CI / golangci-lint (push) Successful in 3m22s
CI / Go vulnerabilities (push) Successful in 1m26s
CI / Frontend QC (typecheck) (push) Successful in 2m18s
CI / Security scan (prod) (push) Successful in 4m18s
CI / Security scan (dev) (push) Successful in 4m40s
CI / Tests (prod) (push) Has been skipped
CI / Tests (dev) (push) Has been skipped
CI / Race (prod) (push) Has been skipped
CI / Race (dev) (push) Has been skipped
CI / Frontend QC (lint) (push) Successful in 2m18s
CI / Svelte strict check (push) Successful in 43s
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)
2026-07-10 18:13:44 +01:00
popertotsandSisyphus 510828c924 chore: run go fix for Go 1.26 modernization
CI / Go vulnerabilities (push) Successful in 1m10s
CI / Build & Vet (push) Successful in 1m39s
CI / Frontend build (gate) (push) Successful in 1m42s
CI / Frontend QC (audit) (push) Successful in 56s
CI / Frontend QC (typecheck) (push) Successful in 1m36s
CI / Frontend QC (lint) (push) Successful in 1m51s
CI / Tests (prod) (push) Has been cancelled
CI / Tests (dev) (push) Has been cancelled
CI / Race (prod) (push) Has been cancelled
CI / Race (dev) (push) Has been cancelled
106 files: interface{}→any, strings.Split→SplitSeq, CutPrefix/Cut, strings.Builder, slices.Contains, remove redundant // +build directives, gofmt import ordering and indentation.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-07-09 17:25:23 +01:00
popertotsandSisyphus 296a67770e fix: resolve RLock deadlock in Square mock client
Move m.mu.RUnlock to defer in GetCheckout and remove duplicate RLock/RUnlock around m.completed lookup. This prevents a deadlock when CheckoutPending error is returned but the RLock was already released.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-07-05 11:48:04 +01:00
popertotsandSisyphus 58996c553a refactor(internal): replace log.Fatal with panic, add timezone to DSN, add empty S3 bucket check
Replace log.Fatal in dev service init with panic for consistency. Add timezone=UTC to DAV connection DSN. Add IsEmpty() check for S3 dev bucket.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-06-24 23:43:14 +01:00
popertotsandSisyphus 2dbb1486b0 feat(backend): update Square integration, validators, and image validation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-06-18 16:26:47 +01:00
popertotsandSisyphus 25ece1e0b4 feat: Square client dev mock for online payments and type additions
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-05-25 18:04:04 +01:00
popertots 2e1ab9d745 feat: Square payment integration, booking flow redesign, and timezone/weekday fixes
- Add Square payment integration (mock + handlers + UI): terminal/online payments,
  refunds, tips, saved cards, webhooks. Build-tagged dev/prod clients.
- Redesign booking flow: Step 4 conditional (deposit only), Step 5 confirmation
  screen with booking ID, auto-submit on transition.
- Redesign schedule modal: 2x3 button grid with Pay Deposit/Pay Early logic.
- Add deposit warning banner at Step 1 for users with outstanding deposits.
- Fix weekday conversion bug: Go 0=Sunday vs DB 0=Monday mismatch in 6 locations.
- Fix timezone bug: UTC vs London time in closing hours validation.
- Fix frontend error parsing: plain text backend errors now displayed correctly.
- Fix crypto.randomUUID fallback for environments without Web Crypto.
- Add 7 new regression tests: closing hours, advance check, active booking limit,
  weekday conversion, UTC/London, deposit snapshot, exceptional hours.
- Fix 3 flaky tests: dynamic dates instead of fixed, no-show timing.
2026-05-23 11:29:34 +01:00