mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(import): canonicalize slug in importFromContent so mixed-case put_page with tags doesn't roll back (#2436)
putPage lowercases the slug via validateSlug, but the tag/link/timeline reconcilers (tx.addTag, addLink, addTimelineEntry) query the slug as passed. A remote put_page with a capitalized slug (e.g. 'Projects/Team-Wiki/Quarterly-Roadmap') stored the page under 'projects/team-wiki/quarterly-roadmap', then threw 'addTag failed: page "…" not found' on the existence check, rolling back the entire write — so the page never persisted under either casing. Any agent driving the HTTP MCP server (where slugs arrive verbatim) lost every page whose slug carried a capital letter plus a frontmatter tag. Normalize the slug once at the top of importFromContent (the shared chokepoint for MCP put_page and CLI capture) so putPage and every reconciler agree on the canonical lowercased slug. No-op for disk imports (already slugifyPath output), idempotent with putPage's own validateSlug call. Engine-agnostic, so PGLite and Postgres move together. Adds test/put-page-mixed-case-slug-tags.test.ts pinning the regression on PGLite.
This commit is contained in:
+10
-1
@@ -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';
|
||||
|
||||
@@ -301,6 +301,15 @@ export async function importFromContent(
|
||||
// silently fabricated a duplicate at (default, slug) — causing later
|
||||
// bare-slug subqueries (getTags, deleteChunks, etc.) to crash with 21000.
|
||||
const sourceId = opts.sourceId;
|
||||
// Canonicalize the slug ONCE up front so every per-page write in this import
|
||||
// agrees on it. putPage lowercases via validateSlug, but the tag/link/timeline
|
||||
// reconcilers (tx.addTag, addLink, addTimelineEntry) query the slug as passed.
|
||||
// A mixed-case slug from a remote put_page (e.g. 'Projects/Team-Wiki/Roadmap')
|
||||
// therefore stored the page as 'projects/team-wiki/roadmap' then threw
|
||||
// `addTag failed: page "…" not found`, rolling back the whole write. Disk
|
||||
// imports already pass slugifyPath() output (lowercase), so this is a no-op
|
||||
// for them and idempotent with putPage's own internal call.
|
||||
slug = validateSlug(slug);
|
||||
// Reject oversized payloads before any parsing, chunking, or embedding happens.
|
||||
// Uses Buffer.byteLength to count UTF-8 bytes the same way disk size would,
|
||||
// so the network path behaves identically to the file path.
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
/**
|
||||
* Regression — put_page with a MIXED-CASE slug + frontmatter tags.
|
||||
*
|
||||
* Bug: `validateSlug` (utils.ts) lowercases, and `putPage` calls it — so the
|
||||
* page row is stored under the lowercased slug. But `addTag` (and addLink /
|
||||
* addTimelineEntry) query the RAW slug. A put_page with a capitalized slug
|
||||
* (e.g. 'Projects/Team-Wiki/Quarterly-Roadmap') therefore stored the page as
|
||||
* 'projects/team-wiki/quarterly-roadmap', then the tag-reconciliation loop
|
||||
* called addTag('Projects/Team-Wiki/Quarterly-Roadmap', …) whose existence
|
||||
* check found no row and threw `addTag failed: page "…" not found`, rolling
|
||||
* back the ENTIRE write — so the page never persisted under either casing.
|
||||
* A capital letter in a slug arriving over the HTTP MCP server (where slugs
|
||||
* are passed verbatim) plus a frontmatter tag was enough to trigger it.
|
||||
*
|
||||
* Fix: the put_page handler canonicalizes the slug once at the boundary
|
||||
* (validateSlug), so the subagent allow-list check, putPage, and the
|
||||
* tag/link/timeline reconciliation all agree on the lowercased slug.
|
||||
*
|
||||
* Runs against in-memory PGLite (hermetic, no DATABASE_URL), mirroring the
|
||||
* isolation discipline of put-page-provenance.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, beforeEach, afterAll } from 'bun:test';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { operations } from '../src/core/operations.ts';
|
||||
import type { OperationContext } from '../src/core/operations.ts';
|
||||
import { configureGateway, resetGateway, __setEmbedTransportForTests } from '../src/core/ai/gateway.ts';
|
||||
|
||||
const putPageOp = operations.find((o) => o.name === 'put_page')!;
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
// Same gateway-hermeticity guard as put-page-provenance.test.ts: pin the
|
||||
// embed model + stub the transport so put_page's embed never hits the net.
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { ...process.env, OPENAI_API_KEY: process.env.OPENAI_API_KEY || 'sk-test-stub' },
|
||||
});
|
||||
__setEmbedTransportForTests(async ({ values }: any) => ({
|
||||
embeddings: values.map(() => new Array(1536).fill(0)),
|
||||
usage: { tokens: 0 },
|
||||
}) as any);
|
||||
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
__setEmbedTransportForTests(null);
|
||||
resetGateway();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await engine.executeRaw('DELETE FROM pages', []);
|
||||
});
|
||||
|
||||
function makeCtx(opts: Partial<OperationContext> = {}): OperationContext {
|
||||
return {
|
||||
engine,
|
||||
config: { engine: 'pglite' as const },
|
||||
logger: {
|
||||
info: () => { /* noop */ },
|
||||
warn: () => { /* noop */ },
|
||||
error: () => { /* noop */ },
|
||||
},
|
||||
dryRun: false,
|
||||
remote: true,
|
||||
sourceId: 'default',
|
||||
...opts,
|
||||
};
|
||||
}
|
||||
|
||||
async function pageExists(slug: string): Promise<boolean> {
|
||||
const rows = await engine.executeRaw('SELECT id FROM pages WHERE slug = $1', [slug]) as unknown[];
|
||||
return rows.length === 1;
|
||||
}
|
||||
|
||||
async function tagsFor(slug: string): Promise<string[]> {
|
||||
const rows = await engine.executeRaw(
|
||||
'SELECT t.tag FROM tags t JOIN pages p ON p.id = t.page_id WHERE p.slug = $1 ORDER BY t.tag',
|
||||
[slug],
|
||||
) as Array<{ tag: string }>;
|
||||
return rows.map((r) => r.tag);
|
||||
}
|
||||
|
||||
describe('put_page — mixed-case slug + frontmatter tags', () => {
|
||||
test('capitalized slug with tags succeeds and lands under the canonical lowercased slug', async () => {
|
||||
const ctx = makeCtx({ remote: true });
|
||||
|
||||
// Pre-fix this threw `addTag failed: page "Projects/Team-Wiki/Quarterly-Roadmap" not found`.
|
||||
await putPageOp.handler(ctx, {
|
||||
slug: 'Projects/Team-Wiki/Quarterly-Roadmap',
|
||||
content: '---\ntype: note\ntitle: Quarterly Roadmap\ntags: [planning, draft]\n---\n\nMixed-case slug plus frontmatter tags.',
|
||||
});
|
||||
|
||||
// Page persisted under the lowercased canonical slug …
|
||||
expect(await pageExists('projects/team-wiki/quarterly-roadmap')).toBe(true);
|
||||
// … and NOT under the original mixed casing.
|
||||
expect(await pageExists('Projects/Team-Wiki/Quarterly-Roadmap')).toBe(false);
|
||||
// Tags reconciled onto the same canonical row (the step that used to throw).
|
||||
expect(await tagsFor('projects/team-wiki/quarterly-roadmap')).toEqual(['draft', 'planning']);
|
||||
});
|
||||
|
||||
test('lowercase slug with tags still works (no regression)', async () => {
|
||||
const ctx = makeCtx({ remote: true });
|
||||
await putPageOp.handler(ctx, {
|
||||
slug: 'projects/team-wiki/release-checklist',
|
||||
content: '---\ntype: note\ntitle: Release Checklist\ntags: [planning]\n---\n\nLowercase control case.',
|
||||
});
|
||||
expect(await pageExists('projects/team-wiki/release-checklist')).toBe(true);
|
||||
expect(await tagsFor('projects/team-wiki/release-checklist')).toEqual(['planning']);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user