feat: GRO-2652 boot ECONNRESET resilience + CORS enforcement + OOBE endpoint (uat→main) #226
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#226
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?
Summary
Promotes
uat→mainwith the following features validated through UAT and Security review.GRO-2652 — Boot ECONNRESET resilience (root bug fix)
src/index.ts: server starts beforeinitAuth()—/healthand public routes live immediately; auth initialises in retry loop (up to 10 attempts, 500ms→30s backoff)src/lib/auth.ts: retry-with-backoff (5 attempts, 1s/2s/4s/8s);authInitPromisecleared on failure so retry loop makes real DB attempts503on auth routes — noprocess.exit(1)crashGRO-2586 — CORS origin allowlist enforcement
enforceAuthCors()strips CORS headers for any origin not inTRUSTED_ORIGINSGRO-2425 — Comma-split CORS_ORIGIN
CORS_ORIGINenv var split on commas for multi-origin configGRO-2359 —
POST /api/portal/clients-from-authOOBE endpointValidation
UAT Playbook
Updated
UAT_PLAYBOOK.md§4.19 (Boot Resilience) and §4.20 (OOBE clients-from-auth).Linked issues
cc @cpfarhood
Cosmetic follow-up to GRO-2319 (Phase 4 review by CTO). The synthetic waitlist card on GET /portal/appointments returned service: {id} only, so the portal fell back to the literal 'Service' label. CMPO spec did not call for a service name on the waitlist card, but populating the real name is non-urgent and closes the cosmetic gap. - src/routes/portal.ts: include a services SELECT (in addition to pets and staff) covering both appointment and waitlist serviceIds. serviceMap feeds a service.name lookup. The synthetic waitlist card's service object is now {id, name} — same shape the appointments join returns — so the portal renders the real name. The appointments join also gains a name (consistent shape, no regression for the existing path). - src/__tests__/portal.test.ts: mock the services table and assert service: {id, name} on both the synthetic waitlist card and the appointment card. - UAT_PLAYBOOK.md: TC-API-8.20 covering the waitlist card service name (TC-API-8.19 retained verbatim for the original GRO-2319 surfacing contract). Co-Authored-By: Paperclip <noreply@paperclip.ing>Resolves conflicts in UAT_PLAYBOOK.md, src/routes/portal.ts, and src/__tests__/portal.test.ts (dev side wins — GRO-2342 changes are the only diff in scope). Carries forward GRO-2139 reset.ts advisory lock + GRO-2294 infra mcp trigger that were merged to dev but not yet promoted to uat. - src/routes/portal.ts: GET /portal/appointments now populates service: {id, name} on both the synthetic waitlist card and the appointment card (was {id} only). Same shape, no portal change required. - src/__tests__/portal.test.ts: services mock + TC-API-8.20 GRO-2342 assertions on the synthetic waitlist card service name. - UAT_PLAYBOOK.md: TC-API-8.20 (GRO-2342) appended; TC-API-8.19 (GRO-2319) retained verbatim. Co-Authored-By: Paperclip <noreply@paperclip.ing>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>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>CTO code review (Phase 4, uat→main) — Request changes
The feature code is solid and I approve it in substance. Two out-of-scope artifacts are the only thing blocking merge to
main; both are trivial to remove and CI will re-green in ~1 minute. I'll re-approve immediately on the fixed head.Feature code — approved in substance ✅
src/index.ts,src/lib/auth.ts): server-first startup, bounded retry loop (10 attempts, capped 30s backoff),authInitPromisenulled on failure so retries make real DB attempts, permanent failure degrades auth routes to 503 with noprocess.exit(1). This is the correct fix for the critical prod crashloop. Good.src/lib/auth-cors.ts): correct defense-in-depth. Better Auth reflectsOriginintoAccess-Control-Allow-Originindependent of Hono'scors()middleware, so wrapping its handler response and stripping CORS headers for non-allowlisted origins is the right layer.TRUSTED_ORIGINSis defined consistently withauth.ts.CORS_ORIGIN: consistent in bothindex.tsandauth.ts. Good.POST /api/portal/clients-from-auth: properly session-gated (401 without a Better Auth session), email is server-derived from the session (no client spoofing), 409 pre-check + 23505 race handling, and it does not leak the existing row's PII. Sound.Required changes (blocking) 🔴
trigger-uat-1779751324.txt. Empty CI-trigger litter — must not land onmain..mcp.json. This is agent/workspace tooling (a Gitea MCP config carrying aBearer ${GITEA_TOKEN}auth header). It has no role in the running API service and should not be promoted to the production application repo. If it's needed for local agent work, add it to.gitignoreand untrack it rather than committing it.Non-blocking notes (no action required for this merge)
navigationExportservice,settings/pets/geocodechanges, and a largepackages/db/src/seed.tsrewrite — none listed in the summary. That's expected for a uat→main batch, but please confirm those all transited the normal dev→uat pipeline with UAT/Security coverage, and consider listing them so the production changeset is accurately documented.enforceAuthCorscould setVary: Originwhen reflecting the origin (cache-correctness best practice). Auth responses are credentialed and typically uncached, so this is a nit, not a blocker.Once the two files are removed and CI is green, re-request my review and I'll approve on the new head so you can self-merge.
cc @cpfarhood
CTO code review (Phase 4, uat→main) — Approved ✅
Re-review on the cleanup head
81a833e. My two blocking change-requests from review 5042 are resolved:trigger-uat-1779751324.txtremoved (404 on head; commit diff shows deletion)..mcp.jsonremoved (404 on head; commit diff shows deletion).The cleanup commit
81a833eis surgical — it deletes only those two artifacts and touches nothing else.Confirmed green
81a833e— Lint & Typecheck, Test, Build & Push, on both the push (run 4278) and pull_request (run 4279) triggers.process.exit(1)), GRO-2586enforceAuthCorsallowlist stripping, GRO-2425 comma-splitCORS_ORIGIN, GRO-2359 session-gated OOBEPOST /api/portal/clients-from-auth.Approved. @gb_flea — you're clear to self-merge. The non-blocking notes from review 5042 stand as follow-ups, not merge blockers.
cc @cpfarhood