fix(test): de-flake authProvider.test.ts OOBE test-connection (GRO-2745) [uat → main]

Merge dev→uat after QA approval (GRO-2746)

UAT_PLAYBOOK.md

@@ -196,6 +196,7 @@ Geocoding turns a client's street address into `latitude`/`longitude` + `geocode
 | 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) |
+| TC-API-3.30 | Verify `reset-demo-data` CronJob is schema-safe — TRUNCATE only, no DDL drops (GRO-2722) | 1. Trigger: `kubectl -n groombook-uat create job --from=cronjob/reset-demo-data reset-verify-$(date +%s) --dry-run=client -o name \| xargs kubectl -n groombook-uat apply -f -` or simply `kubectl -n groombook-uat create job --from=cronjob/reset-demo-data reset-verify-$(date +%s)`. 2. Wait for job completion: `kubectl -n groombook-uat wait job/reset-verify-<ts> --for=condition=complete --timeout=300s`. 3. Check logs: `kubectl -n groombook-uat logs job/reset-verify-<ts>`. 4. Verify schema survival: `kubectl -n groombook-uat exec deploy/api -- psql $DATABASE_URL -c "\dt public.*"` and `kubectl -n groombook-uat exec deploy/api -- psql $DATABASE_URL -c "\dt drizzle.*"`. 5. Verify readyz: `curl -s https://uat.groombook.dev/api/readyz` | Job reaches `Completed` (exit 0); logs show `✓ All public tables truncated, sequences reset` — no `DROP TABLE`/`DROP TYPE`/`DROP SCHEMA` in output; all `public.*` tables still present after reset; `drizzle.__drizzle_migrations` still populated (row count unchanged); `https://uat.groombook.dev/api/readyz` returns HTTP 200; demo data reseeded (500 clients visible via GET /api/clients) |
 
 ### 4.4 Appointment Scheduling
 

apps/api/src/__tests__/authProvider.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { Hono } from "hono";
 import { authProviderRouter } from "../routes/authProvider.js";
 
