mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
`sources.config` is a jsonb OBJECT column, but a read→write cycle that
JSON.stringify'd an already-stringified value re-wrapped it into a JSON string
scalar ("{}", "\"{}\"", ...) that grew one layer per write. parseSourceConfig
only unwrapped one layer, so the corruption never healed and federation/ACL
reads saw a string instead of the settings object.
- Add normalizeSourceConfig: a bounded (10-iteration) loop that JSON.parses
while the value is a string and returns {} (with a console.warn) when the
result is not a plain object. All six `UPDATE sources SET config` writers run
their config through it before stringify, converging the stored value back to
a jsonb object on the next write.
- parseSourceConfig now does the same bounded unwrap and warns once when more
than one layer was found (one layer is the normal PGLite path).
- Add a `source_config_shape` doctor check that flags any sources row where
jsonb_typeof(config) <> 'object', with the repair path.
- Unit-test the helper (object passthrough, 1-layer, 5-layer nested, garbage
and over-bound inputs) and the doctor check (mock engine).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
7bbd087cb7
commit
e36251c023
@@ -528,6 +528,48 @@ export async function childTableOrphansCheck(engine: BrainEngine): Promise<Check
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* #2829: source `config` is a jsonb OBJECT column (`DEFAULT '{}'::jsonb`), but a
|
||||
* re-wrapping bug could store it as a JSON string scalar ("{}", "\"{}\"", ...)
|
||||
* that grows a layer on every read→write cycle. Any row where
|
||||
* `jsonb_typeof(config) <> '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<Check> {
|
||||
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<DoctorReport> {
|
||||
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.
|
||||
|
||||
@@ -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<void>
|
||||
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<vo
|
||||
cfg.webhook_secret = secret;
|
||||
await engine.executeRaw(
|
||||
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
|
||||
[JSON.stringify(cfg), id],
|
||||
[JSON.stringify(normalizeSourceConfig(cfg)), id],
|
||||
);
|
||||
console.log(`New webhook secret for source "${id}":`);
|
||||
console.log(` ${secret}`);
|
||||
@@ -978,7 +979,7 @@ async function runWebhookClear(engine: BrainEngine, args: string[]): Promise<voi
|
||||
delete cfg.github_repo;
|
||||
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 configuration cleared for source "${id}".`);
|
||||
}
|
||||
@@ -1003,7 +1004,7 @@ async function runTrackedBranch(engine: BrainEngine, args: string[]): Promise<vo
|
||||
cfg.tracked_branch = setArg;
|
||||
await engine.executeRaw(
|
||||
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
|
||||
[JSON.stringify(cfg), id],
|
||||
[JSON.stringify(normalizeSourceConfig(cfg)), id],
|
||||
);
|
||||
console.log(`Tracked branch for source "${id}" set to "${setArg}".`);
|
||||
return;
|
||||
@@ -1019,7 +1020,7 @@ async function runTrackedBranch(engine: BrainEngine, args: string[]): Promise<vo
|
||||
cfg.tracked_branch = branch;
|
||||
await engine.executeRaw(
|
||||
`UPDATE sources SET config = $1::text::jsonb WHERE id = $2`,
|
||||
[JSON.stringify(cfg), id],
|
||||
[JSON.stringify(normalizeSourceConfig(cfg)), id],
|
||||
);
|
||||
console.log(`Detected branch "${branch}" for source "${id}"; persisted to config.tracked_branch.`);
|
||||
} catch (e) {
|
||||
|
||||
@@ -98,6 +98,7 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
|
||||
'flagged_pages',
|
||||
'salience_health',
|
||||
'scraper_junk_pages',
|
||||
'source_config_shape',
|
||||
'source_routing_health',
|
||||
'stub_guard_24h',
|
||||
'sync_failures',
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
if (typeof config === 'string') {
|
||||
try { return JSON.parse(config) as Record<string, unknown>; } 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<string, unknown> {
|
||||
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<string, unknown>;
|
||||
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<string, unknown> {
|
||||
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<string, unknown> {
|
||||
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);
|
||||
|
||||
@@ -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<unknown[]>): 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');
|
||||
});
|
||||
});
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user