Skip to content

Booking subsystem production-readiness audit — master tracker #676

Description

@teetangh

Booking Subsystem Production-Readiness Audit — Master Tracker

Scope: this is a single canonical tracker for the booking subsystem end-to-end (Prisma schema booking models, docs/booking/* 17 files, runtime code under app/api/**, lib/payments, lib/waitlist, utils/slotAllocation, utils/appointmentlock, jobs/appointments). It cross-links existing issues rather than duplicating them, and surfaces new findings from a fresh audit wave on 2026-04-17.


TL;DR


Method

Phase 1: three parallel Explore agents read (a) all 17 booking docs, (b) the 3 057-line Prisma schema + 3 recent migrations, (c) booking-adjacent runtime code and crons.

Phase 2: spot-verification of load-bearing claims against actual source. Five code-agent claims were disproved (listed under "Verification corrections" below) and excluded from the findings. Docs-agent and schema-agent findings held up.

Phase 3: cross-reference against existing open issues so we don't restate known work.


Production-readiness scorecard (booking subsystem)

Dimension Score Blocker to higher
Functional correctness 80% concurrency edges on reschedule + cancel, refund two-phase orphan (B1)
Data integrity (schema constraints) 65% 5 P1 CHECK constraints missing (A1–A4); no DB overlap exclusion (#440)
Concurrency / locking 75% cron distributed lock missing (#476); optimistic versioning absent (A11)
Timezone / DST 70% #503 (7 items) unresolved
Observability / audit trail 70% no CancellationAuditLog / RescheduleLog (#448, B4); doc C4/C6 gaps
Documentation accuracy 85% small drift (C1–C8), mostly accurate
Test coverage for booking flow 65% no race-condition CI gate (see #545 referenced by #613)
Overall ~75%

Target trajectory

After Score
Phase 1 (schema + refund) ~82%
Phase 2 (concurrency + timezone) ~89%
Phase 3 (observability + UX) ~94%
Phase 4 (polish + docs) ~98%
Phase 5 (verification) 100%

Findings

P0 — none new

All true showstoppers (camera/microphone Permissions-Policy, config blockers, SSO veto) are already filed in #480 and #673. Nothing new at P0 surfaced.


P1 — New findings (not yet tracked)

A1. SlotOfAppointment missing CHECK (endsAt > startsAt)

File: prisma/schema.prisma:1978-1979
Impact: a misbehaving client (or a later refactor) can insert a slot with endsAt <= startsAt; every downstream duration/availability calculation silently breaks.
Fix: raw SQL migration

ALTER TABLE "SlotOfAppointment"
  ADD CONSTRAINT slot_time_order CHECK ("endsAt" > "startsAt");

A2. Payment.amount / originalAmount allow zero or negative

File: prisma/schema.prisma:2125-2126
Impact: a Payment with amount = 0 or negative bypasses revenue accounting, corrupts earnings, and will be silently rejected by some gateways but accepted by others.
Fix:

ALTER TABLE "Payment"
  ADD CONSTRAINT payment_amount_positive
  CHECK ("amount" > 0 AND "originalAmount" > 0);

Allow 0 only if a free/mock-payment path is ever needed — currently isMockPayment flag exists (line 2136) but amount is still expected to be >= 1. Confirm before narrowing.

A3. Plan prices can be negative

Files: prisma/schema.prisma:1336 (ConsultationPlan.price), 1401 (SubscriptionPlan.price). Check WebinarPlan.price and ClassPlan.price in the same migration.
Fix:

ALTER TABLE "ConsultationPlan" ADD CONSTRAINT consultation_plan_price_nonneg CHECK ("price" >= 0);
ALTER TABLE "SubscriptionPlan" ADD CONSTRAINT subscription_plan_price_nonneg CHECK ("price" >= 0);
-- repeat for WebinarPlan, ClassPlan

A4. maxParticipants can be 0 or negative

Files: prisma/schema.prisma:1598 (WebinarPlan), 1668 (ClassPlan).
Impact: capacity semantics break; waitlist promotion logic divides by maxParticipants in places.
Fix:

ALTER TABLE "WebinarPlan" ADD CONSTRAINT webinar_plan_max_participants_min CHECK ("maxParticipants" >= 1);
ALTER TABLE "ClassPlan"   ADD CONSTRAINT class_plan_max_participants_min   CHECK ("maxParticipants" >= 1);

B1. Refund two-phase commit can orphan PENDING rows

Files:

  • app/api/payments/refunds/route.ts:163-282 (create → gateway → status-update)
  • app/api/webhooks/razorpay/route.ts:231-292 + handleRefundCreated

Flow today:

  1. Phase 1 (in tx, lines 163-176): create Refund { status: PENDING, refundId: "pending_${uuid}" }. This placeholder is chosen to reserve the amount and prevent double-refunds via the totalRefundedOrPending check (line 147-149).
  2. Phase 2 (outside tx, lines 248-268): call Razorpay. On gateway error → mark FAILED (OK).
  3. Phase 3 (line 275-282): update the row with the real Razorpay refund ID + SUCCEEDED.

Gap: if Phase 3 fails (DB blip, connection drop after gateway success), the row stays PENDING with the placeholder refundId. The webhook (refund.createdhandleRefundCreated) keys on the Razorpay refund ID and can't find the row, so it creates a second row or no-ops — leaving the orphan PENDING forever. The next refund attempt sees the PENDING in totalRefundedOrPending (line 147-149) and is blocked.

Fix (sketch):

  • Add Refund.clientRefundId or reuse metadata.clientUuid so the webhook can reconcile by our own UUID.
  • On webhook, if a Razorpay refund ID doesn't match, look up by metadata.clientUuid before inserting a new row.
  • Add a reconciliation cron (alongside reconcile-refunds) that, for any PENDING row older than N minutes with a pending_* refundId, queries Razorpay refunds list and either marks succeeded or failed.

Keep the amount-reservation property (PENDING blocks retries) — only the ID reconciliation is broken.


P1 — Already tracked (status check only)

Topic Issue Status
Consultant slot overlap (DB-level exclusion constraint with btree_gist) #440 Open, deferred (needs Supabase migration + consultantProfileId denormalization)
Timezone / DST (7 high-risk areas: offset heuristic, overnight triple-representation, overlap-detection tests, custom midnight crossing, month overflow, DST weekly processing, reschedule window) #503 Open, 3-5d estimate
Distributed locking for cron jobs #476 Open; wraps 28+ app/api/cleanup/*/route.ts
Reschedule UX / notifications / audit trail / staff visibility / toast copy (7 sub-issues) #448 Open
CI gate on booking concurrency + payment-failure lifecycle #545 Open (referenced by #613)
Production readiness umbrella (showstoppers, config, security, DNS, scaling) #480 Open
Audit scorecard baseline #613 Open, snapshot dated 2026-03-31

P2 — New findings

A6. Payment.expiresAt uses bare DateTime, not @db.Timestamptz

File: prisma/schema.prisma:2135
Impact: PostgreSQL TIMESTAMP stores without zone; multi-region deployments or dev-vs-prod TZ mismatches can drift.
Fix: change to expiresAt DateTime? @db.Timestamptz. Same migration should sweep any other bare DateTime fields.

A7. Refund.createdAt / updatedAt bare DateTime

File: prisma/schema.prisma:2220-2221
Fix: @db.Timestamptz on both.

A8. Audit-field strings not FKs

Files: Consultation.cancelledBy (prisma/schema.prisma:1381), AppointmentDocument.reviewedBy (1901)
Impact: orphan user IDs possible; joins awkward; audit reports unreliable.
Fix: convert to proper User? relations (cancelledByUser, reviewedByUser) with onDelete: SetNull.

A9. SlotOfAppointment.user[] is an implicit many-many

File: prisma/schema.prisma:1976
Impact: can't track per-attendee metadata (confirmed-at, status, added-by). For webinars/classes this limits RSVP tracking.
Fix: introduce explicit SlotOfAppointmentAttendee { slotId, userId, confirmedAt, status } join model.

A10. No soft-delete on financial records

Affects: Appointment, SlotOfAppointment, Payment, Refund
Impact: cascade-delete path wipes audit history. Compliance risk for a financial SaaS.
Fix: add deletedAt DateTime? @db.Timestamptz + @@index([deletedAt]); migrate delete paths to soft-delete.

A11. No row-version on SlotOfAppointment for optimistic concurrency

File: prisma/schema.prisma:1973
Impact: concurrent reschedule/allocate operations race; Redis lock + updatedAt is the only guard. Once lock TTL misfires (during long txns), silent overwrite is possible.
Fix: add version Int @default(0); increment on every write; include in where for updates.

B2. Webhook idempotency fallback key is non-deterministic

File: app/api/webhooks/razorpay/route.ts:131-138

const entityId =
  event.payload?.payment?.entity?.id ||
  event.payload?.order?.entity?.id ||
  // ...
  `noid_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
eventId = `${eventType}:${entityId}`;

Impact: for payloads where no entity ID is resolvable (malformed / new event types), the fallback generates a new random key on every replay → logWebhookEvent returns isNew: true → handler runs twice.
Fix: replace the random fallback with a SHA-256 of the raw body: ```ts
crypto.createHash("sha256").update(body).digest("hex").slice(0, 32)


#### B3. Refund rounding direction not documented
**File:** `app/api/payments/refunds/route.ts:155`
`refundAmount = amount || payment.amount - totalRefundedOrPending` — partial refunds in paise may produce fractions when applied via percentage policy. No explicit rounding.
**Fix:** document and enforce "round **down**" (toward zero) so the platform never over-refunds; add an inline comment + a helper `floorToPaise(n)`.

#### B4. No `CancellationAuditLog`
**Files:** `prisma/schema.prisma` (Consultation/Subscription have `cancelledAt`/`cancelledBy`/`cancellationReason` on the event itself)
**Impact:** once the event is soft-cancelled, the cancellation metadata is mutable; no immutable "who did what, when" record. Complements the `RescheduleLog` proposal in #448.
**Fix:** add
```prisma
model CancellationAuditLog {
  id                String             @id @default(uuid())
  appointmentId     String
  cancelledByUserId String
  cancelledAt       DateTime           @default(now()) @db.Timestamptz
  reason            CancellationReason
  notes             String?            @db.Text
  eventSnapshot     Json               // appointmentType + relevant ids at cancel time
  @@index([appointmentId])
  @@index([cancelledByUserId])
  @@index([cancelledAt])
}

Documentation drift — docs/booking/*

Each item below has a concrete before/after patch.

C1. 01-architecture.md — wrong tentative-slot TTL

Gap: Tentative Appointment Lifecycle diagram says "30 min cleanup". Actual cron TTL is 7 days (TENTATIVE_EXPIRATION_DAYS = 7 in app/api/cleanup/tentative-slots/route.ts; documented in 13-cron-jobs-and-background-tasks.md).
Patch:

- Tentative → CleanedUp: Abandoned (30 min)
+ Tentative → CleanedUp: cron sweep after 7 days
+   (see 13-cron-jobs-and-background-tasks.md §cleanup-tentative-slots)

C2. 07-rescheduling-flow.md — stale "Phase 1" roadmap

Gap: Known Issues #2 ("status always PENDING on partial reschedule") and #5 ("toast shows slot count") marked "Phase 1" with no PR reference. #448 now tracks the broader reschedule UX work.
Patch: either (a) remove the in-doc roadmap and link to #448, or (b) add PR references once #448's sub-issues land.

C3. 09-trial-sessions.md — rejection path under-specified

Gap: PENDING → REJECTED transition shown but (i) who rejects — consultant only? admin? and (ii) which Novu template fires.
Patch: add under "Status Lifecycle":

  • PENDING → REJECTED: initiated by the consultant from their Trials tab. Fires Novu workflow trial-rejected (consultee channel: email + in-app).

C4. 11-waitlist-system.md — cron catch-up not stated

Gap: hourly cron expires notified-but-unresponded entries — doesn't state behaviour when a cron hour is skipped (infra outage).
Patch: add a line:

Expiration is idempotent by expiresAt: the next cron run processes all rows whose expiresAt <= now() regardless of how many hours were skipped.

C5. 02-event-types-and-validation.md — "1 call per day" unit undefined

Gap: Subscription rules say "Max 1 call per day". "Day" is ambiguous (UTC? consultant-local?). Current implementation uses UTC midnight (cross-check utils/slotAllocation/SlotValidationService.ts).
Patch: change rule to

Max 1 call per UTC day (measured from 00:00 UTC to 23:59:59 UTC).

C6. 13-cron-jobs-and-background-tasks.md — HTTP 207 operator response unstated

Gap: reconcile-slot-availability returns HTTP 207 on double-booking detection; doc doesn't say who's paged.
Patch: add under §reconcile-slot-availability:

On 207 Multi-Status, the response body includes doubleBookings[]. These require manual SRE review — no automatic remediation. See runbook docs/infrastructure/runbooks/booking-conflict.md (TODO).

C7 (P3). Missing ADR doc

Gap: no 00-architecture-decisions.md explaining 30-min slot granularity, Upstash vs self-hosted Redis, Prisma over raw SQL, Razorpay primary + Stripe secondary, etc.
Patch: create docs/booking/00-architecture-decisions.md as a short ADR register (one entry per decision: context → decision → consequences).

C8 (P3). 10-checkout-payment-integration.mdPayment.appointmentId write path undocumented

Gap: the doc doesn't spell out that checkout writes Payment.appointmentId and the webhook reads it. New contributors grep'ing for "who sets this" find nothing.
Patch: add to the data flow diagram and a one-liner:

Payment.appointmentId is set by the checkout handler (lib/payments/operations/checkout.ts) when the tentative appointment is created; the Razorpay webhook reads it during handlePaymentSuccess to confirm the booking.


Verification corrections (what the audit agents got wrong)

Logged here so future readers can trust the table above. Five code-agent claims were disproved against current source:

Claim from agent Reality
"refund.created webhook handler missing" Handler is present at app/api/webhooks/razorpay/route.ts:231-261 (also refund.processed, refund.failed at 263-292, refund.speed_changed at 295-300).
"No reschedule route handler" Route exists at app/api/appointments/[appointmentId]/reschedule/route.ts. #448 tracks its UX gaps, not its absence.
"Waitlist position race — infinite loop possible" lib/waitlist/slot-handler.ts:70-84 uses updateMany guarded by status: WAITING. Only one concurrent caller wins; the loser retries via i-- and getNextInQueue returns null on empty queue, terminating the loop.
"CANCELLED slots still block bookings" utils/slotAllocation/occupancyPolicy.ts:23-28 filters at the event level using OCCUPIED_REQUEST_STATUSES = [PENDING, APPROVED, APPROVED_PENDING_PAYMENT, SCHEDULED]. Cancelled events are already excluded.
"Slot boundary overflow at slotTimeUtils.ts:375-383" Misread — those lines are the overnight branch. The same-day branch (365-369) already enforces candidateEndMinutes <= availEndTimeUtc.

Roadmap to 100%

Phase 1 — Schema & refund hardening (P1, ~3d)

Phase 2 — Concurrency & timezone (P1, ~5d)

Phase 3 — Observability & UX (P1/P2, ~3d)

Phase 4 — Polish (P2/P3, ~3d)

Phase 5 — Verification (ongoing, ~2d)

  • Race test: two concurrent checkouts on the same slot → exactly one succeeds.
  • DST transition tests: Mar 8 2026, Nov 1 2026 (US); Mar 29 2026, Oct 25 2026 (EU).
  • Webhook replay test: duplicate payment.captured → exactly one side-effect.
  • Refund orphan test: kill DB mid-Phase-3 after gateway call → reconciliation cron recovers.
  • Target: 100% subject to no new discoveries.

Cross-references

Booking / scheduling / concurrency / docs:
#440 · #448 · #471 · #472 · #476 · #503 · #545 · #309

Production / infra / security:
#480 · #481 · #407 · #534

Payments / payouts / refunds (adjacent context):
#613 · #630 · #535 · #531


Generated from audit run on 2026-04-17 on branch feature/enterprise. Methodology: three parallel Explore subagents (docs, schema, code) with spot-verification pass against the source tree. See plan file /home/kaustav/.claude/plans/please-go-through-the-mossy-nova.md for exploration transcripts and verification notes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bookingBooking, scheduling, slots, reschedule, cancellationenhancementNew feature or requestlaunch: pre-mvpGates launch — money, data, or a failure we would not detectpriority: highHigh priority — needs attention soonproductionProduction deployment and readiness

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions