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

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

.mcp.json

@@ -1,11 +0,0 @@
-{
-  "mcpServers": {
-    "gitea": {
-      "type": "http",
-      "url": "https://git-mcp.farh.net/mcp",
-      "headers": {
-        "Authorization": "Bearer ${GITEA_TOKEN}"
-      }
-    }
-  }
-}

AGENTS.md

@@ -0,0 +1,54 @@
+# AGENTS.md
+
+This repository (`groombook/api`) is part of the GroomBook application stack. The
+authoritative process, quality bar, and safety rules live in the shared
+[`groombook/org`](https://git.farh.net/groombook/org) skills repository. Read
+those first; this file is only a pointer.
+
+## Authoritative skills
+
+- **SDLC (branching, PRs, phases, handoffs):**
+  [`groombook/org/skills/sdlc/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/sdlc/SKILL.md)
+- **Coding standards (priority ordering, PR discipline, tests, no-hardcoded-values, CalVer):**
+  [`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md)
+- **Safety (no plaintext secrets, no direct `kubectl apply` to `groombook`, no self-merge, board approval for destructive actions):**
+  [`groombook/org/skills/safety/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/safety/SKILL.md)
+
+For human contributors and humans reviewing agent work, see
+[`CONTRIBUTING.md`](./CONTRIBUTING.md) in this repo for the phase-by-phase PR
+flow and the `uat→main` merge-gate policy summary.
+
+## Non-negotiable operational rules
+
+These mirror the org skills; they are restated here so any agent landing in
+this repo sees them without a cross-repo fetch.
+
+- **All changes go through a PR.** Never push directly to `dev`, `uat`, or `main`.
+- **Branch strategy:** `feature/<name>` → `dev` → `uat` → `main`. Engineers
+  always target `dev` first.
+- **No self-merge contract.** The engineer who opened a PR clicks merge only
+  after the named reviewer (CI / QA / UAT / Security / CTO per phase)
+  approves. Issue-thread QA / UAT / security approvals do **not** clear the
+  Gitea `required_approvals` gate on `uat→main` — only a Gitea **Approve**
+  click from a member of the `approvals_whitelist_username` does. On this
+  repo that whitelist is `["gb_flea", "gb_dogfather"]` (engineer team).
+  Board-level accounts cannot give the Approve click by policy.
+- **Always include `cc @cpfarhood`** at the bottom of every PR body for
+  board visibility (not as a reviewer).
+- **Secrets in code are forbidden.** Use Bitnami Sealed Secrets; never commit
+  plaintext. See the `safety` skill.
+- **Production (`groombook` namespace) is Flux-managed.** Never
+  `kubectl apply` directly. Infrastructure changes go through PRs in
+  `groombook/infra`.
+
+## Local development
+
+See the repo's own README, package scripts, and CI workflow. The
+authoritative pipeline (Gitea Actions, image build, deploy hooks) is the
+shared `groombook/infra` overlay; do not reimplement it here.
+
+## When uncertain
+
+If a task conflicts with the org skills, **the org skills win**. Open an
+issue in `groombook/org` to propose a change rather than encoding a local
+exception.

CONTRIBUTING.md

@@ -0,0 +1,117 @@
+# Contributing to `groombook/api`
+
+Thanks for contributing. This document is the human-facing companion to
+[`AGENTS.md`](./AGENTS.md) and the authoritative
+[`groombook/org`](https://git.farh.net/groombook/org) skills. The org skills
+govern; this file is a quick-reference for the human/agent PR flow in this
+repo.
+
+## Branch strategy
+
+Three long-lived branches; one PR per promotion step.
+
+| Branch  | Environment | Who merges | Prerequisites for merge |
+|---------|-------------|-----------|-------------------------|
+| `dev`   | Dev         | Engineer  | CI passes               |
+| `uat`   | UAT         | Engineer  | QA code review approval |
+| `main`  | Production  | Engineer  | UAT validation + CTO Gitea Approve when the `uat→main` merge-gate policy applies (see below) |
+
+Engineers always target `dev` first. Feature branches: `<agent-name>/<short-description>`.
+
+## Phase-by-phase PR flow
+
+### Phase 1 — Dev
+
+1. Branch from `dev`: `git checkout -b <name>/<short-description> origin/dev`.
+2. Write code + tests. Run unit tests, type check, and lint locally (or rely on CI).
+3. Open a PR against `dev`:
+   ```bash
+   tea pr create --base dev --title "..." --body "..."
+   ```
+   Include `cc @cpfarhood` at the bottom of the body for board visibility.
+4. CI must pass. CI green → engineer self-merges.
+5. CI builds and deploys to Dev automatically.
+
+### Phase 2 — UAT promotion
+
+1. Open a PR from `dev` to `uat`.
+2. CI must pass.
+3. **QA (Lint Roller)** reviews and approves on the Gitea PR.
+4. QA approved → engineer self-merges.
+5. CI builds and deploys to UAT automatically.
+
+### Phase 3 — UAT regression + Security review
+
+1. **UAT (Shedward Scissorhands)** runs full regression against UAT — every
+   feature, old and new, no exceptions.
+2. **Security (Barkley Trimsworth)** reviews the changes.
+3. Failures in either gate bounce back to Phase 1.
+
+### Phase 4 — Production promotion (`uat → main`)
+
+This is the gate the org PR
+[`groombook/org#13`](https://git.farh.net/groombook/org/pulls/13) defines.
+The full rule is in
+[`groombook/org/skills/sdlc/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/sdlc/SKILL.md)
+and
+[`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md);
+the summary is below.
+
+**The CTO Gitea Approve click is NOT the default gate.** Once the four
+pre-gates (QA, UAT deploy, UAT regression, security) are green, the engineer
+self-merges.
+
+**A CTO Gitea Approve click IS required** only for PRs in one of three
+categories:
+
+1. **Novel auth / session paths** — login, OIDC, OOBE, session middleware,
+   token issuance, password reset, MFA, new auth provider integrations.
+   Routine auth-gated UI (button styling, error messages, form layout) is
+   **not** in this category.
+2. **Infra / prod-affecting merges** — deploys, infra manifests, secrets,
+   GitOps overlays, CI/CD, `main` branch protection, production
+   routing/ingress, prod state mutations. All Phase 5 infra overlay PRs in
+   `groombook/infra` require CTO Gitea Approve without exception.
+3. **Risk-flagged merges** — `risk:cto-approve` label, or explicit CTO/CEO
+   sign-off request in the PR or issue thread.
+
+The engineer opens the `uat→main` PR, classifies it against the three
+categories above, and adds `cc @cpfarhood`. If the PR is in scope, the CTO
+clicks Approve; once approved (and the four pre-gates are green), the
+engineer merges.
+
+### Phase 5 — Production deployment
+
+A separate PR in `groombook/infra` bumps the overlay image tag for prod.
+Handed to QA (Lint Roller) for review, then self-merged by the engineer.
+
+## The four pre-gates (uat→main)
+
+A `uat→main` PR is mergeable when **all four** are green:
+
+1. **QA code review** — done on the dev→uat promotion PR.
+2. **UAT deploy** — the UAT image built from the uat tip is live in UAT.
+3. **UAT regression** — Shedward's full-feature UAT pass is green (no
+   pre-existing defects, no new defects).
+4. **Security review** — Barkley's security code review is green.
+
+Issue-thread QA / UAT / security approvals do **not** clear the Gitea
+`required_approvals` gate. Only a Gitea **Approve** click from a member of
+the `approvals_whitelist_username` for `main` clears it. In this repo that
+whitelist is the engineer team (`gb_flea`, `gb_dogfather`).
+
+## Style, tests, and quality bar
+
+See
+[`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md)
+for the engineering priority ordering, test requirements, no-hardcoded-values
+rules, CalVer versioning policy, and the `git.farh.net` container registry
+policy.
+
+## Safety
+
+See
+[`groombook/org/skills/safety/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/safety/SKILL.md)
+for the non-negotiable rules: no plaintext secrets, no `kubectl apply` to
+`groombook`, no self-merge, no direct `tofu` runs, board approval for
+destructive actions, escalation protocol.

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);
   });
 });