ci: add migration drift check#219
Conversation
Adds a `migration-check` CI job that runs existing migrations against a fresh Postgres, then uses `typeorm migration:generate` to detect if any entity changes lack a corresponding migration. Prevents the exact class of bug that hit in Phase 14 (PR #186) where a missing CREATE TABLE migration was invisible locally due to synchronize:true. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: d092a7bc6505
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a Migration Drift Check job to CI that runs on pushes affecting Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions Runner
participant DB as Postgres 16-alpine (service)
participant NPM as npm / scripts
participant ORM as TypeORM CLI
participant Repo as Repository (migrations/entities)
GH->>DB: spin up Postgres service
GH->>NPM: install dependencies
GH->>NPM: run migration scripts (apply existing migrations)
NPM->>ORM: invoke TypeORM CLI (uses updated ts-node project)
ORM->>Repo: generate drift-check migration
ORM->>GH: return generated migration (diff)
GH->>GH: evaluate migration contents for structural changes
alt structural drift detected
GH->>GH: fail job and print migration contents
else only cosmetic/no drift
GH->>GH: report success
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 213-223: The drift-check currently silences all failures with "||
true" after running "pnpm typeorm migration:generate ...", which masks
datasource/config errors; change the step to capture the command's exit code and
its stdout/stderr (run "pnpm typeorm migration:generate
src/migrations/DriftCheck -d src/data-source.ts" without "|| true"), then
inspect the output and exit code: if the output contains the exact success
string "No changes in database schema were found" treat it as success, but if
the exit code is non-zero and the output does not include that success message,
print the full command output (and the generated files under
src/migrations/*DriftCheck*.ts if any) and exit non‑zero to fail the job so
datasource/config errors are surfaced.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml.planning/todos/done/2026-02-27-ci-migration-drift-check.md
… drift Two issues found during CI validation: 1. TypeORM CLI fails with TS1240 (decorator metadata) when ts-node starts with a .js entry file and lazily initializes the TypeScript service without the correct tsconfig. Fix: add --project tsconfig.json to the typeorm script in package.json. 2. migration:generate always reports "drift" because our hand-written migrations use explicit constraint/index names (e.g., FK_refresh_tokens_user) while TypeORM expects auto-generated hash names. Fix: only flag structural drift (CREATE/DROP TABLE, ADD/DROP COLUMN, type changes) and ignore cosmetic constraint/index renames. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 140578ed345c
The `|| true` suppressed all failures from `migration:generate`, masking real datasource/config errors (TypeORM exits 1 for both "no changes" and actual errors). Capture exit code and output separately, only treating the "No changes" message as success. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: f0b63e72d3e4
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 233-247: The early "exit 0" in the else branch after detecting "No
entity/migration drift detected." causes the subsequent STATUS/OUTPUT checks for
the `migration:generate` command to never run; remove or relocate that early
exit so the script always evaluates STATUS and OUTPUT after running
`migration:generate` (use the existing STATUS variable and the grep check on
OUTPUT to short-circuit only when the known "No changes in database schema were
found" message appears, otherwise let the code reach the block that logs
"::error::migration:generate failed unexpectedly." and exits 1). Ensure you only
short-circuit to success when the grep condition matches, and otherwise allow
the STATUS-based failure handling to execute.
Move STATUS check before the drift file check. Previously, if migration:generate failed without producing a file, the else branch exited 0 and the STATUS check was unreachable — masking real errors like DB connection failures or TypeORM bugs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: a3a82c38a9d8
Summary
migration-checkCI job that detects entity/migration drift by runningtypeorm migration:generateagainst a freshly-migrated Postgres databasebuildjob (parallel withapi-specandtest)Prevents the class of bug from Phase 14 (PR #186) where a missing
CREATE TABLEmigration was invisible locally due tosynchronize:true.Test plan
migration-checkjob appears in CI and passes on current codebase (no drift)migration:generatedetects the drift🤖 Generated with Claude Code
Summary by CodeRabbit