Known Issues

Found during the third /code-review round on fix/pos-terminal-item-category-required (POS-01/02/03) while tracing every write path that touches an item's category/group, not introduced by that PR. The catalogue detail page's 3-dot menu → "Move to category" → "Other categories" submenu (apps/terminal/src/app/(dashboard)/[facility]/menus/[id]/page.tsx, ~line 756-788, useBulkSetCategory) lets staff pick ANY category in the facility, including ones with no pos_catalogue_categories link to the item's current group. The mutation writes category_id with no check against catalogue_id at all. Reproduces the exact group/category mismatch bug class POS-01/02/03 fixed in the item-edit modal, via a different, older menu action that was never audited. Example: item is in Group Retail A / Category "Drinks A". Staff opens Retail A's catalogue detail page, 3-dot → Move to category → Other categories → picks "Snacks B" (only ever linked to Retail B). Save succeeds immediately, no warning. Re-editing the item shows Group=Retail A still selected, but item-sidebar.tsx's group-scoped category picker (filtered by useCatalogueCategoryIds(Retail A)) never highlights "Snacks B" — the item now silently holds an invalid pairing indistinguishable in the UI from "no category chosen", inviting an accidental category loss on the next edit.

Discovered in: code-review round 3 on fix/pos-terminal-item-category-required, 2026-09-29 (not a regression from that PR — pre-existing, in a different code path)

Found during the same third /code-review round as BUG-113, same discovery context (auditing all category/group write paths, not introduced by that PR). The items list's 3-dot menu → "Move to catalogue" action (apps/terminal/src/app/(dashboard)/[facility]/items/page.tsx ~line 619, useMoveItemToCatalogue in use-item-mutations.ts ~line 960) only writes pos_catalog_items.catalogue_id. The actual catalogueId->pos_item_catalogues junction-table query lives in apps/terminal/src/queries/use-catalog-items.ts (~lines 64-73), called from apps/terminal/src/components/terminal/menu-grid.tsx (~lines 1038/1054) for every group tab on the till — this action never touches that junction table. Example: item is in pos_item_catalogues under Retail A (position 0). Staff uses "Move to catalogue" → Retail B. catalogue_id becomes Retail B in the DB, but pos_item_catalogues still only has the old Retail A row — no new Retail B row is added. The item keeps selling under Retail A's tab on the actual till grid and never appears under Retail B, while re-opening the item's edit modal shows "Retail B" as the selected Group (since the modal reads catalogue_id), directly contradicting what staff see on the floor.

Discovered in: code-review round 3 on fix/pos-terminal-item-category-required, 2026-09-29 (not a regression from that PR — pre-existing, in a different code path)

