feat(install): skip legacy chain when modern bootstrap fingerprint detected (#530)

Refined from the original #530 framing after a dry-run uncovered that the
"bootstrap vs migration chain" diff produces mostly noise — most of the
~200 lines of difference are expected (migrations add new tables and
columns over time). initializeDatabase() isn't a parallel path that
diverges from migrations; it's invoked by migration 001 itself, so every
normal install/upgrade runs both.

The genuine drift hazard surfaced during the dry-run: a DB with the
modern bootstrap tables but an empty `migrations` table (which happens
when a backup was restored that lost the migrations table, or someone
invoked initializeDatabase() outside the runner, or the DB was moved
between systems without copying the migrations row) fails to upgrade.

Failure mode:
  1. detectExistingSchema sees the bootstrap tables + empty migrations,
     treats it as an "existing deployment".
  2. Runs the legacy chain first.
  3. legacy/008 renames email_templates.subject → subject_en.
  4. core/029 (later in the chain) inserts email templates referencing
     the pre-rename `subject` column.
  5. Postgres rejects: column "subject" doesn't exist; subject_en is
     NOT NULL with no default.

Fresh installs avoid this because they only run core/* (and core/059
handles the rename AFTER core/029 has inserted). Real legacy upgrades
avoid it because their migrations table already records legacy/008–028
as applied historically.

Fix in detectExistingSchema:
  - Detect the modern bootstrap fingerprint (photo_categories + cms_pages
    both present, which initializeDatabase produces as part of the
    consolidated post-004-era bootstrap).
  - When matched, enumerate every file in migrations/legacy/ and mark
    each as applied. This puts the recovery state on the same code path
    fresh installs use — only core migrations run, in core order.
  - Real legacy upgrades that already have entries in the migrations
    table hit no-op markings (markMigrationAsApplied skips duplicates),
    so their behaviour is unchanged.

New CI workflow (`.github/workflows/schema-drift.yml`):
  - Boots fresh postgres.
  - Seeds via `node -e \"require('./src/database/db').initializeDatabase()\"`
    — reproduces the recovery state in one line.
  - Runs `npm run migrate:safe`.
  - Asserts: precondition (bootstrap fingerprint + empty migrations
    table), migrate:safe exits 0, final schema has ≥40 tables (soft floor,
    not exact pin so future migrations don't force workflow edits),
    legacy migrations marked applied (confirms the fingerprint check
    actually fired vs. the chain silently bailing).
  - Triggers only on PRs that touch backend/migrations/**,
    src/database/db.js, knexfile.js, or this workflow.

Manually verified end-to-end before this commit:
  Before fix:  migrate:safe dies at core/029 with NOT NULL violation
               on email_templates.subject_en (17/48 tables present).
  After fix:   82 migrations applied + 27 marked applied = 109 total,
               final state has all 48 tables matching fresh-install.

Issue body in #530 has been updated to match this refined scope.

Refs: #530, #484, #519
This commit is contained in:
Paul Nothaft
2026-05-19 22:48:54 +02:00
parent bdd973eaf9
commit 8f0108ce23
2 changed files with 221 additions and 2 deletions
+43 -2
View File
@@ -38,7 +38,48 @@ async function markMigrationAsApplied(filename) {
// Detect existing schema and mark migrations as applied
async function detectExistingSchema() {
console.log('Detecting existing schema...');
// Modern-bootstrap fingerprint check (#530).
//
// A DB with the post-initializeDatabase state (photo_categories +
// cms_pages present, which db.js:initializeDatabase() creates as
// part of the consolidated modern bootstrap) but an empty migrations
// table is a recovery scenario — either restored from a backup that
// lost the migrations table, or someone invoked initializeDatabase()
// outside the migration runner.
//
// Treating this as a regular "existing deployment" runs the legacy
// chain first, which renames email_templates.subject → subject_en
// (legacy/008). Then core/029 fails when it tries to insert email
// templates referencing the pre-rename `subject` column. Fresh
// installs avoid this by running ONLY core migrations (core/059
// handles the rename later, after core/029 has inserted templates).
// Real legacy upgrades avoid it because their migrations table
// already records that legacy/008028 ran historically.
//
// The fix: when the modern bootstrap fingerprint is detected, mark
// every legacy migration as applied. This matches what fresh
// installs do (skip legacy entirely) and keeps the legacy chain
// from operating on a schema state it doesn't expect. Real legacy
// upgrades hit no-op markings here because they already have their
// migrations recorded.
const hasPhotoCategoriesTable = await db.schema.hasTable('photo_categories');
const hasCmsPagesTable = await db.schema.hasTable('cms_pages');
if (hasPhotoCategoriesTable && hasCmsPagesTable) {
const legacyDir = path.join(__dirname, 'legacy');
try {
const legacyFiles = await fs.readdir(legacyDir);
const legacyMigrations = legacyFiles.filter((f) => /^\d{3}_.*\.js$/.test(f));
for (const filename of legacyMigrations) {
await markMigrationAsApplied(filename);
}
} catch (err) {
// Non-fatal — only legacy dir absence (very-old test setups)
// would land here. Original table-based markers below still run.
console.log(`Could not enumerate legacy migrations: ${err.message}`);
}
}
const tableChecks = [
{ table: 'events', migration: '001_init.js' },
{ table: 'photos', migration: '001_init.js' },
@@ -49,7 +90,7 @@ async function detectExistingSchema() {
{ table: 'backup_runs', migration: '029_add_backup_service_tables.js' },
{ table: 'gallery_feedback', migration: '033_add_gallery_feedback.js' },
];
for (const check of tableChecks) {
const exists = await db.schema.hasTable(check.table);
if (exists) {