diff --git a/CHANGELOG.md b/CHANGELOG.md index bf7c55fbb..6aeaf6617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to GBrain will be documented in this file. +## [0.42.40.0] - 2026-06-09 + +**`gbrain extract --stale` no longer aborts partway through a brain that contains emoji or other non-BMP characters.** On a large brain, link/timeline extraction could die with `invalid input syntax for type json` and commit nothing — and because the staleness bookmark only advances on a clean finish, every retry re-hit the same point and extraction stayed wedged. The cause: the link-context excerpt was sliced by raw UTF-16 index, so a window boundary landing inside an emoji's surrogate pair left an unpaired surrogate half in the text, which Postgres rejects when the batch is serialized to JSONB — taking down the whole batch, not just the one row. (PGLite is more permissive here, so this primarily bit the managed-Postgres engine.) + +The fix well-forms free text before it is serialized: any unpaired surrogate half is replaced with the Unicode replacement character before it reaches the database. It is applied at the slicer and, as defense in depth, centrally at the batch-insert boundary for every free-text field — link context, timeline summary/detail/source, take claim/source — across both the batch and single-row write paths and both engines. Identity fields (slugs, source ids, holders) are deliberately left untouched, so a malformed identifier still fails loudly instead of being silently rewritten. The same well-forming helper now also backs the brainstorm prompt path, replacing a hand-rolled version that left back-to-back malformed characters half-cleaned. + +### Fixed +- **`extract --stale` runs to completion on brains with emoji / non-BMP text (gbrain#2011).** The link-context slicer no longer leaves an unpaired UTF-16 surrogate that Postgres rejects at the JSONB cast and aborts the whole batch. A 192K-page brain that died at ~1,550 pages now sweeps clean. +- **Defense in depth at the batch-insert boundary.** Every free-text field written to JSONB (link context; timeline summary/detail/source; take claim/source) is now NUL- and surrogate-sanitized centrally, in both the batch and single-row paths and on both engines, so no future slicer can re-trigger this class of crash. Identity/security fields stay un-sanitized and fail closed. + +### Changed +- **One shared well-forming primitive.** The brainstorm cross-prompt path now uses the same surrogate-cleaning helper, which also fixes a case where consecutive malformed characters were only half-cleaned. + +### To take advantage of v0.42.40.0 +`gbrain upgrade`. No configuration needed. If a `gbrain extract` run had been stalling at the same page count every time, this is the release that unsticks it — the next run resumes and completes. ## [0.42.39.0] - 2026-06-09 **Your agent now learns a brain page exists the moment you name someone — instead of talking about a close contact for four messages without ever opening their page.** gbrain was great at *storing* knowledge and at injecting deterministic per-turn context, but it never taught the agent the *policy* of retrieval: when to look something up, and what to pull. That lived in each user's hand-rolled instructions and failed silently. The Retrieval Reflex makes it a property of having a brain. diff --git a/TODOS.md b/TODOS.md index fc74cfaf6..ba96fc81a 100644 --- a/TODOS.md +++ b/TODOS.md @@ -124,8 +124,9 @@ context). Deliberately scoped OUT of that PR. See plan + GSTACK REVIEW REPORT at batch error, retry the batch element-by-element so one bad row can't abort a 353K-page `extract --stale` sweep, logging the offending `(from_slug, context)` instead of dying. The durable JSONB fix removed the known crash class (malformed - array literal) and NUL-stripping removed the other known jsonb-parse failure, so - there is no remaining data-dependent crash for this to catch *today* — it's + array literal), NUL-stripping removed a second jsonb-parse failure, and + v0.42.40.0 lone-surrogate well-forming (#2011) removed a third, so there is no + remaining *known* data-dependent crash for this to catch *today* — it's belt-and-suspenders against unknown future per-row failures. Wire it in `addLinksBatch`/`addTimelineEntriesBatch`/`addTakesBatch` (or in `batchRetry` as a post-classification fallback). Issue #1861 option 2. diff --git a/VERSION b/VERSION index 2a5cc6444..cc968e114 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.42.39.0 +0.42.40.0 \ No newline at end of file diff --git a/docs/TESTING.md b/docs/TESTING.md index 84c05e39b..a02ee8750 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -141,7 +141,7 @@ Unit tests and what they cover: - `test/yaml-lite.test.ts` — YAML parsing. - `test/check-update.test.ts` — version check + update CLI. - `test/pglite-engine.test.ts` — PGLite engine, all BrainEngine methods including `addLinksBatch` / `addTimelineEntriesBatch` (empty batch, missing optionals, within-batch dedup via ON CONFLICT, missing-slug rows dropped by JOIN, half-existing batch, batch of 100) plus `connect()` error-wrap assertion (original error nested, #223 link in message, lock released). -- `test/links-timeline-jsonb-poison.test.ts` — gbrain#1861 PGLite half (always-on, no `DATABASE_URL`). Locks the `jsonb_to_recordset` batch-insert path for links/timeline/takes against free-text "poison" payloads (commas, quotes, backslashes, braces, em-dashes) and asserts NUL is stripped from free-text body fields but rejected in identity fields. The Postgres lane (`test/e2e/jsonb-batch-poison-postgres.test.ts`) is the one that actually reproduced the original crash. +- `test/links-timeline-jsonb-poison.test.ts` — gbrain#1861 PGLite half (always-on, no `DATABASE_URL`). Locks the `jsonb_to_recordset` batch-insert path for links/timeline/takes against free-text "poison" payloads (commas, quotes, backslashes, braces, em-dashes) and asserts NUL is stripped from free-text body fields but rejected in identity fields. gbrain#2011 adds lone-UTF-16-surrogate cases: every free-text field (link context; timeline summary/detail/source; take claim/source) well-forms to U+FFFD across batch + scalar write paths, while a surrogate in an identity field (slug) still fail-closed rejects the batch. The Postgres lane (`test/e2e/jsonb-batch-poison-postgres.test.ts`) is the one that actually reproduced the original crash. - `test/engine-factory.test.ts` — engine factory + dynamic imports. - `test/integrations.test.ts` — recipe parsing, CLI routing, recipe validation. - `test/publish.test.ts` — content stripping, encryption, password generation, HTML output. @@ -212,7 +212,7 @@ E2E tests live in `test/e2e/` and run against real Postgres+pgvector (require `D - `bun run test:e2e` runs Tier 1 (mechanical, all operations, no API keys). Includes dedicated cases for the postgres-engine `addLinksBatch` / `addTimelineEntriesBatch` bind path — postgres-js's JSONB bind (`jsonb_to_recordset(($1::jsonb)->'rows')`) differs from PGLite's and gets its own coverage. - `test/e2e/search-quality.test.ts` — search quality against PGLite (no API keys, in-memory). - `test/e2e/graph-quality.test.ts` — knowledge graph pipeline (auto-link via put_page, reconciliation, traversePaths) against PGLite in-memory. -- `test/e2e/jsonb-batch-poison-postgres.test.ts` — gbrain#1861 regression, the engine that actually crashed. Seeds free-text "poison" context (Zoom URL with `?pwd=`, commas, quotes, Windows backslash path, braces, em-dash) and asserts the links/timeline/takes batch writers no longer error with "malformed array literal"; also asserts NUL is stripped from free-text bodies (`context`/`summary`/`detail`/`claim`) and still rejected in identity fields. `DATABASE_URL`-gated. +- `test/e2e/jsonb-batch-poison-postgres.test.ts` — gbrain#1861 regression, the engine that actually crashed. Seeds free-text "poison" context (Zoom URL with `?pwd=`, commas, quotes, Windows backslash path, braces, em-dash) and asserts the links/timeline/takes batch writers no longer error with "malformed array literal"; also asserts NUL is stripped from free-text bodies (`context`/`summary`/`detail`/`claim`) and still rejected in identity fields. gbrain#2011 adds the lone-surrogate crash lock: a lone UTF-16 surrogate in free text (the value that aborted `extract --stale` with `22P02` on Supabase) well-forms to U+FFFD across batch + scalar paths (incl. timeline + take `source`), while a surrogate in an identity field still rejects the batch. `DATABASE_URL`-gated. - `test/e2e/postgres-jsonb.test.ts` — round-trips all 5 JSONB write sites (`pages.frontmatter`, `raw_data.data`, `ingest_log.pages_updated`, `files.metadata`, `page_versions.frontmatter`) against real Postgres and asserts `jsonb_typeof='object'` plus `->>'key'` returns the expected scalar. Guards against the double-encode bug. - `test/e2e/integrity-batch.test.ts` — parity for `scanIntegrity`'s batch-load fast path vs sequential. Cases (dedup, hits, validate, topPages) seed a fixture and assert both paths return identical results. Dedup case uses raw SQL via `getConn().unsafe()` to seed a `(test-source-2, people/alice)` row alongside the default-source row, since `engine.putPage` doesn't take a `source_id`. Pins multi-source overcounting; the "multi-source duplicate slugs scan once" case expects both batch + sequential paths to report 2. - `test/e2e/jsonb-roundtrip.test.ts` — companion regression against the 4 doctor-scanned JSONB sites. Assertion-level overlap with `postgres-jsonb.test.ts` is intentional defense-in-depth: if doctor's scan surface drifts from the actual write surface, one of these tests catches it. diff --git a/docs/incidents/2026-05-20-lsd-cost-explosion.md b/docs/incidents/2026-05-20-lsd-cost-explosion.md index 96508c948..ed69d84e6 100644 --- a/docs/incidents/2026-05-20-lsd-cost-explosion.md +++ b/docs/incidents/2026-05-20-lsd-cost-explosion.md @@ -208,9 +208,10 @@ architectural rounds shipped in the budget-cathedral wave that followed: - **P3 (judge chunking):** `runJudge` in `src/core/brainstorm/judges.ts` auto-chunks at 100 ideas/call. Context-window overflow is structurally prevented. -- **P4 (unicode sanitization):** `sanitizeUnicode` in - `src/core/brainstorm/orchestrator.ts` strips unpaired surrogates before - serialization. +- **P4 (unicode sanitization):** `ensureWellFormed` (in `src/core/text-safe.ts`, + used by `src/core/brainstorm/orchestrator.ts`) replaces unpaired surrogates + with U+FFFD before serialization. (Consolidated from the original hand-rolled + `sanitizeUnicode` in v0.42.40.0 / #2011.) - **P5 (BudgetTracker at the gateway layer):** new `src/core/budget/budget-tracker.ts` is the canonical primitive. The gateway's `withBudgetTracker(tracker, fn)` composes via diff --git a/package.json b/package.json index e986cad10..85559b546 100644 --- a/package.json +++ b/package.json @@ -143,5 +143,5 @@ "bun": ">=1.3.10" }, "license": "MIT", - "version": "0.42.39.0" + "version": "0.42.40.0" } diff --git a/src/core/batch-rows.ts b/src/core/batch-rows.ts index c1b346c1c..704f41bd9 100644 --- a/src/core/batch-rows.ts +++ b/src/core/batch-rows.ts @@ -18,26 +18,34 @@ * LinkBatchInput[] ---+ * TimelineInput[] ----+--> build*Rows() --> [{...}, ...] --> { rows } wrapper * TakeBatchInput[] ---+ | | - * stripNul free-text executeRawJsonb - * fields only $1::jsonb -> 'rows' + * sanitizeForJsonb executeRawJsonb + * free-text fields only $1::jsonb -> 'rows' * jsonb_to_recordset(...) * - * NUL POLICY (codex P0 hardening): Postgres `jsonb` rejects the Unicode NUL - * escape, and Postgres `text` cannot store a NUL either, so the OLD - * `unnest(::text[])` path rejected (errored) any row carrying an embedded NUL. - * We deliberately PRESERVE that reject semantics for IDENTITY and - * security-relevant fields: slugs, source_ids, `holder`, `kind`, dates, and the - * enum-ish `link_type` / `link_source` / `origin_slug` / `origin_field`. Those - * are left UN-stripped, so a NUL in them still errors the batch and can never - * silently retarget a row to a different page/source or normalize a `holder` - * past the read-side `holder = ANY(allowlist)` privacy filter. + * NUL + SURROGATE POLICY (codex P0 hardening; #2011): Postgres `jsonb` rejects + * both the Unicode NUL escape AND an unpaired UTF-16 surrogate half, and Postgres + * `text` cannot store a NUL either, so the OLD `unnest(::text[])` path rejected + * (errored) any row carrying either. We deliberately PRESERVE that reject + * semantics for IDENTITY and security-relevant fields: slugs, source_ids, + * `holder`, `kind`, dates, and the enum-ish `link_type` / `link_source` / + * `origin_slug` / `origin_field`. Those are left UN-sanitized, so junk in them + * still errors the batch and can never silently retarget a row to a different + * page/source or normalize a `holder` past the read-side `holder = ANY(allowlist)` + * privacy filter. * - * `stripNul` is applied ONLY to genuinely free-prose body fields where a junk - * NUL plausibly arrives from calendar/meeting/LLM content and where dropping the - * whole batch would be the worse outcome: `context` (links), `summary` + `detail` - * (timeline), `claim` (takes). NUL is the ONLY character ever stripped; commas, - * quotes, braces, and em-dashes are exactly what JSONB encodes correctly, and - * stripping them would corrupt user data. + * `sanitizeForJsonb` (strip NUL + well-form lone surrogates) is applied ONLY to + * genuinely free-prose body fields where junk plausibly arrives from + * calendar/meeting/LLM/OCR content and where dropping the whole batch would be + * the worse outcome: `context` (links), `summary` + `detail` + `source` + * (timeline), `claim` + `source` (takes). A lone surrogate is normalized to + * U+FFFD; NUL is removed. Timeline `source` is free-text here (it round-trips + * arbitrary provenance labels and is treated as free text by both engines), so it + * is sanitized too even though it participates in `idx_timeline_dedup` — replacing + * a malformed half-char before dedup is correct, not lossy (#2011). Takes `source` + * is likewise free-text LLM-extracted provenance (not an identity/dedup field — + * the takes key is `(page_id, row_num)`), so it is sanitized as well. Commas, quotes, + * braces, and em-dashes are exactly what JSONB encodes correctly and are never + * touched; stripping them would corrupt user data. * * DEFAULTING NOTE: the builders reproduce each method's exact pre-#1861 * defaulting. `|| ''` / `|| 'markdown'` / `|| 'default'` collapse empty strings; @@ -54,15 +62,31 @@ import type { LinkBatchInput, TimelineBatchInput, TakeBatchInput } from './engine.ts'; import { normalizeWeightForStorage } from './takes-fence.ts'; +import { ensureWellFormed } from './text-safe.ts'; /** * Strip Unicode NUL (U+0000) from a free-text body field. Fast-path the common * case (no NUL) so the regex replace only runs when a NUL is actually present. * Only call this on free-prose columns, never on identity/security fields (see * the NUL POLICY note above). + * + * This is the NUL-only primitive. For anything serialized into a `::jsonb` cast, + * prefer `sanitizeForJsonb` below — NUL alone does not protect against the lone + * surrogate that aborts a jsonb batch (#2011). Exported only as the building + * block for `sanitizeForJsonb`; don't reach for it directly on a jsonb path. */ export const stripNul = (s: string): string => (s.includes('\0') ? s.replace(/\0/g, '') : s); +/** + * Sanitize a free-text body field for JSONB serialization: strip NUL (#1861) + * AND well-form unpaired UTF-16 surrogates (#2011). A lone surrogate half (left + * behind when a slicer like `excerpt()` cuts a window boundary through an emoji) + * is rejected by Postgres inside a `::jsonb` cast and aborts the whole batch; + * `ensureWellFormed` replaces it with U+FFFD. Like `stripNul`, call this ONLY on + * free-prose body columns, NEVER on identity/security fields (see NUL POLICY). + */ +export const sanitizeForJsonb = (s: string): string => ensureWellFormed(stripNul(s)); + /** One links row, keys === the jsonb_to_recordset column list. */ export interface LinkRow { from_slug: string; @@ -109,7 +133,7 @@ export function buildLinkRows(links: LinkBatchInput[]): LinkRow[] { from_slug: l.from_slug, to_slug: l.to_slug, link_type: l.link_type || '', - context: stripNul(l.context || ''), // free-text body: NUL-stripped + context: sanitizeForJsonb(l.context || ''), // free-text body: NUL + lone-surrogate sanitized link_source: l.link_source || 'markdown', origin_slug: l.origin_slug || null, origin_field: l.origin_field || null, @@ -124,9 +148,9 @@ export function buildTimelineRows(entries: TimelineBatchInput[]): TimelineRow[] return entries.map(e => ({ slug: e.slug, date: e.date, - source: e.source || '', - summary: stripNul(e.summary), // free-text body: NUL-stripped - detail: stripNul(e.detail || ''), // free-text body: NUL-stripped + source: sanitizeForJsonb(e.source || ''), // free-text body: NUL + lone-surrogate sanitized (#2011) + summary: sanitizeForJsonb(e.summary), // free-text body: NUL + lone-surrogate sanitized + detail: sanitizeForJsonb(e.detail || ''), // free-text body: NUL + lone-surrogate sanitized source_id: e.source_id || 'default', })); } @@ -144,13 +168,15 @@ export function buildTakeRows(rowsIn: TakeBatchInput[]): { rows: TakeRow[]; weig return { page_id: r.page_id, row_num: r.row_num, - claim: stripNul(r.claim), // free-text body: NUL-stripped + claim: sanitizeForJsonb(r.claim), // free-text body: NUL + lone-surrogate sanitized kind: r.kind, holder: r.holder, weight, since_date: r.since_date ?? null, until_date: r.until_date ?? null, - source: r.source ?? null, + // free-text provenance → NUL + lone-surrogate sanitized (#2011); same + // jsonb crash class as claim. null/undefined stay null; '' stays ''. + source: r.source == null ? null : sanitizeForJsonb(r.source), superseded_by: r.superseded_by ?? null, active: r.active ?? true, }; diff --git a/src/core/brainstorm/orchestrator.ts b/src/core/brainstorm/orchestrator.ts index 9d3b85f1d..3ac7caef2 100644 --- a/src/core/brainstorm/orchestrator.ts +++ b/src/core/brainstorm/orchestrator.ts @@ -47,6 +47,7 @@ import { type ChatFn, } from './judges.ts'; import { canonicalLookup } from '../model-pricing.ts'; +import { ensureWellFormed } from '../text-safe.ts'; // --------------------------------------------------------------------------- // BudgetExhausted is the canonical typed error (Q2) used by every cost @@ -359,21 +360,6 @@ export async function loadCalibrationContext( // Idea generation prompts + response parsing // --------------------------------------------------------------------------- -/** - * Strip lone/orphaned UTF-16 surrogates that would crash JSON encoding - * downstream. The Anthropic SDK and some gateway transports refuse strings - * containing unpaired surrogates (U+D800–U+DFFF). Page content that came - * in via OCR or older imports occasionally has them. - */ -function sanitizeUnicode(s: string): string { - if (!s) return s; - // Replace lone high surrogates (D800-DBFF) not followed by a low surrogate. - // Replace lone low surrogates (DC00-DFFF) not preceded by a high surrogate. - return s - .replace(/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/g, '�') - .replace(/(^|[^\uD800-\uDBFF])[\uDC00-\uDFFF]/g, '$1�'); -} - /** Build a single (close × far) cross-generation prompt. */ function buildCrossPrompt(opts: { profile: BrainstormProfile; @@ -391,14 +377,16 @@ Style rules: - Cite BOTH the close and far slug verbatim — these are the user's own notes. - Never fabricate facts, figures, or quotes. Stay grounded in the cited pages.${opts.profile.generator_constraint ? `\n- ${opts.profile.generator_constraint}` : ''}`; - // Sanitize: unicode surrogates in page content (from OCR or older imports) - // can crash JSON encoding in the chat transport, which would void the - // entire cross. Cheap to fix here. - const closeContent = sanitizeUnicode(opts.close.content); - const farContent = sanitizeUnicode(opts.far.content); - const closeTitle = sanitizeUnicode(opts.close.title ?? '(untitled)'); - const farTitle = sanitizeUnicode(opts.far.title ?? '(untitled)'); - const question = sanitizeUnicode(opts.question); + // Sanitize: unpaired UTF-16 surrogates in page content (from OCR or older + // imports) can crash JSON encoding in the chat transport, which would void + // the entire cross. `ensureWellFormed` (shared with the #2011 jsonb path) is + // the canonical surrogate cleaner — it handles consecutive lone surrogates + // that the prior hand-rolled regex left malformed. + const closeContent = ensureWellFormed(opts.close.content); + const farContent = ensureWellFormed(opts.far.content); + const closeTitle = ensureWellFormed(opts.close.title ?? '(untitled)'); + const farTitle = ensureWellFormed(opts.far.title ?? '(untitled)'); + const question = ensureWellFormed(opts.question); const user = `QUESTION: ${question} diff --git a/src/core/link-extraction.ts b/src/core/link-extraction.ts index caa7c4166..8ebbac318 100644 --- a/src/core/link-extraction.ts +++ b/src/core/link-extraction.ts @@ -13,6 +13,7 @@ import type { BrainEngine } from './engine.ts'; import type { PageType } from './types.ts'; +import { ensureWellFormed } from './text-safe.ts'; /** * v0.42.7 — link-extraction version stamp. Bump this ISO timestamp whenever the @@ -575,12 +576,22 @@ export async function extractPageLinks( return { candidates: result, unresolved: fmUnresolved }; } -/** Excerpt a window of `width` chars around `idx`, collapsed to one line. */ +/** + * Excerpt a window of `width` chars around `idx`, collapsed to one line. + * + * The window is sliced by raw UTF-16 index, so a boundary can land inside a + * surrogate pair (any non-BMP char — emoji, math alphanumerics, non-BMP CJK) + * and leave a lone surrogate half. That lone half flows into the link `context` + * field and, when the batch is serialized for the `jsonb_to_recordset` insert, + * makes Postgres reject the WHOLE batch on the `::jsonb` cast — aborting the + * entire `extract --stale` run (#2011). `ensureWellFormed` replaces any orphaned + * half with U+FFFD before the slice escapes this function. + */ function excerpt(s: string, idx: number, width: number): string { const half = Math.floor(width / 2); const start = Math.max(0, idx - half); const end = Math.min(s.length, idx + half); - return s.slice(start, end).replace(/\s+/g, ' ').trim(); + return ensureWellFormed(s.slice(start, end)).replace(/\s+/g, ' ').trim(); } // ─── Relationship type inference (deterministic, zero LLM) ────── diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index 0a94630e6..65c429f36 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -46,7 +46,7 @@ import { validateSlug, contentHash, rowToPage, rowToStalePage, rowToChunk, rowTo import { deriveResolutionTuple, finalizeScorecard } from './takes-resolution.ts'; import { normalizeWeightForStorage } from './takes-fence.ts'; import { executeRawJsonb } from './sql-query.ts'; -import { stripNul, buildLinkRows, buildTimelineRows, buildTakeRows } from './batch-rows.ts'; +import { sanitizeForJsonb, buildLinkRows, buildTimelineRows, buildTakeRows } from './batch-rows.ts'; import { GBrainError, PAGE_SORT_SQL, ENRICH_ORDER_SQL } from './types.ts'; import { computeAnomaliesFromBuckets } from './cycle/anomaly.ts'; import { resolveBoostMap, resolveHardExcludes } from './search/source-boost.ts'; @@ -2434,7 +2434,7 @@ export class PGLiteEngine implements BrainEngine { ON CONFLICT (from_page_id, to_page_id, link_type, link_source, origin_page_id) DO UPDATE SET context = EXCLUDED.context, origin_field = EXCLUDED.origin_field`, - [from, to, linkType || '', stripNul(context || ''), src, originSlug ?? null, originField ?? null, fromSrc, toSrc, originSrc] + [from, to, linkType || '', sanitizeForJsonb(context || ''), src, originSlug ?? null, originField ?? null, fromSrc, toSrc, originSrc] ); } @@ -3197,12 +3197,14 @@ export class PGLiteEngine implements BrainEngine { // ON CONFLICT DO NOTHING via the (page_id, date, summary) unique index. // Source-qualify the page-id lookup so multi-source brains don't fan // timeline rows out across every source containing the slug. + // Free-text body fields are NUL + lone-surrogate sanitized (#2011), matching + // the batch path and the Postgres engine; identity fields (slug, date) raw. await this.db.query( `INSERT INTO timeline_entries (page_id, date, source, summary, detail) SELECT id, $2::date, $3, $4, $5 FROM pages WHERE slug = $1 AND source_id = $6 ON CONFLICT (page_id, date, summary, source) DO NOTHING`, - [slug, entry.date, entry.source || '', entry.summary, entry.detail || '', sourceId] + [slug, entry.date, sanitizeForJsonb(entry.source || ''), sanitizeForJsonb(entry.summary), sanitizeForJsonb(entry.detail || ''), sourceId] ); } diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index cb7bcb14a..3b8c7ce13 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -22,7 +22,7 @@ import { MAX_SEARCH_LIMIT, clampSearchLimit } from './engine.ts'; import { deriveResolutionTuple, finalizeScorecard } from './takes-resolution.ts'; import { normalizeWeightForStorage } from './takes-fence.ts'; import { executeRawJsonb } from './sql-query.ts'; -import { stripNul, buildLinkRows, buildTimelineRows, buildTakeRows } from './batch-rows.ts'; +import { sanitizeForJsonb, buildLinkRows, buildTimelineRows, buildTakeRows } from './batch-rows.ts'; import { runMigrations } from './migrate.ts'; import { SCHEMA_SQL } from './schema-embedded.ts'; import { verifySchema } from './schema-verify.ts'; @@ -2521,7 +2521,7 @@ export class PostgresEngine implements BrainEngine { await sql` INSERT INTO links (from_page_id, to_page_id, link_type, context, link_source, origin_page_id, origin_field) SELECT f.id, t.id, v.link_type, v.context, v.link_source, o.id, v.origin_field - FROM (VALUES (${from}, ${to}, ${linkType || ''}, ${stripNul(context || '')}, ${src}, ${originSlug ?? null}, ${originField ?? null}, ${fromSrc}, ${toSrc}, ${originSrc})) + FROM (VALUES (${from}, ${to}, ${linkType || ''}, ${sanitizeForJsonb(context || '')}, ${src}, ${originSlug ?? null}, ${originField ?? null}, ${fromSrc}, ${toSrc}, ${originSrc})) AS v(from_slug, to_slug, link_type, context, link_source, origin_slug, origin_field, from_source_id, to_source_id, origin_source_id) JOIN pages f ON f.slug = v.from_slug AND f.source_id = v.from_source_id JOIN pages t ON t.slug = v.to_slug AND t.source_id = v.to_source_id @@ -3297,9 +3297,12 @@ export class PostgresEngine implements BrainEngine { // makes that ambiguity safe (caller asserts page exists). Source-qualify // the page-id lookup so multi-source brains don't fan timeline rows out // across every source containing the slug. + // Free-text body fields are NUL + lone-surrogate sanitized (#2011) so a + // surrogate from sliced/imported content can't reach the (later) ::jsonb + // batch path or corrupt the row; identity fields (slug, date) are left raw. await sql` INSERT INTO timeline_entries (page_id, date, source, summary, detail) - SELECT id, ${entry.date}::date, ${entry.source || ''}, ${entry.summary}, ${entry.detail || ''} + SELECT id, ${entry.date}::date, ${sanitizeForJsonb(entry.source || '')}, ${sanitizeForJsonb(entry.summary)}, ${sanitizeForJsonb(entry.detail || '')} FROM pages WHERE slug = ${slug} AND source_id = ${sourceId} ON CONFLICT (page_id, date, summary, source) DO NOTHING `; diff --git a/src/core/text-safe.ts b/src/core/text-safe.ts index faccfa7be..982bd3de3 100644 --- a/src/core/text-safe.ts +++ b/src/core/text-safe.ts @@ -24,6 +24,25 @@ * low surrogate: U+DC00..U+DFFF */ +/** + * Make a string well-formed UTF-16: replace any unpaired surrogate half (lone + * high U+D800–U+DBFF / lone low U+DC00–U+DFFF) with U+FFFD, preserving valid + * pairs untouched. A lone surrogate is rejected by Postgres inside a `::jsonb` + * cast and aborts the WHOLE batch (#2011 — `extract --stale` died at ~1,550 + * pages because `excerpt()` raw-sliced a window boundary through an emoji); it + * also crashes some JSON transports (the brainstorm cross-prompt path). + * + * Uses the Bun/JSC built-in (ES2024). `isWellFormed()` is the cheap guard that + * avoids the `toWellFormed()` allocation when the string is already clean — the + * common case. Prefer this over hand-rolled surrogate regexes: the two-pass + * lookbehind regex mishandles CONSECUTIVE lone low surrogates + * (`"\uDE80\uDE80"` → `"�\uDE80"`, still malformed), whereas the built-in + * handles every case (`→ "��"`). + */ +export function ensureWellFormed(s: string): string { + return s.isWellFormed() ? s : s.toWellFormed(); +} + function isHighSurrogate(code: number): boolean { return code >= 0xd800 && code <= 0xdbff; } diff --git a/test/doctor-retrieval-reflex.test.ts b/test/doctor-retrieval-reflex.test.ts index d47955a5c..6718dea76 100644 --- a/test/doctor-retrieval-reflex.test.ts +++ b/test/doctor-retrieval-reflex.test.ts @@ -1,46 +1,44 @@ /** * Doctor retrieval_reflex_health check (#1981, T8). */ -import { describe, test, expect, afterEach } from 'bun:test'; +import { describe, test, expect } from 'bun:test'; import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { buildRetrievalReflexCheck } from '../src/commands/doctor.ts'; - -const origEnv = process.env.GBRAIN_RETRIEVAL_REFLEX; -afterEach(() => { - if (origEnv === undefined) delete process.env.GBRAIN_RETRIEVAL_REFLEX; - else process.env.GBRAIN_RETRIEVAL_REFLEX = origEnv; -}); +import { withEnv } from './helpers/with-env.ts'; describe('buildRetrievalReflexCheck', () => { - test('disabled via env → warn, names the right check', () => { - process.env.GBRAIN_RETRIEVAL_REFLEX = 'false'; - const c = buildRetrievalReflexCheck(null); - expect(c.name).toBe('retrieval_reflex_health'); - expect(c.status).toBe('warn'); - expect(c.message).toContain('disabled'); - expect((c.details as any)?.enabled).toBe(false); + test('disabled via env → warn, names the right check', async () => { + await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'false' }, async () => { + const c = buildRetrievalReflexCheck(null); + expect(c.name).toBe('retrieval_reflex_health'); + expect(c.status).toBe('warn'); + expect(c.message).toContain('disabled'); + expect((c.details as any)?.enabled).toBe(false); + }); }); - test('enabled → reports policy-skill install state in details', () => { - process.env.GBRAIN_RETRIEVAL_REFLEX = 'true'; - const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-')); - mkdirSync(join(dir, 'retrieval-reflex'), { recursive: true }); - writeFileSync(join(dir, 'retrieval-reflex', 'SKILL.md'), '# stub\n'); - const c = buildRetrievalReflexCheck(dir); - expect(c.name).toBe('retrieval_reflex_health'); - expect((c.details as any)?.enabled).toBe(true); - expect((c.details as any)?.policy_skill_installed).toBe(true); - rmSync(dir, { recursive: true, force: true }); + test('enabled → reports policy-skill install state in details', async () => { + await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'true' }, async () => { + const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-')); + mkdirSync(join(dir, 'retrieval-reflex'), { recursive: true }); + writeFileSync(join(dir, 'retrieval-reflex', 'SKILL.md'), '# stub\n'); + const c = buildRetrievalReflexCheck(dir); + expect(c.name).toBe('retrieval_reflex_health'); + expect((c.details as any)?.enabled).toBe(true); + expect((c.details as any)?.policy_skill_installed).toBe(true); + rmSync(dir, { recursive: true, force: true }); + }); }); - test('enabled, policy skill absent → message includes the install hint', () => { - process.env.GBRAIN_RETRIEVAL_REFLEX = 'true'; - const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-2-')); - const c = buildRetrievalReflexCheck(dir); - expect((c.details as any)?.policy_skill_installed).toBe(false); - expect(c.message).toContain('gbrain integrations install retrieval-reflex'); - rmSync(dir, { recursive: true, force: true }); + test('enabled, policy skill absent → message includes the install hint', async () => { + await withEnv({ GBRAIN_RETRIEVAL_REFLEX: 'true' }, async () => { + const dir = mkdtempSync(join(tmpdir(), 'rr-doctor-2-')); + const c = buildRetrievalReflexCheck(dir); + expect((c.details as any)?.policy_skill_installed).toBe(false); + expect(c.message).toContain('gbrain integrations install retrieval-reflex'); + rmSync(dir, { recursive: true, force: true }); + }); }); }); diff --git a/test/e2e/jsonb-batch-poison-postgres.test.ts b/test/e2e/jsonb-batch-poison-postgres.test.ts index ca49f9fcd..34608b1f1 100644 --- a/test/e2e/jsonb-batch-poison-postgres.test.ts +++ b/test/e2e/jsonb-batch-poison-postgres.test.ts @@ -20,6 +20,15 @@ const POISON = '"Q2 sync" — notes {a,b}, path C:\\Users\\x, em–dash } and { trailing'; const NUL = String.fromCharCode(0); +// #2011 — lone UTF-16 high surrogate. THIS is the value that crashed Postgres at +// the ::jsonb cast (22P02) and aborted `extract --stale` on a managed Supabase +// brain. The PGLite sibling rejects it too on current builds, but this lane is +// the production engine the bug was reported against. Built via fromCharCode so +// no lone surrogate is a literal in this file. +const LONE_HI = String.fromCharCode(0xd83c); +const SURROGATE = `before${LONE_HI}after`; +const SURROGATE_CLEAN = 'before�after'; + let engine: PostgresEngine; async function seed(slug: string) { @@ -100,4 +109,78 @@ d('JSONB batch poison — Postgres (#1861)', () => { expect(rows[0].active).toBe(false); expect(rows[0].since_date).toBeNull(); }); + + // ── #2011: lone-surrogate ::jsonb crash lock (the abort this PR fixes) ── + + it('addLinksBatch survives a lone surrogate in context (the #2011 crash)', async () => { + await seed('from-page'); await seed('to-page'); + const n = await engine.addLinksBatch([ + { from_slug: 'from-page', to_slug: 'to-page', link_type: 'mentions', + context: SURROGATE, link_source: 'manual' }, + ]); + expect(n).toBe(1); // pre-fix: threw "invalid input syntax for type json" + const links = await engine.getLinks('from-page', { sourceId: 'default' }); + expect(links[0].context).toBe(SURROGATE_CLEAN); + expect(links[0].context.isWellFormed()).toBe(true); + }); + + it('addTimelineEntriesBatch survives a lone surrogate in source (D2)', async () => { + await seed('meeting-page'); + const n = await engine.addTimelineEntriesBatch([ + { slug: 'meeting-page', date: '2026-06-04', source: SURROGATE, + summary: SURROGATE, detail: SURROGATE }, + ]); + expect(n).toBe(1); + const rows = await engine.executeRaw<{ summary: string; source: string }>( + `SELECT te.summary, te.source FROM timeline_entries te + JOIN pages p ON p.id = te.page_id WHERE p.slug = 'meeting-page'`, + ); + expect(rows[0].summary).toBe(SURROGATE_CLEAN); + expect(rows[0].source).toBe(SURROGATE_CLEAN); + }); + + it('addTakesBatch survives a lone surrogate in claim and source', async () => { + await seed('take-page'); + const pid = await pageId('take-page'); + const n = await engine.addTakesBatch([ + { page_id: pid, row_num: 1, claim: SURROGATE, kind: 'fact', holder: 'h', source: SURROGATE }, + ]); + expect(n).toBe(1); + const rows = await engine.executeRaw<{ claim: string; source: string }>( + `SELECT claim, source FROM takes t JOIN pages p ON p.id = t.page_id + WHERE p.slug = 'take-page' AND t.row_num = 1`, + ); + expect(rows[0].claim).toBe(SURROGATE_CLEAN); + expect(rows[0].source).toBe(SURROGATE_CLEAN); // free-text provenance, sanitized too + }); + + it('scalar addLink + addTimelineEntry survive a lone surrogate', async () => { + await seed('sa'); await seed('sb'); + await engine.addLink('sa', 'sb', SURROGATE, 'mentions', 'manual'); + const links = await engine.getLinks('sa', { sourceId: 'default' }); + expect(links[0].context).toBe(SURROGATE_CLEAN); + + await engine.addTimelineEntry('sa', { + date: '2026-06-05', source: SURROGATE, summary: SURROGATE, detail: SURROGATE, + }); + const rows = await engine.executeRaw<{ source: string }>( + `SELECT te.source FROM timeline_entries te JOIN pages p ON p.id = te.page_id + WHERE p.slug = 'sa'`, + ); + expect(rows[0].source).toBe(SURROGATE_CLEAN); + }); + + it('fail-closed: a lone surrogate in an IDENTITY field still rejects the batch', async () => { + // The NUL/surrogate policy deliberately leaves identity fields raw so a + // malformed slug/holder can never silently retarget a row. On Postgres the + // raw lone surrogate hits the ::jsonb cast and rejects the whole batch — + // loud failure, not silent normalization. This locks that we did NOT extend + // sanitization to identity fields. + await seed('idfrom'); await seed('idto'); + await expect( + engine.addLinksBatch([ + { from_slug: `idfrom${LONE_HI}`, to_slug: 'idto', link_type: 'mentions', link_source: 'manual' }, + ]), + ).rejects.toThrow(); + }); }); diff --git a/test/link-extraction.test.ts b/test/link-extraction.test.ts index 56c4fa662..0c180f232 100644 --- a/test/link-extraction.test.ts +++ b/test/link-extraction.test.ts @@ -228,6 +228,32 @@ describe('extractPageLinks', () => { expect(aliceLink!.linkType).toBe('works_at'); }); + test('#2011: excerpt window slicing a non-BMP char yields well-formed context', async () => { + // Reproduce the abort trigger: a markdown ref whose 240-char context window + // boundary lands inside an emoji's surrogate pair. Pre-fix, the slice kept a + // lone high surrogate in `context`, which Postgres rejected at the ::jsonb + // cast and aborted the whole `extract --stale` run. + const ROCKET = '🚀'; // U+1F680 = [0xD83D, 0xDE80] + const head = '[Alice](people/alice)'; + const idx = head.indexOf('Alice'); // excerpt centers on ref.name + const half = 120; // width 240 / 2 + // Place the emoji so its HIGH half sits at index (idx+half-1) and its LOW + // half at (idx+half) — exactly the excerpt `end` boundary, splitting it. + const padLen = idx + half - 1 - head.length; + const content = head + 'x'.repeat(padLen) + ROCKET + ' trailing context'; + + // Sanity: confirm the fixture actually splits a pair (the raw window is + // malformed). If this ever stops being malformed, the regression is moot. + const rawWindow = content.slice(Math.max(0, idx - half), idx + half); + expect(rawWindow.isWellFormed()).toBe(false); + + const { candidates } = await extractPageLinks('docs/x', content, {}, 'concept', allowAllResolver); + const alice = candidates.find(c => c.targetSlug === 'people/alice'); + expect(alice).toBeDefined(); + expect(alice!.context.isWellFormed()).toBe(true); + expect(JSON.parse(JSON.stringify(alice!.context))).toBe(alice!.context); + }); + test('dedups multiple mentions of same entity (within-page dedup)', async () => { const content = '[Alice](people/alice) said this. Later, [Alice](people/alice) said that.'; const { candidates } = await extractPageLinks('docs/x', content, {}, 'concept', allowAllResolver); diff --git a/test/links-timeline-jsonb-poison.test.ts b/test/links-timeline-jsonb-poison.test.ts index 4d6a7cd97..9fa004f4b 100644 --- a/test/links-timeline-jsonb-poison.test.ts +++ b/test/links-timeline-jsonb-poison.test.ts @@ -23,6 +23,17 @@ const POISON = // fromCharCode so no literal NUL byte ever lands in this source file. const NUL = String.fromCharCode(0); +// #2011 — a lone UTF-16 high surrogate (no low partner). This is what excerpt() +// left behind when a context window boundary sliced an emoji; Postgres rejects +// it inside the ::jsonb cast and aborts the whole batch. The row builders now +// well-form it to U+FFFD. Built via fromCharCode so no lone surrogate is ever a +// literal in this source file. PGLite serializes jsonb differently and may not +// reproduce the original Postgres crash, but it still locks the JS-side +// sanitization (buildLinkRows/...): the stored value must be well-formed. +const LONE_HI = String.fromCharCode(0xd83c); +const SURROGATE = `before${LONE_HI}after`; +const SURROGATE_CLEAN = 'before�after'; + let engine: PGLiteEngine; beforeAll(async () => { @@ -244,3 +255,98 @@ describe('scalar addLink — NUL strip on free-text context (#1861 codex #3)', ( expect(links[0].context).toBe('beforeafter'); }); }); + +describe('#2011 — lone-surrogate well-forming across all prose write paths', () => { + it('batch link context: lone surrogate well-formed, insert succeeds', async () => { + await seed('a'); await seed('b'); + const n = await engine.addLinksBatch([ + { from_slug: 'a', to_slug: 'b', link_type: 'mentions', context: SURROGATE, link_source: 'manual' }, + ]); + expect(n).toBe(1); + const links = await engine.getLinks('a', { sourceId: 'default' }); + expect(links[0].context).toBe(SURROGATE_CLEAN); + expect(links[0].context.isWellFormed()).toBe(true); + }); + + it('batch timeline summary/detail/source: all lone surrogates well-formed', async () => { + await seed('m'); + const n = await engine.addTimelineEntriesBatch([ + { slug: 'm', date: '2026-06-04', source: SURROGATE, summary: SURROGATE, detail: SURROGATE }, + ]); + expect(n).toBe(1); + const rows = await engine.executeRaw<{ summary: string; detail: string; source: string }>( + `SELECT te.summary, te.detail, te.source FROM timeline_entries te + JOIN pages p ON p.id = te.page_id WHERE p.slug = 'm'`, + ); + expect(rows[0].summary).toBe(SURROGATE_CLEAN); + expect(rows[0].detail).toBe(SURROGATE_CLEAN); + expect(rows[0].source).toBe(SURROGATE_CLEAN); // D2: timeline source now sanitized + expect(rows[0].source.isWellFormed()).toBe(true); + }); + + it('batch take claim: lone surrogate well-formed', async () => { + await seed('tk'); + const pid = await pageId('tk'); + await engine.addTakesBatch([ + { page_id: pid, row_num: 1, claim: SURROGATE, kind: 'fact', holder: 'h' }, + ]); + const rows = await engine.executeRaw<{ claim: string }>( + `SELECT claim FROM takes t JOIN pages p ON p.id = t.page_id + WHERE p.slug = 'tk' AND t.row_num = 1`, + ); + expect(rows[0].claim).toBe(SURROGATE_CLEAN); + expect(rows[0].claim.isWellFormed()).toBe(true); + }); + + it('batch take source (free-text provenance): lone surrogate well-formed', async () => { + await seed('tks'); + const pid = await pageId('tks'); + await engine.addTakesBatch([ + { page_id: pid, row_num: 1, claim: 'c', kind: 'fact', holder: 'h', source: SURROGATE }, + ]); + const rows = await engine.executeRaw<{ source: string }>( + `SELECT source FROM takes t JOIN pages p ON p.id = t.page_id + WHERE p.slug = 'tks' AND t.row_num = 1`, + ); + expect(rows[0].source).toBe(SURROGATE_CLEAN); + expect(rows[0].source.isWellFormed()).toBe(true); + }); + + it('scalar addLink: lone surrogate in context well-formed', async () => { + await seed('sa'); await seed('sb'); + await engine.addLink('sa', 'sb', SURROGATE, 'mentions', 'manual'); + const links = await engine.getLinks('sa', { sourceId: 'default' }); + expect(links[0].context).toBe(SURROGATE_CLEAN); + expect(links[0].context.isWellFormed()).toBe(true); + }); + + it('scalar addTimelineEntry: lone surrogate in summary/detail/source well-formed', async () => { + await seed('sm'); + await engine.addTimelineEntry('sm', { + date: '2026-06-05', source: SURROGATE, summary: SURROGATE, detail: SURROGATE, + }); + const rows = await engine.executeRaw<{ summary: string; detail: string; source: string }>( + `SELECT te.summary, te.detail, te.source FROM timeline_entries te + JOIN pages p ON p.id = te.page_id WHERE p.slug = 'sm'`, + ); + expect(rows[0].summary).toBe(SURROGATE_CLEAN); + expect(rows[0].detail).toBe(SURROGATE_CLEAN); + expect(rows[0].source).toBe(SURROGATE_CLEAN); + }); + + it('fail-closed: identity fields are NOT sanitized — a lone surrogate in to_slug rejects the batch', async () => { + // The NUL/surrogate policy deliberately leaves identity fields raw so a + // malformed slug can never be silently normalized into one that matches a + // different page. This PGLite build rejects the lone surrogate at the + // ::jsonb cast (22P02) — a loud failure, exactly the intended boundary + // (and proof we did NOT extend sanitizeForJsonb to identity columns). + await seed('idfrom'); await seed('idto'); + await expect( + engine.addLinksBatch([ + { from_slug: 'idfrom', to_slug: `idto${LONE_HI}`, link_type: 'mentions', link_source: 'manual' }, + ]), + ).rejects.toThrow(); + const links = await engine.getLinks('idfrom', { sourceId: 'default' }); + expect(links).toHaveLength(0); + }); +}); diff --git a/test/retrieval-reflex.test.ts b/test/retrieval-reflex.test.ts index 9cf097708..385af152b 100644 --- a/test/retrieval-reflex.test.ts +++ b/test/retrieval-reflex.test.ts @@ -9,6 +9,7 @@ */ import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { withEnv } from './helpers/with-env.ts'; import { normalizeAlias } from '../src/core/search/alias-normalize.ts'; import { resolveEntitiesToPointers } from '../src/core/context/retrieval-reflex.ts'; import { extractCandidates } from '../src/core/context/entity-salience.ts'; @@ -108,68 +109,76 @@ describe('resolveEntitiesToPointers', () => { }); describe('context-engine assemble() — Retrieval Reflex integration', () => { - beforeEach(() => { - process.env.GBRAIN_RETRIEVAL_REFLEX = 'true'; - }); + // Each test wraps its body in withEnv (NOT a beforeEach env mutation) so the + // flag is restored even on throw — required by check-test-isolation rule R1. + const REFLEX_ON = { GBRAIN_RETRIEVAL_REFLEX: 'true' }; test('regression: a named entity with a page surfaces a pointer (host resolver path)', async () => { - await seed('people/alice-example', 'Alice Example', 'Alice is a founder.'); - // Inject a resolver the way the OpenClaw host (ctx.brainQuery) or serve IPC would. - const ce = createGBrainContextEngine({ - workspaceDir: '/tmp/rr-test-ws', - resolveEntities: (candidates, opts) => - resolveEntitiesToPointers(engine, 'default', candidates, opts), + await withEnv(REFLEX_ON, async () => { + await seed('people/alice-example', 'Alice Example', 'Alice is a founder.'); + // Inject a resolver the way the OpenClaw host (ctx.brainQuery) or serve IPC would. + const ce = createGBrainContextEngine({ + workspaceDir: '/tmp/rr-test-ws', + resolveEntities: (candidates, opts) => + resolveEntitiesToPointers(engine, 'default', candidates, opts), + }); + const res = await ce.assemble({ + sessionId: 's1', + messages: [{ role: 'user', content: 'what do you think about Alice Example?' }], + }); + expect(res.systemPromptAddition).toContain('Brain pages mentioned this turn'); + expect(res.systemPromptAddition).toContain('people/alice-example'); + expect(res.systemPromptAddition).toContain('use get_page'); }); - const res = await ce.assemble({ - sessionId: 's1', - messages: [{ role: 'user', content: 'what do you think about Alice Example?' }], - }); - expect(res.systemPromptAddition).toContain('Brain pages mentioned this turn'); - expect(res.systemPromptAddition).toContain('people/alice-example'); - expect(res.systemPromptAddition).toContain('use get_page'); }); test('no resolver available (PGLite, no serve/host) → no throw, live context still present', async () => { - const ce = createGBrainContextEngine({ workspaceDir: '/tmp/rr-test-ws-2' }); - const res = await ce.assemble({ - sessionId: 's2', - messages: [{ role: 'user', content: 'what about Alice Example?' }], + await withEnv(REFLEX_ON, async () => { + const ce = createGBrainContextEngine({ workspaceDir: '/tmp/rr-test-ws-2' }); + const res = await ce.assemble({ + sessionId: 's2', + messages: [{ role: 'user', content: 'what about Alice Example?' }], + }); + // Live Context block always ships; no pointer block (nothing resolved). + expect(res.systemPromptAddition).toContain('Live Context'); + expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn'); }); - // Live Context block always ships; no pointer block (nothing resolved). - expect(res.systemPromptAddition).toContain('Live Context'); - expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn'); }); test('zero salient candidates → no brain touch, no pointer block', async () => { - let called = false; - const ce = createGBrainContextEngine({ - workspaceDir: '/tmp/rr-test-ws-3', - resolveEntities: async () => { called = true; return null; }, + await withEnv(REFLEX_ON, async () => { + let called = false; + const ce = createGBrainContextEngine({ + workspaceDir: '/tmp/rr-test-ws-3', + resolveEntities: async () => { called = true; return null; }, + }); + const res = await ce.assemble({ + sessionId: 's3', + messages: [{ role: 'user', content: 'can you help me with this?' }], + }); + expect(called).toBe(false); + expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn'); }); - const res = await ce.assemble({ - sessionId: 's3', - messages: [{ role: 'user', content: 'can you help me with this?' }], - }); - expect(called).toBe(false); - expect(res.systemPromptAddition).not.toContain('Brain pages mentioned this turn'); }); test('suppression uses PRIOR turns only, not the current message', async () => { - await seed('people/alice-example', 'Alice Example', 'A founder.'); - const ce = createGBrainContextEngine({ - workspaceDir: '/tmp/rr-test-ws-4', - resolveEntities: (candidates, opts) => - resolveEntitiesToPointers(engine, 'default', candidates, opts), + await withEnv(REFLEX_ON, async () => { + await seed('people/alice-example', 'Alice Example', 'A founder.'); + const ce = createGBrainContextEngine({ + workspaceDir: '/tmp/rr-test-ws-4', + resolveEntities: (candidates, opts) => + resolveEntitiesToPointers(engine, 'default', candidates, opts), + }); + // The current message names Alice Example; prior context does NOT. Must fire. + const res = await ce.assemble({ + sessionId: 's4', + messages: [ + { role: 'user', content: 'hello' }, + { role: 'assistant', content: 'hi there' }, + { role: 'user', content: 'what do you think about Alice Example?' }, + ], + }); + expect(res.systemPromptAddition).toContain('people/alice-example'); }); - // The current message names Alice Example; prior context does NOT. Must fire. - const res = await ce.assemble({ - sessionId: 's4', - messages: [ - { role: 'user', content: 'hello' }, - { role: 'assistant', content: 'hi there' }, - { role: 'user', content: 'what do you think about Alice Example?' }, - ], - }); - expect(res.systemPromptAddition).toContain('people/alice-example'); }); }); diff --git a/test/text-safe.test.ts b/test/text-safe.test.ts index 6f0313d9e..5fde97610 100644 --- a/test/text-safe.test.ts +++ b/test/text-safe.test.ts @@ -10,7 +10,7 @@ */ import { describe, test, expect } from 'bun:test'; -import { truncateUtf8, safeSplitIndex } from '../src/core/text-safe.ts'; +import { truncateUtf8, safeSplitIndex, ensureWellFormed } from '../src/core/text-safe.ts'; // 🚀 = U+1F680 (surrogate pair: 0xD83D 0xDE80; JS string length = 2). // 𝕏 = U+1D54F (surrogate pair: 0xD835 0xDD4F; JS string length = 2). @@ -19,6 +19,8 @@ const ROCKET = '🚀'; // 🚀 const MATH_X = '𝕏'; // 𝕏 const NBMP_HAN = '𠀀'; // 𠀀 const STRAY_HIGH = '\uD83D'; // orphaned high surrogate (invalid alone) +const STRAY_LOW = '\uDE80'; // orphaned low surrogate (invalid alone) +const REPLACEMENT = '�'; // U+FFFD, what toWellFormed() substitutes describe('truncateUtf8', () => { test('returns empty for empty input', () => { @@ -73,6 +75,61 @@ describe('truncateUtf8', () => { }); }); +describe('ensureWellFormed (#2011)', () => { + test('lone high surrogate → U+FFFD', () => { + const out = ensureWellFormed('before' + STRAY_HIGH + 'after'); + expect(out).toBe('before' + REPLACEMENT + 'after'); + expect(out.isWellFormed()).toBe(true); + }); + + test('lone low surrogate → U+FFFD', () => { + const out = ensureWellFormed('before' + STRAY_LOW + 'after'); + expect(out).toBe('before' + REPLACEMENT + 'after'); + expect(out.isWellFormed()).toBe(true); + }); + + test('consecutive lone low surrogates → both replaced (the case the old regex got wrong)', () => { + // The prior hand-rolled two-pass regex produced "�\uDE80" here (the second + // lookbehind consumed the just-inserted boundary, leaving the 2nd low + // surrogate orphaned). The built-in replaces both → "��". + const out = ensureWellFormed(STRAY_LOW + STRAY_LOW); + expect(out).toBe(REPLACEMENT + REPLACEMENT); + expect(out.isWellFormed()).toBe(true); + }); + + test('consecutive lone high surrogates → both replaced', () => { + const out = ensureWellFormed(STRAY_HIGH + STRAY_HIGH); + expect(out).toBe(REPLACEMENT + REPLACEMENT); + expect(out.isWellFormed()).toBe(true); + }); + + test('valid pairs (emoji / math / non-BMP CJK) preserved unchanged', () => { + const text = 'a' + ROCKET + 'b' + MATH_X + 'c' + NBMP_HAN + 'd'; + expect(ensureWellFormed(text)).toBe(text); + expect(ensureWellFormed(text).isWellFormed()).toBe(true); + }); + + test('clean ASCII / empty input returns an equal well-formed value', () => { + expect(ensureWellFormed('plain text')).toBe('plain text'); + expect(ensureWellFormed('')).toBe(''); + }); + + test('mixed valid pair + lone half: pair kept, orphan replaced', () => { + // Valid 🚀 then a stray high then text. + const out = ensureWellFormed(ROCKET + STRAY_HIGH + 'x'); + expect(out).toBe(ROCKET + REPLACEMENT + 'x'); + expect(out.isWellFormed()).toBe(true); + }); + + test('output is JSON-serializable without orphaned escapes', () => { + // The #2011 failure was Postgres ::jsonb rejecting a lone surrogate. + // After ensureWellFormed, a JSON round-trip preserves the value. + const out = ensureWellFormed('emoji ' + STRAY_HIGH + ' tail'); + expect(JSON.parse(JSON.stringify(out))).toBe(out); + expect(out.isWellFormed()).toBe(true); + }); +}); + describe('safeSplitIndex', () => { test('maxChars ≤ 0 returns 0', () => { expect(safeSplitIndex('any', 0)).toBe(0);