fix(server): move fork migrations to their own ledger - #48
Merged
Conversation
The six fork-only migrations were numbered 041-046 in upstream's sequence. The Effect Migrator is a high-water mark (runs ids above the largest applied id), so the next upstream migration at 041 would either be skipped as already applied or crash on missing tables at the following sync; upstream's pending orchestrator rewrite ships 041-049. Renumbering into a high range would be worse, parking the mark above every future upstream migration. Fork migrations now live in persistence/ForkMigrations/ with ids 1-6 and their own ledger table (effect_sql_migrations_fork), run by runAllMigrations after upstream's. On first boot the six legacy rows are moved out of upstream's ledger by id and name, so nothing re-runs and upstream's mark drops back to 040. Migrations.ts is upstream-identical again; migrate-dev-db mirrors boot. Verified against a snapshot of the real database: boot adopts the rows in place, no projection rebuild, cursor and thread count unchanged. Model: Claude Fable 5 via Claude Code.
github-actions
Bot
added
vouch:trusted
…lots Adoption inserted with OR IGNORE and deleted the legacy row unconditionally, so a fork ledger already holding that slot under another name would drop the only record that the migration ran. It now fails with a BadState MigrationError inside the transaction, leaving every legacy row in place. migrate-dev-db's slot-collision check now covers the fork ledger as well as upstream's, with the ledger named in the error. Findings from an advisory GPT-5.6 Sol review of the PR. Model: Claude Fable 5 via Claude Code.
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.
Problem
The six fork-only migrations were numbered 041-046 in upstream's sequence. The Effect Migrator is a high-water mark: it runs every loaded migration with an id above the largest id already in
effect_sql_migrations. So the next upstream migration at 041 is either skipped as already applied or crashes on tables that do not exist at the following sync (this already happened once). Upstream's pending orchestrator rewrite (pingdotgg/t3code#2829) ships 041-049. Renumbering the fork's into a high range would be worse: the mark would sit above every future upstream migration.Fix
apps/server/src/persistence/ForkMigrations/with ids 1-6 and their own ledger tableeffect_sql_migrations_fork, run through a second Migrator byrunAllMigrationsafter upstream's.Migrations.tsis byte-identical to upstream again.created_at. Nothing re-runs; upstream's high-water mark drops back to 040.Sqlite.tsboot andmigrate-dev-db(which mirrors boot) callrunAllMigrations. The migration tests move alongside and target the fork ledger; a newForkMigrations.test.tscovers fresh DBs, pre-split DBs (adoption without re-run, timestamps preserved, projections untouched), and an upstream row that reuses a legacy fork number.AGENTS.mdso future threads do not put fork migrations back intoMigrations.ts.Verification
vp test runonsrc/persistence/,ProjectionSnapshotQuery,TurnRetractionReactor,ProjectionPipeline,ProviderSessionDirectory,scripts/migrate-dev-db: 28 files, 110 tests green. Server typecheck and lint clean on the touched scope.VACUUM INTOsnapshot of the real database (legacy rows 41-46, cursor 47081, 78 threads): dev server boots in ~2s, upstream ledger ends at 40, fork ledger holds 1-6 with the original timestamps, projection cursor and thread count unchanged, no rebuild.Model: Claude Fable 5 via Claude Code.