fix(engine,resolver,migrate): put-after-soft-delete resurrection, sole-source guard, migrate failure accounting

- #3071: putPage's ON CONFLICT DO UPDATE now clears deleted_at in BOTH
  engines (engine parity), so a put after soft-delete resurrects the row
  instead of leaving fresh content invisible to deleted_at IS NULL reads.
- #3070: pickSoleNonDefaultSource only auto-routes when the 'default'
  source holds no live pages (the tier's original #1434 charter). An
  established default corpus keeps bare put/capture in 'default'.
  Fails open on probe error (legacy schema without deleted_at).
- #3194: gbrain migrate can no longer report success while pages are
  missing. Per-page copy failures are collected (kept out of the resume
  manifest so a re-run retries them), link-phase failures are collected
  instead of aborting the phase, and per-source live page counts are
  verified source-vs-target before the config switch. Any failure or
  count mismatch prints a summary, leaves config + manifest intact for
  resume, and exits non-zero. Soft-deleted pages are documented as
  deliberately excluded.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-07-23 11:00:57 -07:00
co-authored by Claude Fable 5
parent e79b8d5780
commit 54c7581f91
7 changed files with 400 additions and 83 deletions
+195 -80
View File
@@ -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<void> {
// 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<string[]> {
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<Array<{ source_id: string; source: number; target: number }>> {
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<void> {
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)}`);
}
+3
View File
@@ -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),
+3
View File
@@ -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),
+18 -2
View File
@@ -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<string | n
`SELECT id FROM sources WHERE local_path IS NOT NULL AND id != 'default'`,
);
}
if (rows.length === 1) return rows[0].id;
return null;
if (rows.length !== 1) return null;
// #3070: this tier's charter (#1434) is rescuing brains whose 'default'
// source is EMPTY. When default holds live pages, a bare put/capture must
// not silently re-route into the side source — fall through to tier 6.
// Fail-open on query error (legacy schema without deleted_at) to preserve
// the pre-#3070 behavior.
try {
const live = await engine.executeRaw<{ one: number }>(
`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;
}
/**
+109
View File
@@ -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<PGLiteEngine> {
const engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
return engine;
}
async function seedPage(engine: PGLiteEngine, slug: string): Promise<void> {
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<typeof origAddLink>) => {
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');
});
});
+20
View File
@@ -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');
+52 -1
View File
@@ -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<T>(sql: string, _params?: unknown[]): Promise<T[]> {
// #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 },