From dec4a67fe705e8270879138e12e021c992837e44 Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Fri, 24 Jul 2026 16:44:09 +0200 Subject: [PATCH] fix(memory): self-heal upsertVector/deleteVector from a raced vec_memories drop (#8337) Live incident: memory.vec.upsert.fail {"error":"no such table: vec_memories"} recurred repeatedly right after restarts, even though ensureReady() is called immediately beforehand. ensureReady()'s signature-check-then-maybe-recreate logic (resetForSignature does DROP TABLE IF EXISTS + CREATE VIRTUAL TABLE) is not synchronized against a concurrent caller's upsertVector/deleteVector -- a second in-flight memory write that independently decides (from a stale read of memory_vec_meta) it also needs to reset the table can drop it out from under another write's insert. Confirmed live: memory_vec_meta showed vec_loaded=0 for the entire session across many restarts, then flipped to 1 mid-investigation once one attempt finally completed without interruption -- consistent with an intermittent race, not a permanently broken path (verified the underlying sqlite-vec extension and CREATE VIRTUAL TABLE statement work correctly in isolation, both on the host and inside the production container). Rather than chase the exact interleaving (every underlying SQLite call is synchronous via better-sqlite3, so the race window is narrow and did not reproduce under simple Promise.all stress tests), makes the write path resilient to arriving after the table was dropped: on a "no such table" error, recreate vec_memories from the last-known-good memory_vec_meta dimension and retry once. Co-authored-by: Markus Hartung --- src/lib/memory/vectorStore.ts | 65 ++++++- ...emory-vectorstore-upsert-self-heal.test.ts | 159 ++++++++++++++++++ 2 files changed, 216 insertions(+), 8 deletions(-) create mode 100644 tests/unit/memory-vectorstore-upsert-self-heal.test.ts diff --git a/src/lib/memory/vectorStore.ts b/src/lib/memory/vectorStore.ts index f702f97012..92f706a3d4 100644 --- a/src/lib/memory/vectorStore.ts +++ b/src/lib/memory/vectorStore.ts @@ -126,6 +126,36 @@ function liveVecQuantization(): VecQuantization { return storedVecQuantization(getMemoryVecMeta().embeddingSignature); } +/** + * True for the specific SQLite error `ensureReady()`'s check-then-act race can + * produce: a concurrent caller's `resetForSignature()` (DROP + CREATE) drops + * the table between THIS caller's own `ensureReady()` and its subsequent + * write/delete, so the table is briefly and unexpectedly gone even though + * `memory_vec_meta` still says it's ready. + */ +function isMissingVecTableError(err: unknown): boolean { + const msg = err instanceof Error ? err.message : String(err); + return /no such table:\s*vec_memories\b/.test(msg); +} + +/** + * Recreate `vec_memories` from the last-known-good `memory_vec_meta` row + * (not a fresh `EmbeddingResolution` — the caller may not have one handy at + * this point, e.g. inside a catch block). No-op (returns false) if there is + * no recorded dimension to recreate from, in which case the original error + * should be rethrown by the caller. + */ +function recreateFromMeta(): boolean { + const meta = getMemoryVecMeta(); + if (meta.activeDim == null) return false; + const db = getDbInstance(); + const q = storedVecQuantization(meta.embeddingSignature); + db.exec( + `CREATE VIRTUAL TABLE IF NOT EXISTS vec_memories USING vec0(embedding ${vecColumnType(meta.activeDim, q)})`, + ); + return true; +} + // ──────────────── Implementation ──────────────── class VectorStoreImpl implements VectorStore { @@ -188,18 +218,37 @@ class VectorStoreImpl implements VectorStore { // INSERT OR REPLACE is not supported by vec0 — use DELETE + INSERT for upsert semantics. // int8 tables quantize the float32 blob in SQL (vec_quantize_int8); float32 bind raw. const q = liveVecQuantization(); - db.prepare("DELETE FROM vec_memories WHERE rowid = ?").run(BigInt(row.rowid)); - db.prepare(`INSERT INTO vec_memories(rowid, embedding) VALUES (?, ${vecValueExpr(q)})`).run( - BigInt(row.rowid), - encodeVector(vector), - ); + try { + db.prepare("DELETE FROM vec_memories WHERE rowid = ?").run(BigInt(row.rowid)); + db.prepare(`INSERT INTO vec_memories(rowid, embedding) VALUES (?, ${vecValueExpr(q)})`).run( + BigInt(row.rowid), + encodeVector(vector), + ); + } catch (err: unknown) { + // Self-heal: a concurrent ensureReady()'s reset raced ahead of us and + // dropped the table after our own ensureReady() already ran. Recreate + // from the last-known-good meta and retry once before giving up. + if (!isMissingVecTableError(err) || !recreateFromMeta()) throw err; + db.prepare("DELETE FROM vec_memories WHERE rowid = ?").run(BigInt(row.rowid)); + db.prepare(`INSERT INTO vec_memories(rowid, embedding) VALUES (?, ${vecValueExpr(q)})`).run( + BigInt(row.rowid), + encodeVector(vector), + ); + } } async deleteVector(memoryId: string): Promise { const db = getDbInstance(); - db.prepare( - "DELETE FROM vec_memories WHERE rowid = (SELECT rowid FROM memories WHERE id = ?)", - ).run(memoryId); + try { + db.prepare( + "DELETE FROM vec_memories WHERE rowid = (SELECT rowid FROM memories WHERE id = ?)", + ).run(memoryId); + } catch (err: unknown) { + if (!isMissingVecTableError(err)) throw err; + // Table already gone (racing reset, or nothing to delete) — recreating it + // empty is sufficient; there is nothing left to delete either way. + recreateFromMeta(); + } } async searchVector( diff --git a/tests/unit/memory-vectorstore-upsert-self-heal.test.ts b/tests/unit/memory-vectorstore-upsert-self-heal.test.ts new file mode 100644 index 0000000000..de8657ff60 --- /dev/null +++ b/tests/unit/memory-vectorstore-upsert-self-heal.test.ts @@ -0,0 +1,159 @@ +/** + * tests/unit/memory-vectorstore-upsert-self-heal.test.ts + * + * Regression test: `upsertVector`/`deleteVector` must self-heal from a missing + * `vec_memories` table instead of throwing "no such table: vec_memories". + * + * Live incident: `memory.vec.upsert.fail {"error":"no such table: vec_memories"}` + * recurred repeatedly in production right after restarts, even though + * `ensureReady()` is called immediately beforehand. Root cause: `ensureReady()`'s + * signature-check-then-maybe-recreate logic (`resetForSignature` does + * `DROP TABLE IF EXISTS` + `CREATE VIRTUAL TABLE`) is not synchronized against + * a concurrent caller's `upsertVector`/`deleteVector` — a second in-flight + * memory write that decides (from a stale read of `memory_vec_meta`) it also + * needs to reset the table can drop it out from under another write's insert. + * `store.ts`/`retrieval.ts`/`reindex.ts` also never check `ensureReady()`'s + * `{ready: boolean}` return value before proceeding. + * + * Rather than chase the exact interleaving (every underlying SQLite call is + * synchronous, so the race window is narrow and timing-dependent), this makes + * the write path resilient to arriving after the table was dropped: on a + * "no such table" error, recreate `vec_memories` from the last-known-good + * `memory_vec_meta` dimension and retry once. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { mock } from "node:test"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-vecstore-self-heal-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; + +const core = await import("../../src/lib/db/core.ts"); +const vsModule = await import("../../src/lib/memory/vectorStore.ts"); +const { getVectorStore, _resetVectorStoreSingleton } = vsModule; + +import type { EmbeddingResolution } from "../../src/lib/memory/embedding/types.ts"; + +const DIM = 4; + +function makeResolution(): EmbeddingResolution { + return { + source: "remote", + model: "test/dim4", + dimensions: DIM, + signature: `test:dim4:${DIM}`, + reason: "test", + }; +} + +function makeVec(...values: number[]): Float32Array { + return new Float32Array(values); +} + +function cleanup() { + mock.restoreAll(); + _resetVectorStoreSingleton(); + core.resetDbInstance(); + if (fs.existsSync(TEST_DATA_DIR)) { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.afterEach(() => { + cleanup(); +}); + +test.after(() => { + core.resetDbInstance(); + if (fs.existsSync(TEST_DATA_DIR)) { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } +}); + +function getStoreOrSkip(t: { skip: (msg: string) => void }): ReturnType { + _resetVectorStoreSingleton(); + const store = getVectorStore(); + if (store === null) { + t.skip("sqlite-vec not available in this environment — skipping"); + return null; + } + return store; +} + +function insertMemory( + db: ReturnType, + id: string, + apiKeyId: string, + content: string, +) { + db.prepare( + `INSERT INTO memories (id, api_key_id, type, key, content, created_at) + VALUES (?, ?, 'factual', ?, ?, datetime('now'))`, + ).run(id, apiKeyId, `key-${id}`, content); +} + +test("upsertVector: self-heals when vec_memories is missing after ensureReady already ran once", async (t) => { + const store = getStoreOrSkip(t); + if (!store) return; + + const db = core.getDbInstance(); + await store.ensureReady(makeResolution()); + insertMemory(db, "mem-a", "key1", "alpha"); + insertMemory(db, "mem-b", "key1", "beta"); + + // Baseline: works normally. + await store.upsertVector("mem-a", makeVec(1.0, 0.0, 0.0, 0.0)); + + // Simulate a concurrent coroutine's resetForSignature() racing ahead and + // dropping the table between this caller's own ensureReady() and its + // upsertVector() — the exact live-incident interleaving. + db.exec("DROP TABLE IF EXISTS vec_memories"); + assert.equal( + db.prepare("SELECT name FROM sqlite_master WHERE name = 'vec_memories'").get(), + undefined, + "table must actually be gone for this test to be meaningful", + ); + + // Must NOT throw "no such table: vec_memories" — must self-heal and succeed. + await store.upsertVector("mem-b", makeVec(0.0, 1.0, 0.0, 0.0)); + + const cnt = db.prepare("SELECT COUNT(*) AS cnt FROM vec_memories").get() as { cnt: number }; + assert.equal(cnt.cnt, 1, "table was recreated fresh, so only the post-heal insert is present"); +}); + +test("deleteVector: self-heals when vec_memories is missing (no throw)", async (t) => { + const store = getStoreOrSkip(t); + if (!store) return; + + const db = core.getDbInstance(); + await store.ensureReady(makeResolution()); + insertMemory(db, "mem-a", "key1", "alpha"); + await store.upsertVector("mem-a", makeVec(1.0, 0.0, 0.0, 0.0)); + + db.exec("DROP TABLE IF EXISTS vec_memories"); + + await assert.doesNotReject( + () => store.deleteVector("mem-a"), + "deleteVector must self-heal from a missing table, not throw", + ); +}); + +test("upsertVector: still throws a genuine unrelated error unchanged (no over-broad catch)", async (t) => { + const store = getStoreOrSkip(t); + if (!store) return; + + await store.ensureReady(makeResolution()); + + // memoryId that was never inserted into `memories` — this is a real, + // unrelated error path and must still surface exactly as before. + await assert.rejects( + () => store.upsertVector("nonexistent-id", makeVec(1.0, 0.0, 0.0, 0.0)), + /memory not found/i, + "unrelated errors must not be swallowed by the self-heal retry", + ); +});