Compare commits

..

3 Commits

Author SHA1 Message Date
Paperclip 53579e979d ci: retrigger GRO-2100 PR #151 CI (Lint & Typecheck failed at actions/checkout@v4 — runner infra)
CI / Test (pull_request) Successful in 11s
CI / Lint & Typecheck (pull_request) Successful in 15s
CI / Build & Push Docker Images (pull_request) Successful in 1m0s
2026-06-02 18:05:52 +00:00
Paperclip 8fb6c9375b fix(seed): GRO-2100 deterministic uat-groomer ↔ UAT Pup Alpha linkage
CI / Test (pull_request) Successful in 10s
CI / Lint & Typecheck (pull_request) Failing after 12m51s
CI / Build & Push Docker Images (pull_request) Has been skipped
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 <noreply@paperclip.ing>
2026-06-02 17:41:26 +00:00
Paperclip fcd4c0bf48 fix(db): make services seed idempotent across resets (GRO-2064, GRO-2033 close-out)
CI / Test (pull_request) Successful in 13s
CI / Lint & Typecheck (pull_request) Successful in 16s
CI / Build & Push Docker Images (pull_request) Successful in 1m20s
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 <noreply@paperclip.ing>
2026-06-02 04:25:42 +00:00
4 changed files with 1358 additions and 121 deletions
-3
View File
@@ -147,8 +147,6 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
| TC-API-3.19b | Get pet profile summary — customer cross-tenant blocked (GRO-2013) | Sign in as `uat-customer@groombook.dev`; reuse the customer's sessionId from TC-API-3.19a; `GET /api/pets/{otherClientPetId}/profile-summary` for a pet owned by a different client (`c0000002-...` or any non-customer pet) | 403 Forbidden (owner-bypass requires session.clientId === pet.clientId) |
| TC-API-3.19c | Get pet profile summary — customer without portal session header | Same as TC-API-3.19a but omit the `X-Impersonation-Session-Id` header | 403 Forbidden (no owner-bypass without valid portal session) |
| TC-API-3.19d | Get pet profile summary — owner-bypass writes audit row (GRO-2063) | Same setup as TC-API-3.19a (sign in as `uat-customer@groombook.dev`, establish a portal session for the customer's own clientId, call `GET /api/pets/{ownPetId}/profile-summary` with `X-Impersonation-Session-Id: {sessionId}` and a 200 OK response). Then call `GET /api/impersonation/sessions/{sessionId}/audit-log` and confirm there is exactly one entry with `action === "read_profile_summary"`, `pageVisited` matching the profile-summary path, and `metadata` containing `petId` and `actorStaffId` for the customer. Repeat TC-API-3.19b (cross-tenant attempt) and confirm NO new `read_profile_summary` row was written for the cross-tenant attempt. | 200 OK on the profile-summary call AND an audit log entry is present with the correct shape (defense-in-depth audit row; bypass attempts against other clients must NOT log) |
| TC-UAT-2 | Groomer accesses linked pet profile summary (GRO-2100) | Sign in as `uat-groomer@groombook.dev`; `GET /api/pets/c0000001-0000-0000-0000-000000000002/profile-summary` (UAT Pup Alpha — linked via deterministic completed appointment `a0000001-0000-0000-0000-000000000001`, service `b0000001-…-0001` "Bath & Brush", `startTime` ~7 days ago) | 200 OK, `recentGroomingHistory[]` non-empty (>=1 entry), `visitCount >= 1`, `upcomingAppointment` null (the seeded appointment is in the past) |
| TC-UAT-3 | Groomer blocked from unlinked pet profile summary (GRO-2100) | Sign in as `uat-groomer@groombook.dev`; `GET /api/pets/c0000001-0000-0000-0000-000000000003/profile-summary` (UAT Pup Beta — intentionally UNLINKED; no appointment row references this pet's clientId+groomerId combo) | 403 Forbidden (RBAC `groomer` role lacks the appointment-linkage grant for this pet). NOTE: if 404 is returned instead of 403, file a separate RBAC defect (not against the seed) — see GRO-2100 verification note |
| TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) |
| TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) |
| TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` |
@@ -168,7 +166,6 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
| TC-API-3.26 | Verify 25-35% medicalAlerts distribution | GET /api/pets (first 30 pets), count how many have non-empty medicalAlerts | Ratio is 25-35% (seed uses rand() < 0.3 for ~30% distribution) |
| TC-API-3.27 | Verify coat_type enum has all seed values | After UAT seed completes, inspect the coat_type enum on the UAT DB — it must contain: short, medium, long, double, wire, silky, curly, hairless | UAT seed jobs (`reset-demo-data`, `seed-test-data`) complete 1/1 with no `enum_in` error; coat_type includes all 8 values used by seed.ts `coatTypePool` |
| TC-API-3.28 | Verify pet_size_category enum has all seed values | After UAT seed completes, inspect the pet_size_category enum on the UAT DB — it must contain: small, medium, large, extra_large | UAT seed jobs (`reset-demo-data`, `seed-test-data`) complete 1/1 with no `enum_in` error; pet_size_category includes all 4 values used by seed.ts `petSizeCategoryPool` (regression for GRO-1999, mirrors TC-API-3.27) |
| TC-API-3.29 | Verify `reset-demo-data` CronJob does not fail with FK 23503 on `invoice_tip_splits` (GRO-2123) | Trigger the CronJob manually: `kubectl create job --from=cronjob/reset-demo-data verify-gro2123 -n groombook-uat`. Wait for pod to terminate. Inspect logs: `kubectl logs -n groombook-uat -l job-name=verify-gro2123` | Pod reaches `Completed` state; logs show `✓ Acquired seed advisory lock` and `✓ Released seed advisory lock` from `seed.ts`; no `PostgresError: … violates foreign key constraint "invoice_tip_splits_invoice_id_invoices_id_fk"` (code 23503); final counts unchanged (500 clients, ~4000 invoices) |
### 4.4 Appointment Scheduling
+2 -2
View File
@@ -12,8 +12,8 @@
"test": "vitest run",
"db:generate": "drizzle-kit generate",
"db:migrate": "drizzle-kit migrate",
"db:seed": "pnpm --filter @groombook/db seed",
"db:reset": "pnpm --filter @groombook/db reset",
"db:seed": "tsx src/db/seed.ts",
"db:reset": "tsx src/db/reset.ts && drizzle-kit migrate && tsx src/db/seed.ts",
"db:studio": "drizzle-kit studio"
},
"dependencies": {
File diff suppressed because it is too large Load Diff
+7 -116
View File
@@ -401,9 +401,7 @@ const servicesDef = [
*
* In seedKnownUsers() this replaces the inline UAT-staff block.
*/
async function seedUatStaffAccounts(
db: ReturnType<typeof drizzle>,
): Promise<string | null> {
async function seedUatStaffAccounts(db: ReturnType<typeof drizzle>) {
// ── Staff: UAT Super User (oidcSub from SEED_UAT_SUPER_OIDC_SUB env var) ──
const uatSuperOidcSub = process.env.SEED_UAT_SUPER_OIDC_SUB;
if (uatSuperOidcSub) {
@@ -679,12 +677,7 @@ async function seedUatStaffAccounts(
// 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.
//
// The linkage call itself is performed by the caller AFTER the `services`
// catalogue has been seeded (this helper runs before services exist,
// which previously caused the linkage to be silently skipped on every
// reset). GRO-2100 follow-up.
return uatCustomerClientId;
await seedUatGroomerLinkage(db, uatCustomerClientId);
}
/**
@@ -699,18 +692,12 @@ async function seedUatStaffAccounts(
*/
async function seedUatGroomerLinkage(
db: ReturnType<typeof drizzle>,
customerClientId: string | null,
customerClientId: string,
): Promise<void> {
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";
// Skip silently if the UAT Customer client wasn't created (non-UAT seed
// profile, e.g. seedKnownUsers() in an env without the UAT personas).
if (!customerClientId) {
return;
}
// 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
@@ -733,19 +720,6 @@ async function seedUatGroomerLinkage(
return;
}
// Skip if the linked pet hasn't been seeded yet (defensive: caller should
// ensure pets exist; if the helper is re-ordered later we don't want to
// crash here).
const [linkedPet] = await db
.select({ id: schema.pets.id })
.from(schema.pets)
.where(eq(schema.pets.id, LINKED_PET_ID))
.limit(1);
if (!linkedPet) {
console.warn(`⚠ GRO-2100: UAT Pup Alpha (${LINKED_PET_ID}) not found — skipping uat-groomer linkage`);
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).
@@ -873,7 +847,7 @@ async function seedKnownUsers() {
// ── UAT staff accounts + Better Auth credentials (shared impl) ──────────────
// Extracted into seedUatStaffAccounts() so it runs in both seedKnownUsers()
// and the full seed() UAT branch.
const uatCustomerClientId = await seedUatStaffAccounts(db);
await seedUatStaffAccounts(db);
// ── Services: idempotent upsert keyed on `id` ─────────────────────────────
// GRO-2064: previously keyed on `services.name` while writing a
@@ -901,12 +875,6 @@ async function seedKnownUsers() {
}
console.log(`✓ Seeded ${demoSvcs.length} services`);
// GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run
// AFTER services are seeded (this helper looks up an active service id
// to attach to the appointment; on a fresh reset there are none yet at
// the time seedUatStaffAccounts() returns).
await seedUatGroomerLinkage(db, uatCustomerClientId);
// ── Client: Demo Client ──
const [existingClient] = await db
.select()
@@ -976,63 +944,6 @@ async function seedKnownUsers() {
// ── Main seed ────────────────────────────────────────────────────────────────
// ── GRO-2123: serialize reset+seed with a Postgres advisory lock ────────
// The reset-demo-data CronJob runs on an hourly schedule. With
// concurrencyPolicy=Replace, a new pod can start while the previous one
// is still mid-seed; the new pod's TRUNCATE then deletes rows the old pod
// is still inserting, producing FK 23503 errors non-deterministically
// (see GRO-2123: invoice_tip_splits → invoices).
//
// We hold a session-level advisory lock for the full duration of the
// seed so that overlapping invocations block then proceed in order —
// not skip. The key is a stable 32-bit constant so it can be referenced
// from runbooks without ambiguity and binds to the single-argument
// `pg_advisory_lock(int)` form, which postgres-js serializes as a plain
// number (no bigint type plumbing required).
const SEED_ADVISORY_LOCK_KEY = 0x47524f4f; // "GROO" in ASCII — arbitrary, stable
/**
* Reserve a dedicated connection from `pool`, take the seed advisory lock
* on it, run `fn`, and release the lock + connection in a try/finally.
*
* CRITICAL: with postgres-js connection pooling, a session-level
* `pg_advisory_lock(KEY)` acquired on one pooled connection and released
* on a *different* one is a no-op (the lock is bound to the session /
* pg-backend that took it). We therefore reserve a dedicated connection
* for the lock and release it from the same reserved connection. The
* seed work itself still runs on the pooled connections.
*/
async function withSeedAdvisoryLock<T>(
pool: ReturnType<typeof postgres>,
fn: () => Promise<T>,
): Promise<T> {
const lockConnection = await pool.reserve();
let lockHeld = false;
try {
await lockConnection`SELECT pg_advisory_lock(${SEED_ADVISORY_LOCK_KEY})`;
lockHeld = true;
console.log(`✓ Acquired seed advisory lock (key=${SEED_ADVISORY_LOCK_KEY})`);
const result = await fn();
await lockConnection`SELECT pg_advisory_unlock(${SEED_ADVISORY_LOCK_KEY})`;
lockHeld = false;
console.log(`✓ Released seed advisory lock`);
return result;
} finally {
if (lockHeld) {
try {
await lockConnection`SELECT pg_advisory_unlock(${SEED_ADVISORY_LOCK_KEY})`;
} catch (err) {
console.error("Failed to release seed advisory lock during cleanup:", err);
}
}
try {
lockConnection.release();
} catch (err) {
console.error("Failed to release reserved lock connection:", err);
}
}
}
async function seed() {
const url = process.env.DATABASE_URL;
if (!url) {
@@ -1050,22 +961,6 @@ async function seed() {
const client = postgres(url, { max: 5 });
const db = drizzle(client, { schema });
// GRO-2123: hold the seed advisory lock for the full body of runSeedBody.
// See the withSeedAdvisoryLock comment for why a reserved connection is
// required (postgres-js pooling would silently drop the lock otherwise).
await withSeedAdvisoryLock(client, async () => {
return await runSeedBody(client, db, profile, cfg);
});
await client.end();
}
async function runSeedBody(
client: ReturnType<typeof postgres>,
db: ReturnType<typeof drizzle>,
profile: SeedProfile,
cfg: ProfileConfig,
): Promise<void> {
console.log(`Seeding Groom Book database (profile: ${profile})...\n`);
// ── Staff ──
@@ -1136,7 +1031,7 @@ async function runSeedBody(
// ── UAT staff accounts + Better Auth credentials (shared impl) ──────────────
// Seeds deterministic UAT staff with numeric OIDC subs and Better Auth credentials.
// Must run AFTER random staff are created so upserts land correctly.
const uatCustomerClientId = await seedUatStaffAccounts(db);
await seedUatStaffAccounts(db);
// ── Services ──
// GRO-2064: key the upsert on `services.id` (not `name`) so deterministic
@@ -1163,12 +1058,6 @@ async function runSeedBody(
}
console.log(`✓ Created ${servicesDef.length} services`);
// GRO-2100: deterministic uat-groomer ↔ UAT Pup Alpha linkage. Must run
// AFTER services are seeded (this helper looks up an active service id
// to attach to the appointment; on a fresh reset there are none yet at
// the time seedUatStaffAccounts() returns).
await seedUatGroomerLinkage(db, uatCustomerClientId);
// ── Clients & Pets ──
const now = new Date();
const appointmentsBackDate = new Date(now);
@@ -1687,6 +1576,8 @@ async function runSeedBody(
}
console.log(`✓ Created ${visitLogCount} grooming visit logs`);
console.log("\nSeed complete!");
await client.end();
}
seed().catch((err) => {