From 467c14caa2dc1d94a7082b7e98ee1e0b4e1c1880 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sat, 16 May 2026 11:15:21 +0100 Subject: [PATCH 1/2] feat(cpp): standard-conversion-sequence ranking for overload resolution (#1606) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cpp): add standard-conversion-sequence ranking to overload resolution (#1578) Introduce `ConversionRankFn` abstraction and `cppConversionRank` implementation to disambiguate C++ overloaded calls by argument-to-parameter conversion cost. Exact type match (rank 0) beats standard arithmetic conversion (rank 2), which beats non-viable mismatch (Infinity). Thread the rank function through `narrowOverloadCandidates`, `pickImplicitThisOverload`, `pickOverload`, and `pickUniqueGlobalCallable` via the `ScopeResolver.conversionRankFn` contract. Add `findAllCallableBindingsInScope` scope walker for collecting all overloads at the first binding scope. Guard against false ambiguity suppression when candidates span different files (local-shadows-import preservation). * fix: address Claude review findings on conversion-rank PR Finding 1 (HIGH): add tests that exercise the conversion ranker. - p('a') with p(int)/p(double): char→int promotion (rank 1) beats char→double conversion (rank 2), forcing step 4b in narrowOverloadCandidates. Exact-type filter misses both overloads. - h(42, 2.5) with h(int,int)/h(double,double): multi-arg tied total score forces the ranker, both candidates score 2 → suppressed. Finding 2 (HIGH): unify multi-candidate suppression across all paths. - Non-ADL free-call: suppress when narrowed.length > 1 (same-file guard), mirroring ADL merged-candidate behavior. - ADL ordinary-only: same pattern. - pickOverload: return OVERLOAD_AMBIGUOUS when candidates.length > 1 after normalized-ambiguity check. - Case 0.5 (this receiver): set ambiguous=true when narrowed > 1. Finding 3+4 (MEDIUM): implement rank-1 integral promotions. - char→int and bool→int now return rank 1 (ISO C++ [conv.prom]). - Updated comment to remove misleading ISO table header; document only the post-normalization ranking that is actually implemented. - Updated ConversionRankFn JSDoc in overload-narrowing.ts. 218/218 C++ tests pass (registry-primary). Legacy: 186+32. * fix: implement pairwise dominance comparison for overload ranking Replace the summed per-slot conversion cost with ISO C++-aligned pairwise dominance comparison ([over.ics.rank]). F1 is better than F2 only when F1 is not worse for every argument and strictly better for at least one. Non-dominated candidates are returned; if multiple remain they are genuinely ambiguous. This fixes false CALLS edges for asymmetric multi-arg overloads: h('a', 2.5) against h(int,int) / h(double,double) — the old summed cost picked h(double,double) (cost 2 < 3), but ISO C++ considers the call ambiguous because h(int,int) is better at arg 0 via char promotion. The pairwise check correctly finds neither dominates. Add h('a', 2.5) test case asserting zero CALLS edges alongside the existing h(42, 2.5) symmetric-tie test. 218/218 C++ tests pass (registry-primary). Legacy: 186+32. * docs: update step 4b JSDoc to reflect pairwise dominance --------- Co-authored-by: Gergő Magyar --- .../languages/cpp/conversion-rank.ts | 47 ++++++ .../ingestion/languages/cpp/scope-resolver.ts | 5 + .../contract/scope-resolver.ts | 15 ++ .../passes/free-call-fallback.ts | 135 +++++++++++++++--- .../passes/overload-narrowing.ts | 107 ++++++++++++++ .../passes/receiver-bound-calls.ts | 29 +++- .../scope-resolution/pipeline/run.ts | 1 + .../cpp-overload-conversion-rank/lib.cpp | 10 ++ .../cpp-overload-conversion-rank/lib.h | 33 +++++ .../test/integration/resolvers/cpp.test.ts | 74 ++++++++++ .../test/integration/resolvers/helpers.ts | 13 ++ 11 files changed, 446 insertions(+), 23 deletions(-) create mode 100644 gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h diff --git a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts new file mode 100644 index 000000000..2a9e3bc01 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts @@ -0,0 +1,47 @@ +/** + * C++ conversion-rank scoring for overload resolution (#1578). + * + * Operates on **normalized** type strings (output of + * `normalizeCppParamType` in `arity-metadata.ts`). After normalization: + * - int/long/short/unsigned → 'int' + * - float/double → 'double' + * - char → 'char', bool → 'bool' + * + * Because the normalizer collapses promotion pairs (int↔long, + * float↔double) to the same string, those promotions are invisible at + * this layer — they appear as exact matches (rank 0). + * + * Post-normalization ranking: + * - rank 0 — exact (same normalized type) + * - rank 1 — integral promotion (char→int, bool→int) + * - rank 2 — standard arithmetic conversion (int↔double, char→double, + * bool→double) + * - Infinity — mismatch (string↔int, user types, pointers, etc.) + * + * This function is intentionally C++-specific (issue #1578 pitfall: + * keep conversion-rank tables out of shared overload-narrowing). Other + * languages may define their own `ConversionRankFn` in the future. + */ + +/** Set of normalized arithmetic types that support implicit conversion. */ +const ARITHMETIC = new Set(['int', 'double', 'char', 'bool']); + +/** Integral promotion targets: char→int and bool→int are rank 1. */ +const INTEGRAL_PROMOTION = new Map([ + ['char', 'int'], + ['bool', 'int'], +]); + +/** + * Return the conversion rank from `argType` to `paramType`. + * + * @returns 0 for exact match, 1 for integral promotion (char/bool→int), + * 2 for standard arithmetic conversion, Infinity for mismatch. + */ +export function cppConversionRank(argType: string, paramType: string): number { + if (argType === paramType) return 0; + // Integral promotions: char→int, bool→int (ISO C++ [conv.prom]) + if (INTEGRAL_PROMOTION.get(argType) === paramType) return 1; + if (ARITHMETIC.has(argType) && ARITHMETIC.has(paramType)) return 2; + return Infinity; +} diff --git a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts index 52c575cf4..4e226bbac 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts @@ -9,6 +9,7 @@ import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers. import type { ScopeResolver } from '../../scope-resolution/contract/scope-resolver.js'; import { cppProvider } from '../c-cpp.js'; import { cppArityCompatibility } from './arity.js'; +import { cppConversionRank } from './conversion-rank.js'; import { cppMergeBindings } from './merge-bindings.js'; import { resolveCppImportTarget } from './import-target.js'; import { scanCppHeaderFiles } from './header-scan.js'; @@ -169,6 +170,10 @@ export const cppScopeResolver: ScopeResolver = { propagatesReturnTypesAcrossImports: true, // C++ #include brings in all symbols — enable global free call fallback allowGlobalFreeCallFallback: true, + // C++ standard-conversion-sequence ranking for overload resolution (#1578). + // Disambiguates `f(int)` vs `f(double)` called with `f(2.5)` by scoring + // each candidate's conversion cost; exact match wins over standard conversion. + conversionRankFn: cppConversionRank, // Range-for element type inference: for (auto& user : users) → bind user to User populateRangeBindings: populateCppRangeBindings, // C++ method return-type bindings need to be visible from module scope diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index 315471de1..c6f494368 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -264,6 +264,7 @@ import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { LanguageProvider } from '../../language-provider.js'; import { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import type { SemanticModel } from '../../model/semantic-model.js'; +import type { ConversionRankFn } from '../passes/overload-narrowing.js'; /** A LinearizeStrategy receives the full ancestor map so C3-style * algorithms (which need to merge each parent's MRO) can implement @@ -533,6 +534,20 @@ export interface ScopeResolver { */ readonly allowGlobalFreeCallFallback?: boolean; + /** + * Optional per-slot conversion-rank function for overload resolution. + * When provided, `narrowOverloadCandidates` uses ranked scoring as a + * fallback when the exact-type filter produces no match. The function + * returns a numeric cost (0 = exact, 1 = promotion, 2 = standard + * conversion, Infinity = incompatible) for converting an argument + * type to a parameter type. + * + * The conversion-rank table is language-specific (issue #1578 pitfall: + * keep it out of shared overload-narrowing). C++ provides + * `cppConversionRank`; other languages define their own if needed. + */ + readonly conversionRankFn?: ConversionRankFn; + /** * Optional predicate to identify definitions with file-local linkage * (e.g. C `static` functions). When provided, `pickUniqueGlobalCallable` diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index d9e4cdaa3..2be4a6809 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -25,6 +25,7 @@ import type { WorkspaceResolutionIndex } from '../workspace-index.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js'; import { + findAllCallableBindingsInScope, findCallableBindingInScope, findCallableBindingsAndAdlBlocker, findClassBindingInScope, @@ -32,6 +33,7 @@ import { import { isOverloadAmbiguousAfterNormalization, narrowOverloadCandidates, + type ConversionRankFn, } from './overload-narrowing.js'; export function emitFreeCallFallback( @@ -63,6 +65,7 @@ export function emitFreeCallFallback( scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], ) => readonly SymbolDefinition[] | undefined; + readonly conversionRankFn?: ConversionRankFn; } = {}, ): number { let emitted = 0; @@ -90,16 +93,59 @@ export function emitFreeCallFallback( // the same name in a single class, choose the best match by // arity + argument types. if (fnDef === undefined) { - fnDef = pickImplicitThisOverload(site, scopes, workspaceIndex, model); + fnDef = pickImplicitThisOverload( + site, + scopes, + workspaceIndex, + model, + options.conversionRankFn, + ); } + // Scope-chain callable lookup. First-match preserves scope-chain + // precedence (local shadows import). When a conversion-rank function + // is available AND the binding scope contains multiple overloads, + // refine with `narrowOverloadCandidates` to pick the best overload + // by argument types (#1578). The first-match result is kept as a + // fallback when narrowing is indeterminate. if (fnDef === undefined) { if (options.resolveAdlCandidates === undefined) { + // Non-ADL path: first-match preserves scope-chain precedence + // (local shadows import). When a conversion-rank function is + // available AND the binding scope contains multiple overloads, + // refine with narrowOverloadCandidates (#1578). fnDef = findCallableBindingInScope(site.inScope, site.name, scopes); + if (fnDef !== undefined && options.conversionRankFn !== undefined) { + const allCallables = findAllCallableBindingsInScope(site.inScope, site.name, scopes); + if (allCallables.length > 1) { + const narrowed = narrowOverloadCandidates( + allCallables, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); + if (narrowed.length === 1) { + fnDef = narrowed[0]; + } else if (narrowed.length > 1) { + // Multiple survivors after conversion-rank scoring. + // Suppress when all candidates share the same file (true + // overloads) — mirrors ADL merged-candidate path behavior. + // Cross-file candidates are shadowing; keep first-match. + const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); + if (sameFile) { + handledSites.add( + `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`, + ); + continue; + } + } + // narrowed.length === 0: keep the first-match fnDef — + // preserves local-shadows-import. + } + } } else { - // ISO C++ `[basic.lookup.unqual]` §7: ADL is suppressed when - // ordinary lookup finds a non-function name (variable, class, enum) - // or a block-scope function declaration (not via using-declaration) - // at the nearest scope where the name exists. + // ADL path: ISO C++ `[basic.lookup.unqual]` §7 — ADL is suppressed + // when ordinary lookup finds a non-function name or a block-scope + // function declaration. const { callables: ordinary, nonCallableFound, @@ -120,43 +166,67 @@ export function emitFreeCallFallback( parsedFiles, ); - // Preserve existing ordinary-lookup behavior when ADL contributed - // no candidates. + // When ADL contributed no candidates, narrow ordinary candidates + // with conversion-rank scoring when multiple overloads exist. + // Single candidate or empty falls through to first-match. if (adl === undefined || adl.length === 0) { - fnDef = ordinary[0]; + if (ordinary.length <= 1 || options.conversionRankFn === undefined) { + fnDef = ordinary[0]; + } else { + const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; + const narrowed = narrowOverloadCandidates( + ordinary, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); + if (narrowed.length === 1) { + fnDef = narrowed[0]; + } else if (narrowed.length > 1) { + // Multiple survivors — suppress when same-file (true + // overloads), mirrors ADL merged-candidate behavior. + const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); + if (sameFile) { + handledSites.add(siteKey); + continue; + } + fnDef = ordinary[0]; // cross-file shadowing → first-match + } else { + fnDef = ordinary[0]; // narrowed empty → first-match + } + } } else { const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; const merged: SymbolDefinition[] = []; - const seen = new Set(); + const seenMerge = new Set(); const push = (defs: readonly SymbolDefinition[]): void => { for (const d of defs) { - if (seen.has(d.nodeId)) continue; - seen.add(d.nodeId); + if (seenMerge.has(d.nodeId)) continue; + seenMerge.add(d.nodeId); merged.push(d); } }; push(ordinary); push(adl); - const narrowed = narrowOverloadCandidates(merged, site.arity, site.argumentTypes); + const narrowed = narrowOverloadCandidates( + merged, + site.arity, + site.argumentTypes, + options.conversionRankFn, + ); if (narrowed.length === 1) { fnDef = narrowed[0]; } else if (narrowed.length === 0) { - // ADL contributed candidates, but none survived arity/type - // narrowing. Treat as handled to avoid global-name fallback - // binding to the same mismatched symbol by simple-name - // uniqueness. handledSites.add(siteKey); continue; } else if (narrowed.length > 1) { - // Suppress ambiguous overload calls (emit zero edges) when - // merged ordinary+ADL candidate sets cannot be disambiguated. if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { handledSites.add(siteKey); continue; } - // Multiple survivors remain but no conversion-ranking step - // exists yet; suppress instead of picking arbitrarily. + // Multiple survivors remain after conversion-rank scoring; + // suppress instead of picking arbitrarily. handledSites.add(siteKey); continue; } @@ -184,6 +254,8 @@ export function emitFreeCallFallback( scopes, }) : undefined, + site.argumentTypes, + options.conversionRankFn, ); } if (fnDef === undefined) continue; @@ -222,6 +294,8 @@ function pickUniqueGlobalCallable( isFileLocalDef?: (def: SymbolDefinition) => boolean, callArity?: number, isCallerVisible?: (candidate: SymbolDefinition) => boolean, + callArgTypes?: readonly string[], + conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { const scopeDefs: SymbolDefinition[] = []; const scopeSeen = new Set(); @@ -256,6 +330,14 @@ function pickUniqueGlobalCallable( const arityMatch = narrowByArity(scopeDefs, callArity); if (arityMatch !== undefined) return arityMatch; } + // When arity narrowing left >1 candidate, try overload narrowing with + // argument types + conversion ranking (#1578). This picks the unique + // best-rank candidate when exact-type or conversion-rank scoring can + // disambiguate (e.g., `f(int)` vs `f(double)` called with `f(2.5)`). + if (scopeDefs.length > 1) { + const narrowed = narrowOverloadCandidates(scopeDefs, callArity, callArgTypes, conversionRankFn); + if (narrowed.length === 1) return narrowed[0]; + } const defs: SymbolDefinition[] = []; const seen = new Set(); @@ -289,6 +371,11 @@ function pickUniqueGlobalCallable( const arityMatch = narrowByArity(defs, callArity); if (arityMatch !== undefined) return arityMatch; } + // Same argument-type + conversion-rank narrowing for the model pool. + if (defs.length > 1) { + const narrowed = narrowOverloadCandidates(defs, callArity, callArgTypes, conversionRankFn); + if (narrowed.length === 1) return narrowed[0]; + } return undefined; } @@ -362,6 +449,7 @@ export function pickImplicitThisOverload( scopes: ScopeResolutionIndexes, workspaceIndex: WorkspaceResolutionIndex, model: SemanticModel, + conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { // Find the enclosing Class scope by walking parents. let curId: ScopeId | null = site.inScope; @@ -389,7 +477,12 @@ export function pickImplicitThisOverload( // ambiguous narrowing (multiple compatible candidates with no // disambiguating signal) leaves the call unresolved rather than // routing to an arbitrary first overload by registration order. - const candidates = narrowOverloadCandidates(overloads, site.arity, site.argumentTypes); + const candidates = narrowOverloadCandidates( + overloads, + site.arity, + site.argumentTypes, + conversionRankFn, + ); if (candidates.length !== 1) return undefined; return candidates[0]; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts index bff16d27e..5c9338f40 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts @@ -24,15 +24,35 @@ * equality. An empty string in `argTypes[i]` means "unknown" and * counts as a match. Mismatches disqualify. A non-empty typed * result wins; otherwise return the arity-filtered candidates. + * 4b. When the exact-type filter from step 4 returns empty AND a + * `conversionRankFn` is provided, rank candidates via pairwise + * dominance comparison (ISO C++ [over.ics.rank]): F1 beats F2 + * only when F1 is not worse for every arg and better for at + * least one. Non-dominated candidates are returned; multiple + * survivors are genuinely ambiguous. * 5. Empty input returns empty output. */ import type { SymbolDefinition } from 'gitnexus-shared'; +/** + * Per-slot conversion-rank function. Returns a numeric cost for + * converting `argType` to `paramType`: + * - 0 = exact match (no conversion) + * - 1 = promotion (e.g. char→int, bool→int in C++) + * - 2 = standard conversion (e.g. int→double) + * - Infinity = incompatible types + * + * Each language provides its own implementation. The function operates + * on normalized type strings (output of the language's type normalizer). + */ +export type ConversionRankFn = (argType: string, paramType: string) => number; + export function narrowOverloadCandidates( overloads: readonly SymbolDefinition[], argCount: number | undefined, argTypes: readonly string[] | undefined, + conversionRankFn?: ConversionRankFn, ): readonly SymbolDefinition[] { if (overloads.length === 0) return []; @@ -84,11 +104,98 @@ export function narrowOverloadCandidates( return true; }); if (typed.length > 0) return typed; + + // ── Conversion-rank scoring (step 4b) ────────────────────────── + // The exact-type filter above rejected every candidate. When a + // per-language conversion-rank function is available, rank via + // pairwise dominance: F1 beats F2 only when F1 is not worse for + // every arg and better for at least one. Non-dominated candidates + // are returned; multiple survivors are genuinely ambiguous. + if (conversionRankFn !== undefined) { + const ranked = rankByConversion(candidates, argTypes, conversionRankFn); + if (ranked.length > 0) return ranked; + } } return candidates; } +/** + * Pairwise dominance comparison (ISO C++ [over.ics.rank]). + * + * F1 is a better match than F2 when F1's conversion rank is **not + * worse** for every argument AND **strictly better** for at least one. + * Candidates dominated by any other viable candidate are removed. + * If more than one non-dominated candidate remains, they are genuinely + * ambiguous — callers suppress the edge rather than picking arbitrarily. + * + * Candidates with at least one `Infinity`-ranked slot (incompatible + * type) are excluded before pairwise comparison begins. + */ +function rankByConversion( + candidates: readonly SymbolDefinition[], + argTypes: readonly string[], + rankFn: ConversionRankFn, +): readonly SymbolDefinition[] { + // Step 1: compute per-slot ranks and exclude non-viable candidates. + const viable: Array<{ def: SymbolDefinition; ranks: number[] }> = []; + for (const d of candidates) { + const params = d.parameterTypes; + if (params === undefined) continue; + const ranks: number[] = []; + let ok = true; + for (let i = 0; i < argTypes.length && i < params.length; i++) { + if (argTypes[i] === '') { + ranks.push(0); // unknown arg → any-match (rank 0) + continue; + } + const r = rankFn(argTypes[i], params[i]); + if (!isFinite(r)) { + ok = false; + break; + } + ranks.push(r); + } + if (!ok) continue; + viable.push({ def: d, ranks }); + } + if (viable.length <= 1) return viable.map((v) => v.def); + + // Step 2: pairwise dominance — remove candidates dominated by any other. + const dominated = new Set(); + for (let i = 0; i < viable.length; i++) { + if (dominated.has(i)) continue; + for (let j = i + 1; j < viable.length; j++) { + if (dominated.has(j)) continue; + const cmp = pairwiseCompare(viable[i].ranks, viable[j].ranks); + if (cmp < 0) + dominated.add(j); // i dominates j + else if (cmp > 0) dominated.add(i); // j dominates i + } + } + return viable.filter((_, idx) => !dominated.has(idx)).map((v) => v.def); +} + +/** + * Compare two per-slot rank vectors. + * Returns -1 if `a` dominates `b` (not worse everywhere, better somewhere), + * +1 if `b` dominates `a`, + * 0 if neither dominates (incomparable or equal). + */ +function pairwiseCompare(a: readonly number[], b: readonly number[]): -1 | 0 | 1 { + let aBetter = false; + let bBetter = false; + const len = Math.min(a.length, b.length); + for (let i = 0; i < len; i++) { + if (a[i] < b[i]) aBetter = true; + else if (b[i] < a[i]) bBetter = true; + if (aBetter && bBetter) return 0; // incomparable — early exit + } + if (aBetter && !bBetter) return -1; + if (bBetter && !aBetter) return 1; + return 0; +} + /** * Detect when >1 candidate share identical `parameterTypes` after the * per-language normalizer has collapsed distinct underlying types. This diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 92b4c1ac1..ffc29d176 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -73,6 +73,7 @@ type ReceiverBoundProviderSubset = Pick< | 'hoistTypeBindingsToModule' | 'resolveQualifiedReceiverMember' | 'resolveThisViaEnclosingClass' + | 'conversionRankFn' >; function normalizeTemplateArgToken(value: string): string { @@ -343,6 +344,7 @@ export function emitReceiverBoundCalls( methodOverloads, site.arity, site.argumentTypes, + provider.conversionRankFn, ); if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { ambiguous = true; @@ -356,6 +358,12 @@ export function emitReceiverBoundCalls( hiddenByName = true; break; } + // Multiple tied survivors with distinct param types (e.g. + // h(int,double) vs h(double,int) both scoring 2) → ambiguous. + if (narrowed.length > 1) { + ambiguous = true; + break; + } memberDef = narrowed[0] ?? methodOverloads[0]; break; } @@ -640,7 +648,13 @@ export function emitReceiverBoundCalls( let memberDef: SymbolDefinition | undefined; let ambiguous = false; for (const ownerId of chain) { - const picked = pickOverload(ownerId, memberName, site, model); + const picked = pickOverload( + ownerId, + memberName, + site, + model, + provider.conversionRankFn, + ); if (picked === OVERLOAD_AMBIGUOUS) { ambiguous = true; break; @@ -708,6 +722,7 @@ function pickOverload( memberName: string, site: ParsedFile['referenceSites'][number], model: SemanticModel, + conversionRankFn?: (argType: string, paramType: string) => number, ): SymbolDefinition | typeof OVERLOAD_AMBIGUOUS | undefined { const overloads = model.methods.lookupAllByOwner(ownerId, memberName); if (overloads.length === 0) { @@ -718,7 +733,12 @@ function pickOverload( } if (overloads.length === 1) return overloads[0]; - const candidates = narrowOverloadCandidates(overloads, site.arity, site.argumentTypes); + const candidates = narrowOverloadCandidates( + overloads, + site.arity, + site.argumentTypes, + conversionRankFn, + ); // When narrowing leaves >1 candidate that share identical normalized // parameter-types (e.g., C++ `f(int)` vs `f(long)` both collapsed to // `['int']` by `normalizeCppParamType`), suppress the edge entirely. @@ -726,6 +746,11 @@ function pickOverload( // would arbitrarily pick a candidate and lie about the call's target. // PR #1520 review follow-up plan U2 / Claude review Finding 5. if (isOverloadAmbiguousAfterNormalization(candidates, site.arity)) return OVERLOAD_AMBIGUOUS; + // When conversion-rank scoring leaves >1 tied candidate with distinct + // parameter types (e.g. h(int,double) vs h(double,int) both scoring 2), + // suppress rather than picking arbitrarily — C++ would call this + // ambiguous. Mirrors ADL merged-candidate suppression behavior. + if (candidates.length > 1) return OVERLOAD_AMBIGUOUS; return candidates[0] ?? overloads[0]; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index 0809d59ca..5493368da 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -382,6 +382,7 @@ export function runScopeResolution( isFileLocalDef: provider.isFileLocalDef, isCallableVisibleFromCaller: provider.isCallableVisibleFromCaller, resolveAdlCandidates: provider.resolveAdlCandidates, + conversionRankFn: provider.conversionRankFn, }, ); const { emitted, skipped } = emitReferencesViaLookup( diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp new file mode 100644 index 000000000..cd9c51210 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.cpp @@ -0,0 +1,10 @@ +#include "lib.h" + +void Service::f(int x) {} +void Service::f(double x) {} +void Service::g(int x) {} +void Service::g(long x) {} +void Service::h(int a, int b) {} +void Service::h(double a, double b) {} +void Service::p(int x) {} +void Service::p(double x) {} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h new file mode 100644 index 000000000..5a367ba4b --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-conversion-rank/lib.h @@ -0,0 +1,33 @@ +#pragma once + +class Service { +public: + // Variant 1 & 3: f(int) vs f(double) + void f(int x); + void f(double x); + + // Variant 2: g(int) vs g(long) — both normalize to 'int' + void g(int x); + void g(long x); + + // Variant 4: multi-arg tied total score + void h(int a, int b); + void h(double a, double b); + + // Variant 5: char-literal promotion (exercises conversion ranker) + void p(int x); + void p(double x); + + // Inline: call sites live inside the class scope so the scope-chain + // walk finds the Class scope, enabling pickImplicitThisOverload to + // resolve overloads against the declaration-side Method nodes (which + // carry distinct parameterTypes and graph-node IDs). + void run() { + f(2.5); // Variant 1: double literal -> f(double) wins (exact > standard) + f(42); // Variant 3: int literal -> f(int) wins (exact > standard) + g(42); // Variant 2: int/long both normalize to 'int' -> ambiguous + h(42, 2.5); // Variant 4: incomparable — neither dominates the other -> ambiguous + h('a', 2.5);// Variant 6: asymmetric — h(int,int) better at arg0 (promotion), h(double,double) better at arg1 (exact) -> ambiguous + p('a'); // Variant 5: char literal -> p(int) wins via promotion (rank 1 < rank 2) + } +}; diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index de0ad9632..8e2eaf37f 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1762,6 +1762,80 @@ describe('C++ ambiguous integer-width overloads', () => { }); }); +// --------------------------------------------------------------------------- +// C++ overload resolution: standard-conversion-sequence ranking (#1578) +// Disambiguates overloads when exact normalized-type matching cannot, +// by scoring each candidate's conversion cost. Exact match (rank 0) wins +// over standard conversion (rank 2); same-rank ties still suppress. +// --------------------------------------------------------------------------- + +describe('C++ overload resolution — conversion-rank disambiguation (#1578)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-overload-conversion-rank'), + () => {}, + ); + }, 60000); + + it('f(2.5) resolves to f(double) — exact match beats standard conversion', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'run' && c.target === 'f'); + // Conversion-rank scoring picks f(double) as the unique best: + // f(double) is exact match (rank 0), f(int) is standard conversion (rank 2). + const fDoubleEdges = fCalls.filter((c) => { + const tgt = result.graph.getNode(c.rel.targetId); + return tgt?.properties.parameterTypes?.[0] === 'double'; + }); + expect(fDoubleEdges.length).toBe(1); + }); + + it('f(42) resolves to f(int) — exact match beats standard conversion', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'run' && c.target === 'f'); + // f(int) is exact match (rank 0), f(double) is standard conversion (rank 2). + const fIntEdges = fCalls.filter((c) => { + const tgt = result.graph.getNode(c.rel.targetId); + return tgt?.properties.parameterTypes?.[0] === 'int'; + }); + expect(fIntEdges.length).toBe(1); + }); + + it('g(42) emits zero CALLS edges — int/long normalize to same type, ambiguous', () => { + const calls = getRelationships(result, 'CALLS'); + const gCalls = calls.filter((c) => c.source === 'run' && c.target === 'g'); + // g(int) and g(long) both normalize to parameterTypes=['int'], + // so isOverloadAmbiguousAfterNormalization triggers suppression. + expect(gCalls.length).toBe(0); + }); + + it("p('a') resolves to p(int) — char promotion (rank 1) beats char→double conversion (rank 2)", () => { + const calls = getRelationships(result, 'CALLS'); + const pCalls = calls.filter((c) => c.source === 'run' && c.target === 'p'); + // p('a'): argType='char'. Exact-type filter misses both p(int) and + // p(double), forcing the conversion ranker (step 4b). char→int is an + // integral promotion (rank 1), char→double is a standard conversion + // (rank 2). p(int) wins with the lower total cost. + expect(pCalls.length).toBe(1); + const tgt = result.graph.getNode(pCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes?.[0]).toBe('int'); + }); + + it('h(42, 2.5) emits zero CALLS edges — incomparable multi-arg overloads, ambiguous', () => { + const calls = getRelationships(result, 'CALLS'); + const hCalls = calls.filter((c) => c.source === 'run' && c.target === 'h'); + // h(42, 2.5) + h('a', 2.5): both call sites produce incomparable + // pairwise rankings. For h(42, 2.5) with argTypes=['int','double']: + // h(int,int): [rank('int','int')=0, rank('double','int')=2] + // h(double,double): [rank('int','double')=2, rank('double','double')=0] + // h(int,int) better at arg0, h(double,double) better at arg1 → neither + // dominates → ambiguous. Same pattern for h('a',2.5). + // Contract: zero edges for ALL h() call sites combined (dedup). + expect(hCalls.length).toBe(0); + }); +}); + // --------------------------------------------------------------------------- // U3: anonymous-namespace symbols MUST NOT leak across translation units // (full-pipeline integration test; unit-level coverage exists separately) diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index ae2a75f04..bc18b0374 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -175,6 +175,19 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly::g_unqualified() -> f() does NOT bind to Base::f', 'Derived::g_this() -> this->f() resolves to Base::f (1 edge)', 'Derived::g() -> this->f() emits zero CALLS edges when only hidden derived overload is arity-incompatible', + // Conversion-rank scoring (#1578) disambiguates `f(int)` vs `f(double)` + // by ranking exact match over standard conversion. The legacy DAG has no + // conversion-rank scoring; it either picks arbitrarily or leaves the call + // unresolved. Scope-resolver-only correctness win. + 'f(2.5) resolves to f(double) — exact match beats standard conversion', + 'f(42) resolves to f(int) — exact match beats standard conversion', + 'g(42) emits zero CALLS edges — int/long normalize to same type, ambiguous', + // char-literal promotion exercises the conversion ranker (step 4b). + // Legacy DAG has no conversion-rank scoring. Scope-resolver-only. + "p('a') resolves to p(int) — char promotion (rank 1) beats char→double conversion (rank 2)", + // Multi-arg incomparable overloads: pairwise dominance check finds + // neither h(int,int) nor h(double,double) dominates. Scope-resolver-only. + 'h(42, 2.5) emits zero CALLS edges — incomparable multi-arg overloads, ambiguous', // The legacy DAG path has no inline-namespace same-name ambiguity // detection. When two inline children declare the same name, the // legacy path picks an arbitrary match. The scope-resolver returns From a26ac55fb00c761abf8d157554263c9a4a2cf05c Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 11:45:32 +0100 Subject: [PATCH 2/2] fix(lbug): Recover `gitnexus analyze` from orphan LadybugDB sidecars when main DB file is missing (#1622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * fix: recover from orphan lbug sidecars on init Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6e8ea6e8-f9ab-46ff-9c1b-4d2c73a6452c * test: strengthen orphan sidecar recovery coverage Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6e8ea6e8-f9ab-46ff-9c1b-4d2c73a6452c * fix(lbug): only clean orphan sidecars when DB is missing Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): cover no-cleanup path when db file exists Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): use errno-shaped ENOENT mocks for sidecar recovery Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): cover partial sidecar and unlink-failure recovery cases Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * refactor(lbug): tighten ENOENT detection and test naming Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * test(lbug): normalize errno mock helpers across sidecar tests Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a34217b5-0e98-4949-bae1-2a50933f291e * docs(lbug): annotate orphan `.wal.checkpoint` cleanup provenance Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/7edf5156-43e0-412d-87a4-bf4b2934deac * test(lbug): clarify unlink-failure path test intent Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/7edf5156-43e0-412d-87a4-bf4b2934deac * fix(lbug): handle orphan-sidecar cleanup error paths explicitly Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * refactor(lbug): extract errno and error-summary helpers Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * test(lbug): expand non-ENOENT lstat coverage and remove magic number Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/93294b2c-57f6-459c-8eb2-86e3b8920fb0 * test(lbug): add native integration test for orphan sidecar recovery Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2dd28264-4604-430a-a249-af52afd29245 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(lbug): annotate best-effort catch in integration test cleanup Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2dd28264-4604-430a-a249-af52afd29245 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(lbug): add cross-process init lock for orphan sidecar cleanup with integration tests Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e4cbcfec-a252-449d-8d65-2f3570a253f8 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor(lbug): use INIT_LOCK_STALE_MS in stale lock detection and address review feedback Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/e4cbcfec-a252-449d-8d65-2f3570a253f8 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * style(lbug): fix Prettier line-length violation in acquireInitLock fs.open call Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/a140b567-0e9b-4ec9-a158-9fe6b8685ec2 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(lbug): ensure parent directory exists before creating init lock file acquireInitLock tried to create `${dbPath}.init.lock` using O_CREAT | O_EXCL, but on a fresh repo the parent directory (`.gitnexus/`) doesn't exist yet — the mkdir call was inside the locked section. This caused ENOENT failures on all platforms (Windows, macOS, Ubuntu) during `gitnexus analyze`. Move mkdir to before the lock file creation attempt. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6883dc3c-36eb-4907-bcd8-61d23e2c641a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(lbug): verify acquireInitLock succeeds when parent directory does not exist Adds an integration test proving the fix from the previous commit: acquireInitLock now creates the parent directory before attempting to create the lock file, preventing ENOENT on fresh repos. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6883dc3c-36eb-4907-bcd8-61d23e2c641a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergő Magyar Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- gitnexus/src/core/lbug/lbug-adapter.ts | 220 ++++++++- .../lbug-orphan-sidecar-recovery.test.ts | 330 +++++++++++++ .../unit/lbug-checkpoint-lifecycle.test.ts | 454 ++++++++++++++++++ 3 files changed, 996 insertions(+), 8 deletions(-) create mode 100644 gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index cf8f1cb71..54d98b667 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -1,5 +1,5 @@ import fs from 'fs/promises'; -import { createReadStream, createWriteStream } from 'fs'; +import { createReadStream, createWriteStream, constants as fsConstants } from 'fs'; import { createInterface } from 'readline'; import { once } from 'events'; import { finished } from 'stream/promises'; @@ -201,6 +201,163 @@ export const isReadOnlyDbError = (err: unknown): boolean => { return /read-only database/i.test(msg); }; +const isMissingFileError = (err: unknown): boolean => { + const errno = err as NodeJS.ErrnoException; + return errno?.code === 'ENOENT'; +}; + +const extractErrnoCode = (err: unknown): string | undefined => { + const errno = err as NodeJS.ErrnoException; + return errno?.code; +}; + +const MAX_LOGGED_ERROR_MESSAGE_LENGTH = 160; + +const summarizeError = (err: unknown): string => + (err instanceof Error ? err.message : String(err)).slice(0, MAX_LOGGED_ERROR_MESSAGE_LENGTH); + +// --------------------------------------------------------------------------- +// Cross-process init lock +// +// Prevents a TOCTOU race in orphan sidecar cleanup: between checking that +// the main DB file is missing and unlinking sidecars, another process could +// create a fresh DB. The lock file (`${dbPath}.init.lock`) is created with +// O_CREAT | O_EXCL (atomic create-or-fail) and contains the owning PID + +// timestamp so stale locks from crashed processes can be reclaimed. +// --------------------------------------------------------------------------- + +/** Maximum age (ms) before an init lock is considered stale. */ +const INIT_LOCK_STALE_MS = 30_000; +/** Maximum attempts to acquire the init lock before giving up. */ +const INIT_LOCK_MAX_ATTEMPTS = 6; +/** Delay between lock-acquisition retries (ms). */ +const INIT_LOCK_RETRY_DELAY_MS = 500; + +const initLockPath = (dbPath: string): string => `${dbPath}.init.lock`; + +/** + * Returns true when the process identified by `pid` is still running. + * Uses `process.kill(pid, 0)` which sends signal 0 (a no-op probe) — + * it throws ESRCH when the process does not exist. + */ +const isProcessAlive = (pid: number): boolean => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +}; + +/** + * Try to break a stale lock whose owning process has exited. + * Returns `true` if the stale lock was removed (caller should retry acquire). + * Returns `false` if the lock is still valid (another live process owns it). + */ +const tryBreakStaleLock = async (lockPath: string): Promise => { + try { + const content = await fs.readFile(lockPath, 'utf-8'); + const parsed = JSON.parse(content) as { pid?: number; ts?: number }; + + // If the owning process is still alive AND the lock is not stale, don't break. + if (typeof parsed.pid === 'number' && isProcessAlive(parsed.pid)) { + // Even a live process's lock can be stale if it's been held too long + // (e.g. the process is hung). Check the timestamp. + if (typeof parsed.ts === 'number' && Date.now() - parsed.ts < INIT_LOCK_STALE_MS) { + return false; + } + } + + // PID is gone or lock exceeded INIT_LOCK_STALE_MS — reclaim it. + await fs.unlink(lockPath); + logger.warn( + `GitNexus: removed stale init lock (pid=${parsed.pid ?? '?'}, age=${typeof parsed.ts === 'number' ? `${Date.now() - parsed.ts}ms` : '?'})`, + ); + return true; + } catch (err) { + // Lock file disappeared between our read and unlink, or is unreadable. + // Either way, let the caller retry the acquire. + if (isMissingFileError(err)) return true; + // Permission error or corrupt content — log and let caller retry. + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: unable to inspect init lock (${code ?? 'UNKNOWN'}): ${summarizeError(err)}`, + ); + return false; + } +}; + +/** + * Acquire a cross-process init lock for `dbPath`. + * Uses `O_CREAT | O_EXCL` for atomic create-or-fail semantics. + * + * Returns a release function that removes the lock file. The release + * function is idempotent and safe to call even if the lock was already + * cleaned up externally. + * + * Throws if the lock cannot be acquired after `INIT_LOCK_MAX_ATTEMPTS`. + */ +export const acquireInitLock = async (dbPath: string): Promise<() => Promise> => { + const lockPath = initLockPath(dbPath); + const payload = JSON.stringify({ pid: process.pid, ts: Date.now() }); + + // Ensure the parent directory exists before creating the lock file. + // On a fresh repo the `.gitnexus/` directory may not exist yet, and + // fs.open with O_CREAT | O_EXCL would fail with ENOENT. + await fs.mkdir(path.dirname(lockPath), { recursive: true }); + + for (let attempt = 1; attempt <= INIT_LOCK_MAX_ATTEMPTS; attempt++) { + try { + const handle = await fs.open( + lockPath, + fsConstants.O_CREAT | fsConstants.O_EXCL | fsConstants.O_WRONLY, + ); + await handle.writeFile(payload); + await handle.close(); + + // Return the idempotent release function + return async () => { + try { + await fs.unlink(lockPath); + } catch (err) { + if (!isMissingFileError(err)) { + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: failed to release init lock (${code ?? 'UNKNOWN'}): ${summarizeError(err)}`, + ); + } + } + }; + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code !== 'EEXIST') { + throw err; // Unexpected error — propagate immediately + } + + // Lock file exists — check if it's stale + const broken = await tryBreakStaleLock(lockPath); + if (broken && attempt < INIT_LOCK_MAX_ATTEMPTS) { + continue; // Stale lock removed — retry immediately + } + + if (attempt === INIT_LOCK_MAX_ATTEMPTS) { + throw new Error( + `GitNexus: unable to acquire init lock after ${INIT_LOCK_MAX_ATTEMPTS} attempts — ` + + `another gitnexus process may be initializing the same database (${lockPath})`, + ); + } + + // Live process holds the lock — wait and retry + await new Promise((resolve) => setTimeout(resolve, INIT_LOCK_RETRY_DELAY_MS)); + } + } + + // Unreachable — loop always throws or returns + throw new Error('GitNexus: init lock acquisition failed unexpectedly'); +}; + +/** Exported for testing — returns the lock file path for a given dbPath. */ +export const _initLockPathForTest = initLockPath; + const runWithSessionLock = async (operation: () => Promise): Promise => { const previous = sessionLock; let release: (() => void) | null = null; @@ -364,17 +521,64 @@ const doInitLbug = async (dbPath: string) => { await fs.rm(dbPath, { recursive: true, force: true }); } // If it's a file, assume it's an existing LadybugDB database - LadybugDB will open it - } catch { + } catch (err) { + if (!isMissingFileError(err)) { + throw err; + } // Path doesn't exist, which is what LadybugDB wants for a new database } - // Ensure parent directory exists - const parentDir = path.dirname(dbPath); - await fs.mkdir(parentDir, { recursive: true }); + // --------------------------------------------------------------------------- + // Cross-process critical section: acquire init lock, clean orphan sidecars, + // and open the database. The lock prevents a TOCTOU race where another + // process could create a fresh DB between our access() check and the + // unlink() of stale sidecars. + // --------------------------------------------------------------------------- + const releaseInitLock = await acquireInitLock(dbPath); + try { + // Crash-recovery cleanup: if the main DB file is missing, stale sidecars + // from an interrupted run can block fresh opens indefinitely. + try { + await fs.access(dbPath); + } catch (err) { + if (isMissingFileError(err)) { + // `.shadow` is documented by LadybugDB checkpointing and `.wal.checkpoint` + // was observed in the #1618 crash loop that motivated this recovery path. + const orphanSidecars = [`${dbPath}.shadow`, `${dbPath}.wal.checkpoint`]; + for (const sidecar of orphanSidecars) { + try { + await fs.unlink(sidecar); + logger.warn( + `GitNexus: removed orphan sidecar ${path.basename(sidecar)} (no main DB file present)`, + ); + } catch (err) { + if (isMissingFileError(err)) { + continue; + } + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: failed to remove orphan sidecar ${path.basename(sidecar)} (${code ?? 'UNKNOWN'}) while main DB file is missing; LadybugDB open may still fail: ${summarizeError(err)}`, + ); + } + } + } else { + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: unable to verify main DB file before orphan sidecar cleanup (${code ?? 'UNKNOWN'}); skipping cleanup: ${summarizeError(err)}`, + ); + } + } - const opened = await openLbugConnection(lbug, dbPath); - db = opened.db; - conn = opened.conn; + // Ensure parent directory exists + const parentDir = path.dirname(dbPath); + await fs.mkdir(parentDir, { recursive: true }); + + const opened = await openLbugConnection(lbug, dbPath); + db = opened.db; + conn = opened.conn; + } finally { + await releaseInitLock(); + } for (const schemaQuery of SCHEMA_QUERIES) { try { diff --git a/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts b/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts new file mode 100644 index 000000000..e74cd5cf7 --- /dev/null +++ b/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts @@ -0,0 +1,330 @@ +/** + * Integration test: orphan sidecar recovery in doInitLbug. + * + * Exercises the real `initLbug` → `doInitLbug` path against a native + * LadybugDB instance. Creates actual orphan `.shadow` and + * `.wal.checkpoint` files on disk (without a main DB file) and confirms + * that `initLbug` cleans them up and opens a fresh database successfully. + * + * This complements the unit-level mocked coverage in + * `lbug-checkpoint-lifecycle.test.ts` with a real-filesystem, + * real-LadybugDB integration proof required by DoD §2.7. + */ +import fs from 'fs/promises'; +import path from 'path'; +import { describe, it, expect } from 'vitest'; +import { createTempDir } from '../helpers/test-db.js'; + +/** + * LadybugDB 0.16.0 has a known Windows-only regression: `Database.close()` + * does not release the underlying file lock until the process exits, so any + * `closeLbug()` followed by `initLbug(samePath)` in the same process raises + * Win32 Error 33. Skip reopen-dependent tests on Windows. + */ +const itLbugReopen = process.platform === 'win32' ? it.skip : it; + +describe('orphan sidecar recovery — native integration', () => { + itLbugReopen( + 'initLbug recovers when both .shadow and .wal.checkpoint orphan sidecars are present without a main DB file', + async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const shadowPath = `${dbPath}.shadow`; + const walCheckpointPath = `${dbPath}.wal.checkpoint`; + + try { + // Simulate crash-recovery state: orphan sidecars without main DB file + await fs.writeFile(shadowPath, 'stale-shadow-data'); + await fs.writeFile(walCheckpointPath, 'stale-wal-checkpoint-data'); + + // Confirm precondition: main DB file does NOT exist, sidecars DO + await expect(fs.access(dbPath)).rejects.toThrow(); + await expect(fs.access(shadowPath)).resolves.toBeUndefined(); + await expect(fs.access(walCheckpointPath)).resolves.toBeUndefined(); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // initLbug should clean up orphan sidecars and open a fresh DB + await adapter.initLbug(dbPath); + + // Verify the database is functional — execute a simple query + const rows = await adapter.executeQuery('RETURN 1 AS result'); + expect(rows).toEqual([{ result: 1 }]); + + // Verify orphan sidecars were removed + await expect(fs.access(shadowPath)).rejects.toThrow(); + await expect(fs.access(walCheckpointPath)).rejects.toThrow(); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); + + itLbugReopen( + 'initLbug recovers when only .shadow orphan sidecar is present (partial crash state)', + async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const shadowPath = `${dbPath}.shadow`; + const walCheckpointPath = `${dbPath}.wal.checkpoint`; + + try { + // Only .shadow present — partial crash state + await fs.writeFile(shadowPath, 'stale-shadow-data'); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + const rows = await adapter.executeQuery('RETURN 42 AS answer'); + expect(rows).toEqual([{ answer: 42 }]); + + // .shadow cleaned, .wal.checkpoint was never present + await expect(fs.access(shadowPath)).rejects.toThrow(); + await expect(fs.access(walCheckpointPath)).rejects.toThrow(); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); + + itLbugReopen('initLbug succeeds on a clean path with no orphan sidecars (baseline)', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + const rows = await adapter.executeQuery('RETURN 1 AS ok'); + expect(rows).toEqual([{ ok: 1 }]); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen( + 'initLbug does not attempt orphan cleanup when the main DB file exists', + async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + // Place a marker file with a non-sidecar extension next to the DB path. + // Our cleanup only targets `.shadow` and `.wal.checkpoint` and only when + // the main DB is missing. We verify the DB opens normally and the marker + // remains — proving that init did not perform broad sibling file cleanup. + const markerPath = `${dbPath}.test-marker`; + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // Create a real DB file by initializing normally + await adapter.initLbug(dbPath); + await adapter.closeLbug(); + + // Plant marker file next to the existing DB + await fs.writeFile(markerPath, 'should-survive'); + + // Re-init: main DB exists, so orphan cleanup should NOT fire + await adapter.initLbug(dbPath); + + const rows = await adapter.executeQuery('RETURN 1 AS ok'); + expect(rows).toEqual([{ ok: 1 }]); + + // Marker file survives — no broad cleanup happened + const content = await fs.readFile(markerPath, 'utf-8'); + expect(content).toBe('should-survive'); + + await adapter.closeLbug(); + } finally { + // Clean up marker file — best-effort; may already be absent + await fs.unlink(markerPath).catch(() => { + /* test cleanup only */ + }); + await tmp.cleanup(); + } + }, + ); +}); + +// --------------------------------------------------------------------------- +// Init lock — cross-process ownership contract +// --------------------------------------------------------------------------- + +describe('init lock — single-process ownership contract', () => { + itLbugReopen('acquireInitLock succeeds when parent directory does not exist yet', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + // Use a nested path whose parent directory does NOT exist + const dbPath = path.join(tmp.dbPath, 'nonexistent-subdir', 'lbug'); + const lockPath = `${dbPath}.init.lock`; + + try { + // Precondition: parent directory must not exist + await expect(fs.access(path.dirname(dbPath))).rejects.toThrow(); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const release = await adapter.acquireInitLock(dbPath); + + // Lock file should exist — parent dir was created automatically + const content = await fs.readFile(lockPath, 'utf-8'); + const parsed = JSON.parse(content); + expect(parsed.pid).toBe(process.pid); + + await release(); + + // Lock file gone after release + await expect(fs.access(lockPath)).rejects.toThrow(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen('acquireInitLock creates and releases lock file atomically', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const lockPath = `${dbPath}.init.lock`; + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const release = await adapter.acquireInitLock(dbPath); + + // Lock file should exist while held + const content = await fs.readFile(lockPath, 'utf-8'); + const parsed = JSON.parse(content); + expect(parsed.pid).toBe(process.pid); + expect(typeof parsed.ts).toBe('number'); + + // Release the lock + await release(); + + // Lock file should be gone after release + await expect(fs.access(lockPath)).rejects.toThrow(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen('acquireInitLock blocks concurrent acquire from same process', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + const release1 = await adapter.acquireInitLock(dbPath); + + // Second acquire should fail because the lock is held by this (alive) process. + // The lock retry budget is small enough that this completes quickly. + await expect(adapter.acquireInitLock(dbPath)).rejects.toThrow(/unable to acquire init lock/); + + await release1(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen('acquireInitLock reclaims stale lock from dead process', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const lockPath = `${dbPath}.init.lock`; + + try { + // PID far above any realistic range — guaranteed not running on any OS. + const DEAD_PROCESS_PID = 2_000_000_000; + await fs.writeFile( + lockPath, + JSON.stringify({ pid: DEAD_PROCESS_PID, ts: Date.now() - 60_000 }), + ); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // Should break the stale lock and acquire successfully + const release = await adapter.acquireInitLock(dbPath); + + // Verify we own the lock now + const content = await fs.readFile(lockPath, 'utf-8'); + const parsed = JSON.parse(content); + expect(parsed.pid).toBe(process.pid); + + await release(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen('release is idempotent — calling twice does not throw', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + const release = await adapter.acquireInitLock(dbPath); + + await release(); + // Second release — lock file already gone, should not throw + await release(); + } finally { + await tmp.cleanup(); + } + }); + + itLbugReopen( + 'initLbug cleans up lock file after successful init with orphan sidecars', + async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const lockPath = `${dbPath}.init.lock`; + + try { + // Plant orphan sidecars + await fs.writeFile(`${dbPath}.shadow`, 'stale-shadow'); + await fs.writeFile(`${dbPath}.wal.checkpoint`, 'stale-wal'); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + // Lock file should be released after init completes + await expect(fs.access(lockPath)).rejects.toThrow(); + + // DB should be functional + const rows = await adapter.executeQuery('RETURN 1 AS ok'); + expect(rows).toEqual([{ ok: 1 }]); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); + + itLbugReopen('initLbug cleans up lock file even when DB open fails', async () => { + const tmp = await createTempDir('gitnexus-lbug-orphan-'); + // Use an invalid path that will cause LadybugDB to fail + const dbPath = path.join(tmp.dbPath, 'nonexistent-subdir', 'deep', 'lbug'); + const lockPath = `${dbPath}.init.lock`; + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // initLbug should fail (parent dir structure may cause issues), but + // we primarily care that the lock file is cleaned up even on failure. + // Use a try/catch since the DB open may or may not fail depending + // on how mkdir works. + try { + await adapter.initLbug(dbPath); + await adapter.closeLbug(); + } catch { + // Expected — DB open can fail for various reasons + } + + // Lock file should always be released, even on failure + await expect(fs.access(lockPath)).rejects.toThrow(); + } finally { + await tmp.cleanup(); + } + }); +}); diff --git a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts index 3e9ffe8f6..a63c1b67d 100644 --- a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts +++ b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts @@ -1,13 +1,461 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; +const makeErrnoError = (code: TCode, message: string) => + Object.assign(new Error(message), { code }); + +/** Stub file handle returned by mocked `fs.open` for the init lock. */ +const makeOpenMock = () => + vi.fn(async () => ({ + writeFile: vi.fn(async () => {}), + close: vi.fn(async () => {}), + })); + +/** Standard `fs/promises` mock for tests that only need doInitLbug to succeed. */ +const mockFsForInit = (dbPath: string) => { + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, lstat '${dbPath}'`, + ); + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: vi.fn(async () => { + throw ENOENT_ERROR; + }), + unlink: vi.fn(async () => {}), + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); +}; + describe('lbug adapter CHECKPOINT lifecycle', () => { afterEach(() => { + vi.doUnmock('fs/promises'); vi.doUnmock('../../src/core/lbug/lbug-config.js'); vi.doUnmock('../../src/core/lbug/extension-loader.js'); + vi.doUnmock('../../src/core/logger.js'); vi.resetModules(); vi.clearAllMocks(); }); + it('removes orphan sidecars when main DB file is missing before opening LadybugDB', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-orphan-sidecar/lbug'; + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, access '${dbPath}'`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + + const unlinkMock = vi.fn(async () => {}); + const accessMock = vi.fn(async () => { + throw ENOENT_ERROR; + }); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: vi.fn(async () => ({ db, conn })), + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + const warnMock = vi.fn(); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: warnMock, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + expect(accessMock).toHaveBeenCalledWith(dbPath); + // Unlink called for: .shadow sidecar, .wal.checkpoint sidecar, init lock release + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.shadow`); + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.wal.checkpoint`); + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.init.lock`); + expect(warnMock).toHaveBeenCalledTimes(2); + expect(warnMock).toHaveBeenCalledWith( + 'GitNexus: removed orphan sidecar lbug.shadow (no main DB file present)', + ); + expect(warnMock).toHaveBeenCalledWith( + 'GitNexus: removed orphan sidecar lbug.wal.checkpoint (no main DB file present)', + ); + + await adapter.closeLbug(); + }); + + it('skips orphan sidecar cleanup when db access fails with non-ENOENT errors', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-orphan-sidecar-eacces/lbug'; + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, access '${dbPath}'`, + ); + const EACCES_ERROR = makeErrnoError('EACCES', `EACCES: permission denied, access '${dbPath}'`); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const accessMock = vi.fn(async () => { + throw EACCES_ERROR; + }); + const unlinkMock = vi.fn(async () => {}); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: vi.fn(async () => ({ db, conn })), + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + const warnMock = vi.fn(); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: warnMock, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + expect(accessMock).toHaveBeenCalledWith(dbPath); + // Only the init lock release calls unlink — sidecar cleanup was skipped + expect(unlinkMock).toHaveBeenCalledTimes(1); + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.init.lock`); + expect(warnMock).toHaveBeenCalledTimes(1); + expect(warnMock.mock.calls[0]?.[0]).toContain( + 'GitNexus: unable to verify main DB file before orphan sidecar cleanup (EACCES); skipping cleanup:', + ); + + await adapter.closeLbug(); + }); + + it('does not remove sidecars when main db file is present', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-present/lbug'; + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, access '${dbPath}'`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const accessMock = vi.fn(async () => {}); + const unlinkMock = vi.fn(async () => {}); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: vi.fn(async () => ({ db, conn })), + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + const warnMock = vi.fn(); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: warnMock, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + expect(accessMock).toHaveBeenCalledWith(dbPath); + // Only the init lock release calls unlink — no sidecar cleanup needed + expect(unlinkMock).toHaveBeenCalledTimes(1); + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.init.lock`); + expect(warnMock).not.toHaveBeenCalled(); + + await adapter.closeLbug(); + }); + + it.each([ + { + code: 'EPERM', + message: 'operation not permitted', + dbPath: '/tmp/gitnexus-lbug-lstat-eperm/lbug', + }, + { + code: 'EACCES', + message: 'permission denied', + dbPath: '/tmp/gitnexus-lbug-lstat-eacces/lbug', + }, + ])('throws when db path lstat fails with non-ENOENT %s', async ({ code, message, dbPath }) => { + vi.resetModules(); + + const LSTAT_ERROR = makeErrnoError(code, `${code}: ${message}, lstat '${dbPath}'`); + const accessMock = vi.fn(async () => {}); + const unlinkMock = vi.fn(async () => {}); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw LSTAT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: vi.fn(async () => { + throw new Error('should not be called'); + }), + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: vi.fn(), + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await expect(adapter.initLbug(dbPath)).rejects.toThrow(new RegExp(message, 'i')); + expect(accessMock).not.toHaveBeenCalled(); + expect(unlinkMock).not.toHaveBeenCalled(); + }); + + it('handles partial orphan sidecar state and removes only present sidecars', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-partial-sidecar/lbug'; + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, access '${dbPath}'`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const accessMock = vi.fn(async () => { + throw ENOENT_ERROR; + }); + const unlinkMock = vi.fn(async (target: string) => { + if (target.endsWith('.shadow')) throw ENOENT_ERROR; + }); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: vi.fn(async () => ({ db, conn })), + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + const warnMock = vi.fn(); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: warnMock, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.shadow`); + expect(unlinkMock).toHaveBeenCalledWith(`${dbPath}.wal.checkpoint`); + expect(warnMock).toHaveBeenCalledTimes(1); + expect(warnMock).toHaveBeenCalledWith( + 'GitNexus: removed orphan sidecar lbug.wal.checkpoint (no main DB file present)', + ); + + await adapter.closeLbug(); + }); + + it('proceeds to openLbugConnection when orphan sidecar unlink fails', async () => { + vi.resetModules(); + + const dbPath = '/tmp/gitnexus-lbug-sidecar-unlink-fail/lbug'; + const ENOENT_ERROR = makeErrnoError( + 'ENOENT', + `ENOENT: no such file or directory, access '${dbPath}'`, + ); + const EPERM_ERROR = makeErrnoError( + 'EPERM', + `EPERM: operation not permitted, unlink '${dbPath}.shadow'`, + ); + const queryResult = { getAll: vi.fn(async () => []), close: vi.fn() }; + const conn = { + query: vi.fn(async () => queryResult), + close: vi.fn(async () => {}), + }; + const db = { close: vi.fn(async () => {}) }; + const accessMock = vi.fn(async () => { + throw ENOENT_ERROR; + }); + const unlinkMock = vi.fn(async () => { + throw EPERM_ERROR; + }); + + vi.doMock('fs/promises', () => ({ + default: { + lstat: vi.fn(async () => { + throw ENOENT_ERROR; + }), + access: accessMock, + unlink: unlinkMock, + mkdir: vi.fn(async () => {}), + open: makeOpenMock(), + }, + })); + const openLbugConnectionMock = vi.fn(async () => ({ db, conn })); + vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ + openLbugConnection: openLbugConnectionMock, + closeLbugConnection: vi.fn(async () => {}), + isDbBusyError: vi.fn((err: unknown) => String(err).toLowerCase().includes('lock')), + isOpenRetryExhausted: vi.fn(() => false), + waitForWindowsHandleRelease: vi.fn(async () => true), + })); + vi.doMock('../../src/core/lbug/extension-loader.js', () => ({ + extensionManager: { + ensure: vi.fn(async () => true), + getCapabilities: vi.fn(() => []), + reset: vi.fn(), + }, + })); + const warnMock = vi.fn(); + vi.doMock('../../src/core/logger.js', () => ({ + logger: { + warn: warnMock, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + })); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.initLbug(dbPath); + + expect(unlinkMock).toHaveBeenCalledTimes(3); + expect(warnMock).toHaveBeenCalledTimes(3); + expect(warnMock.mock.calls[0]?.[0]).toContain( + 'GitNexus: failed to remove orphan sidecar lbug.shadow (EPERM) while main DB file is missing; LadybugDB open may still fail:', + ); + expect(warnMock.mock.calls[1]?.[0]).toContain( + 'GitNexus: failed to remove orphan sidecar lbug.wal.checkpoint (EPERM) while main DB file is missing; LadybugDB open may still fail:', + ); + expect(warnMock.mock.calls[2]?.[0]).toContain('GitNexus: failed to release init lock (EPERM)'); + expect(openLbugConnectionMock).toHaveBeenCalledWith(expect.anything(), dbPath); + + await adapter.closeLbug(); + }); + it('drains and closes CHECKPOINT result before closing connection and database handles', async () => { vi.resetModules(); @@ -43,6 +491,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { }), }; + mockFsForInit('/tmp/gitnexus-lbug-checkpoint-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}), @@ -104,6 +553,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { close: vi.fn(async () => {}), }; + mockFsForInit('/tmp/gitnexus-lbug-query-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}), @@ -158,6 +608,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { close: vi.fn(async () => {}), }; + mockFsForInit('/tmp/gitnexus-lbug-sync-close-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}), @@ -223,6 +674,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { close: vi.fn(async () => {}), }; + mockFsForInit('/tmp/gitnexus-lbug-array-error-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}), @@ -303,6 +755,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { close: vi.fn(async () => {}), }; + mockFsForInit('/tmp/gitnexus-lbug-stream-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}), @@ -383,6 +836,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { close: vi.fn(async () => {}), }; + mockFsForInit('/tmp/gitnexus-lbug-stream-error-lifecycle/lbug'); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ openLbugConnection: vi.fn(async () => ({ db, conn })), closeLbugConnection: vi.fn(async () => {}),