From 8eda2d5cc0ee3c545c1bf02806b5b0f2cdbb667d Mon Sep 17 00:00:00 2001 From: Abhishek B R Date: Thu, 24 Sep 2026 03:45:08 +0530 Subject: [PATCH] Clear empty strings left in nullable JSON columns at boot Closes #2092 --- .changeset/legacy-empty-json-columns.md | 6 + apps/host-selfhost/src/db/data-migrations.ts | 5 + .../src/db/legacy-empty-json-boot.test.ts | 89 ++++++++++ apps/local/src/db/data-migrations.ts | 5 + packages/core/sdk/src/index.ts | 8 + .../src/sqlite-empty-json-migration.test.ts | 163 ++++++++++++++++++ .../sdk/src/sqlite-empty-json-migration.ts | 150 ++++++++++++++++ 7 files changed, 426 insertions(+) create mode 100644 .changeset/legacy-empty-json-columns.md create mode 100644 apps/host-selfhost/src/db/legacy-empty-json-boot.test.ts create mode 100644 packages/core/sdk/src/sqlite-empty-json-migration.test.ts create mode 100644 packages/core/sdk/src/sqlite-empty-json-migration.ts diff --git a/.changeset/legacy-empty-json-columns.md b/.changeset/legacy-empty-json-columns.md new file mode 100644 index 000000000..8652aa5ae --- /dev/null +++ b/.changeset/legacy-empty-json-columns.md @@ -0,0 +1,6 @@ +--- +"@executor-js/host-selfhost": patch +"executor": patch +--- + +Repair connections that an older build saved with an empty string instead of NULL in a JSON column. Reading one back threw while mapping the row, so every toolkit MCP endpoint failed `initialize` with an internal error. A boot-time migration now sets those empty strings to NULL in every nullable JSON column the schema declares. It runs once, and rows written by a current build are left exactly as they are. diff --git a/apps/host-selfhost/src/db/data-migrations.ts b/apps/host-selfhost/src/db/data-migrations.ts index aaf3b2295..e1a165cdc 100644 --- a/apps/host-selfhost/src/db/data-migrations.ts +++ b/apps/host-selfhost/src/db/data-migrations.ts @@ -7,6 +7,7 @@ import { bigintStorageClassSqliteMigration, + emptyJsonSqliteMigration, sqliteDataMigration, type SqliteDataMigration, } from "@executor-js/sdk"; @@ -29,6 +30,10 @@ export const selfHostDataMigrations: readonly SqliteDataMigration[] = [ // class cannot be read by the bigint row mapper, so a single legacy // `connection.expires_at` failed every catalog read (issue #1771). bigintStorageClassSqliteMigration, + // Same reason: an older build left `''` in nullable `json` columns, which + // the json row mapper cannot parse, so one legacy `connection.credential_write` + // failed every toolkit MCP session (issue #2092). + emptyJsonSqliteMigration, // Rewrite pre-canonical integration auth configs into the shared // placements model. sqliteDataMigration("2026-06-05-auth-config-placements", (client) => diff --git a/apps/host-selfhost/src/db/legacy-empty-json-boot.test.ts b/apps/host-selfhost/src/db/legacy-empty-json-boot.test.ts new file mode 100644 index 000000000..7b90e4914 --- /dev/null +++ b/apps/host-selfhost/src/db/legacy-empty-json-boot.test.ts @@ -0,0 +1,89 @@ +// --------------------------------------------------------------------------- +// Boot-level proof for issue #2092: a self-host database holding `''` in a +// nullable `json` column of `connection` cannot list connections, and the +// self-host boot sequence heals it. +// +// The migration body is unit-tested in the SDK. What this pins is the WIRING: +// that `selfHostDataMigrations` carries the entry, so the `connections.list` +// every toolkit MCP session runs on `initialize` sees repaired rows. +// --------------------------------------------------------------------------- + +import { afterEach, beforeEach, describe, expect, it } from "@effect/vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Effect } from "effect"; +import { withQueryContext } from "@executor-js/fumadb/query"; + +import { runSqliteDataMigrations } from "@executor-js/sdk"; + +import { selfHostDataMigrations } from "./data-migrations"; +import { createSelfHostDb } from "./self-host-db"; + +const TENANT = "executor-workspace-2092"; +const SUBJECT = "user_a"; + +let workDir: string; + +beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "executor-legacy-empty-json-")); +}); + +afterEach(() => { + rmSync(workDir, { recursive: true, force: true }); +}); + +/** Write the row an older build left behind: `''` rather than NULL in the + * nullable `json` columns of `connection`. */ +const seedLegacyConnection = async (dbPath: string): Promise => { + const sqlite = await createSelfHostDb({ path: dbPath }); + await sqlite.client.execute({ + sql: `INSERT INTO connection + (row_id, tenant, owner, subject, integration, name, template, provider, item_ids, + credential_write, last_health, provider_state, created_at, updated_at) + VALUES ('c1', ?, 'user', ?, 'acme', 'default', 'oauth2', 'file', ?, '', '', '', ?, ?)`, + args: [ + TENANT, + SUBJECT, + JSON.stringify({ token: "item_1" }), + Math.floor(Date.now() / 1000), + Math.floor(Date.now() / 1000), + ], + }); + await sqlite.close(); +}; + +describe("self-host boot over legacy empty json columns", () => { + it("cannot read the connection table before the migrations run", async () => { + const dbPath = join(workDir, "data.db"); + await seedLegacyConnection(dbPath); + + const sqlite = await createSelfHostDb({ path: dbPath }); + const scoped = withQueryContext(sqlite.db, { tenant: TENANT, subject: SUBJECT }); + await expect(scoped.findMany("connection", {})).rejects.toThrow(/JSON/); + await sqlite.close(); + }); + + it("heals it through the self-host data-migration registry", async () => { + const dbPath = join(workDir, "data.db"); + await seedLegacyConnection(dbPath); + + const sqlite = await createSelfHostDb({ path: dbPath }); + const applied = await Effect.runPromise( + runSqliteDataMigrations(sqlite.client, selfHostDataMigrations), + ); + expect(applied).toContain("2026-09-24-empty-json-columns"); + + const scoped = withQueryContext(sqlite.db, { tenant: TENANT, subject: SUBJECT }); + const rows = await scoped.findMany("connection", {}); + expect( + rows.map((row) => [ + row.name, + row.credential_write ?? null, + row.last_health ?? null, + row.provider_state ?? null, + ]), + ).toEqual([["default", null, null, null]]); + await sqlite.close(); + }); +}); diff --git a/apps/local/src/db/data-migrations.ts b/apps/local/src/db/data-migrations.ts index 0534b26f4..411925742 100644 --- a/apps/local/src/db/data-migrations.ts +++ b/apps/local/src/db/data-migrations.ts @@ -8,6 +8,7 @@ import { Effect, bigintStorageClassSqliteMigration, + emptyJsonSqliteMigration, oauthClientGcSqliteMigration, sqliteDataMigration, type SqliteDataMigration, @@ -36,6 +37,10 @@ export const localDataMigrations: readonly SqliteDataMigration[] = [ // class cannot be read by the bigint row mapper, so a single legacy // `connection.expires_at` failed every catalog read (issue #1771). bigintStorageClassSqliteMigration, + // Same reason: an older build left `''` in nullable `json` columns, which + // the json row mapper cannot parse, so one legacy `connection.credential_write` + // failed every toolkit MCP session (issue #2092). + emptyJsonSqliteMigration, // Rewrite pre-canonical integration auth configs (incl. v1→v2 outputs) // into the shared placements model. sqliteDataMigration("2026-06-05-auth-config-placements", (client) => diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index 627d3de1d..bbe222347 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -518,6 +518,14 @@ export { LEGACY_BIGINT_STORAGE_CLASS_COLUMNS, type BigintStorageClassColumn, } from "./sqlite-bigint-storage-class-migration"; +// Rewrite `''` left in nullable `json` columns by pre-FumaDB builds, which the +// json row mapper cannot parse (issue #2092). +export { + emptyJsonSqliteMigration, + runSqliteEmptyJsonMigration, + NULLABLE_JSON_COLUMNS, + type EmptyJsonColumn, +} from "./sqlite-empty-json-migration"; export { authToolFailure, isUnauthorizedToolFailure, diff --git a/packages/core/sdk/src/sqlite-empty-json-migration.test.ts b/packages/core/sdk/src/sqlite-empty-json-migration.test.ts new file mode 100644 index 000000000..c7ddcff84 --- /dev/null +++ b/packages/core/sdk/src/sqlite-empty-json-migration.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; +import { withQueryContext } from "@executor-js/fumadb/query"; + +import { collectTables } from "./executor"; +import { createSqliteTestFumaDb, type SqliteTestFumaDb } from "./sqlite-test-db"; +import { + NULLABLE_JSON_COLUMNS, + emptyJsonSqliteMigration, + runSqliteEmptyJsonMigration, +} from "./sqlite-empty-json-migration"; + +// A `json` column is stored on SQLite as TEXT and read back with `JSON.parse`. +// Databases carried forward from before the FumaDB cutover can hold `''` in +// nullable `json` columns, and `JSON.parse('')` throws on the ROW mapper, so +// one such row takes down the whole `findMany`. +// +// That is issue #2092: `connection.credential_write` held `''`, so +// `connections.list` threw and every toolkit MCP endpoint failed `initialize`. + +const TENANT = "t1"; +const SUBJECT = "user_a"; +const CREDENTIAL_WRITE = { runtimeId: "runtime_1", attemptId: "attempt_1" }; + +const withDb = (body: (db: SqliteTestFumaDb) => Promise): Promise => + Effect.runPromise( + Effect.acquireUseRelease( + Effect.promise(() => createSqliteTestFumaDb({ tables: collectTables() })), + (db) => Effect.promise(() => body(db)), + (db) => Effect.promise(() => db.close()), + ), + ); + +const seconds = (ms: number) => Math.floor(ms / 1000); + +/** Insert a connection row with `credential_write` and `last_health` bound as + * raw values, so the test controls exactly what the legacy build left behind. */ +const insertConnection = ( + db: SqliteTestFumaDb, + row: { + readonly rowId: string; + readonly name: string; + readonly credentialWrite: string | null; + readonly lastHealth?: string | null; + }, +): Promise => + db.client.execute({ + sql: `INSERT INTO connection + (row_id, tenant, owner, subject, integration, name, template, provider, item_ids, + credential_write, last_health, created_at, updated_at) + VALUES (?, ?, 'user', ?, 'acme', ?, 'oauth2', 'file', ?, ?, ?, ?, ?)`, + args: [ + row.rowId, + TENANT, + SUBJECT, + row.name, + JSON.stringify({ token: "item_1" }), + row.credentialWrite, + row.lastHealth ?? null, + seconds(Date.now()), + seconds(Date.now()), + ], + }); + +describe("legacy empty json column migration", () => { + it.effect("reproduces the connection read failure on a legacy empty string", () => + Effect.promise(() => + withDb(async (db) => { + await insertConnection(db, { rowId: "c_legacy", name: "legacy", credentialWrite: "" }); + + const scoped = withQueryContext(db.db, { tenant: TENANT, subject: SUBJECT }); + await expect(scoped.findMany("connection", {})).rejects.toThrow(/JSON/); + }), + ), + ); + + it.effect("rewrites empty strings to NULL so connections read again", () => + Effect.promise(() => + withDb(async (db) => { + await insertConnection(db, { + rowId: "c_legacy", + name: "legacy", + credentialWrite: "", + lastHealth: "", + }); + await insertConnection(db, { + rowId: "c_healthy", + name: "healthy", + credentialWrite: JSON.stringify(CREDENTIAL_WRITE), + }); + await insertConnection(db, { rowId: "c_null", name: "null", credentialWrite: null }); + + const rewritten = await Effect.runPromise(runSqliteEmptyJsonMigration(db.client)); + expect(rewritten).toBe(2); + + const scoped = withQueryContext(db.db, { tenant: TENANT, subject: SUBJECT }); + const rows = await scoped.findMany("connection", {}); + expect( + rows.map((row) => [row.name, row.credential_write ?? null, row.last_health ?? null]), + ).toEqual([ + ["healthy", CREDENTIAL_WRITE, null], + ["legacy", null, null], + ["null", null, null], + ]); + }), + ), + ); + + it.effect("is idempotent", () => + Effect.promise(() => + withDb(async (db) => { + await insertConnection(db, { rowId: "c_legacy", name: "legacy", credentialWrite: "" }); + + expect(await Effect.runPromise(runSqliteEmptyJsonMigration(db.client))).toBe(1); + expect(await Effect.runPromise(runSqliteEmptyJsonMigration(db.client))).toBe(0); + + const scoped = withQueryContext(db.db, { tenant: TENANT, subject: SUBJECT }); + const rows = await scoped.findMany("connection", {}); + expect(rows.map((row) => row.credential_write ?? null)).toEqual([null]); + }), + ), + ); + + it.effect("leaves NOT NULL json columns alone", () => + Effect.promise(() => + withDb(async (db) => { + await insertConnection(db, { rowId: "c1", name: "c1", credentialWrite: null }); + await db.client.execute(`UPDATE connection SET item_ids = '' WHERE row_id = 'c1'`); + + expect(await Effect.runPromise(runSqliteEmptyJsonMigration(db.client))).toBe(0); + + const result = await db.client.execute( + "SELECT item_ids FROM connection WHERE row_id = 'c1'", + ); + expect(result.rows[0]?.["item_ids"]).toBe(""); + }), + ), + ); + + it("covers every nullable json column the core schema declares", () => { + const tables = collectTables() as Record }>; + const declared: string[] = []; + for (const [tableName, table] of Object.entries(tables)) { + for (const [columnName, column] of Object.entries(table.columns)) { + const { type, isNullable } = column as { + readonly type?: string; + readonly isNullable?: boolean; + }; + if (type === "json" && isNullable) declared.push(`${tableName}.${columnName}`); + } + } + const covered = NULLABLE_JSON_COLUMNS.map((entry) => `${entry.table}.${entry.column}`); + expect(covered).toContain("connection.credential_write"); + expect(covered).toContain("connection.last_health"); + expect(covered).toContain("connection.provider_state"); + expect(covered).not.toContain("connection.item_ids"); + expect(covered.slice().sort()).toEqual(declared.sort()); + }); + + it("is registered under a stable, date-prefixed name", () => { + expect(emptyJsonSqliteMigration.name).toBe("2026-09-24-empty-json-columns"); + }); +}); diff --git a/packages/core/sdk/src/sqlite-empty-json-migration.ts b/packages/core/sdk/src/sqlite-empty-json-migration.ts new file mode 100644 index 000000000..6501f4e09 --- /dev/null +++ b/packages/core/sdk/src/sqlite-empty-json-migration.ts @@ -0,0 +1,150 @@ +// --------------------------------------------------------------------------- +// libSQL boot migration: rewrite empty strings in nullable `json` columns to +// NULL (issue #2092). +// +// A `json` column is stored on SQLite as TEXT, and its row mapper reads the +// value back with `JSON.parse`. Databases carried forward from builds before +// the FumaDB cutover can hold `''` where the current schema expects NULL (seen +// in `connection.credential_write`). `JSON.parse('')` throws, and because the +// throw is in the ROW mapper it fails the whole `findMany`: one such row made +// `connections.list` throw, so every `/mcp/toolkits/` session failed on +// `initialize`. +// +// The rewrite is deliberately narrow. It touches only the columns the schema +// declares `json` AND nullable, and within them only values that are exactly +// the empty string. NULL is what the ORM writes for "no value" in these +// columns, so the repaired rows read back the way a current build would have +// written them. NOT NULL `json` columns are left alone: an empty string there +// has no faithful replacement. Idempotent: after a run no nullable `json` +// column holds `''`, so a second run updates nothing. +// --------------------------------------------------------------------------- + +import { Effect } from "effect"; + +import { coreSchema } from "./core-schema"; +import { + DataMigrationError, + type SqliteDataMigration, + type SqliteDataMigrationClient, +} from "./sqlite-data-migrations"; + +const MIGRATION_NAME = "2026-09-24-empty-json-columns"; + +export interface EmptyJsonColumn { + /** SQL table name. */ + readonly table: string; + /** SQL column name. */ + readonly column: string; +} + +/** + * Every nullable `json` column in the core schema, by SQL name. + * + * Derived from the schema rather than hand-listed so a column added later can + * never be silently missed. + */ +export const NULLABLE_JSON_COLUMNS: readonly EmptyJsonColumn[] = Object.values(coreSchema).flatMap( + (table) => + Object.values(table.columns) + .filter((column) => column.type === "json" && column.isNullable) + .map((column) => ({ table: table.names.sql, column: column.names.sql })), +); + +const execute = ( + client: SqliteDataMigrationClient, + stmt: string | { readonly sql: string; readonly args: readonly unknown[] }, +) => + Effect.tryPromise({ + try: () => client.execute(stmt), + catch: (cause) => new DataMigrationError({ migration: MIGRATION_NAME, cause }), + }); + +/** SQLite identifiers are quoted, not parameterized. Every name here comes from + * the compiled-in schema, so this always matches; anything else is refused + * rather than interpolated. */ +const quoteIdentifier = (name: string): Effect.Effect => + /^[A-Za-z_][A-Za-z0-9_]*$/.test(name) + ? Effect.succeed(`"${name}"`) + : Effect.fail( + new DataMigrationError({ + migration: MIGRATION_NAME, + cause: `Refusing to interpolate SQL identifier: ${name}`, + }), + ); + +const hasColumn = ( + client: SqliteDataMigrationClient, + table: string, + quotedTable: string, + column: string, +): Effect.Effect => + execute(client, { + sql: "SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?", + args: [table], + }).pipe( + Effect.flatMap((tables) => + tables.rows.length === 0 + ? Effect.succeed(false) + : execute(client, `PRAGMA table_info(${quotedTable})`).pipe( + Effect.map((info) => info.rows.some((row) => row["name"] === column)), + ), + ), + ); + +/** + * Set empty-string values in the schema's nullable `json` columns to NULL. + * + * Returns the number of values rewritten. Wrapped in BEGIN…COMMIT so a mid-run + * failure leaves the database untouched and the (unstamped) migration re-runs + * cleanly on the next boot. + */ +export const runSqliteEmptyJsonMigration = ( + client: SqliteDataMigrationClient, +): Effect.Effect => + Effect.gen(function* () { + const pending: { readonly sql: string; readonly count: number }[] = []; + + for (const target of NULLABLE_JSON_COLUMNS) { + const table = yield* quoteIdentifier(target.table); + const column = yield* quoteIdentifier(target.column); + if (!(yield* hasColumn(client, target.table, table, target.column))) continue; + + const predicate = `${column} = ''`; + + const counted = yield* execute( + client, + `SELECT COUNT(*) AS n FROM ${table} WHERE ${predicate}`, + ); + const count = Number(counted.rows[0]?.["n"] ?? 0); + if (count === 0) continue; + + pending.push({ + sql: `UPDATE ${table} SET ${column} = NULL WHERE ${predicate}`, + count, + }); + } + + if (pending.length === 0) return 0; + + const applyAll = Effect.gen(function* () { + let rewritten = 0; + for (const statement of pending) { + yield* execute(client, statement.sql); + rewritten += statement.count; + } + yield* execute(client, "COMMIT"); + return rewritten; + }); + + yield* execute(client, "BEGIN"); + return yield* applyAll.pipe( + Effect.tapError(() => execute(client, "ROLLBACK").pipe(Effect.ignore)), + Effect.onInterrupt(() => execute(client, "ROLLBACK").pipe(Effect.ignore)), + ); + }); + +/** Registry entry for the SQLite hosts' boot-time data-migration ledger. */ +export const emptyJsonSqliteMigration: SqliteDataMigration = { + name: MIGRATION_NAME, + run: (client) => runSqliteEmptyJsonMigration(client).pipe(Effect.asVoid), +};