mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-07 02:58:02 +00:00
Merge branch 'main' into feat/group-include-extractor
This commit is contained in:
commit
28a3d99a92
7 changed files with 653 additions and 19 deletions
|
|
@ -19,7 +19,10 @@ import type { CachedEmbedding } from '../embeddings/types.js';
|
|||
import { extensionManager, type ExtensionEnsureOptions } from './extension-loader.js';
|
||||
import {
|
||||
closeLbugConnection,
|
||||
isDbBusyError,
|
||||
isOpenRetryExhausted,
|
||||
openLbugConnection,
|
||||
waitForWindowsHandleRelease,
|
||||
type LbugConnectionHandle,
|
||||
} from './lbug-config.js';
|
||||
import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js';
|
||||
|
|
@ -185,21 +188,6 @@ 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')
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* Return true when the error message indicates a write was attempted against
|
||||
* a read-only LadybugDB connection. The MCP query pool opens DBs read-only,
|
||||
|
|
@ -252,7 +240,11 @@ export const withLbugDb = async <T>(dbPath: string, operation: () => Promise<T>)
|
|||
});
|
||||
} catch (err) {
|
||||
lastError = err;
|
||||
if (!isDbBusyError(err) || attempt === DB_LOCK_RETRY_ATTEMPTS) {
|
||||
// Skip outer retry when the inner open-retry already exhausted: the
|
||||
// ~1.5s open-time budget was just spent, repeating the full reset+
|
||||
// reopen cycle would only add 4-5s of tail latency without changing
|
||||
// the outcome (both layers consult the same isDbBusyError matcher).
|
||||
if (!isDbBusyError(err) || isOpenRetryExhausted(err) || attempt === DB_LOCK_RETRY_ATTEMPTS) {
|
||||
throw err;
|
||||
}
|
||||
// Close stale connection inside the session lock to prevent race conditions
|
||||
|
|
@ -330,7 +322,16 @@ const doInitLbug = async (dbPath: string) => {
|
|||
await conn.query(schemaQuery);
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
if (!msg.includes('already exists')) {
|
||||
// Suppression list:
|
||||
// - "already exists": expected idempotent re-create on existing DBs
|
||||
// - "could not set lock on file": LadybugDB v0.16.1 emits this on
|
||||
// Windows when CREATE NODE TABLE runs against a path that was
|
||||
// just opened (the WAL handle from a fresh Database briefly
|
||||
// contests the table's first-write lock). The table is created
|
||||
// anyway and any genuine cross-process lock contention surfaces
|
||||
// on the next operation via withLbugDb's retry. Logging it here
|
||||
// would just be noise in CI.
|
||||
if (!msg.includes('already exists') && !isDbBusyError(err)) {
|
||||
logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`);
|
||||
}
|
||||
}
|
||||
|
|
@ -1064,6 +1065,9 @@ export const flushWAL = async (): Promise<void> => {
|
|||
*/
|
||||
export const safeClose = async (): Promise<void> => {
|
||||
await flushWAL();
|
||||
// Capture before close — currentDbPath stays set so the Windows post-close
|
||||
// probe below knows which file to wait on.
|
||||
const closingDbPath = currentDbPath;
|
||||
if (conn) {
|
||||
try {
|
||||
// eslint-disable-next-line no-restricted-syntax -- sole authorised close site
|
||||
|
|
@ -1082,6 +1086,24 @@ export const safeClose = async (): Promise<void> => {
|
|||
}
|
||||
db = null;
|
||||
}
|
||||
// Windows: libuv reports `db.close()` resolved before the kernel has
|
||||
// released the file handle. A subsequent `new Database(samePath)` in
|
||||
// the same process can race the release. The probe (lbug-config.ts)
|
||||
// forces any residual lock to surface as EBUSY/EPERM/EACCES so the
|
||||
// open-time retry absorbs the lag.
|
||||
if (process.platform === 'win32' && closingDbPath) {
|
||||
const released = await waitForWindowsHandleRelease(closingDbPath);
|
||||
if (!released) {
|
||||
// Probe exhausted with a lock code still in flight. The next
|
||||
// openLbugConnection will absorb whatever residual lag remains, but
|
||||
// a chronic warning helps operators spot AV interference (Windows
|
||||
// Defender holding the file far past the 250ms budget).
|
||||
logger.warn(
|
||||
{ dbPath: closingDbPath },
|
||||
'⚠️ LadybugDB file handle still locked after close (Windows). If this repeats, check antivirus/Defender exclusions for the GitNexus storage directory.',
|
||||
);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
export const closeLbug = async (): Promise<void> => {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,6 @@
|
|||
import fs from 'fs/promises';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import type lbug from '@ladybugdb/core';
|
||||
|
||||
/**
|
||||
|
|
@ -66,6 +69,28 @@ export interface LbugConnectionHandle {
|
|||
conn: lbug.Connection;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when the error message indicates that a LadybugDB file lock
|
||||
* could not be acquired — either at construction time
|
||||
* (`new lbug.Database(...)` raises from `local_file_system.cpp`) or during
|
||||
* a query (another writer holds the exclusive lock).
|
||||
*
|
||||
* Lives here (not in `lbug-adapter.ts`) so both the construction-time
|
||||
* retry (`openWithLockRetry` in this file) and the query-time retry
|
||||
* (`withLbugDb` in `lbug-adapter.ts`) consult the same matcher. Callers
|
||||
* import directly from this module — no re-export to keep in sync.
|
||||
*/
|
||||
export const isDbBusyError = (err: unknown): boolean => {
|
||||
const msg = (err instanceof Error ? err.message : String(err)).toLowerCase();
|
||||
// `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(
|
||||
lbugModule: LbugModule,
|
||||
databasePath: string,
|
||||
|
|
@ -85,6 +110,159 @@ export function createLbugDatabase(
|
|||
) as lbug.Database;
|
||||
}
|
||||
|
||||
// ─── Lock-busy retry tuning knobs ───────────────────────────────────────────
|
||||
//
|
||||
// All four GitNexus retry pairs that touch native LadybugDB locks live with
|
||||
// a comment cross-reference here so an SRE tuning Windows flakes finds them
|
||||
// in one grep:
|
||||
//
|
||||
// 1. OPEN_LOCK_RETRY_ATTEMPTS / OPEN_LOCK_RETRY_DELAY_MS (this file)
|
||||
// → `new lbug.Database()` constructor lock failures
|
||||
// 2. HANDLE_RELEASE_PROBE_ATTEMPTS / HANDLE_RELEASE_PROBE_DELAY_MS (this file)
|
||||
// → post-close fs.open probe to absorb Windows handle-release lag
|
||||
// 3. DB_LOCK_RETRY_ATTEMPTS / DB_LOCK_RETRY_DELAY_MS (lbug-adapter.ts withLbugDb)
|
||||
// → query-time busy/lock retry around already-open connections
|
||||
//
|
||||
// `new lbug.Database()` calls into the native module which performs an
|
||||
// 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 (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;
|
||||
const OPEN_LOCK_RETRY_DELAY_MS = 100;
|
||||
|
||||
const HANDLE_RELEASE_PROBE_ATTEMPTS = 5;
|
||||
const HANDLE_RELEASE_PROBE_DELAY_MS = 50;
|
||||
const HANDLE_RELEASE_LOCK_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']);
|
||||
|
||||
/**
|
||||
* Test-fixture directory prefixes recognized by `isTestFixturePath`.
|
||||
*
|
||||
* IMPORTANT: this list must stay in sync with the prefixes passed to
|
||||
* `createTempDir` in `gitnexus/test/helpers/test-db.ts` and the prefixes
|
||||
* used by `withTestLbugDB` (`gitnexus/test/helpers/test-indexed-db.ts`).
|
||||
* If you add a new test that passes a custom prefix to `createTempDir`,
|
||||
* add it here too — otherwise the stale-sidecar sweep silently won't
|
||||
* fire for that fixture and CI flakes return.
|
||||
*
|
||||
* The default `createTempDir('gitnexus-test-')` and the lbug variant
|
||||
* `'gitnexus-lbug-'` cover today's call sites.
|
||||
*/
|
||||
const TEST_FIXTURE_PREFIXES = ['gitnexus-lbug-', 'gitnexus-test-'];
|
||||
|
||||
/**
|
||||
* Marker symbol attached to lock errors after `openWithLockRetry` exhausts
|
||||
* its budget. `withLbugDb`'s outer query-time retry consults this so it
|
||||
* does not re-retry a path that just spent up to ~1.5s in the open-time
|
||||
* loop — preventing 6s tail latencies (3× outer × 5× inner attempts).
|
||||
*
|
||||
* The symbol is internal to GitNexus; consumers should treat the underlying
|
||||
* error message as the user-visible signal.
|
||||
*/
|
||||
export const LBUG_OPEN_RETRY_EXHAUSTED = Symbol.for('gitnexus.lbug.openRetryExhausted');
|
||||
|
||||
export const isOpenRetryExhausted = (err: unknown): boolean => {
|
||||
if (err === null || err === undefined || typeof err !== 'object') return false;
|
||||
return (err as { [LBUG_OPEN_RETRY_EXHAUSTED]?: boolean })[LBUG_OPEN_RETRY_EXHAUSTED] === true;
|
||||
};
|
||||
|
||||
const tagOpenRetryExhausted = (err: unknown): unknown => {
|
||||
if (err && typeof err === 'object') {
|
||||
(err as { [LBUG_OPEN_RETRY_EXHAUSTED]?: boolean })[LBUG_OPEN_RETRY_EXHAUSTED] = true;
|
||||
}
|
||||
return err;
|
||||
};
|
||||
|
||||
/**
|
||||
* True when `dbPath` resolves to a recognized test fixture under the OS
|
||||
* temp directory. Used to gate the stale-sidecar sweep so production
|
||||
* paths never have their `.wal` / `.lock` files deleted.
|
||||
*
|
||||
* Defensive shape:
|
||||
* - `path.resolve` normalizes `..` segments before the prefix check, so
|
||||
* `<tmp>/gitnexus-lbug-x/../../etc/passwd` is rejected.
|
||||
* - The tmpRoot check trims any trailing separator returned by some
|
||||
* Windows TMP configurations (`C:\Users\X\Temp\`) so the startsWith
|
||||
* comparison stays correct.
|
||||
* - Only the IMMEDIATE parent directory is matched against the prefix
|
||||
* list. An ancestor walk would let a tmpdir whose own basename starts
|
||||
* with `gitnexus-lbug-` accept arbitrary nested paths under it.
|
||||
*/
|
||||
const isTestFixturePath = (dbPath: string): boolean => {
|
||||
const tmpRoot = os.tmpdir().replace(new RegExp(`${path.sep === '\\' ? '\\\\' : path.sep}+$`), '');
|
||||
const resolved = path.resolve(dbPath);
|
||||
if (!resolved.startsWith(tmpRoot + path.sep) && resolved !== tmpRoot) return false;
|
||||
const parentBase = path.basename(path.dirname(resolved));
|
||||
return TEST_FIXTURE_PREFIXES.some((p) => parentBase.startsWith(p));
|
||||
};
|
||||
|
||||
/** Exported only for direct unit testing — production callers use `openWithLockRetry`. */
|
||||
export const _isTestFixturePathForTest = isTestFixturePath;
|
||||
|
||||
const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
|
||||
|
||||
/**
|
||||
* Attempt to remove stale `.wal` / `.lock` sidecars that a previous aborted
|
||||
* test run may have left behind. Best-effort: ENOENT is normal, anything
|
||||
* else is swallowed so the caller's retry can surface the original error.
|
||||
*/
|
||||
const sweepStaleSidecars = async (dbPath: string): Promise<void> => {
|
||||
for (const suffix of ['.wal', '.lock']) {
|
||||
try {
|
||||
await fs.unlink(dbPath + suffix);
|
||||
} catch {
|
||||
/* missing sidecar or permission error — let the open retry surface it */
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Run `construct` with bounded retries when `new lbug.Database(...)` throws
|
||||
* a busy/lock error. The original (loop-captured) error is preferred over
|
||||
* any post-sweep error so triage sees the real LadybugDB lock message.
|
||||
* On exhaustion the rethrown error is tagged via
|
||||
* `LBUG_OPEN_RETRY_EXHAUSTED` so the outer query-time retry in
|
||||
* `withLbugDb` skips re-retrying a freshly-exhausted path.
|
||||
*/
|
||||
const openWithLockRetry = async (
|
||||
construct: () => lbug.Database,
|
||||
dbPath: string,
|
||||
): Promise<lbug.Database> => {
|
||||
let originalLockError: unknown;
|
||||
for (let attempt = 1; attempt <= OPEN_LOCK_RETRY_ATTEMPTS; attempt++) {
|
||||
try {
|
||||
return construct();
|
||||
} catch (err) {
|
||||
if (!isDbBusyError(err)) throw err;
|
||||
originalLockError = err;
|
||||
if (attempt === OPEN_LOCK_RETRY_ATTEMPTS) break;
|
||||
await sleep(OPEN_LOCK_RETRY_DELAY_MS * attempt);
|
||||
}
|
||||
}
|
||||
|
||||
// Final defense: only for recognized test fixtures, sweep stale sidecars
|
||||
// (a prior aborted test run can leave a `.wal` lock that survives the
|
||||
// tmp dir cleanup). Production paths never reach this branch — the guard
|
||||
// requires the immediate parent dir to match a test prefix AND the
|
||||
// resolved path to live under the OS temp directory.
|
||||
if (isTestFixturePath(dbPath)) {
|
||||
await sweepStaleSidecars(dbPath);
|
||||
try {
|
||||
return construct();
|
||||
} catch {
|
||||
// Intentionally do NOT overwrite originalLockError. The user-actionable
|
||||
// signal is "we exhausted lock retries" — a different error from the
|
||||
// post-sweep attempt is less useful than the lock failure that drove
|
||||
// the sweep in the first place.
|
||||
}
|
||||
}
|
||||
throw tagOpenRetryExhausted(originalLockError);
|
||||
};
|
||||
|
||||
export async function openLbugConnection(
|
||||
lbugModule: LbugModule,
|
||||
databasePath: string,
|
||||
|
|
@ -92,7 +270,10 @@ export async function openLbugConnection(
|
|||
): Promise<LbugConnectionHandle> {
|
||||
let db: lbug.Database | undefined;
|
||||
try {
|
||||
db = createLbugDatabase(lbugModule, databasePath, options);
|
||||
db = await openWithLockRetry(
|
||||
() => createLbugDatabase(lbugModule, databasePath, options),
|
||||
databasePath,
|
||||
);
|
||||
return { db, conn: new lbugModule.Connection(db) };
|
||||
} catch (err) {
|
||||
if (db) await db.close().catch(() => {});
|
||||
|
|
@ -104,3 +285,60 @@ export async function closeLbugConnection(handle: LbugConnectionHandle): Promise
|
|||
await handle.conn.close().catch(() => {});
|
||||
await handle.db.close().catch(() => {});
|
||||
}
|
||||
|
||||
/**
|
||||
* Probe `dbPath` AND its `.wal` sidecar after `db.close()` so any
|
||||
* residual native file handle surfaces as EBUSY/EPERM/EACCES and the
|
||||
* bounded retry absorbs the release lag. Windows-only — Linux/macOS do
|
||||
* not exhibit this race.
|
||||
*
|
||||
* Both files matter. Empirically, on rapid open→close→reopen cycles the
|
||||
* main `dbPath` handle releases first; the `.wal` handle from the
|
||||
* previous Database lingers and the new Database's first write (CREATE
|
||||
* NODE TABLE during schema init) fails with "Could not set lock on
|
||||
* file". Probing both makes safeClose actually return when the kernel
|
||||
* is fully done with the path.
|
||||
*
|
||||
* Returns `true` when both probes succeeded (or skipped on non-lock
|
||||
* errors / missing files). Returns `false` when either probe exhausted
|
||||
* its budget with a lock code still in flight.
|
||||
*
|
||||
* Defensive shape:
|
||||
* - Opens read+write (`'r+'`) so the probe actually surfaces exclusive
|
||||
* locks held by the previous Database. A read-only probe (`'r'`) is
|
||||
* insufficient — Windows will grant read access while the previous
|
||||
* handle's exclusive write lock is still in flight, which lets
|
||||
* `safeClose` return before the next CREATE NODE TABLE can lock the
|
||||
* file.
|
||||
* - `try/finally` around `handle.close()` guarantees no fd leak even
|
||||
* if close itself throws.
|
||||
*/
|
||||
export const waitForWindowsHandleRelease = async (dbPath: string): Promise<boolean> => {
|
||||
const mainReleased = await probeSinglePath(dbPath);
|
||||
const walReleased = await probeSinglePath(dbPath + '.wal');
|
||||
return mainReleased && walReleased;
|
||||
};
|
||||
|
||||
const probeSinglePath = async (filePath: string): Promise<boolean> => {
|
||||
for (let attempt = 1; attempt <= HANDLE_RELEASE_PROBE_ATTEMPTS; attempt++) {
|
||||
let handle: fs.FileHandle | undefined;
|
||||
try {
|
||||
handle = await fs.open(filePath, 'r+');
|
||||
return true;
|
||||
} catch (err) {
|
||||
const code = (err as NodeJS.ErrnoException | undefined)?.code;
|
||||
if (!code || !HANDLE_RELEASE_LOCK_CODES.has(code)) return true; // ENOENT / unrelated → not our problem
|
||||
if (attempt === HANDLE_RELEASE_PROBE_ATTEMPTS) return false;
|
||||
await sleep(HANDLE_RELEASE_PROBE_DELAY_MS * attempt);
|
||||
} finally {
|
||||
if (handle) {
|
||||
try {
|
||||
await handle.close();
|
||||
} catch {
|
||||
/* swallow — caller cannot do anything useful with a probe-close failure */
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -37,6 +37,13 @@ export async function cleanupTempDir(tmpDir: string): Promise<void> {
|
|||
/**
|
||||
* Create a temporary directory for LadybugDB tests.
|
||||
* Returns the path and a cleanup function.
|
||||
*
|
||||
* IMPORTANT: when adding a new test that passes a custom `prefix`, also add
|
||||
* the prefix to `TEST_FIXTURE_PREFIXES` in
|
||||
* `gitnexus/src/core/lbug/lbug-config.ts`. The stale-sidecar sweep relies
|
||||
* on the prefix list to recognize test fixtures; an unknown prefix means
|
||||
* the sweep silently won't fire for that fixture and Windows CI flakes
|
||||
* return.
|
||||
*/
|
||||
export async function createTempDir(prefix: string = 'gitnexus-test-'): Promise<TestDBHandle> {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), prefix));
|
||||
|
|
|
|||
41
gitnexus/test/integration/lbug-close-handle-release.test.ts
Normal file
41
gitnexus/test/integration/lbug-close-handle-release.test.ts
Normal file
|
|
@ -0,0 +1,41 @@
|
|||
/**
|
||||
* Integration test: safeClose's Windows post-close handle-release wait.
|
||||
*
|
||||
* On Windows, libuv reports `db.close()` resolved before the kernel has
|
||||
* released the file handle. A subsequent open of the same path can then
|
||||
* race the release and surface "Could not set lock on file". `safeClose`
|
||||
* probes the file with `fs.open` to force the residual lock to surface,
|
||||
* absorbed by the open-time retry in `lbug-config.ts`.
|
||||
*/
|
||||
import path from 'path';
|
||||
import { describe, it } from 'vitest';
|
||||
import { createTempDir } from '../helpers/test-db.js';
|
||||
|
||||
describe('safeClose — close + reopen does not surface lock errors', () => {
|
||||
it('survives 10 sequential open/close/reopen cycles on the same path', async () => {
|
||||
const tmp = await createTempDir('gitnexus-lbug-close-cycle-');
|
||||
const dbPath = path.join(tmp.dbPath, 'lbug');
|
||||
try {
|
||||
const adapter = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
for (let i = 0; i < 10; i++) {
|
||||
await adapter.initLbug(dbPath);
|
||||
await adapter.closeLbug();
|
||||
}
|
||||
} finally {
|
||||
await tmp.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('safeClose is idempotent — calling twice in a row does not throw', async () => {
|
||||
const tmp = await createTempDir('gitnexus-lbug-idempotent-');
|
||||
const dbPath = path.join(tmp.dbPath, 'lbug');
|
||||
try {
|
||||
const adapter = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
await adapter.initLbug(dbPath);
|
||||
await adapter.closeLbug();
|
||||
await adapter.closeLbug();
|
||||
} finally {
|
||||
await tmp.cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -14,7 +14,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
|||
|
||||
// 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';
|
||||
import { isDbBusyError } from '../../src/core/lbug/lbug-config.js';
|
||||
|
||||
describe('isDbBusyError', () => {
|
||||
it('returns true for "busy" errors (case-insensitive)', () => {
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
310
gitnexus/test/integration/lbug-open-retry.test.ts
Normal file
310
gitnexus/test/integration/lbug-open-retry.test.ts
Normal file
|
|
@ -0,0 +1,310 @@
|
|||
/**
|
||||
* Integration tests: open-time lock-busy retry in `lbug-config.ts`.
|
||||
*
|
||||
* The lock IO exception raised by `local_file_system.cpp` happens
|
||||
* synchronously inside `new lbug.Database(...)`, before any query is
|
||||
* issued — so `withLbugDb`'s query-time retry cannot see it. These tests
|
||||
* exercise the construction-time retry wrapper directly by stubbing the
|
||||
* `Database` constructor.
|
||||
*
|
||||
* See: docs/plans/2026-05-08-002-fix-windows-lbug-lock-ci-flakes-plan.md
|
||||
*/
|
||||
import fs from 'fs/promises';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import {
|
||||
_isTestFixturePathForTest as isTestFixturePath,
|
||||
isDbBusyError,
|
||||
isOpenRetryExhausted,
|
||||
openLbugConnection,
|
||||
waitForWindowsHandleRelease,
|
||||
} from '../../src/core/lbug/lbug-config.js';
|
||||
|
||||
// ─── Minimal stub of the `lbug` module surface used by openLbugConnection ──
|
||||
|
||||
interface StubModuleControl {
|
||||
/** Errors thrown by sequential `new Database(...)` calls. `null` = success. */
|
||||
databaseThrows: Array<Error | null>;
|
||||
/** Number of times the `Database` constructor was invoked. */
|
||||
databaseCallCount: number;
|
||||
/** Number of times `db.close()` was called. */
|
||||
closeCallCount: number;
|
||||
}
|
||||
|
||||
const makeStubLbug = (control: StubModuleControl) => {
|
||||
class FakeDatabase {
|
||||
constructor(_path: string, ..._rest: unknown[]) {
|
||||
control.databaseCallCount++;
|
||||
const next = control.databaseThrows.shift();
|
||||
if (next instanceof Error) throw next;
|
||||
}
|
||||
async close(): Promise<void> {
|
||||
control.closeCallCount++;
|
||||
}
|
||||
}
|
||||
class FakeConnection {
|
||||
constructor(_db: FakeDatabase) {}
|
||||
async close(): Promise<void> {}
|
||||
}
|
||||
return { Database: FakeDatabase, Connection: FakeConnection } as any;
|
||||
};
|
||||
|
||||
describe('isDbBusyError', () => {
|
||||
it('matches the documented Windows lock-error wording', () => {
|
||||
expect(isDbBusyError(new Error('Could not set lock on file foo.lbug'))).toBe(true);
|
||||
expect(isDbBusyError(new Error('database is locked'))).toBe(true);
|
||||
});
|
||||
it('does not match unrelated errors', () => {
|
||||
expect(isDbBusyError(new Error('Cypher syntax error'))).toBe(false);
|
||||
expect(isDbBusyError(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('openLbugConnection — open-time lock-busy retry', () => {
|
||||
it('returns a handle when the constructor succeeds on the first try', async () => {
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [null],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
const handle = await openLbugConnection(stub, '/some/path/lbug');
|
||||
expect(handle.db).toBeDefined();
|
||||
expect(handle.conn).toBeDefined();
|
||||
expect(control.databaseCallCount).toBe(1);
|
||||
});
|
||||
|
||||
it('retries on busy/lock errors and succeeds on a later attempt', async () => {
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [new Error('Could not set lock on file'), null],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
const handle = await openLbugConnection(stub, '/some/path/lbug');
|
||||
expect(handle.db).toBeDefined();
|
||||
expect(control.databaseCallCount).toBe(2);
|
||||
});
|
||||
|
||||
it('exhausts the retry budget and rethrows the last error preserving its message', async () => {
|
||||
const lockErr = new Error('Could not set lock on file foo.lbug');
|
||||
const control: StubModuleControl = {
|
||||
// 5 attempts + production paths get no sweep retry, so 5 throws total.
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
await expect(openLbugConnection(stub, '/var/data/non-test/lbug')).rejects.toThrow(
|
||||
'Could not set lock on file foo.lbug',
|
||||
);
|
||||
expect(control.databaseCallCount).toBe(5);
|
||||
});
|
||||
|
||||
it('tags the exhausted error so withLbugDb skips its outer retry', async () => {
|
||||
const lockErr = new Error('Could not set lock on file');
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
let caught: unknown;
|
||||
try {
|
||||
await openLbugConnection(stub, '/var/data/non-test/lbug');
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect(caught).toBeDefined();
|
||||
expect(isOpenRetryExhausted(caught)).toBe(true);
|
||||
expect(isOpenRetryExhausted(new Error('plain error'))).toBe(false);
|
||||
expect(isOpenRetryExhausted(null)).toBe(false);
|
||||
expect(isOpenRetryExhausted(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('does not retry non-busy errors', async () => {
|
||||
const syntaxErr = new Error('Cypher syntax error');
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [syntaxErr],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
await expect(openLbugConnection(stub, '/some/path/lbug')).rejects.toThrow(
|
||||
'Cypher syntax error',
|
||||
);
|
||||
expect(control.databaseCallCount).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('openLbugConnection — stale-sidecar sweep (test fixtures only)', () => {
|
||||
let fixtureDir: string;
|
||||
let dbPath: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
fixtureDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-lbug-sweep-'));
|
||||
dbPath = path.join(fixtureDir, 'lbug');
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(fixtureDir, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
it('sweeps stale .wal/.lock for a recognized test fixture path and retries once', async () => {
|
||||
await fs.writeFile(dbPath + '.wal', 'stale');
|
||||
await fs.writeFile(dbPath + '.lock', 'stale');
|
||||
|
||||
const lockErr = new Error('Could not set lock on file');
|
||||
const control: StubModuleControl = {
|
||||
// 5 retries throw, then sweep + 1 final attempt succeeds (6 total).
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, null],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
|
||||
const handle = await openLbugConnection(stub, dbPath);
|
||||
expect(handle.db).toBeDefined();
|
||||
expect(control.databaseCallCount).toBe(6);
|
||||
|
||||
// Sidecars removed by the sweep
|
||||
await expect(fs.access(dbPath + '.wal')).rejects.toThrow();
|
||||
await expect(fs.access(dbPath + '.lock')).rejects.toThrow();
|
||||
});
|
||||
|
||||
it('does not sweep production paths even if they share the prefix', async () => {
|
||||
// A non-tmp dir that *starts* with the prefix must still be rejected.
|
||||
const lockErr = new Error('Could not set lock on file');
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
|
||||
// Path is outside os.tmpdir() so the predicate must reject it.
|
||||
await expect(openLbugConnection(stub, '/var/data/gitnexus-lbug-fake/lbug')).rejects.toThrow(
|
||||
'Could not set lock on file',
|
||||
);
|
||||
expect(control.databaseCallCount).toBe(5); // no sweep retry
|
||||
});
|
||||
|
||||
it('handles missing sidecars gracefully (ENOENT swallowed, retry runs)', async () => {
|
||||
// No .wal or .lock pre-created — sweep ENOENTs both, then succeeds.
|
||||
const lockErr = new Error('Could not set lock on file');
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, null],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
|
||||
const handle = await openLbugConnection(stub, dbPath);
|
||||
expect(handle.db).toBeDefined();
|
||||
expect(control.databaseCallCount).toBe(6);
|
||||
});
|
||||
|
||||
it('sweep retry that throws a different error preserves the original lock error', async () => {
|
||||
// 5 lock errors, then sweep fires, then post-sweep throws an unrelated
|
||||
// error. The user-actionable signal is "lock retries exhausted" — the
|
||||
// post-sweep error must NOT shadow the original lock message.
|
||||
const lockErr = new Error('Could not set lock on file foo.lbug');
|
||||
const unrelatedErr = new Error('Schema validation error during open');
|
||||
const control: StubModuleControl = {
|
||||
databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, unrelatedErr],
|
||||
databaseCallCount: 0,
|
||||
closeCallCount: 0,
|
||||
};
|
||||
const stub = makeStubLbug(control);
|
||||
|
||||
let caught: Error | undefined;
|
||||
try {
|
||||
await openLbugConnection(stub, dbPath);
|
||||
} catch (err) {
|
||||
caught = err as Error;
|
||||
}
|
||||
expect(caught?.message).toBe('Could not set lock on file foo.lbug');
|
||||
expect(control.databaseCallCount).toBe(6); // sweep retry did fire
|
||||
});
|
||||
});
|
||||
|
||||
describe('isTestFixturePath — production-safety guard', () => {
|
||||
it('accepts a fixture under os.tmpdir with a recognized prefix on the immediate parent', () => {
|
||||
const tmp = os.tmpdir();
|
||||
expect(isTestFixturePath(path.join(tmp, 'gitnexus-lbug-XXX', 'lbug'))).toBe(true);
|
||||
expect(isTestFixturePath(path.join(tmp, 'gitnexus-test-YYY', 'lbug'))).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects production paths even with a matching prefix', () => {
|
||||
expect(isTestFixturePath('/var/data/gitnexus-lbug-fake/lbug')).toBe(false);
|
||||
expect(isTestFixturePath('/home/user/gitnexus-test-foo/lbug')).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects path traversal attempts that resolve outside tmpdir', () => {
|
||||
const tmp = os.tmpdir();
|
||||
const traversal = path.join(tmp, 'gitnexus-lbug-x', '..', '..', 'etc', 'passwd');
|
||||
expect(isTestFixturePath(traversal)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects when the immediate parent does not match even if a deeper ancestor does', () => {
|
||||
// Tightening: ancestor walk would have allowed nested paths under
|
||||
// `<tmp>/gitnexus-lbug-x/inner/lbug` to satisfy the predicate. We
|
||||
// require the immediate parent to match.
|
||||
const tmp = os.tmpdir();
|
||||
expect(isTestFixturePath(path.join(tmp, 'gitnexus-lbug-x', 'inner', 'lbug'))).toBe(false);
|
||||
});
|
||||
|
||||
it('handles tmpdir trailing-separator gracefully', () => {
|
||||
// Some Windows TMP configs return a trailing separator; the predicate
|
||||
// strips it before the prefix check so fixtures still match.
|
||||
const tmp = os.tmpdir();
|
||||
const fixture = path.join(tmp, 'gitnexus-lbug-trailing', 'lbug');
|
||||
// Whether or not os.tmpdir() itself has a trailing separator,
|
||||
// the predicate must accept legit fixtures.
|
||||
expect(isTestFixturePath(fixture)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects unrelated prefixes in tmpdir', () => {
|
||||
const tmp = os.tmpdir();
|
||||
expect(isTestFixturePath(path.join(tmp, 'random-dir', 'lbug'))).toBe(false);
|
||||
expect(isTestFixturePath(path.join(tmp, 'malicious', 'lbug'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('waitForWindowsHandleRelease', () => {
|
||||
let fixtureDir: string;
|
||||
let dbPath: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
fixtureDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-lbug-probe-'));
|
||||
dbPath = path.join(fixtureDir, 'lbug');
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(fixtureDir, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
it('returns true when the file exists and is openable', async () => {
|
||||
await fs.writeFile(dbPath, 'fake-db-content');
|
||||
const released = await waitForWindowsHandleRelease(dbPath);
|
||||
expect(released).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when the file does not exist (ENOENT is non-lock)', async () => {
|
||||
// No fs.writeFile — path does not exist. Probe should bail to true,
|
||||
// not retry, since ENOENT is not a lock code.
|
||||
const released = await waitForWindowsHandleRelease(dbPath);
|
||||
expect(released).toBe(true);
|
||||
});
|
||||
|
||||
it('does not leak the file handle when close succeeds', async () => {
|
||||
// Smoke test: 50 sequential probes with a real file. If close were
|
||||
// skipped, fd usage would climb. We rely on test process not OOMing
|
||||
// as the simplest indicator; fd table caps catch egregious leaks.
|
||||
await fs.writeFile(dbPath, 'fake-db-content');
|
||||
for (let i = 0; i < 50; i++) {
|
||||
await waitForWindowsHandleRelease(dbPath);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -60,6 +60,8 @@ export default defineConfig({
|
|||
'test/integration/augmentation.test.ts',
|
||||
'test/integration/staleness-and-stability.test.ts',
|
||||
'test/integration/lbug-lock-retry.test.ts',
|
||||
'test/integration/lbug-open-retry.test.ts',
|
||||
'test/integration/lbug-close-handle-release.test.ts',
|
||||
'test/integration/api-impact-e2e.test.ts',
|
||||
'test/integration/shape-check-regression.test.ts',
|
||||
'test/integration/java-class-impact.test.ts',
|
||||
|
|
@ -87,6 +89,8 @@ export default defineConfig({
|
|||
'test/integration/augmentation.test.ts',
|
||||
'test/integration/staleness-and-stability.test.ts',
|
||||
'test/integration/lbug-lock-retry.test.ts',
|
||||
'test/integration/lbug-open-retry.test.ts',
|
||||
'test/integration/lbug-close-handle-release.test.ts',
|
||||
'test/integration/api-impact-e2e.test.ts',
|
||||
'test/integration/shape-check-regression.test.ts',
|
||||
'test/integration/java-class-impact.test.ts',
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue