Merge pull request #425 from ShunsukeHayashi/fix/db-lock-325

gitnexus/src/core/lbug/lbug-adapter.ts

@@ -25,6 +25,26 @@ export const getDatabase = (): lbug.Database | null => db;
 // This guarantees no DB switch can happen while an operation is running.
 let sessionLock: Promise<void> = Promise.resolve();
 
+/** Number of times to retry on a BUSY / lock-held error before giving up. */
+const DB_LOCK_RETRY_ATTEMPTS = 3;
+/** Base back-off in ms between BUSY retries (multiplied by attempt number). */
+const DB_LOCK_RETRY_DELAY_MS = 500;
+
+/**
+ * Return true when the error message indicates that another process holds
+ * an exclusive lock on the LadybugDB file (e.g. `gitnexus analyze` or
+ * `gitnexus serve` running at the same time).
+ */
+export const isDbBusyError = (err: unknown): boolean => {
+  const msg = (err instanceof Error ? err.message : String(err)).toLowerCase();
+  return (
+    msg.includes('busy')
+    || msg.includes('lock')
+    || msg.includes('already in use')
+    || msg.includes('could not set lock')
+  );
+};
+
 const runWithSessionLock = async <T>(operation: () => Promise<T>): Promise<T> => {
   const previous = sessionLock;
   let release: (() => void) | null = null;
@@ -49,12 +69,41 @@ export const initLbug = async (dbPath: string) => {
 /**
  * Execute multiple queries against one repo DB atomically.
  * While the callback runs, no other request can switch the active DB.
+ *
+ * Automatically retries up to DB_LOCK_RETRY_ATTEMPTS times when the
+ * database is busy (e.g. `gitnexus analyze` holds the write lock).
+ * Each retry waits DB_LOCK_RETRY_DELAY_MS * attempt milliseconds.
  */
 export const withLbugDb = async <T>(dbPath: string, operation: () => Promise<T>): Promise<T> => {
-  return runWithSessionLock(async () => {
-    await ensureLbugInitialized(dbPath);
-    return operation();
-  });
+  let lastError: unknown;
+  for (let attempt = 1; attempt <= DB_LOCK_RETRY_ATTEMPTS; attempt++) {
+    try {
+      return await runWithSessionLock(async () => {
+        await ensureLbugInitialized(dbPath);
+        return operation();
+      });
+    } catch (err) {
+      lastError = err;
+      if (!isDbBusyError(err) || attempt === DB_LOCK_RETRY_ATTEMPTS) {
+        throw err;
+      }
+      // Close stale connection inside the session lock to prevent race conditions
+      // with concurrent operations that might acquire the lock between cleanup steps
+      await runWithSessionLock(async () => {
+        try { if (conn) await conn.close(); } catch { /* best-effort */ }
+        try { if (db) await db.close(); } catch { /* best-effort */ }
+        conn = null;
+        db = null;
+        currentDbPath = null;
+        ftsLoaded = false;
+      });
+      // Sleep outside the lock — no need to block others while waiting
+      await new Promise(resolve => setTimeout(resolve, DB_LOCK_RETRY_DELAY_MS * attempt));
+    }
+  }
+  // This line is unreachable — the loop either returns or throws inside,
+  // but TypeScript needs an explicit throw to satisfy the return type.
+  throw lastError;
 };
 
 const ensureLbugInitialized = async (dbPath: string) => {

gitnexus/test/integration/lbug-lock-retry.test.ts

@@ -0,0 +1,106 @@
+/**
+ * Integration Tests: lbug-adapter busy/lock retry logic
+ *
+ * Tests isDbBusyError() detection and withLbugDb() retry behaviour
+ * using the real LadybugDB via withTestLbugDB lifecycle.
+ *
+ * Follows existing lbug integration test patterns (lbug-core-adapter,
+ * lbug-pool, lbug-pool-stability).
+ */
+import { describe, it, expect } from 'vitest';
+import { withTestLbugDB } from '../helpers/test-indexed-db.js';
+
+// ─── isDbBusyError ────────────────────────────────────────────────────────
+
+// Pure-function tests — no DB needed, but grouped here for cohesion
+// with the retry logic they guard.
+import { isDbBusyError } from '../../src/core/lbug/lbug-adapter.js';
+
+describe('isDbBusyError', () => {
+  it('returns true for "busy" errors (case-insensitive)', () => {
+    expect(isDbBusyError(new Error('Database is BUSY'))).toBe(true);
+    expect(isDbBusyError(new Error('busy'))).toBe(true);
+    expect(isDbBusyError('resource busy')).toBe(true);
+  });
+
+  it('returns true for "lock" errors', () => {
+    expect(isDbBusyError(new Error('Could not set lock on file'))).toBe(true);
+    expect(isDbBusyError(new Error('database is locked'))).toBe(true);
+    expect(isDbBusyError(new Error('LOCK'))).toBe(true);
+  });
+
+  it('returns true for "already in use" errors', () => {
+    expect(isDbBusyError(new Error('file already in use by another process'))).toBe(true);
+    expect(isDbBusyError('already in use')).toBe(true);
+  });
+
+  it('returns true for "could not set lock" errors', () => {
+    expect(isDbBusyError(new Error('Could not set lock on the database file'))).toBe(true);
+  });
+
+  it('returns false for unrelated errors', () => {
+    expect(isDbBusyError(new Error('Table not found'))).toBe(false);
+    expect(isDbBusyError(new Error('Connection refused'))).toBe(false);
+    expect(isDbBusyError(new Error('Syntax error in Cypher query'))).toBe(false);
+    expect(isDbBusyError(null)).toBe(false);
+    expect(isDbBusyError(undefined)).toBe(false);
+  });
+
+  it('handles non-Error values gracefully', () => {
+    expect(isDbBusyError('BUSY error')).toBe(true);
+    expect(isDbBusyError(42)).toBe(false);
+    expect(isDbBusyError({ message: 'locked' })).toBe(false); // plain object not Error
+  });
+});
+
+// ─── withLbugDb retry integration tests ───────────────────────────────────
+
+withTestLbugDB('lock-retry', (handle) => {
+  describe('withLbugDb retry behaviour', () => {
+    it('returns the operation result on success', async () => {
+      const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
+      const result = await withLbugDb(handle.dbPath, async () => 'ok');
+      expect(result).toBe('ok');
+    });
+
+    it('retries on BUSY error and succeeds on later attempt', async () => {
+      const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
+      let callCount = 0;
+      const result = await withLbugDb(handle.dbPath, async () => {
+        callCount++;
+        if (callCount === 1) throw new Error('database is BUSY');
+        return 'recovered';
+      });
+
+      expect(result).toBe('recovered');
+      expect(callCount).toBe(2);
+    });
+
+    it('propagates non-BUSY errors immediately without retrying', async () => {
+      const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
+      let callCount = 0;
+      await expect(
+        withLbugDb(handle.dbPath, async () => {
+          callCount++;
+          throw new Error('Syntax error in Cypher');
+        }),
+      ).rejects.toThrow('Syntax error in Cypher');
+
+      expect(callCount).toBe(1); // no retry for non-BUSY errors
+    });
+
+    it('throws after max retry attempts', async () => {
+      const { withLbugDb } = await import('../../src/core/lbug/lbug-adapter.js');
+      let callCount = 0;
+      await expect(
+        withLbugDb(handle.dbPath, async () => {
+          callCount++;
+          throw new Error('Could not set lock');
+        }),
+      ).rejects.toThrow('Could not set lock');
+
+      // DB_LOCK_RETRY_ATTEMPTS = 3 (default in the implementation)
+      expect(callCount).toBe(3);
+    });
+  });
+});

gitnexus/vitest.config.ts

@@ -60,6 +60,7 @@ export default defineConfig({
             'test/integration/search-pool.test.ts',
             'test/integration/augmentation.test.ts',
             'test/integration/staleness-and-stability.test.ts',
+            'test/integration/lbug-lock-retry.test.ts',
           ],
           fileParallelism: false,
           sequence: { groupOrder: 1 },
@@ -81,6 +82,8 @@ export default defineConfig({
             'test/integration/search-pool.test.ts',
             'test/integration/augmentation.test.ts',
             'test/integration/staleness-and-stability.test.ts',
+            'test/integration/lbug-lock-retry.test.ts',
+            'test/integration/lbug-lock-retry.test.ts',
           ],
         },
       },