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 47 additions and 266 deletions
+2 -11
View File
@@ -808,20 +808,12 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
// 'default'. Wrapped in try/catch so a doctor / single-source brain that
// never set up sources still returns 'default' silently.
let sourceId: string | undefined;
// #2561: when the source resolved via a NON-explicit tier (path-match /
// brain default / sole-non-default / seed default), unqualified search-shaped
// reads span every `config.federated = true` source. Computed here (the
// trusted local boundary) and consumed by federatedSearchScope in
// operations.ts, which additionally gates on ctx.remote === false.
let localFederated: string[] | undefined;
try {
const { resolveSourceWithTier, localFederatedSourceIds } = await import('./core/source-resolver.ts');
const { resolveSourceId } = await import('./core/source-resolver.ts');
// params.source is set when a CLI flag was parsed for the op (rare; most
// CLI ops don't take --source). Falls through to env/dotfile/path-match.
const explicit = (params.source as string | undefined) ?? null;
const resolved = await resolveSourceWithTier(engine, explicit);
sourceId = resolved.source_id;
localFederated = await localFederatedSourceIds(engine, resolved.source_id, resolved.tier);
sourceId = await resolveSourceId(engine, explicit);
} catch {
// Source resolution failed (e.g. sources table doesn't exist on a fresh
// pre-init brain). Leave sourceId unset; engine read methods fall through
@@ -842,7 +834,6 @@ async function makeContext(engine: BrainEngine, params: Record<string, unknown>)
// table). Matches dispatch.ts's auto-fill so the contract holds across
// every transport.
sourceId: sourceId ?? 'default',
...(localFederated ? { localFederatedSourceIds: localFederated } : {}),
};
}
+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
+2 -61
View File
@@ -424,23 +424,6 @@ export interface OperationContext {
* satisfied even on single-source brains.
*/
sourceId: string;
/**
* #2561 — federated read scope for UNQUALIFIED local CLI reads.
*
* Set ONLY by the local CLI's context builder (src/cli.ts makeContext), and
* only when the source resolved via a non-explicit tier (local_path /
* brain_default / sole_non_default / seed_default — NOT --source, NOT
* GBRAIN_SOURCE, NOT a .gbrain-source dotfile). Contains the resolved
* source first, then every other `config.federated = true` source, so an
* unqualified `gbrain search "X"` spans federated sources as
* docs/guides/multi-source-brains.md promises.
*
* Consumed exclusively by `federatedSearchScope` and ONLY when
* `ctx.remote === false` — a remote caller's scope stays governed by
* `ctx.auth.allowedSources` / scalar `ctx.sourceId` (source-isolation
* invariant, fail-closed).
*/
localFederatedSourceIds?: string[];
}
/**
@@ -556,45 +539,6 @@ export function resolveRequestedScope(
return sourceScopeOpts(ctx);
}
/**
* #2561 — source scope for the search-shaped read ops (`search`, `query`).
*
* Delegates to `resolveRequestedScope` (the single trust+grant resolver), then
* widens an UNQUALIFIED trusted-local scalar scope to the CLI-computed
* federated set (`ctx.localFederatedSourceIds`, resolved source first). This is
* what makes `sources add --federated` mean something for local search: a
* federated source participates in unqualified `gbrain search "X"` results.
*
* The expansion NEVER applies when:
* - the caller is not strictly trusted-local (`ctx.remote !== false`) —
* remote scope stays grant-governed (fail-closed source isolation);
* - a per-call `source_id` was passed (explicit wins, including `__all__`);
* - the resolver already produced a federated array (OAuth grant);
* - the CLI resolved the source from an explicit signal (--source / env /
* dotfile) — makeContext leaves `localFederatedSourceIds` unset then.
*
* Deliberately NOT inside `sourceScopeOpts`: code-intel ops collapse a
* multi-element scope to an error (`resolveCodeIntelScope`), and non-search
* reads (get_page, get_links, …) keep their long-standing scalar behavior.
*/
export function federatedSearchScope(
ctx: OperationContext,
sourceIdParam?: string,
): { sourceId?: string; sourceIds?: string[] } {
const scope = resolveRequestedScope(ctx, sourceIdParam);
if (
ctx.remote === false &&
sourceIdParam === undefined &&
scope.sourceId !== undefined &&
scope.sourceIds === undefined &&
ctx.localFederatedSourceIds !== undefined &&
ctx.localFederatedSourceIds.length > 1
) {
return { sourceIds: ctx.localFederatedSourceIds };
}
return scope;
}
/**
* Code-intel adapter for `resolveRequestedScope`. Graph traversal
* (code_callers/code_callees/code_blast/code_flow) is single-source by design —
@@ -1504,8 +1448,7 @@ const search: Operation = {
const queryText = p.query as string;
const limit = (p.limit as number) || 20;
const offset = (p.offset as number) || 0;
// #2561: unqualified trusted-local search spans federated sources.
const scope = federatedSearchScope(ctx);
const scope = sourceScopeOpts(ctx);
// T4/D5 — per-call mode honored ONLY for trusted/local callers so a remote
// OAuth client can't escalate to the costly tokenmax bundle. Local + unknown
@@ -1667,9 +1610,7 @@ const query: Operation = {
// is spread into BOTH the image-similarity searchVector path and the text
// hybridSearch path below, so both honor the same grant.
const sourceIdParam = typeof p.source_id === 'string' ? p.source_id : undefined;
// #2561: unqualified trusted-local query spans federated sources (per-call
// source_id / remote grants still resolve through resolveRequestedScope).
const querySourceScope = federatedSearchScope(ctx, sourceIdParam);
const querySourceScope = resolveRequestedScope(ctx, sourceIdParam);
// v0.27.1: image-similarity branch. Bypasses hybridSearch (which is
// text-only); embeds the image via embedMultimodal and runs a direct
+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';
-39
View File
@@ -353,45 +353,6 @@ export async function resolveSourceWithTier(
return { source_id: 'default', tier: 'seed_default' };
}
/**
* #2561 — compute the federated read scope for an UNQUALIFIED local CLI call.
*
* `sources add --federated` promises that a `config.federated = true` source
* "participates in unqualified `gbrain search` results"
* (docs/guides/multi-source-brains.md). This helper turns that promise into a
* scope: given the resolved source and WHICH tier resolved it, return
* `[resolvedSource, ...other federated source ids]` — or `undefined` when the
* expansion must not apply:
*
* - explicit tiers (`flag` / `env` / `dotfile`): the user named a source;
* scalar scope stands (that IS the qualified case);
* - no other federated source exists: keep the scalar fast path unchanged.
*
* Archived sources are excluded (same rationale as pickSoleNonDefaultSource);
* the archived column is v34+, so fall back to the un-archived query on older
* brains. Callers put the result on `OperationContext.localFederatedSourceIds`
* — consumed only by `federatedSearchScope` and only when `remote === false`.
*/
export async function localFederatedSourceIds(
engine: BrainEngine,
sourceId: string,
tier: SourceTier,
): Promise<string[] | undefined> {
if (tier === 'flag' || tier === 'env' || tier === 'dotfile') return undefined;
let rows: Array<{ id: string }>;
try {
rows = await engine.executeRaw<{ id: string }>(
`SELECT id FROM sources WHERE config->>'federated' = 'true' AND archived = false ORDER BY id`,
);
} catch {
rows = await engine.executeRaw<{ id: string }>(
`SELECT id FROM sources WHERE config->>'federated' = 'true' ORDER BY id`,
);
}
const ids = [sourceId, ...rows.map((r) => r.id).filter((id) => id !== sourceId)];
return ids.length > 1 ? ids : undefined;
}
/** Exposed for tests. */
export const __testing = {
readDotfileWalk,
-154
View File
@@ -1,154 +0,0 @@
/**
* #2561 — sources.config.federated participates in UNQUALIFIED local CLI
* search/query.
*
* Pre-fix: the local CLI always emitted a scalar `{sourceId}` scope (required
* field, auto-filled 'default'), so a source registered with
* `gbrain sources add --federated` was invisible to an unqualified
* `gbrain search "X"` — contradicting docs/guides/multi-source-brains.md
* ("Source participates in unqualified `gbrain search` results").
*
* Fix: the CLI context builder computes `ctx.localFederatedSourceIds`
* (resolved source + every other federated source) whenever the source
* resolved via a NON-explicit tier; `federatedSearchScope` widens the scalar
* scope to that set for the `search` / `query` ops — trusted-local only
* (`ctx.remote === false`), never for remote callers, never when a per-call
* `source_id` or an explicit --source/env/dotfile was given.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { localFederatedSourceIds } from '../src/core/source-resolver.ts';
import {
federatedSearchScope,
operations,
type OperationContext,
} from '../src/core/operations.ts';
let engine: PGLiteEngine;
const search = operations.find((o) => o.name === 'search')!;
function ctxOf(overrides: Partial<OperationContext> = {}): OperationContext {
return {
engine: engine as any,
config: {} as any,
logger: console as any,
dryRun: false,
remote: false,
sourceId: 'default',
...overrides,
};
}
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
// Seeded 'default' source is federated=true. Add:
// wiki — federated (must join unqualified search)
// private — NOT federated (must stay invisible unless explicitly named)
// oldnews — federated but archived (must stay excluded)
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config) VALUES ('wiki', 'wiki', '/tmp/wiki', '{"federated": true}'::jsonb)`,
);
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config) VALUES ('private', 'private', '/tmp/private', '{}'::jsonb)`,
);
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, config, archived) VALUES ('oldnews', 'oldnews', '/tmp/oldnews', '{"federated": true}'::jsonb, true)`,
);
const pages: Array<[slug: string, sourceId: string, where: string]> = [
['notes/home', 'default', 'default'],
['wiki/topic', 'wiki', 'wiki'],
['private/topic', 'private', 'private'],
['old/topic', 'oldnews', 'oldnews'],
];
for (const [slug, sourceId, where] of pages) {
await engine.putPage(slug, {
type: 'note', title: `Topic in ${where}`, compiled_truth: `the zebra telescope in ${where}`, frontmatter: {},
}, { sourceId });
await engine.upsertChunks(slug, [
{ chunk_index: 0, chunk_text: `the zebra telescope in ${where}`, chunk_source: 'compiled_truth' },
], { sourceId });
}
// Keyword-only search path: no embedding provider needed in tests.
await engine.setConfig('search.mcp_keyword_only', 'true');
}, 60_000);
afterAll(async () => {
if (engine) await engine.disconnect();
}, 60_000);
describe('localFederatedSourceIds — CLI-side scope computation', () => {
test('non-explicit tier: resolved source first, then other federated, archived excluded', async () => {
expect(await localFederatedSourceIds(engine, 'default', 'seed_default')).toEqual(['default', 'wiki']);
});
test('non-federated resolved source still joins its own scope', async () => {
expect(await localFederatedSourceIds(engine, 'private', 'brain_default')).toEqual(['private', 'default', 'wiki']);
});
test('explicit tiers (--source / env / dotfile) never expand', async () => {
expect(await localFederatedSourceIds(engine, 'default', 'flag')).toBeUndefined();
expect(await localFederatedSourceIds(engine, 'default', 'env')).toBeUndefined();
expect(await localFederatedSourceIds(engine, 'default', 'dotfile')).toBeUndefined();
});
test('single federated source (the resolved one) keeps the scalar fast path', async () => {
const solo = { executeRaw: async () => [{ id: 'default' }] } as any;
expect(await localFederatedSourceIds(solo, 'default', 'seed_default')).toBeUndefined();
});
});
describe('federatedSearchScope — trust + explicitness matrix', () => {
test('trusted local + unqualified widens to the federated set', () => {
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
expect(federatedSearchScope(ctx)).toEqual({ sourceIds: ['default', 'wiki'] });
});
test('remote caller NEVER widens (fail-closed), even if the field is set', () => {
const ctx = ctxOf({ remote: true, localFederatedSourceIds: ['default', 'wiki'] });
expect(federatedSearchScope(ctx)).toEqual({ sourceId: 'default' });
});
test('per-call source_id wins over the federated set', () => {
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
expect(federatedSearchScope(ctx, 'wiki')).toEqual({ sourceId: 'wiki' });
});
test('per-call __all__ keeps the whole-brain semantics for trusted local', () => {
const ctx = ctxOf({ localFederatedSourceIds: ['default', 'wiki'] });
expect(federatedSearchScope(ctx, '__all__')).toEqual({});
});
test('a federated OAuth grant wins over the local set', () => {
const ctx = ctxOf({
localFederatedSourceIds: ['default', 'wiki'],
auth: { allowedSources: ['a', 'b'] } as OperationContext['auth'],
});
expect(federatedSearchScope(ctx)).toEqual({ sourceIds: ['a', 'b'] });
});
test('no local federated set → unchanged scalar scope', () => {
expect(federatedSearchScope(ctxOf())).toEqual({ sourceId: 'default' });
});
});
describe('search op — unqualified local search spans federated sources', () => {
test('federated source results appear; non-federated + archived stay invisible', async () => {
const ctx = ctxOf({
localFederatedSourceIds: await localFederatedSourceIds(engine, 'default', 'seed_default'),
});
const results = (await search.handler(ctx, { query: 'zebra telescope' })) as Array<{ slug: string }>;
const slugs = results.map((r) => r.slug);
expect(slugs).toContain('notes/home');
expect(slugs).toContain('wiki/topic'); // pre-#2561 this was missing
expect(slugs).not.toContain('private/topic');
expect(slugs).not.toContain('old/topic');
});
test('explicit source resolution (no federated set on ctx) stays single-source', async () => {
const results = (await search.handler(ctxOf(), { query: 'zebra telescope' })) as Array<{ slug: string }>;
const slugs = results.map((r) => r.slug);
expect(slugs).toEqual(['notes/home']);
});
});
+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', [