Skip to content

fix: make migrations 034 and 035 idempotent for partial reapply#803

Merged
ascorbic merged 1 commit intoemdash-cms:mainfrom
mvanhorn:osc/788-idempotent-migrations
Apr 28, 2026
Merged

fix: make migrations 034 and 035 idempotent for partial reapply#803
ascorbic merged 1 commit intoemdash-cms:mainfrom
mvanhorn:osc/788-idempotent-migrations

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #788

Migrations 034 and 035 weren't safe to rerun against a partially-applied schema. If a previous run's schema commit succeeded but the row write to _emdash_migrations failed (D1 subrequest limit, transient error mid-batch, Worker CPU limit), the next migration run reapplies the schema and crashes with index already exists: SQLITE_ERROR (migration: 034_published_at_index) — which is the exact failure the reporter hit upgrading from 0.1.1 to 0.6.0/0.7.0.

This makes both migrations safe to re-run on a partially-applied schema while preserving accumulated _emdash_404_log data.

  • 034_published_at_index.ts: CREATE INDEXCREATE INDEX IF NOT EXISTS so re-creating an existing per-table index is a no-op.
  • 035_bounded_404_log.ts: capture hitsExists once before the column adds. Guard addColumn("hits") and addColumn("last_seen_at") with columnExists checks. On rerun (when hits already exists), skip the dedup rollup UPDATE/DELETE block so live counters and last_seen_at aren't reset back to COUNT(*) = 1 / created_at. Add .ifNotExists() to the new index creates and .ifExists() to the path-index drop (and the symmetric calls in down()).
  • dialect-helpers.ts: add columnExists and indexExists helpers. Both branch on isPostgres and follow the same pattern as the existing tableExists. Use pragma_table_info(table) on SQLite, information_schema.columns / pg_indexes on Postgres.
  • Added integration test migrations.test.ts that runs all migrations, deletes the rows for 034 and 035 from _emdash_migrations, and re-runs runMigrations against the partially-applied schema. Both names appear in applied, no error is thrown, and the migration table count returns to MIGRATION_COUNT.
  • Patch-level changeset.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes (no new diagnostics)
  • pnpm test passes (packages/core/tests/integration/database/migrations.test.ts 15/15)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (n/a)
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion: n/a (bug fix)

AI-generated code disclosure

  • This PR includes AI-generated code

Screenshots / test output

Test Files  1 passed (1)
     Tests  15 passed (15)
  Duration  341ms

The new test exercises the partial-reapply path:

it("should re-run migrations 034 and 035 when schema changes were partially applied", async () => {
    await db.destroy();
    db = await setupTestDatabaseWithCollections();

    await db
        .deleteFrom("_emdash_migrations")
        .where("name", "in", ["034_published_at_index", "035_bounded_404_log"])
        .execute();

    const { applied } = await runMigrations(db);

    expect(applied).toContain("034_published_at_index");
    expect(applied).toContain("035_bounded_404_log");

    const migrations = await db.selectFrom("_emdash_migrations").selectAll().execute();
    expect(migrations).toHaveLength(MIGRATION_COUNT);
});

Out of scope

The middleware behavior the reporter also flagged (auth middleware swallowing the migration error and falling through to a permissionless ghost user) is a separate failure mode in auth.ts and worth its own issue. Per CLAUDE.md's "no drive-by changes" rule, that's not addressed here.

…sh-cms#788)

When an earlier migration attempt commits its schema changes but the row
write to _emdash_migrations fails (e.g. D1 subrequest limit, transient
error mid-batch), the next migration run reapplies the schema and crashes
with "index already exists" or "duplicate column".

Make 034 and 035 safe to re-run on a partially-applied schema:

- 034: CREATE INDEX -> CREATE INDEX IF NOT EXISTS
- 035: guard addColumn with columnExists; ifNotExists on createIndex;
  ifExists on dropIndex (both up and down). Capture hitsExists before
  schema changes; skip the dedup rollup/delete on rerun so existing 404
  counters and last_seen_at values are preserved.
- Add columnExists and indexExists helpers in dialect-helpers.ts.
- Add an integration test for the idempotent re-run case.
- Add a changeset (patch).
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 28, 2026

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@803

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@803

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@803

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@803

emdash

npm i https://pkg.pr.new/emdash@803

create-emdash

npm i https://pkg.pr.new/create-emdash@803

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@803

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@803

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@803

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@803

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@803

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@803

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@803

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@803

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@803

commit: 1f6e22c

@ascorbic
Copy link
Copy Markdown
Collaborator

/review

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 28, 2026

LGTM!

github run

Copy link
Copy Markdown
Collaborator

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ascorbic ascorbic merged commit 0041d76 into emdash-cms:main Apr 28, 2026
29 checks passed
@emdashbot emdashbot Bot mentioned this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration runner crashes on partially-applied schema and the middleware silently degrades the admin to a permissionless "ghost" user

2 participants