diff --git a/src/commands/migrate-engine.ts b/src/commands/migrate-engine.ts index 76cf1f34b..ab4445ca2 100644 --- a/src/commands/migrate-engine.ts +++ b/src/commands/migrate-engine.ts @@ -10,7 +10,7 @@ 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'; @@ -143,6 +143,137 @@ export async function copyMigrationSources(source: BrainEngine, target: BrainEng } } +export interface PageCopyFailure { + source_id: string; + slug: string; + error: string; +} + +/** + * Copy one page's full payload (page row, chunks, tags, timeline, raw data) + * from source to target. Throws on the first failure — callers own + * collect-vs-abort policy. + */ +export async function copyOnePage( + sourceEngine: BrainEngine, + targetEngine: BrainEngine, + page: Page, +): Promise { + // 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); + } + + // 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); + } + + // Versions are snapshots; we don't recreate them on the target + // (createVersion takes a snapshot of current state, which we just set). +} + +/** + * Copy links for the given pages. Per-link failures are collected, not + * fatal (#3194) — one bad link must not abort the whole phase. + */ +export async function copyMigrationLinks( + sourceEngine: BrainEngine, + targetEngine: BrainEngine, + pages: Page[], + onTick?: () => void, +): Promise { + const failures: string[] = []; + for (const page of pages) { + const sourceOpts = { sourceId: page.source_id }; + try { + const links = await sourceEngine.getLinks(page.slug, sourceOpts); + for (const link of links) { + try { + await targetEngine.addLink( + link.from_slug, link.to_slug, + link.context, link.link_type, + undefined, undefined, undefined, + { fromSourceId: page.source_id, toSourceId: page.source_id }, + ); + } catch (e) { + failures.push(`${page.source_id}/${link.from_slug} -> ${link.to_slug}: ${e instanceof Error ? e.message : String(e)}`); + } + } + } catch (e) { + failures.push(`${page.source_id}/${page.slug}: getLinks failed: ${e instanceof Error ? e.message : String(e)}`); + } + onTick?.(); + } + return failures; +} + +/** + * Per-source live-page-count comparison between source and target (#3194). + * Soft-deleted pages are excluded on both sides — they are deliberately NOT + * migrated (listPages excludes them), so live counts are the contract. + * Returns mismatches only; empty array means the copy is complete. + */ +export async function comparePageCounts( + sourceEngine: BrainEngine, + targetEngine: BrainEngine, +): Promise> { + const q = `SELECT source_id, count(*)::int AS n FROM pages WHERE deleted_at IS NULL GROUP BY source_id`; + const toMap = (rows: Array<{ source_id: string; n: number | string }>) => + new Map(rows.map(r => [r.source_id, Number(r.n)])); + const src = toMap(await sourceEngine.executeRaw<{ source_id: string; n: number }>(q)); + const tgt = toMap(await targetEngine.executeRaw<{ source_id: string; n: number }>(q)); + const mismatches: Array<{ source_id: string; source: number; target: number }> = []; + for (const [sourceId, n] of src) { + const t = tgt.get(sourceId) ?? 0; + if (t < n) mismatches.push({ source_id: sourceId, source: n, target: t }); + } + return mismatches; +} + export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[]): Promise { const opts = parseArgs(args); const config = loadConfig(); @@ -226,7 +357,6 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] await copyMigrationSources(sourceEngine, targetEngine); // Get all source pages - const sourceStats = await sourceEngine.getStats(); const allPages = await sourceEngine.listPages({ limit: 100000 }); const pagesToMigrate = allPages.filter(p => !completedSet.has(makeManifestKey(p.source_id, p.slug))); @@ -235,70 +365,26 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] const progress = createProgress(cliOptsToProgressOptions(getCliOptions())); progress.start('migrate.copy_pages', pagesToMigrate.length); + // #3194: per-page try/catch — one bad page must not abort the run, but it + // must NEVER be silently dropped either. Failed pages are collected, kept + // OUT of the manifest (so a resume retries them), and reported at the end + // with a non-zero exit. let migrated = 0; + const failedPages: PageCopyFailure[] = []; 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 copyOnePage(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) { + failedPages.push({ + source_id: page.source_id, + slug: page.slug, + error: 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(); @@ -307,19 +393,7 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] // v0.32.8 F8: thread source_id so cross-source links migrate correctly. console.log('Copying links...'); progress.start('migrate.copy_links', allPages.length); - for (const page of allPages) { - const sourceOpts = { sourceId: page.source_id }; - const links = await sourceEngine.getLinks(page.slug, sourceOpts); - for (const link of links) { - await targetEngine.addLink( - link.from_slug, link.to_slug, - link.context, link.link_type, - undefined, undefined, undefined, - { fromSourceId: page.source_id, toSourceId: page.source_id }, - ); - } - progress.tick(1); - } + const failedLinks = await copyMigrationLinks(sourceEngine, targetEngine, allPages, () => progress.tick(1)); progress.finish(); // Copy config (selective). @@ -339,6 +413,39 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] if (val) await targetEngine.setConfig(key, val); } + // #3194: end-of-run verification — per-source live page counts must match + // before we call this migration a success. Catches silent drops (e.g. the + // 100k listPages cap) that per-page error handling can't see. + let countMismatches: Array<{ source_id: string; source: number; target: number }> = []; + try { + countMismatches = await comparePageCounts(sourceEngine, targetEngine); + } catch (e) { + console.warn(` WARN page-count verification could not run: ${e instanceof Error ? e.message : String(e)}`); + } + + if (failedPages.length > 0 || failedLinks.length > 0 || countMismatches.length > 0) { + if (failedPages.length > 0) { + console.error(`\n${failedPages.length} page(s) FAILED to migrate:`); + for (const f of failedPages.slice(0, 20)) { + console.error(` ${f.source_id}/${f.slug}: ${f.error}`); + } + if (failedPages.length > 20) console.error(` ... and ${failedPages.length - 20} more`); + } + if (failedLinks.length > 0) { + console.error(`\n${failedLinks.length} link copy failure(s):`); + for (const f of failedLinks.slice(0, 20)) console.error(` ${f}`); + if (failedLinks.length > 20) console.error(` ... and ${failedLinks.length - 20} more`); + } + for (const m of countMismatches) { + console.error(` WARN pages[${m.source_id}]: target has ${m.target}, source has ${m.source} live pages`); + } + console.error(`\nMigration INCOMPLETE: ${migrated} pages transferred, ${failedPages.length} failed.`); + console.error('Config NOT switched; resume manifest kept. Fix the errors above and re-run the same migrate command to retry.'); + process.exitCode = 1; + await targetEngine.disconnect(); + return; + } + // 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. @@ -360,14 +467,22 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[] if (config.engine === 'pglite' && config.database_path) { console.log(`Original PGLite brain preserved at ${config.database_path} (backup).`); } + // Policy note (#3194): soft-deleted pages are deliberately NOT migrated. + try { + const sd = await sourceEngine.executeRaw<{ n: number | string }>( + `SELECT count(*)::int AS n FROM pages WHERE deleted_at IS NOT NULL`, + ); + const n = Number(sd[0]?.n ?? 0); + if (n > 0) console.log(`Note: ${n} soft-deleted page(s) were excluded (they remain in the source brain).`); + } catch { /* legacy schema without deleted_at */ } // Post-migrate verification: confirm the target is healthy before we - // leave the user. Catches incomplete copies, schema drift, and missing - // embeddings immediately instead of on next CLI use. Non-fatal — prints - // warnings and keeps going so the user sees the full picture. + // leave the user. Catches schema drift and missing embeddings immediately + // instead of on next CLI use. Non-fatal — prints warnings and keeps going + // so the user sees the full picture. console.log('\nVerifying target...'); try { - await verifyTarget(targetEngine, sourceStats.page_count); + await verifyTarget(targetEngine, allPages.length); } catch (e) { console.warn(` Verification could not complete: ${e instanceof Error ? e.message : String(e)}`); } diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index f74e4e964..db1f6c390 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -1048,6 +1048,9 @@ export class PGLiteEngine implements BrainEngine { frontmatter = EXCLUDED.frontmatter, content_hash = EXCLUDED.content_hash, updated_at = now(), + -- #3071: a put after soft-delete is a resurrection — clear deleted_at + -- so the fresh content is visible to deleted_at IS NULL reads. + deleted_at = NULL, effective_date = COALESCE(EXCLUDED.effective_date, pages.effective_date), effective_date_source = COALESCE(EXCLUDED.effective_date_source, pages.effective_date_source), import_filename = COALESCE(EXCLUDED.import_filename, pages.import_filename), diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index 6deaad784..01bb2c3b5 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -1110,6 +1110,9 @@ export class PostgresEngine implements BrainEngine { frontmatter = EXCLUDED.frontmatter, content_hash = EXCLUDED.content_hash, updated_at = now(), + -- #3071: a put after soft-delete is a resurrection — clear deleted_at + -- so the fresh content is visible to deleted_at IS NULL reads. + deleted_at = NULL, effective_date = COALESCE(EXCLUDED.effective_date, pages.effective_date), effective_date_source = COALESCE(EXCLUDED.effective_date_source, pages.effective_date_source), import_filename = COALESCE(EXCLUDED.import_filename, pages.import_filename), diff --git a/src/core/source-resolver.ts b/src/core/source-resolver.ts index 8b9f3bada..e2b5ed285 100644 --- a/src/core/source-resolver.ts +++ b/src/core/source-resolver.ts @@ -167,6 +167,8 @@ export async function resolveSourceId( * - 2+ non-default sources are registered (ambiguous — user must pick) * - the only non-default source has a NULL local_path (no on-disk shape) * - the only registered source IS 'default' + * - the 'default' source holds live pages (#3070 — an established default + * corpus means bare writes stay in 'default') * * Excludes archived sources (`archived = false`) so a soft-deleted source * doesn't auto-resolve. Shared by `resolveSourceId` and `resolveSourceWithTier` @@ -185,8 +187,22 @@ async function pickSoleNonDefaultSource(engine: BrainEngine): Promise( + `SELECT 1 AS one FROM pages WHERE source_id = 'default' AND deleted_at IS NULL LIMIT 1`, + ); + if (live.length > 0) return null; + } catch { + // legacy schema — keep the sole-source auto-route + } + return rows[0].id; } /** diff --git a/test/migrate-engine-copy-failures.test.ts b/test/migrate-engine-copy-failures.test.ts new file mode 100644 index 000000000..6b356e2e5 --- /dev/null +++ b/test/migrate-engine-copy-failures.test.ts @@ -0,0 +1,109 @@ +/** + * #3194 — migrate-engine must never report success while pages are missing. + * + * Pins the three mechanisms added for #3194: + * - copyOnePage: full per-page payload copy (page, chunks, tags, timeline). + * - copyMigrationLinks: per-link failures are COLLECTED, not fatal — one + * bad link no longer aborts the whole link phase. + * - comparePageCounts: per-source live-count verification catches silent + * drops (soft-deleted pages excluded on both sides by design). + * + * Runs against PGLite (same SQL contract as Postgres, DATABASE_URL-free). + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { + copyOnePage, + copyMigrationLinks, + comparePageCounts, + copyMigrationSources, +} from '../src/commands/migrate-engine.ts'; + +delete process.env.GBRAIN_PGLITE_SNAPSHOT; + +async function setupBrain(): Promise { + const engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + return engine; +} + +async function seedPage(engine: PGLiteEngine, slug: string): Promise { + await engine.putPage(slug, { + type: 'note' as any, + title: slug, + compiled_truth: `Content of ${slug}`, + timeline: '', + frontmatter: {}, + }); +} + +describe('#3194 migrate-engine copy failure handling', () => { + let source: PGLiteEngine; + let target: PGLiteEngine; + + beforeAll(async () => { + source = await setupBrain(); + target = await setupBrain(); + await seedPage(source, 'pages/a'); + await seedPage(source, 'pages/b'); + await seedPage(source, 'pages/c'); + await source.addLink('pages/a', 'pages/b', 'ctx', 'related'); + await source.addLink('pages/a', 'pages/c', 'ctx', 'related'); + await copyMigrationSources(source, target); + }, 30000); + + afterAll(async () => { + await source.disconnect(); + await target.disconnect(); + }); + + test('comparePageCounts reports missing pages, then clears once complete', async () => { + const pages = await source.listPages({ limit: 1000 }); + expect(pages.length).toBe(3); + + // Copy only 2 of 3 — verification must flag the gap. + await copyOnePage(source, target, pages[0]); + await copyOnePage(source, target, pages[1]); + const mismatches = await comparePageCounts(source, target); + expect(mismatches).toEqual([{ source_id: 'default', source: 3, target: 2 }]); + + // Copy the third — verification clears. + await copyOnePage(source, target, pages[2]); + expect(await comparePageCounts(source, target)).toEqual([]); + }); + + test('comparePageCounts ignores soft-deleted pages (deliberately not migrated)', async () => { + await seedPage(source, 'pages/ghost'); + await source.softDeletePage('pages/ghost'); + expect(await comparePageCounts(source, target)).toEqual([]); + }); + + test('copyMigrationLinks collects per-link failures instead of aborting the phase', async () => { + const pages = await source.listPages({ limit: 1000 }); + + // Fail exactly one addLink; the rest must still be copied. + const origAddLink = target.addLink.bind(target); + let failedOnce = false; + (target as any).addLink = async (...args: Parameters) => { + if (!failedOnce && args[1] === 'pages/b') { + failedOnce = true; + throw new Error('injected addLink failure'); + } + return origAddLink(...args); + }; + + const failures = await copyMigrationLinks(source, target, pages); + (target as any).addLink = origAddLink; + + expect(failures.length).toBe(1); + expect(failures[0]).toContain('pages/b'); + expect(failures[0]).toContain('injected addLink failure'); + + // The other link survived the injected failure. + const links = await target.getLinks('pages/a'); + const toSlugs = links.map(l => l.to_slug); + expect(toSlugs).toContain('pages/c'); + }); +}); diff --git a/test/pages-soft-delete.test.ts b/test/pages-soft-delete.test.ts index 070c5b9f5..a789d5347 100644 --- a/test/pages-soft-delete.test.ts +++ b/test/pages-soft-delete.test.ts @@ -229,6 +229,26 @@ describe('getPage / listPages includeDeleted contract (Q3 IRON RULE)', () => { expect(restored!.deleted_at).toBeFalsy(); }); + test('#3071: putPage after soft-delete resurrects the row (clears deleted_at)', async () => { + await seedPage(engine, 'people/zed'); + await engine.softDeletePage('people/zed'); + expect(await engine.getPage('people/zed')).toBeNull(); + + // Re-put over the soft-deleted row — must clear deleted_at, not leave + // the fresh content invisible to every deleted_at IS NULL read. + await engine.putPage('people/zed', { + type: 'note' as any, + title: 'Zed v2', + compiled_truth: 'Resurrected content', + timeline: '', + frontmatter: {}, + }); + const restored = await engine.getPage('people/zed'); + expect(restored).not.toBeNull(); + expect(restored!.compiled_truth).toBe('Resurrected content'); + expect(restored!.deleted_at).toBeFalsy(); + }); + test('listPages excludes soft-deleted by default', async () => { await seedPage(engine, 'people/kim'); await seedPage(engine, 'people/larry'); diff --git a/test/source-resolver-sole-non-default.test.ts b/test/source-resolver-sole-non-default.test.ts index c9648b3b7..187463d2d 100644 --- a/test/source-resolver-sole-non-default.test.ts +++ b/test/source-resolver-sole-non-default.test.ts @@ -26,10 +26,18 @@ import { withEnv } from './helpers/with-env.ts'; type StubSource = { id: string; local_path: string | null; archived?: boolean }; -function makeStub(sources: StubSource[], globalDefault: string | null = null) { +function makeStub( + sources: StubSource[], + globalDefault: string | null = null, + defaultHasLivePages = false, +) { return { kind: 'pglite' as const, async executeRaw(sql: string, _params?: unknown[]): Promise { + // #3070 guard probe: does 'default' hold live pages? + if (sql.includes(`FROM pages WHERE source_id = 'default'`)) { + return (defaultHasLivePages ? [{ one: 1 }] : []) as unknown as T[]; + } // Two query shapes hit in the resolver: // 1. tier 4 (local_path match): SELECT id, local_path FROM sources WHERE local_path IS NOT NULL // 2. assertSourceExists: SELECT id FROM sources WHERE id = $1 @@ -132,6 +140,49 @@ describe('#1434 — sole_non_default tier', () => { }); }); + test('#3070: does NOT fire when default holds live pages (established corpus wins)', async () => { + const engine = makeStub( + [ + { id: 'default', local_path: null }, + { id: 'studiovault', local_path: '/Users/india/vault' }, + ], + null, + true, // default has live pages + ); + const result = await resolveSourceWithTier(engine, null, '/tmp'); + expect(result.source_id).toBe('default'); + expect(result.tier).toBe('seed_default'); + }); + + test('#3070: fires when default is empty (original #1434 charter preserved)', async () => { + const engine = makeStub( + [ + { id: 'default', local_path: null }, + { id: 'studiovault', local_path: '/Users/india/vault' }, + ], + null, + false, // default empty + ); + const result = await resolveSourceWithTier(engine, null, '/tmp'); + expect(result.source_id).toBe('studiovault'); + expect(result.tier).toBe('sole_non_default'); + }); + + test('#3070: probe failure fails open (legacy schema keeps the auto-route)', async () => { + const base = makeStub([ + { id: 'default', local_path: null }, + { id: 'studiovault', local_path: '/Users/india/vault' }, + ]); + const origExecuteRaw = (base as any).executeRaw.bind(base); + (base as any).executeRaw = async (sql: string, params?: unknown[]) => { + if (sql.includes('FROM pages')) throw new Error('column "deleted_at" does not exist'); + return origExecuteRaw(sql, params); + }; + const result = await resolveSourceWithTier(base, null, '/tmp'); + expect(result.source_id).toBe('studiovault'); + expect(result.tier).toBe('sole_non_default'); + }); + test('archived non-default source is ignored (does not count toward the 1)', async () => { const engine = makeStub([ { id: 'default', local_path: null },