From 0453568da1a92488d166aa54cf565a243cc7816d Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 23 Apr 2026 20:42:11 +0200 Subject: [PATCH 1/2] feat(migrate): add Cloudflare D1 runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the MigrationRunner contract from the migrate package refactor (this PR stacks on feat/relayauth-migrate-package). Provides createD1Runner(db) for any async driver that matches the D1 prepared- statement surface — Workers D1 binding or an HTTP client wrapper in build-time/CI contexts. Atomicity: each migration is split into statements and submitted via db.batch(...), which the Cloudflare side runs inside a transaction — partially-applied DDL is impossible. The splitter strips -- line comments and splits on ; with no tokenizer; sufficient for pure DDL files that follow the existing relayauth convention. Documented caveats for strings and BEGIN/END blocks so future consumers don't trip over it. Structural typing (D1DatabaseLike, D1PreparedStatementLike, D1ResultLike) instead of @cloudflare/workers-types so Node consumers can wrap the Cloudflare REST API without pulling Workers types into their build. Tests exercise the runner against node:sqlite wrapped in an async D1- like facade — fresh DB, rerun, checksum drift, multi-statement rollback, idempotent recordApplied, splitter correctness. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/migrate/package.json | 4 + .../migrate/src/__tests__/d1-runner.test.ts | 215 ++++++++++++++++++ packages/migrate/src/index.ts | 7 + packages/migrate/src/runners/d1.ts | 106 +++++++++ 4 files changed, 332 insertions(+) create mode 100644 packages/migrate/src/__tests__/d1-runner.test.ts create mode 100644 packages/migrate/src/runners/d1.ts diff --git a/packages/migrate/package.json b/packages/migrate/package.json index 42b68c4..146fb6c 100644 --- a/packages/migrate/package.json +++ b/packages/migrate/package.json @@ -13,6 +13,10 @@ "types": "./dist/runners/node-sqlite.d.ts", "import": "./dist/runners/node-sqlite.js" }, + "./d1": { + "types": "./dist/runners/d1.d.ts", + "import": "./dist/runners/d1.js" + }, "./fs": { "types": "./dist/sources/fs.d.ts", "import": "./dist/sources/fs.js" diff --git a/packages/migrate/src/__tests__/d1-runner.test.ts b/packages/migrate/src/__tests__/d1-runner.test.ts new file mode 100644 index 0000000..a16e6f5 --- /dev/null +++ b/packages/migrate/src/__tests__/d1-runner.test.ts @@ -0,0 +1,215 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { DatabaseSync } from "node:sqlite"; +import test from "node:test"; + +import { + MigrationChecksumMismatchError, + createD1Runner, + createFsMigrationSource, + runMigrations, + sha256, + splitSqlStatements, + type D1DatabaseLike, + type D1PreparedStatementLike, + type D1ResultLike, +} from "../index.js"; + +/** + * Wrap node:sqlite's synchronous DatabaseSync behind the async D1 prepared- + * statement surface so the D1 runner can be exercised without a real + * Cloudflare binding. + * + * The goal is not to emulate D1 perfectly; it's to exercise the runner's + * prepare/batch/run/all/first call patterns against a real SQL engine. + */ +function createFakeD1(db: DatabaseSync): D1DatabaseLike { + function prepare(sql: string): D1PreparedStatementLike { + let boundParams: unknown[] = []; + const api: D1PreparedStatementLike = { + bind(...values: unknown[]) { + boundParams = values; + return api; + }, + async run(): Promise> { + db.prepare(sql).run(...(boundParams as unknown[])); + return { success: true }; + }, + async all(): Promise> { + const rows = db.prepare(sql).all(...(boundParams as unknown[])) as Row[]; + return { results: rows, success: true }; + }, + async first(): Promise { + const row = db.prepare(sql).get(...(boundParams as unknown[])) as T | undefined; + return row ?? null; + }, + }; + return api; + } + + return { + prepare, + async batch( + statements: D1PreparedStatementLike[], + ): Promise[]> { + const results: D1ResultLike[] = []; + db.exec("BEGIN"); + try { + for (const stmt of statements) { + results.push(await stmt.run()); + } + db.exec("COMMIT"); + } catch (err) { + try { + db.exec("ROLLBACK"); + } catch { + // swallow — caller needs the original error + } + throw err; + } + return results; + }, + }; +} + +function createTempDir(): { dir: string; cleanup: () => void } { + const dir = mkdtempSync(join(tmpdir(), "relayauth-migrate-d1-test-")); + return { dir, cleanup: () => rmSync(dir, { recursive: true, force: true }) }; +} + +function writeMigration(dir: string, name: string, sql: string): void { + writeFileSync(join(dir, name), sql, "utf8"); +} + +test("fresh DB: all files apply and journal is populated", async () => { + const { dir, cleanup } = createTempDir(); + const db = new DatabaseSync(":memory:"); + try { + writeMigration(dir, "0001_users.sql", "CREATE TABLE users (id TEXT PRIMARY KEY);"); + writeMigration(dir, "0002_posts.sql", "CREATE TABLE posts (id TEXT PRIMARY KEY);"); + + const runner = createD1Runner(createFakeD1(db)); + const source = createFsMigrationSource(dir); + + const result = await runMigrations(runner, source); + + assert.deepEqual(result.applied, ["0001_users", "0002_posts"]); + assert.deepEqual(result.skipped, []); + + const applied = await runner.listApplied(); + assert.equal(applied.length, 2); + assert.equal(applied[0]?.id, "0001_users"); + assert.equal(applied[0]?.checksum, sha256("CREATE TABLE users (id TEXT PRIMARY KEY);")); + + db.exec("INSERT INTO users (id) VALUES ('u1')"); + db.exec("INSERT INTO posts (id) VALUES ('p1')"); + } finally { + db.close(); + cleanup(); + } +}); + +test("rerun: nothing applies again, skipped equals all", async () => { + const { dir, cleanup } = createTempDir(); + const db = new DatabaseSync(":memory:"); + try { + writeMigration(dir, "0001_users.sql", "CREATE TABLE users (id TEXT PRIMARY KEY);"); + + const runner = createD1Runner(createFakeD1(db)); + const source = createFsMigrationSource(dir); + + await runMigrations(runner, source); + const second = await runMigrations(runner, source); + + assert.deepEqual(second.applied, []); + assert.deepEqual(second.skipped, ["0001_users"]); + } finally { + db.close(); + cleanup(); + } +}); + +test("checksum drift throws MigrationChecksumMismatchError", async () => { + const { dir, cleanup } = createTempDir(); + const db = new DatabaseSync(":memory:"); + try { + writeMigration(dir, "0001_users.sql", "CREATE TABLE users (id TEXT PRIMARY KEY);"); + + const runner = createD1Runner(createFakeD1(db)); + await runMigrations(runner, createFsMigrationSource(dir)); + + writeMigration(dir, "0001_users.sql", "CREATE TABLE users (id TEXT PRIMARY KEY, name TEXT);"); + + await assert.rejects( + () => runMigrations(runner, createFsMigrationSource(dir)), + (err: unknown) => err instanceof MigrationChecksumMismatchError, + ); + } finally { + db.close(); + cleanup(); + } +}); + +test("multi-statement migration is batched atomically — failure rolls back prior statements", async () => { + const { dir, cleanup } = createTempDir(); + const db = new DatabaseSync(":memory:"); + try { + // Second statement is invalid (missing table). The batch must roll back + // the first so `users` doesn't exist after the failed migration. + writeMigration( + dir, + "0001_broken.sql", + `CREATE TABLE users (id TEXT PRIMARY KEY); + INSERT INTO nonexistent (id) VALUES ('x');`, + ); + + const runner = createD1Runner(createFakeD1(db)); + const source = createFsMigrationSource(dir); + + await assert.rejects(() => runMigrations(runner, source)); + + // Prior statement must not have survived the failed batch. + const tables = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'users'") + .all(); + assert.equal(tables.length, 0); + + // Journal must not record the failed migration. + const applied = await runner.listApplied(); + assert.equal(applied.length, 0); + } finally { + db.close(); + cleanup(); + } +}); + +test("splitSqlStatements strips -- comments and splits on unquoted semicolons", () => { + const sql = ` + -- header comment + CREATE TABLE a (id TEXT); -- inline comment + CREATE INDEX idx_a ON a (id); + `; + assert.deepEqual(splitSqlStatements(sql), [ + "CREATE TABLE a (id TEXT)", + "CREATE INDEX idx_a ON a (id)", + ]); +}); + +test("recordApplied is idempotent on duplicate ids", async () => { + const db = new DatabaseSync(":memory:"); + try { + const runner = createD1Runner(createFakeD1(db)); + await runner.initialize(); + + await runner.recordApplied({ id: "0001_x", checksum: "abc" }, 1); + await runner.recordApplied({ id: "0001_x", checksum: "def" }, 2); + + const applied = await runner.listApplied(); + assert.equal(applied.length, 1); + assert.equal(applied[0]?.checksum, "abc"); // first wins; INSERT OR IGNORE + } finally { + db.close(); + } +}); diff --git a/packages/migrate/src/index.ts b/packages/migrate/src/index.ts index 084f76a..187a86d 100644 --- a/packages/migrate/src/index.ts +++ b/packages/migrate/src/index.ts @@ -15,4 +15,11 @@ export { createNodeSqliteRunner, type NodeSqliteDatabase, } from "./runners/node-sqlite.js"; +export { + createD1Runner, + splitSqlStatements, + type D1DatabaseLike, + type D1PreparedStatementLike, + type D1ResultLike, +} from "./runners/d1.js"; export { createFsMigrationSource, sha256 } from "./sources/fs.js"; diff --git a/packages/migrate/src/runners/d1.ts b/packages/migrate/src/runners/d1.ts new file mode 100644 index 0000000..3faf82b --- /dev/null +++ b/packages/migrate/src/runners/d1.ts @@ -0,0 +1,106 @@ +import { MIGRATIONS_TABLE_SQL } from "../runner.js"; +import type { AppliedMigration, MigrationRunner } from "../types.js"; + +/** + * Minimal structural type for a Cloudflare D1 prepared statement. We declare + * only the surface we use so consumers don't have to depend on + * `@cloudflare/workers-types` for the sake of satisfying this runner. + */ +export interface D1PreparedStatementLike { + bind(...values: unknown[]): D1PreparedStatementLike; + run(): Promise>; + all(): Promise>; + first(colName?: string): Promise; +} + +export interface D1ResultLike { + results?: Row[]; + success?: boolean; + meta?: unknown; +} + +export interface D1DatabaseLike { + prepare(sql: string): D1PreparedStatementLike; + batch(statements: D1PreparedStatementLike[]): Promise[]>; +} + +/** + * Build a {@link MigrationRunner} backed by Cloudflare D1 (or any other + * driver that matches the D1 prepared-statement surface). + * + * Atomicity: a migration file is split on unquoted semicolons (see + * {@link splitSqlStatements}) into individual statements, which are then + * submitted to D1 via `db.batch(...)`. D1 batches run inside a single + * transaction on the Cloudflare side — if any statement fails, the whole + * migration rolls back, so partially-applied DDL is impossible. + * + * Caveat: the statement splitter is deliberately naive — it strips + * `--`-prefixed line comments and splits on `;`. Migrations that embed + * semicolons inside string literals or BEGIN/END blocks are not supported + * today; use a dedicated migration file per such statement until the + * splitter grows a tokenizer. + */ +export function createD1Runner(db: D1DatabaseLike): MigrationRunner { + return { + async initialize() { + await db.prepare(MIGRATIONS_TABLE_SQL).run(); + }, + + async exec(sql) { + const statements = splitSqlStatements(sql); + if (statements.length === 0) { + return; + } + + if (statements.length === 1) { + await db.prepare(statements[0]).run(); + return; + } + + await db.batch(statements.map((stmt) => db.prepare(stmt))); + }, + + async listApplied() { + const result = await db + .prepare<{ id: string; applied_at: number; checksum: string }>( + "SELECT id, applied_at, checksum FROM _migrations ORDER BY id ASC", + ) + .all(); + + return (result.results ?? []).map((row) => ({ + id: row.id, + appliedAt: Number(row.applied_at), + checksum: row.checksum, + })); + }, + + async recordApplied(file, appliedAt) { + await db + .prepare("INSERT OR IGNORE INTO _migrations (id, applied_at, checksum) VALUES (?, ?, ?)") + .bind(file.id, appliedAt, file.checksum) + .run(); + }, + }; +} + +/** + * Strip `--`-prefixed line comments and split the remaining SQL on `;`, + * returning one trimmed statement per element with empty entries dropped. + * + * Exported for the test suite — consumers should call {@link createD1Runner} + * rather than invoking the splitter directly. + */ +export function splitSqlStatements(sql: string): string[] { + const stripped = sql + .split("\n") + .map((line) => { + const idx = line.indexOf("--"); + return idx >= 0 ? line.slice(0, idx) : line; + }) + .join("\n"); + + return stripped + .split(";") + .map((chunk) => chunk.trim()) + .filter((chunk) => chunk.length > 0); +} From 2016517b8619c53f612f8060203e711b43d26007 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 23 Apr 2026 20:50:11 +0200 Subject: [PATCH 2/2] fix(migrate): quote-aware splitSqlStatements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged (relayauth#30): the prior indexOf("--") + split(";") splitter corrupted any migration with -- or ; inside a quoted string literal (e.g. INSERT INTO t VALUES ('foo--bar') or ('hello; world')). Replaced with a char-by-char scanner that tracks single-quoted strings (with '' escape), double-quoted identifiers (with "" escape), and -- line comments; semicolons are only statement terminators outside those regions. Not supported with a documented caveat so a bad migration fails at review rather than silently at runtime: - block comments /* ... */ - backslash escapes (SQLite doesn't honor them anyway) Tests added: -- inside strings preserved, ; inside strings preserved, '' escape inside strings, "" in identifiers — all covering the exact patterns Codex pointed at. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../migrate/src/__tests__/d1-runner.test.ts | 33 ++++++ packages/migrate/src/runners/d1.ts | 106 +++++++++++++++--- 2 files changed, 125 insertions(+), 14 deletions(-) diff --git a/packages/migrate/src/__tests__/d1-runner.test.ts b/packages/migrate/src/__tests__/d1-runner.test.ts index a16e6f5..a389cc0 100644 --- a/packages/migrate/src/__tests__/d1-runner.test.ts +++ b/packages/migrate/src/__tests__/d1-runner.test.ts @@ -197,6 +197,39 @@ test("splitSqlStatements strips -- comments and splits on unquoted semicolons", ]); }); +test("splitSqlStatements preserves -- inside single-quoted strings", () => { + const sql = "INSERT INTO t (val) VALUES ('foo--bar'); INSERT INTO t (val) VALUES ('baz');"; + assert.deepEqual(splitSqlStatements(sql), [ + "INSERT INTO t (val) VALUES ('foo--bar')", + "INSERT INTO t (val) VALUES ('baz')", + ]); +}); + +test("splitSqlStatements preserves semicolons inside single-quoted strings", () => { + const sql = "INSERT INTO t (val) VALUES ('hello; world'); INSERT INTO t (val) VALUES ('x');"; + assert.deepEqual(splitSqlStatements(sql), [ + "INSERT INTO t (val) VALUES ('hello; world')", + "INSERT INTO t (val) VALUES ('x')", + ]); +}); + +test("splitSqlStatements handles '' escape inside single-quoted strings", () => { + // 'it''s fine' is the SQL standard for a string containing a literal quote. + const sql = "INSERT INTO t (val) VALUES ('it''s; fine'); INSERT INTO t (val) VALUES ('y');"; + assert.deepEqual(splitSqlStatements(sql), [ + "INSERT INTO t (val) VALUES ('it''s; fine')", + "INSERT INTO t (val) VALUES ('y')", + ]); +}); + +test("splitSqlStatements preserves semicolons and -- inside double-quoted identifiers", () => { + const sql = 'CREATE TABLE "weird;name--here" (id TEXT); CREATE INDEX idx ON "weird;name--here" (id);'; + assert.deepEqual(splitSqlStatements(sql), [ + 'CREATE TABLE "weird;name--here" (id TEXT)', + 'CREATE INDEX idx ON "weird;name--here" (id)', + ]); +}); + test("recordApplied is idempotent on duplicate ids", async () => { const db = new DatabaseSync(":memory:"); try { diff --git a/packages/migrate/src/runners/d1.ts b/packages/migrate/src/runners/d1.ts index 3faf82b..0f29c96 100644 --- a/packages/migrate/src/runners/d1.ts +++ b/packages/migrate/src/runners/d1.ts @@ -84,23 +84,101 @@ export function createD1Runner(db: D1DatabaseLike): MigrationRunner { } /** - * Strip `--`-prefixed line comments and split the remaining SQL on `;`, - * returning one trimmed statement per element with empty entries dropped. + * Split a SQL migration into individual statements. + * + * Walks the input character-by-character tracking whether the cursor is + * inside a single-quoted string (`'...'` with `''` as the SQL standard + * escape), a double-quoted identifier (`"..."` with `""` as escape), or an + * ordinary region. Outside strings, `--` begins a line comment that runs + * to the next newline, and `;` terminates a statement. Inside a string or + * identifier those characters are literal. + * + * Not supported (migrations that need these should fail loudly during + * review rather than silently corrupt at runtime): + * - `/* ... *` `/` block comments — not used by relayauth migrations today. + * - Backslash escapes inside strings — SQLite doesn't honor them. * * Exported for the test suite — consumers should call {@link createD1Runner} * rather than invoking the splitter directly. */ export function splitSqlStatements(sql: string): string[] { - const stripped = sql - .split("\n") - .map((line) => { - const idx = line.indexOf("--"); - return idx >= 0 ? line.slice(0, idx) : line; - }) - .join("\n"); - - return stripped - .split(";") - .map((chunk) => chunk.trim()) - .filter((chunk) => chunk.length > 0); + const statements: string[] = []; + let current = ""; + let mode: "normal" | "single" | "double" | "line-comment" = "normal"; + + for (let i = 0; i < sql.length; i++) { + const ch = sql[i]; + const next = sql[i + 1]; + + if (mode === "line-comment") { + if (ch === "\n") { + mode = "normal"; + current += ch; + } + continue; + } + + if (mode === "single") { + current += ch; + if (ch === "'") { + if (next === "'") { + // Escaped quote — consume both and stay inside the string. + current += next; + i++; + } else { + mode = "normal"; + } + } + continue; + } + + if (mode === "double") { + current += ch; + if (ch === '"') { + if (next === '"') { + current += next; + i++; + } else { + mode = "normal"; + } + } + continue; + } + + if (ch === "-" && next === "-") { + mode = "line-comment"; + i++; + continue; + } + + if (ch === "'") { + mode = "single"; + current += ch; + continue; + } + + if (ch === '"') { + mode = "double"; + current += ch; + continue; + } + + if (ch === ";") { + const trimmed = current.trim(); + if (trimmed.length > 0) { + statements.push(trimmed); + } + current = ""; + continue; + } + + current += ch; + } + + const tail = current.trim(); + if (tail.length > 0) { + statements.push(tail); + } + + return statements; }