Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 38 additions & 11 deletions .github/workflows/migrate-production.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,26 @@ jobs:
run: |
# Mask defensively even though the secrets machinery already does.
echo "::add-mask::$DATABASE_URL"
# prisma migrate deploy is additive and idempotent; every migration
# in this repo uses ADD/DROP COLUMN IF (NOT) EXISTS to tolerate
# prisma migrate deploy is additive and idempotent; the repo's
# migrations use IF (NOT) EXISTS / DO-block guards to tolerate
# databases that were previously synced via `prisma db push`.
#
# If deploy fails with P3018 (a migration was recorded as failed in
# _prisma_migrations, blocking the queue), we resolve it as "applied"
# — safe because the SQL is already present in the DB — then retry.
# We loop up to 5 times to handle the case where multiple migrations
# need resolving.
# Two self-healing cases are handled so the workflow stays green
# against a db-push'd production database (issue #245):
#
# 1. P3018 — a migration is already recorded as failed in
# _prisma_migrations, blocking the queue on a subsequent run.
# 2. First-pass collision — a not-yet-applied migration errors with
# a Postgres "already exists" error (e.g. 42P07 relation / 42710
# type / 42P06 schema) because the object predates the migration
# chain. Prisma records that migration as failed and exits
# non-zero on the SAME run, but reports the raw DB error rather
# than the P3018 code, so it must be matched separately.
#
# In both cases the failed migration is resolved as "applied" — safe
# because the SQL is idempotent and the objects are already present —
# then deploy is retried. We loop up to 5 times to handle the case
# where several migrations need resolving in sequence.
#
# Use set +e + PIPESTATUS[0] to capture prisma's exit code reliably
# regardless of whether bash pipefail is set; the `if !` pattern can
Expand All @@ -97,11 +108,27 @@ jobs:
echo "::error::migrate deploy still failing after ${MAX_ATTEMPTS} attempts — see log above."
exit 1
fi
if grep -q "P3018" /tmp/migrate-deploy.log; then
echo "::warning::P3018 detected (attempt ${ATTEMPT}) — resolving blocked migration and retrying…"
FAILED=$(grep "Migration name:" /tmp/migrate-deploy.log | awk '{print $NF}')

# Recognise either the blocked-queue (P3018) case or a first-pass
# "already exists" collision on a db-push'd database.
COLLISION=""
if grep -qiE "already exists|42P07|42710|42P06|42701" /tmp/migrate-deploy.log; then
COLLISION="1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLSTATE grep matches migration timestamps

Medium Severity

The self-heal branch treats a failed prisma migrate deploy as a db-push collision when the log matches 42710 or 42701 anywhere. Those five-digit sequences also appear inside 14-digit migration folder timestamps in normal “Applying migration …” lines, so an unrelated failure (syntax, connectivity, constraint) can still set COLLISION and run migrate resolve --applied on the wrong migration.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4c00155. Configure here.

