diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 6f7c9aa20..fceb81bd7 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -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 diff --git a/src/core/op-checkpoint.ts b/src/core/op-checkpoint.ts index a338d20b6..f1c0c1e2e 100644 --- a/src/core/op-checkpoint.ts +++ b/src/core/op-checkpoint.ts @@ -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(); + 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); diff --git a/src/core/pglite-schema.ts b/src/core/pglite-schema.ts index 17705f206..1279ce910 100644 --- a/src/core/pglite-schema.ts +++ b/src/core/pglite-schema.ts @@ -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) ); diff --git a/src/core/schema-embedded.ts b/src/core/schema-embedded.ts index fc65638bd..d0e2c51ef 100644 --- a/src/core/schema-embedded.ts +++ b/src/core/schema-embedded.ts @@ -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) ); diff --git a/src/schema.sql b/src/schema.sql index 0ea4a6b09..e02bc7f02 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -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) ); diff --git a/test/op-checkpoint.test.ts b/test/op-checkpoint.test.ts index 6c821e38c..b5fe46127 100644 --- a/test/op-checkpoint.test.ts +++ b/test/op-checkpoint.test.ts @@ -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']);