fix: share one SQLite connection across KV stores on same path (Bug #5)

__tests__/unit/persistent-kv.test.js

@@ -1,3 +1,6 @@
+import os from 'node:os';
+import path from 'node:path';
+import fs from 'node:fs';
 import { initKVStore } from '../../src/persistent-kv.js';
 
 describe('persistent-kv', () => {
@@ -68,4 +71,59 @@ describe('persistent-kv', () => {
     a.close();
     b.close();
   });
+
+  // Bug #5: two file-backed stores against the same dbPath must share one
+  // underlying connection so concurrent writes don't trip SQLITE_BUSY.
+  describe('shared file-backed connection (Bug #5)', () => {
+    let tmpDir;
+    let dbPath;
+
+    beforeEach(() => {
+      tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'temper-kv-test-'));
+      dbPath = path.join(tmpDir, 'shared.db');
+    });
+
+    afterEach(() => {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    });
+
+    it('shares one underlying Database across two stores on the same path', () => {
+      const a = initKVStore(dbPath, 'dedup');
+      const b = initKVStore(dbPath, 'rate_limits');
+      // ._db is the underlying better-sqlite3 Database; identity check is the
+      // tightest assertion that a single connection backs both stores.
+      expect(a._db).toBe(b._db);
+      a.close();
+      b.close();
+    });
+
+    it('keeps the connection alive while at least one store still uses it', () => {
+      const a = initKVStore(dbPath, 'dedup');
+      const b = initKVStore(dbPath, 'rate_limits');
+      a.set('hello', 'world');
+      a.close();
+      // a is closed but b still holds the connection — writes must succeed.
+      expect(() => b.set('still', 'ok')).not.toThrow();
+      expect(b.get('still')).toBe('ok');
+      b.close();
+    });
+
+    it('closes the connection when the last store releases it', () => {
+      const a = initKVStore(dbPath, 'dedup');
+      const dbRef = a._db;
+      a.close();
+      // The Database is closed; using a closed handle throws.
+      expect(() => dbRef.prepare('SELECT 1').get()).toThrow();
+    });
+
+    it('reopens cleanly after all stores have closed', () => {
+      const a = initKVStore(dbPath, 'dedup');
+      a.set('x', '1');
+      a.close();
+
+      const reopened = initKVStore(dbPath, 'dedup');
+      expect(reopened.get('x')).toBe('1');
+      reopened.close();
+    });
+  });
 });

src/persistent-kv.js

@@ -9,11 +9,50 @@
 
 import Database from 'better-sqlite3';
 
+// Bug #5: callers (app.js) use the same SQLite file for two logical
+// stores ('webhook_dedup' and 'ai_rate_limits' both live in dedup.db).
+// Opening two `Database` connections against the same file works for
+// reads under WAL but produces SQLITE_BUSY on concurrent writes — and
+// `markProcessed` + `recordReview` race regularly. Cache by absolute
+// dbPath so the second call against the same file shares the same
+// underlying connection. `:memory:` stays per-call: each `:memory:`
+// open is its own DB by SQLite design, so caching it would produce
+// surprising cross-store sharing in tests.
+const _connections = new Map(); // dbPath → { db, refs }
+
+function acquireConnection(dbPath) {
+  if (dbPath === ':memory:') {
+    const db = new Database(':memory:');
+    db.pragma('journal_mode = WAL');
+    return { db, release: () => db.close() };
+  }
+  let entry = _connections.get(dbPath);
+  if (!entry) {
+    const db = new Database(dbPath);
+    db.pragma('journal_mode = WAL');
+    entry = { db, refs: 0 };
+    _connections.set(dbPath, entry);
+  }
+  entry.refs += 1;
+  return {
+    db: entry.db,
+    release: () => {
+      entry.refs -= 1;
+      if (entry.refs <= 0) {
+        _connections.delete(dbPath);
+        entry.db.close();
+      }
+    }
+  };
+}
+
 /**
  * Initialise (or attach to) a SQLite-backed KV table. Tables are namespaced
- * by `name` so multiple stores can share one database file.
+ * by `name` so multiple stores can share one database file. Multiple stores
+ * against the same file path share one underlying SQLite connection (see
+ * Bug #5 — concurrent writes from two connections produce SQLITE_BUSY).
  *
- * @param {string} dbPath - SQLite file path
+ * @param {string} dbPath - SQLite file path (or ':memory:' for ephemeral)
  * @param {string} name - logical table name (alphanumeric / underscores)
  * @param {object} [opts]
  * @param {number} [opts.ttlMs] - default TTL applied when not provided per-write.
@@ -26,8 +65,7 @@ export function initKVStore(dbPath, name, opts = {}) {
   }
   const ttlMs = opts.ttlMs;
 
-  const db = new Database(dbPath);
-  db.pragma('journal_mode = WAL');
+  const { db, release } = acquireConnection(dbPath);
   db.exec(`
     CREATE TABLE IF NOT EXISTS kv_${name} (
       key       TEXT PRIMARY KEY,
@@ -74,7 +112,7 @@ export function initKVStore(dbPath, name, opts = {}) {
   }
 
   function close() {
-    db.close();
+    release();
   }
 
   return { has, get, set, sweep, close, _db: db };