From fcd4c0bf48c936e7cedbc9dd86f357910df15268 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 04:25:42 +0000 Subject: [PATCH 1/3] fix(db): make services seed idempotent across resets (GRO-2064, GRO-2033 close-out) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seed Job `seed-test-data-b5943fb` failed three times on prod with `duplicate key value violates unique constraint "services_pkey"` after migrations 0039/0040 landed. Two interlocking bugs in `packages/db/src/seed.ts` (and the parallel `apps/api/src/db/seed.ts` tree — both kept in sync per the GRO-2052/2013/2014 lesson): 1. The reset `TRUNCATE` excluded `services`, so a prior `seedKnownUsers` run that wrote `id=b0000001-…-004, name="Nail Trim"` survived every reset. The next full `seed()` then tried to insert `id=b0000001-…-004, name="Full Groom — Large"` and PostgreSQL raised `services_pkey` (id collision) — the name-targeted `ON CONFLICT` couldn't fire because the conflict was on a different column. 2. The `demoSvcs` (used by `seedKnownUsers`) had `id=…-004, name="Nail Trim"` while `servicesDef` (used by the full `seed()`) has `id=…-004, name="Full Groom — Large"`. `Nail Trim` was supposed to be `id=…-005` in the demo subset. Fix: * `TRUNCATE services, …` so each reset rebuilds the catalogue from `servicesDef` (CASCADE handles appointments/invoices FKs). * Key both services upserts on `schema.services.id` (not `name`) so deterministic ids always win — defense-in-depth if a future change drops `services` from the TRUNCATE list again. * Reconcile the id↔name map: `demoSvcs[3]` is now `id=…-005, name="Nail Trim"` to match `servicesDef[4]`. * Update `UAT_PLAYBOOK.md §4.5.1` with regression coverage (TC-SEED-1..4). Required for the GRO-2033 close-out: infra PR #605 must repoint to the new image tag (NOT 2a6242d) and `apps/overlays/prod/reset-cronjob.yaml` must stay suspended until a one-shot seed Job runs 1/1 against prod. Co-Authored-By: Paperclip --- UAT_PLAYBOOK.md | 27 +++++++++++++++++++++++++++ apps/api/src/db/seed.ts | 39 +++++++++++++++++++++++++++------------ packages/db/src/seed.ts | 39 +++++++++++++++++++++++++++------------ 3 files changed, 81 insertions(+), 24 deletions(-) diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 713e418..b8949de 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -192,6 +192,33 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the | TC-API-5.4 | Update service | PATCH /api/services/{id} with updated fields | 200 OK, service updated | | TC-API-5.5 | Delete service | DELETE /api/services/{id} | 200 OK, service deleted | +#### 4.5.1 Seed/Reset idempotency (GRO-2064) + +Services seeding is now keyed on the deterministic `services.id` (not `name`) and +the reset path now `TRUNCATE`s `services` alongside the other dynamic tables. +This means: + +- Running the seed Job twice in a row (no reset in between) converges to the + same catalogue — no `services_pkey` collision. +- A `pnpm reset` followed by `pnpm seed` (or a CronJob reset fire) leaves the + catalogue exactly matching `servicesDef` (10 rows, ids `b0000001-…-001` … + `…-00a`), regardless of any stale rows that were present beforehand. +- Mixed `seedKnownUsers` + full `seed()` invocations are safe — the + `demoSvcs` subset (Bath & Brush, Full Groom Small/Medium, Nail Trim) is + keyed on ids `…-001`, `…-002`, `…-003`, `…-005` and the upsert target + is `services.id`, so the same-id / different-name collision that broke + GRO-2033 (id `…-004` = "Nail Trim" vs servicesDef `…-004` = + "Full Groom — Large") cannot recur. + +**UAT regression** (verify after a new image is rolled out): + +| # | Scenario | Steps | Expected | +|---|----------|-------|----------| +| TC-SEED-1 | Reset → seed converges | `kubectl -n groombook exec deploy/api -- pnpm reset && pnpm seed` | Seed completes 1/1, `services` count = 10, all ids match `servicesDef` | +| TC-SEED-2 | Idempotent re-seed | Re-run `pnpm seed` without reset | Seed completes 1/1, no `services_pkey` errors, `services` count still 10 | +| TC-SEED-3 | Catalogue matches servicesDef | `psql -c "SELECT id, name FROM services ORDER BY id"` | Rows `…-001`…`…-00a` with names "Bath & Brush"…"Sanitary Trim" exactly as in `servicesDef` | +| TC-SEED-4 | Demo subset coexists | Run `seedKnownUsers` then full `seed` | No collision, demo subset (4 services) ends up with the same rows the full seed would write | + ### 4.6 Staff Management | # | Scenario | Steps | Expected | diff --git a/apps/api/src/db/seed.ts b/apps/api/src/db/seed.ts index 5b48dd6..bf57144 100644 --- a/apps/api/src/db/seed.ts +++ b/apps/api/src/db/seed.ts @@ -636,21 +636,28 @@ async function seedKnownUsers() { } } - // ── Services: idempotent upsert using name as unique key ───────────────────── - // UNIQUE constraint on services.name (migration 0020) must exist first. - // Uses b0000001-... IDs to match main seed servicesDef for same-named services. + // ── Services: idempotent upsert keyed on `id` ───────────────────────────── + // GRO-2064: previously keyed on `services.name` while writing a + // deterministic `id`. If a stale row existed with the same `id` but a + // different `name`, PostgreSQL raised `services_pkey` (id collision) + // before the name-targeted ON CONFLICT could fire. Switch the conflict + // target to `services.id` so deterministic ids always win; pair with + // `TRUNCATE services … CASCADE` above so each reset rebuilds the + // catalogue from `servicesDef` cleanly. GRO-2033 close-out. + // Id↔name map MUST stay in sync with `servicesDef` (the canonical source + // of truth in the main `seed()` function). const demoSvcs = [ { id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 }, { id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 }, { id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 }, - { id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 }, + { id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 }, ]; for (const svc of demoSvcs) { await db.insert(schema.services) .values({ ...svc, active: true }) .onConflictDoUpdate({ - target: schema.services.name, - set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true }, + target: schema.services.id, + set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true }, }); } console.log(`✓ Seeded ${demoSvcs.length} services`); @@ -757,7 +764,13 @@ async function seed() { ({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false }) ); - await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`); + // GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue + // from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows + // (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the + // same `id`) would otherwise cause the deterministic upsert to PK-collide + // on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE + // CASCADE handles appointments/invoices FKs to services.id. + await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`); const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers]; for (const s of allStaff) { @@ -828,9 +841,11 @@ async function seed() { } // ── Services ── - // Upsert services using name as unique key. With deterministic IDs in - // servicesDef and TRUNCATE clearing downstream tables first, this is - // idempotent: first run inserts, subsequent runs update existing rows. + // GRO-2064: key the upsert on `services.id` (not `name`) so deterministic + // ids always win, and rely on the TRUNCATE above to clear stale rows before + // the catalogue is rebuilt. The previous name-targeted upsert failed with + // `services_pkey` when a prior run had left a row with the same id but a + // different name (CTO review on infra PR #605, rev #4230). const serviceIds: string[] = []; for (const s of servicesDef) { serviceIds.push(s.id); @@ -844,8 +859,8 @@ async function seed() { active: true, }) .onConflictDoUpdate({ - target: schema.services.name, - set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true }, + target: schema.services.id, + set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true }, }); } console.log(`✓ Created ${servicesDef.length} services`); diff --git a/packages/db/src/seed.ts b/packages/db/src/seed.ts index cf65909..c647098 100644 --- a/packages/db/src/seed.ts +++ b/packages/db/src/seed.ts @@ -747,21 +747,28 @@ async function seedKnownUsers() { // and the full seed() UAT branch. await seedUatStaffAccounts(db); - // ── Services: idempotent upsert using name as unique key ───────────────────── - // UNIQUE constraint on services.name (migration 0020) must exist first. - // Uses b0000001-... IDs to match main seed servicesDef for same-named services. + // ── Services: idempotent upsert keyed on `id` ───────────────────────────── + // GRO-2064: previously keyed on `services.name` while writing a + // deterministic `id`. If a stale row existed with the same `id` but a + // different `name`, PostgreSQL raised `services_pkey` (id collision) + // before the name-targeted ON CONFLICT could fire. Switch the conflict + // target to `services.id` so deterministic ids always win; pair with + // `TRUNCATE services … CASCADE` above so each reset rebuilds the + // catalogue from `servicesDef` cleanly. GRO-2033 close-out. + // Id↔name map MUST stay in sync with `servicesDef` (the canonical source + // of truth in the main `seed()` function). const demoSvcs = [ { id: "b0000001-0000-0000-0000-000000000001", name: "Bath & Brush", description: "Full bath, blow-dry, brush out, and ear cleaning", basePriceCents: 4500, durationMinutes: 45 }, { id: "b0000001-0000-0000-0000-000000000002", name: "Full Groom — Small", description: "Complete grooming for dogs under 25 lbs", basePriceCents: 6500, durationMinutes: 60 }, { id: "b0000001-0000-0000-0000-000000000003", name: "Full Groom — Medium", description: "Complete grooming for dogs 25-50 lbs", basePriceCents: 8000, durationMinutes: 75 }, - { id: "b0000001-0000-0000-0000-000000000004", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 }, + { id: "b0000001-0000-0000-0000-000000000005", name: "Nail Trim", description: "Nail clipping and filing", basePriceCents: 1500, durationMinutes: 15 }, ]; for (const svc of demoSvcs) { await db.insert(schema.services) .values({ ...svc, active: true }) .onConflictDoUpdate({ - target: schema.services.name, - set: { description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true }, + target: schema.services.id, + set: { name: svc.name, description: svc.description, basePriceCents: svc.basePriceCents, durationMinutes: svc.durationMinutes, active: true }, }); } console.log(`✓ Seeded ${demoSvcs.length} services`); @@ -868,7 +875,13 @@ async function seed() { ({ id: uuid(), name: `Bather ${i + 1}`, email: `bather${i + 1}@groombook.dev`, role: "groomer" as const, isSuperUser: false }) ); - await db.execute(sql`TRUNCATE impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`); + // GRO-2064: also TRUNCATE `services` so each reset rebuilds the catalogue + // from `servicesDef` (deterministic IDs + UNIQUE(name)). Stale service rows + // (e.g. a prior `seedKnownUsers` run that wrote a different `name` for the + // same `id`) would otherwise cause the deterministic upsert to PK-collide + // on `services.id` — see CTO review on infra PR #605 (rev #4230). TRUNCATE + // CASCADE handles appointments/invoices FKs to services.id. + await db.execute(sql`TRUNCATE services, impersonation_sessions, impersonation_audit_logs, appointments, invoices, invoice_line_items, invoice_tip_splits, grooming_visit_logs CASCADE`); const allStaff = [...managerStaff, ...receptionistStaff, ...groomers, ...bathers]; for (const s of allStaff) { @@ -919,9 +932,11 @@ async function seed() { await seedUatStaffAccounts(db); // ── Services ── - // Upsert services using name as unique key. With deterministic IDs in - // servicesDef and TRUNCATE clearing downstream tables first, this is - // idempotent: first run inserts, subsequent runs update existing rows. + // GRO-2064: key the upsert on `services.id` (not `name`) so deterministic + // ids always win, and rely on the TRUNCATE above to clear stale rows before + // the catalogue is rebuilt. The previous name-targeted upsert failed with + // `services_pkey` when a prior run had left a row with the same id but a + // different name (CTO review on infra PR #605, rev #4230). const serviceIds: string[] = []; for (const s of servicesDef) { serviceIds.push(s.id); @@ -935,8 +950,8 @@ async function seed() { active: true, }) .onConflictDoUpdate({ - target: schema.services.name, - set: { description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true }, + target: schema.services.id, + set: { name: s.name, description: s.desc, basePriceCents: s.price, durationMinutes: s.dur, active: true }, }); } console.log(`✓ Created ${servicesDef.length} services`); -- 2.52.0 From 8fb6c9375ba14289984e09b1f1c3f1b69f7f115b Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 17:41:26 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(seed):=20GRO-2100=20deterministic=20uat?= =?UTF-8?q?-groomer=20=E2=86=94=20UAT=20Pup=20Alpha=20linkage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UAT seed creates the uat-groomer@groombook.dev Better Auth account (staffId 00000000-0000-0000-0000-000000000004) but no appointments, so GET /api/pets?groomer=me returns [] and GET /api/pets/{anyId}/profile-summary returns 404. This makes GRO-1987 TC-UAT-2/3 (RBAC tests for the profile-summary endpoint) un-runnable. This is the seed-side counterpart of GRO-1983 (stale password hashes): that was the credential row, this is the linkage row. Fix: add seedUatGroomerLinkage() called from seedUatStaffAccounts(), so both the full seed() path and the seedKnownUsers() path (prod reset CronJob with SEED_KNOWN_USERS_ONLY=true) produce a deterministic completed appointment linking the UAT groomer to UAT Pup Alpha (c0000001-0000-0000-0000-000000000002). UAT Pup Beta is intentionally left UNLINKED so TC-UAT-3 can verify the 403 forbidden response. The deterministic appointment id (a0000001-0000-0000-0000-000000000001) makes the function idempotent: re-running the seed (hourly via the reset-demo-data CronJob) is a no-op once the row exists. Verification (after the next 17:00 reset): - GET /api/pets/{c0000001-0000-0000-0000-000000000002}/profile-summary as uat-groomer → 200 with recentGroomingHistory/visitCount/upcomingAppointment - GET /api/pets/{c0000001-0000-0000-0000-000000000003}/profile-summary as uat-groomer → 403 If the unlinked-pet case returns 404 instead of 403, that is a separate RBAC defect — file against the api repo, not the seed. Co-Authored-By: Paperclip --- packages/db/src/seed.ts | 102 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/packages/db/src/seed.ts b/packages/db/src/seed.ts index c647098..13cf103 100644 --- a/packages/db/src/seed.ts +++ b/packages/db/src/seed.ts @@ -668,6 +668,108 @@ async function seedUatStaffAccounts(db: ReturnType) { console.log(`✓ Created UAT pet '${pet.name}' with extended fields`); } } + + // ── GRO-2100: deterministic uat-groomer ↔ pet linkage ─────────────────────── + // The UAT groomer (`uat-groomer@groombook.dev`, staffId 00000000-0000-0000-0000-000000000004) + // needs at least one linked pet/appointment or GRO-1987 TC-UAT-2/3 cannot run + // (the pet profile-summary endpoint returns 404 instead of 200/403). + // + // We deterministically link the UAT groomer to the UAT customer's first pet + // ("UAT Pup Alpha") and leave the second pet ("UAT Pup Beta") UNLINKED so + // TC-UAT-2 (200) and TC-UAT-3 (403) can both hardcode the stable petIds. + await seedUatGroomerLinkage(db, uatCustomerClientId); +} + +/** + * GRO-2100: create a deterministic completed appointment linking the UAT groomer + * to "UAT Pup Alpha" (c0000001-0000-0000-0000-000000000002). "UAT Pup Beta" + * (c0000001-0000-0000-0000-000000000003) is intentionally left UNLINKED so + * GRO-1987 TC-UAT-3 can verify the 403 forbidden response. + * + * Idempotent: the deterministic appointment id (`a0000001-…-0001`) is the + * upsert key, so re-running the seed on every reset-demo-data CronJob + * (hourly per apps/overlays/uat/reset-cronjob.yaml) is safe. + */ +async function seedUatGroomerLinkage( + db: ReturnType, + customerClientId: string, +): Promise { + const uatGroomerEmail = "uat-groomer@groombook.dev"; + const LINKED_PET_ID = "c0000001-0000-0000-0000-000000000002"; // UAT Pup Alpha + const APPT_ID = "a0000001-0000-0000-0000-000000000001"; + + // Only run if the UAT groomer staff record actually exists — dev/test seeds + // that don't set SEED_UAT_STAFF_OIDC_SUB should not crash. + const [uatGroomerStaff] = await db + .select({ id: schema.staff.id }) + .from(schema.staff) + .where(eq(schema.staff.email, uatGroomerEmail)) + .limit(1); + if (!uatGroomerStaff) { + return; + } + + // Skip if this exact appointment already exists (idempotent on re-seed). + const [existing] = await db + .select({ id: schema.appointments.id }) + .from(schema.appointments) + .where(eq(schema.appointments.id, APPT_ID)) + .limit(1); + if (existing) { + console.log(`✓ GRO-2100: uat-groomer linkage appointment already exists — skipping`); + return; + } + + // The "Bath & Brush" service id is stable across the reset; falls back to + // any active service if it has not been seeded yet (e.g. seedKnownUsers + // runs in isolation). + const BATH_AND_BRUSH_ID = "b0000001-0000-0000-0000-000000000001"; + const [bathService] = await db + .select({ id: schema.services.id }) + .from(schema.services) + .where(eq(schema.services.id, BATH_AND_BRUSH_ID)) + .limit(1); + + let serviceId: string; + if (bathService) { + serviceId = bathService.id; + } else { + const [fallback] = await db + .select({ id: schema.services.id }) + .from(schema.services) + .where(eq(schema.services.active, true)) + .limit(1); + if (!fallback) { + console.warn(`⚠ GRO-2100: no active services found — skipping uat-groomer linkage`); + return; + } + serviceId = fallback.id; + } + + // Schedule the completed appointment 7 days ago so the profile-summary's + // "recentGroomingHistory" window (last 10) reliably includes it. + const startTime = new Date(); + startTime.setDate(startTime.getDate() - 7); + startTime.setHours(10, 0, 0, 0); + const endTime = new Date(startTime.getTime() + 45 * 60 * 1000); + + await db.insert(schema.appointments).values({ + id: APPT_ID, + clientId: customerClientId, + petId: LINKED_PET_ID, + serviceId, + staffId: uatGroomerStaff.id, + batherStaffId: null, + status: "completed", + startTime, + endTime, + notes: "GRO-2100: deterministic uat-groomer linkage for TC-UAT-2/3.", + priceCents: null, + confirmationStatus: "confirmed", + }); + console.log( + `✓ GRO-2100: linked uat-groomer (${uatGroomerStaff.id}) → UAT Pup Alpha (${LINKED_PET_ID}) via appointment ${APPT_ID}`, + ); } // ── Known-users-only seed (prod/demo) ─────────────────────────────────────── -- 2.52.0 From 53579e979dbfc277096a3a18d21f50cd6d0b73b0 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 18:05:52 +0000 Subject: [PATCH 3/3] =?UTF-8?q?ci:=20retrigger=20GRO-2100=20PR=20#151=20CI?= =?UTF-8?q?=20(Lint=20&=20Typecheck=20failed=20at=20actions/checkout@v4=20?= =?UTF-8?q?=E2=80=94=20runner=20infra)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit -- 2.52.0