diff --git a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts index 6fe7f831a032..e804c4049a4d 100644 --- a/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts +++ b/apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts @@ -313,6 +313,7 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { snoozedUntil: null, snoozedAt: null, titleRegeneration: null, + titleRegenerationFailure: null, deletedAt: null, messages: [ { @@ -428,6 +429,7 @@ projectionSnapshotLayer("ProjectionSnapshotQuery", (it) => { snoozedUntil: null, snoozedAt: null, titleRegeneration: null, + titleRegenerationFailure: null, session: { threadId: ThreadId.make("thread-1"), status: "running", diff --git a/apps/server/src/persistence/Migrations.ts b/apps/server/src/persistence/Migrations.ts index 9fac92d5655e..7f70643a9cc8 100644 --- a/apps/server/src/persistence/Migrations.ts +++ b/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) => diff --git a/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegenerationFailure.test.ts b/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegenerationFailure.test.ts index fd97c31ac095..4d7cac2688fd 100644 --- a/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegenerationFailure.test.ts +++ b/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegenerationFailure.test.ts @@ -1,47 +1,67 @@ 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"; +import { columnNames, onFreshDatabase } from "./migrationTestSupport.ts"; -const layer = it.layer(Layer.mergeAll(NodeSqliteClient.layerMemory())); +const FAILURE_COLUMNS = [ + "title_regeneration_failure_request_id", + "title_regeneration_failure_at", + "title_regeneration_failure_error", +] as const; -layer("036_ProjectionThreadTitleRegenerationFailure", (it) => { +const projectionThreadColumns = columnNames("projection_threads"); + +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* projectionThreadColumns; + for (const column of FAILURE_COLUMNS) { + assert.ok(!before.includes(column), `${column} should not exist before 39`); + } + + yield* runMigrations({ toMigrationInclusive: 39 }); + + const after = yield* projectionThreadColumns; + for (const column of FAILURE_COLUMNS) { + assert.ok(after.includes(column), `${column} should exist after 39`); + } + // 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("skips columns that already exist", () => + onFreshDatabase( + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + + yield* runMigrations({ toMigrationInclusive: 38 }); + yield* sql` + ALTER TABLE projection_threads + ADD COLUMN title_regeneration_failure_at TEXT + `; + + // Exercises the PRAGMA guard rather than the migrator's id bookkeeping: + // without it this dies on "duplicate column name: …_failure_at". + yield* runMigrations({ toMigrationInclusive: 39 }); + + const columns = yield* projectionThreadColumns; + for (const column of FAILURE_COLUMNS) { + assert.strictEqual( + columns.filter((name) => name === column).length, + 1, + `${column} should exist exactly once`, + ); + } + }), + ), ); }); diff --git a/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.test.ts b/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.test.ts new file mode 100644 index 000000000000..93f0a0b93385 --- /dev/null +++ b/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.test.ts @@ -0,0 +1,101 @@ +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 { columnNames, onFreshDatabase } from "./migrationTestSupport.ts"; + +const FAILURE_COLUMNS = [ + "title_regeneration_failure_request_id", + "title_regeneration_failure_at", + "title_regeneration_failure_error", +] as const; + +const projectionThreadColumns = columnNames("projection_threads"); + +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* projectionThreadColumns; + 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* projectionThreadColumns; + 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* projectionThreadColumns; + for (const column of FAILURE_COLUMNS) { + assert.strictEqual( + columns.filter((name) => name === column).length, + 1, + `${column} should exist exactly once`, + ); + } + }), + ), + ); + + it.effect("applies 39 then 40 in order from a database sitting at 38", () => + onFreshDatabase( + Effect.gen(function* () { + yield* runMigrations({ toMigrationInclusive: 38 }); + + // The mid-sequence case: neither id applied yet, so both must run, in + // ascending order — 40 assumes 39 has already had its chance. + const executed = yield* runMigrations({ toMigrationInclusive: 40 }); + assert.deepStrictEqual( + executed.map(([id]) => id), + [39, 40], + ); + + const columns = yield* projectionThreadColumns; + for (const column of FAILURE_COLUMNS) { + assert.ok(columns.includes(column), `${column} should exist`); + } + }), + ), + ); +}); diff --git a/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.ts b/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.ts new file mode 100644 index 000000000000..d0ad496aa5c8 --- /dev/null +++ b/apps/server/src/persistence/Migrations/037_RepairProjectionThreadTitleRegenerationFailure.ts @@ -0,0 +1,53 @@ +/** + * 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. + * + * Both shapes of 39 reached `main` in the same merge, so no released build ever + * carried the earlier one — the affected databases are those on machines that + * ran the PR branch mid-review. Narrow, but the failure is a hard one: every + * thread and shell snapshot query selects 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 + `; + } +}); diff --git a/apps/server/src/persistence/Migrations/migrationTestSupport.ts b/apps/server/src/persistence/Migrations/migrationTestSupport.ts new file mode 100644 index 000000000000..c002c4eb9eef --- /dev/null +++ b/apps/server/src/persistence/Migrations/migrationTestSupport.ts @@ -0,0 +1,36 @@ +/** + * Shared helpers for migration tests. + * + * The `it.layer` idiom used by most tests in this directory hands every case in + * a block the same in-memory database. Once one case migrates to the head id, + * the migrator skips every later `runMigrations` call — later cases then assert + * against a database no migration body touched and pass vacuously. Migration + * tests therefore need a database per case, which is what `onFreshDatabase` + * provides. + * + * @module migrationTestSupport + */ +import * as Effect from "effect/Effect"; +import * as SqlClient from "effect/unstable/sql/SqlClient"; + +import * as NodeSqliteClient from "../NodeSqliteClient.ts"; + +/** Run one test case against its own empty in-memory database. */ +export const onFreshDatabase = (effect: Effect.Effect) => + Effect.provide(effect, NodeSqliteClient.layerMemory()); + +/** + * Column names of a table, in declaration order. + * + * An array rather than a `Set` so callers can assert on ordering and counts. + * Note that SQLite rejects a duplicate `ADD COLUMN` outright, so a count + * assertion here cannot fail on its own — an unguarded migration errors first. + */ +export const columnNames = (table: string) => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + const columns = yield* sql<{ readonly name: string }>` + SELECT name FROM pragma_table_info(${table}) + `; + return columns.map((column) => column.name); + });