fix(core): restore content bylines table left staged by an interrupted rebuild - #2612
Merged
ascorbic merged 1 commit intoAug 25, 2026
Merged
Conversation
…d rebuild Migration 040 rebuilds `_emdash_content_bylines` on SQLite by staging a copy, dropping the old table and renaming the copy. When the run stops between the drop and the rename, a retry skips the rebuild: its guard reads `PRAGMA foreign_key_list` on a table that no longer exists and treats the empty result as "already rebuilt". The migration is then recorded as applied with only `_emdash_content_bylines_new` present, every byline subquery fails, and the site reports no entries although the rows are intact. Add migration 071, which renames the staged copy back when the live table is missing and re-creates the two indexes the drop removed. It is restart-safe, leaves a healthy table untouched, and is a no-op on Postgres, whose 040 path alters the table in place.
🦋 Changeset detectedLatest commit: a19f8b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: fix(core): restore content bylines table left staged by an interrupted rebuild
Approach
This is the right fix for the right problem. Migration 040’s SQLite rebuild can fail between dropping _emdash_content_bylines and renaming _emdash_content_bylines_new; on retry, its PRAGMA foreign_key_list(_emdash_content_bylines) guard returns no rows for the missing table, so the runner records 040 as applied and leaves the credits orphaned. Since published migrations are immutable per AGENTS.md, adding a focused, idempotent corrective migration is the correct pattern, and leaving 040 untouched is the right call.
The migration is SQLite-only (Postgres alters in place), checks for both the live and staged tables, renames only when safe, and re-creates the dropped indexes with ifNotExists(). The no-op down() is acceptable because the post-071 state (live table properly named with indexes) is the canonical state.
What I checked
- Read the diff and the full changed files.
- Traced 040’s SQLite rebuild path to confirm the stated failure mode:
rebuildContentBylinesreturns early when the live table is missing andPRAGMA foreign_key_listreturns no rows, so the staged copy is never renamed. - Verified migrations 041–070 do not reference
_emdash_content_bylines, so a failed 040 does not cause later migrations to fail before 071 can repair the table. - Confirmed 071’s index names and column lists match those created by 040 (
idx_content_bylines_contentoncollection_slug, content_id, sort_orderandidx_content_bylines_bylineonbyline_id). - Reviewed
runner.tsregistration and the migrations test’strailinglist; both correctly include 071. - Reviewed the new integration tests. They cover the healthy DB, the partial-run repair, re-run idempotency, runner retry from 040, and the stale-staged-copy-left-alone case. They exercise observable behavior (table/index existence and hydrated credits), not implementation trivia.
- Checked
dialect-helpers.tsforisSqlite/tableExistsbehavior; D1 inherits SQLite detection. - Reviewed the changeset against
.changeset/README.md: it leads with “Fixes,” identifies affected users (SQLite/D1), names the observable problem (empty pages/feeds/admin), and states the outcome (next migration run restores the table and credits). - Checked for AGENTS.md convention violations: no unsafe SQL interpolation, no new user-facing strings, no route changes, no comment referencing issues/PRs. The migration’s module comment refers to migration 040 by number, which is appropriate for migration context.
Conclusion
The change is minimal, well-scoped, and the tests are capable of catching a real regression. I found no bugs, regressions, or convention violations. LGTM.
github-actions
Bot
added
review/approved
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
A SQLite or D1 site can report no entries while every row is intact. Public pages render the empty state,
/rss.xmllists nothing, and the admin reports the collection as not found. In that state the byline credits table exists only as_emdash_content_bylines_new, left behind by an interrupted run of migration 040, and_emdash_content_bylinesis missing. Every loader query (loadEntry,loadCollection) then fails on its bylines subquery. Running the migrations again does not repair it.The cause: 040 rebuilds the table on SQLite in four steps. It creates a staged copy, copies the rows, drops the old table, and renames the copy. If the run stops between the drop and the rename, the retry reads
PRAGMA foreign_key_liston a table that no longer exists. SQLite returns no rows for a missing table, so the retry treats the table as already rebuilt and skips the rename. The runner records 040 as applied and the later migrations run. The staged copy stays behind.Migration 071 renames
_emdash_content_bylines_newback to_emdash_content_bylineswhen the live table is missing and the staged copy exists. It then re-creates the two indexes that the drop removed. On a healthy database it changes nothing. On Postgres it changes nothing either, because the 040 path there alters the table in place. It can restart after any of its statements. 040 itself stays unchanged because published migrations are immutable. Covered by integration tests in both dialects.Closes #2241
Details
Numbering: #2584 also adds a migration numbered 071. Whichever of the two merges second needs a renumber. Nothing else in this change depends on the number.
Stale copy: if both tables exist, 071 leaves
_emdash_content_bylines_newalone. The live table is authoritative in that state, and the migration deletes nothing.Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain. (n/a: no admin strings)AI-generated code disclosure
Screenshots / test output