From 23da8b1d30a65a7905b985e9db45057614ff1079 Mon Sep 17 00:00:00 2001 From: wangxc Date: Thu, 21 May 2026 14:40:03 +0800 Subject: [PATCH 1/5] fix(lbug): keep serve stable when sidecars are missing Shared missing-shadow WAL recovery prevents repeated read-only open warnings when LadybugDB sidecars are absent, while the Express preflight fix keeps `gitnexus serve` compatible with Express 5 route parsing. Constraint: LadybugDB read-only replay can require a `.shadow` sidecar that may be absent after interrupted writes or checkpoint edge cases. Rejected: keep reactive WARN-only quarantine in each adapter | it leaves repeated user-visible warnings and duplicate recovery behavior. Confidence: high Scope-risk: broad Directive: Do not silently delete large orphan WALs; only quarantine tiny orphan WALs before open and keep large WALs for explicit recovery. Tested: cd gitnexus && npx vitest run test/unit/sidecar-recovery.test.ts test/unit/lbug-adapter-wal-schema.test.ts test/unit/pool-wal-recovery.test.ts test/unit/web-ui-serving.test.ts && npx tsc --noEmit Not-tested: full npm test in this split branch; full unit suite passed on the source branch before PR split. Co-authored-by: OmX --- gitnexus/src/core/lbug/lbug-adapter.ts | 231 +++++++++++--- gitnexus/src/core/lbug/pool-adapter.ts | 112 +++++++ gitnexus/src/core/lbug/sidecar-recovery.ts | 254 ++++++++++++++++ gitnexus/src/server/api.ts | 3 +- .../test/unit/lbug-adapter-wal-schema.test.ts | 283 +++++++++++++++++- gitnexus/test/unit/pool-wal-recovery.test.ts | 82 ++++- gitnexus/test/unit/sidecar-recovery.test.ts | 123 ++++++++ gitnexus/test/unit/web-ui-serving.test.ts | 8 + 8 files changed, 1052 insertions(+), 44 deletions(-) create mode 100644 gitnexus/src/core/lbug/sidecar-recovery.ts create mode 100644 gitnexus/test/unit/sidecar-recovery.test.ts diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 77e9f65f11..f9291bcc70 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -27,6 +27,13 @@ import { waitForWindowsHandleRelease, type LbugConnectionHandle, } from './lbug-config.js'; +import { + finalizeLbugSidecarsAfterClose, + isMissingShadowSidecarError, + preflightLbugSidecars, + quarantineWalForMissingShadow, + shadowSidecarRecoveryMessage, +} from './sidecar-recovery.js'; import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js'; import { logger } from '../logger.js'; @@ -437,6 +444,157 @@ const queryAndDrain = async (targetConn: lbug.Connection, cypher: string): Promi await drainQueryResult(queryResult); }; +const READ_ONLY_SHADOW_REPLAY_PROBE = 'MATCH (n) RETURN n LIMIT 1'; + +const isReadOnlyShadowReplayError = (err: unknown): boolean => { + const msg = err instanceof Error ? err.message : String(err); + return /replay shadow pages under read-only mode/i.test(msg); +}; + +const reopenReadOnlyAfterMissingShadow = async ( + dbPath: string, + err: unknown, +): Promise => { + try { + await quarantineWalForMissingShadow(dbPath, { + logger, + level: 'warn', + reason: 'read-only recovery', + }); + } catch { + throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } + + const reopened = await openLbugConnection(lbug, dbPath, { readOnly: true }); + try { + await queryAndDrain(reopened.conn, READ_ONLY_SHADOW_REPLAY_PROBE); + return reopened; + } catch (retryErr) { + await closeLbugConnection(reopened); + if (isMissingShadowSidecarError(retryErr) || isReadOnlyShadowReplayError(retryErr)) { + throw new Error(shadowSidecarRecoveryMessage(dbPath, retryErr)); + } + throw retryErr; + } +}; + +const reopenWritableAfterMissingShadow = async ( + dbPath: string, + err: unknown, +): Promise => { + try { + await quarantineWalForMissingShadow(dbPath, { + logger, + level: 'warn', + reason: 'writable recovery', + }); + } catch { + throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } + + return await openLbugConnection(lbug, dbPath); +}; + +const ensureReadOnlyConnectionUsable = async ( + dbPath: string, + handle: LbugConnectionHandle, +): Promise => { + try { + await queryAndDrain(handle.conn, READ_ONLY_SHADOW_REPLAY_PROBE); + return handle; + } catch (err) { + if (isMissingShadowSidecarError(err)) { + await closeLbugConnection(handle); + return await reopenReadOnlyAfterMissingShadow(dbPath, err); + } + if (!isReadOnlyShadowReplayError(err)) { + await closeLbugConnection(handle); + throw err; + } + } + + await closeLbugConnection(handle); + + const writable = await openLbugConnection(lbug, dbPath); + let missingShadowError: unknown; + try { + await queryAndDrain(writable.conn, READ_ONLY_SHADOW_REPLAY_PROBE); + } catch (err) { + if (isMissingShadowSidecarError(err)) { + missingShadowError = err; + } else { + throw err; + } + } finally { + await closeLbugConnection(writable); + } + if (missingShadowError) { + return await reopenReadOnlyAfterMissingShadow(dbPath, missingShadowError); + } + + const reopened = await openLbugConnection(lbug, dbPath, { readOnly: true }); + try { + await queryAndDrain(reopened.conn, READ_ONLY_SHADOW_REPLAY_PROBE); + return reopened; + } catch (err) { + await closeLbugConnection(reopened); + if (isMissingShadowSidecarError(err)) { + throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } + throw err; + } +}; + +const resetOpenConnectionState = (): void => { + currentDbPath = null; + ftsLoaded = false; + vectorExtensionLoaded = false; + ensuredFTSIndexes.clear(); +}; + +const runSchemaCreationQueries = async (dbPath: string): Promise => { + for (const schemaQuery of SCHEMA_QUERIES) { + try { + await queryAndDrain(conn, schemaQuery); + } catch (err) { + if (isMissingShadowSidecarError(err)) { + return err; + } + + const msg = err instanceof Error ? err.message : String(err); + // Suppression list: + // - "already exists": expected idempotent re-create on existing DBs + // - "could not set lock on file": LadybugDB v0.16.1 emits this on + // Windows when CREATE NODE TABLE runs against a path that was + // just opened (the WAL handle from a fresh Database briefly + // contests the table's first-write lock). The table is created + // anyway and any genuine cross-process lock contention surfaces + // on the next operation via withLbugDb's retry. Logging it here + // would just be noise in CI. + // + // WAL corruption: the first DDL write after DB open triggers WAL + // replay — if the WAL file was left in a corrupt state by an + // interrupted previous run, the native engine throws here. Rather + // than logging a WARN and continuing in a broken state, close the + // DB cleanly and surface an actionable error so the caller (serve, + // MCP, analyze) can exit with a clear recovery message. + if (isWalCorruptionError(err)) { + await safeClose(); + resetOpenConnectionState(); + throw new Error( + `LadybugDB WAL corruption detected at ${dbPath}. ${WAL_RECOVERY_SUGGESTION}\n` + + ` Original error: ${msg.slice(0, 200)}`, + ); + } + if (!msg.includes('already exists') && !isDbBusyError(err) && !isReadOnlyDbError(err)) { + logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`); + } + } + } + + return null; +}; + export const initLbug = async (dbPath: string) => { return runWithSessionLock(() => ensureLbugInitialized(dbPath)); }; @@ -580,58 +738,46 @@ const doInitLbug = async (dbPath: string, readOnly: boolean = false) => { // Ensure parent directory exists const parentDir = path.dirname(dbPath); await fs.mkdir(parentDir, { recursive: true }); + await preflightLbugSidecars(dbPath, { + mode: readOnly ? 'read-only' : 'write', + logger, + allowQuarantine: true, + }); const opened = readOnly ? await openLbugConnection(lbug, dbPath, { readOnly: true }) : await openLbugConnection(lbug, dbPath); - db = opened.db; - conn = opened.conn; + const usable = readOnly ? await ensureReadOnlyConnectionUsable(dbPath, opened) : opened; + db = usable.db; + conn = usable.conn; currentDbReadOnly = readOnly; } finally { await releaseInitLock(); } - for (const schemaQuery of SCHEMA_QUERIES) { - try { - await queryAndDrain(conn, schemaQuery); - } catch (err) { - const msg = err instanceof Error ? err.message : String(err); - // Suppression list: - // - "already exists": expected idempotent re-create on existing DBs - // - "could not set lock on file": LadybugDB v0.16.1 emits this on - // Windows when CREATE NODE TABLE runs against a path that was - // just opened (the WAL handle from a fresh Database briefly - // contests the table's first-write lock). The table is created - // anyway and any genuine cross-process lock contention surfaces - // on the next operation via withLbugDb's retry. Logging it here - // would just be noise in CI. - // - // WAL corruption: the first DDL write after DB open triggers WAL - // replay — if the WAL file was left in a corrupt state by an - // interrupted previous run, the native engine throws here. Rather - // than logging a WARN and continuing in a broken state, close the - // DB cleanly and surface an actionable error so the caller (serve, - // MCP, analyze) can exit with a clear recovery message. - if (isWalCorruptionError(err)) { + if (!readOnly) { + const missingShadowError = await runSchemaCreationQueries(dbPath); + if (missingShadowError) { + await safeClose(); + resetOpenConnectionState(); + const reopened = await reopenWritableAfterMissingShadow(dbPath, missingShadowError); + db = reopened.db; + conn = reopened.conn; + currentDbReadOnly = false; + + const retryMissingShadowError = await runSchemaCreationQueries(dbPath); + if (retryMissingShadowError) { await safeClose(); - currentDbPath = null; - ftsLoaded = false; - vectorExtensionLoaded = false; - ensuredFTSIndexes.clear(); - throw new Error( - `LadybugDB WAL corruption detected at ${dbPath}. ${WAL_RECOVERY_SUGGESTION}\n` + - ` Original error: ${msg.slice(0, 200)}`, - ); - } - if (!msg.includes('already exists') && !isDbBusyError(err) && !isReadOnlyDbError(err)) { - logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`); + resetOpenConnectionState(); + throw new Error(shadowSidecarRecoveryMessage(dbPath, retryMissingShadowError)); } } } - // FTS powers baseline search, so initialize it with the core DB. VECTOR is - // only required for semantic embeddings and is probed lazily there. - await loadFTSExtension(); + // FTS powers baseline search, so initialize it with the core DB. Read-only + // serve/MCP paths must never run DDL or trigger network INSTALL; analyze owns + // schema/index creation and extension installation. + await loadFTSExtension(undefined, readOnly ? { policy: 'load-only' } : {}); currentDbPath = dbPath; return { db, conn }; @@ -1348,8 +1494,10 @@ export const flushWAL = async (): Promise => { try { const checkpointResult = await conn.query('CHECKPOINT'); await drainQueryResult(checkpointResult); - } catch { - /* ignore — older LadybugDB or schemaless DB may not accept it */ + } catch (err) { + logger.debug( + `GitNexus: LadybugDB CHECKPOINT skipped/failed during WAL flush: ${summarizeError(err)}`, + ); } }; @@ -1404,6 +1552,9 @@ export const safeClose = async (): Promise => { ); } } + if (closingDbPath) { + await finalizeLbugSidecarsAfterClose(closingDbPath, { logger }); + } }; export const closeLbug = async (): Promise => { diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index f551c65f81..e0dfdab7a4 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -25,6 +25,12 @@ import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION, } from './lbug-config.js'; +import { + isMissingShadowSidecarError, + preflightLbugSidecars, + quarantineWalForMissingShadow, + shadowSidecarRecoveryMessage, +} from './sidecar-recovery.js'; /** * Probe whether a Windows FTS extension binary is locally installed under @@ -304,16 +310,115 @@ const WAITER_TIMEOUT_MS = 15_000; const LOCK_RETRY_ATTEMPTS = 3; const LOCK_RETRY_DELAY_MS = 2000; +const SHADOW_REPLAY_PROBE_QUERY = 'MATCH (n) RETURN n LIMIT 1'; + +const isReadOnlyShadowReplayError = (err: unknown): boolean => { + const msg = err instanceof Error ? err.message : String(err); + return /replay shadow pages under read-only mode/i.test(msg); +}; + +const poolSidecarLogger = { + warn: (message: string): void => { + realStderrWrite(`${message}\n`); + }, + debug: (_message: string): void => {}, + info: (message: string): void => { + realStderrWrite(`${message}\n`); + }, +}; + +async function probeDatabaseForShadowReplay(db: lbug.Database): Promise { + const conn = createConnection(db); + try { + const queryResult = await conn.query(SHADOW_REPLAY_PROBE_QUERY); + const result = Array.isArray(queryResult) ? queryResult[0] : queryResult; + await result.getAll(); + result.close?.(); + } finally { + await conn.close().catch(() => {}); + } +} + +async function replayShadowPagesWithWritableOpen(dbPath: string): Promise { + let db: lbug.Database | undefined; + try { + db = createLbugDatabase(lbug, dbPath, { throwOnWalReplayFailure: false }); + await db.init(); + await probeDatabaseForShadowReplay(db); + } catch (err) { + if (isMissingShadowSidecarError(err)) { + try { + await quarantineWalForMissingShadow(dbPath, { + logger: poolSidecarLogger, + level: 'warn', + reason: 'pool writable replay recovery', + }); + return; + } catch { + throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } + } + throw err; + } finally { + if (db) await db.close().catch(() => {}); + } +} async function openReadOnlyDatabase(dbPath: string): Promise { let db: lbug.Database | undefined; silenceStdout(); try { + await preflightLbugSidecars(dbPath, { + mode: 'read-only', + logger: poolSidecarLogger, + allowQuarantine: true, + }); db = createLbugDatabase(lbug, dbPath, { readOnly: true, throwOnWalReplayFailure: false, }); await db.init(); + try { + await probeDatabaseForShadowReplay(db); + } catch (err) { + if (isMissingShadowSidecarError(err)) { + await db.close().catch(() => {}); + db = undefined; + try { + await quarantineWalForMissingShadow(dbPath, { + logger: poolSidecarLogger, + level: 'warn', + reason: 'pool read-only recovery', + }); + } catch { + throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } + await preflightLbugSidecars(dbPath, { + mode: 'read-only', + logger: poolSidecarLogger, + allowQuarantine: true, + }); + db = createLbugDatabase(lbug, dbPath, { + readOnly: true, + throwOnWalReplayFailure: false, + }); + await db.init(); + await probeDatabaseForShadowReplay(db); + return db; + } + if (!isReadOnlyShadowReplayError(err)) { + throw err; + } + await db.close().catch(() => {}); + db = undefined; + await replayShadowPagesWithWritableOpen(dbPath); + db = createLbugDatabase(lbug, dbPath, { + readOnly: true, + throwOnWalReplayFailure: false, + }); + await db.init(); + await probeDatabaseForShadowReplay(db); + } return db; } catch (err) { if (db) await db.close().catch(() => {}); @@ -423,6 +528,13 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { } } + if ( + lastError.message.startsWith('LadybugDB checkpoint sidecar is missing') || + isMissingShadowSidecarError(lastError) + ) { + throw lastError; + } + const isLockError = lastError.message.includes('Could not set lock') || lastError.message.includes('lock'); if (!isLockError || attempt === LOCK_RETRY_ATTEMPTS) break; diff --git a/gitnexus/src/core/lbug/sidecar-recovery.ts b/gitnexus/src/core/lbug/sidecar-recovery.ts new file mode 100644 index 0000000000..08cc76f9b1 --- /dev/null +++ b/gitnexus/src/core/lbug/sidecar-recovery.ts @@ -0,0 +1,254 @@ +import fs from 'fs/promises'; +import path from 'path'; + +export type LbugSidecarState = + | { kind: 'clean'; dbPath: string } + | { kind: 'wal-with-shadow'; dbPath: string; walBytes: number; shadowBytes: number } + | { kind: 'tiny-orphan-wal'; dbPath: string; walBytes: number } + | { kind: 'orphan-wal'; dbPath: string; walBytes: number } + | { kind: 'orphan-shadow'; dbPath: string; shadowBytes: number }; + +export interface SidecarRecoveryLogger { + warn: (message: string) => void; + info?: (message: string) => void; + debug?: (message: string) => void; +} + +export const TINY_ORPHAN_WAL_BYTES = 4 * 1024; + +const warnedKeys = new Set(); + +const missing = (err: unknown): boolean => + (err as NodeJS.ErrnoException | undefined)?.code === 'ENOENT'; + +const sidecarPreflightDisabled = (): boolean => + /^(1|true|yes|on)$/i.test(process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT ?? ''); + +const statIfExists = async (filePath: string): Promise<{ size: number } | null> => { + try { + const statFn = (fs as typeof fs & { stat?: typeof fs.stat }).stat; + if (typeof statFn === 'function') { + const stat = await statFn(filePath); + return { size: stat.size }; + } + // Some focused unit tests provide a deliberately tiny fs mock. Treat a + // path as present only when access succeeds, with an unknown/zero size. + await fs.access(filePath); + return { size: 0 }; + } catch (err) { + if (missing(err)) return null; + throw err; + } +}; + +const logDebug = (logger: SidecarRecoveryLogger, message: string): void => { + if (logger.debug) logger.debug(message); +}; + +const logInfo = (logger: SidecarRecoveryLogger, message: string): void => { + if (logger.info) logger.info(message); + else logDebug(logger, message); +}; + +const warnOnce = (logger: SidecarRecoveryLogger, key: string, message: string): void => { + if (warnedKeys.has(key)) { + logDebug(logger, message); + return; + } + warnedKeys.add(key); + logger.warn(message); +}; + +export const isMissingShadowSidecarError = (err: unknown): boolean => { + const msg = err instanceof Error ? err.message : String(err); + return /Cannot open file .*\.shadow: No such file or directory/i.test(msg); +}; + +export const shadowSidecarRecoveryMessage = (dbPath: string, err: unknown): string => { + const msg = err instanceof Error ? err.message : String(err); + return ( + `LadybugDB checkpoint sidecar is missing for ${dbPath}. ` + + 'Rebuild the index with `gitnexus analyze --force --index-only` and restart `gitnexus serve`.' + + `\n Original error: ${msg.slice(0, 200)}` + ); +}; + +export async function inspectLbugSidecars(dbPath: string): Promise { + const wal = await statIfExists(`${dbPath}.wal`); + const shadow = await statIfExists(`${dbPath}.shadow`); + + if (wal && shadow) { + return { kind: 'wal-with-shadow', dbPath, walBytes: wal.size, shadowBytes: shadow.size }; + } + if (wal) { + if (wal.size <= TINY_ORPHAN_WAL_BYTES) { + return { kind: 'tiny-orphan-wal', dbPath, walBytes: wal.size }; + } + return { kind: 'orphan-wal', dbPath, walBytes: wal.size }; + } + if (shadow) { + return { kind: 'orphan-shadow', dbPath, shadowBytes: shadow.size }; + } + return { kind: 'clean', dbPath }; +} + +export async function quarantineWalForMissingShadow( + dbPath: string, + options: { + logger: SidecarRecoveryLogger; + level?: 'debug' | 'info' | 'warn'; + reason?: string; + }, +): Promise { + const walPath = `${dbPath}.wal`; + const quarantinePath = `${walPath}.missing-shadow.${Date.now()}-${Math.random() + .toString(36) + .slice(2)}`; + await fs.rename(walPath, quarantinePath); + + const message = + `GitNexus: quarantined WAL ${path.basename(quarantinePath)} because LadybugDB shadow sidecar was missing; ` + + `continuing from last checkpoint${options.reason ? ` (${options.reason})` : ''}`; + + if (options.level === 'warn') { + warnOnce(options.logger, `${dbPath}:missing-shadow-quarantine`, message); + } else if (options.level === 'info') { + logInfo(options.logger, message); + } else { + logDebug(options.logger, message); + } + + return quarantinePath; +} + +export async function preflightLbugSidecars( + dbPath: string, + options: { + mode: 'read-only' | 'write'; + logger: SidecarRecoveryLogger; + allowQuarantine: boolean; + }, +): Promise { + let state: LbugSidecarState; + try { + state = await inspectLbugSidecars(dbPath); + } catch (err) { + logDebug( + options.logger, + `GitNexus: unable to inspect LadybugDB sidecars before ${options.mode} open; continuing without preflight repair: ${(err as Error).message}`, + ); + return { kind: 'clean', dbPath }; + } + if (sidecarPreflightDisabled() || !options.allowQuarantine) return state; + + if (state.kind === 'tiny-orphan-wal') { + await quarantineWalForMissingShadow(dbPath, { + logger: options.logger, + level: 'debug', + reason: `${options.mode} preflight tiny orphan WAL (${state.walBytes} bytes)`, + }); + return inspectLbugSidecars(dbPath); + } + + if (state.kind === 'orphan-wal') { + warnOnce( + options.logger, + `${dbPath}:orphan-wal-preflight:${options.mode}`, + `GitNexus: found ${state.walBytes} byte lbug.wal without lbug.shadow before ${options.mode} open; ` + + 'will rely on LadybugDB replay/recovery instead of deleting pending WAL data.', + ); + } + + return state; +} + +export async function finalizeLbugSidecarsAfterClose( + dbPath: string, + options: { logger: SidecarRecoveryLogger }, +): Promise { + if (sidecarPreflightDisabled()) return; + + let state: LbugSidecarState; + try { + state = await inspectLbugSidecars(dbPath); + } catch (err) { + logDebug( + options.logger, + `GitNexus: unable to inspect LadybugDB sidecars after close; skipping post-close repair: ${(err as Error).message}`, + ); + return; + } + if (state.kind === 'clean' || state.kind === 'wal-with-shadow') return; + + for (const delayMs of [25, 50, 100]) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + try { + state = await inspectLbugSidecars(dbPath); + } catch (err) { + logDebug( + options.logger, + `GitNexus: unable to inspect LadybugDB sidecars after close; skipping post-close repair: ${(err as Error).message}`, + ); + return; + } + if (state.kind === 'clean' || state.kind === 'wal-with-shadow') return; + } + + if (state.kind === 'tiny-orphan-wal') { + try { + await quarantineWalForMissingShadow(dbPath, { + logger: options.logger, + level: 'debug', + reason: `post-close tiny orphan WAL (${state.walBytes} bytes)`, + }); + } catch (err) { + if (!missing(err)) { + warnOnce( + options.logger, + `${dbPath}:post-close-tiny-quarantine-failed`, + `GitNexus: failed to quarantine tiny orphan WAL after close (${(err as Error).message}); next read may recover reactively.`, + ); + } + } + return; + } + + if (state.kind === 'orphan-wal') { + warnOnce( + options.logger, + `${dbPath}:post-close-orphan-wal`, + `GitNexus: lbug.wal (${state.walBytes} bytes) remains without lbug.shadow after close; ` + + 'keeping it for recovery. If this repeats, run `gitnexus analyze --force --index-only` or the sidecar repair command.', + ); + } +} + +export async function listQuarantinedMissingShadowWals(dbPath: string): Promise { + const dir = path.dirname(dbPath); + const base = path.basename(dbPath); + let entries: string[]; + try { + entries = await fs.readdir(dir); + } catch (err) { + if (missing(err)) return []; + throw err; + } + return entries + .filter((entry) => entry.startsWith(`${base}.wal.missing-shadow.`)) + .map((entry) => path.join(dir, entry)) + .sort(); +} + +export async function cleanQuarantinedMissingShadowWals(dbPath: string): Promise { + const files = await listQuarantinedMissingShadowWals(dbPath); + const deleted: string[] = []; + for (const file of files) { + await fs.unlink(file); + deleted.push(file); + } + return deleted; +} + +export const _resetSidecarRecoveryWarningsForTest = (): void => { + warnedKeys.clear(); +}; diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 796fa04e17..87053ffda1 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -130,6 +130,7 @@ export const isIgnorableGraphQueryError = (err: unknown): boolean => { }; export const SPA_FALLBACK_REGEX = /^(?!\/api(?:\/|$))(?!.*\.\w{1,10}$).*/; +export const PNA_PREFLIGHT_PATH_REGEX = /^\/.*$/; export const resolveWebDistDir = async ( primaryDir: string, @@ -718,7 +719,7 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => // on OPTIONS requests and expects the allow header in the response. // Note: the actual Allow-Private-Network header is already set by the global // middleware above, so we just need to call next() here. - app.options('*', (_req, res, next) => { + app.options(PNA_PREFLIGHT_PATH_REGEX, (_req, res, next) => { next(); }); diff --git a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts index c5f4b67730..8c29b203b3 100644 --- a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts +++ b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts @@ -41,6 +41,7 @@ function makeFsMock(dbPath: string) { throw ENOENT; }), unlink: vi.fn(async () => {}), + rename: vi.fn(async () => {}), mkdir: vi.fn(async () => {}), open: makeOpenMock(), }, @@ -78,8 +79,8 @@ describe('doInitLbug WAL corruption guard — structural', () => { expect(schemaLoopBody).toMatch(/await safeClose\(\)/); }); - it('WAL guard resets currentDbPath to null', () => { - expect(schemaLoopBody).toMatch(/currentDbPath = null/); + it('WAL guard resets open connection state', () => { + expect(schemaLoopBody).toMatch(/resetOpenConnectionState\(\)/); }); it('WAL guard throws with WAL_RECOVERY_SUGGESTION in the message', () => { @@ -211,6 +212,284 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { await adapter.closeLbug(); }); + it('quarantines the WAL and retries writable schema creation when shadow sidecar is missing', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-writable-shadow-missing/lbug'; + const missingShadowError = new Error( + `IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const firstConn = { + query: vi.fn().mockRejectedValueOnce(missingShadowError).mockResolvedValue(queryResult), + close: vi.fn(async () => {}), + }; + const firstDb = { close: vi.fn(async () => {}) }; + const recoveredConn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const recoveredDb = { close: vi.fn(async () => {}) }; + const openLbugConnectionMock = vi + .fn() + .mockResolvedValueOnce({ db: firstDb, conn: firstConn }) + .mockResolvedValueOnce({ db: recoveredDb, conn: recoveredConn }); + const fsMock = makeFsMock(dbPath); + const ensureMock = vi.fn(async () => false); + const warnMock = vi.fn(); + + vi.doMock('fs/promises', () => fsMock); + vi.doMock('../../src/core/lbug/schema.js', () => SCHEMA_MOCK); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: async (handle: { conn: typeof firstConn; db: typeof firstDb }) => { + await handle.conn.close(); + await handle.db.close(); + }, + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: + 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: ensureMock, + 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)).resolves.toBeDefined(); + + expect(openLbugConnectionMock).toHaveBeenCalledTimes(2); + expect(fsMock.default.rename).toHaveBeenCalledWith( + `${dbPath}.wal`, + expect.stringContaining(`${dbPath}.wal.missing-shadow.`), + ); + expect(recoveredConn.query).toHaveBeenCalledWith(SCHEMA_MOCK.SCHEMA_QUERIES[0]); + expect(warnMock).not.toHaveBeenCalledWith(expect.stringContaining('Schema creation warning')); + + await adapter.closeLbug(); + }); + + it('skips schema DDL and uses load-only FTS policy for read-only opens', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-readonly-schema-skip/lbug'; + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const openLbugConnectionMock = vi.fn(async () => ({ db, conn })); + const ensureMock = vi.fn(async () => false); + const warnMock = vi.fn(); + + vi.doMock('fs/promises', () => makeFsMock(dbPath)); + vi.doMock('../../src/core/lbug/schema.js', () => SCHEMA_MOCK); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: + 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: ensureMock, + 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.withLbugDb(dbPath, async () => 'ok', { readOnly: true })).resolves.toBe( + 'ok', + ); + + expect(openLbugConnectionMock).toHaveBeenCalledWith(expect.anything(), dbPath, { + readOnly: true, + }); + expect(conn.query).not.toHaveBeenCalledWith(SCHEMA_MOCK.SCHEMA_QUERIES[0]); + expect(ensureMock).toHaveBeenCalledWith(expect.any(Function), 'fts', 'FTS', { + policy: 'load-only', + }); + expect(warnMock).not.toHaveBeenCalledWith(expect.stringContaining('Schema creation warning')); + + await adapter.closeLbug(); + }); + + it('replays dirty shadow pages with a temporary writable open before read-only serving', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-readonly-shadow-replay/lbug'; + const shadowReplayError = new Error( + "Runtime exception: Couldn't replay shadow pages under read-only mode. Please re-open the database with read-write mode to replay shadow pages.", + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const readOnlyConn1 = { + query: vi.fn().mockRejectedValueOnce(shadowReplayError), + close: vi.fn(async () => {}), + }; + const readOnlyDb1 = { close: vi.fn(async () => {}) }; + const writableConn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const writableDb = { close: vi.fn(async () => {}) }; + const readOnlyConn2 = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const readOnlyDb2 = { close: vi.fn(async () => {}) }; + const openLbugConnectionMock = vi + .fn() + .mockResolvedValueOnce({ db: readOnlyDb1, conn: readOnlyConn1 }) + .mockResolvedValueOnce({ db: writableDb, conn: writableConn }) + .mockResolvedValueOnce({ db: readOnlyDb2, conn: readOnlyConn2 }); + const ensureMock = vi.fn(async () => false); + + vi.doMock('fs/promises', () => makeFsMock(dbPath)); + vi.doMock('../../src/core/lbug/schema.js', () => SCHEMA_MOCK); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: async (handle: { + conn: typeof readOnlyConn1; + db: typeof readOnlyDb1; + }) => { + await handle.conn.close(); + await handle.db.close(); + }, + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: + 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: ensureMock, + 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 () => 'ok', { readOnly: true })).resolves.toBe( + 'ok', + ); + + expect(openLbugConnectionMock).toHaveBeenNthCalledWith(1, expect.anything(), dbPath, { + readOnly: true, + }); + expect(openLbugConnectionMock).toHaveBeenNthCalledWith(2, expect.anything(), dbPath); + expect(openLbugConnectionMock).toHaveBeenNthCalledWith(3, expect.anything(), dbPath, { + readOnly: true, + }); + expect(readOnlyConn1.close).toHaveBeenCalled(); + expect(readOnlyDb1.close).toHaveBeenCalled(); + expect(writableConn.query).toHaveBeenCalledWith('MATCH (n) RETURN n LIMIT 1'); + expect(writableConn.close).toHaveBeenCalled(); + expect(writableDb.close).toHaveBeenCalled(); + expect(readOnlyConn2.query).toHaveBeenCalledWith('MATCH (n) RETURN n LIMIT 1'); + expect(ensureMock).toHaveBeenCalledWith(expect.any(Function), 'fts', 'FTS', { + policy: 'load-only', + }); + + await adapter.closeLbug(); + }); + + it('quarantines the WAL and reopens read-only when the shadow sidecar is missing', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-readonly-shadow-missing/lbug'; + const missingShadowError = new Error( + `IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`, + ); + const readOnlyConn = { + query: vi.fn().mockRejectedValueOnce(missingShadowError), + close: vi.fn(async () => {}), + }; + const readOnlyDb = { close: vi.fn(async () => {}) }; + const recoveredConn = { + query: vi.fn(async () => ({ getAll: vi.fn(async () => []), close: vi.fn() })), + close: vi.fn(async () => {}), + }; + const recoveredDb = { close: vi.fn(async () => {}) }; + const openLbugConnectionMock = vi + .fn() + .mockResolvedValueOnce({ + db: readOnlyDb, + conn: readOnlyConn, + }) + .mockResolvedValueOnce({ + db: recoveredDb, + conn: recoveredConn, + }); + const fsMock = makeFsMock(dbPath); + + vi.doMock('fs/promises', () => fsMock); + vi.doMock('../../src/core/lbug/schema.js', () => SCHEMA_MOCK); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: async (handle: { conn: typeof readOnlyConn; db: typeof readOnlyDb }) => { + await handle.conn.close(); + await handle.db.close(); + }, + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: + 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => false), + 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 () => 'ok', { readOnly: true })).resolves.toBe( + 'ok', + ); + expect(openLbugConnectionMock).toHaveBeenCalledTimes(2); + expect(readOnlyConn.close).toHaveBeenCalled(); + expect(readOnlyDb.close).toHaveBeenCalled(); + expect(fsMock.default.rename).toHaveBeenCalledWith( + `${dbPath}.wal`, + expect.stringContaining(`${dbPath}.wal.missing-shadow.`), + ); + + await adapter.closeLbug(); + }); + it('calls safeClose() (db.close) when WAL corruption is detected mid-schema', async () => { vi.resetModules(); diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index cda42806ab..0ccf8362a9 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -6,7 +6,8 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { stderrWriteMock } = vi.hoisted(() => ({ +const { connectionQueryMock, stderrWriteMock } = vi.hoisted(() => ({ + connectionQueryMock: vi.fn(), stderrWriteMock: vi.fn(), })); @@ -23,6 +24,7 @@ vi.mock('@ladybugdb/core', () => ({ Database: vi.fn(), Connection: vi.fn(function (this: any) { this.close = vi.fn().mockResolvedValue(undefined); + this.query = connectionQueryMock; }), }, })); @@ -68,6 +70,11 @@ describe('WAL corruption recovery in doInitLbug (#1402)', () => { (fs.rename as any).mockReset(); mockInit.mockReset(); mockClose.mockReset(); + connectionQueryMock.mockReset(); + connectionQueryMock.mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); mockInit.mockResolvedValue(undefined); mockClose.mockResolvedValue(undefined); (fs.stat as any).mockResolvedValue({}); @@ -110,6 +117,79 @@ describe('WAL corruption recovery in doInitLbug (#1402)', () => { ); }); + it('replays shadow pages with a temporary writable open before pooling read-only DBs', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-shadow-replay/lbug'; + + const readOnlyDb1 = makeMockDb(); + const writableDb = makeMockDb(); + const readOnlyDb2 = makeMockDb(); + connectionQueryMock + .mockRejectedValueOnce( + new Error( + "Runtime exception: Couldn't replay shadow pages under read-only mode. Please re-open the database with read-write mode to replay shadow pages.", + ), + ) + .mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + (createLbugDatabase as any) + .mockReturnValueOnce(readOnlyDb1) + .mockReturnValueOnce(writableDb) + .mockReturnValueOnce(readOnlyDb2); + + await initLbug('test-repo-shadow-replay', dbPath); + + expect(createLbugDatabase).toHaveBeenNthCalledWith( + 1, + expect.anything(), + dbPath, + expect.objectContaining({ readOnly: true, throwOnWalReplayFailure: false }), + ); + expect(createLbugDatabase).toHaveBeenNthCalledWith( + 2, + expect.anything(), + dbPath, + expect.objectContaining({ throwOnWalReplayFailure: false }), + ); + expect(createLbugDatabase).toHaveBeenNthCalledWith( + 3, + expect.anything(), + dbPath, + expect.objectContaining({ readOnly: true, throwOnWalReplayFailure: false }), + ); + expect(readOnlyDb1.close).toHaveBeenCalled(); + expect(writableDb.close).toHaveBeenCalled(); + expect(fs.rename).not.toHaveBeenCalled(); + }); + + it('quarantines WAL and reopens read-only when the Ladybug shadow sidecar is missing', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-shadow-missing/lbug'; + + const readOnlyDb1 = makeMockDb(); + const readOnlyDb2 = makeMockDb(); + connectionQueryMock + .mockRejectedValueOnce( + new Error(`IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`), + ) + .mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1).mockReturnValueOnce(readOnlyDb2); + + await initLbug('test-repo-shadow-missing', dbPath); + + expect(createLbugDatabase).toHaveBeenCalledTimes(2); + expect(readOnlyDb1.close).toHaveBeenCalled(); + expect(fs.rename).toHaveBeenCalledWith( + dbPath + '.wal', + expect.stringContaining('.wal.missing-shadow.'), + ); + }); + it('does not quarantine on lock error (preserves existing lock retry)', async () => { const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); const setTimeoutSpy = vi.spyOn(global, 'setTimeout').mockImplementation((callback: any) => { diff --git a/gitnexus/test/unit/sidecar-recovery.test.ts b/gitnexus/test/unit/sidecar-recovery.test.ts new file mode 100644 index 0000000000..994f6e4e2a --- /dev/null +++ b/gitnexus/test/unit/sidecar-recovery.test.ts @@ -0,0 +1,123 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { + _resetSidecarRecoveryWarningsForTest, + finalizeLbugSidecarsAfterClose, + inspectLbugSidecars, + listQuarantinedMissingShadowWals, + preflightLbugSidecars, + TINY_ORPHAN_WAL_BYTES, +} from '../../src/core/lbug/sidecar-recovery.js'; + +const logger = () => ({ warn: vi.fn(), info: vi.fn(), debug: vi.fn() }); + +describe('LadybugDB sidecar recovery', () => { + let dir: string; + let dbPath: string; + + beforeEach(async () => { + _resetSidecarRecoveryWarningsForTest(); + dir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-sidecar-recovery-')); + dbPath = path.join(dir, 'lbug'); + await fs.writeFile(dbPath, 'db'); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await fs.rm(dir, { recursive: true, force: true }); + }); + + it('classifies clean sidecars', async () => { + await expect(inspectLbugSidecars(dbPath)).resolves.toEqual({ kind: 'clean', dbPath }); + }); + + it('classifies WAL with shadow as replayable by LadybugDB', async () => { + await fs.writeFile(`${dbPath}.wal`, Buffer.alloc(128)); + await fs.writeFile(`${dbPath}.shadow`, Buffer.alloc(64)); + + await expect(inspectLbugSidecars(dbPath)).resolves.toEqual({ + kind: 'wal-with-shadow', + dbPath, + walBytes: 128, + shadowBytes: 64, + }); + }); + + it('preflight quarantines tiny orphan WAL without WARN noise', async () => { + await fs.writeFile(`${dbPath}.wal`, Buffer.alloc(34)); + const log = logger(); + + const state = await preflightLbugSidecars(dbPath, { + mode: 'read-only', + logger: log, + allowQuarantine: true, + }); + + expect(state.kind).toBe('clean'); + await expect(fs.stat(`${dbPath}.wal`)).rejects.toMatchObject({ code: 'ENOENT' }); + const files = await fs.readdir(dir); + expect(files.some((file) => file.startsWith('lbug.wal.missing-shadow.'))).toBe(true); + expect(log.warn).not.toHaveBeenCalled(); + expect(log.debug).toHaveBeenCalledWith(expect.stringContaining('preflight tiny orphan WAL')); + }); + + it('does not silently quarantine large orphan WAL during preflight', async () => { + await fs.writeFile(`${dbPath}.wal`, Buffer.alloc(TINY_ORPHAN_WAL_BYTES + 1)); + const log = logger(); + + const state = await preflightLbugSidecars(dbPath, { + mode: 'read-only', + logger: log, + allowQuarantine: true, + }); + + expect(state).toEqual({ + kind: 'orphan-wal', + dbPath, + walBytes: TINY_ORPHAN_WAL_BYTES + 1, + }); + await expect(fs.stat(`${dbPath}.wal`)).resolves.toBeDefined(); + expect(log.warn).toHaveBeenCalledTimes(1); + }); + + it('finalize quarantines tiny orphan WAL after close', async () => { + await fs.writeFile(`${dbPath}.wal`, Buffer.alloc(34)); + const log = logger(); + + await finalizeLbugSidecarsAfterClose(dbPath, { logger: log }); + + await expect(fs.stat(`${dbPath}.wal`)).rejects.toMatchObject({ code: 'ENOENT' }); + const files = await fs.readdir(dir); + expect(files.some((file) => file.startsWith('lbug.wal.missing-shadow.'))).toBe(true); + expect(log.warn).not.toHaveBeenCalled(); + }); + + it('can be disabled through GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT', async () => { + vi.stubEnv('GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT', '1'); + await fs.writeFile(`${dbPath}.wal`, Buffer.alloc(34)); + const log = logger(); + + const state = await preflightLbugSidecars(dbPath, { + mode: 'read-only', + logger: log, + allowQuarantine: true, + }); + + expect(state.kind).toBe('tiny-orphan-wal'); + await expect(fs.stat(`${dbPath}.wal`)).resolves.toBeDefined(); + }); + + it('lists only missing-shadow WAL quarantine files for cleanup', async () => { + await fs.writeFile(`${dbPath}.wal.missing-shadow.1-a`, ''); + await fs.writeFile(`${dbPath}.wal.missing-shadow.2-b`, ''); + await fs.writeFile(`${dbPath}.wal.corrupt.3-c`, ''); + await fs.writeFile(path.join(dir, 'other.wal.missing-shadow.4-d'), ''); + + await expect(listQuarantinedMissingShadowWals(dbPath)).resolves.toEqual([ + `${dbPath}.wal.missing-shadow.1-a`, + `${dbPath}.wal.missing-shadow.2-b`, + ]); + }); +}); diff --git a/gitnexus/test/unit/web-ui-serving.test.ts b/gitnexus/test/unit/web-ui-serving.test.ts index bd1c7faaf2..4099bebfa6 100644 --- a/gitnexus/test/unit/web-ui-serving.test.ts +++ b/gitnexus/test/unit/web-ui-serving.test.ts @@ -17,6 +17,7 @@ import { registerWebUI, resolveWebDistDir, landingPageHtml, + PNA_PREFLIGHT_PATH_REGEX, SPA_FALLBACK_REGEX, staticCacheControlSetHeaders, } from '../../src/server/api.js'; @@ -323,4 +324,11 @@ describe('Real Express dispatch — API and asset isolation', () => { const status = await makeRequest(app, 'GET', '/'); expect(status).toBe(200); }); + + it('registers the global OPTIONS preflight matcher without Express 5 wildcard errors', async () => { + const app = express(); + expect(() => app.options(PNA_PREFLIGHT_PATH_REGEX, (_req, _res, next) => next())).not.toThrow(); + const status = await makeRequest(app, 'OPTIONS', '/api/health'); + expect(status).toBe(404); + }); }); From 08c910797d41b301a9e1c657c054f0a0c81c6dbe Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 21 May 2026 11:13:07 +0100 Subject: [PATCH 2/5] fix(lbug): pool-caller ENOENT guard, symmetric size gate, permission-aware errors (PR #1747 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the production-readiness review of PR #1747 (Findings 1, 2, 3 of 6). Findings 4, 5, 6 are deferred to follow-ups per the plan. 1. ENOENT-tolerance scoped to pool-adapter callers only - `quarantineWalForMissingShadow` stays strict in `sidecar-recovery.ts`. The direct adapter calls it inside `acquireInitLock` (cross-process file lock) — ENOENT there means the file vanished under lock and remains a real bug to surface. - New `tryQuarantineForMissingShadow` local helper in `pool-adapter.ts` returns a discriminated union { kind: 'quarantined', path } | { kind: 'peer-handled' }. Catches ENOENT, re-verifies via statIfExists, and converts to 'peer-handled' only when WAL really is gone. Defensive: if ENOENT but WAL still present, throws as classified error rather than silently returning success. 2. Symmetric WAL-size gate on both recovery paths - `refuseLargeWalQuarantine` applied in both `reopenReadOnlyAfterMissingShadow` and `reopenWritableAfterMissingShadow`. Closes the read-only data-loss vector (large orphan WAL silently discarded would never be replayed by a later writable open). 3. Permission-aware error classifier - New `renameFailureMessage` and `isPermissionRenameError` in `sidecar-recovery.ts`. EACCES / EPERM / EBUSY now surface a permission-specific message pointing at ACLs, AV exclusions, and file-locks. Other codes (ENOSPC, EROFS, EIO, ENOENT) fall through to `shadowSidecarRecoveryMessage`. - Used at both pool-adapter and direct-adapter caller catches around `quarantineWalForMissingShadow`. - `doInitLbug`'s pass-through classifier extended to include the new permission message. The lock-retry substring match tightened so "file-lock error" in the permission message is not mistaken for a LadybugDB lock-retry trigger. Tests - sidecar-recovery.test.ts: 7 new tests for `renameFailureMessage` and `isPermissionRenameError`. - pool-wal-recovery.test.ts: 6 new tests covering ENOENT race, EACCES/EPERM/EBUSY classification, ENOSPC fallthrough, and the defensive "WAL still present after ENOENT" branch. - lbug-adapter-wal-schema.test.ts: 5 new tests covering the symmetric size gate on both recovery paths, including the boundary at exactly TINY_ORPHAN_WAL_BYTES (4096) and the off-by-one at 4097. Deferred (tracked as follow-up work) - Brittle LadybugDB error-string matching (Finding 4). - PNA header end-to-end coverage gap (Finding 5). - warnedKeys module-global persistence (Finding 6). - Cross-process init lock for pool-adapter. --- gitnexus/src/core/lbug/lbug-adapter.ts | 38 +++- gitnexus/src/core/lbug/pool-adapter.ts | 86 ++++++-- gitnexus/src/core/lbug/sidecar-recovery.ts | 45 ++++- .../test/unit/lbug-adapter-wal-schema.test.ts | 187 ++++++++++++++++++ gitnexus/test/unit/pool-wal-recovery.test.ts | 150 ++++++++++++++ gitnexus/test/unit/sidecar-recovery.test.ts | 72 +++++++ 6 files changed, 551 insertions(+), 27 deletions(-) diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index f9291bcc70..25fa7ed5e9 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -29,9 +29,11 @@ import { } from './lbug-config.js'; import { finalizeLbugSidecarsAfterClose, + inspectLbugSidecars, isMissingShadowSidecarError, preflightLbugSidecars, quarantineWalForMissingShadow, + renameFailureMessage, shadowSidecarRecoveryMessage, } from './sidecar-recovery.js'; import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js'; @@ -451,18 +453,45 @@ const isReadOnlyShadowReplayError = (err: unknown): boolean => { return /replay shadow pages under read-only mode/i.test(msg); }; +/** + * Reject the quarantine path when the orphan WAL is too large to safely + * discard (>TINY_ORPHAN_WAL_BYTES). Mirrors the preflight policy at + * sidecar-recovery.ts:153-160 ("warn, do not quarantine"). Symmetric across + * read-only and writable recovery paths (PR #1747 review D2). + * + * Throws shadowSidecarRecoveryMessage immediately when the WAL is large, + * preserving the uncheckpointed pages for explicit operator recovery. + * Returns silently when the WAL is absent, tiny, or in any other state + * where the existing recovery path is safe to proceed. + */ +const refuseLargeWalQuarantine = async ( + dbPath: string, + mode: 'read-only' | 'writable', + triggeringErr: unknown, +): Promise => { + const state = await inspectLbugSidecars(dbPath); + if (state.kind === 'orphan-wal') { + logger.warn( + `GitNexus: refusing to quarantine large WAL (${state.walBytes} bytes) at ${dbPath}.wal during ${mode} recovery; ` + + 'manual recovery required — run `gitnexus analyze --force --index-only`.', + ); + throw new Error(shadowSidecarRecoveryMessage(dbPath, triggeringErr)); + } +}; + const reopenReadOnlyAfterMissingShadow = async ( dbPath: string, err: unknown, ): Promise => { + await refuseLargeWalQuarantine(dbPath, 'read-only', err); try { await quarantineWalForMissingShadow(dbPath, { logger, level: 'warn', reason: 'read-only recovery', }); - } catch { - throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } catch (renameErr) { + throw new Error(renameFailureMessage(dbPath, renameErr)); } const reopened = await openLbugConnection(lbug, dbPath, { readOnly: true }); @@ -482,14 +511,15 @@ const reopenWritableAfterMissingShadow = async ( dbPath: string, err: unknown, ): Promise => { + await refuseLargeWalQuarantine(dbPath, 'writable', err); try { await quarantineWalForMissingShadow(dbPath, { logger, level: 'warn', reason: 'writable recovery', }); - } catch { - throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); + } catch (renameErr) { + throw new Error(renameFailureMessage(dbPath, renameErr)); } return await openLbugConnection(lbug, dbPath); diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index e0dfdab7a4..0ba7cdf9d1 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -26,10 +26,12 @@ import { WAL_RECOVERY_SUGGESTION, } from './lbug-config.js'; import { + isMissingFsError, isMissingShadowSidecarError, preflightLbugSidecars, quarantineWalForMissingShadow, - shadowSidecarRecoveryMessage, + renameFailureMessage, + statIfExists, } from './sidecar-recovery.js'; /** @@ -327,6 +329,58 @@ const poolSidecarLogger = { }, }; +type TryQuarantineResult = + | { kind: 'quarantined'; path: string } + | { kind: 'peer-handled' }; + +/** + * Pool-local quarantine guard that tolerates the concurrent-peer race the + * direct adapter does NOT face (the direct adapter holds `acquireInitLock`, + * a cross-process file lock, around its quarantine calls — so any ENOENT + * there is a real bug, not a benign race). + * + * On ENOENT from `fs.rename`, re-inspects via `statIfExists` to confirm the + * WAL really is gone. If gone, returns `{ kind: 'peer-handled' }`. If the + * WAL is somehow still present after the ENOENT (filesystem race we don't + * fully model), re-throws as a classified error rather than silently + * returning success — preserves the lock-invariant principle at the pool + * sites too. + * + * On any non-ENOENT failure, classifies through `renameFailureMessage`: + * EACCES/EPERM/EBUSY → permission-specific message; everything else + * (including the LadybugDB missing-shadow error if it ever propagates here) + * → `shadowSidecarRecoveryMessage`. + * + * See plan: docs/plans/2026-05-21-001-fix-pr-1747-quarantine-enoent-and-large-wal-plan.md (U2) + */ +async function tryQuarantineForMissingShadow( + dbPath: string, + opts: { reason: string }, +): Promise { + try { + const quarantinePath = await quarantineWalForMissingShadow(dbPath, { + logger: poolSidecarLogger, + level: 'warn', + reason: opts.reason, + }); + return { kind: 'quarantined', path: quarantinePath }; + } catch (err) { + if (isMissingFsError(err)) { + const walStat = await statIfExists(`${dbPath}.wal`); + if (walStat === null) { + return { kind: 'peer-handled' }; + } + // Defensive: ENOENT during rename but WAL still present afterwards. + // Don't silently swallow — surface a classified error. ENOENT falls + // through to shadowSidecarRecoveryMessage in renameFailureMessage. + throw new Error(renameFailureMessage(dbPath, err)); + } + // Classify the rename failure itself — EACCES/EPERM/EBUSY get the + // permission-specific message; everything else falls through. + throw new Error(renameFailureMessage(dbPath, err)); + } +} + async function probeDatabaseForShadowReplay(db: lbug.Database): Promise { const conn = createConnection(db); try { @@ -347,16 +401,10 @@ async function replayShadowPagesWithWritableOpen(dbPath: string): Promise await probeDatabaseForShadowReplay(db); } catch (err) { if (isMissingShadowSidecarError(err)) { - try { - await quarantineWalForMissingShadow(dbPath, { - logger: poolSidecarLogger, - level: 'warn', - reason: 'pool writable replay recovery', - }); - return; - } catch { - throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); - } + await tryQuarantineForMissingShadow(dbPath, { + reason: 'pool writable replay recovery', + }); + return; } throw err; } finally { @@ -384,15 +432,9 @@ async function openReadOnlyDatabase(dbPath: string): Promise { if (isMissingShadowSidecarError(err)) { await db.close().catch(() => {}); db = undefined; - try { - await quarantineWalForMissingShadow(dbPath, { - logger: poolSidecarLogger, - level: 'warn', - reason: 'pool read-only recovery', - }); - } catch { - throw new Error(shadowSidecarRecoveryMessage(dbPath, err)); - } + await tryQuarantineForMissingShadow(dbPath, { + reason: 'pool read-only recovery', + }); await preflightLbugSidecars(dbPath, { mode: 'read-only', logger: poolSidecarLogger, @@ -530,13 +572,15 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { if ( lastError.message.startsWith('LadybugDB checkpoint sidecar is missing') || + lastError.message.startsWith('GitNexus could not move the LadybugDB WAL sidecar') || isMissingShadowSidecarError(lastError) ) { throw lastError; } const isLockError = - lastError.message.includes('Could not set lock') || lastError.message.includes('lock'); + 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)); } diff --git a/gitnexus/src/core/lbug/sidecar-recovery.ts b/gitnexus/src/core/lbug/sidecar-recovery.ts index 08cc76f9b1..8faa327b76 100644 --- a/gitnexus/src/core/lbug/sidecar-recovery.ts +++ b/gitnexus/src/core/lbug/sidecar-recovery.ts @@ -18,13 +18,15 @@ export const TINY_ORPHAN_WAL_BYTES = 4 * 1024; const warnedKeys = new Set(); -const missing = (err: unknown): boolean => +export const isMissingFsError = (err: unknown): boolean => (err as NodeJS.ErrnoException | undefined)?.code === 'ENOENT'; +const missing = isMissingFsError; + const sidecarPreflightDisabled = (): boolean => /^(1|true|yes|on)$/i.test(process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT ?? ''); -const statIfExists = async (filePath: string): Promise<{ size: number } | null> => { +export const statIfExists = async (filePath: string): Promise<{ size: number } | null> => { try { const statFn = (fs as typeof fs & { stat?: typeof fs.stat }).stat; if (typeof statFn === 'function') { @@ -73,6 +75,45 @@ export const shadowSidecarRecoveryMessage = (dbPath: string, err: unknown): stri ); }; +const PERMISSION_RENAME_CODES = new Set(['EACCES', 'EPERM', 'EBUSY']); + +export const isPermissionRenameError = (err: unknown): boolean => { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + return typeof code === 'string' && PERMISSION_RENAME_CODES.has(code); +}; + +/** + * Classify a failure surfaced by quarantine rename into an actionable user-facing + * message. + * + * - EACCES / EPERM / EBUSY → permission-specific message pointing at filesystem + * ACLs, AV exclusions, and file-locks. Importantly does NOT instruct the user + * to rebuild the index — the underlying problem is environmental, not data + * integrity, and re-running after fixing the lock/permission will succeed. + * - Everything else (including the LadybugDB "Cannot open file *.shadow" + * missing-shadow error, ENOSPC, EROFS, EIO, and any other thrown Error) → + * falls back to `shadowSidecarRecoveryMessage`, preserving today's behavior. + * + * Use at caller catches around `quarantineWalForMissingShadow` and any other + * path where an `fs.rename`-class failure may surface to operators. + */ +export const renameFailureMessage = (dbPath: string, err: unknown): string => { + if (isPermissionRenameError(err)) { + const code = (err as NodeJS.ErrnoException).code; + const msg = err instanceof Error ? err.message : String(err); + return ( + `GitNexus could not move the LadybugDB WAL sidecar at ${dbPath}.wal because of a ` + + `filesystem permission or file-lock error (${code}). ` + + 'Check filesystem ACLs, antivirus exclusions for the index directory, and ' + + 'whether another process holds an open handle on the file. ' + + 'The index does not need to be rebuilt — re-running the failing command after ' + + 'resolving the lock or permission should succeed.' + + `\n Original error: ${msg.slice(0, 200)}` + ); + } + return shadowSidecarRecoveryMessage(dbPath, err); +}; + export async function inspectLbugSidecars(dbPath: string): Promise { const wal = await statIfExists(`${dbPath}.wal`); const shadow = await statIfExists(`${dbPath}.shadow`); diff --git a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts index 8c29b203b3..66dfa5f596 100644 --- a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts +++ b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts @@ -536,3 +536,190 @@ describe('doInitLbug WAL corruption guard — behavioural', () => { expect(db.close).toHaveBeenCalled(); }); }); + +// ─── Symmetric WAL-size gate (PR #1747 review, D2) ────────────────────────── +// +// Both reopenWritableAfterMissingShadow and reopenReadOnlyAfterMissingShadow +// must refuse to quarantine a WAL larger than TINY_ORPHAN_WAL_BYTES (4096). +// The pre-PR behavior silently quarantined any size of WAL during recovery — +// on the read-only path this could permanently orphan uncheckpointed pages +// because a later writable open would see a `clean` state and never replay. + +const TINY_ORPHAN_WAL_BYTES_TEST = 4 * 1024; + +/** + * Variant of makeFsMock where the `.wal` path is classified by + * inspectLbugSidecars based on a chosen size. Use to drive the + * `orphan-wal` vs `tiny-orphan-wal` branches of refuseLargeWalQuarantine + * without spinning up real files. + */ +function makeFsMockWithWalSize(dbPath: string, walBytes: number | 'missing') { + const ENOENT = Object.assign(new Error(`ENOENT: ${dbPath}`), { code: 'ENOENT' }); + const isWal = (p: string): boolean => p === `${dbPath}.wal`; + const isShadow = (p: string): boolean => p === `${dbPath}.shadow`; + return { + default: { + lstat: vi.fn(async () => { + throw ENOENT; + }), + access: vi.fn(async (p: string) => { + if (isWal(p) && walBytes !== 'missing') return; + throw ENOENT; + }), + stat: vi.fn(async (p: string) => { + if (isWal(p)) { + if (walBytes === 'missing') throw ENOENT; + return { size: walBytes }; + } + if (isShadow(p)) throw ENOENT; + return { size: 0 }; + }), + unlink: vi.fn(async () => {}), + rename: vi.fn(async () => {}), + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + }; +} + +describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', () => { + afterEach(() => { + vi.resetModules(); + vi.unstubAllEnvs(); + }); + + const setupShadowMissingRecovery = ( + dbPath: string, + walBytes: number | 'missing', + ) => { + const missingShadowError = new Error( + `IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const firstConn = { + query: vi.fn().mockRejectedValueOnce(missingShadowError).mockResolvedValue(queryResult), + close: vi.fn(async () => {}), + }; + const firstDb = { close: vi.fn(async () => {}) }; + const recoveredConn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const recoveredDb = { close: vi.fn(async () => {}) }; + const openLbugConnectionMock = vi + .fn() + .mockResolvedValueOnce({ db: firstDb, conn: firstConn }) + .mockResolvedValueOnce({ db: recoveredDb, conn: recoveredConn }); + const fsMock = makeFsMockWithWalSize(dbPath, walBytes); + const warnMock = vi.fn(); + + vi.doMock('fs/promises', () => fsMock); + vi.doMock('../../src/core/lbug/schema.js', () => SCHEMA_MOCK); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: async (handle: { conn: typeof firstConn; db: typeof firstDb }) => { + await handle.conn.close(); + await handle.db.close(); + }, + isDbBusyError: vi.fn(() => false), + isOpenRetryExhausted: vi.fn(() => false), + isWalCorruptionError: vi.fn(() => false), + WAL_RECOVERY_SUGGESTION: + 'WAL corruption detected. Run `gitnexus analyze --force` to rebuild the index.', + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => false), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { warn: warnMock, info: vi.fn(), error: vi.fn(), debug: vi.fn() }, + })); + + return { fsMock, openLbugConnectionMock, warnMock }; + }; + + it('writable recovery: refuses to quarantine a large WAL (4097 bytes) and throws shadow-recovery message', async () => { + vi.resetModules(); + const dbPath = '/tmp/gitnexus-lbug-large-wal-writable/lbug'; + const { fsMock, warnMock } = setupShadowMissingRecovery( + dbPath, + TINY_ORPHAN_WAL_BYTES_TEST + 1, + ); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(adapter.initLbug(dbPath)).rejects.toThrow( + /LadybugDB checkpoint sidecar is missing/, + ); + expect(fsMock.default.rename).not.toHaveBeenCalled(); + expect(warnMock).toHaveBeenCalledWith( + expect.stringContaining('refusing to quarantine large WAL'), + ); + expect(warnMock).toHaveBeenCalledWith(expect.stringContaining('writable recovery')); + }); + + it('read-only recovery: refuses to quarantine a large WAL (4097 bytes) and throws shadow-recovery message', async () => { + vi.resetModules(); + const dbPath = '/tmp/gitnexus-lbug-large-wal-readonly/lbug'; + const { fsMock, warnMock } = setupShadowMissingRecovery( + dbPath, + TINY_ORPHAN_WAL_BYTES_TEST + 1, + ); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await expect( + adapter.withLbugDb(dbPath, async () => 'unreached', { readOnly: true }), + ).rejects.toThrow(/LadybugDB checkpoint sidecar is missing/); + expect(fsMock.default.rename).not.toHaveBeenCalled(); + expect(warnMock).toHaveBeenCalledWith( + expect.stringContaining('refusing to quarantine large WAL'), + ); + expect(warnMock).toHaveBeenCalledWith(expect.stringContaining('read-only recovery')); + }); + + it('writable recovery: WAL at exactly TINY_ORPHAN_WAL_BYTES (4096 bytes) is treated as tiny and quarantined', async () => { + vi.resetModules(); + const dbPath = '/tmp/gitnexus-lbug-boundary-tiny/lbug'; + const { fsMock } = setupShadowMissingRecovery(dbPath, TINY_ORPHAN_WAL_BYTES_TEST); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(adapter.initLbug(dbPath)).resolves.toBeDefined(); + expect(fsMock.default.rename).toHaveBeenCalledWith( + `${dbPath}.wal`, + expect.stringContaining(`${dbPath}.wal.missing-shadow.`), + ); + await adapter.closeLbug(); + }); + + it('writable recovery: WAL at TINY_ORPHAN_WAL_BYTES + 1 (4097 bytes) is treated as orphan-wal and refused', async () => { + vi.resetModules(); + const dbPath = '/tmp/gitnexus-lbug-boundary-large/lbug'; + const { fsMock } = setupShadowMissingRecovery(dbPath, TINY_ORPHAN_WAL_BYTES_TEST + 1); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(adapter.initLbug(dbPath)).rejects.toThrow(); + expect(fsMock.default.rename).not.toHaveBeenCalled(); + }); + + it('tiny-WAL recovery path: writable recovery still quarantines and proceeds for a 1024-byte WAL', async () => { + vi.resetModules(); + const dbPath = '/tmp/gitnexus-lbug-tiny-wal/lbug'; + const { fsMock } = setupShadowMissingRecovery(dbPath, 1024); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(adapter.initLbug(dbPath)).resolves.toBeDefined(); + expect(fsMock.default.rename).toHaveBeenCalledWith( + `${dbPath}.wal`, + expect.stringContaining(`${dbPath}.wal.missing-shadow.`), + ); + await adapter.closeLbug(); + }); +}); diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index 0ccf8362a9..5da318ba85 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -257,3 +257,153 @@ describe('WAL corruption recovery in doInitLbug (#1402)', () => { await expect(initLbug('test-repo-enoent', dbPath)).rejects.toThrow(/gitnexus analyze/); }); }); + +describe('Pool-adapter missing-shadow quarantine: TOCTOU + permission classification (PR #1747 review)', () => { + beforeEach(() => { + (createLbugDatabase as any).mockReset(); + (fs.stat as any).mockReset(); + (fs.rename as any).mockReset(); + mockInit.mockReset(); + mockClose.mockReset(); + connectionQueryMock.mockReset(); + connectionQueryMock.mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + mockInit.mockResolvedValue(undefined); + mockClose.mockResolvedValue(undefined); + (fs.stat as any).mockResolvedValue({ size: 128 }); + (fs.rename as any).mockResolvedValue(undefined); + }); + + afterEach(async () => { + vi.useRealTimers(); + await closeLbug().catch(() => {}); + vi.clearAllMocks(); + }); + + const enoent = (): NodeJS.ErrnoException => { + const e = new Error('ENOENT: peer already moved it') as NodeJS.ErrnoException; + e.code = 'ENOENT'; + return e; + }; + const fsErr = (code: string): NodeJS.ErrnoException => { + const e = new Error(`simulated ${code}`) as NodeJS.ErrnoException; + e.code = code; + return e; + }; + const shadowError = (dbPath: string): Error => + new Error(`IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`); + + /** + * Make fs.stat ENOENT for the .wal path only — simulates "peer process + * already quarantined the WAL". Other paths (the main dbPath, .shadow) + * resolve normally so doInitLbug's existence check and preflight don't trip. + */ + const stubWalGoneAfterRename = (walPath: string): void => { + (fs.stat as any).mockImplementation((p: string) => { + if (p === walPath) return Promise.reject(enoent()); + return Promise.resolve({ size: 128 }); + }); + }; + + it('treats ENOENT on rename as peer-handled when WAL is confirmed gone (openReadOnlyDatabase)', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-enoent-race/lbug'; + + stubWalGoneAfterRename(`${dbPath}.wal`); + (fs.rename as any).mockRejectedValueOnce(enoent()); + + const readOnlyDb1 = makeMockDb(); + const readOnlyDb2 = makeMockDb(); + connectionQueryMock + .mockRejectedValueOnce(shadowError(dbPath)) + .mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + (createLbugDatabase as any) + .mockReturnValueOnce(readOnlyDb1) + .mockReturnValueOnce(readOnlyDb2); + + await initLbug('test-repo-pool-enoent', dbPath); + + expect(createLbugDatabase).toHaveBeenCalledTimes(2); + expect(readOnlyDb1.close).toHaveBeenCalled(); + expect(fs.rename).toHaveBeenCalledWith( + `${dbPath}.wal`, + expect.stringContaining('.wal.missing-shadow.'), + ); + }); + + it('classifies EACCES on rename with permission-specific message (openReadOnlyDatabase)', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-eacces/lbug'; + + (fs.rename as any).mockRejectedValueOnce(fsErr('EACCES')); + + const readOnlyDb1 = makeMockDb(); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1); + + await expect(initLbug('test-repo-pool-eacces', dbPath)).rejects.toThrow( + /EACCES.*permission|permission.*EACCES|file-lock.*EACCES|EACCES.*file-lock/s, + ); + }); + + it('classifies EPERM on rename with permission-specific message', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-eperm/lbug'; + + (fs.rename as any).mockRejectedValueOnce(fsErr('EPERM')); + + const readOnlyDb1 = makeMockDb(); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1); + + await expect(initLbug('test-repo-pool-eperm', dbPath)).rejects.toThrow(/EPERM/); + }); + + it('classifies EBUSY on rename with permission-specific message (common on Windows under AV)', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-ebusy/lbug'; + + (fs.rename as any).mockRejectedValueOnce(fsErr('EBUSY')); + + const readOnlyDb1 = makeMockDb(); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1); + + await expect(initLbug('test-repo-pool-ebusy', dbPath)).rejects.toThrow(/EBUSY/); + }); + + it('falls through to shadowSidecarRecoveryMessage for ENOSPC on rename', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-enospc/lbug'; + + (fs.rename as any).mockRejectedValueOnce(fsErr('ENOSPC')); + + const readOnlyDb1 = makeMockDb(); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1); + + await expect(initLbug('test-repo-pool-enospc', dbPath)).rejects.toThrow(/Rebuild the index/); + }); + + it('defensive: ENOENT on rename but WAL still present → classified error (not silent peer-handled)', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-pool-defensive/lbug'; + + // Note: NOT calling stubWalGoneAfterRename — fs.stat defaults to resolve. + (fs.rename as any).mockRejectedValueOnce(enoent()); + + const readOnlyDb1 = makeMockDb(); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1); + + // ENOENT → defensive branch sees WAL still present → throws classified error. + // Since ENOENT does not match permission codes, classifier falls through to + // shadowSidecarRecoveryMessage. + await expect(initLbug('test-repo-pool-defensive', dbPath)).rejects.toThrow(/Rebuild the index/); + }); +}); diff --git a/gitnexus/test/unit/sidecar-recovery.test.ts b/gitnexus/test/unit/sidecar-recovery.test.ts index 994f6e4e2a..2288b7f5dc 100644 --- a/gitnexus/test/unit/sidecar-recovery.test.ts +++ b/gitnexus/test/unit/sidecar-recovery.test.ts @@ -6,8 +6,11 @@ import { _resetSidecarRecoveryWarningsForTest, finalizeLbugSidecarsAfterClose, inspectLbugSidecars, + isPermissionRenameError, listQuarantinedMissingShadowWals, preflightLbugSidecars, + renameFailureMessage, + shadowSidecarRecoveryMessage, TINY_ORPHAN_WAL_BYTES, } from '../../src/core/lbug/sidecar-recovery.js'; @@ -109,6 +112,75 @@ describe('LadybugDB sidecar recovery', () => { await expect(fs.stat(`${dbPath}.wal`)).resolves.toBeDefined(); }); + describe('renameFailureMessage classifier (PR #1747 review)', () => { + const fsErr = (code: string, message = `simulated ${code}`): NodeJS.ErrnoException => { + const e = new Error(message) as NodeJS.ErrnoException; + e.code = code; + return e; + }; + + it('classifies EACCES as a permission/file-lock error (not "rebuild")', () => { + const out = renameFailureMessage('/tmp/lbug', fsErr('EACCES', 'permission denied')); + expect(out).toContain('/tmp/lbug.wal'); + expect(out).toContain('EACCES'); + expect(out).toContain('permission'); + expect(out).not.toContain('Rebuild the index'); + }); + + it('classifies EPERM as a permission/file-lock error', () => { + const out = renameFailureMessage('/tmp/lbug', fsErr('EPERM')); + expect(out).toContain('EPERM'); + expect(out).not.toContain('Rebuild the index'); + }); + + it('classifies EBUSY as a permission/file-lock error (common on Windows under AV)', () => { + const out = renameFailureMessage('/tmp/lbug', fsErr('EBUSY')); + expect(out).toContain('EBUSY'); + expect(out).not.toContain('Rebuild the index'); + }); + + it('falls through to shadowSidecarRecoveryMessage for the LadybugDB missing-shadow error', () => { + const shadowErr = new Error('Cannot open file /tmp/lbug.shadow: No such file or directory'); + expect(renameFailureMessage('/tmp/lbug', shadowErr)).toBe( + shadowSidecarRecoveryMessage('/tmp/lbug', shadowErr), + ); + }); + + it('falls through to shadowSidecarRecoveryMessage for ENOSPC (residual; flagged in plan)', () => { + const err = fsErr('ENOSPC'); + expect(renameFailureMessage('/tmp/lbug', err)).toBe( + shadowSidecarRecoveryMessage('/tmp/lbug', err), + ); + }); + + it('falls through to shadowSidecarRecoveryMessage for EROFS and EIO (residual; flagged in plan)', () => { + const eRofs = fsErr('EROFS'); + const eIo = fsErr('EIO'); + expect(renameFailureMessage('/tmp/lbug', eRofs)).toBe( + shadowSidecarRecoveryMessage('/tmp/lbug', eRofs), + ); + expect(renameFailureMessage('/tmp/lbug', eIo)).toBe( + shadowSidecarRecoveryMessage('/tmp/lbug', eIo), + ); + }); + + it('falls through to shadowSidecarRecoveryMessage for a generic Error without a code', () => { + const generic = new Error('something else broke'); + expect(renameFailureMessage('/tmp/lbug', generic)).toBe( + shadowSidecarRecoveryMessage('/tmp/lbug', generic), + ); + }); + + it('isPermissionRenameError returns true only for EACCES/EPERM/EBUSY', () => { + expect(isPermissionRenameError(fsErr('EACCES'))).toBe(true); + expect(isPermissionRenameError(fsErr('EPERM'))).toBe(true); + expect(isPermissionRenameError(fsErr('EBUSY'))).toBe(true); + expect(isPermissionRenameError(fsErr('ENOENT'))).toBe(false); + expect(isPermissionRenameError(fsErr('ENOSPC'))).toBe(false); + expect(isPermissionRenameError(new Error('shadow missing'))).toBe(false); + }); + }); + it('lists only missing-shadow WAL quarantine files for cleanup', async () => { await fs.writeFile(`${dbPath}.wal.missing-shadow.1-a`, ''); await fs.writeFile(`${dbPath}.wal.missing-shadow.2-b`, ''); From 3d70660b522ca7fb6cdec344d2cac5fa64a29294 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 21 May 2026 11:47:15 +0100 Subject: [PATCH 3/5] fix(lbug): dedup shadow-replay predicate + counter-based warn anti-spam (PR #1747 review, Findings 4 & 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Smallest viable response to the two remaining non-blocking findings from the production-readiness review of PR #1747. An earlier-revision plan proposed regex widening + a near-miss detector + per-dbPath warn scoping; an adversarial doc-review found those defended against hypothetical strings LadybugDB does not produce, added observability theater with no recovery behavior change, and did not actually fix the long-running gitnexus serve case for hot dbPaths (where finalizeLbugSidecarsAfterClose rarely fires). Scope shrunk to dedup + counter-based — strictly behavior-changing and fully testable. Finding 4 — dedup + version-coupling markers - `isReadOnlyShadowReplayError` was inlined in both `lbug-adapter.ts:451` and `pool-adapter.ts:317`. Centralized as an export from `sidecar-recovery.ts`. The two local copies are removed; both adapters now import from the shared module. - Both LadybugDB-coupled predicates (`isMissingShadowSidecarError` and `isReadOnlyShadowReplayError`) gain a `// LADYBUGDB-CONTRACT:` marker comment citing `@ladybugdb/core ^0.16.1`. When bumping LadybugDB, `git grep "LADYBUGDB-CONTRACT"` enumerates every version-coupled spot. - Strict matcher unchanged — when LadybugDB actually changes the error format, the failure mode stays loud (raw native error propagates) and the markers make every affected predicate trivially greppable. Finding 6 — counter-based warn anti-spam - `warnedKeys: Set` → `warnedKeyCounts: Map`. `warnOnce` keeps its signature `(logger, key, message)` and keying convention unchanged — the swap is internal. - `WARN_MILESTONES = [1, 10, 100, 1000, 10000]`. Logarithmic spacing gives O(log N) warns for a condition that fires N times. Past the first occurrence the warn message is suffixed with "(Nth occurrence of this condition)" so persistence is visible in the log line itself. - Solves the long-running serve case: a hot dbPath hitting the same condition 100 times now fires 3 warns (occurrences 1, 10, 100) instead of 1 warn + 99 silent debug lines. Tests (10 new in sidecar-recovery.test.ts, all green) - Centralized isReadOnlyShadowReplayError: positive match, false-positive guard, structural assertion that the duplicate regex is gone from both adapter files, LADYBUGDB-CONTRACT marker count. - Counter-based warnOnce: milestone-at-10 with suffix, milestone-at-100, key isolation across dbPaths, reset zeroes the counter, first-occurrence message does NOT carry the suffix. Deferred (tracked separately) - Finding 5 — PNA header end-to-end coverage gap (CORS boundary is sound). - LadybugDB structured error codes (if/when the library exposes them). - Per-call milestone configurability — re-open if tuning is needed. --- gitnexus/src/core/lbug/lbug-adapter.ts | 6 +- gitnexus/src/core/lbug/pool-adapter.ts | 6 +- gitnexus/src/core/lbug/sidecar-recovery.ts | 68 ++++++++++- gitnexus/test/unit/sidecar-recovery.test.ts | 129 ++++++++++++++++++++ 4 files changed, 194 insertions(+), 15 deletions(-) diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 25fa7ed5e9..da9809dbae 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -31,6 +31,7 @@ import { finalizeLbugSidecarsAfterClose, inspectLbugSidecars, isMissingShadowSidecarError, + isReadOnlyShadowReplayError, preflightLbugSidecars, quarantineWalForMissingShadow, renameFailureMessage, @@ -448,11 +449,6 @@ const queryAndDrain = async (targetConn: lbug.Connection, cypher: string): Promi const READ_ONLY_SHADOW_REPLAY_PROBE = 'MATCH (n) RETURN n LIMIT 1'; -const isReadOnlyShadowReplayError = (err: unknown): boolean => { - const msg = err instanceof Error ? err.message : String(err); - return /replay shadow pages under read-only mode/i.test(msg); -}; - /** * Reject the quarantine path when the orphan WAL is too large to safely * discard (>TINY_ORPHAN_WAL_BYTES). Mirrors the preflight policy at diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index 0ba7cdf9d1..a73de8d4b2 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -28,6 +28,7 @@ import { import { isMissingFsError, isMissingShadowSidecarError, + isReadOnlyShadowReplayError, preflightLbugSidecars, quarantineWalForMissingShadow, renameFailureMessage, @@ -314,11 +315,6 @@ const LOCK_RETRY_ATTEMPTS = 3; const LOCK_RETRY_DELAY_MS = 2000; const SHADOW_REPLAY_PROBE_QUERY = 'MATCH (n) RETURN n LIMIT 1'; -const isReadOnlyShadowReplayError = (err: unknown): boolean => { - const msg = err instanceof Error ? err.message : String(err); - return /replay shadow pages under read-only mode/i.test(msg); -}; - const poolSidecarLogger = { warn: (message: string): void => { realStderrWrite(`${message}\n`); diff --git a/gitnexus/src/core/lbug/sidecar-recovery.ts b/gitnexus/src/core/lbug/sidecar-recovery.ts index 8faa327b76..f9e86719d5 100644 --- a/gitnexus/src/core/lbug/sidecar-recovery.ts +++ b/gitnexus/src/core/lbug/sidecar-recovery.ts @@ -16,7 +16,39 @@ export interface SidecarRecoveryLogger { export const TINY_ORPHAN_WAL_BYTES = 4 * 1024; -const warnedKeys = new Set(); +/** + * Counter-based warn anti-spam (PR #1747 review, Finding 6). + * + * The previous design (`warnedKeys: Set`) warned exactly once per key + * per process and silently downgraded all subsequent occurrences to debug. In + * a long-lived `gitnexus serve` process touching the same dbPath repeatedly, + * a persistent condition produced one warn at the first occurrence and then + * 99+ silent debug lines — invisible to operators reading warn-level logs. + * + * The counter-based design warns on logarithmic milestones so persistence + * stays visible. Geometric spacing keeps total warn count bounded at O(log N) + * for a condition that fires N times. + */ +const warnedKeyCounts = new Map(); + +const WARN_MILESTONES = [1, 10, 100, 1000, 10000] as const; + +const ordinal = (n: number): string => { + switch (n) { + case 1: + return '1st'; + case 10: + return '10th'; + case 100: + return '100th'; + case 1000: + return '1000th'; + case 10000: + return '10000th'; + default: + return `${n}th`; + } +}; export const isMissingFsError = (err: unknown): boolean => (err as NodeJS.ErrnoException | undefined)?.code === 'ENOENT'; @@ -52,20 +84,46 @@ const logInfo = (logger: SidecarRecoveryLogger, message: string): void => { else logDebug(logger, message); }; +/** + * Log at warn-level on logarithmic milestone occurrences (1st, 10th, 100th, + * 1000th, 10000th); debug-level otherwise. Past the first occurrence the warn + * message is suffixed with the occurrence count so operators can see the + * condition's persistence at a glance. + * + * The signature and key convention (`${dbPath}:suffix`) are unchanged from the + * previous warn-once implementation — call sites need no edits. + */ const warnOnce = (logger: SidecarRecoveryLogger, key: string, message: string): void => { - if (warnedKeys.has(key)) { + const next = (warnedKeyCounts.get(key) ?? 0) + 1; + warnedKeyCounts.set(key, next); + const isMilestone = (WARN_MILESTONES as readonly number[]).includes(next); + if (!isMilestone) { logDebug(logger, message); return; } - warnedKeys.add(key); - logger.warn(message); + if (next === 1) { + logger.warn(message); + return; + } + logger.warn(`${message} (${ordinal(next)} occurrence of this condition)`); }; +// LADYBUGDB-CONTRACT: matches @ladybugdb/core ^0.16.1 native error text. +// When bumping LadybugDB, re-validate this regex against the new error format +// — `git grep "LADYBUGDB-CONTRACT"` enumerates every version-coupled spot. export const isMissingShadowSidecarError = (err: unknown): boolean => { const msg = err instanceof Error ? err.message : String(err); return /Cannot open file .*\.shadow: No such file or directory/i.test(msg); }; +// LADYBUGDB-CONTRACT: matches @ladybugdb/core ^0.16.1 native error text. +// When bumping LadybugDB, re-validate this regex against the new error format +// — `git grep "LADYBUGDB-CONTRACT"` enumerates every version-coupled spot. +export const isReadOnlyShadowReplayError = (err: unknown): boolean => { + const msg = err instanceof Error ? err.message : String(err); + return /replay shadow pages under read-only mode/i.test(msg); +}; + export const shadowSidecarRecoveryMessage = (dbPath: string, err: unknown): string => { const msg = err instanceof Error ? err.message : String(err); return ( @@ -291,5 +349,5 @@ export async function cleanQuarantinedMissingShadowWals(dbPath: string): Promise } export const _resetSidecarRecoveryWarningsForTest = (): void => { - warnedKeys.clear(); + warnedKeyCounts.clear(); }; diff --git a/gitnexus/test/unit/sidecar-recovery.test.ts b/gitnexus/test/unit/sidecar-recovery.test.ts index 2288b7f5dc..25489e7c75 100644 --- a/gitnexus/test/unit/sidecar-recovery.test.ts +++ b/gitnexus/test/unit/sidecar-recovery.test.ts @@ -2,11 +2,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; +import { readFileSync } from 'node:fs'; import { _resetSidecarRecoveryWarningsForTest, finalizeLbugSidecarsAfterClose, inspectLbugSidecars, isPermissionRenameError, + isReadOnlyShadowReplayError, listQuarantinedMissingShadowWals, preflightLbugSidecars, renameFailureMessage, @@ -181,6 +183,47 @@ describe('LadybugDB sidecar recovery', () => { }); }); + describe('Centralized isReadOnlyShadowReplayError (PR #1747 review, F4 dedup)', () => { + it('matches LadybugDB read-only shadow-replay error', () => { + const err = new Error( + "Runtime exception: Couldn't replay shadow pages under read-only mode. Please re-open the database with read-write mode to replay shadow pages.", + ); + expect(isReadOnlyShadowReplayError(err)).toBe(true); + }); + + it('false-positive guard: rejects unrelated errors', () => { + expect(isReadOnlyShadowReplayError(new Error('something else entirely'))).toBe(false); + expect(isReadOnlyShadowReplayError(new Error('replay shadow pages'))).toBe(false); // missing "under read-only mode" + }); + + it('structural: lbug-adapter.ts no longer defines isReadOnlyShadowReplayError locally', () => { + const source = readFileSync( + path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'lbug-adapter.ts'), + 'utf-8', + ); + // The original regex literal should appear nowhere in lbug-adapter.ts + // (it now lives in sidecar-recovery.ts only). + expect(source).not.toMatch(/replay shadow pages under read-only mode/); + }); + + it('structural: pool-adapter.ts no longer defines isReadOnlyShadowReplayError locally', () => { + const source = readFileSync( + path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'pool-adapter.ts'), + 'utf-8', + ); + expect(source).not.toMatch(/replay shadow pages under read-only mode/); + }); + + it('structural: sidecar-recovery.ts carries exactly two LADYBUGDB-CONTRACT markers (one per shadow predicate)', () => { + const source = readFileSync( + path.join(__dirname, '..', '..', 'src', 'core', 'lbug', 'sidecar-recovery.ts'), + 'utf-8', + ); + const markers = source.match(/\/\/ LADYBUGDB-CONTRACT:/g) ?? []; + expect(markers.length).toBe(2); + }); + }); + it('lists only missing-shadow WAL quarantine files for cleanup', async () => { await fs.writeFile(`${dbPath}.wal.missing-shadow.1-a`, ''); await fs.writeFile(`${dbPath}.wal.missing-shadow.2-b`, ''); @@ -192,4 +235,90 @@ describe('LadybugDB sidecar recovery', () => { `${dbPath}.wal.missing-shadow.2-b`, ]); }); + + describe('Counter-based warnOnce milestones (PR #1747 review, F6)', () => { + // Use the public observable surface: drive `warnOnce` indirectly via + // `preflightLbugSidecars` (which calls warnOnce for orphan-WAL) and count + // logger.warn vs logger.debug invocations across many cycles. This avoids + // coupling tests to `warnOnce`'s private signature. + + const triggerOrphanWalPreflight = async (path: string, log: ReturnType) => { + // Each call must restage a >TINY_ORPHAN_WAL_BYTES WAL because preflight + // does not consume large WALs (it returns 'orphan-wal' and warns). + await fs.writeFile(`${path}.wal`, Buffer.alloc(TINY_ORPHAN_WAL_BYTES + 1)); + await preflightLbugSidecars(path, { + mode: 'read-only', + logger: log, + allowQuarantine: true, + }); + }; + + it('first occurrence warns; occurrences 2-9 debug; 10th warns with "10th occurrence" suffix', async () => { + const log = logger(); + for (let i = 1; i <= 10; i++) { + await triggerOrphanWalPreflight(dbPath, log); + } + expect(log.warn).toHaveBeenCalledTimes(2); + expect(log.warn).toHaveBeenNthCalledWith(1, expect.stringContaining('lbug.wal without lbug.shadow')); + expect(log.warn).toHaveBeenNthCalledWith( + 2, + expect.stringContaining('(10th occurrence of this condition)'), + ); + expect(log.debug).toHaveBeenCalledTimes(8); + }); + + it('100th occurrence warns with "100th occurrence" suffix', async () => { + const log = logger(); + for (let i = 1; i <= 100; i++) { + await triggerOrphanWalPreflight(dbPath, log); + } + // Milestones at 1, 10, 100 → 3 warns total. + expect(log.warn).toHaveBeenCalledTimes(3); + expect(log.warn).toHaveBeenNthCalledWith( + 3, + expect.stringContaining('(100th occurrence of this condition)'), + ); + }); + + it('different keys do not share counters (different dbPaths warn independently)', async () => { + const log = logger(); + const dirB = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-sidecar-recovery-B-')); + const dbPathB = path.join(dirB, 'lbug'); + await fs.writeFile(dbPathB, 'db'); + + try { + await triggerOrphanWalPreflight(dbPath, log); + await triggerOrphanWalPreflight(dbPathB, log); + + // Each path fires its first-occurrence warn independently. + expect(log.warn).toHaveBeenCalledTimes(2); + expect(log.debug).toHaveBeenCalledTimes(0); + } finally { + await fs.rm(dirB, { recursive: true, force: true }); + } + }); + + it('_resetSidecarRecoveryWarningsForTest zeroes the counter so the next call fires warn again', async () => { + const log = logger(); + await triggerOrphanWalPreflight(dbPath, log); + await triggerOrphanWalPreflight(dbPath, log); + expect(log.warn).toHaveBeenCalledTimes(1); + expect(log.debug).toHaveBeenCalledTimes(1); + + _resetSidecarRecoveryWarningsForTest(); + + await triggerOrphanWalPreflight(dbPath, log); + // Post-reset, counter is back to 1 — fires warn (not debug). + expect(log.warn).toHaveBeenCalledTimes(2); + expect(log.debug).toHaveBeenCalledTimes(1); + }); + + it('first-occurrence warn message does NOT include the occurrence-count suffix', async () => { + const log = logger(); + await triggerOrphanWalPreflight(dbPath, log); + expect(log.warn).toHaveBeenCalledTimes(1); + const firstWarnMessage = (log.warn as any).mock.calls[0][0] as string; + expect(firstWarnMessage).not.toContain('occurrence of this condition'); + }); + }); }); From b24436e28e7d4e5fa47bcf26dd07295fb3a293f3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 10:59:05 +0000 Subject: [PATCH 4/5] chore(autofix): apply prettier + eslint fixes via /autofix command --- gitnexus/src/core/lbug/pool-adapter.ts | 4 +--- .../test/unit/lbug-adapter-wal-schema.test.ts | 15 +++------------ gitnexus/test/unit/pool-wal-recovery.test.ts | 14 +++++--------- gitnexus/test/unit/sidecar-recovery.test.ts | 5 ++++- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index a73de8d4b2..7eb8e7d140 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -325,9 +325,7 @@ const poolSidecarLogger = { }, }; -type TryQuarantineResult = - | { kind: 'quarantined'; path: string } - | { kind: 'peer-handled' }; +type TryQuarantineResult = { kind: 'quarantined'; path: string } | { kind: 'peer-handled' }; /** * Pool-local quarantine guard that tolerates the concurrent-peer race the diff --git a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts index 66dfa5f596..2cd1b1ed98 100644 --- a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts +++ b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts @@ -588,10 +588,7 @@ describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', vi.unstubAllEnvs(); }); - const setupShadowMissingRecovery = ( - dbPath: string, - walBytes: number | 'missing', - ) => { + const setupShadowMissingRecovery = (dbPath: string, walBytes: number | 'missing') => { const missingShadowError = new Error( `IO exception: Cannot open file ${dbPath}.shadow: No such file or directory`, ); @@ -645,10 +642,7 @@ describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', it('writable recovery: refuses to quarantine a large WAL (4097 bytes) and throws shadow-recovery message', async () => { vi.resetModules(); const dbPath = '/tmp/gitnexus-lbug-large-wal-writable/lbug'; - const { fsMock, warnMock } = setupShadowMissingRecovery( - dbPath, - TINY_ORPHAN_WAL_BYTES_TEST + 1, - ); + const { fsMock, warnMock } = setupShadowMissingRecovery(dbPath, TINY_ORPHAN_WAL_BYTES_TEST + 1); const adapter = await import('../../src/core/lbug/lbug-adapter.js'); @@ -665,10 +659,7 @@ describe('Symmetric WAL-size gate during missing-shadow recovery (PR #1747 D2)', it('read-only recovery: refuses to quarantine a large WAL (4097 bytes) and throws shadow-recovery message', async () => { vi.resetModules(); const dbPath = '/tmp/gitnexus-lbug-large-wal-readonly/lbug'; - const { fsMock, warnMock } = setupShadowMissingRecovery( - dbPath, - TINY_ORPHAN_WAL_BYTES_TEST + 1, - ); + const { fsMock, warnMock } = setupShadowMissingRecovery(dbPath, TINY_ORPHAN_WAL_BYTES_TEST + 1); const adapter = await import('../../src/core/lbug/lbug-adapter.js'); diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index 5da318ba85..647c80f1ed 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -316,15 +316,11 @@ describe('Pool-adapter missing-shadow quarantine: TOCTOU + permission classifica const readOnlyDb1 = makeMockDb(); const readOnlyDb2 = makeMockDb(); - connectionQueryMock - .mockRejectedValueOnce(shadowError(dbPath)) - .mockResolvedValue({ - getAll: vi.fn().mockResolvedValue([]), - close: vi.fn(), - }); - (createLbugDatabase as any) - .mockReturnValueOnce(readOnlyDb1) - .mockReturnValueOnce(readOnlyDb2); + connectionQueryMock.mockRejectedValueOnce(shadowError(dbPath)).mockResolvedValue({ + getAll: vi.fn().mockResolvedValue([]), + close: vi.fn(), + }); + (createLbugDatabase as any).mockReturnValueOnce(readOnlyDb1).mockReturnValueOnce(readOnlyDb2); await initLbug('test-repo-pool-enoent', dbPath); diff --git a/gitnexus/test/unit/sidecar-recovery.test.ts b/gitnexus/test/unit/sidecar-recovery.test.ts index 25489e7c75..c0a2c47529 100644 --- a/gitnexus/test/unit/sidecar-recovery.test.ts +++ b/gitnexus/test/unit/sidecar-recovery.test.ts @@ -259,7 +259,10 @@ describe('LadybugDB sidecar recovery', () => { await triggerOrphanWalPreflight(dbPath, log); } expect(log.warn).toHaveBeenCalledTimes(2); - expect(log.warn).toHaveBeenNthCalledWith(1, expect.stringContaining('lbug.wal without lbug.shadow')); + expect(log.warn).toHaveBeenNthCalledWith( + 1, + expect.stringContaining('lbug.wal without lbug.shadow'), + ); expect(log.warn).toHaveBeenNthCalledWith( 2, expect.stringContaining('(10th occurrence of this condition)'), From 9e09470eb979ed7b0251f2f8011c593abc51cee0 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 21 May 2026 12:05:55 +0100 Subject: [PATCH 5/5] ci: trigger CI rebuild