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
8 changed files with 55 additions and 84 deletions
-7
View File
@@ -438,13 +438,6 @@ export async function runApplyMigrations(args: string[]): Promise<void> {
const result = await m.orchestrator(orchestratorOptsFrom(cli));
if (result.status === 'failed') {
console.error(`Migration v${m.version} reported status=failed.`);
// Surface each failed phase's detail — the ledger records it, but
// the operator needs it on stderr to act (#921).
for (const p of result.phases) {
if (p.status === 'failed') {
console.error(` phase ${p.name}: ${p.detail ?? '(no detail)'}`);
}
}
// Record the attempt as 'partial' (not 'complete') so the cap counts
// it. Don't let a failed orchestrator look like it never ran.
try {
+11 -15
View File
@@ -186,6 +186,17 @@ async function phaseBFenceFacts(
const localPathById = new Map<string, string | null>();
for (const s of sources) localPathById.set(s.id, s.local_path);
// Dirty-tree refusal: check every source's local_path before writing.
for (const [id, localPath] of localPathById) {
if (localPath && isLocalPathDirty(localPath)) {
return {
name: 'fence_facts',
status: 'failed',
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
};
}
}
// Walk legacy rows in (source_id, entity_slug) groups for per-page
// atomic writes.
const legacy = await engine.executeRaw<LegacyFactRow>(
@@ -224,21 +235,6 @@ async function phaseBFenceFacts(
groups.set(key, list);
}
// Dirty-tree refusal: check ONLY the sources we are about to write
// into. A dirty tree in an unrelated source (or zero fenceable rows
// at all) must not block a no-op or a targeted backfill (#927).
const targetSourceIds = new Set([...groups.keys()].map(k => k.split('\0')[0]));
for (const id of targetSourceIds) {
const localPath = localPathById.get(id);
if (localPath && isLocalPathDirty(localPath)) {
return {
name: 'fence_facts',
status: 'failed',
detail: `source "${id}" has uncommitted changes in ${localPath}. Commit or stash, then re-run.`,
};
}
}
for (const [key, group] of groups) {
const [sourceId, entitySlug] = key.split('\0');
const localPath = localPathById.get(sourceId)!;
+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';
-13
View File
@@ -180,16 +180,3 @@ describe('runApplyMigrations exit codes (v0.36.1.x #1062)', () => {
expect(src).toMatch(/All migrations up to date[\s\S]{0,80}process\.exit\(0\)/);
});
});
// #921: a failed orchestrator must print each failed phase's detail to
// stderr — not just "reported status=failed" — so the operator can act
// without digging through the ledger.
describe('failed migration prints phase detail (#921)', () => {
test('runner loops result.phases and console.errors failed phase details', async () => {
const { readFileSync } = await import('fs');
const src = readFileSync('src/commands/apply-migrations.ts', 'utf8');
expect(src).toMatch(
/reported status=failed[\s\S]{0,400}for \(const p of result\.phases\)[\s\S]{0,200}p\.status === 'failed'[\s\S]{0,200}console\.error\([\s\S]{0,80}p\.name[\s\S]{0,80}p\.detail/,
);
});
});
+1 -48
View File
@@ -10,11 +10,10 @@
* __setTestEngineOverride so we don't need a configured brain.
*/
import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test';
import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync, mkdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { execFileSync } from 'node:child_process';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { v0_32_2, __setTestEngineOverride, __testing } from '../src/commands/migrations/v0_32_2.ts';
@@ -239,52 +238,6 @@ describe('phaseBFenceFacts — happy path backfill', () => {
});
});
describe('phaseBFenceFacts — dirty-tree refusal scoping (#927)', () => {
let dirtyDir: string;
beforeEach(async () => {
// A second source whose local_path is a git repo with uncommitted changes.
dirtyDir = mkdtempSync(join(tmpdir(), 'mig-v0_32_2-dirty-'));
execFileSync('git', ['-C', dirtyDir, 'init', '-q']);
writeFileSync(join(dirtyDir, 'uncommitted.md'), 'dirty', 'utf-8');
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await (engine as any).db.query(
`INSERT INTO sources (id, name, local_path) VALUES ('other', 'other', $1)`,
[dirtyDir],
);
});
afterEach(async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
await (engine as any).db.query(`DELETE FROM sources WHERE id = 'other'`);
rmSync(dirtyDir, { recursive: true, force: true });
});
test('no legacy facts at all → complete, dirty unrelated source ignored', async () => {
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('complete');
expect(r.detail).toContain('scanned=0');
});
test('facts scoped to a clean source fence despite dirty unrelated source', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'Founded Acme' });
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('complete');
expect(r.detail).toContain('fenced=1');
expect(existsSync(join(brainDir, 'people/alice.md'))).toBe(true);
});
test('still refuses when the TARGETED source is dirty', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1', source_id: 'other' });
const r = await __testing.phaseBFenceFacts(engine, OPTS);
expect(r.status).toBe('failed');
expect(r.detail).toContain('"other"');
expect(r.detail).toContain('uncommitted changes');
});
});
describe('phaseCVerify', () => {
test('returns complete when fence + DB row counts match', async () => {
await seedLegacyFact({ entity_slug: 'people/alice', fact: 'F1' });
+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', [