diff --git a/apps/web/src/backend/api/routes.ts b/apps/web/src/backend/api/routes.ts index f859c3e8..1e66975e 100644 --- a/apps/web/src/backend/api/routes.ts +++ b/apps/web/src/backend/api/routes.ts @@ -1267,7 +1267,7 @@ export function createApiRoutes(connectionModule: ConnectionModule, connectionSt res.status(400).json({ error: 'dialect and script are required' }); return; } - const id = await dataMigrateHistory.start((req as AuthedRequest).userId!, { + const started = await dataMigrateHistory.start((req as AuthedRequest).userId!, { dialect: body.dialect, sourceHost: body.sourceHost, targetHost: body.targetHost, @@ -1287,7 +1287,7 @@ export function createApiRoutes(connectionModule: ConnectionModule, connectionSt script: body.script, snapshotJson: body.snapshotJson, }); - res.json({ id }); + res.json(started); } ); diff --git a/apps/web/src/backend/modules/data-migrate-history.module.test.ts b/apps/web/src/backend/modules/data-migrate-history.module.test.ts new file mode 100644 index 00000000..a312e7f8 --- /dev/null +++ b/apps/web/src/backend/modules/data-migrate-history.module.test.ts @@ -0,0 +1,32 @@ +/** + * Fox Schema (foxschema) + * Copyright 2024-2026 Huy Phan + * SPDX-License-Identifier: Apache-2.0 + */ +import { describe, expect, it } from 'vitest'; +import { + DATA_MIGRATE_MAX_TEXT_LEN, + storeableSnapshotJson, +} from './data-migrate-history.module'; + +describe('storeableSnapshotJson', () => { + it('stores intact JSON under the size limit', () => { + const json = JSON.stringify({ version: 1, rows: [], columns: ['id'] }); + expect(storeableSnapshotJson(json)).toEqual({ json, stored: true }); + }); + + it('omits oversized snapshots instead of appending a truncation marker', () => { + const json = `${'{"rows":['}${'1,'.repeat(DATA_MIGRATE_MAX_TEXT_LEN)}]`; + expect(json.length).toBeGreaterThan(DATA_MIGRATE_MAX_TEXT_LEN); + expect(storeableSnapshotJson(json)).toEqual({ json: undefined, stored: false }); + }); + + it('rejects non-JSON so History never offers a corrupt Restore payload', () => { + const truncated = `${JSON.stringify({ rows: [1], columns: ['id'] }).slice(0, 20)}\n… (truncated)`; + expect(storeableSnapshotJson(truncated)).toEqual({ json: undefined, stored: false }); + }); + + it('treats missing snapshot as not stored', () => { + expect(storeableSnapshotJson(undefined)).toEqual({ json: undefined, stored: false }); + }); +}); diff --git a/apps/web/src/backend/modules/data-migrate-history.module.ts b/apps/web/src/backend/modules/data-migrate-history.module.ts index 3c5ce008..00c010c6 100644 --- a/apps/web/src/backend/modules/data-migrate-history.module.ts +++ b/apps/web/src/backend/modules/data-migrate-history.module.ts @@ -68,11 +68,32 @@ interface Row { } const MAX_RUNS_PER_USER = 200; -const MAX_TEXT_LEN = 1_000_000; +/** Bound script / snapshot blobs so one run cannot bloat the metadata DB. */ +export const DATA_MIGRATE_MAX_TEXT_LEN = 1_000_000; -function cap(text: string | undefined, max = MAX_TEXT_LEN): string | undefined { +/** Truncate free-form script text for display; never used for machine-parsed JSON. */ +function capScript(text: string | undefined, max = DATA_MIGRATE_MAX_TEXT_LEN): string | undefined { if (text == null) return text; - return text.length > max ? `${text.slice(0, max)}\n… (truncated)` : text; + return text.length > max ? `${text.slice(0, max)}\n-- … (truncated)` : text; +} + +/** + * Store snapshot JSON only when it fits intact. Appending a truncation marker + * (the old `cap()` behavior) produced invalid JSON, so History still offered + * Restore while `JSON.parse` always failed — a silent loss of the undo safety net. + */ +export function storeableSnapshotJson( + text: string | undefined, + max = DATA_MIGRATE_MAX_TEXT_LEN +): { json: string | undefined; stored: boolean } { + if (text == null) return { json: undefined, stored: false }; + if (text.length > max) return { json: undefined, stored: false }; + try { + JSON.parse(text); + } catch { + return { json: undefined, stored: false }; + } + return { json: text, stored: true }; } function parseOps(raw: string | null): { insert: boolean; update: boolean; delete: boolean } { @@ -105,8 +126,9 @@ export class DataMigrateHistoryStore { script: string; snapshotJson?: string; } - ): Promise { + ): Promise<{ id: string; snapshotStored: boolean }> { const id = randomUUID(); + const snapshot = storeableSnapshotJson(input.snapshotJson); const store = await getStore(); await store.run( `INSERT INTO data_migrate_runs @@ -126,13 +148,13 @@ export class DataMigrateHistoryStore { JSON.stringify(input.opsEnabled), input.includeIdentity ? 1 : 0, JSON.stringify(input.keyColumns), - cap(input.script) ?? null, - cap(input.snapshotJson) ?? null, + capScript(input.script) ?? null, + snapshot.json ?? null, new Date().toISOString(), ] ); await this.prune(userId); - return id; + return { id, snapshotStored: snapshot.stored }; } private async prune(userId: string): Promise { diff --git a/apps/web/src/frontend/api/dataMigrateApi.ts b/apps/web/src/frontend/api/dataMigrateApi.ts index ff50aa2c..a6a1433e 100644 --- a/apps/web/src/frontend/api/dataMigrateApi.ts +++ b/apps/web/src/frontend/api/dataMigrateApi.ts @@ -64,12 +64,11 @@ export async function apiStartDataMigrate(input: { keyColumns: string[]; script: string; snapshotJson?: string; -}): Promise { - const { id } = await request<{ id: string }>('/data-migrations/start', { +}): Promise<{ id: string; snapshotStored: boolean }> { + return request<{ id: string; snapshotStored: boolean }>('/data-migrations/start', { method: 'POST', body: JSON.stringify(input), }); - return id; } export async function apiFinishDataMigrate( diff --git a/apps/web/src/frontend/components/sql-editor/DataMigrateBar.tsx b/apps/web/src/frontend/components/sql-editor/DataMigrateBar.tsx index 8bb61f53..03e1d45f 100644 --- a/apps/web/src/frontend/components/sql-editor/DataMigrateBar.tsx +++ b/apps/web/src/frontend/components/sql-editor/DataMigrateBar.tsx @@ -19,7 +19,7 @@ import { type DataMigrateRunDetail, type DataMigrateRunSummary, } from '../../api/dataMigrateApi'; -import { buildDataMigratePlans, buildDestSnapshotJson, buildRestorePlansFromSnapshot } from '../../lib/dataMigratePlans'; +import { buildDataMigratePlans, buildDestSnapshotJson, buildRestorePlansFromSnapshot, isUsableDataMigrateSnapshot, snapshotTargetConnectionId } from '../../lib/dataMigratePlans'; import { classifyRowsByKey, DATA_MIGRATE_ROW_CAP, @@ -150,6 +150,8 @@ export const DataMigrateBar: React.FC = ({ const [backupEnabled, setBackupEnabled] = useState(true); const [lastBackup, setLastBackup] = useState<{ runId: string | null; + /** Destination that was migrated — Restore must not follow a later Destination switch. */ + connectionId: string; snapshotJson: string; results: DataMigrateOpResult[]; tableName: string; @@ -434,6 +436,7 @@ export const DataMigrateBar: React.FC = ({ ? buildDestSnapshotJson({ tableName, dialect: dest.dialect, + connectionId: dest.connectionId, destColumns: dest.columns, sourceColumns: source.columns, keyNames, @@ -457,8 +460,9 @@ export const DataMigrateBar: React.FC = ({ ); let runId: string | null = null; + let snapshotStored = false; try { - runId = await apiStartDataMigrate({ + const started = await apiStartDataMigrate({ dialect: dest.dialect, sourceHost: sourceConn?.host || source.label, targetHost: destConn?.host || dest.label, @@ -472,6 +476,15 @@ export const DataMigrateBar: React.FC = ({ script, snapshotJson, }); + runId = started.id; + snapshotStored = started.snapshotStored; + if (backupEnabled && snapshotJson && !snapshotStored) { + toast({ + tone: 'warning', + title: 'Backup too large for History', + body: 'The pre-apply snapshot exceeds the 1MB History limit, so durable Restore after reload is unavailable. Use Restore from the toast while this session still has the in-memory backup.', + }); + } } catch (e) { toast({ tone: 'warning', @@ -557,13 +570,19 @@ export const DataMigrateBar: React.FC = ({ !rolledBack && succeeded.length > 0; + // Capture at apply time — toast onAction must not read a later Destination switch. + const migratedConnectionId = dest.connectionId; + const migratedDialect = dest.dialect; + const migratedLabel = dest.label; + if (canOfferRestore && snapshotJson) { setLastBackup({ runId, + connectionId: migratedConnectionId, snapshotJson, results, tableName, - dialect: dest.dialect, + dialect: migratedDialect, }); } else { setLastBackup(null); @@ -583,7 +602,13 @@ export const DataMigrateBar: React.FC = ({ : `Finished with ${failCount} failure(s)`, body: failCount === 0 - ? `Destination: ${dest.label}.${backupEnabled ? ' Backup snapshot saved to history.' : ''}` + ? `Destination: ${migratedLabel}.${ + backupEnabled + ? snapshotStored + ? ' Backup snapshot saved to history.' + : ' Backup kept in-session only (too large for History).' + : '' + }` : `Failed: ${failedKeys.join('; ')}${ results.filter((r) => r.status === 'FAILED').length > 5 ? '…' : '' }. ${rolledBack ? 'Transaction rolled back. ' : ''}${ @@ -594,10 +619,11 @@ export const DataMigrateBar: React.FC = ({ ? () => void restoreFromBackup({ runId, + connectionId: migratedConnectionId, snapshotJson: snapshotJson!, results, tableName, - dialect: dest.dialect, + dialect: migratedDialect, }) : () => void openHistory(), durationMs: 12_000, @@ -607,6 +633,7 @@ export const DataMigrateBar: React.FC = ({ const restoreFromBackup = async (backup: { runId: string | null; + connectionId: string; snapshotJson: string; results: DataMigrateOpResult[]; tableName: string; @@ -621,6 +648,31 @@ export const DataMigrateBar: React.FC = ({ return; } + // Prefer the connection recorded at migrate time (snapshot / caller). Never + // silently follow a later Compare Destination switch — that would reverse + // DML onto the wrong database (same failure class as Lokee #256). + const targetConnectionId = + backup.connectionId.trim() || + snapshotTargetConnectionId(backup.snapshotJson) || + ''; + if (!targetConnectionId) { + toast({ + tone: 'warning', + title: 'Cannot restore', + body: 'This backup does not record which destination was migrated. Re-run migrate with Backup on, or restore only while the original Destination is selected.', + }); + return; + } + const targetConn = connections.find((c) => c.id === targetConnectionId); + if (!targetConn) { + toast({ + tone: 'warning', + title: 'Cannot restore', + body: 'The destination credential for this backup is no longer available.', + }); + return; + } + const { plans, errors } = buildRestorePlansFromSnapshot({ snapshotJson: backup.snapshotJson, successfulOps, @@ -647,12 +699,12 @@ export const DataMigrateBar: React.FC = ({ let restoreRunId: string | null = null; try { - restoreRunId = await apiStartDataMigrate({ + const started = await apiStartDataMigrate({ dialect: backup.dialect, sourceHost: 'backup-restore', - targetHost: destConn?.host || dest.label, - database: destConn?.database, - schema: destConn?.schema, + targetHost: targetConn.host || targetConn.name || targetConnectionId, + database: targetConn.database, + schema: targetConn.schema, tableName: backup.tableName, rowCount: plans.length, opsEnabled: { insert: true, update: true, delete: true }, @@ -664,6 +716,7 @@ export const DataMigrateBar: React.FC = ({ ].join('\n\n'), snapshotJson: backup.snapshotJson, }); + restoreRunId = started.id; } catch { /* history best-effort */ } @@ -671,9 +724,9 @@ export const DataMigrateBar: React.FC = ({ try { const out = await apiExecuteDataMigrate( { - connectionId: dest.connectionId, - password: sessionPasswords[dest.connectionId] || undefined, - schema: destConn?.schema?.trim() || undefined, + connectionId: targetConnectionId, + password: sessionPasswords[targetConnectionId] || undefined, + schema: targetConn.schema?.trim() || undefined, }, plans.map((p) => ({ op: p.op, @@ -1113,7 +1166,7 @@ export const DataMigrateBar: React.FC = ({ 
                           {historyDetail.snapshotJson || '(none — Backup was off)'}
- {historyDetail.snapshotJson && + {isUsableDataMigrateSnapshot(historyDetail.snapshotJson) && historyDetail.results.some((r) => r.status === 'SUCCESS') && historyDetail.status !== 'SUCCESS' && ( )} + {historyDetail.snapshotJson && + !isUsableDataMigrateSnapshot(historyDetail.snapshotJson) && ( +

+ Snapshot is not valid JSON — Restore is unavailable (likely an + older truncated History entry). +

+ )}
Script diff --git a/apps/web/src/frontend/lib/dataMigratePlans.test.ts b/apps/web/src/frontend/lib/dataMigratePlans.test.ts index 884a61f5..61901015 100644 --- a/apps/web/src/frontend/lib/dataMigratePlans.test.ts +++ b/apps/web/src/frontend/lib/dataMigratePlans.test.ts @@ -4,9 +4,30 @@ * SPDX-License-Identifier: Apache-2.0 */ import { describe, expect, it } from 'vitest'; -import { buildDataMigratePlans, buildDestSnapshotJson, buildRestorePlansFromSnapshot } from './dataMigratePlans'; +import { + buildDataMigratePlans, + buildDestSnapshotJson, + buildRestorePlansFromSnapshot, + isUsableDataMigrateSnapshot, + snapshotTargetConnectionId, +} from './dataMigratePlans'; import type { ClassifiedRowDiff } from './resultRowDiff'; +const DEST_CONN = 'conn-dest-b'; + +function snapshotFor(ops: ClassifiedRowDiff[], cols = ['id', 'name']) { + return buildDestSnapshotJson({ + tableName: 'customers', + dialect: 'sqlite', + connectionId: DEST_CONN, + destColumns: cols, + sourceColumns: cols, + keyNames: ['id'], + includeIdentity: true, + ops, + }); +} + describe('buildDataMigratePlans', () => { const cols = ['id', 'name']; const ops: ClassifiedRowDiff[] = [ @@ -42,19 +63,16 @@ describe('buildDataMigratePlans', () => { }); it('snapshots dest rows for update/delete and source rows for insert', () => { - const json = buildDestSnapshotJson({ - tableName: 'customers', - dialect: 'sqlite', - destColumns: cols, - sourceColumns: cols, - keyNames: ['id'], - includeIdentity: true, - ops, - }); - const parsed = JSON.parse(json) as { rows: Array<{ _op: string }>; includeIdentity: boolean }; + const json = snapshotFor(ops); + const parsed = JSON.parse(json) as { + rows: Array<{ _op: string }>; + includeIdentity: boolean; + connectionId: string; + }; expect(parsed.rows).toHaveLength(3); expect(parsed.rows.map((r) => r._op).sort()).toEqual(['delete', 'insert', 'update']); expect(parsed.includeIdentity).toBe(true); + expect(parsed.connectionId).toBe(DEST_CONN); }); it('omits identity columns from INSERT when includeIdentity is false', () => { @@ -131,15 +149,7 @@ describe('buildRestorePlansFromSnapshot', () => { ]; it('reverses successful insert/update/delete from Backup', () => { - const snapshotJson = buildDestSnapshotJson({ - tableName: 'customers', - dialect: 'sqlite', - destColumns: cols, - sourceColumns: cols, - keyNames: ['id'], - includeIdentity: true, - ops, - }); + const snapshotJson = snapshotFor(ops); const { plans, errors } = buildRestorePlansFromSnapshot({ snapshotJson, successfulOps: [ @@ -157,10 +167,23 @@ describe('buildRestorePlansFromSnapshot', () => { expect(plans[2]!.plan.sql.toLowerCase()).toContain('delete'); }); + it('records the destination connection id for Restore binding', () => { + const snapshotJson = snapshotFor(ops); + expect(snapshotTargetConnectionId(snapshotJson)).toBe(DEST_CONN); + expect(isUsableDataMigrateSnapshot(snapshotJson)).toBe(true); + }); + + it('rejects truncated / non-JSON History snapshots', () => { + const bad = `${snapshotFor(ops).slice(0, 40)}\n… (truncated)`; + expect(isUsableDataMigrateSnapshot(bad)).toBe(false); + expect(snapshotTargetConnectionId(bad)).toBeUndefined(); + }); + it('skips insert restore when identity was not preserved', () => { const snapshotJson = buildDestSnapshotJson({ tableName: 'customers', dialect: 'sqlite', + connectionId: DEST_CONN, destColumns: cols, sourceColumns: cols, keyNames: ['id'], @@ -176,22 +199,14 @@ describe('buildRestorePlansFromSnapshot', () => { }); it('restores update to the pre-apply dest values', () => { - const snapshotJson = buildDestSnapshotJson({ - tableName: 'customers', - dialect: 'sqlite', - destColumns: cols, - sourceColumns: cols, - keyNames: ['id'], - includeIdentity: true, - ops: [ - { - op: 'update', - keyLabel: 'id=1', - sourceRow: [1, 'Alice'], - destRow: [1, 'Bob'], - }, - ], - }); + const snapshotJson = snapshotFor([ + { + op: 'update', + keyLabel: 'id=1', + sourceRow: [1, 'Alice'], + destRow: [1, 'Bob'], + }, + ]); const { plans, errors } = buildRestorePlansFromSnapshot({ snapshotJson, successfulOps: [{ op: 'update', key: 'id=1' }], @@ -206,15 +221,9 @@ describe('buildRestorePlansFromSnapshot', () => { }); it('re-inserts a deleted dest row on restore', () => { - const snapshotJson = buildDestSnapshotJson({ - tableName: 'customers', - dialect: 'sqlite', - destColumns: cols, - sourceColumns: cols, - keyNames: ['id'], - includeIdentity: true, - ops: [{ op: 'delete', keyLabel: 'id=4', destRow: [4, 'Gone'] }], - }); + const snapshotJson = snapshotFor([ + { op: 'delete', keyLabel: 'id=4', destRow: [4, 'Gone'] }, + ]); const { plans, errors } = buildRestorePlansFromSnapshot({ snapshotJson, successfulOps: [{ op: 'delete', key: 'id=4' }], @@ -226,15 +235,9 @@ describe('buildRestorePlansFromSnapshot', () => { }); it('ignores FAILED ops and reports missing snapshot rows', () => { - const snapshotJson = buildDestSnapshotJson({ - tableName: 'customers', - dialect: 'sqlite', - destColumns: cols, - sourceColumns: cols, - keyNames: ['id'], - includeIdentity: true, - ops: [{ op: 'delete', keyLabel: 'id=4', destRow: [4, 'Gone'] }], - }); + const snapshotJson = snapshotFor([ + { op: 'delete', keyLabel: 'id=4', destRow: [4, 'Gone'] }, + ]); const { plans, errors } = buildRestorePlansFromSnapshot({ snapshotJson, successfulOps: [ @@ -260,6 +263,7 @@ describe('buildRestorePlansFromSnapshot', () => { expect(errors).toEqual([]); expect(plans).toHaveLength(1); expect(plans[0]!.op).toBe('insert'); + expect(snapshotTargetConnectionId(legacy)).toBeUndefined(); }); }); diff --git a/apps/web/src/frontend/lib/dataMigratePlans.ts b/apps/web/src/frontend/lib/dataMigratePlans.ts index e1b1d53f..ccad14da 100644 --- a/apps/web/src/frontend/lib/dataMigratePlans.ts +++ b/apps/web/src/frontend/lib/dataMigratePlans.ts @@ -197,6 +197,8 @@ export interface DataMigrateSnapshot { version: 1; tableName: string; dialect: string; + /** Destination credential the migrate wrote to — restore must use this, not the current Compare Destination. */ + connectionId?: string; columns: string[]; keyColumns: string[]; /** True when INSERT preserved source identity values (required to DELETE inserts on restore). */ @@ -213,6 +215,8 @@ export interface DataMigrateSnapshot { export function buildDestSnapshotJson(opts: { tableName: string; dialect: string; + /** Destination connection id — required so Restore cannot target a different Compare Destination. */ + connectionId: string; destColumns: string[]; sourceColumns: string[]; keyNames: string[]; @@ -222,6 +226,7 @@ export function buildDestSnapshotJson(opts: { const { tableName, dialect, + connectionId, destColumns, sourceColumns, keyNames, @@ -253,6 +258,7 @@ export function buildDestSnapshotJson(opts: { version: 1, tableName, dialect, + connectionId, columns: destColumns, keyColumns: keyNames, includeIdentity, @@ -261,6 +267,20 @@ export function buildDestSnapshotJson(opts: { return JSON.stringify(snapshot, null, 2); } +/** True when History can safely offer Restore (parseable JSON with rows/columns). */ +export function isUsableDataMigrateSnapshot(json: string | undefined | null): boolean { + if (!json) return false; + return !('error' in parseSnapshot(json)); +} + +/** Destination connection recorded in the snapshot, if any. */ +export function snapshotTargetConnectionId(json: string): string | undefined { + const parsed = parseSnapshot(json); + if ('error' in parsed) return undefined; + const id = parsed.connectionId?.trim(); + return id || undefined; +} + function parseSnapshot(json: string): DataMigrateSnapshot | { error: string } { try { const raw = JSON.parse(json) as Partial & { @@ -275,6 +295,10 @@ function parseSnapshot(json: string): DataMigrateSnapshot | { error: string } { version: 1, tableName: raw.tableName || '', dialect: raw.dialect || 'sqlite', + connectionId: + typeof raw.connectionId === 'string' && raw.connectionId.trim() + ? raw.connectionId.trim() + : undefined, columns: raw.columns, keyColumns: raw.keyColumns?.length ? raw.keyColumns : guessKeyColumns(raw.rows), includeIdentity: raw.includeIdentity !== false,