From d9d7a137913126496a8208f4e03cd048195d738d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 07:50:51 +0100 Subject: [PATCH 01/14] chore(deps)(deps): bump js-yaml from 4.3.0 to 5.0.0 in /gitnexus (#2618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(deps)(deps): bump js-yaml from 4.3.0 to 5.0.0 in /gitnexus Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 4.3.0 to 5.0.0. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](https://github.com/nodeca/js-yaml/compare/4.3.0...5.0.0) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] * fix(spring-config): migrate YAML parsing to js-yaml 5 event API js-yaml 5 removed the loadAll `listener` callback, the EventType/State types, and DEFAULT_SCHEMA that spring-config relied on, breaking the build. Rebuild the per-key line tree from parseEvents()/constructFromEvents() (positions are source offsets → mapped to lines), apply the `<<` merge tag via CORE_SCHEMA.withTags(mergeTag) (CORE alone leaves merge keys unexpanded), and resolve aliases by anchor name, which lets the object-identity WeakMap go. Behavior preserved: 9 unit + 8 integration spring-config tests pass, including merged-key declaration-line, cyclic-alias termination, and the depth budget. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(spring-config): restore v4 tag coverage, cover the v5 rewrite with tests Review follow-up for #2618. CORE_SCHEMA.withTags(mergeTag) was a narrowing, not a port: js-yaml 5 throws "unknown tag" on !!timestamp/!!binary/!!set/!!omap/!!pairs, and an unknown tag aborts the whole parse, which readConfigKeys swallows — so an application.yml using any of them would have gone from its full key set to zero keys, silently. Carry the rest of what DEFAULT_SCHEMA was; none of these tags can execute code. Add tests for every path the review flagged as uncovered: multi-document files, empty/comment-only/bare-`---`/bare-scalar documents, sequence-form merge keys, and explicitly tagged values (which fail against the one-tag schema, so they target the changed line). Clear the anchor map per document. It cannot change output today — constructFromEvents rejects a cross-document alias before the event tree is built, now asserted — but it keeps both layers on YAML's scoping rule. Drop the stale @types/js-yaml devDependency; js-yaml 5 ships its own types and tsc --noEmit is clean without it. Lockfile hand-edited because npm uninstall also strips every libc field. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(spring-config): flatten !!set members, walk YAML iteratively Review follow-up for #2618. js-yaml 5 constructs `!!set` as a native Set; v4 built a plain `{member: null}` object. Object.entries of a Set is empty, so a tagged set collapsed to a bare leaf key and lost every member. Enumerate the Set instead. Sets arrive as mapping events with key/value scalar pairs, so member lines resolve through the usual lookup. !!binary and !!timestamp are unaffected — both are scalar events and take the leaf path, which is why a Uint8Array never explodes into one key per byte. Convert findYamlMappingLocation and flattenYamlValue from recursion to an explicit stack. Children are pushed in reverse so pops happen in declaration order, preserving "first match" and `out` insertion order; `leave` frames release the cycle guard where the old `finally` did. The depth budget still throws at the same boundary with the same message. Cover the gaps the review named: !!pairs (both duplicate entries survive), anchor-name reuse resolving to the nearest preceding declaration, and marker-only leading documents staying index-aligned across the two streams. buildYamlEventTree keeps no node budget by design — one node per event over an already-materialized array, bounded by MAX_CONFIG_FILE_BYTES. The docstring now says so rather than implying MAX_YAML_TRAVERSAL_NODES covers it. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- gitnexus/package-lock.json | 18 +- gitnexus/package.json | 3 +- .../pipeline-phases/spring-config.ts | 371 ++++++++++++------ .../test/unit/spring-config-bindings.test.ts | 104 +++++ 4 files changed, 358 insertions(+), 138 deletions(-) diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 96fd95b1e..2800a564c 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -24,7 +24,7 @@ "graphology-indices": "^0.17.0", "graphology-utils": "^2.3.0", "ignore": "^7.0.5", - "js-yaml": "^4.1.1", + "js-yaml": "^5.0.0", "jsonc-parser": "^3.3.1", "mnemonist": "^0.40.3", "node-addon-api": "^8.0.0", @@ -58,7 +58,6 @@ "@types/cli-progress": "^3.11.6", "@types/cors": "^2.8.17", "@types/express": "^5.0.6", - "@types/js-yaml": "^4.0.9", "@types/node": "^26.0.0", "@vitest/coverage-v8": "^4.0.18", "gitnexus-shared": "file:../gitnexus-shared", @@ -1930,13 +1929,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/js-yaml": { - "version": "4.0.9", - "resolved": "https://registry.npmjs.org/@types/js-yaml/-/js-yaml-4.0.9.tgz", - "integrity": "sha512-k4MGaQl5TGo/iipqb2UDG2UwjXziSWkh0uysQelTlJpX1qGlpUZYm8PnO4DxG1qBomtJUdYJ6qR6xdIah10JLg==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/jsesc": { "version": "2.5.1", "resolved": "https://registry.npmjs.org/@types/jsesc/-/jsesc-2.5.1.tgz", @@ -3590,9 +3582,9 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.0.tgz", - "integrity": "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==", + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-5.0.0.tgz", + "integrity": "sha512-GSvaPUbk1U+FMZ7rJzF+F8e5YVtu7KnD40et/5rBXXRBv2jCO9L3qCewvIDDdudC0QycTFlf6EAA+h3kxBsuUw==", "funding": [ { "type": "github", @@ -3608,7 +3600,7 @@ "argparse": "^2.0.1" }, "bin": { - "js-yaml": "bin/js-yaml.js" + "js-yaml": "bin/js-yaml.mjs" } }, "node_modules/jsesc": { diff --git a/gitnexus/package.json b/gitnexus/package.json index 387c89811..114b71ada 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -70,7 +70,7 @@ "graphology-indices": "^0.17.0", "graphology-utils": "^2.3.0", "ignore": "^7.0.5", - "js-yaml": "^4.1.1", + "js-yaml": "^5.0.0", "jsonc-parser": "^3.3.1", "mnemonist": "^0.40.3", "node-addon-api": "^8.0.0", @@ -105,7 +105,6 @@ "@types/cli-progress": "^3.11.6", "@types/cors": "^2.8.17", "@types/express": "^5.0.6", - "@types/js-yaml": "^4.0.9", "@types/node": "^26.0.0", "@vitest/coverage-v8": "^4.0.18", "gitnexus-shared": "file:../gitnexus-shared", diff --git a/gitnexus/src/core/ingestion/pipeline-phases/spring-config.ts b/gitnexus/src/core/ingestion/pipeline-phases/spring-config.ts index 90fa040c6..738bc3e15 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/spring-config.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/spring-config.ts @@ -15,7 +15,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { createRequire } from 'node:module'; -import type { EventType as YamlEventType, State as YamlState } from 'js-yaml'; +import type { Event as YamlEvent } from 'js-yaml'; import { SPRING_CONFIG_DESCRIPTION } from '../frameworks/spring/config-bindings.js'; import { generateId } from '../../../lib/utils.js'; import type { PipelineContext, PipelinePhase, PhaseResult } from './types.js'; @@ -24,6 +24,17 @@ import type { StructureOutput } from './structure.js'; const require = createRequire(import.meta.url); const yaml = require('js-yaml') as typeof import('js-yaml'); +// js-yaml 5 dropped DEFAULT_SCHEMA; CORE plus these tags is what it used to be, so +// explicitly tagged values keep parsing instead of throwing (an unknown tag aborts +// the whole file). None of them can execute code. +const SPRING_YAML_SCHEMA = yaml.CORE_SCHEMA.withTags( + yaml.mergeTag, + yaml.timestampTag, + yaml.binaryTag, + yaml.omapTag, + yaml.pairsTag, + yaml.setTag, +); const MAX_CONFIG_FILE_BYTES = 2 * 1024 * 1024; const MAX_YAML_TRAVERSAL_DEPTH = 128; const MAX_YAML_TRAVERSAL_NODES = 100_000; @@ -134,9 +145,9 @@ export function parseSpringProperties( interface YamlParseEvent { readonly startLine: number; - kind: string | null; - result: unknown; - tag: string | null; + readonly kind: 'scalar' | 'sequence' | 'mapping' | 'alias' | null; + readonly result: unknown; + readonly aliasOf: YamlParseEvent | undefined; readonly children: YamlParseEvent[]; } @@ -164,12 +175,10 @@ function isObjectValue(value: unknown): value is object { return value !== null && typeof value === 'object'; } -function resolveYamlAliasEvent( - event: YamlParseEvent | undefined, - objectEvents: WeakMap, -): YamlParseEvent | undefined { - if (event?.kind !== null || !isObjectValue(event.result)) return event; - return objectEvents.get(event.result) ?? event; +// Aliases are resolved to their anchor event by name while the tree is built, +// so following one here is a single pointer hop. +function resolveYamlAliasEvent(event: YamlParseEvent | undefined): YamlParseEvent | undefined { + return event?.aliasOf ?? event; } function yamlMappingPairs(event: YamlParseEvent): Array<{ @@ -187,116 +196,257 @@ function yamlMappingPairs(event: YamlParseEvent): Array<{ return pairs; } +/** + * First match in a pre-order walk of `event`, following sequences and `<<` merge + * chains. Iterative: children are pushed in reverse so the explicit stack pops + * them in declaration order, which is what makes "first match" mean the same + * thing it did when this recursed. + */ function findYamlMappingLocation( event: YamlParseEvent | undefined, key: string, - objectEvents: WeakMap, traversal: YamlTraversalState, - visited = new Set(), - depth = 0, ): YamlMappingLocation | undefined { - consumeYamlTraversalBudget(traversal, depth); - const resolved = resolveYamlAliasEvent(event, objectEvents); - if (resolved === undefined || visited.has(resolved)) return undefined; - visited.add(resolved); + const visited = new Set(); + const stack: Array<{ event: YamlParseEvent | undefined; depth: number }> = [{ event, depth: 0 }]; - if (resolved.kind === 'sequence') { - for (const child of resolved.children) { - const found = findYamlMappingLocation( - child, - key, - objectEvents, - traversal, - visited, - depth + 1, - ); - if (found !== undefined) return found; + while (stack.length > 0) { + const step = stack.pop(); + if (step === undefined) break; + consumeYamlTraversalBudget(traversal, step.depth); + const resolved = resolveYamlAliasEvent(step.event); + if (resolved === undefined || visited.has(resolved)) continue; + visited.add(resolved); + + if (resolved.kind === 'sequence') { + for (let index = resolved.children.length - 1; index >= 0; index--) { + stack.push({ event: resolved.children[index], depth: step.depth + 1 }); + } + continue; } - return undefined; - } - if (resolved.kind !== 'mapping') return undefined; + if (resolved.kind !== 'mapping') continue; - const pairs = yamlMappingPairs(resolved); - const direct = pairs.find((pair) => pair.key === key); - if (direct !== undefined) { - return { valueEvent: direct.valueEvent, line: direct.keyEvent.startLine }; - } - for (const merge of pairs.filter((pair) => pair.key === '<<')) { - const found = findYamlMappingLocation( - merge.valueEvent, - key, - objectEvents, - traversal, - visited, - depth + 1, - ); - if (found !== undefined) return found; + const pairs = yamlMappingPairs(resolved); + const direct = pairs.find((pair) => pair.key === key); + if (direct !== undefined) { + return { valueEvent: direct.valueEvent, line: direct.keyEvent.startLine }; + } + const merges = pairs.filter((pair) => pair.key === '<<'); + for (let index = merges.length - 1; index >= 0; index--) { + stack.push({ event: merges[index].valueEvent, depth: step.depth + 1 }); + } } return undefined; } +type YamlFlattenStep = + | { + readonly kind: 'visit'; + readonly value: unknown; + readonly event: YamlParseEvent | undefined; + readonly prefix: string; + readonly sourceLine: number; + readonly depth: number; + } + // Pops after every descendant of the object that pushed it, which is where the + // recursive form's `finally` used to release the cycle guard. + | { readonly kind: 'leave'; readonly object: object }; + +/** + * Flatten a document to `dotted.key -> line`, iteratively. Children are pushed in + * reverse so the stack pops them in declaration order, keeping `out` in the same + * insertion order — and the traversal budget consumed in the same sequence — as + * the recursive walk this replaced. + */ function flattenYamlValue( value: unknown, event: YamlParseEvent | undefined, prefix: string, out: Map, - objectEvents: WeakMap, traversal: YamlTraversalState, - sourceLine = event?.startLine ?? 1, - depth = 0, ): void { - consumeYamlTraversalBudget(traversal, depth); - const resolvedEvent = resolveYamlAliasEvent(event, objectEvents); - const trackedObject = isObjectValue(value) ? value : undefined; - if (trackedObject !== undefined && traversal.activeObjects.has(trackedObject)) return; - if (trackedObject !== undefined) traversal.activeObjects.add(trackedObject); - try { - if (Array.isArray(value)) { - if (value.length === 0 && prefix.length > 0 && !out.has(prefix)) out.set(prefix, sourceLine); - value.forEach((item, index) => - flattenYamlValue( - item, - resolvedEvent?.children[index], - `${prefix}[${index}]`, - out, - objectEvents, - traversal, - sourceLine, - depth + 1, - ), - ); - return; + const stack: YamlFlattenStep[] = [ + { kind: 'visit', value, event, prefix, sourceLine: event?.startLine ?? 1, depth: 0 }, + ]; + + while (stack.length > 0) { + const step = stack.pop(); + if (step === undefined) break; + if (step.kind === 'leave') { + traversal.activeObjects.delete(step.object); + continue; } + + const { value: current, prefix: currentPrefix, sourceLine, depth } = step; + consumeYamlTraversalBudget(traversal, depth); + const resolvedEvent = resolveYamlAliasEvent(step.event); + const trackedObject = isObjectValue(current) ? current : undefined; + if (trackedObject !== undefined) { + if (traversal.activeObjects.has(trackedObject)) continue; + traversal.activeObjects.add(trackedObject); + stack.push({ kind: 'leave', object: trackedObject }); + } + + if (Array.isArray(current)) { + if (current.length === 0 && currentPrefix.length > 0 && !out.has(currentPrefix)) { + out.set(currentPrefix, sourceLine); + } + for (let index = current.length - 1; index >= 0; index--) { + stack.push({ + kind: 'visit', + value: current[index], + event: resolvedEvent?.children[index], + prefix: `${currentPrefix}[${index}]`, + sourceLine, + depth: depth + 1, + }); + } + continue; + } + if ( - value !== null && - typeof value === 'object' && + current !== null && + typeof current === 'object' && (resolvedEvent?.kind === 'mapping' || resolvedEvent === undefined) ) { - const entries = Object.entries(value as Record); - if (entries.length === 0 && prefix.length > 0 && !out.has(prefix)) - out.set(prefix, sourceLine); - for (const [key, nested] of entries) { - const next = prefix.length === 0 ? key : `${prefix}.${key}`; - const location = findYamlMappingLocation(resolvedEvent, key, objectEvents, traversal); - flattenYamlValue( - nested, - location?.valueEvent, - next, - out, - objectEvents, - traversal, - location?.line ?? sourceLine, - depth + 1, - ); + // js-yaml 5 builds `!!set` as a native Set, whose members are not own + // properties; v4 built a plain `{member: null}` object. Enumerate them so a + // tagged set still contributes one key per member instead of a bare leaf. + const entries: Array<[string, unknown]> = + current instanceof Set + ? [...current].map((member) => [String(member), null]) + : Object.entries(current as Record); + if (entries.length === 0 && currentPrefix.length > 0 && !out.has(currentPrefix)) { + out.set(currentPrefix, sourceLine); } - return; + for (let index = entries.length - 1; index >= 0; index--) { + const [key, nested] = entries[index]; + const location = findYamlMappingLocation(resolvedEvent, key, traversal); + stack.push({ + kind: 'visit', + value: nested, + event: location?.valueEvent, + prefix: currentPrefix.length === 0 ? key : `${currentPrefix}.${key}`, + sourceLine: location?.line ?? sourceLine, + depth: depth + 1, + }); + } + continue; } - if (prefix.length > 0 && !out.has(prefix)) out.set(prefix, sourceLine); - } finally { - if (trackedObject !== undefined) traversal.activeObjects.delete(trackedObject); + + if (currentPrefix.length > 0 && !out.has(currentPrefix)) out.set(currentPrefix, sourceLine); } } +// js-yaml 5 reports node positions as source offsets; map them to 1-based lines. +function makeLineResolver(source: string): (offset: number) => number { + const lineStarts = [0]; + for (let index = 0; index < source.length; index++) { + if (source[index] === '\n') lineStarts.push(index + 1); + } + return (offset: number): number => { + let low = 0; + let high = lineStarts.length - 1; + let line = 0; + while (low <= high) { + const mid = (low + high) >> 1; + if (lineStarts[mid] <= offset) { + line = mid; + low = mid + 1; + } else { + high = mid - 1; + } + } + return line + 1; + }; +} + +/** + * Rebuild the parse tree from js-yaml 5's event stream (v4's `listener` option + * was removed). Returns each document's root event, with aliases already + * resolved to their anchor event so merged/aliased keys keep the line where + * they were declared. + * + * One node per event, so this pass is bounded by MAX_CONFIG_FILE_BYTES alone — + * MAX_YAML_TRAVERSAL_NODES governs the later walk, which can revisit a shared + * anchor many times and so needs a budget this linear pass does not. + */ +function buildYamlEventTree( + events: readonly YamlEvent[], + source: string, +): Array { + const lineOf = makeLineResolver(source); + const anchors = new Map(); + const stack: YamlParseEvent[] = []; + const documentRoots: Array = []; + + const anchorName = (start: number, end: number): string | null => + start >= 0 && end > start ? source.slice(start, end) : null; + const attach = (node: YamlParseEvent): void => { + stack[stack.length - 1]?.children.push(node); + }; + const register = (name: string | null, node: YamlParseEvent): void => { + if (name !== null) anchors.set(name, node); + }; + + for (const event of events) { + switch (event.type) { + case yaml.EVENT_DOCUMENT: + // Anchors are document-scoped. constructFromEvents already rejects a + // cross-document alias before we get here, so this only keeps the two + // layers from disagreeing. + anchors.clear(); + stack.push({ + startLine: 1, + kind: null, + result: undefined, + aliasOf: undefined, + children: [], + }); + break; + case yaml.EVENT_MAPPING: + case yaml.EVENT_SEQUENCE: { + const node: YamlParseEvent = { + startLine: lineOf(event.start), + kind: event.type === yaml.EVENT_MAPPING ? 'mapping' : 'sequence', + result: undefined, + aliasOf: undefined, + children: [], + }; + register(anchorName(event.anchorStart, event.anchorEnd), node); + attach(node); + stack.push(node); + break; + } + case yaml.EVENT_SCALAR: { + const node: YamlParseEvent = { + startLine: lineOf(event.valueStart), + kind: 'scalar', + result: yaml.getScalarValue(source, event), + aliasOf: undefined, + children: [], + }; + register(anchorName(event.anchorStart, event.anchorEnd), node); + attach(node); + break; + } + case yaml.EVENT_ALIAS: { + const target = anchors.get(anchorName(event.anchorStart, event.anchorEnd) ?? ''); + attach({ startLine: 1, kind: 'alias', result: undefined, aliasOf: target, children: [] }); + break; + } + case yaml.EVENT_POP: { + const done = stack.pop(); + // Documents are the only top-level containers, so a pop that empties the + // stack closes a document; its single child is the document's root value. + if (done !== undefined && stack.length === 0) documentRoots.push(done.children[0]); + break; + } + } + } + return documentRoots; +} + /** Parse and flatten YAML leaves without retaining their values. */ export function parseSpringYaml( content: string, @@ -304,46 +454,21 @@ export function parseSpringYaml( profile?: string, ): SpringConfigKey[] { const flattened = new Map(); - const eventStack: YamlParseEvent[] = []; - const documentEvents: YamlParseEvent[] = []; - const objectEvents = new WeakMap(); - const documents: unknown[] = []; const traversal: YamlTraversalState = { remainingNodes: MAX_YAML_TRAVERSAL_NODES, activeObjects: new Set(), }; - yaml.loadAll(content, (document) => documents.push(document), { - schema: yaml.DEFAULT_SCHEMA, + const events = yaml.parseEvents(content, { maxDepth: MAX_YAML_TRAVERSAL_DEPTH }); + const documents = yaml.constructFromEvents(events, { + source: content, + schema: SPRING_YAML_SCHEMA, json: true, - listener: (eventType: YamlEventType, state: YamlState) => { - if (eventType === 'open') { - eventStack.push({ - startLine: state.line + 1, - kind: null, - result: undefined, - tag: null, - children: [], - }); - return; - } - - const event = eventStack.pop(); - if (event === undefined) return; - event.kind = state.kind ?? null; - event.result = state.result; - event.tag = (state as YamlState & { tag?: string | null }).tag ?? null; - if (isObjectValue(event.result) && event.kind !== null) { - objectEvents.set(event.result, event); - } - const parent = eventStack[eventStack.length - 1]; - if (parent === undefined) documentEvents.push(event); - else parent.children.push(event); - }, }); + const documentEvents = buildYamlEventTree(events, content); documents.forEach((document, index) => - flattenYamlValue(document, documentEvents[index], '', flattened, objectEvents, traversal), + flattenYamlValue(document, documentEvents[index], '', flattened, traversal), ); return [...flattened.entries()] .sort(([left], [right]) => left.localeCompare(right)) diff --git a/gitnexus/test/unit/spring-config-bindings.test.ts b/gitnexus/test/unit/spring-config-bindings.test.ts index a935039fc..1e64fd7df 100644 --- a/gitnexus/test/unit/spring-config-bindings.test.ts +++ b/gitnexus/test/unit/spring-config-bindings.test.ts @@ -79,6 +79,110 @@ describe('Spring configuration parsing', () => { expect(keys.some((entry) => entry.key.includes('<<'))).toBe(false); }); + it('flattens every document of a multi-document file and ignores empty ones', () => { + expect( + parseSpringYaml( + 'server:\n port: 8080\n---\nservice:\n name: demo\n', + 'application.yml', + ).map((entry) => [entry.key, entry.line]), + ).toEqual([ + ['server.port', 2], + ['service.name', 5], + ]); + + expect(parseSpringYaml('', 'application.yml')).toEqual([]); + expect(parseSpringYaml('# only a comment\n\n', 'application.yml')).toEqual([]); + expect(parseSpringYaml('---\n', 'application.yml')).toEqual([]); + // A bare top-level scalar has no key to attribute, so it contributes nothing. + expect(parseSpringYaml('just-a-scalar\n', 'application.yml')).toEqual([]); + // Anchors are document-scoped: an alias may not reach into a previous document. + expect(() => + parseSpringYaml( + 'base: &base\n timeout: 30\n---\nservice:\n <<: *base\n', + 'application.yml', + ), + ).toThrow('unidentified alias'); + }); + + it('resolves sequence-form merge keys and explicitly tagged values', () => { + expect( + parseSpringYaml('a: &a\n x: 1\nb: &b\n y: 2\nc:\n <<: [*a, *b]\n', 'application.yml').map( + (entry) => [entry.key, entry.line], + ), + ).toEqual([ + ['a.x', 2], + ['b.y', 4], + ['c.x', 2], + ['c.y', 4], + ]); + + // js-yaml 5's CORE schema alone rejects these tags; the file-level catch would + // then drop every key in the file, so the schema must keep carrying them. + // `!!set` constructs a native Set in v5 (a plain object in v4), so its members + // are only reachable by enumerating the Set itself. + const tagged = parseSpringYaml( + [ + 'when: !!timestamp 2001-12-14', + 'blob: !!binary "R0lGODlh"', + 'flags: !!set\n ? a\n ? b', + 'ordered: !!omap\n - first: 1', + 'listed: !!pairs\n - dup: 1\n - dup: 2', + ].join('\n'), + 'application.yml', + ); + expect(tagged.map((entry) => [entry.key, entry.line])).toEqual([ + ['blob', 2], + ['flags.a', 4], + ['flags.b', 5], + // `!!pairs` keeps both `dup` entries instead of collapsing them, which is the + // point of the tag. Nested sequence items inherit their parent's line here, + // as they did under v4 — the mapping lookup that refines a line has no + // equivalent for a bare array index. + ['listed[0][0]', 8], + ['listed[0][1]', 8], + ['listed[1][0]', 8], + ['listed[1][1]', 8], + ['ordered[0].first', 7], + ['when', 1], + ]); + expect(JSON.stringify(tagged)).not.toContain('R0lGODlh'); + }); + + it('resolves an alias to the nearest preceding anchor when a name is reused', () => { + // v4 keyed aliases on constructed-object identity; v5 keys them by anchor + // name, so redeclaring a name is a case the old scheme could not express. + expect( + parseSpringYaml( + 'first: &shared\n a: 1\nsecond: &shared\n b: 2\nthird: *shared\n', + 'application.yml', + ).map((entry) => [entry.key, entry.line]), + ).toEqual([ + ['first.a', 2], + ['second.b', 4], + ['third.b', 4], + ]); + }); + + it('keeps document and event streams aligned across marker-only documents', () => { + // The value tree and the line tree are built from the same DOCUMENT events but + // zipped by index, so a leading empty document must consume a slot in both. + expect( + parseSpringYaml('---\n---\nfoo: 1\n', 'application.yml').map((entry) => [ + entry.key, + entry.line, + ]), + ).toEqual([['foo', 3]]); + expect( + parseSpringYaml('a: 1\n---\n---\nb: 2\n', 'application.yml').map((entry) => [ + entry.key, + entry.line, + ]), + ).toEqual([ + ['a', 1], + ['b', 4], + ]); + }); + it('terminates cyclic YAML aliases and bounds deeply nested expansion', () => { expect( parseSpringYaml('cycle: &cycle { self: *cycle }\nhealthy: true\n', 'application.yml'), From bb424cef7770c76d7b12427b8ce0937c8c0d6988 Mon Sep 17 00:00:00 2001 From: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:55:46 +0530 Subject: [PATCH 02/14] fix(eval): mount the node prefix for npx and catch nested Claude Code bootstrap noise (#2627) * fix(eval): ignore Claude Code bootstrap noise nested below the workspace root The planning-phase boundary check excluded Claude Code's own sandbox-bootstrap paths only at the workspace root: workspace_snapshot tested relative.parts[0] against WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE. But Claude Code bootstraps into whatever directory it is running in, and the benchmark's task prompts cd into gitnexus/, so the same noise landed one level down as gitnexus/.claude/.cc-writes -- whose parts[0] is "gitnexus", so it was never excluded. In skill-evolution run 29861768554 that accounted for 13 of 18 sessions, each failing with error_kind plan-evidence-invalid and the identical error_detail "phase changed unauthorized workspace path(s): gitnexus/.claude/.cc-writes". The same code path also guards the review phase (runner.py:499), so review arms hit it as review-evidence-invalid. Widening the whole set to match at any depth would be wrong: it also contains package.json, package-lock.json, node_modules and the .env family, and both gitnexus/package.json and gitnexus/.claude/settings.local.json are real tracked files whose edits must still be caught. So the root-anchored rule is unchanged, and a second narrow rule matches only the entries Claude Code itself creates inside a .claude directory (.cc-writes, agents, commands) at any depth -- never .claude itself. The predicate moves into _is_bootstrap_noise so it is directly testable. It is still evaluated before pending.append, so an excluded directory is never descended into. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(eval): mount the node install prefix so npx and npm resolve in the sandbox _runtime_mount_args bound only the `node` binary itself to SANDBOX_NODE. npm and npx are not standalone binaries -- they are symlinks into ../lib/node_modules/npm/bin/*-cli.js -- so the install prefix carrying both bin/ and lib/node_modules has to be mounted for them to resolve at all. On GitHub-hosted images node lives in /usr/local/bin, whose prefix (/usr/local) is already inside the wholesale /usr read-only bind, so npm and npx came along for free and the gap stayed invisible. A self-hosted runner's actions/setup-node installs into its own tool cache, outside /usr, so only the single node file was bound. Every task's verify command is "cd gitnexus && npx tsc --noEmit && npx vitest run ", so in skill-evolution run 29861768554 all 18 of 18 result records carried the identical verify_output "/bin/sh: 1: npx: not found" -- no run could resolve regardless of model output. It reached the model too: the session transcripts show 12 "npm: not found" failures, with gitnexus/scripts/build.js dying on `npm ci` with status 127. Binds Path(node_bin).resolve().parent.parent read-only at /opt/claude/nodejs, a fresh target outside the already-read-only trees (same constraint that put SANDBOX_NODE under /opt/claude), and adds its bin/ to SANDBOX_PATH. The bind is skipped when the prefix already sits inside /usr, /bin, /lib or /lib64, so the already-covered case does not widen the mount surface redundantly. SANDBOX_NODE is deliberately unchanged -- sanitized_graph.py and runner_sessions.py invoke it directly. SANDBOX_PATH is now derived from SANDBOX_NODE_PREFIX so the two cannot drift, and the minimal-mounts probe asserts against the constant instead of a duplicated literal. The real-Bubblewrap npx canary lives in test_proposer_sandbox.py deliberately: test_workflow_bench.py pins the set of files carrying the canary marker, and it runs in the eval-containment-linux job, where actions/setup-node also installs into the tool cache -- so the canary exercises the real failure shape. Combines plan steps 3-5 into one commit: the mount, SANDBOX_PATH and the pinned probe assertion are one behavioural change, and splitting them would leave a commit whose asserted PATH disagrees with the mounted reality. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(eval): only bind a verified node prefix, and stop excluding .claude/agents Addresses two findings from the branch review of the two preceding commits. 1. The prefix was derived as Path(node_bin).resolve().parent.parent with no check that the layout is really /bin/node. Probed: /opt/bin/node bound ALL of /opt (every tool cache on a hosted runner), /mnt/tools/node bound /mnt, and a bare /node bound 's parent. That last shape is not hypothetical -- the pre-existing real-Bubblewrap node canary builds exactly it (tmp_path/toolcache/node), so eval-containment-linux would have silently read-only mounted the whole pytest tmp_path inside a containment test, passing while doing it. This function exists to keep the sandbox surface minimal, so an unrecognized layout now binds nothing extra and simply leaves npx unavailable, exactly as before the mount was added. 2. CLAUDE_BOOTSTRAP_ENTRIES also excluded "agents" and "commands" on the theory that they might appear nested too; only .cc-writes ever was observed. Every excluded name is a blind spot: once a .claude directory exists (gitnexus/.claude/settings.local.json is tracked) anything written under an excluded entry is invisible to the phase-boundary check, and Claude Code loads .claude/agents relative to its cwd -- which these tasks point at gitnexus/. Probed: a planning phase could plant gitnexus/.claude/agents/planted.md with the check reporting nothing, then the work phase reads it. Narrowed to .cc-writes alone; extend the set from an observed failure, never pre-emptively. Re-probed after both fixes: the over-broad mounts are gone while a genuine tool-cache prefix carrying npm still binds; planted agents/commands content is caught again; gitnexus/.claude/.cc-writes (the real run-29861768554 failure) stays ignored; and edits to gitnexus/.claude/settings.local.json are still caught. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(eval): gate the node-prefix bind on a working npx, not on an npm directory The guard tested (prefix)/lib/node_modules/npm as a proxy for "this prefix supplies npx". Test the property actually required instead: a working npx sitting beside node in a real bin/ directory. .exists() follows the symlink, so a dangling npx correctly fails the check -- it would not survive the mount either. The "bin" name requirement stays, because it is what keeps the parent.parent derivation honest; an npx sitting directly beside node in a flat directory would make that derivation name the wrong prefix. This matters because the guard can silently disable the fix it guards: if a runner's layout failed the proxy check, the prefix would not be bound and npx would still be missing, reproducing the original failure with no signal. Testing npx directly means the guard can only pass when the bind will actually achieve its purpose. Validated against a real extracted Node distribution (the official nodejs.org tarball layout that actions/setup-node unpacks into the tool cache) staged at a tool-cache-shaped path: bin/node is a real file, bin/npx resolves to ../lib/node_modules/npm/bin/npx-cli.js, and the prefix binds while SANDBOX_NODE is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Gergo Magyar Co-authored-by: Claude Opus 4.8 (1M context) --- eval/tests/test_proposer_sandbox.py | 139 +++++++++++++++++++++++- eval/tests/test_runner_hardening.py | 85 +++++++++++++++ eval/workflow_bench/proposer_sandbox.py | 39 ++++++- eval/workflow_bench/runner_artifacts.py | 35 +++++- 4 files changed, 294 insertions(+), 4 deletions(-) diff --git a/eval/tests/test_proposer_sandbox.py b/eval/tests/test_proposer_sandbox.py index 2c4c37259..e907c1a0c 100644 --- a/eval/tests/test_proposer_sandbox.py +++ b/eval/tests/test_proposer_sandbox.py @@ -21,6 +21,8 @@ from workflow_bench.proposer_sandbox import ( MAX_BUNDLE_BYTES, MAX_EVIDENCE_FILE_BYTES, SANDBOX_NODE, + SANDBOX_NODE_PREFIX, + SANDBOX_PATH, SANDBOX_PYTHON3, SANDBOX_SHELL_PREFIX, SANDBOX_USER_SKILLS, @@ -165,7 +167,7 @@ def test_sandbox_command_has_minimal_mounts_and_no_host_root_bind(tmp_path: Path check=False, ) assert probe.returncode == 0, probe.stderr - assert probe.stdout == "/home/agent|/opt/claude:/usr/local/bin:/usr/bin:/bin" + assert probe.stdout == f"/home/agent|{SANDBOX_PATH}" # The evidence-provenance.mjs plan-writer's PATH-scan trusts a Python 3 # candidate only if it (and its directory) is owned by root or by the @@ -218,6 +220,106 @@ def test_runtime_mounts_bind_the_resolved_node_to_a_fresh_sandbox_path(monkeypat assert not any(SANDBOX_NODE.startswith(bound + "/") for bound in ("/usr", "/bin", "/lib", "/lib64")) +def test_runtime_mounts_bind_the_node_prefix_so_npx_and_npm_resolve(monkeypatch, tmp_path) -> None: + # npx and npm are not standalone binaries -- they are symlinks into + # ../lib/node_modules/npm/bin/*-cli.js -- so binding the sibling files is + # not enough; the install prefix carrying both bin/ and lib/node_modules + # has to be mounted. Without this, a self-hosted runner (where + # actions/setup-node installs into its own tool cache, outside /usr) gets + # a sandbox with node but no npx, and every task verify command dies with + # "/bin/sh: 1: npx: not found" -- all 18 runs of skill-evolution run + # 29861768554 did exactly that. + prefix = tmp_path / "hostedtoolcache" / "node" / "22.18.0" / "x64" + (prefix / "bin").mkdir(parents=True) + (prefix / "bin" / "node").write_text("#!/bin/sh\nexit 0\n") + (prefix / "lib" / "node_modules" / "npm" / "bin").mkdir(parents=True) + (prefix / "lib" / "node_modules" / "npm" / "bin" / "npx-cli.js").write_text("") + (prefix / "bin" / "npx").symlink_to("../lib/node_modules/npm/bin/npx-cli.js") + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: str(prefix / "bin" / "node") if name == "node" else None, + ) + args = _runtime_mount_args() + prefix_index = args.index(str(prefix)) + assert args[prefix_index - 1] == "--ro-bind" + assert args[prefix_index + 1] == SANDBOX_NODE_PREFIX + # the single-binary bind stays: sanitized_graph.py and runner_sessions.py + # invoke SANDBOX_NODE directly. + node_index = args.index(str(prefix / "bin" / "node")) + assert args[node_index + 1] == SANDBOX_NODE + # and the prefix's bin/ must actually be on PATH for npx to resolve. + assert f"{SANDBOX_NODE_PREFIX}/bin" in SANDBOX_PATH.split(":") + + +def test_runtime_mounts_skip_the_prefix_bind_for_an_unrecognized_node_layout(monkeypatch, tmp_path) -> None: + # The prefix is derived from the node binary's path, so it must only be + # trusted when the layout really is /bin/node carrying npm. + # Otherwise parent.parent names an unrelated ancestor: /opt/bin/node would + # bind ALL of /opt (every tool cache on a hosted runner) and a bare + # /node would bind 's parent -- an over-broad mount into a + # sandbox that runs untrusted model-authored code. The pre-existing + # real-Bubblewrap node canary builds exactly this bare /node shape. + bare = tmp_path / "toolcache" + bare.mkdir() + (bare / "node").write_text("#!/bin/sh\nexit 0\n") + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: str(bare / "node") if name == "node" else None, + ) + args = _runtime_mount_args() + assert SANDBOX_NODE_PREFIX not in args + assert str(tmp_path) not in args + # the node bind itself is unaffected -- SANDBOX_NODE still works. + assert args[args.index(str(bare / "node")) + 1] == SANDBOX_NODE + + +def test_runtime_mounts_skip_the_prefix_bind_without_npx_beside_node(monkeypatch, tmp_path) -> None: + # Right /bin/node shape, but no working npx beside it: binding the + # prefix would widen the mount surface without making npx resolvable. + prefix = tmp_path / "x64" + (prefix / "bin").mkdir(parents=True) + (prefix / "bin" / "node").write_text("#!/bin/sh\nexit 0\n") + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: str(prefix / "bin" / "node") if name == "node" else None, + ) + args = _runtime_mount_args() + assert SANDBOX_NODE_PREFIX not in args + + +def test_runtime_mounts_bind_a_real_tool_cache_layout(monkeypatch, tmp_path) -> None: + # The positive counterpart: a genuine /bin/node install carrying + # npm, outside the system trees, is bound so npx resolves. + prefix = tmp_path / "node" / "22.18.0" / "x64" + (prefix / "bin").mkdir(parents=True) + (prefix / "bin" / "node").write_text("#!/bin/sh\nexit 0\n") + (prefix / "lib" / "node_modules" / "npm" / "bin").mkdir(parents=True) + (prefix / "lib" / "node_modules" / "npm" / "bin" / "npx-cli.js").write_text("") + (prefix / "bin" / "npx").symlink_to("../lib/node_modules/npm/bin/npx-cli.js") + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: str(prefix / "bin" / "node") if name == "node" else None, + ) + args = _runtime_mount_args() + prefix_index = args.index(SANDBOX_NODE_PREFIX) + assert args[prefix_index - 2] == "--ro-bind" + assert args[prefix_index - 1] == str(prefix) + + +def test_runtime_mounts_skip_the_prefix_bind_when_it_is_already_bound(monkeypatch) -> None: + # On an image where node genuinely lives in /usr/local/bin, the prefix is + # /usr/local -- already inside the wholesale /usr read-only bind. Binding + # it again would be redundant and would needlessly widen the argv, so the + # containment surface stays minimal. + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: "/usr/local/bin/node" if name == "node" else None, + ) + args = _runtime_mount_args() + assert SANDBOX_NODE_PREFIX not in args + assert args[args.index("/usr/local/bin/node") + 1] == SANDBOX_NODE + + def test_runtime_mounts_skip_the_node_bind_when_node_is_unresolvable(monkeypatch) -> None: monkeypatch.setattr("workflow_bench.proposer_sandbox.shutil.which", lambda name: None) args = _runtime_mount_args() @@ -289,6 +391,41 @@ def test_real_bubblewrap_runs_node_from_outside_the_bound_trees(tmp_path: Path, assert result.ok, result.stderr_tail +@pytest.mark.skipif( + os.environ.get("GITNEXUS_REQUIRE_BWRAP_CANARY") != "1", + reason="real Bubblewrap canary is mandatory in the named Ubuntu CI job", +) +def test_real_bubblewrap_runs_npx_from_outside_the_bound_trees(tmp_path: Path, monkeypatch) -> None: + # The npx half of the self-hosted-runner failure. Relocating a real node + # INSTALL (bin/ + lib/node_modules, not just the binary) to a fresh path + # outside /usr, /bin, /lib and /lib64 reproduces actions/setup-node's + # tool-cache convention. Every task verify command is + # "cd gitnexus && npx tsc ... && npx vitest ...", so npx must resolve + # inside the sandbox; argv assertions cannot prove a bwrap-level mount + # actually works, only a real invocation can. + real_node = shutil.which("node") + if not real_node: + pytest.skip("no node on PATH to relocate for this canary") + real_prefix = Path(real_node).resolve().parent.parent + if not (real_prefix / "lib" / "node_modules" / "npm").is_dir(): + pytest.skip(f"node at {real_node} has no npm under its install prefix") + toolcache = tmp_path / "toolcache" / "node" / "22.18.0" / "x64" + shutil.copytree(real_prefix, toolcache, symlinks=True) + relocated_node = toolcache / "bin" / "node" + assert relocated_node.exists() + real_which = shutil.which + monkeypatch.setattr( + "workflow_bench.proposer_sandbox.shutil.which", + lambda name: str(relocated_node) if name == "node" else real_which(name), + ) + + clone = tmp_path / "clone" + clone.mkdir() + with prepare_sandbox(clone=clone, claude_bin=Path(sys.executable), preflight=True) as sandbox: + result = sandbox.run(["/bin/sh", "-c", "command -v npx && npx --version"], timeout=60) + assert result.ok, result.stderr_tail + + @pytest.mark.skipif( os.environ.get("GITNEXUS_REQUIRE_BWRAP_CANARY") != "1", reason="real Bubblewrap canary is mandatory in the named Ubuntu CI job", diff --git a/eval/tests/test_runner_hardening.py b/eval/tests/test_runner_hardening.py index 6905891c0..0c93eeec0 100644 --- a/eval/tests/test_runner_hardening.py +++ b/eval/tests/test_runner_hardening.py @@ -296,3 +296,88 @@ def test_phase_workspace_still_rejects_a_genuinely_unauthorized_change(tmp_path) with pytest.raises(ValueError, match="unauthorized workspace path"): runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) + + +def test_phase_workspace_ignores_nested_claude_sandbox_bootstrap_noise(tmp_path): + # Claude Code bootstraps into whatever directory it is running in, not just + # the workspace root. The benchmark's task prompts cd into gitnexus/, so the + # same noise lands one level down -- observed verbatim in skill-evolution run + # 29861768554, where 13 of 18 sessions failed with + # "phase changed unauthorized workspace path(s): gitnexus/.claude/.cc-writes". + nested = tmp_path / "gitnexus" / ".claude" + nested.mkdir(parents=True) + (nested / "settings.local.json").write_text("{}") + before = runner_artifacts.workspace_snapshot(tmp_path) + (nested / ".cc-writes").write_text("{}") + artifact = tmp_path / "review-output.md" + artifact.write_text("new review") + + runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) + + +def test_phase_workspace_does_not_descend_into_nested_bootstrap_directories(tmp_path): + # The exclusion must skip an entry before it is queued for traversal, so + # content created *inside* the ignored directory stays invisible too. + nested = tmp_path / "gitnexus" / ".claude" / ".cc-writes" + nested.mkdir(parents=True) + before = runner_artifacts.workspace_snapshot(tmp_path) + (nested / "pending.json").write_text('{"writes": 1}') + artifact = tmp_path / "review-output.md" + artifact.write_text("new review") + + runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) + + +def test_phase_workspace_still_rejects_nested_real_claude_config(tmp_path): + # gitnexus/.claude/settings.local.json is real tracked repository content. + # Excluding ".claude" wholesale at depth would blind the check to it, so the + # exclusion must name only the entries Claude Code itself creates. + nested = tmp_path / "gitnexus" / ".claude" + nested.mkdir(parents=True) + settings = nested / "settings.local.json" + settings.write_text("{}") + before = runner_artifacts.workspace_snapshot(tmp_path) + settings.write_text('{"permissions": "changed"}') + artifact = tmp_path / "review-output.md" + artifact.write_text("new review") + + with pytest.raises(ValueError, match="unauthorized workspace path"): + runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) + + +def test_phase_workspace_still_rejects_nested_package_json(tmp_path): + # package.json is in WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE, but only as a + # workspace-root entry: gitnexus/package.json is real tracked content whose + # edits must still be caught. + nested = tmp_path / "gitnexus" + nested.mkdir() + manifest = nested / "package.json" + manifest.write_text("{}") + before = runner_artifacts.workspace_snapshot(tmp_path) + manifest.write_text('{"version": "9.9.9"}') + artifact = tmp_path / "review-output.md" + artifact.write_text("new review") + + with pytest.raises(ValueError, match="unauthorized workspace path"): + runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) + + +def test_phase_workspace_still_sees_writes_under_a_pre_existing_nested_claude_dir(tmp_path): + # Every excluded name is a blind spot. .claude/agents and .claude/commands + # are deliberately NOT excluded at depth: once a .claude directory exists + # (gitnexus/.claude/settings.local.json is tracked), anything written + # underneath an excluded entry is invisible to this check, and Claude Code + # loads .claude/agents relative to its cwd -- which these tasks point at + # gitnexus/. A planning phase must not be able to plant a definition there + # for the later work phase to read. + nested = tmp_path / "gitnexus" / ".claude" + nested.mkdir(parents=True) + (nested / "settings.local.json").write_text("{}") + before = runner_artifacts.workspace_snapshot(tmp_path) + (nested / "agents").mkdir() + (nested / "agents" / "planted.md").write_text("planted agent definition") + artifact = tmp_path / "review-output.md" + artifact.write_text("new review") + + with pytest.raises(ValueError, match="unauthorized workspace path"): + runner_artifacts.enforce_phase_workspace(tmp_path, before, allowed_artifact=artifact) diff --git a/eval/workflow_bench/proposer_sandbox.py b/eval/workflow_bench/proposer_sandbox.py index 2884eb067..7bd46e03c 100644 --- a/eval/workflow_bench/proposer_sandbox.py +++ b/eval/workflow_bench/proposer_sandbox.py @@ -28,7 +28,8 @@ SANDBOX_CLAUDE = "/opt/claude/claude" SANDBOX_SHELL_PREFIX = "/opt/claude/shell-prefix" SANDBOX_PYTHON3 = "/opt/claude/python3" SANDBOX_NODE = "/opt/claude/node" -SANDBOX_PATH = "/opt/claude:/usr/local/bin:/usr/bin:/bin" +SANDBOX_NODE_PREFIX = "/opt/claude/nodejs" +SANDBOX_PATH = f"/opt/claude:{SANDBOX_NODE_PREFIX}/bin:/usr/local/bin:/usr/bin:/bin" SANDBOX_GITNEXUS = "/opt/gitnexus" SANDBOX_GITNEXUS_SHARED = "/opt/gitnexus-shared" SANDBOX_GITNEXUS_REGISTRY = "/opt/gitnexus-registry" @@ -351,7 +352,8 @@ def build_claude_settings() -> str: def _runtime_mount_args() -> list[str]: args: list[str] = [] - for raw in ("/usr", "/bin", "/lib", "/lib64"): + system_trees = ("/usr", "/bin", "/lib", "/lib64") + for raw in system_trees: path = Path(raw) if path.exists(): args += ["--ro-bind", raw, raw] @@ -369,6 +371,39 @@ def _runtime_mount_args() -> list[str]: node_bin = shutil.which("node") if node_bin: args += ["--ro-bind", node_bin, SANDBOX_NODE] + # The single-binary bind above gives SANDBOX_NODE but NOT npm or npx: + # those are symlinks into ../lib/node_modules/npm/bin/*-cli.js, so the + # install prefix carrying both bin/ and lib/node_modules has to be + # mounted for them to resolve at all. When node really lives under a + # system tree (/usr/local/bin on GitHub-hosted images) the prefix is + # already inside the wholesale read-only binds above and npm/npx came + # along for free -- which is exactly why this gap stayed invisible + # until a self-hosted runner put node in actions/setup-node's tool + # cache, outside /usr, and every task verify command + # ("cd gitnexus && npx tsc ... && npx vitest ...") died with + # "/bin/sh: 1: npx: not found". Skip the redundant bind in the + # already-covered case so the mount surface stays minimal. + # + # The prefix is only ever derived from a real /bin/node layout + # that actually carries npm. Deriving it as parent.parent unconditionally + # would mount an unrelated ancestor whenever node sits somewhere else: + # /opt/bin/node would bind all of /opt (every tool cache on a hosted + # runner) and a bare /node would bind 's parent. This function + # exists to keep the sandbox surface minimal, so an unrecognized layout + # binds nothing extra and simply leaves npx unavailable, exactly as + # before. + node_bin_dir = Path(node_bin).resolve().parent + node_prefix = node_bin_dir.parent + # Test the property actually needed -- a working npx next to node in a + # real bin/ directory -- rather than a proxy like lib/node_modules/npm. + # .exists() follows the symlink, so a dangling npx correctly fails: it + # would not survive the mount either. Requiring the "bin" name keeps + # the parent.parent derivation honest; an npx sitting directly beside + # node in a flat directory would make that derivation name the wrong + # prefix. + provides_npx = node_bin_dir.name == "bin" and (node_bin_dir / "npx").exists() + if provides_npx and not any(node_prefix.is_relative_to(tree) for tree in system_trees): + args += ["--ro-bind", str(node_prefix), SANDBOX_NODE_PREFIX] for raw in ( "/etc/ssl", "/etc/hosts", diff --git a/eval/workflow_bench/runner_artifacts.py b/eval/workflow_bench/runner_artifacts.py index da6b9ae58..2f6dabc90 100644 --- a/eval/workflow_bench/runner_artifacts.py +++ b/eval/workflow_bench/runner_artifacts.py @@ -55,6 +55,30 @@ WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE = frozenset( } ) +# The set above is matched at the workspace ROOT only, because most of its +# entries (package.json, node_modules, the .env family) are also legitimate +# repository content further down the tree -- gitnexus/package.json and +# gitnexus/.claude/settings.local.json are both tracked files whose edits must +# still be caught. But Claude Code bootstraps into whatever directory it is +# running in, so a task whose prompt cd's into a subdirectory gets the same +# noise one level down. Observed in skill-evolution run 29861768554: 13 of 18 +# sessions failed with "phase changed unauthorized workspace path(s): +# gitnexus/.claude/.cc-writes". That entry is matched at ANY depth -- never +# ".claude" itself, which holds real configuration. +# +# Deliberately only .cc-writes. Every excluded name is a blind spot: once a +# .claude directory already exists (gitnexus/.claude/settings.local.json is +# tracked), anything a phase writes underneath an excluded entry becomes +# invisible to this check, and Claude Code loads .claude/agents relative to +# its cwd -- which these tasks point at gitnexus/. Adding "agents" and +# "commands" here on the theory that they might also appear nested would let a +# planning phase plant a definition that the later work phase reads, with no +# evidence in the boundary check. Only .cc-writes was ever observed nested, so +# only .cc-writes is excluded; extend this set from an observed failure, never +# pre-emptively. +CLAUDE_BOOTSTRAP_DIR = ".claude" +CLAUDE_BOOTSTRAP_ENTRIES = frozenset({".cc-writes"}) + IMPLEMENTATION_ARMS = frozenset( { "workflow", @@ -86,6 +110,15 @@ class VerificationResult: yield self.output +def _is_bootstrap_noise(relative: PurePosixPath) -> bool: + """Report whether a walked entry is harness noise rather than workspace change.""" + + parts = relative.parts + if parts[0] == ".git" or parts[0] in WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE: + return True + return len(parts) >= 2 and parts[-2] == CLAUDE_BOOTSTRAP_DIR and parts[-1] in CLAUDE_BOOTSTRAP_ENTRIES + + def workspace_snapshot(worktree: Path) -> dict[str, str]: """Hash the workspace without following links, excluding Git internals and Claude Code's own sandbox-bootstrap noise (see @@ -110,7 +143,7 @@ def workspace_snapshot(worktree: Path) -> dict[str, str]: raise ValueError(f"workspace snapshot directory is unreadable: {directory}: {exc}") from exc for entry in children: relative = relative_dir / entry.name - if relative.parts[0] == ".git" or relative.parts[0] in WORKSPACE_SNAPSHOT_BOOTSTRAP_NOISE: + if _is_bootstrap_noise(relative): continue entry_count += 1 path_bytes += len(relative.as_posix().encode()) From 902291d2b11c0cc2d73e8b567a6cd5a334ef51d3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:27:11 +0100 Subject: [PATCH 03/14] chore(deps)(deps): bump fast-uri from 3.1.2 to 3.1.4 in /gitnexus (#2626) Bumps [fast-uri](https://github.com/fastify/fast-uri) from 3.1.2 to 3.1.4. - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.2...v3.1.4) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- gitnexus/package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index 2800a564c..e8b4557e8 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -3042,9 +3042,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.2", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", - "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", + "version": "3.1.4", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.4.tgz", + "integrity": "sha512-8JnbkQ4juDyvYs4mgFGQqg4yCYtFDtUtmp2QIQq11ZZe5CFQ5wcqm1rqDgAh/QdMySuBnPzMUiJUNZG5N/AiQw==", "funding": [ { "type": "github", From 1bc9df78477d4842055a4dd87ef17a6fc974e06c Mon Sep 17 00:00:00 2001 From: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:46:25 +0530 Subject: [PATCH 04/14] fix(eval): give vitest a writable .vite-temp inside read-only dependency mounts (#2630) * fix(eval): give vitest a writable .vite-temp inside read-only dependency mounts Every task verify command and every hidden oracle ends in `npx vitest run `, and both run through run_verify with read_only_workspace=True. Vite transpiles a TypeScript config by writing /.vite-temp/.timestamp-*.mjs before it loads anything, so against a read-only dependency mount vitest dies with EROFS before a single test executes: EROFS ... /workspace/gitnexus/node_modules/.vite-temp/vitest.config.ts.timestamp-*.mjs This is pre-existing and was masked: until #2627 the verify command died at `npx: not found`, short-circuiting the `&&` chain before vitest ran. Confirmed by reproducing it at that merge base with npx bypassed entirely (`./node_modules/.bin/vitest`), so it is independent of the node-prefix mount. Because it blocks the oracle as well as the authored-test verify, `resolved` stays 0/N without this. bwrap cannot create a mount point inside an already-read-only bind -- the same constraint that put SANDBOX_NODE under /opt/claude -- so overlaying a tmpfs only works if the directory already exists in the mounted bytes. It cannot be mkdir'd into the dependency snapshot after capture either: the snapshot is digest-bound and validate_dependency_binding fails closed on drift. So the empty directory is captured during dependency capture, before the manifest and both dependency digests are computed, making it part of the snapshot rather than an untracked mutation of it. The sandbox then overlays a tmpfs on exactly that path; everything else in the mount, and the whole workspace, stays read-only, and the overlay never reaches the host clone the credited patch comes from. Scoped to dependency mounts whose target basename is node_modules, so hidden oracle and skill mounts stay wholly read-only with no writable island. Note: this shifts sandbox_dependency_content_digest and sandbox_dependency_manifest_digest, so promotion evidence recorded before this change is no longer comparable. That is already true of any harness fix that changes what the sandbox exposes. Verified on the self-hosted runner through the real path -- TaskAssetCache .prepare -> stage_task_assets -> prepare_sandbox -> run_verify with the actual trivial-version-alias verify string: passed, 15/15 tests, no EROFS. Full eval suite there with GITNEXUS_REQUIRE_BWRAP_CANARY=1: 337 passed, 4 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(eval): only overlay .vite-temp where the mount source actually carries it The tmpfs overlay keyed purely on the mount target basename being node_modules, which also matched the trusted GitNexus runtime mount at /opt/gitnexus/node_modules. That mount's source is the built runtime and does not carry a .vite-temp, and bwrap cannot create a mount point inside an already-read-only bind, so the containment CI job failed: bwrap: Can't mkdir /opt/gitnexus/node_modules/.vite-temp: Read-only file system FAILED test_real_bubblewrap_runtime_mount_imports_cli_without_exposing_checkout My runner probe only exercised the dependency-mount path, so it missed this. Gate the overlay on the mount SOURCE actually containing the directory rather than on the target name. task_assets.py captures .vite-temp only into dependency-snapshot node_modules, so the overlay now fires exactly there and never on the runtime mount -- and the gate is correct by construction, since a tmpfs can only overlay a mount point that already exists in the bound bytes. Adds a regression test for a node_modules mount whose source has no captured .vite-temp (the runtime-mount shape) getting no overlay, and updates the positive test to create the directory in its mount source. Verified on the self-hosted runner: the exact failing test now passes, and the full containment selection is 124 passed, 4 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Gergo Magyar Co-authored-by: Claude Opus 4.8 (1M context) --- eval/tests/test_proposer_sandbox.py | 88 +++++++++++++++++++++++++ eval/tests/test_task_assets.py | 35 +++++++++- eval/workflow_bench/proposer_sandbox.py | 22 +++++++ eval/workflow_bench/task_assets.py | 61 ++++++++++++----- 4 files changed, 187 insertions(+), 19 deletions(-) diff --git a/eval/tests/test_proposer_sandbox.py b/eval/tests/test_proposer_sandbox.py index e907c1a0c..fc3ddef4b 100644 --- a/eval/tests/test_proposer_sandbox.py +++ b/eval/tests/test_proposer_sandbox.py @@ -22,6 +22,7 @@ from workflow_bench.proposer_sandbox import ( MAX_EVIDENCE_FILE_BYTES, SANDBOX_NODE, SANDBOX_NODE_PREFIX, + VITE_TEMP_DIR, SANDBOX_PATH, SANDBOX_PYTHON3, SANDBOX_SHELL_PREFIX, @@ -326,6 +327,93 @@ def test_runtime_mounts_skip_the_node_bind_when_node_is_unresolvable(monkeypatch assert SANDBOX_NODE not in args +def test_node_modules_mounts_get_a_writable_vite_temp_overlay(tmp_path: Path) -> None: + # vite writes /.vite-temp/.timestamp-*.mjs before + # loading a TypeScript config, so a read-only dependency mount makes vitest + # fail with EROFS before any test runs -- and every task verify command and + # every hidden oracle ends in "npx vitest run ". Reproduced on the + # self-hosted runner with npx bypassed entirely, proving it is independent + # of the node-prefix mount. + clone = tmp_path / "clone" + clone.mkdir() + deps = tmp_path / "deps" + deps.mkdir() + # task_assets.py captures this directory into the dependency snapshot; the + # overlay is gated on the mount source actually carrying it. + (deps / VITE_TEMP_DIR).mkdir() + executable = tmp_path / "executable" + executable.write_text("#!/bin/sh\nexit 0\n") + executable.chmod(0o755) + + with prepare_sandbox( + clone=clone, + claude_bin=executable, + bwrap_bin=executable, + preflight=False, + read_only_mounts=(ReadOnlyMount(source=deps, target="/workspace/gitnexus/node_modules"),), + ) as sandbox: + argv = sandbox.command_prefix + + bind_index = argv.index("/workspace/gitnexus/node_modules") + assert argv[bind_index - 2 : bind_index + 1] == ["--ro-bind", str(deps), "/workspace/gitnexus/node_modules"] + overlay = f"/workspace/gitnexus/node_modules/{VITE_TEMP_DIR}" + overlay_index = argv.index(overlay) + assert argv[overlay_index - 1] == "--tmpfs" + # the overlay must come AFTER the read-only bind, or the bind would mask it + assert overlay_index > bind_index + + +def test_node_modules_mount_without_a_captured_vite_temp_gets_no_overlay(tmp_path: Path) -> None: + # The trusted GitNexus runtime mounts /opt/gitnexus/node_modules, whose + # source is the built runtime and does NOT carry a .vite-temp. bwrap cannot + # mkdir a mount point inside a read-only bind, so overlaying it would fail + # with "Can't mkdir .../node_modules/.vite-temp: Read-only file system". + # Regression for that CI failure: the overlay must fire only where the + # source actually contains the directory, not for every node_modules mount. + clone = tmp_path / "clone" + clone.mkdir() + runtime = tmp_path / "runtime-node-modules" + runtime.mkdir() # deliberately no .vite-temp + executable = tmp_path / "executable" + executable.write_text("#!/bin/sh\nexit 0\n") + executable.chmod(0o755) + + with prepare_sandbox( + clone=clone, + claude_bin=executable, + bwrap_bin=executable, + preflight=False, + read_only_mounts=(ReadOnlyMount(source=runtime, target="/opt/gitnexus/node_modules"),), + ) as sandbox: + argv = sandbox.command_prefix + + assert "/opt/gitnexus/node_modules" in argv + assert not any(str(item).endswith(f"/{VITE_TEMP_DIR}") for item in argv) + + +def test_non_node_modules_mounts_get_no_vite_temp_overlay(tmp_path: Path) -> None: + # Scoped to dependency mounts: a hidden-oracle or skill mount stays wholly + # read-only, with no writable island inside it. + clone = tmp_path / "clone" + clone.mkdir() + other = tmp_path / "oracle" + other.mkdir() + executable = tmp_path / "executable" + executable.write_text("#!/bin/sh\nexit 0\n") + executable.chmod(0o755) + + with prepare_sandbox( + clone=clone, + claude_bin=executable, + bwrap_bin=executable, + preflight=False, + read_only_mounts=(ReadOnlyMount(source=other, target="/workspace/.wfbench-oracle-abc"),), + ) as sandbox: + argv = sandbox.command_prefix + + assert not any(str(item).endswith(f"/{VITE_TEMP_DIR}") for item in argv) + + def test_stricter_prefix_freezes_evaluated_skills_and_can_unshare_network(tmp_path: Path) -> None: clone = tmp_path / "clone" skill = clone / ".claude" / "skills" / "gitnexus-work" diff --git a/eval/tests/test_task_assets.py b/eval/tests/test_task_assets.py index 4479fce4c..dc5d63ac7 100644 --- a/eval/tests/test_task_assets.py +++ b/eval/tests/test_task_assets.py @@ -9,7 +9,7 @@ from pathlib import Path import pytest -from workflow_bench.proposer_sandbox import SandboxError +from workflow_bench.proposer_sandbox import VITE_TEMP_DIR, SandboxError from workflow_bench.oracle_assets import TaskOracleSnapshot from workflow_bench.runner_tasks import resolve_task_bindings from workflow_bench.task_assets import TaskAssetCache, stage_task_assets @@ -410,3 +410,36 @@ def test_resolved_task_binding_carries_dependency_digests_and_rejects_live_drift (repo / "dependency" / "package.json").write_bytes(b'{"version":2}') with pytest.raises(ValueError, match="definition drifted"): resolve_task_bindings([task], [binding], oracle_snapshots=[oracle]) + + +def test_node_modules_dependency_snapshot_captures_the_vite_temp_mount_point(tmp_path: Path) -> None: + # bwrap cannot mkdir a mount point inside an already-read-only bind, so the + # directory vite needs must exist in the captured dependency bytes. It is + # recorded during capture, which puts it inside the manifest and both + # dependency digests rather than leaving it an untracked mutation of a + # digest-bound snapshot. + repo, _ = _repo_and_task(tmp_path, {"dependency/package.json": b'{"version":1}'}) + task = { + "sandbox_copy": [], + "sandbox_dependencies": [{"source": "dependency", "target": "gitnexus/node_modules"}], + } + with TaskAssetCache(tmp_path / "cache") as cache: + snapshot = cache.prepare(task, repo=repo, resolved_sha=SHA) + captured = {entry.path.as_posix() for entry in snapshot.dependencies[0].entries} + assert f"payload/{VITE_TEMP_DIR}" in captured + vite_temp = next((snapshot.root / "dependencies").glob(f"*/payload/{VITE_TEMP_DIR}")) + assert vite_temp.is_dir() + + +def test_non_node_modules_dependency_snapshot_has_no_vite_temp(tmp_path: Path) -> None: + # The capture is scoped to dependency mounts whose target is node_modules; + # an unrelated vendored dependency is captured byte-for-byte as declared. + repo, _ = _repo_and_task(tmp_path, {"dependency/package.json": b'{"version":1}'}) + task = { + "sandbox_copy": [], + "sandbox_dependencies": [{"source": "dependency", "target": "vendor/dependency"}], + } + with TaskAssetCache(tmp_path / "cache") as cache: + snapshot = cache.prepare(task, repo=repo, resolved_sha=SHA) + captured = {entry.path.as_posix() for entry in snapshot.dependencies[0].entries} + assert not any(path.endswith(VITE_TEMP_DIR) for path in captured) diff --git a/eval/workflow_bench/proposer_sandbox.py b/eval/workflow_bench/proposer_sandbox.py index 7bd46e03c..ea84c545a 100644 --- a/eval/workflow_bench/proposer_sandbox.py +++ b/eval/workflow_bench/proposer_sandbox.py @@ -29,6 +29,14 @@ SANDBOX_SHELL_PREFIX = "/opt/claude/shell-prefix" SANDBOX_PYTHON3 = "/opt/claude/python3" SANDBOX_NODE = "/opt/claude/node" SANDBOX_NODE_PREFIX = "/opt/claude/nodejs" +# Vite transpiles a TypeScript config into /.vite-temp before it +# loads anything, so a read-only dependency mount makes `vitest` die with EROFS +# before a single test runs -- and every task verify command and every hidden +# oracle ends in `npx vitest run `. bwrap cannot create a mount point +# inside an already-read-only bind, so the directory is captured into the +# dependency snapshot (task_assets.py) and a tmpfs is overlaid on it here. +VITE_TEMP_DIR = ".vite-temp" +DEPENDENCY_MOUNT_BASENAME = "node_modules" SANDBOX_PATH = f"/opt/claude:{SANDBOX_NODE_PREFIX}/bin:/usr/local/bin:/usr/bin:/bin" SANDBOX_GITNEXUS = "/opt/gitnexus" SANDBOX_GITNEXUS_SHARED = "/opt/gitnexus-shared" @@ -678,6 +686,20 @@ def _sandbox_command_prefix( ] for mount in mounts: args += ["--ro-bind", str(mount.source), mount.target] + # Overlay an empty writable tmpfs on the one path vite must write. + # Everything else in the mount, and the whole workspace, stays + # read-only, and the overlay lives only inside the sandbox -- it never + # reaches the host clone the credited patch is captured from. + # + # Gate on the mount SOURCE actually containing the directory, not on + # the target name: bwrap cannot create a mount point inside an + # already-read-only bind, so a tmpfs can only be overlaid where the + # directory already exists in the bound bytes. task_assets.py captures + # it into dependency-snapshot node_modules; other node_modules mounts + # (e.g. the trusted GitNexus runtime at /opt/gitnexus/node_modules) do + # not carry it, and overlaying them would fail with EROFS. + if PurePosixPath(mount.target).name == DEPENDENCY_MOUNT_BASENAME and (mount.source / VITE_TEMP_DIR).is_dir(): + args += ["--tmpfs", f"{mount.target}/{VITE_TEMP_DIR}"] args += ["--chdir", SANDBOX_WORKSPACE, "--"] return args diff --git a/eval/workflow_bench/task_assets.py b/eval/workflow_bench/task_assets.py index 14815e0d6..6cd96d84d 100644 --- a/eval/workflow_bench/task_assets.py +++ b/eval/workflow_bench/task_assets.py @@ -25,7 +25,9 @@ from pathlib import Path, PurePosixPath from typing import Any from .proposer_sandbox import ( + DEPENDENCY_MOUNT_BASENAME, SANDBOX_WORKSPACE, + VITE_TEMP_DIR, ReadOnlyMount, SandboxError, _prepare_clone_target, @@ -160,9 +162,11 @@ class TaskAssetSnapshot: source = snapshot_root / Path(*dependency.snapshot_path.parts) metadata = source.lstat() expected_directory = dependency.kind == "directory" - if stat.S_ISLNK(metadata.st_mode) or ( - expected_directory and not stat.S_ISDIR(metadata.st_mode) - ) or (not expected_directory and not stat.S_ISREG(metadata.st_mode)): + if ( + stat.S_ISLNK(metadata.st_mode) + or (expected_directory and not stat.S_ISDIR(metadata.st_mode)) + or (not expected_directory and not stat.S_ISREG(metadata.st_mode)) + ): raise SandboxError(f"dependency snapshot changed: {dependency.source}") target = PurePosixPath(dependency.target) _prepare_clone_target( @@ -213,9 +217,7 @@ class TaskAssetCache: repo_identity = _real_directory(repo, label="task asset repository") declarations, relative_paths = _sandbox_copy_declarations(task) dependency_declarations = _sandbox_dependency_declarations(task) - dependency_identity = tuple( - (declaration.source, declaration.target) for declaration in dependency_declarations - ) + dependency_identity = tuple((declaration.source, declaration.target) for declaration in dependency_declarations) definition = (str(repo_identity), resolved_sha, declarations, dependency_identity) existing = self._by_definition.get(definition) if existing is not None: @@ -258,6 +260,21 @@ class TaskAssetCache: dependency_builder.copy_descriptor(descriptor, PurePosixPath("payload")) finally: os.close(descriptor) + # vitest cannot start against a read-only node_modules: vite + # writes /.vite-temp/.timestamp-*.mjs + # before loading a TypeScript config. bwrap cannot create + # that mount point inside an already-read-only bind, so the + # empty directory is captured here -- before the manifest and + # both dependency digests are computed, so it is part of the + # snapshot rather than an untracked mutation of it. The + # sandbox overlays a tmpfs on it; see VITE_TEMP_DIR. + payload_entry = dependency_builder.entries.get(PurePosixPath("payload")) + if ( + payload_entry is not None + and payload_entry.kind == "directory" + and PurePosixPath(declaration.target).name == DEPENDENCY_MOUNT_BASENAME + ): + dependency_builder.ensure_directory(PurePosixPath("payload") / VITE_TEMP_DIR) dependency_entries = dependency_builder.finished_entries() _validate_dependency_symlinks( container, @@ -462,10 +479,14 @@ class _SnapshotBuilder: destination = self.destination / Path(*relative.parts) os.symlink(target, destination) after = os.stat(name, dir_fd=parent_descriptor, follow_symlinks=False) - if _mutation_identity(before) != _mutation_identity(after) or os.readlink( - name, - dir_fd=parent_descriptor, - ) != target: + if ( + _mutation_identity(before) != _mutation_identity(after) + or os.readlink( + name, + dir_fd=parent_descriptor, + ) + != target + ): raise SandboxError(f"dependency symlink changed while snapshotting: {relative}") self.total_bytes += len(target_bytes) self.budget.total_bytes += len(target_bytes) @@ -503,6 +524,15 @@ class _SnapshotBuilder: self.entries[entry.path] = entry self.budget.entries += 1 + def ensure_directory(self, relative: PurePosixPath) -> None: + """Record and create one extra directory inside this snapshot. + + Used for harness-owned mount points that must exist in the captured + bytes rather than be created against a read-only bind at runtime. + """ + + self._record_directory(relative) + def finished_entries(self) -> tuple[AssetManifestEntry, ...]: return tuple(sorted(self.entries.values(), key=lambda entry: entry.path.as_posix())) @@ -567,9 +597,7 @@ def _sandbox_dependency_declarations( or declaration.target_path in other.target_path.parents or other.target_path in declaration.target_path.parents ): - raise SandboxError( - f"sandbox dependency targets overlap: {declaration.target} and {other.target}" - ) + raise SandboxError(f"sandbox dependency targets overlap: {declaration.target} and {other.target}") return tuple(declarations) @@ -651,9 +679,7 @@ def _validate_dependency_symlinks( ) if sandbox_resolved != sandbox_boundary and sandbox_boundary not in sandbox_resolved.parents: raise SandboxError(f"dependency symlink escapes the sandbox workspace: {entry.path}") - manifest_resolved = PurePosixPath( - posixpath.normpath((entry.path.parent / target).as_posix()) - ) + manifest_resolved = PurePosixPath(posixpath.normpath((entry.path.parent / target).as_posix())) if manifest_resolved != manifest_boundary and manifest_boundary not in manifest_resolved.parents: continue link = container / Path(*entry.path.parts) @@ -1021,8 +1047,7 @@ def _dependency_mounts( snapshot: TaskAssetSnapshot, ) -> list[ReadOnlyMount]: declarations = tuple( - (declaration.source, declaration.target) - for declaration in _sandbox_dependency_declarations(task) + (declaration.source, declaration.target) for declaration in _sandbox_dependency_declarations(task) ) if snapshot.dependency_declarations != declarations: raise SandboxError("task asset snapshot does not match this dependency declaration") From 4c551b719fdc0dd9c88f3d4b73ada9541665d0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 22 Jul 2026 12:27:00 +0100 Subject: [PATCH 05/14] fix(analyze): load VECTOR before the incremental writeback touches embedding rows (#2623) (#2624) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(lbug): add ensureEmbeddingRowDmlSafe VECTOR gate for embedding-row DML LadybugDB refuses every mutation of a table carrying an HNSW index while the VECTOR extension is not loaded on that connection: DELETE and CREATE raise a Binder exception, DROP TABLE is refused while the index references it, and SET segfaults the process. Dropping the index is not an available recovery either — CALL DROP_VECTOR_INDEX is itself a VECTOR-extension function and is undefined in exactly that state. Add a single primitive that loads VECTOR under the analyze install policy and, only when that fails, reads CALL SHOW_INDEXES (which works without the extension) to decide whether an index actually exists to trip over. No call sites yet. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) * test(lbug): pin the #2623 VECTOR gate for embedding-row DML Three cases: no index + VECTOR unavailable stays safe (no needless escalation); index present + VECTOR unavailable is reported blocked AND the raw deleteNodesForFiles genuinely throws 'extension is not loaded' (proving the hazard is real, not theoretical); index present + VECTOR loadable is safe, the delete works, and the HNSW index survives — the invariant run-analyze relies on when it keeps the index across a surgical incremental run. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) * fix(analyze): load VECTOR before the incremental writeback touches embedding rows Incremental analyze died on every content change once a repo had built code_embedding_idx: Analysis failed: Binder exception: Trying to delete from an index on table CodeEmbedding but its extension is not loaded. The surgical writeback's first statement is deleteNodesForFiles' CodeEmbedding join-delete, but nothing on that path loaded VECTOR until Phase 4 — so the engine refused the delete. This is an ordering defect, not an environment one: it reproduces on machines where VECTOR loads fine. The dirty-flag recovery then forced a full rebuild on the next run, which is why it read as 'just slow'. Call ensureEmbeddingRowDmlSafe() once, before the escalation gate and before any row is touched — the same 'index lifecycle before row DML' seam dropSearchFTSIndexes occupies for FTS (#2589). Unconditional, because a DB carrying the index from an earlier --embeddings run hits the same wall on a plain incremental run. When VECTOR truly cannot load the table is immutable (the index cannot be dropped without the extension either), so the run falls through to the existing wipe-and-COPY escalation with a message naming cause, consequence and remedy. Fixes #2623 Co-Authored-By: Claude Opus 4.8 (1M context) * test(analyze): pin the #2623 VECTOR-before-embedding-DML ordering end-to-end Sibling of the #2589 FTS drop-before-delete suite, same shape: drive the real runFullAnalysis incremental path over a real git repo and a real LadybugDB, seed real embedding rows, build the HNSW index, then assert the index state at the exact moment deleteNodesForFiles is invoked. Both cases were confirmed to discriminate — with the run-analyze change reverted they fail with the reported 'Trying to delete from an index on table CodeEmbedding but its extension is not loaded', and pass with it: - surgical path: the run completes, the index is still present AND extension_loaded at delete time, exactly one row per nodeId survives, and the untouched file's rows are preserved - blocked path: with GITNEXUS_LBUG_EXTENSION_INSTALL=never the run escalates to a full DB write and says so, instead of crashing Also applies prettier's reindent to the run-analyze log ternary. Refs #2623 Co-Authored-By: Claude Opus 4.8 (1M context) * docs(lbug): cite the pinned LadybugDB version in the #2623 probe note The probe matrix behind ensureEmbeddingRowDmlSafe was first recorded on 0.18.0, but gitnexus/package-lock.json pins 0.18.2 (#2587). Re-ran every case on 0.18.2: refused DELETE, refused CREATE, SIGSEGV on SET, DROP_VECTOR_INDEX undefined, DROP TABLE refused, SHOW_INDEXES readable with extension_loaded intact. Identical on both, so the design is unchanged — only the citation was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(analyze): preserve embeddings across the VECTOR-blocked rebuild, and check the catalog before loading Three follow-ups from reviewing the fix itself. 1. Data loss on the blocked path. Escalating wipes the DB files, and Phase 3.5 restores embedding rows from cachedEmbeddings — which deriveEmbeddingMode only populates when meta.stats.embeddings > 0. A DB holding embedding rows that its meta does not account for therefore had every vector destroyed silently by a rebuild it never asked for. Probe on a 3-file repo: 3 rows before, 0 after, no warning. Read the rows before escalating (a plain MATCH, no extension needed) so the existing restore has something to restore, and say so in the log. The blocked-path test now asserts the seeded rows survive exactly once, and that assertion fails without this rescue. 2. Catalog before extension. ensureEmbeddingRowDmlSafe loaded VECTOR first and only read SHOW_INDEXES on failure, so every incremental analyze on a machine without VECTOR paid a bounded out-of-process INSTALL attempt plus an 'extension unavailable' warning — including repos that never built an embedding index and can never hit this bug. One local catalog read settles that case first; the load is attempted only when an index actually gates DML, or when the catalog cannot be read. 3. Dead branch. targetConn is always the module singleton there, so the isSharedSingletonConn ternary could never take its second arm. Collapsed to withConnLock. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(doctor): live-probe the VECTOR extension instead of printing the static platform capability Review finding on #2624 (MEDIUM), and exactly what #2623's reporter hit: doctor printed 'VECTOR index: available' — derived from a static platform check — while every incremental analyze on the same machine was dying on an unloaded VECTOR extension. The FTS line was switched to a live LOAD probe for the identical contradiction under #2374; VECTOR now gets the same treatment. probeVectorExtensionLoad shares the FTS probe's implementation (bounded, offline-safe, never runs the installer) and doctor's semantic-mode line now follows the probe, not the platform: without a loadable extension the vector index can be neither built nor queried, so search really is on exact scan. The load-error classifier's remedies are label-parameterized so the VECTOR row stops dispensing FTS-specific advice — 'run analyze --repair-fts' repairs FTS indexes only and was actively wrong for a missing vector extension. Default label stays 'FTS'; every existing caller and pinned remedy string is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(lbug): remove the stale Windows VECTOR gate — the extension ships for win_amd64 The codebase categorically refused VECTOR on Windows (platform !== 'win32' in isVectorExtensionSupportedByPlatform, plus a hard early-return in loadVectorExtension) on the strength of an early-era report that in-process INSTALL VECTOR could SIGSEGV (#1365). That belief is stale, verified directly: - the extension server hosts win_amd64 VECTOR artifacts for every 0.18.x extension version — v0.18.0 and v0.18.1 both serve a real 14 MB PE32+ DLL (curl-probed; 'file' confirms PE32+ x86-64) - the pinned 0.18.2 core resolves its extension directory to 0.18.1 (strace-verified LOAD open()), so the pinned version's Windows artifact exists too - INSTALL now runs in a spawned child (installDuckDbExtensionOutOfProcess), so even a crashing installer kills only the child and degrades to unavailable — the original hazard cannot reach the parent process any more Windows now takes the same runtime path as every other OS: try LOAD, install out-of-process when policy allows, degrade to exact scan when it truly fails. The MCP semantic-search lane loses its static platform gate too — it always attempts the vector index and falls back to the exact scan on runtime failure, with a once-per-backend diagnostic naming the real error instead of a platform-policy message. isVectorExtensionSupportedByPlatform is deleted; getRuntimeCapabilities reports the platform capability as available everywhere and defers machine truth to the live probe. Windows CI is the enforcement: the vector suites skip visibly only when the extension genuinely cannot load, so green Windows lanes now actually exercise VECTOR instead of silently skipping by policy. Co-Authored-By: Claude Opus 4.8 (1M context) * test(lbug): pin the catalog-read-failure fallback in ensureEmbeddingRowDmlSafe Review finding on #2624 (LOW): the one branch where the gate cannot cheaply prove safety — SHOW_INDEXES itself erroring — was exercised only by inference. Force it with a Connection.prototype.query spy over the real DB: the catalog read fails, and the gate must fall through to actually attempting the extension load (asserted via the recorded statement stream) rather than guessing, returning true here because the extension is loadable. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(mcp): load VECTOR on the pool's shared Database so the semantic vector lane actually works Review finding on #2624 (MEDIUM): extension load scope is per-Database (probe-verified — LOAD on one connection enables QUERY_VECTOR_INDEX on every connection of the same Database), and the pool pre-warm loaded only FTS. So LocalBackend's vector lane has ALWAYS raised 'Catalog exception: function QUERY_VECTOR_INDEX is not defined' through the pool and silently fallen back to the exact scan — repos above the 10k exact-scan cap got empty semantic results. The serve path was unaffected (the embedding pipeline loads the extension itself). Mirror the FTS line at BOTH load sites — doInitLbug's pre-warm and initLbugWithDb's external-Database adoption — under the same load-only contract (the read pool never triggers a network install), tracked by a new SharedDB.vectorLoaded flag reset where ftsLoaded resets. The new pool test is discriminating and deliberately closes the writable core adapter before the pool opens: a shared/injected Database would inherit the VECTOR load from test seeding and pass either way, so the case forces the pool onto its OWN fresh read-only Database where only the pre-warm can make the lane legal. Verified: fails at the pre-fix tree with the exact Catalog exception, passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: run the #2623 ordering suite on Windows/macOS and pre-install VECTOR alongside FTS Two review findings on #2624, both landing in existing seams: - scripts/cross-platform-tests.ts gains incremental-vector-extension-ordering .test.ts: the win32 VECTOR gate is gone in this PR, so the #2623 drop-ordering + blocked-path escalation must be proven on the windows-latest native addon, not just Ubuntu. (The review's claim that lbug-delete-nodes-for-files.test.ts was also missing was wrong — it has been on the roster since #2409.) - scripts/ensure-fts.ts now pre-installs VECTOR under the same best-effort auto-policy contract, so every sharded CI process LOADs from ~/.lbdb instead of racing its own bounded out-of-process INSTALL; the workflow's extension cache already covers it (path is the whole extension dir — key kept for cache continuity). The cross-platform job sets GITNEXUS_REQUIRE_VECTOR=1 beside GITNEXUS_REQUIRE_FTS so a genuinely unavailable VECTOR is a loud failure, never a silent skip. Windows/macOS cannot be executed locally; the PR's CI lanes are the proof for this commit. Linux smoke: ensure-fts.ts reports both extensions ready; all 79 roster entries resolve. Co-Authored-By: Claude Opus 4.8 (1M context) * test(pool): register loadVectorExtension in the pool unit-suite mocks The pool adapter's new loadVectorExtension import surfaced in four suites that mock lbug-adapter.js with explicit factories (vitest fails loudly on a missing mocked export). Register the export in each — resolving false where the suite's world assumes no vector, true where it mirrors FTS — and extend lbug-pool-fts-load.test.ts, the suite that owns pre-warm extension loading, with the vector pair: successful load cached per shared Database, failed load retried on the next open, both pinned to policy load-only. Co-Authored-By: Claude Opus 4.8 (1M context) * test(analyze): use POSIX literals for graph paths in the #2623 ordering suite First Windows CI run of this suite (it joined the cross-platform roster this PR) failed with 'Parser exception: Invalid input --------- Co-authored-by: Gergo Magyar Co-authored-by: Claude Opus 4.8 (1M context) --- .github/workflows/ci-tests.yml | 24 +- gitnexus/scripts/cross-platform-tests.ts | 6 + gitnexus/scripts/ensure-fts.ts | 17 +- gitnexus/src/cli/doctor.ts | 34 ++- .../src/core/lbug/extension-load-error.ts | 74 +++-- gitnexus/src/core/lbug/extension-loader.ts | 2 +- gitnexus/src/core/lbug/lbug-adapter.ts | 97 ++++++- gitnexus/src/core/lbug/native-check.ts | 40 ++- gitnexus/src/core/lbug/pool-adapter.ts | 23 +- gitnexus/src/core/platform/capabilities.ts | 23 +- gitnexus/src/core/run-analyze.ts | 63 ++++- gitnexus/src/mcp/local/local-backend.ts | 59 ++-- .../lbug-delete-nodes-for-files.test.ts | 178 +++++++++++- gitnexus/test/integration/lbug-pool.test.ts | 88 ++++++ .../integration/lbug-vector-extension.test.ts | 8 +- .../unit/calltool-dispatch-id-bridge.test.ts | 14 +- gitnexus/test/unit/calltool-dispatch.test.ts | 35 +-- .../group/sync-windowed-resolution.test.ts | 4 +- .../unit/incremental-orchestration.test.ts | 5 +- ...remental-vector-extension-ordering.test.ts | 267 ++++++++++++++++++ gitnexus/test/unit/lbug-pool-fts-load.test.ts | 33 ++- gitnexus/test/unit/lbug-pool-pinning.test.ts | 4 +- gitnexus/test/unit/mcp-wal-feedback.test.ts | 13 +- gitnexus/test/unit/native-check-probe.test.ts | 31 +- .../test/unit/platform-capabilities.test.ts | 18 +- gitnexus/test/unit/pool-wal-recovery.test.ts | 1 + gitnexus/test/unit/trace-bfs.test.ts | 10 +- 27 files changed, 989 insertions(+), 182 deletions(-) create mode 100644 gitnexus/test/unit/incremental-vector-extension-ordering.test.ts diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index d9aa33a4e..c290fa29a 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -46,7 +46,7 @@ jobs: with: path: ~/.lbdb/extension key: lbug-fts-${{ runner.os }}-${{ hashFiles('gitnexus/package-lock.json') }} - - name: Ensure FTS extension installed + - name: Ensure FTS + VECTOR extensions installed run: npx tsx scripts/ensure-fts.ts working-directory: gitnexus - name: Run sharded tests with coverage (blob) @@ -205,6 +205,10 @@ jobs: # tsx-on-source path in CI (both entry points stay covered). env: GITNEXUS_REQUIRE_FTS: '1' + # #2623: the win32 VECTOR gate is gone, so the vector suites genuinely + # run here — require the extension so an unavailable VECTOR is a loud + # failure, never a silent skip (same contract as GITNEXUS_REQUIRE_FTS). + GITNEXUS_REQUIRE_VECTOR: '1' GITNEXUS_E2E_CLI: dist # #2449: hosted Windows runners intermittently push the busiest shard past # the default 15-minute watchdog. 20 minutes restores real headroom while @@ -219,19 +223,21 @@ jobs: - uses: ./.github/actions/setup-gitnexus with: build: 'true' - # Warm-cache the installed LadybugDB FTS extension (~/.lbdb/extension) per - # OS + lockfile so a warm run skips the network install entirely, and the - # parallel shards share one download across runs. Pure reliability/speed: - # on a cache miss the tests self-install FTS on demand (see - # test/helpers/fts-availability.ts), so a miss just falls back to install — - # never a correctness dependency. Keyed by lockfile hash so a LadybugDB - # version bump re-installs; per-OS because the extension is a native binary. + # Warm-cache the installed LadybugDB FTS + VECTOR extensions + # (~/.lbdb/extension) per OS + lockfile so a warm run skips the network + # install entirely, and the parallel shards share one download across + # runs. Pure reliability/speed: on a cache miss the tests self-install on + # demand (see test/helpers/fts-availability.ts), so a miss just falls + # back to install — never a correctness dependency. Keyed by lockfile + # hash so a LadybugDB version bump re-installs; per-OS because the + # extensions are native binaries. (Key name kept as lbug-fts for cache + # continuity — the path covers every extension in the shared home.) - name: Cache LadybugDB FTS extension uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v5 with: path: ~/.lbdb/extension key: lbug-fts-${{ runner.os }}-${{ hashFiles('gitnexus/package-lock.json') }} - - name: Ensure FTS extension installed + - name: Ensure FTS + VECTOR extensions installed run: npx tsx scripts/ensure-fts.ts working-directory: gitnexus - name: Run platform-sensitive tests diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 5d6b177f5..0ccd6beb8 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -117,6 +117,12 @@ const LBUG_NATIVE = [ // to a live native DB, rm-then-rename over an existing parked copy) before // any open — rename semantics are exactly what differs on Windows. 'test/unit/incremental-dirty-recovery.test.ts', + // #2623: the incremental writeback must load VECTOR before the CodeEmbedding + // join-delete, and the blocked path must escalate instead of crashing. The + // win32 VECTOR gate was removed in the same PR, so this ordering must be + // proven on the windows-latest native addon, not just Ubuntu. Budget: ~25s + // on Linux → expect ~2min on the slowest Windows shard. + 'test/unit/incremental-vector-extension-ordering.test.ts', ]; // Process spawning and CLI tests — exercise child_process with real diff --git a/gitnexus/scripts/ensure-fts.ts b/gitnexus/scripts/ensure-fts.ts index a3611de1b..94f781374 100644 --- a/gitnexus/scripts/ensure-fts.ts +++ b/gitnexus/scripts/ensure-fts.ts @@ -1,6 +1,6 @@ /** - * Install the LadybugDB FTS extension into the shared home (~/.lbdb) up front, so - * every test in a sharded CI run finds it regardless of which shard it lands in. + * Install the LadybugDB FTS and VECTOR extensions into the shared home (~/.lbdb) + * up front, so every test in a sharded CI run finds them regardless of shard. * * FTS-dependent tests split two ways: the LOAD-path gate (skipUnlessFtsAvailable) * self-installs on miss, but the FILE-path gate (requireFtsResourceOrSkip, e.g. @@ -17,13 +17,24 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { initLbug, loadFTSExtension, closeLbug } from '../src/core/lbug/lbug-adapter.js'; +import { + initLbug, + loadFTSExtension, + loadVectorExtension, + closeLbug, +} from '../src/core/lbug/lbug-adapter.js'; const dir = mkdtempSync(join(tmpdir(), 'gn-ensure-fts-')); try { await initLbug(join(dir, 'ensure-fts.lbug')); const ok = await loadFTSExtension(undefined, { policy: 'auto' }); console.log(ok ? 'FTS extension ready.' : 'FTS extension unavailable (continuing).'); + // VECTOR rides the same pre-install (#2623): the win32 gate is gone, so the + // vector suites genuinely run on Windows/macOS — installing once here means + // every sharded test process LOADs from ~/.lbdb instead of racing its own + // out-of-process INSTALL (bounded 15s each when the server is unreachable). + const vec = await loadVectorExtension(undefined, { policy: 'auto' }); + console.log(vec ? 'VECTOR extension ready.' : 'VECTOR extension unavailable (continuing).'); } catch (err) { console.warn(`ensure-fts: skipped (${err instanceof Error ? err.message : String(err)})`); } finally { diff --git a/gitnexus/src/cli/doctor.ts b/gitnexus/src/cli/doctor.ts index ce9811de6..7ec8f30f5 100644 --- a/gitnexus/src/cli/doctor.ts +++ b/gitnexus/src/cli/doctor.ts @@ -12,7 +12,11 @@ import { type EmbeddingRuntimeResolution, } from '../core/embeddings/runtime-install.js'; import { cudaRedirectDoctorStatus } from '../core/embeddings/onnxruntime-node-resolver.js'; -import { checkLbugNative, probeFtsExtensionLoad } from '../core/lbug/native-check.js'; +import { + checkLbugNative, + probeFtsExtensionLoad, + probeVectorExtensionLoad, +} from '../core/lbug/native-check.js'; import { getOsPageSize, isPageSizeAwareLadybug } from '../core/lbug/lbug-config.js'; import { diagnoseExtensionLoad } from '../core/lbug/extension-load-error.js'; import { getExtensionInstallPolicy } from '../core/lbug/extension-loader.js'; @@ -195,8 +199,32 @@ export const doctorCommand = async () => { console.log(` ${padDisplayEnd('', 18)}${remedy}`); } } - console.log(` ${label('doctor.labels.vectorIndex', 18)}${capabilities.vector}`); - console.log(` ${label('doctor.labels.semanticMode', 18)}${capabilities.semanticMode}`); + // Live LOAD probe for VECTOR too (#2623). The static capability is just + // `platform !== 'win32'`, so it printed "available" on the very machines + // where analyze was failing to load the extension — the same contradiction + // #2374 fixed for FTS above, and exactly what #2623's reporter saw while + // every incremental analyze died on an unloaded VECTOR extension. + const vectorProbe = nativeCheck.ok + ? await probeVectorExtensionLoad() + : { loaded: false, reason: 'LadybugDB native module (lbugjs.node) failed to load' }; + console.log( + ` ${label('doctor.labels.vectorIndex', 18)}${vectorProbe.loaded ? 'available' : 'unavailable'}`, + ); + if (!vectorProbe.loaded && vectorProbe.reason) { + console.log(` ${padDisplayEnd('', 18)}${vectorProbe.reason}`); + const { kind, remedy } = diagnoseExtensionLoad(vectorProbe.reason, 'VECTOR'); + if (kind !== 'unknown') { + console.log(` ${padDisplayEnd('', 18)}${remedy}`); + } + } + // Semantic mode follows the probe, not the platform: without a loadable + // VECTOR extension the index can be neither built nor queried, so search is + // really on exact scan no matter what the platform would allow. + console.log( + ` ${label('doctor.labels.semanticMode', 18)}${ + vectorProbe.loaded ? capabilities.semanticMode : 'exact-scan' + }`, + ); // Surface the optional-extension install policy so offline users can see // whether analyze/query will reach the network (extension.ladybugdb.com). // Literal label (like the 'native' line) to avoid adding i18n keys. diff --git a/gitnexus/src/core/lbug/extension-load-error.ts b/gitnexus/src/core/lbug/extension-load-error.ts index 67886fe96..712730164 100644 --- a/gitnexus/src/core/lbug/extension-load-error.ts +++ b/gitnexus/src/core/lbug/extension-load-error.ts @@ -101,18 +101,24 @@ const POSIX_MISSING_DEPENDENCY_SIGNATURES: readonly RegExp[] = [ * display language — the only localized part is the OS-error tail after it. So * it is the language-independent fallback signal once the specific tails miss: a * French/German/Japanese Windows 126 has a localized tail we cannot enumerate, - * but it still carries this wrapper. See HEDGED_LOAD_FAILURE_REMEDY. + * but it still carries this wrapper. See hedgedLoadFailureRemedy. */ const LOAD_FAILURE_WRAPPER = /failed to load library/i; -const MISSING_FILE_REMEDY = - 'The FTS extension is not installed. Re-run with network access and ' + - 'GITNEXUS_LBUG_EXTENSION_INSTALL=auto (or `gitnexus analyze --repair-fts`) to download it.'; +// Remedies are label-parameterized (#2623 follow-up): doctor now live-probes +// VECTOR through the same classifier, and FTS-specific advice (`--repair-fts` +// repairs FTS indexes only) must not be dispensed for other extensions. +const repairFtsHint = (label: string, lead: string): string => + label === 'FTS' ? ` (${lead}\`gitnexus analyze --repair-fts\`)` : ''; -const CORRUPT_FILE_REMEDY = - 'The FTS extension file is present but unreadable (corrupt, truncated, or built for another ' + - 'platform). Re-download it with network access and GITNEXUS_LBUG_EXTENSION_INSTALL=auto ' + - '(`gitnexus analyze --repair-fts`).'; +const missingFileRemedy = (label: string): string => + `The ${label} extension is not installed. Re-run with network access and ` + + `GITNEXUS_LBUG_EXTENSION_INSTALL=auto${repairFtsHint(label, 'or ')} to download it.`; + +const corruptFileRemedy = (label: string): string => + `The ${label} extension file is present but unreadable (corrupt, truncated, or built for another ` + + `platform). Re-download it with network access and ` + + `GITNEXUS_LBUG_EXTENSION_INSTALL=auto${repairFtsHint(label, '')}.`; // Single source of truth for the VC++ runtime-install pointer, shared by the // Windows-126 and structural missing-dependency remedies so the name/URL cannot @@ -122,15 +128,15 @@ const VC_REDIST_INSTALL_HINT = 'https://aka.ms/vs/17/release/vc_redist.x64.exe'; // MSVC-first per DuckDB's canonical answer for this exact error; OpenSSL second. -const WINDOWS_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension is present but a required runtime library is missing (Windows error 126). ' + +const windowsMissingDependencyRemedy = (label: string): string => + `The ${label} extension is present but a required runtime library is missing (Windows error 126). ` + 'Reinstalling the extension will NOT help. Install ' + VC_REDIST_INSTALL_HINT + '; if the error persists, the extension also needs OpenSSL 3 ' + '(libcrypto-3-x64.dll / libssl-3-x64.dll) on the DLL search path.'; -const POSIX_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension is present but a shared library it depends on could not be loaded (named in ' + +const posixMissingDependencyRemedy = (label: string): string => + `The ${label} extension is present but a shared library it depends on could not be loaded (named in ` + 'the error above). Reinstalling the extension will NOT help — install that library or add it to ' + 'your loader search path.'; @@ -140,16 +146,18 @@ const POSIX_MISSING_DEPENDENCY_REMEDY = // branches — rather than confidently prescribing the wrong single fix. The clean // long-term fix is upstream: have LadybugDB include the numeric GetLastError/errno // in the message (as it already does elsewhere), so this becomes a code match. -const HEDGED_LOAD_FAILURE_REMEDY = - 'The FTS extension file was found but could not be loaded — see the "Error:" text above (shown ' + +const hedgedLoadFailureRemedy = (label: string): string => + `The ${label} extension file was found but could not be loaded — see the "Error:" text above (shown ` + "in your system's language). Reinstalling usually will not help. If it names a missing module or " + 'library, install the required runtime (on Windows: the Microsoft Visual C++ 2015-2022 ' + - 'Redistributable x64 and OpenSSL 3); if it names a corrupt or invalid file, run ' + - '`gitnexus analyze --repair-fts` to re-download.'; + 'Redistributable x64 and OpenSSL 3); if it names a corrupt or invalid file, ' + + (label === 'FTS' + ? 'run `gitnexus analyze --repair-fts` to re-download.' + : 're-run analyze with network access and GITNEXUS_LBUG_EXTENSION_INSTALL=auto to re-download.'); -const UNKNOWN_REMEDY = - 'The FTS extension failed to load for an unrecognized reason. Run `gitnexus doctor` for live ' + - 'FTS status and verify the extension file and platform.'; +const unknownRemedy = (label: string): string => + `The ${label} extension failed to load for an unrecognized reason. Run \`gitnexus doctor\` for live ` + + `${label} status and verify the extension file and platform.`; const matchesAny = (reason: string, signatures: readonly RegExp[]): boolean => signatures.some((re) => re.test(reason)); @@ -162,19 +170,20 @@ const matchesAny = (reason: string, signatures: readonly RegExp[]): boolean => */ export function classifyExtensionLoadError( reason: string | undefined | null, + label: string = 'FTS', ): ExtensionLoadDiagnosis { const text = reason ?? ''; if (matchesAny(text, MISSING_FILE_SIGNATURES)) { - return { kind: 'missing_file', remedy: MISSING_FILE_REMEDY }; + return { kind: 'missing_file', remedy: missingFileRemedy(label) }; } if (matchesAny(text, FILE_CORRUPTION_SIGNATURES)) { - return { kind: 'corrupt_file', remedy: CORRUPT_FILE_REMEDY }; + return { kind: 'corrupt_file', remedy: corruptFileRemedy(label) }; } if (matchesAny(text, WINDOWS_MISSING_DEPENDENCY_SIGNATURES)) { - return { kind: 'missing_dependency', remedy: WINDOWS_MISSING_DEPENDENCY_REMEDY }; + return { kind: 'missing_dependency', remedy: windowsMissingDependencyRemedy(label) }; } if (matchesAny(text, POSIX_MISSING_DEPENDENCY_SIGNATURES)) { - return { kind: 'missing_dependency', remedy: POSIX_MISSING_DEPENDENCY_REMEDY }; + return { kind: 'missing_dependency', remedy: posixMissingDependencyRemedy(label) }; } // Language-independent fallback: the extension demonstrably failed to load // (lbug's English wrapper is present) but the localized OS tail matched no @@ -182,9 +191,9 @@ export function classifyExtensionLoadError( // remedy — strictly better than the generic `unknown` for non-English hosts, // and it never prescribes the wrong fix. if (LOAD_FAILURE_WRAPPER.test(text)) { - return { kind: 'missing_dependency', remedy: HEDGED_LOAD_FAILURE_REMEDY }; + return { kind: 'missing_dependency', remedy: hedgedLoadFailureRemedy(label) }; } - return { kind: 'unknown', remedy: UNKNOWN_REMEDY }; + return { kind: 'unknown', remedy: unknownRemedy(label) }; } // ── Language-independent structural layer ──────────────────────────────────── @@ -192,8 +201,8 @@ export function classifyExtensionLoadError( /** Well-formedness of the extension binary for the host platform + arch. */ export type ExtensionBinaryState = 'absent' | 'corrupt' | 'valid' | 'indeterminate'; -const STRUCTURAL_MISSING_DEPENDENCY_REMEDY = - 'The FTS extension file is valid, so the failure is a missing or incompatible runtime dependency, ' + +const structuralMissingDependencyRemedy = (label: string): string => + `The ${label} extension file is valid, so the failure is a missing or incompatible runtime dependency, ` + 'not the extension itself — reinstalling will NOT help. On Windows, install ' + VC_REDIST_INSTALL_HINT + ' and ensure OpenSSL 3 is available; on Linux/macOS install the shared library named in the error above.'; @@ -332,13 +341,16 @@ export function inspectExtensionBinary( * classifier (which still carries the language-independent hedged fallback). This * is the entry point every surface should call. */ -export function diagnoseExtensionLoad(reason: string | undefined | null): ExtensionLoadDiagnosis { +export function diagnoseExtensionLoad( + reason: string | undefined | null, + label: string = 'FTS', +): ExtensionLoadDiagnosis { const text = reason ?? ''; - const stringResult = classifyExtensionLoadError(text); + const stringResult = classifyExtensionLoadError(text, label); const fileState = inspectExtensionBinary(extractExtensionPath(text)); if (fileState === 'corrupt') { - return { kind: 'corrupt_file', remedy: CORRUPT_FILE_REMEDY }; + return { kind: 'corrupt_file', remedy: corruptFileRemedy(label) }; } if (fileState === 'valid') { // The structural probe only inspects the first BINARY_HEADER_BYTES, so a file @@ -357,7 +369,7 @@ export function diagnoseExtensionLoad(reason: string | undefined | null): Extens const remedy = stringResult.kind === 'missing_dependency' ? stringResult.remedy - : STRUCTURAL_MISSING_DEPENDENCY_REMEDY; + : structuralMissingDependencyRemedy(label); return { kind: 'missing_dependency', remedy }; } // 'absent' or 'indeterminate' → no positive structural evidence, so defer to the diff --git a/gitnexus/src/core/lbug/extension-loader.ts b/gitnexus/src/core/lbug/extension-loader.ts index c1705336a..b6166a1aa 100644 --- a/gitnexus/src/core/lbug/extension-loader.ts +++ b/gitnexus/src/core/lbug/extension-loader.ts @@ -323,7 +323,7 @@ export class ExtensionManager { name, loaded: false, reason, - diagnosis: diagnoseExtensionLoad(reason), + diagnosis: diagnoseExtensionLoad(reason, label), }); const key = `${name}:${reason}`; if (this.warnedKeys.has(key)) return; diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 0be12b947..2319507ea 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -23,7 +23,11 @@ import { streamAllCSVsToDisk, type StreamedCSVResult } from './csv-generator.js' import type { PdgEmitManifest } from './pdg-emit-sink.js'; import { getNodeLabel as deriveNodeLabel, type WriteStreamFactory } from './rel-pair-routing.js'; import { EMBEDDABLE_LABELS, type CachedEmbedding } from '../embeddings/types.js'; -import { extensionManager, type ExtensionEnsureOptions } from './extension-loader.js'; +import { + extensionManager, + resolveAnalyzeInstallPolicy, + type ExtensionEnsureOptions, +} from './extension-loader.js'; import { classifyDeleteAllError, closeLbugConnection, @@ -51,7 +55,6 @@ import { renameFailureMessage, shadowSidecarRecoveryMessage, } from './sidecar-recovery.js'; -import { isVectorExtensionSupportedByPlatform } from '../platform/capabilities.js'; import { logger } from '../logger.js'; // --------------------------------------------------------------------------- @@ -2696,14 +2699,16 @@ export const loadVectorExtension = async ( ): Promise => { const useModuleState = targetConn === undefined; if (useModuleState && vectorExtensionLoaded) return true; - // INSTALL VECTOR crashes with SIGSEGV on Windows: the KuzuDB native extension - // installer has an unhandled error path on Windows that raises a fatal signal - // that JS try/catch cannot intercept. Skip loading — vector/embedding search - // is unavailable but all graph index queries still work. Do NOT set - // vectorExtensionLoaded here: the flag means "successfully loaded", and a - // subsequent call would otherwise short-circuit to `return true` at the top. - if (process.platform === 'win32') return false; - if (!isVectorExtensionSupportedByPlatform()) return false; + // No platform gate. Windows was hard-refused here for years on the strength + // of an early-era report that in-process INSTALL VECTOR could SIGSEGV + // (#1365) — but the extension server ships win_amd64 VECTOR artifacts for + // every 0.18.x extension version (probed live: v0.18.0 and v0.18.1 both + // serve a real PE32+ DLL; the pinned 0.18.2 core resolves its extension + // directory to 0.18.1, strace-verified), and INSTALL now runs in a spawned + // child process (installDuckDbExtensionOutOfProcess), so even a crashing + // installer kills only the child and degrades to `false` here. LOAD of a + // present extension file is an ordinary in-process load whose failures + // surface as catchable errors, exactly like FTS. const c: lbug.Connection | null = targetConn ?? conn; if (!c) { @@ -2812,6 +2817,78 @@ export const createVectorIndex = async (): Promise => { } }; +/** + * Make DML against {@link EMBEDDING_TABLE_NAME} legal on the writable + * connection when it can be, and report whether it is. + * + * LadybugDB refuses EVERY mutation of a table carrying an HNSW index while + * the VECTOR extension is not loaded on that connection: `DELETE` fails with + * "Trying to delete from an index on table CodeEmbedding but its extension is + * not loaded", `CREATE` with the matching "insert into an index" variant, + * `DROP TABLE` is refused while the index references it, and `SET` — even on + * a NON-indexed property — segfaults the process outright. Probed against + * @ladybugdb/core 0.18.2 (the lockfile-pinned version) and 0.18.0 — every + * result identical on both (#2623). + * + * Dropping the index is NOT an available recovery: `CALL DROP_VECTOR_INDEX` + * is itself a VECTOR-extension function and resolves to "Catalog exception: + * function DROP_VECTOR_INDEX is not defined" in exactly the state it would + * need to rescue. Loading the extension is the only in-place repair, which is + * why this returns a verdict instead of attempting a fixup. + * + * `true` = embedding-row DML is safe: either VECTOR is now loaded, or the + * table carries no index to trip over. `false` = genuinely blocked (index + * present, extension unloadable); the analyze orchestrator answers that by + * escalating to the wipe-and-rebuild write plan instead of failing + * mid-writeback. + * + * Cheap by construction: one local `SHOW_INDEXES` read settles the common + * "this repo never built an embedding index" case without touching the + * extension machinery at all, so a VECTOR-less machine is not charged a + * bounded INSTALL attempt on every incremental analyze. `SHOW_INDEXES` is + * readable WITHOUT the extension and reports `extension_loaded` per index, so + * no error-string sniffing is needed; it runs through the unprepared + * `conn.query()` path like every other `CALL` procedure here (#2114). + */ +export const ensureEmbeddingRowDmlSafe = async (): Promise => { + const targetConn = conn; + if (!targetConn) { + throw new Error('LadybugDB not initialized. Call initLbug first.'); + } + // Catalog FIRST. The overwhelmingly common case on a repo that never enabled + // embeddings is "no index at all", and that is provable with one local read + // — no extension needed. Loading first would make every incremental analyze + // on a VECTOR-less machine pay a bounded out-of-process INSTALL attempt (the + // `auto` policy) plus an "extension unavailable" warning, for a repo that + // can never hit this hazard. + let indexRows: any[] | undefined; + try { + indexRows = await withConnLock(async () => + readQueryRows(await targetConn.query('CALL SHOW_INDEXES() RETURN *')), + ); + } catch (err) { + // Fall through to the load attempt: unable to prove the index is absent, + // so the extension is the only thing that can make DML safe. + logger.warn( + { err }, + `Could not read the index catalog to check for a ${EMBEDDING_TABLE_NAME} vector index; ` + + 'falling back to loading the VECTOR extension.', + ); + } + // Any non-HASH index on the embedding table gates DML. Keyed on index TYPE, + // not name, so an index built under a different name still counts; the + // implicit primary-key HASH index is engine-internal and never gates. + const indexGatesDml = + indexRows === undefined || + indexRows.some((row) => { + const table = row?.table_name ?? row?.[0]; + if (table !== EMBEDDING_TABLE_NAME) return false; + return (row?.index_type ?? row?.[2]) !== 'HASH'; + }); + if (!indexGatesDml) return true; + return await loadVectorExtension(undefined, { policy: resolveAnalyzeInstallPolicy() }); +}; + /** * Lazy-create an FTS index, caching the fact in-process. * diff --git a/gitnexus/src/core/lbug/native-check.ts b/gitnexus/src/core/lbug/native-check.ts index accf43a52..a9971a5e9 100644 --- a/gitnexus/src/core/lbug/native-check.ts +++ b/gitnexus/src/core/lbug/native-check.ts @@ -96,6 +96,9 @@ export interface FtsProbeResult { reason?: string; } +/** Same shape for every optional extension; `FtsProbeResult` is the legacy name. */ +export type ExtensionProbeResult = FtsProbeResult; + const DEFAULT_FTS_PROBE_TIMEOUT_MS = 10_000; /** A LadybugDB query result exposes a synchronous `close()`. */ @@ -136,8 +139,39 @@ const closeProbeResults = (result: unknown): void => { export async function probeFtsExtensionLoad( timeoutMs: number = DEFAULT_FTS_PROBE_TIMEOUT_MS, ): Promise { + return await probeExtensionLoad('fts', timeoutMs); +} + +/** + * Live-probe `LOAD EXTENSION vector`, the VECTOR counterpart of the FTS probe. + * + * Needed for the same reason #2374 needed the FTS one, and reported the same + * way: #2623's reporter saw `doctor` print `VECTOR index: available` while + * every incremental `analyze` was dying because the extension had not loaded. + * `doctor` derived that line from a static platform capability, so it read + * "available" no matter what the extension file was doing. + * + * Probes for real on every platform, Windows included: the extension server + * ships win_amd64 VECTOR artifacts for every 0.18.x extension version (the + * old blanket Windows refusal was stale, #1365-era). LOAD never touches the + * network and never invokes the installer, so this probe is exactly as safe + * as the FTS one above. + */ +export async function probeVectorExtensionLoad( + timeoutMs: number = DEFAULT_FTS_PROBE_TIMEOUT_MS, +): Promise { + return await probeExtensionLoad('vector', timeoutMs); +} + +/** + * Shared LOAD probe. `extension` is a fixed internal literal, never user input. + */ +async function probeExtensionLoad( + extension: 'fts' | 'vector', + timeoutMs: number, +): Promise { let timer: ReturnType | undefined; - const timeout = new Promise((resolve) => { + const timeout = new Promise((resolve) => { timer = setTimeout( () => resolve({ @@ -148,7 +182,7 @@ export async function probeFtsExtensionLoad( ); }); - const probe = (async (): Promise => { + const probe = (async (): Promise => { try { const { default: lbug } = await import('@ladybugdb/core'); const db = new lbug.Database(':memory:'); @@ -156,7 +190,7 @@ export async function probeFtsExtensionLoad( try { const conn = new lbug.Connection(db); try { - const result = await conn.query('LOAD EXTENSION fts'); + const result = await conn.query(`LOAD EXTENSION ${extension}`); closeProbeResults(result); return { loaded: true }; } finally { diff --git a/gitnexus/src/core/lbug/pool-adapter.ts b/gitnexus/src/core/lbug/pool-adapter.ts index a53ca0c92..d88976fa0 100644 --- a/gitnexus/src/core/lbug/pool-adapter.ts +++ b/gitnexus/src/core/lbug/pool-adapter.ts @@ -17,7 +17,7 @@ import fs from 'fs/promises'; import lbug from '@ladybugdb/core'; -import { isReadOnlyDbError, loadFTSExtension } from './lbug-adapter.js'; +import { isReadOnlyDbError, loadFTSExtension, loadVectorExtension } from './lbug-adapter.js'; import { closeQueryResults } from './query-result-utils.js'; import { createLbugDatabase, @@ -126,6 +126,14 @@ interface SharedDB { db: lbug.Database; refCount: number; ftsLoaded: boolean; + /** VECTOR loaded on this Database. Extension load scope is per-Database + * (probe-verified on @ladybugdb/core 0.18.x): loading on any one + * connection enables QUERY_VECTOR_INDEX on every connection of the same + * Database. Without this load the pool's vector lane raised a Catalog + * exception on every semantic query and silently fell back to the exact + * scan (#2623 follow-up). Optional with `?? false` semantics so the + * construction sites stay minimal. */ + vectorLoaded?: boolean; /** File identity at open — used to detect reuse of a shared read-only handle * whose on-disk index was rebuilt/swapped since it opened (only reachable * when a second pool consumer shares this dbPath; #2614 F2). */ @@ -358,6 +366,7 @@ function closeOne(repoId: string): void { // for the same dbPath reuse it instead of hitting a file lock. shared.refCount = 0; shared.ftsLoaded = false; + shared.vectorLoaded = false; } else { shared.db.close().catch(() => {}); dbCache.delete(entry.dbPath); @@ -810,6 +819,13 @@ async function doInitLbug(repoId: string, dbPath: string): Promise { if (!shared.ftsLoaded) { shared.ftsLoaded = await loadFTSExtension(available[0], { policy: 'load-only' }); } + // VECTOR too — extension load scope is per-Database, so this one load + // makes QUERY_VECTOR_INDEX legal on every pooled connection. Same + // load-only contract as FTS above; on failure the semantic-query lane + // falls back to the exact scan with its own diagnostic (#2623 follow-up). + if (!shared.vectorLoaded) { + shared.vectorLoaded = await loadVectorExtension(available[0], { policy: 'load-only' }); + } // Register pool entry only after all connections are pre-warmed and FTS is // loaded. Concurrent executeQuery calls see either "not initialized" @@ -880,6 +896,11 @@ export async function initLbugWithDb( if (!shared.ftsLoaded) { shared.ftsLoaded = await loadFTSExtension(available[0], { policy: 'load-only' }); } + // VECTOR too — same per-Database scope and load-only contract as the + // doInitLbug site above (#2623 follow-up). + if (!shared.vectorLoaded) { + shared.vectorLoaded = await loadVectorExtension(available[0], { policy: 'load-only' }); + } pool.set(repoId, { db: existingDb, diff --git a/gitnexus/src/core/platform/capabilities.ts b/gitnexus/src/core/platform/capabilities.ts index 6bfe459e4..ff2195e22 100644 --- a/gitnexus/src/core/platform/capabilities.ts +++ b/gitnexus/src/core/platform/capabilities.ts @@ -86,23 +86,24 @@ export const getRuntimeFingerprint = (): RuntimeFingerprint => ({ onnxruntime: packageVersion('onnxruntime-node'), }); -export const isVectorExtensionSupportedByPlatform = ( - platform: NodeJS.Platform = process.platform, -): boolean => platform !== 'win32'; - export const getRuntimeCapabilities = (): RuntimeCapabilities => { - const vector = isVectorExtensionSupportedByPlatform() ? 'available' : 'unavailable'; const exactScanLimit = getExactScanLimit(); + // Static PLATFORM capability only. LadybugDB ships the VECTOR extension for + // every platform gitnexus supports — the extension server hosts win_amd64 + // artifacts for every 0.18.x extension version (probed: v0.18.0 and v0.18.1 + // both return a real 14 MB PE32+ DLL; the pinned 0.18.2 core resolves its + // extension directory to 0.18.1, strace-verified), so the old + // `platform !== 'win32'` gate was stale (#1365-era). Whether the extension + // actually LOADS on a given machine is a runtime question — doctor answers + // it with probeVectorExtensionLoad, and analyze/query degrade to exact scan + // when the load fails. return { graph: 'available', fts: 'available', - vector, - semanticMode: vector === 'available' ? 'vector-index' : 'exact-scan', + vector: 'available', + semanticMode: 'vector-index', exactScanLimit, - reason: - vector === 'unavailable' - ? 'LadybugDB VECTOR is disabled on this platform; semantic search uses exact scan when embeddings exist.' - : undefined, + reason: undefined, }; }; diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index cd3c9a458..b404866a5 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -25,6 +25,7 @@ import { closeLbugBeforeExit, loadCachedEmbeddings, deleteNodesForFiles, + ensureEmbeddingRowDmlSafe, deleteAllCommunitiesAndProcesses, deleteAllInterprocTaintPaths, deleteAllCallSummaries, @@ -1686,7 +1687,44 @@ export async function runFullAnalysis( // DB write plan changes here; fileHashes/meta bookkeeping is identical. // Thresholds + the AND-gate live in incremental/escalation-gate.ts. const writeFraction = effectiveWriteSet.size / Math.max(1, allFilePaths.length); + // VECTOR gate (#2623) — load the extension BEFORE a single embedding row + // is touched. `deleteNodesForFiles` below opens with the CodeEmbedding + // join-delete, and LadybugDB refuses all DML on a table carrying its HNSW + // index unless VECTOR is loaded on this connection; nothing else on this + // path loads it until Phase 4, so every incremental run over a DB that + // already built `code_embedding_idx` died here. Same seam the FTS drop + // occupies at the head of this branch (#2589): index lifecycle first, + // then rows. UNCONDITIONAL — not gated on `shouldGenerateEmbeddings` — + // because a DB carrying the index from an earlier `--embeddings` run hits + // the identical wall on a plain incremental run. + // + // When VECTOR genuinely cannot load, the table is immutable (the index + // cannot be dropped without the extension either), so surgery is + // impossible: fall through to the escalation valve's wipe-and-COPY plan, + // which rebuilds the DB files outright and needs no embedding-row DML. + const embeddingRowDmlSafe = await ensureEmbeddingRowDmlSafe(); + if (!embeddingRowDmlSafe && cachedEmbeddings.length === 0) { + // The escalation below WIPES the DB files, and Phase 3.5 restores + // embedding rows from `cachedEmbeddings` — which is only populated when + // `deriveEmbeddingMode` saw `meta.stats.embeddings > 0`. A DB whose meta + // under-reports its embeddings (meta restored from an older run, or a + // count that never got stamped) would therefore have every vector + // silently destroyed by a rebuild it did not ask for. Read them now, + // while the DB is still intact — a plain MATCH, which needs no VECTOR + // extension. Rows whose owning node is gone are dropped by Phase 3.5's + // live-graph filter, exactly as on any other wiped path. + const rescued = await loadCachedEmbeddings(); + if (rescued.embeddings.length > 0) { + cachedEmbeddings = rescued.embeddings; + cachedEmbeddingNodeIds = rescued.embeddingNodeIds; + log( + `Preserving ${rescued.embeddings.length} embedding row(s) across the forced rebuild ` + + `(the index metadata did not account for them).`, + ); + } + } if ( + !embeddingRowDmlSafe || shouldEscalateIncrementalWrite( filesToDelete.length, effectiveWriteSet.size, @@ -1695,13 +1733,20 @@ export async function runFullAnalysis( ) { escalatedFullWrite = true; log( - `Incremental: effective write set covers ${effectiveWriteSet.size}/${allFilePaths.length} ` + - // Display clamp only (predicate unchanged): BFS-found deleted - // importers can push the numerator past the CURRENT file list, so - // the raw fraction can exceed 1 — see the population-mismatch note - // on shouldEscalateIncrementalWrite (tri-review 4669518496). - `files (${Math.min(100, Math.round(writeFraction * 100))}%) — switching to a full DB write ` + - `(wipe + bulk COPY) for this run; file-level incremental bookkeeping is unaffected.`, + !embeddingRowDmlSafe + ? `Incremental: the ${EMBEDDING_TABLE_NAME} vector index exists but the VECTOR ` + + `extension could not be loaded, so embedding rows cannot be rewritten in place — ` + + `switching to a full DB write (wipe + bulk COPY) for this run. Semantic search ` + + `falls back to exact scan until VECTOR is available; run \`gitnexus doctor\` for ` + + `live extension status, or set GITNEXUS_LBUG_EXTENSION_INSTALL=auto to allow one ` + + `bounded install attempt.` + : `Incremental: effective write set covers ${effectiveWriteSet.size}/${allFilePaths.length} ` + + // Display clamp only (predicate unchanged): BFS-found deleted + // importers can push the numerator past the CURRENT file list, so + // the raw fraction can exceed 1 — see the population-mismatch note + // on shouldEscalateIncrementalWrite (tri-review 4669518496). + `files (${Math.min(100, Math.round(writeFraction * 100))}%) — switching to a full DB write ` + + `(wipe + bulk COPY) for this run; file-level incremental bookkeeping is unaffected.`, ); // toWriteCount: 0 is the established full-path dirty-flag sentinel; // the real counters ride along for crash diagnostics. @@ -2067,8 +2112,8 @@ export async function runFullAnalysis( // the case a naive gate would leave index-less again. // buildVectorIndex carries its own extension-policy gate and // warn-on-failure; the boolean feeds semanticMode so the finalize stamp - // reflects the DB's ACTUAL state even when recreation fails (win32 / - // extension unavailable → 'exact-scan'). + // reflects the DB's ACTUAL state even when recreation fails (extension + // unavailable → 'exact-scan'). const dbWasWiped = !isIncremental || escalatedFullWrite; if (restoredEmbeddingCount > 0 && dbWasWiped && embeddingSkipped) { // Re-import at the seam rather than thread a mutable capture from diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 2ed048fb9..7cc72eccb 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -61,10 +61,7 @@ import { type ExactEmbeddingRow, } from '../../core/embeddings/exact-search.js'; import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME } from '../../core/lbug/schema.js'; -import { - getExactScanLimit, - isVectorExtensionSupportedByPlatform, -} from '../../core/platform/capabilities.js'; +import { getExactScanLimit } from '../../core/platform/capabilities.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; import { ftsDegradedWarning } from '../../core/search/fts-indexes.js'; import { @@ -2419,10 +2416,16 @@ export class LocalBackend { string, { distance: number; chunkIndex: number; startLine: number; endLine: number } >(); - if (isVectorExtensionSupportedByPlatform()) { - try { - bestChunks = await collectBestChunks(limit, async (fetchLimit) => { - const vectorQuery = ` + // Always TRY the vector lane — no platform gate. LadybugDB ships the + // VECTOR extension for every supported platform, Windows included + // (#2623 follow-up; the old `platform !== 'win32'` gate was stale), so + // whether the index is queryable is a per-machine runtime fact. The + // catch below is the fallback: any failure (extension unloadable, index + // absent, older DB) degrades to the exact scan with a once-per-backend + // diagnostic instead of being silently swallowed. + try { + bestChunks = await collectBestChunks(limit, async (fetchLimit) => { + const vectorQuery = ` CALL QUERY_VECTOR_INDEX('${EMBEDDING_TABLE_NAME}', '${EMBEDDING_INDEX_NAME}', CAST(${queryVecStr} AS FLOAT[${dims}]), ${fetchLimit}) YIELD node AS emb, distance @@ -2433,27 +2436,27 @@ export class LocalBackend { ORDER BY distance `; - const embResults = await executeQuery(repo.lbugPath, vectorQuery); - return embResults.map((row) => ({ - nodeId: row.nodeId ?? row[0], - chunkIndex: row.chunkIndex ?? row[1] ?? 0, - startLine: row.startLine ?? row[2] ?? 0, - endLine: row.endLine ?? row[3] ?? 0, - distance: row.distance ?? row[4], - })); - }); - } catch { - bestChunks = new Map(); + const embResults = await executeQuery(repo.lbugPath, vectorQuery); + return embResults.map((row) => ({ + nodeId: row.nodeId ?? row[0], + chunkIndex: row.chunkIndex ?? row[1] ?? 0, + startLine: row.startLine ?? row[2] ?? 0, + endLine: row.endLine ?? row[3] ?? 0, + distance: row.distance ?? row[4], + })); + }); + } catch (err) { + bestChunks = new Map(); + if (!this.warnedVectorUnsupported) { + // Rare diagnostic: surface why semantic search fell back to the + // exact scan. Emitted once per `LocalBackend` instance lifetime to + // avoid noisy stderr on hot semantic-search paths (DoD §2.8). + this.warnedVectorUnsupported = true; + logger.warn( + { err }, + 'GitNexus [query:vector]: vector index query failed; using exact scan fallback', + ); } - } else if (!this.warnedVectorUnsupported) { - // Rare diagnostic: surface why we fell back to the exact scan path so - // operators can see at a glance that VECTOR is disabled by platform - // policy. Emitted once per `LocalBackend` instance lifetime to avoid - // noisy stderr on hot semantic-search paths (DoD §2.8). - this.warnedVectorUnsupported = true; - logger.warn( - 'GitNexus [query:vector]: VECTOR extension not supported on this platform; using exact scan fallback', - ); } if (bestChunks.size === 0) { diff --git a/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts b/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts index 736dba3f5..d8e037ba3 100644 --- a/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts +++ b/gitnexus/test/integration/lbug-delete-nodes-for-files.test.ts @@ -18,7 +18,7 @@ * the delete joins `e.nodeId = n.id` through the still-present nodes — * deleted/quoted files' rows go, survivors' rows stay. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import path from 'path'; import { withTestLbugDB } from '../helpers/test-indexed-db.js'; import { buildTestGraph, type TestNodeInput, type TestRelInput } from '../helpers/test-graph.js'; @@ -276,3 +276,179 @@ withTestLbugDB('delete-nodes-missing-embedding-table', () => { }, 120_000); }); }); + +/** + * VECTOR-extension gate for embedding-row DML (#2623). + * + * LadybugDB refuses EVERY mutation of a table carrying an HNSW index while + * the VECTOR extension is not loaded on the connection. The surgical + * incremental writeback's FIRST statement is `deleteNodesForFiles`' embedding + * join-delete, and nothing on that path loaded VECTOR until Phase 4 — so an + * incremental analyze over a DB that already had `code_embedding_idx` died + * with "Trying to delete from an index on table CodeEmbedding but its + * extension is not loaded". + * + * `ensureEmbeddingRowDmlSafe` is the seam that answers "is embedding-row DML + * legal right now?" before a single row is touched. Own withTestLbugDB block: + * these cases close and reopen the DB under a different extension-install + * policy, which would wreck the sibling suites' shared connection. + */ +withTestLbugDB('embedding-row-dml-vector-gate', (handle) => { + describe('ensureEmbeddingRowDmlSafe (#2623)', () => { + const FILE_A = 'src/gate-a.ts'; + const FILE_B = 'src/gate-b.ts'; + const nodeIdFor = (fp: string): string => `Function:${fp}:fn:1`; + + /** Reopen the singleton connection under an explicit install policy. */ + const reopenWithPolicy = async (policy: string | undefined): Promise => { + const { initLbug, closeLbug } = await import('../../src/core/lbug/lbug-adapter.js'); + await closeLbug(); + if (policy === undefined) delete process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + else process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = policy; + await initLbug(handle.dbPath); + }; + + const seedTwoFilesWithEmbeddings = async (): Promise => { + const { executeQuery, executeWithReusedStatement } = + await import('../../src/core/lbug/lbug-adapter.js'); + const { batchInsertEmbeddings } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + for (const fp of [FILE_A, FILE_B]) { + await executeQuery( + `CREATE (:Function {id: '${nodeIdFor(fp)}', name: 'fn', filePath: '${fp}', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + ); + } + await batchInsertEmbeddings( + executeWithReusedStatement, + [FILE_A, FILE_B].map((fp) => ({ + nodeId: nodeIdFor(fp), + chunkIndex: 0, + startLine: 1, + endLine: 3, + embedding: new Array(EMBEDDING_DIMS).fill(0.1), + contentHash: `hash-${fp}`, + })), + ); + }; + + const embeddingCountFor = async (fp: string): Promise => { + const { executeQuery } = await import('../../src/core/lbug/lbug-adapter.js'); + const rows = (await executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) WHERE e.nodeId = '${nodeIdFor(fp)}' RETURN count(e) AS c`, + )) as Array<{ c: number | bigint }>; + return Number(rows[0]?.c ?? 0); + }; + + const clearSeed = async (): Promise => { + const { executeQuery } = await import('../../src/core/lbug/lbug-adapter.js'); + await executeQuery(`MATCH (e:${EMBEDDING_TABLE_NAME}) DELETE e`); + await executeQuery(`MATCH (n:Function) DETACH DELETE n`); + }; + + afterEach(async () => { + await reopenWithPolicy(undefined); + // Teardown deletes embedding rows, so it is itself subject to #2623 once + // a case has built the index — load VECTOR before clearing. + const { ensureEmbeddingRowDmlSafe } = await import('../../src/core/lbug/lbug-adapter.js'); + await ensureEmbeddingRowDmlSafe(); + await clearSeed(); + }); + + it('no vector index + VECTOR unavailable → safe, and the delete still works', async () => { + await seedTwoFilesWithEmbeddings(); + await reopenWithPolicy('never'); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles } = + await import('../../src/core/lbug/lbug-adapter.js'); + + // No HNSW index was ever built, so there is nothing to gate on — the + // degraded path must NOT escalate needlessly. + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + await expect(deleteNodesForFiles([FILE_A])).resolves.toBeUndefined(); + expect(await embeddingCountFor(FILE_A)).toBe(0); + expect(await embeddingCountFor(FILE_B)).toBe(1); + }, 120_000); + + it('vector index present + VECTOR unavailable → blocked, and the raw delete throws', async () => { + await seedTwoFilesWithEmbeddings(); + const { createVectorIndex } = await import('../../src/core/lbug/lbug-adapter.js'); + const built = await createVectorIndex(); + if (!built) return; // VECTOR not installable here — nothing to assert. + + await reopenWithPolicy('never'); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles } = + await import('../../src/core/lbug/lbug-adapter.js'); + + // The gate must SEE the hazard… + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(false); + // …and the hazard must be real: this is the exact #2623 failure. + await expect(deleteNodesForFiles([FILE_A])).rejects.toThrow(/extension is not loaded/); + // Nothing was destroyed by the refused statement. + expect(await embeddingCountFor(FILE_A)).toBe(1); + }, 120_000); + + it('vector index present + VECTOR loadable → safe, delete works, index survives', async () => { + await seedTwoFilesWithEmbeddings(); + const { createVectorIndex } = await import('../../src/core/lbug/lbug-adapter.js'); + const built = await createVectorIndex(); + if (!built) return; // VECTOR not installable here — nothing to assert. + + // Reopen so the in-process "already loaded" latch cannot mask a missing + // load — this is the state a second `analyze` run actually starts from. + await reopenWithPolicy(undefined); + const { ensureEmbeddingRowDmlSafe, deleteNodesForFiles, executeQuery } = + await import('../../src/core/lbug/lbug-adapter.js'); + + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + await expect(deleteNodesForFiles([FILE_A])).resolves.toBeUndefined(); + expect(await embeddingCountFor(FILE_A)).toBe(0); + expect(await embeddingCountFor(FILE_B)).toBe(1); + + // The surgical path KEEPS its index (run-analyze relies on HNSW + // self-maintaining across insert/delete) — it must still be there. + const indexes = (await executeQuery('CALL SHOW_INDEXES() RETURN *')) as Array<{ + table_name?: string; + index_type?: string; + }>; + expect( + indexes.some((r) => r.table_name === EMBEDDING_TABLE_NAME && r.index_type === 'HNSW'), + ).toBe(true); + }, 120_000); + + it('catalog read fails → falls back to attempting the extension load (fail-safe)', async () => { + // The one branch where the gate cannot cheaply prove safety: SHOW_INDEXES + // itself errors. It must fall through to loadVectorExtension — in this + // environment the extension IS loadable, so the verdict is still `true` + // and DML proceeds safely despite the unreadable catalog. + await seedTwoFilesWithEmbeddings(); + // Reopen so the module-level "already loaded" latch cannot let + // loadVectorExtension return true without issuing a LOAD statement. + await reopenWithPolicy('load-only'); + const { ensureEmbeddingRowDmlSafe } = await import('../../src/core/lbug/lbug-adapter.js'); + const { default: lbug } = await import('@ladybugdb/core'); + + const originalQuery = lbug.Connection.prototype.query; + const seen: string[] = []; + const spy = vi.spyOn(lbug.Connection.prototype, 'query').mockImplementation(function ( + this: unknown, + sql: string, + ...rest: unknown[] + ) { + seen.push(sql); + if (sql.includes('SHOW_INDEXES')) { + return Promise.reject(new Error('Catalog exception: forced by test')); + } + return originalQuery.call(this, sql, ...rest); + }); + + try { + await expect(ensureEmbeddingRowDmlSafe()).resolves.toBe(true); + // The catalog read was attempted and failed… + expect(seen.some((s) => s.includes('SHOW_INDEXES'))).toBe(true); + // …and the fallback really attempted the LOAD instead of guessing. + expect(seen.some((s) => s.toUpperCase().includes('LOAD'))).toBe(true); + } finally { + spy.mockRestore(); + } + }, 120_000); + }); +}); diff --git a/gitnexus/test/integration/lbug-pool.test.ts b/gitnexus/test/integration/lbug-pool.test.ts index 561772a75..083974674 100644 --- a/gitnexus/test/integration/lbug-pool.test.ts +++ b/gitnexus/test/integration/lbug-pool.test.ts @@ -315,3 +315,91 @@ withTestLbugDB( poolAdapter: true, }, ); + +/** + * Pool vector lane (#2623 follow-up). + * + * Extension load scope is per-Database, and the pool pre-warm historically + * loaded only FTS — so `CALL QUERY_VECTOR_INDEX` through the pool ALWAYS + * raised `Catalog exception: function QUERY_VECTOR_INDEX is not defined` and + * LocalBackend's semantic lane silently exact-scanned. This block pins that + * the pool's shared Database really can serve the vector lane: rows and the + * HNSW index are built through the core adapter first (the state `analyze + * --embeddings` leaves behind), then the pool opens and must answer a vector + * query. Own withTestLbugDB block: the vector index would leak into the + * sibling suites' shared fixture expectations. + */ +withTestLbugDB( + 'lbug-pool-vector-lane', + (handle) => { + describe('pool vector lane (#2623 follow-up)', () => { + afterEach(async () => { + try { + await closeLbug('vec-repo'); + } catch { + /* best-effort */ + } + }); + + it('QUERY_VECTOR_INDEX works through the pool once the pre-warm loads VECTOR', async (ctx) => { + const core = await import('../../src/core/lbug/lbug-adapter.js'); + const { batchInsertEmbeddings } = + await import('../../src/core/embeddings/embedding-pipeline.js'); + const { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME, EMBEDDING_DIMS } = + await import('../../src/core/lbug/schema.js'); + + // Seed one embedding row for the fixture Function through the CORE + // adapter (writable), then build the HNSW index — skip visibly when + // VECTOR is unavailable in this environment, matching the + // lbug-vector-extension suite convention. + const embedding = new Array(EMBEDDING_DIMS).fill(0); + embedding[0] = 1; + await batchInsertEmbeddings(core.executeWithReusedStatement, [ + { + nodeId: 'func:vec', + chunkIndex: 0, + startLine: 1, + endLine: 3, + embedding, + contentHash: 'vec-hash', + }, + ]); + const indexBuilt = await core.createVectorIndex(); + if (!indexBuilt) { + console.warn('[lbug-pool-vector-lane] Skipping — VECTOR unavailable.'); + ctx.skip(); + return; + } + + // Close the writable core adapter so the pool opens its OWN read-only + // Database. This is what makes the case discriminating: extension + // loads are per-Database, so a shared/injected Database would inherit + // the VECTOR load from createVectorIndex above and pass even without + // the pre-warm fix. A fresh Database has nothing loaded — only the + // pool's own pre-warm can make the vector lane legal. + await core.closeLbug(); + + // The regression: through the POOL, the vector lane must work without + // any caller loading the extension. Pre-fix this rejects with + // "Catalog exception: function QUERY_VECTOR_INDEX is not defined". + await initLbug('vec-repo', handle.dbPath); + const vec = `CAST([${embedding.join(',')}] AS FLOAT[${EMBEDDING_DIMS}])`; + const rows = (await executeQuery( + 'vec-repo', + `CALL QUERY_VECTOR_INDEX('${EMBEDDING_TABLE_NAME}', '${EMBEDDING_INDEX_NAME}', ${vec}, 1) + YIELD node AS emb, distance + RETURN emb.nodeId AS nodeId, distance`, + )) as Array<{ nodeId: string; distance: number }>; + + expect(rows.length).toBe(1); + expect(String(rows[0].nodeId)).toBe('func:vec'); + expect(Number(rows[0].distance)).toBeLessThan(1e-6); + }, 120_000); + }); + }, + { + seed: [ + `CREATE (fn:Function {id: 'func:vec', name: 'vec', filePath: 'src/vec.ts', startLine: 1, endLine: 3, isExported: true, content: '', description: ''})`, + ], + }, +); diff --git a/gitnexus/test/integration/lbug-vector-extension.test.ts b/gitnexus/test/integration/lbug-vector-extension.test.ts index ba436f183..72097089e 100644 --- a/gitnexus/test/integration/lbug-vector-extension.test.ts +++ b/gitnexus/test/integration/lbug-vector-extension.test.ts @@ -88,9 +88,11 @@ withTestLbugDB('vector-extension', (handle) => { * LadybugDB so a revert to the prepared path fails loudly. */ withTestLbugDB('vector-index-creation', () => { - // VECTOR is platform-sensitive (skipped on win32 / unsupported platforms, - // and when it cannot be installed offline). Probe once, skip the suite if - // unavailable — mirrors the FTS-skip convention in withTestLbugDB. + // VECTOR is environment-sensitive (skipped when the extension cannot be + // loaded or installed — offline machines without a pre-installed file). + // Probe once, skip the suite if unavailable — mirrors the FTS-skip + // convention in withTestLbugDB. No platform is categorically excluded: + // win_amd64 artifacts ship for every 0.18.x extension version (#2623). let vectorAvailable = false; let skipWarned = false; beforeAll(async () => { diff --git a/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts b/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts index ca009c542..0390caab6 100644 --- a/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts +++ b/gitnexus/test/unit/calltool-dispatch-id-bridge.test.ts @@ -18,7 +18,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -26,9 +26,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -66,14 +63,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { }; }); -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({ results: [], ftsAvailable: true }), })); @@ -145,7 +134,6 @@ describe('LocalBackend PDG impact — resolved-callee-id bridge (U6)', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); // READY PDG layer so the dispatch reaches the mode-dispatch / bridge surface. vi.mocked(loadMeta).mockResolvedValue({ pdg: { maxCdgEdgesPerFunction: 0, maxReachingDefEdgesPerFunction: 0 }, diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index eb0b94e26..18206ff80 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -17,7 +17,7 @@ import path from 'path'; // local-backend.ts imports from core/lbug/pool-adapter.js; the mcp/core/lbug-adapter.js // re-exports from the same module, so we mock the canonical source. // vi.hoisted runs before vi.mock hoisting, making the fns available to both factories. -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -25,9 +25,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -76,14 +73,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { }; }); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - isVectorExtensionSupportedByPlatform: platformMocks.isVectorExtensionSupportedByPlatform, - }; -}); - // Also mock the search modules to avoid loading onnxruntime vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), @@ -300,7 +289,6 @@ describe('LocalBackend.callTool', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); backend = new LocalBackend(); setupSingleRepo(); await backend.init(); @@ -549,11 +537,18 @@ describe('LocalBackend.callTool', () => { } }); - it('skips vector index query when VECTOR is unsupported by the platform', async () => { + it('falls back to the exact scan with a once-per-backend warning when the vector index query fails', async () => { + // The platform gate is gone (#2623 follow-up): the vector lane is always + // ATTEMPTED, and a runtime failure (extension unloadable, index absent) is + // what routes semantic search onto the exact scan. const cap = _captureLogger(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(false); (executeQuery as any).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; + if (cypher.includes('QUERY_VECTOR_INDEX')) { + throw new Error( + 'Binder exception: Trying to read from an index on table CodeEmbedding but its extension is not loaded.', + ); + } if (cypher.includes('MATCH (e:CodeEmbedding)')) return []; return []; }); @@ -565,7 +560,9 @@ describe('LocalBackend.callTool', () => { const queries = (executeQuery as any).mock.calls.map( ([, cypher]: [string, string]) => cypher, ); - expect(queries.some((cypher: string) => cypher.includes('QUERY_VECTOR_INDEX'))).toBe(false); + // The vector lane was attempted… + expect(queries.some((cypher: string) => cypher.includes('QUERY_VECTOR_INDEX'))).toBe(true); + // …and its failure routed the query onto the exact scan. expect( queries.some( (cypher: string) => @@ -578,7 +575,7 @@ describe('LocalBackend.callTool', () => { .records() .some((r) => String(r.msg ?? '').includes( - 'GitNexus [query:vector]: VECTOR extension not supported on this platform', + 'GitNexus [query:vector]: vector index query failed; using exact scan fallback', ), ), ).toBe(true); @@ -588,7 +585,6 @@ describe('LocalBackend.callTool', () => { }); it('issues vector index query when VECTOR is supported by the platform', async () => { - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); (executeQuery as any).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; return []; @@ -605,7 +601,6 @@ describe('LocalBackend.callTool', () => { }); it('threads GITNEXUS_VECTOR_MAX_DISTANCE into the vector index WHERE clause', async () => { - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); vi.mocked(executeQuery).mockImplementation(async (_repoId: string, cypher: string) => { if (cypher.includes('COUNT(*) AS cnt')) return [{ cnt: 1 }]; return []; @@ -1924,7 +1919,6 @@ describe('LocalBackend impact mode (KTD1/KTD5/KTD12)', () => { beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); // U2: stamp a READY PDG layer (both caps) so the layer-presence probe in // `_impactImpl` falls THROUGH to the mode-dispatch surface these tests pin // (the `_runImpactPDG` delegate / the ambiguous fan-out under `mode:'pdg'`). @@ -3338,7 +3332,6 @@ describe('LocalBackend.listReposPage / callTool list_repos pagination (#2119)', beforeEach(async () => { vi.clearAllMocks(); - platformMocks.isVectorExtensionSupportedByPlatform.mockReturnValue(true); backend = new LocalBackend(); }); diff --git a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts index 25db417c5..6827376f1 100644 --- a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts +++ b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts @@ -92,8 +92,9 @@ describe('partitionManifestWindows (issue #2189 windowed resolution)', () => { // ── Surface 2: real-pool residency bound through syncGroup ─────────────────── -const { loadFTSExtensionMock, openCounter } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock, openCounter } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), openCounter: { live: 0, peak: 0 }, })); @@ -122,6 +123,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/incremental-orchestration.test.ts b/gitnexus/test/unit/incremental-orchestration.test.ts index b74efcb47..fda37c3b5 100644 --- a/gitnexus/test/unit/incremental-orchestration.test.ts +++ b/gitnexus/test/unit/incremental-orchestration.test.ts @@ -931,8 +931,9 @@ describe('runFullAnalysis — incremental orchestration', () => { * and boot a real embedder in CI. This run stays preserve-only (no force). * * Skip-gated on VECTOR availability (the lbug-vector-extension.test.ts - * pattern): hard-false on win32; statically linked on linux-x64, so the - * assertions genuinely run in CI — and on win32 the honest stamp is + * pattern): skipped only where the extension genuinely cannot load — + * no platform is categorically excluded any more (#2623 follow-up) — and + * where it cannot, the honest stamp is * 'exact-scan', which the unit-level wiring pin in * run-analyze-fts-repair.test.ts covers platform-independently. */ diff --git a/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts b/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts new file mode 100644 index 000000000..749a89697 --- /dev/null +++ b/gitnexus/test/unit/incremental-vector-extension-ordering.test.ts @@ -0,0 +1,267 @@ +/** + * #2623: the incremental writeback must have the VECTOR extension loaded + * BEFORE `deleteNodesForFiles` runs — its very first statement is the + * `CodeEmbedding` join-delete, and LadybugDB refuses every mutation of a + * table carrying an HNSW index while the extension is unloaded: + * + * Binder exception: Trying to delete from an index on table CodeEmbedding + * but its extension is not loaded. + * + * Nothing on that path loaded VECTOR until Phase 4, so any repo that had + * built `code_embedding_idx` crashed on the next content change — on machines + * where VECTOR loads perfectly well. This is the sibling of the #2589 FTS + * drop-before-delete ordering test and deliberately mirrors its shape: drive + * the real `runFullAnalysis` incremental path against a real git repo and a + * real LadybugDB, and assert the index state at the exact moment + * `deleteNodesForFiles` is invoked. + */ +import { readFile, writeFile } from 'fs/promises'; +import { execSync } from 'child_process'; +import path from 'path'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { setupMiniRepo } from '../helpers/mini-repo.js'; +import { seedEmbeddingsForFiles, stampEmbeddingCount } from '../helpers/embedding-seed.js'; +import { getStoragePaths } from '../../src/storage/repo-manager.js'; +import { createTempDir } from '../helpers/test-db.js'; +import { EMBEDDING_TABLE_NAME } from '../../src/core/lbug/schema.js'; +import { resolveAnalyzeInstallPolicy } from '../../src/core/lbug/extension-loader.js'; + +const vectorMustBeAvailable = process.env.GITNEXUS_REQUIRE_VECTOR === '1'; + +const commitAll = (cwd: string, message: string): void => { + execSync('git -c user.name=test -c user.email=t@t -c commit.gpgsign=false add -A', { + cwd, + stdio: 'pipe', + }); + execSync( + `git -c user.name=test -c user.email=t@t -c commit.gpgsign=false commit -q -m "${message}"`, + { cwd, stdio: 'pipe' }, + ); +}; + +describe('runFullAnalysis incremental writeback — VECTOR loaded before embedding-row DML (#2623)', () => { + let vectorAvailable = true; + let skipWarned = false; + + beforeAll(async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + // Cheap standalone probe, matching the #2589 suite's convention: settle + // availability once, up front, not inside the expensive test body. + const probe = await createTempDir('gitnexus-2623-vector-probe-'); + try { + await lbugAdapter.initLbug(probe.dbPath); + vectorAvailable = await lbugAdapter.loadVectorExtension(undefined, { + policy: resolveAnalyzeInstallPolicy(), + }); + } finally { + await lbugAdapter.closeLbug(); + await probe.cleanup(); + } + }, 120_000); + + // Skip VISIBLY: a silent `return` would report a false pass and hide an + // ordering regression in exactly the environments least likely to notice. + beforeEach((ctx) => { + if (!vectorAvailable) { + if (vectorMustBeAvailable) { + throw new Error( + 'GITNEXUS_REQUIRE_VECTOR=1 but the VECTOR extension is unavailable — cannot verify the #2623 ordering fix.', + ); + } + if (!skipWarned) { + skipWarned = true; + console.warn( + '[incremental-vector-extension-ordering] Skipping — the LadybugDB VECTOR extension is unavailable.', + ); + } + ctx.skip(); + } + }); + + afterEach(() => { + vi.doUnmock('../../src/core/lbug/lbug-adapter.js'); + vi.resetModules(); + }); + + it('completes the surgical incremental run with the HNSW index present, and VECTOR is loaded by the time deleteNodesForFiles runs', async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + const repo = await setupMiniRepo('gitnexus-2623-vector-order-'); + try { + // First run: full rebuild, real graph. + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + + // Seed real embedding rows for two files, then build the HNSW index — + // the state a prior `analyze --embeddings` leaves behind. Zero vectors + // need no extension for the TABLE; only the index is extension-gated. + // POSIX literals, NOT path.join: the graph stores repo-relative + // filePaths with forward slashes on every OS, and a Windows backslash + // inside the seed helper's single-quoted Cypher literal is a parser + // error ("Invalid input <... n.filePath = '>"). path.join stays only + // for real filesystem access below. + const changedFile = 'src/handler.ts'; + const untouchedFile = 'src/validator.ts'; + const seeded = await seedEmbeddingsForFiles(repo.dbPath, [changedFile, untouchedFile], 2); + const changedIds = seeded.get(changedFile) ?? []; + const untouchedIds = seeded.get(untouchedFile) ?? []; + expect(changedIds.length).toBeGreaterThan(0); + expect(untouchedIds.length).toBeGreaterThan(0); + + const { lbugPath, storagePath } = getStoragePaths(repo.dbPath); + // Without this, deriveEmbeddingMode sees a repo with no embeddings, the + // Phase 3.5 restore never engages, and rows deleted by the importer-BFS + // write-set expansion simply never come back — which would make the + // preservation assertion below measure the wrong thing. + await stampEmbeddingCount(storagePath, changedIds.length + untouchedIds.length); + + const readEmbeddingIndexRows = async (): Promise>> => { + const rows = (await lbugAdapter.executeQuery('CALL SHOW_INDEXES() RETURN *')) as Array< + Record + >; + return rows.filter((r) => r.table_name === EMBEDDING_TABLE_NAME && r.index_type !== 'HASH'); + }; + + await lbugAdapter.initLbug(lbugPath); + const indexBuilt = await lbugAdapter.createVectorIndex(); + const indexRowsBefore = await readEmbeddingIndexRows(); + await lbugAdapter.closeLbug(); + + // The beforeEach gate already proved VECTOR loads here, so a failure to + // build the index is a real bug, not an environment gap. + expect(indexBuilt).toBe(true); + expect(indexRowsBefore.length).toBeGreaterThan(0); + + // Record the index's extension state at the exact moment the embedding + // join-delete is about to run. Pre-fix this is `false` and the run then + // throws; post-fix the gate has already loaded VECTOR. + let embeddingIndexAtDeleteTime: Array> | undefined; + const originalDeleteNodesForFiles = lbugAdapter.deleteNodesForFiles; + vi.spyOn(lbugAdapter, 'deleteNodesForFiles').mockImplementation(async (filePaths, opts) => { + embeddingIndexAtDeleteTime = await readEmbeddingIndexRows(); + return originalDeleteNodesForFiles(filePaths, opts); + }); + + // One-file change keeps this well under the 50-file escalation + // threshold on a 7-file repo, so it takes the surgical branch. + const handlerPath = path.join(repo.dbPath, changedFile); + await writeFile( + handlerPath, + (await readFile(handlerPath, 'utf-8')) + '\n// #2623 ordering-test touch\n', + 'utf-8', + ); + commitAll(repo.dbPath, '#2623 ordering touch'); + + // THE regression: before the fix this rejects with + // "Trying to delete from an index on table CodeEmbedding". + await expect( + runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }), + ).resolves.toBeDefined(); + + // Ordering proof: the index was still there AND its extension was + // loaded when the delete ran — the fix loads VECTOR rather than + // dropping the index (run-analyze relies on HNSW self-maintaining + // across a surgical run). + expect(embeddingIndexAtDeleteTime).toBeDefined(); + expect(embeddingIndexAtDeleteTime!.length).toBeGreaterThan(0); + for (const row of embeddingIndexAtDeleteTime!) { + expect(row.extension_loaded).toBe(true); + } + + // Data outcome: the untouched file's rows survive, and nothing is + // duplicated. (The changed file's rows are removed by the join-delete + // and restored by Phase 3.5, so their count must stay at exactly one + // per nodeId — a PK duplicate would mean the delete silently no-op'd.) + await lbugAdapter.initLbug(lbugPath); + try { + const perNode = (await lbugAdapter.executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN e.nodeId AS nodeId, count(e) AS c`, + )) as Array<{ nodeId: string; c: number | bigint }>; + const counts = new Map(perNode.map((r) => [String(r.nodeId), Number(r.c)])); + for (const id of untouchedIds) { + expect(counts.get(id)).toBe(1); + } + for (const [, c] of counts) { + expect(c).toBe(1); + } + // The index is still there — the surgical path keeps it. + expect((await readEmbeddingIndexRows()).length).toBeGreaterThan(0); + } finally { + await lbugAdapter.closeLbug(); + } + } finally { + await repo.cleanup(); + } + }, 300_000); + + it('escalates to a full DB write instead of crashing when VECTOR cannot be loaded', async () => { + const lbugAdapter = await import('../../src/core/lbug/lbug-adapter.js'); + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + + const repo = await setupMiniRepo('gitnexus-2623-vector-blocked-'); + const previousPolicy = process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + try { + await runFullAnalysis(repo.dbPath, { skipAgentsMd: true }, { onProgress: () => {} }); + // POSIX literal for the graph-side path (see the note in the first case). + const seeded = await seedEmbeddingsForFiles(repo.dbPath, ['src/handler.ts'], 2); + const seededIds = seeded.get('src/handler.ts') ?? []; + expect(seededIds.length).toBeGreaterThan(0); + // Deliberately NOT stampEmbeddingCount: this pins the case where the DB + // holds embedding rows that meta does not account for. The escalation + // wipes the DB, so without an explicit rescue read those rows would be + // destroyed silently — the run would still "succeed" and the loss would + // be invisible. + + const { lbugPath } = getStoragePaths(repo.dbPath); + await lbugAdapter.initLbug(lbugPath); + const indexBuilt = await lbugAdapter.createVectorIndex(); + await lbugAdapter.closeLbug(); + expect(indexBuilt).toBe(true); + + const handlerPath = path.join(repo.dbPath, 'src', 'handler.ts'); + await writeFile( + handlerPath, + (await readFile(handlerPath, 'utf-8')) + '\n// #2623 blocked-path touch\n', + 'utf-8', + ); + commitAll(repo.dbPath, '#2623 blocked touch'); + + // VECTOR becomes unloadable for this run. The table is now immutable + // (the index cannot be dropped without the extension either), so the + // run must abandon surgery rather than fail mid-writeback. + process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = 'never'; + const logs: string[] = []; + await expect( + runFullAnalysis( + repo.dbPath, + { skipAgentsMd: true }, + { onProgress: () => {}, onLog: (m: string) => logs.push(m) }, + ), + ).resolves.toBeDefined(); + + expect(logs.some((m) => m.includes('full DB write'))).toBe(true); + expect(logs.some((m) => m.includes('VECTOR'))).toBe(true); + + // The forced rebuild must NOT eat the embeddings it never asked to touch. + await lbugAdapter.initLbug(lbugPath); + try { + const surviving = (await lbugAdapter.executeQuery( + `MATCH (e:${EMBEDDING_TABLE_NAME}) RETURN e.nodeId AS nodeId`, + )) as Array<{ nodeId: string }>; + const survivingIds = new Set(surviving.map((r) => String(r.nodeId))); + for (const id of seededIds) { + expect(survivingIds.has(id)).toBe(true); + } + // …and exactly once each — the restore must not double-insert. + expect(surviving.length).toBe(survivingIds.size); + } finally { + await lbugAdapter.closeLbug(); + } + expect(logs.some((m) => m.includes('Preserving'))).toBe(true); + } finally { + if (previousPolicy === undefined) delete process.env.GITNEXUS_LBUG_EXTENSION_INSTALL; + else process.env.GITNEXUS_LBUG_EXTENSION_INSTALL = previousPolicy; + await repo.cleanup(); + } + }, 300_000); +}); diff --git a/gitnexus/test/unit/lbug-pool-fts-load.test.ts b/gitnexus/test/unit/lbug-pool-fts-load.test.ts index d62735c27..7f5b6ff87 100644 --- a/gitnexus/test/unit/lbug-pool-fts-load.test.ts +++ b/gitnexus/test/unit/lbug-pool-fts-load.test.ts @@ -1,7 +1,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -const { loadFTSExtensionMock } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn(), })); vi.mock('@ladybugdb/core', () => ({ @@ -16,6 +17,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ @@ -31,10 +33,13 @@ describe('read-pool FTS loading', () => { afterEach(async () => { await closeLbug().catch(() => {}); loadFTSExtensionMock.mockReset(); + loadVectorExtensionMock.mockReset(); + loadVectorExtensionMock.mockResolvedValue(false); }); it('loads FTS with load-only policy and caches a successful load', async () => { loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(true); const db = {} as any; await initLbugWithDb('repo-a', db, '/tmp/shared-fts-db'); @@ -46,6 +51,7 @@ describe('read-pool FTS loading', () => { it('does not fake a successful load when FTS is unavailable', async () => { loadFTSExtensionMock.mockResolvedValue(false); + loadVectorExtensionMock.mockResolvedValue(false); const db = {} as any; await initLbugWithDb('repo-a', db, '/tmp/shared-fts-db'); @@ -59,4 +65,29 @@ describe('read-pool FTS loading', () => { policy: 'load-only', }); }); + + it('loads VECTOR with load-only policy and caches a successful load (#2623 follow-up)', async () => { + loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(true); + const db = {} as any; + + await initLbugWithDb('repo-a', db, '/tmp/shared-vec-db'); + await initLbugWithDb('repo-b', db, '/tmp/shared-vec-db'); + + expect(loadVectorExtensionMock).toHaveBeenCalledTimes(1); + expect(loadVectorExtensionMock).toHaveBeenCalledWith(expect.anything(), { + policy: 'load-only', + }); + }); + + it('retries the VECTOR load on the next open when it was unavailable', async () => { + loadFTSExtensionMock.mockResolvedValue(true); + loadVectorExtensionMock.mockResolvedValue(false); + const db = {} as any; + + await initLbugWithDb('repo-a', db, '/tmp/shared-vec-db'); + await initLbugWithDb('repo-b', db, '/tmp/shared-vec-db'); + + expect(loadVectorExtensionMock).toHaveBeenCalledTimes(2); + }); }); diff --git a/gitnexus/test/unit/lbug-pool-pinning.test.ts b/gitnexus/test/unit/lbug-pool-pinning.test.ts index 4ce154023..125c803e0 100644 --- a/gitnexus/test/unit/lbug-pool-pinning.test.ts +++ b/gitnexus/test/unit/lbug-pool-pinning.test.ts @@ -14,8 +14,9 @@ import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; // repo resident through deferred manifest/workspace resolution. Pinning makes // that resident set survive automatic (LRU + idle) eviction. -const { loadFTSExtensionMock } = vi.hoisted(() => ({ +const { loadFTSExtensionMock, loadVectorExtensionMock } = vi.hoisted(() => ({ loadFTSExtensionMock: vi.fn(), + loadVectorExtensionMock: vi.fn().mockResolvedValue(false), })); vi.mock('@ladybugdb/core', () => ({ @@ -36,6 +37,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ isReadOnlyDbError: vi.fn(() => false), loadFTSExtension: loadFTSExtensionMock, + loadVectorExtension: loadVectorExtensionMock, })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/mcp-wal-feedback.test.ts b/gitnexus/test/unit/mcp-wal-feedback.test.ts index 710d81369..65515a987 100644 --- a/gitnexus/test/unit/mcp-wal-feedback.test.ts +++ b/gitnexus/test/unit/mcp-wal-feedback.test.ts @@ -3,7 +3,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest'; -const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({ +const { lbugMocks, repoMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn(), @@ -11,9 +11,6 @@ const { lbugMocks, platformMocks, repoMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, repoMocks: { listRegisteredRepos: vi.fn(), }, @@ -40,14 +37,6 @@ vi.mock('../../src/core/git-staleness.js', () => ({ 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([]), })); diff --git a/gitnexus/test/unit/native-check-probe.test.ts b/gitnexus/test/unit/native-check-probe.test.ts index 3593977ab..8d18663aa 100644 --- a/gitnexus/test/unit/native-check-probe.test.ts +++ b/gitnexus/test/unit/native-check-probe.test.ts @@ -29,7 +29,10 @@ vi.mock('@ladybugdb/core', () => { return { default: { Database, Connection } }; }); -import { probeFtsExtensionLoad } from '../../src/core/lbug/native-check.js'; +import { + probeFtsExtensionLoad, + probeVectorExtensionLoad, +} from '../../src/core/lbug/native-check.js'; const closeable = () => ({ close: vi.fn() }); @@ -90,3 +93,29 @@ describe('probeFtsExtensionLoad (#2374)', () => { await expect(probeFtsExtensionLoad()).resolves.toEqual({ loaded: true }); }); }); + +describe('probeVectorExtensionLoad (#2623 follow-up)', () => { + it('issues LOAD EXTENSION vector and reports loaded on success — no platform short-circuit', async () => { + h.query.mockResolvedValue(closeable()); + await expect(probeVectorExtensionLoad()).resolves.toEqual({ loaded: true }); + // The probe must really attempt the LOAD (the old code refused Windows + // before ever touching the engine; the artifact ships for win_amd64 too). + expect(h.query).toHaveBeenCalledWith('LOAD EXTENSION vector'); + }); + + it('reports the collapsed reason when LOAD fails', async () => { + h.query.mockRejectedValue(new Error('IO exception:\n extension file not found')); + await expect(probeVectorExtensionLoad()).resolves.toMatchObject({ + loaded: false, + reason: 'IO exception: extension file not found', + }); + }); + + it('times out instead of hanging when the native call never settles', async () => { + h.query.mockReturnValue(new Promise(() => undefined)); + await expect(probeVectorExtensionLoad(20)).resolves.toMatchObject({ + loaded: false, + reason: expect.stringContaining('timed out'), + }); + }); +}); diff --git a/gitnexus/test/unit/platform-capabilities.test.ts b/gitnexus/test/unit/platform-capabilities.test.ts index 3b58d9d16..7497eb1a7 100644 --- a/gitnexus/test/unit/platform-capabilities.test.ts +++ b/gitnexus/test/unit/platform-capabilities.test.ts @@ -1,17 +1,19 @@ import { describe, expect, it } from 'vitest'; import { + getRuntimeCapabilities, getRuntimeFingerprint, - isVectorExtensionSupportedByPlatform, } from '../../src/core/platform/capabilities.js'; describe('platform capabilities', () => { - it('keeps Ladybug VECTOR disabled by default on Windows', () => { - expect(isVectorExtensionSupportedByPlatform('win32')).toBe(false); - }); - - it('allows VECTOR probing on Linux and macOS', () => { - expect(isVectorExtensionSupportedByPlatform('linux')).toBe(true); - expect(isVectorExtensionSupportedByPlatform('darwin')).toBe(true); + it('reports VECTOR as platform-available everywhere, Windows included (#2623 follow-up)', () => { + // LadybugDB ships win_amd64 VECTOR artifacts for every 0.18.x extension + // version, so no platform is categorically excluded any more. Whether the + // extension actually LOADS on a machine is a runtime question answered by + // probeVectorExtensionLoad (doctor) and loadVectorExtension (analyze). + const caps = getRuntimeCapabilities(); + expect(caps.vector).toBe('available'); + expect(caps.semanticMode).toBe('vector-index'); + expect(caps.reason).toBeUndefined(); }); it('resolves the LadybugDB version even though @ladybugdb/core exports omit ./package.json (#2374)', () => { diff --git a/gitnexus/test/unit/pool-wal-recovery.test.ts b/gitnexus/test/unit/pool-wal-recovery.test.ts index 77ccea84d..ca7517878 100644 --- a/gitnexus/test/unit/pool-wal-recovery.test.ts +++ b/gitnexus/test/unit/pool-wal-recovery.test.ts @@ -31,6 +31,7 @@ vi.mock('@ladybugdb/core', () => ({ vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({ loadFTSExtension: vi.fn().mockResolvedValue(true), + loadVectorExtension: vi.fn().mockResolvedValue(true), })); vi.mock('../../src/core/lbug/lbug-config.js', () => ({ diff --git a/gitnexus/test/unit/trace-bfs.test.ts b/gitnexus/test/unit/trace-bfs.test.ts index 33ab72bff..1f3a97baf 100644 --- a/gitnexus/test/unit/trace-bfs.test.ts +++ b/gitnexus/test/unit/trace-bfs.test.ts @@ -6,7 +6,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -const { lbugMocks, platformMocks } = vi.hoisted(() => ({ +const { lbugMocks } = vi.hoisted(() => ({ lbugMocks: { initLbug: vi.fn().mockResolvedValue(undefined), executeQuery: vi.fn().mockResolvedValue([]), @@ -14,9 +14,6 @@ const { lbugMocks, platformMocks } = vi.hoisted(() => ({ closeLbug: vi.fn().mockResolvedValue(undefined), isLbugReady: vi.fn().mockReturnValue(true), }, - platformMocks: { - isVectorExtensionSupportedByPlatform: vi.fn().mockReturnValue(true), - }, })); vi.mock('../../src/core/lbug/pool-adapter.js', async (importOriginal) => { @@ -59,11 +56,6 @@ vi.mock('../../src/storage/git.js', async (importOriginal) => { return { ...actual, getGitRoot: vi.fn().mockReturnValue(null) }; }); -vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => { - const actual = await importOriginal(); - return { ...actual, ...platformMocks }; -}); - vi.mock('../../src/core/search/bm25-index.js', () => ({ searchFTSFromLbug: vi.fn().mockResolvedValue({ results: [], ftsAvailable: true }), })); From e9e2903466eca03afe72d17d4e242484da6246d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 22 Jul 2026 16:31:56 +0100 Subject: [PATCH 06/14] =?UTF-8?q?fix(deps):=20bump=20@ladybugdb/core=20to?= =?UTF-8?q?=20^0.18.3=20=E2=80=94=20rel-property=20IN-predicate=20fix=20(#?= =?UTF-8?q?2508)=20(#2634)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LadybugDB ≤0.18.2 mis-evaluated `r.type IN [...]` on relationship table groups: the boolean-filter fallback skipped writing selection buffers for single-row unflat chunks, dropping/duplicating callers in context() and impact() (upstream LadybugDB#692, fixed by LadybugDB#699, shipped in 0.18.3). Floor the dependency at ^0.18.3 and lock core + all five platform packages. Resurrect the caller-identity regression test from PR #2553 (closed as superseded by the upstream fix): it pins context()/impact() to exact caller IDs across CodeRelation sub-table pairs so any future predicate regression fails loudly. Note: with CREATE-seeded data the test also passes on 0.18.2 (the upstream repro needs COPY-written chunk layouts) — it is a behavioural pin, not a bug reproduction. Fixes #2508 Co-authored-by: Claude Fable 5 --- gitnexus/package-lock.json | 48 ++++---- gitnexus/package.json | 2 +- .../caller-identity-regression.test.ts | 109 ++++++++++++++++++ 3 files changed, 134 insertions(+), 25 deletions(-) create mode 100644 gitnexus/test/integration/caller-identity-regression.test.ts diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index e8b4557e8..7b1d22ad1 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "PolyForm-Noncommercial-1.0.0", "dependencies": { - "@ladybugdb/core": "^0.18.0", + "@ladybugdb/core": "^0.18.3", "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", @@ -1252,9 +1252,9 @@ } }, "node_modules/@ladybugdb/core": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core/-/core-0.18.2.tgz", - "integrity": "sha512-222FjGciEO5Z+/MRQGU+b4IaGAjOgSQzj7fMpOuhMQN4F8nf654kuKRk1iybSiNy6XSw69hIJ0mKwUeBQ8y6Fg==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core/-/core-0.18.3.tgz", + "integrity": "sha512-XjpPKW4MrL28D2gYGTZuIjiEcPx12L21lx58QggrdrItw8o/e9Lmg/Ejoo4Kz08lZj+rIcC1Fu9thzIYOTUlJw==", "hasInstallScript": true, "license": "MIT", "dependencies": { @@ -1263,17 +1263,17 @@ "node-addon-api": "^6.0.0" }, "optionalDependencies": { - "@ladybugdb/core-darwin-arm64": "0.18.2", - "@ladybugdb/core-darwin-x64": "0.18.2", - "@ladybugdb/core-linux-arm64": "0.18.2", - "@ladybugdb/core-linux-x64": "0.18.2", - "@ladybugdb/core-win32-x64": "0.18.2" + "@ladybugdb/core-darwin-arm64": "0.18.3", + "@ladybugdb/core-darwin-x64": "0.18.3", + "@ladybugdb/core-linux-arm64": "0.18.3", + "@ladybugdb/core-linux-x64": "0.18.3", + "@ladybugdb/core-win32-x64": "0.18.3" } }, "node_modules/@ladybugdb/core-darwin-arm64": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-arm64/-/core-darwin-arm64-0.18.2.tgz", - "integrity": "sha512-gAwxsdijBFTz4aZ9ITG6zdQw3lAki0eY33hNBLCfKXjKJLvW/8wCvgCVBglqfqBF5WyI7icFUt+wfy/Fbdfl5A==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-arm64/-/core-darwin-arm64-0.18.3.tgz", + "integrity": "sha512-DGZTOlvSS4esEb1vTekY5IDoAvZAeYzR5cXVkECtQj9BVkk05zsvCAdTPo1Rz1BuI0qvqUVF+2WlIerI67iA2g==", "cpu": [ "arm64" ], @@ -1284,9 +1284,9 @@ ] }, "node_modules/@ladybugdb/core-darwin-x64": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-x64/-/core-darwin-x64-0.18.2.tgz", - "integrity": "sha512-oUjYLc1fW3ntCrO9te55PoPfvhFo8AKeNa/sU66fiQyEAZB7qZJxeHnnLgl/bLueTF2os3RSawq46ZftoD/9Eg==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-darwin-x64/-/core-darwin-x64-0.18.3.tgz", + "integrity": "sha512-Qp6j0CM/orBlK6KD0p/s4ofkIhNUwi1hdCgMw+fj81UHugWHkVLiYV4grRBdHhyplw+snchZpTxvfpxFbkG1Cw==", "cpu": [ "x64" ], @@ -1297,9 +1297,9 @@ ] }, "node_modules/@ladybugdb/core-linux-arm64": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-arm64/-/core-linux-arm64-0.18.2.tgz", - "integrity": "sha512-UppokeTaPl9pN0xOsdMa+hmM68zbN2eKReTZhZNYM16qX0d2OlgbS/NlXi09Wdot+w5qvlZ9Q0iCCPfr7qvPaw==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-arm64/-/core-linux-arm64-0.18.3.tgz", + "integrity": "sha512-F9miYjBuS43I7uNG199FNMqwdHJ98WA6dU3v2SZCeLXmXCdRzmYcuHQWlbNr2Tba9CX58w2XvBZoUaXZKJ/yKQ==", "cpu": [ "arm64" ], @@ -1310,9 +1310,9 @@ ] }, "node_modules/@ladybugdb/core-linux-x64": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-x64/-/core-linux-x64-0.18.2.tgz", - "integrity": "sha512-GypOxCnP2ix/FWM8YhQ41aQYlS+ruoNMJp7pmaF5laJHhL/a+P/apywNTE+9N41Walsl+Emgg9xwxwTC93slow==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-linux-x64/-/core-linux-x64-0.18.3.tgz", + "integrity": "sha512-AfG5RDp/f/IDctDMpTAT5+2MYNtlWT191xiQNjSaWD4X85DhY3Dzps8Qu5VteIAPih5d6mmoaKGs8q0XIjfkFA==", "cpu": [ "x64" ], @@ -1323,9 +1323,9 @@ ] }, "node_modules/@ladybugdb/core-win32-x64": { - "version": "0.18.2", - "resolved": "https://registry.npmjs.org/@ladybugdb/core-win32-x64/-/core-win32-x64-0.18.2.tgz", - "integrity": "sha512-hvFwjhTYdwG2sijapx963a27jP9mlLW0ZFv5Yfj19e0B3T/FqD9CaKULPy23mU2XlkISN2LUkY5qdgvCFztl/g==", + "version": "0.18.3", + "resolved": "https://registry.npmjs.org/@ladybugdb/core-win32-x64/-/core-win32-x64-0.18.3.tgz", + "integrity": "sha512-bHuFk0m9cnq0WGd9I4D8or8g6cC/BS58iatMtilqM3JpDPIQIFk6MQl6exL7P4xyWbkLwQgsrv2ToDnyoQNKvg==", "cpu": [ "x64" ], diff --git a/gitnexus/package.json b/gitnexus/package.json index 114b71ada..6c438f879 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -56,7 +56,7 @@ "version": "node scripts/sync-plugin-manifests.mjs" }, "dependencies": { - "@ladybugdb/core": "^0.18.0", + "@ladybugdb/core": "^0.18.3", "@modelcontextprotocol/sdk": "^1.0.0", "@scarf/scarf": "^1.4.0", "busboy": "^1.6.0", diff --git a/gitnexus/test/integration/caller-identity-regression.test.ts b/gitnexus/test/integration/caller-identity-regression.test.ts new file mode 100644 index 000000000..b72693c69 --- /dev/null +++ b/gitnexus/test/integration/caller-identity-regression.test.ts @@ -0,0 +1,109 @@ +/** + * Regression test for #2508 — context()/impact() must return EXACT caller + * identities, never counts alone. + * + * The #2508 failure shape: a target Function with two CALLS edges arriving + * through two different CodeRelation sub-table pairs — a production + * Function→Function caller and a File→Function test-file caller. On affected + * LadybugDB versions (≤0.18.2), `r.type IN [...]` predicates could drop the + * production caller and duplicate the test caller: the boolean-filter + * fallback skipped writing selection buffers for single-row unflat chunks + * (LadybugDB#692). Fixed upstream in LadybugDB#699, shipped in + * @ladybugdb/core 0.18.3. These assertions pin the IN-predicate query paths + * to exact caller IDs so any future predicate regression that drops or + * duplicates a caller fails loudly here. + */ +import { describe, it, expect, beforeAll, vi } from 'vitest'; +import { LocalBackend } from '../../src/mcp/local/local-backend.js'; +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { withTestLbugDB, type IndexedDBHandle } from '../helpers/test-indexed-db.js'; + +vi.mock('../../src/storage/repo-manager.js', async (importActual) => ({ + ...(await importActual()), + listRegisteredRepos: vi.fn().mockResolvedValue([]), + cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), +})); + +type BackendHandle = IndexedDBHandle & { _backend?: LocalBackend }; + +const TARGET_ID = 'func:classifyOutcome'; +const PROD_CALLER_ID = 'func:resilientFetch'; +const TEST_CALLER_ID = 'file:resilient-fetch.test'; + +withTestLbugDB( + 'caller-identity-2508', + (handle) => { + describe('caller identity across CodeRelation sub-table pairs (#2508)', () => { + let backend: LocalBackend; + + beforeAll(() => { + const ext = handle as BackendHandle; + if (!ext._backend) { + throw new Error('LocalBackend not initialized — afterSetup did not attach _backend'); + } + backend = ext._backend; + }); + + it('context() lists the Function caller and the File caller exactly once each', async () => { + const result = await backend.callTool('context', { name: 'classifyOutcome' }); + expect(result).not.toHaveProperty('error'); + expect(result.status).toBe('found'); + const callerUids = (result.incoming?.calls ?? []).map((c: { uid: string }) => c.uid); + expect(callerUids.sort()).toEqual([TEST_CALLER_ID, PROD_CALLER_ID].sort()); + }); + + it('impact(upstream) returns the production caller by exact id with tests excluded', async () => { + const result = await backend.callTool('impact', { + target: 'classifyOutcome', + direction: 'upstream', + }); + expect(result).not.toHaveProperty('error'); + expect(result.impactedCount).toBeGreaterThanOrEqual(1); + const directIds = (result.byDepth?.[1] ?? []).map((d: { id: string }) => d.id); + expect(directIds).toContain(PROD_CALLER_ID); + expect(directIds).not.toContain(TEST_CALLER_ID); + }); + + it('impact(upstream, includeTests) returns both callers by exact id', async () => { + const result = await backend.callTool('impact', { + target: 'classifyOutcome', + direction: 'upstream', + includeTests: true, + }); + expect(result).not.toHaveProperty('error'); + const directIds = (result.byDepth?.[1] ?? []).map((d: { id: string }) => d.id); + expect(directIds).toContain(PROD_CALLER_ID); + expect(directIds).toContain(TEST_CALLER_ID); + expect(directIds.filter((id: string) => id === TEST_CALLER_ID)).toHaveLength(1); + }); + }); + }, + { + seed: [ + `CREATE (t:Function {id: '${TARGET_ID}', name: 'classifyOutcome', filePath: 'src/integrations/resilient-fetch.ts', startLine: 10, endLine: 20, isExported: true, content: 'function classifyOutcome() {}', description: 'classifies fetch outcomes'})`, + `CREATE (p:Function {id: '${PROD_CALLER_ID}', name: 'resilientFetch', filePath: 'src/integrations/resilient-fetch.ts', startLine: 30, endLine: 60, isExported: true, content: 'function resilientFetch() {}', description: 'production caller'})`, + `CREATE (f:File {id: '${TEST_CALLER_ID}', name: 'resilient-fetch.test.ts', filePath: 'test/unit/resilient-fetch.test.ts', content: 'test module'})`, + `MATCH (a:Function), (b:Function) WHERE a.id = '${PROD_CALLER_ID}' AND b.id = '${TARGET_ID}' + CREATE (a)-[:CodeRelation {type: 'CALLS', confidence: 0.85, reason: 'direct', step: 0}]->(b)`, + `MATCH (a:File), (b:Function) WHERE a.id = '${TEST_CALLER_ID}' AND b.id = '${TARGET_ID}' + CREATE (a)-[:CodeRelation {type: 'CALLS', confidence: 0.9, reason: 'direct', step: 0}]->(b)`, + ], + poolAdapter: true, + afterSetup: async (h) => { + vi.mocked(listRegisteredRepos).mockResolvedValue([ + { + name: 'caller-identity-repo', + path: '/caller-identity/repo', + storagePath: h.tmpHandle.dbPath, + indexedAt: new Date().toISOString(), + lastCommit: 'abc123', + stats: { files: 2, nodes: 3, communities: 0, processes: 0 }, + }, + ]); + const backend = new LocalBackend(); + await backend.init(); + (h as BackendHandle)._backend = backend; + }, + }, +); From 3b074ac289757f6bded042747c03fbb6ccde62ce Mon Sep 17 00:00:00 2001 From: Abhigyan Patwari <126312502+abhigyanpatwari@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:02:53 +0530 Subject: [PATCH 07/14] fix(python): resolve calls through constructor-injected fields (#2628) * fix(python): resolve calls through injected fields * fix(ci): update python capture benchmark fingerprint * fix(python): make constructor field inference conservative --------- Co-authored-by: Gergo Magyar --- .../python-scope/baseline-fingerprint.txt | 2 +- .../ingestion/languages/python/captures.ts | 13 +- .../ingestion/languages/python/interpret.ts | 5 +- .../languages/python/receiver-binding.ts | 115 +++++++- .../languages/python/simple-hooks.ts | 22 +- .../src/core/ingestion/scope-extractor.ts | 8 +- .../knowledge_graph_service.py | 3 + .../memory_service.py | 23 ++ .../test_fixture.py | 6 + .../expected-captures.json | 56 ++-- .../python-constructor-field-receiver.test.ts | 41 +++ .../python-constructor-field-bindings.test.ts | 249 ++++++++++++++++++ 12 files changed, 502 insertions(+), 41 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/knowledge_graph_service.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/memory_service.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/test_fixture.py create mode 100644 gitnexus/test/integration/resolvers/python-constructor-field-receiver.test.ts create mode 100644 gitnexus/test/unit/scope-resolution/python/python-constructor-field-bindings.test.ts diff --git a/gitnexus/bench/python-scope/baseline-fingerprint.txt b/gitnexus/bench/python-scope/baseline-fingerprint.txt index 3853b2f93..cfd834954 100644 --- a/gitnexus/bench/python-scope/baseline-fingerprint.txt +++ b/gitnexus/bench/python-scope/baseline-fingerprint.txt @@ -1 +1 @@ -a99e69ab2dfb897ed771c6a8e29c5b32843a7f734db701e0699afc07c090e4d5 +36e29abc0780bc857b6df6dd180a0b6036c8a28f927ccc2d4fe50eede24d0c99 diff --git a/gitnexus/src/core/ingestion/languages/python/captures.ts b/gitnexus/src/core/ingestion/languages/python/captures.ts index 6b360a20d..2a196bb32 100644 --- a/gitnexus/src/core/ingestion/languages/python/captures.ts +++ b/gitnexus/src/core/ingestion/languages/python/captures.ts @@ -8,10 +8,9 @@ * 1. **Per-name import statements** — `import a, b` and * `from m import x, y` decompose to one match per imported name * (see `import-decomposer.ts`). - * 2. **Receiver type bindings** — each `function_definition` inside a - * class body emits a `@type-binding.self` (or `@type-binding.cls` - * for `@classmethod`) capture so Pass-4 attaches the implicit - * receiver (see `receiver-binding.ts`). + * 2. **Receiver type bindings** — methods emit an implicit `self` / `cls` + * binding, and `__init__` assignments from annotated parameters emit + * class-scoped instance-field bindings (see `receiver-binding.ts`). * * Pure given the input source text. No I/O, no globals consulted. */ @@ -25,7 +24,10 @@ import { } from '../../utils/ast-helpers.js'; import { splitImportStatement } from './import-decomposer.js'; import { getPythonParser, getPythonScopeQuery } from './query.js'; -import { synthesizeReceiverTypeBinding } from './receiver-binding.js'; +import { + synthesizeConstructorFieldTypeBindings, + synthesizeReceiverTypeBinding, +} from './receiver-binding.js'; import { synthesizeDependsReferences } from './depends-references.js'; import { computePythonArityMetadata } from './arity-metadata.js'; import { recordCacheHit, recordCacheMiss } from './cache-stats.js'; @@ -133,6 +135,7 @@ export function emitPythonScopeCaptures( if (fnNode !== null) { const synth = synthesizeReceiverTypeBinding(fnNode); if (synth !== null) out.push(synth); + out.push(...synthesizeConstructorFieldTypeBindings(fnNode)); for (const depRef of synthesizeDependsReferences(fnNode)) out.push(depRef); } continue; diff --git a/gitnexus/src/core/ingestion/languages/python/interpret.ts b/gitnexus/src/core/ingestion/languages/python/interpret.ts index 1d98ee5e3..d435809be 100644 --- a/gitnexus/src/core/ingestion/languages/python/interpret.ts +++ b/gitnexus/src/core/ingestion/languages/python/interpret.ts @@ -119,7 +119,10 @@ export function interpretPythonTypeBinding(captures: CaptureMatch): ParsedTypeBi // `cls` is a self-like receiver; share the source label so downstream // `Registry.lookup` Step 2 treats them identically. else if (captures['@type-binding.cls'] !== undefined) source = 'self'; - else if (captures['@type-binding.constructor'] !== undefined) source = 'constructor-inferred'; + else if (captures['@type-binding.instance-field'] !== undefined) { + source = + captures['@type-binding.parameter'] !== undefined ? 'parameter-annotation' : 'annotation'; + } else if (captures['@type-binding.constructor'] !== undefined) source = 'constructor-inferred'; else if (captures['@type-binding.annotation'] !== undefined) source = 'annotation'; else if (captures['@type-binding.alias'] !== undefined) source = 'assignment-inferred'; else if (captures['@type-binding.return'] !== undefined) source = 'return-annotation'; diff --git a/gitnexus/src/core/ingestion/languages/python/receiver-binding.ts b/gitnexus/src/core/ingestion/languages/python/receiver-binding.ts index ec525d398..620b19302 100644 --- a/gitnexus/src/core/ingestion/languages/python/receiver-binding.ts +++ b/gitnexus/src/core/ingestion/languages/python/receiver-binding.ts @@ -1,6 +1,6 @@ /** - * Synthesize `@type-binding.self` / `@type-binding.cls` captures for - * methods. + * Synthesize implicit receiver and constructor-assigned field type bindings + * for methods. * * Tree-sitter can't easily express "the first parameter of a function * defined directly inside a class body" via a single static query. @@ -113,3 +113,114 @@ export function synthesizeReceiverTypeBinding(fnNode: SyntaxNode): CaptureMatch '@type-binding.type': syntheticCapture('@type-binding.type', first, className), }; } + +/** + * Synthesize class-scope field bindings for the common Python constructor + * injection pattern: + * + * def __init__(self, service: Service): + * self.service = service + * + * An explicit field annotation (`self.service: Service = ...`) is also + * accepted and takes precedence over a parameter annotation. Deliberately do + * not infer from arbitrary unannotated RHS expressions: the receiver resolver + * needs a declared type, not a name-only guess. + */ +export function synthesizeConstructorFieldTypeBindings(fnNode: SyntaxNode): CaptureMatch[] { + if (fnNode.childForFieldName('name')?.text !== '__init__') return []; + if (findEnclosingClassDefinition(fnNode) === null) return []; + if (hasDecorator(fnNode, 'staticmethod') || hasDecorator(fnNode, 'classmethod')) return []; + + const receiver = synthesizeReceiverTypeBinding(fnNode); + const receiverName = receiver?.['@type-binding.self']?.text; + if (receiverName === undefined) return []; + + const parameters = fnNode.childForFieldName('parameters'); + const body = fnNode.childForFieldName('body'); + if (parameters === null || body === null) return []; + + const parameterTypes = new Map(); + for (let i = 0; i < parameters.namedChildCount; i++) { + const parameter = parameters.namedChild(i); + if (parameter === null) continue; + const name = firstParameterName(parameter); + const annotation = parameter.childForFieldName('type'); + if (name !== null && annotation !== null) parameterTypes.set(name, annotation.text); + } + + type Candidate = { readonly match: CaptureMatch; readonly explicit: boolean }; + const candidates = new Map(); + + const stack: SyntaxNode[] = [body]; + while (stack.length > 0) { + const node = stack.pop()!; + if ( + node !== body && + (node.type === 'function_definition' || + node.type === 'lambda' || + node.type === 'class_definition' || + node.type === 'if_statement' || + node.type === 'for_statement' || + node.type === 'while_statement' || + node.type === 'try_statement' || + node.type === 'match_statement') + ) { + continue; + } + + if (node.type === 'assignment') { + const left = node.childForFieldName('left'); + const right = node.childForFieldName('right'); + if (left?.type === 'attribute') { + const object = left.childForFieldName('object'); + const field = left.childForFieldName('attribute'); + if (object?.type === 'identifier' && object.text === receiverName && field !== null) { + const explicitType = node.childForFieldName('type'); + const parameterType = + right?.type === 'identifier' ? parameterTypes.get(right.text) : undefined; + const typeName = explicitType?.text ?? parameterType; + if (typeName !== undefined) { + const explicit = explicitType !== null; + const existing = candidates.get(field.text); + if (existing === undefined || explicit || !existing.explicit) { + candidates.set(field.text, { + explicit, + match: { + '@type-binding.name': syntheticCapture('@type-binding.name', field, field.text), + '@type-binding.type': syntheticCapture( + '@type-binding.type', + explicitType ?? right ?? field, + typeName, + ), + ...(explicit + ? {} + : { + '@type-binding.parameter': syntheticCapture( + '@type-binding.parameter', + right ?? field, + '1', + ), + }), + '@type-binding.instance-field': syntheticCapture( + '@type-binding.instance-field', + node, + '1', + ), + }, + }); + } + } + } + } + } + + // Push in reverse so the LIFO walk visits source order. That keeps Map + // insertion order (and therefore emitted capture order) deterministic. + for (let i = node.namedChildCount - 1; i >= 0; i--) { + const child = node.namedChild(i); + if (child !== null) stack.push(child); + } + } + + return [...candidates.values()].map(({ match }) => match); +} diff --git a/gitnexus/src/core/ingestion/languages/python/simple-hooks.ts b/gitnexus/src/core/ingestion/languages/python/simple-hooks.ts index aafc374a6..5784f7ad7 100644 --- a/gitnexus/src/core/ingestion/languages/python/simple-hooks.ts +++ b/gitnexus/src/core/ingestion/languages/python/simple-hooks.ts @@ -36,15 +36,23 @@ export function pythonFunctionDefinitionLabel( // ─── bindingScopeFor ────────────────────────────────────────────────────── /** Python has no block scope, so the central extractor's "innermost - * enclosing scope" default is already correct: `for x in …` creates - * `x` in the enclosing function/module scope (because we never emit a - * `@scope.block` for the for-loop body), comprehension variables stay - * in their expression context, etc. Returns `null` to delegate. */ + * enclosing scope" default is already correct for ordinary bindings. + * Constructor-injected instance fields are the exception: their marker is + * anchored inside `__init__`, but compound receiver resolution needs the + * field type on the enclosing Class scope. */ export function pythonBindingScopeFor( - _decl: CaptureMatch, - _innermost: Scope, - _tree: ScopeTree, + decl: CaptureMatch, + innermost: Scope, + tree: ScopeTree, ): ScopeId | null { + if (decl['@type-binding.instance-field'] !== undefined) { + let current: Scope | undefined = innermost; + while (current !== undefined) { + if (current.kind === 'Class') return current.id; + if (current.parent === null) break; + current = tree.getScope(current.parent); + } + } return null; } diff --git a/gitnexus/src/core/ingestion/scope-extractor.ts b/gitnexus/src/core/ingestion/scope-extractor.ts index 08d2a689d..9fe8ae8b9 100644 --- a/gitnexus/src/core/ingestion/scope-extractor.ts +++ b/gitnexus/src/core/ingestion/scope-extractor.ts @@ -982,13 +982,15 @@ function followChainedRef(start: TypeRef, draftById: ReadonlyMap None: + pass diff --git a/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/memory_service.py b/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/memory_service.py new file mode 100644 index 000000000..9095229b6 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/memory_service.py @@ -0,0 +1,23 @@ +from knowledge_graph_service import KnowledgeGraphService + + +class MemoryService: + def __init__(self, knowledge_graph_service: KnowledgeGraphService): + self.knowledge_graph_service = knowledge_graph_service + + def store_memory(self, text: str) -> None: + self.knowledge_graph_service.extract_and_store_graph(text) + + def archive_memory(self, text: str) -> None: + self.knowledge_graph_service.extract_and_store_graph(text) + + def restore_memory(self, text: str) -> None: + self.knowledge_graph_service.extract_and_store_graph(text) + + +class ExplicitFieldMemoryService: + def __init__(self, knowledge_graph_service): + self.knowledge_graph_service: KnowledgeGraphService = knowledge_graph_service + + def ingest_memory(self, text: str) -> None: + self.knowledge_graph_service.extract_and_store_graph(text) diff --git a/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/test_fixture.py b/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/test_fixture.py new file mode 100644 index 000000000..02c3b2894 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-constructor-field-receiver/test_fixture.py @@ -0,0 +1,6 @@ +def extract_and_store_graph(text: str) -> None: + pass + + +def exercise_decoy(text: str) -> None: + extract_and_store_graph(text) diff --git a/gitnexus/test/fixtures/python-captures-golden/expected-captures.json b/gitnexus/test/fixtures/python-captures-golden/expected-captures.json index 254f2bb0d..616bc191b 100644 --- a/gitnexus/test/fixtures/python-captures-golden/expected-captures.json +++ b/gitnexus/test/fixtures/python-captures-golden/expected-captures.json @@ -88,8 +88,8 @@ "digest": "338c3922981604e71ddfc60ad61eba4b17f68ca654644e01add942c729b422cf" }, "python-call-result-binding/models.py": { - "captureGroups": 16, - "digest": "cbbb5168c28123820a70fe24016b0ed26ab02a6339d21ffe40fbccad940c1d70" + "captureGroups": 17, + "digest": "441e2596001c4eaea4808ae6dd195a031f99d8ffe38c8fd450415c109d3365e2" }, "python-call-result-binding/service.py": { "captureGroups": 9, @@ -171,13 +171,25 @@ "captureGroups": 15, "digest": "201ce01b83b21d729aca89c6299570df55393a95f1899a2eaacd989873950177" }, + "python-constructor-field-receiver/knowledge_graph_service.py": { + "captureGroups": 10, + "digest": "83dcf9f81ac7d0a9e9ed5e467e41acd07e2ec581926d85eea5d4b5e1e4157744" + }, + "python-constructor-field-receiver/memory_service.py": { + "captureGroups": 59, + "digest": "ad9c3be5a7c10e112bb20eac20b8603196fc2e739b7567159c58a607639ce192" + }, + "python-constructor-field-receiver/test_fixture.py": { + "captureGroups": 13, + "digest": "6f642e752086a5e9337d21accb65ca90ef5af2b3e139239ea12c5cd031844dd2" + }, "python-constructor-type-inference/models/repo.py": { - "captureGroups": 16, - "digest": "ad11823ee187cc3e1efab34a67a5013119b4ada87c5701b080921c0b0be09e62" + "captureGroups": 17, + "digest": "3c400c7a331d7796a730e1ba53b91c4f5ec4799121044b0c160844988fca8662" }, "python-constructor-type-inference/models/user.py": { - "captureGroups": 16, - "digest": "cbbb5168c28123820a70fe24016b0ed26ab02a6339d21ffe40fbccad940c1d70" + "captureGroups": 17, + "digest": "441e2596001c4eaea4808ae6dd195a031f99d8ffe38c8fd450415c109d3365e2" }, "python-constructor-type-inference/services/app.py": { "captureGroups": 13, @@ -192,12 +204,12 @@ "digest": "dd51c32d705934b1384991ad2291869f446327752481abc20600d4ad9f553ea3" }, "python-dict-items-loop/repo.py": { - "captureGroups": 15, - "digest": "8116cf4cbf4dca377e88f97ca645f40fab648a4e9a8e790b5b3761e4a3e17d7c" + "captureGroups": 16, + "digest": "2d283b4acbc71e318a4520cb7557508b9084213ba53fbdac07457e348a8b24c6" }, "python-dict-items-loop/user.py": { - "captureGroups": 15, - "digest": "15984fa30be4603f3e47c27342352dd602d104b33ac5224dc911d78a82d87926" + "captureGroups": 16, + "digest": "6568834a7f228e78a11b08282196e138795980aed6c55b9953cc89e87c998e52" }, "python-django-app-imports/accounts/__init__.py": { "captureGroups": 0, @@ -288,8 +300,8 @@ "digest": "d1e23831dcae38034b278bfefa2b8c4e21ca722ab2c79bfb3126744338a2a401" }, "python-enumerate-loop/user.py": { - "captureGroups": 16, - "digest": "cbbb5168c28123820a70fe24016b0ed26ab02a6339d21ffe40fbccad940c1d70" + "captureGroups": 17, + "digest": "441e2596001c4eaea4808ae6dd195a031f99d8ffe38c8fd450415c109d3365e2" }, "python-field-type-disambig/address.py": { "captureGroups": 9, @@ -316,8 +328,8 @@ "digest": "5c290b3b34f3f5e9dcdd6ee3ae72ba4223337cd64592330f9a8c6c6f13b2fd2d" }, "python-for-call-expr/models.py": { - "captureGroups": 39, - "digest": "133a14c0543a41412d9d4fd5485d6270e1f4b0cd247f0ec0d71974850e4918c1" + "captureGroups": 41, + "digest": "e8807a9969732197feb04204d200d5810b895c006f1424a7a6f5f0d5762ef49a" }, "python-function-local-import-chain/app.py": { "captureGroups": 9, @@ -464,8 +476,8 @@ "digest": "01fe4805f59723a5f163d26b7be3ed3e456eb3a093e3df3a8f034cecee22ebb9" }, "python-method-chain-binding/models.py": { - "captureGroups": 45, - "digest": "ea3f745514a330d86447796734faff29f0c88ea49a7ef8781087c537426d29bf" + "captureGroups": 48, + "digest": "f049817034428759193fce03b417ca23c133f6ead87f960e173afba9b79b88e8" }, "python-method-enrichment/app.py": { "captureGroups": 13, @@ -680,8 +692,8 @@ "digest": "741f690b6330491303b9b58cb31027a33600973265b59428facfefbabf0cf7e1" }, "python-return-type-inference/models.py": { - "captureGroups": 16, - "digest": "cbbb5168c28123820a70fe24016b0ed26ab02a6339d21ffe40fbccad940c1d70" + "captureGroups": 17, + "digest": "441e2596001c4eaea4808ae6dd195a031f99d8ffe38c8fd450415c109d3365e2" }, "python-return-type-inference/service.py": { "captureGroups": 9, @@ -760,8 +772,8 @@ "digest": "4df7ea089c43552ca4ea5a51f8e985d11d351b86949efb2f7ebefdf6a9ffd689" }, "python-walrus-operator/models.py": { - "captureGroups": 21, - "digest": "cf014d1bad66ea61e327c146fd1a52159a96faaf264555bb164230a5218e6948" + "captureGroups": 22, + "digest": "9ab1a8a69970e8c875bd103251ecf9c8af2c1464a6bdc1213fe7560c4fa34063" }, "python-write-access/models.py": { "captureGroups": 11, @@ -772,7 +784,7 @@ "digest": "6e3690ec68d8de54f376bb6f5f7a29da829a8a6a24001eabae9ec66a327f3409" }, "synthetic:dao-20": { - "captureGroups": 733, - "digest": "c540f2143882137e6c6f7996bf9b87a0117f41915b1d0989d3d6bb9db5a5a1ab" + "captureGroups": 773, + "digest": "37e047eda37477bbc33f4dd8ba259c3f876580378566c0c14dfe580795a952af" } } diff --git a/gitnexus/test/integration/resolvers/python-constructor-field-receiver.test.ts b/gitnexus/test/integration/resolvers/python-constructor-field-receiver.test.ts new file mode 100644 index 000000000..8d6f4613c --- /dev/null +++ b/gitnexus/test/integration/resolvers/python-constructor-field-receiver.test.ts @@ -0,0 +1,41 @@ +import { beforeAll, describe, expect, it } from 'vitest'; +import path from 'node:path'; +import { FIXTURES, getRelationships, runPipelineFromRepo, type PipelineResult } from './helpers.js'; + +describe('Python calls through constructor-assigned receiver fields', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'python-constructor-field-receiver'), + () => {}, + ); + }, 60_000); + + it('resolves all production callers to the receiver-constrained method', () => { + const productionCalls = getRelationships(result, 'CALLS').filter( + (edge) => + edge.target === 'extract_and_store_graph' && + edge.targetFilePath === 'knowledge_graph_service.py', + ); + + expect(productionCalls.map((edge) => `${edge.sourceFilePath}:${edge.source}`).sort()).toEqual([ + 'memory_service.py:archive_memory', + 'memory_service.py:ingest_memory', + 'memory_service.py:restore_memory', + 'memory_service.py:store_memory', + ]); + expect(productionCalls.every((edge) => edge.rel.confidence >= 0.85)).toBe(true); + }); + + it('does not redirect production calls to the same-named decoy', () => { + const misresolved = getRelationships(result, 'CALLS').filter( + (edge) => + edge.sourceFilePath === 'memory_service.py' && + edge.target === 'extract_and_store_graph' && + edge.targetFilePath === 'test_fixture.py', + ); + + expect(misresolved).toEqual([]); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/python/python-constructor-field-bindings.test.ts b/gitnexus/test/unit/scope-resolution/python/python-constructor-field-bindings.test.ts new file mode 100644 index 000000000..de2a69508 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/python/python-constructor-field-bindings.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, it } from 'vitest'; +import type { CaptureMatch } from 'gitnexus-shared'; +import { + emitPythonScopeCaptures, + interpretPythonTypeBinding, +} from '../../../../src/core/ingestion/languages/python/index.js'; +import { extractParsedFile } from '../../../../src/core/ingestion/scope-extractor-bridge.js'; +import { pythonProvider } from '../../../../src/core/ingestion/languages/python.js'; + +function constructorFieldBindings(source: string): CaptureMatch[] { + return emitPythonScopeCaptures(source, 'fixture.py').filter( + (match) => match['@type-binding.instance-field'] !== undefined, + ); +} + +function interpretedBindings(source: string): Array<{ + boundName: string; + rawTypeName: string; + source: string; +}> { + return constructorFieldBindings(source).map((match) => { + const binding = interpretPythonTypeBinding(match); + expect(binding).not.toBeNull(); + return binding!; + }); +} + +describe('Python constructor field type bindings', () => { + it.each([ + { + name: 'annotated constructor parameter', + source: ` +class Facade: + def __init__(self, service: Service): + self.service = service +`, + }, + { + name: 'typed default parameter with a nullable forward reference', + source: ` +class Facade: + def __init__(self, service: "Service | None" = None): + self.service = service +`, + }, + { + name: 'custom receiver name', + source: ` +class Facade: + def __init__(this, service: Service): + this.service = service +`, + }, + ])('synthesizes a parameter-derived class field for $name', ({ source }) => { + expect(interpretedBindings(source)).toEqual([ + { boundName: 'service', rawTypeName: 'Service', source: 'parameter-annotation' }, + ]); + }); + + it('preserves explicit field-annotation provenance', () => { + const source = ` +class Facade: + def __init__(self, service): + self.service: Service = service +`; + + expect(interpretedBindings(source)).toEqual([ + { boundName: 'service', rawTypeName: 'Service', source: 'annotation' }, + ]); + }); + + it.each([ + { + name: 'an unannotated constructor parameter', + source: ` +class Facade: + def __init__(self, service): + self.service = service +`, + }, + { + name: 'an assignment on a different receiver', + source: ` +class Facade: + def __init__(self, service: Service): + other.service = service +`, + }, + { + name: 'a non-constructor method', + source: ` +class Facade: + def configure(self, service: Service): + self.service = service +`, + }, + { + name: 'a static constructor-shaped method', + source: ` +class Facade: + @staticmethod + def __init__(self, service: Service): + self.service = service +`, + }, + { + name: 'an annotation inside a nested function', + source: ` +class Facade: + def __init__(self, value): + def configure(): + self.service: Service = value +`, + }, + { + name: 'an annotation inside a nested class', + source: ` +class Facade: + def __init__(self, value): + class Nested: + self.service: Service = value +`, + }, + { + name: 'an assignment inside an if branch', + source: ` +class Facade: + def __init__(self, service: Service, enabled: bool): + if enabled: + self.service = service +`, + }, + { + name: 'an assignment inside a for loop', + source: ` +class Facade: + def __init__(self, services: list[Service]): + for service in services: + self.service: Service = service +`, + }, + { + name: 'an assignment inside a while loop', + source: ` +class Facade: + def __init__(self, service: Service, enabled: bool): + while enabled: + self.service = service +`, + }, + { + name: 'an assignment inside a try statement', + source: ` +class Facade: + def __init__(self, service: Service): + try: + self.service = service + except RuntimeError: + pass +`, + }, + ])('does not synthesize a binding for $name', ({ source }) => { + expect(constructorFieldBindings(source)).toEqual([]); + }); + + it('uses the final inferred assignment for a repeatedly assigned field', () => { + const source = ` +class Facade: + def __init__(self, primary: PrimaryService, fallback: FallbackService): + self.service = primary + self.service = fallback +`; + + expect(interpretedBindings(source)).toEqual([ + { + boundName: 'service', + rawTypeName: 'FallbackService', + source: 'parameter-annotation', + }, + ]); + }); + + it('prefers an explicit field annotation over the parameter annotation', () => { + const source = ` +class Facade: + def __init__(self, service: Protocol): + self.service: ConcreteService = service +`; + + expect(interpretedBindings(source)).toEqual([ + { boundName: 'service', rawTypeName: 'ConcreteService', source: 'annotation' }, + ]); + }); + + it('hoists only the synthesized field binding to the enclosing class scope', () => { + const parsed = extractParsedFile( + pythonProvider, + ` +class Facade: + def __init__(self, service: Service): + self.service = service +`, + 'fixture.py', + ); + expect(parsed).toBeDefined(); + + const classScope = parsed!.scopes.find((scope) => scope.kind === 'Class'); + const constructorScope = parsed!.scopes.find((scope) => scope.kind === 'Function'); + expect(classScope?.typeBindings.get('service')).toMatchObject({ + rawName: 'Service', + source: 'parameter-annotation', + }); + expect(constructorScope?.typeBindings.get('service')).toMatchObject({ + rawName: 'Service', + source: 'parameter-annotation', + }); + }); + + it('does not override a class-body field annotation with constructor inference', () => { + const parsed = extractParsedFile( + pythonProvider, + ` +class Facade: + service: ServiceProtocol + + def __init__(self, service: ConcreteService): + self.service = service +`, + 'fixture.py', + ); + expect(parsed).toBeDefined(); + + const classScope = parsed!.scopes.find((scope) => scope.kind === 'Class'); + expect(classScope?.typeBindings.get('service')).toMatchObject({ + rawName: 'ServiceProtocol', + source: 'annotation', + }); + }); + + it('is deterministic across repeated capture runs', () => { + const source = ` +class Facade: + def __init__(self, service: Service): + self.service = service +`; + + expect(constructorFieldBindings(source)).toEqual(constructorFieldBindings(source)); + }); +}); From 44edcbc9a38418f7311d7b91afaf1b66a2d46631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 22 Jul 2026 20:09:48 +0100 Subject: [PATCH 08/14] fix(lbug): scale the buffer-pool budget by the OS page-size granule ratio (#2631) (#2636) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(lbug): scale the buffer-pool budget by the OS-page discard-granule ratio (#2631) LadybugDB bills buffer-pool budget per discard granule, not per 4 KiB frame: the engine's vm_region.cpp sets discardGranuleSize = max(frameSize, osPageSize), claimFrame charges the whole granule when its first frame becomes resident, and releaseFrame refunds only when the granule's last frame leaves — while BufferManager::reserve measures eviction progress in refunded bytes and throws 'The buffer pool is full and no memory could be freed!' after three zero-refund passes. On a 64 KiB-page kernel (Ascend/aarch64 openEuler — the #2631 reporter's host) that is 16 frames per granule: the same COPY bills up to 16× the budget it needs on x86, and whole eviction passes can evict frames yet refund nothing. Apple Silicon macOS (16 KiB pages) is the same mechanism at 4×. Measured with the reporter's exact command and version: vllm-ascend needs a (128, 256] MiB pool on 4 KiB pages — 64/128 MiB reproduce the reporter's byte-identical error, 256 MiB and the 576 MiB adaptive pool succeed — so their 64 KiB host cannot survive on a page-size-blind budget. Scale every derived pool size by granuleRatio = max(1, osPageSize/4096): the per-element estimate, the COPY-safety floor, and the default cap (still bounded by 80% of RAM). 4 KiB hosts are byte-identical to before — proven by pinning the existing sizing tests to an explicit 4096 page size, which also stops them drifting on 16 KiB Apple Silicon runners. GITNEXUS_LBUG_BUFFER_POOL_SIZE keeps absolute precedence and 0 still restores the native default. Also: bufferPoolExhaustionRemedy() gives the exhaustion error an actionable cause→consequence→remedy message; the isLbugPageSizeFrameError comment that called pool exhaustion 'a sizing problem, not a page-size one' is corrected — that framing inverted when #2582 made pool size a function of a page-size-blind estimate. Cannot execute on a 64 KiB kernel here: the scaled path is proven by unit stubs plus the engine-source math above; the env override remains the field escape hatch. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(cli): actionable pool-exhaustion remedies at the COPY sites and a doctor pool line (#2631) The node-COPY throw and the relationship-COPY warning now append bufferPoolExhaustionRemedy() when the failure is the engine's pool-exhaustion class: the raw binder text gave the operator nothing to act on, and on non-4K-page hosts the pool bills up to pageSize/4KiB × faster than the sizing was calibrated for. The relationship path appends the remedy once per bulk load, not once per failed pair. doctor prints the effective pool size next to the page-size line ('pool size 2048 MiB', with an '(×N page-size scaling)' suffix on non-4K hosts) so support triage sees the sizing inputs at a glance. Co-Authored-By: Claude Opus 4.8 (1M context) * refactor(lbug): re-anchor getEffectiveBufferPoolSize's placement and reuse granuleRatio in doctor Self-review fixes: the getter's insertion had orphaned resolveBufferManagerSize's doc comment (it read as documenting the wrong function), and doctor's scale note duplicated the granule math with a hardcoded 4096. granuleRatio is now exported (it already carried the test-seam default param) and doctor consumes it. No behavioral change — the sizing suite pins byte-identical outputs. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(lbug): keep the hintless pool default unscaled and make both remedies visible (#2631) Review fixes: - Scale only the analyze-path cap (scaledAnalyzePoolCap), not defaultBufferPoolSize: the pool is an eager native allocation at DB open (measured, see POOL_BYTES_PER_ELEMENT), so a page-size-scaled hintless default would hand a long-lived MCP process up to 80% of RAM — the #2557 OOM exposure the 2 GiB cap removed. Fix the MAP_NORESERVE claim that contradicted that measurement. - Log the rel-pair pool remedy (loadGraphToLbug returns warnings that no call site reads) and dedup it with a local boolean instead of matching the remedy's own wording. - Label the GITNEXUS_LBUG_BUFFER_POOL_SIZE=0 sentinel as the native 80%-of-RAM default in both the remedy and doctor instead of '0 MiB'. - Extract poolSizeDoctorLine (pageSizeDoctorLines convention): mark env overrides, drop the scaling suffix that misdescribed absolute values. - Fold _resetOsPageSizeCacheForTest into _setOsPageSizeForTests(undefined). - Document the analyze-path scaling in both README env tables. --------- Co-authored-by: Gergo Magyar Co-authored-by: Claude Opus 4.8 (1M context) --- README.md | 2 +- gitnexus/README.md | 2 +- gitnexus/src/cli/doctor.ts | 27 ++- gitnexus/src/core/lbug/lbug-adapter.ts | 22 ++- gitnexus/src/core/lbug/lbug-config.ts | 178 +++++++++++++++--- gitnexus/test/unit/doctor-format.test.ts | 23 +++ .../test/unit/lbug-config-pagesize.test.ts | 6 +- gitnexus/test/unit/lbug-config-wal.test.ts | 151 ++++++++++++++- 8 files changed, 381 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 495483cf8..a1eef4304 100644 --- a/README.md +++ b/README.md @@ -491,7 +491,7 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_WORKER_READY_TIMEOUT_MS` | `5000` | Startup budget in milliseconds for a parse worker to load its grammar bindings and report `{type:'ready'}`. Slots that miss it are treated as startup crashes. | Slow or heavily loaded hosts where a full pool cold-starting concurrently needs more than 5s, and analyze aborts with "did not report ready within 5000ms". | | `GITNEXUS_FTS_STEMMER` | `porter` | Stemmer used when rebuilding BM25/FTS indexes. Use `none` for CJK-heavy repositories, or a language stemmer such as `german`, `french`, or `spanish` for matching repository comments. Re-run `gitnexus analyze --repair-fts` after changing it. | Keyword search quality is poor for non-English comments or identifiers under English stemming. | | `GITNEXUS_WAL_CHECKPOINT_THRESHOLD` | `67108864` (64 MiB) | LadybugDB WAL auto-checkpoint threshold in bytes. Equivalent to `--wal-checkpoint-threshold `. `-1` keeps LadybugDB's stock threshold (~16 MiB). Larger thresholds reduce checkpoint frequency but increase the WAL size at rotation time — choose a smaller value on disk-constrained environments. | You need a larger or smaller WAL auto-checkpoint threshold for your analyze workload. | -| `GITNEXUS_LBUG_BUFFER_POOL_SIZE` | min(2 GiB, 80% RAM) | LadybugDB buffer-pool ceiling in bytes for every GitNexus database (analyze, MCP server, serve, group bridges). `0` restores LadybugDB's native unbounded default of 80% of system RAM; invalid values warn and fall back to the default (#2557). | A long-lived `gitnexus mcp` or a big incremental `analyze` uses too much memory, or a huge repo's working set genuinely needs a pool larger than 2 GiB. | +| `GITNEXUS_LBUG_BUFFER_POOL_SIZE` | min(2 GiB, 80% RAM) | LadybugDB buffer-pool ceiling in bytes for every GitNexus database (analyze, MCP server, serve, group bridges). `0` restores LadybugDB's native unbounded default of 80% of system RAM; invalid values warn and fall back to the default (#2557). During `analyze` the pool is right-sized to the graph, scaled on non-4 KiB-page hosts by the page-size granule ratio up to min(2 GiB × pageSize/4 KiB, 80% RAM) (#2631); this env var overrides all of that as an absolute value. | A long-lived `gitnexus mcp` or a big incremental `analyze` uses too much memory, or a huge repo's working set genuinely needs a pool larger than 2 GiB. | | `GITNEXUS_LBUG_MAX_DB_SIZE` | `17179869184` (16 GiB) | Maximum size in bytes of a single LadybugDB database file — an mmap/disk-address-space ceiling, not a memory limit (it does not constrain the buffer pool). Invalid values silently fall back to the default. | Indexing a genuinely huge monorepo whose on-disk graph index approaches 16 GiB. | | `GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES` | `8388608` (8 MB) | Per-job byte budget the pool will send to a worker in one `postMessage`. | Very large individual files; mostly diagnostic — bumping past 8 MB risks structured-clone memory pressure. | | `GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT` | `3` | Max replacement spawns per worker slot before the slot is dropped from the active rotation. Bounds respawn loops on a chronically-crashing slot. | Hosts where a flaky worker should retry more (raise) or fail-fast (lower) before the slot is dropped. | diff --git a/gitnexus/README.md b/gitnexus/README.md index edee4da57..6e92c69d8 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -484,7 +484,7 @@ Configure the behavior with these environment variables: | `GITNEXUS_FTS_CJK_SEGMENTATION` | `none`, `bigram` | `none` | `bigram` inserts overlapping character-bigram boundaries into Chinese/Japanese Han-ideograph spans in `content`/`description` before FTS indexing, so LadybugDB's space-only tokenizer can see sub-phrase word boundaries. Scoped to CJK Unified Ideographs only — Japanese Hiragana/Katakana and Korean Hangul are not currently segmented. Unlike `GITNEXUS_FTS_STEMMER`, this rewrites stored text — enabling it on an already-indexed repo requires a full `gitnexus analyze --force`; neither `--repair-fts` nor a plain incremental `analyze` applies it to previously-indexed files. Set the same value wherever `analyze` and search-serving processes (CLI query, MCP server, web server) run. | | `GITNEXUS_COMMUNITY_ENGINE` | `graphology`, `icebug`, `auto` | `graphology` | Community-detection engine used during analyze. `graphology` uses the bundled default path. `icebug` and `auto` currently behave identically: both try the experimental Icebug CSR path and fall back to Graphology if the optional native module is unavailable or incompatible. | | `GITNEXUS_WAL_CHECKPOINT_THRESHOLD` | integer `>= -1` | `67108864` (64 MiB) | LadybugDB WAL auto-checkpoint threshold during analyze (bytes). Auto-checkpoint remains enabled; `-1` keeps Ladybug's stock ~16 MiB. Larger thresholds reduce checkpoint frequency but increase the WAL size at rotation time — choose a smaller value on disk-constrained environments. | -| `GITNEXUS_LBUG_BUFFER_POOL_SIZE` | integer `>= 0` (bytes) | min(2 GiB, 80% RAM) | LadybugDB buffer-pool ceiling for every GitNexus database (analyze, MCP server, serve, group bridges). Bounded so a long-lived `gitnexus mcp` process or a large incremental `analyze` cannot grow toward LadybugDB's native 80%-of-RAM default and OOM the host (#2557). `0` restores that native unbounded default; invalid values warn and fall back to the default. | +| `GITNEXUS_LBUG_BUFFER_POOL_SIZE` | integer `>= 0` (bytes) | min(2 GiB, 80% RAM) | LadybugDB buffer-pool ceiling for every GitNexus database (analyze, MCP server, serve, group bridges). Bounded so a long-lived `gitnexus mcp` process or a large incremental `analyze` cannot grow toward LadybugDB's native 80%-of-RAM default and OOM the host (#2557). `0` restores that native unbounded default; invalid values warn and fall back to the default. During `analyze` the pool is right-sized to the graph and, on non-4 KiB-page hosts (Apple Silicon 16 KiB, Ascend/aarch64 64 KiB), scaled by the page-size granule ratio up to min(2 GiB × pageSize/4 KiB, 80% RAM) (#2631); this env var overrides all of that as an absolute value. | | `GITNEXUS_LBUG_MAX_DB_SIZE` | positive integer (bytes) | `17179869184` (16 GiB) | Upper bound for a single LadybugDB database file. This is an mmap/disk-address-space ceiling, not a memory limit — it does not constrain the buffer pool (use `GITNEXUS_LBUG_BUFFER_POOL_SIZE` for that). Raise it when indexing genuinely huge monorepos; invalid values silently fall back to the default. | ```bash diff --git a/gitnexus/src/cli/doctor.ts b/gitnexus/src/cli/doctor.ts index 7ec8f30f5..6f7fe40c9 100644 --- a/gitnexus/src/cli/doctor.ts +++ b/gitnexus/src/cli/doctor.ts @@ -17,7 +17,11 @@ import { probeFtsExtensionLoad, probeVectorExtensionLoad, } from '../core/lbug/native-check.js'; -import { getOsPageSize, isPageSizeAwareLadybug } from '../core/lbug/lbug-config.js'; +import { + getEffectiveBufferPoolSize, + getOsPageSize, + isPageSizeAwareLadybug, +} from '../core/lbug/lbug-config.js'; import { diagnoseExtensionLoad } from '../core/lbug/extension-load-error.js'; import { getExtensionInstallPolicy } from '../core/lbug/extension-loader.js'; import { t } from './i18n/index.js'; @@ -150,6 +154,22 @@ export function pageSizeDoctorLines( return lines; } +/** + * The hintless buffer-pool doctor line (#2631) — the pool the next Database + * open in THIS process would get. Same plain-params testable-helper shape as + * pageSizeDoctorLines above. `pool` is getEffectiveBufferPoolSize(): `0` is + * the pass-through sentinel for LadybugDB's native 80%-of-RAM default, never + * printed as "0 MiB". `envRaw` (the raw GITNEXUS_LBUG_BUFFER_POOL_SIZE value) + * marks operator-supplied absolute values as "(env override)" — no scaling + * suffix: the hintless default is deliberately unscaled (#2557), and an env + * value is absolute, so a "×N" note would misdescribe both. + */ +export function poolSizeDoctorLine(pool: number, envRaw: string | undefined): string { + const value = pool === 0 ? 'native 80% of RAM' : `${Math.round(pool / (1024 * 1024))} MiB`; + const envNote = envRaw !== undefined && envRaw.trim().length > 0 ? ' (env override)' : ''; + return ` ${padDisplayEnd('pool size', 10)}${value}${envNote}`; +} + export const doctorCommand = async () => { const fingerprint = getRuntimeFingerprint(); const capabilities = getRuntimeCapabilities(); @@ -168,6 +188,11 @@ export const doctorCommand = async () => { for (const line of pageSizeDoctorLines(getOsPageSize(), fingerprint.ladybugdb)) { console.log(line); } + // Hintless buffer pool for the next DB open (#2631). Literal label like + // the page size line above (no i18n key). + console.log( + poolSizeDoctorLine(getEffectiveBufferPoolSize(), process.env.GITNEXUS_LBUG_BUFFER_POOL_SIZE), + ); const nativeCheck = checkLbugNative(); if (nativeCheck.ok) { console.log(` ${padDisplayEnd('native', 10)}✓ lbugjs.node loaded`); diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 2319507ea..7a153a7de 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -36,6 +36,7 @@ import { isDbBusyError, isOpenRetryExhausted, isWalCorruptionError, + bufferPoolExhaustionRemedy, openLbugConnection, sleep, toNativeSafePath, @@ -952,7 +953,14 @@ const copyNodeCSVs = async ( const copyQuery = getCopyQuery(table, normalizeCopyPath(csvPath)); await copyCsvWithRetry(targetConn, copyQuery, (retryErr) => { const retryMsg = retryErr instanceof Error ? retryErr.message : String(retryErr); - throw new Error(`COPY failed for ${table}: ${retryMsg.slice(0, 200)}`); + // Pool exhaustion gets a remedy (#2631): the raw binder text gives the + // operator nothing to act on, and on non-4K-page hosts (Ascend aarch64, + // Apple Silicon) the pool bills up to pageSize/4KiB x faster than the + // sizing was calibrated for — name the knob and the mechanism. + const remedy = bufferPoolExhaustionRemedy(retryMsg); + throw new Error( + `COPY failed for ${table}: ${retryMsg.slice(0, 200)}${remedy ? ` ${remedy}` : ''}`, + ); }); } }; @@ -1124,6 +1132,7 @@ export const loadGraphToLbug = async ( const insertedRels = totalValidRels; const warnings: string[] = []; + let poolRemedyIssued = false; if (insertedRels > 0) { log(`Loading edges: ${insertedRels.toLocaleString()} across ${relsByPair.size} types`); @@ -1150,6 +1159,17 @@ export const loadGraphToLbug = async ( await copyCsvWithRetry(writeConn, copyQuery, (retryErr) => { const retryMsg = retryErr instanceof Error ? retryErr.message : String(retryErr); warnings.push(`${fromLabel}->${toLabel} (${rows} edges): ${retryMsg.slice(0, 80)}`); + // One remedy per bulk load, not per pair (#2631): pool exhaustion + // repeats for every remaining pair once it starts. logger.warn, not + // just warnings.push — the returned warnings array has no consumer at + // any call site, so a push alone would leave the remedy invisible + // while the row-by-row fallback quietly degrades the load. + const remedy = poolRemedyIssued ? undefined : bufferPoolExhaustionRemedy(retryMsg); + if (remedy) { + poolRemedyIssued = true; + warnings.push(remedy); + logger.warn(remedy); + } failedPairEdges += rows; failedPairCsvPaths.add(pairCsvPath); }); diff --git a/gitnexus/src/core/lbug/lbug-config.ts b/gitnexus/src/core/lbug/lbug-config.ts index d5de2161e..cc88878a5 100644 --- a/gitnexus/src/core/lbug/lbug-config.ts +++ b/gitnexus/src/core/lbug/lbug-config.ts @@ -340,18 +340,84 @@ const parseBufferPoolSize = (raw: string | undefined): number | undefined => { return Math.floor(parsed); }; +/** + * The buffer-manager frame size compiled into every shipped `@ladybugdb/core` + * binary (`LBUG_PAGE_SIZE_LOG2 = 12` in the engine's CMake) — frames are 4 KiB + * on every platform, independent of the OS page size. + */ +const LBUG_ASSUMED_FRAME_SIZE = 4096; + +/** + * How much the OS page size amplifies buffer-pool consumption (#2631). + * + * LadybugDB's VM region charges pool budget per DISCARD GRANULE, not per + * frame: `discardGranuleSize = max(frameSize, osPageSize)` (vm_region.cpp), + * `claimFrame` bills the whole granule when its first 4 KiB frame becomes + * resident, and `releaseFrame` refunds only when the granule's LAST frame + * leaves. On a 64 KiB-page kernel (aarch64 openEuler — Ascend hosts) that is + * 16 frames per granule: scattered access is billed up to 16× its real bytes, + * and whole eviction passes can evict frames yet refund nothing — which is + * exactly the engine's "buffer pool is full and no memory could be freed" + * throw. Apple Silicon macOS (16 KiB pages) is the same mechanism at 4×. + * + * So the ANALYZE-path pool sizes (the per-element estimate, the COPY-safety + * floor, and the cap the hint is clamped against) are scaled by this ratio: + * the budget must cover worst-case granule charging or COPY dies on non-4K + * hosts with a pool that would be ample on x86. The hintless default + * (defaultBufferPoolSize — MCP serve, doctor, native-check) is deliberately + * NOT scaled: the pool is a native eager allocation committed at DB open + * (measured — see POOL_BYTES_PER_ELEMENT below), so scaling the global + * default would revert the #2557 OOM cap on every 16 KiB/64 KiB host. If the + * engine ever charges per-frame (or ships page-size-matched frames), this + * collapses back to 1 and the scaling disappears. + * + * Fail-safe: an undetectable page size (win32 — where the granule mechanism + * is absent anyway — or a failed `getconf`) means ratio 1, i.e. today's + * behavior. + */ +export const granuleRatio = (pageSize: number | undefined = getOsPageSize()): number => { + if (pageSize === undefined || !Number.isFinite(pageSize)) return 1; + return Math.max(1, Math.floor(pageSize / LBUG_ASSUMED_FRAME_SIZE)); +}; + +/** + * Hintless pool default — MCP serve, doctor, native-check, any open without a + * per-run hint. Deliberately UNSCALED (#2557): the pool is an eager native + * allocation at DB open, so a page-size-scaled default would hand a + * long-lived `gitnexus mcp` on a 16 KiB/64 KiB host up to 80% of RAM — the + * exact OOM exposure the 2 GiB cap was added to remove. + */ const defaultBufferPoolSize = (): number => Math.min(DEFAULT_BUFFER_POOL_CAP, Math.max(BUFFER_POOL_FLOOR, Math.floor(os.totalmem() * 0.8))); /** - * Clamp an adaptive pool request to [ADAPTIVE_POOL_FLOOR, default]. The lower - * bound keeps LadybugDB's COPY viable; the upper bound (defaultBufferPoolSize) - * means the hint can only shrink the pool from today's default and can never - * exceed the 2 GiB / 80%-RAM cap — and on a machine whose default is below the - * COPY floor, the default wins, so the pool is never over-committed. + * Upper bound for the ANALYZE-path (hinted) pool: the #2557 cap scaled by the + * granule ratio, still bounded by 80% of RAM. Scaling only this bound — and + * not defaultBufferPoolSize — is what lets the #2631 fix take effect during + * the bulk COPY without touching hintless opens: with an unscaled cap the + * min() below would clamp the scaled COPY floor straight back to 2 GiB. */ -const clampBufferPool = (bytes: number): number => - Math.min(defaultBufferPoolSize(), Math.max(ADAPTIVE_POOL_FLOOR, Math.floor(bytes))); +const scaledAnalyzePoolCap = (pageSize: number | undefined): number => + Math.min( + DEFAULT_BUFFER_POOL_CAP * granuleRatio(pageSize), + Math.max(BUFFER_POOL_FLOOR, Math.floor(os.totalmem() * 0.8)), + ); + +/** + * Clamp an adaptive pool request to [ADAPTIVE_POOL_FLOOR × granuleRatio, + * scaledAnalyzePoolCap]. The lower bound keeps LadybugDB's COPY viable + * (scaled because the granule accounting inflates consumption on non-4K + * hosts, see granuleRatio); the upper bound means the hint can never exceed + * the page-size-scaled #2557 cap or 80% of RAM — and on a machine whose cap + * is below the COPY floor, the cap wins, so the pool is never over-committed. + * On 4 KiB hosts (ratio 1) this is byte-identical to clamping against the + * hintless default. + */ +const clampBufferPool = (bytes: number, pageSize: number | undefined = getOsPageSize()): number => + Math.min( + scaledAnalyzePoolCap(pageSize), + Math.max(ADAPTIVE_POOL_FLOOR * granuleRatio(pageSize), Math.floor(bytes)), + ); /** * Buffer-pool bytes to provision per graph element (node + relationship). @@ -372,13 +438,22 @@ const POOL_BYTES_PER_ELEMENT = 4 * 1024; /** * Size the buffer pool to an estimated graph size (node + relationship count), - * clamped to [ADAPTIVE_POOL_FLOOR, defaultBufferPoolSize()]. The estimate can - * only *shrink* the pool from the default — never above the 2 GiB / 80%-RAM cap, - * never below the COPY-safety floor — so no repo is under-sized or gets more - * than the default it would have today. + * clamped to [ADAPTIVE_POOL_FLOOR, scaledAnalyzePoolCap], with every term + * scaled by granuleRatio (#2631): on non-4K hosts the engine bills pool + * budget per OS-page-sized granule, so the same graph consumes up to + * pageSize/4096 × the budget it needs on x86. On 4 KiB hosts the ratio is 1 + * and this is byte-identical to the pre-#2631 behavior. The estimate is never + * above the page-size-scaled #2557 cap bounded by 80% of RAM, never below the + * scaled COPY-safety floor; the hintless default stays unscaled. + * + * `pageSize` is a test seam (the pageSizeDoctorLines convention); production + * callers omit it and get the memoized real OS page size. */ -export const estimateBufferPool = (graphElementCount: number): number => - clampBufferPool(graphElementCount * POOL_BYTES_PER_ELEMENT); +export const estimateBufferPool = ( + graphElementCount: number, + pageSize: number | undefined = getOsPageSize(), +): number => + clampBufferPool(graphElementCount * POOL_BYTES_PER_ELEMENT * granuleRatio(pageSize), pageSize); /** * Optional per-run buffer-pool size hint (bytes). The analyze orchestrator sets @@ -417,12 +492,64 @@ const resolveBufferManagerSize = (): number => { if (raw.trim().length > 0) { logger.warn( { rawValue: raw, fallback: defaultBufferPoolSize() }, - `Ignoring invalid GITNEXUS_LBUG_BUFFER_POOL_SIZE=${raw}; expected integer >= 0 (bytes; 0 restores the native 80%-of-RAM default); falling back to min(2 GiB, 80% of RAM).`, + `Ignoring invalid GITNEXUS_LBUG_BUFFER_POOL_SIZE=${raw}; expected integer >= 0 (bytes; 0 restores the native 80%-of-RAM default); falling back to the platform default pool size.`, ); } return defaultBufferPoolSize(); }; +/** + * Doctor-facing view of the pool size the next Database open would get + * (#2631): env override > clamped hint > unscaled hintless default. Read-only; + * doctor prints it next to the page-size lines so support triage sees the + * sizing inputs at a glance. `0` is the pass-through sentinel for LadybugDB's + * native 80%-of-RAM default — callers must label it, not print "0 MiB". + */ +export const getEffectiveBufferPoolSize = (): number => resolveBufferManagerSize(); + +/** + * Matches the engine's buffer-pool exhaustion throw (buffer_manager.cpp: + * "Unable to allocate memory! The buffer pool is full and no memory could be + * freed!"). Distinct from isLbugPageSizeFrameError above, which matches the + * madvise/frame-release failure class. + */ +const BUFFER_POOL_EXHAUSTION_RE = /buffer pool is full|unable to allocate memory/i; + +const formatMiB = (bytes: number): string => `${Math.round(bytes / (1024 * 1024))} MiB`; + +/** + * Actionable remedy for a buffer-pool exhaustion error (#2631), or undefined + * when `message` is not that class. Cause → consequence → remedy, the + * diagnoseExtensionLoad convention: names the effective pool, the override + * knob, and — on non-4K hosts — the granule amplification that makes the + * budget exhaust early (the reporter's Ascend/aarch64 64 KiB kernel billed a + * pool up to 16× faster than the same analyze on x86). + */ +export const bufferPoolExhaustionRemedy = ( + message: string, + pageSize: number | undefined = getOsPageSize(), +): string | undefined => { + if (!BUFFER_POOL_EXHAUSTION_RE.test(message)) return undefined; + const ratio = granuleRatio(pageSize); + const pool = resolveBufferManagerSize(); + // 0 is the pass-through sentinel (GITNEXUS_LBUG_BUFFER_POOL_SIZE=0 → + // LadybugDB's native 80%-of-RAM default) — "0 MiB" would be nonsense in the + // very triage text this remedy exists to provide. + const poolLabel = pool === 0 ? "LadybugDB's native 80%-of-RAM default" : formatMiB(pool); + const pageNote = + ratio > 1 + ? ` This host's ${(pageSize ?? 0) / 1024} KiB OS page size makes the engine bill pool ` + + `memory in ${(pageSize ?? 0) / 1024} KiB granules — up to ${ratio}× faster budget use ` + + `than a 4 KiB-page host running the same analyze.` + : ''; + return ( + `The LadybugDB buffer pool (${poolLabel}) was exhausted during the bulk COPY.` + + pageNote + + ` Set GITNEXUS_LBUG_BUFFER_POOL_SIZE= to raise it (e.g. ${4 * 1024 * 1024 * 1024}` + + ` for 4 GiB); 0 restores LadybugDB's native 80%-of-RAM default.` + ); +}; + /** 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; @@ -509,8 +636,12 @@ const LBUG_PAGE_COMBO_RE = /unsupported page size combination/i; * True when `err` looks like the LadybugDB buffer manager failing to release * frame memory — the failure mode of a 4 KiB page-size assumption on a * 16 KiB/64 KiB-page kernel (#1231). Deliberately does NOT match the - * generic "buffer pool is full" exhaustion error, which is a sizing - * problem, not a page-size one. + * generic "buffer pool is full" exhaustion error: that one is handled as a + * SIZING problem — though since #2631 we know page size drives sizing too + * (the engine bills pool budget per OS-page-sized discard granule, so non-4K + * hosts exhaust the same budget up to pageSize/4096× earlier; see + * granuleRatio, which scales the pool accordingly, and + * bufferPoolExhaustionRemedy, which explains it to the operator). */ export const isLbugPageSizeFrameError = (err: unknown): boolean => { if (!err) return false; @@ -537,6 +668,16 @@ export const isPageSizeAwareLadybug = (version: string | undefined): boolean => // because analyze error paths and doctor may both ask, and getconf forks. let cachedOsPageSize: number | null | undefined; +/** + * Test seam (the `_captureLogger` convention): pin the memoized OS page size + * so sizing tests are host-independent — without this they would silently + * drift on 16 KiB-page Apple Silicon runners. `number` pins a value, `null` + * pins "undetectable", `undefined` clears the memo so the next call re-probes. + */ +export const _setOsPageSizeForTests = (pageSize: number | null | undefined): void => { + cachedOsPageSize = pageSize; +}; + /** * OS memory page size in bytes, or `undefined` when it cannot be determined * (Windows, missing getconf, sandboxed exec). Node exposes no page-size API, @@ -575,11 +716,6 @@ export const getOsPageSize = (): number | undefined => { return cachedOsPageSize ?? undefined; }; -/** Exported only for unit tests — clears the getconf probe cache. */ -export const _resetOsPageSizeCacheForTest = (): void => { - cachedOsPageSize = undefined; -}; - type LbugModule = typeof lbug; export interface LbugDatabaseOptions { diff --git a/gitnexus/test/unit/doctor-format.test.ts b/gitnexus/test/unit/doctor-format.test.ts index 2d06f5124..416544ed8 100644 --- a/gitnexus/test/unit/doctor-format.test.ts +++ b/gitnexus/test/unit/doctor-format.test.ts @@ -5,6 +5,7 @@ import { localEmbeddingDoctorStatus, padDisplayEnd, pageSizeDoctorLines, + poolSizeDoctorLine, } from '../../src/cli/doctor.js'; describe('doctor output formatting', () => { @@ -164,6 +165,28 @@ describe('doctor page-size lines (#1231, #2424 review)', () => { }); }); +describe('doctor pool-size line (#2631)', () => { + const MiB = 1024 * 1024; + + it('prints the hintless pool in MiB with no env note when the env var is unset', () => { + expect(poolSizeDoctorLine(2048 * MiB, undefined)).toBe( + ` ${padDisplayEnd('pool size', 10)}2048 MiB`, + ); + }); + + it('marks an operator-supplied absolute value as an env override, with no scaling suffix', () => { + expect(poolSizeDoctorLine(4096 * MiB, String(4096 * MiB))).toBe( + ` ${padDisplayEnd('pool size', 10)}4096 MiB (env override)`, + ); + }); + + it('labels the 0 sentinel as the native default instead of "0 MiB"', () => { + expect(poolSizeDoctorLine(0, '0')).toBe( + ` ${padDisplayEnd('pool size', 10)}native 80% of RAM (env override)`, + ); + }); +}); + describe('doctor survives a malformed GITNEXUS_EMBEDDING_DIMS (#2385)', () => { const ENV_KEYS = [ 'GITNEXUS_EMBEDDING_URL', diff --git a/gitnexus/test/unit/lbug-config-pagesize.test.ts b/gitnexus/test/unit/lbug-config-pagesize.test.ts index f2108d5f8..0acd9e990 100644 --- a/gitnexus/test/unit/lbug-config-pagesize.test.ts +++ b/gitnexus/test/unit/lbug-config-pagesize.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { execFileSync } from 'child_process'; import { - _resetOsPageSizeCacheForTest, + _setOsPageSizeForTests, getOsPageSize, isLbugPageSizeFrameError, isPageSizeAwareLadybug, @@ -92,7 +92,7 @@ describe('isPageSizeAwareLadybug', () => { describe('getOsPageSize', () => { afterEach(() => { - _resetOsPageSizeCacheForTest(); + _setOsPageSizeForTests(undefined); execFileSyncSpy.mockClear(); }); @@ -155,7 +155,7 @@ describe('getOsPageSize', () => { it.skipIf(onWindows)('probes at most once per process (cached)', () => { expect(getOsPageSize()).toBe(getOsPageSize()); expect(execFileSyncSpy).toHaveBeenCalledTimes(1); - _resetOsPageSizeCacheForTest(); + _setOsPageSizeForTests(undefined); getOsPageSize(); expect(execFileSyncSpy).toHaveBeenCalledTimes(2); }); diff --git a/gitnexus/test/unit/lbug-config-wal.test.ts b/gitnexus/test/unit/lbug-config-wal.test.ts index 94395541a..1150a0da7 100644 --- a/gitnexus/test/unit/lbug-config-wal.test.ts +++ b/gitnexus/test/unit/lbug-config-wal.test.ts @@ -1,11 +1,13 @@ import os from 'os'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createLbugDatabase, estimateBufferPool, isLbugCheckpointIoError, isWalCorruptionError, setBufferPoolSizeHint, + _setOsPageSizeForTests, + bufferPoolExhaustionRemedy, } from '../../src/core/lbug/lbug-config.js'; import { _captureLogger } from '../../src/core/logger.js'; @@ -166,6 +168,12 @@ describe('createLbugDatabase WAL replay option', () => { describe('createLbugDatabase buffer pool size (#2557)', () => { const GiB = 1024 * 1024 * 1024; + // Pin a 4 KiB page so every expectation below is host-independent — on a + // 16 KiB-page Apple Silicon runner the #2631 granule scaling would + // otherwise multiply them by 4. + beforeEach(() => _setOsPageSizeForTests(4096)); + afterEach(() => _setOsPageSizeForTests(undefined)); + const bufferPoolArg = (Database: ReturnType): unknown => Database.mock.calls[0][1]; it.each([ @@ -259,7 +267,11 @@ describe('adaptive buffer pool hint', () => { const MiB = 1024 * 1024; const bufferPoolArg = (Database: ReturnType): unknown => Database.mock.calls[0][1]; - afterEach(() => setBufferPoolSizeHint(undefined)); + beforeEach(() => _setOsPageSizeForTests(4096)); + afterEach(() => { + setBufferPoolSizeHint(undefined); + _setOsPageSizeForTests(undefined); + }); describe('estimateBufferPool', () => { it.each([ @@ -355,3 +367,138 @@ describe('isLbugCheckpointIoError', () => { expect(isLbugCheckpointIoError(undefined)).toBe(false); }); }); + +// ─── #2631: page-size-scaled pool sizing (granule accounting) ─────────────── +describe('page-size-scaled buffer pool sizing (#2631)', () => { + const MiB = 1024 * 1024; + const GiB = 1024 * MiB; + const bufferPoolArg = (Database: ReturnType): unknown => Database.mock.calls[0][1]; + + afterEach(() => { + setBufferPoolSizeHint(undefined); + _setOsPageSizeForTests(undefined); + vi.unstubAllEnvs(); + }); + + it.each([ + ['64 KiB pages scale the floor ×16', 65536, 41, 16 * 256 * MiB], + [ + '64 KiB pages scale the estimate ×16 (100k × 4 KiB × 16 = 6.4 GB)', + 65536, + 100_000, + 100_000 * 4 * 1024 * 16, + ], + ['16 KiB pages (Apple Silicon) scale the floor ×4', 16384, 41, 4 * 256 * MiB], + ['4 KiB pages are byte-identical to the unscaled behavior', 4096, 100_000, 100_000 * 4 * 1024], + ])('%s', (_label, pageSize, elements, expected) => { + const totalmemSpy = vi.spyOn(os, 'totalmem').mockReturnValue(32 * GiB); + try { + _setOsPageSizeForTests(pageSize); + expect(estimateBufferPool(elements)).toBe(expected); + } finally { + totalmemSpy.mockRestore(); + } + }); + + it('the scaled cap is still bounded by 80% of RAM (64 KiB pages, huge graph)', () => { + const totalmemSpy = vi.spyOn(os, 'totalmem').mockReturnValue(32 * GiB); + try { + _setOsPageSizeForTests(65536); + // min(2 GiB × 16, 0.8 × 32 GiB) = min(32 GiB, 25.6 GiB) = 25.6 GiB + expect(estimateBufferPool(100_000_000)).toBe(Math.floor(0.8 * 32 * GiB)); + } finally { + totalmemSpy.mockRestore(); + } + }); + + it('an undetectable page size behaves exactly like 4 KiB (ratio 1)', () => { + const totalmemSpy = vi.spyOn(os, 'totalmem').mockReturnValue(32 * GiB); + try { + _setOsPageSizeForTests(null); + expect(estimateBufferPool(100_000)).toBe(100_000 * 4 * 1024); + } finally { + totalmemSpy.mockRestore(); + } + }); + + it('the hintless default passed to the Database ctor stays at the unscaled #2557 cap on 64 KiB hosts', () => { + // The guard for the #2557 OOM protection: MCP serve / doctor / any open + // without a per-run hint must NOT inherit the page-size-scaled budget — + // the pool is an eager allocation at DB open. + const totalmemSpy = vi.spyOn(os, 'totalmem').mockReturnValue(32 * GiB); + try { + _setOsPageSizeForTests(65536); + const Database = vi.fn(function (this: any) {}); + createLbugDatabase({ Database } as any, '/tmp/lbug-pool-64k'); + expect(bufferPoolArg(Database)).toBe(2 * GiB); + } finally { + totalmemSpy.mockRestore(); + } + }); + + it('the analyze hint path DOES scale on 64 KiB hosts (scaled floor, bounded by 80% RAM)', () => { + const totalmemSpy = vi.spyOn(os, 'totalmem').mockReturnValue(32 * GiB); + try { + _setOsPageSizeForTests(65536); + setBufferPoolSizeHint(estimateBufferPool(41)); + const Database = vi.fn(function (this: any) {}); + createLbugDatabase({ Database } as any, '/tmp/lbug-pool-64k-hint'); + // 41 elements → below the scaled COPY floor → 16 × 256 MiB = 4 GiB + expect(bufferPoolArg(Database)).toBe(16 * 256 * MiB); + } finally { + totalmemSpy.mockRestore(); + } + }); + + it('GITNEXUS_LBUG_BUFFER_POOL_SIZE stays absolute on 64 KiB hosts (incl. 0 = native default)', () => { + _setOsPageSizeForTests(65536); + vi.stubEnv('GITNEXUS_LBUG_BUFFER_POOL_SIZE', String(512 * MiB)); + const Database = vi.fn(function (this: any) {}); + createLbugDatabase({ Database } as any, '/tmp/lbug-pool-64k-env'); + expect(bufferPoolArg(Database)).toBe(512 * MiB); + }); +}); + +// ─── #2631: actionable pool-exhaustion remedy ─────────────────────────────── +describe('bufferPoolExhaustionRemedy (#2631)', () => { + afterEach(() => _setOsPageSizeForTests(undefined)); + + const EXHAUSTION = + 'Buffer manager exception: Unable to allocate memory! The buffer pool is full and no memory could be freed!'; + + it('names the override knob for the exhaustion error', () => { + _setOsPageSizeForTests(4096); + const remedy = bufferPoolExhaustionRemedy(EXHAUSTION); + expect(remedy).toContain('GITNEXUS_LBUG_BUFFER_POOL_SIZE'); + expect(remedy).toContain('buffer pool'); + // ratio 1 → no page-size amplification note + expect(remedy).not.toContain('OS page size'); + }); + + it('explains the granule amplification on a 64 KiB-page host', () => { + _setOsPageSizeForTests(65536); + const remedy = bufferPoolExhaustionRemedy(EXHAUSTION); + expect(remedy).toContain('64 KiB OS page size'); + expect(remedy).toContain('16×'); + expect(remedy).toContain('GITNEXUS_LBUG_BUFFER_POOL_SIZE'); + }); + + it('is silent for non-exhaustion errors', () => { + _setOsPageSizeForTests(65536); + expect( + bufferPoolExhaustionRemedy('Binder exception: Table CodeEmbedding does not exist.'), + ).toBeUndefined(); + }); + + it('labels the 0 sentinel as the native default instead of "0 MiB"', () => { + _setOsPageSizeForTests(4096); + vi.stubEnv('GITNEXUS_LBUG_BUFFER_POOL_SIZE', '0'); + try { + const remedy = bufferPoolExhaustionRemedy(EXHAUSTION); + expect(remedy).toContain('native 80%-of-RAM default'); + expect(remedy).not.toContain('(0 MiB)'); + } finally { + vi.unstubAllEnvs(); + } + }); +}); From a3396ef4ecffb911353f7b09d2fb0e421c8842e2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:14:16 +0000 Subject: [PATCH 09/14] chore(deps)(deps-dev): bump vite from 8.1.4 to 8.1.5 in /gitnexus-web Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 8.1.4 to 8.1.5. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v8.1.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 8.1.5 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- gitnexus-web/package-lock.json | 26 +++++++++++++------------- gitnexus-web/package.json | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index e5fda5365..71ff2d424 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -63,7 +63,7 @@ "jsdom": "^29.1.1", "tree-sitter-wasms": "^0.1.13", "typescript": "^5.4.5", - "vite": "^8.1.4", + "vite": "^8.1.5", "vitest": "^4.1.10", "wait-on": "^9.0.10" }, @@ -6826,9 +6826,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.16", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", + "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", "funding": [ { "type": "github", @@ -7216,9 +7216,9 @@ } }, "node_modules/postcss": { - "version": "8.5.16", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.16.tgz", - "integrity": "sha512-vuwillviilfKZsg0VGj5R/YwwcHx4SLsIOI/7K6mQkWx+l5cUHTjj5g0AasTBcyXsbfTgrwsUNmVUb5xVwyPwg==", + "version": "8.5.22", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.22.tgz", + "integrity": "sha512-KBDEIpLrvpv16pp3K0Fw+UCoZfopFjjgeB+0tA/aaThfEE74kKDLrgg603YvOWJyg3+WYtyq3xYsQWsIyZlPqQ==", "funding": [ { "type": "opencollective", @@ -7235,7 +7235,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, @@ -8296,15 +8296,15 @@ } }, "node_modules/vite": { - "version": "8.1.4", - "resolved": "https://registry.npmjs.org/vite/-/vite-8.1.4.tgz", - "integrity": "sha512-bTT9PsdWO+MQMNG9ZXIP/qM9wGh37DFxTV/sPq9cFpHr3w4jkgef032PkAL9jAqhk3Nz8NQw3O8n6/xFkqO4QQ==", + "version": "8.1.5", + "resolved": "https://registry.npmjs.org/vite/-/vite-8.1.5.tgz", + "integrity": "sha512-7ULLwsCdYx/nRyrpiEwvqb5TFHrMVZyBt+rg/OAXT7rgj/z+DtTDyKFeLAdDkubDVDKD8jOsndmy7m55XcfUsw==", "license": "MIT", "dependencies": { "lightningcss": "^1.32.0", "picomatch": "^4.0.5", - "postcss": "^8.5.16", - "rolldown": "~1.1.4", + "postcss": "^8.5.17", + "rolldown": "~1.1.5", "tinyglobby": "^0.2.17" }, "bin": { diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a9aa163da..8ef288f37 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -73,7 +73,7 @@ "jsdom": "^29.1.1", "tree-sitter-wasms": "^0.1.13", "typescript": "^5.4.5", - "vite": "^8.1.4", + "vite": "^8.1.5", "vitest": "^4.1.10", "wait-on": "^9.0.10" }, From b5d3f331ca61d3da43638da26ade15565878809e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:14:22 +0000 Subject: [PATCH 10/14] chore(deps)(deps): bump react-i18next in /gitnexus-web Bumps [react-i18next](https://github.com/i18next/react-i18next) from 17.0.8 to 17.0.10. - [Changelog](https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md) - [Commits](https://github.com/i18next/react-i18next/compare/v17.0.8...v17.0.10) --- updated-dependencies: - dependency-name: react-i18next dependency-version: 17.0.10 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- gitnexus-web/package-lock.json | 10 +++++----- gitnexus-web/package.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index e5fda5365..8c6df430d 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -36,7 +36,7 @@ "pandemonium": "^2.4.0", "react": "^19.2.5", "react-dom": "^19.2.7", - "react-i18next": "^17.0.8", + "react-i18next": "^17.0.10", "react-markdown": "^10.1.0", "react-syntax-highlighter": "^16.1.1", "react-zoom-pan-pinch": "^4.0.3", @@ -7356,9 +7356,9 @@ } }, "node_modules/react-i18next": { - "version": "17.0.8", - "resolved": "https://registry.npmjs.org/react-i18next/-/react-i18next-17.0.8.tgz", - "integrity": "sha512-0ooKbGLU8JXhe1zwpQUWIeXSgLPOfwJmgheWRIUpcoA0CpyabpGhayjdG+/eA5esC1AQ8h2jWpXjJfzQzeDOCw==", + "version": "17.0.10", + "resolved": "https://registry.npmjs.org/react-i18next/-/react-i18next-17.0.10.tgz", + "integrity": "sha512-XneHftyYA774MJkkccSkZ5oKrUpCnXIPmxio3wemqrVzCRLWiGXOMbIzObrer03fNDEnm8g8R5yYls4HcE+esg==", "license": "MIT", "dependencies": { "@babel/runtime": "^7.29.2", @@ -7368,7 +7368,7 @@ "peerDependencies": { "i18next": ">= 26.2.0", "react": ">= 16.8.0", - "typescript": "^5 || ^6" + "typescript": "^5 || ^6 || ^7" }, "peerDependenciesMeta": { "react-dom": { diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a9aa163da..2a608e99d 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -46,7 +46,7 @@ "pandemonium": "^2.4.0", "react": "^19.2.5", "react-dom": "^19.2.7", - "react-i18next": "^17.0.8", + "react-i18next": "^17.0.10", "react-markdown": "^10.1.0", "react-syntax-highlighter": "^16.1.1", "react-zoom-pan-pinch": "^4.0.3", From 575234977536c987ff985617a573c284f53261cc Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:14:28 +0000 Subject: [PATCH 11/14] chore(deps)(deps): bump lru-cache from 11.5.1 to 11.5.2 in /gitnexus-web Bumps [lru-cache](https://github.com/isaacs/node-lru-cache) from 11.5.1 to 11.5.2. - [Changelog](https://github.com/isaacs/node-lru-cache/blob/main/CHANGELOG.md) - [Commits](https://github.com/isaacs/node-lru-cache/compare/v11.5.1...v11.5.2) --- updated-dependencies: - dependency-name: lru-cache dependency-version: 11.5.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- gitnexus-web/package-lock.json | 8 ++++---- gitnexus-web/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index e5fda5365..3b8f0c76f 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -29,7 +29,7 @@ "i18next": "^26.3.0", "i18next-browser-languagedetector": "^8.2.1", "langchain": "^1.4.6", - "lru-cache": "^11.5.1", + "lru-cache": "^11.5.2", "lucide-react": "^1.23.0", "mermaid": "^11.15.0", "mnemonist": "^0.40.4", @@ -5672,9 +5672,9 @@ } }, "node_modules/lru-cache": { - "version": "11.5.1", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.5.1.tgz", - "integrity": "sha512-RPimw/7aMdv2oqRrxKwvZXcPfwBrn/JZ2xYcY9Hus/6LaS3VOAKVWKWgNLCFSiOm1ESXinjsDlidVU7JlnCN2A==", + "version": "11.5.2", + "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-11.5.2.tgz", + "integrity": "sha512-4pfM1Ff0x50o0tQwb5ucw/RzNyD0/YJME6IVcStalZuMWxdt3sR3huStTtxz4PUmvZfRguvDejasvQ2kifR11g==", "license": "BlueOak-1.0.0", "engines": { "node": "20 || >=22" diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a9aa163da..6f0377ac3 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -39,7 +39,7 @@ "i18next": "^26.3.0", "i18next-browser-languagedetector": "^8.2.1", "langchain": "^1.4.6", - "lru-cache": "^11.5.1", + "lru-cache": "^11.5.2", "lucide-react": "^1.23.0", "mermaid": "^11.15.0", "mnemonist": "^0.40.4", From 2ba64db022b11a254be21fb9a69e17ac5a1131ca Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:14:38 +0000 Subject: [PATCH 12/14] chore(deps)(deps): bump @langchain/langgraph in /gitnexus-web Bumps [@langchain/langgraph](https://github.com/langchain-ai/langgraphjs/tree/HEAD/libs/langgraph-core) from 1.4.7 to 1.4.8. - [Release notes](https://github.com/langchain-ai/langgraphjs/releases) - [Changelog](https://github.com/langchain-ai/langgraphjs/blob/main/libs/langgraph-core/CHANGELOG.md) - [Commits](https://github.com/langchain-ai/langgraphjs/commits/@langchain/langgraph@1.4.8/libs/langgraph-core) --- updated-dependencies: - dependency-name: "@langchain/langgraph" dependency-version: 1.4.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- gitnexus-web/package-lock.json | 22 +++++++++++----------- gitnexus-web/package.json | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index e5fda5365..a35e1f917 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -11,7 +11,7 @@ "@langchain/anthropic": "^1.5.1", "@langchain/core": "^1.2.2", "@langchain/google-genai": "^2.2.0", - "@langchain/langgraph": "^1.4.7", + "@langchain/langgraph": "^1.4.8", "@langchain/ollama": "^1.3.0", "@langchain/openai": "^1.5.3", "@sigma/edge-curve": "^3.1.0", @@ -1138,13 +1138,13 @@ } }, "node_modules/@langchain/langgraph": { - "version": "1.4.7", - "resolved": "https://registry.npmjs.org/@langchain/langgraph/-/langgraph-1.4.7.tgz", - "integrity": "sha512-2tcyf3QGC7v89kqSxMCtRvzg/3L/4yHtOaWC49A8KieCciWJs7LGaxHoPB6QRxXyUgyR+Zg9Q1ss/XJIE+JuSQ==", + "version": "1.4.8", + "resolved": "https://registry.npmjs.org/@langchain/langgraph/-/langgraph-1.4.8.tgz", + "integrity": "sha512-DN1Np1XefdBEbp1qBKlt39cwoL743AAGpR5Ipja0gY2YbWvsoQnOTIrjnj/orSAhaUYsdTKS8VSWdFzsHZo6Ig==", "license": "MIT", "dependencies": { "@langchain/langgraph-checkpoint": "^1.1.3", - "@langchain/langgraph-sdk": "~1.9.25", + "@langchain/langgraph-sdk": "~1.9.26", "@langchain/protocol": "^0.0.18", "@standard-schema/spec": "1.1.0" }, @@ -1169,9 +1169,9 @@ } }, "node_modules/@langchain/langgraph-sdk": { - "version": "1.9.25", - "resolved": "https://registry.npmjs.org/@langchain/langgraph-sdk/-/langgraph-sdk-1.9.25.tgz", - "integrity": "sha512-mRKW8zyQUaHox+HirRFMRrPqOvNbQI3xeXDt6kkk4PbBg77V92bsO1WzUVNrmJ81zCkvxyOrWSK8D6ioCj0a8A==", + "version": "1.9.28", + "resolved": "https://registry.npmjs.org/@langchain/langgraph-sdk/-/langgraph-sdk-1.9.28.tgz", + "integrity": "sha512-4j3XuM0PvtmAbL8mPfBS99ez3+ytRfgbOpAR/nOeaejTRF3Q9dNw2QnaGLGng8wLPtGLoSj+SYgUOVxy9Bv9vg==", "license": "MIT", "dependencies": { "@langchain/protocol": "^0.0.18", @@ -1208,9 +1208,9 @@ "license": "MIT" }, "node_modules/@langchain/langgraph-sdk/node_modules/p-queue": { - "version": "9.3.0", - "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-9.3.0.tgz", - "integrity": "sha512-7NED7xhQ74Ngp4JP/2e0VZHp7vSWfJfqeiR92jPgxsz6m0Se4P03YoTKa9dDXyZ3r6P616gUXttrB6nnHYKang==", + "version": "9.3.3", + "resolved": "https://registry.npmjs.org/p-queue/-/p-queue-9.3.3.tgz", + "integrity": "sha512-NXAOdnEe5FsZJfT4oK84lE1Y5cFFdWlRuOo5tww8DyNMxyRXwn39fIkUtNLKppcPC+UYU/bXujNCUGDv01y7CA==", "license": "MIT", "dependencies": { "eventemitter3": "^5.0.4", diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a9aa163da..3ab7d417b 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -21,7 +21,7 @@ "@langchain/anthropic": "^1.5.1", "@langchain/core": "^1.2.2", "@langchain/google-genai": "^2.2.0", - "@langchain/langgraph": "^1.4.7", + "@langchain/langgraph": "^1.4.8", "@langchain/ollama": "^1.3.0", "@langchain/openai": "^1.5.3", "@sigma/edge-curve": "^3.1.0", From d8198131cd1852c1e5560ecd2f0588c21fc74d7a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:14:44 +0000 Subject: [PATCH 13/14] chore(deps)(deps-dev): bump @babel/types in /gitnexus-web Bumps [@babel/types](https://github.com/babel/babel/tree/HEAD/packages/babel-types) from 7.29.7 to 8.0.0. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v8.0.0/packages/babel-types) --- updated-dependencies: - dependency-name: "@babel/types" dependency-version: 8.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- gitnexus-web/package-lock.json | 78 +++++++++++++++++++++++++++++----- gitnexus-web/package.json | 2 +- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index e5fda5365..81501ea82 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -47,7 +47,7 @@ "zod": "^4.4.3" }, "devDependencies": { - "@babel/types": "^7.29.0", + "@babel/types": "^8.0.0", "@playwright/test": "^1.61.1", "@testing-library/jest-dom": "^6.9.1", "@testing-library/react": "^16.3.2", @@ -186,13 +186,13 @@ } }, "node_modules/@babel/helper-string-parser": { - "version": "7.29.7", - "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.29.7.tgz", - "integrity": "sha512-Pb5ijPrZ89GDH8223L4UP8i6QApWxs04RbPQJTeWDV0/keR2E36MeKnyr6LYmUUvqRRI+Iv87SuF1W6ErINzYw==", + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-8.0.0.tgz", + "integrity": "sha512-6mJgmFFFIIO82vvoLt9XtRC7/TkzXfts1t/SpRX4IHSzMgqoPYCWesVu1udUPUWioAE/2fcG6WuI8zrkE1gwrg==", "dev": true, "license": "MIT", "engines": { - "node": ">=6.9.0" + "node": "^22.18.0 || >=24.11.0" } }, "node_modules/@babel/helper-validator-identifier": { @@ -221,16 +221,17 @@ "node": ">=6.0.0" } }, - "node_modules/@babel/runtime": { - "version": "7.29.2", - "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.29.2.tgz", - "integrity": "sha512-JiDShH45zKHWyGe4ZNVRrCjBz8Nh9TMmZG1kh4QTK8hCBTWBi8Da+i7s1fJw7/lYpM4ccepSNfqzZ/QvABBi5g==", + "node_modules/@babel/parser/node_modules/@babel/helper-string-parser": { + "version": "7.29.7", + "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.29.7.tgz", + "integrity": "sha512-Pb5ijPrZ89GDH8223L4UP8i6QApWxs04RbPQJTeWDV0/keR2E36MeKnyr6LYmUUvqRRI+Iv87SuF1W6ErINzYw==", + "dev": true, "license": "MIT", "engines": { "node": ">=6.9.0" } }, - "node_modules/@babel/types": { + "node_modules/@babel/parser/node_modules/@babel/types": { "version": "7.29.7", "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.29.7.tgz", "integrity": "sha512-4zBIxpPzowiZpusoFkyGVwakdRJUyuH5PxQ/PrqghfdFWWasvnCdPfQXHrenDai+gyLARulZjZowCOj6fjT4pA==", @@ -244,6 +245,39 @@ "node": ">=6.9.0" } }, + "node_modules/@babel/runtime": { + "version": "7.29.2", + "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.29.2.tgz", + "integrity": "sha512-JiDShH45zKHWyGe4ZNVRrCjBz8Nh9TMmZG1kh4QTK8hCBTWBi8Da+i7s1fJw7/lYpM4ccepSNfqzZ/QvABBi5g==", + "license": "MIT", + "engines": { + "node": ">=6.9.0" + } + }, + "node_modules/@babel/types": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@babel/types/-/types-8.0.0.tgz", + "integrity": "sha512-K8ponJDxBwDHigkeFqaqT5wLGl4bTlwMafR8k7b5CPxr6Ww+UG9ls8Yx6Tcpboxu97eeGVEEyKcHmEyOwN1vSw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@babel/helper-string-parser": "^8.0.0", + "@babel/helper-validator-identifier": "^8.0.0" + }, + "engines": { + "node": "^22.18.0 || >=24.11.0" + } + }, + "node_modules/@babel/types/node_modules/@babel/helper-validator-identifier": { + "version": "8.0.4", + "resolved": "https://registry.npmjs.org/@babel/helper-validator-identifier/-/helper-validator-identifier-8.0.4.tgz", + "integrity": "sha512-4wFaiLd0bVo4cIoTXI3zKI038NIWE/cr3jvBjejOVYVxV/m8Ltav1USiGzG1fmS5J2RhgEOgXNNK46cRPnRsrg==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^22.18.0 || >=24.11.0" + } + }, "node_modules/@bcoe/v8-coverage": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-1.0.2.tgz", @@ -5721,6 +5755,30 @@ "source-map-js": "^1.2.1" } }, + "node_modules/magicast/node_modules/@babel/helper-string-parser": { + "version": "7.29.7", + "resolved": "https://registry.npmjs.org/@babel/helper-string-parser/-/helper-string-parser-7.29.7.tgz", + "integrity": "sha512-Pb5ijPrZ89GDH8223L4UP8i6QApWxs04RbPQJTeWDV0/keR2E36MeKnyr6LYmUUvqRRI+Iv87SuF1W6ErINzYw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=6.9.0" + } + }, + "node_modules/magicast/node_modules/@babel/types": { + "version": "7.29.7", + "resolved": "https://registry.npmjs.org/@babel/types/-/types-7.29.7.tgz", + "integrity": "sha512-4zBIxpPzowiZpusoFkyGVwakdRJUyuH5PxQ/PrqghfdFWWasvnCdPfQXHrenDai+gyLARulZjZowCOj6fjT4pA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@babel/helper-string-parser": "^7.29.7", + "@babel/helper-validator-identifier": "^7.29.7" + }, + "engines": { + "node": ">=6.9.0" + } + }, "node_modules/make-dir": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-4.0.0.tgz", diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a9aa163da..16346dc2a 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -57,7 +57,7 @@ "zod": "^4.4.3" }, "devDependencies": { - "@babel/types": "^7.29.0", + "@babel/types": "^8.0.0", "@playwright/test": "^1.61.1", "@testing-library/jest-dom": "^6.9.1", "@testing-library/react": "^16.3.2", From 3d6049793b5cdee556fc837132641874587fbbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Wed, 22 Jul 2026 21:30:52 +0100 Subject: [PATCH 14/14] fix(lbug): reclaim missing-shadow WAL quarantine files on write-path init (#2638) --- gitnexus/src/core/lbug/lbug-adapter.ts | 26 ++++ gitnexus/src/core/lbug/sidecar-recovery.ts | 2 +- .../lbug-orphan-sidecar-recovery.test.ts | 137 ++++++++++++++++++ .../test/unit/lbug-adapter-wal-schema.test.ts | 2 + .../unit/lbug-checkpoint-lifecycle.test.ts | 7 + 5 files changed, 173 insertions(+), 1 deletion(-) diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 7a153a7de..75e8dccbe 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -46,6 +46,7 @@ import { type LbugConnectionHandle, } from './lbug-config.js'; import { + cleanQuarantinedMissingShadowWals, finalizeLbugSidecarsAfterClose, guardWalQuarantine, isMissingShadowSidecarError, @@ -55,6 +56,7 @@ import { quarantineWalForMissingShadow, renameFailureMessage, shadowSidecarRecoveryMessage, + sidecarPreflightDisabled, } from './sidecar-recovery.js'; import { logger } from '../logger.js'; @@ -822,6 +824,30 @@ const doInitLbug = async (dbPath: string, readOnly: boolean = false) => { // ------------------------------------------------------------------------- const releaseInitLock = await acquireInitLock(dbPath); try { + // Reclaim missing-shadow WAL quarantines from a PRIOR crash (#2637). + // LadybugDB renames an unrecoverable WAL aside as + // `${dbPath}.wal.missing-shadow.-` (quarantineWalForMissingShadow) + // instead of deleting it. Once quarantined it is permanently detached from + // the live store and never reopened, so reclaiming it is safe regardless of + // whether the main DB file exists this run — unlike the orphan-sidecar + // cleanup below, this must NOT be gated on "main DB missing": a quarantine + // event and a healthy main DB are independent facts. Never let a reclaim + // failure (e.g. a transient EBUSY from an antivirus scan) block DB startup. + if (!sidecarPreflightDisabled()) { + try { + const reclaimed = await cleanQuarantinedMissingShadowWals(dbPath); + for (const file of reclaimed) { + logger.warn( + `GitNexus: reclaimed quarantined WAL ${path.basename(file)} from a prior crash`, + ); + } + } catch (err) { + logger.warn( + `GitNexus: failed to reclaim missing-shadow WAL quarantines: ${summarizeError(err)}`, + ); + } + } + // Crash-recovery cleanup: if the main DB file is missing, stale sidecars // from an interrupted run can block fresh opens indefinitely. try { diff --git a/gitnexus/src/core/lbug/sidecar-recovery.ts b/gitnexus/src/core/lbug/sidecar-recovery.ts index c00c1aa7d..882041747 100644 --- a/gitnexus/src/core/lbug/sidecar-recovery.ts +++ b/gitnexus/src/core/lbug/sidecar-recovery.ts @@ -60,7 +60,7 @@ export const isMissingFsError = (err: unknown): boolean => const missing = isMissingFsError; -const sidecarPreflightDisabled = (): boolean => +export const sidecarPreflightDisabled = (): boolean => /^(1|true|yes|on)$/i.test(process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT ?? ''); export const statIfExists = async (filePath: string): Promise<{ size: number } | null> => { diff --git a/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts b/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts index e74cd5cf7..97e71cfb2 100644 --- a/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts +++ b/gitnexus/test/integration/lbug-orphan-sidecar-recovery.test.ts @@ -328,3 +328,140 @@ describe('init lock — single-process ownership contract', () => { } }); }); + +// --------------------------------------------------------------------------- +// Missing-shadow WAL quarantine reclaim (issue #2637) +// --------------------------------------------------------------------------- + +const plantMissingShadowQuarantine = async (dbPath: string): Promise => { + const quarantinePath = `${dbPath}.wal.missing-shadow.${Date.now()}-${Math.random() + .toString(36) + .slice(2)}`; + await fs.writeFile(quarantinePath, 'stale-quarantined-wal-bytes'); + return quarantinePath; +}; + +describe('missing-shadow quarantine reclaim — native integration (issue #2637)', () => { + itLbugReopen( + 'reclaims a pre-existing missing-shadow WAL quarantine file on write-path init when the main DB is present', + async () => { + const tmp = await createTempDir('gitnexus-lbug-quarantine-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + + try { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // Create a real DB first, then close it. + await adapter.initLbug(dbPath); + await adapter.closeLbug(); + + // Plant a quarantine file left over from an earlier crash. + const quarantinePath = await plantMissingShadowQuarantine(dbPath); + await expect(fs.access(quarantinePath)).resolves.toBeUndefined(); + + // Re-init with the main DB present — reclaim must fire unconditionally. + await adapter.initLbug(dbPath); + + const rows = await adapter.executeQuery('RETURN 1 AS ok'); + expect(rows).toEqual([{ ok: 1 }]); + + await expect(fs.access(quarantinePath)).rejects.toThrow(); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); + + itLbugReopen( + 'reclaims a pre-existing missing-shadow WAL quarantine file when the main DB is ALSO missing (crash-recovery path)', + async () => { + const tmp = await createTempDir('gitnexus-lbug-quarantine-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const shadowPath = `${dbPath}.shadow`; + const walCheckpointPath = `${dbPath}.wal.checkpoint`; + + try { + // No main DB file — plant the quarantine file alongside the #1618 + // orphan sidecars to prove both cleanup blocks coexist correctly. + const quarantinePath = await plantMissingShadowQuarantine(dbPath); + await fs.writeFile(shadowPath, 'stale-shadow-data'); + await fs.writeFile(walCheckpointPath, 'stale-wal-checkpoint-data'); + + 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 expect(fs.access(quarantinePath)).rejects.toThrow(); + await expect(fs.access(shadowPath)).rejects.toThrow(); + await expect(fs.access(walCheckpointPath)).rejects.toThrow(); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); + + itLbugReopen( + 'leaves a missing-shadow quarantine file untouched when GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT=1', + async () => { + const tmp = await createTempDir('gitnexus-lbug-quarantine-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const previousEnv = process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT; + + try { + const quarantinePath = await plantMissingShadowQuarantine(dbPath); + + process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT = '1'; + + 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 }]); + + // Reclaim was suppressed — the quarantine file survives. + await expect(fs.access(quarantinePath)).resolves.toBeUndefined(); + + await adapter.closeLbug(); + } finally { + if (previousEnv === undefined) { + delete process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT; + } else { + process.env.GITNEXUS_DISABLE_LBUG_SIDECAR_PREFLIGHT = previousEnv; + } + await tmp.cleanup(); + } + }, + ); + + itLbugReopen( + 'does not touch a .dirty-recovery parked sidecar (isolation from the missing-shadow family)', + async () => { + const tmp = await createTempDir('gitnexus-lbug-quarantine-'); + const dbPath = path.join(tmp.dbPath, 'lbug'); + const dirtyRecoveryPath = `${dbPath}.wal.dirty-recovery`; + + try { + await fs.writeFile(dirtyRecoveryPath, 'parked-from-an-interrupted-dirty-recovery-rebuild'); + + 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 }]); + + // Different sidecar family, different lifecycle — must survive. + await expect(fs.access(dirtyRecoveryPath)).resolves.toBeUndefined(); + + await adapter.closeLbug(); + } finally { + await tmp.cleanup(); + } + }, + ); +}); diff --git a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts index ebd956228..3243c1cfd 100644 --- a/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts +++ b/gitnexus/test/unit/lbug-adapter-wal-schema.test.ts @@ -44,6 +44,7 @@ function makeFsMock(dbPath: string) { rename: vi.fn(async () => {}), mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, }; } @@ -586,6 +587,7 @@ function makeFsMockWithWalSize( rename: vi.fn(async () => {}), mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, }; } diff --git a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts index 286c30a99..e06e7445c 100644 --- a/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts +++ b/gitnexus/test/unit/lbug-checkpoint-lifecycle.test.ts @@ -45,6 +45,7 @@ const mockFsForInit = (dbPath: string) => { unlink: vi.fn(async () => {}), mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); }; @@ -85,6 +86,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ @@ -156,6 +158,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ @@ -220,6 +223,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ @@ -285,6 +289,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ @@ -345,6 +350,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); vi.doMock('../../src/core/lbug/lbug-config.js', () => ({ @@ -415,6 +421,7 @@ describe('lbug adapter CHECKPOINT lifecycle', () => { unlink: unlinkMock, mkdir: vi.fn(async () => {}), open: makeOpenMock(), + readdir: vi.fn(async () => []), }, })); const openLbugConnectionMock = vi.fn(async () => ({ db, conn }));