From 526c597ccfcc2ec5e618874997463a6429f5a598 Mon Sep 17 00:00:00 2001 From: Masa <98894508+Masashi-Ono0611@users.noreply.github.com> Date: Fri, 24 Jul 2026 06:21:56 +0900 Subject: [PATCH] fix(migrate): count and surface per-page copy failures instead of silently advancing (#3241) gbrain migrate's per-page copy loop had no failure handling at all: a page write that threw (e.g. a NOT-NULL column with no protection against the JS `undefined` postgres.js's UNDEFINED_VALUE guard rejects) crashed the whole command outright, with no per-page accounting and no way to tell which page caused it. Two changes: - Normalize `undefined` column values to explicit `null` at the migrate copy boundary before calling putPage. PGLite can hand back `undefined` for a column that is legitimately NULL/empty; postgres.js rejects a raw `undefined` bound parameter but accepts `null` fine. This is the root cause behind the report: a page whose title/compiled_truth/type came back `undefined` threw mid-insert. - Wrap the per-page copy in try/catch: failures are tracked (slug + reason), excluded from the resume manifest's completed_slugs (so a retry picks them back up), and the run ends with a non-zero exit verdict + an honest "N copied, M failed" summary instead of a bare crash or a false "N/N copied" success. Fixing this properly also required making the pre-existing resume manifest actually usable without --force (a matching manifest now bypasses the non-empty-target guard instead of demanding a wipe that would orphan already-copied pages), always resetting the manifest on --force regardless of whether the target looked empty, persisting the manifest before the copy loop starts (so a run where every page fails after its row lands still leaves a resumable manifest on disk), only flipping the active config to the target once the migration is fully clean, and skipping link-copy for slugs known to have failed above (avoiding an FK-violation crash on the next phase). Addresses the report in #3194 (reported by @hbohlen). Co-authored-by: Claude Fable 5 --- src/commands/migrate-engine.ts | 314 ++++++++++----- ...te-engine-page-copy-failure.serial.test.ts | 374 ++++++++++++++++++ 2 files changed, 590 insertions(+), 98 deletions(-) create mode 100644 test/migrate-engine-page-copy-failure.serial.test.ts diff --git a/src/commands/migrate-engine.ts b/src/commands/migrate-engine.ts index 76cf1f34b..6048b11a7 100644 --- a/src/commands/migrate-engine.ts +++ b/src/commands/migrate-engine.ts @@ -10,12 +10,13 @@ import { createEngine } from '../core/engine-factory.ts'; import { loadConfig, saveConfig, toEngineConfig, gbrainPath, effectiveEnvDatabaseUrl, type GBrainConfig } from '../core/config.ts'; import type { BrainEngine } from '../core/engine.ts'; -import type { EngineConfig } from '../core/types.ts'; +import type { EngineConfig, Page } from '../core/types.ts'; import { writeFileSync, readFileSync, existsSync, unlinkSync } from 'fs'; import { createHash } from 'crypto'; import { resolve } from 'path'; import { createProgress } from '../core/progress.ts'; import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts'; +import { setCliExitVerdict } from '../core/cli-force-exit.ts'; interface MigrateOpts { targetEngine: 'postgres' | 'pglite'; @@ -143,6 +144,99 @@ export async function copyMigrationSources(source: BrainEngine, target: BrainEng } } +/** + * postgres.js's UNDEFINED_VALUE guard rejects any bound parameter that is JS + * `undefined` — unlike PGLite, it will not silently treat it as SQL NULL. + * A page read back from a PGLite source can carry `undefined` for a column + * that is legitimately empty/NULL (a read-side driver-shape difference, not + * a data problem), and passing that value straight into a Postgres + * `putPage` throws mid-insert (#3194). Normalizing at this migrate-only + * boundary — rather than inside `putPage` itself, which many non-migrate + * callers also use — turns that driver-shape difference into an explicit + * SQL NULL, so only a genuine NOT-NULL constraint violation (an actual data + * problem) still surfaces as a page-copy failure. + */ +function nullifyUndefinedColumns>(row: T): T { + const normalized = { ...row }; + for (const key of Object.keys(normalized) as (keyof T)[]) { + if (normalized[key] === undefined) normalized[key] = null as T[typeof key]; + } + return normalized; +} + +/** + * Copy one page's full row (page body, chunks, tags, timeline, raw data) + * from source to target. Throws on any failure — the caller (the per-page + * loop in runMigrateEngine) decides how to account for that: track it as a + * failed page and keep going, rather than letting one bad row silently + * disappear from the progress count (#3194). Exported so unit tests can + * inject fake engines and exercise the failure path without a live + * DATABASE_URL. + */ +export async function copyPageToTarget( + source: BrainEngine, + target: BrainEngine, + page: Page, +): Promise { + const sourceOpts = { sourceId: page.source_id }; + + // Copy page (preserve source_id). v0.32.8 F8: thread source_id end-to-end + // so multi-source pages migrate intact. + await target.putPage(page.slug, nullifyUndefinedColumns({ + type: page.type, + title: page.title, + compiled_truth: page.compiled_truth, + timeline: page.timeline, + frontmatter: page.frontmatter, + content_hash: page.content_hash, + }), sourceOpts); + + // Copy chunks with embeddings. + const chunks = await source.getChunksWithEmbeddings(page.slug, sourceOpts); + if (chunks.length > 0) { + await target.upsertChunks(page.slug, chunks.map(c => ({ + chunk_index: c.chunk_index, + chunk_text: c.chunk_text, + chunk_source: c.chunk_source, + embedding: c.embedding || undefined, + model: c.model, + token_count: c.token_count || undefined, + })), sourceOpts); + } + + // Copy tags + const tags = await source.getTags(page.slug, sourceOpts); + for (const tag of tags) { + await target.addTag(page.slug, tag, sourceOpts); + } + + // Copy timeline + const timeline = await source.getTimeline(page.slug, sourceOpts); + for (const entry of timeline) { + await target.addTimelineEntry(page.slug, { + date: entry.date, + source: entry.source, + summary: entry.summary, + detail: entry.detail, + }, sourceOpts); + } + + // Copy raw data + const rawData = await source.getRawData(page.slug, undefined, sourceOpts); + for (const rd of rawData) { + await target.putRawData(page.slug, rd.source, rd.data, sourceOpts); + } +} + +/** A page that failed to copy during migrate — tracked so the run's final + * summary reports it honestly instead of letting the "N copied" counter + * imply every page landed (#3194). */ +export interface MigratePageFailure { + source_id: string; + slug: string; + reason: string; +} + export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[]): Promise { const opts = parseArgs(args); const config = loadConfig(); @@ -177,32 +271,47 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] await targetEngine.connect(targetConfig); await targetEngine.initSchema(); - // Check if target has data - const targetStats = await targetEngine.getStats(); - if (targetStats.page_count > 0 && !opts.force) { - console.error(`Target brain is not empty (${targetStats.page_count} pages).`); - console.error('Run with --force to overwrite, or migrate to an empty brain.'); - await targetEngine.disconnect(); - process.exit(1); - } - - if (targetStats.page_count > 0 && opts.force) { - console.log('--force: wiping target brain...'); - // v0.18.0+ multi-source: deletePage(slug) is now source-scoped (defaults - // to 'default'), so per-page iteration would skip non-default-source - // rows. migrate-engine --force is a destructive wipe across the entire - // brain — all sources, all pages — so we issue a raw DELETE that matches - // the original semantic. Cascades through content_chunks / page_links / - // tags / timeline_entries / page_versions via existing FKs. - await targetEngine.executeRaw('DELETE FROM pages'); - } - - // Load or create manifest for resume + // Load or create manifest for resume. Checked BEFORE the non-empty-target + // guard below: a manifest matching this exact target means the target's + // existing rows came from OUR OWN in-progress migration (#3194's per-page + // failures now leave the target non-empty by design instead of crashing), + // so a resume must not be treated as "attempting to migrate into a + // foreign non-empty brain". let manifest = loadManifest(); if (manifest && !manifestMatchesTarget(manifest, targetId)) { console.log('Previous migration was to a different target. Starting fresh.'); manifest = null; } + const resumingMatchingManifest = manifest !== null; + + // Check if target has data + const targetStats = await targetEngine.getStats(); + if (opts.force) { + if (targetStats.page_count > 0) { + console.log('--force: wiping target brain...'); + // v0.18.0+ multi-source: deletePage(slug) is now source-scoped (defaults + // to 'default'), so per-page iteration would skip non-default-source + // rows. migrate-engine --force is a destructive wipe across the entire + // brain — all sources, all pages — so we issue a raw DELETE that matches + // the original semantic. Cascades through content_chunks / page_links / + // tags / timeline_entries / page_versions via existing FKs. + await targetEngine.executeRaw('DELETE FROM pages'); + } + // --force always starts this exact migration fresh against this target: + // a manifest tracking a previous attempt must not be trusted to skip + // pages, regardless of whether the target LOOKED non-empty just now + // (e.g. the target DB file was recreated out-of-band but + // ~/.gbrain/migrate-manifest.json survived) — round 2 of #3194. + manifest = null; + } else if (targetStats.page_count > 0 && !resumingMatchingManifest) { + console.error(`Target brain is not empty (${targetStats.page_count} pages).`); + console.error('Run with --force to overwrite, or migrate to an empty brain.'); + await targetEngine.disconnect(); + process.exit(1); + } else if (targetStats.page_count > 0 && resumingMatchingManifest) { + console.log(`Resuming previous migration: ${manifest!.completed_slugs.length} page(s) already copied.`); + } + // v0.32.8 F8: manifest keys are now `${source_id}::${slug}` so multi-source // migrations don't collide on same-slug-different-source pages. Pre-v0.32.8 // entries were bare slugs; we keep treating those as default-source for @@ -219,6 +328,13 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] started_at: new Date().toISOString(), }; } + // Persist immediately, before any page copy runs. Otherwise a run where + // EVERY page fails after its putPage lands (but before completed_slugs + // ever gets a successful entry) leaves the target non-empty with no + // manifest file on disk at all — the next invocation can't tell this + // was a resumable in-progress migration and hits the non-empty guard + // above requiring --force (round 2 of #3194). + saveManifest(manifest); // Pages.source_id is a foreign key. Copy the complete source catalog first, // including archived rows and sync/routing metadata, so every page write has @@ -235,82 +351,68 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] const progress = createProgress(cliOptsToProgressOptions(getCliOptions())); progress.start('migrate.copy_pages', pagesToMigrate.length); + // v0.32.8 F8: thread source_id end-to-end so multi-source pages migrate + // intact. Pre-fix: putPage / getTags / getTimeline / getRawData / getLinks + // all silently defaulted to source_id='default', so non-default-source + // tags / timeline / raw / links were either dropped or attached to the + // wrong row. let migrated = 0; + const failures: MigratePageFailure[] = []; for (const page of pagesToMigrate) { - // v0.32.8 F8: thread source_id end-to-end so multi-source pages migrate - // intact. Pre-fix: putPage / getTags / getTimeline / getRawData / getLinks - // all silently defaulted to source_id='default', so non-default-source - // tags / timeline / raw / links were either dropped or attached to the - // wrong row. - const sourceOpts = { sourceId: page.source_id }; - - // Copy page (preserve source_id) - await targetEngine.putPage(page.slug, { - type: page.type, - title: page.title, - compiled_truth: page.compiled_truth, - timeline: page.timeline, - frontmatter: page.frontmatter, - content_hash: page.content_hash, - }, sourceOpts); - - // Copy chunks with embeddings. - const chunks = await sourceEngine.getChunksWithEmbeddings(page.slug, sourceOpts); - if (chunks.length > 0) { - await targetEngine.upsertChunks(page.slug, chunks.map(c => ({ - chunk_index: c.chunk_index, - chunk_text: c.chunk_text, - chunk_source: c.chunk_source, - embedding: c.embedding || undefined, - model: c.model, - token_count: c.token_count || undefined, - })), sourceOpts); + try { + await copyPageToTarget(sourceEngine, targetEngine, page); + // Track progress with composite key so multi-source resume is correct. + manifest!.completed_slugs.push(makeManifestKey(page.source_id, page.slug)); + saveManifest(manifest!); + migrated++; + } catch (e) { + // #3194: a per-page write failure must never be swallowed into the + // success count. Leave it OUT of completed_slugs (a resume retries + // it — putPage/upsertChunks/etc. are all upserts, so re-running the + // whole page copy is safe) and surface it in the final summary below + // instead of letting "N pages copied" imply everything landed. + failures.push({ + source_id: page.source_id, + slug: page.slug, + reason: e instanceof Error ? e.message : String(e), + }); } - - // Copy tags - const tags = await sourceEngine.getTags(page.slug, sourceOpts); - for (const tag of tags) { - await targetEngine.addTag(page.slug, tag, sourceOpts); - } - - // Copy timeline - const timeline = await sourceEngine.getTimeline(page.slug, sourceOpts); - for (const entry of timeline) { - await targetEngine.addTimelineEntry(page.slug, { - date: entry.date, - source: entry.source, - summary: entry.summary, - detail: entry.detail, - }, sourceOpts); - } - - // Copy raw data - const rawData = await sourceEngine.getRawData(page.slug, undefined, sourceOpts); - for (const rd of rawData) { - await targetEngine.putRawData(page.slug, rd.source, rd.data, sourceOpts); - } - - // Copy versions - const versions = await sourceEngine.getVersions(page.slug, sourceOpts); - // Versions are snapshots, we recreate them on the target - // (createVersion takes a snapshot of current state, which we just set) - - // Track progress with composite key so multi-source resume is correct. - manifest!.completed_slugs.push(makeManifestKey(page.source_id, page.slug)); - saveManifest(manifest!); - migrated++; progress.tick(1, page.slug); } progress.finish(); + if (failures.length > 0) { + console.error(`\n${failures.length} of ${pagesToMigrate.length} page(s) FAILED to copy and were NOT migrated:`); + for (const f of failures) { + const key = f.source_id === 'default' ? f.slug : `${f.source_id}::${f.slug}`; + console.error(` - ${key}: ${f.reason}`); + } + console.error('Re-run `gbrain migrate` to retry the failed pages (already-copied pages resume via the manifest).'); + // Non-fatal so the run still copies links + config for everything that + // DID land, but the process must exit non-zero — a partial migration + // must never look identical to a clean one. + setCliExitVerdict(1); + } + // Copy links (after all pages exist in target). // v0.32.8 F8: thread source_id so cross-source links migrate correctly. + // #3194: a page that failed to copy above does NOT exist on the target, + // so any link touching it would violate the target's FK and abort this + // whole phase (the exact "addLink failed: page ... not found" crash from + // the original report). Skip links on either end of a known-failed page — + // a retry that successfully copies the page also re-copies its links. + const failedKeys = new Set(failures.map(f => makeManifestKey(f.source_id, f.slug))); console.log('Copying links...'); progress.start('migrate.copy_links', allPages.length); for (const page of allPages) { + if (failedKeys.has(makeManifestKey(page.source_id, page.slug))) { + progress.tick(1); + continue; + } const sourceOpts = { sourceId: page.source_id }; const links = await sourceEngine.getLinks(page.slug, sourceOpts); for (const link of links) { + if (failedKeys.has(makeManifestKey(page.source_id, link.to_slug))) continue; await targetEngine.addLink( link.from_slug, link.to_slug, link.context, link.link_type, @@ -342,22 +444,38 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] // Update local config. v0.37 fix wave: preserve existing file-plane // embedding/expansion/chat config across the engine migration; only // the engine + connection target should change. - const existingFile = (await import('../core/config.ts')).loadConfigFileOnly() ?? ({} as GBrainConfig); - const newConfig: GBrainConfig = { - ...existingFile, - engine: opts.targetEngine, - ...(opts.targetEngine === 'postgres' - ? { database_url: targetConfig.database_url, database_path: undefined } - : { database_path: targetConfig.database_path, database_url: undefined }), - }; - saveConfig(newConfig); + // + // #3194: only flip the ACTIVE config when the migration is fully clean. + // A partial migration leaves the target's data incomplete; auto-switching + // every subsequent `gbrain` invocation onto that incomplete target would + // (a) make the failure invisible behind otherwise-normal usage and (b) + // break the natural retry — `gbrain migrate --to X` again would hit the + // "Already using X engine" guard even though the migration never actually + // finished. Leaving the file-plane config untouched keeps the source the + // active engine, so a retry (which resumes via the still-intact manifest) + // is a same-shaped command, not a special case. + if (failures.length === 0) { + const existingFile = (await import('../core/config.ts')).loadConfigFileOnly() ?? ({} as GBrainConfig); + const newConfig: GBrainConfig = { + ...existingFile, + engine: opts.targetEngine, + ...(opts.targetEngine === 'postgres' + ? { database_url: targetConfig.database_url, database_path: undefined } + : { database_path: targetConfig.database_path, database_url: undefined }), + }; + saveConfig(newConfig); + // Clean up the resume manifest — only safe once nothing is left pending. + clearManifest(); + } - // Clean up - clearManifest(); - - console.log(`\nMigration complete. ${migrated} pages transferred.`); - console.log(`Config updated to engine: ${opts.targetEngine}`); - if (config.engine === 'pglite' && config.database_path) { + if (failures.length > 0) { + console.log(`\nMigration completed with errors. ${migrated} of ${pagesToMigrate.length} pages copied, ${failures.length} failed (${completedSet.size} already done from a prior run). See failure list above.`); + console.log(`Config NOT switched — still using engine: ${config.engine}. Re-run \`gbrain migrate --to ${opts.targetEngine}\` to retry; already-copied pages resume via the manifest.`); + } else { + console.log(`\nMigration complete. ${migrated} pages transferred.`); + console.log(`Config updated to engine: ${opts.targetEngine}`); + } + if (failures.length === 0 && config.engine === 'pglite' && config.database_path) { console.log(`Original PGLite brain preserved at ${config.database_path} (backup).`); } diff --git a/test/migrate-engine-page-copy-failure.serial.test.ts b/test/migrate-engine-page-copy-failure.serial.test.ts new file mode 100644 index 000000000..2f3fe5703 --- /dev/null +++ b/test/migrate-engine-page-copy-failure.serial.test.ts @@ -0,0 +1,374 @@ +/** + * #3194 — `gbrain migrate` must not silently drop pages while reporting + * success. + * + * Two things are pinned here: + * + * 1. `copyPageToTarget` normalizes JS `undefined` column values to an + * explicit `null` before handing them to `target.putPage`. PGLite can + * hand back `undefined` for a column that is legitimately NULL/empty; + * postgres.js's `UNDEFINED_VALUE` guard rejects a raw `undefined` bound + * parameter (but accepts `null` fine). Without this normalization, a + * migrated page carrying an `undefined` field throws mid-insert. + * + * 2. `runMigrateEngine`'s per-page copy loop must not let an unrecoverable + * per-page failure disappear into the success count: the failed page + * must be excluded from the resume manifest's `completed_slugs` (so a + * retry picks it back up) and the run must end with a non-zero CLI exit + * verdict instead of looking identical to a clean migration. + */ + +import { describe, test, expect, afterEach } from 'bun:test'; +import { mkdtempSync, rmSync, readFileSync, existsSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { copyPageToTarget, runMigrateEngine } from '../src/commands/migrate-engine.ts'; +import { saveConfig, loadConfigFileOnly } from '../src/core/config.ts'; +import { currentExitCode, _resetCliExitVerdictForTests } from '../src/core/cli-force-exit.ts'; +import type { BrainEngine } from '../src/core/engine.ts'; +import type { Page } from '../src/core/types.ts'; + +function fakePage(overrides: Partial = {}): Page { + return { + id: 1, + slug: 'test-page', + type: 'note', + title: 'a title', + compiled_truth: 'body', + timeline: '', + frontmatter: {}, + source_id: 'default', + created_at: new Date(), + updated_at: new Date(), + ...overrides, + }; +} + +describe('copyPageToTarget — undefined-column normalization (#3194)', () => { + test('undefined page fields become explicit null before reaching target.putPage', async () => { + const putPageCalls: unknown[] = []; + const target = { + putPage: async (slug: string, page: unknown, opts: unknown) => { + putPageCalls.push({ slug, page, opts }); + return fakePage(); + }, + } as unknown as BrainEngine; + const source = { + getChunksWithEmbeddings: async () => [], + getTags: async () => [], + getTimeline: async () => [], + getRawData: async () => [], + } as unknown as BrainEngine; + + // Simulate the exact PGLite read-side shape from #3194: a page whose + // `type` / `compiled_truth` / `content_hash` came back `undefined` + // (legitimately NULL/absent on the source) rather than an empty string + // or explicit `null`. + const page = fakePage({ + type: undefined as unknown as string, + compiled_truth: undefined as unknown as string, + content_hash: undefined, + }); + + await copyPageToTarget(source, target, page); + + expect(putPageCalls.length).toBe(1); + const call = putPageCalls[0] as { slug: string; page: Record; opts: unknown }; + expect(call.slug).toBe('test-page'); + // The driver-shape `undefined` must have become an explicit SQL NULL... + expect(call.page.type).toBeNull(); + expect(call.page.compiled_truth).toBeNull(); + expect(call.page.content_hash).toBeNull(); + // ...while legitimately-populated fields pass through untouched. + expect(call.page.title).toBe('a title'); + expect(call.opts).toEqual({ sourceId: 'default' }); + }); + + test('already-null / already-populated fields are left as-is (no double-mapping)', async () => { + const putPageCalls: unknown[] = []; + const target = { + putPage: async (slug: string, page: unknown, opts: unknown) => { + putPageCalls.push({ slug, page, opts }); + return fakePage(); + }, + } as unknown as BrainEngine; + const source = { + getChunksWithEmbeddings: async () => [], + getTags: async () => [], + getTimeline: async () => [], + getRawData: async () => [], + } as unknown as BrainEngine; + + const page = fakePage({ content_hash: 'abc123' }); + await copyPageToTarget(source, target, page); + + const call = putPageCalls[0] as { page: Record }; + expect(call.page.content_hash).toBe('abc123'); + expect(call.page.type).toBe('note'); + }); +}); + +describe('runMigrateEngine — per-page failures are surfaced, not swallowed (#3194)', () => { + afterEach(() => { + _resetCliExitVerdictForTests(); + }); + + test('a page whose target write throws is excluded from the resume manifest and flips the exit verdict', async () => { + const gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-migrate-home-')); + const targetDbPath = join(mkdtempSync(join(tmpdir(), 'gbrain-migrate-target-')), 'brain.pglite'); + const prevGbrainHome = process.env.GBRAIN_HOME; + const prevDatabaseUrl = process.env.DATABASE_URL; + const prevGbrainDatabaseUrl = process.env.GBRAIN_DATABASE_URL; + const prevExitCode = process.exitCode; + + let source: PGLiteEngine | null = null; + let verifyEngine: PGLiteEngine | null = null; + const originalPutPage = PGLiteEngine.prototype.putPage; + + try { + // #427-style hermeticity: no live DATABASE_URL must leak into the + // config engine-inference logic (would force engine='postgres' with + // database_path cleared, unrelated to what we're testing here). + delete process.env.DATABASE_URL; + delete process.env.GBRAIN_DATABASE_URL; + process.env.GBRAIN_HOME = gbrainHome; + + // `runMigrateEngine`'s only use of the on-disk config is the + // "already using this engine" guard + preserving unrelated file-plane + // settings; it never reconnects using it (the caller-supplied + // `sourceEngine` instance is used directly). engine='postgres' here + // just satisfies "config.engine !== --to pglite" so the guard passes. + saveConfig({ engine: 'postgres', database_url: 'postgresql://unused/guard-only' }); + + source = new PGLiteEngine(); + await source.connect({}); + await source.initSchema(); + await source.putPage('good-page', { + type: 'note', title: 'Good', compiled_truth: 'good body', timeline: '', frontmatter: {}, + }); + await source.putPage('bad-page', { + type: 'note', title: 'Bad', compiled_truth: 'bad body', timeline: '', frontmatter: {}, + }); + + // Fault injection: the target's putPage throws for exactly one slug, + // simulating the real #3194 failure mode (a per-page write that + // can't land on the target) without needing a live Postgres target. + PGLiteEngine.prototype.putPage = async function ( + this: PGLiteEngine, + slug: string, + page: Parameters[1], + opts?: Parameters[2], + ) { + if (slug === 'bad-page') { + throw new Error('simulated unrecoverable write failure for bad-page'); + } + return originalPutPage.call(this, slug, page, opts); + }; + + await runMigrateEngine(source, ['--to', 'pglite', '--path', targetDbPath]); + + // 1. Exit verdict must reflect the partial failure. + expect(currentExitCode()).toBe(1); + + // 2. The resume manifest must exist (not cleared) and must exclude + // the failed page while including the successful one. + const manifestPath = join(gbrainHome, '.gbrain', 'migrate-manifest.json'); + expect(existsSync(manifestPath)).toBe(true); + const manifest = JSON.parse(readFileSync(manifestPath, 'utf-8')) as { completed_slugs: string[] }; + expect(manifest.completed_slugs).toContain('good-page'); + expect(manifest.completed_slugs).not.toContain('bad-page'); + + // 3. The target actually has the good page and does NOT have the bad + // one — i.e. the bad page did not silently vanish while counted + // as copied. + verifyEngine = new PGLiteEngine(); + await verifyEngine.connect({ database_path: targetDbPath }); + expect(await verifyEngine.getPage('good-page')).not.toBeNull(); + expect(await verifyEngine.getPage('bad-page')).toBeNull(); + await verifyEngine.disconnect(); + verifyEngine = null; + + // 4. A partial run must NOT flip the active config onto the + // incomplete target — otherwise every subsequent `gbrain` + // invocation would silently start using a brain missing pages, + // AND the natural retry below would hit the "already using X" + // guard instead of actually resuming. + expect(loadConfigFileOnly()?.engine).toBe('postgres'); + + // 5. Resume: fix the fault, re-run the SAME command with no --force. + // This must not hit the "target brain is not empty" guard (the + // target already has `good-page` from the run above) and must + // NOT re-wipe/lose `good-page` — only the previously-failed page + // should be (re-)written. + _resetCliExitVerdictForTests(); + PGLiteEngine.prototype.putPage = originalPutPage; + await runMigrateEngine(source, ['--to', 'pglite', '--path', targetDbPath]); + + expect(currentExitCode()).toBe(0); + expect(existsSync(manifestPath)).toBe(false); // clean run clears the manifest + expect(loadConfigFileOnly()?.engine).toBe('pglite'); // now safe to switch + + verifyEngine = new PGLiteEngine(); + await verifyEngine.connect({ database_path: targetDbPath }); + expect(await verifyEngine.getPage('good-page')).not.toBeNull(); + expect(await verifyEngine.getPage('bad-page')).not.toBeNull(); + } finally { + PGLiteEngine.prototype.putPage = originalPutPage; + if (source) await source.disconnect(); + if (verifyEngine) await verifyEngine.disconnect(); + _resetCliExitVerdictForTests(); + process.exitCode = prevExitCode; + if (prevGbrainHome !== undefined) process.env.GBRAIN_HOME = prevGbrainHome; else delete process.env.GBRAIN_HOME; + if (prevDatabaseUrl !== undefined) process.env.DATABASE_URL = prevDatabaseUrl; + if (prevGbrainDatabaseUrl !== undefined) process.env.GBRAIN_DATABASE_URL = prevGbrainDatabaseUrl; + rmSync(gbrainHome, { recursive: true, force: true }); + rmSync(join(targetDbPath, '..'), { recursive: true, force: true }); + } + }, 30000); + + test('a run where every page fails AFTER putPage lands still writes a manifest — no --force needed to resume', async () => { + const gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-migrate-home-')); + const targetDbPath = join(mkdtempSync(join(tmpdir(), 'gbrain-migrate-target-')), 'brain.pglite'); + const prevGbrainHome = process.env.GBRAIN_HOME; + const prevDatabaseUrl = process.env.DATABASE_URL; + const prevGbrainDatabaseUrl = process.env.GBRAIN_DATABASE_URL; + const prevExitCode = process.exitCode; + + let source: PGLiteEngine | null = null; + let verifyEngine: PGLiteEngine | null = null; + const originalGetRawData = PGLiteEngine.prototype.getRawData; + + try { + delete process.env.DATABASE_URL; + delete process.env.GBRAIN_DATABASE_URL; + process.env.GBRAIN_HOME = gbrainHome; + saveConfig({ engine: 'postgres', database_url: 'postgresql://unused/guard-only' }); + + source = new PGLiteEngine(); + await source.connect({}); + await source.initSchema(); + await source.putPage('only-page', { + type: 'note', title: 'Only', compiled_truth: 'only body', timeline: '', frontmatter: {}, + }); + + // Fault injection: the SOURCE's getRawData throws — this runs AFTER + // putPage has already landed the row on the target, so the page's + // copy fails mid-way rather than before anything was written. + // completed_slugs therefore never gets an entry for it. + PGLiteEngine.prototype.getRawData = async function ( + this: PGLiteEngine, + slug: string, + rdSource?: string, + opts?: { sourceId?: string }, + ) { + if (slug === 'only-page') throw new Error('simulated post-putPage failure'); + return originalGetRawData.call(this, slug, rdSource, opts); + }; + + await runMigrateEngine(source, ['--to', 'pglite', '--path', targetDbPath]); + expect(currentExitCode()).toBe(1); + + // The target actually has the row (putPage succeeded) even though + // the whole page-copy was counted as failed. + verifyEngine = new PGLiteEngine(); + await verifyEngine.connect({ database_path: targetDbPath }); + expect(await verifyEngine.getPage('only-page')).not.toBeNull(); + await verifyEngine.disconnect(); + verifyEngine = null; + + // The manifest file must exist on disk (with an empty completed_slugs) + // even though not a single page fully succeeded — otherwise the next + // invocation can't tell this was a resumable in-progress migration + // and would hit the non-empty guard demanding --force. + const manifestPath = join(gbrainHome, '.gbrain', 'migrate-manifest.json'); + expect(existsSync(manifestPath)).toBe(true); + const manifest = JSON.parse(readFileSync(manifestPath, 'utf-8')) as { completed_slugs: string[] }; + expect(manifest.completed_slugs).toEqual([]); + + // Retry with no --force: must resume cleanly (not hit the "target + // brain is not empty" abort) since a matching manifest is present. + _resetCliExitVerdictForTests(); + PGLiteEngine.prototype.getRawData = originalGetRawData; + await runMigrateEngine(source, ['--to', 'pglite', '--path', targetDbPath]); + expect(currentExitCode()).toBe(0); + expect(existsSync(manifestPath)).toBe(false); + } finally { + PGLiteEngine.prototype.getRawData = originalGetRawData; + if (source) await source.disconnect(); + if (verifyEngine) await verifyEngine.disconnect(); + _resetCliExitVerdictForTests(); + process.exitCode = prevExitCode; + if (prevGbrainHome !== undefined) process.env.GBRAIN_HOME = prevGbrainHome; else delete process.env.GBRAIN_HOME; + if (prevDatabaseUrl !== undefined) process.env.DATABASE_URL = prevDatabaseUrl; + if (prevGbrainDatabaseUrl !== undefined) process.env.GBRAIN_DATABASE_URL = prevGbrainDatabaseUrl; + rmSync(gbrainHome, { recursive: true, force: true }); + rmSync(join(targetDbPath, '..'), { recursive: true, force: true }); + } + }, 30000); + + test('--force always resets the manifest, even when the target looks empty (stale manifest from a recreated target)', async () => { + const gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-migrate-home-')); + const targetDir = mkdtempSync(join(tmpdir(), 'gbrain-migrate-target-')); + const targetDbPath = join(targetDir, 'brain.pglite'); + const prevGbrainHome = process.env.GBRAIN_HOME; + const prevDatabaseUrl = process.env.DATABASE_URL; + const prevGbrainDatabaseUrl = process.env.GBRAIN_DATABASE_URL; + const prevExitCode = process.exitCode; + + let source: PGLiteEngine | null = null; + let verifyEngine: PGLiteEngine | null = null; + + try { + delete process.env.DATABASE_URL; + delete process.env.GBRAIN_DATABASE_URL; + process.env.GBRAIN_HOME = gbrainHome; + saveConfig({ engine: 'postgres', database_url: 'postgresql://unused/guard-only' }); + + source = new PGLiteEngine(); + await source.connect({}); + await source.initSchema(); + await source.putPage('real-page', { + type: 'note', title: 'Real', compiled_truth: 'real body', timeline: '', frontmatter: {}, + }); + + // Simulate a stale manifest surviving a target that was recreated + // out-of-band (e.g. the operator deleted/rebuilt the target DB file + // but ~/.gbrain/migrate-manifest.json was left behind): a manifest + // matching this exact target_id claims `real-page` is already done, + // even though the target directory is otherwise fresh/empty. + const { migrationTargetId } = await import('../src/commands/migrate-engine.ts'); + const targetId = migrationTargetId({ engine: 'pglite', database_path: targetDbPath }); + const manifestPath = join(gbrainHome, '.gbrain', 'migrate-manifest.json'); + const fakeStaleManifest = { + completed_slugs: ['real-page'], + target_engine: 'pglite', + target_id: targetId, + schema_version: 2, + started_at: new Date().toISOString(), + }; + const { mkdirSync, writeFileSync } = await import('fs'); + mkdirSync(join(gbrainHome, '.gbrain'), { recursive: true }); + writeFileSync(manifestPath, JSON.stringify(fakeStaleManifest, null, 2)); + + // --force on an empty target must NOT trust that stale manifest — + // `real-page` must actually get copied, not skipped as "already done". + await runMigrateEngine(source, ['--to', 'pglite', '--path', targetDbPath, '--force']); + expect(currentExitCode()).toBe(0); + + verifyEngine = new PGLiteEngine(); + await verifyEngine.connect({ database_path: targetDbPath }); + expect(await verifyEngine.getPage('real-page')).not.toBeNull(); + } finally { + if (source) await source.disconnect(); + if (verifyEngine) await verifyEngine.disconnect(); + _resetCliExitVerdictForTests(); + process.exitCode = prevExitCode; + if (prevGbrainHome !== undefined) process.env.GBRAIN_HOME = prevGbrainHome; else delete process.env.GBRAIN_HOME; + if (prevDatabaseUrl !== undefined) process.env.DATABASE_URL = prevDatabaseUrl; + if (prevGbrainDatabaseUrl !== undefined) process.env.GBRAIN_DATABASE_URL = prevGbrainDatabaseUrl; + rmSync(gbrainHome, { recursive: true, force: true }); + rmSync(targetDir, { recursive: true, force: true }); + } + }, 30000); +});