From b5c76af700132d3b968a4e453c707bf523adc419 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Mon, 13 Jul 2026 23:42:16 -0700 Subject: [PATCH] fix(core): preserve jsonb defaults during PostgreSQL migration (#2080) ## Summary - preserve target defaults when legacy SQLite rows contain `NULL` or empty strings for `NOT NULL` jsonb columns - derive the fallback from PostgreSQL column metadata instead of hard-coding table or column names - keep migration checksum conversion aligned with inserted values - add regression coverage for legacy null JSON fields ## Test plan - `corepack pnpm@10.33.0 --filter @fusion/core typecheck` - `FUSION_PG_TEST_SKIP=1 corepack pnpm@10.33.0 --filter @fusion/core exec vitest run src/__tests__/postgres/sqlite-migrator.test.ts` - `corepack pnpm@10.33.0 --filter @fusion/core build` The PostgreSQL-backed integration suite requires `psql`, which is unavailable in this environment; CI should exercise the added migration case against PostgreSQL. ## Summary by CodeRabbit * **Bug Fixes** * Improved SQLite-to-PostgreSQL migration for legacy rows containing `NULL` or empty JSON values. * For eligible `NOT NULL` `jsonb` columns, the migrator now preserves/apply compatible PostgreSQL column defaults instead of writing SQL `NULL`. * Migration verification now aligns with the final values inserted into PostgreSQL to prevent checksum mismatches. * **Tests** * Added an end-to-end legacy migration case to confirm `jsonb` fields materialize as empty defaults (e.g., `[]`) rather than staying `NULL`. --- .changeset/pg-migrator-jsonb-null-defaults.md | 7 +++ .../postgres/sqlite-migrator.test.ts | 41 ++++++++++++++++ packages/core/src/postgres/sqlite-migrator.ts | 48 +++++++++++++++---- 3 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 .changeset/pg-migrator-jsonb-null-defaults.md diff --git a/.changeset/pg-migrator-jsonb-null-defaults.md b/.changeset/pg-migrator-jsonb-null-defaults.md new file mode 100644 index 0000000000..29103349c2 --- /dev/null +++ b/.changeset/pg-migrator-jsonb-null-defaults.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Preserve PostgreSQL jsonb defaults when legacy SQLite rows contain NULL. +category: fix +dev: The SQLite-to-PostgreSQL migrator now reads target nullability and jsonb defaults, replacing legacy NULL or empty-string values only for NOT NULL jsonb columns with valid defaults. This prevents first-boot migration failures such as research_runs.sources violating its NOT NULL constraint while keeping checksum verification aligned with the migrated values. diff --git a/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts b/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts index a419fcf40e..ba1c621ca4 100644 --- a/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts +++ b/packages/core/src/__tests__/postgres/sqlite-migrator.test.ts @@ -129,6 +129,20 @@ CREATE TABLE IF NOT EXISTS activityLog ( ); `; +/** Legacy research rows allowed NULL before PostgreSQL made these JSON fields required. */ +const RESEARCH_RUNS_SQLITE_DDL = ` +CREATE TABLE IF NOT EXISTS researchRuns ( + id TEXT PRIMARY KEY, + query TEXT NOT NULL, + status TEXT NOT NULL, + sources TEXT, + events TEXT, + tags TEXT, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL +); +`; + /** * A minimal agents table so agent_heartbeats has a parent row to satisfy the * FK constraint that is re-enabled after the migration completes. Includes @@ -162,6 +176,7 @@ function buildPopulatedSqliteProject(fusionDir: string): void { db.exec(CONFIG_SQLITE_DDL); db.exec(AGENTS_SQLITE_DDL); db.exec(ACTIVITY_LOG_SQLITE_DDL); + db.exec(RESEARCH_RUNS_SQLITE_DDL); // Legacy camelCase table rows — must land in project.activity_log. const insertActivity = db.prepare( @@ -170,6 +185,20 @@ function buildPopulatedSqliteProject(fusionDir: string): void { insertActivity.run("act-1", "2026-06-01T00:00:00Z", "task:created", "FN-100", "First task", "created", JSON.stringify({ source: "test" })); insertActivity.run("act-2", "2026-06-01T01:00:00Z", "task:moved", "FN-100", "First task", "todo -> in-progress", null); + db.prepare( + `INSERT INTO researchRuns (id, query, status, sources, events, tags, createdAt, updatedAt) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + ).run( + "RR-legacy-null-json", + "legacy research", + "failed", + null, + "", + null, + "2026-06-01T00:00:00Z", + "2026-06-01T00:01:00Z", + ); + // Insert agents so agent_heartbeats FK is satisfiable post-migration. const insertAgent = db.prepare(`INSERT INTO agents (id, name, role, state, createdAt, updatedAt) VALUES (?, ?, ?, ?, ?, ?)`); insertAgent.run("agent-1", "Agent One", "coder", "idle", "2026-06-01T00:00:00Z", "2026-06-01T00:00:00Z"); @@ -482,6 +511,18 @@ pgDescribe("SQLite-to-PostgreSQL migrator", () => { expect(colInfo[0].data_type).toBe("jsonb"); }); + it("materializes defaults for legacy NULL values targeting required jsonb columns", async () => { + await migrateSqliteToPostgres(ctx!.db, [ + { sqlitePath: join(ctx!.fusionDir, "fusion.db"), pgSchema: "project" as const }, + ]); + + const rows = (await ctx!.db.execute(sql` + SELECT sources, events, tags + FROM project.research_runs WHERE id = 'RR-legacy-null-json' + `)) as unknown as Array<{ sources: unknown; events: unknown; tags: unknown }>; + expect(rows[0]).toEqual({ sources: [], events: [], tags: [] }); + }); + // VAL-MIGRATE-003 — bytea fidelity it("round-trips bytea columns (BLOB → bytea) byte-identical", async () => { await migrateSqliteToPostgres(ctx!.db, [ diff --git a/packages/core/src/postgres/sqlite-migrator.ts b/packages/core/src/postgres/sqlite-migrator.ts index 99b1da7b6d..47ff2a318c 100644 --- a/packages/core/src/postgres/sqlite-migrator.ts +++ b/packages/core/src/postgres/sqlite-migrator.ts @@ -114,6 +114,8 @@ interface ColumnMapping { readonly pgName: string; /** The resolved type for value conversion. */ readonly type: ColumnType; + /** JSON text to use when a legacy NULL targets a NOT NULL jsonb default. */ + readonly nullJsonbFallback?: string; } /** A table to migrate. */ @@ -401,6 +403,8 @@ async function resolveColumnMapping( SELECT c.column_name, c.data_type, + c.is_nullable, + c.column_default, a.attidentity, CASE WHEN a.attgenerated <> '' THEN 1 ELSE 0 END AS is_generated FROM information_schema.columns c @@ -413,7 +417,14 @@ async function resolveColumnMapping( AND n.nspname = c.table_schema AND cls.relname = c.table_name AND a.attnum > 0 - `)) as unknown as Array<{ column_name: string; data_type: string; attidentity: string | null; is_generated: number | string }>; + `)) as unknown as Array<{ + column_name: string; + data_type: string; + is_nullable: string; + column_default: string | null; + attidentity: string | null; + is_generated: number | string; + }>; if (pgCols.length === 0) { // No PostgreSQL table with this name — skip. @@ -439,7 +450,24 @@ async function resolveColumnMapping( continue; } const type = classifyColumnType(pgCol); - mapping.push({ sqliteName: sc.name, pgName, type }); + let nullJsonbFallback: string | undefined; + if (type === "jsonb" && pgCol.is_nullable === "NO" && pgCol.column_default) { + // FNXC:PostgresMigration 2026-07-14-05:30: + // Legacy SQLite rows can contain NULL/empty JSON even when the target is + // NOT NULL with a default. Materialize that default during conversion so + // one stale row cannot abort the entire first-boot migration. + const match = /^'(.*)'::jsonb?$/s.exec(pgCol.column_default); + if (match) { + const candidate = match[1].replace(/''/g, "'"); + try { + JSON.parse(candidate); + nullJsonbFallback = candidate; + } catch { + // Leave malformed defaults to PostgreSQL rather than inventing data. + } + } + } + mapping.push({ sqliteName: sc.name, pgName, type, nullJsonbFallback }); } return mapping; @@ -479,9 +507,9 @@ function classifyColumnType(pgCol: { * postgres.js's raw `sql` template does NOT auto-serialize JS objects for * jsonb columns (it tries to send the object as a byte string and fails), so * jsonb values MUST be passed as strings with an explicit `::jsonb` cast. - * NULL stays NULL (emitted as SQL NULL, not the string "null"). An empty - * string is treated as NULL because some legacy rows stored '' where the new - * schema expects NULL jsonb. + * NULL stays NULL unless the target is NOT NULL with a valid jsonb default; + * in that case legacy NULL/empty-string values materialize the target default + * so the migration does not violate the target constraint. * - bytea: SQLite stores BLOB. We wrap it in a Buffer (postgres.js handles * Buffer natively for bytea). NULL stays NULL. * - plain: passed through verbatim. @@ -489,9 +517,9 @@ function classifyColumnType(pgCol: { * Identity and generated columns are omitted at the insert-builder level * (never passed here). */ -function convertValue(value: unknown, type: ColumnType): unknown { +function convertValue(value: unknown, type: ColumnType, nullJsonbFallback?: string): unknown { if (value === null || value === undefined) { - return null; + return type === "jsonb" && nullJsonbFallback !== undefined ? nullJsonbFallback : null; } switch (type) { case "jsonb": { @@ -502,7 +530,7 @@ function convertValue(value: unknown, type: ColumnType): unknown { if (typeof value === "string") { const trimmed = value.trim(); if (trimmed === "") { - return null; + return nullJsonbFallback ?? null; } try { return JSON.stringify(JSON.parse(trimmed)); @@ -615,7 +643,7 @@ async function migrateTable( for (const row of stmt.all() as Array>) { const converted: Record = {}; for (const col of insertableCols) { - converted[col.pgName] = convertValue(row[col.sqliteName], col.type); + converted[col.pgName] = convertValue(row[col.sqliteName], col.type, col.nullJsonbFallback); } batch.push(converted); if (batch.length >= INSERT_BATCH_SIZE) { @@ -923,7 +951,7 @@ function computeSourceContentChecksum( const hash = createHash("md5"); for (const row of rows) { for (const col of cols) { - const converted = convertValue(row[col.sqliteName], col.type); + const converted = convertValue(row[col.sqliteName], col.type, col.nullJsonbFallback); hash.update(canonicalizeCell(converted)); hash.update("\u0001"); // cell separator }