diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index b07ccd7cb..04233b1a1 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -2065,9 +2065,9 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "25.6.1", - "resolved": "https://registry.npmjs.org/@types/node/-/node-25.6.1.tgz", - "integrity": "sha512-coJCN8O1q4AGyyqCAUSP06P+SrMTu18BkEj3NVAK07q6QUneD2wzj3CLv9+yP+BMeZQlMvneXqqvDe3w+xcq7g==", + "version": "25.6.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-25.6.2.tgz", + "integrity": "sha512-sokuT28dxf9JT5Kady1fsXOvI4HVpjZa95NKT5y9PNTIrs2AsobR4GFAA90ZG8M+nxVRLysCXsVj6eGC7Vbrlw==", "license": "MIT", "dependencies": { "undici-types": "~7.19.0" @@ -3110,9 +3110,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.0.tgz", - "integrity": "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", + "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", "funding": [ { "type": "github", @@ -3476,9 +3476,9 @@ "license": "MIT" }, "node_modules/hono": { - "version": "4.12.16", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.16.tgz", - "integrity": "sha512-jN0ZewiNAWSe5khM3EyCmBb250+b40wWbwNILNfEvq84VREWwOIkuUsFONk/3i3nqkz7Oe1PcpM2mwQEK2L9Kg==", + "version": "4.12.18", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.18.tgz", + "integrity": "sha512-RWzP96k/yv0PQfyXnWjs6zot20TqfpfsNXhOnev8d1InAxubW93L11/oNUc3tQqn2G0bSdAOBpX+2uDFHV7kdQ==", "license": "MIT", "engines": { "node": ">=16.9.0" @@ -4282,15 +4282,15 @@ } }, "node_modules/onnxruntime-common": { - "version": "1.25.1", - "resolved": "https://registry.npmjs.org/onnxruntime-common/-/onnxruntime-common-1.25.1.tgz", - "integrity": "sha512-kKvYQFdos4LWJqhZ+nmKu3NT8NXzw8I5x9fNUKe1rNKcPfNKnYXUtW7JBpcKFsvLtrJashRgVYSbFap4cHxvNg==", + "version": "1.26.0", + "resolved": "https://registry.npmjs.org/onnxruntime-common/-/onnxruntime-common-1.26.0.tgz", + "integrity": "sha512-qVyMR4lcWgbkc4getFV+GQijsTnbg/siteoqcDwa3sI/LxbrMSNw4ePyvCq/ymdQaRomCA7YuWmhzsswxvymdw==", "license": "MIT" }, "node_modules/onnxruntime-node": { - "version": "1.25.1", - "resolved": "https://registry.npmjs.org/onnxruntime-node/-/onnxruntime-node-1.25.1.tgz", - "integrity": "sha512-N0M58CGTiTsLkPpx9bxmRFi24GT6r67Qei/GrBEIiDyntcYdXU5vQZp112ypydG9vEKRFgbgUYQJnEi+jll8dg==", + "version": "1.26.0", + "resolved": "https://registry.npmjs.org/onnxruntime-node/-/onnxruntime-node-1.26.0.tgz", + "integrity": "sha512-OHl6PiOEOqxaLHL0N9eFrbzS7IGmu3BtJNH3RTEnRAheCIkfc3gjcjl4sGcjp9C22ZC9YTquDOxSdT/stBQ6BQ==", "hasInstallScript": true, "license": "MIT", "os": [ @@ -4301,7 +4301,7 @@ "dependencies": { "adm-zip": "^0.5.16", "global-agent": "^4.1.3", - "onnxruntime-common": "1.25.1" + "onnxruntime-common": "1.26.0" } }, "node_modules/onnxruntime-web": { diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 175db47e6..47745c575 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -27,6 +27,7 @@ import { warnMissingOptionalGrammars } from './optional-grammars.js'; import { glob } from 'glob'; import fs from 'fs/promises'; import { cliError } from './cli-message.js'; +import { isHfDownloadFailure } from '../core/embeddings/hf-env.js'; // Capture stderr.write at module load BEFORE anything (LadybugDB native // init, progress bar, console redirection) can monkey-patch it. The @@ -576,6 +577,26 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption return; } + // HF download failure β€” show clean guidance without the raw stack trace. + // Checked before writeFatalToStderr so the user sees one focused message + // rather than a stack-trace dump followed by a second remediation block. + if (isHfDownloadFailure(msg) || msg.includes('Failed to download embedding model')) { + cliError( + ` The embedding model could not be downloaded.\n` + + ` huggingface.co may be unreachable from your network\n` + + ` (e.g. behind a corporate proxy or a regional firewall).\n` + + ` Suggestions:\n` + + ` 1. Set HF_ENDPOINT to a mirror and retry:\n` + + ` HF_ENDPOINT=https://hf-mirror.com npx gitnexus analyze --embeddings\n` + + ` (Windows: set HF_ENDPOINT=https://hf-mirror.com && npx gitnexus analyze --embeddings)\n` + + ` 2. Check your proxy / VPN settings.\n` + + ` 3. Once downloaded the model is cached β€” future runs work offline.\n`, + { recoveryHint: 'hf-endpoint-unreachable' }, + ); + process.exitCode = 1; + return; + } + // Bypass the redirected console.error and write the full stack to // the real stderr captured at module load. The redirected // console.error wraps every line with `\\x1b[2K\\r` (ANSI clear-line) diff --git a/gitnexus/src/core/augmentation/engine.ts b/gitnexus/src/core/augmentation/engine.ts index 896813c22..f97415cc9 100644 --- a/gitnexus/src/core/augmentation/engine.ts +++ b/gitnexus/src/core/augmentation/engine.ts @@ -104,7 +104,7 @@ export async function augment(pattern: string, cwd?: string): Promise { } // Step 1: BM25 search (fast, no embeddings) - const bm25Results = await searchFTSFromLbug(pattern, 10, repoId); + const { results: bm25Results } = await searchFTSFromLbug(pattern, 10, repoId); if (bm25Results.length === 0) return ''; diff --git a/gitnexus/src/core/embeddings/embedder.ts b/gitnexus/src/core/embeddings/embedder.ts index b37fb45f3..72ddcbd70 100644 --- a/gitnexus/src/core/embeddings/embedder.ts +++ b/gitnexus/src/core/embeddings/embedder.ts @@ -14,7 +14,12 @@ if (!process.env.ORT_LOG_LEVEL) { process.env.ORT_LOG_LEVEL = '3'; } -import { pipeline, env, type FeatureExtractionPipeline } from '@huggingface/transformers'; +import { + pipeline, + env, + type FeatureExtractionPipeline, + type ProgressInfo, +} from '@huggingface/transformers'; import { existsSync } from 'fs'; import { execFileSync } from 'child_process'; import { join, dirname } from 'path'; @@ -22,7 +27,7 @@ import { createRequire } from 'module'; import { DEFAULT_EMBEDDING_CONFIG, type EmbeddingConfig, type ModelProgress } from './types.js'; import { isHttpMode, getHttpDimensions, httpEmbed } from './http-client.js'; import { resolveEmbeddingConfig } from './config.js'; -import { applyHfEnvOverrides } from './hf-env.js'; +import { applyHfEnvOverrides, isHfDownloadFailure, withHfDownloadRetry } from './hf-env.js'; import { logger } from '../logger.js'; /** @@ -171,13 +176,18 @@ export const initEmbedder = async ( } const progressCallback = onProgress - ? (data: any) => { + ? (data: ProgressInfo) => { const progress: ModelProgress = { - status: data.status || 'progress', - file: data.file, - progress: data.progress, - loaded: data.loaded, - total: data.total, + // Map the `progress_total` aggregate event (not in ModelProgress.status) + // back to 'progress' so callers don't need to handle it separately. + status: + data.status === 'progress_total' + ? 'progress' + : ((data.status as ModelProgress['status']) ?? 'progress'), + file: 'file' in data ? data.file : undefined, + progress: 'progress' in data ? data.progress : undefined, + loaded: 'loaded' in data ? data.loaded : undefined, + total: 'total' in data ? data.total : undefined, }; onProgress(progress); } @@ -202,17 +212,29 @@ export const initEmbedder = async ( logger.info('πŸ”§ Using WASM backend (slower)...'); } - embedderInstance = await (pipeline as any)('feature-extraction', finalConfig.modelId, { - device: device, - dtype: 'fp32', - progress_callback: progressCallback, - session_options: { - logSeverityLevel: 3, - intraOpNumThreads: finalConfig.threads, - interOpNumThreads: 1, - executionMode: 'sequential', + embedderInstance = await withHfDownloadRetry( + () => + pipeline('feature-extraction', finalConfig.modelId, { + device: device, + dtype: 'fp32', + progress_callback: progressCallback, + session_options: { + logSeverityLevel: 3, + intraOpNumThreads: finalConfig.threads, + interOpNumThreads: 1, + executionMode: 'sequential', + }, + }), + { + onRetry: isDev + ? (attempt, max, err) => + logger.warn( + { attempt, max, err: err.message }, + `⚠️ Model download network error (attempt ${attempt}/${max}), retrying…`, + ) + : undefined, }, - }); + ); currentDevice = device; if (isDev) { @@ -228,6 +250,20 @@ export const initEmbedder = async ( return embedderInstance!; } catch (deviceError) { + // Network errors and circuit-open errors are not device-specific β€” + // they will fail the same way on every device. Rethrow immediately + // with actionable HF_ENDPOINT guidance rather than silently falling + // back to the next device. + const errMsg = deviceError instanceof Error ? deviceError.message : String(deviceError); + if (isHfDownloadFailure(errMsg)) { + const endpointHint = process.env.HF_ENDPOINT + ? `The configured endpoint (${process.env.HF_ENDPOINT}) may be unreachable.` + : `huggingface.co may be unreachable from your network.\n` + + ` Set HF_ENDPOINT to a mirror and retry:\n` + + ` HF_ENDPOINT=https://hf-mirror.com npx gitnexus analyze --embeddings\n` + + ` (Windows: set HF_ENDPOINT=https://hf-mirror.com && npx gitnexus analyze --embeddings)`; + throw new Error(`Failed to download embedding model: ${errMsg}\n ${endpointHint}`); + } if (isDev && (device === 'cuda' || device === 'dml')) { const gpuType = device === 'dml' ? 'DirectML' : 'CUDA'; logger.info(`⚠️ ${gpuType} not available, falling back to CPU...`); diff --git a/gitnexus/src/core/embeddings/hf-env.ts b/gitnexus/src/core/embeddings/hf-env.ts index 6a977a76d..95548fff2 100644 --- a/gitnexus/src/core/embeddings/hf-env.ts +++ b/gitnexus/src/core/embeddings/hf-env.ts @@ -1,6 +1,25 @@ import os from 'node:os'; import { join } from 'node:path'; +// --------------------------------------------------------------------------- +// Download resilience defaults +// --------------------------------------------------------------------------- + +/** Per-attempt timeout for the full model download (5 minutes). */ +export const HF_DOWNLOAD_TIMEOUT_MS = 5 * 60 * 1_000; +/** Maximum total download attempts (1 initial + N-1 retries). */ +export const HF_MAX_ATTEMPTS = 3; +/** Initial delay between retry attempts; doubles on each subsequent retry. */ +export const HF_BASE_DELAY_MS = 2_000; +/** Number of consecutive failures required to open the circuit. */ +export const CB_FAILURE_THRESHOLD = 3; +/** How long the circuit stays open before transitioning to half-open. */ +export const CB_RESET_TIMEOUT_MS = 60_000; +/** Upper bound clamped on the env-override per-attempt timeout (30 minutes). */ +export const HF_MAX_TIMEOUT_MS = 30 * 60 * 1_000; +/** Upper bound clamped on the env-override attempt count. */ +export const HF_MAX_ATTEMPTS_CAP = 10; + /** * @internal Exported only for unit tests and the two embedder entry points * (`core/embeddings/embedder.ts` + `mcp/core/embedder.ts`). Not part of the @@ -60,3 +79,265 @@ export function applyHfEnvOverrides(env: HfEnvSubset): void { env.remoteHost = endpoint.endsWith('/') ? endpoint : endpoint + '/'; } } + +/** + * @internal Exported for unit tests and the two embedder entry points. + * + * Returns true when an error message indicates a network-level fetch failure + * during HuggingFace model download (e.g. `TypeError: fetch failed`, + * `ECONNREFUSED`, `ENOTFOUND`, `ETIMEDOUT`, `ECONNRESET`). + * + * These errors are not device-specific and cannot be fixed by falling back to + * a different ONNX device β€” the caller should rethrow immediately with + * guidance about `HF_ENDPOINT`. + */ +export function isNetworkFetchError(message: string): boolean { + return ( + message.includes('fetch failed') || + message.includes('ECONNREFUSED') || + message.includes('ENOTFOUND') || + message.includes('ETIMEDOUT') || + message.includes('ECONNRESET') + ); +} + +// --------------------------------------------------------------------------- +// Circuit breaker +// --------------------------------------------------------------------------- + +/** @internal Used by `withHfDownloadRetry` to mark a circuit-open rejection. */ +export const CIRCUIT_OPEN_TAG = 'hf-circuit-open'; + +/** Circuit-breaker states. */ +type CircuitState = 'closed' | 'open' | 'half-open'; + +/** + * Circuit breaker for HuggingFace model downloads. + * + * After `failureThreshold` consecutive network failures the circuit opens and + * all subsequent calls to `withHfDownloadRetry` fail immediately without + * issuing any network requests. After `resetTimeoutMs` the circuit enters the + * half-open state and the next call is attempted β€” if it succeeds the circuit + * closes again; if it fails the circuit re-opens. + * + * Exported for unit-testing; production code should use the module-level + * `hfDownloadCircuit` singleton. + */ +export class HfDownloadCircuitBreaker { + private _state: CircuitState = 'closed'; + private _failures = 0; + /** Timestamp of the last recorded failure (ms since epoch). */ + lastFailureAt = 0; + + constructor( + readonly failureThreshold: number = CB_FAILURE_THRESHOLD, + readonly resetTimeoutMs: number = CB_RESET_TIMEOUT_MS, + ) {} + + /** Effective state, factoring in the reset-timeout transition. */ + get state(): CircuitState { + if (this._state === 'open' && Date.now() - this.lastFailureAt > this.resetTimeoutMs) { + this._state = 'half-open'; + } + return this._state; + } + + /** Returns true when the circuit is open and calls should be rejected. */ + isOpen(): boolean { + return this.state === 'open'; + } + + /** Record a successful call β€” resets the failure counter and closes the circuit. */ + recordSuccess(): void { + this._failures = 0; + this._state = 'closed'; + } + + /** Record a failed call β€” increments the counter and opens the circuit when the threshold is reached. */ + recordFailure(): void { + this._failures++; + this.lastFailureAt = Date.now(); + if (this._failures >= this.failureThreshold) { + this._state = 'open'; + } + } + + /** @internal Reset to initial state (used in tests). */ + reset(): void { + this._failures = 0; + this._state = 'closed'; + this.lastFailureAt = 0; + } +} + +/** Module-level singleton shared by both embedder entry points. */ +export const hfDownloadCircuit = new HfDownloadCircuitBreaker(); + +// --------------------------------------------------------------------------- +// Retry + timeout wrapper +// --------------------------------------------------------------------------- + +/** @internal Returns true for errors that should abort without retry (circuit-open). */ +export function isHfCircuitOpenError(message: string): boolean { + return message.includes(CIRCUIT_OPEN_TAG); +} + +/** + * Returns true for any HuggingFace download failure that warrants showing the + * `HF_ENDPOINT` remediation hint: either a raw network error or a + * circuit-open rejection (which itself was caused by repeated network errors). + */ +export function isHfDownloadFailure(message: string): boolean { + return isNetworkFetchError(message) || isHfCircuitOpenError(message); +} + +/** @internal Wraps `fn` in a hard time-limit. The timeout error contains + * `ETIMEDOUT` so that `isNetworkFetchError` classifies it correctly. + */ +export function withDownloadTimeout(fn: () => Promise, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout( + () => + reject( + new Error( + `ETIMEDOUT: model download timed out after ${Math.round(timeoutMs / 1000)}s β€” ` + + `check your network speed or set HF_ENDPOINT to a faster mirror`, + ), + ), + timeoutMs, + ); + fn().then( + (v) => { + clearTimeout(timer); + resolve(v); + }, + (e) => { + clearTimeout(timer); + reject(e); + }, + ); + }); +} + +/** @internal Async sleep (exposed for testing). */ +export function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export interface HfRetryOptions { + /** Maximum total attempts including the initial one (default: `HF_MAX_ATTEMPTS`). */ + maxAttempts?: number; + /** Delay before the first retry; doubles on each subsequent attempt (default: `HF_BASE_DELAY_MS`). */ + baseDelayMs?: number; + /** Per-attempt wall-clock timeout in ms (default: `HF_DOWNLOAD_TIMEOUT_MS`). */ + timeoutMs?: number; + /** + * Circuit-breaker instance to use. Defaults to the module-level + * `hfDownloadCircuit` singleton. Pass a fresh instance in tests. + */ + circuit?: HfDownloadCircuitBreaker; + /** + * Optional callback invoked before each retry (not the initial attempt). + * @param attempt - 1-based retry number + * @param max - total allowed attempts + * @param error - the error that triggered the retry + */ + onRetry?: (attempt: number, max: number, error: Error) => void; +} + +/** + * Retry wrapper for HuggingFace model downloads with per-attempt timeout and + * circuit-breaker protection. + * + * Behaviour: + * - If the circuit is **open**, fails immediately with a `CIRCUIT_OPEN_TAG` + * message (so `isHfDownloadFailure` still returns true and the caller can + * show `HF_ENDPOINT` guidance). + * - Each attempt is wrapped in `withDownloadTimeout`. + * - On a network-level error (`isNetworkFetchError`) the attempt is retried + * with exponential back-off; non-network errors (e.g. ONNX device failure) + * are rethrown immediately without retry. + * - Every network failure is recorded on the circuit breaker; a success resets + * it. + * - After all attempts are exhausted, the last network error is rethrown + * so the existing `isNetworkFetchError` / `isHfDownloadFailure` guards in + * the calling code still fire. + */ +export async function withHfDownloadRetry( + fn: () => Promise, + options: HfRetryOptions = {}, +): Promise { + // Resolve effective values β€” explicit options take precedence over env vars, + // which take precedence over built-in defaults. This lets users lower the + // per-attempt timeout without rebuilding (e.g. + // HF_DOWNLOAD_TIMEOUT_MS=60000 npx gitnexus analyze --embeddings + // reduces the worst-case wait from 15 minutes to ~3 minutes). + // + // Upper bounds are clamped to prevent accidental runaway configuration: + // - timeoutMs is capped at HF_MAX_TIMEOUT_MS (30 min) + // - maxAttempts is floored (fractional values β†’ integer) and capped at + // HF_MAX_ATTEMPTS_CAP (10). Values ≀ 0, NaN, or Infinity fall back to + // the built-in defaults. + const envTimeout = Number(process.env.HF_DOWNLOAD_TIMEOUT_MS); + const envMaxAttempts = Number(process.env.HF_MAX_ATTEMPTS); + const resolvedTimeout = + Number.isFinite(envTimeout) && envTimeout > 0 + ? Math.min(envTimeout, HF_MAX_TIMEOUT_MS) + : HF_DOWNLOAD_TIMEOUT_MS; + const resolvedMaxAttempts = + Number.isFinite(envMaxAttempts) && envMaxAttempts > 0 + ? Math.min(Math.floor(envMaxAttempts), HF_MAX_ATTEMPTS_CAP) + : HF_MAX_ATTEMPTS; + const { + maxAttempts = resolvedMaxAttempts, + baseDelayMs = HF_BASE_DELAY_MS, + timeoutMs = resolvedTimeout, + circuit = hfDownloadCircuit, + onRetry, + } = options; + if (circuit.isOpen()) { + const secsUntilReset = Math.ceil( + (circuit.resetTimeoutMs - (Date.now() - circuit.lastFailureAt)) / 1000, + ); + throw new Error( + `${CIRCUIT_OPEN_TAG}: HuggingFace download circuit is open after repeated network failures` + + (secsUntilReset > 0 ? ` β€” will reset in ~${secsUntilReset}s` : ''), + ); + } + + let lastError: Error = new Error('unknown error'); + + for (let attempt = 0; attempt < maxAttempts; attempt++) { + try { + const result = await withDownloadTimeout(fn, timeoutMs); + circuit.recordSuccess(); + return result; + } catch (err) { + lastError = err instanceof Error ? err : new Error(String(err)); + + if (!isNetworkFetchError(lastError.message)) { + // Non-network error (e.g. CUDA unavailable) β€” propagate without retry + throw lastError; + } + + circuit.recordFailure(); + + if (circuit.isOpen()) { + // Circuit just tripped β€” fail fast, no more retries + throw new Error( + `${CIRCUIT_OPEN_TAG}: HuggingFace download circuit opened after ${circuit.failureThreshold} consecutive failures`, + ); + } + + if (attempt < maxAttempts - 1) { + const delay = baseDelayMs * Math.pow(2, attempt); + onRetry?.(attempt + 1, maxAttempts, lastError); + await sleep(delay); + } + } + } + + // All retries exhausted β€” throw the last network error so isNetworkFetchError + // patterns in the calling code still match and surface HF_ENDPOINT guidance. + throw lastError; +} diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index 96f70ddd6..c90cef85e 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -3,11 +3,14 @@ * Lives in core/ so application code does not depend on the MCP package layer. */ -import { execFileSync } from 'node:child_process'; +import { execFile, execFileSync } from 'node:child_process'; +import { promisify } from 'node:util'; import path from 'path'; import { readRegistry, type RegistryEntry, type CwdMatch } from '../storage/repo-manager.js'; import { findGitRootByDotGit, getCurrentCommit, getRemoteUrl } from '../storage/git.js'; +const execFileAsync = promisify(execFile); + export interface StalenessInfo { isStale: boolean; commitsBehind: number; @@ -41,6 +44,39 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI } } +/** + * Async variant of {@link checkStaleness} β€” spawns git as a child process + * instead of blocking the event loop. Used by `listRepos()` to check many + * repos in parallel (issue #1363: 200 repos Γ— sync spawn β‰ˆ 50 s). + */ +export async function checkStalenessAsync( + repoPath: string, + lastCommit: string, +): Promise { + try { + // Note: promisified execFile captures stdout/stderr by default (no stdio option needed, + // unlike the sync variant which requires explicit stdio: ['pipe','pipe','pipe']). + const { stdout } = await execFileAsync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], { + cwd: repoPath, + encoding: 'utf-8', + }); + + const commitsBehind = parseInt(stdout.trim(), 10) || 0; + + if (commitsBehind > 0) { + return { + isStale: true, + commitsBehind, + hint: `⚠️ Index is ${commitsBehind} commit${commitsBehind > 1 ? 's' : ''} behind HEAD. Run analyze tool to update.`, + }; + } + + return { isStale: false, commitsBehind: 0 }; + } catch { + return { isStale: false, commitsBehind: 0 }; + } +} + /** * Compare a sibling-clone HEAD against an indexed `lastCommit`. Returns * `undefined` when the indexed commit is not reachable from the sibling diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index f8625cdc5..eab942a62 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -91,6 +91,25 @@ function clampCrossDepth(raw: unknown): { depth: number; warning?: string } { return { depth: d }; } +/** + * Clamp the impact timeout to a sane bounded range. Callers can feed this + * via tool params, so an unclamped value lets a single request hold a + * timer slot for an arbitrarily long duration (CodeQL js/resource- + * exhaustion). 100ms lower bound preserves test-suite scenarios that + * exercise tight timeouts; 5min upper bound is well above any legitimate + * single-impact compute. Applied at the validate boundary so the + * downstream `deadline` (Date.now() + timeoutMs) and the local-leg + * `setTimeout` see the same clamped value β€” earlier shapes had a 1hr + * outer cap and a 5min inner clamp that disagreed. + */ +export const IMPACT_TIMEOUT_MIN_MS = 100; +export const IMPACT_TIMEOUT_MAX_MS = 5 * 60 * 1_000; + +export function clampTimeout(timeoutMs: number): number { + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) return IMPACT_TIMEOUT_MIN_MS; + return Math.min(IMPACT_TIMEOUT_MAX_MS, Math.max(IMPACT_TIMEOUT_MIN_MS, Math.trunc(timeoutMs))); +} + export function validateGroupImpactParams(params: Record): | { ok: true; @@ -143,13 +162,19 @@ export function validateGroupImpactParams(params: Record): const service = normalizeServicePrefix(params.service); const subgroup = typeof params.subgroup === 'string' ? params.subgroup : undefined; - let timeoutMs = + // Clamp at the validate boundary so the downstream `deadline` (line + // ~366) and `safeLocalImpact`'s `setTimeout` both see a single + // bounded value. Without this, the outer deadline budgeted Phase-2 + // cross-repo fanout up to 1hr while only the inner setTimeout was + // capped to 5min β€” the two halves of CodeQL #184's mitigation + // disagreed. + const rawTimeoutMs = typeof params.timeoutMs === 'number' && params.timeoutMs > 0 ? params.timeoutMs : typeof params.timeout === 'number' && params.timeout > 0 ? params.timeout : DEFAULT_LOCAL_IMPACT_TIMEOUT_MS; - if (timeoutMs > 3_600_000) timeoutMs = 3_600_000; + const timeoutMs = clampTimeout(rawTimeoutMs); return { ok: true, @@ -191,12 +216,13 @@ async function safeLocalImpact( impactParams: Parameters[1], timeoutMs: number, ): Promise<{ value: unknown; timedOut: boolean }> { + const safeTimeoutMs = clampTimeout(timeoutMs); let timer: ReturnType | undefined; const impactP = port.impact(repo, impactParams).catch((err) => ({ error: err instanceof Error ? err.message : String(err), })); const timeoutP = new Promise<'timeout'>((resolve) => { - timer = setTimeout(() => resolve('timeout'), timeoutMs); + timer = setTimeout(() => resolve('timeout'), safeTimeoutMs); }); const won = await Promise.race([ impactP.then((v) => ({ tag: 'impact' as const, v })), @@ -212,6 +238,65 @@ async function safeLocalImpact( return { value: won.v, timedOut: false }; } +/** + * Race a single Phase-2 `impactByUid` call against a remaining-budget + * timer. The Codex adversarial review on PR #1331 surfaced that the + * fanout loop only checked `Date.now() > deadline` *between* neighbor + * calls β€” once `await port.impactByUid(...)` was reached, a hung + * neighbor could pin the request indefinitely, and slow neighbors + * could compound past the 5-min `IMPACT_TIMEOUT_MAX_MS` cap. + * + * This helper wraps each call: a `setTimeout(remainingMs)` aborts an + * `AbortController` whose signal is forwarded to `impactByUid`, and a + * `Promise.race` resolves to `{ timedOut: true }` when the timer + * fires before the call completes. Implementors that ignore the + * signal (current local backend) still see their await resolved by + * the race; full cooperative cancellation inside the BFS is a future + * follow-up. On rejection, the value is `null` (matching the + * fanout's existing `if (fan == null)` truncation contract). + * + * Exported for direct unit testing β€” the helper IS the load-bearing + * mitigation surface, so the U3 regression test pins it directly + * rather than driving the full `runGroupImpact` path. + */ +export async function safeNeighborImpact( + port: GroupToolPort, + repoId: string, + uid: string, + direction: string, + opts: { + maxDepth: number; + relationTypes: string[]; + minConfidence: number; + includeTests: boolean; + }, + remainingMs: number, +): Promise<{ value: unknown; timedOut: boolean }> { + const controller = new AbortController(); + let timer: ReturnType | undefined; + const callP = port + .impactByUid(repoId, uid, direction, { ...opts, signal: controller.signal }) + .catch(() => null); + const timeoutP = new Promise<'timeout'>((resolve) => { + timer = setTimeout( + () => { + controller.abort(); + resolve('timeout'); + }, + Math.max(0, remainingMs), + ); + }); + const won = await Promise.race([ + callP.then((v) => ({ tag: 'impact' as const, v })), + timeoutP.then(() => ({ tag: 'timeout' as const })), + ]); + if (timer !== undefined) clearTimeout(timer); + if (won.tag === 'timeout') { + return { value: null, timedOut: true }; + } + return { value: won.v, timedOut: false }; +} + export function collectImpactSymbolUids( local: unknown, servicePrefix: string | undefined, @@ -476,7 +561,8 @@ export async function runGroupImpact( if (seen.has(key)) continue; seen.add(key); - if (Date.now() > deadline) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) { truncatedRepos.push(n.neighborRepo); continue; } @@ -492,13 +578,25 @@ export async function runGroupImpact( continue; } - const fan = await deps.port.impactByUid(neighborHandle.id, n.neighborUid, direction, { - maxDepth, - relationTypes: relationTypes ?? [], - minConfidence, - includeTests, - }); - if (fan == null) { + // Phase-2 hardening: race each impactByUid against a per-call + // timeout derived from the remaining budget. Without this wrap a + // single hung neighbor would pin the request past the clamped + // timeout, which Codex's adversarial review on PR #1331 flagged + // as the still-open half of CodeQL #184 / js/resource-exhaustion. + const { value: fan, timedOut: neighborTimedOut } = await safeNeighborImpact( + deps.port, + neighborHandle.id, + n.neighborUid, + direction, + { + maxDepth, + relationTypes: relationTypes ?? [], + minConfidence, + includeTests, + }, + remainingMs, + ); + if (neighborTimedOut || fan == null) { truncatedRepos.push(n.neighborRepo); continue; } diff --git a/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts b/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts index 63fe7ea82..d58c3e08f 100644 --- a/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts +++ b/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts @@ -31,6 +31,32 @@ interface ImportedSymbol { filePath: string; } +/** + * Linear-time `[package].name = "..."` lookup. The previous regex + * `^\[package\]\s*\n(?:[^\[]*?\n)*?name\s*=\s*"([^"]+)"` had a nested + * lazy quantifier on `\n` that CodeQL js/redos flagged as exponential + * on inputs like `[package]\n` + many bare `\n`. We walk lines + * explicitly: scan from the first `[package]` header until we hit the + * next `[...]` section header, looking for the `name = "..."` line. + * O(n) with the line count. + * + * Exported so the U8 ReDoS regression test can drive the production + * line-walk directly with adversarial fixtures (multi-line strings, + * trailing sections, etc.) instead of duplicating it inline. + */ +export function parseCargoPackageName(content: string): string | null { + const lines = content.split('\n'); + const packageStart = lines.findIndex((l) => l.trim() === '[package]'); + if (packageStart < 0) return null; + for (let i = packageStart + 1; i < lines.length; i++) { + const line = lines[i].trimStart(); + if (line.startsWith('[')) break; // hit the next section header + const m = /^name\s*=\s*"([^"]+)"/.exec(line); + if (m) return m[1]; + } + return null; +} + /** * Parse a Cargo.toml to extract the crate name and workspace dependency * names. Uses simple line-based parsing β€” no TOML library needed for @@ -47,12 +73,9 @@ async function parseCrateManifest( return null; } - let name = ''; + const name = parseCargoPackageName(content) ?? ''; const workspaceDeps: string[] = []; - const nameMatch = content.match(/^\[package\]\s*\n(?:[^\[]*?\n)*?name\s*=\s*"([^"]+)"/m); - if (nameMatch) name = nameMatch[1]; - // Match dependencies that use workspace = true, which indicates they // are workspace-internal deps: // dep_name = { workspace = true } diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index de324e70b..d0473048f 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -65,6 +65,15 @@ export interface GroupToolPort { relationTypes: string[]; minConfidence: number; includeTests: boolean; + // Optional cancellation signal. Callers (notably the cross-impact + // Phase-2 fanout) wrap this call in a Promise.race against a + // setTimeout-driven AbortController so a single hung neighbor + // cannot exceed the request's clamped timeout budget. Implementors + // may honor the signal cooperatively or simply let the caller's + // race resolve the await β€” the latter is sufficient for the + // resource-exhaustion mitigation. When the signal is absent or + // already aborted at call time, behavior is unchanged. + signal?: AbortSignal; }, ): Promise; context( diff --git a/gitnexus/src/core/ingestion/cobol/cobol-preprocessor.ts b/gitnexus/src/core/ingestion/cobol/cobol-preprocessor.ts index 23cedb99b..34be6bc03 100644 --- a/gitnexus/src/core/ingestion/cobol/cobol-preprocessor.ts +++ b/gitnexus/src/core/ingestion/cobol/cobol-preprocessor.ts @@ -369,9 +369,20 @@ const RE_USE_AFTER = /\bUSE\s+(?:AFTER\s+)?(?:STANDARD\s+)?(?:EXCEPTION|ERROR)\s+ON\s+([A-Z][A-Z0-9-]+|INPUT|OUTPUT|I-O|EXTEND)\b/i; // SET statement (condition, index) -const RE_SET_TO_TRUE = /\bSET\s+((?:[A-Z][A-Z0-9-]+(?:\s+OF\s+[A-Z][A-Z0-9-]+)?\s+)+)TO\s+TRUE\b/i; -const RE_SET_INDEX = - /\bSET\s+((?:[A-Z][A-Z0-9-]+\s+)+)(TO|UP\s+BY|DOWN\s+BY)\s+(\d+|[A-Z][A-Z0-9-]+)/i; +// +// Catastrophic-backtracking note (CodeQL js/redos): the previous shape +// `((?:[A-Z][A-Z0-9-]+(?:\s+OF\s+[A-Z][A-Z0-9-]+)?\s+)+)TO\s+TRUE` +// nested `\s+` quantifiers across alternations and was exponential on +// inputs like "SET a OF a OF a ... TO TRUE". Replaced with a lazy +// dot-match bounded by the explicit `\s+TO\s+TRUE` suffix β€” `.+?` is +// O(n) with the trailing anchor, and the captured group is parsed +// downstream the same way as before. +// Exported so the U8 ReDoS regression test can pin the exact production +// pattern. Direct import is the only way to ensure the test's +// pathological-input timing assertion exercises the production regex +// instead of an inline copy that drifts. +export const RE_SET_TO_TRUE = /\bSET\s+(.+?)\s+TO\s+TRUE\b/i; +export const RE_SET_INDEX = /\bSET\s+(.+?)\s+(TO|UP\s+BY|DOWN\s+BY)\s+(\d+|[A-Z][A-Z0-9-]+)/i; // INITIALIZE statement β€” data reset (captures targets before REPLACING/WITH clause) const RE_INITIALIZE = /\bINITIALIZE\s+([\s\S]*?)(?=\bREPLACING\b|\bWITH\b|\.\s*$|$)/i; diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 331ebd7dd..cb88986ca 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -19,7 +19,10 @@ import type { CachedEmbedding } from '../embeddings/types.js'; import { extensionManager, type ExtensionEnsureOptions } from './extension-loader.js'; import { closeLbugConnection, + isDbBusyError, + isOpenRetryExhausted, openLbugConnection, + waitForWindowsHandleRelease, type LbugConnectionHandle, } from './lbug-config.js'; import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js'; @@ -185,21 +188,6 @@ const DB_LOCK_RETRY_ATTEMPTS = 3; /** Base back-off in ms between BUSY retries (multiplied by attempt number). */ const DB_LOCK_RETRY_DELAY_MS = 500; -/** - * Return true when the error message indicates that another process holds - * an exclusive lock on the LadybugDB file (e.g. `gitnexus analyze` or - * `gitnexus serve` running at the same time). - */ -export const isDbBusyError = (err: unknown): boolean => { - const msg = (err instanceof Error ? err.message : String(err)).toLowerCase(); - return ( - msg.includes('busy') || - msg.includes('lock') || - msg.includes('already in use') || - msg.includes('could not set lock') - ); -}; - /** * Return true when the error message indicates a write was attempted against * a read-only LadybugDB connection. The MCP query pool opens DBs read-only, @@ -252,7 +240,11 @@ export const withLbugDb = async (dbPath: string, operation: () => Promise) }); } catch (err) { lastError = err; - if (!isDbBusyError(err) || attempt === DB_LOCK_RETRY_ATTEMPTS) { + // Skip outer retry when the inner open-retry already exhausted: the + // ~1.5s open-time budget was just spent, repeating the full reset+ + // reopen cycle would only add 4-5s of tail latency without changing + // the outcome (both layers consult the same isDbBusyError matcher). + if (!isDbBusyError(err) || isOpenRetryExhausted(err) || attempt === DB_LOCK_RETRY_ATTEMPTS) { throw err; } // Close stale connection inside the session lock to prevent race conditions @@ -330,7 +322,16 @@ const doInitLbug = async (dbPath: string) => { await conn.query(schemaQuery); } catch (err) { const msg = err instanceof Error ? err.message : String(err); - if (!msg.includes('already exists')) { + // Suppression list: + // - "already exists": expected idempotent re-create on existing DBs + // - "could not set lock on file": LadybugDB v0.16.1 emits this on + // Windows when CREATE NODE TABLE runs against a path that was + // just opened (the WAL handle from a fresh Database briefly + // contests the table's first-write lock). The table is created + // anyway and any genuine cross-process lock contention surfaces + // on the next operation via withLbugDb's retry. Logging it here + // would just be noise in CI. + if (!msg.includes('already exists') && !isDbBusyError(err)) { logger.warn(`⚠️ Schema creation warning: ${msg.slice(0, 120)}`); } } @@ -1077,6 +1078,9 @@ export const flushWAL = async (): Promise => { */ export const safeClose = async (): Promise => { await flushWAL(); + // Capture before close β€” currentDbPath stays set so the Windows post-close + // probe below knows which file to wait on. + const closingDbPath = currentDbPath; if (conn) { try { // eslint-disable-next-line no-restricted-syntax -- sole authorised close site @@ -1095,6 +1099,24 @@ export const safeClose = async (): Promise => { } db = null; } + // Windows: libuv reports `db.close()` resolved before the kernel has + // released the file handle. A subsequent `new Database(samePath)` in + // the same process can race the release. The probe (lbug-config.ts) + // forces any residual lock to surface as EBUSY/EPERM/EACCES so the + // open-time retry absorbs the lag. + if (process.platform === 'win32' && closingDbPath) { + const released = await waitForWindowsHandleRelease(closingDbPath); + if (!released) { + // Probe exhausted with a lock code still in flight. The next + // openLbugConnection will absorb whatever residual lag remains, but + // a chronic warning helps operators spot AV interference (Windows + // Defender holding the file far past the 250ms budget). + logger.warn( + { dbPath: closingDbPath }, + '⚠️ LadybugDB file handle still locked after close (Windows). If this repeats, check antivirus/Defender exclusions for the GitNexus storage directory.', + ); + } + } }; export const closeLbug = async (): Promise => { diff --git a/gitnexus/src/core/lbug/lbug-config.ts b/gitnexus/src/core/lbug/lbug-config.ts index 5534f7d8b..ceb445693 100644 --- a/gitnexus/src/core/lbug/lbug-config.ts +++ b/gitnexus/src/core/lbug/lbug-config.ts @@ -1,3 +1,6 @@ +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; import type lbug from '@ladybugdb/core'; /** @@ -42,10 +45,23 @@ export const LBUG_MAX_DB_SIZE: number = (() => { return 16 * 1024 * 1024 * 1024; })(); +/** Matches WAL corruption errors from the LadybugDB engine. */ +const WAL_CORRUPTION_RE = /corrupt(ed)?\s+wal|invalid\s+wal\s+record|wal.*corrupt|checksum.*wal/i; + +export const WAL_RECOVERY_SUGGESTION = + 'WAL corruption detected. Run `gitnexus analyze` to rebuild the index.'; + +export function isWalCorruptionError(err: unknown): boolean { + if (!err) return false; + const msg = err instanceof Error ? err.message : String(err); + return WAL_CORRUPTION_RE.test(msg); +} + type LbugModule = typeof lbug; export interface LbugDatabaseOptions { readOnly?: boolean; + throwOnWalReplayFailure?: boolean; } export interface LbugConnectionHandle { @@ -53,20 +69,200 @@ export interface LbugConnectionHandle { conn: lbug.Connection; } +/** + * Return true when the error message indicates that a LadybugDB file lock + * could not be acquired β€” either at construction time + * (`new lbug.Database(...)` raises from `local_file_system.cpp`) or during + * a query (another writer holds the exclusive lock). + * + * Lives here (not in `lbug-adapter.ts`) so both the construction-time + * retry (`openWithLockRetry` in this file) and the query-time retry + * (`withLbugDb` in `lbug-adapter.ts`) consult the same matcher. Callers + * import directly from this module β€” no re-export to keep in sync. + */ +export const isDbBusyError = (err: unknown): boolean => { + const msg = (err instanceof Error ? err.message : String(err)).toLowerCase(); + // `lock` already subsumes `could not set lock`; the broader term is kept + // because graph-DB transient errors include "deadlock", "lock contention", + // and the LadybugDB native module's "could not set lock on file" β€” all of + // which deserve a retry. If a non-transient lock-shaped error ever + // surfaces (e.g., "lock file missing" during recovery), tighten this + // matcher rather than raising the retry budget. + return msg.includes('busy') || msg.includes('lock') || msg.includes('already in use'); +}; + export function createLbugDatabase( lbugModule: LbugModule, databasePath: string, options: LbugDatabaseOptions = {}, ): lbug.Database { - return new lbugModule.Database( + // .d.ts declares fewer args than the native constructor accepts. + return new (lbugModule.Database as any)( databasePath, - 0, - false, + 0, // bufferManagerSize + false, // enableCompression (pinned for v0.16.0) options.readOnly ?? false, LBUG_MAX_DB_SIZE, - ); + true, // autoCheckpoint + -1, // checkpointThreshold + options.throwOnWalReplayFailure ?? true, + true, // enableChecksums + ) as lbug.Database; } +// ─── Lock-busy retry tuning knobs ─────────────────────────────────────────── +// +// All four GitNexus retry pairs that touch native LadybugDB locks live with +// a comment cross-reference here so an SRE tuning Windows flakes finds them +// in one grep: +// +// 1. OPEN_LOCK_RETRY_ATTEMPTS / OPEN_LOCK_RETRY_DELAY_MS (this file) +// β†’ `new lbug.Database()` constructor lock failures +// 2. HANDLE_RELEASE_PROBE_ATTEMPTS / HANDLE_RELEASE_PROBE_DELAY_MS (this file) +// β†’ post-close fs.open probe to absorb Windows handle-release lag +// 3. DB_LOCK_RETRY_ATTEMPTS / DB_LOCK_RETRY_DELAY_MS (lbug-adapter.ts withLbugDb) +// β†’ query-time busy/lock retry around already-open connections +// +// `new lbug.Database()` calls into the native module which performs an +// OS-level exclusive lock on ``. On Windows that lock can fail +// for reasons specific to the OS (Defender briefly opens new files, +// libuv handle release lags the JS-side close). 5 attempts Γ— 100ms +// linear back-off (max sleep 100+200+300+400 = 1s, plus 5 ctor RTTs +// of 10–50ms each = ~1.0–1.2s worst case) clears the typical +// AV-scanner hold without masking real cross-process conflicts. +// +// Source: https://github.com/LadybugDB/ladybug/blob/v0.16.1/src/common/file_system/local_file_system.cpp#L126 +const OPEN_LOCK_RETRY_ATTEMPTS = 5; +const OPEN_LOCK_RETRY_DELAY_MS = 100; + +const HANDLE_RELEASE_PROBE_ATTEMPTS = 5; +const HANDLE_RELEASE_PROBE_DELAY_MS = 50; +const HANDLE_RELEASE_LOCK_CODES = new Set(['EBUSY', 'EPERM', 'EACCES']); + +/** + * Test-fixture directory prefixes recognized by `isTestFixturePath`. + * + * IMPORTANT: this list must stay in sync with the prefixes passed to + * `createTempDir` in `gitnexus/test/helpers/test-db.ts` and the prefixes + * used by `withTestLbugDB` (`gitnexus/test/helpers/test-indexed-db.ts`). + * If you add a new test that passes a custom prefix to `createTempDir`, + * add it here too β€” otherwise the stale-sidecar sweep silently won't + * fire for that fixture and CI flakes return. + * + * The default `createTempDir('gitnexus-test-')` and the lbug variant + * `'gitnexus-lbug-'` cover today's call sites. + */ +const TEST_FIXTURE_PREFIXES = ['gitnexus-lbug-', 'gitnexus-test-']; + +/** + * Marker symbol attached to lock errors after `openWithLockRetry` exhausts + * its budget. `withLbugDb`'s outer query-time retry consults this so it + * does not re-retry a path that just spent up to ~1.5s in the open-time + * loop β€” preventing 6s tail latencies (3Γ— outer Γ— 5Γ— inner attempts). + * + * The symbol is internal to GitNexus; consumers should treat the underlying + * error message as the user-visible signal. + */ +export const LBUG_OPEN_RETRY_EXHAUSTED = Symbol.for('gitnexus.lbug.openRetryExhausted'); + +export const isOpenRetryExhausted = (err: unknown): boolean => { + if (err === null || err === undefined || typeof err !== 'object') return false; + return (err as { [LBUG_OPEN_RETRY_EXHAUSTED]?: boolean })[LBUG_OPEN_RETRY_EXHAUSTED] === true; +}; + +const tagOpenRetryExhausted = (err: unknown): unknown => { + if (err && typeof err === 'object') { + (err as { [LBUG_OPEN_RETRY_EXHAUSTED]?: boolean })[LBUG_OPEN_RETRY_EXHAUSTED] = true; + } + return err; +}; + +/** + * True when `dbPath` resolves to a recognized test fixture under the OS + * temp directory. Used to gate the stale-sidecar sweep so production + * paths never have their `.wal` / `.lock` files deleted. + * + * Defensive shape: + * - `path.resolve` normalizes `..` segments before the prefix check, so + * `/gitnexus-lbug-x/../../etc/passwd` is rejected. + * - The tmpRoot check trims any trailing separator returned by some + * Windows TMP configurations (`C:\Users\X\Temp\`) so the startsWith + * comparison stays correct. + * - Only the IMMEDIATE parent directory is matched against the prefix + * list. An ancestor walk would let a tmpdir whose own basename starts + * with `gitnexus-lbug-` accept arbitrary nested paths under it. + */ +const isTestFixturePath = (dbPath: string): boolean => { + const tmpRoot = os.tmpdir().replace(new RegExp(`${path.sep === '\\' ? '\\\\' : path.sep}+$`), ''); + const resolved = path.resolve(dbPath); + if (!resolved.startsWith(tmpRoot + path.sep) && resolved !== tmpRoot) return false; + const parentBase = path.basename(path.dirname(resolved)); + return TEST_FIXTURE_PREFIXES.some((p) => parentBase.startsWith(p)); +}; + +/** Exported only for direct unit testing β€” production callers use `openWithLockRetry`. */ +export const _isTestFixturePathForTest = isTestFixturePath; + +const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +/** + * Attempt to remove stale `.wal` / `.lock` sidecars that a previous aborted + * test run may have left behind. Best-effort: ENOENT is normal, anything + * else is swallowed so the caller's retry can surface the original error. + */ +const sweepStaleSidecars = async (dbPath: string): Promise => { + for (const suffix of ['.wal', '.lock']) { + try { + await fs.unlink(dbPath + suffix); + } catch { + /* missing sidecar or permission error β€” let the open retry surface it */ + } + } +}; + +/** + * Run `construct` with bounded retries when `new lbug.Database(...)` throws + * a busy/lock error. The original (loop-captured) error is preferred over + * any post-sweep error so triage sees the real LadybugDB lock message. + * On exhaustion the rethrown error is tagged via + * `LBUG_OPEN_RETRY_EXHAUSTED` so the outer query-time retry in + * `withLbugDb` skips re-retrying a freshly-exhausted path. + */ +const openWithLockRetry = async ( + construct: () => lbug.Database, + dbPath: string, +): Promise => { + let originalLockError: unknown; + for (let attempt = 1; attempt <= OPEN_LOCK_RETRY_ATTEMPTS; attempt++) { + try { + return construct(); + } catch (err) { + if (!isDbBusyError(err)) throw err; + originalLockError = err; + if (attempt === OPEN_LOCK_RETRY_ATTEMPTS) break; + await sleep(OPEN_LOCK_RETRY_DELAY_MS * attempt); + } + } + + // Final defense: only for recognized test fixtures, sweep stale sidecars + // (a prior aborted test run can leave a `.wal` lock that survives the + // tmp dir cleanup). Production paths never reach this branch β€” the guard + // requires the immediate parent dir to match a test prefix AND the + // resolved path to live under the OS temp directory. + if (isTestFixturePath(dbPath)) { + await sweepStaleSidecars(dbPath); + try { + return construct(); + } catch { + // Intentionally do NOT overwrite originalLockError. The user-actionable + // signal is "we exhausted lock retries" β€” a different error from the + // post-sweep attempt is less useful than the lock failure that drove + // the sweep in the first place. + } + } + throw tagOpenRetryExhausted(originalLockError); +}; + export async function openLbugConnection( lbugModule: LbugModule, databasePath: string, @@ -74,7 +270,10 @@ export async function openLbugConnection( ): Promise { let db: lbug.Database | undefined; try { - db = createLbugDatabase(lbugModule, databasePath, options); + db = await openWithLockRetry( + () => createLbugDatabase(lbugModule, databasePath, options), + databasePath, + ); return { db, conn: new lbugModule.Connection(db) }; } catch (err) { if (db) await db.close().catch(() => {}); @@ -86,3 +285,60 @@ export async function closeLbugConnection(handle: LbugConnectionHandle): Promise await handle.conn.close().catch(() => {}); await handle.db.close().catch(() => {}); } + +/** + * Probe `dbPath` AND its `.wal` sidecar after `db.close()` so any + * residual native file handle surfaces as EBUSY/EPERM/EACCES and the + * bounded retry absorbs the release lag. Windows-only β€” Linux/macOS do + * not exhibit this race. + * + * Both files matter. Empirically, on rapid openβ†’closeβ†’reopen cycles the + * main `dbPath` handle releases first; the `.wal` handle from the + * previous Database lingers and the new Database's first write (CREATE + * NODE TABLE during schema init) fails with "Could not set lock on + * file". Probing both makes safeClose actually return when the kernel + * is fully done with the path. + * + * Returns `true` when both probes succeeded (or skipped on non-lock + * errors / missing files). Returns `false` when either probe exhausted + * its budget with a lock code still in flight. + * + * Defensive shape: + * - Opens read+write (`'r+'`) so the probe actually surfaces exclusive + * locks held by the previous Database. A read-only probe (`'r'`) is + * insufficient β€” Windows will grant read access while the previous + * handle's exclusive write lock is still in flight, which lets + * `safeClose` return before the next CREATE NODE TABLE can lock the + * file. + * - `try/finally` around `handle.close()` guarantees no fd leak even + * if close itself throws. + */ +export const waitForWindowsHandleRelease = async (dbPath: string): Promise => { + const mainReleased = await probeSinglePath(dbPath); + const walReleased = await probeSinglePath(dbPath + '.wal'); + return mainReleased && walReleased; +}; + +const probeSinglePath = async (filePath: string): Promise => { + for (let attempt = 1; attempt <= HANDLE_RELEASE_PROBE_ATTEMPTS; attempt++) { + let handle: fs.FileHandle | undefined; + try { + handle = await fs.open(filePath, 'r+'); + return true; + } catch (err) { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (!code || !HANDLE_RELEASE_LOCK_CODES.has(code)) return true; // ENOENT / unrelated β†’ not our problem + if (attempt === HANDLE_RELEASE_PROBE_ATTEMPTS) return false; + await sleep(HANDLE_RELEASE_PROBE_DELAY_MS * attempt); + } finally { + if (handle) { + try { + await handle.close(); + } catch { + /* swallow β€” caller cannot do anything useful with a probe-close failure */ + } + } + } + } + return false; +}; diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index ca1c45611..ed999907e 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -18,7 +18,7 @@ import fs from 'fs/promises'; import lbug from '@ladybugdb/core'; import { loadFTSExtension } from './lbug-adapter.js'; -import { createLbugDatabase } from './lbug-config.js'; +import { createLbugDatabase, isWalCorruptionError } from './lbug-config.js'; /** Per-repo pool: one Database, many Connections */ interface PoolEntry { @@ -97,7 +97,7 @@ let idleTimer: ReturnType | null = null; // @ladybugdb/core), corrupting stdout in the pre-sentinel window. Routing // through the leaf breaks that chain. export { realStdoutWrite, realStderrWrite, setActiveStdoutWrite } from '../../mcp/stdio-capture.js'; -import { getActiveStdoutWrite } from '../../mcp/stdio-capture.js'; +import { getActiveStdoutWrite, realStderrWrite } from '../../mcp/stdio-capture.js'; let stdoutSilenceCount = 0; /** True while pre-warming connections β€” prevents watchdog from prematurely restoring stdout */ @@ -263,6 +263,46 @@ const WAITER_TIMEOUT_MS = 15_000; const LOCK_RETRY_ATTEMPTS = 3; const LOCK_RETRY_DELAY_MS = 2000; +async function openReadOnlyDatabase(dbPath: string): Promise { + let db: lbug.Database | undefined; + silenceStdout(); + try { + db = createLbugDatabase(lbug, dbPath, { + readOnly: true, + throwOnWalReplayFailure: false, + }); + await db.init(); + return db; + } catch (err) { + if (db) await db.close().catch(() => {}); + throw err; + } finally { + restoreStdout(); + } +} + +/** + * Quarantine the .wal file and retry opening the database. + * Used when the initial open fails with a WAL corruption error. + */ +async function tryQuarantineAndReopen(dbPath: string, repoId: string): Promise { + const walPath = dbPath + '.wal'; + const quarantineName = `${walPath}.corrupt.${Date.now()}-${Math.random().toString(36).slice(2)}`; + try { + await fs.rename(walPath, quarantineName); + } catch { + throw new Error( + `LadybugDB WAL corruption detected for ${repoId}. ` + + `Run \`gitnexus analyze\` to rebuild the index. (quarantine failed)`, + ); + } + realStderrWrite( + `GitNexus: LadybugDB WAL quarantined for ${repoId}; graph may be stale. ` + + `Run \`gitnexus analyze\` to rebuild the index.\n`, + ); + return await openReadOnlyDatabase(dbPath); +} + /** Deduplicates concurrent initLbug calls for the same repoId */ const initPromises = new Map>(); @@ -319,16 +359,29 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { // avoids lock conflicts when `gitnexus analyze` is writing. let lastError: Error | null = null; for (let attempt = 1; attempt <= LOCK_RETRY_ATTEMPTS; attempt++) { - silenceStdout(); try { - const db = createLbugDatabase(lbug, dbPath, { readOnly: true }); - restoreStdout(); + const db = await openReadOnlyDatabase(dbPath); shared = { db, refCount: 0, ftsLoaded: false }; dbCache.set(dbPath, shared); break; } catch (err: any) { - restoreStdout(); lastError = err instanceof Error ? err : new Error(String(err)); + + if (isWalCorruptionError(lastError)) { + try { + const db = await tryQuarantineAndReopen(dbPath, repoId); + shared = { db, refCount: 0, ftsLoaded: false }; + dbCache.set(dbPath, shared); + break; + } catch (retryErr) { + throw new Error( + `LadybugDB WAL corruption detected for ${repoId}. ` + + `Run \`gitnexus analyze\` to rebuild the index. ` + + `(${retryErr instanceof Error ? retryErr.message : String(retryErr)})`, + ); + } + } + 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/search/bm25-index.ts b/gitnexus/src/core/search/bm25-index.ts index 58c0343c9..27a7b9d8d 100644 --- a/gitnexus/src/core/search/bm25-index.ts +++ b/gitnexus/src/core/search/bm25-index.ts @@ -15,9 +15,16 @@ export interface BM25SearchResult { nodeIds?: string[]; } +export interface FTSSearchResponse { + results: BM25SearchResult[]; + /** True when at least one FTS index query succeeded (index exists). */ + ftsAvailable: boolean; +} + /** * Execute a single FTS query via a custom executor (for MCP connection pool). - * Returns the same shape as core queryFTS (from LadybugDB adapter). + * Returns `null` when the query fails (e.g. FTS index does not exist) so the + * caller can distinguish "zero matches" from "index missing". */ async function queryFTSViaExecutor( executor: (cypher: string) => Promise, @@ -25,7 +32,7 @@ async function queryFTSViaExecutor( indexName: string, query: string, limit: number, -): Promise> { +): Promise | null> { // Escape single quotes and backslashes to prevent Cypher injection const escapedQuery = query.replace(/\\/g, '\\\\').replace(/'/g, "''"); const cypher = ` @@ -46,7 +53,7 @@ async function queryFTSViaExecutor( }; }); } catch { - return []; + return null; } } @@ -65,8 +72,9 @@ export const searchFTSFromLbug = async ( query: string, limit: number = 20, repoId?: string, -): Promise => { +): Promise => { const resultsByIndex: any[][] = []; + let queriesSucceeded = 0; if (repoId) { // Use MCP connection pool via dynamic import @@ -77,15 +85,27 @@ export const searchFTSFromLbug = async ( const executor = (cypher: string) => executeQuery(repoId, cypher); for (const { table, indexName } of FTS_INDEXES) { - resultsByIndex.push(await queryFTSViaExecutor(executor, table, indexName, query, limit)); + const result = await queryFTSViaExecutor(executor, table, indexName, query, limit); + if (result !== null) { + queriesSucceeded++; + resultsByIndex.push(result); + } } } else { // Use core lbug adapter (CLI / pipeline context) β€” also sequential for safety. for (const { table, indexName } of FTS_INDEXES) { - resultsByIndex.push(await queryFTS(table, indexName, query, limit, false).catch(() => [])); + try { + const result = await queryFTS(table, indexName, query, limit, false); + queriesSucceeded++; + resultsByIndex.push(result); + } catch { + // FTS index may not exist β€” count as failed + } } } + const ftsAvailable = queriesSucceeded > 0; + // Collect all node scores per filePath to track which nodes actually matched const fileNodeScores = new Map>(); @@ -116,10 +136,13 @@ export const searchFTSFromLbug = async ( .sort((a, b) => b.score - a.score) .slice(0, limit); - return sorted.map((r, index) => ({ - filePath: r.filePath, - score: r.score, - rank: index + 1, - nodeIds: r.nodeIds, - })); + return { + results: sorted.map((r, index) => ({ + filePath: r.filePath, + score: r.score, + rank: index + 1, + nodeIds: r.nodeIds, + })), + ftsAvailable, + }; }; diff --git a/gitnexus/src/core/search/hybrid-search.ts b/gitnexus/src/core/search/hybrid-search.ts index 72dd1c9b5..b76a9f5e9 100644 --- a/gitnexus/src/core/search/hybrid-search.ts +++ b/gitnexus/src/core/search/hybrid-search.ts @@ -113,12 +113,13 @@ export const mergeWithRRF = ( }; /** - * Check if hybrid search is available - * LadybugDB FTS is always available once the database is initialized. - * Semantic search is optional - hybrid works with just FTS if embeddings aren't ready. + * Check if hybrid search is available. + * FTS indexes may be missing on read-only MCP connections (see #1403); + * callers should inspect `ftsAvailable` from searchFTSFromLbug for + * per-query availability. This helper is a coarse gate only. */ export const isHybridSearchReady = (): boolean => { - return true; // FTS is always available via LadybugDB when DB is open + return true; // FTS is attempted on every query; ftsAvailable signals actual availability }; /** @@ -160,7 +161,7 @@ export const hybridSearch = async ( ) => Promise, ): Promise => { // Use LadybugDB FTS for always-fresh BM25 results - const bm25Results = await searchFTSFromLbug(query, limit); + const { results: bm25Results } = await searchFTSFromLbug(query, limit); const semanticResults = await semanticSearch(executeQuery, query, limit); return mergeWithRRF(bm25Results, semanticResults, limit); }; diff --git a/gitnexus/src/mcp/core/embedder.ts b/gitnexus/src/mcp/core/embedder.ts index ff3a12fd8..f529f2805 100644 --- a/gitnexus/src/mcp/core/embedder.ts +++ b/gitnexus/src/mcp/core/embedder.ts @@ -12,7 +12,11 @@ import { httpEmbedQuery, } from '../../core/embeddings/http-client.js'; import { resolveEmbeddingConfig } from '../../core/embeddings/config.js'; -import { applyHfEnvOverrides } from '../../core/embeddings/hf-env.js'; +import { + applyHfEnvOverrides, + isHfDownloadFailure, + withHfDownloadRetry, +} from '../../core/embeddings/hf-env.js'; import { silenceStdout, restoreStdout, realStderrWrite } from '../../core/lbug/pool-adapter.js'; import { logger } from '../../core/logger.js'; @@ -69,23 +73,39 @@ export const initEmbedder = async (): Promise => { silenceStdout(); process.stderr.write = (() => true) as any; try { - embedderInstance = await (pipeline as any)('feature-extraction', MODEL_ID, { - device: device, - dtype: 'fp32', - session_options: { - logSeverityLevel: 3, - intraOpNumThreads: embeddingConfig.threads, - interOpNumThreads: 1, - executionMode: 'sequential', - }, - }); + embedderInstance = await withHfDownloadRetry(() => + pipeline('feature-extraction', MODEL_ID, { + device: device, + dtype: 'fp32', + session_options: { + logSeverityLevel: 3, + intraOpNumThreads: embeddingConfig.threads, + interOpNumThreads: 1, + executionMode: 'sequential', + }, + }), + ); } finally { restoreStdout(); process.stderr.write = realStderrWrite; } logger.info({ device }, 'GitNexus: Embedding model loaded'); return embedderInstance!; - } catch { + } catch (deviceError) { + // Network errors and circuit-open errors are not device-specific β€” + // they will fail the same way on every device. Rethrow immediately + // with actionable HF_ENDPOINT guidance rather than silently falling + // back to the next device. + const errMsg = deviceError instanceof Error ? deviceError.message : String(deviceError); + if (isHfDownloadFailure(errMsg)) { + const endpointHint = process.env.HF_ENDPOINT + ? `The configured endpoint (${process.env.HF_ENDPOINT}) may be unreachable.` + : `huggingface.co may be unreachable from your network.\n` + + ` Set HF_ENDPOINT to a mirror and retry:\n` + + ` HF_ENDPOINT=https://hf-mirror.com npx gitnexus analyze --embeddings\n` + + ` (Windows: set HF_ENDPOINT=https://hf-mirror.com && npx gitnexus analyze --embeddings)`; + throw new Error(`Failed to download embedding model: ${errMsg}\n ${endpointHint}`); + } if (device === 'cpu') throw new Error('Failed to load embedding model'); } } diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index c8bc01875..167c1db81 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -16,6 +16,7 @@ import { isLbugReady, isWriteQuery, } from '../../core/lbug/pool-adapter.js'; +import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/lbug-config.js'; export { isWriteQuery }; // Embedding imports are lazy (dynamic import) to avoid loading onnxruntime-node // at MCP server startup β€” crashes on unsupported Node ABI versions (#89) @@ -40,7 +41,7 @@ import { isVectorExtensionSupportedByPlatform, } from '../../core/platform/capabilities.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; -import { checkStaleness, checkCwdMatch } from '../../core/git-staleness.js'; +import { checkStalenessAsync, checkCwdMatch } from '../../core/git-staleness.js'; import { logger } from '../../core/logger.js'; // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; @@ -554,8 +555,15 @@ export class LocalBackend { byRemote.set(h.remoteUrl, list); } - return handles.map((h) => { - const stale = checkStaleness(h.repoPath, h.lastCommit); + // Check staleness for all repos in parallel instead of sequentially. + // Each check spawns an async `git rev-list` β€” with 200 repos the sync + // variant took ~50 s; parallel async brings it under a second (#1363). + const stalenessResults = await Promise.all( + handles.map((h) => checkStalenessAsync(h.repoPath, h.lastCommit)), + ); + + return handles.map((h, i) => { + const stale = stalenessResults[i]; const selfNorm = norm(h.repoPath); const siblings = h.remoteUrl ? (byRemote.get(h.remoteUrl) ?? []).filter((e) => norm(e.repoPath) !== selfNorm) @@ -971,7 +979,7 @@ export class LocalBackend { timing, ...(!ftsUsed && { warning: - 'FTS extension unavailable - keyword search degraded. Run: gitnexus analyze --force to rebuild indexes.', + 'FTS indexes missing β€” keyword search degraded. Run: gitnexus analyze --force to rebuild indexes.', }), }; } @@ -985,9 +993,9 @@ export class LocalBackend { limit: number, ): Promise<{ results: any[]; ftsUsed: boolean }> { const { searchFTSFromLbug } = await import('../../core/search/bm25-index.js'); - let bm25Results; + let ftsResponse; try { - bm25Results = await searchFTSFromLbug(query, limit, repo.id); + ftsResponse = await searchFTSFromLbug(query, limit, repo.id); } catch (err: any) { logger.error( { err: err.message }, @@ -996,7 +1004,8 @@ export class LocalBackend { return { results: [], ftsUsed: false }; } - const ftsUsed = bm25Results.length === 0 || bm25Results[0]?.ftsUsed !== false; + const bm25Results = ftsResponse.results; + const ftsUsed = ftsResponse.ftsAvailable; const results: any[] = []; @@ -1218,7 +1227,14 @@ export class LocalBackend { const result = await executeQuery(repo.id, params.query); return result; } catch (err: any) { - return { error: err.message || 'Query failed' }; + const msg = err.message || 'Query failed'; + if (isWalCorruptionError(err)) { + return { + error: msg, + recoverySuggestion: WAL_RECOVERY_SUGGESTION, + }; + } + return { error: msg }; } } @@ -1672,6 +1688,30 @@ export class LocalBackend { kind?: string; include_content?: boolean; }, + ): Promise { + try { + return await this._contextImpl(repo, params); + } catch (err: any) { + const msg = (err instanceof Error ? err.message : String(err)) || 'Context query failed'; + if (isWalCorruptionError(err)) { + return { + error: msg, + recoverySuggestion: WAL_RECOVERY_SUGGESTION, + }; + } + throw err; + } + } + + private async _contextImpl( + repo: RepoHandle, + params: { + name?: string; + uid?: string; + file_path?: string; + kind?: string; + include_content?: boolean; + }, ): Promise { await this.ensureInitialized(repo.id); @@ -2482,6 +2522,7 @@ export class LocalBackend { impactedCount: 0, risk: 'UNKNOWN', suggestion: 'The graph query failed β€” try gitnexus context as a fallback', + ...(isWalCorruptionError(err) ? { recoverySuggestion: WAL_RECOVERY_SUGGESTION } : {}), }; } } @@ -3067,8 +3108,14 @@ export class LocalBackend { relationTypes: string[]; minConfidence: number; includeTests: boolean; + signal?: AbortSignal; }, ): Promise { + // Honor an already-aborted signal at the entry boundary as a fast + // path. Cooperative cancellation inside _runImpactBFS is out of + // scope β€” the caller's Promise.race against the same signal + // resolves the await regardless of how long this body runs. + if (opts.signal?.aborted) return null; try { await this.refreshRepos(); await this.ensureInitialized(repoId); diff --git a/gitnexus/src/server/api.ts b/gitnexus/src/server/api.ts index 773190785..cc65daa3d 100644 --- a/gitnexus/src/server/api.ts +++ b/gitnexus/src/server/api.ts @@ -1060,11 +1060,12 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => const results = await withLbugDb(lbugPath, async () => { let searchResults: any[]; + let ftsAvailable: boolean | undefined; if (mode === 'semantic') { const { isEmbedderReady } = await import('../core/embeddings/embedder.js'); if (!isEmbedderReady()) { - return [] as any[]; + return { searchResults: [] as any[], ftsAvailable: undefined }; } const { semanticSearch: semSearch } = await import('../core/embeddings/embedding-pipeline.js'); @@ -1077,8 +1078,9 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => sources: ['semantic'], })); } else if (mode === 'bm25') { - searchResults = await searchFTSFromLbug(query, limit); - searchResults = searchResults.map((r: any, i: number) => ({ + const ftsResponse = await searchFTSFromLbug(query, limit); + ftsAvailable = ftsResponse.ftsAvailable; + searchResults = ftsResponse.results.map((r: any, i: number) => ({ ...r, rank: i + 1, sources: ['bm25'], @@ -1091,11 +1093,13 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => await import('../core/embeddings/embedding-pipeline.js'); searchResults = await hybridSearch(query, limit, executeQuery, semSearch); } else { - searchResults = await searchFTSFromLbug(query, limit); + const ftsResponse = await searchFTSFromLbug(query, limit); + ftsAvailable = ftsResponse.ftsAvailable; + searchResults = ftsResponse.results; } } - if (!enrich) return searchResults; + if (!enrich) return { searchResults, ftsAvailable }; // Server-side enrichment: add connections, cluster, processes per result // Uses parameterized queries to prevent Cypher injection via nodeId @@ -1177,9 +1181,14 @@ export const createServer = async (port: number, host: string = '127.0.0.1') => }), ); - return enriched; + return { searchResults: enriched, ftsAvailable }; }); - res.json({ results }); + const response: any = { results: results.searchResults ?? results }; + if (results.ftsAvailable === false) { + response.warning = + 'FTS indexes missing β€” keyword search degraded. Run: gitnexus analyze --force to rebuild indexes.'; + } + res.json(response); } catch (err: any) { res.status(500).json({ error: err.message || 'Search failed' }); } diff --git a/gitnexus/test/helpers/test-db.ts b/gitnexus/test/helpers/test-db.ts index 5818fdc8e..37032ebc8 100644 --- a/gitnexus/test/helpers/test-db.ts +++ b/gitnexus/test/helpers/test-db.ts @@ -37,6 +37,13 @@ export async function cleanupTempDir(tmpDir: string): Promise { /** * Create a temporary directory for LadybugDB tests. * Returns the path and a cleanup function. + * + * IMPORTANT: when adding a new test that passes a custom `prefix`, also add + * the prefix to `TEST_FIXTURE_PREFIXES` in + * `gitnexus/src/core/lbug/lbug-config.ts`. The stale-sidecar sweep relies + * on the prefix list to recognize test fixtures; an unknown prefix means + * the sweep silently won't fire for that fixture and Windows CI flakes + * return. */ export async function createTempDir(prefix: string = 'gitnexus-test-'): Promise { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); diff --git a/gitnexus/test/integration/lbug-close-handle-release.test.ts b/gitnexus/test/integration/lbug-close-handle-release.test.ts new file mode 100644 index 000000000..c0a3b8758 --- /dev/null +++ b/gitnexus/test/integration/lbug-close-handle-release.test.ts @@ -0,0 +1,41 @@ +/** + * Integration test: safeClose's Windows post-close handle-release wait. + * + * On Windows, libuv reports `db.close()` resolved before the kernel has + * released the file handle. A subsequent open of the same path can then + * race the release and surface "Could not set lock on file". `safeClose` + * probes the file with `fs.open` to force the residual lock to surface, + * absorbed by the open-time retry in `lbug-config.ts`. + */ +import path from 'path'; +import { describe, it } from 'vitest'; +import { createTempDir } from '../helpers/test-db.js'; + +describe('safeClose β€” close + reopen does not surface lock errors', () => { + it('survives 10 sequential open/close/reopen cycles on the same path', async () => { + const tmp = await createTempDir('gitnexus-lbug-close-cycle-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + for (let i = 0; i < 10; i++) { + await adapter.initLbug(dbPath); + await adapter.closeLbug(); + } + } finally { + await tmp.cleanup(); + } + }); + + it('safeClose is idempotent β€” calling twice in a row does not throw', async () => { + const tmp = await createTempDir('gitnexus-lbug-idempotent-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + await adapter.closeLbug(); + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }); +}); diff --git a/gitnexus/test/integration/lbug-lock-retry.test.ts b/gitnexus/test/integration/lbug-lock-retry.test.ts index 1279c77fb..b95092e2d 100644 --- a/gitnexus/test/integration/lbug-lock-retry.test.ts +++ b/gitnexus/test/integration/lbug-lock-retry.test.ts @@ -14,7 +14,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js'; // Pure-function tests β€” no DB needed, but grouped here for cohesion // with the retry logic they guard. -import { isDbBusyError } from '../../src/core/lbug/lbug-adapter.js'; +import { isDbBusyError } from '../../src/core/lbug/lbug-config.js'; describe('isDbBusyError', () => { it('returns true for "busy" errors (case-insensitive)', () => { @@ -46,6 +46,18 @@ describe('isDbBusyError', () => { expect(isDbBusyError(undefined)).toBe(false); }); + // Documented behavior for lock-shaped strings: the matcher is intentionally + // broad because in graph-DB contexts these are all transient. If LadybugDB + // ever surfaces a non-transient lock-shaped error (e.g., a recovery-time + // "lock file missing"), tighten the matcher and add a negative test here + // rather than raising the retry budget. + it('treats other lock-shaped errors as transient (current intentional behavior)', () => { + expect(isDbBusyError(new Error('deadlock detected'))).toBe(true); + expect(isDbBusyError(new Error('unlock failed'))).toBe(true); + expect(isDbBusyError(new Error('lock contention'))).toBe(true); + expect(isDbBusyError(new Error('Could not open lock file'))).toBe(true); + }); + it('handles non-Error values gracefully', () => { expect(isDbBusyError('BUSY error')).toBe(true); expect(isDbBusyError(42)).toBe(false); diff --git a/gitnexus/test/integration/lbug-open-retry.test.ts b/gitnexus/test/integration/lbug-open-retry.test.ts new file mode 100644 index 000000000..80e68617d --- /dev/null +++ b/gitnexus/test/integration/lbug-open-retry.test.ts @@ -0,0 +1,310 @@ +/** + * Integration tests: open-time lock-busy retry in `lbug-config.ts`. + * + * The lock IO exception raised by `local_file_system.cpp` happens + * synchronously inside `new lbug.Database(...)`, before any query is + * issued β€” so `withLbugDb`'s query-time retry cannot see it. These tests + * exercise the construction-time retry wrapper directly by stubbing the + * `Database` constructor. + * + * See: docs/plans/2026-05-08-002-fix-windows-lbug-lock-ci-flakes-plan.md + */ +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { + _isTestFixturePathForTest as isTestFixturePath, + isDbBusyError, + isOpenRetryExhausted, + openLbugConnection, + waitForWindowsHandleRelease, +} from '../../src/core/lbug/lbug-config.js'; + +// ─── Minimal stub of the `lbug` module surface used by openLbugConnection ── + +interface StubModuleControl { + /** Errors thrown by sequential `new Database(...)` calls. `null` = success. */ + databaseThrows: Array; + /** Number of times the `Database` constructor was invoked. */ + databaseCallCount: number; + /** Number of times `db.close()` was called. */ + closeCallCount: number; +} + +const makeStubLbug = (control: StubModuleControl) => { + class FakeDatabase { + constructor(_path: string, ..._rest: unknown[]) { + control.databaseCallCount++; + const next = control.databaseThrows.shift(); + if (next instanceof Error) throw next; + } + async close(): Promise { + control.closeCallCount++; + } + } + class FakeConnection { + constructor(_db: FakeDatabase) {} + async close(): Promise {} + } + return { Database: FakeDatabase, Connection: FakeConnection } as any; +}; + +describe('isDbBusyError', () => { + it('matches the documented Windows lock-error wording', () => { + expect(isDbBusyError(new Error('Could not set lock on file foo.lbug'))).toBe(true); + expect(isDbBusyError(new Error('database is locked'))).toBe(true); + }); + it('does not match unrelated errors', () => { + expect(isDbBusyError(new Error('Cypher syntax error'))).toBe(false); + expect(isDbBusyError(null)).toBe(false); + }); +}); + +describe('openLbugConnection β€” open-time lock-busy retry', () => { + it('returns a handle when the constructor succeeds on the first try', async () => { + const control: StubModuleControl = { + databaseThrows: [null], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + const handle = await openLbugConnection(stub, '/some/path/lbug'); + expect(handle.db).toBeDefined(); + expect(handle.conn).toBeDefined(); + expect(control.databaseCallCount).toBe(1); + }); + + it('retries on busy/lock errors and succeeds on a later attempt', async () => { + const control: StubModuleControl = { + databaseThrows: [new Error('Could not set lock on file'), null], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + const handle = await openLbugConnection(stub, '/some/path/lbug'); + expect(handle.db).toBeDefined(); + expect(control.databaseCallCount).toBe(2); + }); + + it('exhausts the retry budget and rethrows the last error preserving its message', async () => { + const lockErr = new Error('Could not set lock on file foo.lbug'); + const control: StubModuleControl = { + // 5 attempts + production paths get no sweep retry, so 5 throws total. + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + await expect(openLbugConnection(stub, '/var/data/non-test/lbug')).rejects.toThrow( + 'Could not set lock on file foo.lbug', + ); + expect(control.databaseCallCount).toBe(5); + }); + + it('tags the exhausted error so withLbugDb skips its outer retry', async () => { + const lockErr = new Error('Could not set lock on file'); + const control: StubModuleControl = { + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + let caught: unknown; + try { + await openLbugConnection(stub, '/var/data/non-test/lbug'); + } catch (err) { + caught = err; + } + expect(caught).toBeDefined(); + expect(isOpenRetryExhausted(caught)).toBe(true); + expect(isOpenRetryExhausted(new Error('plain error'))).toBe(false); + expect(isOpenRetryExhausted(null)).toBe(false); + expect(isOpenRetryExhausted(undefined)).toBe(false); + }); + + it('does not retry non-busy errors', async () => { + const syntaxErr = new Error('Cypher syntax error'); + const control: StubModuleControl = { + databaseThrows: [syntaxErr], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + await expect(openLbugConnection(stub, '/some/path/lbug')).rejects.toThrow( + 'Cypher syntax error', + ); + expect(control.databaseCallCount).toBe(1); + }); +}); + +describe('openLbugConnection β€” stale-sidecar sweep (test fixtures only)', () => { + let fixtureDir: string; + let dbPath: string; + + beforeEach(async () => { + fixtureDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-lbug-sweep-')); + dbPath = path.join(fixtureDir, 'lbug'); + }); + + afterEach(async () => { + await fs.rm(fixtureDir, { recursive: true, force: true }).catch(() => {}); + }); + + it('sweeps stale .wal/.lock for a recognized test fixture path and retries once', async () => { + await fs.writeFile(dbPath + '.wal', 'stale'); + await fs.writeFile(dbPath + '.lock', 'stale'); + + const lockErr = new Error('Could not set lock on file'); + const control: StubModuleControl = { + // 5 retries throw, then sweep + 1 final attempt succeeds (6 total). + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, null], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + + const handle = await openLbugConnection(stub, dbPath); + expect(handle.db).toBeDefined(); + expect(control.databaseCallCount).toBe(6); + + // Sidecars removed by the sweep + await expect(fs.access(dbPath + '.wal')).rejects.toThrow(); + await expect(fs.access(dbPath + '.lock')).rejects.toThrow(); + }); + + it('does not sweep production paths even if they share the prefix', async () => { + // A non-tmp dir that *starts* with the prefix must still be rejected. + const lockErr = new Error('Could not set lock on file'); + const control: StubModuleControl = { + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + + // Path is outside os.tmpdir() so the predicate must reject it. + await expect(openLbugConnection(stub, '/var/data/gitnexus-lbug-fake/lbug')).rejects.toThrow( + 'Could not set lock on file', + ); + expect(control.databaseCallCount).toBe(5); // no sweep retry + }); + + it('handles missing sidecars gracefully (ENOENT swallowed, retry runs)', async () => { + // No .wal or .lock pre-created β€” sweep ENOENTs both, then succeeds. + const lockErr = new Error('Could not set lock on file'); + const control: StubModuleControl = { + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, null], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + + const handle = await openLbugConnection(stub, dbPath); + expect(handle.db).toBeDefined(); + expect(control.databaseCallCount).toBe(6); + }); + + it('sweep retry that throws a different error preserves the original lock error', async () => { + // 5 lock errors, then sweep fires, then post-sweep throws an unrelated + // error. The user-actionable signal is "lock retries exhausted" β€” the + // post-sweep error must NOT shadow the original lock message. + const lockErr = new Error('Could not set lock on file foo.lbug'); + const unrelatedErr = new Error('Schema validation error during open'); + const control: StubModuleControl = { + databaseThrows: [lockErr, lockErr, lockErr, lockErr, lockErr, unrelatedErr], + databaseCallCount: 0, + closeCallCount: 0, + }; + const stub = makeStubLbug(control); + + let caught: Error | undefined; + try { + await openLbugConnection(stub, dbPath); + } catch (err) { + caught = err as Error; + } + expect(caught?.message).toBe('Could not set lock on file foo.lbug'); + expect(control.databaseCallCount).toBe(6); // sweep retry did fire + }); +}); + +describe('isTestFixturePath β€” production-safety guard', () => { + it('accepts a fixture under os.tmpdir with a recognized prefix on the immediate parent', () => { + const tmp = os.tmpdir(); + expect(isTestFixturePath(path.join(tmp, 'gitnexus-lbug-XXX', 'lbug'))).toBe(true); + expect(isTestFixturePath(path.join(tmp, 'gitnexus-test-YYY', 'lbug'))).toBe(true); + }); + + it('rejects production paths even with a matching prefix', () => { + expect(isTestFixturePath('/var/data/gitnexus-lbug-fake/lbug')).toBe(false); + expect(isTestFixturePath('/home/user/gitnexus-test-foo/lbug')).toBe(false); + }); + + it('rejects path traversal attempts that resolve outside tmpdir', () => { + const tmp = os.tmpdir(); + const traversal = path.join(tmp, 'gitnexus-lbug-x', '..', '..', 'etc', 'passwd'); + expect(isTestFixturePath(traversal)).toBe(false); + }); + + it('rejects when the immediate parent does not match even if a deeper ancestor does', () => { + // Tightening: ancestor walk would have allowed nested paths under + // `/gitnexus-lbug-x/inner/lbug` to satisfy the predicate. We + // require the immediate parent to match. + const tmp = os.tmpdir(); + expect(isTestFixturePath(path.join(tmp, 'gitnexus-lbug-x', 'inner', 'lbug'))).toBe(false); + }); + + it('handles tmpdir trailing-separator gracefully', () => { + // Some Windows TMP configs return a trailing separator; the predicate + // strips it before the prefix check so fixtures still match. + const tmp = os.tmpdir(); + const fixture = path.join(tmp, 'gitnexus-lbug-trailing', 'lbug'); + // Whether or not os.tmpdir() itself has a trailing separator, + // the predicate must accept legit fixtures. + expect(isTestFixturePath(fixture)).toBe(true); + }); + + it('rejects unrelated prefixes in tmpdir', () => { + const tmp = os.tmpdir(); + expect(isTestFixturePath(path.join(tmp, 'random-dir', 'lbug'))).toBe(false); + expect(isTestFixturePath(path.join(tmp, 'malicious', 'lbug'))).toBe(false); + }); +}); + +describe('waitForWindowsHandleRelease', () => { + let fixtureDir: string; + let dbPath: string; + + beforeEach(async () => { + fixtureDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-lbug-probe-')); + dbPath = path.join(fixtureDir, 'lbug'); + }); + + afterEach(async () => { + await fs.rm(fixtureDir, { recursive: true, force: true }).catch(() => {}); + }); + + it('returns true when the file exists and is openable', async () => { + await fs.writeFile(dbPath, 'fake-db-content'); + const released = await waitForWindowsHandleRelease(dbPath); + expect(released).toBe(true); + }); + + it('returns true when the file does not exist (ENOENT is non-lock)', async () => { + // No fs.writeFile β€” path does not exist. Probe should bail to true, + // not retry, since ENOENT is not a lock code. + const released = await waitForWindowsHandleRelease(dbPath); + expect(released).toBe(true); + }); + + it('does not leak the file handle when close succeeds', async () => { + // Smoke test: 50 sequential probes with a real file. If close were + // skipped, fd usage would climb. We rely on test process not OOMing + // as the simplest indicator; fd table caps catch egregious leaks. + await fs.writeFile(dbPath, 'fake-db-content'); + for (let i = 0; i < 50; i++) { + await waitForWindowsHandleRelease(dbPath); + } + }); +}); diff --git a/gitnexus/test/integration/search-core.test.ts b/gitnexus/test/integration/search-core.test.ts index 2ccc14706..e49169b30 100644 --- a/gitnexus/test/integration/search-core.test.ts +++ b/gitnexus/test/integration/search-core.test.ts @@ -19,7 +19,7 @@ withTestLbugDB( (_handle) => { describe('searchFTSFromLbug β€” core adapter (no repoId)', () => { it('returns ranked results for a matching query', async () => { - const results = await searchFTSFromLbug('user authentication', 10); + const { results } = await searchFTSFromLbug('user authentication', 10); expect(results.length).toBeGreaterThan(0); @@ -40,7 +40,7 @@ withTestLbugDB( }); it('results are ordered by descending score', async () => { - const results = await searchFTSFromLbug('user authentication', 10); + const { results } = await searchFTSFromLbug('user authentication', 10); for (let i = 1; i < results.length; i++) { expect(results[i - 1].score).toBeGreaterThanOrEqual(results[i].score); @@ -48,7 +48,7 @@ withTestLbugDB( }); it('auth-related files rank higher than unrelated files', async () => { - const results = await searchFTSFromLbug('user authentication', 10); + const { results } = await searchFTSFromLbug('user authentication', 10); const filePaths = results.map((r) => r.filePath); expect(filePaths).toContain('src/auth.ts'); @@ -61,7 +61,7 @@ withTestLbugDB( }); it('merges scores from multiple node types for the same filePath', async () => { - const results = await searchFTSFromLbug('user authentication', 20); + const { results } = await searchFTSFromLbug('user authentication', 20); const authResult = results.find((r) => r.filePath === 'src/auth.ts'); expect(authResult).toBeDefined(); @@ -73,12 +73,12 @@ withTestLbugDB( }); it('respects limit parameter', async () => { - const results = await searchFTSFromLbug('user authentication', 2); + const { results } = await searchFTSFromLbug('user authentication', 2); expect(results.length).toBeLessThanOrEqual(2); }); it('returns empty array for a non-matching query', async () => { - const results = await searchFTSFromLbug('xyzzyplughtwisty', 10); + const { results } = await searchFTSFromLbug('xyzzyplughtwisty', 10); expect(results).toEqual([]); }); }); @@ -87,32 +87,32 @@ withTestLbugDB( describe('unhappy paths', () => { it('returns empty array for empty query string', async () => { - const results = await searchFTSFromLbug('', 10); + const { results } = await searchFTSFromLbug('', 10); expect(results).toEqual([]); }); it('returns empty array for whitespace-only query', async () => { - const results = await searchFTSFromLbug(' ', 10); + const { results } = await searchFTSFromLbug(' ', 10); expect(results).toEqual([]); }); it('handles special characters in query gracefully', async () => { - const results = await searchFTSFromLbug('user* OR auth+', 10); + const { results } = await searchFTSFromLbug('user* OR auth+', 10); expect(Array.isArray(results)).toBe(true); }); it('handles limit of 0', async () => { - const results = await searchFTSFromLbug('user authentication', 0); + const { results } = await searchFTSFromLbug('user authentication', 0); expect(results).toEqual([]); }); it('handles negative limit gracefully', async () => { - const results = await searchFTSFromLbug('user authentication', -1); + const { results } = await searchFTSFromLbug('user authentication', -1); expect(Array.isArray(results)).toBe(true); }); it('handles very large limit', async () => { - const results = await searchFTSFromLbug('user authentication', 100000); + const { results } = await searchFTSFromLbug('user authentication', 100000); expect(results.length).toBeLessThanOrEqual(100000); expect(results.length).toBeGreaterThan(0); }); diff --git a/gitnexus/test/integration/search-pool.test.ts b/gitnexus/test/integration/search-pool.test.ts index c0943483b..88128fcfa 100644 --- a/gitnexus/test/integration/search-pool.test.ts +++ b/gitnexus/test/integration/search-pool.test.ts @@ -19,7 +19,7 @@ withTestLbugDB( (handle) => { describe('searchFTSFromLbug β€” MCP pool adapter (with repoId)', () => { it('returns ranked results via pool adapter', async () => { - const results = await searchFTSFromLbug('user authentication', 10, handle.repoId); + const { results } = await searchFTSFromLbug('user authentication', 10, handle.repoId); expect(results.length).toBeGreaterThan(0); @@ -35,7 +35,7 @@ withTestLbugDB( }); it('results are ordered by descending score via pool adapter', async () => { - const results = await searchFTSFromLbug('user authentication', 10, handle.repoId); + const { results } = await searchFTSFromLbug('user authentication', 10, handle.repoId); for (let i = 1; i < results.length; i++) { expect(results[i - 1].score).toBeGreaterThanOrEqual(results[i].score); @@ -43,12 +43,12 @@ withTestLbugDB( }); it('returns empty array for non-matching query via pool adapter', async () => { - const results = await searchFTSFromLbug('xyzzyplughtwisty', 10, handle.repoId); + const { results } = await searchFTSFromLbug('xyzzyplughtwisty', 10, handle.repoId); expect(results).toEqual([]); }); it('respects limit parameter via pool adapter', async () => { - const results = await searchFTSFromLbug('user authentication', 1, handle.repoId); + const { results } = await searchFTSFromLbug('user authentication', 1, handle.repoId); expect(results.length).toBeLessThanOrEqual(1); }); }); @@ -57,22 +57,22 @@ withTestLbugDB( describe('unhappy paths', () => { it('returns empty array for empty query via pool', async () => { - const results = await searchFTSFromLbug('', 10, handle.repoId); + const { results } = await searchFTSFromLbug('', 10, handle.repoId); expect(results).toEqual([]); }); it('returns empty array for whitespace-only query via pool', async () => { - const results = await searchFTSFromLbug(' ', 10, handle.repoId); + const { results } = await searchFTSFromLbug(' ', 10, handle.repoId); expect(results).toEqual([]); }); it('handles special characters in query via pool', async () => { - const results = await searchFTSFromLbug('user* OR auth+', 10, handle.repoId); + const { results } = await searchFTSFromLbug('user* OR auth+', 10, handle.repoId); expect(Array.isArray(results)).toBe(true); }); it('handles limit of 0 via pool', async () => { - const results = await searchFTSFromLbug('user authentication', 0, handle.repoId); + const { results } = await searchFTSFromLbug('user authentication', 0, handle.repoId); expect(results).toEqual([]); }); }); diff --git a/gitnexus/test/unit/bm25-search.test.ts b/gitnexus/test/unit/bm25-search.test.ts index 9b232688f..03a591599 100644 --- a/gitnexus/test/unit/bm25-search.test.ts +++ b/gitnexus/test/unit/bm25-search.test.ts @@ -42,20 +42,24 @@ describe('BM25 search', () => { }); describe('searchFTSFromLbug', () => { - it('returns empty array when LadybugDB is not initialized', async () => { - // Without LadybugDB init, search should return empty (not crash) - const results = await searchFTSFromLbug('test query'); + it('returns empty results when LadybugDB is not initialized', async () => { + // Simulate an uninitialized DB: queryFTS throws instead of returning rows + const { queryFTS } = await import('../../src/core/lbug/lbug-adapter.js'); + vi.mocked(queryFTS).mockRejectedValue(new Error('DB not initialized')); + + const { results, ftsAvailable } = await searchFTSFromLbug('test query'); expect(Array.isArray(results)).toBe(true); expect(results).toHaveLength(0); + expect(ftsAvailable).toBe(false); }); it('handles empty query', async () => { - const results = await searchFTSFromLbug(''); + const { results } = await searchFTSFromLbug(''); expect(Array.isArray(results)).toBe(true); }); it('accepts custom limit parameter', async () => { - const results = await searchFTSFromLbug('test', 5); + const { results } = await searchFTSFromLbug('test', 5); expect(Array.isArray(results)).toBe(true); }); }); @@ -105,7 +109,7 @@ describe('BM25 search', () => { .mockResolvedValueOnce([]) // Method .mockResolvedValueOnce([]); // Interface - const results = await searchFTSFromLbug('queryset'); + const { results } = await searchFTSFromLbug('queryset'); expect(results).toHaveLength(1); expect(results[0].filePath).toBe('src/views.py'); @@ -127,7 +131,7 @@ describe('BM25 search', () => { .mockResolvedValueOnce([]) // Method .mockResolvedValueOnce([]); // Interface - const results = await searchFTSFromLbug('model'); + const { results } = await searchFTSFromLbug('model'); expect(results).toHaveLength(1); expect(results[0].score).toBe(8); // 5+3 @@ -147,7 +151,7 @@ describe('BM25 search', () => { .mockResolvedValueOnce([]) // Method .mockResolvedValueOnce([]); // Interface - const results = await searchFTSFromLbug('util'); + const { results } = await searchFTSFromLbug('util'); expect(results).toHaveLength(1); expect(results[0].nodeIds).toEqual([]); @@ -171,7 +175,7 @@ describe('BM25 search', () => { .mockResolvedValueOnce([]) // Method .mockResolvedValueOnce([]); // Interface - const results = await searchFTSFromLbug('auth'); + const { results } = await searchFTSFromLbug('auth'); expect(results).toHaveLength(1); // All 3 hits (scores 9+7+4=20) β€” each from a different table, all top-3 @@ -192,7 +196,7 @@ describe('BM25 search', () => { .mockResolvedValueOnce([]) // Method .mockResolvedValueOnce([]); // Interface - const results = await searchFTSFromLbug('fn'); + const { results } = await searchFTSFromLbug('fn'); expect(results[0].filePath).toBe('src/high.py'); expect(results[1].filePath).toBe('src/low.py'); @@ -220,7 +224,7 @@ describe('BM25 search', () => { return []; }); - const results = await searchFTSFromLbug('login', 5, REPO); + const { results } = await searchFTSFromLbug('login', 5, REPO); expect(results).toEqual([ { filePath: 'src/auth.ts', score: 8, rank: 1, nodeIds: ['func:login'] }, diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index f8d94d890..45e1b71d2 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -48,6 +48,7 @@ vi.mock('../../src/storage/repo-manager.js', () => ({ // tests don't shell out to git. vi.mock('../../src/core/git-staleness.js', () => ({ checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkStalenessAsync: vi.fn().mockResolvedValue({ isStale: false, commitsBehind: 0 }), checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), })); @@ -61,7 +62,7 @@ vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { // Also mock the search modules to avoid loading onnxruntime vi.mock('../../src/core/search/bm25-index.js', () => ({ - searchFTSFromLbug: vi.fn().mockResolvedValue([]), + searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), })); vi.mock('../../src/mcp/core/embedder.js', () => ({ @@ -194,6 +195,27 @@ describe('LocalBackend.callTool', () => { expect(result).toHaveProperty('definitions'); }); + it('includes FTS-unavailable warning when ftsAvailable is false (#1403)', async () => { + const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js'); + vi.mocked(searchFTSFromLbug).mockResolvedValueOnce({ results: [], ftsAvailable: false }); + (executeParameterized as any).mockResolvedValue([]); + + const result = await backend.callTool('query', { query: 'ProcessActivity' }); + + expect(result).toHaveProperty('warning'); + expect((result as any).warning).toMatch(/gitnexus analyze --force/); + }); + + it('does not include warning when ftsAvailable is true with zero results', async () => { + const { searchFTSFromLbug } = await import('../../src/core/search/bm25-index.js'); + vi.mocked(searchFTSFromLbug).mockResolvedValueOnce({ results: [], ftsAvailable: true }); + (executeParameterized as any).mockResolvedValue([]); + + const result = await backend.callTool('query', { query: 'nonexistent' }); + + expect(result).not.toHaveProperty('warning'); + }); + it('skips vector index query when VECTOR is unsupported by the platform', async () => { const cap = _captureLogger(); platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(false); diff --git a/gitnexus/test/unit/group/cross-impact-phase2-timeout.test.ts b/gitnexus/test/unit/group/cross-impact-phase2-timeout.test.ts new file mode 100644 index 000000000..90b8a80a2 --- /dev/null +++ b/gitnexus/test/unit/group/cross-impact-phase2-timeout.test.ts @@ -0,0 +1,121 @@ +/** + * Phase-2 fanout timeout regression test. + * + * Codex adversarial review on PR #1331 surfaced that `validateGroupImpactParams` + * clamps `timeoutMs` and `safeLocalImpact` enforces it on the local leg, but + * the Phase-2 cross-repo fanout (`cross-impact.ts:521-526`) awaits each + * `port.impactByUid(...)` call without a per-call timeout. A single hung + * neighbor pins the request indefinitely; multiple slow neighbors compound + * past the clamped budget because each starts before `Date.now() > deadline`. + * + * This test pins the contract of the mitigation: a `safeNeighborImpact` + * helper that races `port.impactByUid` against a remaining-budget timer + * and returns `{ value: null, timedOut: true }` when the call cannot + * complete in time. + * + * Direct import + named symbol so this is a real regression net β€” no + * `??`-fallback or dynamic-import dance (the U8 false-green pattern). + */ +import { describe, expect, it } from 'vitest'; +import { safeNeighborImpact } from '../../../src/core/group/cross-impact.js'; +import type { GroupToolPort } from '../../../src/core/group/service.js'; + +const minimalOpts = { + maxDepth: 3, + relationTypes: [] as string[], + minConfidence: 0, + includeTests: false, +}; + +function makePort(impactByUid: GroupToolPort['impactByUid']): GroupToolPort { + return { + resolveRepo: async () => { + throw new Error('not used'); + }, + impact: async () => { + throw new Error('not used'); + }, + query: async () => { + throw new Error('not used'); + }, + context: async () => { + throw new Error('not used'); + }, + impactByUid, + }; +} + +describe('safeNeighborImpact β€” Phase-2 fanout per-call timeout', () => { + it('returns timedOut=true when impactByUid never resolves, within ~remainingMs', async () => { + // Hung neighbor: the promise never resolves. Without the timeout wrap + // this would hang the test runner. + const port = makePort(() => new Promise(() => {})); + const start = performance.now(); + const result = await safeNeighborImpact(port, 'repo-id', 'uid:1', 'upstream', minimalOpts, 150); + const elapsedMs = performance.now() - start; + expect(result.timedOut).toBe(true); + expect(result.value).toBeNull(); + // Allow generous slack for slow CI; the contract is "bounded", not + // "exactly remainingMs". A regression that drops the timeout entirely + // would hang far past 1500ms; a regression that uses the wrong unit + // (seconds vs ms) would fire much faster. + expect(elapsedMs).toBeGreaterThanOrEqual(140); + expect(elapsedMs).toBeLessThan(1500); + }); + + it('returns the resolved value and timedOut=false on a fast happy path', async () => { + const fakeFan = { byDepth: { 1: [{ id: 'u1' }] } }; + const port = makePort(async () => fakeFan); + const result = await safeNeighborImpact( + port, + 'repo-id', + 'uid:1', + 'upstream', + minimalOpts, + 1000, + ); + expect(result.timedOut).toBe(false); + expect(result.value).toBe(fakeFan); + }); + + it('returns timedOut=true immediately when remainingMs is 0 and the call still hangs', async () => { + // Defensive: even if the caller passes 0, the helper must not block. + const port = makePort(() => new Promise(() => {})); + const start = performance.now(); + const result = await safeNeighborImpact(port, 'repo-id', 'uid:1', 'upstream', minimalOpts, 0); + const elapsedMs = performance.now() - start; + expect(result.timedOut).toBe(true); + expect(result.value).toBeNull(); + // 0ms timeout fires on the next tick β€” should be well under 50ms even on slow CI. + expect(elapsedMs).toBeLessThan(50); + }); + + it('does not compound across calls β€” two hung neighbors complete within ~2Γ— remainingMs total', async () => { + // The contract is per-call timeout. Two sequential hung calls should + // total ~2Γ— remainingMs, not (numNeighbors Γ— remainingMsΒ² / 2) or + // anything compounding. A regression that shares one timer across + // calls would pass the first test but fail this one. + const port = makePort(() => new Promise(() => {})); + const start = performance.now(); + const r1 = await safeNeighborImpact(port, 'repo', 'u1', 'upstream', minimalOpts, 100); + const r2 = await safeNeighborImpact(port, 'repo', 'u2', 'upstream', minimalOpts, 100); + const elapsedMs = performance.now() - start; + expect(r1.timedOut).toBe(true); + expect(r2.timedOut).toBe(true); + expect(elapsedMs).toBeGreaterThanOrEqual(180); + expect(elapsedMs).toBeLessThan(1000); + }); + + it('propagates an immediate rejection from impactByUid as timedOut=false with null value', async () => { + // If the port itself rejects (rather than hangs), the helper should + // surface that as a non-timeout failure β€” the existing fanout block + // already handles `if (fan == null)` truncation, so returning null + // here keeps that path intact. + const port = makePort(async () => { + throw new Error('connection refused'); + }); + const result = await safeNeighborImpact(port, 'repo', 'u1', 'upstream', minimalOpts, 1000); + expect(result.timedOut).toBe(false); + expect(result.value).toBeNull(); + }); +}); diff --git a/gitnexus/test/unit/hf-env.test.ts b/gitnexus/test/unit/hf-env.test.ts index 6a5697059..fa8f23bd3 100644 --- a/gitnexus/test/unit/hf-env.test.ts +++ b/gitnexus/test/unit/hf-env.test.ts @@ -1,7 +1,20 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import os from 'node:os'; import { join } from 'node:path'; -import { applyHfEnvOverrides, type HfEnvSubset } from '../../src/core/embeddings/hf-env.js'; +import { + applyHfEnvOverrides, + isNetworkFetchError, + isHfDownloadFailure, + isHfCircuitOpenError, + HfDownloadCircuitBreaker, + withDownloadTimeout, + withHfDownloadRetry, + CIRCUIT_OPEN_TAG, + HF_MAX_ATTEMPTS, + HF_MAX_TIMEOUT_MS, + HF_MAX_ATTEMPTS_CAP, + type HfEnvSubset, +} from '../../src/core/embeddings/hf-env.js'; describe('applyHfEnvOverrides', () => { let envStub: HfEnvSubset; @@ -82,3 +95,399 @@ describe('applyHfEnvOverrides', () => { expect(envStub.remoteHost).toBe('https://hf-mirror.com/'); }); }); + +describe('isNetworkFetchError', () => { + it('returns true for "fetch failed" (the undici error seen on macOS/Node 24)', () => { + expect(isNetworkFetchError('fetch failed')).toBe(true); + }); + + it('returns true for ECONNREFUSED', () => { + expect(isNetworkFetchError('connect ECONNREFUSED 13.45.67.89:443')).toBe(true); + }); + + it('returns true for ENOTFOUND (DNS failure)', () => { + expect(isNetworkFetchError('getaddrinfo ENOTFOUND huggingface.co')).toBe(true); + }); + + it('returns true for ETIMEDOUT', () => { + expect(isNetworkFetchError('connect ETIMEDOUT 13.45.67.89:443')).toBe(true); + }); + + it('returns true for ECONNRESET', () => { + expect(isNetworkFetchError('read ECONNRESET')).toBe(true); + }); + + it('returns false for generic model-load errors (ONNX device failure)', () => { + expect(isNetworkFetchError('Failed to initialize CUDA backend')).toBe(false); + }); + + it('returns false for empty string', () => { + expect(isNetworkFetchError('')).toBe(false); + }); + + it('returns false for module-not-found errors', () => { + expect(isNetworkFetchError('Cannot find module onnxruntime-node')).toBe(false); + }); +}); + +describe('isHfCircuitOpenError', () => { + it('returns true for a circuit-open tag message', () => { + expect(isHfCircuitOpenError(`${CIRCUIT_OPEN_TAG}: circuit is open`)).toBe(true); + }); + + it('returns false for a plain network error', () => { + expect(isHfCircuitOpenError('fetch failed')).toBe(false); + }); +}); + +describe('isHfDownloadFailure', () => { + it('returns true for network fetch errors', () => { + expect(isHfDownloadFailure('ECONNREFUSED 127.0.0.1:443')).toBe(true); + }); + + it('returns true for circuit-open errors', () => { + expect(isHfDownloadFailure(`${CIRCUIT_OPEN_TAG}: open`)).toBe(true); + }); + + it('returns false for ONNX device errors', () => { + expect(isHfDownloadFailure('Failed to initialize CUDA')).toBe(false); + }); +}); + +describe('HfDownloadCircuitBreaker', () => { + it('starts in closed state', () => { + const cb = new HfDownloadCircuitBreaker(); + expect(cb.isOpen()).toBe(false); + expect(cb.state).toBe('closed'); + }); + + it('opens after reaching the failure threshold', () => { + const cb = new HfDownloadCircuitBreaker(3); + cb.recordFailure(); + cb.recordFailure(); + expect(cb.isOpen()).toBe(false); + cb.recordFailure(); // threshold reached + expect(cb.isOpen()).toBe(true); + expect(cb.state).toBe('open'); + }); + + it('closes on recordSuccess after being open', () => { + const cb = new HfDownloadCircuitBreaker(1); + cb.recordFailure(); + expect(cb.isOpen()).toBe(true); + cb.recordSuccess(); + expect(cb.isOpen()).toBe(false); + expect(cb.state).toBe('closed'); + }); + + it('transitions to half-open after the reset timeout', () => { + vi.useFakeTimers(); + try { + const cb = new HfDownloadCircuitBreaker(1, 100 /* 100ms */); + cb.recordFailure(); + expect(cb.isOpen()).toBe(true); + vi.advanceTimersByTime(200); + expect(cb.isOpen()).toBe(false); + expect(cb.state).toBe('half-open'); + } finally { + vi.useRealTimers(); + } + }); + + it('reset() restores closed state', () => { + const cb = new HfDownloadCircuitBreaker(1); + cb.recordFailure(); + expect(cb.isOpen()).toBe(true); + cb.reset(); + expect(cb.isOpen()).toBe(false); + expect(cb.state).toBe('closed'); + }); + + it('re-opens when a failure is recorded in half-open state', () => { + vi.useFakeTimers(); + try { + const cb = new HfDownloadCircuitBreaker(1, 100 /* 100ms */); + cb.recordFailure(); // opens the circuit + vi.advanceTimersByTime(200); // advance past reset timeout + expect(cb.state).toBe('half-open'); // getter transitions _state to half-open + cb.recordFailure(); // failure in half-open β†’ re-opens + expect(cb.isOpen()).toBe(true); + expect(cb.state).toBe('open'); + } finally { + vi.useRealTimers(); + } + }); + + it('closes the circuit when success is recorded in half-open state', () => { + vi.useFakeTimers(); + try { + const cb = new HfDownloadCircuitBreaker(1, 100 /* 100ms */); + cb.recordFailure(); // opens the circuit + vi.advanceTimersByTime(200); // advance past reset timeout + expect(cb.state).toBe('half-open'); + cb.recordSuccess(); // success in half-open β†’ closes + expect(cb.isOpen()).toBe(false); + expect(cb.state).toBe('closed'); + } finally { + vi.useRealTimers(); + } + }); +}); + +describe('withDownloadTimeout', () => { + it('resolves when fn completes before the timeout', async () => { + const result = await withDownloadTimeout(() => Promise.resolve(42), 1_000); + expect(result).toBe(42); + }); + + it('rejects with ETIMEDOUT when fn takes too long', async () => { + vi.useFakeTimers(); + try { + const neverResolves = () => new Promise(() => {}); + const promise = withDownloadTimeout(neverResolves, 20); + vi.advanceTimersByTime(30); + await expect(promise).rejects.toThrow('ETIMEDOUT'); + } finally { + vi.useRealTimers(); + } + }); + + it('propagates non-timeout errors from fn', async () => { + await expect( + withDownloadTimeout(() => Promise.reject(new Error('download error')), 1_000), + ).rejects.toThrow('download error'); + }); +}); + +describe('withHfDownloadRetry', () => { + it('returns the result on first success', async () => { + const fn = vi.fn().mockResolvedValue('ok'); + const cb = new HfDownloadCircuitBreaker(); + const result = await withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 }); + expect(result).toBe('ok'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('retries on network errors and succeeds on second attempt', async () => { + const fn = vi.fn().mockRejectedValueOnce(new Error('fetch failed')).mockResolvedValue('ok'); + const cb = new HfDownloadCircuitBreaker(); + const result = await withHfDownloadRetry(fn, { + circuit: cb, + maxAttempts: 3, + baseDelayMs: 0, + }); + expect(result).toBe('ok'); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('throws the last network error after all attempts are exhausted', async () => { + const fn = vi.fn().mockRejectedValue(new Error('ECONNREFUSED 127.0.0.1:443')); + const cb = new HfDownloadCircuitBreaker(99 /* high threshold */); + await expect( + withHfDownloadRetry(fn, { circuit: cb, maxAttempts: 3, baseDelayMs: 0 }), + ).rejects.toThrow('ECONNREFUSED'); + expect(fn).toHaveBeenCalledTimes(3); + }); + + it('does not retry non-network errors', async () => { + const fn = vi.fn().mockRejectedValue(new Error('Failed to initialize CUDA backend')); + const cb = new HfDownloadCircuitBreaker(); + await expect( + withHfDownloadRetry(fn, { circuit: cb, maxAttempts: 3, baseDelayMs: 0 }), + ).rejects.toThrow('Failed to initialize CUDA backend'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('fails immediately when the circuit is already open', async () => { + const fn = vi.fn().mockResolvedValue('ok'); + const cb = new HfDownloadCircuitBreaker(1); + cb.recordFailure(); // open the circuit + await expect(withHfDownloadRetry(fn, { circuit: cb })).rejects.toThrow(CIRCUIT_OPEN_TAG); + expect(fn).not.toHaveBeenCalled(); + }); + + it('opens the circuit after failureThreshold failures and throws a circuit-open error', async () => { + const fn = vi.fn().mockRejectedValue(new Error('ENOTFOUND huggingface.co')); + const cb = new HfDownloadCircuitBreaker(2 /* threshold */, 60_000); + // First call: 2 attempts, threshold=2 β†’ circuit opens on 2nd failure + await expect( + withHfDownloadRetry(fn, { circuit: cb, maxAttempts: 2, baseDelayMs: 0 }), + ).rejects.toThrow(CIRCUIT_OPEN_TAG); + expect(cb.isOpen()).toBe(true); + }); + + it('calls onRetry with correct arguments on each retry', async () => { + const fn = vi + .fn() + .mockRejectedValueOnce(new Error('fetch failed')) + .mockRejectedValueOnce(new Error('fetch failed')) + .mockResolvedValue('ok'); + const cb = new HfDownloadCircuitBreaker(99); + const onRetry = vi.fn(); + await withHfDownloadRetry(fn, { circuit: cb, maxAttempts: 3, baseDelayMs: 0, onRetry }); + expect(onRetry).toHaveBeenCalledTimes(2); + expect(onRetry).toHaveBeenNthCalledWith( + 1, + 1, + 3, + expect.objectContaining({ message: 'fetch failed' }), + ); + expect(onRetry).toHaveBeenNthCalledWith( + 2, + 2, + 3, + expect.objectContaining({ message: 'fetch failed' }), + ); + }); + + it('resets the circuit on success', async () => { + const fn = vi.fn().mockResolvedValue('value'); + const cb = new HfDownloadCircuitBreaker(5); + cb.recordFailure(); + cb.recordFailure(); // 2 failures, circuit still closed + await withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 }); + expect(cb.state).toBe('closed'); + }); +}); + +describe('withHfDownloadRetry env overrides', () => { + let originalTimeout: string | undefined; + let originalMaxAttempts: string | undefined; + + beforeEach(() => { + originalTimeout = process.env.HF_DOWNLOAD_TIMEOUT_MS; + originalMaxAttempts = process.env.HF_MAX_ATTEMPTS; + delete process.env.HF_DOWNLOAD_TIMEOUT_MS; + delete process.env.HF_MAX_ATTEMPTS; + }); + + afterEach(() => { + if (originalTimeout === undefined) delete process.env.HF_DOWNLOAD_TIMEOUT_MS; + else process.env.HF_DOWNLOAD_TIMEOUT_MS = originalTimeout; + if (originalMaxAttempts === undefined) delete process.env.HF_MAX_ATTEMPTS; + else process.env.HF_MAX_ATTEMPTS = originalMaxAttempts; + }); + + it('HF_MAX_ATTEMPTS=1 gives exactly 1 attempt', async () => { + process.env.HF_MAX_ATTEMPTS = '1'; + const fn = vi.fn().mockRejectedValue(new Error('ECONNREFUSED 127.0.0.1:443')); + const cb = new HfDownloadCircuitBreaker(99_999 /* high threshold */); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'ECONNREFUSED', + ); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('HF_MAX_ATTEMPTS=2 gives exactly 2 attempts', async () => { + process.env.HF_MAX_ATTEMPTS = '2'; + const fn = vi.fn().mockRejectedValue(new Error('ENOTFOUND huggingface.co')); + const cb = new HfDownloadCircuitBreaker(99_999); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'ENOTFOUND', + ); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('HF_MAX_ATTEMPTS=abc falls back to the built-in default', async () => { + process.env.HF_MAX_ATTEMPTS = 'abc'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99_999); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'fetch failed', + ); + expect(fn).toHaveBeenCalledTimes(HF_MAX_ATTEMPTS); + }); + + it('HF_MAX_ATTEMPTS=0 falls back to the built-in default', async () => { + process.env.HF_MAX_ATTEMPTS = '0'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99_999); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'fetch failed', + ); + expect(fn).toHaveBeenCalledTimes(HF_MAX_ATTEMPTS); + }); + + it('HF_MAX_ATTEMPTS=-1 falls back to the built-in default', async () => { + process.env.HF_MAX_ATTEMPTS = '-1'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99_999); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'fetch failed', + ); + expect(fn).toHaveBeenCalledTimes(HF_MAX_ATTEMPTS); + }); + + it('HF_MAX_ATTEMPTS is clamped to HF_MAX_ATTEMPTS_CAP', async () => { + process.env.HF_MAX_ATTEMPTS = '9999'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99_999 /* very high threshold */); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'fetch failed', + ); + expect(fn).toHaveBeenCalledTimes(HF_MAX_ATTEMPTS_CAP); + }); + + it('HF_MAX_ATTEMPTS=2.9 is floored to 2', async () => { + process.env.HF_MAX_ATTEMPTS = '2.9'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99_999); + await expect(withHfDownloadRetry(fn, { circuit: cb, baseDelayMs: 0 })).rejects.toThrow( + 'fetch failed', + ); + expect(fn).toHaveBeenCalledTimes(2); + }); + + it('HF_DOWNLOAD_TIMEOUT_MS is used as the per-attempt timeout when valid', async () => { + vi.useFakeTimers(); + try { + process.env.HF_DOWNLOAD_TIMEOUT_MS = '50'; + const neverResolves = () => new Promise(() => {}); + const cb = new HfDownloadCircuitBreaker(99); + const promise = withHfDownloadRetry(neverResolves, { circuit: cb, maxAttempts: 1 }); + vi.advanceTimersByTime(100); + await expect(promise).rejects.toThrow('ETIMEDOUT'); + } finally { + vi.useRealTimers(); + } + }); + + it('HF_DOWNLOAD_TIMEOUT_MS=-1 falls back to the built-in default', async () => { + process.env.HF_DOWNLOAD_TIMEOUT_MS = '-1'; + // Passing explicit timeoutMs=0 (no real wait) so the test doesn't block; + // we just verify that the env var rejection causes options.timeoutMs to be + // the default constant (not -1) by confirming the resolved value is used. + const fn = vi.fn().mockResolvedValue('ok'); + const cb = new HfDownloadCircuitBreaker(99); + // Provide explicit timeoutMs to avoid the default 5-minute wait + const result = await withHfDownloadRetry(fn, { circuit: cb, timeoutMs: 100 }); + expect(result).toBe('ok'); + }); + + it('HF_DOWNLOAD_TIMEOUT_MS is clamped to HF_MAX_TIMEOUT_MS', async () => { + vi.useFakeTimers(); + try { + // Set an env value exceeding the 30-minute cap + process.env.HF_DOWNLOAD_TIMEOUT_MS = String(HF_MAX_TIMEOUT_MS + 60_000); + const neverResolves = () => new Promise(() => {}); + const cb = new HfDownloadCircuitBreaker(99); + const promise = withHfDownloadRetry(neverResolves, { circuit: cb, maxAttempts: 1 }); + // Advance just past the 30-minute cap + vi.advanceTimersByTime(HF_MAX_TIMEOUT_MS + 1); + await expect(promise).rejects.toThrow('ETIMEDOUT'); + } finally { + vi.useRealTimers(); + } + }); + + it('explicit options override env vars', async () => { + process.env.HF_MAX_ATTEMPTS = '5'; + const fn = vi.fn().mockRejectedValue(new Error('fetch failed')); + const cb = new HfDownloadCircuitBreaker(99); + // explicit maxAttempts: 2 must win over HF_MAX_ATTEMPTS=5 + await expect( + withHfDownloadRetry(fn, { circuit: cb, maxAttempts: 2, baseDelayMs: 0 }), + ).rejects.toThrow('fetch failed'); + expect(fn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/gitnexus/test/unit/lbug-config-wal.test.ts b/gitnexus/test/unit/lbug-config-wal.test.ts new file mode 100644 index 000000000..6baea3621 --- /dev/null +++ b/gitnexus/test/unit/lbug-config-wal.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from 'vitest'; +import { createLbugDatabase, isWalCorruptionError } from '../../src/core/lbug/lbug-config.js'; + +describe('isWalCorruptionError', () => { + it.each([ + [ + 'Corrupted wal file', + 'Runtime exception: Corrupted wal file. Read out invalid WAL record type.', + ], + ['invalid WAL record', 'Error: invalid WAL record type'], + ['WAL checksum', 'Checksum verification failed, the WAL file is corrupted.'], + ['WAL + corrupt', 'the WAL file is corrupted'], + ])('matches WAL corruption: %s', (_label, msg) => { + expect(isWalCorruptionError(msg)).toBe(true); + expect(isWalCorruptionError(new Error(msg))).toBe(true); + }); + + it.each([ + ['lock error', 'Could not set lock on file : /path/to/db'], + ['generic', 'Query failed'], + ['not found', 'LadybugDB not found at /path'], + ['checksum without WAL', 'Checksum verification failed for parquet file'], + ['permission path with WAL', "EACCES: permission denied '/path/to/wal'"], + ['schema mismatch WAL', 'schema version mismatch in WAL'], + ])('does not match non-WAL error: %s', (_label, msg) => { + expect(isWalCorruptionError(msg)).toBe(false); + }); + + it('handles non-string input', () => { + expect(isWalCorruptionError(undefined)).toBe(false); + expect(isWalCorruptionError(null)).toBe(false); + expect(isWalCorruptionError(42)).toBe(false); + expect(isWalCorruptionError(new Error('ok'))).toBe(false); + }); +}); + +describe('createLbugDatabase WAL replay option', () => { + it('passes throwOnWalReplayFailure and checksum constructor args explicitly', () => { + const Database = vi.fn(function (this: any) {}); + const lbugModule = { Database } as any; + + createLbugDatabase(lbugModule, '/tmp/lbug', { + readOnly: true, + throwOnWalReplayFailure: false, + }); + + expect(Database).toHaveBeenCalledWith( + '/tmp/lbug', + 0, + false, + true, + expect.any(Number), + true, + -1, + false, + true, + ); + }); +}); diff --git a/gitnexus/test/unit/mcp-wal-feedback.test.ts b/gitnexus/test/unit/mcp-wal-feedback.test.ts new file mode 100644 index 000000000..e387e0b97 --- /dev/null +++ b/gitnexus/test/unit/mcp-wal-feedback.test.ts @@ -0,0 +1,160 @@ +/** + * Tests for WAL corruption feedback in MCP error responses (#1402). + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({ + lbugMocks: { + initLbug: vi.fn().mockResolvedValue(undefined), + executeQuery: vi.fn(), + executeParameterized: vi.fn(), + closeLbug: vi.fn().mockResolvedValue(undefined), + isLbugReady: vi.fn().mockReturnValue(true), + isWriteQuery: vi.fn().mockReturnValue(false), + }, + platformMocks: { + isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), + }, + repoMocks: { + listRegisteredRepos: vi.fn(), + }, +})); + +vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, ...lbugMocks }; +}); + +vi.mock('../../src/storage/repo-manager.js', () => ({ + listRegisteredRepos: repoMocks.listRegisteredRepos, + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), +})); + +vi.mock('../../src/core/git-staleness.js', () => ({ + checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), +})); + +vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + isVectorExtensionSupportedByPlatform: platformMocks.isVectorExtensionSupportedByPlatform, + }; +}); + +vi.mock('../../src/core/search/bm25-index.js', () => ({ + searchFTSFromLbug: vi.fn().mockResolvedValue([]), +})); + +vi.mock('../../src/mcp/core/embedder.js', () => ({ + embedQuery: vi.fn().mockResolvedValue([]), + getEmbeddingDims: vi.fn().mockReturnValue(384), +})); + +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; + +const MOCK_REPO_ENTRY = { + name: 'test-repo', + path: '/tmp/test', + storagePath: '/tmp/test/.gitnexus', + indexedAt: '2026-05-01T00:00:00Z', + lastCommit: 'abc1234', +}; + +async function makeBackend(): Promise { + const backend = new LocalBackend(); + await backend.init(); + return backend; +} + +describe('WAL corruption feedback in MCP responses (#1402)', () => { + beforeEach(() => { + vi.clearAllMocks(); + lbugMocks.initLbug.mockResolvedValue(undefined); + lbugMocks.executeQuery.mockResolvedValue([]); + lbugMocks.executeParameterized.mockResolvedValue([]); + lbugMocks.isLbugReady.mockReturnValue(true); + lbugMocks.isWriteQuery.mockReturnValue(false); + repoMocks.listRegisteredRepos.mockResolvedValue([MOCK_REPO_ENTRY]); + }); + + it('impact returns WAL suggestion on corrupted WAL error', async () => { + const backend = await makeBackend(); + lbugMocks.executeParameterized.mockRejectedValueOnce( + new Error('Runtime exception: Corrupted wal file. Read out invalid WAL record type.'), + ); + + const result = await backend.callTool('impact', { + repo: 'test-repo', + target: 'MyClass', + direction: 'upstream', + }); + + expect(result.error).toBeDefined(); + expect(result.suggestion).toBe( + 'The graph query failed β€” try gitnexus context as a fallback', + ); + expect(result.recoverySuggestion).toBeDefined(); + }); + + it('cypher returns WAL recoverySuggestion on corrupted WAL error', async () => { + const backend = await makeBackend(); + lbugMocks.executeQuery.mockRejectedValueOnce(new Error('Corrupted wal file')); + + const result = await backend.callTool('cypher', { + repo: 'test-repo', + query: 'MATCH (n) RETURN n LIMIT 1', + }); + + expect(result.error).toBe('Corrupted wal file'); + expect(result.recoverySuggestion).toBeDefined(); + }); + + it('context returns WAL recoverySuggestion on corrupted WAL error', async () => { + const backend = await makeBackend(); + lbugMocks.executeParameterized.mockRejectedValueOnce(new Error('Corrupted wal file')); + + const result = await backend.callTool('context', { + repo: 'test-repo', + name: 'MyClass', + }); + + expect(result.error).toBe('Corrupted wal file'); + expect(result.recoverySuggestion).toBeDefined(); + }); + + it('non-WAL errors do not include WAL suggestion', async () => { + const backend = await makeBackend(); + lbugMocks.executeParameterized.mockRejectedValueOnce(new Error('Some other error')); + + const result = await backend.callTool('impact', { + repo: 'test-repo', + target: 'MyClass', + direction: 'upstream', + }); + + expect(result.error).toBeDefined(); + expect(result.suggestion).toBe( + 'The graph query failed β€” try gitnexus context as a fallback', + ); + }); + + it('context preserves non-WAL throw behavior', async () => { + const backend = await makeBackend(); + lbugMocks.executeParameterized.mockRejectedValueOnce(new Error('Some other error')); + + await expect( + backend.callTool('context', { + repo: 'test-repo', + name: 'MyClass', + }), + ).rejects.toThrow('Some other error'); + }); +}); diff --git a/gitnexus/test/unit/mcp/group-repo-routing.test.ts b/gitnexus/test/unit/mcp/group-repo-routing.test.ts index ddf7d03a5..474abeaa8 100644 --- a/gitnexus/test/unit/mcp/group-repo-routing.test.ts +++ b/gitnexus/test/unit/mcp/group-repo-routing.test.ts @@ -32,7 +32,7 @@ vi.mock('../../../src/storage/repo-manager.js', () => ({ })); vi.mock('../../../src/core/search/bm25-index.js', () => ({ - searchFTSFromLbug: vi.fn().mockResolvedValue([]), + searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), })); vi.mock('../../../src/mcp/core/embedder.js', () => ({ diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts new file mode 100644 index 000000000..19b24c583 --- /dev/null +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -0,0 +1,177 @@ +/** + * Tests for WAL corruption recovery in the connection pool (#1402). + * + * Mocks createLbugDatabase and fs to verify quarantine + retry behavior + * without needing a real LadybugDB instance or corrupted WAL file. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { stderrWriteMock } = vi.hoisted(() => ({ + stderrWriteMock: vi.fn(), +})); + +vi.mock('fs/promises', () => ({ + default: { + stat: vi.fn().mockResolvedValue({}), + unlink: vi.fn().mockResolvedValue(undefined), + rename: vi.fn().mockResolvedValue(undefined), + }, +})); + +vi.mock('@ladybugdb/core', () => ({ + default: { + Database: vi.fn(), + Connection: vi.fn(function (this: any) { + this.close = vi.fn().mockResolvedValue(undefined); + }), + }, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ + loadFTSExtension: vi.fn().mockResolvedValue(true), +})); + +vi.mock('../../src/core/lbug/lbug-config.js', () => ({ + createLbugDatabase: vi.fn(), + LBUG_MAX_DB_SIZE: 1024, + isWalCorruptionError: vi.fn((err: unknown) => { + const msg = err instanceof Error ? err.message : String(err ?? ''); + return /corrupt(ed)?\s+wal|invalid\s+wal\s+record/i.test(msg); + }), +})); + +vi.mock('../../src/mcp/stdio-capture.js', () => ({ + realStdoutWrite: vi.fn(), + realStderrWrite: stderrWriteMock, + setActiveStdoutWrite: vi.fn(), + getActiveStdoutWrite: vi.fn(() => vi.fn()), +})); + +import fs from 'fs/promises'; +import { createLbugDatabase } from '../../src/core/lbug/lbug-config.js'; + +const { closeLbug } = await import('../../src/core/lbug/pool-adapter.js'); + +const mockInit = vi.fn().mockResolvedValue(undefined); +const mockClose = vi.fn().mockResolvedValue(undefined); + +function makeMockDb() { + return { init: mockInit, close: mockClose, _isClosed: false } as any; +} + +describe('WAL corruption recovery in doInitLbug (#1402)', () => { + beforeEach(() => { + (createLbugDatabase as any).mockReset(); + (fs.stat as any).mockReset(); + (fs.rename as any).mockReset(); + mockInit.mockReset(); + mockClose.mockReset(); + mockInit.mockResolvedValue(undefined); + mockClose.mockResolvedValue(undefined); + (fs.stat as any).mockResolvedValue({}); + (fs.rename as any).mockResolvedValue(undefined); + }); + + afterEach(async () => { + vi.useRealTimers(); + await closeLbug().catch(() => {}); + vi.clearAllMocks(); + }); + + it('retries with WAL quarantine on corrupted WAL init error', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-wal-recovery/lbug'; + + const badDb = makeMockDb(); + const goodDb = makeMockDb(); + badDb.init = vi.fn().mockRejectedValueOnce(new Error('Corrupted wal file')); + (createLbugDatabase as any).mockReturnValueOnce(badDb).mockReturnValueOnce(goodDb); + + await initLbug('test-repo-init', dbPath); + + expect(badDb.init).toHaveBeenCalledTimes(1); + expect(createLbugDatabase).toHaveBeenCalledTimes(2); + expect(createLbugDatabase).toHaveBeenCalledWith( + expect.anything(), + dbPath, + expect.objectContaining({ + readOnly: true, + throwOnWalReplayFailure: false, + }), + ); + expect(fs.rename).toHaveBeenCalledWith( + dbPath + '.wal', + expect.stringContaining('.wal.corrupt.'), + ); + expect(stderrWriteMock).toHaveBeenCalledWith( + expect.stringContaining('WAL quarantined for test-repo-init'), + ); + }); + + 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) => { + callback(); + return 0 as any; + }); + const dbPath = '/tmp/test-wal-recovery/lbug'; + + (createLbugDatabase as any).mockImplementation(() => { + throw new Error('Could not set lock on file'); + }); + + try { + await expect(initLbug('test-repo-lock', dbPath)).rejects.toThrow(); + } finally { + setTimeoutSpy.mockRestore(); + } + + expect(fs.rename).not.toHaveBeenCalled(); + }); + + it('throws with analyze suggestion after retry also fails', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-wal-recovery/lbug'; + + (createLbugDatabase as any) + .mockImplementationOnce(() => { + throw new Error('Corrupted wal file'); + }) + .mockImplementationOnce(() => { + throw new Error('Still broken'); + }); + + await expect(initLbug('test-repo-fail', dbPath)).rejects.toThrow(/gitnexus analyze/); + expect(createLbugDatabase).toHaveBeenCalledTimes(2); + }); + + it('does not reuse poisoned state after WAL failure', async () => { + const { initLbug, isLbugReady: ready } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-wal-recovery/lbug'; + + (createLbugDatabase as any) + .mockImplementationOnce(() => { + throw new Error('Corrupted wal file'); + }) + .mockImplementationOnce(() => { + throw new Error('Still broken'); + }); + + await expect(initLbug('test-repo-nocache', dbPath)).rejects.toThrow(); + + expect(ready('test-repo-nocache')).toBe(false); + }); + + it('handles quarantine gracefully when .wal file does not exist', async () => { + const { initLbug } = await import('../../src/core/lbug/pool-adapter.js'); + const dbPath = '/tmp/test-wal-recovery/lbug'; + + (fs.rename as any).mockRejectedValueOnce(new Error('ENOENT: no such file')); + + (createLbugDatabase as any).mockImplementationOnce(() => { + throw new Error('Corrupted wal file'); + }); + + await expect(initLbug('test-repo-enoent', dbPath)).rejects.toThrow(/gitnexus analyze/); + }); +}); diff --git a/gitnexus/test/unit/staleness.test.ts b/gitnexus/test/unit/staleness.test.ts index b8ea398ff..1645d9987 100644 --- a/gitnexus/test/unit/staleness.test.ts +++ b/gitnexus/test/unit/staleness.test.ts @@ -8,7 +8,7 @@ */ import { describe, it, expect } from 'vitest'; import { execFileSync } from 'child_process'; -import { checkStaleness } from '../../src/core/git-staleness.js'; +import { checkStaleness, checkStalenessAsync } from '../../src/core/git-staleness.js'; // We test checkStaleness with a real git repo (the project itself) // since mocking execFileSync across ESM modules is complex. @@ -65,3 +65,82 @@ describe('checkStaleness', () => { expect(result.commitsBehind).toBe(0); }); }); + +describe('checkStalenessAsync', () => { + it('returns not stale when HEAD matches lastCommit', async () => { + let headCommit: string; + try { + headCommit = execFileSync('git', ['rev-parse', 'HEAD'], { + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + } catch { + return; + } + + const result = await checkStalenessAsync(process.cwd(), headCommit); + expect(result.isStale).toBe(false); + expect(result.commitsBehind).toBe(0); + expect(result.hint).toBeUndefined(); + }); + + it('returns stale when lastCommit is behind HEAD', async () => { + let previousCommit: string; + try { + previousCommit = execFileSync('git', ['rev-parse', 'HEAD~1'], { + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + } catch { + return; + } + + if (!previousCommit) return; + + const result = await checkStalenessAsync(process.cwd(), previousCommit); + expect(result.isStale).toBe(true); + expect(result.commitsBehind).toBeGreaterThan(0); + expect(result.hint).toContain('behind HEAD'); + }); + + it('fails open when git command fails (e.g., invalid path)', async () => { + const result = await checkStalenessAsync('/nonexistent/path', 'abc123'); + expect(result.isStale).toBe(false); + expect(result.commitsBehind).toBe(0); + }); + + it('fails open with invalid commit hash', async () => { + const result = await checkStalenessAsync(process.cwd(), 'not-a-real-commit-hash'); + expect(result.isStale).toBe(false); + expect(result.commitsBehind).toBe(0); + }); + + it('parallel calls complete faster than sequential', async () => { + let headCommit: string; + try { + headCommit = execFileSync('git', ['rev-parse', 'HEAD'], { + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + } catch { + return; + } + + const cwd = process.cwd(); + const N = 10; + + // Parallel + const t0 = performance.now(); + await Promise.all(Array.from({ length: N }, () => checkStalenessAsync(cwd, headCommit))); + const parallelMs = performance.now() - t0; + + // Sequential sync + const t1 = performance.now(); + for (let i = 0; i < N; i++) checkStaleness(cwd, headCommit); + const sequentialMs = performance.now() - t1; + + // Parallel should be meaningfully faster than sequential. + // Use a generous ratio to avoid flakiness on slow CI machines. + expect(parallelMs).toBeLessThan(sequentialMs * 1.5); + }); +}); diff --git a/gitnexus/test/unit/u8-redos-resource-exhaustion.test.ts b/gitnexus/test/unit/u8-redos-resource-exhaustion.test.ts new file mode 100644 index 000000000..6dd6d9955 --- /dev/null +++ b/gitnexus/test/unit/u8-redos-resource-exhaustion.test.ts @@ -0,0 +1,196 @@ +/** + * Regression tests for U8 β€” closes: + * #186 js/redos rust-workspace-extractor.ts + * #187 js/redos cobol-preprocessor.ts + * #184 js/resource-exhaustion cross-impact.ts + * + * These tests import the production symbols directly. A previous shape + * dynamic-imported names that did not exist (`extractRustWorkspace` vs. + * the real `extractRustWorkspaceLinks`) and `??`-fell-back to inline + * regex copies, so the tests stayed green even when the production + * fixes regressed. Static imports + named symbols make a regression in + * any of the three sites a hard test failure. + */ +import { describe, expect, it } from 'vitest'; +import { RE_SET_TO_TRUE, RE_SET_INDEX } from '../../src/core/ingestion/cobol/cobol-preprocessor.js'; +import { parseCargoPackageName } from '../../src/core/group/extractors/rust-workspace-extractor.js'; +import { + clampTimeout, + IMPACT_TIMEOUT_MIN_MS, + IMPACT_TIMEOUT_MAX_MS, +} from '../../src/core/group/cross-impact.js'; + +/** + * Time a single regex.exec call. Used by the linearity tests below to + * compute a 10k/5k ratio in addition to the absolute <500ms bound. + * + * Ratio assertions catch sub-exponential O(nΒ²) regressions that fit + * inside the absolute cap on warm CI; the absolute cap catches + * catastrophic backtracking on cold CI. Two complementary signals. + */ +function timeRegex(re: RegExp, input: string): number { + // Reset regex.lastIndex for global/sticky regexes β€” ours are not, but + // be defensive in case future shape changes add the `g` flag. + re.lastIndex = 0; + const start = performance.now(); + re.exec(input); + return performance.now() - start; +} + +function timeFn(fn: () => T): number { + const start = performance.now(); + fn(); + return performance.now() - start; +} + +// Linear scaling is ~2.0Γ— when input doubles; 3.0Γ— allows generous +// slack for CI-runner GC and tier-up jitter. An O(nΒ²) regression on a +// 2Γ— input takes ~4Γ— as long, well outside this bound. +const LINEAR_RATIO_BOUND = 3.0; + +/** + * Minimum elapsed time (in ms) below which `performance.now()` ratios + * are dominated by scheduler jitter and become meaningless. When both + * timed runs come in below this floor, we skip the ratio assertion β€” + * the absolute <500ms bound still catches catastrophic backtracking, + * and the next CI run will measure higher absolute times that the + * ratio assertion can evaluate reliably. + * + * Calibrated empirically: a flake on macOS reported ratio 5.29Γ— + * between two sub-millisecond measurements (~0.5ms vs ~2.6ms), both + * genuinely linear but indistinguishable from noise. 5ms is a + * comfortable floor where individual measurements are well-separated + * from the ~10-100Β΅s `performance.now()` resolution band. + */ +const RATIO_MEASUREMENT_FLOOR_MS = 5; + +/** + * Assert linear scaling between two timed runs on inputs that differ + * by 2Γ—. When measurements are too small to be reliable, the ratio + * assertion is skipped (the absolute bound still fires elsewhere). + */ +function assertSubLinearRatio(elapsedSmall: number, elapsedLarge: number, label: string): void { + if (elapsedSmall < RATIO_MEASUREMENT_FLOOR_MS && elapsedLarge < RATIO_MEASUREMENT_FLOOR_MS) { + // Both runs completed faster than the noise floor β€” the ratio is + // not meaningful. The absolute <500ms bound elsewhere in this + // describe block still pins linearity; we skip rather than risk a + // flake on a genuinely-linear implementation. + return; + } + const ratio = elapsedLarge / Math.max(elapsedSmall, 0.001); + if (ratio >= LINEAR_RATIO_BOUND) { + throw new Error( + `${label}: ratio ${ratio.toFixed(2)}Γ— exceeds bound ${LINEAR_RATIO_BOUND}Γ— ` + + `(small=${elapsedSmall.toFixed(2)}ms, large=${elapsedLarge.toFixed(2)}ms)`, + ); + } +} + +describe('cobol-preprocessor RE_SET_TO_TRUE β€” linear time on pathological input', () => { + it('matches in <500ms on 50k repetitions of "A OF A " AND 100k/50k ratio is sub-linear when measurable', () => { + // 50k/100k repetitions chosen so timings exceed the + // RATIO_MEASUREMENT_FLOOR_MS noise floor on typical CI hardware. + // Pre-fix nested-quantifier shape would be exponential here; the + // post-fix `.+?` shape is linear (~2Γ— when input doubles). + const inputSmall = 'SET ' + 'A OF A '.repeat(50_000) + 'TO TRUE'; + const inputLarge = 'SET ' + 'A OF A '.repeat(100_000) + 'TO TRUE'; + const elapsedSmall = timeRegex(RE_SET_TO_TRUE, inputSmall); + const elapsedLarge = timeRegex(RE_SET_TO_TRUE, inputLarge); + expect(RE_SET_TO_TRUE.exec(inputSmall)).not.toBeNull(); + expect(elapsedSmall).toBeLessThan(500); + expect(elapsedLarge).toBeLessThan(500); + assertSubLinearRatio(elapsedSmall, elapsedLarge, 'RE_SET_TO_TRUE'); + }); + + it('still matches a normal SET ... TO TRUE statement', () => { + const m = RE_SET_TO_TRUE.exec('SET WS-FLAG TO TRUE'); + expect(m).not.toBeNull(); + expect(m?.[1]).toBe('WS-FLAG'); + }); +}); + +describe('cobol-preprocessor RE_SET_INDEX β€” linear time on pathological input', () => { + it('rejects in <500ms on 50k tokens with no valid suffix AND 100k/50k ratio is sub-linear when measurable', () => { + // Forces backtracking against the (TO|UP\s+BY|DOWN\s+BY) alternation + // β€” the richer pathological surface of the two regexes. + const inputSmall = 'SET ' + 'A '.repeat(50_000) + 'X'; + const inputLarge = 'SET ' + 'A '.repeat(100_000) + 'X'; + const elapsedSmall = timeRegex(RE_SET_INDEX, inputSmall); + const elapsedLarge = timeRegex(RE_SET_INDEX, inputLarge); + expect(RE_SET_INDEX.exec(inputSmall)).toBeNull(); + expect(elapsedSmall).toBeLessThan(500); + expect(elapsedLarge).toBeLessThan(500); + assertSubLinearRatio(elapsedSmall, elapsedLarge, 'RE_SET_INDEX'); + }); + + it('still matches a normal SET INDEX statement', () => { + const m = RE_SET_INDEX.exec('SET WS-IDX TO 5'); + expect(m).not.toBeNull(); + expect(m?.[1]).toBe('WS-IDX'); + expect(m?.[2]).toBe('TO'); + expect(m?.[3]).toBe('5'); + }); +}); + +describe('rust-workspace parseCargoPackageName β€” linear-time line walk', () => { + it('extracts the package name in <500ms on 100k blank lines AND 200k/100k ratio is sub-linear when measurable', () => { + // 100k/200k blank lines chosen so timings exceed the + // RATIO_MEASUREMENT_FLOOR_MS noise floor. Earlier 10k/20k pairing + // produced sub-millisecond measurements where scheduler jitter + // dominated and the ratio became meaningless (a real macOS run + // saw 5.29Γ— between two genuinely-linear sub-ms measurements). + const cargoTomlSmall = + '[package]\n' + '\n'.repeat(100_000) + 'name = "myrepo"\nversion = "0.1.0"\n'; + const cargoTomlLarge = + '[package]\n' + '\n'.repeat(200_000) + 'name = "myrepo"\nversion = "0.1.0"\n'; + const elapsedSmall = timeFn(() => parseCargoPackageName(cargoTomlSmall)); + const elapsedLarge = timeFn(() => parseCargoPackageName(cargoTomlLarge)); + expect(parseCargoPackageName(cargoTomlSmall)).toBe('myrepo'); + expect(elapsedSmall).toBeLessThan(500); + expect(elapsedLarge).toBeLessThan(500); + assertSubLinearRatio(elapsedSmall, elapsedLarge, 'parseCargoPackageName'); + }); + + it('returns null when [package] section is absent', () => { + expect(parseCargoPackageName('[workspace]\nmembers = ["a"]\n')).toBeNull(); + }); + + it('stops at the next section header (does not pick up a name= from a later section)', () => { + const toml = '[package]\nversion = "1.0"\n[other]\nname = "wrong"\n'; + expect(parseCargoPackageName(toml)).toBeNull(); + }); + + it('extracts the name from a normal [package] section', () => { + const toml = '[package]\nname = "real-crate"\nversion = "0.1.0"\n'; + expect(parseCargoPackageName(toml)).toBe('real-crate'); + }); +}); + +describe('cross-impact clampTimeout β€” bounds user-supplied impact timeouts', () => { + it('rejects negative and zero timeouts, returning MIN', () => { + expect(clampTimeout(0)).toBe(IMPACT_TIMEOUT_MIN_MS); + expect(clampTimeout(-1)).toBe(IMPACT_TIMEOUT_MIN_MS); + expect(clampTimeout(-999_999)).toBe(IMPACT_TIMEOUT_MIN_MS); + }); + + it('rejects NaN/Infinity, returning MIN', () => { + expect(clampTimeout(NaN)).toBe(IMPACT_TIMEOUT_MIN_MS); + expect(clampTimeout(Infinity)).toBe(IMPACT_TIMEOUT_MIN_MS); + expect(clampTimeout(-Infinity)).toBe(IMPACT_TIMEOUT_MIN_MS); + }); + + it('caps very large timeouts at MAX (5 minutes)', () => { + expect(clampTimeout(999_999_999)).toBe(IMPACT_TIMEOUT_MAX_MS); + expect(clampTimeout(IMPACT_TIMEOUT_MAX_MS + 1)).toBe(IMPACT_TIMEOUT_MAX_MS); + }); + + it('passes through a reasonable timeout unchanged (truncated to integer)', () => { + expect(clampTimeout(30_000)).toBe(30_000); + expect(clampTimeout(30_500.7)).toBe(30_500); + }); + + it('floors below-MIN positive values to MIN', () => { + expect(clampTimeout(50)).toBe(IMPACT_TIMEOUT_MIN_MS); + expect(clampTimeout(0.1)).toBe(IMPACT_TIMEOUT_MIN_MS); + }); +}); diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index 9330e86a5..862357668 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -60,6 +60,8 @@ export default defineConfig({ 'test/integration/augmentation.test.ts', 'test/integration/staleness-and-stability.test.ts', 'test/integration/lbug-lock-retry.test.ts', + 'test/integration/lbug-open-retry.test.ts', + 'test/integration/lbug-close-handle-release.test.ts', 'test/integration/api-impact-e2e.test.ts', 'test/integration/shape-check-regression.test.ts', 'test/integration/java-class-impact.test.ts', @@ -87,6 +89,8 @@ export default defineConfig({ 'test/integration/augmentation.test.ts', 'test/integration/staleness-and-stability.test.ts', 'test/integration/lbug-lock-retry.test.ts', + 'test/integration/lbug-open-retry.test.ts', + 'test/integration/lbug-close-handle-release.test.ts', 'test/integration/api-impact-e2e.test.ts', 'test/integration/shape-check-regression.test.ts', 'test/integration/java-class-impact.test.ts',