fix(db): refuse to roll back a migration that defines no down section
Quality / quality (ubuntu-latest) (push) Failing after 6m4s
Quality / quality (windows-latest) (push) Canceled after 0s

rollback ran a migration's down section when it had one and then deleted
its tracking row either way. A migration that changed the schema without a
`-- +down` section was therefore recorded as rolled back while its change
stayed in the database, and the next migrate replayed statements such as
ALTER TABLE ADD COLUMN that cannot run twice, leaving the database stuck
between the two states with no way forward or back.

It now refuses that case with WRN-DB-MIGRATION-IRREVERSIBLE and leaves the
tracking row alone, so the database stays consistent and the operator
chooses: add a down section, or write a new migration that reverses the
change. The check runs before the lock is taken, so a dry run reports the
same refusal rather than promising a rollback that would fail.

A migration whose up is empty still rolls back by dropping its tracking
row, since there is nothing to undo. That keeps placeholder migrations
working, and `wrnexus db rollback` prints the refusal as a message rather
than a stack trace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
ClintchizandClaude Opus 5 committed 2026-09-03 09:57:43 +05:30
1 parent d3f26ad16e
commit b807f7b5a7
3 files changed
+95 -3

No files matched your search

+61
View File
@@ -145,6 +145,67 @@ test("an empty migration set does not touch the database", async () => {
expect(await applyMigrations(db, [])).toEqual([]);
});
test("rollback refuses a schema change that defines no down section", async () => {
const dir = mkdtempSync(join(tmpdir(), "wrn-mig-nodown-"));
writeFileSync(join(dir, "0001_add.sql"), "-- +up\nCREATE TABLE t (id INTEGER PRIMARY KEY);");
const db = createDb(sqlite());
expect(await migrate(db, dir)).toEqual(["0001_add"]);
// Rolling this back would drop the record and leave the table, and the next
// migrate would replay a CREATE that cannot run twice.
await expect(rollback(db, dir)).rejects.toThrow(/WRN-DB-MIGRATION-IRREVERSIBLE/);
// The refusal leaves the record alone, so the database stays consistent.
expect(await appliedMigrations(db)).toEqual(["0001_add"]);
expect((await status(db, dir))[0]!.applied).toBe(true);
db.close();
});
test("a dry run reports the same refusal instead of promising a rollback", async () => {
const dir = mkdtempSync(join(tmpdir(), "wrn-mig-dry-"));
writeFileSync(join(dir, "0001_add.sql"), "-- +up\nCREATE TABLE t (id INTEGER PRIMARY KEY);");
const db = createDb(sqlite());
await migrate(db, dir);
await expect(rollback(db, dir, { dryRun: true })).rejects.toThrow(
/WRN-DB-MIGRATION-IRREVERSIBLE/,
);
db.close();
});
test("rollback still undoes a migration that defines a down section", async () => {
const dir = mkdtempSync(join(tmpdir(), "wrn-mig-down-"));
writeFileSync(
join(dir, "0001_add.sql"),
"-- +up\nCREATE TABLE t (id INTEGER PRIMARY KEY);\n-- +down\nDROP TABLE t;",
);
const db = createDb(sqlite());
await migrate(db, dir);
expect(await rollback(db, dir)).toBe("0001_add");
expect(await appliedMigrations(db)).toEqual([]);
const remaining = await db.all<{ name: string }>(
"SELECT name FROM sqlite_master WHERE type='table' AND name='t'",
);
expect(remaining.length).toBe(0);
db.close();
});
test("a migration whose up is only comments has nothing to undo and rolls back", async () => {
const dir = mkdtempSync(join(tmpdir(), "wrn-mig-noop-"));
writeFileSync(
join(dir, "0001_placeholder.sql"),
"-- +up\n-- Nothing yet.\n-- +down\n-- Nothing to undo.",
);
const db = createDb(sqlite());
await migrate(db, dir);
expect(await rollback(db, dir)).toBe("0001_placeholder");
expect(await appliedMigrations(db)).toEqual([]);
db.close();
});
test("comment-only migrations are recorded without executing empty SQL", async () => {
const db = createDb(sqlite());
const migrations = [