Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions packages/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,13 +367,18 @@ as `io.github.akalsoftware/supaforge`.

The server exposes these tools:

| Tool | Annotation |
| --- | --- |
| `scan_drift` | Read-only |
| `get_check_result` | Read-only |
| `apply_fixes` | Destructive. Previews unless `dryRun: false` |
| `take_snapshot` | Writes files |
| `create_migration` | Writes files |
| Tool | What it does | Annotation |
| --- | --- | --- |
| `list_environments` | The environments in the config, without their credentials | Read-only |
| `diff` | Every difference, with the plan `sync` would run | Read-only |
| `get_check_result` | One check's result from the last `diff` or `sync` | Read-only |
| `sync` | Runs the plan on the target; `prove: true` rehearses it on a copy first | Destructive |
| `create_migration` | Writes the drift as a migration file, as `migrate create` does | Writes files |
| `take_snapshot` | Snapshots an environment to local files | Writes files |

`diff` and `sync` take the same arguments, so the plan an agent shows you is
the plan that runs. Fixes that drop tables or columns are held back unless
`allowDestructive` is set.

It reads `supaforge.config.json` in the directory given by `--project` (or the
one it starts in), and refuses a `configPath` from a client unless started
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/mcpb/manifest.template.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@
}
},
"tools": [
{ "name": "scan_drift", "description": "Compare two environments and report drift, with the SQL to fix it" },
{ "name": "apply_fixes", "description": "Apply fixes from a scan to the target environment" },
{ "name": "list_environments", "description": "List the environments in the config, without their credentials" },
{ "name": "diff", "description": "Compare two environments and report every difference, with the plan sync would run" },
{ "name": "sync", "description": "Make the target match the source by running that plan, optionally proved on a copy first" },
{ "name": "take_snapshot", "description": "Snapshot an environment to local files" },
{ "name": "create_migration", "description": "Write a migration file for the drift found" },
{ "name": "get_check_result", "description": "Read one check's result from the last scan" }
Expand Down
122 changes: 43 additions & 79 deletions packages/cli/src/commands/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { createDefaultRegistry } from '../checks/index.js'
import { scan } from '../scanner.js'
import type { ScanProgressEvent } from '../scanner.js'
import { renderSummary, renderDetailed, formatSkips } from '../render.js'
import { promote, planWork, type PromoteResult } from '../promote.js'
import { promote, type PromoteResult } from '../promote.js'
import type { CheckName, ScanResult } from '../types/drift.js'
import { CHECK_NAMES, CHECK_META } from '../types/drift.js'
import { ok, warn, dim, cmd, bold } from '../ui.js'
Expand All @@ -14,9 +14,9 @@ import { formatGitHubAnnotations, computeCiExitCode, formatCiSummary, type FailO
import { resolveTableFilter, isFiltered, describeTableFilter } from '../utils/table-filter.js'
import { parseFlagList } from '../utils/strings.js'
import { isCloneDatabase, sourceLooksLikeCloneOf } from '../branch.js'
import { proveConvergence, residualHeldBack, proofScope, comparedSchemas } from '../prove.js'
import { proveBeforeApply } from '../prove-gate.js'
import { summarizeByKind } from '../scoring.js'
import { CLONE_SKIP_FLAGS, DEFAULT_IGNORE_SCHEMAS, schemaDiffIgnores } from '../defaults.js'
import { CLONE_SKIP_FLAGS, DEFAULT_IGNORE_SCHEMAS } from '../defaults.js'
import { Preflight } from '../preflight.js'
import type { SupaForgeConfig } from '../types/config.js'
import { redactUrls, describeFailure } from '../utils/error.js'
Expand Down Expand Up @@ -438,83 +438,56 @@ export default class Diff extends BaseCommand {
const targetEnv = config.environments[config.target!]
const dryRun = flags['dry-run']

// Prove before touching the target. A migration that executes without
// error can still leave the database in a state that is not the source —
// a flattened partition or an index that never reached its partitions
// both apply cleanly. Only replaying it and looking at the result catches
// that, so when --prove is set a failure blocks the apply rather than
// warning after the fact.
// Prove before touching the target — see prove-gate.
if (flags.prove && !dryRun) {
const sourceEnv = config.environments[config.source!]
const planned = planWork(scanResult, {
checks, only: parseFlagList(flags.only),
allowDestructive: flags['allow-destructive'], tableFilter,
applyPosture: flags['apply-posture'],
})
const { replay, unproved } = proofScope(planned.sqlStatements)
const migrationSql = replay.map(s => s.sql).join('\n')

this.log(`\n ${dim('Proving convergence on a throwaway clone…')}`)
if (unproved.length > 0) {
const proof = await proveBeforeApply({
scanResult,
plan: {
checks, only: parseFlagList(flags.only),
allowDestructive: flags['allow-destructive'], tableFilter,
applyPosture: flags['apply-posture'],
},
sourceUrl: config.environments[config.source!].dbUrl,
targetUrl: targetEnv.dbUrl,
ignoreSchemas: config.ignoreSchemas,
})
if (proof.unproved.length > 0) {
// Applied all the same; the proof just cannot speak for them.
const byCheck = [...new Set(unproved.map(s => s.check))]
.map(c => `${c} ×${unproved.filter(s => s.check === c).length}`).join(', ')
const byCheck = [...new Set(proof.unproved.map(s => s.check))]
.map(c => `${c} ×${proof.unproved.filter(s => s.check === c).length}`).join(', ')
this.log(` ${dim(`Not proved — act outside the proved schemas or on the whole server: ${byCheck}`)}`)
}
// Nothing left to replay means nothing to prove. Running it anyway
// compared the whole schema, and refused a cron-only apply over
// tables it was never going to touch.
const proof = replay.length === 0
? { converged: false, residual: [], cloneName: '', skipped: 'none of the planned fixes is one the proof can replay' }
: await proveConvergence({
sourceUrl: sourceEnv.dbUrl,
targetUrl: targetEnv.dbUrl,
migrationSql,
fixes: replay,
// Every schema the schema diff compares, not `public` alone: a
// migration dropping a schema the source lacks runs against it.
schemas: await comparedSchemas(sourceEnv.dbUrl, targetEnv.dbUrl,
schemaDiffIgnores(config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS)),
// The checks this run compares: without the schema check, what the
// target already lacks elsewhere is not this migration's to fix.
checks: scanResult.checks.map(c => c.check),
})

// What the replay left out because this server can't hold it, as the
// apply will: expected to stay different, so it counts as held back.
const heldBack = [...heldBackSql(scanResult, planned), ...(proof.notApplied ?? []).map(n => n.sql)]
if (proof.notApplied?.length) {
if (proof.notApplied.length > 0) {
this.log(` ${dim(`Left out, as the apply will leave them — this server can't hold them: ${proof.notApplied.map(n => n.issueId).join(', ')}`)}`)
}

if (proof.skipped && replay.length === 0) {
// Nothing the proof could replay: every planned fix is listed above
// as not proved, and there is nothing a rehearsal would add.
this.log(` ${dim(`Nothing to prove: ${proof.skipped}.`)}\n`)
} else if (proof.skipped) {
// Could not prove is not the same as failed to converge, so say which —
// but either way nothing is applied. --prove is asked for as a gate,
// and it used to apply anyway, exiting 0, when it could not run.
this.refuseUnproven(`Not proven: ${proof.skipped}`, [], flags.json)
} else if (!proof.converged && residualHeldBack(proof.residual, heldBack).unexplained.length === 0) {
// Everything left over is what this run deliberately holds back.
this.log(` ${ok('Converged')}, apart from what is held back and stays on the target:`)
for (const line of proof.residual.slice(0, 15)) this.log(` ${dim(line)}`)
this.log('')
} else if (!proof.converged) {
const { unexplained } = residualHeldBack(proof.residual, heldBack)
this.refuseUnproven('Migration does not reproduce the source.', unexplained, flags.json)
} else {
if (proof.outOfScope?.length) {
this.log(` ${ok('Converged')} on what this run compares. Outside it, and already different before:`)
for (const line of proof.outOfScope.slice(0, 15)) this.log(` ${dim(line)}`)
if (proof.outOfScope.length > 15) this.log(` ${dim(`…and ${proof.outOfScope.length - 15} more`)}`)
switch (proof.verdict) {
case 'nothing-to-prove':
this.log(` ${dim(`Nothing to prove: ${proof.reason}.`)}\n`)
break
case 'not-proven':
this.refuseUnproven(`Not proven: ${proof.reason}`, [], flags.json)
break
case 'diverged':
this.refuseUnproven('Migration does not reproduce the source.', proof.residual, flags.json)
break
case 'converged-apart-from-held-back':
this.log(` ${ok('Converged')}, apart from what is held back and stays on the target:`)
for (const line of proof.residual.slice(0, 15)) this.log(` ${dim(line)}`)
this.log('')
} else {
this.log(` ${ok('Converged')} — the migration reproduces the source exactly.\n`)
}
break
case 'converged':
if (proof.outOfScope.length > 0) {
this.log(` ${ok('Converged')} on what this run compares. Outside it, and already different before:`)
for (const line of proof.outOfScope.slice(0, 15)) this.log(` ${dim(line)}`)
if (proof.outOfScope.length > 15) this.log(` ${dim(`…and ${proof.outOfScope.length - 15} more`)}`)
this.log('')
} else {
this.log(` ${ok('Converged')} — the migration reproduces the source exactly.\n`)
}
}
if (proof.data && !proof.skipped) {
if (proof.data) {
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) {
Expand Down Expand Up @@ -756,15 +729,6 @@ export default class Diff extends BaseCommand {
}
}

/** The SQL of the fixes this run plans to skip, which the proof did not replay. */
function heldBackSql(scanResult: ScanResult, planned: { skipped: Array<{ issueId: string }> }): string[] {
const skipped = new Set(planned.skipped.map(s => s.issueId))
return scanResult.checks.flatMap(c => c.issues)
.filter(i => skipped.has(i.id) && i.sql?.up)
.map(i => i.sql!.up)
}


/** A preflight entry without the raw error object, which may not serialise. */
function stripError(c: { label: string; passed: boolean; detail?: string; error?: unknown }) {
return { label: c.label, passed: c.passed, ...(c.detail ? { detail: c.detail } : {}),
Expand Down
90 changes: 4 additions & 86 deletions packages/cli/src/commands/migrate/create.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
import { writeFile, mkdir } from 'node:fs/promises'
import { join } from 'node:path'
import { Flags } from '@oclif/core'
import { BaseCommand } from '../../base-command.js'
import { runDbDiff, isDestructiveSql } from '../../dbdiff.js'
import { RolesCheck } from '../../checks/roles.js'
import type { CheckContext } from '../../checks/base.js'
import { pgQuery } from '../../db.js'
import { DEFAULT_IGNORE_SCHEMAS } from '../../defaults.js'
import { writeMigrationFile } from '../../migration-file.js'
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.
Expand Down Expand Up @@ -48,7 +39,6 @@ export default class MigrateCreate extends BaseCommand {
const sourceEnv = config.environments[config.source!]
const targetEnv = config.environments[config.target!]
const dir = this.resolveMigrationsDir(config)
const ignoreSchemas = config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS

// Preflight: verify both databases are reachable
const pre = this.createPreflight('Migrate create preflight checks')
Expand All @@ -58,69 +48,12 @@ export default class MigrateCreate extends BaseCommand {

this.log(`${bold('migrate create')} — diffing ${dim(config.source!)} → ${dim(config.target!)}\n`)

const result = await runDbDiff({
sourceUrl: sourceEnv.dbUrl,
targetUrl: targetEnv.dbUrl,
type: 'schema',
include: 'both',
ignoreSchemas,
})

// The grants on what it makes, written out: the file is replayed where
// default privileges differ — a new project, a preview branch, a local
// reset — and Supabase no longer grants the Data API roles on new tables.
const grants = (await new RolesCheck(pgQuery, { explicit: true }).scan({
source: sourceEnv, target: targetEnv, config,
} as CheckContext)).filter(i => i.id.startsWith('roles-grant-') && i.sql?.up)

if (!result.up.trim() && grants.length === 0) {
const { filePath, destructive } = await writeMigrationFile({ config, name: flags.name, dir })
if (!filePath) {
this.log(`${ok('No schema drift detected.')} Nothing to write. ✓`)
return
}

// Generate timestamped filename
const timestamp = generateTimestamp()
const slug = slugify(flags.name)
const filename = `${timestamp}_${slug}.sql`

// Said when the file is written, not first when `migrate run` refuses it:
// a diff in which the target has a table the source lacks becomes a
// DROP TABLE here, and the file looks like any other.
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.`]
: []),
'',
result.up,
...(grants.length > 0 ? ['', '-- Grants', ...grants.map(i => i.sql!.up)] : []),
]

if (result.down.trim() || grants.length > 0) {
lines.push(
'',
'-- ==================== DOWN ====================',
'-- To revert this migration, run the SQL below manually:',
'--',
...[...grants.map(i => i.sql?.down ?? '').filter(Boolean).reverse(), result.down].join('\n').split('\n').map(l => `-- ${l}`),
)
}

const content = lines.join('\n') + '\n'

// Write file
await mkdir(dir, { recursive: true })
const filePath = join(dir, filename)
await writeFile(filePath, content, 'utf8')

this.log(`${ok('Created migration:')} ${cmd(filePath)}`)
if (destructive.length > 0) {
this.log(`\n${warn(`${destructive.length} destructive statement(s) in it`)} ${dim('— they drop or delete data:')}`)
Expand All @@ -133,22 +66,7 @@ export default class MigrateCreate extends BaseCommand {
}
}

/** Generate a UTC timestamp in Supabase migration format: YYYYMMDDHHmmss */
export function generateTimestamp(): string {
const now = new Date()
return [
now.getUTCFullYear(),
pad(now.getUTCMonth() + 1),
pad(now.getUTCDate()),
pad(now.getUTCHours()),
pad(now.getUTCMinutes()),
pad(now.getUTCSeconds()),
].join('')
}

function pad(n: number): string {
return String(n).padStart(2, '0')
}
export { generateTimestamp } from '../../migration-file.js'

/** Re-export for backward-compat — uses shared slugify with underscore separator. */
export { slugify } from '../../utils/strings.js'
21 changes: 19 additions & 2 deletions packages/cli/src/dbdiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ function unfoldedIssues(
// The DOWN counterpart is passed so a title can recover a schema the UP
// statement does not carry — see routineLabel (issue #47).
title: summariseStatement(lead, check, downSql),
description: `${check === 'schema' ? 'Schema' : 'Data'} difference detected by @dbdiff/cli.`,
description: describeDifference(type),
sql: { up: upSql, down: downSql },
}
})
Expand Down Expand Up @@ -848,7 +848,7 @@ function issuesFromUnits(
check,
severity: DROP_TYPES.includes(type) ? 'critical' : 'warning',
title: summariseStatement(lead, check, downSql),
description: `${check === 'schema' ? 'Schema' : 'Data'} difference detected by @dbdiff/cli.`,
description: describeDifference(type),
sql: { up: upSql, down: downSql },
...manual,
}
Expand Down Expand Up @@ -1410,6 +1410,23 @@ function withoutLeadingComments(chunk: string): string {
return chunk.trim().replace(/^(?:--[^\n]*(?:\n|$)\s*)+/, '').trim()
}

/**
* What a finding means and what its fix does, from the kind of statement that
* leads it. Said in words because an agent relays the description to a person,
* who should not need to read the SQL to know whether it adds or removes.
*/
export function describeDifference(type: string): string {
if (type === 'insert') return 'Rows in the source are missing from the target. The fix inserts them.'
if (type === 'update') return 'Rows differ between source and target. The fix updates the target\'s rows to match.'
if (type === 'delete') return 'The target has rows the source does not. The fix deletes them from the target.'
if (type === 'drop' || type.startsWith('drop-')) {
return 'The target has this and the source does not. The fix drops it from the target, '
+ 'and with it any data it holds.'
}
if (type.startsWith('create')) return 'The source has this and the target does not. The fix creates it on the target.'
return 'This exists on both sides but differs. The fix changes the target to match the source.'
}

export function classifyStatement(sql: string): string {
const upper = sql.toUpperCase().trimStart()
// Views
Expand Down
Loading
Loading