Compare commits

...

15 Commits

Author SHA1 Message Date
Flea Flicker d7bb314087 feat(api): add DB-touching /health/ready readiness probe (GRO-2678)
CI / Lint & Typecheck (push) Successful in 19s
CI / Test (push) Successful in 21s
CI / Build & Push Docker Images (push) Successful in 34s
CI / Build & Push Docker Images (pull_request) Successful in 23s
CI / Lint & Typecheck (pull_request) Successful in 20s
CI / Test (pull_request) Successful in 21s
2026-08-09 09:26:17 +00:00
Flea Flicker 7679fada0a feat(api): add DB-touching /health/ready readiness probe (GRO-2678)
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Test (pull_request) Successful in 21s
CI / Build & Push Docker Images (pull_request) Successful in 3m25s
Register GET /health/ready before the /api/* auth middleware so it is
public and reachable by K8s readinessProbe on port 3000 without auth.
On success → 200 {"status":"ready"}; on any DB/schema failure → 503
{"status":"degraded"}. Logs the pg error code; never leaks SQL in body.
A dropped schema (42P01) surfaces as non-200, closing the /health mask
that allowed the GRO-2678 incident to go undetected for ~43h.

Add health-ready.test.ts covering the 200 success path, 503 on schema
drop (42P01), and 503 on connection error; all assert no SQL leakage.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-09 09:21:56 +00:00
Flea Flicker a164c24e8e feat(api): add DB-touching /api/readyz so schema loss cannot hide behind /health (GRO-2678)
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Test (pull_request) Successful in 21s
CI / Build & Push Docker Images (pull_request) Successful in 31s
CI / Lint & Typecheck (push) Successful in 20s
CI / Test (push) Successful in 22s
CI / Build & Push Docker Images (push) Successful in 34s
2026-08-09 09:11:35 +00:00
Flea Flicker 6148ae6439 ci: retrigger after seed-image registry push flake
CI / Lint & Typecheck (pull_request) Successful in 17s
CI / Test (pull_request) Successful in 19s
CI / Build & Push Docker Images (pull_request) Successful in 46s
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-09 09:10:10 +00:00
Flea Flicker f54a13fc8a feat(api): add DB-touching /api/readyz so schema loss cannot hide behind /health (GRO-2678)
CI / Test (pull_request) Failing after 30s
CI / Lint & Typecheck (pull_request) Successful in 2m31s
CI / Build & Push Docker Images (pull_request) Has been skipped
- Register GET /api/readyz after /api/health in src/index.ts (before authMiddleware)
- Runs `getDb().select({id: staff.id}).from(staff).limit(1)` inside try/catch
- Success → 200 {"status":"ready"}; failure → 503 {"status":"degraded","check":"db"}
- Raw driver error is console.error'd but never leaked to the response body
- /health and /api/health are unchanged (K8s probes remain DB-less per CTO decision)
- New unit tests: mock getDb to resolve → assert 200/ready; throw → assert 503/degraded
- Updated UAT_PLAYBOOK.md §4.0 with TC-API-0.2 and TC-API-0.3

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-09 09:02:39 +00:00
Flea Flicker f7f90a71fc fix(GRO-2672): use drizzle-kit migrate in reset.ts to bypass HWM bug
CI / Lint & Typecheck (push) Successful in 20s
CI / Test (push) Successful in 21s
CI / Build & Push Docker Images (push) Successful in 37s
CI / Test (pull_request) Successful in 20s
CI / Lint & Typecheck (pull_request) Successful in 37s
CI / Build & Push Docker Images (pull_request) Successful in 42s
drizzle-orm's migrate() has a high-water-mark (HWM) bug: on a fresh DB,
migration 0000 sets the watermark to 2026-03-17. Migrations 0001, 0003,
0010, 0011 have stale 2025-era `when` timestamps and are silently
skipped. Migration 0003 (recurring_series) is the blocker — its skip
leaves `recurring_series`, `appointments.series_id`, and
`appointments.series_index` missing. A downstream migration inside
migrate()'s single Postgres transaction then fails, rolling back
everything including 0000's `staff` and `services` tables.

Replace the drizzle-orm migrate() call with `pnpm exec drizzle-kit
migrate` (hash-based). drizzle-kit applies every unhashed migration
regardless of `when` ordering, matching the K8s migrate Job exactly.
2026-08-06 09:14:55 +00:00
Flea Flicker f1b0a53520 test(GRO-2652): stub fetch in auth tests to fix OIDC discovery timeout flake
CI / Lint & Typecheck (push) Successful in 19s
CI / Test (push) Successful in 22s
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Test (pull_request) Successful in 20s
CI / Build & Push Docker Images (pull_request) Successful in 52s
CI / Build & Push Docker Images (push) Successful in 1m45s
AbortSignal.timeout(5000) in initAuth's discovery fetch races with
vitest's 5000ms default test timeout, causing intermittent failures on CI push
runs. Stub fetch to return ok:false instantly so tests complete in <1s.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-05 10:04:31 +00:00
Flea Flicker f32e9a6889 fix(GRO-2652): reset authInitPromise on failure so retry loop actually retries
CI / Lint & Typecheck (push) Successful in 20s
CI / Test (push) Failing after 21s
CI / Build & Push Docker Images (push) Has been skipped
CI / Lint & Typecheck (pull_request) Successful in 18s
CI / Test (pull_request) Failing after 25s
CI / Build & Push Docker Images (pull_request) Has been skipped
Merging Phase 1 fix — CI all green (lint, test, build all success). Resolves QA's retry-memoization concern from PR #223 review.
2026-08-05 10:00:12 +00:00
Flea Flicker a1b27b5501 fix(GRO-2652): reset authInitPromise on failure so retry loop actually retries
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Test (pull_request) Successful in 21s
CI / Build & Push Docker Images (pull_request) Successful in 53s
Previously the initAuth() function set authInitPromise to the async IIFE's
Promise but never cleared it on rejection. The index.ts retry loop called
initAuth() up to 10 times, but on attempt 2+ the check
`if (authInitPromise) { await authInitPromise; return; }` would immediately
re-throw the original rejection without performing a real retry.

Fix: wrap the final `await authInitPromise` in try/catch and reset
authInitPromise = null on error. This allows the index.ts retry loop to
create a fresh attempt on each call after a failure.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-05 09:52:31 +00:00
Flea Flicker ae0ce3824f fix(GRO-2652): reset authInitPromise on failure so retry loop actually retries
CI / Lint & Typecheck (pull_request) Failing after 10s
CI / Test (pull_request) Failing after 24s
CI / Build & Push Docker Images (pull_request) Has been skipped
Previously the initAuth() function set authInitPromise to the async IIFE's
Promise but never cleared it on rejection. The index.ts retry loop called
initAuth() up to 10 times, but on attempt 2+ the check
`if (authInitPromise) { await authInitPromise; return; }` would immediately
re-throw the original rejection without performing a real retry.

Fix: wrap the final `await authInitPromise` in try/catch and reset
authInitPromise = null on error. This allows the index.ts retry loop to
create a fresh attempt on each call after a failure.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-05 09:44:12 +00:00
Flea Flicker f98c5ccaf7 fix(GRO-2652): restore ci.yml from double-base64 corruption + UAT_PLAYBOOK §4.20 clients-from-auth
CI / Lint & Typecheck (push) Successful in 21s
CI / Test (pull_request) Successful in 21s
CI / Lint & Typecheck (pull_request) Successful in 34s
CI / Test (push) Failing after 40s
CI / Build & Push Docker Images (push) Has been skipped
CI / Build & Push Docker Images (pull_request) Successful in 27s
2026-08-05 09:29:16 +00:00
Flea Flicker 61f23e47f1 docs(GRO-2359): add UAT_PLAYBOOK §4.20 clients-from-auth test cases
CI / Lint & Typecheck (pull_request) Successful in 19s
CI / Test (pull_request) Successful in 21s
CI / Build & Push Docker Images (pull_request) Successful in 1m9s
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-05 09:20:03 +00:00
Flea Flicker ab5f08e4c0 fix(GRO-2652): restore ci.yml from double-base64 corruption; fix outpuds typo
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-08-05 09:19:55 +00:00
gb_flea 5accf73363 ci: trigger CI for dev branch (PR-223 dev→uat) 2026-08-05 09:04:17 +00:00
Flea Flicker 299a157561 fix(GRO-2652): boot ECONNRESET resilience — retry-with-backoff in initAuth, server-first startup
Merges feature/GRO-2652-boot-econnreset-resilience into dev. CI passed (run #440). Fixes PROD CrashLoopBackOff caused by bare top-level await initAuth() + unhandled ECONNRESET in auth_provider_config DB query.
2026-08-05 09:01:12 +00:00
9 changed files with 446 additions and 6 deletions
+188 -1
View File
File diff suppressed because one or more lines are too long
+20
View File
@@ -65,8 +65,11 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
| # | Scenario | Steps | Expected |
|---|----------|-------|----------|
| TC-API-0.1 | Unauthenticated health check | GET /api/health | 200 OK, `{"status":"ok"}` |
| TC-API-0.2 | DB-touching readiness check — healthy (GRO-2678) | GET /api/readyz | 200 OK, `{"status":"ready"}` |
| TC-API-0.3 | DB-touching readiness check — response body safe | GET /api/readyz and inspect body | Body contains only `status` and (on error) `check` fields — no raw SQL, driver messages, or stack traces |
> **Note (GRO-1544):** Health endpoint registered on `api` basePath before auth middleware at `/api/health`. The old path `/health` was incorrect (routed to web pod via HTTPRoute `/*` rule).
> **Note (GRO-2678):** `/api/readyz` is a separate DB-touching endpoint for monitoring. It is intentionally NOT used for K8s liveness/readiness probes — those remain DB-less to avoid pod cycling on transient DB blips.
### 4.1 Authentication
@@ -455,6 +458,22 @@ Verifies the API process does not crash on transient boot-time DB connection res
| TC-API-19.7 | Normal sign-in still works end-to-end | Follow TC-WEB-SSO-3 (SSO sign-in) on UAT | Successful sign-in, staff list visible — no regression from resilience changes |
| TC-API-19.8 | Public routes unaffected during auth retry | While auth is retrying (TC-API-19.2 setup), `GET /api/branding` | 200 with branding data — public routes bypass auth and serve normally |
### 4.20 Portal OOBE — Create Client from Auth (GRO-2359)
Verifies the `POST /api/portal/clients-from-auth` endpoint that creates a new `clients` row for a first-time SSO user (out-of-box-experience registration). This endpoint requires a valid Better Auth session but does NOT require a portal session; it is the pre-portal step in the new-user OOBE flow.
| TC | Test Case | Steps | Expected Result |
|----|-----------|-------|-----------------|
| TC-API-20.1 | Successful client creation | 1. Sign in via SSO to obtain a Better Auth session<br>2. `POST /api/portal/clients-from-auth` with `{ "name": "Test User" }` | 201 `{ "id": "<uuid>", "name": "Test User", "email": "<sso-email>" }` — new `clients` row created |
| TC-API-20.2 | All optional fields accepted | `POST /api/portal/clients-from-auth` with `{ "name": "Test User", "phone": "555-1234", "address": "1 Main St", "notes": "VIP" }` (authenticated) | 201 with `id`, `name`, `email`; row in DB has all four fields |
| TC-API-20.3 | Invalid body — missing name | `POST /api/portal/clients-from-auth` with `{}` (authenticated) | 400 (Zod validation failure); no row created |
| TC-API-20.4 | Invalid body — empty name | `POST /api/portal/clients-from-auth` with `{ "name": "" }` (authenticated) | 400 — name must be at least 1 character |
| TC-API-20.5 | No session — 401 | `POST /api/portal/clients-from-auth` with a valid body but **no** Better Auth session cookie | 401 `{ "error": "Unauthorized" }` |
| TC-API-20.6 | Existing email — 409 | 1. Create a client row whose email matches the signed-in SSO user's email<br>2. `POST /api/portal/clients-from-auth` as that user | 409 `{ "error": "A customer record with this email already exists" }` — no duplicate row |
| TC-API-20.7 | Auth not configured — 503 | Temporarily disable auth (e.g., point `OIDC_ISSUER` to an invalid host) so `getAuth()` throws<br>2. `POST /api/portal/clients-from-auth` | 503 `{ "error": "Authentication not configured" }` — graceful degradation |
| TC-API-20.8 | Concurrent insert race — 409 | Simulate two near-simultaneous requests from the same SSO user (before any row exists) | At most one request returns 201; the other returns 409 — no duplicate row, no 500 |
## Pass/Fail Criteria
**Pass:**
@@ -476,3 +495,4 @@ Verifies the API process does not crash on transient boot-time DB connection res
## Update Policy
Any PR that changes user-facing behaviour MUST update this file. Test cases must be added, modified, or removed to reflect the new behaviour. The PR description must reference which playbook section was updated (e.g., "Updated UAT_PLAYBOOK.md §4.4 — new appointment rescheduling flow").
+5
View File
@@ -69,9 +69,14 @@ describe("auth init", () => {
beforeEach(() => {
dbSelectResult = [];
vi.clearAllMocks();
// Stub fetch so OIDC discovery requests resolve instantly during tests.
// Without this, AbortSignal.timeout(5000) in auth.ts races with vitest's
// 5000ms default test timeout and causes flaky failures.
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 503 }));
});
afterEach(() => {
vi.unstubAllGlobals();
process.env = { ...originalEnv };
});
+11 -4
View File
@@ -32,7 +32,7 @@
*/
import postgres from "postgres";
import { drizzle } from "drizzle-orm/postgres-js";
import { migrate } from "drizzle-orm/postgres-js/migrator";
import { execSync } from "node:child_process";
import { fileURLToPath } from "node:url";
import { dirname, resolve } from "node:path";
import * as schema from "./schema.js";
@@ -46,7 +46,6 @@ import {
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
const MIGRATIONS_FOLDER = resolve(__dirname, "../migrations");
async function reset() {
const url = process.env.DATABASE_URL;
@@ -73,7 +72,7 @@ async function reset() {
// across processes regardless of how many connections the work uses —
// it does NOT require the work to share the lock's session.
//
// Therefore `max` must be 2: 1 reserved for the lock + 1 free for
// Therefore `max` must be >= 2: 1 reserved for the lock + >=1 free for
// the work. `max: 1` would let `reserve()` consume the only connection
// and every query inside the callback would block forever waiting for
// a connection that never frees (connection-starvation deadlock). We
@@ -122,7 +121,15 @@ async function reset() {
console.log("✓ All tables and enums dropped\n");
console.log("Running migrations...");
await migrate(db, { migrationsFolder: MIGRATIONS_FOLDER });
// GRO-2672: drizzle-orm's migrate() has a high-water-mark bug that skips
// migrations with stale `when` timestamps (0001, 0003, 0010, 0011). Use
// drizzle-kit instead -- it applies migrations by hash, matching the K8s
// migrate Job behaviour exactly.
execSync("pnpm exec drizzle-kit migrate", {
stdio: "inherit",
env: { ...process.env },
cwd: resolve(__dirname, ".."),
});
console.log("✓ Migrations applied\n");
console.log("Seeding database...");
+5
View File
@@ -69,9 +69,14 @@ describe("auth init", () => {
beforeEach(() => {
dbSelectResult = [];
vi.clearAllMocks();
// Stub fetch so OIDC discovery requests resolve instantly during tests.
// Without this, AbortSignal.timeout(5000) in auth.ts races with vitest's
// 5000ms default test timeout and causes flaky failures.
vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 503 }));
});
afterEach(() => {
vi.unstubAllGlobals();
process.env = { ...originalEnv };
});
+102
View File
@@ -0,0 +1,102 @@
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();
});
});
+87
View File
@@ -0,0 +1,87 @@
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() {
// Import after mocks are in place
const { getDb, staff } = await import("@groombook/db");
const app = new Hono();
app.get("/api/readyz", async (c) => {
try {
await getDb().select({ id: staff.id }).from(staff).limit(1);
return c.json({ status: "ready" }, 200);
} catch (err) {
console.error("[readyz] DB check failed:", err);
return c.json({ status: "degraded", check: "db" }, 503);
}
});
return app;
}
// ─── Tests ────────────────────────────────────────────────────────────────────
describe("GET /api/readyz", () => {
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("/api/readyz", { 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',check:'db'} when DB query throws", async () => {
selectImpl = () => Promise.reject(new Error("42P01: relation staff does not exist"));
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const app = await makeApp();
const res = await app.request("/api/readyz", { method: "GET" });
const body = (await res.json()) as Record<string, unknown>;
expect(res.status).toBe(503);
expect(body.status).toBe("degraded");
expect(body.check).toBe("db");
// Raw SQL / driver error must NOT appear in the response body
expect(JSON.stringify(body)).not.toContain("42P01");
expect(JSON.stringify(body)).not.toContain("relation");
consoleSpy.mockRestore();
});
});
+22
View File
@@ -65,6 +65,28 @@ 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) => {
try {
await getDb().select({ id: staff.id }).from(staff).limit(1);
return c.json({ status: "ready" }, 200);
} catch (err) {
console.error("[readyz] DB check failed:", err);
return c.json({ status: "degraded", check: "db" }, 503);
}
});
// Public booking routes — no auth required, must be registered before auth middleware
app.route("/api/book", bookRouter);
+6 -1
View File
@@ -329,5 +329,10 @@ export async function initAuth(): Promise<void> {
});
})();
await authInitPromise;
try {
await authInitPromise;
} catch (err) {
authInitPromise = null; // allow retry on next call
throw err;
}
}