Repair databases that applied migration 39 in its pre-review shape

apps/server/src/persistence/Migrations.ts

@@ -59,6 +59,7 @@ import Migration0036 from "./Migrations/033_ProjectionThreadsSettled.ts";
 import Migration0037 from "./Migrations/034_ProjectionThreadsSnoozed.ts";
 import Migration0038 from "./Migrations/035_ProjectionThreadTitleRegeneration.ts";
 import Migration0039 from "./Migrations/036_ProjectionThreadTitleRegenerationFailure.ts";
+import Migration0040 from "./Migrations/037_RepairProjectionThreadTitleRegenerationFailure.ts";
 
 /**
  * Migration loader with all migrations defined inline.
@@ -110,6 +111,7 @@ export const migrationEntries = [
   [37, "ProjectionThreadsSnoozed", Migration0037],
   [38, "ProjectionThreadTitleRegeneration", Migration0038],
   [39, "ProjectionThreadTitleRegenerationFailure", Migration0039],
+  [40, "RepairProjectionThreadTitleRegenerationFailure", Migration0040],
 ] as const;
 
 export const makeMigrationLoader = (throughId?: number) =>

apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegenerationFailure.test.ts

@@ -1,47 +1,64 @@
 import { assert, it } from "@effect/vitest";
 import * as Effect from "effect/Effect";
-import * as Layer from "effect/Layer";
 import * as SqlClient from "effect/unstable/sql/SqlClient";
+import { describe } from "vite-plus/test";
 
 import { runMigrations } from "../Migrations.ts";
 import * as NodeSqliteClient from "../NodeSqliteClient.ts";
 
-const layer = it.layer(Layer.mergeAll(NodeSqliteClient.layerMemory()));
+/** See 037's note: a shared database makes every case after the first vacuous. */
+const onFreshDatabase = <A, E>(effect: Effect.Effect<A, E, SqlClient.SqlClient>) =>
+  Effect.provide(effect, NodeSqliteClient.layerMemory());
 