DSC-01/DSC-02: apps/pos's discount modal 'Customer group pricing' picker (discount-modal.tsx, via queries/use-customer-groups.ts's useCustomerGroups()) queried the legacy, admin-UI-less pos_customer_groups table instead of crm_customer_groups — the real source of truth, created via apps/admin's CRM ([facility]/crm/groups). apps/terminal already read crm_customer_groups correctly for group CRUD + membership; apps/pos was left behind mid-migration when the Centralized Discount Engine (BUG-082, v0.24.0) introduced the crm_ tables — the legacy pos_customer_groups/pos_customer_group_members tables predate it and were never migrated off. The landmine that made this more than a one-line fix: pos_discount_group_variations (the table group-based price overrides save into, via useSaveDiscountGroupVariations) had a live FK on customer_group_id pointing at pos_customer_groups(id), not crm_customer_groups(id). Fixing only the picker would have let staff select a real crm_ group and then hit a 23503 FK violation on Save — a worse regression than the original stale-list bug. Confirmed apps/terminal was ALREADY exposed to this exact write-side bug (its group CRUD was correct, but useSaveDiscountGroupVariations shared the same broken FK) — any attempt to save a group-price override against a PMS-created group in Terminal would already have failed silently (see next paragraph). Second, independently-discovered bug in the same function (both apps): useSaveDiscountGroupVariations's delete/update/insert calls never checked their Supabase `{ error }` result — a failed write (including the FK violation above) was silently swallowed. The UI showed success (modal closed) while the override was never actually saved. Fixed alongside the main bug (CLAUDE.md 'No silent errors' — STRONG BIAS). Third, independently-discovered RLS gap: crm_customer_groups_read / crm_customer_group_members_read (20260506094046_centralized_discount_engine.sql) only allow role IN (owner, admin, manager, coach, member) — no 'staff', which is a real, commonly-assigned POS/Terminal till-operator role (apps/pos/src/queries/use-team.ts's FacilityRole list). The legacy tables' equivalent policies (via pos_operator_facility_ids()) DO include staff. Confirmed empirically against local Supabase by simulating an RLS-scoped SELECT as a 'staff'-role user: pos_customer_groups readable, crm_customer_groups NOT (0 rows). Without also fixing this, repointing the picker at crm_customer_groups would have traded 'shows the wrong groups' for 'shows zero groups' for any facility whose till operators log in as 'staff'. 'cashier' remains unable to read either table (pos_operator_facility_ids() itself excludes cashier — separate, pre-existing, already-documented gap per 20260928102157_restore_cashier_to_facility_tickets_write.sql's own note; not worsened or fixed by this change).

Discovered in: prod-adjacent investigation 2026-09-29 (fix/pos-discount-groups-crm-source)

Reported live against a real check-in screenshot: 'this is still happening ... whenever the court is attached like that then when i add other items it looks the price to the right' — the Court E card showed no price once another item sat in the cart. Root cause: cart-sidebar.tsx's isBooking render branch replaced the price with `#{bookingId.slice(0,8)}` (a booking-ref code) whenever the booking was unpaid — the normal cash/pay-at-club case — instead of showing the amount owed. checkout-dialog.tsx's equivalent card had already been fixed to show the real price under AC-9 (ac-checkin-pos-court-fee-pricing.ts); this second copy of the same rendering logic was never updated to match. Separately: 'i want that the court when i click on it it also allows me to split or move, not just at the end when charging' — the booking card had no onClick at all in the main cart; the div returned by its isBooking branch simply ignored the onClick prop passed in.

Discovered in: prod report 2026-08-15 (screenshot against a real Pad Thai Padel coach check-in, Court E)

Reported live: 'the bookings tab it tries to load fully all the bookings then it times out and nothing shows we need pagination for that'. usePmsBookings() was already scoped to the active date window (BUG fixed under PR #986) and its per-booking enrichment lookups parallelized, but the fetch had no upper bound — a custom range spanning weeks/months on a busy facility pulled every matching booking (plus every enrichment query batched off that full set) in one request.

Discovered in: prod report 2026-08-14

Reported live: 'i cannot split the court by 4 ... i tried to do it and still it is disappearing'. Selecting the Court Booking line item in Split Bill → By items defaults its action to 'Move to ticket' (curAction = itemAction[id] ?? "move"), not 'Split cost'. For a booking item, Move fully detaches the cart_json line from the ticket that's actually linked to the booking (facility_tickets.booking_id) and writes it wholesale onto the destination ticket instead — the court fee vanished from the organizer's (booking-linked) ticket entirely, while an unrelated ticket silently absorbed the full amount, with no warning and no on-screen indication of where it went. 'Split cost' (the other action) worked correctly the whole time — the bug was that Move was ever offered/defaulted-to for a booking item at all.

Discovered in: prod report 2026-08-14, reproduced via Playwright against local Supabase (moveEntries path specifically, not splitEntries)

Reported live against a real Pad Thai Padel booking (guest chose cash/pay-at-club from TPS): clicking Mark-as-Paid in admin showed 'External payment — local change only — This booking was paid via an external system', which was false. Root cause: protocol/server/src/db/bookings.rs::confirm flipped bookings.payment_method to 'external' whenever the OpenRally payment block carried an external_ref — but external_ref is a REQUIRED field on every payment block per the OpenRally spec, including method_type='pay_at_venue' (TPS's cash option). method_type itself was never consulted, so any payment block at all — settled or not — produced the same tag. A test in protocol/server/tests/payment_block.rs asserted this as correct, with a comment describing the OPPOSITE intended behavior ('Setpoint admin sees pending and knows it's a venue-collect'), so the wrong assertion was locking the bug in rather than catching it.

Discovered in: prod report 2026-08-14 (Pad Thai Padel, Alba Garrido test booking, Court A)

Found investigating BUG-107 (pay_at_venue mistagged 'external' on court bookings, fixed same day in protocol/server/src/db/bookings.rs::confirm). The court-booking fix does NOT cover the lesson/coaching side: confirm_lesson_from_hold (supabase/migrations/20260515084759), confirm_session_seat_from_hold (20260616000100), and confirm_group_lesson_seat_from_hold (latest definition in 20260619000000) each independently re-derive bookings.payment_method from the OpenRally payment block instead of sharing one function — the same fragmentation pattern already found and fixed on the terminal check-in pricing side this same day (DEC-291/292/297). confirm_lesson_from_hold and confirm_session_seat_from_hold appear to use the same 'ref present → external' pattern the court path had (unconfirmed — not read in full during this pass). confirm_group_lesson_seat_from_hold is worse and confirmed: `payment_method = CASE WHEN p_payment_status = 'paid' THEN COALESCE(p_external_payment_method_type, 'stripe') ELSE 'pending' END` writes the RAW method_type string (e.g. 'pay_at_venue', 'comp', 'credit') directly into payment_method whenever payment_status='paid' — but payment_method's CHECK constraint only allows stripe|xendit|credits|pending|other|package|external. 'credit' (protocol singular) isn't 'credits' (Setpoint plural); 'comp'/'pay_at_venue' aren't valid at all. A group-lesson booking confirmed with payment_status='paid' and method_type IN ('comp','credit','pay_at_venue') will hit a live CHECK constraint violation on write, not just a display bug.

Discovered in: code review while fixing BUG-107, 2026-08-14

Reported from the consumer app at Pad Thai Padel for 2026-08-12: the coaching picker offered Jose Mouliaa at 15:00 and 16:00 on The Padel Society; tapping Book returned 'OpenRally lesson-hold failed (409): slot_unavailable'. He was teaching 15:30–17:00 on Padel Galis at the time. Root cause: GET /openrally/v1/facilities/{f}/coaches/{c}/availability derived each slot's status from the `bookings` table alone, matched on the slot's OWN court. A coach reserved on N courts emits N parallel slots per instant (Jose held 09:00–17:00 windows on all three Pad Thai courts), and a lesson's booking row lands on ONE court — so the other N-1 kept reading 'available' while the coach was mid-lesson. Same-court lessons were caught only incidentally, because create_lesson_with_bookings / hold_lesson_slot happen to insert a linked booking on that court. The handler's own header comment claimed it considered lessons ('booked — a confirmed/checked_in booking or scheduled lesson overlaps') but the SQL never touched the `lessons` table. hold_lesson_slot has always disagreed: it checks coach lesson overlap BEFORE courts and raises 'This coach already has a lesson at this time.', which map_hold_error turns into 409 Conflict 'Slot is already held or booked'. So the 409 users saw was the coach guard, never the court guard. The admin twin (apps/admin/src/app/api/v1/facilities/[slug]/lessons/available/route.ts, `coachBusy`) already did this correctly — OpenRally was the sole divergent surface.

Discovered in: prod report 2026-08-10 (Pad Thai Padel, coach Jose Mouliaa, 2026-08-12)

Recurring reception report at Pad Thai Padel (latest: Royal Padel, 10:00–12:00): the admin DAY calendar showed a court a coach genuinely holds as FREE; reception booked it and the authoritative DB write-gate then rejected with 'This court is reserved for a coach at this time' — a confusing read/write disagreement. Root cause: coach availability_exceptions carry an optional court_id (DEC-237, v0.30.0) where NULL = the time-off applies to ALL the coach's reserved courts and a UUID = that ONE court only. Several surfaces subtracted exceptions WITHOUT honoring that court scope — hand-rolled copies of the interval-subtraction logic that had drifted from the shared availabilityWindowsForDate helper and never read court_id — so a time-off scoped to court B wrongly carved court A's availability. Effects: the day calendar mis-showed a held court as free (the reported screenshot); the lesson slot pickers (available-slots, lessons/available) hid or over-rejected genuine slots; the lesson create/edit/generate friendly gates could false-reject ('instructor not available') when the lesson used a different court than the time-off. The known-correct reference twin (availability/route.ts) already loops per court passing courtId; the affected surfaces had diverged from it.

Discovered in: prod report 2026-06-05 (Pad Thai Padel, Royal Padel)

Reported from prod (Pad Thai Padel, Padel Galis, 2026-06-10): a coach was reserved 11:00–14:00 (strict); the operator removed 11:00–12:00 (a windowed availability_exception), so the coach genuinely held only 12:00–14:00. A 10:00–12:00 court booking — which does NOT overlap 12:00–14:00 — was rejected with 'This court is reserved for a coach at this time.' Root cause: the court_booking branch of check_booking_against_coach_reservations (origin 20260527073858, body in 20260527141545) lifted a strict reservation only when a SINGLE windowed exception FULLY CONTAINED the whole booking (numrange(ae) @> v_lesson_range). The booking starts at 10:00, before the 11:00–12:00 exception, so [11:00,12:00) does not @> [10:00,12:00) → block not lifted. The four other paths (admin calendar subtractTimeRanges, public availability availabilityWindowsForDate, OpenRally coach_blocked_intervals_for_facility, and the lesson_court branch) all do correct half-open interval subtraction, so the calendar showed the slot free while the write-gate refused it — and because the bookings trigger is origin-agnostic, a native TPS booking would hit the same false rejection despite OpenRally advertising the slot bookable. Mis-triage note: this @> behavior was previously examined (coach-exception-interval-fix-plan.md §3a/§6.1, DEC-256 follow-ups) and dismissed as 'errs SAFE — over-blocks a customer booking, never orphans a court, NOT a bug,' with an optional 'range_agg(...) @> v_lesson_range' hardening. That was wrong on both counts: the over-block makes a legitimate booking impossible, and the range_agg @> hardening does NOT fix the leading-edge case ({[11:00,12:00)} @> [10:00,12:00) is still false).

Discovered in: prod report 2026-06-04 (Pad Thai Padel)

PR #705 colors booking cells by string-concatenating an alpha suffix onto the court color: `style={{ backgroundColor: court.colorCode + "26" }}` in calendar-grid.tsx (day), week-calendar-grid.tsx, and month-calendar-grid.tsx, plus the court-form-dialog.tsx preview (`+ "26"`) and operations-settings-tab.tsx color previews (`+ "33"`). This only yields a valid 8-digit #RRGGBBAA color when colorCode is EXACTLY 6 hex digits. There is no DB CHECK on courts.color_code (plain nullable text) and the court form saves the value unconditionally (`color_code: colorCode || null`); the `/^#[0-9A-Fa-f]{3,6}$/` regex added in this PR only gates the swatch/preview JSX, NOT the save. So a stored 3- or 5-digit value (e.g. `#abc` → `#abc26`, `#abc12` → `#abc1226`) is an invalid CSS color → the cell renders with no/transparent background, AND the sport-color fallback is simultaneously suppressed because the className uses `!court.colorCode && sportColorBg(...)` and `!court.colorCode` is false for a non-empty string → silent blank cell, no error logged. A 4-digit value like `#FFFF` → `#FFFF26` is a VALID but WRONG 8-digit color (silently mis-rendered). Exposure is bounded: the 12 preset swatches are all 6-digit, so the picker default path is safe; the bug only manifests via the free hex input or any pre-existing non-6-digit color_code (the column existed before this PR but was unused in calendar views, so legacy values were never rendered/validated).

Discovered in: PR #705 review (calendar month-view + court-color coding, 2026-06-04)

In the week calendar, each day header rendered a per-court name chips strip (one truncated court-name label per lane, shown only in all-courts mode, carrying `data-testid="week-court-chips-{date}"`). PR #705 ("feat(calendar): month view + court color coding", merged as commit 61bc7c53) deleted that block entirely from apps/admin/src/components/admin/week-calendar-grid.tsx (the `isAllCourts && courts.length > 0` header block). E2E apps/admin/e2e/tc-145-week-view-all-courts-per-lane.e2e.ts hard-depends on it: it does `getByTestId("week-court-chips-{date}")` then an UNCONDITIONAL `expect(...).toBeVisible({ timeout: 10_000 })` plus `toContainText("Court A"/"Court B"/"Court C")` (the only `.catch` is on a loop guard, not the assertions), so removing the element makes tc-145 fail deterministically → CI is red on main. Re-verified still-broken on origin/main after the #705 / #708–#711 merges (chips strip still absent, tc-145 still asserts it, no test.skip). The removal also silently dropped a documented shipped feature (apps/docs/src/data/versions.ts ~line 1153 — "Day-header chips strip … data-testid='week-court-chips-{date}'", done: true). The PR's 2nd commit message ("fix(calendar): remove court dots from week view header") MISLABELS the change: the deleted element renders court NAMES, not dots, and the diff adds no dots — this looks accidental, not an intended retirement.

Discovered in: PR #705 review (calendar month-view + court-color coding, 2026-06-04)

get-courts-v2 builds each time-chip's `available_durations` from the Upstash `court_combinations:<club>:<date>` hash, but falls back to `[60, 90, 120]` when the hash lacks the frame (index.ts ~542-550). For OpenRally clubs (platform_name='setpoint') that hash is not populated, and OpenRally availability is only fetched at duration=60 (index.ts ~302), so EVERY 60-min chip is advertised as also bookable at 90 and 120 min — even chips whose adjacent base slot is already booked. Confirmed live (Pad Thai Padel, club 673 / facility 2f3ad3e1) 2026-06-03: all 60-min chips returned available_durations=[60,90,120], while the authoritative OpenRally server reported only a subset bookable at the longer durations (06-04: 20 slots at 60min, 10 at 90min, 4 at 120min). The 60-min availability is always correct (matches OpenRally exactly).

Discovered in: Production read-only availability audit 2026-06-03

GET /api/admin/crm/customers fetched the entire `customers` table with no .limit/.order and no facility filter. PostgREST's max_rows=1000 capped the response in arbitrary heap order, and the route then filtered/sorted/paginated in JavaScript on the truncated array. On Pad Thai Padel prod (4,195 customer rows), this silently dropped ~3,195 rows on every load (~76%). Any customer past row 1000 — for example Alba Garrido (albagm2206@gmail.com), who sat at heap position 4,195/4,195 — was unreachable: no search term, filter, or sort could surface them, because the search predicate ran AFTER truncation. Secondary symptom: the global-scope fetch also leaked customers from other facilities into a facility-scoped admin's CRM (small effect on Pad Thai prod — 24 of the truncated rows were non-facility — but a privacy issue in principle). Booking-creation player lookup (/api/admin/players/search) was unaffected: it filters .ilike server-side so the cap fires after the predicate. That's why operators kept finding Alba when adding her to a booking even though she was invisible in CRM.

Discovered in: Production triage 2026-05-26 (customer report: 'Alba's profile doesn't appear in PMS CRM')

The reverse-direction gate (v0.28.0, DEC-228/DEC-229, migration 20260522082857) blocks creating a coach reservation that overlaps an existing booking and lists every conflict. For consistency between its two layers it intentionally does NOT subtract availability_exceptions: the per-row backstop trigger fires on coach_availabilities INSERT, which the coach_schedule_replace RPC runs BEFORE it inserts the same payload's exceptions — so the trigger cannot see them. Making only the route-level enumerate fn exception-aware would let the route pass while the trigger rejects (self-inflicted inconsistency), so BOTH layers ignore exceptions. Effect: if an operator sets weekly availability AND a same-day exception (coach unavailable that date) AND a booking exists on that excepted date+time, the gate still lists that booking as a conflict even though the reservation wouldn't actually block it. This only ever OVER-flags — it never misses a real conflict, so no orphan overlap can slip through.

Discovered in: v0.28.0 build (coach-availability booking-conflict gate)

When an OpenRally coaching lesson is cancelled — by a TPS user in-app OR a Setpoint admin — the TPS-side cancel runs through handleExternalCancellation → cancel-booking-v1, which cancels bookings + booked_slots + booking_third_party_references and sends the cancellation email, but does NOT touch coach_reservations (cancel-booking-v1 has zero references to that table). The native coaching-cancel function (padel-backend cancel-reserve-coach) only cancels the Setpoint side via OpenRallyCoachService.cancelLesson for OpenRally lessons — its coachService.cancelReserveCoach call lives in the legacy/non-OpenRally branch — then relies on the returning lesson_cancelled webhook → cancel-booking-v1 for the TPS rows. So BOTH cancel origins leave coach_reservations='confirmed'; it is pre-existing OpenRally-coaching behavior, not a regression from the PR #211 dual-key fix. Verified live 2026-05-22 on Sandbox booking 60143: after admin cancel, booking + booked_slot + btpr = cancelled and the email sent, but coach_reservation stayed confirmed. Confirmed harmless to the player — the cancelled lesson correctly disappears from the app coaching tab (gated on booking/slot status, not coach_reservation).

Discovered in: Setpoint-TPS OpenRally lesson cancel verification (2026-05-22, Sandbox booking 60143)

TPS prod booking_third_party_references has only pkey(id) + FK(booking_id) — NO unique index on (platform_name, third_party_reference) (verified 2026-05-22). The openrally-lesson-webhook handler (padel-backend/supabase/functions/openrally-lesson-webhook/handlers.ts) has a 23505-conflict branch that rolls back + acks on a concurrent duplicate insert, but with no unique constraint that branch can never fire — it is a latent fallback. Today the real dedup is the findLessonBtpr READ-guard at the top of handleLessonCreated, which catches the common sequential re-delivery. The gap: a truly concurrent lesson double-emit (BUG-090), where both deliveries run the read-guard before either commits, could create two reverse bookings. Fix: add a UNIQUE index on (platform_name, third_party_reference) — but FIRST find + resolve any existing true-duplicate rows (same platform_name + third_party_reference) or the CREATE INDEX will fail. Adding the index also activates the existing 23505 backstop. TPS-side schema change (padel-backend), via migration.

Discovered in: Code review of the OpenRally lesson dual-key fix (PR #211, padel-backend, 2026-05-22)

When a lesson is created or cancelled on the Setpoint side, two or_events rows are written ~25-75ms apart. Verified live 2026-05-22: lesson 07fb2500 produced or_events 1670+1671 (lesson_created) and 1672+1674 (lesson_cancelled); all four fanned out to webhook_deliveries and POSTed to the TPS openrally-lesson-webhook, all succeeded. Harmless today because the TPS handler is idempotent: handleLessonCreated's findLessonBtpr guard acks the 2nd delivery (tps_owns_creation / already_written, no duplicate booking) and handleLessonCancelled's already-cancelled check no-ops the 2nd (no double email) — proven live (1 booking, 1 email despite 2 events each). Root cause is Setpoint-side: likely a duplicate trigger or a hold→confirm double-fire on the lessons table. Investigate emit_lesson_status_event / the lesson lifecycle triggers in setpoint/supabase/migrations/.

Discovered in: Setpoint-TPS OpenRally lesson integration verification (2026-05-22, Sandbox booking 60143)

The new TPS OpenRally club 'New Pad Thai Padel' (TPS courts.id=673, linked via clubs_3rd to Setpoint facility 2f3ad3e1 'Pad Thai Padel') was brought up with a single payment method — 'Pay at club' (manual; payment_methods.id=2; one row in club_payment_methods) — mirroring the Sandbox club so booking confirmation works without any online payment processing. The real production club 'Pad Thai Padel' (TPS courts.id=388) has three: Credit Card (active), Pay at club (active, default), and QR Code (configured but inactive). Before club 673 can serve as / replace the production club, it needs Credit Card (and optionally QR Code) added. CAUTION: Credit Card and QR Code require working online payment processing (Stripe for cards, PromptPay for the QR). Adding those club_payment_methods rows WITHOUT that processing configured will reproduce the 'cannot confirm payment' failure on those paths — exactly the gap that left 673 unbookable until 'Pay at club' was added on 2026-05-22.

Discovered in: TPS 'New Pad Thai Padel' club bring-up (2026-05-22)

`apps/admin/src/components/admin/lesson-form-fields.tsx:345` reads `data.schedules` from the response of GET /api/admin/coaches/[id]/schedule. The endpoint (`apps/admin/src/app/api/admin/coaches/[id]/schedule/route.ts:71-74`) actually returns `{ availabilities, exceptions }` — the field is called `availabilities`, not `schedules`. Result: `schedules` is always undefined → `schedules.length === 0` → `setHasSchedules(false)` → the amber 'No schedule set — configure in Academy first' warning fires below the Coach select for EVERY coach, regardless of whether they have a configured schedule. Cosmetic-only: the booking system itself works correctly because the 'Available Times' slot grid is fetched from a different API (lesson slot materialization at `/api/v1/facilities/[slug]/lessons/available` or the equivalent calendar endpoint) that reads the actual coach availability server-side. Operators can ignore the warning and proceed with the booking — they just see a misleading message. Secondary effect: the `allCoachSchedules` state (line 346) is also empty for the same reason, which means the 'Coach available 09:00–17:00 on Tuesdays' label below the Date picker (lines 762-765) NEVER renders — operators lose the day-availability hint. Root cause: the endpoint was refactored to return a coalesced shape (`{ recurrence_type, recurrence_config, start_time, end_time, court_ids[] }`) keyed by `availabilities` — but the lesson form was never updated. The OLD response shape (a flat array of `{ day_of_week, start_time, end_time, court_id }`) is what `lesson-form-fields.tsx:360-365` still expects when filtering by `s.day_of_week === dayOfWeek`. So even if the field name were fixed, the shape mismatch would still break the day-availability filter. Fix path: 1. In `lesson-form-fields.tsx:345`, change `data.schedules || []` to `data.availabilities || []`. 2. Update the `allCoachSchedules` state shape (line 186) and the day-filter logic (line 361) to handle the coalesced shape: - Each `availability` row has `recurrence_type`, `recurrence_config`, `start_time`, `end_time`, `court_ids[]`. - For weekly recurrence: `recurrence_config.days_of_week` is the array of day numbers (0=Sun..6=Sat). Replace `s.day_of_week === dayOfWeek` with `s.recurrence_type === 'weekly' && s.recurrence_config.days_of_week?.includes(dayOfWeek)`. - For other recurrence types (single_date, date_range, monthly): use the existing `appliesOnDate` helper from `@setpoint/common/coach-availability` to evaluate whether the row applies to the selected date. 3. Test: pick a coach with a configured schedule (Miguel in Pad Thai Padel demo data), open the Create Booking dialog → Lesson tab → select Miguel → verify the 'No schedule set' warning does NOT appear AND the 'Coach available HH:MM-HH:MM on Xdays' label DOES appear below the Date input. Same bug surface likely exists in `lesson-form-dialog.tsx:736` and `program-form-dialog.tsx:412` (other surfaces that render the same 'No schedule set' warning string). Worth fixing all three in one PR. Why deferred: cosmetic, doesn't block any booking. Surfaced today (2026-05-18) during the v0.27.2 reverse-flow verification when the operator noticed Miguel showed the warning despite having a schedule. Filed for the next admin UI cleanup pass.

Discovered in: v0.27.2 (DEC-226 verification — operator noticed during reverse-flow test)

`mobile-app-padel/lib/features/profile/controllers/profile_coaching_controller.dart:43-66` (`getMyCoachingSessions` and `getMyRecentSessions`) has try/catch blocks that only call `errorHandler(error)` inside `if (kDebugMode)`. In release builds (TestFlight, App Store), any exception thrown during the Supabase query, JSON decoding, or model deserialization is silently swallowed — the RxList remains empty, the screen renders 'Book your first session', no log surface, no crash report. Today (2026-05-18), this hid a `Coaching.fromJson()` throw on `expiry_time=null` (the model declares `final DateTime expiryTime` as non-nullable required) — a real-device iPhone test showed the empty state while the underlying DB row was present and the PostgREST query returned it. Diagnosis required running the exact mobile query via curl + reading the model file to find the required field — neither of which a normal operator could do. Same pattern likely exists in other controllers across the codebase (default GetX template). Worth a sweep. Fix path: 1. Replace `if (kDebugMode) errorHandler(error)` with always-on logging: at minimum `print('[ProfileCoachingController.getMyCoachingSessions] error: $error')` so the failure shows up in iOS device logs. 2. Better: forward to Firebase Crashlytics via `FirebaseCrashlytics.instance.recordError(error, stackTrace, reason: 'getMyCoachingSessions failed')` so production errors aggregate centrally. Crashlytics is already initialized in main.dart. 3. Best: introduce a Rx error state on the controller and render an error banner in `profile_coaching_screen.dart` (instead of falling back to empty state) so users see 'Something went wrong, please retry' rather than 'Book your first session' — which is actively misleading when a booking exists. Acceptance criteria: • Throw an exception inside `getMyCoachingSessions` (e.g., stub the repository to throw). Run the app in release mode. Verify the error surfaces (logs OR Crashlytics OR UI banner) instead of silently rendering the empty state. • Grep the codebase for `if (kDebugMode)` wrapping catch blocks. Audit each for whether silent-fail-in-release is the right behavior (usually not).

Discovered in: v0.27.1 (DEC-226 — coach booking native-equivalence)

The OpenRally webhook dispatcher (`protocol/server/src/services/webhook_dispatcher.rs`) running on Fly.io is supposed to pick up rows from `or_events` and POST them to subscribed endpoints (creating a `webhook_deliveries` row per attempt). Observed today and in prior sessions: some events are emitted into `or_events` correctly, but no `webhook_deliveries` row is ever created — the dispatcher silently skips them. The TPS-side webhook never fires, so reverse-flow lessons don't reach the TPS database without manual intervention. Today's evidence: • `or_events` row id=260 (event_type='lesson_created', entity_id='or://setpoint/lesson/2bc4bf1b-486b-47aa-8f04-056caa6e45ee') created at 2026-05-18 07:39 UTC. • `openrally_webhook_endpoints` had a valid active row pointing at the TPS Edge Function URL (last_delivered_at 2026-05-17 11:21 — endpoint was working ~1 day prior). • No `webhook_deliveries` row was ever created for event 260. • The TPS Edge Function works perfectly when invoked directly (manually POSTing the event payload returned `{ok:true,written:true,bookingId:59680}` with all 5 native rows landing correctly). Prior session note (from compaction summary): 'OpenRally dispatcher flakiness: After fly.io redeploy, only one auto-delivery worked. Pattern: 2 of 3 lessons created today needed manual delivery.' So the issue has been intermittent for at least 2 days. Code-level root cause (confirmed by reading protocol/server/src/services/{event_fan,webhook_dispatcher}.rs): • `event_fan.rs` opens a single `PgListener::connect_with(pool)` and `LISTEN or_events_channel`. On any error it sleeps 1s and reconnects (event_fan.rs:12-17). **During the reconnect window, every NOTIFY fired by the trigger is LOST PERMANENTLY** — PostgreSQL does not buffer notifications for disconnected listeners. There is no replay or persistence mechanism for missed notifications. • The dispatcher uses a `tokio::sync::broadcast::channel(1024)` (main.rs:55) — capacity is fine. • There is NO fallback polling task. The system is 100% dependent on the LISTEN connection being uninterrupted. Any process restart on Fly.io (deploys, OOM, machine moves) drops some events. • The dispatcher does NOT write to `webhook_deliveries` — it only UPDATEs `openrally_webhook_endpoints.last_delivered_at`. So `webhook_deliveries` row count is irrelevant to diagnosis; check `openrally_webhook_endpoints.last_delivered_at` instead. Proposed code-level fix (clean, scoped): 1. Migration: ADD COLUMN `or_events.dispatched_at TIMESTAMPTZ NULL` (nullable, no default — null = not dispatched). 2. webhook_dispatcher.rs: after a successful 2xx response on `deliver`, also UPDATE or_events SET dispatched_at = NOW() WHERE id = $1. Idempotent. 3. New service `services/dispatch_recovery.rs`: every 60s, SELECT id FROM or_events WHERE dispatched_at IS NULL AND created_at < NOW() - INTERVAL '30 seconds' ORDER BY id LIMIT 100. For each, fetch the event via `db::events::get_by_id` and call `event_tx.send(event)` (same broadcast channel the listener uses). Wire it into `main.rs` alongside event_fan + webhook_dispatcher. 4. After the migration backfills dispatched_at for all existing events that ARE delivered (a one-shot UPDATE setting dispatched_at = openrally_webhook_endpoints.last_delivered_at OR a safe-default like 'now() - INTERVAL 1 hour' for events older than 1 hour), deploy via flyctl. Why this fix: • Guarantees no event is lost regardless of LISTEN reliability — the poll catches anything the listener misses. • 30s lag for missed events is acceptable (compared to permanent loss today). • The poll is bounded: only 100 events per cycle, indexed scan via `WHERE dispatched_at IS NULL ORDER BY id`. Default LISTEN remains the fast path; poll is the safety net. • Migration is non-destructive (adds nullable column, no data migration required for correctness). Workaround (manual delivery): ``` # 1. NOTIFY the channel to nudge the dispatcher: PSQL: NOTIFY or_events_channel, '<event_id>'; # 2. OR direct POST the payload to TPS Edge Function (bypass dispatcher): curl -X POST 'https://tnyqbaqnugjgefkdvmag.supabase.co/functions/v1/openrally-lesson-webhook' \ -H 'Content-Type: application/json' \ -H 'x-openrally-event: lesson_created' \ -H 'x-openrally-event-id: <id>' \ -H 'apikey: <SUPABASE_KEY>' \ -d '<envelope JSON>' ``` Fix path: 1. SSH into Fly.io app `openrally-server` and inspect dispatcher logs around a known-dropped event to identify the failure mode. 2. Add a fallback poll (e.g., every 60s scan `or_events` rows that don't yet have a `webhook_deliveries` entry and dispatch them) so missed NOTIFYs don't lose events. 3. Add a health endpoint that surfaces 'pending events count' so this can be monitored proactively. 4. If the dispatcher uses LISTEN/NOTIFY, add reconnect-on-drop logic and verify LISTEN survives across deploy restarts. Why deferred: doesn't block the v0.27.1 patch shipping (the writeback code is correct and deployed; manual delivery is a viable stopgap). But every reverse-flow lesson in production requires manual intervention until this is fixed, which doesn't scale. Should be the next OpenRally infra task.

Discovered in: v0.27.1 (DEC-226 verification — fresh reverse-flow test on 2026-05-18)

There are four distinct code paths that INSERT into the `lesson_players` table on the Setpoint side. Each was written independently. The risk is structural: any new 'create a lesson' path added in the future may forget to insert lesson_players — which is exactly how the v0.26.0 'OpenRally lessons show empty roster' bug happened (the OpenRally confirm_lesson_from_hold RPC was added without inserting into lesson_players, so the admin UI rendered `—` / `0/max` for every TPS-originated lesson). That bug was fixed in migration 20260515084759 but the architectural smell remains. The four current write paths: 1. `create_lesson_atomic(...)` RPC — supabase/migrations/00000000000000_initial_schema.sql:~1421. Used by admin native lesson creation (POST /api/admin/lessons) for both single-player and group lessons. Reads from p_primary + p_players jsonb args. 2. `confirm_lesson_from_hold(...)` RPC — added in supabase/migrations/20260515084759_openrally_confirm_lesson_inserts_player.sql. Used by the OpenRally Rust handler after a TPS user confirms a coach booking. Reads player fields from the just-UPDATEd bookings row. Idempotent via NOT EXISTS guard. `is_primary=TRUE` always (OpenRally is 1-on-1 by design per DEC-207). 3. `apps/admin/src/app/api/[facility]/lessons/[id]/confirm/route.ts` — public web 'join this lesson' endpoint. Inserts a row when a guest user confirms attendance to an existing lesson (used by the player-facing lesson-confirm flow). 4. `apps/admin/src/app/api/[facility]/bookings/[id]/link-user/route.ts` — admin-side helper that links a user account to a previously-created lesson booking. Also inserts into lesson_players. Why the four diverged: each path arrived in a different release for a different use case; the input shapes are subtly different (RPC jsonb args vs API request bodies vs DB-trigger reads from caller row vs no-input link-by-user-id), and there's no shared helper. Until v0.26.0, nobody noticed they could drift — the OpenRally path drifted silently for ~2 weeks until the admin UI surfaced 'roster shows —'. Fix path (architectural — defer unless 5th path is added or two existing ones diverge again): 1. Define a shared SQL helper RPC, e.g. `ensure_primary_lesson_player(p_lesson_id uuid, p_user_id uuid, p_email text, p_phone text, p_first_name text, p_last_name text, p_payment_method text) RETURNS lesson_players`. Idempotent via the same `NOT EXISTS` guard pattern as the current 20260515084759 fix. 2. Convert each of the 4 paths to call this helper. The RPC paths (`create_lesson_atomic`, `confirm_lesson_from_hold`) can `PERFORM ensure_primary_lesson_player(...)` mid-transaction. The TS-side paths (`/lessons/[id]/confirm`, `/bookings/[id]/link-user`) call it via supabase.rpc(...). 3. The shape difference is the deepest issue: `create_lesson_atomic` takes a jsonb p_primary AND a jsonb p_players[] (multi-player groups), while the other three are single-player. The helper handles a single player only; for the multi-player case in create_lesson_atomic, iterate p_players and call the helper per row. is_primary becomes a 5th argument with a default. 4. Test seam: TC-130 (already exercises lesson_players writes through the admin booking path) + TC-143 (exercises OpenRally path). Extend both to also assert that the row's email/phone/payment_method match what was passed in. Add a new TC for the public-web /lessons/[id]/confirm and /bookings/[id]/link-user paths if not already covered. Why defer: the bug only manifests when a NEW write path is added and the dev forgets. Today's 4 paths are individually correct. The cost of the refactor (4 callsite changes + a new RPC + 2-4 new tests) doesn't pay back at today's scale. Revisit when a 5th write path is genuinely needed, or if two of the current paths diverge further. Logged here so a future agent doesn't have to rediscover the audit.

Discovered in: v0.26.0 (DEC-224 — OpenRally lessons empty roster fix)

`padel-backend/supabase/functions/src/domain/bookings/bookingRepository.ts` line ~3613-3633 invokes three follow-up Edge Functions via `EdgeRuntime.waitUntil(Promise.allSettled([...]))` after a booking finalizes: • supabase.functions.invoke('process-new-confirmed-booking', { body: { bookingId } }) • supabase.functions.invoke('confirm-coaching-booking-v2', { body: { bookingId } }) • supabase.functions.invoke('update-user-upcoming-flag', { body: { userIds, updateType: 'add', startTime } }) The `Promise.allSettled` wrapper means rejections never throw. The `EdgeRuntime.waitUntil` wrapper means the function returns before the calls finish. Combined: any Edge Function that 500s, times out, or throws will fail SILENTLY — no logs, no alerts, no caller-visible signal. The booking response to the TPS client succeeds with 200. This is exactly what hid Bug 1 (v0.26.0) for two days: `confirm-coaching-booking-v2` was throwing on every single invocation (SELECTing a non-existent `club_id` column), and TPS was getting back successful booking responses, while Setpoint had nothing — the booking was stuck at `status='hold'` and the cleanup_expired_lesson_holds cron deleted the orphan within ~6 minutes. The error went straight into the swallowed Promise.allSettled and nobody saw it until a customer-facing demo failed. PR #197 fixed the underlying Edge Functions so they don't throw anymore for that specific bug. But the SWALLOWING behavior at the caller is unchanged. The next regression of this shape — any Edge Function in this list that starts failing for any reason — will hide for the same two days until a customer notices. Fix path (small, mechanical): Option A (minimal): keep fire-and-forget but log rejections. `EdgeRuntime.waitUntil( Promise.allSettled([...]).then((results) => { results.forEach((r, i) => { if (r.status === 'rejected') { const fnName = ['process-new-confirmed-booking', 'confirm-coaching-booking-v2', 'update-user-upcoming-flag'][i]; console.error(\`[handleSuccessPayment] \${fnName} failed for booking \${bookingId}:\`, r.reason); } }); }) );` Option B (cleaner): move `confirm-coaching-booking-v2` (the one that actually affects whether a Setpoint coach lesson finalizes) into the synchronous path BEFORE return. Failures surface to the TPS booking response as a 500, the user retries, the booking re-attempts. The other two (notification flag updates, etc.) can stay async since their failure is cosmetic. Recommended: Option B for confirm-coaching-booking-v2 (because its failure leaves Setpoint state inconsistent), Option A for the other two. This way operator-visible failures throw loudly, while non-critical post-actions stay non-blocking. Acceptance criteria: • A TPS booking whose confirm-coaching-booking-v2 throws results in either (a) a non-200 TPS booking response that retries (Option B) OR (b) a structured console.error visible in Supabase Edge Function logs with the bookingId and the function name (Option A). • Add a test: temporarily stub confirm-coaching-booking-v2 to throw, exercise handleSuccessPayment, assert the failure surfaces (either as a thrown response or a console.error call). Why defer: the underlying Edge Functions don't currently throw (post-PR #197). This is a forward-looking observability gap. Fix when next touching this file, or as a small standalone PR. Matches CLAUDE.md 'No silent errors' principle.

Discovered in: v0.26.0 (DEC-223 — Bug 1 root-cause investigation)

`padel-backend/supabase/functions/stripe-webhook-v2/index.ts` lines 186-197 invokes the coach-confirm Edge Function TWICE for non-coaching bookings: Lines 186-190 (UNCONDITIONAL, runs for every booking type): ``` supabase.functions.invoke('confirm-coaching-booking-v2', { body: { bookingId: bookingId }, }); ``` Lines 192-197 (inside `if (booking.type !== 'coaching')`, so ONLY for non-coaching bookings): ``` if (booking.type !== 'coaching') { supabase.functions.invoke('confirm-coaching-booking-v2', { body: { bookingId: bookingId }, }); ... } ``` Net behavior: • Coaching bookings → confirm-coaching-booking-v2 invoked once (lines 186-190). Correct. • Non-coaching bookings → confirm-coaching-booking-v2 invoked TWICE (lines 186-190 + lines 192-197). The function early-returns for non-coaching bookings, so both calls are effective no-ops, but the duplicate invocation is still wasteful (two Edge Function spin-ups, two HTTP round trips) and clearly unintentional. Most-likely origin: a refactor moved the coach-confirm call out of the conditional block (lines 186-190) but left the original call inside the conditional (lines 192-197). The conditional may also be inverted from its original intent — if the author meant to invoke confirm-coaching-booking-v2 only for coaching bookings, the `!==` should be `===` and the unconditional call at 186-190 is the typo. Either way, the code as written is duplicate / probably-stale. Fix path (one-line): Option A (most likely correct intent): delete the duplicate invocation at lines 193-197, keep the unconditional call at 186-190 as the single source of truth. Option B (if the original intent was to ONLY invoke for coaching bookings): delete the unconditional call at 186-190 AND flip lines 192-197's condition to `if (booking.type === 'coaching')`. Recommendation: Option A. Reasoning: confirm-coaching-booking-v2 already guards internally on booking.type, so the unconditional pattern is safe and minimizes call-site logic. Plus the other non-coaching branches at lines 199+ (createMatch, match PATCH) suggest the `if (booking.type !== 'coaching')` block is specifically for non-coaching post-actions (match creation), and the coach-confirm invocation inside it is a leftover. Acceptance criteria: • After fix: stripe-webhook-v2 invokes confirm-coaching-booking-v2 at most once per booking, regardless of booking type. • Add/extend a stripe-webhook-v2 unit test (or smoke test) that asserts the invocation count is 1 for both coaching and non-coaching bookings. • Verify no regression: a Stripe-paid coaching booking still finalizes correctly end-to-end (existing TC-021 or TC-143 coach-flow tests should suffice). Why defer: the duplicate invocation has zero user-facing impact today (the second call no-ops). The cost is wasted compute. Fix when next touching stripe-webhook-v2 or as a small standalone PR.

Discovered in: v0.26.0 (DEC-224 — code review during OpenRally roster fix)

v0.24.0 ships the engine + admin only. POS and Terminal apps do not yet consume resolve_discounts at checkout time — orders ring up the cashier preset/ad-hoc + group-variation totals only, with no rule-driven group / membership / credit-tier discount applied automatically. Why delegated, not built: ownership lives where the code is written. The POS/Terminal dev owns apps/pos and apps/terminal; writing the consumer integration in their territory is a clean ownership boundary (so they own bug fixes + future evolution forever) and avoids forcing my opinionated POS conventions (how cart lines map to resolver shape, how the discount line surfaces in the cart, how useOrderMutations is plumbed) onto code I don't normally touch. Decision logged in DEC-186. What ships in v0.24.0 as the contract (frozen — do not change without a migration): • 5 new tables: crm_customer_groups, crm_customer_group_members, crm_discount_rules, crm_discount_rule_targets, crm_discount_applications (with RLS, indexes, idempotency unique key) • SQL functions: resolve_discounts(p_facility_id, p_profile_id, p_lines jsonb, p_channel text, p_at timestamptz) — STABLE/read-only — and commit_discount_applications(p_applications jsonb) — append-only, idempotent on (rule_id, document_type, document_id, COALESCE(line_ref,'')) • ALTER pos_orders ADD discount_kind text CHECK preset|cashier_adhoc|group_variation NULL + discount_note text NULL • ALTER pos_order_items ADD is_foc boolean DEFAULT false + foc_reason text NULL (per-line FOC groundwork) • Shared types: import { callResolveDiscounts, callCommitDiscountApplications, resolverApplicationsForCommit } from '@setpoint/common/discounts' What pattern to mirror (canonical consumer reference): • apps/admin/src/app/api/admin/bookings/route.ts ~line 264+ — the admin booking POST resolves + commits post-insert. Sequence: build cart context → resolve_discounts(facility_id, profile_id, lines, channel, now) → apply (subtract from total / store reason) → on finalize, commit_discount_applications with the idempotency key. Integration steps for the POS/Terminal dev's PR: 1. Add an apps/{pos,terminal}/src/queries/use-discount-engine.ts hook that wraps callResolveDiscounts with React Query (debounced on cart changes). 2. In apps/{pos,terminal}/src/components/terminal/checkout-dialog.tsx, compute resolverDiscountCents = min(resolver.total_discount, discountableSubtotal) and add it to the existing discount total (alongside the cashier preset/ad-hoc layer — they stack additively per DEC-186, capped at 100% of line total). 3. Plumb discountKind ('preset' | 'cashier_adhoc' | 'group_variation') + discountNote through use-order-mutations.ts → pos_orders insert. 4. On order finalize, call commit_discount_applications via callCommitDiscountApplications with the resolver's per-rule applications + a deterministic idempotency key (e.g. order_id-based). 5. Test seam: TC-130 already exercises resolve+commit on a booking; mirror the pattern for an order at the till in a new TC-XXX. Stackability semantics (frozen — do not deviate): • Per-rule is_stackable flag (default false). Sum of stackable rules + best non-stackable rule. Capped at 100% of line total. • Cashier preset/ad-hoc layer + resolver layer stack additively in totals (two separate lines on the receipt, clear provenance). Reference materials on main post-merge: • Migration: supabase/migrations/20260506094046_centralized_discount_engine.sql — the full schema + functions • Eval suite: supabase/snippets/discount_engine_evals.sql (17 cells) — runnable as your test seam • Decision rationale: apps/docs/src/data/decisions.ts → DEC-186 • Full handoff doc: apps/docs/src/data/plan-pos-terminal-discount-integration.md • Pattern: apps/admin/src/app/api/admin/bookings/route.ts (the admin booking consumer) Earlier prototype: a previous internal branch wired the consumer code directly (apps/{pos,terminal}/src/queries/use-discount-engine.ts + checkout-dialog plumbing). It was reverted before merge as part of the delegation decision (DEC-186). The dev should write the integration fresh against their own conventions; do not try to recover that branch as a starting point — the canonical reference is the admin booking route.

Discovered in: v0.24.0 (Centralized Discount Engine)

v0.24.0 ships resolve_discounts taking a `p_channel text` parameter and crm_discount_applications enforcing channel ∈ ('admin','terminal','pos','web','openrally') so the ledger can record the source. The OpenRally protocol server in protocol/server/ continues to compute prices via its own materializer + L1 cache without calling Setpoint's resolver, so customer-group discounts never reach TPS users. Notes for the implementer: • The current resolver does NOT filter rules by channel. p_channel is captured for the ledger only — extending the resolver to filter on a future `crm_discount_rules.channels jsonb` column is a small follow-up. • Add a Setpoint-side admin or v1 endpoint (e.g. POST /api/v1/openrally/pricing) that takes (facility_slug, profile_id, lines[]) and calls resolve_discounts internally with channel='openrally'. • In protocol/server/, on the availability + booking-create paths, call this endpoint when the user is identified (OpenRally consumer token → Setpoint profile_id), and merge the resolver's per-line discount into the response. • Cache TTL: short (60s) — group memberships change rarely but rule edits should land in <1 minute. • Eval: extend supabase/snippets/discount_engine_evals.sql with an OpenRally-channel cell. Add a Rust integration test on the protocol server that asserts a customer-group rule surfaces in the availability response when channel='openrally'. Defer until the TPS-Setpoint integration go-live work picks back up. The infrastructure (resolver + ledger channel column) is in place; only the consumer call (and optional channel-scoped rule filter) is missing.

Discovered in: v0.24.0 (Centralized Discount Engine)

v0.24.0 lands the resolver and the crm_discount_rules schema with scope_domain ∈ ('booking','lesson','program','pos','any'). For BOOKINGS, /api/admin/bookings now resolves + commits post-insert, and bookings.price_amount itself reflects the discount via calculate_member_price (which delegates to resolve_discounts). For LESSONS and PROGRAMS, no consumer flow currently consults the resolver at charge time: • /api/admin/lessons commits effectivePrice = price_per_pax_amount_override ?? cls.price_per_pax_amount via create_lesson_atomic — the resolver result is ignored. v0.24.0 originally tried to commit ledger rows post-hoc here but that was misleading (ledger said 'X off' for a charge that was never reduced) and was removed during PR self-review. • /api/v1/programs/[id]/confirm-enrollment + the admin /programs/* equivalents bill list price — same problem. • Open matches have no scope_domain entry today (CHECK constraint is 'booking','lesson','program','pos','any'). Matches generally inherit booking pricing, so they get the discount via the booking path. Suggested fix path: 1. Push resolve_discounts INTO create_lesson_atomic (and the program/enrollment equivalents) so the ledger row, the price the customer pays, and the credit deduction all agree. 2. After the in-RPC change, mirror the booking-route post-insert wiring in /api/admin/lessons + /api/v1/programs/[id]/confirm-enrollment + /api/admin/programs/* with the same guard pattern (only commit when the cascade ran). 3. Tests: add TC-131 (lesson) + TC-132 (program enrollment) following TC-130's shape — create rule, schedule/enroll, assert the resolver discount lands in crm_discount_applications AND that the lesson_payments / program_enrollments row reflects the reduced amount AND that the customer-detail panel returns it. Defer until a consumer actually wants lesson/program-scoped discounts AND someone designs the in-RPC discount integration. Today no facility has asked for them.

Discovered in: v0.24.0 (Centralized Discount Engine)

v0.24.0 adds a new crm_customer_groups + crm_customer_group_members + crm_discount_rules path. The legacy POS-side group tables (whatever the POS app uses today for its existing group-variation discount surface) are intentionally left untouched per POS dev's call (γ pure, additive only — POS dev wants to retire them on their own schedule). Suggested approach (when the time comes): 1. Audit reads: grep for any remaining consumer of the legacy tables. Confirm no admin/web/POS/terminal/api/protocol code still SELECTs from them. 2. Audit writes: same — confirm no INSERT / UPDATE writes are happening anywhere. 3. Backfill any data still in legacy tables into crm_customer_groups + crm_customer_group_members if the data hasn't been migrated. 4. Drop the legacy tables in a fresh migration. 5. Update any out-of-date references in docs (apps/docs/src/data/, CLAUDE.md, apps/apidocs). Defer until the POS dev confirms the legacy path has zero production readers. UPDATE 2026-09-29 (fix/pos-discount-groups-crm-source, DSC-01/DSC-02): apps/pos was found still reading/writing the legacy tables for its 'Customer group pricing' picker + 'add to group' flow (the actual root cause of DSC-01/DSC-02 — a separate, narrower bug about a stale group list, not this retirement). Fixing that also completed steps 1-2 here: apps/pos's queries/use-customer-groups.ts, mutations/use-customer-group-mutations.ts, and components/orders/order-detail-modal.tsx now all target crm_customer_groups / crm_customer_group_members, matching apps/terminal (which already did). `grep -rn '"pos_customer_group' apps/pos/src apps/terminal/src apps/admin/src` (excluding type declarations) now returns zero matches — no application code reads or writes either legacy table anymore. Step 3 (backfill) is DONE unconditionally by 20260929054028_pos_discount_group_variations_crm_fk.sql — that migration's Steps 1-2 find-or-create a matching crm_customer_groups row and backfill crm_customer_group_members for EVERY pos_customer_groups / pos_customer_group_members row, independent of whether pos_discount_group_variations references them (its own job, repointing that table's FK, is Steps 3-4, layered on top of the now-complete backfill). Verified against real production data before this migration ran: 10/10 legacy groups had zero name collisions against existing crm_customer_groups rows, and only 2 of 22,302 legacy membership rows had a resolvable profile_id (the other 22,300 share one identical timestamp — the fingerprint of an unrelated historical bulk-insert bug, not real memberships; left in the legacy table, reported via migration NOTICE, not silently dropped). Step 4 (drop the legacy tables) can now proceed once the sibling pos_customer_group_members_group_id_fkey and the legacy tables' own write RLS policies are also addressed (neither was touched by this fix — deliberately out of scope, see the PR). Steps 4-5 remain a separate, explicit decision — not taken as part of this fix.

Discovered in: v0.24.0 (Centralized Discount Engine)

Surfaced during v0.24.0's full-build sweep. The customer-picker dropdown ({showCustomerPicker && customerSearch && (…)}) was placed AFTER the closing </div> of the .space-y-2 wrapper instead of inside the <div className="relative"> that contains the Search Input — and was followed by an extra </div>. Pre-existing on the feature/loyalty-sections-and-error-handling branch base (verified via git show HEAD). Resolution: relocated the dropdown JSX inside the .relative Input wrapper (correct anchor for top-full / left-0 / right-0 absolute positioning) and dropped the orphan </div>. No behavior change at runtime — the dropdown still renders only when both showCustomerPicker is true AND customerSearch has text — but Turbopack now parses the file. The terminal app's checkout-dialog mirror was already correctly nested.

Discovered in: v0.24.0 full-build sweep (verified pre-existing via git show HEAD on feature/loyalty-sections-and-error-handling)

Companion to BUG-076 — surfaced during v0.24.0's full-build sweep. The full usePmsBookings function does proper lesson enrichment (lines 246+), populating lesson and lessonStudents on each returned object. The today-only variant useTodayPmsBookings (line 436) did not — its return-shape literal was missing both fields, but the function was annotated as returning PmsBooking[]. Pre-existing on feature/loyalty-sections-and-error-handling. Resolution: minimal fix — added `lesson: null, lessonStudents: []` to the today-filter map. This matches the PmsBooking contract without changing runtime behavior (the today filter doesn't surface lesson detail today; that's a pre-existing UX gap, separate from the typing fix). A future PR can backport the full lesson enrichment from usePmsBookings if today-filter consumers want lesson detail.

Discovered in: v0.24.0 full-build sweep (verified pre-existing via git show HEAD on feature/loyalty-sections-and-error-handling)

After v0.23.10 (DEC-185), the Loyalty page's Subscriptions sub-list shows everyone with a subscription record (active + pending + cancelled + expired). Operators can Cancel an Active or Pending row from the row dropdown. BUT cancelling a paid-via-Stripe row only flips user_memberships.status to 'cancelled' on our side — it does NOT call Stripe's subscription cancel API, so the customer keeps getting billed. There is no UI surface today that calls stripe.subscriptions.cancel via the SDK. Operator workflow: customer wants to cancel their paid subscription. Operator goes to /[facility]/crm/customers/[id] (the only place that shows the membership). The Membership card there (apps/admin/src/app/[facility]/crm/customers/[id]/page.tsx around line 645) is read-only — name + status + 'Member since' date, no actions menu. Workaround today: cancel directly in the Stripe Dashboard, then either wait for the subscription_deleted webhook to flip user_memberships.status='cancelled' (apps/admin/src/app/api/v1/webhooks/stripe/route.ts handles this), or PATCH the row directly via SQL. Suggested implementation: 1. Extend /api/admin/crm/customers/[id] response (or use a separate /api/admin/user-memberships/{id} GET) to include the membership's `id` + `stripe_subscription_id` so the UI can route the cancel correctly. 2. In apps/admin/src/app/[facility]/crm/customers/[id]/page.tsx, add a dropdown / actions menu to the Membership card. For paid subs (stripe_subscription_id !== null), the Cancel action should call POST /api/admin/user-memberships/{id}/cancel which both (a) cancels the Stripe subscription via the SDK (stripe.subscriptions.cancel) and (b) flips the row status. For unpaid orphans (stripe_subscription_id === null), reuse the existing PATCH user-memberships {status:'cancelled'} flow. 3. Two-step destructive confirm dialog (same Dialog pattern as apps/admin/src/components/admin/subscriptions-list.tsx). 4. Test: extend tc-110 with an end-to-end case that creates a paid sub via the existing tc-110 Stripe checkout flow, then cancels it from CRM customer detail, asserts both DB row.status='cancelled' AND Stripe sub status='canceled' (use the existing Stripe SDK in test-helpers). Files to read first: apps/admin/src/app/[facility]/crm/customers/[id]/page.tsx (where the action lives), apps/admin/src/app/api/v1/webhooks/stripe/route.ts (cancellation webhook handler — mirror its data-flip logic), apps/admin/src/lib/stripe.ts (lazy proxy).

Discovered in: v0.23.10 UX review (Loyalty restructure + error-handling pass)

The Subscriptions list (apps/admin/src/components/admin/subscriptions-list.tsx) surfaces admin-assigned user_memberships with NULL stripe_subscription_id under the 'Pending' status badge. The row dropdown today offers View Customer / Get Payment Link / Cancel Subscription. There is no third option for 'customer paid in cash at the front desk' — i.e. mark this orphan as paid without going through Stripe Checkout. Today the operator either (a) tells the customer 'pay this link online', (b) has them pay in person and the orphan stays in the queue forever, or (c) cancels the orphan and creates an entirely separate paper trail. Suggested implementation: 1. Add a new admin-only column to user_memberships: `paid_offline_at TIMESTAMPTZ NULL` (or repurpose an existing column — verify before migration). When non-null, treat the row as Active rather than Pending for derivedStatus(). 2. Update derivedStatus() in subscriptions-list.tsx to surface paid_offline_at !== null as 'Active' (with maybe a separate 'Paid offline' sub-badge). 3. Add 'Mark as Paid (cash)' DropdownMenuItem in the row actions. POST /api/admin/user-memberships/{id}/mark-paid-cash sets paid_offline_at=now() and writes a corresponding payments row with payment_method='cash' (the wallet payment_method CHECK constraint already accepts 'cash' — verify in supabase/migrations/). 4. Optional: a 'Paid via cash on {date}' badge or detail somewhere (CRM customer page) so the audit trail is visible. 5. Two-step confirm dialog: 'Mark {name}'s {plan_name} as paid in cash?' — destructive only because there's no undo without a manual SQL row delete. 6. Test: extend tc-122 (or new tc-XXX) — assign a membership via the dialog, click Mark as Paid in row, assert the row's status badge flips from Pending to Active, assert payments row exists with payment_method='cash'. Files to read: apps/admin/src/components/admin/subscriptions-list.tsx, apps/admin/src/app/api/admin/user-memberships/[id]/route.ts (existing PATCH handler), apps/admin/src/app/api/v1/webhooks/stripe/route.ts (Stripe-success path — the cash path should mirror the data writes there minus the Stripe-specific fields).

Discovered in: v0.23.10 UX review (Loyalty restructure + error-handling pass)

The Subscriptions list (apps/admin/src/components/admin/subscriptions-list.tsx) shows a 'Since' column that renders subscription.start_date — but operators triaging orphans typically want assigned_at / created_at (when did the admin click Assign Subscription?), not the plan's start_date. For a club with 5+ pending rows, age-based triage ('this customer was assigned 30 days ago, chase them or cancel') is currently impossible. The /api/admin/members endpoint (apps/admin/src/app/api/admin/members/route.ts) currently returns only the user_id, profile, subscription, tier, and balance fields. user_memberships.created_at exists in the DB but isn't surfaced. Suggested implementation: 1. Update the API: in /api/admin/members route.ts, include `created_at` in the membership SELECT and project it into the response shape under subscription.created_at (or a new top-level assigned_at). 2. Update the Member type in subscriptions-list.tsx (and optionally the shared MembersApiMember in use-members-list.ts) to include subscription.created_at. 3. Add a new column to the table — 'Pending for' — using `formatDistanceToNow(new Date(m.subscription.created_at), { addSuffix: true })` from date-fns (already a dep — used in apps/admin/src/app/[facility]/crm/customers/[id]/page.tsx). 4. Sort the list by subscription.created_at ASC (oldest first) so stale orphans bubble up. 5. Test: extend tc-122 — seed two orphans with different created_at, assert the table renders them in oldest-first order. Files: apps/admin/src/app/api/admin/members/route.ts (API), apps/admin/src/components/admin/subscriptions-list.tsx (UI), apps/admin/e2e/tc-122-loyalty-error-handling.e2e.ts (test extension).

Discovered in: v0.23.10 UX review (Loyalty restructure + error-handling pass)

Originally reported as 'no sort or filter on Pending Payments.' The filter half is RESOLVED by the per-section list rebuild — both subscriptions-list.tsx and wallet-holders-list.tsx now have text search + segment dropdowns. What remains: no clickable column-sort headers (the deleted UnifiedMembersList had toggleable Sort by user / email / subscription_status / tier / balance / total_topups via a SortHeader sub-component). Suggested implementation (minimal viable): 1. Resurrect the SortHeader sub-component from git history (commit before this PR removed unified-members-list.tsx) — it's ~25 lines, takes (keyId, active, dir, onToggle, children) and renders a button with arrow icons. 2. Add useState for sortKey / sortDir per list. Default sort key 'default' = server order (oldest pending first for Subscriptions, server order for Wallet Holders). 3. Wrap each TableHead label in <SortHeader>. Wire the toggleSort + comparator switch to each list's columns. 4. Three-click cycle: asc → desc → reset to default. Same UX as the deleted component. Files: apps/admin/src/components/admin/subscriptions-list.tsx, apps/admin/src/components/admin/wallet-holders-list.tsx. Pattern to copy: git show <pre-DEC-185-commit>:apps/admin/src/components/admin/unified-members-list.tsx — SortHeader at the top + toggleSort + comparator block in visibleMembers.

Discovered in: v0.23.10 UX review (Loyalty restructure + error-handling pass)

Original report: pending-memberships-list.tsx returned null when zero orphans; brand-new operators had no signal the queue existed. Resolved by the per-section customer-list rebuild on the same branch. The replacement subscriptions-list.tsx renders a permanent table with an empty-state cell ('No subscriptions yet. Use Assign Subscription to enroll a customer.') AND a segment filter that includes 'Pending'. The discoverability gap closes.

Discovered in: v0.23.10 UX review (Loyalty restructure + error-handling pass)

Two routes were missed by v0.23.10's BUG-045 fix and still write the legacy 'card' literal: • apps/admin/src/app/api/v1/lessons/[id]/join/route.ts:161 — `payment_method: isPackagePayment ? "package" : "card"`. The route does not check the insert's error return, so the constraint violation is fully silent. • apps/admin/src/app/api/v1/lessons/[id]/confirm/route.ts:272 — same expression. The route checks payErr but only `console.error`s; the response still returns 200 with confirmed:true. Both routes (a) update bookings.payment_method to 'stripe' correctly (different table, different CHECK), and (b) try to write the corresponding payments ledger row with the now-disallowed 'card' literal — Postgres raises payments_payment_method_check, the insert is rejected, the lesson appears confirmed but the payments ledger has no row for the Stripe charge. Mirror fix to the four already-fixed routes: replace 'card' with 'stripe' on both lines.

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, source-level grep — no E2E assertion exercises this path so no test failed)

Root cause: apps/terminal/src/queries/use-pms-bookings.ts (useTodayPmsBookings) and apps/terminal/src/app/(dashboard)/[facility]/bookings/page.tsx (Today/Yesterday/Tomorrow day-filter buttons) both computed the day window with date-fns startOfDay/endOfDay against `new Date()` — the BROWSER's local timezone, not the facility's. Operationally, any operator on a device whose system TZ ≠ facility TZ saw the wrong day's bookings. In the headless-Chromium test environment, runtime TZ is UTC, so a booking at 'today Bangkok 10:00' (= UTC 03:00) fell outside the 'today UTC' window once Bangkok crossed 17:00 local. Real-world impact: receptionists on UTC-set devices, airplane-mode wifi, or any non-Bangkok-TZ operator viewing the terminal saw bookings vanish from Today/Tomorrow. Fix landed in feature/loyalty-sections-and-error-handling: added facilityDayWindow(offset, tz) and todayInFacilityTzRange(tz) helpers using Intl.DateTimeFormat to derive the facility-local YYYY-MM-DD, then mapped via @setpoint/common/datetime's facilityLocalToUTC for UTC bounds. timezone pulled from useAuth() (already exposed by auth-provider.tsx with 'UTC' fallback). queryKey now includes timezone so re-renders refresh on TZ change. Same pattern as 8+ admin API routes (availability, lessons/available, pricing/calculate, etc.) — facilityLocalToUTC is the canonical TZ helper.

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, root-caused 2026-05-05)

apps/admin/e2e/tc-072-web-slot-blocking-ui-assertions.e2e.ts tests 72h, 72j, 72l fail deterministically (confirmed by retry). All three assert `await expect(tenAmSlot).toContainText("Booked", { timeout: 15_000 })` after seeding a held/confirmed booking on the same court+slot. The button renders (locator finds it) but its text content does NOT include 'Booked'. Same pattern across: • 72h — confirmed booking on court 1 → 10:00 AM slot should show 'Booked' • 72j — 2hr booking should mark BOTH sub-hour slots as 'Booked' • 72l — held slot (during another user's checkout) should show 'Booked' to other users Either the web app's availability response no longer marks held/confirmed slots, or the button render path drops the 'Booked' label. Likely a recent change to apps/web/src/components/booking/* or the public availability endpoint.

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, retry confirmed deterministic on all three sub-tests)

apps/admin/e2e/tc-025-guest-auto-account.e2e.ts:201 fails deterministically (confirmed by retry). The flow: 1. Admin creates a guest booking via the calendar (succeeds). 2. Test opens a new web page targeting the customer-facing login URL. 3. Asserts `getByLabel('Email').or(locator("input[type='email']"))` becomes visible. Step 3 times out at 30s. Either the web login page failed to navigate (URL redirect issue) or the email input markup no longer matches the locator (label, type='email', or both). Same web-app login flow used by other tests passes elsewhere — this is specific to the post-admin-booking flow path in tc-025. Possibly the navigation lands on a different page (e.g. a bookings list when already authenticated).

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, retry confirmed deterministic)

RESOLVED. The check-in itself worked end-to-end — TC-021 was just impatient. The for-loop in handleCheckInAllStudents iterates per-student (PATCH /api/admin/lessons/{id}/students), which for 3 students takes roughly 3 × API-latency. The test used `await page.waitForTimeout(1000)` after the click, then asserted body text. With slow webserver responses on a freshly compiled route, the assertion fired before the loop finished and the button was still showing 'Checking in...'. Page snapshot at failure confirmed: 'Checking in...' button still disabled, no 'All Checked In' state yet. Fix in v0.24.0: replaced the fixed sleep with `await expect(checkInBtn).toContainText(/all checked in/i, { timeout: 30_000 })` — wait for the terminal state instead. Original failure shape: 1. Admin creates a coach lesson booking. 2. Opens detail dialog, clicks `data-testid='booking-checkin-all'`. 3. Asserts body text contains 'Checked In' / 'IN' / 'checked_in'. Step 3 fails — none of those substrings appear. The button click either silently fails (no API call, or API fails server-side without surfacing in UI) OR the dialog re-render path doesn't pick up the new state. Comparison: tc-020 (admin court booking check-in) used to fail with the same error during initial run but the failure list shows tc-021 as the canonical one (tc-020 likely passed during this session). Pattern suggests coach-lesson check-in path specifically.

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, retry confirmed deterministic)

apps/admin/e2e/tc-001-web-booking.e2e.ts:104 fails deterministically on a fresh dev server (confirmed by retry). The web app's Select step renders all time-slot buttons immediately with a '...' placeholder, then fetches prices and replaces '...' with 'THB X'. The test does: await page.waitForTimeout(5000) // hardcoded const timeButtons = page.locator("button").filter({ hasText: /\d{1,2}:\d{2}\s*(AM|PM)/ }) for (const btn of timeButtons) { if (text.includes('THB')) { ... break } } On a cold dev server, the pricing API (Next.js route compile + DB query) takes ~7-10s — 5s isn't enough. None of the buttons have 'THB' in their text yet, so the loop completes with `selectedSlot = false` and the test errors with `'No available time slot found'`. Same vulnerability in tc-076-cross-app-payment-verification:217 (similar pattern), tc-015 lesson booking, etc. — anywhere that uses `await waitForTimeout(N)` followed by a text-based filter on async-loaded data.

Discovered in: Post-v0.23.10 full-suite triage (2026-05-04, retry confirmed deterministic)

calculate_credit_tier reads facilities.settings.top_up_validity_months (defaulting to 36 months) when computing the rolling-window topup sum that drives tier qualification. The setting is honored end-to-end on the backend (RPC + indexes + tc-112 CT-L1-13a pinning the override). The gap is purely UX: there is no admin form field that writes this setting. Operators who want a non-default window (e.g. 12 months for a more frequent re-qualification cadence, or 60 months for a more lenient program) cannot do it from the UI — they have to UPDATE facilities.settings via SQL directly. The Tiers tab description on apps/admin/src/components/admin/tier-config-tab.tsx also hardcodes the string '36-month rolling window' regardless of the actual setting, so even reading the live value isn't possible from the UI. Recommended path: add a 'Topup window (months)' input to facility-settings-form.tsx (settings JSON write), and switch the Tiers tab description from a hardcoded literal to the configured value.

Discovered in: v0.23.10 source review (originally Finding #12 in the local review-loyalty-ux-2026-05-04.md working doc)

apps/admin/src/components/admin/tier-config-tab.tsx ships Add (new in v0.23.10) and Edit (Pencil button per row) but no Delete button. The credit-tiers route also doesn't expose a DELETE handler, so both layers are missing. Why deferred from v0.23.10: data side is clean (DB doesn't care, tiers are independent), but the UX safeguards are the substantial part: (1) Members who currently qualify lose their badge instantly — public site reflects the change. (2) Historical bookings unaffected (price baked in at booking creation). (3) next_tier_threshold_amount recalculates correctly via the RPC. (4) A responsible Delete dialog has to count affected customers, spell out the consequence ('12 customers will fall back to Club'), possibly require typing the tier name to confirm, possibly offer a 'deactivate instead' alternative. (5) Setting court_discount_pct=0 already accomplishes 'this tier confers no benefit' — combined with the route's 0%-suppression (shipped in v0.23.10), the badge stops showing without any DELETE. Recommended path if/when picked up: add an is_active column to credit_tier_configs, deactivate-instead-of-delete (mirrors plan semantics), and only ever hard-delete from a separate 'danger zone' in settings.

Discovered in: v0.23.10 source review (originally Finding #7 in the local review-loyalty-ux-2026-05-04.md working doc)

apps/admin/src/app/api/admin/bookings/[id]/players/route.ts L7-47 (recalculateCostSplit) updates cost_per_player_amount whenever a player is added (POST) or removed (DELETE) on a 'split' booking. The court total (price_amount) correctly stays fixed — the court is what's paid for, not per-head. The bug: existing per-player payments are not adjusted to match the new split. Worked example: booking 1,400 THB, cost_split='split', 3 players, all 3 paid 466.67 each (1,400 collected). Admin adds player 4 → cost_per_player_amount recalculates to 350, but players 1-3 are still recorded as having paid 466.67 (overpaid by 116.67 each), and player 4 has no payment record (owes 350). No PaymentIntent is created for the new player, no refund is issued to the overpayers, no payment_status updates, and no ledger reconciliation entry is written. Booking-level total still reconciles (1,400 in = 1,400 court price), but the per-player breakdown is silently wrong. DELETE has the inverse problem: removing a paid player leaves them recorded as paid for a share they no longer owe; remaining players are now under-charged on the new split. The 'organizer' cost_split mode is even less well-defined: recalculateCostSplit L27-29 short-circuits on cost_split='organizer', so adding a player to an organizer-pays booking produces no recalc at all — new player joins for free from the system's perspective, with no signal to the admin.

Discovered in: Multi-player booking review (2026-04-30, originally PR #560)

Two compounding behaviors on the admin-assigned membership flow: (1) apps/admin/src/app/api/admin/user-memberships/route.ts L154-164 inserts a user_memberships row with status='active' the moment an admin assigns a plan, regardless of whether the customer has paid. The customer can immediately use member benefits (discounts, booking privileges) without paying. (2) apps/admin/src/app/api/v1/webhooks/stripe/route.ts L267-329 — when the customer eventually pays via Stripe Checkout, the checkout.session.completed webhook adopts the orphan row (NULL stripe_subscription_id) and overwrites start_date to `new Date()` (today) and recalculates end_date as today + plan.billing_period. Combined effect: customer gets the pre-payment period for free, and on payment the membership's clock resets so they get a full month/quarter/year from payment date — not from the admin's originally-chosen start_date. Example: admin assigns April 1 (intended end May 1). Customer pays April 20. Result: free benefits April 1–19, then a fresh month April 20 → May 20. ~50 days for the price of 30. Open product questions: should an unpaid assignment be 'pending' instead of 'active'? Should payment honor the admin's start_date or use the payment date? Should there be a payment deadline that auto-expires unpaid assignments?

Discovered in: Membership flow review (2026-04-30, originally PR #560)

apps/admin/src/components/admin/tier-config-tab.tsx renders a row per existing credit_tier_configs entry with a Pencil edit affordance — but no 'Add Tier' button, dialog, or empty-state CTA. The credit-tiers POST route validates against four hardcoded tier names (club, pro, all_star, hall_of_fame), so the system DOES support adding any of those four — but operators have no way to do it from the UI. If a facility's credit_tier_configs row was never seeded for a given tier (or rows were deleted), the only path to add it is direct SQL or a hand-rolled API call. The empty-state placeholder reads 'No tiers configured yet.' with no CTA, which is misleading. Surfaced during v0.23.10 demo recording when the user (correctly) noticed that the demo only showed an already-populated tier table. Significance grew with v0.23.10 because the Tiers tab is now visible on every facility (previously only on credit-accumulation ones), so this gap reaches more operators.

Discovered in: v0.23.10 demo recording

PR #563 renamed the operator-visible Memberships page to Loyalty (sidebar, route, page H1, settings card label). Per DEC-182, internal names that describe sub-resources (DB tables, APIs, list components like membership-plans-list.tsx) intentionally kept their entity-scoped names. BUT one component is named after the *old page*, not a sub-resource: `apps/admin/src/components/admin/memberships-page-client.tsx` exporting `MembershipsPageClient`. It IS the page client for the Loyalty page (now renders the two stacked Memberships + Wallets sections per DEC-185), not a sub-resource component. Deferred from PR #563 to keep that diff bounded — the rename is a 5-minute mechanical change (rename file, rename function/interface, update the one import in apps/admin/src/app/[facility]/loyalty/page.tsx). Boss flagged this as a future cleanup.

Discovered in: PR #563 self-review

apps/admin/src/app/api/admin/bookings/[id]/players/route.ts L27-37 has a recalc helper that runs after a player is added/removed. It guards: `if (booking?.cost_split === 'organizer') return` — so an operator's organizer-pays choice is preserved correctly. Otherwise, when count > 1 it sets cost_split='split' + cost_per_player_amount=price/count. This contradicts DEC-152 (admin always writes cost_split='organizer', cost_per_player_amount NULL) for a narrow edge: a booking originally created with no players (cost_split=null) that later has players added via PATCH transitions to 'split' instead of 'organizer'. The dialog never produces this state — it always sets cost_split='organizer' on multi-player creation (and after BUG-041, the route default does the same). The bug only fires for direct API callers that create a single-guest booking and then PATCH players in afterwards. No operator-driven flow hits this path today, so impact is zero in practice. Spotted during BUG-041 source review.

Discovered in: BUG-041 source review (2026-04-29)

apps/admin/src/app/api/admin/bookings/route.ts had `body.cost_split_mode || "split"` in two places (single-booking branch and recurring branch's post-insert side-effects block). DEC-152 documents that admin always writes cost_split='organizer' and leaves cost_per_player_amount NULL — the dialog masks this drift today by always sending cost_split_mode='organizer' explicitly, so no operator hit the buggy default in practice. But any direct API caller (e.g., a future automation script, a TPS-side multi-player booking pass-through) that omitted cost_split_mode would have triggered the wrong default. Spotted during BUG-039 PR self-review: my TC-116 multi-player test had passed `cost_split_mode: 'split'` and asserted cost_per_player_amount = price/2, which was technically valid on the API but contradicted the documented architecture and what TC-103 enforces.

Discovered in: BUG-039 PR self-review (2026-04-29)

POST /api/admin/bookings recurring branch (apps/admin/src/app/api/admin/bookings/route.ts L408–L501; weeks > 1) writes each weekly row with `payment_status: 'paid'` whenever `payment_method === 'credits'` but never calls `adjust_club_credits` and never goes through the new `create_booking_with_credits` RPC added for BUG-021. The customer's `club_credits.balance_amount` is unchanged and no `club_credit_transactions` row is written for any of the N created bookings. Operationally, recurring credit-paid bookings are effectively free for the customer. Discovered while writing the BUG-021 fix (the single-booking atomicity work). Out of scope for that PR — recurring also needs a different atomicity story (N bookings, N deductions, partial-week skips on conflict). Likely fix: extend the RPC (or add a sibling) that takes a list of (start_time, end_time) tuples and deducts per-booking inside one transaction per week, returning per-week success/skipped status. Filed as a separate bug per the BUG-021 prompt's scope guard.

Discovered in: BUG-021 fix self-review (2026-04-29)

All four payment-link routes set `success_url` to `${ADMIN_APP_URL}/{slug}/...?payment=success` on the admin app domain. Payment links are admin-generated and emailed to customers, who don't have admin accounts. After paying in Stripe Checkout, the customer's browser is redirected to the admin domain; the admin middleware (apps/admin/src/middleware.ts:62-69) bounces unauthenticated requests to `/auth/login?payment=success&redirect=/{slug}/memberships`. The customer cannot authenticate and is stuck. The `?payment=success` query param has no consumer anywhere in the codebase — no banner, no toast, no logic — so it serves purely as a (broken) hand-off cue. Critically, **the payment itself completes correctly**. The Stripe webhook (apps/admin/src/app/api/v1/webhooks/stripe/route.ts:43+) handles `checkout.session.completed` server-to-server and creates/adopts the `user_memberships` row independently of where the browser lands. Stripe also shows its own 'Payment successful' confirmation in the Checkout UI and emails a receipt. The customer just doesn't get a polished hand-off back into the app — no data is lost, no double-charging, no membership-creation failures. Affected routes: - apps/admin/src/app/api/admin/memberships/payment-link/route.ts:78 - apps/admin/src/app/api/admin/user-memberships/[id]/payment-link/route.ts:76 - apps/admin/src/app/api/admin/bookings/[id]/payment-link/route.ts:106 - apps/admin/src/app/api/admin/lessons/[id]/payment-link/route.ts:139 Why this slipped past TC-110: apps/admin/e2e/tc-110-membership-payment-link.e2e.ts:232-263 runs a real Stripe checkout but only asserts the DB row was adopted. It closes the Stripe browser tab without reading the post-redirect URL, so the login dead-end is invisible to the test. Recommended fix (Option A — clean): move `success_url` to the public web app (`${WEB_APP_URL}/{slug}/memberships?payment=success` and equivalents for bookings/lessons). Add a `?payment=success` toast/banner to the existing `apps/web/src/app/(public)/[facility]/memberships/page.tsx`. Requires adding a `WEB_APP_URL` env var to the Vercel admin project (mirrors the `ADMIN_APP_URL` pattern). Update TC-110 to assert the landing URL after Stripe checkout completes. Recommended fix (Option B — minimal): add a public `/{slug}/payment-success` route on the admin app exempt from the auth middleware. Mixes a customer-facing page into the admin domain — works, but messy.

Discovered in: v0.21.2 production smoke test (2026-04-28)

Supabase Auth stores `auth.users.email` lowercase, and the `on_auth_user_created` trigger copies that into `customers.email`. But many admin endpoints look up customers by an admin-typed email using `.eq("email", input)` directly, with no `.toLowerCase()` on the input. If an admin types `Foo@Example.com` for a customer who signed up as `foo@example.com`, the lookup returns null and the endpoint behaves as if the user doesn't exist (404, or attempts to create a duplicate). The `findOrCreateUserByEmail()` helper (`apps/admin/src/lib/find-or-create-user.ts`) already does the lowercasing correctly — the fix is to either route through the helper or to call `email.toLowerCase()` before the `.eq()`. Discovered while wiring auto-create into the assign-membership endpoint (PR #548); that one path is now fixed by switching to the helper. The remaining endpoints below still have the latent bug. Affected files (production code paths): - apps/admin/src/app/api/v1/bookings/hold/route.ts:46 — guest_email lookup - apps/admin/src/app/api/v1/pricing/calculate/route.ts:52 — userEmail lookup - apps/admin/src/app/api/admin/credit-tiers/user-tier/route.ts:36 — userEmail lookup (read-only GET) - apps/admin/src/app/api/admin/bookings/price/route.ts:48 — userEmail lookup - apps/admin/src/app/api/admin/booking-passes/route.ts:36 — userEmail lookup (read-only GET) - apps/admin/src/app/api/admin/players/search/route.ts:47 — exact email branch in player search - apps/admin/src/app/api/admin/matches/route.ts:209 — organizer_email lookup - apps/admin/src/app/api/admin/programs/[id]/enrollments/route.ts:100 — user_email lookup (program enrollments — also returns 404 instead of auto-creating, see scope note in PR #548) - apps/admin/src/app/api/admin/team/route.ts:103 — email lookup before auth.admin.createUser fallback - apps/admin/src/app/api/admin/club-credits/route.ts:34, 151 — userEmail lookups - apps/admin/src/app/api/admin/club-credits/topup/route.ts:33 — user_email lookup Recommended fix: a single sweep replacing each `.eq("email", X)` with `.eq("email", X.toLowerCase().trim())`, OR refactoring write paths (team, programs/enrollments, club-credits, etc.) to use `findOrCreateUserByEmail()` for consistency.

Discovered in: PR #548 review (assign-membership auto-create)

Each `credit_tier_configs` row has a `booking_window_days` integer (Club=10, Pro=14, All Star=21, Hall of Fame=30 at Pad Thai Padel). The admin tier-config form edits it, `calculate_credit_tier()` returns it, and the member wallet card displays it. But no code path actually uses it to gate how far in advance a player can book. A Club-tier player can currently book as far ahead as a Hall of Fame player. Needs enforcement in: admin booking create (reject if start_time > now + window), web app booking flow (disable date chips beyond the window), and OpenRally availability (filter slots). Related: BUG-CT-A (now fixed — the rolling window setting is respected) and the threshold-ordering validator (now fixed) were part of the same tc-112 battle-test sweep.

Discovered in: TC-112 credit-tier auto-leveling battle-test (2026-04-23)

Admin and terminal both support per-player check-in for multi-player COURT bookings via booking_players.checked_in_at (terminal useCheckInPlayer + admin PATCH /api/admin/bookings/[id]/players, shipped in v0.23.1 / DEC-173). LESSON bookings take a different shape: students live in lesson_players (keyed to lesson_id) which tracks enrollment and payment (paid_amount, payment_method, player_package_id) but has no checked_in_at column. Consequence: clicking Check In on a lesson booking creates ONE ticket for the booking's owner, with the court fee as the line item (not price_per_pax × player_count), and ticket_name = the booker (not each student). This is identical to terminal's current behavior — neither surface does per-student lesson check-in today. It's a SCHEMA limitation, not a limitation of either surface.

Discovered in: v0.23.1 scope audit (DEC-173 follow-up)

On the booking-search screen of the TPS mobile app, each club card renders time-slot chips. For OpenRally-routed clubs (currently: Sandbox Pad Thai Padel, club_id 625), chips for hours that have already passed today (e.g. 14:00 Bangkok time when it's already 18:00 UTC / 01:00 next-day Bangkok) are still rendered and tappable. Backend safety is NOT compromised — OpenRally's availability endpoint (PR #508) filters slots where `start_time <= now()`, so the `get-court-combination-v1` call for a past time returns `{60:[],90:[],120:[]}` and no Book button renders. The user literally cannot complete a past booking. The issue is only cosmetic: tapping a past chip shows 'no courts' or briefly displays stale combination state from a previous successful tap.

Discovered in: TPS↔Setpoint OpenRally integration E2E testing (2026-04-17)

When a booking is cancelled via the OpenRally protocol (`POST /openrally/v1/bookings/{id}/cancel`), the Rust handler in `setpoint/protocol/server/src/handlers/bookings.rs::cancel` flips `bookings.status` to 'cancelled' but does NOT populate `cancelled_at`, `cancellation_reason`, or `cancelled_by` columns. Consumer apps (TPS mobile via `processOpenRallyCancellation` in padel-backend) pass a `reason` field in the request body, but it's discarded server-side. Direct Setpoint admin/web cancels (via `/api/v1/me/bookings/[id]/cancel` → RPC) populate these fields correctly — only the OpenRally path is affected. Verified 2026-04-17 during TPS↔Setpoint integration tests on booking b645ff6d-77bc-423b-b31f-c6f7751cd4ef (TPS #56439) and 834fac86-... (TPS #56457): both show status=cancelled but cancelled_at=null, cancellation_reason=null after TPS cancellation.

Discovered in: TPS↔Setpoint OpenRally integration E2E testing (2026-04-17)

When an admin books a group lesson with multiple players, only the primary player's package is checked. If they have a package, 1 session is deducted and the entire lesson is considered paid. Other players are not charged (no cash, no card, no package deduction). A 4-player group lesson at ฿800/pax should generate ฿3,200, but currently generates 1 package session. Boss confirmed (DEC-146): group lessons are public classes, each player pays individually. Fix requires: (1) move payment_method and player_package_id from lessons table to lesson_players, (2) per-player payment UI in the lesson form, (3) consume_package_session per player, not per lesson.

Discovered in: Multi-court/multi-player testing session (2026-04-08)

generate-lessons.ts (used by player package subscriptions) defaults to a single court when generating lessons, even if the class requires num_courts > 1. The generate function reads court_ids from the schedule or falls back to one default court. No validation that court_ids.length matches class.num_courts. Not exposed in admin UI — recurring lessons are only created via Stripe subscription webhooks for player packages. Fix when recurring lessons are built as a proper admin feature (DEC-140).

Discovered in: Multi-court audit (PR #445)

POST /api/v1/lessons/hold accepts any number of court_ids without checking against class.num_courts. The admin POST /api/admin/lessons correctly validates this (returns 400 if mismatched). The public web app doesn't expose multi-court lesson booking yet, so this is not user-facing. Fix when the public web app is updated for multi-court (DEC-138).

Discovered in: Multi-court audit (PR #445)

When an admin creates a court booking or lesson booking with a guest email that does NOT match any existing profile, the system stores user_id as null (guest_email/phone only for court bookings, email/phone only in lesson_players). No Supabase account or profile is created for the guest. Both flows DO auto-link by email if a matching profile already exists: court bookings set user_id on the bookings row (POST /api/admin/bookings), lesson bookings set user_id on lesson_players rows (POST /api/admin/lessons, POST /api/admin/lessons/[id]/players). Additionally, lesson booking records in the bookings table have NO user_id at all — player association is only through lesson_players, not the bookings row. If a guest later signs up via OTP, existing bookings/lesson_players are NOT retroactively linked to the new profile (BUG-024 added email fallback queries but not retroactive linking).

Discovered in: codebase audit 2026-04-03

When an admin creates a booking via the calendar (Create Booking dialog) and enters a guest email, the booking is stored with guest_email but no user_id. If that person later logs in on the web app via OTP (which creates or matches a user account), the My Bookings page shows 'No upcoming bookings' because it queries by user_id, not guest_email. The booking exists in the database but is not linked to the user's profile.

Discovered in: TC-002 manual test (Playwright)

The web booking payment step uses Stripe PaymentElement which renders card fields inside cross-origin iframes with security restrictions. Playwright cannot programmatically fill the card number, expiry, or CVC fields. 10 Stripe iframes are detected but none expose accessible input elements. This prevents automated E2E testing of the full booking-to-payment flow.

Discovered in: TC-001 manual test (Playwright)

GET /api/admin/bookings/search interpolated the `q` parameter directly into a PostgREST .or() filter string. A crafted `q` value with commas (and balancing parens / quotes) could break out of the ilike clause and inject additional PostgREST predicates — e.g. `q=zzz,guest_email.like.b` returned bookings whose email starts with `b` even though `zzz` matched nothing. Verified locally against PostgREST. Practical impact was low because the endpoint is admin-only, scoped by `requireFacilityAccess`, and cross-facility access is enforced by a separate AND filter that the injection cannot touch — admins already have full read access to their facility's bookings via `/api/admin/bookings`. Still fixed as defense-in-depth and to avoid leaking enumeration / unintended-operator capabilities if a less-privileged role is introduced later. Not SQL injection (Supabase parameterizes); fix sanitizes `q` by stripping `,`, `(`, `)`, and `"` before interpolation, preserving emails, names, and other realistic search inputs.

Discovered in: v0.21.0 self-review

In POST /api/admin/bookings, credits are deducted via adjust_club_credits before the booking is inserted. If the booking insert fails, credits are lost with no rollback. The reference_id update is race-prone under concurrency. Should be wrapped in a DB transaction or RPC.

Discovered in: v0.21.0 self-review

After subscribing and paying via Stripe Checkout in production, the webhook event (checkout.session.completed) is delivered to https://admin-setpoint.vercel.app/api/v1/webhooks/stripe with pending_webhooks=0 (Stripe considers it delivered), but no user_memberships record was created. Root cause (reproduced locally with tc-107-membership-webhook): the handler called `await supabase.from('user_memberships').insert({...})` and discarded the returned `{error}`. When the INSERT violated the partial unique index `idx_user_memberships_active_unique` (`user_id, facility_id WHERE status='active'`) — which happens whenever a user already has an orphaned active row, most commonly from the previous Stripe account's subscriptions or from an admin manual-assign — Postgres raised a unique_violation, Supabase's client surfaced it on `.error`, the handler ignored it, and returned 200 to Stripe. Same pattern silently swallowed errors on the payments insert, invoice.paid renewal update, and subscription.deleted cancels. Fix: every mutation in the handler now checks `.error`; on unexpected errors we return 500 so Stripe retries and the failure is visible in both the Stripe Dashboard Event deliveries tab and Vercel logs. For the specific orphan-active case (active row with NULL stripe_subscription_id), the handler now UPDATEs the existing row in place to link it to the new subscription rather than trying to INSERT a duplicate. Concurrent-sub anomalies (active row with a *different* non-null sub_id) return 500 so admins see them instead of a silent overwrite.

Discovered in: PR #280 production testing

The GET /api/v1/me/membership endpoint filters by status IN ('active', 'cancelled') but does not include 'expired'. When a membership expires (end_date passes, pg_cron sets status='expired'), the portal shows 'No active membership' empty state instead of showing the expired membership with an option to renew. User has no indication they ever had a membership.

Discovered in: PR #280 subagent audit

Operating hours are set as a default (08:00–22:00) when the facility is created but there is no UI to edit them in the settings page. The only way to change operating hours is via direct SQL update on the facilities table. The facility-settings-form.tsx does not include operating_hours fields.

Discovered in: production setup — Pad Thai Padel onboarding

The Add Court dialog had a visible 'Sort Order' number input that users had to fill manually. This is a technical field that should auto-increment based on existing court count. Fixed by removing the field from the UI and auto-setting sort_order to the current court count. Also fixed snake_case placeholder in Surface Type field.

Discovered in: production setup — Pad Thai Padel onboarding

Tests in admin-programs, admin-academy, admin-memberships reference hardcoded UUIDs (00000000-...) from the old seed.sql that was deleted. Multi-facility tests depend on second facility + user roles not set up by demo-populate. Tests need rewriting to be self-contained (create own data via API).

Discovered in: v0.18.0 testing

If a facility uses JPY or KRW (truly zero-decimal in Stripe), stored values in smallest subunit would need ÷ 100 before passing to Stripe. No conversion service exists yet. THB is NOT affected (it's two-decimal).

Discovered in: v0.18.0 planning

CoachesList crashes when rendering a coach whose profile has a null email. The code calls `c.profile.email.split('@')[0]` without null-checking email. Same pattern existed in classes-list, customers-list, customer-detail-panel, team-list, and lessons-list. Fixed by adding optional chaining (`?.`) on all `.email.split()` calls across 6 files. Root cause for missing E2E coverage: existing tests only checked page loads (asserting h1 text), never tested adding a coach through the UI. Demo-populate suite creates coaches via API, bypassing the UI rendering path. Added admin-coach-creation.e2e.ts regression test.

Discovered in: production setup — Pad Thai Padel onboarding

admin-auth.e2e.ts T2 (Sign-out redirects to login page) intermittently fails because the NextJS dev overlay (`<nextjs-portal>`) intercepts pointer events on the avatar button, preventing the dropdown menu from opening. force:true bypasses actionability but the click doesn't reach the actual button.

Discovered in: hardening pass — health check

The calculate_member_price() RPC function used `IF v_membership IS NOT NULL` to check if an active membership was found. In PL/pgSQL, RECORD IS NOT NULL returns false when ANY field in the record is NULL. user_memberships rows have nullable stripe_subscription_id and stripe_price_id fields, so the check always returned false. Fixed by using the FOUND special variable instead of IS NOT NULL.

Discovered in: hardening pass — health check

The POST /api/admin/team endpoint used supabase.auth.admin.createUser() which requires HS256 service_role JWT. Local Supabase CLI v2 uses ES256 JWTs, causing all team member creation to return 403/500. Fixed by using the signup endpoint (/auth/v1/signup) instead, which works with both JWT formats.

Discovered in: hardening pass — health check

MatchJoinFlow component calls join API which creates PaymentIntent and returns client_secret, but never mounts PaymentElement or calls stripe.confirmPayment(). Player is added to match_players with payment_intent_id but the card is never charged. Fixed by adding a 'paying' step that mounts Elements + PaymentElement, following the same pattern as credit-topup-form.tsx.

Discovered in: payment E2E test planning

supabase/seed.sql had zero coach, lesson, or package seed data — only the academy itself and classes existed (seeded in migrations). This caused demo-academy-lessons and demo-web-user-journey to show empty coaches/lessons/packages tables. Additionally, the web user journey demo navigated to /portal/lessons (404) instead of /portal/academy/lessons. Fixed by adding 3 coaches, 8 schedules, 3 packages, 5 package items, 5 lessons, 5 lesson courts, and 9 lesson players to seed.sql, and fixing the portal route in the demo test.

Discovered in: demo video review

Both send-match-invite and send-match-email Edge Functions used `from: notifications@setpoint.app`, but setpoint.app is not verified in Resend. Switched to `onboarding@resend.dev` but Resend's free tier only allows sending to the account owner (gianfranco@omnigpt.co). Fixed by verifying omnigpt.co domain in Resend and updating from address to `notifications@omnigpt.co`.

Discovered in: production E2E testing

The send-match-invite and send-match-email Edge Functions were deployed but RESEND_API_KEY was not set as a Supabase secret. Both functions silently returned { sent: 0 } without attempting email delivery. Fixed by running: supabase secrets set RESEND_API_KEY=<key> --project-ref utommwznukufegwhltqw

Discovered in: production E2E testing

When creating a match that overlaps an existing booking, the API returns the raw PostgreSQL constraint name `idx_bookings_no_overlap` in the error response. The admin UI shows this raw string to the user. The API should catch this constraint violation and return a clear message like 'This court is already booked at that time'.

Discovered in: demo restructuring

The web app (apps/web) has no UI for players to submit match results, confirm submitted results, or dispute results. All 3 actions only exist as API endpoints (v1 routes). The admin app has a Resolve Dispute dialog but no submit/confirm/dispute. This means the full email notification flow cannot be demo'd end-to-end in the browser — curl is required for submit/confirm/dispute steps.

Discovered in: v0.15.1 demo preparation

calculate_elo_for_match() attempts to INSERT level=1500 (the default Elo starting rating) into player_ratings.level, which is defined as numeric(4,2) — max value 99.99. This causes a numeric overflow error and the entire auto_confirm_match_results() function to abort, meaning no pending results get auto-confirmed and no emails are sent for auto-confirmed results.

Discovered in: v0.15.1 testing

Tests T23-T31 (JWT auth validation) and T38-T42 (match results endpoints) fail consistently. These failures exist on the main branch and are not caused by any recent changes. Likely related to JWT token handling or test environment auth setup.

Discovered in: v0.15.1 testing

database.types.ts was regenerated in PR #90 to fix corrupted CLI output (PR #97 cleaned up further), but several files in application code still use (supabase as any) casts that were added as workarounds for the old broken types. These should be removed now that types are correct.

Discovered in: PR #90 / #97

All /api/admin/* routes use createAdminClient() (service role key) which bypasses RLS. The middleware skips all /api/* paths from auth checks. This means any HTTP request — authenticated or not — can call admin API endpoints including court image upload/delete. This is a pre-existing pattern across ALL admin routes (courts, coaches, lessons, schedules), not specific to court images. Fix requires: (1) add session verification to admin API routes (getUser() check), (2) add facility-level authorization (check user_facility_roles), (3) restrict middleware to only skip specific public paths, not all /api/*. Reference: apps/admin/src/middleware.ts line 15, and every route in apps/admin/src/app/api/admin/.

Discovered in: Court images security audit (PR #289)

In /api/admin/courts/[id]/images POST handler, the image count check (SELECT COUNT) and the INSERT are two separate queries. If two uploads happen concurrently, both can pass the count check and exceed the MAX_IMAGES limit (10). Fix options: (1) add a database CHECK constraint or trigger that enforces max 10 images per court, (2) use a Supabase RPC with a transaction, (3) add a unique partial index. File: apps/admin/src/app/api/admin/courts/[id]/images/route.ts, lines 62-72 (count check) and 138-148 (insert).

Discovered in: Court images security audit (PR #289)

Upload endpoint validates file type via Content-Type header (MIME type) which is client-controlled and can be spoofed. A malicious file with a spoofed image/jpeg Content-Type would pass validation. Fix: read the first few bytes of the uploaded file and validate against known image signatures — JPEG (FF D8 FF), PNG (89 50 4E 47), WebP (RIFF...WEBP). File: apps/admin/src/app/api/admin/courts/[id]/images/route.ts, line 81. Add a validateImageSignature() function that reads the ArrayBuffer and checks magic bytes before proceeding with upload.

Discovered in: Court images security audit (PR #289)

The GET /api/admin/courts/[id]/images endpoint returns storage_path in the response. This field is an internal implementation detail (Supabase Storage path) that clients don't need. Exposing it leaks internal storage structure. Fix: change the select in the GET handler from 'id, court_id, image_url, storage_path, sort_order, created_at' to 'id, court_id, image_url, sort_order, created_at'. The storage_path is only needed internally for DELETE operations (where it's fetched separately). File: apps/admin/src/app/api/admin/courts/[id]/images/route.ts, line 31.

Discovered in: Court images security audit (PR #289)

Two migration files share version 20260306000002: pos_module.sql and v0_1_openrally_protocol.sql. This causes supabase db reset to fail with 'duplicate key violates unique constraint schema_migrations_pkey'. The OpenRally migration needs to be renamed to 20260306000003. Additionally, several seed migrations (20260317000001 through 20260317000007) reference facility ID 9be75541 which only exists after demo-populate runs — they fail on fresh supabase db reset. These seeds need conditional WHERE EXISTS guards. Also, migration 20260329000001 tries to CREATE OR REPLACE a function with a changed return type — PostgreSQL requires DROP FUNCTION first. All these are pre-existing issues not introduced by court images.

Discovered in: Court images development (PR #289)

Any authenticated admin user can access ANY facility's admin panel and ALL its data by navigating to /{facility-slug}/ in the browser or calling /api/admin/* endpoints with any facility_slug parameter. This is a complete multi-tenancy breakdown across 3 layers: LAYER 1 — MIDDLEWARE (apps/admin/src/middleware.ts, line 15): The middleware skips ALL /api/* routes from auth checks: `if (pathname.startsWith("/auth") || pathname.startsWith("/api")) { return NextResponse.next(); }`. This means API endpoints receive zero authentication verification. For page routes, it only checks that a session EXISTS (user is logged in) — it never checks if the user belongs to the facility in the URL. LAYER 2 — PAGE LAYOUT (apps/admin/src/app/[facility]/layout.tsx, lines 14-24): The facility layout loads ANY facility by slug using createAdminClient(). No check against user_facility_roles table. Code: `const { data: facility } = await supabase.from("facilities").select("...").eq("slug", slug).single()`. If the facility exists, it renders. The FacilityProvider (apps/admin/src/providers/facility-provider.tsx) just passes this data through without authorization. LAYER 3 — API ROUTES (ALL files in apps/admin/src/app/api/admin/): Every admin API route uses createAdminClient() from @setpoint/database/admin which authenticates with SUPABASE_SERVICE_ROLE_KEY. This COMPLETELY BYPASSES Row-Level Security (RLS). Even though tables like court_images, bookings, courts have RLS policies that check user_facility_role(), the service role key ignores them entirely. No API route independently verifies the requesting user has a role on the target facility. WHAT AN ATTACKER CAN DO (any authenticated user against any facility): - VIEW: all courts, bookings, customers, pricing rules, team members, membership plans, classes, CRM campaigns, club credits, financial data - CREATE: courts, bookings (including recurring), pricing rules, team members (with any role including owner), membership plans, classes, CRM campaigns (can send emails to customers), promo codes - MODIFY: court details, booking status/times/prices/refunds, pricing rules, membership plans, team roles - DELETE: courts (deactivate), court images, team members, bookings (cancel) - UPLOAD: court images to any facility's courts FILES THAT NEED CHANGES (comprehensive list): 1. apps/admin/src/middleware.ts — Add facility membership check for /[facility]/* page routes 2. apps/admin/src/app/[facility]/layout.tsx — Verify user has role on facility before rendering 3. Create helper: apps/admin/src/lib/require-facility-access.ts — Reusable function that: (a) gets current user session, (b) queries user_facility_roles for user_id + facility_id, (c) returns 401 if no session, 403 if no role 4. Apply helper to ALL routes in apps/admin/src/app/api/admin/ — there are 30+ route files API ROUTES TO FIX (every file in these directories): - apps/admin/src/app/api/admin/courts/ (route.ts, [id]/route.ts, [id]/images/route.ts) - apps/admin/src/app/api/admin/bookings/ (route.ts, [id]/route.ts) - apps/admin/src/app/api/admin/customers/ (route.ts) - apps/admin/src/app/api/admin/pricing-rules/ (route.ts) - apps/admin/src/app/api/admin/membership-plans/ (route.ts) - apps/admin/src/app/api/admin/team/ (route.ts, [userId]/route.ts) - apps/admin/src/app/api/admin/classes/ (route.ts) - apps/admin/src/app/api/admin/coaches/ (route.ts, [id]/route.ts, [id]/schedule/route.ts) - apps/admin/src/app/api/admin/court-schedules/ (route.ts) - apps/admin/src/app/api/admin/lessons/ (route.ts, [id]/route.ts, [id]/players/route.ts) - apps/admin/src/app/api/admin/matches/ (route.ts, [id]/route.ts, [id]/players/route.ts) - apps/admin/src/app/api/admin/club-credits/ (route.ts) - apps/admin/src/app/api/admin/credit-tiers/ (route.ts, user-tier/route.ts) - apps/admin/src/app/api/admin/crm/ (campaigns/, customers/, tags/, segments/) - apps/admin/src/app/api/admin/facility-contacts/ (route.ts) - apps/admin/src/app/api/admin/upload/ (route.ts) - apps/admin/src/app/api/admin/waivers/ (route.ts) IMPLEMENTATION PLAN: Step 1: Create requireFacilityAccess() helper that accepts (req: NextRequest, facilityId: string) and returns { user, role } or throws 401/403 Step 2: For GET routes that take facility_slug as query param — resolve slug to facility_id, then call requireFacilityAccess() Step 3: For POST/PATCH/DELETE routes that take facility_id in body — extract it, then call requireFacilityAccess() Step 4: For nested routes like /courts/[id]/images — look up the court's facility_id first, then call requireFacilityAccess() Step 5: Update middleware to check facility membership on page routes (not just session existence) Step 6: Consider switching from createAdminClient() to user-scoped client for admin routes so RLS applies as defense-in-depth ALTERNATIVE APPROACH (simpler): Instead of adding auth checks to 30+ route files, add a single Next.js middleware rule that: 1. Extracts facility slug from URL (for pages) or facility_slug/facility_id from request (for APIs) 2. Queries user_facility_roles once 3. Blocks the request if no role found This centralizes the check but requires parsing request bodies in middleware (complex for POST). TESTING: After fix, verify: - User A (owner of Facility X) CANNOT access /facility-y/courts or call /api/admin/courts?facility_slug=facility-y - User A CAN still access /facility-x/courts - User B (no facility roles) sees empty dashboard or redirect to onboarding - E2E test: create two facilities, two users, verify cross-facility access returns 403

Discovered in: Court images security audit (PR #289)

Several public API v1 endpoints accept resource IDs directly without validating they belong to the expected facility. This allows cross-facility data access and manipulation. AFFECTED ENDPOINTS: 1. DELETE /api/v1/bookings/hold/[id] (apps/admin/src/app/api/v1/bookings/hold/[id]/route.ts): - No authentication at all - No facility_id validation - Anyone can release any booking hold by ID - Fix: Add facility_id to URL path, validate hold belongs to that facility 2. POST /api/v1/bookings/[id]/confirm (apps/admin/src/app/api/v1/bookings/[id]/confirm/route.ts): - Accepts booking ID from URL, no facility_id cross-check - Payment intent is validated but not scoped to a facility - Fix: Add facility_id validation after retrieving booking: eq("facility_id", expectedFacilityId) 3. GET /api/v1/pricing/calculate (apps/admin/src/app/api/v1/pricing/calculate/route.ts): - Accepts facility_id as query parameter from client (untrusted input) - Any user can query any facility's pricing structure - Fix: Derive facility_id from the URL slug, not from client params 4. POST /api/v1/promo-codes/validate (apps/admin/src/app/api/v1/promo-codes/validate/route.ts): - Accepts facility_id from request body (untrusted input) - Can enumerate promo codes across facilities - Fix: Derive facility_id from authenticated context or URL slug 5. GET /api/v1/matches/[id] (apps/admin/src/app/api/v1/matches/[id]/route.ts): - Returns match details without facility scoping - Exposes player names, emails, organizer info, pricing - No visibility check for private matches - Fix: Add facility_id to URL, check match.facility_id matches, respect visibility field 6. POST /api/v1/lessons/hold (apps/admin/src/app/api/v1/lessons/hold/route.ts): - Accepts facility_id from request body with no validation - Can block lesson slots at any facility - Fix: Validate facility_id against the coach's academy facility IMPLEMENTATION PLAN: Step 1: For endpoints under /api/v1/facilities/[slug]/* — facility_id is already derived from slug (correct pattern, already used by courts and availability endpoints) Step 2: Move endpoints that accept facility_id as params to be nested under /api/v1/facilities/[slug]/ instead Step 3: For direct-ID endpoints (/bookings/[id], /matches/[id]) — after fetching the resource, validate its facility_id matches the expected facility from the URL or auth context Step 4: Add rate limiting to hold/release endpoints to prevent DOS

Discovered in: Court images security audit (PR #289)

The Academy page showed lesson packages for purchase even when the facility had no active coaches or coach schedules. A player could buy a package they couldn't use.

Discovered in: v0.20.0 manual testing

apps/admin/src/app/api/admin/crm/campaigns/webhook/route.ts returns true from verifySignature() when RESEND_WEBHOOK_SECRET env var is not set, allowing unsigned payloads to be processed. An attacker could fabricate email events (opened, clicked, bounced) to pollute campaign analytics.

Discovered in: BUG-015 security audit (PR #340)

The admin app can only "assign" packages to players via POST /api/admin/player-packages (apps/admin/src/app/api/admin/player-packages/route.ts:84-155). This endpoint accepts an optional stripe_payment_intent_id but never creates a PaymentIntent — it just stores whatever ID is passed (or null). There is no admin UI or API to charge a player's card for a package. Compare with the public web app flow which works correctly: 1. POST /api/v1/lesson-packages/[id]/purchase → creates Stripe PaymentIntent, returns client_secret 2. Frontend collects card via Stripe PaymentElement 3. POST /api/v1/lesson-packages/[id]/confirm-purchase → verifies payment succeeded, creates player_package + sessions + payment record The admin app needs the same two-step flow: create PaymentIntent for the player, collect card (or use saved payment method), then confirm. The admin should be able to sell session_bank packages by card just like the public app does for self-purchase. Existing code to reference: - Public purchase flow: apps/admin/src/app/api/v1/lesson-packages/[id]/purchase/route.ts (creates PaymentIntent) - Public confirm flow: apps/admin/src/app/api/v1/lesson-packages/[id]/confirm-purchase/route.ts (verifies + creates records + inserts payment row) - Admin assign (current): apps/admin/src/app/api/admin/player-packages/route.ts:84-155 (no payment processing) - Admin UI: apps/admin/src/components/admin/player-packages-list.tsx (management only, no purchase dialog)

Discovered in: v0.20.0 package audit

When an admin assigns a package via POST /api/admin/player-packages (apps/admin/src/app/api/admin/player-packages/route.ts:84-155), the endpoint creates player_packages and player_package_sessions rows but never inserts a row into the payments table. This means cash sales have no financial audit trail. Compare with the public card purchase flow in apps/admin/src/app/api/v1/lesson-packages/[id]/confirm-purchase/route.ts:100-111 which correctly inserts a payment record: await supabase.from("payments").insert({ facility_id, reference_id: playerPkg.id, type: "booking", amount: paymentIntent.amount, currency: paymentIntent.currency, status: "succeeded", stripe_payment_id: payment_intent_id, payment_method: "card", user_id: user.id, }); The admin assign endpoint needs to also insert a payment record. For cash sales: payment_method: "cash", stripe_payment_id: null, amount from the package's price_amount. For free/comp assignments: payment_method: "comp" (or similar), amount: 0. Additionally, the player_packages table has no payment_method column, so there is no way to distinguish after the fact whether a package was purchased by card, cash, or given for free. Consider adding a payment_method column to player_packages (values: "card", "cash", "comp") alongside the existing stripe_payment_intent_id. Files to modify: - apps/admin/src/app/api/admin/player-packages/route.ts — add payment record insertion + accept payment_method in body - Database migration — add payment_method column to player_packages table - apps/admin/src/components/admin/player-packages-list.tsx — display payment method in the list

Discovered in: v0.20.0 package audit

The admin app has no flow to create Stripe Subscriptions for recurring packages on behalf of a player. The public web app has POST /api/v1/lesson-packages/[id]/subscribe (apps/admin/src/app/api/v1/lesson-packages/[id]/subscribe/route.ts) which creates a Stripe Subscription with payment_behavior: "default_incomplete", returns a client_secret for the player to complete payment, and then the checkout.session.completed webhook creates the player_package. The admin app needs an equivalent flow: 1. Admin selects a recurring package and a player 2. API creates a Stripe Subscription for the player's Stripe customer 3. Admin collects card payment (or uses player's saved payment method) 4. Webhook (or confirm endpoint) creates player_package + sessions + generates lessons via generateLessonsForPeriod() The subscribe endpoint already exists for public use. The admin flow could either reuse it (passing user_id from admin context instead of JWT) or create a parallel admin endpoint. The webhook handler in apps/admin/src/app/api/v1/webhooks/stripe/route.ts already handles checkout.session.completed for recurring packages (lines 27-224), so the backend is ready — only the admin API trigger + UI is missing. Existing code to reference: - Public subscribe: apps/admin/src/app/api/v1/lesson-packages/[id]/subscribe/route.ts - Webhook handler: apps/admin/src/app/api/v1/webhooks/stripe/route.ts (checkout.session.completed, invoice.paid) - Lesson generation: apps/admin/src/app/api/admin/player-packages/_lib/generate-lessons.ts

Discovered in: v0.20.0 package audit

The public web app (apps/web) only supports Stripe card payments for package purchases. The PackagePurchaseDialog (apps/web/src/components/academy/package-purchase-dialog.tsx) goes directly to Stripe PaymentElement — there is no option for a player to indicate they want to pay cash at the front desk. For facilities that accept walk-in cash payments, the flow should be: 1. Player selects a package on the public web app 2. Player chooses "Pay at Front Desk" (cash) instead of card 3. This creates a pending/held package purchase request visible to the admin 4. Admin confirms cash received, which activates the package (similar to how booking holds work) Alternatively, this could be admin-only (admin sells cash packages on behalf of walk-in players via BUG-027 fix). In that case, the public web app doesn't need cash — but the admin app must support it properly with payment records (see BUG-027). Current public web app files: - Package card UI: apps/web/src/components/academy/package-card.tsx - Purchase dialog: apps/web/src/components/academy/package-purchase-dialog.tsx (Stripe-only) - Payment step: apps/web/src/components/academy/step-lesson-payment-method.tsx (shows packages for lesson payment, not package purchase)

Discovered in: v0.20.0 package audit

The bookings.status CHECK constraint only allowed 'hold', 'confirmed', 'cancelled', 'completed', 'no_show', 'expired'. The value 'checked_in' was used throughout the UI and API but was never added to the database constraint, causing check-in to fail with a constraint violation error. Fixed by migration 20260406132142_add_checked_in_to_bookings_status.sql which adds 'checked_in' to the constraint.

Discovered in: v0.22.0 manual testing

The Terminal app (apps/terminal) formats booking start_time and end_time using the browser's local timezone via date-fns format(), instead of the facility's configured timezone. A booking created at 7:00 AM Bangkok time (UTC+7) displays as 8:00 AM if the browser is in UTC+8. The admin app correctly uses facilityLocalToUTC() from @setpoint/common/datetime for storage and Intl.DateTimeFormat with the facility timezone for display. The Terminal needs to adopt the same pattern — format times using the facility's timezone from the auth context, not the browser's timezone. This is a display-only bug. The database stores correct UTC values.

Discovered in: v0.22.0 cross-app manual testing

The facilityLocalToUTC() function in @setpoint/common/datetime uses the naive UTC date as the reference point for computing the timezone offset. During DST transition hours, this can produce an offset that is off by 1 hour. The code acknowledges this in comments (lines 98-101). Not an issue for Thailand (no DST), but becomes a bug when expanding to DST-observing countries (US, Europe, Australia). The fix would be to iterate: compute offset, apply it, recompute offset at the resulting UTC time, and verify convergence.

Discovered in: v0.22.0 code review