mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-10 22:43:40 +00:00
fix(lbug): robust Windows lock acquisition for CI integration tests
LadybugDB's `new Database()` raises `Could not set lock on file` from local_file_system.cpp synchronously inside the constructor — before any query is issued, so `withLbugDb`'s query-time retry never sees it. On Windows CI this surfaces as flaky integration tests due to AV-scanner holds, libuv handle-release lag, and stale `.wal` sidecars from aborted prior runs. This change closes the gap at *open time*: - `openLbugConnection` now wraps `new lbug.Database()` in a bounded busy-retry (5x100ms back-off) inside `lbug-config.ts`. Errors that exhaust the budget are tagged via `LBUG_OPEN_RETRY_EXHAUSTED` so `withLbugDb`'s outer 3x retry skips re-retrying a freshly-exhausted path (eliminates the 3x5=15-attempt / ~6s tail latency). - For recognized test fixtures only (immediate-parent dir matches a known prefix AND resolves under `os.tmpdir()`), one final stale- sidecar sweep removes `.wal`/`.lock` and retries once. Production paths never enter this branch. - `safeClose` on Windows runs a bounded `fs.open` probe to absorb native handle-release lag; logs a warning if the probe exhausts so operators can spot AV interference. - `isDbBusyError` is now defined in `lbug-config.ts` as the single source of truth, re-exported from `lbug-adapter.ts` for compatibility. - New tests cover open-time retry (happy/retry/exhaust/non-busy/tag), stale-sidecar sweep (test-fixture-only, production-rejection, preserves-original-error), `isTestFixturePath` direct unit suite (accept/reject/traversal/nested/trailing-sep), and `waitForWindowsHandleRelease` (openable/ENOENT/no-leak). - The two new test files are added to vitest's existing serialized `lbug-db` project (already `fileParallelism: false`). Closes the chronic Windows CI flake on lbug-touching integration tests while preserving the existing single-writable-Database-per-process LadybugDB contract. No public API surface changed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar
parent
c8a1ecf69d
commit
e6924f31a7
6 changed files with 631 additions and 14 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';
|
||||
|
|
@ -186,19 +189,10 @@ const DB_LOCK_RETRY_ATTEMPTS = 3;
|
|||
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).
|
||||
* Re-exported from `lbug-config.ts` (single source of truth) so existing
|
||||
* importers continue to resolve `isDbBusyError` from this module.
|
||||
*/
|
||||
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')
|
||||
);
|
||||
};
|
||||
export { isDbBusyError };
|
||||
|
||||
/**
|
||||
* Return true when the error message indicates a write was attempted against
|
||||
|
|
@ -252,7 +246,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
|
||||
|
|
@ -1064,6 +1062,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 +1083,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';
|
||||
|
||||
/**
|
||||
|
|
@ -53,6 +56,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).
|
||||
*
|
||||
* Source of truth lives here so both the construction-time retry
|
||||
* (`openWithLockRetry`) and the query-time retry (`withLbugDb` in
|
||||
* `lbug-adapter.ts`) agree on the matcher. Other matchers (e.g.
|
||||
* `isReadOnlyDbError`) should follow this pattern: define here, re-export
|
||||
* from the adapter — see `references/findings-schema.json` adv-1 / maint-4.
|
||||
*/
|
||||
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')
|
||||
);
|
||||
};
|
||||
|
||||
export function createLbugDatabase(
|
||||
lbugModule: LbugModule,
|
||||
databasePath: string,
|
||||
|
|
@ -67,6 +92,158 @@ export function createLbugDatabase(
|
|||
);
|
||||
}
|
||||
|
||||
// ─── 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 (~1.5s 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,
|
||||
|
|
@ -74,7 +251,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(() => {});
|
||||
|
|
@ -86,3 +266,45 @@ export async function closeLbugConnection(handle: LbugConnectionHandle): Promise
|
|||
await handle.conn.close().catch(() => {});
|
||||
await handle.db.close().catch(() => {});
|
||||
}
|
||||
|
||||
/**
|
||||
* Probe `dbPath` 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.
|
||||
*
|
||||
* Returns `true` when the probe succeeded (handle was openable, lock
|
||||
* released cleanly) or when the probe was not needed (path missing,
|
||||
* non-lock error). Returns `false` when the probe exhausted its budget
|
||||
* with a lock code still in flight — caller may log a warning if the
|
||||
* platform is expected to be free of AV interference.
|
||||
*
|
||||
* Defensive shape:
|
||||
* - Opens read-only (`'r'`) to minimize sharing-mode contention with
|
||||
* concurrent processes (a writable handle would request more rights
|
||||
* and could itself trigger sharing violations on Windows).
|
||||
* - `try/finally` around `handle.close()` guarantees no fd leak even
|
||||
* if close itself throws.
|
||||
*/
|
||||
export const waitForWindowsHandleRelease = async (dbPath: string): Promise<boolean> => {
|
||||
for (let attempt = 1; attempt <= HANDLE_RELEASE_PROBE_ATTEMPTS; attempt++) {
|
||||
let handle: fs.FileHandle | undefined;
|
||||
try {
|
||||
handle = await fs.open(dbPath, '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));
|
||||
|
|
|
|||
51
gitnexus/test/integration/lbug-close-handle-release.test.ts
Normal file
51
gitnexus/test/integration/lbug-close-handle-release.test.ts
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
/**
|
||||
* 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`.
|
||||
*
|
||||
* The Windows-specific assertion is skipped on Linux/macOS — those
|
||||
* platforms do not exhibit the race so the test would not be meaningful
|
||||
* there. The cross-platform sanity case (close-then-reopen works) does
|
||||
* run everywhere.
|
||||
*
|
||||
* See: docs/plans/2026-05-08-002-fix-windows-lbug-lock-ci-flakes-plan.md
|
||||
*/
|
||||
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 25 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');
|
||||
// 25 iterations is enough on Windows CI to flush out the race
|
||||
// empirically (10 iterations was insufficient in pre-fix runs).
|
||||
// Tight loop with no inserts isolates the open/close path.
|
||||
for (let i = 0; i < 25; 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();
|
||||
}
|
||||
});
|
||||
});
|
||||
314
gitnexus/test/integration/lbug-open-retry.test.ts
Normal file
314
gitnexus/test/integration/lbug-open-retry.test.ts
Normal file
|
|
@ -0,0 +1,314 @@
|
|||
/**
|
||||
* 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';
|
||||
import * as adapterReexport from '../../src/core/lbug/lbug-adapter.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 (re-exported source of truth)', () => {
|
||||
it('matcher in lbug-adapter is the same function as in lbug-config', () => {
|
||||
expect(adapterReexport.isDbBusyError).toBe(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