From f0cb368580ef2199e0b75668a32617f9f5539bfc Mon Sep 17 00:00:00 2001
From: Gergo Magyar <gergomagyar@icloud.com>
Date: Sun, 21 Jun 2026 06:10:49 +0000
Subject: [PATCH] fix(lbug): make the conn-lock non-reentrancy invariant
 enforced, not just documented (#2264)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A future withConnLock-wrapped helper calling another wrapped helper would await
its own holder's tail and hang silently. Add an AsyncLocalStorage-based re-entry
guard: withConnLock throws a clear error when invoked from within a holding fn's
async context. A boolean flag can't do this — a legitimately-queued top-level
caller also runs while the lock is held; only AsyncLocalStorage distinguishes a
true nested call from normal contention.

Tests: re-entry throws (not deadlocks); sequential and concurrent top-level calls
do NOT false-fire; the lock releases after a re-entry throw.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm
---
 gitnexus/src/core/lbug/conn-lock.ts  | 24 ++++++++++++++++---
 gitnexus/test/unit/conn-lock.test.ts | 36 ++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/gitnexus/src/core/lbug/conn-lock.ts b/gitnexus/src/core/lbug/conn-lock.ts
index bb10d8113..3a92bb8cc 100644
--- a/gitnexus/src/core/lbug/conn-lock.ts
+++ b/gitnexus/src/core/lbug/conn-lock.ts
@@ -17,12 +17,30 @@
  *
  * Implementation: a promise chain. Each caller installs a fresh unresolved tail,
  * awaits the previous holder's tail, runs, then releases its own in `finally`
- * (so a thrown op never wedges the connection). FIFO and re-entrancy-unsafe by
- * design — a wrapped helper MUST NOT call another wrapped helper.
+ * (so a thrown op never wedges the connection). FIFO and non-reentrant: a wrapped
+ * helper MUST NOT call another wrapped helper — the inner call would await its own
+ * holder's tail and deadlock. The re-entry guard below catches this and throws
+ * instead of hanging. A boolean flag can't do this: a legitimately-queued
+ * top-level caller also runs while the lock is held, so only AsyncLocalStorage —
+ * which marks the *async context* of the running `fn` — distinguishes a true
+ * nested call from normal contention.
  */
+import { AsyncLocalStorage } from 'node:async_hooks';
+
 let tail: Promise<void> = Promise.resolve();
 
+// Set (to `true`) only inside a holding `fn`'s async context. A withConnLock call
+// that observes it set is a nested/re-entrant call from within a critical section.
+const inCriticalSection = new AsyncLocalStorage<true>();
+
 export const withConnLock = async <T>(fn: () => Promise<T>): Promise<T> => {
+  if (inCriticalSection.getStore()) {
+    throw new Error(
+      'conn-lock re-entry: a withConnLock-wrapped helper called another wrapped ' +
+        'helper, which would deadlock the single LadybugDB connection. Run the inner ' +
+        'work outside the lock, or inline it. See src/core/lbug/conn-lock.ts.',
+    );
+  }
   const prior = tail;
   let release!: () => void;
   tail = new Promise<void>((resolve) => {
@@ -30,7 +48,7 @@ export const withConnLock = async <T>(fn: () => Promise<T>): Promise<T> => {
   });
   await prior;
   try {
-    return await fn();
+    return await inCriticalSection.run(true, fn);
   } finally {
     release();
   }
diff --git a/gitnexus/test/unit/conn-lock.test.ts b/gitnexus/test/unit/conn-lock.test.ts
index cbce6ac0c..699160b94 100644
--- a/gitnexus/test/unit/conn-lock.test.ts
+++ b/gitnexus/test/unit/conn-lock.test.ts
@@ -71,3 +71,39 @@ describe('withConnLock — connection serialization', () => {
     await expect(withConnLock(async () => 42)).resolves.toBe(42);
   });
 });
+
+describe('withConnLock — re-entry guard', () => {
+  it('throws on a nested (wrapped-in-wrapped) call instead of deadlocking', async () => {
+    await expect(withConnLock(async () => withConnLock(async () => 'inner'))).rejects.toThrow(
+      /re-entry/,
+    );
+  });
+
+  it('does NOT false-fire on sequential (non-nested) calls', async () => {
+    // Mirrors getLbugStats: many withConnLock calls in a loop, each awaited to
+    // completion before the next — distinct async contexts, never nested.
+    const results: number[] = [];
+    for (let i = 0; i < 5; i++) {
+      results.push(await withConnLock(async () => i));
+    }
+    expect(results).toEqual([0, 1, 2, 3, 4]);
+  });
+
+  it('does NOT false-fire on concurrent top-level (queued) callers', async () => {
+    // Legitimate contention: B and C call while A holds the lock. They are
+    // separate async contexts (not nested in A's fn), so they queue, not throw.
+    const out = await Promise.all([
+      withConnLock(async () => 'a'),
+      withConnLock(async () => 'b'),
+      withConnLock(async () => 'c'),
+    ]);
+    expect(out).toEqual(['a', 'b', 'c']);
+  });
+
+  it('releases the lock after a re-entry throw so later callers proceed', async () => {
+    await expect(withConnLock(async () => withConnLock(async () => 'inner'))).rejects.toThrow(
+      /re-entry/,
+    );
+    await expect(withConnLock(async () => 'ok')).resolves.toBe('ok');
+  });
+});