mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
Masa
+216
-98
@@ -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<T extends Record<string, unknown>>(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<void> {
|
||||
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<void> {
|
||||
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).`);
|
||||
}
|
||||
|
||||
|
||||
@@ -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> = {}): 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<string, unknown>; 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<string, unknown> };
|
||||
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<typeof originalPutPage>[1],
|
||||
opts?: Parameters<typeof originalPutPage>[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);
|
||||
});
|
||||
Reference in New Issue
Block a user