Skip to content

drizzle v1 - #2338

Open
thibaultleouay wants to merge 2 commits into
mainfrom
drizzle-v1
Open

drizzle v1#2338
thibaultleouay wants to merge 2 commits into
mainfrom
drizzle-v1

Conversation

@thibaultleouay

@thibaultleouay thibaultleouay commented Jul 2, 2026

Copy link
Copy Markdown
Member

Review in cubic

@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
openstatus-dashboard Error Error Jul 2, 2026 11:11am
openstatus-status-page Error Error Jul 2, 2026 11:11am
openstatus-web Ready Ready Preview, Comment Jul 2, 2026 11:11am

Request Review

@thibaultleouay

Copy link
Copy Markdown
Member Author

@claude review this

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown

Claude finished @thibaultleouay's task in 2m 30s —— View job


Claude finished reviewing this PR

Todo list

  • Gather context (PR diff, changed files, CLAUDE.md conventions)
  • Understand the request (code review of drizzle v1 migration PR)
  • Review Prisma-era → Drizzle v1 migration in packages/db (client wiring, relations.ts, drizzle.config.ts, migration folder restructure)
  • Review call-site rewrites in apps/server, apps/dashboard, apps/workflows, packages/api, packages/services
  • Check for RQBv2/through correctness and services/audit-log conformance
  • Post review

Summary

This is a large, mechanical, well-executed migration off Prisma-flavored Drizzle relations to Drizzle ORM v1's defineRelations/RQBv2 API, plus a drizzle-zoddrizzle-orm/zod swap and a migration-folder restructure. packages/db/DRIZZLE_V1_MIGRATION.md is an excellent artifact — it documents the plan, decisions, and validation state in detail, and the code matches what it describes. Spot-checked the central relations.ts, all 3 drizzle() client wirings, the heaviest call site (statusPage.ts), several packages/services verbs, and the drizzle-zoddrizzle-orm/zod validation-schema fixes (audit_logs, external_services) — all consistent and correct.

🔴 Blocking: this PR ships with drizzle-orm/drizzle-kit still pinned to an RC, contradicting the migration's own plan

pnpm-workspace.yaml catalog pins:

drizzle-kit: 1.0.0-rc.4
drizzle-orm: 1.0.0-rc.4

But DRIZZLE_V1_MIGRATION.md decision #1 states explicitly: "do not merge/deploy to prod until v1 is GA. No RC in production." Phase 4 ("Ship (at GA)") is entirely unchecked and marked BLOCKED, including flipping the catalog to GA versions.

More importantly, this PR also restructures packages/db/drizzle/** (renaming every migration file into the new v1 timestamped-folder format + adding snapshot.json per folder, deleting meta/_journal.json). .github/workflows/migrate.yml fires pnpm migrate against prod on every push to main that touches packages/db/drizzle/**. Per the doc's own decision #3, that migrator run needs to be proven idempotent via a local dry-run seeded from a dump of prod's __drizzle_migrations table first — and Phase 1b lists that dry-run as PENDING, not run in this session, along with the pre-migrate prod snapshot (decision #4).

So: merging this PR to main as-is would auto-fire an unvalidated v1 migrator against production, on an RC dependency version, without the safety snapshot the plan itself requires. This looks like exactly the scenario the migration doc was written to prevent. Worth confirming before merge:

  • Has the clone dry-run against a prod __drizzle_migrations dump been run since this doc was last updated?
  • Is the catalog meant to flip to GA drizzle-orm/drizzle-kit in this same PR, or a follow-up right before merge?
  • Has the prod DB snapshot (decision ✏️ #4) been taken?

Fix this →%20flip%20pnpm-workspace.yaml%20catalog%20drizzle-orm%2Fdrizzle-kit%20from%201.0.0-rc.4%20to%20the%20GA%20release%2C%202)%20run%20the%20clone%20dry-run%20described%20in%20packages%2Fdb%2FDRIZZLE_V1_MIGRATION.md%20Phase%201b%20(seed%20a%20local%20libsql%20file%20with%20a%20dump%20of%20prod%27s%20__drizzle_migrations%20table%20and%20confirm%20zero%20pending%20migrations%20under%20the%20restructured%20packages%2Fdb%2Fdrizzle%2F%20folder)%2C%203)%20take%20the%20prod%20DB%20snapshot%20per%20decision%20%234%20before%20merging%2C%20since%20merging%20to%20main%20auto-fires%20.github%2Fworkflows%2Fmigrate.yml%20against%20prod.&repo=openstatusHQ/openstatus)

Nits (non-blocking)

  • apps/dashboard/src/lib/auth/adapter.ts: a second // @ts-expect-error: problem with type was added for verificationTokensTable (matching the existing pattern for the other 3 tables). Per CLAUDE.md's Type Cast Discipline, these @ts-expect-errors on @auth/drizzle-adapter are a boundary workaround for the library's v1 type mismatch — consider a one-line comment on why it's safe (e.g. pointing at the upstream @auth/drizzle-adapter issue/version) so it isn't mistaken for an accidental suppression later, and so it's easy to find and remove once the adapter catches up to Drizzle v1 types.
  • packages/db/DRIZZLE_V1_MIGRATION.md is a working/planning doc with checkboxes and "PENDING"/"BLOCKED" items still open. If it's meant to stay in the repo as historical record, that's fine, but it currently reads like a todo list left in-tree rather than documentation — worth a final pass (or deletion) once Phase 4 actually completes, so it doesn't go stale.

What looks solid

  • relations.ts: all 26 previously-scattered relations() blocks consolidated correctly; the through deviation for payload-carrying junctions (statusReportUpdateToPageComponents.impact etc.) is sensible and documented.
  • Call-site rewrites (spot-checked statusPage.ts, resolve-active-workspace.ts, several packages/services verbs) consistently use object-form where for simple predicates and callback/RAW form only where needed (OR, raw SQL) — matches decision Init clerk webhook #7, and the RAW table-aliasing bug ({RAW: sql\...`}{RAW: (t) => sql`...`}`) is fixed everywhere it appeared.
  • drizzle-zoddrizzle-orm/zod swap is clean; no leftover drizzle-zod imports or package.json/catalog entries found.
  • Migration-folder rename is pure git mv (0 content diff) plus additive snapshot.json — content-hash-based migration tracking should be unaffected, consistent with the doc's reasoning.

I wasn't able to run pnpm check/deno test in this environment (no network access to fetch origin/main or install deps beyond what's checked out), so I relied on static diff review — the doc states these were run and passing locally, which lines up with what I found reading the code.
· branch: drizzle-v1

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This PR is large and would use a significant portion of your monthly review quota. Comment @cubic-dev-ai review this to confirm that you want cubic to review it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant