diff --git a/README.md b/README.md index ab381a8..812106d 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,9 @@ Nail salon booking platform — Go 1.26.5 backend + SvelteKit 5 SPA + PostgreSQL ## Features -**Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). **Self-blocking prevention**: `excludeUserID` parameter filters a user's own `RESERVATION` entries from time blocker overlap checks, allowing re-reservation and booking at overlapping slots. **Explicit cancellation**: `DELETE /api/bookings/reserve` releases a user reservation; `DELETE /api/admin/bookings/reserve` releases an admin walk-in/call-in reservation. **Background cleanup**: Centralised cron scheduler (`backend/internal/jobs/`) runs 23 maintenance jobs: reservation/deposit cleanup every 5min, hourly campaign transitions, daily unpaid-booking notifications, staged default hours auto-apply, GDPR anonymization, financial aggregation, and token/code cleanup. Guest accounts with GDPR-compliant anonymization (including `RESERVATION:edit_request:%` scrubbing). Service eligibility based on age + patch test validity. Overlap checks use `FOR UPDATE` row locks inside transactions. Closing-hours validation (`closing_time.go`) resolves both current and staged default hours. +**Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). **Self-blocking prevention**: `excludeUserID` parameter filters a user's own `RESERVATION` entries from time blocker overlap checks, allowing re-reservation and booking at overlapping slots. **Explicit cancellation**: `DELETE /api/bookings/reserve` releases a user reservation; `DELETE /api/admin/bookings/reserve` releases an admin walk-in/call-in reservation. **Background cleanup**: Centralised cron scheduler (`backend/internal/jobs/`) runs 24 maintenance jobs: reservation/deposit cleanup every 5min, hourly campaign transitions, daily unpaid-booking notifications, staged default hours auto-apply, GDPR anonymization, financial aggregation, and token/code cleanup. Guest accounts with GDPR-compliant anonymization (including `RESERVATION:edit_request:%` scrubbing). Service eligibility based on age + patch test validity. Overlap checks use `FOR UPDATE` row locks inside transactions. Closing-hours validation (`closing_time.go`) resolves both current and staged default hours. -**Payments**: Square Terminal (in-person, via `CreateTerminalCheckout`) + online card payments via saved cards or new cards tokenized through the Square Web Payments SDK (`cnon:` nonces — new-card entry falls back to `CardEntryUnavailable` only when neither mock mode nor Square credentials are configured). The backend accepts only tokens, never raw PANs (PCI-DSS parity, mirrored in the dev mock). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record (same-key retries reuse it). Three background sweeps close Square's ~24h idempotency-key retention window: `sweep-pending-square-refunds` reconciles/retries stuck refunds (with a 23h age guard), `sweep-stale-pending-payments` fails stale pending payments/till-sales so a late retry cannot issue a second charge, and `sweep-stale-terminal-checkouts` cancels card-machine checkouts still pending at Square after an hour so a never-polled checkout cannot complete into an invisible, untracked charge. +**Payments**: Square Terminal (in-person, via `CreateTerminalCheckout`) + online card payments via saved cards or new cards tokenized through the Square Web Payments SDK (`cnon:` nonces — new-card entry falls back to `CardEntryUnavailable` only when neither mock mode nor Square credentials are configured). The backend accepts only tokens, never raw PANs (PCI-DSS parity, mirrored in the dev mock). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A bounded PostgreSQL advisory try-lock (`pg_try_advisory_lock`, ~30 × 100ms ≈ 3s bound) serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record (same-key retries reuse it). Three background sweeps close Square's ~24h idempotency-key retention window: `sweep-pending-square-refunds` reconciles/retries stuck refunds (with a 23h age guard), `sweep-stale-pending-payments` fails stale pending payments/till-sales so a late retry cannot issue a second charge, and `sweep-stale-terminal-checkouts` cancels card-machine checkouts still pending at Square after an hour so a never-polled checkout cannot complete into an invisible, untracked charge. **Gift Cards**: Multi-method purchase (cash, card machine, online card, giveaway). Inventory cards for stock management. 24-month rolling expiry. Idle account cleanup (2yr/5yr thresholds). Expired balance recovery with admin audit trail. Transaction audit log. Idempotency keys for purchases. @@ -85,7 +85,7 @@ Default logins (password: `password`): ```bash cd backend && go build -o bin/backend ./main.go cd frontend && npm ci && npm run build -cd backend && go test -tags "test,dev" -count=1 -parallel 8 ./... # 1,902 tests passed (4 skipped, ~2min) +cd backend && go test -tags "test,dev" -count=1 -parallel 8 ./... # 1,934 tests passed (4 skipped, ~2min) cd backend && go test -tags "test,dev" -count=1 -race -timeout 480s ./... # race detector (all packages, ~4min) cd backend && go test -tags "test,dev" -count=10 -parallel 8 ./... # thorough verification (~2-3min) ``` @@ -136,6 +136,12 @@ CREATE TABLE IF NOT EXISTS terminal_checkouts ( ); CREATE INDEX IF NOT EXISTS idx_terminal_checkouts_booking ON terminal_checkouts(booking_id, status); CREATE INDEX IF NOT EXISTS idx_terminal_checkouts_status ON terminal_checkouts(status); + +-- Square webhook dedup (square_webhook_events table) +-- NOTE: CREATE TABLE is a fresh addition, not a column change. Apply before deploying +-- the webhook handler changes or the dedup INSERT fails at runtime. +-- Required for the Square webhook dedup; without it webhooks fail closed 503. +CREATE TABLE IF NOT EXISTS square_webhook_events (event_id TEXT PRIMARY KEY, received_at TIMESTAMPTZ NOT NULL DEFAULT NOW()); ``` `backend/handlers/payments/refunds.go` casts `'refund_failed'::admin_notification_reason` (the sweep job in `internal/jobs/cleanup.go` only registers the handler), so an un-migrated DB fails at runtime — apply these before deploying the payment changes. diff --git a/obsidian/Crussell/Feature Catalog.md b/obsidian/Crussell/Feature Catalog.md index e2440a8..8836a3f 100644 --- a/obsidian/Crussell/Feature Catalog.md +++ b/obsidian/Crussell/Feature Catalog.md @@ -473,7 +473,7 @@ The central management hub for salon operations — managing users, bookings, se **Related:** [[Patch Tests|1.6 Patch Tests]], [[Services Catalog|1.5 Services Catalog]] ### 5.9 Till Purchases (POS) -**What it does:** Point-of-sale interface for selling gift cards at the counter (cash, card machine, saved card, online card entry, on the house). The saved-card path charges the customer's card-on-file directly via Square (pending-first, deterministic idempotency key, advisory lock per booking); card-machine sales create a Square Terminal checkout and are completed by polling. Pending card till-sales are failed by the stale-pending sweep after Square's ~24h key-retention window. +**What it does:** Point-of-sale interface for selling gift cards at the counter (cash, card machine, saved card, online card entry, on the house). The saved-card path charges the customer's card-on-file directly via Square (pending-first, client-supplied UUID idempotency key — or a generated `till-` key when absent — with a bounded advisory try-lock keyed on that idempotency key); card-machine sales create a Square Terminal checkout and are completed by polling. Pending card till-sales are failed by the stale-pending sweep after Square's ~24h key-retention window. **Layman summary:** "Sell gift cards at the till — take cash or card." diff --git a/obsidian/Crussell/Technical Manual.md b/obsidian/Crussell/Technical Manual.md index 86670fe..e2b89bc 100644 --- a/obsidian/Crussell/Technical Manual.md +++ b/obsidian/Crussell/Technical Manual.md @@ -87,7 +87,7 @@ Backend (:8080) | `ProgressiveRateLimit` | Per-IP dual-window rate limiter for bot-spam prevention. Burst: 30 req/5s, Sustained: 120 req/60s. Progressive delays (500ms–10s). Applied to login/register. Skips in dev build. | | `JsonContentType` | Sets `Content-Type: application/json` on all API responses. Individual handlers that need to override (e.g. binary/image responses) set their own Content-Type header after this middleware runs. Applied globally in `main.go:182`. This replaced ~80+ individual `w.Header().Set("Content-Type", "application/json")` calls across all handlers. | | `RespondJSON(w, status, data)` | Helper function (in `response.go`, not middleware) for consistent JSON responses. Always sets `Content-Type: application/json`. | -| `RespondError(w, status, message)` | Helper that wraps `RespondJSON` with `{"error": message}`. Replaces `http.Error()` in all new code for consistent JSON error format. | +| `RespondError(w, status, message)` | Helper that wraps `RespondJSON` with `{"error": message}`. **De-scoped for the payments package (Aug 2026 remediation):** the payments handlers deliberately keep using raw `http.Error` — the argument order differs (`http.Error(w, msg, status)` vs `RespondError(w, status, msg)`), the frontend only checks `res.ok` on those endpoints, and migrating ~396 call sites mid money-critical remediation added regression risk for zero functional gain. Recorded as a permanent MINOR/consistency backlog item; revisit only if the frontend starts reading structured error bodies from payments endpoints. Elsewhere, `RespondError` is the standard for consistent JSON error format. | ### Database Layer (`db/`) @@ -664,7 +664,7 @@ CORS uses `*` in local dev. In production behind Cloudflare, nginx handles CORS. All split records share the same `square_payment_id` so the refund loop can avoid duplicate Square API calls. After the booking start time, no split is applied — the charge records as a single payment with its original type. - **Concurrency guard:** `CreateBookingPayment` acquires a PostgreSQL session-level advisory lock (`pg_advisory_lock(hashtext('crussell:payment:' || booking_id))`) at entry and releases it in a defer. This serializes all payment attempts for the same booking — if two browser tabs try to pay simultaneously, the second request blocks until the first commits or rolls back. After the lock, the handler re-checks booking status (a concurrent payment may have promoted it) and runs a payment-type duplicate guard that prevents two `'full'` or `'deposit'` payments from being created for the same booking, even with different idempotency keys. + **Concurrency guard:** `CreateBookingPayment` acquires a PostgreSQL session-level advisory **try-lock** (`pg_try_advisory_lock(hashtext('crussell:payment:' || booking_id))`, via `acquireAdvisoryLock` in `handlers/payments/locks.go`) at entry and releases it in a defer. The lock is **bounded**: `tryAdvisoryLock` retries `pg_try_advisory_lock` ~30 times with a 100ms backoff (~3s total), and a contended second request gets a 409 "payment in progress, try again" instead of blocking a pool connection (blocking would hold the pinned connection hostage across the lock-holder's up-to-30s Square round-trip and could exhaust the pool). After the lock, the handler re-checks booking status (a concurrent payment may have promoted it) and runs a payment-type duplicate guard that prevents two `'full'` or `'deposit'` payments from being created for the same booking, even with different idempotency keys. 3. **Deposit deadline passes without payment** → `CleanupExpiredDeposits()` moves the booking to `pending_release`. The slot becomes vulnerable — another booking can claim it via eviction. An admin notification `deposit_not_paid_by_deadline` is created. The user's time blocker reservations are also cleaned up. 4. **Slot claimed by another booking** → If a new booking overlaps a `pending_release` slot, `EvictPendingReleaseOverlapping` (a shared function in `bookings.go`) evicts the pending_release booking to `deposit_lapsed`. The eviction runs inside the same transaction as the new booking's creation, so it rolls back if the new booking fails. The function is called by all 4 eviction sites: `CreateBookingHandler`, `ConfirmBookingHandler`, `AdminCreateBookingForUserHandler`, and `AdminRescheduleBookingHandler`. A `PAYMENT_IN_FLIGHT` time_blocker guard prevents evicting a booking that the user is currently paying for (5-minute window). @@ -1218,19 +1218,25 @@ Each test package has a `TestMain` that runs schema migration once per package ( Between tests, `TruncateTables()` runs `TRUNCATE TABLE ... CASCADE` on all tables. This is ~60% faster than DROP+CREATE. -### Payment Serialization Lock (advisory lock 1339) +### Payment Serialization Lock (bounded advisory try-lock) -`CreateBookingPayment` acquires a PostgreSQL session-level advisory lock (`pg_advisory_lock(hashtext('crussell:payment:' || booking_id))`) to serialize concurrent payment attempts for the same booking. This prevents the two-tab double-payment race where two browser tabs submit payments with different idempotency keys but the same payment type. +`CreateBookingPayment` acquires a PostgreSQL session-level advisory **try-lock** (`pg_try_advisory_lock(hashtext('crussell:payment:' || booking_id))`, via `acquireAdvisoryLock` in `handlers/payments/locks.go`) to serialize concurrent payment attempts for the same booking. This prevents the two-tab double-payment race where two browser tabs submit payments with different idempotency keys but the same payment type. **Key implementation detail:** The lock must be acquired and released on the **same** database connection. Using `db.DB.Exec()` for both would be unsafe — each call may get a different pool connection, and `pg_advisory_unlock` on a different session is a silent no-op, leaking the lock. The code uses `db.DB.Acquire()` to pin a dedicated connection for the duration of the handler, with `defer pinConn.Release()` ensuring the connection is returned when done. ```go pinConn, err := db.DB.Acquire(r.Context()) defer pinConn.Release() -pinConn.Exec(ctx, "SELECT pg_advisory_lock(hashtext('crussell:payment:' || $1))", bookingID) -defer pinConn.Exec(ctx, "SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))", bookingID) +lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:payment:"+bookingID) +if !lockOK { + http.Error(w, "Payment in progress, try again", http.StatusConflict) + return +} +defer pinConn.Exec(context.Background(), "SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))", bookingID) ``` +The lock is **bounded**: `tryAdvisoryLock` retries `pg_try_advisory_lock` up to `advisoryLockAttempts` (30) times with a 100ms backoff (`advisoryLockRetryDelay`), giving up after ~3s total. On timeout the handler returns a 409 "payment in progress, try again" rather than blocking. Blocking would hold the pinned pool connection for the full Square round-trip (up to ~30s) of whichever request holds the lock, and a handful of concurrent same-key requests would exhaust the whole pool (`max(4, numCPU)`) and hang the app. The deliberately-blocking variant (`pg_advisory_xact_lock`, unbounded) is used on ONE path only: the admin-cancellation path (`lockCancellationPayments`), where a bound would risk dropping a cancellation refund silently. See the comment block in `locks.go` for the full rationale. + After acquiring the lock, the handler re-checks the booking status (a concurrent payment may have promoted it) and runs a payment-type duplicate guard that prevents two `'full'` or `'deposit'` payments from being created for the same booking, even with different idempotency keys. The guard accounts for `buildSplitRecords` which converts `'full'` input to `'deposit'` + `'balance'` records. ### Cursor-Based Pagination Pattern @@ -1258,11 +1264,7 @@ Files with this pattern: `bookings.go` (4 handlers), `custom_services.go`, `user ### Advisory Lock Pattern (test infrastructure) -**Migration lock (1337):** `pg_advisory_lock(1337)` protects concurrent schema migration. When multiple test packages run simultaneously, only one executes the migration DDL at a time. - -**Truncation lock (1338):** `pg_advisory_lock(1338)` prevents CASCADE truncation deadlocks. When two tests try to truncate simultaneously, one waits for the other. - -**Implementation:** Both use `pool.Acquire()` for a dedicated connection (same pattern as the payment serialization lock above). +**Removed.** Test infrastructure no longer uses advisory locks. With per-package databases (each test binary gets its own `crussell_test_*` DB) and per-test transaction rollback, there is no cross-package or cross-test contention, so the old `pg_advisory_lock(1337)` migration lock and `pg_advisory_lock(1338)` truncation lock were deleted. `migratePool` runs on a fresh database created inside `CreateTestDatabase`, and `Migrate` runs once per package via `TestMain`. Production payment paths use the bounded try-lock in `handlers/payments/locks.go` instead (see "Payment Serialization Lock" above). ### Statement-by-Statement SQL Parser @@ -1355,23 +1357,23 @@ sequenceDiagram participant A as Admin participant F as Frontend participant B as Backend + participant DB as PostgreSQL participant S as Square API participant T as Terminal Device A->>F: Click "Take Payment" → Select Card (Terminal) - F->>B: POST /api/admin/bookings/{id}/payment + F->>B: POST /api/admin/bookings/{id}/payment (payment_type) + B->>DB: INSERT terminal_checkouts (provisional "tmp-" checkout_id, PENDING, payment_type) B->>S: Create checkout S-->>B: checkout_id - B->>S: Create terminal payment - S-->>B: terminal_payment_id + B->>DB: UPDATE terminal_checkouts (attach real checkout_id) B-->>F: 202 + checkout_id F-->>A: Show "Waiting for terminal..." T->>S: Customer inserts card B->>S: Poll GetCheckoutStatus S-->>B: COMPLETED + payment_ids - B->>DB: INSERT payment (status=completed) - B->>DB: Update booking status - B->>DB: Apply discounts (if completed) + B->>DB: Read payment_type from terminal_checkouts, INSERT payment (status=completed) + B->>DB: UPDATE terminal_checkouts status=COMPLETED B-->>F: SSE / Poll: payment complete F-->>A: Show "Payment successful" ``` @@ -1384,6 +1386,7 @@ sequenceDiagram participant F as Frontend participant B as Backend participant DB as PostgreSQL + participant S as Square API C->>F: Go to gift card purchase page F->>C: Enter amount, payment method @@ -1394,11 +1397,11 @@ sequenceDiagram DB-->>B: Return existing gift card B-->>F: 200 + existing else Key new + B->>DB: INSERT payment (status=pending) + VAT + B->>DB: COMMIT B->>S: Process payment (if online) S-->>B: Payment confirmed - B->>DB: INSERT gift_card - B->>DB: INSERT gift_card_transaction (purchase) - B->>DB: INSERT payment (if applicable) + B->>DB: UPDATE payment → completed + INSERT gift_card + gift_card_transaction (one tx) DB-->>B: OK B-->>F: 201 + gift card end @@ -1489,7 +1492,7 @@ sequenceDiagram ## Key Code Patterns -### Test Pattern: Advisory Lock +### Test Pattern: Migration ```go // testdb.go @@ -1500,14 +1503,10 @@ func runMigrations(pool *pgxpool.Pool) error { } defer conn.Release() - // Acquire lock on dedicated connection - _, err = conn.Exec(context.Background(), "SELECT pg_advisory_lock(1337)") - if err != nil { - return err - } - defer conn.Exec(context.Background(), "SELECT pg_advisory_unlock(1337)") - - // Run migration + // Run migration — no advisory lock: this always runs on a fresh + // per-package database created inside CreateTestDatabase, so there is no + // cross-package contention to serialize (the old pg_advisory_lock(1337) + // guard was removed). statements := splitSQLStatements(migrationSQL) for _, stmt := range statements { _, err = conn.Exec(context.Background(), stmt) diff --git a/obsidian/Crussell/Testing Architecture & DB Management.md b/obsidian/Crussell/Testing Architecture & DB Management.md index 3ce2381..e12263c 100644 --- a/obsidian/Crussell/Testing Architecture & DB Management.md +++ b/obsidian/Crussell/Testing Architecture & DB Management.md @@ -586,7 +586,7 @@ It's called inside `CreateTestDatabase`, which is called in `TestMain`. Since ea ### Q: Do I need to worry about advisory locks? -**No.** Advisory locks (`pg_advisory_lock`) have been removed from test infrastructure. With per-package databases and per-test transactions, there's no cross-package or cross-test contention. The single production use of `pg_advisory_lock` is in the payment handler (`handlers/payments/handlers.go:706`) for serializing concurrent payment attempts on the same booking — this is unrelated to test infrastructure. +**No.** Advisory locks (`pg_advisory_lock`) have been removed from test infrastructure. With per-package databases and per-test transactions, there's no cross-package or cross-test contention. Production payment paths use a **bounded try-lock** (`pg_try_advisory_lock`/`pg_try_advisory_xact_lock` via `acquireAdvisoryLock`/`acquireAdvisoryXactLock` in `handlers/payments/locks.go`, ~30 attempts × 100ms ≈ 3s bound) to serialize concurrent payment attempts on the same booking/key — this is unrelated to test infrastructure. The only deliberately-blocking variant (`pg_advisory_xact_lock`, unbounded) is the admin-cancellation path (`lockCancellationPayments`). ### Q: What's the `GO_TESTING` env var for? diff --git a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md index df18617..626ccbe 100644 --- a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md +++ b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md @@ -20,7 +20,7 @@ All 8 flows now render `SquareCardInput` (`frontend/src/lib/components/payments/ 1. `frontend/src/routes/tip/+page.svelte` — tip; saved-card list + `card_id`, SquareCardInput for new card (renders the shared `TipPayment.svelte` component) 2. `frontend/src/routes/pay-tip/[id]/+page.svelte` — tip; same pattern (also renders the shared `TipPayment.svelte` component) -3. `frontend/src/lib/components/account/UserBookingModal.svelte` — tip modal; same pattern +3. `frontend/src/lib/components/account/UserBookingModal.svelte` — tip modal; uses `CardSelection` directly (saved-card list + new-card toggle + SquareCardInput + card-save consent) — it does **not** render the shared `TipPayment.svelte` component 4. `frontend/src/lib/components/payments/UserPaymentModal.svelte` — booking payment; uses `CardSelection` with SquareCardInput 5. `frontend/src/lib/components/booking/BookingFlow.svelte` — deposit; saved-card list + `card_id`, SquareCardInput for new card (incl. guest flow) 6. `frontend/src/routes/account/+page.svelte` — Buy a Gift Card; saved-card list + `card_id`, SquareCardInput for new card