From 8f0108ce233f457d6a0f4f3dbc3e1b0a7217e74e Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 19 May 2026 22:48:54 +0200 Subject: [PATCH 1/2] feat(install): skip legacy chain when modern bootstrap fingerprint detected (#530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/schema-drift.yml | 178 ++++++++++++++++++++++ backend/migrations/run-migrations-safe.js | 45 +++++- 2 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/schema-drift.yml diff --git a/.github/workflows/schema-drift.yml b/.github/workflows/schema-drift.yml new file mode 100644 index 00000000..367ee25a --- /dev/null +++ b/.github/workflows/schema-drift.yml @@ -0,0 +1,178 @@ +name: Schema drift (#530) + +# Verifies that `migrate:safe` can recover a DB that's been seeded only +# by `initializeDatabase()` — the recovery scenario where the migrations +# tracking table is empty but the schema already has the modern bootstrap. +# +# This is NOT how production reaches its state on normal installs or +# upgrades. The scenario only fires when: +# - A backup was restored that captured tables but not the migrations +# table (manifest divergence), +# - Someone manually invoked initializeDatabase() outside the migration +# runner (recovery / debugging), +# - The DB was moved between systems and the migrations table was not +# copied along. +# +# When `detectExistingSchema()` sees the modern-bootstrap fingerprint +# (photo_categories + cms_pages tables) but an empty migrations table, +# it treats it as an "existing deployment" — which runs the legacy +# chain first. Legacy/008 renames email_templates.subject → subject_en, +# but core/029 (which runs later in this chain) inserts email templates +# referencing the pre-rename column name. The chain dies with a +# "column subject does not exist" error. +# +# Fix (in the same PR as this workflow): when the modern-bootstrap +# fingerprint is detected, mark all legacy migrations as applied so the +# chain matches what a fresh install runs — only core/*, in order. +# +# This workflow boots the failing scenario from scratch on every PR +# that touches the migrations or db.js, so any future migration with +# the same shape is caught before merge. + +on: + push: + branches: [main, beta] + paths: + - 'backend/migrations/**' + - 'backend/src/database/db.js' + - 'backend/knexfile.js' + - '.github/workflows/schema-drift.yml' + pull_request: + branches: [main, beta] + paths: + - 'backend/migrations/**' + - 'backend/src/database/db.js' + - 'backend/knexfile.js' + - '.github/workflows/schema-drift.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + upgrade-from-bootstrap: + runs-on: ubuntu-latest + timeout-minutes: 10 + + services: + postgres: + image: postgres:15-alpine + env: + POSTGRES_USER: picpeak + POSTGRES_PASSWORD: testpass + POSTGRES_DB: picpeak_drift + options: >- + --health-cmd "pg_isready -U picpeak -d picpeak_drift" + --health-interval 2s + --health-timeout 2s + --health-retries 30 + ports: + - 5432:5432 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + cache-dependency-path: backend/package-lock.json + + - name: Install backend deps + working-directory: ./backend + run: npm ci + + # Step 1: simulate the recovery state — DB has the modern bootstrap + # (post-initializeDatabase) but no migrations recorded. Calling + # initializeDatabase() directly outside the migration runner is the + # one-line repro for backup-restore-lost-migrations and manual- + # invocation paths. + - name: Seed DB with initializeDatabase() only + working-directory: ./backend + env: + NODE_ENV: production + DATABASE_CLIENT: pg + DB_HOST: localhost + DB_PORT: 5432 + DB_USER: picpeak + DB_PASSWORD: testpass + DB_NAME: picpeak_drift + run: | + node -e "require('./src/database/db').initializeDatabase().then(() => { console.log('bootstrap ok'); process.exit(0); }).catch(e => { console.error('bootstrap FAILED:', e.message); process.exit(1); })" + + # Sanity-check the recovery shape before migrate:safe runs. If + # initializeDatabase() ever stops producing photo_categories + + # cms_pages, the fingerprint check would silently no-op and this + # workflow would lose its teeth — assert the precondition. + - name: Assert recovery-state fingerprint + env: + PGPASSWORD: testpass + run: | + installed=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT count(*) FROM pg_tables WHERE schemaname='public' AND tablename IN ('photo_categories', 'cms_pages')") + if [ "$installed" != "2" ]; then + echo "FAIL: expected photo_categories + cms_pages from initializeDatabase(); got $installed." + psql -h localhost -U picpeak -d picpeak_drift -c "SELECT tablename FROM pg_tables WHERE schemaname='public' ORDER BY tablename" + exit 1 + fi + migrations_count=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT CASE WHEN to_regclass('public.migrations') IS NULL THEN 0 ELSE (SELECT count(*) FROM migrations) END") + if [ "$migrations_count" != "0" ]; then + echo "FAIL: migrations table should be empty for the recovery scenario; has $migrations_count rows." + exit 1 + fi + echo "ok: recovery state confirmed (bootstrap tables present, migrations table empty)." + + # Step 2: run migrate:safe — the test. Before #530's fix in + # detectExistingSchema, this died at core/029 with a "column + # subject does not exist" error. After the fix, it should complete + # cleanly with every migration either applied or marked. + - name: Run migrate:safe against the recovery state + working-directory: ./backend + env: + NODE_ENV: production + DATABASE_CLIENT: pg + DB_HOST: localhost + DB_PORT: 5432 + DB_USER: picpeak + DB_PASSWORD: testpass + DB_NAME: picpeak_drift + run: npm run migrate:safe + + # Step 3: schema-shape assertion. A fresh install through migrate: + # safe produces 48 tables; the recovery scenario should converge + # to the same number. Off-by-one is fine but a 10+ table delta + # means a migration silently bailed in the recovery path. + - name: Assert final schema matches fresh-install shape + env: + PGPASSWORD: testpass + run: | + tables=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT count(*) FROM pg_tables WHERE schemaname='public'") + echo "Final table count: $tables" + # Allow a small drift window — exact count creeps over time as + # new migrations land; tight pin would force a workflow edit + # on every schema PR. 40+ is a healthy floor that catches the + # original bug (which left 17 tables) while staying robust to + # forward changes. + if [ "$tables" -lt 40 ]; then + echo "FAIL: too few tables ($tables) — migrate:safe likely bailed mid-chain." + psql -h localhost -U picpeak -d picpeak_drift -c "SELECT tablename FROM pg_tables WHERE schemaname='public' ORDER BY tablename" + exit 1 + fi + echo "ok: schema converged to a fresh-install-equivalent shape." + + # Step 4: verify the legacy migrations were all marked applied + # (rather than silently bailing inside the chain). The fix in + # detectExistingSchema marks legacy/* when the modern bootstrap + # is detected — confirm the markings actually landed. + - name: Assert legacy migrations marked applied + env: + PGPASSWORD: testpass + run: | + legacy_count=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT count(*) FROM migrations WHERE filename LIKE '008_%' OR filename LIKE '009_%' OR filename LIKE '013_%' OR filename LIKE '019_%' OR filename LIKE '020_%' OR filename LIKE '026_%' OR filename LIKE '028_%'") + if [ "$legacy_count" -lt 7 ]; then + echo "FAIL: legacy migrations not marked applied ($legacy_count of 7 expected)." + psql -h localhost -U picpeak -d picpeak_drift -c "SELECT filename FROM migrations WHERE filename LIKE '0%' ORDER BY filename" + exit 1 + fi + echo "ok: legacy migrations marked applied by detectExistingSchema." diff --git a/backend/migrations/run-migrations-safe.js b/backend/migrations/run-migrations-safe.js index dd7b1491..eeeca335 100644 --- a/backend/migrations/run-migrations-safe.js +++ b/backend/migrations/run-migrations-safe.js @@ -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/008–028 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) { From 4d3f2470bc7a9e67da549da663802ee2844d088a Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 19 May 2026 22:53:30 +0200 Subject: [PATCH 2/2] ci(schema-drift): handle absent migrations table in precondition (#530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First CI run failed at the precondition check because the SQL `CASE WHEN to_regclass(...) IS NULL THEN 0 ELSE (SELECT count(*) FROM migrations)` expression doesn't short-circuit at parse time — Postgres parses the subquery against `migrations` even when the outer guard would skip it, fails the run with "relation 'migrations' does not exist". initializeDatabase() doesn't create the `migrations` tracking table — that's the migrate:safe runner's responsibility — so in the recovery scenario the table genuinely doesn't exist yet. Both "absent table" and "present but empty table" are valid recovery states. Split the check into two shell steps: to_regclass first, then count only if the table exists. Avoids the parse-time subquery error and accepts either state. --- .github/workflows/schema-drift.yml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/workflows/schema-drift.yml b/.github/workflows/schema-drift.yml index 367ee25a..8811d77b 100644 --- a/.github/workflows/schema-drift.yml +++ b/.github/workflows/schema-drift.yml @@ -116,12 +116,23 @@ jobs: psql -h localhost -U picpeak -d picpeak_drift -c "SELECT tablename FROM pg_tables WHERE schemaname='public' ORDER BY tablename" exit 1 fi - migrations_count=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT CASE WHEN to_regclass('public.migrations') IS NULL THEN 0 ELSE (SELECT count(*) FROM migrations) END") + # initializeDatabase() doesn't create the `migrations` tracking + # table — that's the migrate:safe runner's job. So in the recovery + # scenario, the table either (a) doesn't exist yet or (b) exists + # but is empty (e.g. someone created it but didn't populate it). + # Both are valid recovery states; check via to_regclass first so + # we don't parse a SELECT against a nonexistent table. + has_migrations_table=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT to_regclass('public.migrations')::text") + if [ -z "$has_migrations_table" ]; then + migrations_count=0 + else + migrations_count=$(psql -h localhost -U picpeak -d picpeak_drift -tAc "SELECT count(*) FROM migrations") + fi if [ "$migrations_count" != "0" ]; then echo "FAIL: migrations table should be empty for the recovery scenario; has $migrations_count rows." exit 1 fi - echo "ok: recovery state confirmed (bootstrap tables present, migrations table empty)." + echo "ok: recovery state confirmed (bootstrap tables present, migrations table empty or absent)." # Step 2: run migrate:safe — the test. Before #530's fix in # detectExistingSchema, this died at core/029 with a "column