diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 4cc1e06e6..ec2f9273b 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "PolyForm-Noncommercial-1.0.0", "dependencies": { - "@ladybugdb/core": "^0.19.0", + "@ladybugdb/core": "0.18.3", "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", @@ -1269,9 +1269,9 @@ } }, "node_modules/@ladybugdb/core": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core/-/core-0.19.1.tgz", - "integrity": "sha512-8W2g6xUi4jm96fs4EayyMcsvEEtIb8vboZhw9/YG98881cIcmZjmqAN91XGUp4vb8NqoFr3Wp7wcu3dqJk0b7w==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core/-/core-0.18.3.tgz", + "integrity": "sha512-XjpPKW4MrL28D2gYGTZuIjiEcPx12L21lx58QggrdrItw8o/e9Lmg/Ejoo4Kz08lZj+rIcC1Fu9thzIYOTUlJw==", "hasInstallScript": true, "license": "MIT", "dependencies": { @@ -1280,17 +1280,17 @@ "node-addon-api": "^6.0.0" }, "optionalDependencies": { - "@ladybugdb/core-darwin-arm64": "0.19.1", - "@ladybugdb/core-darwin-x64": "0.19.1", - "@ladybugdb/core-linux-arm64": "0.19.1", - "@ladybugdb/core-linux-x64": "0.19.1", - "@ladybugdb/core-win32-x64": "0.19.1" + "@ladybugdb/core-darwin-arm64": "0.18.3", + "@ladybugdb/core-darwin-x64": "0.18.3", + "@ladybugdb/core-linux-arm64": "0.18.3", + "@ladybugdb/core-linux-x64": "0.18.3", + "@ladybugdb/core-win32-x64": "0.18.3" } }, "node_modules/@ladybugdb/core-darwin-arm64": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-arm64/-/core-darwin-arm64-0.19.1.tgz", - "integrity": "sha512-VGQs1NThAygMsoOlxud05pqKA9xfUptl55iYkwvW45As5MSI7+M86WN0Pp0VdPEfw8vNQJehrlHR5LvVAuWc2Q==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-arm64/-/core-darwin-arm64-0.18.3.tgz", + "integrity": "sha512-DGZTOlvSS4esEb1vTekY5IDoAvZAeYzR5cXVkECtQj9BVkk05zsvCAdTPo1Rz1BuI0qvqUVF+2WlIerI67iA2g==", "cpu": [ "arm64" ], @@ -1301,9 +1301,9 @@ ] }, "node_modules/@ladybugdb/core-darwin-x64": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-x64/-/core-darwin-x64-0.19.1.tgz", - "integrity": "sha512-CGfM6ostxDS5jztxwjkXXtxrjMDgsFMRoyr5HZDCFw1+iXC1rIzmK/Y7RIw+KbQ49aPzSmkhBC447mFviBJxoA==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-x64/-/core-darwin-x64-0.18.3.tgz", + "integrity": "sha512-Qp6j0CM/orBlK6KD0p/s4ofkIhNUwi1hdCgMw+fj81UHugWHkVLiYV4grRBdHhyplw+snchZpTxvfpxFbkG1Cw==", "cpu": [ "x64" ], @@ -1314,9 +1314,9 @@ ] }, "node_modules/@ladybugdb/core-linux-arm64": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-arm64/-/core-linux-arm64-0.19.1.tgz", - "integrity": "sha512-BZUQwlkvNXENc5GVyXdfRF0Dv9JX8XMlcdMMiB5GKrEhTCpajQ3D58woHPVvn0JEjw7Ms3tHo6kXUAMZKYXIVg==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-arm64/-/core-linux-arm64-0.18.3.tgz", + "integrity": "sha512-F9miYjBuS43I7uNG199FNMqwdHJ98WA6dU3v2SZCeLXmXCdRzmYcuHQWlbNr2Tba9CX58w2XvBZoUaXZKJ/yKQ==", "cpu": [ "arm64" ], @@ -1327,9 +1327,9 @@ ] }, "node_modules/@ladybugdb/core-linux-x64": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-x64/-/core-linux-x64-0.19.1.tgz", - "integrity": "sha512-LDx+E1UHlmNXSb3F9QmvdBgZGfB3wI/DcrHzfOwXgT3BP8C4ScB2tZdpiYQiuPp8MiSZ9kuuGqos8A4tQKQu8Q==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-x64/-/core-linux-x64-0.18.3.tgz", + "integrity": "sha512-AfG5RDp/f/IDctDMpTAT5+2MYNtlWT191xiQNjSaWD4X85DhY3Dzps8Qu5VteIAPih5d6mmoaKGs8q0XIjfkFA==", "cpu": [ "x64" ], @@ -1340,9 +1340,9 @@ ] }, "node_modules/@ladybugdb/core-win32-x64": { - "version": "0.19.1", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-win32-x64/-/core-win32-x64-0.19.1.tgz", - "integrity": "sha512-2spst1g+Z050Fz/5z7Pc6Fuc5dVXLzekOuWW4lP+mEGCL3tkv3QWYxk37DiFy+O9fDFxiWK2f3aauab58/f9kQ==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-win32-x64/-/core-win32-x64-0.18.3.tgz", + "integrity": "sha512-bHuFk0m9cnq0WGd9I4D8or8g6cC/BS58iatMtilqM3JpDPIQIFk6MQl6exL7P4xyWbkLwQgsrv2ToDnyoQNKvg==", "cpu": [ "x64" ], diff --git a/gitnexus/package.json b/gitnexus/package.json index 1d8fc8c62..78776ec72 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -57,7 +57,7 @@ "version": "node scripts/sync-plugin-manifests.mjs" }, "dependencies": { - "@ladybugdb/core": "^0.19.0", + "@ladybugdb/core": "0.18.3", "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index ce98dcc71..71c3dfe69 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -49,7 +49,9 @@ import { HANDLE_RELEASE_PROBE_DELAY_MS, isDbBusyError, isOpenRetryExhausted, + isStorageVersionMismatchError, isWalCorruptionError, + throwIfStorageVersionMismatch, bufferPoolExhaustionRemedy, openLbugConnection, sleep, @@ -700,6 +702,11 @@ const runSchemaCreationQueries = async (dbPath: string): Promise ` Original error: ${msg.slice(0, 200)}`, ); } + if (isStorageVersionMismatchError(err)) { + await safeClose(); + resetOpenConnectionState(); + throwIfStorageVersionMismatch(err); + } if (!msg.includes('already exists') && !isDbBusyError(err) && !isReadOnlyDbError(err)) { logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`); } @@ -812,8 +819,27 @@ const doInitLbug = async ( allowQuarantine: false, }); - const opened = await openLbugConnection(lbug, dbPath, { readOnly: true }); - const usable = await ensureReadOnlyConnectionUsable(dbPath, opened); + let usable: Awaited>; + try { + const opened = await openLbugConnection(lbug, dbPath, { readOnly: true }); + // The storage-version check isn't necessarily enforced by the native + // engine until the first real query runs (ensureReadOnlyConnectionUsable's + // own probe query) — openLbugConnection alone can succeed on a + // mismatched file. Wrap both. + usable = await ensureReadOnlyConnectionUsable(dbPath, opened); + } catch (err) { + // Not retryable: the on-disk file's storage version doesn't change on + // its own, so withLbugDb's retry loop (which only handles + // isDbBusyError) would just repeat the same native exception. Fail + // immediately with an actionable message instead (review finding on + // PR #3189 — this became reachable once the pinned engine version can + // trail behind whatever version last wrote an index, e.g. after + // downgrading the dependency). Mirrors the pool-adapter.ts check for + // the same error, on the separate open path /api/graph and /api/query + // actually use (withLbugDb, not the pool). + throwIfStorageVersionMismatch(err); + throw err; + } db = usable.db; conn = usable.conn; currentDbReadOnly = true; @@ -922,10 +948,18 @@ const doInitLbug = async ( allowQuarantine: true, }); - const opened = await openLbugConnection(lbug, dbPath); - db = opened.db; - conn = opened.conn; - currentDbReadOnly = false; + try { + const opened = await openLbugConnection(lbug, dbPath); + db = opened.db; + conn = opened.conn; + currentDbReadOnly = false; + } catch (err) { + // Incremental analyze can hit a storage-version mismatch on construct + // or (more often) on the first schema query below. Fail immediately + // with the rebuild hint instead of warn-and-continue. + throwIfStorageVersionMismatch(err); + throw err; + } } finally { await releaseInitLock(); } diff --git a/gitnexus/src/core/lbug/lbug-config.ts b/gitnexus/src/core/lbug/lbug-config.ts index cc88878a5..3fe32b1aa 100644 --- a/gitnexus/src/core/lbug/lbug-config.ts +++ b/gitnexus/src/core/lbug/lbug-config.ts @@ -562,6 +562,30 @@ export function isWalCorruptionError(err: unknown): boolean { return WAL_CORRUPTION_RE.test(msg); } +/** Matches a LadybugDB storage-version mismatch: the on-disk file was + * written by a different @ladybugdb/core build (a different storage + * version) than the one currently installed — e.g. an index built by a + * newer engine, opened after downgrading the pinned dependency. Example: + * "Runtime exception: Trying to read a database file with a different + * version. Database file version: 43, Current build storage version: 42" */ +const STORAGE_VERSION_MISMATCH_RE = /database file with a different version/i; + +export const STORAGE_VERSION_MISMATCH_SUGGESTION = + 'This index was written by a different @ladybugdb/core build (a different storage version) than the one currently installed. Run `gitnexus analyze --force` on this repo to rebuild it with the current engine.'; + +export function isStorageVersionMismatchError(err: unknown): boolean { + if (!err) return false; + const msg = err instanceof Error ? err.message : String(err); + return STORAGE_VERSION_MISMATCH_RE.test(msg); +} + +/** Throws the rebuild-hint Error when `err` is a storage-version mismatch. */ +export function throwIfStorageVersionMismatch(err: unknown): void { + if (!isStorageVersionMismatchError(err)) return; + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`${STORAGE_VERSION_MISMATCH_SUGGESTION} (${msg})`); +} + // ─── Ladybug WAL checkpoint IO error matchers ─────────────────────────────── // // Matched against LadybugDB v0.18.0 (see `gitnexus/package.json` diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index 7f38e48ba..cedc5ae45 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -23,6 +23,8 @@ import { warnIfQueryTextUnbounded } from './query-batch.js'; import { createLbugDatabase, isWalCorruptionError, + sleep, + throwIfStorageVersionMismatch, toNativeSafePath, WAL_RECOVERY_SUGGESTION, } from './lbug-config.js'; @@ -224,7 +226,39 @@ function ensureIdleTimer(): void { for (const [repoId, entry] of pool) { if (pinnedRepos.has(repoId)) continue; if (now - entry.lastUsed > IDLE_TIMEOUT_MS && entry.checkedOut === 0) { - closeOne(repoId); + // Routed through the same mutex as initLbug (not awaited here — this + // sweep is periodic best-effort cleanup with nothing waiting on it). + // closeOne now removes the pool entry before its awaited db.close(), + // so an unsynchronized idle close racing a concurrent initLbug for + // the same repoId would let that init treat the repo as absent and + // open a fresh native handle on the same file while the idle close's + // checkpoint is still in flight — reopening the exact race this pool + // rework exists to close, just via the idle path instead of LRU + // eviction (review finding on PR #3187). withPoolLock serializes it + // against every initLbug call the same way evictLRU already is. + // + // This callback can now sit queued behind an in-progress initLbug + // before its turn comes, and that init's existing-entry path (or a + // concurrent touchRepo()) can refresh lastUsed in the meantime — so + // the repo may no longer be idle by the time this actually runs. + // Re-check inside the lock, right before closing, instead of trusting + // the snapshot taken above (second review finding on PR #3187). + // Also re-check pinnedRepos: the outer loop's check above is the + // same kind of stale snapshot — pinRepo() can run while this + // callback is queued behind an in-progress initLbug, and closing a + // repo the caller just pinned would drop that lease entirely + // (review finding on PR #3189). + withPoolLock(async () => { + const current = pool.get(repoId); + if ( + current && + !pinnedRepos.has(repoId) && + Date.now() - current.lastUsed > IDLE_TIMEOUT_MS && + current.checkedOut === 0 + ) { + await closeOne(repoId); + } + }); } } }, 60_000); @@ -304,7 +338,7 @@ export const getMaxResidentRepos = (): number => MAX_POOL_SIZE; * entry is pinned, no eviction occurs and the pool transiently exceeds * MAX_POOL_SIZE (see the pinnedRepos docstring). */ -function evictLRU(): void { +async function evictLRU(): Promise { if (pool.size < MAX_POOL_SIZE) return; let oldestId: string | null = null; @@ -317,7 +351,12 @@ function evictLRU(): void { } } if (oldestId) { - closeOne(oldestId); + // Awaited: the caller opens a new connection right after evicting one, and + // closeOne's db.close() below triggers a checkpoint. A fire-and-forget close + // here let that new open race the still-in-flight checkpoint of the evicted + // repo, surfacing as "Cannot open database in read-only mode while checkpoint + // is in progress" on the read path. + await closeOne(oldestId); } } @@ -326,7 +365,7 @@ function evictLRU(): void { * shared Database ref. Only closes the Database when no other repoIds * reference it (refCount === 0). */ -function closeOne(repoId: string): void { +async function closeOne(repoId: string): Promise { const entry = pool.get(repoId); if (!entry) return; @@ -358,6 +397,27 @@ function closeOne(repoId: string): void { // Checked-out connections can't be closed here — they're in-flight. // The checkin() function detects entry.closed and closes them on return. + // Remove the entry — and clear its pin, and notify listeners — BEFORE the + // possible await below. `available` is already empty and `closed` is + // already set, so nothing further to lose; but `shared.db.close()` can + // suspend, and until this repoId is actually gone from `pool`, + // `isLbugReady(repoId)` (a bare `pool.has`) still reports true. A + // concurrent same-repo `initLbug`/query during that window would see a + // "ready" pool entry with no available connections and no in-flight + // open — a zombie that `checkout` can only fail on with a misleading + // "pool integrity error" instead of just reopening. Deleting first makes + // that window disappear: any concurrent caller instead sees "not + // initialized" and takes the normal fresh-open path. + pool.delete(repoId); + pinnedRepos.delete(repoId); + for (const listener of poolCloseListeners) { + try { + listener(repoId); + } catch { + // Isolate listener failures — teardown must complete. + } + } + // Only close the Database when no other repoIds reference it. // External databases (injected via initLbugWithDb) are never closed here — // the core adapter owns them and handles their lifecycle. @@ -374,29 +434,21 @@ function closeOne(repoId: string): void { shared.vectorLoaded = false; shared.vectorLoadPromise = undefined; } else { - shared.db.close().catch(() => {}); + // Awaited (unlike the per-connection closes above): this is the shared + // Database handle whose close() drives the checkpoint that the caller's + // subsequent reopen (evictLRU / the "idle & changed" path below) must not + // race. See the awaited call site in evictLRU for the full rationale. + await shared.db.close().catch(() => {}); dbCache.delete(entry.dbPath); } } } - pool.delete(repoId); - - // Clear any eviction pin — the entry is gone, so the pin is meaningless and - // would otherwise leak across operations in a long-lived process. Teardown - // is authoritative: an explicit close always wins over a pin. + // Close yields on native db.close() above. A pinRepo during that await + // would otherwise survive teardown and apply to the next init, contradicting + // the documented lease contract (pins do not outlive closeOne). pinnedRepos.delete(repoId); - // Notify listeners AFTER the pool entry is gone so any cache-invalidation - // they perform is consistent with `isLbugReady(repoId) === false`. - for (const listener of poolCloseListeners) { - try { - listener(repoId); - } catch { - // Isolate listener failures — teardown must complete. - } - } - traceRss('close', repoId); } @@ -650,17 +702,40 @@ async function tryQuarantineAndReopen(dbPath: string, repoId: string): Promise>(); +// Serializes pool mutations (evict / close / native open / register) across +// concurrent callers. Awaiting closeOne/evictLRU closes the race within a +// single call; this mutex makes those mutations mutually exclusive across +// repos so two inits cannot race each other's checkpoint. +let poolLock: Promise = Promise.resolve(); +function withPoolLock(fn: () => Promise): Promise { + const run = poolLock.then(fn, fn); + poolLock = run.then( + () => undefined, + () => undefined, + ); + return run; +} + +type InitLbugAttempt = { status: 'done'; reopened: boolean } | { status: 'retry'; error: Error }; + +function ladybugUnavailableError(repoId: string, err: Error | undefined): Error { + return new Error( + `LadybugDB unavailable for ${repoId}. Another process may be rebuilding the index. ` + + `Retry later. (${err?.message || 'unknown error'})`, + ); +} /** * Initialize (or reuse) a Database + connection pool for a specific repo. * Retries on lock errors (e.g., when `gitnexus analyze` is running). * - * Concurrent calls for the same repoId are deduplicated — the second caller - * awaits the first's in-progress init rather than starting a redundant one. - */ -/** + * Concurrent calls (for the same repoId or different ones) serialize on + * poolLock below for evict / close / native open / register, so a second + * caller for a repo already being initialized waits its turn and then hits + * the "existing" fast path — no separate per-repoId dedup needed. Lock-retry + * *sleeps* run outside the mutex so one analyze-locked repo does not block + * every other pool init for LOCK_RETRY_DELAY_MS * attempt. + * * Returns `true` when this call (re)opened a fresh handle onto the current * on-disk file, `false` when it reused/served the existing handle (unchanged, * or changed-but-a-query-is-in-flight). Callers that gate their own freshness @@ -668,6 +743,18 @@ const initPromises = new Map>(); * return value; callers that only need the pool ready can ignore it. */ export const initLbug = async (repoId: string, dbPath: string): Promise => { + let lastError: Error | undefined; + for (let attempt = 1; attempt <= LOCK_RETRY_ATTEMPTS; attempt++) { + const result = await withPoolLock(() => initLbugInner(repoId, dbPath)); + if (result.status === 'done') return result.reopened; + lastError = result.error; + if (attempt === LOCK_RETRY_ATTEMPTS) break; + await sleep(LOCK_RETRY_DELAY_MS * attempt); + } + throw ladybugUnavailableError(repoId, lastError); +}; + +const initLbugInner = async (repoId: string, dbPath: string): Promise => { const existing = pool.get(repoId); if (existing) { existing.lastUsed = Date.now(); @@ -676,7 +763,9 @@ export const initLbug = async (repoId: string, dbPath: string): Promise // unlinked-but-open) inode until LRU/idle eviction — a stale-read window // of up to IDLE_TIMEOUT_MS after analyze finishes. const current = await statDbIdentity(dbPath); - if (!dbIdentityChanged(existing.dbIdentity, current)) return false; // unchanged → reuse + if (!dbIdentityChanged(existing.dbIdentity, current)) { + return { status: 'done', reopened: false }; // unchanged → reuse + } // A query is in flight on this entry; closing its connection (and the // shared Database at refCount 0) mid-use is a native use-after-free. Serve // the current handle for this dispatch — the next initLbug that finds the @@ -687,27 +776,12 @@ export const initLbug = async (repoId: string, dbPath: string): Promise // (a complete older snapshot), just not the newest. Callers that route // freshness THROUGH initLbug (rather than calling closeLbug directly) get // this guard for free; that is why LocalBackend delegates here (#2614). - if (existing.checkedOut > 0) return false; - closeOne(repoId); // idle & changed → evict, then fall through to reopen the new file + if (existing.checkedOut > 0) return { status: 'done', reopened: false }; + // Awaited: see the rationale on the evictLRU call site in doInitLbug below. + await closeOne(repoId); // idle & changed → evict, then fall through to reopen the new file } - // Deduplicate concurrent init calls for the same repoId — - // prevents double-init race when multiple parallel tool calls - // trigger initialization for the same repo simultaneously. - const pending = initPromises.get(repoId); - if (pending) { - await pending; - return true; - } - - const promise = doInitLbug(repoId, dbPath); - initPromises.set(repoId, promise); - try { - await promise; - } finally { - initPromises.delete(repoId); - } - return true; + return doInitLbug(repoId, dbPath); }; /** @@ -715,7 +789,7 @@ export const initLbug = async (repoId: string, dbPath: string): Promise * Pool entry is registered LAST so concurrent executeQuery calls see either * "not initialized" (and throw) or a fully ready pool — never a half-built one. */ -async function doInitLbug(repoId: string, dbPath: string): Promise { +async function doInitLbug(repoId: string, dbPath: string): Promise { // Check if database exists try { await fs.stat(dbPath); @@ -723,7 +797,15 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { throw new Error(`LadybugDB not found at ${dbPath}. Run: gitnexus analyze`); } - evictLRU(); + // Awaited: without this, the connection opened just below could race the + // checkpoint from the LRU victim's still-in-flight close (see evictLRU / + // closeOne). The caller holds withPoolLock for this attempt, so this await + // only covers this call's own evict-then-reopen — not other callers. + // Lock-retry re-enters this function after sleeping *outside* the mutex. + // evictLRU is a no-op unless the pool is full again (another repo may have + // taken the slot we freed on a prior attempt). Skipping it on retry would + // let a 6th native open race a still-resident victim's checkpoint. + await evictLRU(); // Reuse an existing native Database if another repoId already opened this path. // This prevents buffer manager exhaustion from multiple mmap regions on the same file. @@ -748,36 +830,38 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { if (!shared) { // Open in read-only mode — MCP server never writes to the database. // This allows multiple MCP server instances to read concurrently, and - // avoids lock conflicts when `gitnexus analyze` is writing. - let lastError: Error | null = null; - for (let attempt = 1; attempt <= LOCK_RETRY_ATTEMPTS; attempt++) { - try { - const db = await openReadOnlyDatabase(dbPath); - shared = { db, refCount: 0, ftsLoaded: false, dbIdentity: await statDbIdentity(dbPath) }; - dbCache.set(dbPath, shared); - break; - } catch (err: any) { - lastError = err instanceof Error ? err : new Error(String(err)); + // avoids lock conflicts when `gitnexus analyze` is writing. This attempt + // is one native open; lock-retry backoff lives in initLbug. + try { + const db = await openReadOnlyDatabase(dbPath); + shared = { db, refCount: 0, ftsLoaded: false, dbIdentity: await statDbIdentity(dbPath) }; + dbCache.set(dbPath, shared); + } catch (err: unknown) { + const lastError = err instanceof Error ? err : new Error(String(err)); - if (isWalCorruptionError(lastError)) { - try { - const db = await tryQuarantineAndReopen(dbPath, repoId); - shared = { - db, - refCount: 0, - ftsLoaded: false, - dbIdentity: await statDbIdentity(dbPath), - }; - dbCache.set(dbPath, shared); - break; - } catch (retryErr) { - throw new Error( - `LadybugDB WAL corruption detected for ${repoId}. ${WAL_RECOVERY_SUGGESTION} ` + - `(${retryErr instanceof Error ? retryErr.message : String(retryErr)})`, - ); - } + // Not retryable: the on-disk file's storage version doesn't change + // on its own. Fail immediately with an actionable message. + throwIfStorageVersionMismatch(lastError); + + if (isWalCorruptionError(lastError)) { + try { + const db = await tryQuarantineAndReopen(dbPath, repoId); + shared = { + db, + refCount: 0, + ftsLoaded: false, + dbIdentity: await statDbIdentity(dbPath), + }; + dbCache.set(dbPath, shared); + } catch (retryErr) { + throw new Error( + `LadybugDB WAL corruption detected for ${repoId}. ${WAL_RECOVERY_SUGGESTION} ` + + `(${retryErr instanceof Error ? retryErr.message : String(retryErr)})`, + ); } + } + if (!shared) { if ( lastError.message.startsWith('LadybugDB checkpoint sidecar is missing') || lastError.message.startsWith('LadybugDB checkpoint sidecar is present but unreachable') || @@ -786,21 +870,15 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { ) { throw lastError; } - - const isLockError = + if ( lastError.message.includes('Could not set lock') || - /\block(\b|ed|ing)/i.test(lastError.message); - if (!isLockError || attempt === LOCK_RETRY_ATTEMPTS) break; - await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_DELAY_MS * attempt)); + /\block(\b|ed|ing)/i.test(lastError.message) + ) { + return { status: 'retry', error: lastError }; + } + throw ladybugUnavailableError(repoId, lastError); } } - - if (!shared) { - throw new Error( - `LadybugDB unavailable for ${repoId}. Another process may be rebuilding the index. ` + - `Retry later. (${lastError?.message || 'unknown error'})`, - ); - } } shared.refCount++; @@ -846,6 +924,7 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { }); ensureIdleTimer(); traceRss('init', repoId); + return { status: 'done', reopened: true }; } /** @@ -863,6 +942,14 @@ export async function initLbugWithDb( repoId: string, existingDb: lbug.Database, dbPath: string, +): Promise { + return withPoolLock(() => initLbugWithDbInner(repoId, existingDb, dbPath)); +} + +async function initLbugWithDbInner( + repoId: string, + existingDb: lbug.Database, + dbPath: string, ): Promise { const existing = pool.get(repoId); if (existing) { @@ -1223,13 +1310,30 @@ export const executeParameterized = async ( */ export const closeLbug = async (repoId?: string): Promise => { if (repoId) { - closeOne(repoId); + // Locked: closeOne now deletes the pool entry before its awaited + // db.close() finishes, so an unlocked call here could race a concurrent + // initLbug(repoId, ...) — that init could acquire the lock right after + // the delete, see no cached entry, and start opening a fresh connection + // while this close's checkpoint is still in flight, reopening the exact + // race withPoolLock exists to close (review finding on PR #3189). + // Awaited: closeOne is now async (see evictLRU's rationale); callers of + // closeLbug rely on pool.delete() having already run — e.g. isLbugReady() + // returning false — by the time this promise resolves. + await withPoolLock(() => closeOne(repoId)); return; } - for (const id of [...pool.keys()]) { - closeOne(id); - } + // Locked for the same reason as the per-repoId branch above, plus: without + // this, an initLbug that runs while this loop is mid-await (closeOne + // yields during the native close) can register a fresh pool entry after + // `pool.keys()` was already snapshotted, so a caller expecting closeLbug() + // to mean "pool is now empty" would find that new entry still resident + // (review finding on PR #3187). + await withPoolLock(async () => { + for (const id of [...pool.keys()]) { + await closeOne(id); + } + }); if (idleTimer) { clearInterval(idleTimer); diff --git a/gitnexus/test/unit/basicblock-callee-ids-schema.test.ts b/gitnexus/test/unit/basicblock-callee-ids-schema.test.ts index b91a7e469..a9e7ab1ad 100644 --- a/gitnexus/test/unit/basicblock-callee-ids-schema.test.ts +++ b/gitnexus/test/unit/basicblock-callee-ids-schema.test.ts @@ -177,6 +177,9 @@ const makeConfigMock = () => { resolveNativeSafeStorageDir: (p: string) => p, WAL_RECOVERY_SUGGESTION: 'run analyze --force', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', }; return { mock, queries }; }; diff --git a/gitnexus/test/unit/convex-metadata-persistence-contract.test.ts b/gitnexus/test/unit/convex-metadata-persistence-contract.test.ts index 8fd89e820..b5e55a1cd 100644 --- a/gitnexus/test/unit/convex-metadata-persistence-contract.test.ts +++ b/gitnexus/test/unit/convex-metadata-persistence-contract.test.ts @@ -32,6 +32,9 @@ function makeConfigMock() { resolveNativeSafeStorageDir: (value: string) => value, WAL_RECOVERY_SUGGESTION: 'run analyze --force', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', }, }; } diff --git a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts index d96eeec26..ec42adc3d 100644 --- a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts +++ b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts @@ -141,6 +141,10 @@ vi.mock('../../../src/core/lbug/lbug-config.js', () => ({ toNativeSafePath: vi.fn((p: string) => p), isWalCorruptionError: vi.fn(() => false), WAL_RECOVERY_SUGGESTION: '', + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + sleep: vi.fn(async () => {}), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.mock('../../../src/core/lbug/sidecar-recovery.js', () => ({ diff --git a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts index 5822728a8..13a1c3940 100644 --- a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts +++ b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts @@ -13,6 +13,7 @@ import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import fs from 'node:fs/promises'; import path from 'node:path'; +import { STORAGE_VERSION_MISMATCH_SUGGESTION } from '../../src/core/lbug/lbug-config.js'; // ─── Helpers ───────────────────────────────────────────────────────────────── @@ -37,6 +38,20 @@ const schemaMockFactory = async () => ({ ...SCHEMA_MOCK, }); +async function mockLbugConfigForStorageVersion(overrides: Record) { + const actual = await vi.importActual( + '../../src/core/lbug/lbug-config.js', + ); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + ...actual, + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + ...overrides, + })); +} + function makeFsMock(dbPath: string) { const ENOENT = Object.assign(new Error(`ENOENT: ${dbPath}`), { code: 'ENOENT' }); return { @@ -104,6 +119,16 @@ describe('doInitLbug WAL corruption guard — structural', () => { expect(warnIdx).toBeGreaterThan(-1); expect(walGuardIdx).toBeLessThan(warnIdx); }); + + it('imports throwIfStorageVersionMismatch and uses it in the schema catch', () => { + expect(adapterSource).toMatch(/throwIfStorageVersionMismatch/); + expect(schemaLoopBody).toMatch(/isStorageVersionMismatchError\(err\)/); + expect(schemaLoopBody).toMatch(/throwIfStorageVersionMismatch\(err\)/); + const mismatchIdx = schemaLoopBody.indexOf('isStorageVersionMismatchError(err)'); + const warnIdx = schemaLoopBody.indexOf('Schema creation warning'); + expect(mismatchIdx).toBeGreaterThan(-1); + expect(mismatchIdx).toBeLessThan(warnIdx); + }); }); // ─── Behavioural tests ──────────────────────────────────────────────────────── @@ -147,6 +172,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -199,6 +227,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -260,6 +291,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -312,6 +346,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -389,6 +426,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -470,6 +510,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -524,6 +567,9 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -646,6 +692,9 @@ describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', WAL_RECOVERY_SUGGESTION: 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -821,3 +870,128 @@ describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', ); }); }); + +const NATIVE_STORAGE_VERSION_MISMATCH = + 'Runtime exception: Trying to read a database file with a different version. Database file version: 43, Current build storage version: 42'; + +describe('doInitLbug storage-version fail-fast — behavioural', () => { + afterEach(() => { + vi.doUnmock('fs/promises'); + vi.doUnmock('../../src/core/lbug/schema.js'); + vi.doUnmock('../../src/core/lbug/lbug-config.js'); + vi.doUnmock('../../src/core/lbug/extension-loader.js'); + vi.doUnmock('../../src/core/logger.js'); + vi.resetModules(); + vi.clearAllMocks(); + }); + + it('throws the rebuild hint when a writable schema query raises a storage-version mismatch', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-storage-version-schema/lbug'; + const versionError = new Error(NATIVE_STORAGE_VERSION_MISMATCH); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn().mockRejectedValueOnce(versionError).mockResolvedValue(queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const warnMock = vi.fn(); + + vi.doMock('fs/promises', () => makeFsMock(dbPath)); + vi.doMock('../../src/core/lbug/schema.js', schemaMockFactory); + await mockLbugConfigForStorageVersion({ + openLbugConnection: vi.fn(async () => ({ db, conn })), + closeLbugConnection: vi.fn(async () => {}), + }); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { warn: warnMock, info: vi.fn(), error: vi.fn(), debug: vi.fn() }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const err = await adapter.initLbug(dbPath).catch((e: unknown) => e); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toContain(STORAGE_VERSION_MISMATCH_SUGGESTION); + expect((err as Error).message).toMatch(/database file with a different version/i); + expect(warnMock).not.toHaveBeenCalledWith(expect.stringContaining('Schema creation warning')); + expect(db.close).toHaveBeenCalled(); + // Schema DDL is not retried. A second query is the CHECKPOINT inside safeClose. + expect(conn.query).toHaveBeenNthCalledWith(1, SCHEMA_MOCK.SCHEMA_QUERIES[0]); + expect( + conn.query.mock.calls.filter((call) => call[0] === SCHEMA_MOCK.SCHEMA_QUERIES[0]), + ).toHaveLength(1); + }); + + it('throws the rebuild hint when writable openLbugConnection raises a storage-version mismatch', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-storage-version-writable-open/lbug'; + const versionError = new Error(NATIVE_STORAGE_VERSION_MISMATCH); + const openLbugConnection = vi.fn(async () => { + throw versionError; + }); + const warnMock = vi.fn(); + + vi.doMock('fs/promises', () => makeFsMock(dbPath)); + vi.doMock('../../src/core/lbug/schema.js', schemaMockFactory); + await mockLbugConfigForStorageVersion({ + openLbugConnection, + closeLbugConnection: vi.fn(async () => {}), + }); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { warn: warnMock, info: vi.fn(), error: vi.fn(), debug: vi.fn() }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await expect(adapter.initLbug(dbPath)).rejects.toThrow(STORAGE_VERSION_MISMATCH_SUGGESTION); + expect(openLbugConnection).toHaveBeenCalledTimes(1); + expect(warnMock).not.toHaveBeenCalledWith(expect.stringContaining('Schema creation warning')); + }); + + it('throws the rebuild hint on a read-only open storage-version mismatch without lock-retrying', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-storage-version-readonly/lbug'; + const versionError = new Error(NATIVE_STORAGE_VERSION_MISMATCH); + const openLbugConnection = vi.fn(async () => { + throw versionError; + }); + + vi.doMock('fs/promises', () => makeFsMock(dbPath)); + vi.doMock('../../src/core/lbug/schema.js', schemaMockFactory); + await mockLbugConfigForStorageVersion({ + openLbugConnection, + closeLbugConnection: vi.fn(async () => {}), + }); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await expect( + adapter.withLbugDb(dbPath, async () => 'unreached', { readOnly: true }), + ).rejects.toThrow(STORAGE_VERSION_MISMATCH_SUGGESTION); + expect(openLbugConnection).toHaveBeenCalledTimes(1); + }); +}); diff --git a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts index e06e7445c..8592546dc 100644 --- a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts +++ b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts @@ -95,6 +95,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -167,6 +170,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -232,6 +238,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -300,6 +309,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -359,6 +371,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -431,6 +446,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -506,6 +524,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -565,6 +586,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -617,6 +641,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -680,6 +707,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -758,6 +788,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { @@ -836,6 +869,9 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), isOpenRetryExhausted: vi.fn(() => false), waitForWindowsHandleRelease: vi.fn(async () => true), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ extensionManager: { diff --git a/gitnexus/test/unit/lbug-config-wal.test.ts b/gitnexus/test/unit/lbug-config-wal.test.ts index 1150a0da7..61a291da7 100644 --- a/gitnexus/test/unit/lbug-config-wal.test.ts +++ b/gitnexus/test/unit/lbug-config-wal.test.ts @@ -4,8 +4,11 @@ import { createLbugDatabase, estimateBufferPool, isLbugCheckpointIoError, + isStorageVersionMismatchError, isWalCorruptionError, setBufferPoolSizeHint, + STORAGE_VERSION_MISMATCH_SUGGESTION, + throwIfStorageVersionMismatch, _setOsPageSizeForTests, bufferPoolExhaustionRemedy, } from '../../src/core/lbug/lbug-config.js'; @@ -46,6 +49,53 @@ describe('isWalCorruptionError', () => { }); }); +describe('isStorageVersionMismatchError / throwIfStorageVersionMismatch', () => { + const NATIVE = + 'Runtime exception: Trying to read a database file with a different version. Database file version: 43, Current build storage version: 42'; + + it.each([ + ['documented native message', NATIVE], + ['bare phrase', 'Trying to read a database file with a different version'], + ])('matches storage-version mismatch: %s', (_label, msg) => { + expect(isStorageVersionMismatchError(msg)).toBe(true); + expect(isStorageVersionMismatchError(new Error(msg))).toBe(true); + }); + + it.each([ + ['lock error', 'Could not set lock on file : /path/to/db'], + ['WAL corruption', 'Runtime exception: Corrupted wal file. Read out invalid WAL record type.'], + ['generic', 'Query failed'], + ['schema version wording', 'schema version mismatch in WAL'], + ])('does not match non-version error: %s', (_label, msg) => { + expect(isStorageVersionMismatchError(msg)).toBe(false); + expect(isStorageVersionMismatchError(new Error(msg))).toBe(false); + }); + + it('handles non-string input', () => { + expect(isStorageVersionMismatchError(undefined)).toBe(false); + expect(isStorageVersionMismatchError(null)).toBe(false); + expect(isStorageVersionMismatchError(42)).toBe(false); + expect(isStorageVersionMismatchError(new Error('ok'))).toBe(false); + }); + + it('throwIfStorageVersionMismatch wraps the native message with the rebuild hint', () => { + expect(() => throwIfStorageVersionMismatch(new Error(NATIVE))).toThrow( + new RegExp( + `${STORAGE_VERSION_MISMATCH_SUGGESTION.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*different version`, + ), + ); + }); + + it('throwIfStorageVersionMismatch is a no-op for lock and WAL errors', () => { + expect(() => + throwIfStorageVersionMismatch(new Error('Could not set lock on file : /path')), + ).not.toThrow(); + expect(() => + throwIfStorageVersionMismatch(new Error('Runtime exception: Corrupted wal file.')), + ).not.toThrow(); + }); +}); + describe('createLbugDatabase WAL replay option', () => { it('enables auto-checkpoint by default and uses default threshold (64 MiB)', () => { const Database = vi.fn(function (this: any) {}); diff --git a/gitnexus/test/unit/lbug-pool-evict-reopen-race.test.ts b/gitnexus/test/unit/lbug-pool-evict-reopen-race.test.ts new file mode 100644 index 000000000..5cc2fc32f --- /dev/null +++ b/gitnexus/test/unit/lbug-pool-evict-reopen-race.test.ts @@ -0,0 +1,165 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; + +// Regression test: closeOne() used to close the evicted repo's shared Database +// with a fire-and-forget `db.close().catch(() => {})` (no await), and evictLRU +// (plus the "idle & changed" reopen path in initLbug) proceeded to open the +// next repo's connection immediately after, without waiting for that close — +// and the checkpoint it triggers — to finish. On the real LadybugDB engine +// this surfaces as: "Runtime exception: Cannot open database in read-only +// mode while checkpoint is in progress. Please retry later." on the very next +// read against ANY repo, not just the one being evicted. +// +// The native engine isn't mockable down to that exact error, so this test +// instead asserts the ordering guarantee the fix provides: the evicted repo's +// close() must fully resolve before the caller that triggered the eviction +// (initLbug for a new, distinct repo) itself resolves. Mock setup mirrors +// lbug-pool-pinning.test.ts (same native/adapter/sidecar-recovery mocks), +// driving the real initLbug -> evictLRU -> closeOne path. + +const { loadFTSExtensionMock, loadVectorExtensionMock } = vi.hoisted(() => ({ + loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), +})); + +vi.mock('@ladybugdb/core', () => ({ + default: { + Database: vi.fn(), + Connection: vi.fn(function (this: any) { + this.query = vi.fn().mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + this.close = vi.fn().mockResolvedValue(undefined); + }), + }, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ + isReadOnlyDbError: vi.fn(() => false), + loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, +})); + +vi.mock('../../src/core/lbug/lbug-config.js', () => ({ + createLbugDatabase: vi.fn(() => ({ + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + })), + toNativeSafePath: vi.fn((p: string) => p), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: '', + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + sleep: vi.fn(async () => {}), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', +})); + +vi.mock('../../src/core/lbug/sidecar-recovery.js', () => ({ + preflightLbugSidecars: vi.fn().mockResolvedValue(undefined), + guardWalQuarantine: vi.fn().mockResolvedValue(undefined), + isMissingFsError: vi.fn(() => false), + isMissingShadowSidecarError: vi.fn(() => false), + isReadOnlyShadowReplayError: vi.fn(() => false), + quarantineWalForMissingShadow: vi.fn().mockResolvedValue(''), + quarantineSidecarsForDirtyRecovery: vi + .fn() + .mockResolvedValue({ moved: [], removed: [], failed: [] }), + renameFailureMessage: vi.fn((p: string) => `rename failed for ${p}`), + statIfExists: vi.fn().mockResolvedValue(null), +})); + +const { initLbug, closeLbug, isLbugReady, unpinRepo } = + await import('../../src/core/lbug/pool-adapter.js'); +const { createLbugDatabase } = await import('../../src/core/lbug/lbug-config.js'); + +describe('pool-adapter evict-then-reopen race (fire-and-forget close fix)', () => { + let tmpDir: string; + const touched = new Set(); + + const dbPathFor = (repoId: string): string => { + const p = path.join(tmpDir, `${repoId}.lbug`); + writeFileSync(p, ''); + return p; + }; + + const init = async (repoId: string): Promise => { + touched.add(repoId); + await initLbug(repoId, dbPathFor(repoId)); + }; + + beforeEach(() => { + tmpDir = mkdtempSync(path.join(os.tmpdir(), 'gn-evict-race-test-')); + loadFTSExtensionMock.mockResolvedValue(true); + }); + + afterEach(async () => { + vi.useRealTimers(); + await closeLbug().catch(() => {}); + for (const id of touched) unpinRepo(id); + touched.clear(); + loadFTSExtensionMock.mockReset(); + rmSync(tmpDir, { recursive: true, force: true }); + }); + + it("evictLRU awaits the evicted repo's close() before the triggering initLbug settles", async () => { + // MAX_POOL_SIZE is 5; repo-1 is the first (and, once repo-6 inits, the + // LRU-oldest unpinned) entry, so it is the eviction victim below. + // Give repo-1's shared Database a close() gated on a real (short) delay, + // simulating a slow checkpoint, and record the relative order of "close + // finished" vs. "the repo-6 init that triggered the eviction finished". + const order: string[] = []; + + vi.mocked(createLbugDatabase).mockImplementationOnce(() => ({ + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + order.push('repo1-close-end'); + }), + })); + + for (let i = 1; i <= 5; i++) await init(`repo-${i}`); + + await init('repo-6'); + order.push('repo6-init-done'); + + // With the fix: evictLRU awaits closeOne's close(), so the 20ms-delayed + // close of the evicted repo-1 must finish BEFORE initLbug('repo-6', ...) + // itself resolves — 'repo1-close-end' precedes 'repo6-init-done'. + // Without the fix (fire-and-forget close), repo-6's own (near-instant, + // mock-driven) init finishes well before the unrelated 20ms delay elapses, + // so the order is reversed. + expect(order).toEqual(['repo1-close-end', 'repo6-init-done']); + expect(isLbugReady('repo-1')).toBe(false); + expect(isLbugReady('repo-6')).toBe(true); + }); + + it('closeLbug(repoId) awaits the underlying close() before its own promise resolves', async () => { + // Regression guard for a second-order effect of making closeOne async: + // closeLbug must still await it, or its promise can resolve before the + // real (here, delayed-mock) close() has actually finished. Gate the + // mock's close() on a real short delay, like the first test, so an + // implementation that drops the await is caught regardless of how fast + // the mock itself would otherwise resolve. + const order: string[] = []; + + vi.mocked(createLbugDatabase).mockImplementationOnce(() => ({ + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + order.push('close-end'); + }), + })); + + await init('repo-solo'); + expect(isLbugReady('repo-solo')).toBe(true); + + await closeLbug('repo-solo'); + order.push('closeLbug-done'); + + expect(order).toEqual(['close-end', 'closeLbug-done']); + expect(isLbugReady('repo-solo')).toBe(false); + }); +}); diff --git a/gitnexus/test/unit/lbug-pool-fts-load.test.ts b/gitnexus/test/unit/lbug-pool-fts-load.test.ts index 920c8b994..0bf9cfb83 100644 --- a/gitnexus/test/unit/lbug-pool-fts-load.test.ts +++ b/gitnexus/test/unit/lbug-pool-fts-load.test.ts @@ -41,6 +41,10 @@ vi.mock('../../src/core/lbug/lbug-config.js', () => ({ toNativeSafePath: vi.fn((p: string) => p), isWalCorruptionError: vi.fn(() => false), WAL_RECOVERY_SUGGESTION: '', + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + sleep: vi.fn(async () => {}), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); const { closeLbug, ensureVectorExtension, executeParameterized, initLbug, initLbugWithDb } = diff --git a/gitnexus/test/unit/lbug-pool-pinning.test.ts b/gitnexus/test/unit/lbug-pool-pinning.test.ts index 125c803e0..4b5b5ce24 100644 --- a/gitnexus/test/unit/lbug-pool-pinning.test.ts +++ b/gitnexus/test/unit/lbug-pool-pinning.test.ts @@ -50,6 +50,10 @@ vi.mock('../../src/core/lbug/lbug-config.js', () => ({ toNativeSafePath: vi.fn((p: string) => p), isWalCorruptionError: vi.fn(() => false), WAL_RECOVERY_SUGGESTION: '', + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + sleep: vi.fn(async () => {}), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.mock('../../src/core/lbug/sidecar-recovery.js', () => ({ @@ -69,8 +73,9 @@ vi.mock('../../src/core/lbug/sidecar-recovery.js', () => ({ statIfExists: vi.fn().mockResolvedValue(null), })); -const { initLbug, closeLbug, isLbugReady, pinRepo, unpinRepo } = +const { initLbug, initLbugWithDb, closeLbug, isLbugReady, pinRepo, unpinRepo } = await import('../../src/core/lbug/pool-adapter.js'); +const { createLbugDatabase } = await import('../../src/core/lbug/lbug-config.js'); const { initWikiDb, closeWikiDb, pinWikiDb } = await import('../../src/core/wiki/graph-queries.js'); describe('pool-adapter repo pinning (issue #2189)', () => { @@ -247,4 +252,49 @@ describe('pool-adapter repo pinning (issue #2189)', () => { await init('d-extra2'); expect(isLbugReady('d-shared')).toBe(false); }); + + it('a pin acquired while closeOne awaits db.close() does not survive teardown', async () => { + vi.mocked(createLbugDatabase).mockImplementationOnce(() => ({ + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }), + })); + + await init('late-pin'); + const closing = closeLbug('late-pin'); + await Promise.resolve(); + pinRepo('late-pin'); + await closing; + expect(isLbugReady('late-pin')).toBe(false); + + await init('late-pin'); + for (let i = 1; i <= 5; i++) await init(`late-fresh-${i}`); + expect(isLbugReady('late-pin')).toBe(false); + }); + + it('initLbugWithDb waits for an in-flight closeOne before registering', async () => { + vi.mocked(createLbugDatabase).mockImplementationOnce(() => ({ + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }), + })); + + await init('injected-overlap'); + const closing = closeLbug('injected-overlap'); + await Promise.resolve(); + const injected = { + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; + const injecting = initLbugWithDb( + 'injected-overlap', + injected as never, + dbPathFor('injected-overlap'), + ); + await closing; + await injecting; + expect(isLbugReady('injected-overlap')).toBe(true); + }); }); diff --git a/gitnexus/test/unit/lbug-pool-storage-version-lock-retry.test.ts b/gitnexus/test/unit/lbug-pool-storage-version-lock-retry.test.ts new file mode 100644 index 000000000..f5b4946b9 --- /dev/null +++ b/gitnexus/test/unit/lbug-pool-storage-version-lock-retry.test.ts @@ -0,0 +1,149 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; + +const { loadFTSExtensionMock, loadVectorExtensionMock, createLbugDatabaseMock } = vi.hoisted( + () => ({ + loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), + createLbugDatabaseMock: vi.fn(), + }), +); + +vi.mock('@ladybugdb/core', () => ({ + default: { + Database: vi.fn(), + Connection: vi.fn(function (this: any) { + this.query = vi.fn().mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + this.close = vi.fn().mockResolvedValue(undefined); + }), + }, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ + isReadOnlyDbError: vi.fn(() => false), + loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, +})); + +vi.mock('../../src/core/lbug/lbug-config.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createLbugDatabase: createLbugDatabaseMock, + toNativeSafePath: vi.fn((p: string) => p), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: '', + }; +}); + +vi.mock('../../src/core/lbug/sidecar-recovery.js', () => ({ + preflightLbugSidecars: vi.fn().mockResolvedValue(undefined), + guardWalQuarantine: vi.fn().mockResolvedValue(undefined), + isMissingFsError: vi.fn(() => false), + isMissingShadowSidecarError: vi.fn(() => false), + isReadOnlyShadowReplayError: vi.fn(() => false), + quarantineWalForMissingShadow: vi.fn().mockResolvedValue(''), + quarantineSidecarsForDirtyRecovery: vi + .fn() + .mockResolvedValue({ moved: [], removed: [], failed: [] }), + renameFailureMessage: vi.fn((p: string) => `rename failed for ${p}`), + statIfExists: vi.fn().mockResolvedValue(null), +})); + +const { initLbug, closeLbug, isLbugReady, unpinRepo } = + await import('../../src/core/lbug/pool-adapter.js'); +const { STORAGE_VERSION_MISMATCH_SUGGESTION } = await import('../../src/core/lbug/lbug-config.js'); + +const NATIVE_STORAGE_VERSION_MISMATCH = + 'Runtime exception: Trying to read a database file with a different version. Database file version: 43, Current build storage version: 42'; + +function goodDb() { + return { + init: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +describe('pool-adapter storage-version fail-fast and lock-retry backoff', () => { + let tmpDir: string; + const touched = new Set(); + + const dbPathFor = (repoId: string): string => { + const p = path.join(tmpDir, `${repoId}.lbug`); + writeFileSync(p, ''); + return p; + }; + + beforeEach(() => { + tmpDir = mkdtempSync(path.join(os.tmpdir(), 'gn-pool-sv-lock-')); + loadFTSExtensionMock.mockResolvedValue(true); + createLbugDatabaseMock.mockReset(); + createLbugDatabaseMock.mockImplementation(() => goodDb()); + }); + + afterEach(async () => { + vi.useRealTimers(); + await closeLbug().catch(() => {}); + for (const id of touched) unpinRepo(id); + touched.clear(); + loadFTSExtensionMock.mockReset(); + rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('fail-fasts a storage-version mismatch with the rebuild hint and does not lock-retry', async () => { + const repoId = 'version-mismatch'; + touched.add(repoId); + createLbugDatabaseMock.mockImplementation(() => { + throw new Error(NATIVE_STORAGE_VERSION_MISMATCH); + }); + + await expect(initLbug(repoId, dbPathFor(repoId))).rejects.toThrow( + STORAGE_VERSION_MISMATCH_SUGGESTION, + ); + expect(createLbugDatabaseMock).toHaveBeenCalledTimes(1); + }); + + it('does not hold withPoolLock across lock-retry sleep — another repo can init during backoff', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + + const lockedId = 'locked'; + const otherId = 'other'; + touched.add(lockedId); + touched.add(otherId); + const lockedPath = dbPathFor(lockedId); + const otherPath = dbPathFor(otherId); + + createLbugDatabaseMock.mockImplementation((_mod: unknown, dbPath: string) => { + if ( + String(dbPath).includes(`${path.sep}locked.lbug`) || + String(dbPath).endsWith('locked.lbug') + ) { + throw new Error('Could not set lock on file : /tmp/locked.lbug'); + } + return goodDb(); + }); + + const lockedInit = initLbug(lockedId, lockedPath); + await vi.waitFor(() => { + expect(createLbugDatabaseMock).toHaveBeenCalledTimes(1); + }); + + const otherInit = initLbug(otherId, otherPath); + await expect(otherInit).resolves.toBe(true); + expect(isLbugReady(otherId)).toBe(true); + expect(createLbugDatabaseMock).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(2000); + await vi.waitFor(() => { + expect(createLbugDatabaseMock).toHaveBeenCalledTimes(3); + }); + await vi.advanceTimersByTimeAsync(4000); + await expect(lockedInit).rejects.toThrow(/LadybugDB unavailable for locked/); + expect(createLbugDatabaseMock).toHaveBeenCalledTimes(4); + }); +}); diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index ca7517878..6ae7cfe06 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -44,6 +44,10 @@ vi.mock('../../src/core/lbug/lbug-config.js', () => ({ const msg = err instanceof Error ? err.message : String(err ?? ''); return /corrupt(ed)?\s+wal|invalid\s+wal\s+record/i.test(msg); }), + isStorageVersionMismatchError: vi.fn(() => false), + throwIfStorageVersionMismatch: vi.fn(), + sleep: vi.fn(async () => {}), + STORAGE_VERSION_MISMATCH_SUGGESTION: '', })); vi.mock('../../src/mcp/stdio-capture.js', () => ({