Skip to content

Enforce drizzle-kit generated migrations with a custom lint check#2430

Merged
andrew-bierman merged 1 commit into
developmentfrom
codex/add-custom-check-for-migration-scripts
May 17, 2026
Merged

Enforce drizzle-kit generated migrations with a custom lint check#2430
andrew-bierman merged 1 commit into
developmentfrom
codex/add-custom-check-for-migration-scripts

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

@andrew-bierman andrew-bierman commented May 16, 2026

Motivation

  • Prevent ad-hoc handwritten SQL migrations that diverge from the drizzle-kit workflow and cause migration/journal mismatches.
  • Give CI and local developers an automated guardrail so new migrations are generated via drizzle-kit rather than hand-edited.
  • Allow existing legacy manual migrations to remain while enforcing the rule for new files.

Description

  • Add a new check script at scripts/lint/check-drizzle-migrations.ts that scans packages/api/drizzle and packages/osm-db/drizzle migration folders and reports violations.
  • The script enforces filename format using the regex ^\d{4}_[a-z0-9]+(?:_[a-z0-9]+)+\.sql$ and flags files that contain the drizzle template marker -- Custom SQL migration file, put your code below! --.
  • Include an allowlist for known legacy/manual migrations: packages/api/drizzle/0010_great_colleen_wing.sql and packages/osm-db/drizzle/0000_extensions.sql.
  • Wire the check into the master runner scripts/check-all.ts as check-drizzle-migrations and into root lint:custom in package.json.

Testing

  • Ran bun scripts/lint/check-drizzle-migrations.ts, which passed (no violations after adding the intentional allowlist).
  • Ran bun scripts/check-all.ts, which included the new check and reported the new check as passing while an unrelated existing check-react-doctor failed due to external npm registry 403 responses in this environment.
  • The pre-commit biome check lint step executed as part of the commit and passed for the modified files.

Codex Task

Summary by CodeRabbit

  • Chores
    • Enhanced database migration validation to enforce consistent naming and structure across SQL migration files, improving data integrity during deployments.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Walkthrough

A new lint script validates Drizzle SQL migration files against naming conventions and template markers, then integrates the check into the build orchestrator and custom lint script chain.

Drizzle Migration File Validation

Layer / File(s) Summary
Drizzle migration validation and integration
scripts/lint/check-drizzle-migrations.ts, scripts/check-all.ts, package.json
New validation script scans SQL files under configured targets, enforces Drizzle-kit naming patterns (NNNN_word[_word...].sql), and rejects template-generated files. Registered in check-all.ts orchestrator and added to lint:custom script chain.

🎯 1 (Trivial) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a custom lint check that enforces drizzle-kit generated migrations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/add-custom-check-for-migration-scripts

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label May 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for API Unit Tests Coverage (./packages/api)

Status Category Percentage Covered / Total
🔵 Lines 69.62% 502 / 721
🔵 Statements 69.62% (🎯 65%) 502 / 721
🔵 Functions 92.68% 38 / 41
🔵 Branches 88.32% 227 / 257
File CoverageNo changed files found.
Generated in workflow #1270 for commit 0528473 by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for Expo Unit Tests Coverage (./apps/expo)

Status Category Percentage Covered / Total
🔵 Lines 82.61% 480 / 581
🔵 Statements 82.61% (🎯 75%) 480 / 581
🔵 Functions 92.59% 50 / 54
🔵 Branches 90.9% 170 / 187
File CoverageNo changed files found.
Generated in workflow #1270 for commit 0528473 by the Vitest Coverage Report Action

@andrew-bierman
Copy link
Copy Markdown
Collaborator Author

Nice guardrail. Two follow-ups worth considering before merge: (1) When the check fails, print the literal regeneration command (bun --cwd packages/api drizzle-kit generate) — saves a lookup. (2) Consider globbing packages/*/drizzle instead of hand-listing MIGRATION_TARGETS, so future packages (osm-db, the upcoming @packrat/db extraction in #2423) opt in by default. Also: please add a one-line comment next to the 0010_great_colleen_wing.sql allowlist entry explaining why it's exempt, so we remember in six months. None of this is blocking; happy to mark ready as is once these are addressed (or land it and follow up).

@andrew-bierman andrew-bierman marked this pull request as ready for review May 16, 2026 19:37
Copilot AI review requested due to automatic review settings May 16, 2026 19:37
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/lint/check-drizzle-migrations.ts`:
- Around line 53-56: Update the failure logging to include a per-package
regeneration command alongside the violation details: when iterating the
violations array (violations, violation.packageName, violation.message), print a
suggested shell command that uses violation.packageName to regenerate migrations
(for example: cd packages/${violation.packageName} && npm run
generate-migrations or npm run db:migrate:generate
--workspace=${violation.packageName}) so maintainers can copy-paste the recovery
step directly; ensure the command string is clearly labeled and uses the
packageName variable.
- Around line 8-11: MIGRATION_TARGETS is hard-coded so new packages with a
drizzle folder are missed; replace the static MIGRATION_TARGETS constant with
code that auto-discovers packages/*/drizzle (e.g., using fs.readdir or glob) and
builds the array of targets (each with name and allowManual Set), preserving
existing allowManual entries for known packages (0010_great_colleen_wing.sql and
0000_extensions.sql) and defaulting to an empty Set for others so the lint check
covers all current and future drizzle folders.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 865e0bbe-698a-403e-bade-6ccfe9137b3a

