feat(migrations): add rollback support with destructive-migration safeguards #848

[big6isaac]

Overview

The listener's migration runner required every migration to export a down() function, but exposed no way to invoke one. A schema change that turned out to be incompatible therefore had no way back out. This PR implements a real rollback path, guards destructive reverts so they cannot happen by accident, and documents the operational procedure.

Related Issue

Closes #848

Changes

Rollback engine

• [ADD] listener/src/database/migration-system.ts — MigrationRunner.rollback() unwinds applied migrations newest first, each in its own transaction. Every target is validated before any write, so a refusal partway through a multi-step rollback cannot leave the database half-reverted.
• [MODIFY] listener/src/database/migration-system.ts — reverts stamp a new migrations.reverted_at column instead of deleting the row, preserving the audit trail of what was applied and when it was reverted. The column is added defensively so databases that predate it keep working.
• [MODIFY] listener/src/database/migration-system.ts — re-applying a reverted migration now upserts rather than inserts, because id is the primary key and the revert is recorded in place.
• [ADD] getRollbackCandidates() reports what is currently in effect, newest first, with a reversible/destructive marker.

Destructive-migration safeguards

• [MODIFY] Migration interface — new optional destructive flag declaring that down() cannot reconstruct its data with up().
• [ADD] DestructiveMigrationError — rollback() refuses a destructive migration unless the caller passes allowDestructive, so a routine npm run migrate:rollback can never silently drop production rows.
• [MODIFY] 001-initial-schema.ts — flagged destructive: true; its down() drops all ten listener tables.
• [MODIFY] 002-query-performance-indexes.ts — flagged destructive: false; it only drops indexes, which up() recreates without touching rows.

CI gate

• [ADD] listener/src/scripts/check-migration-safety.ts — audits every migration and exits non-zero when one has no down(), or when a down() using DROP TABLE / DROP COLUMN / TRUNCATE is not flagged destructive. The second rule matters because an unflagged destructive migration is invisible to the runtime guard and would be reverted like any other.
• [ADD] .github/workflows/migration-safety.yml — runs the audit, a typecheck scoped to the migration surface, and the rollback tests.

Operator tooling and docs

• [ADD] listener/src/scripts/rollback-db.ts — CLI with --status, --steps N, and --allow-destructive. Exits 3 on a destructive refusal so the guard is scriptable.
• [MODIFY] listener/package.json — adds migrate:rollback and check-migration-safety.
• [ADD] MIGRATION_ROLLBACK.md — rollback procedures, safeguard behaviour, an authoring guide for new migrations, and an operational runbook.

Latent defects fixed along the way

Building the tests surfaced three bugs that made migrations unreliable, all of which had to be fixed for a rollback to be trustworthy:

• [FIX] listener/src/database/migration-system.ts — sqlite3 only resolves when given a callback, so await db.run(...) on a raw handle was a no-op. Migration DDL was fire-and-forget and could still be in flight when the surrounding transaction committed, so a migration could half-apply. Statements are now awaited through a promisified proxy.
• [FIX] listener/src/database/migration-system.ts — db.serialize() invokes its callback synchronously and does not await an async one, so the runner resolved before work finished and callers could close the database mid-transaction (SQLITE_MISUSE: Database handle is closed).
• [FIX] listener/src/migrations/001-initial-schema.ts — up() split its schema on ;, which cut CREATE TRIGGER ... BEGIN ... END bodies in half and left every trigger unparseable. The resulting SQLITE_ERROR: incomplete input was swallowed by the two defects above and only logged. The script is now passed to exec(), which understands statement boundaries.

Verification Results

$ npx jest src/database/migration-rollback.test.ts
Test Suites: 1 passed, 1 total
Tests:       13 passed, 13 total

$ npx ts-node src/scripts/check-migration-safety.ts
✅ All migrations define a rollback path and flag destructive reverts

