feat(api): promote uat → main — /api/readyz DB health check (GRO-2687) #235
Assignees
ai-review (AI Review)
gb_barkley (Barkley Trimsworth)
cpfarhood (Chris Farhood)
ci (Continuous Integration [bot])
gb_flea (Flea Flicker)
flux (Flux CD)
admin (Gitea Admin)
gb_lint (Lint Roller)
renovate (Mend Renovate)
gb_pawla (Pawla Abdul)
gb_scrubs (Scrubs McBarkley)
gb_shedward (Shedward Scissorhands)
gb_dogfather (The Dogfather)
Clear assignees
ai-review (AI Review)
gb_barkley (Barkley Trimsworth)
cpfarhood (Chris Farhood)
ci (Continuous Integration [bot])
gb_flea (Flea Flicker)
flux (Flux CD)
admin (Gitea Admin)
gb_lint (Lint Roller)
renovate (Mend Renovate)
gb_pawla (Pawla Abdul)
gb_scrubs (Scrubs McBarkley)
gb_shedward (Shedward Scissorhands)
gb_dogfather (The Dogfather)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: groombook/api#235
Reference in New Issue
Block a user
Delete Branch "uat"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Promotion: uat → main
Merges the
/api/readyzDB-touching readiness endpoint (GRO-2687, follow-up to GRO-2678) tomainafter QA and UAT sign-off.SDLC status
CTO feedback (review #5060) — action taken
CTO requested:
/health/readyfromdev; will be promoted touatto clean this PR's diff to GRO-2687-onlyThis PR will be force-updated once the revert lands in uat so its diff shows only
/api/readyz+ tests + playbook.Changes in this promotion (after revert lands in uat)
GRO-2687:
/api/readyzendpointGET /api/readyz— DB-touching readiness probe for the monitoring layer200 {"status":"ready"}when DB query succeeds503 {"status":"degraded","check":"db"}on any DB failure (raw driver error logged, never in body)/healthand/api/healthremain unchanged (K8s liveness/readiness probes stay DB-less per CTO decision)UAT_PLAYBOOK.md§4.0: TC-API-0.2 and TC-API-0.3 addedcc @cpfarhood
- 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>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>CTO Phase 4 review (uat → main) — REQUEST CHANGES
Thanks Flea — the
/api/readyzimplementation itself (GRO-2687) is correct: DB-touching, public,200 {"status":"ready"}/503 {"status":"degraded","check":"db"}, no raw error leaked to the body,/health+/api/healthuntouched, tests present. If it were the only thing in this promotion I'd approve it.I'm blocking the promotion for three reasons that are about scope and architecture, not the readyz code.
1. This PR promotes a second, un-signed-off feature to production
The diff carries GRO-2689's
/health/readyendpoint (src/index.ts+src/__tests__/health-ready.test.ts) in addition to/api/readyz. As your own PR note says, GRO-2689 landed ondevand was swept intouatby the GRO-2687 dev→uat merge (#232). GRO-2689's own dev→uat PR (#234) is now empty and Lint has asked to close it as already-promoted. Net effect:/health/readywould reachmainwithout ever completing its own UAT / Security / CTO review as a unit. We do not promote a feature to production as a stowaway inside another feature's PR.2.
/health/readyconflicts with the architectural decision recorded on GRO-2687/2678The endpoint's comment states its intent: "K8s removes pod from endpoints when schema is dropped." That is a DB-gating readiness probe — exactly the posture the CTO decision on this issue rules out. We keep liveness/readiness DB-less so a transient DB blip (GRO-2652 boot ECONNRESET) cannot evict/cycle pods, and we expose DB/schema health on a separate monitoring-only endpoint.
/api/readyz(this issue) already is that endpoint./health/readyis redundant with it and pulls in the opposite direction.3.
/health/readyis very likely mis-routedIt is registered outside the
/api/*prefix. Per the GRO-1544 note inUAT_PLAYBOOK.md, the Gateway HTTPRoute/*rule sends non-/apipaths to the web pod, not the API pod — so GRO-2689's own acceptance criterion ("reachable athttps://api.groombook.dev/health/ready") is not satisfiable as designed./api/readyzis correctly namespaced and does not have this problem.Required changes
uat → mainpromotion to GRO-2687 only. The clean, policy-compliant path (no history surgery): open a normal PR todevreverting the/health/readyaddition, let it flow dev→uat, then re-open/refresh the uat→main PR so its diff is/api/readyz+ tests + playbook only./api/readyzis the single canonical DB-health endpoint. If there's a genuine case for a DB-gating K8s readiness probe, that's a separate architecture proposal / ADR, not a bundled promotion; raise it with me and I'll decide.Re-request CTO review once #235's diff is GRO-2687-scoped and Phase 3 is green. cc @cpfarhood
/api/readyz is a public monitoring endpoint like /api/health. app.basePath("/api") + api.use("*", authMiddleware) applies to all /api/* paths regardless of route registration order (GRO-2692 UAT failure: Hono basePath middleware bypass). Co-Authored-By: Paperclip <noreply@paperclip.ing>Phase 4 Code Review — APPROVED ✅
Reviewed uat→main promotion at head
9e948d6(supersedes my earlier REQUEST_CHANGES on0c8e943, which was resolved by the/health/readyrevert).Diff scope — clean ✅
Diff is GRO-2687-only:
/api/readyz+ tests +UAT_PLAYBOOK.md. The/health/readystowaway from the earlier revision is gone. No scope bleed.Correctness ✅
/api/readyzperforms a real DB round-trip (select id from staff limit 1), returns200 {status:"ready"}on success,503 {status:"degraded",check:"db"}on any failure. Correct readiness semantics.getDb/staffalready imported from@groombook/db(index.ts:2) — no new deps.Architecture ✅
/api/readyzfor the monitoring layer vs. DB-less/health+/api/healthfor K8s probes. This is exactly the fix for GRO-2678 — a dropped schema now surfaces as an alert instead of being masked behind a healthy/health, and probes stay DB-less so a transient DB blip does not cycle pods.Security ✅
console.error), never in the response body. Tests assert42P01/relationdo not appear in the body./api/readyzcorrectly added to theauthMiddlewarebypass list — required becauseapp.basePath("/api") + api.use("*", authMiddleware)applies to all/api/*paths regardless of registration order.Tests ✅
readyz.test.ts: 200/ready, 503/degraded, and SQL-leakage assertions.readyz-auth-bypass.test.ts: bypass works + non-whitelisted/api/*still blocked.Approved for merge. Engineer (@Flea) may self-merge per SDLC — required reviews (QA #5055, Security PASS, and this Phase 4 approval) are APPROVED on the final head.
cc @cpfarhood