fi
if grep -q "P3018" /tmp/migrate-deploy.log || [ -n "$COLLISION" ]; then
if [ -n "$COLLISION" ] && ! grep -q "P3018" /tmp/migrate-deploy.log; then
echo "::warning::Object-already-exists collision (attempt ${ATTEMPT}) on a previously db-push'd database — resolving the failed migration and retrying…"
else
echo "::warning::P3018 detected (attempt ${ATTEMPT}) — resolving blocked migration and retrying…"
fi
# Prisma prints the offending migration as "Migration name: <name>";
# fall back to the latest applied/failed folder name if absent.
FAILED=$(grep -i "Migration name:" /tmp/migrate-deploy.log | tail -n1 | awk '{print $NF}')
if [ -z "$FAILED" ]; then
FAILED=$(grep -oE "[0-9]{14}_[a-zA-Z0-9_]+" /tmp/migrate-deploy.log | tail -n1)
fi
if [ -z "$FAILED" ]; then
echo "::error::Could not extract the failed migration name from the P3018 log."
echo "::error::Could not extract the failed migration name from the deploy log."
exit 1
fi
echo "Resolving as applied: $FAILED"
Expand Down
1 change: 1 addition & 0 deletions docs/KNOWN_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ go-live checklist.
| KI-016 | 36 | Info | Deferred from Phase 36 (documented, not built): ~~batch "book all horses at a yard" (H)~~ **H shipped in Phase 40** — one appointment covering N horses via `specificHorses` (horse checklist + `horses` deep-link param on `/appointments/new`; entry points on the yard page and recalls yard groups). Still deferred: opt-in vet geolocation on the planner (L), full-flow intake→confirmation e2e (O). | Pick L/O up in a later phase; neither blocks the demo. |
| KI-017 | 36 | Info | **Producer side closed in Phase 44**: the app now emits a fire-and-forget `customer-upserted` n8n event from customer create (`POST /api/customers`) and update (`PATCH /api/customers/[id]`) via `emitN8nEvent`, with payload `{ action: 'created'\|'updated', customerId, vetupOwnerId, fullName, firstName, lastName, email, mobilePhone }` — a no-op without `N8N_WEBHOOK_URL`, and emission can never block or fail the API response. The **consumer** — workflow `n8n/10-customer-to-vetup.json` — remains **inactive** with its `BLOCKED-ON-VETUP-API` placeholder (the Vetup create endpoint is unconfirmed; same blocker class as KI-014). | Once Vetup support confirms their API: fill workflow 10's placeholder URL + auth and activate it. No further app-side change is needed. |
| KI-018 | 37 | Info | The recurring "click a client/horse and it errors" failure was deployed code running ahead of the prod schema (Prisma `P2022` → API 500). `.github/workflows/migrate-production.yml` now auto-runs `prisma migrate deploy` on every merge to main that touches `prisma/` — **but only once the repo secret `PRODUCTION_DATABASE_URL` is set**. The same missing secret is the root cause of the seed-workflow failure tracked as issue #190 (`seed-demo-database.yml` errors out with "No DATABASE_URL secret set"). | Set the `PRODUCTION_DATABASE_URL` repo secret once (Settings → Secrets and variables → Actions); both workflows then work. Detail pages now also show an explicit migration-hint error card instead of a silent blank page. |
| KI-028 | 45 | Fixed | **"Migrate production database" failed red on `main`** (issue #245, commit `3ce0f13`, Phase 2/#244). Migration `20260612000000_inventory_stock_movements` used bare `CREATE TYPE` / `ADD COLUMN` / `CREATE TABLE`, which collide on a database historically synced with `prisma db push` (production was — see KI-018). The collision surfaced as a Postgres "already exists" error on the **first** `prisma migrate deploy`, exiting non-zero with the raw DB error rather than the `P3018` code; the workflow's retry loop only matched `P3018`, so it bailed. A later merge saw the now-blocked queue as `P3018`, auto-resolved it, and unblocked deploy — so production self-healed and is migrated/seeded, but the SQL and the workflow were both fragile. | **Fixed (this session):** (1) the migration SQL is now idempotent — enum + FK constraints guarded by `DO $$ … IF NOT EXISTS … $$`, column/table/index use `IF NOT EXISTS`, so a replay against a db-push'd or clean DB is a no-op where objects exist. (2) `migrate-production.yml`'s self-heal loop now also matches the first-pass collision class (`already exists` / `42P07` / `42710` / `42P06` / `42701`), resolves the failed migration as applied, and retries — so this failure class can't go red again. The owner action remains: keep the `PRODUCTION_DATABASE_URL` repo secret set (no secret → the workflow still SKIPS green). |
| KI-019 | 38 | Info | **Two scheduling surfaces read two different models.** My Day + Calendar + Planning read **route-run stops** (`RouteRun`/`RouteRunStop`, by `runDate`); Dashboard + Appointments read the **`Appointment`** model (`appointmentStart`). The demo seed creates many route runs but only a handful of appointments, so the Dashboard "today" count and the Appointments list look sparse next to a full My Day/Calendar. This is the booking-lifecycle duality (a planned stop becomes an Appointment only when confirmed), **not** a data-loss bug. For demos, treat My Day/Calendar/Planning as "the schedule". | **Partially unified in PR #229**: `/api/calendar` now merges Appointment-model bookings (PROPOSED/CONFIRMED/COMPLETED) into the consolidated grid — assigned ones in their practitioner's column, unassigned ones in a synthetic "Unassigned" lane — so manually-booked appointments no longer leave the calendar blank. My Day **Week + All practitioners** shares that endpoint and gets them too. **Residual closed in Phase 40**: `/api/my-day` now merges the same status trio (PROPOSED/CONFIRMED/COMPLETED) into the day view and the single-practitioner week, bucketed per assignee, with the synthetic "Unassigned" lane in the all-practitioners view. Dedup rule: an appointment is suppressed in a lane only when its `routeRunId` is already rendered there as a run — manual bookings always show, route-born twins never double-render. The calendar **week grid** intentionally still shows both a booked run's stops and their appointments (pre-existing PR #229 behaviour, unchanged this phase). |
| KI-020 | 38 | Fixed | Calendar previously **dropped any stop without a `plannedArrival`**, so a manually-planned/imported run could appear on My Day (which lists every stop) yet vanish from the Calendar — a confusing "same week, opposite result". The reported live instance was a **stale Calendar tab** (loaded before the production seed finished; the query itself is correct — proven by replay → 18 placed items), but the silent-drop was a genuine latent divergence. | **Fixed in PR #226**: Calendar now falls back to the run's `runDate` (placed at the practice day-start) when a stop has no arrival time, so My Day and Calendar can't disagree by construction; `seed-my-day` dates are anchored to midday UTC for timezone stability; regression tests added. |
| KI-021 | 38 | Fixed | My Day **Week + "All practitioners"** had no real layout: it fell through to the day-view lanes, dumping each practitioner's whole week of stop cards as one flat undated stack — no day grouping, no side-by-side comparison, so the head vet could not see all practitioners' weeks at once (UX review "Req 4"). | **Fixed in PR #226**: that state now renders the consolidated calendar week grid (`CalendarWeekGrid` on `lg+`, `CalendarStackedList` on mobile) fed by `/api/calendar`, i.e. the exact components + endpoint the Calendar page uses, so the two surfaces stay in lockstep. The anchor date snaps to Monday (shared `mondayOf` helper in `lib/calendar/calendar-grid.ts`) while the single-practitioner week keeps its rolling 7-day window. |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
-- CreateEnum
CREATE TYPE "StockMovementType" AS ENUM ('ADJUSTMENT', 'RESTOCK', 'USAGE', 'DISPOSAL');
-- Phase 2 §21 — inventory stock movements.
--
-- 2026-06-12 hardening (issue #245): the original form of this migration used
-- bare `CREATE TYPE` / `ADD COLUMN` / `CREATE TABLE`, which fail on a database
-- that was historically synced with `prisma db push` (production was) because
-- the objects already exist. That surfaced as a non-P3018 error on the first
-- `prisma migrate deploy`, which the workflow retry loop did not catch.
--
-- The house rule is additive + idempotent migrations. Every statement below is
-- now guarded so it is a no-op when the object already exists, making the
-- migration safe to (re)apply against both clean migration-chain databases and
-- previously db-push'd ones.

-- CreateEnum (idempotent: skip if the type already exists)
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'StockMovementType') THEN
CREATE TYPE "StockMovementType" AS ENUM ('ADJUSTMENT', 'RESTOCK', 'USAGE', 'DISPOSAL');
END IF;
END
$$;

-- AlterTable
ALTER TABLE "Product" ADD COLUMN "stockManagedLocally" BOOLEAN NOT NULL DEFAULT false;
ALTER TABLE "Product" ADD COLUMN IF NOT EXISTS "stockManagedLocally" BOOLEAN NOT NULL DEFAULT false;

-- CreateTable
CREATE TABLE "StockMovement" (
CREATE TABLE IF NOT EXISTS "StockMovement" (
"id" TEXT NOT NULL,
"productId" TEXT NOT NULL,
"type" "StockMovementType" NOT NULL,
Expand All @@ -19,13 +38,25 @@ CREATE TABLE "StockMovement" (
);

-- CreateIndex
CREATE INDEX "StockMovement_productId_createdAt_idx" ON "StockMovement"("productId", "createdAt");
CREATE INDEX IF NOT EXISTS "StockMovement_productId_createdAt_idx" ON "StockMovement"("productId", "createdAt");

-- CreateIndex
CREATE INDEX "StockMovement_type_idx" ON "StockMovement"("type");
CREATE INDEX IF NOT EXISTS "StockMovement_type_idx" ON "StockMovement"("type");

-- AddForeignKey
ALTER TABLE "StockMovement" ADD CONSTRAINT "StockMovement_productId_fkey" FOREIGN KEY ("productId") REFERENCES "Product"("id") ON DELETE CASCADE ON UPDATE CASCADE;
-- AddForeignKey (idempotent: skip if the constraint already exists)
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'StockMovement_productId_fkey') THEN
ALTER TABLE "StockMovement" ADD CONSTRAINT "StockMovement_productId_fkey" FOREIGN KEY ("productId") REFERENCES "Product"("id") ON DELETE CASCADE ON UPDATE CASCADE;
END IF;
END
$$;

-- AddForeignKey
ALTER TABLE "StockMovement" ADD CONSTRAINT "StockMovement_createdById_fkey" FOREIGN KEY ("createdById") REFERENCES "Staff"("id") ON DELETE SET NULL ON UPDATE CASCADE;
-- AddForeignKey (idempotent: skip if the constraint already exists)
DO $$
BEGIN
IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'StockMovement_createdById_fkey') THEN
ALTER TABLE "StockMovement" ADD CONSTRAINT "StockMovement_createdById_fkey" FOREIGN KEY ("createdById") REFERENCES "Staff"("id") ON DELETE SET NULL ON UPDATE CASCADE;
END IF;
END
$$;
Loading