authorgravatar for 91240327+shivamhwp@users.noreply.github.comshivam <91240327+shivamhwp@users.noreply.github.com> 2026-10-13 08:21:55+05:30
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-13 08:21:55+05:30
loge24ea1d28ef9159cc6cc98a1e9d9de5f52ed385c
treeca5ae030745fbd9c9df920ead60bdb41c8d732ed
parent1af138668b8bcf5c289382dde1ccaecdd79614b8
signature Signed by PGP key B5690EEEBB952194

fix(server): a migration id clash stops startup with a clear error instead of crash-looping (#15925)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: PR Batch Tester <agent@local.test>

3 files changed, 187 insertions(+), 8 deletions(-)

apps/server/src/persistence/Migrations.test.ts created+133
......@@ -0,0 +1,133 @@
1import { assert, describe, it } from "@effect/vitest";
2import * as NodeSqliteClient from "@t3tools/shared/nodeSqliteClient";
3import * as Effect from "effect/Effect";
4import * as Exit from "effect/Exit";
5import * as SqlClient from "effect/sql/SqlClient";
6
7import { migrationManifest, runMigrations } from "./Migrations.ts";
8
9const v2Migrations = migrationManifest.filter(([id]) => id >= 55);
10const newerMigrationId = Math.max(...migrationManifest.map(([id]) => id)) + 1;
11
12const history = Effect.gen(function* () {
13 const sql = yield* SqlClient.SqlClient;
14 const rows = yield* sql<{ readonly migration_id: number; readonly name: string }>`
15 SELECT migration_id, name FROM effect_sql_migrations ORDER BY migration_id
16 `;
17 return rows.map((row) => [row.migration_id, row.name] as const);
18});
19
20const v2TableCount = Effect.gen(function* () {
21 const sql = yield* SqlClient.SqlClient;
22 const rows = yield* sql`
23 SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE 'orchestration_v2_%'
24 `;
25 return rows.length;
26});
27
28describe("runMigrations", () => {
29 it.effect("migrates a fresh database through the whole manifest", () =>
30 Effect.gen(function* () {
31 yield* runMigrations();
32 assert.deepStrictEqual(yield* history, migrationManifest);
33 assert.isAbove(yield* v2TableCount, 0);
34 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
35 );
36
37 it.effect("refuses to start when another migration took a V2 id", () =>
38 Effect.gen(function* () {
39 const sql = yield* SqlClient.SqlClient;
40 yield* runMigrations({ toMigrationInclusive: 54 });
41 yield* sql`
42 INSERT INTO effect_sql_migrations (migration_id, name) VALUES (55, 'FutureMainMigration')
43 `;
44 const before = yield* history;
45
46 assert.deepStrictEqual(yield* runMigrations({ toMigrationInclusive: 54 }), []);
47 const exit = yield* Effect.exit(runMigrations());
48
49 assert.isTrue(Exit.isFailure(exit));
50 assert.include(String(exit), "55_OrchestrationV2 (recorded: FutureMainMigration)");
51 assert.deepStrictEqual(yield* history, before);
52 assert.strictEqual(yield* v2TableCount, 0);
53 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
54 );
55
56 it.effect("refuses to start when a later id would skip V2 migrations", () =>
57 Effect.gen(function* () {
58 const sql = yield* SqlClient.SqlClient;
59 yield* runMigrations({ toMigrationInclusive: 54 });
60 yield* sql`
61 INSERT INTO effect_sql_migrations (migration_id, name)
62 VALUES (${newerMigrationId}, 'FutureMainMigration')
63 `;
64 const before = yield* history;
65
66 const exit = yield* Effect.exit(runMigrations());
67
68 assert.isTrue(Exit.isFailure(exit));
69 for (const [id, name] of v2Migrations) {
70 assert.include(String(exit), `${id}_${name} (recorded: nothing)`);
71 }
72 assert.deepStrictEqual(yield* history, before);
73 assert.strictEqual(yield* v2TableCount, 0);
74 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
75 );
76
77 it.effect("upgrades a database at exactly 54", () =>
78 Effect.gen(function* () {
79 yield* runMigrations({ toMigrationInclusive: 54 });
80 assert.deepStrictEqual(yield* runMigrations(), v2Migrations);
81 assert.deepStrictEqual(yield* history, migrationManifest);
82 assert.isAbove(yield* v2TableCount, 0);
83 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
84 );
85
86 it.effect.each([30, 54])("still starts with a site-local migration at id %s", (id) =>
87 Effect.gen(function* () {
88 const sql = yield* SqlClient.SqlClient;
89 yield* runMigrations({ toMigrationInclusive: 54 });
90 yield* sql`
91 UPDATE effect_sql_migrations SET name = 'SiteLocalMigration' WHERE migration_id = ${id}
92 `;
93
94 assert.deepStrictEqual(yield* runMigrations(), v2Migrations);
95 assert.isAbove(yield* v2TableCount, 0);
96 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
97 );
98
99 it.effect("only checks V2 migrations within the requested limit", () =>
100 Effect.gen(function* () {
101 const sql = yield* SqlClient.SqlClient;
102 yield* runMigrations({ toMigrationInclusive: 55 });
103 yield* sql`
104 INSERT INTO effect_sql_migrations (migration_id, name)
105 VALUES (56, 'SiteLocalMigration'), (${newerMigrationId}, 'NewerV2Migration')
106 `;
107 const before = yield* history;
108
109 assert.deepStrictEqual(yield* runMigrations({ toMigrationInclusive: 55 }), []);
110 const exit = yield* Effect.exit(runMigrations({ toMigrationInclusive: 56 }));
111 assert.isTrue(Exit.isFailure(exit));
112 assert.include(
113 String(exit),
114 "56_RemoveRedundantProjectionIndexes (recorded: SiteLocalMigration)",
115 );
116 assert.notInclude(String(exit), "57_ScheduledTaskWebhooks");
117 assert.deepStrictEqual(yield* history, before);
118 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
119 );
120
121 it.effect("still starts after a newer build recorded a later V2 migration", () =>
122 Effect.gen(function* () {
123 const sql = yield* SqlClient.SqlClient;
124 yield* runMigrations();
125 yield* sql`
126 INSERT INTO effect_sql_migrations (migration_id, name)
127 VALUES (${newerMigrationId}, 'NewerV2Migration')
128 `;
129
130 assert.deepStrictEqual(yield* runMigrations(), []);
131 }).pipe(Effect.provide(NodeSqliteClient.layer({ filename: ":memory:" }))),
132 );
133});
apps/server/src/persistence/Migrations.ts+48-4
......@@ -163,6 +163,48 @@ const makeMigrationLoader = (throughId?: number) =>
163163 ),
164164 );
165165
166// V2's schema migrations start at 55 after the legacy schema migrations.
167const FIRST_V2_MIGRATION_ID = 55;
168
169/**
170 * Fail before migrating when the migrator would skip one of V2's own
171 * migrations. It skips every id up to the highest recorded one, whatever name
172 * was recorded, so a database that took a V2 id for another migration would
173 * start without V2's schema and fail on first use. Earlier ids keep the
174 * warning below: databases with a site-local migration there still work.
175 */
176const assertV2MigrationsCanRun = Effect.fn("assertV2MigrationsCanRun")(function* (
177 throughId?: number,
178) {
179 const sql = yield* SqlClient.SqlClient;
180 const tables = yield* sql`
181 SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'effect_sql_migrations'
182 `;
183 if (tables.length === 0) return;
184 const recorded = yield* sql<{
185 readonly migration_id: number;
186 readonly name: string;
187 }>`SELECT migration_id, name FROM effect_sql_migrations`;
188 const latestRecordedId = Math.max(0, ...recorded.map((row) => row.migration_id));
189 const recordedNames = new Map(recorded.map((row) => [row.migration_id, row.name]));
190 const skipped = migrationEntries.flatMap(([id, name]) => {
191 if (
192 id < FIRST_V2_MIGRATION_ID ||
193 id > latestRecordedId ||
194 (throughId !== undefined && id > throughId)
195 )
196 return [];
197 const recordedName = recordedNames.get(id);
198 if (recordedName === name) return [];
199 return [`${id}_${name} (recorded: ${recordedName ?? "nothing"})`];
200 });
201 if (skipped.length === 0) return;
202 return yield* new Migrator.MigrationError({
203 kind: "BadState",
204 message: `This database's migration history would skip required schema migrations: ${skipped.join(", ")}. The migrator skips every id up to the highest recorded id (${latestRecordedId}), regardless of name. It was probably written by a different build. Start the build that wrote it, or report this migration history so an upgrade path can be added.`,
205 });
206});
207
166208/**
167209 * Migrator run function - no schema dumping needed
168210 * Uses the base Migrator.make without platform dependencies
......@@ -186,10 +228,12 @@ export interface RunMigrationsOptions {
186228export const runMigrations = Effect.fn("runMigrations")(function* ({
187229 toMigrationInclusive,
188230}: RunMigrationsOptions = {}) {
189 const previewMigrations =
190 toMigrationInclusive === undefined || toMigrationInclusive >= 55
191 ? yield* reconcileV2PreviewMigration()
192 : [];
231 const includesV2 =
232 toMigrationInclusive === undefined || toMigrationInclusive >= FIRST_V2_MIGRATION_ID;
233 const previewMigrations = includesV2 ? yield* reconcileV2PreviewMigration() : [];
234 if (includesV2) {
235 yield* assertV2MigrationsCanRun(toMigrationInclusive);
236 }
193237 const executedMigrations = [
194238 ...previewMigrations,
195239 ...(yield* run({ loader: makeMigrationLoader(toMigrationInclusive) })),
docs/internals/legacy-orchestration-migration.md+6-4
......@@ -46,10 +46,12 @@ running half-upgraded. See `packages/client-runtime/src/connection/compatibility
4646`effect_sql_migrations` records `migration_id` and `name`, but the migrator compares ids only:
4747rows at or below the recorded maximum are skipped without checking names. A database that ran a
4848local or fork migration under an id this build later assigns to a different migration therefore
49never receives this build's migration at that id. `runMigrations` logs each recorded id whose name
50differs from the manifest so the skipped schema change is diagnosable. There is no safe id range
51for a fork inside this ledger: any id at or below a future upstream id masks it forever, so fork
52schema changes belong in a separate migration table or outside the migrator entirely.
49never receives this build's migration at that id. After repairing supported V2 preview ledgers,
50[`runMigrations`](../../apps/server/src/persistence/Migrations.ts) rejects history that would skip a
51requested migration at id 55 or later without its expected name. Earlier name clashes remain
52warnings for compatibility with site-local migrations. There is no safe id range for a fork inside
53this ledger: any id at or below a future upstream id masks it forever, so fork schema changes belong
54in a separate migration table or outside the migrator entirely.
5355
5456## Recovery
5557