Add extra deployment phase for destructive migrations#5509
Open
backspace wants to merge 10 commits into
Open
Conversation
Migrations run before the app rolls out, so a destructive change to
boxel_index (a column/table drop or rename) takes effect while the
previous realm-server revision is still serving. During the rolling
deploy window that old code queries the now-missing column and every
published-realm serve 500s until the old tasks drain.
Split migrations into two phases:
- migrations/ additive, backward-compatible; runs pre-deploy
(migrate-db), as before
- migrations-removal/ destructive; runs post-deploy (migrate-db-remove),
gated on deploy-realm-server reaching stability so
the previous realm-server and worker task sets have
fully drained before any drop executes
A failure in the removal phase is non-fatal to serving: the new code no
longer reads the removed columns, so it is already live and healthy.
Each phase has its own directory and node-pg-migrate tracking table. The
gated one-shot-ECS-task orchestration is extracted to
.github/scripts/run-gated-migration-task.sh so both jobs stay in lockstep.
run-migrations.sh takes an optional (dir, table); the local/CI driver
migrate-local.sh runs both phases together (removal last on up, first on
down). Schema-filename derivation and validation, the migration linter, the
changed-migration detector, and the schema dump's excluded bookkeeping
tables all now account for both directories.
The already-applied drop-boxel-index-html-columns migration moves into the
removal phase; its up is idempotent, so re-running under the new tracking
table is a no-op where the columns are already gone.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Preview deploymentsHost Test Results 1 files 1 suites 3h 3m 34s ⏱️ Results for commit d064426. For more details on these errors, see this check. Realm Server Test Results 1 files ± 0 1 suites +1 17m 54s ⏱️ + 17m 54s Results for commit d064426. ± Comparison against earlier commit c39a2e5. |
backspace
marked this pull request as ready for review
July 16, 2026 18:16
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f7ec3bcf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The migrate-local.sh dispatch ran every non-`down` action against both the additive (migrations/) and removal (migrations-removal/) phases. For `create` that scaffolded two files per invocation — the intended migrations/ file plus a spurious migrations-removal/ one — so the documented `pnpm migrate create <name>` no longer had a single unambiguous output. Handle `create` explicitly: scaffold one file in migrations/. To author a removal migration, create it this way and `git mv` it into migrations-removal/ (tracked by filename, so moving a not-yet-applied file is clean). Apply actions (up, redo) still run against both phases; `down` still rolls the removal phase back first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `down` branch always rolled the entire removal phase back, then applied the caller's count to the additive phase — so a bare `pnpm migrate down` reverted two migrations (all removal + one additive) instead of the single latest, and it ignored the combined ordering (removal migrations aren't necessarily the most recent once an additive migration is added and applied after them). During local recovery that can silently revert unrelated destructive changes. Revert exactly the requested count (default 1) across the combined newest-first timeline of both phases, using the tracking tables as the source of truth (--no-check-order lets migrations apply out of filename order). Consecutive same-phase entries coalesce into one `down K`, so a full-chain rollback stays ~2 calls rather than one node-pg-migrate invocation per migration. `determine-changed-migrations.sh` now counts both phases so CI's full-chain down_count reverts the entire combined chain. Verified locally: full-chain up/down/up (112+1 → 0 → 112+1, 2 down calls), bare down reverts only the newest, and an interleave case (fresh additive applied after the removal) reverts the additive and leaves the removal intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…plit Make the two-phase migration system discoverable and hard to misuse: - CI guard (scripts/check-removal-phase.cjs, "Guard removal-phase migrations" step): AST-parses newly added migrations/ files and fails if an up() drops or renames a column/table, pointing to migrations-removal/. Scoped to changed files (historical drops grandfathered) and to up() only, so an additive migration's down() reversing itself isn't flagged. Uses the TypeScript parser (added as a devDependency) rather than a regex so it can distinguish up() from down(). Heuristic — covers the rolling-deploy outage class (drop/rename), not NOT-NULL tightening or type narrowing. - `pnpm migrate:create-removal <name>` scaffolds a destructive migration straight into migrations-removal/ (via a create-removal case in migrate-local.sh), so authoring one no longer needs a manual git mv. - packages/postgres/README.md documents which directory a migration belongs in, how to create each kind, and the twin-table / already-applied-move gotchas. - .claude/skills/postgres-migrations surfaces the same guidance to agents working on migrations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Describe the failure mode timelessly (a destructive change breaks the previous code revision during a rolling deploy) rather than citing specific past dates or counts, which don't help a future reader and rot. Affects the check-removal-phase guard comment, the postgres README, and the postgres-migrations skill. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Intentionally adds a DROP-in-up() migration to migrations/ (should be in migrations-removal/) so the "Guard removal-phase migrations" CI check fails. Remove this commit/file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
determine-changed-migrations.sh emits a newline-separated file list. Inlining it into the guard step's run script made each newline a command separator, so the guard only received the first file and the rest ran as bogus shell commands. Feed the list through the CHANGED_MIGRATIONS env var and have check-removal-phase.cjs read and split it, so every changed migration is actually checked without relying on shell word-splitting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…iling" This reverts commit ad7c11d.
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
1 participant
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.
We’ve had some production downtime on published sites because of destructive migrations having run while not-yet-updated components are expecting the old schema. This adds a
migrations-removaldirectory for such migrations, which runs after the realm server has reached stability, when no components should still be relying on the old schema.This includes a new skill and a CI job that tries to prevent destructive migrations being added to
migrationsdirectory, here it is failing on an example: