fix(cli): make migrate --verify a real integrity gate, --plan a pure dry-run - #56
Merged
Merged
Conversation
…dry-run --verify and --plan previously ran identical code and differed only in the final printed line. --verify now collects integrity findings between the tracking table and local files — hash drift on applied migrations, applied rows missing locally, duplicate tracking rows, and interrupted (dirty) migrations — and exits non-zero on any, while --plan (new visible alias --dry-run) reports the same findings as warnings without failing. --safe runs the verify gate before applying, matching its documentation. The restructure also fixes the plan path counting interrupted rows as applied (they now show as pending with a finding), collapses the duplicate verify_migrations/plan_migrations pair, and replaces six hand-rolled record queries with a dialect-aware Migrations::applied_records_sql(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
migrate --verifyandmigrate --planpreviously ran identical code and differed only in the final printed line. This differentiates them properly:--verifynow checks the tracking table against local migration files — hash drift on applied migrations, applied rows with no local migration, duplicate tracking rows, and interrupted (dirty) migrations — reporting all findings together and exiting non-zero on any.--planstays a pure dry-run and gains a visible--dry-runalias; it shows the same findings as warnings without failing.--safenow actually runs the verify gate before applying, matching its long-standing docstring.Bonus fixes surfaced by the restructure: the plan path was counting interrupted (dirty) rows as applied; the duplicate
verify_migrations/plan_migrationsdb functions collapsed to one; six hand-rolled record queries replaced with a dialect-awareMigrations::applied_records_sql()(hash, name, dirty) shared by all drivers including D1.Test plan
cargo test -p drizzle-cli --all-features— includes updated drift test (verify still fails end-to-end) and a new--dry-runwarns-but-succeeds counterpartcargo test -p drizzle-migrations— 302 lib tests incl. newapplied_records_sqltest--all-features --all-targets -D warnings+ nightly fmt (pre-commit hook)🤖 Generated with Claude Code