Compare commits

..
Author SHA1 Message Date
8900e478d1 fix(import): normalize mixed-case slugs before chunk upsert (#430)
putPage lowercases slugs via validateSlug, but upsertChunks queried
pages by the caller's raw slug — so a mixed-case slug through
importFromContent created the page row, then failed the chunk upsert
with 'Page not found' and rolled back the whole import.

Normalize via validateSlug at importFromContent entry and inside
_upsertChunksOnce on BOTH engines (postgres + pglite parity).

Takeover of #855, rebased onto current master shapes (batchRetry
wrapper / _upsertChunksOnce, rewritten importFromContent opts block).

Co-authored-by: Kage18 <Kage18@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-21 14:44:24 -07:00
6 changed files with 45 additions and 91 deletions
+2 -50
View File
@@ -3,11 +3,7 @@
*
* Usage:
* gbrain migrate --to supabase [--url <connection_string>]
* (--url is persisted to config.json, mode 0600, so the migrated brain
* works without env — #1271)
* gbrain migrate --to pglite [--path <db_path>]
* (an explicit --path destination is bootstrapped with its own
* <path>/.gbrain/config.json so GBRAIN_HOME=<path> just works — #1271)
* gbrain migrate --to <engine> --force (overwrite non-empty target)
*/
@@ -15,9 +11,9 @@ 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 { writeFileSync, readFileSync, existsSync, unlinkSync, mkdirSync, chmodSync } from 'fs';
import { writeFileSync, readFileSync, existsSync, unlinkSync } from 'fs';
import { createHash } from 'crypto';
import { resolve, join } from 'path';
import { resolve } from 'path';
import { createProgress } from '../core/progress.ts';
import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts';
@@ -63,31 +59,6 @@ export interface MigrateManifest {
started_at: string;
}
/**
* #1271 Finding 1: make an explicit `--to pglite --path P` destination usable
* as a standalone brain. Writes `P/.gbrain/config.json` (mode 0600, plus a
* `*` .gitignore) so `GBRAIN_HOME=P` resolves without a manual `gbrain init`.
* Never clobbers an existing config at the destination. Returns the written
* config path, or null when skipped.
*/
export function bootstrapDestinationConfig(dbPath: string): string | null {
const abs = resolve(dbPath);
const dir = join(abs, '.gbrain');
const file = join(dir, 'config.json');
if (existsSync(file)) return null;
mkdirSync(dir, { recursive: true });
const cfg: GBrainConfig = { engine: 'pglite', database_path: abs };
writeFileSync(file, JSON.stringify(cfg, null, 2) + '\n', { mode: 0o600 });
try { chmodSync(file, 0o600); } catch { /* platform-specific */ }
// Same worktree-safety pattern as saveConfig()'s ensureGitignore, scoped
// to the destination home. Don't clobber a user-customized .gitignore.
const gitignore = join(dir, '.gitignore');
if (!existsSync(gitignore)) {
writeFileSync(gitignore, '*\n', { mode: 0o600 });
}
return file;
}
export function migrationTargetId(config: EngineConfig): string {
const locator = config.engine === 'postgres'
? config.database_url ?? ''
@@ -381,25 +352,6 @@ export async function runMigrateEngine(sourceEngine: BrainEngine, args: string[]
};
saveConfig(newConfig);
// #1271 Finding 2 (by design, but say it out loud): the connection string
// is persisted so the migrated brain works without env. Mode 0600.
if (opts.targetEngine === 'postgres' && opts.targetUrl) {
console.error('Note: the --url connection string (including credentials) is persisted to config.json (mode 0600).');
}
// #1271 Finding 1: an explicit --path destination doubles as a standalone
// GBRAIN_HOME. Best-effort — never fail a completed migration over it.
if (opts.targetEngine === 'pglite' && opts.targetPath) {
try {
const written = bootstrapDestinationConfig(opts.targetPath);
if (written) {
console.log(`Destination bootstrapped: ${written} (usable via GBRAIN_HOME=${resolve(opts.targetPath)})`);
}
} catch (e) {
console.warn(` WARN could not bootstrap destination config: ${e instanceof Error ? e.message : String(e)}`);
}
}
// Clean up
clearManifest();
+7 -1
View File
@@ -36,7 +36,7 @@ import {
} from './embedding-context.ts';
import { loadSearchModeConfig, resolveSearchMode } from './search/mode.ts';
import { normalizeAliasList } from './search/alias-normalize.ts';
import { isUndefinedTableError, warnOncePerProcess } from './utils.ts';
import { isUndefinedTableError, warnOncePerProcess, validateSlug } from './utils.ts';
import { computeCorpusGeneration } from './contextual-retrieval-service.ts';
import { runGuardrails } from './guardrails.ts';
@@ -295,6 +295,12 @@ export async function importFromContent(
remote?: boolean;
} = {},
): Promise<ImportResult> {
// Normalize BEFORE any tx write: putPage lowercases via validateSlug but
// upsertChunks used to query by the caller's raw slug, so a mixed-case slug
// created the page row then failed the chunk upsert with "Page not found",
// rolling back the whole import (#430).
slug = validateSlug(slug);
// v0.18.0+ multi-source: when caller is syncing under a non-default source,
// every per-page tx call must carry `sourceId` so writes target the right
// (source_id, slug) row. Pre-fix, putPage relied on the schema DEFAULT and
+3
View File
@@ -2235,6 +2235,9 @@ export class PGLiteEngine implements BrainEngine {
}
private async _upsertChunksOnce(slug: string, chunks: ChunkInput[], opts?: { sourceId?: string }): Promise<void> {
// Normalize the same way putPage does — pages.slug is stored lowercased,
// so a raw mixed-case slug here would miss the row it just wrote (#430).
slug = validateSlug(slug);
const sourceId = opts?.sourceId ?? 'default';
// Source-scope the page-id lookup so duplicate slugs in different sources
+3
View File
@@ -2385,6 +2385,9 @@ export class PostgresEngine implements BrainEngine {
}
private async _upsertChunksOnce(slug: string, chunks: ChunkInput[], opts?: { sourceId?: string }): Promise<void> {
// Normalize the same way putPage does — pages.slug is stored lowercased,
// so a raw mixed-case slug here would miss the row it just wrote (#430).
slug = validateSlug(slug);
const sql = this.sql;
const sourceId = opts?.sourceId ?? 'default';
@@ -1,40 +0,0 @@
import { describe, expect, test } from 'bun:test';
import { mkdtempSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'fs';
import { tmpdir } from 'os';
import { join, resolve } from 'path';
import { bootstrapDestinationConfig } from '../src/commands/migrate-engine.ts';
import { loadConfigFileOnly } from '../src/core/config.ts';
import { withEnv } from './helpers/with-env.ts';
describe('migrate --to pglite destination bootstrap (#1271)', () => {
test('writes <path>/.gbrain/config.json so GBRAIN_HOME=<path> resolves a brain', async () => {
const dest = mkdtempSync(join(tmpdir(), 'gbrain-dest-'));
const written = bootstrapDestinationConfig(dest);
const file = join(dest, '.gbrain', 'config.json');
expect(written).toBe(file);
const cfg = JSON.parse(readFileSync(file, 'utf-8'));
expect(cfg.engine).toBe('pglite');
expect(cfg.database_path).toBe(resolve(dest));
expect(statSync(file).mode & 0o777).toBe(0o600);
// worktree safety: destination home is git-ignored like saveConfig()'s home
expect(readFileSync(join(dest, '.gbrain', '.gitignore'), 'utf-8')).toBe('*\n');
// The exact failure mode from #1271: config resolution under
// GBRAIN_HOME=<path> used to find nothing ("No brain configured").
await withEnv({ GBRAIN_HOME: dest }, () => {
const loaded = loadConfigFileOnly();
expect(loaded?.engine).toBe('pglite');
expect(loaded?.database_path).toBe(resolve(dest));
});
});
test('never clobbers an existing destination config', () => {
const dest = mkdtempSync(join(tmpdir(), 'gbrain-dest-'));
mkdirSync(join(dest, '.gbrain'), { recursive: true });
writeFileSync(join(dest, '.gbrain', 'config.json'), '{"engine":"postgres"}\n');
expect(bootstrapDestinationConfig(dest)).toBe(null);
expect(JSON.parse(readFileSync(join(dest, '.gbrain', 'config.json'), 'utf-8')).engine).toBe('postgres');
});
});
+30
View File
@@ -6,6 +6,7 @@
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { importFromContent } from '../src/core/import-file.ts';
import type { BrainEngine } from '../src/core/engine.ts';
import type { PageInput, ChunkInput } from '../src/core/types.ts';
@@ -181,6 +182,24 @@ describe('PGLiteEngine: Pages', () => {
const page = await engine.putPage('Test/UPPER', testPage);
expect(page.slug).toBe('test/upper');
});
test('importFromContent normalizes mixed-case slugs before all tx writes (#430)', async () => {
const result = await importFromContent(
engine,
'TestNamespace/Page-Name',
'---\ntype: note\ntitle: Mixed Case\n---\n\nbody text',
{ noEmbed: true },
);
expect(result.status).toBe('imported');
expect(result.slug).toBe('testnamespace/page-name');
const page = await engine.getPage('testnamespace/page-name');
expect(page).not.toBeNull();
expect(page!.title).toBe('Mixed Case');
const chunks = await engine.getChunks('testnamespace/page-name');
expect(chunks.length).toBeGreaterThan(0);
});
});
// ─────────────────────────────────────────────────────────────────
@@ -364,6 +383,17 @@ describe('PGLiteEngine: Chunks', () => {
expect(chunks[1].chunk_text).toBe('Chunk one');
});
test('upsertChunks normalizes mixed-case slugs like putPage (#430)', async () => {
await engine.putPage('Test/ChunkCase', testPage);
await engine.upsertChunks('Test/ChunkCase', [
{ chunk_index: 0, chunk_text: 'Mixed-case chunk', chunk_source: 'compiled_truth' },
]);
const chunks = await engine.getChunks('test/chunkcase');
expect(chunks.length).toBe(1);
expect(chunks[0].chunk_text).toBe('Mixed-case chunk');
});
test('upsertChunks removes orphan chunks', async () => {
await engine.putPage('test/orphan', testPage);
await engine.upsertChunks('test/orphan', [