📥 Commits

Reviewing files that changed from the base of the PR and between 1917ea7 and 0528473.

📒 Files selected for processing (3)
  • package.json
  • scripts/check-all.ts
  • scripts/lint/check-drizzle-migrations.ts

Comment on lines +8 to +11
const MIGRATION_TARGETS = [
{ name: 'packages/api', allowManual: new Set(['0010_great_colleen_wing.sql']) },
{ name: 'packages/osm-db', allowManual: new Set(['0000_extensions.sql']) },
];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Auto-discover drizzle targets to avoid silent policy gaps.

Line 8 hard-codes MIGRATION_TARGETS, so new packages/*/drizzle folders can bypass this check until someone updates this file.

Proposed refactor
 import { existsSync, readdirSync, readFileSync } from 'node:fs';
 import { join } from 'node:path';

 const ROOT = join(import.meta.dir, '..', '..');

-const MIGRATION_TARGETS = [
-  { name: 'packages/api', allowManual: new Set(['0010_great_colleen_wing.sql']) },
-  { name: 'packages/osm-db', allowManual: new Set(['0000_extensions.sql']) },
-];
+const ALLOW_MANUAL_BY_PACKAGE = new Map<string, Set<string>>([
+  ['packages/api', new Set(['0010_great_colleen_wing.sql'])],
+  ['packages/osm-db', new Set(['0000_extensions.sql'])],
+]);
+
+const MIGRATION_TARGETS = readdirSync(join(ROOT, 'packages'), { withFileTypes: true })
+  .filter((d) => d.isDirectory())
+  .map((d) => `packages/${d.name}`)
+  .filter((name) => existsSync(join(ROOT, name, 'drizzle')))
+  .map((name) => ({
+    name,
+    allowManual: ALLOW_MANUAL_BY_PACKAGE.get(name) ?? new Set<string>(),
+  }));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/lint/check-drizzle-migrations.ts` around lines 8 - 11,
MIGRATION_TARGETS is hard-coded so new packages with a drizzle folder are
missed; replace the static MIGRATION_TARGETS constant with code that
auto-discovers packages/*/drizzle (e.g., using fs.readdir or glob) and builds
the array of targets (each with name and allowManual Set), preserving existing
allowManual entries for known packages (0010_great_colleen_wing.sql and
0000_extensions.sql) and defaulting to an empty Set for others so the lint check
covers all current and future drizzle folders.

Comment on lines +53 to +56
console.log(`Drizzle migration checks failed (${violations.length}):\n`);
for (const violation of violations) {
console.log(`${violation.packageName}: ${violation.message}`);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Print per-package regeneration commands in failures.

Current output reports the violation but not the exact recovery command, which slows remediation.

Proposed improvement
 if (violations.length > 0) {
   console.log(`Drizzle migration checks failed (${violations.length}):\n`);
   for (const violation of violations) {
     console.log(`${violation.packageName}: ${violation.message}`);
   }
+  console.log('\nSuggested fix commands:');
+  for (const pkg of new Set(violations.map((v) => v.packageName))) {
+    console.log(`  bun --cwd ${pkg} drizzle-kit generate`);
+  }
   process.exit(1);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(`Drizzle migration checks failed (${violations.length}):\n`);
for (const violation of violations) {
console.log(`${violation.packageName}: ${violation.message}`);
}
console.log(`Drizzle migration checks failed (${violations.length}):\n`);
for (const violation of violations) {
console.log(`${violation.packageName}: ${violation.message}`);
}
console.log('\nSuggested fix commands:');
for (const pkg of new Set(violations.map((v) => v.packageName))) {
console.log(` bun --cwd ${pkg} drizzle-kit generate`);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/lint/check-drizzle-migrations.ts` around lines 53 - 56, Update the
failure logging to include a per-package regeneration command alongside the
violation details: when iterating the violations array (violations,
violation.packageName, violation.message), print a suggested shell command that
uses violation.packageName to regenerate migrations (for example: cd
packages/${violation.packageName} && npm run generate-migrations or npm run
db:migrate:generate --workspace=${violation.packageName}) so maintainers can
copy-paste the recovery step directly; ensure the command string is clearly
labeled and uses the packageName variable.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new custom lint check to enforce that new SQL migrations in the Drizzle migration directories are generated via drizzle-kit (and not hand-written), while allowing a small set of known legacy/manual migrations to remain.

Changes:

  • Added scripts/lint/check-drizzle-migrations.ts to validate migration filenames and detect the Drizzle “custom SQL migration template” marker.
  • Wired the new check into the master custom check runner (scripts/check-all.ts).
  • Added the new check to the root lint:custom script in package.json.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
scripts/lint/check-drizzle-migrations.ts Implements migration enforcement (filename format + template marker detection) with legacy allowlist support.
scripts/check-all.ts Adds check-drizzle-migrations to the unified custom-check runner.
package.json Runs the new migration check as part of lint:custom.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@andrew-bierman andrew-bierman merged commit 1ead2e7 into development May 17, 2026
13 checks passed
@andrew-bierman andrew-bierman deleted the codex/add-custom-check-for-migration-scripts branch May 17, 2026 00:19
andrew-bierman added a commit that referenced this pull request May 17, 2026
After #2430 and #2431 merged to development, the lint:custom script picked up check-drizzle-migrations.ts. Combined both lint chains: this PR's no-owned-max-params + dev's check-drizzle-migrations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants