mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(sync): op_checkpoints array-shape guard — CHECK + repair + defensive loader
completed_keys is JSONB and the checkpoint loader runs jsonb_array_elements_text over it. A non-array (scalar) value makes that throw "cannot extract elements from a scalar", which takes down the whole UNION load — including the valid op_checkpoint_paths child rows — and loses all checkpoint progress for that key. No current writer produces a scalar, but an older binary / external script / future bug could. Make the corruption class structurally impossible and self-healing: - migration v119: LOCK TABLE (so an out-of-band scalar can't land between repair and constrain; no-op on single-connection PGLite), repair any pre-existing scalar to '[]' (op_checkpoint_paths child rows are the append-only source of truth, so the reset loses nothing), then add the named CHECK (jsonb_typeof(completed_keys) = 'array') via a pg_constraint IF NOT EXISTS guard. A DB-enforced always-on guard — the correct pattern vs a migration verify-hook, which never runs on already-stamped brains. - schema.sql + pglite-schema.ts (+ regenerated schema-embedded.ts) ship the same NAMED inline CHECK so fresh installs match migrated brains and v119 skips the duplicate. - op-checkpoint.ts loader: gate the legacy arm on jsonb_typeof = 'array' so a scalar parent is skipped (children still load) instead of throwing the whole union, and log a specific corruption warning when one is seen. - tests: CHECK rejects a scalar (exactly one constraint, no blob+migration dupe); loader survives a scalar parent and returns the children; v119 repair converts a scalar to '[]'. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
36a9fad214
commit
c4106e706c
@@ -5326,6 +5326,44 @@ export const MIGRATIONS: Migration[] = [
|
||||
DELETE FROM query_cache;
|
||||
`,
|
||||
},
|
||||
{
|
||||
version: 119,
|
||||
name: 'op_checkpoints_completed_keys_array_check',
|
||||
// v0.42.x — make the op_checkpoints scalar-corruption class structurally
|
||||
// impossible. completed_keys is JSONB and the loader runs
|
||||
// jsonb_array_elements_text(completed_keys); a non-array (scalar) value
|
||||
// makes that throw "cannot extract elements from a scalar", which takes
|
||||
// down the whole UNION load (including the valid op_checkpoint_paths child
|
||||
// rows) and loses all checkpoint progress for that key. No current writer
|
||||
// can produce a scalar, but an older binary / external script / future bug
|
||||
// could — the CHECK is a DB-enforced, always-on guard (the correct pattern
|
||||
// vs a migration verify-hook, which would not run on already-stamped
|
||||
// brains). LOCK first so an out-of-band scalar write can't land between the
|
||||
// repair and the ADD CONSTRAINT (no-op on single-connection PGLite). The
|
||||
// repair resets any pre-existing scalar to '[]'; op_checkpoint_paths child
|
||||
// rows are the append-only source of truth, so the reset loses nothing.
|
||||
// Mirrored in src/schema.sql, src/core/pglite-schema.ts, and the generated
|
||||
// src/core/schema-embedded.ts so fresh installs carry the same CHECK.
|
||||
idempotent: true,
|
||||
sql: `
|
||||
LOCK TABLE op_checkpoints IN SHARE ROW EXCLUSIVE MODE;
|
||||
|
||||
UPDATE op_checkpoints
|
||||
SET completed_keys = '[]'::jsonb, updated_at = now()
|
||||
WHERE jsonb_typeof(completed_keys) <> 'array';
|
||||
|
||||
DO $$
|
||||
BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'op_checkpoints_completed_keys_array'
|
||||
) THEN
|
||||
ALTER TABLE op_checkpoints
|
||||
ADD CONSTRAINT op_checkpoints_completed_keys_array
|
||||
CHECK (jsonb_typeof(completed_keys) = 'array');
|
||||
END IF;
|
||||
END $$;
|
||||
`,
|
||||
},
|
||||
];
|
||||
|
||||
export const LATEST_VERSION = MIGRATIONS.length > 0
|
||||
|
||||
@@ -122,18 +122,40 @@ export async function loadOpCheckpoint(
|
||||
// dedupes, so we skip a server-side dedup sort over up to 204K rows on every
|
||||
// resume. `jsonb_array_elements_text` expands the legacy array server-side,
|
||||
// which also removes the old postgres.js-vs-PGLite string/array handling.
|
||||
const rows = await engine.executeRaw<{ ckey: unknown }>(
|
||||
`SELECT path AS ckey FROM op_checkpoint_paths
|
||||
//
|
||||
// v0.42.x (BUG 3 guard): the legacy arm is gated on
|
||||
// `jsonb_typeof(completed_keys) = 'array'`. Without it a non-array (scalar)
|
||||
// parent row makes jsonb_array_elements_text throw "cannot extract elements
|
||||
// from a scalar", which kills the WHOLE union — including the valid child
|
||||
// rows — and loses all checkpoint progress for the key. Skipping the scalar
|
||||
// keeps the child rows; the third arm flags the corruption so we log it once
|
||||
// (migration v119's CHECK makes this impossible going forward; a hit implies
|
||||
// schema drift / disabled constraint / an out-of-band writer).
|
||||
const rows = await engine.executeRaw<{ ckey: unknown; corrupt: number }>(
|
||||
`SELECT path AS ckey, 0 AS corrupt FROM op_checkpoint_paths
|
||||
WHERE op = $1 AND fingerprint = $2
|
||||
UNION ALL
|
||||
SELECT jsonb_array_elements_text(completed_keys) AS ckey FROM op_checkpoints
|
||||
WHERE op = $1 AND fingerprint = $2`,
|
||||
SELECT jsonb_array_elements_text(completed_keys) AS ckey, 0 AS corrupt FROM op_checkpoints
|
||||
WHERE op = $1 AND fingerprint = $2 AND jsonb_typeof(completed_keys) = 'array'
|
||||
UNION ALL
|
||||
SELECT NULL AS ckey, 1 AS corrupt FROM op_checkpoints
|
||||
WHERE op = $1 AND fingerprint = $2 AND jsonb_typeof(completed_keys) <> 'array'`,
|
||||
[key.op, key.fingerprint],
|
||||
);
|
||||
const set = new Set<string>();
|
||||
let corruptParent = false;
|
||||
for (const r of rows) {
|
||||
if (Number(r.corrupt) === 1) {
|
||||
corruptParent = true;
|
||||
continue;
|
||||
}
|
||||
if (typeof r.ckey === 'string') set.add(r.ckey);
|
||||
}
|
||||
if (corruptParent) {
|
||||
console.error(
|
||||
`[op-checkpoint] WARNING: op_checkpoints.completed_keys for (${key.op}, ${key.fingerprint}) is a non-array (scalar) and was skipped to protect the load — child op_checkpoint_paths rows still applied. This implies schema drift, a disabled CHECK constraint, or an out-of-band writer.`,
|
||||
);
|
||||
}
|
||||
return [...set];
|
||||
} catch (e) {
|
||||
console.error(`[op-checkpoint] load failed (${key.op}, ${key.fingerprint}):`, (e as Error).message);
|
||||
|
||||
@@ -932,7 +932,11 @@ CREATE TABLE IF NOT EXISTS oauth_codes (
|
||||
CREATE TABLE IF NOT EXISTS op_checkpoints (
|
||||
op TEXT NOT NULL,
|
||||
fingerprint TEXT NOT NULL,
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb,
|
||||
-- v0.42.x: must be a JSONB array. The loader runs jsonb_array_elements_text
|
||||
-- over it; a scalar would throw and wipe the whole checkpoint load. CHECK is
|
||||
-- the DB-enforced always-on guard (mirrors migration v119).
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb
|
||||
CONSTRAINT op_checkpoints_completed_keys_array CHECK (jsonb_typeof(completed_keys) = 'array'),
|
||||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
PRIMARY KEY (op, fingerprint)
|
||||
);
|
||||
|
||||
@@ -698,7 +698,11 @@ CREATE INDEX IF NOT EXISTS idx_mcp_log_agent_time ON mcp_request_log(agent_name,
|
||||
CREATE TABLE IF NOT EXISTS op_checkpoints (
|
||||
op TEXT NOT NULL,
|
||||
fingerprint TEXT NOT NULL,
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb,
|
||||
-- v0.42.x: must be a JSONB array. The loader runs jsonb_array_elements_text
|
||||
-- over it; a scalar would throw and wipe the whole checkpoint load. CHECK is
|
||||
-- the DB-enforced always-on guard (mirrors migration v119).
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb
|
||||
CONSTRAINT op_checkpoints_completed_keys_array CHECK (jsonb_typeof(completed_keys) = 'array'),
|
||||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
PRIMARY KEY (op, fingerprint)
|
||||
);
|
||||
|
||||
+5
-1
@@ -694,7 +694,11 @@ CREATE INDEX IF NOT EXISTS idx_mcp_log_agent_time ON mcp_request_log(agent_name,
|
||||
CREATE TABLE IF NOT EXISTS op_checkpoints (
|
||||
op TEXT NOT NULL,
|
||||
fingerprint TEXT NOT NULL,
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb,
|
||||
-- v0.42.x: must be a JSONB array. The loader runs jsonb_array_elements_text
|
||||
-- over it; a scalar would throw and wipe the whole checkpoint load. CHECK is
|
||||
-- the DB-enforced always-on guard (mirrors migration v119).
|
||||
completed_keys JSONB NOT NULL DEFAULT '[]'::jsonb
|
||||
CONSTRAINT op_checkpoints_completed_keys_array CHECK (jsonb_typeof(completed_keys) = 'array'),
|
||||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||||
PRIMARY KEY (op, fingerprint)
|
||||
);
|
||||
|
||||
@@ -243,6 +243,82 @@ describe('resumeFilter (pure)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('BUG 3: completed_keys array-shape guard (v119 CHECK + defensive loader)', () => {
|
||||
const CONSTRAINT = 'op_checkpoints_completed_keys_array';
|
||||
|
||||
test('CHECK rejects a scalar completed_keys write — exactly one constraint (no blob+migration dupe)', async () => {
|
||||
// Fresh PGLite install: the schema blob ships the NAMED inline CHECK and
|
||||
// migration v119's IF NOT EXISTS skips re-adding it. Exactly one constraint.
|
||||
const c = await engine.executeRaw<{ n: number }>(
|
||||
`SELECT count(*)::int AS n FROM pg_constraint WHERE conname = $1`,
|
||||
[CONSTRAINT],
|
||||
);
|
||||
expect(Number(c[0].n)).toBe(1);
|
||||
|
||||
let threw = false;
|
||||
try {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at)
|
||||
VALUES ('embed', 'fp-reject', '"not-an-array"'::jsonb, now())`,
|
||||
);
|
||||
} catch {
|
||||
threw = true;
|
||||
}
|
||||
expect(threw).toBe(true);
|
||||
});
|
||||
|
||||
test('loader survives a scalar parent: returns child rows (not []), does not throw', async () => {
|
||||
const key = { op: 'sync', fingerprint: 'fp-scalar-survive' };
|
||||
// Bypass the CHECK to simulate pre-migration / out-of-band corruption.
|
||||
await engine.executeRaw(`ALTER TABLE op_checkpoints DROP CONSTRAINT ${CONSTRAINT}`);
|
||||
try {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at)
|
||||
VALUES ('sync', 'fp-scalar-survive', '"corrupt-scalar"'::jsonb, now())`,
|
||||
);
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO op_checkpoint_paths (op, fingerprint, path)
|
||||
VALUES ('sync', 'fp-scalar-survive', 'child-a.md')`,
|
||||
);
|
||||
// Pre-guard, jsonb_array_elements_text on the scalar threw and the catch
|
||||
// returned [] — losing child-a.md. The typeof guard skips the scalar so
|
||||
// the valid child survives.
|
||||
const loaded = await loadOpCheckpoint(engine, key);
|
||||
expect(loaded).toEqual(['child-a.md']);
|
||||
} finally {
|
||||
await engine.executeRaw(
|
||||
`UPDATE op_checkpoints SET completed_keys = '[]'::jsonb WHERE jsonb_typeof(completed_keys) <> 'array'`,
|
||||
);
|
||||
await engine.executeRaw(
|
||||
`ALTER TABLE op_checkpoints ADD CONSTRAINT ${CONSTRAINT} CHECK (jsonb_typeof(completed_keys) = 'array')`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('v119 repair converts a scalar parent to an empty array', async () => {
|
||||
await engine.executeRaw(`ALTER TABLE op_checkpoints DROP CONSTRAINT ${CONSTRAINT}`);
|
||||
try {
|
||||
await engine.executeRaw(
|
||||
`INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at)
|
||||
VALUES ('embed', 'fp-repair', '"scalar"'::jsonb, now())`,
|
||||
);
|
||||
// Migration v119's repair statement.
|
||||
await engine.executeRaw(
|
||||
`UPDATE op_checkpoints SET completed_keys = '[]'::jsonb, updated_at = now()
|
||||
WHERE jsonb_typeof(completed_keys) <> 'array'`,
|
||||
);
|
||||
const typ = await engine.executeRaw<{ t: string }>(
|
||||
`SELECT jsonb_typeof(completed_keys) AS t FROM op_checkpoints WHERE op = 'embed' AND fingerprint = 'fp-repair'`,
|
||||
);
|
||||
expect(typ[0].t).toBe('array');
|
||||
} finally {
|
||||
await engine.executeRaw(
|
||||
`ALTER TABLE op_checkpoints ADD CONSTRAINT ${CONSTRAINT} CHECK (jsonb_typeof(completed_keys) = 'array')`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('purgeStaleCheckpoints', () => {
|
||||
test('no stale rows: returns 0', async () => {
|
||||
await recordCompleted(engine, { op: 'embed', fingerprint: 'fresh' }, ['x']);
|
||||
|
||||
Reference in New Issue
Block a user