diff --git a/.agents/skills/trogonstack-sync-upstream/SKILL.md b/.agents/skills/trogonstack-sync-upstream/SKILL.md index 6b2a3abc771..509b61fa51f 100644 --- a/.agents/skills/trogonstack-sync-upstream/SKILL.md +++ b/.agents/skills/trogonstack-sync-upstream/SKILL.md @@ -39,9 +39,10 @@ Record every nontrivial resolution decision as you go; each one becomes a PR bod ### Migration rules (apps/server/src/persistence) -- Fork-only migrations live in `apps/server/src/persistence/Migrations/fork/` and use ids >= 1000, a block upstream never grows into. Id `33` (ProjectionThreadParent) predates this convention and is grandfathered. -- Never renumber a migration id that has already shipped on this fork, even if upstream independently reuses that id. The migrator only remembers the highest id that ran, so reassigning a shipped id makes installs silently skip it. Slot incoming upstream migrations around what already shipped instead. -- After resolving `Migrations.ts`, run the fail-fast guard: `vp test run apps/server/src/persistence/Migrations.test.ts`. It rejects fork migrations outside their reserved id block. +- Upstream migrations sync as-is, keeping their own ids. `Migrations.ts` and every file under `apps/server/src/persistence/Migrations/` (excluding `fork/`) must stay byte-identical to upstream. If either conflicts during a sync, take upstream's side wholesale rather than hand-merging. +- Fork-only schema never lands in `Migrations.ts`. It lives only in `apps/server/src/persistence/ForkMigrations.ts`, numbered independently under `apps/server/src/persistence/Migrations/fork/`, and tracked in its own `trogonstack_fork_migrations` ledger table instead of the shared `effect_sql_migrations` table. +- Because the two chains never share a ledger, id collisions between fork and upstream migrations are structurally impossible - there is nothing to renumber or offset during a sync. +- After resolving `Migrations.ts`, run the fail-fast guard: `vp test run apps/server/src/persistence/ForkMigrations.test.ts`. It rejects any fork migration name that leaks into the shared `migrationEntries` chain. ## 4. Verify diff --git a/apps/server/src/persistence/ForkMigrations.test.ts b/apps/server/src/persistence/ForkMigrations.test.ts new file mode 100644 index 00000000000..c76b03bef8a --- /dev/null +++ b/apps/server/src/persistence/ForkMigrations.test.ts @@ -0,0 +1,167 @@ +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, expect, it as vpIt } from "vite-plus/test"; + +import { migrationEntries, runMigrations } from "./Migrations.ts"; +import { + forkMigrationEntries, + realignSharedMigrationLedger, + runForkMigrations, +} from "./ForkMigrations.ts"; +import * as NodeSqliteClient from "./NodeSqliteClient.ts"; + +const UPSTREAM_MAX = Math.max(...migrationEntries.map(([id]) => id)); + +const selectSharedLedger = (sql: SqlClient.SqlClient) => + sql<{ readonly migration_id: number; readonly name: string }>` + SELECT migration_id, name FROM effect_sql_migrations ORDER BY migration_id + `; + +const selectForkLedger = (sql: SqlClient.SqlClient) => + sql<{ readonly migration_id: number; readonly name: string }>` + SELECT migration_id, name FROM trogonstack_fork_migrations ORDER BY migration_id + `; + +describe("forkMigrationEntries", () => { + vpIt("never overlaps with the shared migrationEntries names", () => { + const sharedNames: ReadonlySet = new Set(migrationEntries.map(([, name]) => name)); + for (const [, name] of forkMigrationEntries) { + expect(sharedNames.has(name)).toBe(false); + } + }); + + vpIt("lists entries with unique, ascending ids", () => { + const ids = forkMigrationEntries.map(([id]) => id); + expect(new Set(ids).size).toBe(ids.length); + expect(ids).toEqual([...ids].sort((a, b) => a - b)); + }); +}); + +const layer = it.layer(Layer.mergeAll(NodeSqliteClient.layerMemory())); + +layer("fresh install", (it) => { + it.effect("runs the fork chain on its own ledger without touching shared numbering", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + + yield* realignSharedMigrationLedger(); + yield* runMigrations(); + yield* runForkMigrations(); + + const columns = yield* sql<{ readonly name: string }>` + PRAGMA table_info(projection_threads) + `; + assert.ok(columns.some((column) => column.name === "parent_thread_id")); + + const forkLedger = yield* selectForkLedger(sql); + assert.deepStrictEqual(forkLedger, [{ migration_id: 1, name: "ProjectionThreadParent" }]); + + const sharedLedger = yield* selectSharedLedger(sql); + assert.strictEqual(sharedLedger[sharedLedger.length - 1]?.migration_id, UPSTREAM_MAX); + assert.ok(!sharedLedger.some((row) => row.name === "ProjectionThreadParent")); + }), + ); +}); + +layer("legacy full install", (it) => { + it.effect("realigns a ledger that ran the old fork chain through upstream 35 (legacy 36)", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + + yield* runMigrations(); + + yield* sql` + ALTER TABLE projection_threads + ADD COLUMN parent_thread_id TEXT + `; + yield* sql` + CREATE INDEX IF NOT EXISTS idx_projection_threads_parent + ON projection_threads(parent_thread_id) + `; + + yield* sql`UPDATE effect_sql_migrations SET migration_id = 36 WHERE migration_id = 35`; + yield* sql`UPDATE effect_sql_migrations SET migration_id = 35 WHERE migration_id = 34`; + yield* sql`UPDATE effect_sql_migrations SET migration_id = 34 WHERE migration_id = 33`; + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name, created_at) + VALUES (33, 'ProjectionThreadParent', CURRENT_TIMESTAMP) + `; + + yield* realignSharedMigrationLedger(); + yield* runMigrations(); + yield* runForkMigrations(); + + const sharedLedger = yield* selectSharedLedger(sql); + assert.deepStrictEqual( + sharedLedger.map((row) => row.migration_id), + Array.from({ length: UPSTREAM_MAX }, (_, index) => index + 1), + ); + assert.deepStrictEqual( + sharedLedger.find((row) => row.migration_id === 33)?.name, + "ProjectionThreadsSettled", + ); + assert.deepStrictEqual( + sharedLedger.find((row) => row.migration_id === 34)?.name, + "ProjectionThreadsSnoozed", + ); + assert.deepStrictEqual( + sharedLedger.find((row) => row.migration_id === 35)?.name, + "ProjectionThreadTitleRegeneration", + ); + assert.ok(!sharedLedger.some((row) => row.name === "ProjectionThreadParent")); + + const forkLedger = yield* selectForkLedger(sql); + assert.deepStrictEqual(forkLedger, [{ migration_id: 1, name: "ProjectionThreadParent" }]); + + const columns = yield* sql<{ readonly name: string }>` + PRAGMA table_info(projection_threads) + `; + assert.ok(columns.some((column) => column.name === "parent_thread_id")); + + const ledgerBeforeSecondRealign = yield* selectSharedLedger(sql); + yield* realignSharedMigrationLedger(); + const ledgerAfterSecondRealign = yield* selectSharedLedger(sql); + assert.deepStrictEqual(ledgerAfterSecondRealign, ledgerBeforeSecondRealign); + }), + ); +}); + +layer("legacy mid-history install", (it) => { + it.effect("realigns a ledger that stopped right after the old fork migration 33", () => + Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + + yield* runMigrations({ toMigrationInclusive: 32 }); + + yield* sql` + ALTER TABLE projection_threads + ADD COLUMN parent_thread_id TEXT + `; + yield* sql` + CREATE INDEX IF NOT EXISTS idx_projection_threads_parent + ON projection_threads(parent_thread_id) + `; + yield* sql` + INSERT INTO effect_sql_migrations (migration_id, name, created_at) + VALUES (33, 'ProjectionThreadParent', CURRENT_TIMESTAMP) + `; + + yield* realignSharedMigrationLedger(); + yield* runMigrations(); + yield* runForkMigrations(); + + const sharedLedger = yield* selectSharedLedger(sql); + assert.strictEqual(sharedLedger[sharedLedger.length - 1]?.migration_id, UPSTREAM_MAX); + + const columns = yield* sql<{ readonly name: string }>` + PRAGMA table_info(projection_threads) + `; + assert.ok(columns.some((column) => column.name === "title_regeneration_request_id")); + + const forkLedger = yield* selectForkLedger(sql); + assert.deepStrictEqual(forkLedger, [{ migration_id: 1, name: "ProjectionThreadParent" }]); + }), + ); +}); diff --git a/apps/server/src/persistence/ForkMigrations.ts b/apps/server/src/persistence/ForkMigrations.ts new file mode 100644 index 00000000000..117e39494df --- /dev/null +++ b/apps/server/src/persistence/ForkMigrations.ts @@ -0,0 +1,113 @@ +/** + * ForkMigrationsLive - fork-only migration runner with its own ledger + * + * Fork-only schema (TrogonStack additions that do not exist upstream) lives + * in this second migration chain, tracked in its own migrations table + * (`trogonstack_fork_migrations`) instead of the shared `effect_sql_migrations` + * ledger. This keeps `Migrations.ts` and every file under `./Migrations/` + * byte-identical to upstream (pingdotgg/t3code) forever, since the shared + * chain never has to make room for fork ids. A fork migration must never be + * added to `Migrations.ts` - new fork-only schema belongs here, under + * `./Migrations/fork/`, numbered independently of the shared chain. + * + * The fork once shipped its first migration (ProjectionThreadParent) as id 33 + * in the shared chain, before this second chain existed, which permanently + * offset every upstream id after it. `realignSharedMigrationLedger` repairs + * that on installs that ran the old shared-chain history, moving the shared + * ledger back to upstream's numbering before the shared migrator runs. + */ + +import * as Migrator from "effect/unstable/sql/Migrator"; +import * as Layer from "effect/Layer"; +import * as Effect from "effect/Effect"; +import * as SqlClient from "effect/unstable/sql/SqlClient"; + +import Migration0001 from "./Migrations/fork/001_ProjectionThreadParent.ts"; + +export const FORK_MIGRATIONS_TABLE = "trogonstack_fork_migrations"; + +export const forkMigrationEntries = [[1, "ProjectionThreadParent", Migration0001]] as const; + +export const makeForkMigrationLoader = (throughId?: number) => + Migrator.fromRecord( + Object.fromEntries( + forkMigrationEntries + .filter(([id]) => throughId === undefined || id <= throughId) + .map(([id, name, migration]) => [`${id}_${name}`, migration]), + ), + ); + +const run = Migrator.make({}); + +export interface RunForkMigrationsOptions { + readonly toMigrationInclusive?: number | undefined; +} + +/** + * Run all pending fork-only migrations against the fork's own ledger table. + * + * @returns Effect containing array of executed migrations + */ +export const runForkMigrations = Effect.fn("runForkMigrations")(function* ({ + toMigrationInclusive, +}: RunForkMigrationsOptions = {}) { + const executedMigrations = yield* run({ + loader: makeForkMigrationLoader(toMigrationInclusive), + table: FORK_MIGRATIONS_TABLE, + }); + const migrations = executedMigrations.map(([id, name]) => `${id}_${name}`); + yield* migrations.length === 0 + ? Effect.logDebug("Fork database schema is current") + : Effect.log("Fork migrations ran successfully").pipe(Effect.annotateLogs({ migrations })); + return executedMigrations; +}); + +/** + * Realign the shared `effect_sql_migrations` ledger for installs that ran + * the fork's old migration chain, where ProjectionThreadParent shipped as + * shared id 33 and pushed upstream's 33/34/35 to 34/35/36. + * + * No-ops for fresh installs (no shared ledger table yet) and for installs + * already realigned (no id-33 ProjectionThreadParent row left to move). + * + * TODO: delete this function, its call in Layers/Sqlite.ts, and the legacy + * scenarios in ForkMigrations.test.ts once every install that ran the old + * shared-chain fork history has started up with it at least once. Fresh + * installs never need it. + */ +export const realignSharedMigrationLedger = Effect.fn("realignSharedMigrationLedger")(function* () { + const sql = yield* SqlClient.SqlClient; + + const tables = yield* sql<{ readonly name: string }>` + SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'effect_sql_migrations' + `; + if (tables.length === 0) { + return; + } + + const legacyRows = yield* sql<{ readonly migration_id: number; readonly name: string }>` + SELECT migration_id, name FROM effect_sql_migrations + WHERE migration_id = 33 AND name = 'ProjectionThreadParent' + `; + if (legacyRows.length === 0) { + return; + } + + yield* sql.withTransaction( + Effect.gen(function* () { + yield* sql` + DELETE FROM effect_sql_migrations WHERE migration_id = 33 AND name = 'ProjectionThreadParent' + `; + yield* sql`UPDATE effect_sql_migrations SET migration_id = 33 WHERE migration_id = 34`; + yield* sql`UPDATE effect_sql_migrations SET migration_id = 34 WHERE migration_id = 35`; + yield* sql`UPDATE effect_sql_migrations SET migration_id = 35 WHERE migration_id = 36`; + }), + ); + + yield* Effect.log("Realigned shared migration ledger to upstream numbering"); +}); + +/** + * Layer that runs fork migrations when the layer is built. + */ +export const ForkMigrationsLive = Layer.effectDiscard(runForkMigrations()); diff --git a/apps/server/src/persistence/Layers/Sqlite.ts b/apps/server/src/persistence/Layers/Sqlite.ts index dfd338b7159..e73005a913f 100644 --- a/apps/server/src/persistence/Layers/Sqlite.ts +++ b/apps/server/src/persistence/Layers/Sqlite.ts @@ -6,6 +6,7 @@ import * as SqlClient from "effect/unstable/sql/SqlClient"; import type { SqlError } from "effect/unstable/sql/SqlError"; import { runMigrations } from "../Migrations.ts"; +import { realignSharedMigrationLedger, runForkMigrations } from "../ForkMigrations.ts"; import { ServerConfig } from "../../config.ts"; type RuntimeSqliteLayerConfig = { @@ -35,7 +36,9 @@ const setup = Layer.effectDiscard( const sql = yield* SqlClient.SqlClient; yield* sql`PRAGMA journal_mode = WAL;`; yield* sql`PRAGMA foreign_keys = ON;`; + yield* realignSharedMigrationLedger(); yield* runMigrations(); + yield* runForkMigrations(); }), ); diff --git a/apps/server/src/persistence/Migrations.test.ts b/apps/server/src/persistence/Migrations.test.ts deleted file mode 100644 index 8ce7ff4210d..00000000000 --- a/apps/server/src/persistence/Migrations.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, expect, it } from "vite-plus/test"; - -import { migrationEntries } from "./Migrations.ts"; - -const RESERVED_FORK_ID_START = 1000; -const GRANDFATHERED_FORK_IDS = new Set([33]); - -// Register a migration's name here the moment it is authored for a -// TrogonStack-only feature, so this test can catch it landing outside the -// reserved block instead of that surfacing as a collision with upstream. -const FORK_MIGRATION_NAMES = new Set(["ProjectionThreadParent"]); - -describe("migrationEntries", () => { - it("never assigns the same id twice", () => { - const ids = migrationEntries.map(([id]) => id); - expect(new Set(ids).size).toBe(ids.length); - }); - - it("lists entries in ascending id order", () => { - const ids = migrationEntries.map(([id]) => id); - expect(ids).toEqual([...ids].sort((a, b) => a - b)); - }); - - it("keeps fork-only migrations out of the shared low-id range", () => { - for (const [id, name] of migrationEntries) { - if (!FORK_MIGRATION_NAMES.has(name)) { - continue; - } - expect(id >= RESERVED_FORK_ID_START || GRANDFATHERED_FORK_IDS.has(id)).toBe(true); - } - }); -}); diff --git a/apps/server/src/persistence/Migrations.ts b/apps/server/src/persistence/Migrations.ts index 9afd6c38c0d..95cb6b17f84 100644 --- a/apps/server/src/persistence/Migrations.ts +++ b/apps/server/src/persistence/Migrations.ts @@ -45,10 +45,9 @@ import Migration0029 from "./Migrations/029_ProjectionThreadDetailOrderingIndexe import Migration0030 from "./Migrations/030_ProjectionThreadShellArchiveIndexes.ts"; import Migration0031 from "./Migrations/031_AuthAuthorizationScopes.ts"; import Migration0032 from "./Migrations/032_AuthPairingProofKeyThumbprint.ts"; -import Migration0033 from "./Migrations/fork/033_ProjectionThreadParent.ts"; -import Migration0034 from "./Migrations/034_ProjectionThreadsSettled.ts"; -import Migration0035 from "./Migrations/035_ProjectionThreadsSnoozed.ts"; -import Migration0036 from "./Migrations/036_ProjectionThreadTitleRegeneration.ts"; +import Migration0033 from "./Migrations/033_ProjectionThreadsSettled.ts"; +import Migration0034 from "./Migrations/034_ProjectionThreadsSnoozed.ts"; +import Migration0035 from "./Migrations/035_ProjectionThreadTitleRegeneration.ts"; /** * Migration loader with all migrations defined inline. @@ -59,16 +58,6 @@ import Migration0036 from "./Migrations/036_ProjectionThreadTitleRegeneration.ts * * Uses Migrator.fromRecord which parses the key format and * returns migrations sorted by ID. - * - * The migrator only remembers the highest id that has run, not which ids - * ran, so an id already shipped here must never be reassigned - even when it - * collides with an id upstream (pingdotgg/t3code) adds independently. Slot - * the incoming migration after whatever is already used instead. - * - * New TrogonStack-only migrations should use ids >= 1000, a block upstream - * will never grow into, so future syncs can't collide with them at all, and - * live under `./Migrations/fork/` so they read as fork-only on sight. `33` - * (ProjectionThreadParent) predates this convention and is grandfathered. */ export const migrationEntries = [ [1, "OrchestrationEvents", Migration0001], @@ -103,10 +92,9 @@ export const migrationEntries = [ [30, "ProjectionThreadShellArchiveIndexes", Migration0030], [31, "AuthAuthorizationScopes", Migration0031], [32, "AuthPairingProofKeyThumbprint", Migration0032], - [33, "ProjectionThreadParent", Migration0033], - [34, "ProjectionThreadsSettled", Migration0034], - [35, "ProjectionThreadsSnoozed", Migration0035], - [36, "ProjectionThreadTitleRegeneration", Migration0036], + [33, "ProjectionThreadsSettled", Migration0033], + [34, "ProjectionThreadsSnoozed", Migration0034], + [35, "ProjectionThreadTitleRegeneration", Migration0035], ] as const; export const makeMigrationLoader = (throughId?: number) => diff --git a/apps/server/src/persistence/Migrations/034_ProjectionThreadsSettled.ts b/apps/server/src/persistence/Migrations/033_ProjectionThreadsSettled.ts similarity index 100% rename from apps/server/src/persistence/Migrations/034_ProjectionThreadsSettled.ts rename to apps/server/src/persistence/Migrations/033_ProjectionThreadsSettled.ts diff --git a/apps/server/src/persistence/Migrations/035_ProjectionThreadsSnoozed.ts b/apps/server/src/persistence/Migrations/034_ProjectionThreadsSnoozed.ts similarity index 100% rename from apps/server/src/persistence/Migrations/035_ProjectionThreadsSnoozed.ts rename to apps/server/src/persistence/Migrations/034_ProjectionThreadsSnoozed.ts diff --git a/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegeneration.test.ts b/apps/server/src/persistence/Migrations/035_ProjectionThreadTitleRegeneration.test.ts similarity index 88% rename from apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegeneration.test.ts rename to apps/server/src/persistence/Migrations/035_ProjectionThreadTitleRegeneration.test.ts index 53f755d3127..755591201de 100644 --- a/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegeneration.test.ts +++ b/apps/server/src/persistence/Migrations/035_ProjectionThreadTitleRegeneration.test.ts @@ -8,13 +8,13 @@ import * as NodeSqliteClient from "../NodeSqliteClient.ts"; const layer = it.layer(Layer.mergeAll(NodeSqliteClient.layerMemory())); -layer("036_ProjectionThreadTitleRegeneration", (it) => { +layer("035_ProjectionThreadTitleRegeneration", (it) => { it.effect("adds pending title regeneration columns", () => Effect.gen(function* () { const sql = yield* SqlClient.SqlClient; + yield* runMigrations({ toMigrationInclusive: 34 }); yield* runMigrations({ toMigrationInclusive: 35 }); - yield* runMigrations({ toMigrationInclusive: 36 }); const columns = yield* sql<{ readonly name: string }>` PRAGMA table_info(projection_threads) diff --git a/apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegeneration.ts b/apps/server/src/persistence/Migrations/035_ProjectionThreadTitleRegeneration.ts similarity index 100% rename from apps/server/src/persistence/Migrations/036_ProjectionThreadTitleRegeneration.ts rename to apps/server/src/persistence/Migrations/035_ProjectionThreadTitleRegeneration.ts diff --git a/apps/server/src/persistence/Migrations/fork/033_ProjectionThreadParent.ts b/apps/server/src/persistence/Migrations/fork/001_ProjectionThreadParent.ts similarity index 53% rename from apps/server/src/persistence/Migrations/fork/033_ProjectionThreadParent.ts rename to apps/server/src/persistence/Migrations/fork/001_ProjectionThreadParent.ts index 439f0d5d534..61328cd58af 100644 --- a/apps/server/src/persistence/Migrations/fork/033_ProjectionThreadParent.ts +++ b/apps/server/src/persistence/Migrations/fork/001_ProjectionThreadParent.ts @@ -3,12 +3,17 @@ import * as SqlClient from "effect/unstable/sql/SqlClient"; export default Effect.gen(function* () { const sql = yield* SqlClient.SqlClient; - - yield* sql` - ALTER TABLE projection_threads - ADD COLUMN parent_thread_id TEXT + const columns = yield* sql<{ readonly name: string }>` + PRAGMA table_info(projection_threads) `; + if (!columns.some((column) => column.name === "parent_thread_id")) { + yield* sql` + ALTER TABLE projection_threads + ADD COLUMN parent_thread_id TEXT + `; + } + yield* sql` CREATE INDEX IF NOT EXISTS idx_projection_threads_parent ON projection_threads(parent_thread_id)