From 8604cc4fe5a003d0621b70f05a35c2115559fbee Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 07:55:17 +0000 Subject: [PATCH 01/11] feat: name the kinds of object dbdiff now compares, and show what it does not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dbdiff now diffs range types, default privileges, event triggers, rules, aggregates, extended statistics, text search configurations and dictionaries, casts, foreign servers, user mappings and foreign tables, and renames columns and tables rather than dropping them. Each is titled by what it is ("Cast missing: flag AS boolean", "Column renamed: public.people.name → full_name"); a changed one is " modified", named from the statement that makes it rather than the DROP that replaces it, which read as the object going and was marked critical. dbdiff's warnings — kinds it does not compare, columns or tables it would not take for renames — are now findings too, with nothing to apply, so they are seen before an apply rather than missed. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/dbdiff.ts | 165 ++++++++++++++++++++++++- packages/cli/test/dbdiff-units.test.ts | 70 ++++++++++- 2 files changed, 229 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/dbdiff.ts b/packages/cli/src/dbdiff.ts index 0508330..63a5b79 100644 --- a/packages/cli/src/dbdiff.ts +++ b/packages/cli/src/dbdiff.ts @@ -45,6 +45,21 @@ export interface DbDiffOptions { export interface DbDiffResult { up: string down: string + /** + * What dbdiff warned about without changing anything: kinds of object it + * does not compare, columns or tables it would not take for renames. + */ + notices?: string[] +} + +/** dbdiff's warnings (`⚠ ...` lines), colour codes stripped. */ +export function dbdiffNotices(stdout: string): string[] { + return stdout + .replace(/\u001b\[[0-9;]*m/g, '') + .split('\n') + .map(line => line.trim()) + .filter(line => line.startsWith('⚠')) + .map(line => line.replace(/^⚠\s*/, '')) } /** @@ -285,16 +300,17 @@ export async function runDbDiff(options: DbDiffOptions): Promise { // so timeout, maxBuffer and every error path stay exactly as they were. attachProgress(running, options.onProgress) - await running + const { stdout } = await running + const notices = dbdiffNotices(String(stdout ?? '')) // When schemas are identical, dbdiff exits 0 but doesn't write the file const fileExists = await access(outputFile).then(() => true, () => false) if (!fileExists) { - return { up: '', down: '' } + return { up: '', down: '', notices } } const output = await readFile(outputFile, 'utf8') - return parseDbDiffOutput(output) + return { ...parseDbDiffOutput(output), notices } } catch (err: unknown) { const errObj = (err ?? {}) as Record const message = errMsg(err) @@ -564,7 +580,27 @@ export function sqlToIssues( ignoredSchemaTables?: Set, ): DriftIssue[] { const issues = unfoldedIssues(result, check, ignoreSchemas, ignoredSchemaTables) - return check === 'schema' ? foldCommentsIntoCreates(issues) : issues + return check === 'schema' + ? [...foldCommentsIntoCreates(issues), ...noticeIssues(result.notices ?? [])] + : issues +} + +/** + * A dbdiff warning as a finding of its own, with nothing to apply: what was + * not compared, or not taken for a rename, is worth knowing before an apply + * rather than finding out after. + */ +function noticeIssues(notices: string[]): DriftIssue[] { + return notices.map((notice, i) => { + const [what, advice] = notice.split(' — ') + return { + id: `schema-notice-${i + 1}`, + check: 'schema' as const, + severity: 'info' as const, + title: /not compared/.test(what) ? `Not compared: ${what.replace(/ objects are not compared/, '')}` : what, + description: advice ? `${what} — ${advice}.` : `${what}.`, + } + }) } /** What a comment title names: `public.orders.total (column)` → the name and kind. */ @@ -758,6 +794,19 @@ function issuesFromUnits( } } + const modified = modifiedTitle(up, statements) + if (modified) { + return { + id: `${check}-alter-${up.kind.replace(/^Alter/, '').toLowerCase()}-${i + 1}`, + check, + severity: 'warning' as const, + title: modified, + description: 'Definition differs between source and target.', + sql: { up: upSql, down: downSql }, + ...manual, + } + } + const lead = leadStatement(up, statements) const type = classifyStatement(lead) return { @@ -845,6 +894,36 @@ function replacedView(unit: DbDiffUnit, statements: string[]): { kind: Replaceab /** A name without its schema or quotes, for matching one to dbdiff's unit object. */ const bareName = (name: string) => name.slice(name.lastIndexOf('.') + 1).replace(/"/g, '').toLowerCase() +/** Changed objects dbdiff replaces, or changes where they stand, by unit kind. */ +const MODIFIED_KINDS: Record = { + AlterRangeType: 'Type modified', + AlterAggregate: 'Aggregate modified', + AlterRule: 'Rule modified', + AlterStatistics: 'Statistics modified', + AlterEventTrigger: 'Event trigger modified', + AlterCast: 'Cast modified', + AlterTextSearchConfiguration: 'Text search configuration modified', + AlterTextSearchDictionary: 'Text search dictionary modified', + AlterForeignServer: 'Foreign server modified', + AlterUserMapping: 'User mapping modified', + AlterForeignTable: 'Foreign table modified', + AlterDefaultPrivileges: 'Default privileges differ', +} + +/** + * " modified: " for a changed object of the kinds above, named + * from the statement that makes it — its DROP would read as the object + * being removed. + */ +function modifiedTitle(unit: DbDiffUnit, statements: string[]): string | undefined { + const label = MODIFIED_KINDS[unit.kind] + if (!label) return undefined + const making = statements.find(sql => !/^\s*DROP\b/i.test(sql)) ?? statements[0] + const summary = making ? summariseStatement(making, 'schema') : '' + const name = summary.slice(summary.indexOf(': ') + 2) + return `${label}: ${name || unit.object}` +} + /** * The statement a unit is named by: for a table change, its first ALTER * TABLE (a column type change opens by standing views aside; a serial column @@ -1358,6 +1437,8 @@ interface SummaryRule { match: RegExp label: string name: (sql: string, downSql?: string) => string + /** The name is not a schema object's — a role's privileges, a cast — so no schema is added. */ + unqualified?: boolean } /** Head patterns for the identifier that follows each keyword. */ @@ -1371,12 +1452,58 @@ const AFTER = { index: /\bINDEX\s+(?:CONCURRENTLY\s+)?(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, policy: /\bPOLICY\s+(?:IF\s+EXISTS\s+)?/i, collation: /\bCOLLATION\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + eventTrigger: /\bEVENT\s+TRIGGER\s+(?:IF\s+EXISTS\s+)?/i, + aggregate: /\bAGGREGATE\s+(?:IF\s+EXISTS\s+)?/i, + statistics: /\bSTATISTICS\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + textSearch: /\bTEXT\s+SEARCH\s+(?:CONFIGURATION|DICTIONARY)\s+(?:IF\s+EXISTS\s+)?/i, + server: /\bSERVER\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + foreignTable: /\bFOREIGN\s+TABLE\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, on: /\bON\s+(?:ONLY\s+)?/i, } /** Name an object by the identifier following `head`. */ const named = (head: RegExp) => (sql: string) => extractQualifiedName(sql, head) +/** + * `public.people.name → full_name`, `public.clients → customers`: what a + * rename statement renames, and to what. + */ +function renameLabel(sql: string): string { + const table = extractQualifiedName(sql, /\b(?:TABLE|INDEX)\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?/i) + const to = /\bTO\s+("(?:[^"]|"")*"|[\w$]+)\s*;?\s*$/i.exec(sql.trim())?.[1] + const from = /\bRENAME\s+(?:COLUMN|CONSTRAINT)\s+("(?:[^"]|"")*"|[\w$]+)/i.exec(sql)?.[1] + return `${qualifySchemaName(table)}${from ? '.' + unquoted(from) : ''} → ${to ? unquoted(to) : UNKNOWN_NAME}` +} + +/** `role in schema on tables` for an ALTER DEFAULT PRIVILEGES. */ +function defaultPrivilegesLabel(sql: string): string { + const role = /\bFOR\s+ROLE\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] + const schema = /\bIN\s+SCHEMA\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] + const kind = /\bON\s+(TABLES|SEQUENCES|FUNCTIONS|ROUTINES|TYPES|SCHEMAS)\b/i.exec(sql)?.[1] + return [role && unquoted(role), schema ? `in ${unquoted(schema)}` : 'in every schema', kind && `on ${kind.toLowerCase()}`] + .filter(Boolean).join(' ') +} + +/** `table.rule` for a rule statement. */ +function ruleLabel(sql: string): string { + const rule = extractQualifiedName(sql, /\bRULE\s+(?:IF\s+EXISTS\s+)?/i) + const table = /\bTO\s+((?:"(?:[^"]|"")*"|[\w$]+)(?:\.(?:"(?:[^"]|"")*"|[\w$]+))?)/i.exec(sql)?.[1] + ?? /\bON\s+((?:"(?:[^"]|"")*"|[\w$]+)(?:\.(?:"(?:[^"]|"")*"|[\w$]+))?)/i.exec(sql)?.[1] + return table ? `${unquoted(table.split('.').pop() ?? table)}.${rule.split('.').pop()}` : rule +} + +/** `(a AS b)` for a cast statement. */ +function castLabel(sql: string): string { + return /\(([^()]+?\s+AS\s+[^()]+?)\)/i.exec(sql)?.[1] ?? UNKNOWN_NAME +} + +/** `user@server` for a user mapping statement. */ +function userMappingLabel(sql: string): string { + const user = /\bFOR\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] ?? 'unknown' + const server = /\bSERVER\s+("(?:[^"]|"")*"|[^\s;]+)/i.exec(sql)?.[1] ?? 'unknown' + return `${unquoted(user)}@${unquoted(server)}` +} + /** * `table.policy` for a policy statement. * @@ -1488,6 +1615,10 @@ const SCHEMA_RULES: SummaryRule[] = [ { match: /^\s*ALTER\s+DOMAIN\b/i, label: 'Domain altered', name: named(AFTER.domain) }, { match: /^\s*DROP\s+DOMAIN\b/i, label: 'Extra domain', name: named(AFTER.domain) }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+COLUMN\b/is, label: 'Column renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+CONSTRAINT\b/is, label: 'Constraint renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+TO\b/is, label: 'Table renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+INDEX\b[^;]*\bRENAME\s+TO\b/is, label: 'Index renamed', name: renameLabel, unqualified: true }, { match: /^\s*ALTER\s+TABLE\b/i, label: 'Table altered', name: named(AFTER.table) }, { match: /^\s*CREATE\s+TABLE\b/i, label: 'Table missing', name: named(AFTER.table) }, { match: /^\s*DROP\s+TABLE\b/i, label: 'Extra table', name: named(AFTER.table) }, @@ -1514,6 +1645,29 @@ const SCHEMA_RULES: SummaryRule[] = [ { match: /^\s*ALTER\s+COLLATION\b/i, label: 'Collation altered', name: named(AFTER.collation) }, { match: /^\s*DROP\s+COLLATION\b/i, label: 'Extra collation', name: named(AFTER.collation) }, + { match: /^\s*ALTER\s+DEFAULT\s+PRIVILEGES\b/i, label: 'Default privileges differ', name: defaultPrivilegesLabel, unqualified: true }, + { match: /^\s*CREATE\s+EVENT\s+TRIGGER\b/i, label: 'Event trigger missing', name: named(AFTER.eventTrigger) }, + { match: /^\s*DROP\s+EVENT\s+TRIGGER\b/i, label: 'Extra event trigger', name: named(AFTER.eventTrigger) }, + { match: /^\s*CREATE\s+(?:OR\s+REPLACE\s+)?RULE\b/i, label: 'Rule missing', name: ruleLabel }, + { match: /^\s*DROP\s+RULE\b/i, label: 'Extra rule', name: ruleLabel }, + { match: /^\s*CREATE\s+(?:OR\s+REPLACE\s+)?AGGREGATE\b/i, label: 'Aggregate missing', name: named(AFTER.aggregate) }, + { match: /^\s*DROP\s+AGGREGATE\b/i, label: 'Extra aggregate', name: named(AFTER.aggregate) }, + { match: /^\s*CREATE\s+STATISTICS\b/i, label: 'Statistics missing', name: named(AFTER.statistics) }, + { match: /^\s*DROP\s+STATISTICS\b/i, label: 'Extra statistics', name: named(AFTER.statistics) }, + { match: /^\s*(?:CREATE|ALTER)\s+TEXT\s+SEARCH\s+CONFIGURATION\b/i, label: 'Text search configuration missing', name: named(AFTER.textSearch) }, + { match: /^\s*DROP\s+TEXT\s+SEARCH\s+CONFIGURATION\b/i, label: 'Extra text search configuration', name: named(AFTER.textSearch) }, + { match: /^\s*CREATE\s+TEXT\s+SEARCH\s+DICTIONARY\b/i, label: 'Text search dictionary missing', name: named(AFTER.textSearch) }, + { match: /^\s*DROP\s+TEXT\s+SEARCH\s+DICTIONARY\b/i, label: 'Extra text search dictionary', name: named(AFTER.textSearch) }, + { match: /^\s*CREATE\s+CAST\b/i, label: 'Cast missing', name: castLabel, unqualified: true }, + { match: /^\s*DROP\s+CAST\b/i, label: 'Extra cast', name: castLabel, unqualified: true }, + { match: /^\s*CREATE\s+SERVER\b/i, label: 'Foreign server missing', name: named(AFTER.server) }, + { match: /^\s*ALTER\s+SERVER\b/i, label: 'Foreign server altered', name: named(AFTER.server) }, + { match: /^\s*DROP\s+SERVER\b/i, label: 'Extra foreign server', name: named(AFTER.server) }, + { match: /^\s*(?:CREATE|ALTER)\s+USER\s+MAPPING\b/i, label: 'User mapping missing', name: userMappingLabel, unqualified: true }, + { match: /^\s*DROP\s+USER\s+MAPPING\b/i, label: 'Extra user mapping', name: userMappingLabel, unqualified: true }, + { match: /^\s*CREATE\s+FOREIGN\s+TABLE\b/i, label: 'Foreign table missing', name: named(AFTER.foreignTable) }, + { match: /^\s*DROP\s+FOREIGN\s+TABLE\b/i, label: 'Extra foreign table', name: named(AFTER.foreignTable) }, + { match: /^\s*CREATE\s+SEQUENCE\b/i, label: 'Sequence missing', name: named(AFTER.sequence) }, { match: /^\s*ALTER\s+SEQUENCE\b/i, label: 'Sequence altered', name: named(AFTER.sequence) }, { match: /^\s*DROP\s+SEQUENCE\b/i, label: 'Extra sequence', name: named(AFTER.sequence) }, @@ -1592,7 +1746,8 @@ export function summariseStatement(sql: string, check: 'schema' | 'data', downSq for (const rule of SCHEMA_RULES) { if (rule.match.test(sql)) { const label = rule.name === commentLabel ? commentChange(sql, downSql) : rule.label - return `${label}: ${qualifySchemaName(rule.name(sql, downSql))}` + const name = rule.name(sql, downSql) + return `${label}: ${rule.unqualified ? name : qualifySchemaName(name)}` } } diff --git a/packages/cli/test/dbdiff-units.test.ts b/packages/cli/test/dbdiff-units.test.ts index c37270b..465aa10 100644 --- a/packages/cli/test/dbdiff-units.test.ts +++ b/packages/cli/test/dbdiff-units.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { pairUnits, parseUnits } from '../src/dbdiff-units.js' -import { buildDbDiffArgs, destructiveReason, sqlToIssues } from '../src/dbdiff.js' +import { buildDbDiffArgs, dbdiffNotices, destructiveReason, sqlToIssues } from '../src/dbdiff.js' /** A section as `dbdiff --units` writes it. */ function units(...parts: Array<[kind: string, object: string, sql: string]>): string { @@ -246,3 +246,71 @@ describe('statements after a comment', () => { ]) }) }) + +describe('sqlToIssues: kinds dbdiff learned to compare, and renames', () => { + const titles = (...parts: Array<[string, string, string]>) => + sqlToIssues({ up: units(...parts), down: '' }, 'schema').map(i => i.title) + + it('names each kind', () => { + expect(titles( + ['CreateDefaultPrivileges', 'postgres.public.tables', 'ALTER DEFAULT PRIVILEGES FOR ROLE "postgres" IN SCHEMA "public" GRANT SELECT ON TABLES TO "anon";'], + ['CreateEventTrigger', 'audit', 'CREATE EVENT TRIGGER "audit" ON ddl_command_end EXECUTE FUNCTION audit_ddl();'], + ['CreateRule', 'orders.keep', 'CREATE RULE keep AS ON DELETE TO public.orders DO INSTEAD NOTHING;'], + ['DropRule', 'orders.old', 'DROP RULE IF EXISTS "old" ON "orders";'], + ['CreateAggregate', 'my_sum(integer)', 'CREATE AGGREGATE "my_sum"(integer) (SFUNC = add_int, STYPE = integer);'], + ['CreateStatistics', 'addr_stx', 'CREATE STATISTICS public.addr_stx (ndistinct) ON city, zip FROM addr;'], + ['CreateTextSearchConfiguration', 'my_cfg', 'CREATE TEXT SEARCH CONFIGURATION "my_cfg" (PARSER = pg_catalog."default");'], + ['CreateCast', 'flag AS boolean', 'CREATE CAST (flag AS boolean) WITH FUNCTION flag_to_bool(flag);'], + ['CreateForeignServer', 'warehouse', `CREATE SERVER "warehouse" FOREIGN DATA WRAPPER "postgres_fdw" OPTIONS ("host" 'db');`], + ['CreateUserMapping', 'reader@warehouse', `CREATE USER MAPPING FOR "reader" SERVER "warehouse" OPTIONS ("user" 'r');`], + ['CreateForeignTable', 'remote', 'CREATE FOREIGN TABLE "remote" (\n "id" integer\n) SERVER "warehouse";'], + )).toEqual([ + 'Default privileges differ: postgres in public on tables', + 'Event trigger missing: public.audit', + 'Rule missing: orders.keep', + 'Extra rule: orders.old', + 'Aggregate missing: public.my_sum', + 'Statistics missing: public.addr_stx', + 'Text search configuration missing: public.my_cfg', + 'Cast missing: flag AS boolean', + 'Foreign server missing: public.warehouse', + 'User mapping missing: reader@warehouse', + 'Foreign table missing: public.remote', + ]) + }) + + it('names renames by what they keep', () => { + expect(titles( + ['AlterTableRenameColumn', 'people.full_name', 'ALTER TABLE "people" RENAME COLUMN "name" TO "full_name";'], + ['AlterTableRename', 'customers', 'ALTER TABLE "clients" RENAME TO "customers";'], + )).toEqual(['Column renamed: public.people.name → full_name', 'Table renamed: public.clients → customers']) + }) + + it('names a changed object as modified, not by the DROP that replaces it', () => { + const issues = sqlToIssues({ up: units( + ['AlterAggregate', 'my_sum(integer)', 'DROP AGGREGATE IF EXISTS "my_sum"(integer);\nCREATE AGGREGATE "my_sum"(integer) (SFUNC = add_int, STYPE = integer);'], + ['AlterForeignServer', 'warehouse', `ALTER SERVER "warehouse" OPTIONS (DROP "host");\nALTER SERVER "warehouse" OPTIONS (ADD "host" 'db');`], + ), down: '' }, 'schema') + expect(issues.map(i => [i.title, i.severity])).toEqual([ + ['Aggregate modified: public.my_sum', 'warning'], + ['Foreign server modified: public.warehouse', 'warning'], + ]) + }) + + it('turns dbdiff warnings into findings with nothing to apply', () => { + const issues = sqlToIssues({ + up: '', down: '', + notices: ['Operator objects are not compared (source 1, target 0: ===) — check them by hand'], + }, 'schema') + expect(issues).toHaveLength(1) + expect(issues[0]).toMatchObject({ severity: 'info', title: 'Not compared: Operator (source 1, target 0: ===)' }) + expect(issues[0].sql).toBeUndefined() + }) +}) + +describe('dbdiffNotices', () => { + it('reads the warnings out of dbdiff\'s output, colour and all', () => { + expect(dbdiffNotices('\u001b[0;36mℹ Now generating UP migration\u001b[0m\n\u001b[0;33m⚠ Operator objects are not compared (source 1, target 0: ===) — check them by hand\u001b[0m\nWritten: x.sql')) + .toEqual(['Operator objects are not compared (source 1, target 0: ===) — check them by hand']) + }) +}) From 63487bfb97b0a1000c4d4b5adc189c104e281c55 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 07:58:51 +0000 Subject: [PATCH 02/11] feat: compare Supabase Queues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pgmq makes each queue's tables and registers them as its extension's own, so the schema diff — which leaves an extension's objects to the extension — never saw a queue: one the target lacked went unreported. A queues check now compares pgmq.meta by name and kind, creating a missing queue with pgmq.create (or create_unlogged), installing pgmq first where the target lacks it, and dropping an extra one with pgmq.drop_queue — held back as destructive, since it drops the queue's messages. A partitioned queue, whose pg_partman settings pgmq.meta does not record, and a change of kind, are left as manual steps. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- packages/cli/README.md | 5 +- packages/cli/src/checks/index.ts | 3 + packages/cli/src/checks/queues.ts | 115 ++++++++++++++++++++++++ packages/cli/src/dbdiff.ts | 5 ++ packages/cli/src/types/drift.ts | 6 +- packages/cli/test/checks/queues.test.ts | 38 ++++++++ 7 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 packages/cli/src/checks/queues.ts create mode 100644 packages/cli/test/checks/queues.test.ts diff --git a/README.md b/README.md index 04f10b3..871b4b9 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Built by **[Akal](https://github.com/akalsoftware)**. - RLS policies and RLS coverage; - storage buckets and their policies; - auth config and Edge Functions; - - cron jobs, webhooks and Realtime; + - cron jobs, queues, webhooks and Realtime; - Vault, extensions, roles and grants; - reference data and migration history. - **Fixes, not just reports.** Fixes run in dependency order, in one diff --git a/packages/cli/README.md b/packages/cli/README.md index c19a013..4a37b0b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -9,8 +9,8 @@ order, in one transaction, and can be proved on a throwaway copy before anything changes. With a single project, it snapshots, restores and clones it. - **The whole project.** It covers schema, RLS, storage, auth, Edge Functions, - cron, webhooks, Realtime, Vault, extensions, grants, reference data and - migration history. + cron, webhooks, Realtime, queues, Vault, extensions, grants, reference data + and migration history. - **Safe by default.** Every write needs `--apply`, and anything that loses data or opens up access also needs `--allow-destructive`. A failed apply leaves the target exactly as it was. @@ -63,6 +63,7 @@ Docker image needs nothing at all, and carries the PostgreSQL client tools | Storage | Buckets and their policies (objects are never copied) | API and SQL | | Auth config | Management API, or GoTrue on self-hosted | API (hosted) | | Cron jobs | `cron.job` | SQL | +| Queues | Supabase Queues (`pgmq.meta`), created and dropped through pgmq | SQL | | Webhooks | Triggers calling `supabase_functions.http_request` | SQL | | Realtime | Publications, and policies on `realtime.messages` | SQL | | Vault | Secret names and descriptions (values can't be read) | Command to run | diff --git a/packages/cli/src/checks/index.ts b/packages/cli/src/checks/index.ts index 97c43ae..8af219b 100644 --- a/packages/cli/src/checks/index.ts +++ b/packages/cli/src/checks/index.ts @@ -13,6 +13,7 @@ import { VaultCheck } from './vault.js' import { ExtensionsCheck } from './extensions.js' import { MigrationsCheck } from './migrations.js' import { RolesCheck } from './roles.js' +import { QueuesCheck } from './queues.js' export interface RegistryOptions { /** Enable file-level drift detection in the storage check. */ @@ -35,6 +36,7 @@ export function createDefaultRegistry(options: RegistryOptions = {}): CheckRegis registry.register(new ExtensionsCheck()) registry.register(new MigrationsCheck()) registry.register(new RolesCheck()) + registry.register(new QueuesCheck()) return registry } @@ -54,3 +56,4 @@ export { VaultCheck } from './vault.js' export { ExtensionsCheck } from './extensions.js' export { MigrationsCheck } from './migrations.js' export { RolesCheck } from './roles.js' +export { QueuesCheck } from './queues.js' diff --git a/packages/cli/src/checks/queues.ts b/packages/cli/src/checks/queues.ts new file mode 100644 index 0000000..f95fcee --- /dev/null +++ b/packages/cli/src/checks/queues.ts @@ -0,0 +1,115 @@ +import type { QueryFn } from '../db' +import { pgQuery } from '../db' +import type { DriftIssue } from '../types/drift' +import { quoteLiteral } from '../utils/sql' +import { Check, CheckSkipped, readOrAbsent, type CheckContext } from './base' + +/** A pgmq queue, as `pgmq.meta` records it. */ +export interface Queue { + queue_name: string + is_partitioned: boolean + is_unlogged: boolean +} + +/** + * Supabase Queues: pgmq's queues, compared by name and kind. + * + * A queue is tables pgmq makes and registers as its extension's own, so a + * schema diff — which leaves an extension's objects to the extension — + * never saw one: a queue the target lacked went unreported. Compared here + * from `pgmq.meta`, and created or dropped through pgmq's own functions, + * which keep its registry and the queue's tables in step. Dropping a queue + * drops its messages, so it is held back like any destructive fix. + */ +export class QueuesCheck extends Check { + readonly name = 'queues' as const + + constructor(private queryFn: QueryFn = pgQuery) { + super() + } + + async scan(ctx: CheckContext): Promise { + const [source, target] = await Promise.all([ + this.fetchQueues(ctx.source.dbUrl), + this.fetchQueues(ctx.target.dbUrl), + ]) + if (!source.available && !target.available) { + throw new CheckSkipped('pgmq extension not installed') + } + return diffQueues(source.queues, target.queues, target.available) + } + + private async fetchQueues(dbUrl: string): Promise<{ queues: Queue[]; available: boolean }> { + return readOrAbsent<{ queues: Queue[]; available: boolean }>( + async () => ({ queues: await this.queryFn(dbUrl, QUEUES_SQL) as unknown as Queue[], available: true }), + { queues: [], available: false }, + ) + } +} + +const QUEUES_SQL = ` + SELECT queue_name, is_partitioned, is_unlogged + FROM pgmq.meta + ORDER BY queue_name +` + +/** The statement pgmq creates one with, or undefined for a partitioned queue. */ +function createSql(q: Queue): string | undefined { + if (q.is_partitioned) return undefined + return `SELECT pgmq.${q.is_unlogged ? 'create_unlogged' : 'create'}(${quoteLiteral(q.queue_name)});` +} + +const dropSql = (q: Queue) => `SELECT pgmq.drop_queue(${quoteLiteral(q.queue_name)});` + +const kind = (q: Queue) => q.is_partitioned ? 'partitioned' : q.is_unlogged ? 'unlogged' : 'standard' + +export function diffQueues(source: Queue[], target: Queue[], targetHasPgmq = true): DriftIssue[] { + const issues: DriftIssue[] = [] + const sourceMap = new Map(source.map(q => [q.queue_name, q])) + const targetMap = new Map(target.map(q => [q.queue_name, q])) + const needsExtension = targetHasPgmq ? '' : 'CREATE EXTENSION IF NOT EXISTS pgmq;\n' + + for (const [name, q] of sourceMap) { + const t = targetMap.get(name) + if (!t) { + const up = createSql(q) + issues.push({ + id: `queues-missing-${name}`, + check: 'queues', + severity: 'warning', + title: `Missing queue: ${name}`, + description: `Queue "${name}" (${kind(q)}) exists in source but not in target.`, + sourceValue: q, + ...(up + ? { sql: { up: needsExtension + up, down: dropSql(q) } } + : { manualOnly: 'a partitioned queue is created with pg_partman settings pgmq.meta does not record — create it with pgmq.create_partitioned on the target' }), + }) + } else if (kind(q) !== kind(t)) { + issues.push({ + id: `queues-kind-${name}`, + check: 'queues', + severity: 'warning', + title: `Queue kind differs: ${name}`, + description: `Queue "${name}" is ${kind(q)} in source and ${kind(t)} in target. Changing it means dropping the queue and its messages, so it is left to you.`, + sourceValue: q, + targetValue: t, + manualOnly: 'changing a queue\'s kind drops it and its messages; recreate it by hand when it is empty', + }) + } + } + + for (const [name, t] of targetMap) { + if (sourceMap.has(name)) continue + const down = createSql(t) + issues.push({ + id: `queues-extra-${name}`, + check: 'queues', + severity: 'critical', + title: `Extra queue: ${name}`, + description: `Queue "${name}" exists in target but not in source. Dropping it drops its messages.`, + targetValue: t, + sql: { up: dropSql(t), down: down ?? '' }, + }) + } + return issues +} diff --git a/packages/cli/src/dbdiff.ts b/packages/cli/src/dbdiff.ts index 63a5b79..8d8cbe8 100644 --- a/packages/cli/src/dbdiff.ts +++ b/packages/cli/src/dbdiff.ts @@ -424,6 +424,11 @@ export function destructiveReason( sql: string, recreatedElsewhere: ReadonlySet = new Set(), ): string | undefined { + // A queue's messages are rows, and pgmq.drop_queue drops its tables. A + // function call, so neither reading below would see it as a DROP. + if (/\bPGMQ\s*\.\s*"?DROP_QUEUE"?\s*\(/.test(sqlSkeleton(sql).toUpperCase())) { + return 'drops a queue and the messages in it' + } const statements = parseStatements(sql) return statements ? destructiveInTree(statements, recreatedElsewhere) diff --git a/packages/cli/src/types/drift.ts b/packages/cli/src/types/drift.ts index 5c7cbf6..8a4e975 100644 --- a/packages/cli/src/types/drift.ts +++ b/packages/cli/src/types/drift.ts @@ -1,6 +1,6 @@ export const CHECK_NAMES = [ 'schema', 'rls', 'rls-coverage', 'edge-functions', 'storage', 'auth', 'cron', 'data', - 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', + 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', 'queues', ] as const export type CheckName = (typeof CHECK_NAMES)[number] @@ -22,13 +22,14 @@ export const CHECK_META: Record = { 'extensions': 'comparison', 'migrations': 'posture', 'roles': 'comparison', + 'queues': 'comparison', } /** Checks that compare source against target. */ diff --git a/packages/cli/test/checks/queues.test.ts b/packages/cli/test/checks/queues.test.ts new file mode 100644 index 0000000..9b9e5b0 --- /dev/null +++ b/packages/cli/test/checks/queues.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { QueuesCheck, diffQueues, type Queue } from '../../src/checks/queues.js' +import { destructiveReason } from '../../src/dbdiff.js' + +const q = (queue_name: string, o: Partial = {}): Queue => ({ queue_name, is_partitioned: false, is_unlogged: false, ...o }) + +describe('queues check', () => { + it('creates a missing queue through pgmq, and drops it again on the way down', () => { + const [issue] = diffQueues([q('jobs')], []) + expect(issue).toMatchObject({ title: 'Missing queue: jobs', severity: 'warning' }) + expect(issue.sql).toEqual({ up: "SELECT pgmq.create('jobs');", down: "SELECT pgmq.drop_queue('jobs');" }) + }) + + it('creates an unlogged queue as one, and installs pgmq first where the target lacks it', () => { + const [issue] = diffQueues([q('fast', { is_unlogged: true })], [], false) + expect(issue.sql!.up).toBe("CREATE EXTENSION IF NOT EXISTS pgmq;\nSELECT pgmq.create_unlogged('fast');") + }) + + it('leaves a partitioned queue, and a change of kind, as manual steps', () => { + const issues = diffQueues([q('big', { is_partitioned: true }), q('k')], [q('k', { is_unlogged: true })]) + expect(issues.map(i => [i.title, Boolean(i.manualOnly), i.sql])).toEqual([ + ['Missing queue: big', true, undefined], + ['Queue kind differs: k', true, undefined], + ]) + }) + + it('reports an extra queue as critical, and the drop is held back as destructive', () => { + const [issue] = diffQueues([], [q('old')]) + expect(issue).toMatchObject({ title: 'Extra queue: old', severity: 'critical' }) + expect(destructiveReason(issue.sql!.up)).toBe('drops a queue and the messages in it') + }) + + it('is skipped when neither side has pgmq', async () => { + const check = new QueuesCheck(async () => { throw Object.assign(new Error('relation "pgmq.meta" does not exist'), { code: '42P01' }) }) + await expect(check.scan({ source: { dbUrl: 's' }, target: { dbUrl: 't' }, config: { environments: {} } } as never)) + .rejects.toThrow('pgmq extension not installed') + }) +}) From 8784bcba346dbbbb0f706d2728311fddf8d10ff3 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:04:19 +0000 Subject: [PATCH 03/11] feat: compare pg_partman partition sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pg_partman's partitions are its own to make and drop, so the schema diff leaves them alone; what makes a partition set is its registration in part_config, written by create_parent(). A partitions check now compares those: a set the target lacks is registered through create_parent, with its settings (premake, retention and the rest) put as the source has them, and a setting that differs is updated where it stands. How a set is partitioned — control column, interval, type — cannot be changed in place, so a difference there, and a set only the target has, are left as manual steps. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 5 +- packages/cli/src/checks/index.ts | 3 + packages/cli/src/checks/partitions.ts | 179 ++++++++++++++++++++ packages/cli/src/types/drift.ts | 4 +- packages/cli/test/checks/partitions.test.ts | 41 +++++ 5 files changed, 229 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/checks/partitions.ts create mode 100644 packages/cli/test/checks/partitions.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index 4a37b0b..acd675d 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -9,8 +9,8 @@ order, in one transaction, and can be proved on a throwaway copy before anything changes. With a single project, it snapshots, restores and clones it. - **The whole project.** It covers schema, RLS, storage, auth, Edge Functions, - cron, webhooks, Realtime, queues, Vault, extensions, grants, reference data - and migration history. + cron, webhooks, Realtime, queues, partition sets, Vault, extensions, grants, + reference data and migration history. - **Safe by default.** Every write needs `--apply`, and anything that loses data or opens up access also needs `--allow-destructive`. A failed apply leaves the target exactly as it was. @@ -64,6 +64,7 @@ Docker image needs nothing at all, and carries the PostgreSQL client tools | Auth config | Management API, or GoTrue on self-hosted | API (hosted) | | Cron jobs | `cron.job` | SQL | | Queues | Supabase Queues (`pgmq.meta`), created and dropped through pgmq | SQL | +| Partition sets | pg_partman registrations (`part_config`), made with `create_parent` | SQL | | Webhooks | Triggers calling `supabase_functions.http_request` | SQL | | Realtime | Publications, and policies on `realtime.messages` | SQL | | Vault | Secret names and descriptions (values can't be read) | Command to run | diff --git a/packages/cli/src/checks/index.ts b/packages/cli/src/checks/index.ts index 8af219b..f8e5b19 100644 --- a/packages/cli/src/checks/index.ts +++ b/packages/cli/src/checks/index.ts @@ -14,6 +14,7 @@ import { ExtensionsCheck } from './extensions.js' import { MigrationsCheck } from './migrations.js' import { RolesCheck } from './roles.js' import { QueuesCheck } from './queues.js' +import { PartitionsCheck } from './partitions.js' export interface RegistryOptions { /** Enable file-level drift detection in the storage check. */ @@ -37,6 +38,7 @@ export function createDefaultRegistry(options: RegistryOptions = {}): CheckRegis registry.register(new MigrationsCheck()) registry.register(new RolesCheck()) registry.register(new QueuesCheck()) + registry.register(new PartitionsCheck()) return registry } @@ -57,3 +59,4 @@ export { ExtensionsCheck } from './extensions.js' export { MigrationsCheck } from './migrations.js' export { RolesCheck } from './roles.js' export { QueuesCheck } from './queues.js' +export { PartitionsCheck } from './partitions.js' diff --git a/packages/cli/src/checks/partitions.ts b/packages/cli/src/checks/partitions.ts new file mode 100644 index 0000000..90a5b75 --- /dev/null +++ b/packages/cli/src/checks/partitions.ts @@ -0,0 +1,179 @@ +import type { QueryFn } from '../db' +import { pgQuery } from '../db' +import type { DriftIssue } from '../types/drift' +import { quoteIdent, quoteLiteral } from '../utils/sql' +import { Check, CheckSkipped, type CheckContext } from './base' + +/** A pg_partman partition set, as its part_config row records it. */ +export interface PartitionSet { + parent_table: string + control: string + partition_interval: string + partition_type: string + epoch: string + premake: number + automatic_maintenance: string + constraint_cols: string[] | null + jobmon: boolean + date_trunc_interval: string | null + time_encoder: string | null + time_decoder: string | null + has_default: boolean + /** The settings part_config takes by UPDATE, by column. */ + settings: Record +} + +/** What decides how the set is partitioned: changed, it has to be rebuilt. */ +const SHAPE = ['control', 'partition_interval', 'partition_type', 'epoch', 'time_encoder', 'time_decoder'] as const + +/** part_config columns pg_partman documents setting with UPDATE. */ +const SETTINGS = [ + 'premake', 'automatic_maintenance', 'retention', 'retention_schema', 'retention_keep_index', 'retention_keep_table', + 'optimize_constraint', 'infinite_time_partitions', 'inherit_privileges', 'constraint_valid', 'ignore_default_data', + 'maintenance_order', 'retention_keep_publication', 'detach_before_drop', 'jobmon', 'date_trunc_interval', +] + +/** + * pg_partman partition sets, compared by their registration. + * + * The partitions themselves are pg_partman's to make and drop, so the schema + * diff leaves them alone; what makes a set is its row in part_config, written + * by create_parent(). A set the target lacks is registered through that + * function, and its settings put as the source has them; a setting that + * differs is updated where it stands. How the set is partitioned — its + * control column, interval or type — cannot be changed in place, so a + * difference there, and a set only the target has, are left to the user. + */ +export class PartitionsCheck extends Check { + readonly name = 'partitions' as const + + constructor(private queryFn: QueryFn = pgQuery) { + super() + } + + async scan(ctx: CheckContext): Promise { + const [source, target] = await Promise.all([this.fetch(ctx.source.dbUrl), this.fetch(ctx.target.dbUrl)]) + if (source.schema === null && target.schema === null) { + throw new CheckSkipped('pg_partman extension not installed') + } + return diffPartitionSets(source.sets, target.sets, target.schema) + } + + private async fetch(dbUrl: string): Promise<{ schema: string | null; sets: PartitionSet[] }> { + const rows = await this.queryFn(dbUrl, `SELECT n.nspname AS schema FROM pg_extension e + JOIN pg_namespace n ON n.oid = e.extnamespace WHERE e.extname = 'pg_partman'`) as Array<{ schema: string }> + const schema = rows[0]?.schema ?? null + if (schema === null) return { schema, sets: [] } + const sets = await this.queryFn(dbUrl, ` + SELECT (SELECT n.nspname || '.' || c.relname FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.oid = to_regclass(pc.parent_table)) AS parent_table, pc.control, pc.partition_interval, pc.partition_type, + pc.epoch, pc.premake, pc.automatic_maintenance, pc.constraint_cols, pc.jobmon, pc.date_trunc_interval, + pc.time_encoder, pc.time_decoder, + to_regclass(pc.parent_table || '_default') IS NOT NULL AS has_default, + (to_jsonb(pc) - ARRAY(SELECT k FROM jsonb_object_keys(to_jsonb(pc)) k WHERE k <> ALL($1::text[]))) AS settings + FROM ${quoteIdent(schema)}.part_config pc + WHERE to_regclass(pc.parent_table) IS NOT NULL + ORDER BY 1`, [SETTINGS]) as unknown as PartitionSet[] + return { schema, sets } + } +} + +const literal = (v: unknown): string => + v === null || v === undefined ? 'NULL' + : typeof v === 'boolean' || typeof v === 'number' ? String(v) + : Array.isArray(v) ? `ARRAY[${v.map(x => quoteLiteral(String(x))).join(', ')}]::text[]` + : quoteLiteral(String(v)) + +function registerSql(set: PartitionSet, schema: string): string { + const args = [ + `p_parent_table => ${quoteLiteral(set.parent_table)}`, + `p_control => ${quoteLiteral(set.control)}`, + `p_interval => ${quoteLiteral(set.partition_interval)}`, + `p_type => ${quoteLiteral(set.partition_type)}`, + `p_epoch => ${quoteLiteral(set.epoch)}`, + `p_premake => ${set.premake}`, + `p_default_table => ${set.has_default}`, + `p_automatic_maintenance => ${quoteLiteral(set.automatic_maintenance)}`, + `p_jobmon => ${set.jobmon}`, + ...(set.constraint_cols?.length ? [`p_constraint_cols => ${literal(set.constraint_cols)}`] : []), + ...(set.date_trunc_interval ? [`p_date_trunc_interval => ${quoteLiteral(set.date_trunc_interval)}`] : []), + ...(set.time_encoder ? [`p_time_encoder => ${quoteLiteral(set.time_encoder)}`] : []), + ...(set.time_decoder ? [`p_time_decoder => ${quoteLiteral(set.time_decoder)}`] : []), + ] + return `SELECT ${quoteIdent(schema)}.create_parent(${args.join(', ')});` +} + +function updateSql(schema: string, parent: string, settings: Record): string { + const sets = Object.entries(settings).map(([k, v]) => `${quoteIdent(k)} = ${literal(v)}`) + return `UPDATE ${quoteIdent(schema)}.part_config SET ${sets.join(', ')} WHERE parent_table = ${quoteLiteral(parent)};` +} + +function undoSql(schema: string, parent: string): string { + return `DELETE FROM ${quoteIdent(schema)}.part_config WHERE parent_table = ${quoteLiteral(parent)};` +} + +export function diffPartitionSets(source: PartitionSet[], target: PartitionSet[], targetSchema: string | null): DriftIssue[] { + const issues: DriftIssue[] = [] + const targetMap = new Map(target.map(s => [s.parent_table, s])) + const sourceNames = new Set(source.map(s => s.parent_table)) + + for (const s of source) { + const t = targetMap.get(s.parent_table) + if (!t) { + issues.push({ + id: `partitions-missing-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set not registered: ${s.parent_table}`, + description: `pg_partman manages ${s.parent_table} in source (${s.partition_type} by ${s.control}, every ${s.partition_interval}) but not in target.`, + sourceValue: s, + ...(targetSchema + ? { sql: { up: `${registerSql(s, targetSchema)}\n${updateSql(targetSchema, s.parent_table, s.settings)}`, down: undoSql(targetSchema, s.parent_table) } } + : { manualOnly: 'the target does not have pg_partman; install it, then re-run to register the partition set' }), + }) + continue + } + const reshaped = SHAPE.filter(k => String(s[k] ?? '') !== String(t[k] ?? '')) + if (reshaped.length > 0) { + issues.push({ + id: `partitions-shape-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set differs: ${s.parent_table}`, + description: `${reshaped.join(', ')} differ${reshaped.length === 1 ? 's' : ''} for ${s.parent_table}. pg_partman cannot change that in place.`, + sourceValue: s, + targetValue: t, + manualOnly: 'changing how a set is partitioned means undoing and recreating it with pg_partman (undo_partition, then create_parent)', + }) + continue + } + const changed = Object.fromEntries(Object.entries(s.settings).filter(([k, v]) => JSON.stringify(v) !== JSON.stringify(t.settings[k]))) + if (Object.keys(changed).length > 0 && targetSchema) { + const previous = Object.fromEntries(Object.keys(changed).map(k => [k, t.settings[k]])) + issues.push({ + id: `partitions-settings-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set settings differ: ${s.parent_table}`, + description: `${Object.keys(changed).join(', ')} differ for ${s.parent_table}.`, + sourceValue: s.settings, + targetValue: t.settings, + sql: { up: updateSql(targetSchema, s.parent_table, changed), down: updateSql(targetSchema, s.parent_table, previous) }, + }) + } + } + + for (const t of target) { + if (sourceNames.has(t.parent_table)) continue + issues.push({ + id: `partitions-extra-${t.parent_table}`, + check: 'partitions', + severity: 'info', + title: `Extra partition set: ${t.parent_table}`, + description: `pg_partman manages ${t.parent_table} in target but not in source.`, + targetValue: t, + manualOnly: 'unregistering a set leaves its partitions as ordinary tables; use pg_partman\'s undo_partition if that is what you want', + }) + } + return issues +} diff --git a/packages/cli/src/types/drift.ts b/packages/cli/src/types/drift.ts index 8a4e975..86c56d4 100644 --- a/packages/cli/src/types/drift.ts +++ b/packages/cli/src/types/drift.ts @@ -1,6 +1,6 @@ export const CHECK_NAMES = [ 'schema', 'rls', 'rls-coverage', 'edge-functions', 'storage', 'auth', 'cron', 'data', - 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', 'queues', + 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', 'queues', 'partitions', ] as const export type CheckName = (typeof CHECK_NAMES)[number] @@ -23,6 +23,7 @@ export const CHECK_META: Record = { 'migrations': 'posture', 'roles': 'comparison', 'queues': 'comparison', + 'partitions': 'comparison', } /** Checks that compare source against target. */ diff --git a/packages/cli/test/checks/partitions.test.ts b/packages/cli/test/checks/partitions.test.ts new file mode 100644 index 0000000..a57687b --- /dev/null +++ b/packages/cli/test/checks/partitions.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import { diffPartitionSets, type PartitionSet } from '../../src/checks/partitions.js' + +const set = (o: Partial = {}): PartitionSet => ({ + parent_table: 'public.events', control: 'created_at', partition_interval: '1 day', partition_type: 'range', + epoch: 'none', premake: 4, automatic_maintenance: 'on', constraint_cols: null, jobmon: true, + date_trunc_interval: null, time_encoder: null, time_decoder: null, has_default: true, + settings: { premake: 4, retention: null, retention_keep_table: true }, ...o, +}) + +describe('partition sets check', () => { + it('registers a set the target lacks through create_parent, with its settings', () => { + const [issue] = diffPartitionSets([set()], [], 'partman') + expect(issue.title).toBe('Partition set not registered: public.events') + expect(issue.sql!.up).toContain(`SELECT "partman".create_parent(p_parent_table => 'public.events', p_control => 'created_at', p_interval => '1 day'`) + expect(issue.sql!.up).toContain(`UPDATE "partman".part_config SET "premake" = 4, "retention" = NULL, "retention_keep_table" = true WHERE parent_table = 'public.events';`) + expect(issue.sql!.down).toBe(`DELETE FROM "partman".part_config WHERE parent_table = 'public.events';`) + }) + + it('updates only the settings that differ, and back', () => { + const [issue] = diffPartitionSets( + [set({ settings: { premake: 4, retention: '30 days', retention_keep_table: false } })], + [set()], 'partman') + expect(issue.sql).toEqual({ + up: `UPDATE "partman".part_config SET "retention" = '30 days', "retention_keep_table" = false WHERE parent_table = 'public.events';`, + down: `UPDATE "partman".part_config SET "retention" = NULL, "retention_keep_table" = true WHERE parent_table = 'public.events';`, + }) + }) + + it('leaves a change of interval, and a set only the target has, to the user', () => { + const issues = diffPartitionSets([set({ partition_interval: '1 week' })], [set(), set({ parent_table: 'public.logs' })], 'partman') + expect(issues.map(i => [i.title, Boolean(i.manualOnly)])).toEqual([ + ['Partition set differs: public.events', true], + ['Extra partition set: public.logs', true], + ]) + }) + + it('reports nothing for sets alike', () => { + expect(diffPartitionSets([set()], [set()], 'partman')).toEqual([]) + }) +}) From 572275d769ca2176d698b3ac9bfe1c4298fcfdb6 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:07:37 +0000 Subject: [PATCH 04/11] feat: say what each fix does to a live database MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fix can be correct and still take production down — an ALTER TABLE holds a lock that stops every read and write, a column type change may rewrite the table under it, a new CHECK scans every row — and a report showed a one-line fix and an hour-long outage the same way. Every fix now carries hazards, read from PostgreSQL's own parse of its SQL with the lock levels PostgreSQL documents: blocks reads and writes, blocks writes, may rewrite, scans every row (with how to avoid holding the lock: NOT VALID, then VALIDATE), cannot run in a transaction, and data loss. A table the same fix creates is not flagged. They show under each finding in the report and are in --json and over MCP. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 3 + packages/cli/src/hazards.ts | 131 ++++++++++++++++++++++++++++++ packages/cli/src/render.ts | 3 + packages/cli/src/scanner.ts | 13 ++- packages/cli/src/types/drift.ts | 5 ++ packages/cli/test/hazards.test.ts | 49 +++++++++++ 6 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/hazards.ts create mode 100644 packages/cli/test/hazards.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index acd675d..8909fd5 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -14,6 +14,9 @@ anything changes. With a single project, it snapshots, restores and clones it. - **Safe by default.** Every write needs `--apply`, and anything that loses data or opens up access also needs `--allow-destructive`. A failed apply leaves the target exactly as it was. +- **Production-aware.** Each fix says what it does to a live database: the + lock it holds and on which table, whether it may rewrite the table or scan + every row, whether it can't run in a transaction. - **Provable.** `--prove` rehearses the migration and refuses to apply one that wouldn't reproduce the source. - **Anywhere.** It works with Supabase Cloud, self-hosted stacks, plain diff --git a/packages/cli/src/hazards.ts b/packages/cli/src/hazards.ts new file mode 100644 index 0000000..fb62d20 --- /dev/null +++ b/packages/cli/src/hazards.ts @@ -0,0 +1,131 @@ +import { destructiveReason } from './dbdiff.js' +import { parseStatements, relationName, type Statement } from './sql-ast.js' + +/** + * What applying a fix does to a live database, beyond what it changes. + * + * A fix can be correct and still take production down: an ALTER TABLE holds + * a lock that stops every read and write while it runs, a column type change + * may rewrite the whole table under it, a new CHECK scans every row. None of + * that showed in a report, so a one-line fix and an hour-long outage looked + * the same. Read from PostgreSQL's own parse of the fix, per statement, from + * the lock levels PostgreSQL documents. A table the same fix creates has no + * one waiting on it, so it is not flagged. + */ +export type HazardKind = 'data-loss' | 'blocks-reads-and-writes' | 'blocks-writes' | 'rewrite' | 'full-scan' | 'not-transactional' + +export interface Hazard { + kind: HazardKind + /** The table it concerns, where there is one. */ + on?: string + /** For a person. */ + detail: string +} + +/** Defaults whose value differs per row, so adding the column writes every row. */ +const VOLATILE = new Set(['random', 'gen_random_uuid', 'uuid_generate_v4', 'uuid_generate_v1', 'clock_timestamp', 'nextval', 'timeofday']) + +const table = (rv: Record | undefined) => + rv ? (rv.schemaname ? `${rv.schemaname}.${relationName(rv)}` : relationName(rv)) : '' + +export function hazardsOf(sql: string): Hazard[] { + const out: Hazard[] = [] + const add = (h: Hazard) => { + if (!out.some(o => o.kind === h.kind && o.on === h.on)) out.push(h) + } + const loss = destructiveReason(sql) + if (loss) add({ kind: 'data-loss', detail: loss }) + + const statements = parseStatements(sql) + if (!statements) return out + const created = new Set(statements.filter(s => s.kind === 'CreateStmt').map(s => table(s.node.relation))) + const existing = (name: string) => name !== '' && !created.has(name) + + for (const s of statements) { + for (const h of statementHazards(s)) { + if (h.on === undefined || existing(h.on)) add(h) + } + } + return out +} + +function statementHazards(s: Statement): Hazard[] { + const n = s.node + switch (s.kind) { + case 'AlterTableStmt': return alterTableHazards(table(n.relation), n.cmds ?? []) + case 'IndexStmt': + return n.concurrent + ? [{ kind: 'not-transactional', on: table(n.relation), detail: `builds an index concurrently, which cannot run inside a transaction` }] + : [{ kind: 'blocks-writes', on: table(n.relation), detail: `blocks writes to ${table(n.relation)} while the index builds` }] + case 'CreateTrigStmt': + return [{ kind: 'blocks-writes', on: table(n.relation), detail: `blocks writes to ${table(n.relation)} briefly` }] + case 'CreatePolicyStmt': + case 'AlterPolicyStmt': + case 'RuleStmt': + return [{ kind: 'blocks-reads-and-writes', on: table(n.table ?? n.relation), detail: `blocks reads and writes on ${table(n.table ?? n.relation)} briefly` }] + case 'RenameStmt': + return n.relation ? [{ kind: 'blocks-reads-and-writes', on: table(n.relation), detail: `blocks reads and writes on ${table(n.relation)} briefly` }] : [] + case 'RefreshMatViewStmt': + return n.concurrent ? [] : [{ kind: 'blocks-reads-and-writes', on: table(n.relation), detail: `blocks reads of ${table(n.relation)} while it refreshes` }] + case 'VacuumStmt': + return [{ kind: 'not-transactional', detail: 'VACUUM cannot run inside a transaction' }] + case 'ReindexStmt': + return (n.params ?? []).some((p: any) => p.DefElem?.defname === 'concurrently') + ? [{ kind: 'not-transactional', detail: 'REINDEX CONCURRENTLY cannot run inside a transaction' }] : [] + default: + return [] + } +} + +function alterTableHazards(name: string, cmds: Array>): Hazard[] { + const out: Hazard[] = [] + let exclusive = false + let writesOnly = false + for (const { AlterTableCmd: cmd } of cmds) { + if (!cmd) continue + switch (cmd.subtype) { + // Lighter locks: other DDL waits, reads and writes go on. + case 'AT_SetStatistics': case 'AT_SetOptions': case 'AT_ResetOptions': case 'AT_SetRelOptions': + case 'AT_ResetRelOptions': case 'AT_ValidateConstraint': case 'AT_ClusterOn': case 'AT_DropCluster': + continue + case 'AT_AlterColumnType': + out.push({ kind: 'rewrite', on: name, detail: `may rewrite ${name}: a column's type changes, unless the new type is binary-compatible` }) + break + case 'AT_SetLogged': case 'AT_SetUnLogged': + out.push({ kind: 'rewrite', on: name, detail: `rewrites ${name}: LOGGED and UNLOGGED tables are stored differently` }) + break + case 'AT_AddColumn': { + const def = cmd.def?.ColumnDef + const constraints: any[] = def?.constraints ?? [] + const generated = constraints.some(c => c.Constraint?.contype === 'CONSTR_GENERATED') + const volatile = constraints.some(c => c.Constraint?.contype === 'CONSTR_DEFAULT' + && VOLATILE.has(String(c.Constraint.raw_expr?.FuncCall?.funcname?.at(-1)?.String?.sval ?? ''))) + if (generated || volatile) { + out.push({ kind: 'rewrite', on: name, detail: `rewrites ${name}: the new column's ${generated ? 'generated value' : 'default'} is computed for every row` }) + } + break + } + case 'AT_AddConstraint': { + const c = cmd.def?.Constraint + if (c && !c.skip_validation && (c.contype === 'CONSTR_CHECK' || c.contype === 'CONSTR_FOREIGN')) { + out.push({ kind: 'full-scan', on: name, detail: `scans every row of ${name} to check the new constraint (NOT VALID, then VALIDATE, avoids holding the lock)` }) + } + if (c?.contype === 'CONSTR_FOREIGN') { + writesOnly = true + continue + } + break + } + case 'AT_SetNotNull': + out.push({ kind: 'full-scan', on: name, detail: `scans every row of ${name} to check for NULLs` }) + break + } + exclusive = true + } + if (exclusive) { + out.unshift({ kind: 'blocks-reads-and-writes', on: name, detail: `blocks reads and writes on ${name} while it runs` }) + } else if (writesOnly) { + out.unshift({ kind: 'blocks-writes', on: name, detail: `blocks writes to ${name} while the key is checked` }) + } + return out +} diff --git a/packages/cli/src/render.ts b/packages/cli/src/render.ts index 4dc2a7e..56b3900 100644 --- a/packages/cli/src/render.ts +++ b/packages/cli/src/render.ts @@ -242,6 +242,9 @@ function formatIssue(issue: DriftIssue): string { const lines: string[] = [] lines.push(` ${severityIcon(issue.severity)} ${colorSeverity(issue.severity)} ${bold(issue.title)}`) lines.push(` ${dim(issue.description)}`) + for (const hazard of issue.hazards ?? []) { + lines.push(` ${warn(hazard.kind === 'data-loss' ? '⛔' : '⏳')} ${warn(hazard.detail)}`) + } if (issue.sql) { lines.push('') diff --git a/packages/cli/src/scanner.ts b/packages/cli/src/scanner.ts index d21a817..a2ffcff 100644 --- a/packages/cli/src/scanner.ts +++ b/packages/cli/src/scanner.ts @@ -11,6 +11,8 @@ import { computeScore, computePostureScore, summarize } from './scoring' import type { Check, CheckContext } from './checks/base' import { isCheckSkipped } from './checks/base' import { friendlyDbError } from './utils/error' +import { hazardsOf } from './hazards.js' +import type { DriftIssue } from './types/drift.js' export type ScanProgressEvent = | { phase: 'check:start'; check: CheckName; index: number; total: number } @@ -83,6 +85,15 @@ function checkConcurrency(): number { return 4 } +/** Each fix with what applying it does to a live database — see hazards.ts. */ +function withHazards(issues: DriftIssue[]): DriftIssue[] { + return issues.map(issue => { + if (!issue.sql?.up) return issue + const hazards = hazardsOf(issue.sql.up) + return hazards.length > 0 ? { ...issue, hazards } : issue + }) +} + async function runCheck( check: Check, ctx: CheckContext, @@ -91,7 +102,7 @@ async function runCheck( ): Promise { const start = performance.now() try { - const issues = await check.scan(ctx) + const issues = withHazards(await check.scan(ctx)) return { check: name, status: issues.length > 0 ? 'drifted' : 'clean', diff --git a/packages/cli/src/types/drift.ts b/packages/cli/src/types/drift.ts index 86c56d4..a0c31d2 100644 --- a/packages/cli/src/types/drift.ts +++ b/packages/cli/src/types/drift.ts @@ -111,6 +111,11 @@ export interface DriftIssue { * its SQL: a webhook is a trigger, and dropping a trigger usually isn't. */ destructive?: string + /** + * What applying the fix does to a live database beyond what it changes — + * the locks it holds, a table rewrite, a full scan. See hazards.ts. + */ + hazards?: Array<{ kind: string; on?: string; detail: string }> } export interface CheckResult { diff --git a/packages/cli/test/hazards.test.ts b/packages/cli/test/hazards.test.ts new file mode 100644 index 0000000..5c344ad --- /dev/null +++ b/packages/cli/test/hazards.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' +import { hazardsOf } from '../src/hazards.js' + +const kinds = (sql: string) => hazardsOf(sql).map(h => `${h.kind}${h.on ? ' ' + h.on : ''}`) + +describe('hazardsOf', () => { + it('a column type change locks the table and may rewrite it', () => { + expect(kinds('ALTER TABLE "public"."orders" ALTER COLUMN "total" TYPE numeric(12,2);')) + .toEqual(['blocks-reads-and-writes public.orders', 'rewrite public.orders']) + }) + + it('a new CHECK scans the table, unless NOT VALID', () => { + expect(kinds('ALTER TABLE orders ADD CONSTRAINT pos CHECK (total > 0);')).toEqual(['blocks-reads-and-writes orders', 'full-scan orders']) + expect(kinds('ALTER TABLE orders ADD CONSTRAINT pos CHECK (total > 0) NOT VALID;')).toEqual(['blocks-reads-and-writes orders']) + }) + + it('a foreign key blocks writes only, and scans', () => { + expect(kinds('ALTER TABLE orders ADD CONSTRAINT fk FOREIGN KEY (c) REFERENCES customers (id);')) + .toEqual(['blocks-writes orders', 'full-scan orders']) + }) + + it('a column with a volatile default rewrites; a constant default does not', () => { + expect(kinds('ALTER TABLE t ADD COLUMN id uuid DEFAULT gen_random_uuid();')).toEqual(['blocks-reads-and-writes t', 'rewrite t']) + expect(kinds("ALTER TABLE t ADD COLUMN state text DEFAULT 'new';")).toEqual(['blocks-reads-and-writes t']) + }) + + it('statistics and storage options take no lock reads or writes wait on', () => { + expect(kinds('ALTER TABLE t ALTER COLUMN v SET STATISTICS 500;')).toEqual([]) + }) + + it('an index blocks writes; built concurrently, it cannot run in a transaction', () => { + expect(kinds('CREATE INDEX i ON orders (total);')).toEqual(['blocks-writes orders']) + expect(kinds('CREATE INDEX CONCURRENTLY i ON orders (total);')).toEqual(['not-transactional orders']) + }) + + it('a table the same fix creates is nobody\'s to wait on', () => { + expect(kinds('CREATE TABLE fresh (id int);\nALTER TABLE fresh ADD CONSTRAINT c CHECK (id > 0);\nCREATE INDEX f ON fresh (id);')).toEqual([]) + }) + + it('names data loss', () => { + expect(hazardsOf('DROP TABLE "old";')[0]).toMatchObject({ kind: 'data-loss', detail: 'drops a table and its rows' }) + }) + + it('policies, triggers and renames', () => { + expect(kinds('CREATE POLICY p ON orders USING (true);')).toEqual(['blocks-reads-and-writes orders']) + expect(kinds('CREATE TRIGGER t BEFORE INSERT ON orders FOR EACH ROW EXECUTE FUNCTION f();')).toEqual(['blocks-writes orders']) + expect(kinds('ALTER TABLE "people" RENAME COLUMN "name" TO "full_name";')).toEqual(['blocks-reads-and-writes people']) + }) +}) From 7df5475c4e7003b9f68511b3591463e35558d8c6 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:15:21 +0000 Subject: [PATCH 05/11] feat: --prove checks that the rows survive, and names the tables rewritten MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A schema can converge and the data not — a table recreated, a cast that loses values — and the proof's copy held no rows, so none of that showed. The copy now gets a sample of each table's rows first (the first 1000, SUPAFORGE_PROVE_ROWS to change it, 0 for structure only), piped from the target with psql's \copy, foreign keys set aside where the role may. After the fixes, a table they keep must hold as many rows, with the same values in each column whose type did not change — unless a fix deletes from or truncates it — or the proof fails, naming the table. A table whose sample would not load is listed as unchecked. A table whose storage the fixes replaced on the copy is named as rewritten: on the real target, a long lock. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 15 ++- packages/cli/src/commands/diff.ts | 9 ++ packages/cli/src/prove-data.ts | 166 +++++++++++++++++++++++++++ packages/cli/src/prove.ts | 23 +++- packages/cli/test/prove-data.test.ts | 41 +++++++ 5 files changed, 247 insertions(+), 7 deletions(-) create mode 100644 packages/cli/src/prove-data.ts create mode 100644 packages/cli/test/prove-data.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index 8909fd5..f92b194 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -18,7 +18,7 @@ anything changes. With a single project, it snapshots, restores and clones it. lock it holds and on which table, whether it may rewrite the table or scan every row, whether it can't run in a transaction. - **Provable.** `--prove` rehearses the migration and refuses to apply one - that wouldn't reproduce the source. + that wouldn't reproduce the source or would lose rows. - **Anywhere.** It works with Supabase Cloud, self-hosted stacks, plain PostgreSQL targets, CI pipelines and AI agents (MCP). @@ -134,11 +134,13 @@ Useful `diff` flags: is unchanged. A fix the target can't hold, because it needs an extension or a Supabase schema the target lacks, is skipped with the reason, and the rest still applies. -- **`--prove`.** The fixes are replayed on a structure-only copy of the target, - made on the target's own server, the way the apply will run them. Nothing is - applied unless the result matches the source, and nothing is applied if the - proof can't run. Changes a copy can't hold, such as cron jobs (pg_cron lives - in one database per server), are listed as not proved. +- **`--prove`.** The fixes are replayed on a copy of the target's structure, + made on the target's own server, the way the apply will run them, with a + sample of each table's rows. Nothing is applied unless the result matches + the source and the rows survive, and nothing is applied if the proof can't + run. It also names the tables the fixes rewrite. Changes a copy can't hold, + such as cron jobs (pg_cron lives in one database per server), are listed as + not proved. ```bash supaforge diff --dry-run # the plan, in order @@ -265,6 +267,7 @@ supaforge migrate run --env=prod --apply | `SUPAFORGE_DBDIFF_MEMORY` | `1G` | Memory limit for `@dbdiff/cli` (`2G`, `-1` for none) | | `SUPAFORGE_CONNECT_TIMEOUT` | `15` | Seconds to wait for a connection | | `SUPAFORGE_CHECK_CONCURRENCY` | `4` | Checks run at once. Lower it behind a tight pooler | +| `SUPAFORGE_PROVE_ROWS` | `1000` | Rows of each table `--prove` copies to check they survive; `0` checks structure only | | `SUPAFORGE_PG_BIN` | — | A directory of PostgreSQL client tools (`pg_dump`, `psql`), used ahead of `PATH` — for a server newer than the tools on `PATH` | ## Self-hosted Supabase diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index fbf7fd9..17bf763 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -466,6 +466,15 @@ export default class Diff extends BaseCommand { this.log(` ${ok('Converged')} — the migration reproduces the source exactly.\n`) } } + if (proof.data && !proof.skipped) { + const { sampled, unchecked, rewritten } = proof.data + if (sampled.length > 0) this.log(` ${ok('Rows kept')} in the ${sampled.length} table(s) sampled.`) + if (rewritten.length > 0) { + this.log(` ${warn('Rewritten')} on the copy — a long lock on the real target: ${rewritten.join(', ')}`) + } + if (unchecked.length > 0) this.log(` ${dim(`Rows not checked (the sample would not load): ${unchecked.join(', ')}`)}`) + this.log('') + } } const result = await promote({ diff --git a/packages/cli/src/prove-data.ts b/packages/cli/src/prove-data.ts new file mode 100644 index 0000000..f813cd7 --- /dev/null +++ b/packages/cli/src/prove-data.ts @@ -0,0 +1,166 @@ +import { spawn } from 'node:child_process' +import { pgQuery } from './db' +import { parseStatements, relationName } from './sql-ast.js' +import { quoteIdent, quoteLiteral } from './utils/sql.js' + +/** + * The data half of a proof: that the rows survive the fixes. + * + * A schema can converge and the data not: a column dropped and added again + * under a new name, a table recreated, a type changed with a lossy cast. The + * proof's copy held no rows, so none of that showed. Now a bounded sample of + * the target's rows — the first `rows` of each table — is copied into it + * first. After the fixes, a table they keep must hold as many rows, and the + * same ones where its columns are unchanged — unless a fix deletes from or + * truncates it. A table rewritten on the copy (its storage replaced) is said + * so too: on the real target that is a long lock. + */ +export interface TableSnapshot { + rows: number + /** Per column, by name: its type and a hash of its values (as a multiset). */ + columns: Record + filenode: string +} + +export interface DataProof { + /** Tables sampled, and those whose sample could not be loaded (left unchecked). */ + sampled: string[] + unchecked: string[] + /** Kept tables that lost rows or whose rows changed, and how. */ + lost: string[] + /** Tables whose storage the fixes replaced on the copy. */ + rewritten: string[] +} + +export const DEFAULT_SAMPLE_ROWS = 1000 + +/** How many rows of each table to sample, from SUPAFORGE_PROVE_ROWS; 0 turns it off. */ +export function sampleRows(): number { + const raw = process.env.SUPAFORGE_PROVE_ROWS + const n = raw === undefined ? DEFAULT_SAMPLE_ROWS : Number(raw) + return Number.isInteger(n) && n >= 0 ? n : DEFAULT_SAMPLE_ROWS +} + +const TABLES_SQL = ` + SELECT n.nspname || '.' || c.relname AS name, quote_ident(n.nspname) || '.' || quote_ident(c.relname) AS ident, + (SELECT string_agg(quote_ident(a.attname), ', ' ORDER BY a.attnum) FROM pg_attribute a + WHERE a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped AND a.attgenerated = '') AS columns + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relkind = 'r' AND NOT c.relispartition AND n.nspname = ANY($1::text[]) + ORDER BY 1` + +/** Copy up to `limit` rows of each table from `fromUrl` into the same tables at `intoUrl`. */ +export async function sampleInto(psql: string, fromUrl: string, intoUrl: string, schemas: string[], limit: number): Promise<{ sampled: string[]; unchecked: string[] }> { + const tables = await pgQuery(fromUrl, TABLES_SQL, [schemas]) as Array<{ name: string; ident: string; columns: string | null }> + const sampled: string[] = [] + const unchecked: string[] = [] + for (const t of tables) { + if (!t.columns) continue + try { + await pipeCopy(psql, fromUrl, intoUrl, + `\\copy (SELECT ${t.columns} FROM ${t.ident} LIMIT ${limit}) TO STDOUT`, + `\\copy ${t.ident} (${t.columns}) FROM STDIN`) + sampled.push(t.name) + } catch { + // A sample that will not load — a row whose foreign key points at one + // not sampled, where triggers cannot be set aside — is left unchecked. + unchecked.push(t.name) + } + } + return { sampled, unchecked } +} + +function pipeCopy(psql: string, fromUrl: string, intoUrl: string, out: string, into: string): Promise { + return new Promise((resolve, reject) => { + const reader = spawn(psql, [fromUrl, '-X', '-q', '-v', 'ON_ERROR_STOP=1', '-c', out], { stdio: ['ignore', 'pipe', 'pipe'] }) + const writer = spawn(psql, [intoUrl, '-X', '-q', '-v', 'ON_ERROR_STOP=1', '-f', '-'], { stdio: ['pipe', 'pipe', 'pipe'] }) + let stderr = '' + reader.stderr.on('data', (d: Buffer) => { stderr += d.toString() }) + writer.stderr.on('data', (d: Buffer) => { stderr += d.toString() }) + reader.on('error', reject) + writer.on('error', reject) + writer.stdin.on('error', () => undefined) + // Foreign keys and triggers set aside where the role may (a superuser), + // so a sampled row need not find its parent sampled too; then the COPY, + // whose rows follow on the same stream until it ends. + writer.stdin.write(`\\set ON_ERROR_STOP 0\nSET session_replication_role = replica;\n\\set ON_ERROR_STOP 1\n${into}\n`) + reader.stdout.pipe(writer.stdin) + let done = 0 + let failed = false + const finish = (code: number | null) => { + if (code !== 0) failed = true + if (++done === 2) failed ? reject(new Error(stderr.trim() || 'copy failed')) : resolve() + } + reader.on('close', finish) + writer.on('close', finish) + }) +} + +/** Each table's row count, a hash of each column's values, and its storage. */ +export async function snapshotTables(dbUrl: string, schemas: string[]): Promise> { + const rows = await pgQuery(dbUrl, ` + SELECT n.nspname || '.' || c.relname AS name, quote_ident(n.nspname) || '.' || quote_ident(c.relname) AS ident, + json_agg(json_build_array(a.attname, format_type(a.atttypid, a.atttypmod)) ORDER BY a.attnum) AS columns + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + JOIN pg_attribute a ON a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped + WHERE c.relkind = 'r' AND NOT c.relispartition AND n.nspname = ANY($1::text[]) + GROUP BY 1, 2 ORDER BY 1`, [schemas]) as Array<{ name: string; ident: string; columns: Array<[string, string]> }> + const out = new Map() + for (const t of rows) { + // Each value hashed, a NULL as 'n' (no hash is one letter long). + const hashes = t.columns.map(([col], i) => { + const value = `coalesce(md5(x.${quoteIdent(col)}::text), 'n')` + return `coalesce(md5(string_agg(${value}, '' ORDER BY ${value})), '') AS h${i}` + }) + const [row] = await pgQuery(dbUrl, + `SELECT count(*)::text AS rows, pg_relation_filenode(${quoteLiteral(t.ident)}::regclass)::text AS filenode${hashes.length ? ', ' + hashes.join(', ') : ''} + FROM ${t.ident} x`) as Array> + out.set(t.name, { + rows: Number(row.rows), + filenode: row.filenode, + columns: Object.fromEntries(t.columns.map(([col, type], i) => [col, { type, hash: row[`h${i}`] }])), + }) + } + return out +} + +/** The tables a fix deletes from or empties, whose rows are meant to go. */ +export function emptiedTables(sql: string): Set { + const out = new Set() + for (const s of parseStatements(sql) ?? []) { + if (s.kind === 'DeleteStmt') out.add(relationName(s.node.relation)) + if (s.kind === 'TruncateStmt') for (const r of s.node.relations ?? []) out.add(relationName(r.RangeVar)) + } + return out +} + +/** + * What became of the sampled rows: fewer of them in a table the fixes keep, + * or, in a column that is there before and after with the same type, + * different values. + */ +export function compareTables(before: Map, after: Map, sampled: string[], migrationSql: string): Pick { + const emptied = emptiedTables(migrationSql) + const lost: string[] = [] + const rewritten: string[] = [] + for (const name of sampled) { + const b = before.get(name) + const a = after.get(name) + if (!b || !a) continue + if (b.filenode !== a.filenode) rewritten.push(name) + if (emptied.has(name.slice(name.indexOf('.') + 1))) continue + if (a.rows < b.rows) { + lost.push(`${name}: ${b.rows} row(s) before, ${a.rows} after`) + continue + } + if (a.rows !== b.rows) continue + const changed = Object.entries(b.columns) + .filter(([col, c]) => a.columns[col] && a.columns[col].type === c.type && a.columns[col].hash !== c.hash) + .map(([col]) => col) + if (changed.length > 0) { + lost.push(`${name}: values changed in ${changed.join(', ')}`) + } + } + return { lost, rewritten } +} + diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index 11267a6..b3c0f77 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -34,6 +34,7 @@ import { splitSqlStatements, isCommentOnly } from './utils/sql-split.js' import { quoteName } from './utils/sql.js' import { globToRegExp } from './utils/strings.js' import { join, dirname } from 'node:path' +import { compareTables, sampleInto, sampleRows, snapshotTables, type DataProof } from './prove-data.js' const exec = promisify(execFile) @@ -87,6 +88,11 @@ export interface ProofResult { * outsideTheRun(). They do not count against convergence. */ outOfScope?: string[] + /** + * What became of a sample of the target's rows — see prove-data.ts. A + * kept table that lost rows fails the proof. + */ + data?: DataProof } /** Replace the database name in a libpq URL, keeping everything else. */ @@ -278,6 +284,11 @@ export async function proveConvergence(opts: { const scoped = opts.checks !== undefined && !opts.checks.includes('schema') const before = scoped ? await schemaState(cloneUrl, schemas) : undefined + // A sample of the target's rows, to see that they survive. + const limit = sampleRows() + const sample = limit > 0 ? await sampleInto(psql, opts.targetUrl, cloneUrl, schemas, limit) : undefined + const tablesBefore = sample ? await snapshotTables(cloneUrl, schemas) : undefined + // The migration under test. let notApplied: ProofResult['notApplied'] if (opts.fixes) { @@ -297,7 +308,17 @@ export async function proveConvergence(opts: { } else { await runSql(psql, cloneUrl, opts.migrationSql) } - const reported = (r: ProofResult): ProofResult => (notApplied?.length ? { ...r, notApplied } : r) + let data: DataProof | undefined + if (sample && tablesBefore) { + data = { ...sample, ...compareTables(tablesBefore, await snapshotTables(cloneUrl, schemas), sample.sampled, opts.migrationSql) } + if (data.lost.length > 0) { + return { converged: false, cloneName, data, residual: data.lost.map(l => `rows lost in ${l}`) } + } + } + const reported = (r: ProofResult): ProofResult => { + const withData = data ? { ...r, data } : r + return notApplied?.length ? { ...withData, notApplied } : withData + } const [want, got] = await Promise.all([ fingerprint(opts.sourceUrl, schemas), diff --git a/packages/cli/test/prove-data.test.ts b/packages/cli/test/prove-data.test.ts new file mode 100644 index 0000000..25a342e --- /dev/null +++ b/packages/cli/test/prove-data.test.ts @@ -0,0 +1,41 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { compareTables, emptiedTables, sampleRows, type TableSnapshot } from '../src/prove-data.js' + +const snap = (rows: number, columns: Record, filenode = '1'): TableSnapshot => ({ + rows, filenode, columns: Object.fromEntries(Object.entries(columns).map(([k, [type, hash]]) => [k, { type, hash }])), +}) + +describe('the data half of a proof', () => { + afterEach(() => { delete process.env.SUPAFORGE_PROVE_ROWS }) + + it('passes rows kept as they were, and says which tables were rewritten', () => { + const before = new Map([['public.o', snap(3, { id: ['integer', 'a'], note: ['text', 'b'] }, '10')]]) + const after = new Map([['public.o', snap(3, { id: ['bigint', 'changed-text'], note: ['text', 'b'] }, '11')]]) + expect(compareTables(before, after, ['public.o'], 'ALTER TABLE o ALTER COLUMN id TYPE bigint;')) + .toEqual({ lost: [], rewritten: ['public.o'] }) + }) + + it('catches rows lost, and values changed in a column whose type did not', () => { + const before = new Map([['public.a', snap(5, { x: ['text', 'h'] })], ['public.b', snap(2, { y: ['text', 'h1'] })]]) + const after = new Map([['public.a', snap(0, { x: ['text', ''] })], ['public.b', snap(2, { y: ['text', 'h2'] })]]) + expect(compareTables(before, after, ['public.a', 'public.b'], '').lost).toEqual([ + 'public.a: 5 row(s) before, 0 after', + 'public.b: values changed in y', + ]) + }) + + it('expects rows to go from a table a fix deletes from or truncates', () => { + expect([...emptiedTables('DELETE FROM "public"."plans" WHERE id = 1;\nTRUNCATE flags;')]).toEqual(['plans', 'flags']) + const before = new Map([['public.plans', snap(5, { id: ['integer', 'h'] })]]) + const after = new Map([['public.plans', snap(4, { id: ['integer', 'h2'] })]]) + expect(compareTables(before, after, ['public.plans'], 'DELETE FROM plans WHERE id = 1;').lost).toEqual([]) + }) + + it('samples a thousand rows a table unless told otherwise, and none for 0', () => { + expect(sampleRows()).toBe(1000) + process.env.SUPAFORGE_PROVE_ROWS = '0' + expect(sampleRows()).toBe(0) + process.env.SUPAFORGE_PROVE_ROWS = 'lots' + expect(sampleRows()).toBe(1000) + }) +}) From 2e910663184a22ecfb0927f7b5a1676cfc824d97 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:18:24 +0000 Subject: [PATCH 06/11] feat: migrate run refuses a migration written against a target that has since changed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A migration file runs later than it is written — after review, after a merge — against a target that may have moved on: a column added by hand, another migration run. Applied then, it could fail half way, or succeed and leave something wrong. migrate create now records the target environment and a digest of its structure in the file. migrate run checks the first pending migration written against the same environment before running anything: if the target has changed since, it says so and applies nothing, unless --force. The migrations after it follow from it, so they are not checked. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/commands/migrate/create.ts | 4 +++ packages/cli/src/commands/migrate/run.ts | 31 ++++++++++++++++++++ packages/cli/src/prove.ts | 2 +- packages/cli/src/written-against.ts | 32 +++++++++++++++++++++ packages/cli/test/written-against.test.ts | 13 +++++++++ 5 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/written-against.ts create mode 100644 packages/cli/test/written-against.test.ts diff --git a/packages/cli/src/commands/migrate/create.ts b/packages/cli/src/commands/migrate/create.ts index b26a7e0..1e85d60 100644 --- a/packages/cli/src/commands/migrate/create.ts +++ b/packages/cli/src/commands/migrate/create.ts @@ -8,6 +8,7 @@ import { ok, warn, dim, bold, cmd } from '../../ui.js' import { splitSqlStatements } from '../../utils/sql-split.js' import { truncateSql } from './run.js' import { slugify } from '../../utils/strings.js' +import { targetFingerprint, writtenAgainstLine } from '../../written-against.js' /** * Generate a new migration file from the current schema drift. @@ -78,10 +79,13 @@ export default class MigrateCreate extends BaseCommand { const destructive = splitSqlStatements(result.up).filter(isDestructiveSql) // Build file content with UP and optional DOWN + // The target as the file was written against it, for migrate run to check. + const digest = await targetFingerprint(targetEnv.dbUrl, ignoreSchemas).catch(() => undefined) const lines: string[] = [ `-- Migration: ${flags.name}`, `-- Generated by supaforge migrate create`, `-- Source: ${config.source!} → Target: ${config.target!}`, + ...(digest ? [writtenAgainstLine(config.target!, digest)] : []), ...(destructive.length > 0 ? [`-- WARNING: ${destructive.length} destructive statement(s) — drops or deletes data. Review before running.`] : []), diff --git a/packages/cli/src/commands/migrate/run.ts b/packages/cli/src/commands/migrate/run.ts index 360eead..f1f7976 100644 --- a/packages/cli/src/commands/migrate/run.ts +++ b/packages/cli/src/commands/migrate/run.ts @@ -11,6 +11,8 @@ import { isDestructiveSql } from '../../dbdiff.js' import { splitSqlStatements } from '../../utils/sql-split.js' import { readFile } from 'node:fs/promises' import { errMsg } from '../../utils/error.js' +import { DEFAULT_IGNORE_SCHEMAS } from '../../defaults.js' +import { targetFingerprint, writtenAgainst } from '../../written-against.js' /** * Execute pending migrations against a Supabase environment. @@ -56,6 +58,10 @@ export default class MigrateRun extends BaseCommand { description: 'Permit migrations that drop tables or columns, delete rows, or drop policies', default: false, }), + force: Flags.boolean({ + description: 'Run a migration even though the target has changed since it was written against it', + default: false, + }), } async run(): Promise { @@ -116,6 +122,19 @@ export default class MigrateRun extends BaseCommand { } } + // The first pending migration written against this environment is + // checked against the target as it is now; the ones after it follow it. + const stale = await this.changedSinceWritten(pending, envName, env.dbUrl, config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS) + if (stale) { + this.log('') + this.log(warn(`${stale} was written against ${envName} as it was then, and ${envName} has changed since.`)) + this.log(` ${dim('Re-create it with migrate create, or pass --force to run it anyway.')}`) + if (flags.apply && !flags.force) { + this.log(`\n${warn('Nothing was applied.')}`) + this.exit(1) + } + } + if (!flags.apply) { this.log(`\n${dim('Preview only — nothing was applied.')} Re-run with ${bold('--apply')} to execute` + (destructive.length > 0 ? `, and ${bold('--allow-destructive')} for the statements above.` : '.')) @@ -145,6 +164,18 @@ export default class MigrateRun extends BaseCommand { this.log(`\n${ok(`Applied ${applied_count} migration(s) successfully.`)} ✓`) } + + /** The first pending migration written against `envName`, if the target has changed since. */ + private async changedSinceWritten( + pending: Array<{ filename: string; path: string }>, envName: string, dbUrl: string, ignoreSchemas: readonly string[], + ): Promise { + const [first] = pending + if (!first) return undefined + const header = writtenAgainst(await readFile(first.path, 'utf-8').catch(() => '')) + if (!header || header.env !== envName) return undefined + const now = await targetFingerprint(dbUrl, ignoreSchemas).catch(() => header.digest) + return now === header.digest ? undefined : first.filename + } } /** diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index b3c0f77..d8e74f3 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -116,7 +116,7 @@ function withDatabase(dbUrl: string, database: string): string { * lives in one place that both projects depend on. */ -async function fingerprint(dbUrl: string, schemas: string[]): Promise { +export async function fingerprint(dbUrl: string, schemas: string[]): Promise { const rows = await pgQuery(dbUrl, fingerprintSql(schemas)) as unknown as Array<{ fingerprint: string | null }> diff --git a/packages/cli/src/written-against.ts b/packages/cli/src/written-against.ts new file mode 100644 index 0000000..a42648f --- /dev/null +++ b/packages/cli/src/written-against.ts @@ -0,0 +1,32 @@ +import { createHash } from 'node:crypto' +import { comparedSchemas, fingerprint } from './prove.js' + +/** + * Which target a migration file was written against, and as it was then. + * + * `migrate create` turns the drift between two databases into a file that + * runs later — after review, after a merge — against a target that may have + * moved on: a column added by hand, another migration run. Applied then, a + * file written for the old state could fail half way, or succeed and leave + * something wrong. The target's schema fingerprint is recorded in the file, + * and `migrate run` checks it before running the first pending migration + * written against the same environment. + */ +export const WRITTEN_AGAINST = '-- supaforge:written-against' + +/** A short digest of the target's structure, over the schemas a diff compares. */ +export async function targetFingerprint(dbUrl: string, ignoreSchemas: readonly string[]): Promise { + const schemas = await comparedSchemas(dbUrl, dbUrl, ignoreSchemas) + return createHash('sha256').update(await fingerprint(dbUrl, schemas)).digest('hex').slice(0, 16) +} + +export function writtenAgainstLine(env: string, digest: string): string { + return `${WRITTEN_AGAINST} ${env} ${digest}` +} + +/** The environment and digest a migration's header records, if it has one. */ +export function writtenAgainst(sql: string): { env: string; digest: string } | undefined { + const line = sql.split('\n').find(l => l.startsWith(WRITTEN_AGAINST + ' ')) + const [env, digest] = line?.slice(WRITTEN_AGAINST.length + 1).trim().split(/\s+/) ?? [] + return env && digest ? { env, digest } : undefined +} diff --git a/packages/cli/test/written-against.test.ts b/packages/cli/test/written-against.test.ts new file mode 100644 index 0000000..46de5de --- /dev/null +++ b/packages/cli/test/written-against.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from 'vitest' +import { writtenAgainst, writtenAgainstLine } from '../src/written-against.js' + +describe('written-against header', () => { + it('reads back what migrate create writes', () => { + const sql = `-- Migration: x\n${writtenAgainstLine('production', 'abc123')}\n\nCREATE TABLE t (id int);` + expect(writtenAgainst(sql)).toEqual({ env: 'production', digest: 'abc123' }) + }) + + it('is absent from a migration written by hand', () => { + expect(writtenAgainst('CREATE TABLE t (id int);')).toBeUndefined() + }) +}) From 2207123ef880344f1ca9bd46fd7f94b77324a75e Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:21:15 +0000 Subject: [PATCH 07/11] fix: explain a restore that runs out of locks In one transaction, every table, index and sequence a restore creates holds a lock until the commit, and the server keeps a fixed number of them. A large project's restore ran out half way, rolled back, and said only "out of shared memory". It now says why, and the two ways round it: raise max_locks_per_transaction, or --no-transaction to commit as it goes. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/restore.ts | 24 ++++++++++++++++++++++-- packages/cli/test/restore.test.ts | 10 ++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/restore.ts b/packages/cli/src/restore.ts index a971dbb..cbb48eb 100644 --- a/packages/cli/src/restore.ts +++ b/packages/cli/src/restore.ts @@ -307,7 +307,7 @@ export async function restoreFromSnapshot(options: RestoreOptions): Promise { expect(isCommentOnly('-- add the column\nALTER TABLE t ADD COLUMN c text;')).toBe(false) }) }) + +describe('restoreError', () => { + it('explains running out of locks, and the two ways round it', async () => { + const { restoreError } = await import('../src/restore.js') + const err = Object.assign(new Error('out of shared memory'), { code: '53200' }) + expect(restoreError(err, true)).toMatch(/more locks than the server allows in one transaction.*max_locks_per_transaction.*--no-transaction/) + expect(restoreError(err, false)).toBe('out of shared memory') + expect(restoreError(new Error('syntax error'), true)).toBe('syntax error') + }) +}) From 14ea142d6d948b19e1560a24c5c85d0fdf7d6bf0 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 08:27:43 +0000 Subject: [PATCH 08/11] feat: diff --source-dir compares a directory of schema files with the target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declarative schemas keep the desired state as SQL files (Supabase's supabase/schemas). diff --source-dir loads them into a throwaway database on the target's server — on top of the target's platform schemas, so a reference to auth.users or a call to auth.uid() resolves, as --prove does — and compares that with the target: schema, RLS and extensions unless --check says otherwise. Files load in name order, and a statement needing something a later file creates is tried again once the rest have run. What the platform owns and the copy cannot hold (pg_cron lives in one database per server) is not reported as extra. The throwaway database is dropped afterwards, whatever happens. New config field ignoreExtensions: extensions not reported as extra. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 1 + packages/cli/src/checks/extensions.ts | 3 +- packages/cli/src/commands/diff.ts | 47 ++++++++- packages/cli/src/prove.ts | 12 +-- packages/cli/src/schema-dir.ts | 127 +++++++++++++++++++++++ packages/cli/src/types/config.ts | 2 + packages/cli/test/e2e/schema-dir.test.ts | 47 +++++++++ 7 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/schema-dir.ts create mode 100644 packages/cli/test/e2e/schema-dir.test.ts diff --git a/packages/cli/README.md b/packages/cli/README.md index f92b194..6b729d1 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -114,6 +114,7 @@ Useful `diff` flags: | `--only=` | Apply only these findings (ids come from `--json`, globs allowed) | | `--dry-run` | Print the fixes in the order they'd run, run nothing | | `--prove` | Rehearse the fixes on a throwaway copy first | +| `--source-dir ` | Compare a directory of schema files (such as `supabase/schemas`) with the target instead of a source database | | `--allow-destructive` | Also apply drops and deletes | | `--apply-posture` | Also apply the target-only checks' fixes | | `--no-transaction` | Apply fix by fix, keeping what works | diff --git a/packages/cli/src/checks/extensions.ts b/packages/cli/src/checks/extensions.ts index 055bab9..4c791a4 100644 --- a/packages/cli/src/checks/extensions.ts +++ b/packages/cli/src/checks/extensions.ts @@ -22,7 +22,8 @@ export class ExtensionsCheck extends Check { this.fetchExtensions(ctx.source.dbUrl), this.fetchExtensions(ctx.target.dbUrl), ]) - return diffExtensions(source, target) + const ignored = new Set(ctx.config.ignoreExtensions ?? []) + return diffExtensions(source, target.filter(e => !ignored.has(e.name) || source.some(s => s.name === e.name))) } private async fetchExtensions(dbUrl: string): Promise { diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index 17bf763..06e9410 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -1,4 +1,4 @@ -import { Flags } from '@oclif/core' +import { Flags, type Interfaces } from '@oclif/core' import { BaseCommand } from '../base-command.js' import { createDefaultRegistry } from '../checks/index.js' import { scan } from '../scanner.js' @@ -20,6 +20,14 @@ import { CLONE_SKIP_FLAGS, DEFAULT_IGNORE_SCHEMAS, schemaDiffIgnores } from '../ import { Preflight } from '../preflight.js' import type { SupaForgeConfig } from '../types/config.js' import { redactUrls, describeFailure } from '../utils/error.js' +import { openSchemaDir } from '../schema-dir.js' + + +/** The source environment a --source-dir run compares from. */ +const SCHEMA_FILES_ENV = 'schema files' + +/** What schema files can describe: the database's structure, its policies, its extensions. */ +const SCHEMA_FILE_CHECKS: CheckName[] = ['schema', 'rls', 'extensions'] /** * Glyph and text for a finished check, so the three outcomes are visually @@ -136,6 +144,11 @@ export default class Diff extends BaseCommand { ] static override flags = { + 'source-dir': Flags.string({ + description: 'Compare a directory of schema files (such as supabase/schemas) with the target, ' + + 'instead of a source database. They are loaded into a throwaway database on the target\'s server.', + exclusive: ['source'], + }), check: Flags.string({ char: 'l', description: 'Limit to a specific check', @@ -286,10 +299,40 @@ export default class Diff extends BaseCommand { } const config = await this.loadConfigOrFail() + + // Schema files as the source: loaded into a throwaway database on the + // target's server, compared, and dropped — see schema-dir.ts. + if (flags['source-dir']) { + const { envName: targetName } = this.resolveEnv(config, flags.target ?? config.target) + const loaded = await openSchemaDir(flags['source-dir'], config.environments[targetName].dbUrl) + this.log(` ${dim(`Loaded ${loaded.statements} statement(s) from ${loaded.files} file(s) in ${flags['source-dir']}.`)}\n`) + config.environments = { ...config.environments, [SCHEMA_FILES_ENV]: { dbUrl: loaded.url } } + // What the platform owns and the copy could not hold is not the files'. + config.ignoreSchemas = [...(config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS), ...loaded.unavailable.schemas] + config.ignoreExtensions = [...(config.ignoreExtensions ?? []), ...loaded.unavailable.extensions] + config.source = SCHEMA_FILES_ENV + config.target = targetName + try { + await this.compare(flags, config, flags.check ? undefined : SCHEMA_FILE_CHECKS) + } finally { + await loaded.close() + } + return + } + this.validateDualEnvConfig(config, flags.source, flags.target) + await this.compare(flags, config) + } + + /** The comparison itself, between the config's source and target. */ + private async compare( + flags: Interfaces.InferredFlags, + config: SupaForgeConfig, + defaultChecks?: CheckName[], + ): Promise { const registry = createDefaultRegistry({ includeFiles: flags['include-files'] }) - const checks = flags.check ? [flags.check as CheckName] : undefined + const checks = flags.check ? [flags.check as CheckName] : defaultChecks const skip = flags.skip?.length ? (flags.skip as CheckName[]) : undefined const tableFilter = resolveTableFilter(config, { diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index d8e74f3..8fffb58 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -96,7 +96,7 @@ export interface ProofResult { } /** Replace the database name in a libpq URL, keeping everything else. */ -function withDatabase(dbUrl: string, database: string): string { +export function withDatabase(dbUrl: string, database: string): string { const url = new URL(dbUrl) url.pathname = `/${database}` return url.toString() @@ -374,7 +374,7 @@ export async function proveConvergence(opts: { } } -interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMajor: number } +export interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMajor: number } /** * Copy `fromUrl`'s structure in the given schemas into the empty database at @@ -388,7 +388,7 @@ interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMaj * into a clone that deliberately has no such schema. `--prove` therefore * failed on every real Supabase project, whatever the client version. */ -async function copyStructure( +export async function copyStructure( fromUrl: string, intoUrl: string, schemas: string[], tools: PgTools, migratingFrom?: string, ): Promise { // Prepare the copy to receive the proved schemas, and nothing else. @@ -525,7 +525,7 @@ async function copyOfSource( } /** Drop a throwaway database, ending any session still attached first. */ -async function dropDatabase(adminUrl: string, name: string): Promise { +export async function dropDatabase(adminUrl: string, name: string): Promise { await pgQuery(adminUrl, `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '${name}' AND pid <> pg_backend_pid()`).catch(() => undefined) @@ -541,7 +541,7 @@ async function dropDatabase(adminUrl: string, name: string): Promise { * loudly instead of producing a partially-migrated clone that then reports * confusing residual drift. */ -function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { +export function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { const trimmed = sql.trim() if (!trimmed) return Promise.resolve() @@ -565,7 +565,7 @@ function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { } /** psql ships alongside pg_dump; reuse the version-matched directory. */ -function psqlBeside(pgDumpPath: string): string { +export function psqlBeside(pgDumpPath: string): string { return pgDumpPath === 'pg_dump' ? 'psql' : join(dirname(pgDumpPath), 'psql') } diff --git a/packages/cli/src/schema-dir.ts b/packages/cli/src/schema-dir.ts new file mode 100644 index 0000000..b73faa6 --- /dev/null +++ b/packages/cli/src/schema-dir.ts @@ -0,0 +1,127 @@ +import { randomBytes } from 'node:crypto' +import { readdir, readFile } from 'node:fs/promises' +import { join, relative } from 'node:path' +import { pgQuery } from './db' +import { SUPABASE_PLATFORM_SCHEMAS } from './defaults' + +import { describeMissingPgDump, getServerMajorVersion, resolvePgDumpPath } from './pg-tools' +import { copyStructure, dropDatabase, psqlBeside, withDatabase } from './prove.js' +import { isCommentOnly, splitSqlStatements } from './utils/sql-split.js' +import { quoteName } from './utils/sql.js' +import { DiagnosticError, errMsg } from './utils/error.js' + +/** + * A directory of schema files — Supabase's `supabase/schemas` — as a + * database to compare against. + * + * Declarative schemas keep the desired state as SQL files. To compare them + * with a live target they are loaded, in a throwaway database on the + * target's own server (as `--prove` makes its copy), on top of the target's + * platform schemas — so a table referencing auth.users, or a policy calling + * auth.uid(), finds them — and that database is the source. Files load in + * name order; a statement that fails because something it needs comes later + * is tried again once the others have run, so the order the files are in + * matters as little as it can. + */ +export interface SchemaDirSource { + url: string + /** + * The target's extensions the copy could not install (pg_cron lives in one + * database per server), and the schemas holding their tables: schema files + * do not declare what the platform owns, so they are not compared. + */ + unavailable: { extensions: string[]; schemas: string[] } + /** Statements loaded, and how many files they came from. */ + statements: number + files: number + close(): Promise +} + +export async function openSchemaDir(dir: string, targetUrl: string): Promise { + const files = await sqlFiles(dir) + if (files.length === 0) throw new DiagnosticError(`--source-dir: no .sql files in ${dir}`) + + const serverMajor = await getServerMajorVersion(targetUrl) + const resolved = await resolvePgDumpPath(serverMajor) + if (!resolved) throw new DiagnosticError(`--source-dir needs pg_dump to copy the target's platform schemas: ${await describeMissingPgDump(serverMajor)}`) + const tools = { pgDump: resolved.path, psql: psqlBeside(resolved.path), clientMajor: resolved.major, serverMajor } + + const name = `supaforge_schema_${randomBytes(4).toString('hex')}` + const adminUrl = withDatabase(targetUrl, 'postgres') + const url = withDatabase(targetUrl, name) + try { + await pgQuery(adminUrl, `CREATE DATABASE ${quoteName(name)}`) + } catch (err) { + throw new DiagnosticError(`--source-dir: could not create a throwaway database on the target's server: ${errMsg(err)}`) + } + const close = () => dropDatabase(adminUrl, name) + try { + const present = new Set((await pgQuery(targetUrl, 'SELECT nspname FROM pg_namespace') as Array<{ nspname: string }>).map(r => r.nspname)) + const platform = SUPABASE_PLATFORM_SCHEMAS.filter(s => present.has(s) && s !== 'pg_catalog' && s !== 'information_schema') + if (platform.length > 0) await copyStructure(targetUrl, url, platform, tools) + const statements = await loadStatements(url, files) + return { url, statements, files: files.length, close, unavailable: await unavailableExtensions(targetUrl, url) } + } catch (err) { + await close() + throw err + } +} + +/** The directory's .sql files, at any depth, in name order. */ +async function sqlFiles(dir: string): Promise> { + const entries = await readdir(dir, { recursive: true, withFileTypes: true }) + return entries + .filter(e => e.isFile() && e.name.endsWith('.sql')) + .map(e => { + const path = join(e.parentPath ?? (e as unknown as { path: string }).path, e.name) + return { path, name: relative(dir, path) } + }) + .sort((a, b) => a.name.localeCompare(b.name)) +} + +/** + * Run every statement, each on its own; what fails is tried again after the + * rest, as long as each round gets further. What still fails is reported + * with its file. + */ +async function loadStatements(url: string, files: Array<{ path: string; name: string }>): Promise { + let pending: Array<{ file: string; sql: string; error?: string }> = [] + for (const f of files) { + for (const sql of splitSqlStatements(await readFile(f.path, 'utf8'))) { + if (!isCommentOnly(sql)) pending.push({ file: f.name, sql }) + } + } + const total = pending.length + for (;;) { + const failed: typeof pending = [] + for (const p of pending) { + try { + await pgQuery(url, p.sql) + } catch (err) { + failed.push({ ...p, error: errMsg(err) }) + } + } + if (failed.length === 0) return total + if (failed.length === pending.length) { + const first = failed[0] + throw new DiagnosticError( + `--source-dir: ${failed.length} statement(s) could not be loaded. First, in ${first.file}: ${first.error}\n` + + ` ${first.sql.slice(0, 200).replace(/\s+/g, ' ')}`, + ) + } + pending = failed + } +} + +/** The target's extensions the copy lacks, and the schemas their tables are in. */ +async function unavailableExtensions(targetUrl: string, copyUrl: string): Promise<{ extensions: string[]; schemas: string[] }> { + const sql = `SELECT e.extname AS name, + ARRAY(SELECT DISTINCT n.nspname FROM pg_depend d JOIN pg_class c ON c.oid = d.objid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE d.classid = 'pg_class'::regclass AND d.refobjid = e.oid AND d.deptype = 'e')::text[] AS schemas + FROM pg_extension e` + const [target, copy] = await Promise.all([targetUrl, copyUrl].map(url => pgQuery(url, sql))) as Array> + const have = new Set(copy.map(e => e.name)) + const missing = target.filter(e => !have.has(e.name)) + return { extensions: missing.map(e => e.name), schemas: [...new Set(missing.flatMap(e => e.schemas))] } +} diff --git a/packages/cli/src/types/config.ts b/packages/cli/src/types/config.ts index 33c2615..ca127b7 100644 --- a/packages/cli/src/types/config.ts +++ b/packages/cli/src/types/config.ts @@ -84,6 +84,8 @@ export interface SupaForgeConfig { source?: string target?: string ignoreSchemas?: string[] + /** Extensions the extensions check does not report as extra on the target. */ + ignoreExtensions?: string[] checks?: ChecksConfig } diff --git a/packages/cli/test/e2e/schema-dir.test.ts b/packages/cli/test/e2e/schema-dir.test.ts new file mode 100644 index 0000000..42b4474 --- /dev/null +++ b/packages/cli/test/e2e/schema-dir.test.ts @@ -0,0 +1,47 @@ +/** + * A directory of schema files as the source (`diff --source-dir`). + * + * The files are loaded into a throwaway database on the target's server and + * compared with the target; a file using what a later one creates still + * loads. Verdicts are read from the target, and the throwaway database must + * be gone afterwards. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { PgHarness } from '../harness/PgHarness.js' +import { describeWithContainers } from '../harness/containers.js' + +const describeE2E = describeWithContainers() + +describeE2E('e2e: schema files as the source', () => { + let h: PgHarness + + beforeAll(async () => { + h = new PgHarness({ verbose: !!process.env.E2E_VERBOSE, keep: !!process.env.E2E_KEEP }) + await h.up() + await h.applySql('target', 'CREATE TABLE public.tasks (id int PRIMARY KEY);') + }, 300_000) + + afterAll(async () => { await h?.down() }, 120_000) + + it('applies the files to the target, whatever order they name things in', async () => { + const ws = await h.workspace() + const dir = join(ws, 'supabase', 'schemas') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, '01_views.sql'), 'create view public.open_tasks as select id, title from public.tasks where not done;\n') + await writeFile(join(dir, '02_tasks.sql'), ` + create table public.tasks (id int primary key, title text not null default '', done boolean not null default false); + create index tasks_open on public.tasks (id) where not done; + `) + + const r = await h.cli(['diff', '--source-dir', dir, '--check', 'schema', '--apply'], { cwd: ws }) + expect(r.code, r.stdout + r.stderr).toBe(0) + expect(await h.sql('target', `SELECT string_agg(attname, ',' ORDER BY attnum) FROM pg_attribute WHERE attrelid = 'public.tasks'::regclass AND attnum > 0`)).toBe('id,title,done') + expect(await h.sql('target', `SELECT to_regclass('public.open_tasks') IS NOT NULL`)).toBe('t') + + const again = await h.cli(['diff', '--source-dir', dir, '--check', 'schema', '--json'], { cwd: ws }) + expect(JSON.parse(again.stdout).checks[0].issues).toEqual([]) + expect(await h.sql('target', `SELECT count(*) FROM pg_database WHERE datname LIKE 'supaforge_schema_%'`)).toBe('0') + }, 300_000) +}) From b4fd04e9728aec455c05db4cc3b62e0322a4f480 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 12:21:16 +0000 Subject: [PATCH 09/11] fix: --prove lets a generated column take its new expression The data half of a proof hashed every column of a sampled table, so a fix that changes a generated column's expression read as values lost, and the whole migration was refused. A generated column's values follow its expression; only the columns it is computed from are compared. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/prove-data.ts | 8 ++++++-- packages/cli/test/e2e/prove.test.ts | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/prove-data.ts b/packages/cli/src/prove-data.ts index f813cd7..57d06e2 100644 --- a/packages/cli/src/prove-data.ts +++ b/packages/cli/src/prove-data.ts @@ -96,13 +96,17 @@ function pipeCopy(psql: string, fromUrl: string, intoUrl: string, out: string, i }) } -/** Each table's row count, a hash of each column's values, and its storage. */ +/** + * Each table's row count, a hash of each column's values, and its storage. + * A generated column is left out: its values follow its expression, which a + * fix may mean to change. + */ export async function snapshotTables(dbUrl: string, schemas: string[]): Promise> { const rows = await pgQuery(dbUrl, ` SELECT n.nspname || '.' || c.relname AS name, quote_ident(n.nspname) || '.' || quote_ident(c.relname) AS ident, json_agg(json_build_array(a.attname, format_type(a.atttypid, a.atttypmod)) ORDER BY a.attnum) AS columns FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace - JOIN pg_attribute a ON a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped + JOIN pg_attribute a ON a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped AND a.attgenerated = '' WHERE c.relkind = 'r' AND NOT c.relispartition AND n.nspname = ANY($1::text[]) GROUP BY 1, 2 ORDER BY 1`, [schemas]) as Array<{ name: string; ident: string; columns: Array<[string, string]> }> const out = new Map() diff --git a/packages/cli/test/e2e/prove.test.ts b/packages/cli/test/e2e/prove.test.ts index 5f87365..cba821c 100644 --- a/packages/cli/test/e2e/prove.test.ts +++ b/packages/cli/test/e2e/prove.test.ts @@ -219,6 +219,25 @@ describeE2E('convergence proof', () => { expect(residual).toMatch(/table public\.sales_2026: missing/) }, 300_000) + // A sample of the target's rows goes through the fixes on the copy. A + // generated column's values follow its expression, so a fix that changes + // the expression changes them, as it means to; the base columns must hold. + it('lets a generated column take its new expression, and still guards the rest', async () => { + const table = (expr: string) => `CREATE TABLE t (id int PRIMARY KEY, a int, g int GENERATED ALWAYS AS (${expr}) STORED);` + await h.applySql('source', table('a * 3')) + await h.applySql('target', table('a * 2') + ' INSERT INTO t (id, a) SELECT i, i FROM generate_series(1, 20) i;') + const regenerate = 'ALTER TABLE t DROP COLUMN g; ALTER TABLE t ADD COLUMN g int GENERATED ALWAYS AS (a * 3) STORED;' + + const proof = await prove(regenerate) + expect(proof.data?.sampled).toContain('public.t') + expect(proof.data?.lost).toEqual([]) + expect(proof.converged).toBe(true) + + const clobbered = await prove(regenerate + ' UPDATE t SET a = a + 1;') + expect(clobbered.converged).toBe(false) + expect(clobbered.residual).toEqual(['rows lost in public.t: values changed in a']) + }, 300_000) + it('reports a migration that fails to execute, rather than claiming drift', async () => { await expect(prove('CREATE TABLE ;')).rejects.toThrow() }, 300_000) From dfe82c4100deb4be9a0a6a14be5257b0eff80d5a Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 12:33:05 +0000 Subject: [PATCH 10/11] fix: --prove counts the sequence a held-back table owns as held back A destructive drop held back without --allow-destructive leaves its table on the target, and the proof accounted for the table but not for the sequence its serial or identity column owns, so the whole apply was refused. An unexpected sequence now names the column that owns it, and a held-back drop of that table or column accounts for it. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/prove.ts | 11 +++++++++-- packages/cli/src/state-diff.ts | 10 +++++++--- packages/cli/test/prove-system-schemas.test.ts | 17 +++++++++++++++++ packages/cli/test/state-diff.test.ts | 12 ++++++++++++ 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index 8fffb58..842d10b 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -709,6 +709,11 @@ export function explainStructureFailure( /** `DROP [IF EXISTS] ` and `DROP COLUMN [IF EXISTS] `, capturing the bare name. */ const DROPPED_NAME = /\bDROP\s+(?:TABLE|VIEW|MATERIALIZED\s+VIEW|TYPE|DOMAIN|SEQUENCE|FUNCTION|PROCEDURE|INDEX|COLUMN|CONSTRAINT|POLICY|TRIGGER)\s+(?:IF\s+EXISTS\s+)?(?:(?:"[^"]+"|[\w$]+)\s*\.\s*)?("[^"]+"|[\w$]+)/gi +/** `table.column` as written by format('%I.%I'), each part unquoted and lower-cased. */ +function identParts(qualified: string): string[] { + return [...qualified.matchAll(/"((?:[^"]|"")+)"|([^."]+)/g)].map(m => (m[1]?.replace(/""/g, '"') ?? m[2]).toLowerCase()) +} + /** `CREATE [OR REPLACE] [IF NOT EXISTS] `, capturing the bare name. */ const CREATED_NAME = /\bCREATE\s+(?:OR\s+REPLACE\s+)?(?:UNIQUE\s+)?(?:TABLE|VIEW|MATERIALIZED\s+VIEW|TYPE|DOMAIN|SEQUENCE|FUNCTION|PROCEDURE|INDEX|POLICY|TRIGGER|EXTENSION)\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:(?:"[^"]+"|[\w$]+)\s*\.\s*)?("[^"]+"|[\w$]+)/gi @@ -742,10 +747,12 @@ export function residualHeldBack( const heldBack: string[] = [] const unexplained: string[] = [] for (const line of residual) { - const subject = /^[a-z ]+?\s([^\s:]+(?:\s[^\s:]+)?):\s*unexpected$/i.exec(line)?.[1] + const [, subject, owner] = /^[a-z ]+?\s([^\s:]+(?:\s[^\s:]+)?):\s*unexpected(?: \(owned by (.+)\))?$/i.exec(line) ?? [] const name = subject?.split(/[.\s]/).pop()?.replace(/^"|"$/g, '').toLowerCase() + // A sequence a column owns goes with the column, or its table. + const owners = owner ? identParts(owner) : [] const missing = /^(.*):\s*missing$/i.exec(line)?.[1].toLowerCase() - if (name && dropped.has(name)) heldBack.push(line) + if ((name && dropped.has(name)) || owners.some(o => dropped.has(o))) heldBack.push(line) else if (missing && created.some(n => missing.endsWith(` ${n}`) || missing.endsWith(`.${n}`))) heldBack.push(line) else unexplained.push(line) } diff --git a/packages/cli/src/state-diff.ts b/packages/cli/src/state-diff.ts index 203a497..9cbdca5 100644 --- a/packages/cli/src/state-diff.ts +++ b/packages/cli/src/state-diff.ts @@ -54,6 +54,8 @@ function compareKeyed( after: T[] | null | undefined, keyOf: (item: T) => string, compare: (out: string[], subject: string, a: T, b: T) => void, + /** Said after `unexpected`, of an object only the second side has. */ + note: (item: T) => string = () => '', ): void { const a = new Map((before ?? []).map(x => [keyOf(x), x])) const b = new Map((after ?? []).map(x => [keyOf(x), x])) @@ -62,8 +64,8 @@ function compareKeyed( if (!b.has(key)) out.push(`${kind} ${key}: missing`) else compare(out, `${kind} ${key}`, item, b.get(key) as T) } - for (const key of b.keys()) { - if (!a.has(key)) out.push(`${kind} ${key}: unexpected`) + for (const [key, item] of b) { + if (!a.has(key)) out.push(`${kind} ${key}: unexpected${note(item)}`) } } @@ -150,7 +152,9 @@ export function diffState(before: SchemaState, after: SchemaState): string[] { for (const k of ['type', 'start', 'increment', 'min', 'max', 'cache', 'cycle', 'owned_by'] as const) { compareField(o, s, k.replace('_', ' '), a[k], b[k]) } - }) + }, + // A serial or identity column's: it goes with its table. + q => q.owned_by ? ` (owned by ${q.owned_by})` : '') compareKeyed(out, 'routine', before.routines, after.routines, r => `${qualify(r.schema, r.name)}(${r.arguments})`, diff --git a/packages/cli/test/prove-system-schemas.test.ts b/packages/cli/test/prove-system-schemas.test.ts index 02afde6..9342129 100644 --- a/packages/cli/test/prove-system-schemas.test.ts +++ b/packages/cli/test/prove-system-schemas.test.ts @@ -164,6 +164,23 @@ describe('residual the held-back fixes account for', () => { ]) }) + // A serial or identity column's sequence goes with its table, or with the + // column: a held-back drop of either leaves the sequence too. + it('counts the sequence a held-back table or column owns', () => { + const r = residualHeldBack([ + 'sequence public.legacy_id_seq: unexpected (owned by legacy.id)', + 'sequence public.t_old_seq: unexpected (owned by t.old)', + 'sequence public."Odd_n_seq": unexpected (owned by "Odd"."n")', + 'sequence public.free_seq: unexpected', + 'sequence public.kept_id_seq: unexpected (owned by kept.id)', + ], [...held, 'DROP TABLE IF EXISTS "Odd";']) + expect(r.heldBack).toHaveLength(3) + expect(r.unexplained).toEqual([ + 'sequence public.free_seq: unexpected', + 'sequence public.kept_id_seq: unexpected (owned by kept.id)', + ]) + }) + it('accounts for nothing when nothing is held back', () => { expect(residualHeldBack(['table public.legacy: unexpected'], []).unexplained).toHaveLength(1) }) diff --git a/packages/cli/test/state-diff.test.ts b/packages/cli/test/state-diff.test.ts index 611bf8c..18b7e01 100644 --- a/packages/cli/test/state-diff.test.ts +++ b/packages/cli/test/state-diff.test.ts @@ -92,4 +92,16 @@ describe('diffState', () => { 'extension btree_gist: missing', ]) }) + + it('names the column that owns an unexpected sequence', () => { + const seq = (name: string, owned_by: string | null) => + ({ schema: 'public', name, type: 'integer', start: 1, increment: 1, min: 1, max: 9, cache: 1, cycle: false, owned_by }) + const b = empty() + b.sequences = [seq('tasks_id_seq', 'tasks.id'), seq('free_seq', null)] + + expect(diffState(empty(), b)).toEqual([ + 'sequence public.tasks_id_seq: unexpected (owned by tasks.id)', + 'sequence public.free_seq: unexpected', + ]) + }) }) From 369f93fc32580d1002b3227d45c0a79301d10662 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Thu, 15 Oct 2026 12:37:13 +0000 Subject: [PATCH 11/11] fix: say what the schema check could not compare beside it, not as drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dbdiff's warnings about kinds it does not compare were findings of their own, so a database with, say, an operator class never read as converged: a re-diff after an apply still found one, and a CI gate on any finding never passed. They are now the check's notices — under its line in the summary, in the JSON beside its issues, and in the CI summary — neither counted nor scored. Publications are left out: the Realtime check compares them. The README names the kinds the schema check now covers. Co-Authored-By: Claude Opus 5.5 --- packages/cli/README.md | 15 ++++++++---- packages/cli/src/checks/base.ts | 2 ++ packages/cli/src/checks/schema.ts | 3 ++- packages/cli/src/ci.ts | 5 ++++ packages/cli/src/dbdiff.ts | 33 +++++++++++++------------- packages/cli/src/render.ts | 1 + packages/cli/src/scanner.ts | 4 +++- packages/cli/src/types/drift.ts | 6 +++++ packages/cli/test/ci.test.ts | 17 +++++++++++++ packages/cli/test/dbdiff-units.test.ts | 21 ++++++++++++---- packages/cli/test/render.test.ts | 11 +++++++++ packages/cli/test/scanner.test.ts | 23 ++++++++++++++++++ 12 files changed, 113 insertions(+), 28 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index 6b729d1..9f5934e 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -76,10 +76,17 @@ Docker image needs nothing at all, and carries the PostgreSQL client tools | Roles and grants | Custom roles, and grants on tables, views, sequences and functions, including `anon`, `authenticated`, `service_role` | SQL | The schema check covers tables, columns, keys, indexes, partitions, views, -materialized views, functions, triggers, sequences, enums, composite types, -domains, policies, extensions and comments. It runs in every schema except -Supabase's own (`auth`, `storage`, and so on) and those in `ignoreSchemas`. -Objects owned by an extension belong to it and aren't compared. +materialized views, functions, aggregates, triggers, event triggers, rules, +sequences, enums, composite and range types, domains, casts, extended +statistics, text search configurations and dictionaries, foreign servers, +user mappings and foreign tables, policies, default privileges, replica +identity, statistics targets, extensions and comments. A renamed column, +table or sequence is renamed, keeping its rows, rather than dropped and +created again. It runs in every schema except Supabase's own (`auth`, +`storage`, and so on) and those in `ignoreSchemas`. Objects owned by an +extension belong to it and aren't compared. Objects of a kind it doesn't +compare, such as operator classes, are named under the check so you can look +at them by hand; they aren't counted as drift. A check that can't run, for example without API credentials, says so and why. It's never reported as a clean pass. diff --git a/packages/cli/src/checks/base.ts b/packages/cli/src/checks/base.ts index 1edf035..d30a2bd 100644 --- a/packages/cli/src/checks/base.ts +++ b/packages/cli/src/checks/base.ts @@ -18,6 +18,8 @@ export interface CheckContext { * fine-grained to report simply ignore it. */ onDetail?: (detail: string) => void + /** Sink for what the check could not compare — see CheckResult.notices. */ + onNotice?: (notice: string) => void source: EnvironmentConfig target: EnvironmentConfig config: SupaForgeConfig diff --git a/packages/cli/src/checks/schema.ts b/packages/cli/src/checks/schema.ts index 472e024..2bfc3f4 100644 --- a/packages/cli/src/checks/schema.ts +++ b/packages/cli/src/checks/schema.ts @@ -1,7 +1,7 @@ import type { QueryFn } from '../db' import { pgQuery } from '../db' import type { DriftIssue } from '../types/drift' -import { runDbDiff, sqlToIssues, type DbDiffOptions } from '../dbdiff' +import { runDbDiff, schemaNotices, sqlToIssues, type DbDiffOptions } from '../dbdiff' import { Check, CheckSkipped, type CheckContext } from './base' import { DEFAULT_IGNORE_SCHEMAS } from '../defaults' @@ -47,6 +47,7 @@ export class SchemaCheck extends Check { }), this.fetchIgnoredSchemaTables(ctx.target.dbUrl, ignoreSchemas), ]) + for (const notice of schemaNotices(result.notices ?? [])) ctx.onNotice?.(notice) return sqlToIssues(result, 'schema', ignoreSchemas, ignoredSchemaTables) } catch (err) { if (err instanceof Error && err.message.includes('@dbdiff/cli is not installed')) { diff --git a/packages/cli/src/ci.ts b/packages/cli/src/ci.ts index 99ee87f..b19687a 100644 --- a/packages/cli/src/ci.ts +++ b/packages/cli/src/ci.ts @@ -123,6 +123,8 @@ export function formatCiSummary(result: ScanResult): { skipped: Array<{ check: string; reason: string }> /** How many checks compared the two environments, over how many were attempted. */ coverage: { compared: number; total: number } + /** What a check could not compare, said beside its findings — see CheckResult.notices. */ + notices: Array<{ check: string; notice: string }> criticalIssues: Array<{ check: string; id: string; title: string }> warningIssues: Array<{ check: string; id: string; title: string }> } { @@ -130,6 +132,7 @@ export function formatCiSummary(result: ScanResult): { const warningIssues: Array<{ check: string; id: string; title: string }> = [] const errors: Array<{ check: string; message: string }> = [] const skipped: Array<{ check: string; reason: string }> = [] + const notices: Array<{ check: string; notice: string }> = [] for (const check of result.checks ?? []) { if (check?.status === 'error') { @@ -138,6 +141,7 @@ export function formatCiSummary(result: ScanResult): { if (check?.status === 'skipped') { skipped.push({ check: check.check, reason: check.skipReason ?? 'no reason given' }) } + for (const notice of check?.notices ?? []) notices.push({ check: check.check, notice }) for (const issue of check?.issues ?? []) { const entry = { check: check.check, id: issue.id, title: issue.title } if (issue.severity === 'critical') { @@ -156,6 +160,7 @@ export function formatCiSummary(result: ScanResult): { errors, skipped, coverage: coverage(result), + notices, criticalIssues, warningIssues, } diff --git a/packages/cli/src/dbdiff.ts b/packages/cli/src/dbdiff.ts index 8d8cbe8..fd24f8e 100644 --- a/packages/cli/src/dbdiff.ts +++ b/packages/cli/src/dbdiff.ts @@ -585,27 +585,26 @@ export function sqlToIssues( ignoredSchemaTables?: Set, ): DriftIssue[] { const issues = unfoldedIssues(result, check, ignoreSchemas, ignoredSchemaTables) - return check === 'schema' - ? [...foldCommentsIntoCreates(issues), ...noticeIssues(result.notices ?? [])] - : issues + return check === 'schema' ? foldCommentsIntoCreates(issues) : issues } +/** Kinds dbdiff does not compare that another check does: publications, the Realtime check. */ +const COMPARED_ELSEWHERE = /^Publication objects are not compared/ + /** - * A dbdiff warning as a finding of its own, with nothing to apply: what was - * not compared, or not taken for a rename, is worth knowing before an apply - * rather than finding out after. + * dbdiff's warnings, as said beside the schema check's findings: what it + * did not compare, or would not take for a rename, is worth knowing before + * an apply rather than after. `Operator objects are not compared (…) — check + * them by hand` reads `Not compared: Operator (…) — check them by hand.` */ -function noticeIssues(notices: string[]): DriftIssue[] { - return notices.map((notice, i) => { - const [what, advice] = notice.split(' — ') - return { - id: `schema-notice-${i + 1}`, - check: 'schema' as const, - severity: 'info' as const, - title: /not compared/.test(what) ? `Not compared: ${what.replace(/ objects are not compared/, '')}` : what, - description: advice ? `${what} — ${advice}.` : `${what}.`, - } - }) +export function schemaNotices(notices: string[]): string[] { + return notices + .filter(n => !COMPARED_ELSEWHERE.test(n)) + .map(notice => { + const [what, advice] = notice.split(' — ') + const title = /not compared/.test(what) ? `Not compared: ${what.replace(/ objects are not compared/, '')}` : what + return advice ? `${title} — ${advice}.` : `${title}.` + }) } /** What a comment title names: `public.orders.total (column)` → the name and kind. */ diff --git a/packages/cli/src/render.ts b/packages/cli/src/render.ts index 56b3900..1aeb7ba 100644 --- a/packages/cli/src/render.ts +++ b/packages/cli/src/render.ts @@ -143,6 +143,7 @@ export function renderSummary(result: ScanResult): string { for (const lr of result.checks) { lines.push(formatCheckLine(lr)) + for (const notice of lr.notices ?? []) lines.push(` ${dim(`ⓘ ${notice}`)}`) } lines.push('') diff --git a/packages/cli/src/scanner.ts b/packages/cli/src/scanner.ts index a2ffcff..b85bf36 100644 --- a/packages/cli/src/scanner.ts +++ b/packages/cli/src/scanner.ts @@ -101,12 +101,14 @@ async function runCheck( sourceDbUrl: string, ): Promise { const start = performance.now() + const notices: string[] = [] try { - const issues = withHazards(await check.scan(ctx)) + const issues = withHazards(await check.scan({ ...ctx, onNotice: n => notices.push(n) })) return { check: name, status: issues.length > 0 ? 'drifted' : 'clean', issues, + ...(notices.length > 0 ? { notices } : {}), durationMs: Math.round(performance.now() - start), } } catch (err) { diff --git a/packages/cli/src/types/drift.ts b/packages/cli/src/types/drift.ts index a0c31d2..1a9a9e3 100644 --- a/packages/cli/src/types/drift.ts +++ b/packages/cli/src/types/drift.ts @@ -139,6 +139,12 @@ export interface CheckResult { * the difference from the live progress line is explained. */ folded?: Array<{ count: number; into: CheckName }> + /** + * What the check could not compare or decide, said beside its findings: + * objects of a kind it does not compare, a rename it would not guess. Not + * drift — nothing to apply — so neither counted nor scored. + */ + notices?: string[] durationMs: number } diff --git a/packages/cli/test/ci.test.ts b/packages/cli/test/ci.test.ts index 413f9a5..dbe1047 100644 --- a/packages/cli/test/ci.test.ts +++ b/packages/cli/test/ci.test.ts @@ -465,3 +465,20 @@ describe('computeCiExitCode — drift and posture are different scopes (issue #6 expect(computeCiExitCode(mixed, 'critical')).toBe(1) }) }) + +describe('formatCiSummary carries what a check could not compare', () => { + it('lists each notice with its check, and none when there are none', () => { + const result: ScanResult = { + timestamp: '2026-03-21T00:00:00.000Z', source: 'dev', target: 'prod', score: 100, postureScore: null, + summary: { total: 0, critical: 0, warning: 0, info: 0 }, + checks: [ + { check: 'schema', status: 'clean', issues: [], notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], durationMs: 1 }, + { check: 'rls', status: 'clean', issues: [], durationMs: 1 }, + ], + } + expect(formatCiSummary(result).notices).toEqual([ + { check: 'schema', notice: 'Not compared: Operator (source 1, target 0: ===) — check them by hand.' }, + ]) + expect(formatCiSummary({ ...result, checks: [result.checks[1]] }).notices).toEqual([]) + }) +}) diff --git a/packages/cli/test/dbdiff-units.test.ts b/packages/cli/test/dbdiff-units.test.ts index 465aa10..a846e74 100644 --- a/packages/cli/test/dbdiff-units.test.ts +++ b/packages/cli/test/dbdiff-units.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { pairUnits, parseUnits } from '../src/dbdiff-units.js' -import { buildDbDiffArgs, dbdiffNotices, destructiveReason, sqlToIssues } from '../src/dbdiff.js' +import { buildDbDiffArgs, dbdiffNotices, destructiveReason, sqlToIssues, schemaNotices } from '../src/dbdiff.js' /** A section as `dbdiff --units` writes it. */ function units(...parts: Array<[kind: string, object: string, sql: string]>): string { @@ -297,14 +297,25 @@ describe('sqlToIssues: kinds dbdiff learned to compare, and renames', () => { ]) }) - it('turns dbdiff warnings into findings with nothing to apply', () => { + it('leaves dbdiff warnings out of the findings: they are not drift', () => { const issues = sqlToIssues({ up: '', down: '', notices: ['Operator objects are not compared (source 1, target 0: ===) — check them by hand'], }, 'schema') - expect(issues).toHaveLength(1) - expect(issues[0]).toMatchObject({ severity: 'info', title: 'Not compared: Operator (source 1, target 0: ===)' }) - expect(issues[0].sql).toBeUndefined() + expect(issues).toEqual([]) + }) +}) + +describe('schemaNotices', () => { + it('says what dbdiff did not compare, and leaves out what the Realtime check does', () => { + expect(schemaNotices([ + 'Operator objects are not compared (source 1, target 0: ===) — check them by hand', + 'Publication objects are not compared (source 1, target 1: supabase_realtime) — check them by hand', + 'public.a.b → c not taken for a rename', + ])).toEqual([ + 'Not compared: Operator (source 1, target 0: ===) — check them by hand.', + 'public.a.b → c not taken for a rename.', + ]) }) }) diff --git a/packages/cli/test/render.test.ts b/packages/cli/test/render.test.ts index 949b230..1439274 100644 --- a/packages/cli/test/render.test.ts +++ b/packages/cli/test/render.test.ts @@ -77,6 +77,17 @@ describe('renderSummary', () => { }) }) +describe('notices', () => { + it('are said under their layer, and the headline still reads clean', () => { + const out = renderSummary({ + ...cleanResult, + checks: [{ check: 'schema', status: 'clean', issues: [], notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], durationMs: 5 }], + }) + expect(out).toMatch(/Layer 1 \(Schema\):\s+0 issues\n\s+ⓘ Not compared: Operator \(source 1, target 0: ===\) — check them by hand\./) + expect(out).toMatch(/No drift detected/i) + }) +}) + describe('renderDetailed', () => { it('includes SQL fixes', () => { const output = renderDetailed(driftedResult) diff --git a/packages/cli/test/scanner.test.ts b/packages/cli/test/scanner.test.ts index 0ffdde7..f15299b 100644 --- a/packages/cli/test/scanner.test.ts +++ b/packages/cli/test/scanner.test.ts @@ -50,6 +50,29 @@ describe('scan', () => { expect(result.score).toBe(100) }) + // What a check could not compare is said beside its result, and is not + // drift: the check stays clean and the score whole. + it('keeps a check\'s notices beside its findings, out of the count', async () => { + class NoticingLayer extends Check { + readonly name = 'schema' as const + async scan(ctx: CheckContext): Promise { + ctx.onNotice?.('Not compared: Operator (source 1, target 0: ===) — check them by hand.') + return [] + } + } + const registry = new CheckRegistry() + registry.register(new NoticingLayer()) + + const result = await scan(registry, { config, checks: ['schema'] }) + + expect(result.checks[0]).toMatchObject({ + status: 'clean', + notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], + }) + expect(result.summary.total).toBe(0) + expect(result.score).toBe(100) + }) + it('returns drifted status when issues found', async () => { const registry = new CheckRegistry() registry.register(new MockLayer('rls', [