fix(server): refuse mismatched fork ledger adoption and verify fork slots

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.

Author: asfires

apps/server/scripts/migrate-dev-db.test.ts

@@ -132,6 +132,36 @@ it.layer(NodeServices.layer)("migrate-dev-db", (it) => {
     }),
   );
 
+  it.effect("fails loudly on a fork migration slot collision", () =>
+    Effect.gen(function* () {
+      const fs = yield* FileSystem.FileSystem;
+      const sourceDir = yield* fs.makeTempDirectoryScoped({ prefix: "migrate-dev-db-fork-slot-" });
+      const destDir = yield* fs.makeTempDirectoryScoped({
+        prefix: "migrate-dev-db-fork-slot-dest-",
+      });
+      const source = yield* createFixtureSource(sourceDir);
+      yield* withDatabase(
+        source,
+        Effect.gen(function* () {
+          const sql = yield* SqlClient.SqlClient;
+          yield* sql`UPDATE effect_sql_migrations_fork
+            SET name = 'SomebodyElsesForkMigration' WHERE migration_id = 1`;
+        }),
+      );
+
+      const error = yield* runMigrateDevDb(
+        { baseDir: destDir, source, projects: 5, threadsPerProject: 10 },
+        { sharedHome: sourceDir },
+      ).pipe(Effect.flip);
+      assert.equal(error._tag, "MigrateDevDbSlotCollisionError");
+      if (error._tag === "MigrateDevDbSlotCollisionError") {
+        assert.equal(error.ledger, "effect_sql_migrations_fork");
+        assert.equal(error.slot, 1);
+        assert.equal(error.appliedName, "SomebodyElsesForkMigration");
+      }
+    }),
+  );
+
   it.effect("refuses while a dev server holds the destination", () =>
     Effect.gen(function* () {
       const fs = yield* FileSystem.FileSystem;

apps/server/scripts/migrate-dev-db.ts

@@ -38,7 +38,11 @@ import * as Schema from "effect/Schema";
 import * as SqlClient from "effect/unstable/sql/SqlClient";
 import { Command, Flag } from "effect/unstable/cli";
 
-import { runAllMigrations } from "../src/persistence/ForkMigrations.ts";
+import {
+  forkMigrationManifest,
+  forkMigrationsTable,
+  runAllMigrations,
+} from "../src/persistence/ForkMigrations.ts";
 import { migrationManifest } from "../src/persistence/Migrations.ts";
 import * as NodeSqliteClient from "../src/persistence/NodeSqliteClient.ts";
 
@@ -119,13 +123,14 @@ export class MigrateDevDbDestinationBusyError extends Schema.TaggedErrorClass<Mi
 export class MigrateDevDbSlotCollisionError extends Schema.TaggedErrorClass<MigrateDevDbSlotCollisionError>()(
   "MigrateDevDbSlotCollisionError",
   {
+    ledger: Schema.String,
     slot: Schema.Number,
     codeName: Schema.String,
     appliedName: Schema.String,
   },
 ) {
   override get message(): string {
-    return `Migration slot collision at ${this.slot}: this checkout registers '${this.codeName}' but the database already applied '${this.appliedName}' in that slot. Renumber the new migration to a free slot.`;
+    return `Migration slot collision at ${this.ledger} ${this.slot}: this checkout registers '${this.codeName}' but the database already applied '${this.appliedName}' in that slot. Renumber the new migration to a free slot.`;
   }
 }
 
@@ -337,15 +342,22 @@ const pruneSnapshot = Effect.fn("pruneDevDbSnapshot")(function* (input: RunMigra
 /** Compare this checkout's migration registry against what the cloned
  * database recorded: same slot under a different name means the migration
  * was skipped, not applied. */
+const migrationLedgers = [
+  ["effect_sql_migrations", migrationManifest],
+  [forkMigrationsTable, forkMigrationManifest],
+] as const;
+
 const verifyMigrationSlots = Effect.fn("verifyMigrationSlots")(function* () {
   const sql = yield* SqlClient.SqlClient;
-  const applied = yield* sql<{ migration_id: number; name: string }>`
-    SELECT migration_id, name FROM effect_sql_migrations`;
-  const appliedById = new Map(applied.map((row) => [Number(row.migration_id), row.name]));
-  for (const [slot, codeName] of migrationManifest) {
-    const appliedName = appliedById.get(slot);
-    if (appliedName !== undefined && appliedName !== codeName) {
-      return yield* new MigrateDevDbSlotCollisionError({ slot, codeName, appliedName });
+  for (const [ledger, manifest] of migrationLedgers) {
+    const applied = yield* sql<{ migration_id: number; name: string }>`
+      SELECT migration_id, name FROM ${sql(ledger)}`;
+    const appliedById = new Map(applied.map((row) => [Number(row.migration_id), row.name]));
+    for (const [slot, codeName] of manifest) {
+      const appliedName = appliedById.get(slot);
+      if (appliedName !== undefined && appliedName !== codeName) {
+        return yield* new MigrateDevDbSlotCollisionError({ ledger, slot, codeName, appliedName });
+      }
     }
   }
 });

apps/server/src/persistence/ForkMigrations.test.ts

@@ -119,7 +119,7 @@ freshLayer()(
         yield* applyLegacyForkMigrations;
         const takenId = legacyForkRow(forkMigrationEntries.length);
         yield* sql`
-        UPDATE effect_sql_migrations SET name = 'OrchestrationV2' WHERE migration_id = ${takenId}
+        UPDATE effect_sql_migrations SET name = 'ApplicationEventSource' WHERE migration_id = ${takenId}
       `;
 
         const result = yield* runAllMigrations();
@@ -142,3 +142,36 @@ freshLayer()(
     );
   },
 );
+
+freshLayer()("ForkMigrations when the fork ledger already holds a different migration", (it) => {
+  it.effect("refuses to adopt rather than dropping the legacy row", () =>
+    Effect.gen(function* () {
+      const sql = yield* SqlClient.SqlClient;
+      yield* applyLegacyForkMigrations;
+      yield* sql`
+        CREATE TABLE ${sql(forkMigrationsTable)} (
+          migration_id integer PRIMARY KEY NOT NULL,
+          created_at datetime NOT NULL DEFAULT current_timestamp,
+          name VARCHAR(255) NOT NULL
+        )
+      `;
+      yield* sql`
+        INSERT INTO ${sql(forkMigrationsTable)} (migration_id, name)
+        VALUES (${forkMigrationEntries.length}, 'DifferentMigration')
+      `;
+
+      const error = yield* runAllMigrations().pipe(Effect.flip);
+
+      assert.equal(error._tag, "MigrationError");
+      if (error._tag === "MigrationError") {
+        assert.equal(error.kind, "BadState");
+      }
+      // The transaction rolled back: every legacy row is still in place.
+      assert.deepEqual(yield* readIds("effect_sql_migrations"), [
+        ...migrationManifest.map(([id]) => id),
+        ...forkMigrationManifest.map(([id]) => legacyForkRow(id)),
+      ]);
+      assert.deepEqual(yield* readIds(forkMigrationsTable), [forkMigrationEntries.length]);
+    }),
+  );
+});

apps/server/src/persistence/ForkMigrations.ts

@@ -82,12 +82,30 @@ const adoptLegacyForkLedgerRows = Effect.fn("adoptLegacyForkLedgerRows")(functio
       ([id, name]) => {
         const legacyId = legacyUpstreamIdOffset + id;
         return Effect.gen(function* () {
-          yield* sql`
-            INSERT OR IGNORE INTO ${sql(forkMigrationsTable)} (migration_id, created_at, name)
-            SELECT ${id}, created_at, name
+          const legacy = yield* sql<{ readonly createdAt: string }>`
+            SELECT created_at AS "createdAt"
             FROM ${sql(upstreamMigrationsTable)}
             WHERE migration_id = ${legacyId} AND name = ${name}
           `;
+          if (legacy.length === 0) {
+            return;
+          }
+          const existing = yield* sql<{ readonly name: string }>`
+            SELECT name FROM ${sql(forkMigrationsTable)} WHERE migration_id = ${id}
+          `;
+          if (existing.length === 0) {
+            yield* sql`
+              INSERT INTO ${sql(forkMigrationsTable)} (migration_id, created_at, name)
+              VALUES (${id}, ${legacy[0]!.createdAt}, ${name})
+            `;
+          } else if (existing[0]!.name !== name) {
+            // Deleting the legacy row here would lose the only record that the
+            // fork migration ran; refuse instead of silently skipping it.
+            return yield* new Migrator.MigrationError({
+              kind: "BadState",
+              message: `Fork migration ledger slot ${id} holds "${existing[0]!.name}" but upstream ledger row ${legacyId} records "${name}"`,
+            });
+          }
           yield* sql`
             DELETE FROM ${sql(upstreamMigrationsTable)}
             WHERE migration_id = ${legacyId} AND name = ${name}