@@ -227,6 +227,7 @@ describe("PUT /admin/auth-provider", () => {
 
 describe("POST /admin/auth-provider/test", () => {
   beforeEach(resetMock);
+  afterEach(() => vi.restoreAllMocks());
 
   it("returns ok=false for unreachable issuer", async () => {
     const app = makeApp(mockSuperUser);
@@ -242,7 +243,12 @@ describe("POST /admin/auth-provider/test", () => {
     expect(body.error).toBeTruthy();
   }, 15000); // timeout must exceed the 10s fetch timeout in the route handler
 
-  it("returns 400 for missing clientSecret (not required for test)", async () => {
+  it("returns 200 when clientSecret is omitted (not required by test-connection schema)", async () => {
+    // Mock fetch so the route does not make a real network request.
+    vi.spyOn(global, "fetch").mockResolvedValueOnce({
+      ok: true,
+      json: async () => ({ issuer: "https://auth.example.com" }),
+    } as Response);
     const app = makeApp(mockSuperUser);
     const { status } = await post(app, "/admin/auth-provider/test", {
       providerId: "authentik",
@@ -250,7 +256,7 @@ describe("POST /admin/auth-provider/test", () => {
       issuerUrl: "https://auth.example.com",
       clientId: "client",
     }, mockSuperUser);
-    expect(status).toBe(200); // clientSecret omitted intentionally for test
+    expect(status).toBe(200);
   });
 });
 

apps/api/src/db/reset.ts

@@ -1,10 +1,17 @@
 /**
- * reset.ts — Drop all application tables and re-run migrations + seed.
+ * reset.ts — Truncate all public tables and restart identity sequences.
  *
- * Intended for local development only. Never run against production.
+ * Schema-safe: never issues destructive DDL against any schema.
+ * The drizzle schema and __drizzle_migrations table are preserved so
+ * drizzle-kit migrate remains a no-op on an already-migrated DB.
+ *
+ * NOTE: this file is NOT the deployed reset entrypoint — the reset image
+ * builds from packages/db and runs `pnpm --filter @groombook/db reset`.
+ * apps/api db:reset delegates there too (see apps/api/package.json).
+ * Keep in sync with packages/db/src/reset.ts (GRO-2722).
  *
  * Usage:
- *   DATABASE_URL=postgres://... npx tsx packages/db/src/reset.ts
+ *   DATABASE_URL=postgres://... npx tsx apps/api/src/db/reset.ts
  */
 
 import postgres from "postgres";
@@ -23,43 +30,30 @@ async function reset() {
 
   const client = postgres(url, { max: 1 });
 
-  console.log("Dropping all application tables...\n");
+  console.log("Truncating all public tables...\n");
 
-  // Drop in dependency order (children before parents)
-  await client`
-    DO $$ DECLARE
-      r RECORD;
-    BEGIN
-      FOR r IN (
-        SELECT tablename FROM pg_tables
-        WHERE schemaname = 'public'
-      ) LOOP
-        EXECUTE 'DROP TABLE IF EXISTS public.' || quote_ident(r.tablename) || ' CASCADE';
-      END LOOP;
-    END $$;
+  // Enumerate all base tables in the public schema dynamically, then
+  // issue a single TRUNCATE with RESTART IDENTITY CASCADE so FK cycles
+  // are not a problem. The drizzle schema and __drizzle_migrations are
+  // intentionally excluded (different schema) so drizzle-kit migrate
+  // stays a no-op on an already-migrated DB.
+  const tables = await client<{ tablename: string }[]>`
+    SELECT tablename FROM pg_tables
+    WHERE schemaname = 'public'
   `;
 
-  // Drop custom enums
-  await client`
-    DO $$ DECLARE
-      r RECORD;
-    BEGIN
-      FOR r IN (
-        SELECT typname FROM pg_type
-        WHERE typtype = 'e' AND typnamespace = (
-          SELECT oid FROM pg_namespace WHERE nspname = 'public'
-        )
-      ) LOOP
-        EXECUTE 'DROP TYPE IF EXISTS ' || quote_ident(r.typname) || ' CASCADE';
-      END LOOP;
-    END $$;
-  `;
+  if (tables.length > 0) {
+    // Double-quote each identifier (escaping embedded quotes) to handle
+    // any table name safely without a pg-specific quote_ident helper.
+    const tableList = tables
+      .map((t) => `"${t.tablename.replace(/"/g, '""')}"`)
+      .join(", ");
+    await client.unsafe(
+      `TRUNCATE ${tableList} RESTART IDENTITY CASCADE`,
+    );
+  }
 
-  // Drop the drizzle migrations tracking table
-  await client`DROP TABLE IF EXISTS drizzle.__drizzle_migrations CASCADE`;
-  await client`DROP SCHEMA IF EXISTS drizzle CASCADE`;
-
-  console.log("✓ All tables and enums dropped\n");
+  console.log("✓ All public tables truncated, sequences reset\n");
 
   await client.end();
 }

packages/db/src/reset.ts

@@ -1,15 +1,14 @@
 /**
- * reset.ts — Drop all application tables, re-run migrations, and re-seed.
+ * reset.ts — Truncate all public tables and restart identity sequences.
  *
- * Intended for local development only. Never run against production.
+ * Schema-safe: never issues destructive DDL against any schema.
+ * The drizzle schema and __drizzle_migrations table are preserved so
+ * drizzle-kit migrate remains a no-op on an already-migrated DB.
  *
- * Usage:
- *   DATABASE_URL=postgres://... npx tsx packages/db/src/reset.ts
- *
- * GRO-2139: the entire drop→migrate→seed chain runs inside a single
+ * GRO-2139: the entire truncate→migrate→seed chain runs inside a single
  * Postgres advisory lock (SEED_ADVISORY_LOCK_KEY) so a concurrent
  * `seed.ts` (e.g. the dev `seed-test-data-*` Job being recreated at
- * the top of the hour) cannot interleave between `reset.ts` (DROP)
+ * the top of the hour) cannot interleave between `reset.ts` (TRUNCATE)
  * and `seed.ts` (TRUNCATE+insert) and collide on `invoices_pkey`.
  *
  * Why this matters: `seed.ts` derives every primary key from a single
@@ -22,10 +21,14 @@
  * GRO-2123 added the advisory lock around `runSeedBody` but left
  * `reset.ts` and `drizzle-kit migrate` outside the lock. This script
  * now wraps the *whole* chain in the same lock: `withSeedAdvisoryLock`
- * pins the lock to one reserved session and the DROP → migrate → seed
+ * pins the lock to one reserved session and the TRUNCATE → migrate → seed
  * work runs on the rest of the pool, so the lock guarantees mutual
  * exclusion against any concurrent seeder for the entire chain.
  *
+ * For a full local schema teardown (nuke tables, enums, drizzle schema)
+ * use `pnpm --filter @groombook/db db:nuke` instead — never change this
+ * script to be destructive (GRO-2722 / GRO-2678 prod incident).
+ *
  * See: groombook/infra `apps/base/reset-cronjob.yaml` (CronJob) and
  * `apps/base/seed-job.yaml` (one-shot Job) — both invoke the same
  * `seed.ts` code path on the same database in `groombook-dev`.
@@ -67,7 +70,7 @@ async function reset() {
   // Pool sizing is load-bearing here. `withSeedAdvisoryLock` does
   // `pool.reserve()` to pin the advisory lock to one dedicated session
   // (a session-level lock released on a *different* pooled connection is
-  // a no-op), and the DROP / migrate / seed work then runs on the
+  // a no-op), and the TRUNCATE / migrate / seed work then runs on the
   // *remaining* pooled connections. The lock provides mutual exclusion
   // across processes regardless of how many connections the work uses —
   // it does NOT require the work to share the lock's session.
@@ -82,43 +85,30 @@ async function reset() {
 
   try {
     await withSeedAdvisoryLock(client, async () => {
-      console.log("Dropping all application tables...\n");
+      console.log("Truncating all public tables...\n");
 
-      // Drop dependencies (tables) first
-      await client`
-        DO $$ DECLARE
-          r RECORD;
-        BEGIN
-          FOR r IN (
-            SELECT tablename FROM pg_tables
-            WHERE schemaname = 'public'
-          ) LOOP
-            EXECUTE 'DROP TABLE IF EXISTS public.' || quote_ident(r.tablename) || ' CASCADE';
-          END LOOP;
-        END $$;
+      // Enumerate all base tables in the public schema dynamically, then
+      // issue a single TRUNCATE with RESTART IDENTITY CASCADE so FK cycles
+      // are not a problem. The drizzle schema and __drizzle_migrations are
+      // intentionally excluded (different schema) so drizzle-kit migrate
+      // stays a no-op on an already-migrated DB.
+      const tables = await client<{ tablename: string }[]>`
+        SELECT tablename FROM pg_tables
+        WHERE schemaname = 'public'
       `;
 
-      // Drop custom enums
-      await client`
-        DO $$ DECLARE
-          r RECORD;
-        BEGIN
-          FOR r IN (
-            SELECT typname FROM pg_type
-            WHERE typtype = 'e' AND typnamespace = (
-              SELECT oid FROM pg_namespace WHERE nspname = 'public'
-            )
-          ) LOOP
-            EXECUTE 'DROP TYPE IF EXISTS ' || quote_ident(r.typname) || ' CASCADE';
-          END LOOP;
-        END $$;
-      `;
+      if (tables.length > 0) {
+        // Double-quote each identifier (escaping embedded quotes) to handle
+        // any table name safely without a pg-specific quote_ident helper.
+        const tableList = tables
+          .map((t) => `"${t.tablename.replace(/"/g, '""')}"`)
+          .join(", ");
+        await client.unsafe(
+          `TRUNCATE ${tableList} RESTART IDENTITY CASCADE`,
+        );
+      }
 
-      // Drop the drizzle migrations tracking table
-      await client`DROP TABLE IF EXISTS drizzle.__drizzle_migrations CASCADE`;
-      await client`DROP SCHEMA IF EXISTS drizzle CASCADE`;
-
-      console.log("✓ All tables and enums dropped\n");
+      console.log("✓ All public tables truncated, sequences reset\n");
 
       console.log("Running migrations...");
       // GRO-2672: drizzle-orm's migrate() has a high-water-mark bug that skips

src/__tests__/authProvider.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { Hono } from "hono";
 import { authProviderRouter } from "../routes/authProvider.js";
 
@@ -227,6 +227,7 @@ describe("PUT /admin/auth-provider", () => {
 
 describe("POST /admin/auth-provider/test", () => {
   beforeEach(resetMock);
+  afterEach(() => vi.restoreAllMocks());
 
   it("returns ok=false for unreachable issuer", async () => {
     const app = makeApp(mockSuperUser);
@@ -242,7 +243,12 @@ describe("POST /admin/auth-provider/test", () => {
     expect(body.error).toBeTruthy();
   }, 15000); // timeout must exceed the 10s fetch timeout in the route handler
 
-  it("returns 400 for missing clientSecret (not required for test)", async () => {
+  it("returns 200 when clientSecret is omitted (not required by test-connection schema)", async () => {
+    // Mock fetch so the route does not make a real network request.
+    vi.spyOn(global, "fetch").mockResolvedValueOnce({
+      ok: true,
+      json: async () => ({ issuer: "https://auth.example.com" }),
+    } as Response);
     const app = makeApp(mockSuperUser);
     const { status } = await post(app, "/admin/auth-provider/test", {
       providerId: "authentik",
@@ -250,7 +256,7 @@ describe("POST /admin/auth-provider/test", () => {
       issuerUrl: "https://auth.example.com",
       clientId: "client",
     }, mockSuperUser);
-    expect(status).toBe(200); // clientSecret omitted intentionally for test
+    expect(status).toBe(200);
   });
 });
 

src/__tests__/health-ready.test.ts

@@ -1,102 +0,0 @@
-import { describe, it, expect, vi, beforeEach } from "vitest";
-import { Hono } from "hono";
-
-// ─── Mock db module ───────────────────────────────────────────────────────────
-
-let selectImpl: () => Promise<unknown>;
-
-vi.mock("@groombook/db", () => {
-  const staff = new Proxy(
-    { _name: "staff" },
-    {
-      get(_target, prop) {
-        if (prop === "_name") return "staff";
-        return { table: "staff", column: prop };
-      },
-    }
-  );
-
-  return {
-    getDb: () => ({
-      select: (_fields: unknown) => ({
-        from: (_table: unknown) => ({
-          limit: (_n: number) => selectImpl(),
-        }),
-      }),
-    }),
-    staff,
-  };
-});
-
-// ─── Build test app ───────────────────────────────────────────────────────────
-
-async function makeApp() {
-  const { getDb, staff } = await import("@groombook/db");
-
-  const app = new Hono();
-  app.get("/health/ready", async (c) => {
-    try {
-      await getDb().select({ id: staff.id }).from(staff).limit(1);
-      return c.json({ status: "ready" }, 200);
-    } catch (err) {
-      const pgCode = (err as Record<string, unknown>).code ?? "unknown";
-      console.error("[health/ready] DB check failed:", pgCode);
-      return c.json({ status: "degraded" }, 503);
-    }
-  });
-  return app;
-}
-
-// ─── Tests ────────────────────────────────────────────────────────────────────
-
-describe("GET /health/ready", () => {
-  beforeEach(() => {
-    vi.restoreAllMocks();
-  });
-
-  it("returns 200 {status:'ready'} when DB query succeeds", async () => {
-    selectImpl = () => Promise.resolve([{ id: "staff-1" }]);
-
-    const app = await makeApp();
-    const res = await app.request("/health/ready", { method: "GET" });
-    const body = (await res.json()) as Record<string, unknown>;
-
-    expect(res.status).toBe(200);
-    expect(body.status).toBe("ready");
-  });
-
-  it("returns 503 {status:'degraded'} when DB query throws (schema dropped)", async () => {
-    const schemaErr = Object.assign(new Error("relation \"staff\" does not exist"), { code: "42P01" });
-    selectImpl = () => Promise.reject(schemaErr);
-
-    const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
-
-    const app = await makeApp();
-    const res = await app.request("/health/ready", { method: "GET" });
-    const body = (await res.json()) as Record<string, unknown>;
-
-    expect(res.status).toBe(503);
-    expect(body.status).toBe("degraded");
-
-    // Must not leak SQL error details in the response body
-    expect(JSON.stringify(body)).not.toContain("42P01");
-    expect(JSON.stringify(body)).not.toContain("relation");
-
-    consoleSpy.mockRestore();
-  });
-
-  it("returns 503 {status:'degraded'} on any DB connection error", async () => {
-    selectImpl = () => Promise.reject(new Error("ECONNREFUSED"));
-
-    const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
-
-    const app = await makeApp();
-    const res = await app.request("/health/ready", { method: "GET" });
-    const body = (await res.json()) as Record<string, unknown>;
-
-    expect(res.status).toBe(503);
-    expect(body.status).toBe("degraded");
-
-    consoleSpy.mockRestore();
-  });
-});

src/__tests__/readyz-auth-bypass.test.ts

@@ -0,0 +1,44 @@
+import { describe, it, expect, vi } from "vitest";
+import { Hono } from "hono";
+
+// Mock auth lib so getAuth() throws — non-bypass paths hit the 503 "not configured" guard.
+vi.mock("../lib/auth.js", () => ({
+  getAuth: () => {
+    throw new Error("auth not configured");
+  },
+  initAuth: vi.fn(),
+  getActiveProviders: vi.fn(() => []),
+}));
+
+describe("authMiddleware bypass: /api/readyz", () => {
+  it("serves /api/readyz without auth (bypass before auth check)", async () => {
+    // Ensure AUTH_DISABLED is not set so the bypass is exercised, not AUTH_DISABLED shortcut.
+    const prev = process.env.AUTH_DISABLED;
+    delete process.env.AUTH_DISABLED;
+
+    const { authMiddleware } = await import("../middleware/auth.js");
+    const app = new Hono();
+    app.use("/api/*", authMiddleware);
+    app.get("/api/readyz", (c) => c.json({ status: "ok" }, 200));
+
+    const res = await app.request("/api/readyz", { method: "GET" });
+    expect(res.status).toBe(200);
+
+    if (prev !== undefined) process.env.AUTH_DISABLED = prev;
+  });
+
+  it("blocks non-whitelisted /api/* paths when auth is not configured", async () => {
+    const prev = process.env.AUTH_DISABLED;
+    delete process.env.AUTH_DISABLED;
+
+    const { authMiddleware } = await import("../middleware/auth.js");
+    const app = new Hono();
+    app.use("/api/*", authMiddleware);
+    app.get("/api/staff", (c) => c.json({ ok: true }, 200));
+
+    const res = await app.request("/api/staff", { method: "GET" });
+    expect(res.status).toBe(503);
+
+    if (prev !== undefined) process.env.AUTH_DISABLED = prev;
+  });
+});

src/index.ts

@@ -65,17 +65,6 @@ app.use(
 app.get("/health", (c) => c.json({ status: "ok" }));
 // /api/health: used by Gateway HTTPRoute (/api/* → API pod)
 app.get("/api/health", (c) => c.json({ status: "ok" }));
-// /health/ready: DB-touching readiness probe — K8s removes pod from endpoints when schema is dropped (GRO-2689)
-app.get("/health/ready", async (c) => {
-  try {
-    await getDb().select({ id: staff.id }).from(staff).limit(1);
-    return c.json({ status: "ready" }, 200);
-  } catch (err) {
-    const pgCode = (err as Record<string, unknown>).code ?? "unknown";
-    console.error("[health/ready] DB check failed:", pgCode);
-    return c.json({ status: "degraded" }, 503);
-  }
-});
 // /api/readyz: DB-touching deep health check consumed by monitoring (not K8s probes)
 // Distinct from /health so a dropped schema triggers an alert without cycling pods (GRO-2678)
 app.get("/api/readyz", async (c) => {

src/middleware/auth.ts

@@ -23,7 +23,7 @@ if (process.env.AUTH_DISABLED === "true") {
 }
 
 export const authMiddleware: MiddlewareHandler = async (c, next) => {
-  if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health") {
+  if (c.req.path.startsWith("/api/auth/") || c.req.path === "/api/health" || c.req.path === "/api/readyz") {
     await next();
     return;
   }