diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index a4c822a1a..f49bf68fd 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -528,6 +528,48 @@ export async function childTableOrphansCheck(engine: BrainEngine): Promise 'object'` is corrupted — federation and ACL settings + * on that source are read off a string instead of the settings object. Surface + * the affected sources with the repair path. The `gbrain sources` config writers + * now normalize before write, so any config-writing command self-heals the row + * (the app unwraps up to 10 nested layers); the SQL below repairs one layer + * directly for the common case. + */ +export async function checkSourceConfigShape(engine: BrainEngine): Promise { + try { + const rows = await engine.executeRaw<{ id: string; typ: string | null }>( + `SELECT id, jsonb_typeof(config) AS typ FROM sources WHERE jsonb_typeof(config) <> 'object'`, + ); + if (rows.length === 0) { + return { + name: 'source_config_shape', + status: 'ok', + message: 'All source config values are JSON objects', + }; + } + const affected = rows.map((r) => `${r.id} (${r.typ ?? 'null'})`).join(', '); + return { + name: 'source_config_shape', + status: 'warn', + message: + `${rows.length} source(s) have a non-object config — a JSON string/scalar ` + + `instead of an object (the #2829 re-wrapping bug): ${affected}. ` + + `Federation and ACL settings on these sources won't be read correctly. ` + + `Repair by running any 'gbrain sources' config write (self-heals up to 10 ` + + `nested layers), or in SQL: ` + + `UPDATE sources SET config = (config #>> '{}')::jsonb ` + + `WHERE jsonb_typeof(config) <> 'object';`, + }; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return { name: 'source_config_shape', status: 'warn', message: `Check failed: ${msg}` }; + } +} + export async function doctorReportRemote(engine: BrainEngine): Promise { const checks: Check[] = []; @@ -6201,6 +6243,12 @@ export async function buildChecks( progress.heartbeat('child_table_orphans'); checks.push(await childTableOrphansCheck(engine)); + // #2829: detect sources whose jsonb `config` was re-wrapped into a string + // scalar (grows a layer per read→write cycle). Non-object configs break + // federation + ACL reads; surface them with the repair path. + progress.heartbeat('source_config_shape'); + checks.push(await checkSourceConfigShape(engine)); + // v0.33: whoknows_health — fixture presence + row count. The eval // gate itself runs via `gbrain eval whoknows`; this check is the // "did you do the assignment?" signal. diff --git a/src/commands/sources.ts b/src/commands/sources.ts index cb855b3f6..02182ddd0 100644 --- a/src/commands/sources.ts +++ b/src/commands/sources.ts @@ -53,6 +53,7 @@ import { import { loadAllSources, parseSourceConfig, + normalizeSourceConfig, isSourceFederated, type SourceRow as LoadedSourceRow, } from '../core/sources-load.ts'; @@ -711,7 +712,7 @@ async function runFederate(engine: BrainEngine, args: string[], value: boolean): config.federated = value; await engine.executeRaw( `UPDATE sources SET config = $1::text::jsonb WHERE id = $2`, - [JSON.stringify(config), id], + [JSON.stringify(normalizeSourceConfig(config)), id], ); console.log(`Source "${id}" is now ${value ? 'federated (appears in cross-source default search)' : 'isolated (only searched when explicitly named)'}.`); @@ -898,7 +899,7 @@ async function runWebhookSet(engine: BrainEngine, args: string[]): Promise cfg.github_repo = githubRepo; await engine.executeRaw( `UPDATE sources SET config = $1::text::jsonb WHERE id = $2`, - [JSON.stringify(cfg), id], + [JSON.stringify(normalizeSourceConfig(cfg)), id], ); console.log(`Webhook configured for source "${id}":`); @@ -954,7 +955,7 @@ async function runWebhookRotate(engine: BrainEngine, args: string[]): Promise = new Set([ 'flagged_pages', 'salience_health', 'scraper_junk_pages', + 'source_config_shape', 'source_routing_health', 'stub_guard_24h', 'sync_failures', diff --git a/src/core/sources-load.ts b/src/core/sources-load.ts index a72219f03..92c10ffd2 100644 --- a/src/core/sources-load.ts +++ b/src/core/sources-load.ts @@ -45,15 +45,70 @@ export interface LoadAllSourcesOpts { federatedOnly?: boolean; } -/** Parse `sources.config` to a plain object regardless of driver shape. */ -export function parseSourceConfig(config: unknown): Record { - if (typeof config === 'string') { - try { return JSON.parse(config) as Record; } catch { return {}; } +/** + * #2829: max JSON.parse passes when unwrapping a possibly multiply-stringified + * `sources.config`. A re-wrapping bug could store config as a JSON *string + * scalar* ("{}", "\"{}\"", ...) that grows one layer per read→write cycle; the + * bound keeps a pathological value from spinning forever. + */ +const MAX_CONFIG_UNWRAP_DEPTH = 10; + +function isPlainObject(v: unknown): v is Record { + return typeof v === 'object' && v !== null && !Array.isArray(v); +} + +/** Unwrap a value that may be JSON-stringified 0..N times. Bounded; never throws. */ +function unwrapConfigLayers(config: unknown): { value: unknown; layers: number } { + let value = config; + let layers = 0; + while (typeof value === 'string' && layers < MAX_CONFIG_UNWRAP_DEPTH) { + try { + value = JSON.parse(value); + } catch { + break; + } + layers++; } - if (typeof config === 'object' && config !== null) return config as Record; + return { value, layers }; +} + +/** + * #2829: coerce a config value to the underlying plain object before it is + * written back, fully unwrapping any accidental JSON-string nesting so a + * re-wrapping bug can't keep growing a layer on every write. Returns {} (with a + * warning) when the value never resolves to a plain object. Every `sources` + * config writer runs its config through this before `JSON.stringify` + the + * `$1::text::jsonb` cast, which converges the stored value back to a jsonb + * object. + */ +export function normalizeSourceConfig(config: unknown): Record { + const { value } = unwrapConfigLayers(config); + if (isPlainObject(value)) return value; + console.warn( + `[gbrain] source config was not a JSON object (got ${value === null ? 'null' : typeof value}); ` + + `storing {} instead. Run 'gbrain doctor' to find affected sources.`, + ); return {}; } +/** + * Parse `sources.config` to a plain object regardless of driver shape (Postgres + * returns an object; PGLite returns a JSON string). #2829: also unwraps a config + * that was accidentally stored as a nested JSON string scalar, and warns once + * when more than one unwrap layer is needed (one layer is the normal PGLite + * path; two or more means the value was re-wrapped and should be repaired). + */ +export function parseSourceConfig(config: unknown): Record { + const { value, layers } = unwrapConfigLayers(config); + if (layers > 1) { + console.warn( + `[gbrain] source config was stored as a ${layers}-layer nested JSON string; ` + + `it will be repaired on the next config write. Run 'gbrain doctor' to find affected sources.`, + ); + } + return isPlainObject(value) ? value : {}; +} + /** True iff the source's config.federated field is the literal boolean true. */ export function isSourceFederated(config: unknown): boolean { const parsed = parseSourceConfig(config); diff --git a/test/doctor-source-config-shape.test.ts b/test/doctor-source-config-shape.test.ts new file mode 100644 index 000000000..40519a306 --- /dev/null +++ b/test/doctor-source-config-shape.test.ts @@ -0,0 +1,63 @@ +/** + * Test: `checkSourceConfigShape` (#2829 — source config string-scalar re-wrapping). + * + * Pure-helper surface — the check only consumes `engine.executeRaw`, so a + * structurally-typed mock satisfies the contract (same pattern as + * `doctor-child-orphans.test.ts`). No PGLite spin-up required. + */ + +import { describe, test, expect } from 'bun:test'; +import { checkSourceConfigShape } from '../src/commands/doctor.ts'; +import type { BrainEngine } from '../src/core/engine.ts'; + +/** Build a structurally-typed BrainEngine whose executeRaw returns per-SQL results. */ +function makeMockEngine(handler: (sql: string) => Promise): BrainEngine { + return { + executeRaw: handler, + } as unknown as BrainEngine; +} + +describe('checkSourceConfigShape (#2829)', () => { + test('all configs are objects → status:ok', async () => { + const engine = makeMockEngine(async () => []); + const result = await checkSourceConfigShape(engine); + expect(result.name).toBe('source_config_shape'); + expect(result.status).toBe('ok'); + expect(result.message).toContain('JSON objects'); + }); + + test('non-object configs → warn naming affected sources + repair hint', async () => { + const engine = makeMockEngine(async () => [ + { id: 'default', typ: 'string' }, + { id: 'wiki', typ: 'string' }, + ]); + const result = await checkSourceConfigShape(engine); + expect(result.status).toBe('warn'); + expect(result.message).toContain('2 source(s)'); + expect(result.message).toContain('default (string)'); + expect(result.message).toContain('wiki (string)'); + expect(result.message).toContain('#2829'); + // Paste-ready repair SQL is part of the hint. + expect(result.message).toContain('UPDATE sources SET config'); + }); + + test('detection query targets the exact jsonb_typeof predicate', async () => { + let captured = ''; + const engine = makeMockEngine(async (sql: string) => { + captured = sql; + return []; + }); + await checkSourceConfigShape(engine); + expect(captured).toContain('jsonb_typeof(config) AS typ'); + expect(captured).toContain("WHERE jsonb_typeof(config) <> 'object'"); + }); + + test('engine error → warn, never a false ok', async () => { + const engine = makeMockEngine(async () => { + throw new Error('relation "sources" does not exist'); + }); + const result = await checkSourceConfigShape(engine); + expect(result.status).toBe('warn'); + expect(result.message).toContain('Check failed'); + }); +}); diff --git a/test/sources-load.test.ts b/test/sources-load.test.ts index ad202ba89..d6d392e53 100644 --- a/test/sources-load.test.ts +++ b/test/sources-load.test.ts @@ -11,6 +11,7 @@ import { loadAllSources, fetchSource, parseSourceConfig, + normalizeSourceConfig, isSourceFederated, } from '../src/core/sources-load.ts'; @@ -120,6 +121,46 @@ describe('parseSourceConfig', () => { test('returns empty object on malformed JSON string', () => { expect(parseSourceConfig('{')).toEqual({}); }); + + test('#2829: unwraps an accidental multi-layer nested string (self-heal read path)', () => { + const wrapped = JSON.stringify(JSON.stringify({ federated: true })); + expect(parseSourceConfig(wrapped)).toEqual({ federated: true }); + }); +}); + +describe('normalizeSourceConfig (#2829)', () => { + test('passes a plain object through unchanged', () => { + expect(normalizeSourceConfig({ federated: true, webhook_secret: 'x' })).toEqual({ + federated: true, + webhook_secret: 'x', + }); + }); + + test('unwraps a single JSON-string layer', () => { + expect(normalizeSourceConfig('{"federated":true}')).toEqual({ federated: true }); + }); + + test('unwraps a 5-layer nested JSON string back to the object', () => { + let v: unknown = { federated: true, tracked_branch: 'main' }; + for (let i = 0; i < 5; i++) v = JSON.stringify(v); // 5 stringify passes = 5 layers + expect(normalizeSourceConfig(v)).toEqual({ federated: true, tracked_branch: 'main' }); + }); + + test('non-object garbage resolves to {}', () => { + expect(normalizeSourceConfig('not json')).toEqual({}); + expect(normalizeSourceConfig('42')).toEqual({}); // parses to a number + expect(normalizeSourceConfig('"just a string"')).toEqual({}); + expect(normalizeSourceConfig(null)).toEqual({}); + expect(normalizeSourceConfig(undefined)).toEqual({}); + expect(normalizeSourceConfig(['a', 'b'])).toEqual({}); // array is not a plain object + expect(normalizeSourceConfig(JSON.stringify(['a']))).toEqual({}); + }); + + test('respects the unwrap bound instead of spinning forever', () => { + let v: unknown = { federated: true }; + for (let i = 0; i < 12; i++) v = JSON.stringify(v); // 12 layers, past the bound of 10 + expect(normalizeSourceConfig(v)).toEqual({}); // gives up to {} once the bound is hit + }); }); describe('isSourceFederated', () => {