$ npx tsc --noEmit --strict --esModuleInterop --skipLibCheck --module CommonJS \
    --target es2020 --types node,jest \
    src/database/migration-system.ts src/scripts/check-migration-safety.ts
clean (no output)

$ npx prettier --check <changed files> --config ../.prettierrc
All matched files use Prettier code style!

Audit fails closed, confirmed by temporarily unflagging migration 001:

$ npx ts-node src/scripts/check-migration-safety.ts
❌ Migration rollback safety issues found:
  001-initial-schema.ts: down() uses DROP TABLE but the migration is not flagged
  `destructive: true`, so rollback() would silently discard data
exit code: 1

End-to-end run against a scratch database, as an operator would experience it:

$ npm run migrate:rollback -- --status          # never-migrated database
No applied migrations to roll back

$ npm run migrate                               # apply
  002  query-performance-indexes  [reversible]
  001  initial-schema  [destructive]

$ npm run migrate:rollback                      # revert 1
Rolled back 1 migration(s):
  002  query-performance-indexes
   indexes left from 002: 0

$ npm run migrate:rollback                      # destructive, no flag
❌ Refusing to roll back destructive migration 001 (initial-schema).
   Re-run with allowDestructive to confirm the data loss is intended.
exit code: 3
   # fail-closed: 002's revert was not consumed, 001 still applied

$ npm run migrate:rollback -- --allow-destructive
Rolled back 1 migration(s):
  001  initial-schema  [destructive]
   scheduled_notifications exists? 0

No regressions: the full suite matches the pre-change baseline exactly, with 13 new tests passing.

                          pristine main    this branch
Test Suites:                 53 failed, 44 passed, 97    53 failed, 45 passed, 98
Tests:                       136 failed, 930 passed      136 failed, 943 passed

The 136 pre-existing failures are unrelated to this change (for example SyntaxError: Unexpected token '*' in request-id.ts) and are present on main. A repo-wide tsc --noEmit likewise reports the same 32 pre-existing errors on both, so the workflow's typecheck is scoped to the migration surface rather than gating on files it does not touch.

Acceptance Criteria	Status
Migration rollback steps are documented.	✅ MIGRATION_ROLLBACK.md covers CLI usage, revert semantics, an authoring guide for new migrations, and an operational runbook for failed deploys and bad migrations.
Destructive migrations receive appropriate safeguards.	✅ destructive flag plus runtime refusal unless allowDestructive is passed (CLI --allow-destructive, exit code 3). Validated fail-closed: a refusal leaves the database untouched. check-migration-safety additionally fails the build on any destructive down() missing the flag, so the safeguard cannot be forgotten.
Migration behavior is covered by CI where practical.	✅ .github/workflows/migration-safety.yml runs the safety audit, a scoped typecheck, and 13 rollback tests covering reverse-order unwinding, schema restoration on re-apply, destructive refusal, revert audit trail, and atomicity when a revert fails. Scope is limited to the migration paths to keep the gate fast.

Notes for reviewers

• Reverts are recorded, not erased. reverted_at is stamped in place because id is the primary key. This keeps an audit trail and means a reverted-then-reapplied migration revives its row instead of failing on a uniqueness collision. Worth a look, since the alternative (deleting the row) is simpler but loses history.
• The workflow typecheck is deliberately scoped. The listener has 32 pre-existing type errors in unrelated files, so a repo-wide tsc --noEmit would fail on a workflow that never touches them. I did not fix those, as they are outside this issue.
• The 001 trigger bug is pre-existing and unrelated to rollback. I fixed it because the rollback tests could not pass otherwise, and because a schema whose triggers never actually created is a serious latent problem in its own right. Happy to split it into its own PR if you would prefer to review it separately.
• package-lock.json is intentionally untouched. npm install regenerated it with 197 added and 164 removed transitive entries, which is out of scope here and a needless conflict surface.

[drips-wave]

@big6isaac Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits