From fcd4c0bf48c936e7cedbc9dd86f357910df15268 Mon Sep 17 00:00:00 2001 From: Paperclip Date: Tue, 2 Jun 2026 04:25:42 +0000 Subject: [PATCH] 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`);