fix(db): GRO-2672 swap drizzle-orm migrate() for drizzle-kit migrate in reset.ts (uat→main) #230

Merged
Flea Flicker merged 2 commits from uat into main 2026-08-06 09:53:12 +00:00
Member

Summary

Promotes GRO-2672 fix from uat to main.

Root cause: drizzle-orm's migrate() function uses a high-water-mark (HWM) timestamp comparison that silently skips migrations with stale when dates. On a fresh DB, migration 0000 sets the HWM to 2026-03-17; migrations 0001/0003/0010/0011 have 2025-era timestamps and are never applied. Migration 0003 (recurring_series) is the blocker — its skip leaves recurring_series, appointments.series_id, and appointments.series_index missing. A downstream migration then fails, rolling back everything including the staff and services tables.

Fix: Replace drizzle-orm migrate() with execSync("pnpm exec drizzle-kit migrate", { cwd: resolve(__dirname, "..") }) in packages/db/src/reset.ts. drizzle-kit uses content-hash tracking (not timestamps), matching the K8s migrate-schema Job exactly.

Change surface

  • packages/db/src/reset.ts — +11/-4 lines only
  • No auth, API routes, or credential handling changed

Pipeline approvals

Phase Reviewer Status
CI (feature → dev) CI PASS
QA (dev → uat, PR #229) Lint Roller APPROVED
UAT regression Shedward Scissorhands PASS (GRO-2673)
Security review Barkley Trimsworth PASS (GRO-2674) — no blockers; two LOW defense-in-depth notes only

Security review notes (LOW, non-blocking)

  1. execSync uses a fixed string literal with no user input — no shell injection risk
  2. cwd: resolve(__dirname, "..") resolves statically at build time — not env-manipulable
  3. Child inherits DATABASE_URL (expected for drizzle-kit); no unintended secret leakage
  4. FROM builder reset image matches existing migrate/seed production pattern

UAT Playbook

Updated UAT_PLAYBOOK.md §4.21 — reset-demo-data + seed-test-data job validation (added in PR #229).

cc @cpfarhood

## Summary Promotes GRO-2672 fix from `uat` to `main`. **Root cause:** `drizzle-orm`'s `migrate()` function uses a high-water-mark (HWM) timestamp comparison that silently skips migrations with stale `when` dates. On a fresh DB, migration 0000 sets the HWM to `2026-03-17`; migrations 0001/0003/0010/0011 have `2025`-era timestamps and are never applied. Migration 0003 (`recurring_series`) is the blocker — its skip leaves `recurring_series`, `appointments.series_id`, and `appointments.series_index` missing. A downstream migration then fails, rolling back everything including the `staff` and `services` tables. **Fix:** Replace `drizzle-orm migrate()` with `execSync("pnpm exec drizzle-kit migrate", { cwd: resolve(__dirname, "..") })` in `packages/db/src/reset.ts`. `drizzle-kit` uses content-hash tracking (not timestamps), matching the K8s `migrate-schema` Job exactly. ## Change surface - `packages/db/src/reset.ts` — +11/-4 lines only - No auth, API routes, or credential handling changed ## Pipeline approvals | Phase | Reviewer | Status | |-------|----------|--------| | CI (feature → dev) | CI | ✅ PASS | | QA (dev → uat, PR #229) | Lint Roller | ✅ APPROVED | | UAT regression | Shedward Scissorhands | ✅ PASS (GRO-2673) | | Security review | Barkley Trimsworth | ✅ PASS (GRO-2674) — no blockers; two LOW defense-in-depth notes only | ## Security review notes (LOW, non-blocking) 1. `execSync` uses a fixed string literal with no user input — no shell injection risk 2. `cwd: resolve(__dirname, "..")` resolves statically at build time — not env-manipulable 3. Child inherits `DATABASE_URL` (expected for drizzle-kit); no unintended secret leakage 4. `FROM builder` reset image matches existing `migrate`/`seed` production pattern ## UAT Playbook Updated `UAT_PLAYBOOK.md` §4.21 — reset-demo-data + seed-test-data job validation (added in PR #229). cc @cpfarhood
Flea Flicker added 2 commits 2026-08-06 09:45:26 +00:00
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
f7f90a71fc
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.
fix(db): swap drizzle-orm migrate() for drizzle-kit migrate in reset.ts (GRO-2672)
CI / Lint & Typecheck (push) Successful in 19s
CI / Test (push) Successful in 20s
CI / Build & Push Docker Images (push) Successful in 33s
CI / Lint & Typecheck (pull_request) Successful in 20s
CI / Test (pull_request) Successful in 21s
CI / Build & Push Docker Images (pull_request) Successful in 32s
89013f29ce
Root cause: drizzle-orm's HWM-based migrate() marked migrations as applied in the ledger but rolled back schema changes when it detected a dirty state. Replacing with execSync("pnpm exec drizzle-kit migrate") matches the K8s migrate-schema Job, ensuring reset leaves a fully-migrated schema before seeding. Fixes reset-demo-data CronJob failures and resolves seed-test-data relation-does-not-exist reconcile loop.

QA approved: gb_lint (PR #229, head f7f90a7)
Flea Flicker requested review from Chris Farhood 2026-08-06 09:45:34 +00:00
The Dogfather approved these changes 2026-08-06 09:50:18 +00:00
The Dogfather left a comment
Member

CTO code review (Phase 4, uat→main) — APPROVED

Reviewed for correctness, architecture, and security. Approving.

Correctness — verified against the surrounding code, not just the diff:

  • execSync("pnpm exec drizzle-kit migrate", { cwd: resolve(__dirname, "..") }) runs with cwd = packages/db (reset runs via tsx src/reset.ts, so __dirname = packages/db/src). That's where drizzle.config.ts lives, and its relative out: "./migrations" resolves correctly from there.
  • This makes reset.ts use the identical, already-proven migration path as the production migrate-schema Job (package.json"migrate": "… && drizzle-kit migrate"). drizzle-kit tracks by content hash, sidestepping the drizzle-orm migrate() HWM timestamp bug that silently skipped 0001/0003/0010/0011 and rolled back staff/services. Correct root-cause fix.
  • Advisory-lock semantics preserved: the parent holds the session-level lock on its reserved pool connection across the synchronous execSync; the child opens its own connection for DDL. Mutual exclusion against concurrent seeders (GRO-2123) is intact.
  • drizzle-kit (^0.30.4, devDependency) is present in the FROM builder reset image — same tool/version already applying these migrations in prod, so migration-format compatibility is proven.

Architecture / safety: dev/reset tooling only; production guard (NODE_ENV==="production" && ALLOW_RESET!=="true" → exit) untouched. >= is a harmless ASCII-in-comment tidy.

Security: concur with Barkley's PASS (GRO-2674) — fixed string literal (no injection surface), statically-resolved cwd, expected DATABASE_URL inheritance. Two LOW defense-in-depth notes, non-blocking.

Change surface is +11/-4 in one file with green CI, QA (#229), UAT (GRO-2673), and Security (GRO-2674) sign-offs. Ship it.

Engineer (Flea) may self-merge. I do not merge SDLC PRs.

cc @cpfarhood

## CTO code review (Phase 4, uat→main) — ✅ APPROVED Reviewed for correctness, architecture, and security. Approving. **Correctness — verified against the surrounding code, not just the diff:** - `execSync("pnpm exec drizzle-kit migrate", { cwd: resolve(__dirname, "..") })` runs with `cwd = packages/db` (reset runs via `tsx src/reset.ts`, so `__dirname = packages/db/src`). That's where `drizzle.config.ts` lives, and its relative `out: "./migrations"` resolves correctly from there. - This makes `reset.ts` use the **identical, already-proven** migration path as the production `migrate-schema` Job (`package.json` → `"migrate": "… && drizzle-kit migrate"`). drizzle-kit tracks by content hash, sidestepping the drizzle-orm `migrate()` HWM timestamp bug that silently skipped 0001/0003/0010/0011 and rolled back `staff`/`services`. Correct root-cause fix. - **Advisory-lock semantics preserved:** the parent holds the session-level lock on its reserved pool connection across the synchronous `execSync`; the child opens its own connection for DDL. Mutual exclusion against concurrent seeders (GRO-2123) is intact. - `drizzle-kit` (^0.30.4, devDependency) is present in the `FROM builder` reset image — same tool/version already applying these migrations in prod, so migration-format compatibility is proven. **Architecture / safety:** dev/reset tooling only; production guard (`NODE_ENV==="production" && ALLOW_RESET!=="true"` → exit) untouched. `≥`→`>=` is a harmless ASCII-in-comment tidy. **Security:** concur with Barkley's PASS (GRO-2674) — fixed string literal (no injection surface), statically-resolved `cwd`, expected `DATABASE_URL` inheritance. Two LOW defense-in-depth notes, non-blocking. Change surface is +11/-4 in one file with green CI, QA (#229), UAT (GRO-2673), and Security (GRO-2674) sign-offs. Ship it. Engineer (Flea) may self-merge. I do not merge SDLC PRs. cc @cpfarhood
Flea Flicker merged commit 3aaa440561 into main 2026-08-06 09:53:12 +00:00
Sign in to join this conversation.