-layer("036_ProjectionThreadTitleRegenerationFailure", (it) => {
+/** Raw PRAGMA rows, not a Set — a Set cannot express a duplicate column. */
+const columnNames = Effect.gen(function* () {
+  const sql = yield* SqlClient.SqlClient;
+  const columns = yield* sql<{ readonly name: string }>`
+    PRAGMA table_info(projection_threads)
+  `;
+  return columns.map((column) => column.name);
+});
+
+describe("036_ProjectionThreadTitleRegenerationFailure", () => {
   it.effect("adds the title regeneration failure columns", () =>
-    Effect.gen(function* () {
-      const sql = yield* SqlClient.SqlClient;
-
-      yield* runMigrations({ toMigrationInclusive: 38 });
-      yield* runMigrations({ toMigrationInclusive: 39 });
-
-      const columns = yield* sql<{ readonly name: string }>`
-        PRAGMA table_info(projection_threads)
-      `;
-      const names = new Set(columns.map((column) => column.name));
-      assert.ok(names.has("title_regeneration_failure_request_id"));
-      assert.ok(names.has("title_regeneration_failure_at"));
-      assert.ok(names.has("title_regeneration_failure_error"));
-      // The pending columns stay separate so "in flight" keeps its meaning.
-      assert.ok(names.has("title_regeneration_request_id"));
-      assert.ok(names.has("title_regeneration_started_at"));
-    }),
+    onFreshDatabase(
+      Effect.gen(function* () {
+        yield* runMigrations({ toMigrationInclusive: 38 });
+
+        const before = yield* columnNames;
+        assert.ok(!before.includes("title_regeneration_failure_error"));
+
+        yield* runMigrations({ toMigrationInclusive: 39 });
+
+        const after = yield* columnNames;
+        assert.ok(after.includes("title_regeneration_failure_request_id"));
+        assert.ok(after.includes("title_regeneration_failure_at"));
+        assert.ok(after.includes("title_regeneration_failure_error"));
+        // The pending columns stay separate so "in flight" keeps its meaning.
+        assert.ok(after.includes("title_regeneration_request_id"));
+        assert.ok(after.includes("title_regeneration_started_at"));
+      }),
+    ),
   );
 
-  it.effect("is idempotent when the columns already exist", () =>
-    Effect.gen(function* () {
-      yield* runMigrations({ toMigrationInclusive: 39 });
-      yield* runMigrations({ toMigrationInclusive: 39 });
-
-      const sql = yield* SqlClient.SqlClient;
-      const columns = yield* sql<{ readonly name: string }>`
-        PRAGMA table_info(projection_threads)
-      `;
-      assert.strictEqual(
-        columns.filter((column) => column.name === "title_regeneration_failure_error").length,
-        1,
-      );
-    }),
+  it.effect("adds each column exactly once", () =>
+    onFreshDatabase(
+      Effect.gen(function* () {
+        yield* runMigrations({ toMigrationInclusive: 39 });
+
+        const columns = yield* columnNames;
+        for (const column of [
+          "title_regeneration_failure_request_id",
+          "title_regeneration_failure_at",
+          "title_regeneration_failure_error",
+        ]) {
+          assert.strictEqual(
+            columns.filter((name) => name === column).length,
+            1,
+            `${column} should exist exactly once`,
+          );
+        }
+      }),
+    ),
   );
 });

apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.test.ts

@@ -0,0 +1,113 @@
+import { assert, it } from "@effect/vitest";
+import * as Effect from "effect/Effect";
+import * as SqlClient from "effect/unstable/sql/SqlClient";
+import { describe } from "vite-plus/test";
+
+import { runMigrations } from "../Migrations.ts";
+import * as NodeSqliteClient from "../NodeSqliteClient.ts";
+
+/**
+ * Each case gets its own database.
+ *
+ * The shared-layer idiom used elsewhere in this directory hands every case in a
+ * block the same in-memory database, so once one case migrates to the head id
+ * the migrator skips every later `runMigrations` call — the bodies under test
+ * never execute again and the cases pass vacuously. That is exactly the failure
+ * mode being guarded against here, so the database has to be per-case.
+ */
+const onFreshDatabase = <A, E>(effect: Effect.Effect<A, E, SqlClient.SqlClient>) =>
+  Effect.provide(effect, NodeSqliteClient.layerMemory());
+
+/** Raw PRAGMA rows, not a Set — a Set cannot express a duplicate column. */
+const columnNames = Effect.gen(function* () {
+  const sql = yield* SqlClient.SqlClient;
+  const columns = yield* sql<{ readonly name: string }>`
+    PRAGMA table_info(projection_threads)
+  `;
+  return columns.map((column) => column.name);
+});
+
+const FAILURE_COLUMNS = [
+  "title_regeneration_failure_request_id",
+  "title_regeneration_failure_at",
+  "title_regeneration_failure_error",
+] as const;
+
+describe("037_RepairProjectionThreadTitleRegenerationFailure", () => {
+  it.effect("adds the failure columns to a database that applied the pre-review 39", () =>
+    onFreshDatabase(
+      Effect.gen(function* () {
+        const sql = yield* SqlClient.SqlClient;
+
+        // Reproduce the shape migration 39 had before it was rewritten: one
+        // `title_regeneration_error` column, recorded under the old name. The
+        // migrator skips by id without comparing names, so the rewritten 39 is
+        // treated as already applied and never runs.
+        yield* runMigrations({ toMigrationInclusive: 38 });
+        yield* sql`
+          ALTER TABLE projection_threads
+          ADD COLUMN title_regeneration_error TEXT
+        `;
+        yield* sql`
+          INSERT INTO effect_sql_migrations (migration_id, name)
+          VALUES (39, 'ProjectionThreadTitleRegenerationError')
+        `;
+
+        // The defect itself: 39 is skipped, so the columns are genuinely absent.
+        const beforeRepair = yield* columnNames;
+        for (const column of FAILURE_COLUMNS) {
+          assert.ok(
+            !beforeRepair.includes(column),
+            `${column} should be missing before the repair`,
+          );
+        }
+
+        const executed = yield* runMigrations({ toMigrationInclusive: 40 });
+        assert.deepStrictEqual(
+          executed.map(([id]) => id),
+          [40],
+        );
+
+        const afterRepair = yield* columnNames;
+        for (const column of FAILURE_COLUMNS) {
+          assert.ok(afterRepair.includes(column), `${column} should exist after the repair`);
+        }
+      }),
+    ),
+  );
+
+  it.effect("no-ops on a fresh database where 39 already added the columns", () =>
+    onFreshDatabase(
+      Effect.gen(function* () {
+        // Without the PRAGMA guard this dies on "duplicate column name": on a
+        // fresh install 39 and 40 run in the same pass, 39 having just added
+        // every column 40 adds.
+        const executed = yield* runMigrations({ toMigrationInclusive: 40 });
+        assert.ok(executed.some(([id]) => id === 40));
+
+        const columns = yield* columnNames;
+        for (const column of FAILURE_COLUMNS) {
+          assert.strictEqual(
+            columns.filter((name) => name === column).length,
+            1,
+            `${column} should exist exactly once`,
+          );
+        }
+      }),
+    ),
+  );
+
+  it.effect("leaves the pending columns untouched", () =>
+    onFreshDatabase(
+      Effect.gen(function* () {
+        yield* runMigrations({ toMigrationInclusive: 40 });
+
+        // The repair must not disturb the pending record, whose separation from
+        // the failure state is what keeps older clients working.
+        const columns = yield* columnNames;
+        assert.ok(columns.includes("title_regeneration_request_id"));
+        assert.ok(columns.includes("title_regeneration_started_at"));
+      }),
+    ),
+  );
+});

apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.ts

@@ -0,0 +1,48 @@
+/**
+ * Repairs databases that applied migration 39 while it still carried its
+ * pre-review shape.
+ *
+ * Migration 39 originally added a single `title_regeneration_error` column, and
+ * was then rewritten in place to add the three `title_regeneration_failure_*`
+ * columns instead. The migrator records applied migrations by numeric id and
+ * skips anything at or below the latest recorded id without comparing names, so
+ * a database that ran the earlier shape treats the rewritten 39 as done and
+ * never grows the failure columns — every thread and shell snapshot query then
+ * fails on the missing columns.
+ *
+ * Adding them again under a fresh id fixes those databases and no-ops on any
+ * database that got them from 39. The orphaned `title_regeneration_error`
+ * column is left in place: it is nullable and unreferenced, and dropping it
+ * would rewrite the table for no benefit.
+ */
+import * as Effect from "effect/Effect";
+import * as SqlClient from "effect/unstable/sql/SqlClient";
+
+export default Effect.gen(function* () {
+  const sql = yield* SqlClient.SqlClient;
+  const columns = yield* sql<{ readonly name: string }>`
+    PRAGMA table_info(projection_threads)
+  `;
+  const names = new Set(columns.map((column) => column.name));
+
+  if (!names.has("title_regeneration_failure_request_id")) {
+    yield* sql`
+      ALTER TABLE projection_threads
+      ADD COLUMN title_regeneration_failure_request_id TEXT
+    `;
+  }
+
+  if (!names.has("title_regeneration_failure_at")) {
+    yield* sql`
+      ALTER TABLE projection_threads
+      ADD COLUMN title_regeneration_failure_at TEXT
+    `;
+  }
+
+  if (!names.has("title_regeneration_failure_error")) {
+    yield* sql`
+      ALTER TABLE projection_threads
+      ADD COLUMN title_regeneration_failure_error TEXT
+    `;
+  }
+});