From 6ffef3ab9940710f1e4977ef2cae94a3fdb920c8 Mon Sep 17 00:00:00 2001
From: Gergo Magyar <gergomagyar@icloud.com>
Date: Fri, 8 May 2026 11:30:36 +0100
Subject: [PATCH] chore(lbug): isDbBusyError review fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Drop redundant `could not set lock` term — already subsumed by `lock`.
- Document the intentionally-broad matcher: graph-DB lock-shaped errors
  ("deadlock", "unlock failed", "lock contention", "could not open lock
  file") are all treated as transient. If a non-transient surfaces,
  tighten the matcher rather than raise the retry budget.
- Add positive test cases covering those lock-shaped strings so the
  intent is visible and a future tightening would deliberately break
  these.
- Fix the open-retry back-off comment: max sleep is 100+200+300+400 =
  1000ms (no sleep after the final attempt), not 1.5s.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 gitnexus/src/core/lbug/lbug-config.ts          | 18 ++++++++++--------
 .../test/integration/lbug-lock-retry.test.ts   | 12 ++++++++++++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/gitnexus/src/core/lbug/lbug-config.ts b/gitnexus/src/core/lbug/lbug-config.ts
index 2f5f7d0b1..ceb445693 100644
--- a/gitnexus/src/core/lbug/lbug-config.ts
+++ b/gitnexus/src/core/lbug/lbug-config.ts
@@ -82,12 +82,13 @@ export interface LbugConnectionHandle {
  */
 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')
-  );
+  // `lock` already subsumes `could not set lock`; the broader term is kept
+  // because graph-DB transient errors include "deadlock", "lock contention",
+  // and the LadybugDB native module's "could not set lock on file" — all of
+  // which deserve a retry. If a non-transient lock-shaped error ever
+  // surfaces (e.g., "lock file missing" during recovery), tighten this
+  // matcher rather than raising the retry budget.
+  return msg.includes('busy') || msg.includes('lock') || msg.includes('already in use');
 };
 
 export function createLbugDatabase(
@@ -126,8 +127,9 @@ export function createLbugDatabase(
 // OS-level exclusive lock on `<dbPath>`. On Windows that lock can fail
 // for reasons specific to the OS (Defender briefly opens new files,
 // libuv handle release lags the JS-side close). 5 attempts × 100ms
-// linear back-off (~1.5s worst case) clears the typical AV-scanner hold
-// without masking real cross-process conflicts.
+// linear back-off (max sleep 100+200+300+400 = 1s, plus 5 ctor RTTs
+// of 10–50ms each = ~1.0–1.2s worst case) clears the typical
+// AV-scanner hold without masking real cross-process conflicts.
 //
 // Source: https://github.com/LadybugDB/ladybug/blob/v0.16.1/src/common/file_system/local_file_system.cpp#L126
 const OPEN_LOCK_RETRY_ATTEMPTS = 5;
diff --git a/gitnexus/test/integration/lbug-lock-retry.test.ts b/gitnexus/test/integration/lbug-lock-retry.test.ts
index 7724e5512..b95092e2d 100644
--- a/gitnexus/test/integration/lbug-lock-retry.test.ts
+++ b/gitnexus/test/integration/lbug-lock-retry.test.ts
@@ -46,6 +46,18 @@ describe('isDbBusyError', () => {
     expect(isDbBusyError(undefined)).toBe(false);
   });
 
+  // Documented behavior for lock-shaped strings: the matcher is intentionally
+  // broad because in graph-DB contexts these are all transient. If LadybugDB
+  // ever surfaces a non-transient lock-shaped error (e.g., a recovery-time
+  // "lock file missing"), tighten the matcher and add a negative test here
+  // rather than raising the retry budget.
+  it('treats other lock-shaped errors as transient (current intentional behavior)', () => {
+    expect(isDbBusyError(new Error('deadlock detected'))).toBe(true);
+    expect(isDbBusyError(new Error('unlock failed'))).toBe(true);
+    expect(isDbBusyError(new Error('lock contention'))).toBe(true);
+    expect(isDbBusyError(new Error('Could not open lock file'))).toBe(true);
+  });
+
   it('handles non-Error values gracefully', () => {
     expect(isDbBusyError('BUSY error')).toBe(true);
     expect(isDbBusyError(42)).toBe(false);