From 2b6e7ffbd93b5f9922a6a28d5e978f6748fcfaa3 Mon Sep 17 00:00:00 2001 From: Minidoracat Date: Sun, 24 May 2026 06:37:40 +0800 Subject: [PATCH 01/10] fix(php): avoid Blade templates entering PHP analysis (#1790) --- gitnexus-shared/src/index.ts | 6 +- gitnexus-shared/src/language-detection.ts | 12 + gitnexus/src/config/ignore-service.ts | 10 + .../group/extractors/http-patterns/index.ts | 2 + .../src/core/ingestion/languages/index.ts | 4 +- .../core/ingestion/pipeline-phases/routes.ts | 117 ++++- .../ingestion/route-extractors/laravel.ts | 495 ++++++++++++++++++ .../core/ingestion/workers/parse-worker.ts | 434 +-------------- .../test/unit/blade-template-routes.test.ts | 165 ++++++ .../unit/group/http-route-extractor.test.ts | 8 + gitnexus/test/unit/ignore-service.test.ts | 6 + gitnexus/test/unit/ingestion-utils.test.ts | 42 +- .../unit/laravel-route-extraction.test.ts | 217 ++++++++ gitnexus/test/unit/php-template-scope.test.ts | 72 +++ 14 files changed, 1133 insertions(+), 457 deletions(-) create mode 100644 gitnexus/src/core/ingestion/route-extractors/laravel.ts create mode 100644 gitnexus/test/unit/blade-template-routes.test.ts create mode 100644 gitnexus/test/unit/laravel-route-extraction.test.ts create mode 100644 gitnexus/test/unit/php-template-scope.test.ts diff --git a/gitnexus-shared/src/index.ts b/gitnexus-shared/src/index.ts index e2d5df284..fc591fdb0 100644 --- a/gitnexus-shared/src/index.ts +++ b/gitnexus-shared/src/index.ts @@ -18,7 +18,11 @@ export type { NodeTableName, RelType } from './lbug/schema-constants.js'; // Language support export { SupportedLanguages } from './languages.js'; -export { getLanguageFromFilename, getSyntaxLanguageFromFilename } from './language-detection.js'; +export { + getLanguageFromFilename, + getSyntaxLanguageFromFilename, + isBladeTemplateFilename, +} from './language-detection.js'; export type { MroStrategy } from './mro-strategy.js'; // Pipeline progress diff --git a/gitnexus-shared/src/language-detection.ts b/gitnexus-shared/src/language-detection.ts index d31f9d58d..f9073b8c0 100644 --- a/gitnexus-shared/src/language-detection.ts +++ b/gitnexus-shared/src/language-detection.ts @@ -56,11 +56,21 @@ for (const [lang, exts] of Object.entries(EXTENSION_MAP) as [ } } +/** + * Laravel Blade templates are source templates whose filename convention ends + * in `.blade.php`. They may contain PHP snippets, but the full file is not a + * pure PHP translation unit and must not enter the generic PHP provider path. + */ +export const isBladeTemplateFilename = (filePath: string): boolean => + filePath.replace(/\\/g, '/').toLowerCase().endsWith('.blade.php'); + /** * Map file extension to SupportedLanguage enum. * Returns null if the file extension is not recognized. */ export const getLanguageFromFilename = (filename: string): SupportedLanguages | null => { + if (isBladeTemplateFilename(filename)) return null; + // Fast path: check the extension map const lastDot = filename.lastIndexOf('.'); if (lastDot >= 0) { @@ -138,6 +148,8 @@ const AUXILIARY_BASENAME_MAP: Record = { * Returns 'text' for unrecognised files. */ export const getSyntaxLanguageFromFilename = (filePath: string): string => { + if (isBladeTemplateFilename(filePath)) return 'markup'; + const lang = getLanguageFromFilename(filePath); if (lang) return SYNTAX_MAP[lang]; const ext = filePath.split('.').pop()?.toLowerCase(); diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index 2c3eebe3b..17b9d14bd 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -283,10 +283,20 @@ const IGNORED_FILES = new Set([ // deterministic results independent of per-repo config. export const shouldIgnorePath = (filePath: string): boolean => { const normalizedPath = filePath.replace(/\\/g, '/'); + const normalizedPathLower = normalizedPath.toLowerCase(); const parts = normalizedPath.split('/'); const fileName = parts[parts.length - 1]; const fileNameLower = fileName.toLowerCase(); + // Laravel compiles Blade templates into generated PHP cache files under + // storage/framework/views. Source templates live in resources/views and are + // handled separately; compiled cache should not become source-of-truth. Keep + // storage/framework/cache parseable unless a separate warning source is proven: + // Laravel route/config cache files are ordinary generated PHP, not Blade. + if (/(^|\/)storage\/framework\/views(\/|$)/.test(normalizedPathLower)) { + return true; + } + // Check if any path segment is in the hardcoded ignore list. for (const part of parts) { if (DEFAULT_IGNORE_LIST.has(part)) { diff --git a/gitnexus/src/core/group/extractors/http-patterns/index.ts b/gitnexus/src/core/group/extractors/http-patterns/index.ts index e33d32a79..c62eb618c 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/index.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/index.ts @@ -1,4 +1,5 @@ import * as path from 'node:path'; +import { isBladeTemplateFilename } from 'gitnexus-shared'; import type { HttpLanguagePlugin } from './types.js'; import { JAVA_HTTP_PLUGIN } from './java.js'; import { GO_HTTP_PLUGIN } from './go.js'; @@ -45,6 +46,7 @@ export const HTTP_SCAN_GLOB = '**/*.{ts,tsx,js,jsx,java,go,py,php}'; * or `undefined` if the extension is not registered. */ export function getPluginForFile(rel: string): HttpLanguagePlugin | undefined { + if (isBladeTemplateFilename(rel)) return undefined; const ext = path.extname(rel).toLowerCase(); return REGISTRY[ext]; } diff --git a/gitnexus/src/core/ingestion/languages/index.ts b/gitnexus/src/core/ingestion/languages/index.ts index 041af775d..6191e881b 100644 --- a/gitnexus/src/core/ingestion/languages/index.ts +++ b/gitnexus/src/core/ingestion/languages/index.ts @@ -8,7 +8,7 @@ * 4. Run `tsc --noEmit` to verify */ -import { SupportedLanguages } from 'gitnexus-shared'; +import { SupportedLanguages, isBladeTemplateFilename } from 'gitnexus-shared'; import type { LanguageProvider } from '../language-provider.js'; import { typescriptProvider, javascriptProvider } from './typescript.js'; @@ -61,6 +61,8 @@ for (const provider of Object.values(providers)) { /** Look up a language provider from a file path by extension. * Returns null if the file extension is not recognized. */ export function getProviderForFile(filePath: string): LanguageProvider | null { + if (isBladeTemplateFilename(filePath)) return null; + const lastDot = filePath.lastIndexOf('.'); const ext = lastDot >= 0 ? filePath.slice(lastDot).toLowerCase() : ''; const basename = filePath.slice(filePath.lastIndexOf('/') + 1); diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index de3a8ddb8..a1ee73eea 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -14,6 +14,7 @@ import type { PipelinePhase, PipelineContext, PhaseResult } from './types.js'; import { getPhaseOutput } from './types.js'; import type { ParseOutput } from './parse.js'; +import { isBladeTemplateFilename } from 'gitnexus-shared'; import { nextjsFileToRouteURL, normalizeFetchURL } from '../route-extractors/nextjs.js'; import { expoFileToRouteURL } from '../route-extractors/expo.js'; import { phpFileToRouteURL } from '../route-extractors/php.js'; @@ -47,6 +48,89 @@ export interface RoutesOutput { routeRegistry: Map; } +export interface TemplateFetchCall { + filePath: string; + fetchURL: string; + lineNumber: number; +} + +const TEMPLATE_URL_PATTERNS: readonly RegExp[] = [ + /\b(?:action|href)\s*=\s*["']([^"']+)["']/gi, + /\burl\s*:\s*["']([^"']+)["'](?!\s*\+)/g, + // Laravel asset() points at static assets, not application routes; keep it + // out of route matching so asset paths cannot collide with real route URLs. + /\{\{[\s\S]{0,200}?\burl\(\s*["']([^"']+)["']\s*\)[\s\S]{0,200}?\}\}/g, + /\{!![\s\S]{0,200}?\burl\(\s*["']([^"']+)["']\s*\)[\s\S]{0,200}?!\}/g, +]; + +const TEMPLATE_NAMED_ROUTE_PATTERNS: readonly RegExp[] = [ + // Parameterless Laravel route('name') helpers can be resolved from extracted + // route names. Parameterized helpers are intentionally deferred because they + // require binding runtime values onto route placeholders. + /\{\{[\s\S]{0,200}?\broute\(\s*["']([^"']+)["']\s*\)[\s\S]{0,200}?\}\}/g, + /\{!![\s\S]{0,200}?\broute\(\s*["']([^"']+)["']\s*\)[\s\S]{0,200}?!\}/g, +]; + +function hasRouteParameters(routeUrl: string): boolean { + return /\{[^}]+\}/.test(routeUrl); +} + +export const isTemplateRouteCandidate = (filePath: string): boolean => { + const normalized = filePath.replace(/\\/g, '/').toLowerCase(); + return ( + normalized.endsWith('.html') || + normalized.endsWith('.htm') || + normalized.endsWith('.ejs') || + normalized.endsWith('.hbs') || + isBladeTemplateFilename(normalized) + ); +}; + +export function extractTemplateStaticFetchCalls( + filePath: string, + content: string, + namedRouteUrls: ReadonlyMap = new Map(), +): TemplateFetchCall[] { + const calls: TemplateFetchCall[] = []; + const seen = new Set(); + + for (const pattern of TEMPLATE_URL_PATTERNS) { + pattern.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = pattern.exec(content)) !== null) { + const normalized = normalizeFetchURL(match[1]); + if (!normalized) continue; + if (seen.has(normalized)) continue; + seen.add(normalized); + calls.push({ filePath, fetchURL: normalized, lineNumber: 0 }); + } + } + + for (const pattern of TEMPLATE_NAMED_ROUTE_PATTERNS) { + pattern.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = pattern.exec(content)) !== null) { + const routeUrl = namedRouteUrls.get(match[1]); + if (!routeUrl) continue; + if (hasRouteParameters(routeUrl)) continue; + const normalized = normalizeFetchURL(routeUrl); + if (!normalized) continue; + if (seen.has(normalized)) continue; + seen.add(normalized); + calls.push({ filePath, fetchURL: normalized, lineNumber: 0 }); + } + } + + return calls; +} + +export function normalizeExtractedRoutePath(routePath: string, prefix: string | null): string { + const pathPart = routePath.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); + const prefixPart = prefix?.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); + const joined = prefixPart ? `/${prefixPart}${pathPart ? `/${pathPart}` : ''}` : `/${pathPart}`; + return joined.replace(/\/+/g, '/') || '/'; +} + export const routesPhase: PipelinePhase = { name: 'routes', deps: ['parse'], @@ -111,6 +195,7 @@ export const routesPhase: PipelinePhase = { const ensureSlash = (path: string) => (path.startsWith('/') ? path : '/' + path); let duplicateRoutes = 0; + const namedRouteRegistry = new Map(); const addRoute = (url: string, entry: RouteEntry) => { if (routeRegistry.has(url)) { duplicateRoutes++; @@ -120,10 +205,14 @@ export const routesPhase: PipelinePhase = { }; for (const route of allExtractedRoutes) { if (!route.routePath) continue; - addRoute(ensureSlash(route.routePath), { + const routeUrl = normalizeExtractedRoutePath(route.routePath, route.prefix); + addRoute(routeUrl, { filePath: route.filePath, source: 'framework-route', }); + if (route.routeName && !namedRouteRegistry.has(route.routeName)) { + namedRouteRegistry.set(route.routeName, routeUrl); + } } for (const dr of allDecoratorRoutes) { addRoute(ensureSlash(dr.routePath), { @@ -233,29 +322,15 @@ export const routesPhase: PipelinePhase = { } } - // Scan HTML/template files for form action and AJAX url patterns - const htmlCandidates = allPaths.filter( - (p) => - p.endsWith('.html') || - p.endsWith('.htm') || - p.endsWith('.ejs') || - p.endsWith('.hbs') || - p.endsWith('.blade.php'), - ); + // Scan HTML/template files for safe static form/link/AJAX URL patterns. + // Blade stays template-only here; it must not re-enter PHP provider paths. + const htmlCandidates = allPaths.filter(isTemplateRouteCandidate); if (htmlCandidates.length > 0 && routeRegistry.size > 0) { const htmlContents = await readFileContents(ctx.repoPath, htmlCandidates); - const htmlPatterns = [/action=["']([^"']+)["']/g, /url:\s*["']([^"']+)["']/g]; for (const [filePath, content] of htmlContents) { - for (const pattern of htmlPatterns) { - pattern.lastIndex = 0; - let match; - while ((match = pattern.exec(content)) !== null) { - const normalized = normalizeFetchURL(match[1]); - if (normalized) { - allFetchCalls.push({ filePath, fetchURL: normalized, lineNumber: 0 }); - } - } - } + allFetchCalls.push( + ...extractTemplateStaticFetchCalls(filePath, content, namedRouteRegistry), + ); } } diff --git a/gitnexus/src/core/ingestion/route-extractors/laravel.ts b/gitnexus/src/core/ingestion/route-extractors/laravel.ts new file mode 100644 index 000000000..6d934bf5b --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/laravel.ts @@ -0,0 +1,495 @@ +import type Parser from 'tree-sitter'; +import { extractStringContent, findDescendant, type SyntaxNode } from '../utils/ast-helpers.js'; + +export interface ExtractedRoute { + filePath: string; + httpMethod: string; + routePath: string | null; + routeName: string | null; + controllerName: string | null; + methodName: string | null; + middleware: string[]; + prefix: string | null; + lineNumber: number; +} + +interface RouteGroupContext { + middleware: string[]; + prefix: string | null; + namePrefix: string | null; + controller: string | null; +} + +const ROUTE_HTTP_METHODS = new Set([ + 'get', + 'post', + 'put', + 'patch', + 'delete', + 'options', + 'any', + 'match', +]); + +const ROUTE_RESOURCE_METHODS = new Set(['resource', 'apiResource']); +const RESOURCE_ACTIONS = ['index', 'create', 'store', 'show', 'edit', 'update', 'destroy']; +const API_RESOURCE_ACTIONS = ['index', 'store', 'show', 'update', 'destroy']; + +/** Check if node is a scoped_call_expression with object 'Route' */ +function isRouteStaticCall(node: SyntaxNode): boolean { + if (node.type !== 'scoped_call_expression') return false; + const obj = node.childForFieldName?.('object') ?? node.children?.[0]; + return obj?.text === 'Route'; +} + +/** Get the method name from a scoped_call_expression or member_call_expression */ +function getCallMethodName(node: SyntaxNode): string | null { + const nameNode = + node.childForFieldName?.('name') ?? node.children?.find((c: SyntaxNode) => c.type === 'name'); + return nameNode?.text ?? null; +} + +/** Get the arguments node from a call expression */ +function getArguments(node: SyntaxNode): SyntaxNode | null { + return node.children?.find((c: SyntaxNode) => c.type === 'arguments') ?? null; +} + +/** Find the closure body inside arguments */ +function findClosureBody(argsNode: SyntaxNode | null): SyntaxNode | null { + if (!argsNode) return null; + for (const child of argsNode.children ?? []) { + if (child.type === 'argument') { + for (const inner of child.children ?? []) { + if (inner.type === 'anonymous_function' || inner.type === 'arrow_function') { + return ( + inner.childForFieldName?.('body') ?? + inner.children?.find((c: SyntaxNode) => c.type === 'compound_statement') ?? + null + ); + } + } + } + if (child.type === 'anonymous_function' || child.type === 'arrow_function') { + return ( + child.childForFieldName?.('body') ?? + child.children?.find((c: SyntaxNode) => c.type === 'compound_statement') ?? + null + ); + } + } + return null; +} + +/** Extract first string argument from arguments node */ +function extractFirstStringArg(argsNode: SyntaxNode | null): string | null { + if (!argsNode) return null; + for (const child of argsNode.children ?? []) { + const target = child.type === 'argument' ? child.children?.[0] : child; + if (!target) continue; + if (target.type === 'string' || target.type === 'encapsed_string') { + return extractStringContent(target); + } + } + return null; +} + +/** Extract middleware from arguments — handles string or array */ +function extractMiddlewareArg(argsNode: SyntaxNode | null): string[] { + if (!argsNode) return []; + for (const child of argsNode.children ?? []) { + const target = child.type === 'argument' ? child.children?.[0] : child; + if (!target) continue; + if (target.type === 'string' || target.type === 'encapsed_string') { + const val = extractStringContent(target); + return val ? [val] : []; + } + if (target.type === 'array_creation_expression') { + const items: string[] = []; + for (const el of target.children ?? []) { + if (el.type === 'array_element_initializer') { + const str = el.children?.find( + (c: SyntaxNode) => c.type === 'string' || c.type === 'encapsed_string', + ); + const val = str ? extractStringContent(str) : null; + if (val) items.push(val); + } + } + return items; + } + } + return []; +} + +/** Extract Controller::class from arguments */ +function extractClassArg(argsNode: SyntaxNode | null): string | null { + if (!argsNode) return null; + for (const child of argsNode.children ?? []) { + const target = child.type === 'argument' ? child.children?.[0] : child; + if (target?.type === 'class_constant_access_expression') { + return target.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; + } + } + return null; +} + +function joinRouteName(prefix: string | null, name: string | null): string | null { + if (!name) return null; + return prefix ? `${prefix}${name}` : name; +} + +function routeNameBaseFromPath(routePath: string | null): string | null { + const base = routePath + ?.trim() + .replace(/^\/+|\/+$/g, '') + .replace(/\//g, '.'); + return base || null; +} + +function appendResourceActionName(base: string | null, action: string): string | null { + if (!base) return null; + return base.endsWith('.') ? `${base}${action}` : `${base}.${action}`; +} + +/** Extract controller class name from common Laravel handler argument shapes. */ +function extractControllerTarget(argsNode: SyntaxNode | null): { + controller: string | null; + method: string | null; + bareMethod: string | null; +} { + if (!argsNode) return { controller: null, method: null, bareMethod: null }; + + const args: (SyntaxNode | undefined)[] = []; + for (const child of argsNode.children ?? []) { + if (child.type === 'argument') args.push(child.children?.[0]); + else if (child.type !== '(' && child.type !== ')' && child.type !== ',') args.push(child); + } + + // Second arg is the handler + const handlerNode = args[1]; + if (!handlerNode) return { controller: null, method: null, bareMethod: null }; + + // Array syntax: [UserController::class, 'index'] + if (handlerNode.type === 'array_creation_expression') { + let controller: string | null = null; + let method: string | null = null; + const elements: SyntaxNode[] = []; + for (const el of handlerNode.children ?? []) { + if (el.type === 'array_element_initializer') elements.push(el); + } + if (elements[0]) { + const classAccess = findDescendant(elements[0], 'class_constant_access_expression'); + if (classAccess) { + controller = classAccess.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; + } + } + if (elements[1]) { + const str = findDescendant(elements[1], 'string'); + method = str ? extractStringContent(str) : null; + } + return { controller, method, bareMethod: null }; + } + + // String syntax: 'UserController@index'. A bare string such as 'index' + // becomes a method only when a surrounding Route::controller(...) group + // supplies the controller class. + if (handlerNode.type === 'string' || handlerNode.type === 'encapsed_string') { + const text = extractStringContent(handlerNode); + if (text?.includes('@')) { + const [controller, method] = text.split('@'); + return { controller, method, bareMethod: null }; + } + if (text) return { controller: null, method: null, bareMethod: text }; + } + + // Class reference: UserController::class (invokable controller) + if (handlerNode.type === 'class_constant_access_expression') { + const controller = + handlerNode.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; + return { controller, method: '__invoke', bareMethod: null }; + } + + return { controller: null, method: null, bareMethod: null }; +} + +interface ChainedRouteCall { + isRouteFacade: boolean; + terminalMethod: string; + attributes: { method: string; argsNode: SyntaxNode | null }[]; + terminalArgs: SyntaxNode | null; + node: SyntaxNode; +} + +/** + * Unwrap a chained call like Route::middleware('auth')->prefix('api')->group(fn) + */ +function unwrapRouteChain(node: SyntaxNode): ChainedRouteCall | null { + if (node.type !== 'member_call_expression') return null; + + const terminalMethod = getCallMethodName(node); + if (!terminalMethod) return null; + + const terminalArgs = getArguments(node); + const attributes: { method: string; argsNode: SyntaxNode | null }[] = []; + + let current = node.children?.[0]; + + while (current) { + if (current.type === 'member_call_expression') { + const method = getCallMethodName(current); + const args = getArguments(current); + if (method) attributes.unshift({ method, argsNode: args }); + current = current.children?.[0]; + } else if (current.type === 'scoped_call_expression') { + const obj = current.childForFieldName?.('object') ?? current.children?.[0]; + if (obj?.text !== 'Route') return null; + + const method = getCallMethodName(current); + const args = getArguments(current); + if (method) attributes.unshift({ method, argsNode: args }); + + return { isRouteFacade: true, terminalMethod, attributes, terminalArgs, node }; + } else { + break; + } + } + + return null; +} + +/** Parse Route::group(['middleware' => ..., 'prefix' => ...], fn) array syntax */ +function parseArrayGroupArgs(argsNode: SyntaxNode | null): RouteGroupContext { + const ctx: RouteGroupContext = { + middleware: [], + prefix: null, + namePrefix: null, + controller: null, + }; + if (!argsNode) return ctx; + + for (const child of argsNode.children ?? []) { + const target = child.type === 'argument' ? child.children?.[0] : child; + if (target?.type === 'array_creation_expression') { + for (const el of target.children ?? []) { + if (el.type !== 'array_element_initializer') continue; + const children = el.children ?? []; + const arrowIdx = children.findIndex((c: SyntaxNode) => c.type === '=>'); + if (arrowIdx === -1) continue; + const key = extractStringContent(children[arrowIdx - 1]); + const val = children[arrowIdx + 1]; + if (key === 'middleware') { + if (val?.type === 'string') { + const s = extractStringContent(val); + if (s) ctx.middleware.push(s); + } else if (val?.type === 'array_creation_expression') { + for (const item of val.children ?? []) { + if (item.type === 'array_element_initializer') { + const str = item.children?.find((c: SyntaxNode) => c.type === 'string'); + const s = str ? extractStringContent(str) : null; + if (s) ctx.middleware.push(s); + } + } + } + } else if (key === 'prefix') { + ctx.prefix = extractStringContent(val) ?? null; + } else if (key === 'as' || key === 'name') { + ctx.namePrefix = extractStringContent(val) ?? null; + } else if (key === 'controller') { + if (val?.type === 'class_constant_access_expression') { + ctx.controller = val.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; + } + } + } + } + } + return ctx; +} + +export function extractLaravelRoutes(tree: Parser.Tree, filePath: string): ExtractedRoute[] { + const routes: ExtractedRoute[] = []; + + function resolveStack(stack: RouteGroupContext[]): { + middleware: string[]; + prefix: string | null; + namePrefix: string | null; + controller: string | null; + } { + const middleware: string[] = []; + let prefix: string | null = null; + let namePrefix: string | null = null; + let controller: string | null = null; + for (const ctx of stack) { + middleware.push(...ctx.middleware); + if (ctx.prefix) prefix = prefix ? `${prefix}/${ctx.prefix}`.replace(/\/+/g, '/') : ctx.prefix; + if (ctx.namePrefix) + namePrefix = namePrefix ? `${namePrefix}${ctx.namePrefix}` : ctx.namePrefix; + if (ctx.controller) controller = ctx.controller; + } + return { middleware, prefix, namePrefix, controller }; + } + + function emitRoute( + httpMethod: string, + argsNode: SyntaxNode | null, + lineNumber: number, + groupStack: RouteGroupContext[], + chainAttrs: { method: string; argsNode: SyntaxNode | null }[], + ) { + const effective = resolveStack(groupStack); + let routeName: string | null = null; + + for (const attr of chainAttrs) { + if (attr.method === 'middleware') + effective.middleware.push(...extractMiddlewareArg(attr.argsNode)); + if (attr.method === 'prefix') { + const p = extractFirstStringArg(attr.argsNode); + if (p) effective.prefix = effective.prefix ? `${effective.prefix}/${p}` : p; + } + if (attr.method === 'controller') { + const cls = extractClassArg(attr.argsNode); + if (cls) effective.controller = cls; + } + if (attr.method === 'name') { + routeName = joinRouteName(effective.namePrefix, extractFirstStringArg(attr.argsNode)); + } + } + + const routePath = extractFirstStringArg(argsNode); + + if (ROUTE_RESOURCE_METHODS.has(httpMethod)) { + const target = extractControllerTarget(argsNode); + const actions = httpMethod === 'apiResource' ? API_RESOURCE_ACTIONS : RESOURCE_ACTIONS; + const routeNameBase = + routeName ?? joinRouteName(effective.namePrefix, routeNameBaseFromPath(routePath)); + for (const action of actions) { + routes.push({ + filePath, + httpMethod, + routePath, + routeName: appendResourceActionName(routeNameBase, action), + controllerName: target.controller ?? effective.controller, + methodName: action, + middleware: [...effective.middleware], + prefix: effective.prefix, + lineNumber, + }); + } + } else { + const target = extractControllerTarget(argsNode); + routes.push({ + filePath, + httpMethod, + routePath, + routeName, + controllerName: target.controller ?? effective.controller, + methodName: target.method ?? (effective.controller ? target.bareMethod : null), + middleware: [...effective.middleware], + prefix: effective.prefix, + lineNumber, + }); + } + } + + // Iterative traversal using an explicit stack to avoid V8 call stack overflow + // on deeply nested ASTs (e.g. Go stdlib, large Grafana components). + // Each frame tracks the node and a snapshot of the group stack at that depth. + interface WalkFrame { + node: SyntaxNode; + groupSnapshot: RouteGroupContext[]; + } + + const walkStack: WalkFrame[] = [{ node: tree.rootNode, groupSnapshot: [] }]; + + while (walkStack.length > 0) { + const { node, groupSnapshot } = walkStack.pop()!; + + // Case 1: Simple Route::get(...), Route::post(...), etc. + if (isRouteStaticCall(node)) { + const method = getCallMethodName(node); + if (method && (ROUTE_HTTP_METHODS.has(method) || ROUTE_RESOURCE_METHODS.has(method))) { + emitRoute(method, getArguments(node), node.startPosition.row, groupSnapshot, []); + continue; + } + if (method === 'group') { + const argsNode = getArguments(node); + const groupCtx = parseArrayGroupArgs(argsNode); + const body = findClosureBody(argsNode); + if (body) { + const childSnapshot = [...groupSnapshot, groupCtx]; + const children = body.children ?? []; + for (let i = children.length - 1; i >= 0; i--) { + walkStack.push({ node: children[i], groupSnapshot: childSnapshot }); + } + } + continue; + } + } + + // Case 2: Fluent chain — Route::middleware(...)->group(...) or Route::middleware(...)->get(...) + const chain = unwrapRouteChain(node); + if (chain) { + if (chain.terminalMethod === 'group') { + const groupCtx: RouteGroupContext = { + middleware: [], + prefix: null, + namePrefix: null, + controller: null, + }; + for (const attr of chain.attributes) { + if (attr.method === 'middleware') + groupCtx.middleware.push(...extractMiddlewareArg(attr.argsNode)); + if (attr.method === 'prefix') groupCtx.prefix = extractFirstStringArg(attr.argsNode); + if (attr.method === 'name') groupCtx.namePrefix = extractFirstStringArg(attr.argsNode); + if (attr.method === 'controller') groupCtx.controller = extractClassArg(attr.argsNode); + } + const body = findClosureBody(chain.terminalArgs); + if (body) { + const childSnapshot = [...groupSnapshot, groupCtx]; + const children = body.children ?? []; + for (let i = children.length - 1; i >= 0; i--) { + walkStack.push({ node: children[i], groupSnapshot: childSnapshot }); + } + } + continue; + } + if ( + ROUTE_HTTP_METHODS.has(chain.terminalMethod) || + ROUTE_RESOURCE_METHODS.has(chain.terminalMethod) + ) { + emitRoute( + chain.terminalMethod, + chain.terminalArgs, + node.startPosition.row, + groupSnapshot, + chain.attributes, + ); + continue; + } + const chainedRouteIndex = chain.attributes.findIndex( + (attr) => ROUTE_HTTP_METHODS.has(attr.method) || ROUTE_RESOURCE_METHODS.has(attr.method), + ); + if (chainedRouteIndex >= 0) { + const routeAttr = chain.attributes[chainedRouteIndex]!; + const routeAttrs = [ + ...chain.attributes.slice(0, chainedRouteIndex), + ...chain.attributes.slice(chainedRouteIndex + 1), + { method: chain.terminalMethod, argsNode: chain.terminalArgs }, + ]; + emitRoute( + routeAttr.method, + routeAttr.argsNode, + node.startPosition.row, + groupSnapshot, + routeAttrs, + ); + continue; + } + } + + // Default: push children in reverse so leftmost is processed first + const children = node.children ?? []; + for (let i = children.length - 1; i >= 0; i--) { + walkStack.push({ node: children[i], groupSnapshot }); + } + } + return routes; +} diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index ccb88e0de..69e28b91f 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -53,8 +53,6 @@ import { findObjectLiteralBindingInfo, type EnclosingClassInfo, getLabelFromCaptures, - findDescendant, - extractStringContent, genericFuncName, inferFunctionLabel, CLASS_CONTAINER_TYPES, @@ -87,8 +85,10 @@ import { extractTemplateArguments, templateArgumentsIdTag } from '../utils/templ import type { LanguageProvider } from '../language-provider.js'; import type { ParsedFile } from 'gitnexus-shared'; import { extractParsedFile } from '../scope-extractor-bridge.js'; +import { extractLaravelRoutes, type ExtractedRoute } from '../route-extractors/laravel.js'; import { logger } from '../../logger.js'; +export type { ExtractedRoute } from '../route-extractors/laravel.js'; // ============================================================================ // Types for serializable results // ============================================================================ @@ -192,17 +192,6 @@ export interface ExtractedAssignment { // `ExtractedHeritage` now lives in `../model/heritage-map.ts` and is // re-exported at the top of this file. -export interface ExtractedRoute { - filePath: string; - httpMethod: string; - routePath: string | null; - controllerName: string | null; - methodName: string | null; - middleware: string[]; - prefix: string | null; - lineNumber: number; -} - export interface ExtractedFetchCall { filePath: string; fetchURL: string; @@ -841,29 +830,6 @@ const processBatch = ( return result; }; -// ============================================================================ -// Laravel Route Extraction (procedural AST walk) -// ============================================================================ - -interface RouteGroupContext { - middleware: string[]; - prefix: string | null; - controller: string | null; -} - -const ROUTE_HTTP_METHODS = new Set([ - 'get', - 'post', - 'put', - 'patch', - 'delete', - 'options', - 'any', - 'match', -]); - -const ROUTE_RESOURCE_METHODS = new Set(['resource', 'apiResource']); - // Express/Hono method names that register routes const EXPRESS_ROUTE_METHODS = new Set([ 'get', @@ -925,402 +891,6 @@ const ROUTE_DECORATOR_NAMES = new Set([ 'DeleteMapping', ]); -const RESOURCE_ACTIONS = ['index', 'create', 'store', 'show', 'edit', 'update', 'destroy']; -const API_RESOURCE_ACTIONS = ['index', 'store', 'show', 'update', 'destroy']; - -/** Check if node is a scoped_call_expression with object 'Route' */ -function isRouteStaticCall(node: SyntaxNode): boolean { - if (node.type !== 'scoped_call_expression') return false; - const obj = node.childForFieldName?.('object') ?? node.children?.[0]; - return obj?.text === 'Route'; -} - -/** Get the method name from a scoped_call_expression or member_call_expression */ -function getCallMethodName(node: SyntaxNode): string | null { - const nameNode = - node.childForFieldName?.('name') ?? node.children?.find((c: SyntaxNode) => c.type === 'name'); - return nameNode?.text ?? null; -} - -/** Get the arguments node from a call expression */ -function getArguments(node: SyntaxNode): SyntaxNode | null { - return node.children?.find((c: SyntaxNode) => c.type === 'arguments') ?? null; -} - -/** Find the closure body inside arguments */ -function findClosureBody(argsNode: SyntaxNode | null): SyntaxNode | null { - if (!argsNode) return null; - for (const child of argsNode.children ?? []) { - if (child.type === 'argument') { - for (const inner of child.children ?? []) { - if (inner.type === 'anonymous_function' || inner.type === 'arrow_function') { - return ( - inner.childForFieldName?.('body') ?? - inner.children?.find((c: SyntaxNode) => c.type === 'compound_statement') ?? - null - ); - } - } - } - if (child.type === 'anonymous_function' || child.type === 'arrow_function') { - return ( - child.childForFieldName?.('body') ?? - child.children?.find((c: SyntaxNode) => c.type === 'compound_statement') ?? - null - ); - } - } - return null; -} - -/** Extract first string argument from arguments node */ -function extractFirstStringArg(argsNode: SyntaxNode | null): string | null { - if (!argsNode) return null; - for (const child of argsNode.children ?? []) { - const target = child.type === 'argument' ? child.children?.[0] : child; - if (!target) continue; - if (target.type === 'string' || target.type === 'encapsed_string') { - return extractStringContent(target); - } - } - return null; -} - -/** Extract middleware from arguments — handles string or array */ -function extractMiddlewareArg(argsNode: SyntaxNode | null): string[] { - if (!argsNode) return []; - for (const child of argsNode.children ?? []) { - const target = child.type === 'argument' ? child.children?.[0] : child; - if (!target) continue; - if (target.type === 'string' || target.type === 'encapsed_string') { - const val = extractStringContent(target); - return val ? [val] : []; - } - if (target.type === 'array_creation_expression') { - const items: string[] = []; - for (const el of target.children ?? []) { - if (el.type === 'array_element_initializer') { - const str = el.children?.find( - (c: SyntaxNode) => c.type === 'string' || c.type === 'encapsed_string', - ); - const val = str ? extractStringContent(str) : null; - if (val) items.push(val); - } - } - return items; - } - } - return []; -} - -/** Extract Controller::class from arguments */ -function extractClassArg(argsNode: SyntaxNode | null): string | null { - if (!argsNode) return null; - for (const child of argsNode.children ?? []) { - const target = child.type === 'argument' ? child.children?.[0] : child; - if (target?.type === 'class_constant_access_expression') { - return target.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; - } - } - return null; -} - -/** Extract controller class name from arguments: [Controller::class, 'method'] or 'Controller@method' */ -function extractControllerTarget(argsNode: SyntaxNode | null): { - controller: string | null; - method: string | null; -} { - if (!argsNode) return { controller: null, method: null }; - - const args: (SyntaxNode | undefined)[] = []; - for (const child of argsNode.children ?? []) { - if (child.type === 'argument') args.push(child.children?.[0]); - else if (child.type !== '(' && child.type !== ')' && child.type !== ',') args.push(child); - } - - // Second arg is the handler - const handlerNode = args[1]; - if (!handlerNode) return { controller: null, method: null }; - - // Array syntax: [UserController::class, 'index'] - if (handlerNode.type === 'array_creation_expression') { - let controller: string | null = null; - let method: string | null = null; - const elements: SyntaxNode[] = []; - for (const el of handlerNode.children ?? []) { - if (el.type === 'array_element_initializer') elements.push(el); - } - if (elements[0]) { - const classAccess = findDescendant(elements[0], 'class_constant_access_expression'); - if (classAccess) { - controller = classAccess.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; - } - } - if (elements[1]) { - const str = findDescendant(elements[1], 'string'); - method = str ? extractStringContent(str) : null; - } - return { controller, method }; - } - - // String syntax: 'UserController@index' - if (handlerNode.type === 'string' || handlerNode.type === 'encapsed_string') { - const text = extractStringContent(handlerNode); - if (text?.includes('@')) { - const [controller, method] = text.split('@'); - return { controller, method }; - } - } - - // Class reference: UserController::class (invokable controller) - if (handlerNode.type === 'class_constant_access_expression') { - const controller = - handlerNode.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; - return { controller, method: '__invoke' }; - } - - return { controller: null, method: null }; -} - -interface ChainedRouteCall { - isRouteFacade: boolean; - terminalMethod: string; - attributes: { method: string; argsNode: SyntaxNode | null }[]; - terminalArgs: SyntaxNode | null; - node: SyntaxNode; -} - -/** - * Unwrap a chained call like Route::middleware('auth')->prefix('api')->group(fn) - */ -function unwrapRouteChain(node: SyntaxNode): ChainedRouteCall | null { - if (node.type !== 'member_call_expression') return null; - - const terminalMethod = getCallMethodName(node); - if (!terminalMethod) return null; - - const terminalArgs = getArguments(node); - const attributes: { method: string; argsNode: SyntaxNode | null }[] = []; - - let current = node.children?.[0]; - - while (current) { - if (current.type === 'member_call_expression') { - const method = getCallMethodName(current); - const args = getArguments(current); - if (method) attributes.unshift({ method, argsNode: args }); - current = current.children?.[0]; - } else if (current.type === 'scoped_call_expression') { - const obj = current.childForFieldName?.('object') ?? current.children?.[0]; - if (obj?.text !== 'Route') return null; - - const method = getCallMethodName(current); - const args = getArguments(current); - if (method) attributes.unshift({ method, argsNode: args }); - - return { isRouteFacade: true, terminalMethod, attributes, terminalArgs, node }; - } else { - break; - } - } - - return null; -} - -/** Parse Route::group(['middleware' => ..., 'prefix' => ...], fn) array syntax */ -function parseArrayGroupArgs(argsNode: SyntaxNode | null): RouteGroupContext { - const ctx: RouteGroupContext = { middleware: [], prefix: null, controller: null }; - if (!argsNode) return ctx; - - for (const child of argsNode.children ?? []) { - const target = child.type === 'argument' ? child.children?.[0] : child; - if (target?.type === 'array_creation_expression') { - for (const el of target.children ?? []) { - if (el.type !== 'array_element_initializer') continue; - const children = el.children ?? []; - const arrowIdx = children.findIndex((c: SyntaxNode) => c.type === '=>'); - if (arrowIdx === -1) continue; - const key = extractStringContent(children[arrowIdx - 1]); - const val = children[arrowIdx + 1]; - if (key === 'middleware') { - if (val?.type === 'string') { - const s = extractStringContent(val); - if (s) ctx.middleware.push(s); - } else if (val?.type === 'array_creation_expression') { - for (const item of val.children ?? []) { - if (item.type === 'array_element_initializer') { - const str = item.children?.find((c: SyntaxNode) => c.type === 'string'); - const s = str ? extractStringContent(str) : null; - if (s) ctx.middleware.push(s); - } - } - } - } else if (key === 'prefix') { - ctx.prefix = extractStringContent(val) ?? null; - } else if (key === 'controller') { - if (val?.type === 'class_constant_access_expression') { - ctx.controller = val.children?.find((c: SyntaxNode) => c.type === 'name')?.text ?? null; - } - } - } - } - } - return ctx; -} - -function extractLaravelRoutes(tree: Parser.Tree, filePath: string): ExtractedRoute[] { - const routes: ExtractedRoute[] = []; - - function resolveStack(stack: RouteGroupContext[]): { - middleware: string[]; - prefix: string | null; - controller: string | null; - } { - const middleware: string[] = []; - let prefix: string | null = null; - let controller: string | null = null; - for (const ctx of stack) { - middleware.push(...ctx.middleware); - if (ctx.prefix) prefix = prefix ? `${prefix}/${ctx.prefix}`.replace(/\/+/g, '/') : ctx.prefix; - if (ctx.controller) controller = ctx.controller; - } - return { middleware, prefix, controller }; - } - - function emitRoute( - httpMethod: string, - argsNode: SyntaxNode | null, - lineNumber: number, - groupStack: RouteGroupContext[], - chainAttrs: { method: string; argsNode: SyntaxNode | null }[], - ) { - const effective = resolveStack(groupStack); - - for (const attr of chainAttrs) { - if (attr.method === 'middleware') - effective.middleware.push(...extractMiddlewareArg(attr.argsNode)); - if (attr.method === 'prefix') { - const p = extractFirstStringArg(attr.argsNode); - if (p) effective.prefix = effective.prefix ? `${effective.prefix}/${p}` : p; - } - if (attr.method === 'controller') { - const cls = extractClassArg(attr.argsNode); - if (cls) effective.controller = cls; - } - } - - const routePath = extractFirstStringArg(argsNode); - - if (ROUTE_RESOURCE_METHODS.has(httpMethod)) { - const target = extractControllerTarget(argsNode); - const actions = httpMethod === 'apiResource' ? API_RESOURCE_ACTIONS : RESOURCE_ACTIONS; - for (const action of actions) { - routes.push({ - filePath, - httpMethod, - routePath, - controllerName: target.controller ?? effective.controller, - methodName: action, - middleware: [...effective.middleware], - prefix: effective.prefix, - lineNumber, - }); - } - } else { - const target = extractControllerTarget(argsNode); - routes.push({ - filePath, - httpMethod, - routePath, - controllerName: target.controller ?? effective.controller, - methodName: target.method, - middleware: [...effective.middleware], - prefix: effective.prefix, - lineNumber, - }); - } - } - - // Iterative traversal using an explicit stack to avoid V8 call stack overflow - // on deeply nested ASTs (e.g. Go stdlib, large Grafana components). - // Each frame tracks the node and a snapshot of the group stack at that depth. - interface WalkFrame { - node: SyntaxNode; - groupSnapshot: RouteGroupContext[]; - } - - const walkStack: WalkFrame[] = [{ node: tree.rootNode, groupSnapshot: [] }]; - - while (walkStack.length > 0) { - const { node, groupSnapshot } = walkStack.pop()!; - - // Case 1: Simple Route::get(...), Route::post(...), etc. - if (isRouteStaticCall(node)) { - const method = getCallMethodName(node); - if (method && (ROUTE_HTTP_METHODS.has(method) || ROUTE_RESOURCE_METHODS.has(method))) { - emitRoute(method, getArguments(node), node.startPosition.row, groupSnapshot, []); - continue; - } - if (method === 'group') { - const argsNode = getArguments(node); - const groupCtx = parseArrayGroupArgs(argsNode); - const body = findClosureBody(argsNode); - if (body) { - const childSnapshot = [...groupSnapshot, groupCtx]; - const children = body.children ?? []; - for (let i = children.length - 1; i >= 0; i--) { - walkStack.push({ node: children[i], groupSnapshot: childSnapshot }); - } - } - continue; - } - } - - // Case 2: Fluent chain — Route::middleware(...)->group(...) or Route::middleware(...)->get(...) - const chain = unwrapRouteChain(node); - if (chain) { - if (chain.terminalMethod === 'group') { - const groupCtx: RouteGroupContext = { middleware: [], prefix: null, controller: null }; - for (const attr of chain.attributes) { - if (attr.method === 'middleware') - groupCtx.middleware.push(...extractMiddlewareArg(attr.argsNode)); - if (attr.method === 'prefix') groupCtx.prefix = extractFirstStringArg(attr.argsNode); - if (attr.method === 'controller') groupCtx.controller = extractClassArg(attr.argsNode); - } - const body = findClosureBody(chain.terminalArgs); - if (body) { - const childSnapshot = [...groupSnapshot, groupCtx]; - const children = body.children ?? []; - for (let i = children.length - 1; i >= 0; i--) { - walkStack.push({ node: children[i], groupSnapshot: childSnapshot }); - } - } - continue; - } - if ( - ROUTE_HTTP_METHODS.has(chain.terminalMethod) || - ROUTE_RESOURCE_METHODS.has(chain.terminalMethod) - ) { - emitRoute( - chain.terminalMethod, - chain.terminalArgs, - node.startPosition.row, - groupSnapshot, - chain.attributes, - ); - continue; - } - } - - // Default: push children in reverse so leftmost is processed first - const children = node.children ?? []; - for (let i = children.length - 1; i >= 0; i--) { - walkStack.push({ node: children[i], groupSnapshot }); - } - } - return routes; -} - // ============================================================================ // ORM Query Detection (Prisma + Supabase) // ============================================================================ diff --git a/gitnexus/test/unit/blade-template-routes.test.ts b/gitnexus/test/unit/blade-template-routes.test.ts new file mode 100644 index 000000000..9edd6e791 --- /dev/null +++ b/gitnexus/test/unit/blade-template-routes.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { + extractTemplateStaticFetchCalls, + isTemplateRouteCandidate, + normalizeExtractedRoutePath, + routesPhase, +} from '../../src/core/ingestion/pipeline-phases/routes.js'; +import type { ParseOutput } from '../../src/core/ingestion/pipeline-phases/parse.js'; +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import { generateId } from '../../src/lib/utils.js'; + +describe('Blade/template static route extraction', () => { + it('keeps Blade files as template route candidates', () => { + expect(isTemplateRouteCandidate('resources/views/orders/index.blade.php')).toBe(true); + expect(isTemplateRouteCandidate('resources\\views\\Orders\\INDEX.BLADE.PHP')).toBe(true); + }); + + it('extracts safe static form, href, AJAX, and Blade URL helper URLs', () => { + const calls = extractTemplateStaticFetchCalls( + 'resources/views/orders/index.blade.php', + `
+ History + + Checkout + Raw checkout +
`, + ); + + expect(new Set(calls.map((call) => call.fetchURL))).toEqual( + new Set(['/orders', '/orders/history', '/api/orders', '/checkout', '/checkout/raw']), + ); + expect(new Set(calls.map((call) => call.filePath))).toEqual( + new Set(['resources/views/orders/index.blade.php']), + ); + }); + + it('does not treat Laravel asset helper URLs as route signals', () => { + const calls = extractTemplateStaticFetchCalls( + 'resources/views/layouts/app.blade.php', + ` +`, + ); + + expect(calls).toEqual([]); + }); + + it('resolves parameterless Blade named route helpers from extracted route names', () => { + const calls = extractTemplateStaticFetchCalls( + 'resources/views/auth/login.blade.php', + `Login +
+Missing parameter +Dynamic order`, + new Map([ + ['login', '/login'], + ['log-viewer.login.submit', '/logs/login'], + ['orders.show', '/orders/{order}'], + ]), + ); + + expect(calls.map((call) => call.fetchURL)).toEqual(['/login', '/logs/login']); + }); + + it('does not turn dynamic Blade expressions or parameterized named routes into static URL signals', () => { + const calls = extractTemplateStaticFetchCalls( + 'resources/views/orders/show.blade.php', + `Dynamic +Named route follow-up +Dynamic helper +`, + ); + + expect(calls).toEqual([]); + }); + + it('normalizes Laravel route prefixes before matching template URL signals', () => { + expect(normalizeExtractedRoutePath('/orders', 'admin')).toBe('/admin/orders'); + expect(normalizeExtractedRoutePath('orders', '/admin/')).toBe('/admin/orders'); + expect(normalizeExtractedRoutePath('/', 'admin')).toBe('/admin'); + expect(normalizeExtractedRoutePath('/orders', null)).toBe('/orders'); + }); + + it('links Blade static URL signals to matching route graph nodes without PHP parsing', async () => { + const repoPath = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-blade-routes-')); + try { + await fs.mkdir(path.join(repoPath, 'routes'), { recursive: true }); + await fs.mkdir(path.join(repoPath, 'resources/views/orders'), { recursive: true }); + await fs.writeFile(path.join(repoPath, 'routes/web.php'), ` + Orders + +`, + ); + + const graph = createKnowledgeGraph(); + const parseOutput = { + allPaths: ['routes/web.php', 'resources/views/orders/index.blade.php'], + allFetchCalls: [], + allExtractedRoutes: [ + { + filePath: 'routes/web.php', + httpMethod: 'post', + routePath: '/orders', + routeName: 'admin.orders', + controllerName: null, + methodName: null, + middleware: [], + prefix: 'admin', + lineNumber: 1, + }, + { + filePath: 'routes/web.php', + httpMethod: 'get', + routePath: '/css/app.css', + routeName: 'assets.css', + controllerName: null, + methodName: null, + middleware: [], + prefix: null, + lineNumber: 2, + }, + ], + allDecoratorRoutes: [], + } as unknown as ParseOutput; + + const output = await routesPhase.execute( + { + repoPath, + graph, + onProgress: () => {}, + pipelineStart: Date.now(), + }, + new Map([['parse', { phaseName: 'parse', output: parseOutput, durationMs: 0 }]]), + ); + + expect(output.routeRegistry.get('/admin/orders')).toEqual({ + filePath: 'routes/web.php', + source: 'framework-route', + }); + + const fetchEdges = graph.relationships.filter((rel) => rel.type === 'FETCHES'); + expect(fetchEdges).toHaveLength(1); + expect(fetchEdges.map((rel) => graph.getNode(rel.targetId)?.properties.name)).toEqual([ + '/admin/orders', + ]); + const target = graph.getNode(fetchEdges[0]!.targetId); + expect(fetchEdges[0]!.sourceId).toBe( + generateId('File', 'resources/views/orders/index.blade.php'), + ); + expect(target?.properties.name).toBe('/admin/orders'); + } finally { + await fs.rm(repoPath, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index a1756be3b..000403bce 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -11,6 +11,7 @@ vi.mock('../../../src/core/tree-sitter/safe-parse.js', async () => { }); import { HttpRouteExtractor } from '../../../src/core/group/extractors/http-route-extractor.js'; +import { getPluginForFile } from '../../../src/core/group/extractors/http-patterns/index.js'; import type { RepoHandle } from '../../../src/core/group/types.js'; describe('HttpRouteExtractor', () => { @@ -33,6 +34,13 @@ describe('HttpRouteExtractor', () => { storagePath: path.join(repoPath, '.gitnexus'), }); + describe('plugin selection', () => { + it('does not route Blade templates through the PHP source-scan plugin', () => { + expect(getPluginForFile('resources/views/welcome.blade.php')).toBeUndefined(); + expect(getPluginForFile('routes/web.php')).toBeDefined(); + }); + }); + describe('provider extraction — graph-first (Strategy A)', () => { it('extracts routes from Route/HANDLES_ROUTE graph + source scan for method', async () => { const dir = path.join(tmpDir, 'graph-first'); diff --git a/gitnexus/test/unit/ignore-service.test.ts b/gitnexus/test/unit/ignore-service.test.ts index 1e1908137..cd5ebdb4e 100644 --- a/gitnexus/test/unit/ignore-service.test.ts +++ b/gitnexus/test/unit/ignore-service.test.ts @@ -178,6 +178,12 @@ describe('shouldIgnorePath', () => { it('ignores TypeScript declaration files', () => { expect(shouldIgnorePath('types/index.d.ts')).toBe(true); }); + + it('ignores Laravel compiled Blade view cache files', () => { + expect(shouldIgnorePath('storage/framework/views/1a2b3c.php')).toBe(true); + expect(shouldIgnorePath('project/storage/framework/views/1a2b3c.php')).toBe(true); + expect(shouldIgnorePath('project\\storage\\framework\\views\\1a2b3c.php')).toBe(true); + }); }); describe('Windows path normalization', () => { diff --git a/gitnexus/test/unit/ingestion-utils.test.ts b/gitnexus/test/unit/ingestion-utils.test.ts index c75006a96..c11e716eb 100644 --- a/gitnexus/test/unit/ingestion-utils.test.ts +++ b/gitnexus/test/unit/ingestion-utils.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from 'vitest'; -import { getLanguageFromFilename, SupportedLanguages } from 'gitnexus-shared'; -import { getProvider } from '../../src/core/ingestion/languages/index.js'; +import { + getLanguageFromFilename, + getSyntaxLanguageFromFilename, + isBladeTemplateFilename, + SupportedLanguages, +} from 'gitnexus-shared'; +import { getProvider, getProviderForFile } from '../../src/core/ingestion/languages/index.js'; import type { SyntaxNode } from '../../src/core/ingestion/utils/ast-helpers.js'; import type { NodeLabel } from 'gitnexus-shared'; import type { LanguageProvider } from '../../src/core/ingestion/language-provider.js'; @@ -87,6 +92,24 @@ describe('getLanguageFromFilename', () => { it.each(['.php', '.phtml', '.php3', '.php4', '.php5', '.php8'])('detects %s files', (ext) => { expect(getLanguageFromFilename(`file${ext}`)).toBe(SupportedLanguages.PHP); }); + + it('treats Laravel Blade templates as markup templates, not PHP code files', () => { + expect(getLanguageFromFilename('resources/views/users/index.blade.php')).toBeNull(); + expect(getSyntaxLanguageFromFilename('resources/views/users/index.blade.php')).toBe('markup'); + expect(isBladeTemplateFilename('resources/views/users/index.blade.php')).toBe(true); + }); + + it('recognises Blade templates with Windows paths and case variants', () => { + const file = 'resources\\views\\Users\\INDEX.BLADE.PHP'; + expect(isBladeTemplateFilename(file)).toBe(true); + expect(getLanguageFromFilename(file)).toBeNull(); + expect(getSyntaxLanguageFromFilename(file)).toBe('markup'); + }); + + it('keeps .phtml classified as provider-backed PHP', () => { + expect(getLanguageFromFilename('templates/product/list.phtml')).toBe(SupportedLanguages.PHP); + expect(getSyntaxLanguageFromFilename('templates/product/list.phtml')).toBe('php'); + }); }); describe('Swift', () => { @@ -133,6 +156,21 @@ describe('getLanguageFromFilename', () => { }); }); +describe('getProviderForFile', () => { + it('does not route Blade templates to the PHP provider', () => { + expect(getProviderForFile('resources/views/users/index.blade.php')).toBeNull(); + }); + + it('keeps PHP and PHTML files on the PHP provider', () => { + expect(getProviderForFile('app/Http/Controllers/UserController.php')?.id).toBe( + SupportedLanguages.PHP, + ); + expect(getProviderForFile('vendor/mage-os/templates/product/list.phtml')?.id).toBe( + SupportedLanguages.PHP, + ); + }); +}); + describe('isBuiltInOrNoise', () => { const js = getProvider(SupportedLanguages.JavaScript); const py = getProvider(SupportedLanguages.Python); diff --git a/gitnexus/test/unit/laravel-route-extraction.test.ts b/gitnexus/test/unit/laravel-route-extraction.test.ts new file mode 100644 index 000000000..5f77eaa69 --- /dev/null +++ b/gitnexus/test/unit/laravel-route-extraction.test.ts @@ -0,0 +1,217 @@ +import { describe, expect, it } from 'vitest'; +import Parser from 'tree-sitter'; +import PHP from 'tree-sitter-php'; +import { extractLaravelRoutes } from '../../src/core/ingestion/route-extractors/laravel.js'; + +const parser = new Parser(); +parser.setLanguage(PHP.php_only); + +const extract = (source: string) => + extractLaravelRoutes(parser.parse(source), 'routes/web.php').map((route) => ({ + httpMethod: route.httpMethod, + routePath: route.routePath, + controllerName: route.controllerName, + methodName: route.methodName, + routeName: route.routeName, + middleware: route.middleware, + prefix: route.prefix, + })); + +describe('Laravel route extraction', () => { + it('extracts representative HTTP verb route declarations', () => { + const routes = extract(` { + const routes = extract(` route.routePath === '/photos'); + expect(photos.map((route) => route.methodName)).toEqual([ + 'index', + 'create', + 'store', + 'show', + 'edit', + 'update', + 'destroy', + ]); + expect(new Set(photos.map((route) => route.controllerName))).toEqual( + new Set(['PhotoController']), + ); + expect(photos.map((route) => route.routeName)).toEqual([ + 'photos.index', + 'photos.create', + 'photos.store', + 'photos.show', + 'photos.edit', + 'photos.update', + 'photos.destroy', + ]); + + const apiPhotos = routes.filter((route) => route.routePath === '/api/photos'); + expect(apiPhotos.map((route) => route.methodName)).toEqual([ + 'index', + 'store', + 'show', + 'update', + 'destroy', + ]); + expect(new Set(apiPhotos.map((route) => route.controllerName))).toEqual( + new Set(['ApiPhotoController']), + ); + expect(apiPhotos.map((route) => route.routeName)).toEqual([ + 'api.photos.index', + 'api.photos.store', + 'api.photos.show', + 'api.photos.update', + 'api.photos.destroy', + ]); + }); + + it('threads middleware, prefix, and controller chains into grouped routes', () => { + const routes = extract(` 'api', + 'middleware' => ['auth'], + 'controller' => ApiOrderController::class, +], function () { + Route::post('/orders', 'store'); +}); + +Route::middleware(['auth', 'verified']) + ->prefix('admin') + ->controller(OrderController::class) + ->group(function () { + Route::get('/orders', 'index'); + }); +`); + + expect(routes).toContainEqual( + expect.objectContaining({ + httpMethod: 'get', + routePath: '/orders', + controllerName: 'OrderController', + methodName: 'index', + middleware: ['auth', 'verified'], + prefix: 'admin', + }), + ); + + expect(routes).toContainEqual( + expect.objectContaining({ + httpMethod: 'post', + routePath: '/orders', + controllerName: 'ApiOrderController', + methodName: 'store', + middleware: ['auth'], + prefix: 'api', + }), + ); + + expect(routes).toContainEqual( + expect.objectContaining({ + httpMethod: 'get', + routePath: '/loose', + controllerName: null, + methodName: null, + }), + ); + }); + + it('extracts named routes from fluent routes and named groups', () => { + const routes = extract(`name('login'); +Route::post('/logout', [AuthController::class, 'logout'])->middleware('auth')->name('logout'); + +Route::name('admin.') + ->prefix('admin') + ->group(function () { + Route::post('/settings/cache', [SettingsController::class, 'refresh']) + ->name('settings.refresh-cache'); + }); + +Route::group([ + 'as' => 'log-viewer.', + 'prefix' => 'logs', +], function () { + Route::post('/login', [LogViewerController::class, 'login']) + ->name('login.submit'); +}); +`); + + expect(routes).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + httpMethod: 'get', + routePath: '/login', + routeName: 'login', + }), + expect.objectContaining({ + httpMethod: 'post', + routePath: '/logout', + routeName: 'logout', + middleware: ['auth'], + }), + expect.objectContaining({ + httpMethod: 'post', + routePath: '/settings/cache', + routeName: 'admin.settings.refresh-cache', + prefix: 'admin', + }), + expect.objectContaining({ + httpMethod: 'post', + routePath: '/login', + routeName: 'log-viewer.login.submit', + prefix: 'logs', + }), + ]), + ); + }); +}); diff --git a/gitnexus/test/unit/php-template-scope.test.ts b/gitnexus/test/unit/php-template-scope.test.ts new file mode 100644 index 000000000..937dda1b3 --- /dev/null +++ b/gitnexus/test/unit/php-template-scope.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest'; +import { SupportedLanguages, getLanguageFromFilename } from 'gitnexus-shared'; +import { getProviderForFile } from '../../src/core/ingestion/languages/index.js'; +import { extractParsedFile } from '../../src/core/ingestion/scope-extractor-bridge.js'; + +const parseWithWarnings = (filePath: string, source: string) => { + const provider = getProviderForFile(filePath); + expect(provider?.id).toBe(SupportedLanguages.PHP); + + const warnings: string[] = []; + const parsed = extractParsedFile(provider!, source, filePath, (message) => { + warnings.push(message); + }); + + return { parsed, warnings }; +}; + +describe('PHP template scope extraction', () => { + it('keeps Magento/Mage-OS PHTML templates on PHP scope extraction without module warnings', () => { + const filePath = 'vendor/mage-os/module-catalog/view/frontend/templates/product/list.phtml'; + const source = `
getChildHtml() ?>
+escapeHtml($title); ?>`; + + expect(getLanguageFromFilename(filePath)).toBe(SupportedLanguages.PHP); + + const { parsed, warnings } = parseWithWarnings(filePath, source); + + expect(warnings).toEqual([]); + expect(parsed?.scopes[0]?.kind).toBe('Module'); + expect(parsed?.referenceSites).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'getChildHtml', + kind: 'call', + callForm: 'member', + explicitReceiver: { name: '$block' }, + }), + expect.objectContaining({ + name: 'escapeHtml', + kind: 'call', + callForm: 'member', + explicitReceiver: { name: '$escaper' }, + }), + ]), + ); + }); + + it('keeps namespace-less PHP config files module-scoped without module warnings', () => { + const filePath = 'config/database.php'; + const source = ` env('DB_CONNECTION', 'mysql'), +];`; + + expect(getLanguageFromFilename(filePath)).toBe(SupportedLanguages.PHP); + + const { parsed, warnings } = parseWithWarnings(filePath, source); + + expect(warnings).toEqual([]); + expect(parsed?.scopes[0]?.kind).toBe('Module'); + expect(parsed?.referenceSites).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: 'env', + kind: 'call', + callForm: 'free', + arity: 2, + }), + ]), + ); + }); +}); From eb69f667aba83c013a42555d582b2f76957da1c7 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sun, 24 May 2026 07:34:40 +0100 Subject: [PATCH 02/10] feat(cpp): Add structured resolver suppression outcomes (#1785) --- gitnexus/src/core/ingestion/pipeline.ts | 6 + .../passes/free-call-fallback.ts | 123 ++++++++++++++++-- .../passes/receiver-bound-calls.ts | 82 ++++++++++++ .../scope-resolution/pipeline/phase.ts | 9 ++ .../scope-resolution/pipeline/run.ts | 18 +++ .../scope-resolution/resolution-outcome.ts | 34 +++++ gitnexus/src/types/pipeline.ts | 7 + .../test/integration/resolvers/cpp.test.ts | 58 +++++++++ .../test/integration/resolvers/helpers.ts | 9 ++ 9 files changed, 336 insertions(+), 10 deletions(-) create mode 100644 gitnexus/src/core/ingestion/scope-resolution/resolution-outcome.ts diff --git a/gitnexus/src/core/ingestion/pipeline.ts b/gitnexus/src/core/ingestion/pipeline.ts index ec5828b3b..8506b4226 100644 --- a/gitnexus/src/core/ingestion/pipeline.ts +++ b/gitnexus/src/core/ingestion/pipeline.ts @@ -34,6 +34,7 @@ import { mroPhase, communitiesPhase, processesPhase, + type ScopeResolutionOutput, type PipelinePhase, type CommunitiesOutput, type ProcessesOutput, @@ -182,6 +183,10 @@ export const runPipelineFromRepo = async ( let communityResult: CommunitiesOutput['communityResult'] | undefined; let processResult: ProcessesOutput['processResult'] | undefined; + const resolutionOutcomes = getPhaseOutput( + results, + 'scopeResolution', + ).resolutionOutcomes; if (!options?.skipGraphPhases) { communityResult = getPhaseOutput(results, 'communities').communityResult; @@ -208,6 +213,7 @@ export const runPipelineFromRepo = async ( totalFileCount: totalFiles, communityResult, processResult, + resolutionOutcomes, usedWorkerPool, }; }; diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index 0d537db07..fcfb6ef32 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -30,6 +30,10 @@ import type { SemanticModel } from '../../model/semantic-model.js'; import type { WorkspaceResolutionIndex } from '../workspace-index.js'; import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; import type { ScopeResolver } from '../contract/scope-resolver.js'; +import type { + ResolutionOutcomeRecorder, + ResolutionSuppressionReason, +} from '../resolution-outcome.js'; import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js'; import { findAllCallableBindingsInScope, @@ -79,6 +83,7 @@ export function emitFreeCallFallback( * fail at the call site. Three-valued; `'unknown'` keeps the * candidate (monotonicity). */ readonly constraintCompatibility?: ScopeResolver['constraintCompatibility']; + readonly recordResolutionOutcome?: ResolutionOutcomeRecorder; } = {}, ): number { let emitted = 0; @@ -152,9 +157,18 @@ export function emitFreeCallFallback( // Cross-file candidates are shadowing; keep first-match. const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); if (sameFile) { - handledSites.add( - `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`, - ); + recordSuppressedOutcome(options.recordResolutionOutcome, { + phase: 'free-call-fallback', + filePath: parsed.filePath, + name: site.name, + range: site.atRange, + reason: suppressionReasonForOverload(narrowed, site.arity, { + conversionRankFn: options.conversionRankFn, + argumentTypes: site.argumentTypes, + }), + candidates: narrowed, + }); + handledSites.add(siteKey(parsed.filePath, site)); continue; } } @@ -186,7 +200,19 @@ export function emitFreeCallFallback( parsedFiles, ); - const siteKey = `${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; + const key = siteKey(parsed.filePath, site); + if (adlSuppressed && ordinary.length === 0) { + recordSuppressedOutcome(options.recordResolutionOutcome, { + phase: 'free-call-fallback', + filePath: parsed.filePath, + name: site.name, + range: site.atRange, + reason: 'adl-ordinary-lookup-blocked', + candidates: ordinary, + }); + handledSites.add(key); + continue; + } if (adl === undefined || adl.length === 0) { // No ADL contribution. Default behavior: `ordinary[0]` — // scope-chain walk preserves local-shadows-import precedence. @@ -210,7 +236,7 @@ export function emitFreeCallFallback( if (narrowed.length === 1) { fnDef = narrowed[0]; } else if (narrowed.length === 0) { - handledSites.add(siteKey); + handledSites.add(key); continue; } else { // >1 survivors: same-file → suppress (true overloads, @@ -219,7 +245,18 @@ export function emitFreeCallFallback( // first-match (shadowing semantics). const sameFile = narrowed.every((d) => d.filePath === narrowed[0]!.filePath); if (sameFile) { - handledSites.add(siteKey); + recordSuppressedOutcome(options.recordResolutionOutcome, { + phase: 'free-call-fallback', + filePath: parsed.filePath, + name: site.name, + range: site.atRange, + reason: suppressionReasonForOverload(narrowed, site.arity, { + conversionRankFn: options.conversionRankFn, + argumentTypes: site.argumentTypes, + }), + candidates: narrowed, + }); + handledSites.add(key); continue; } fnDef = ordinary[0]; @@ -246,16 +283,27 @@ export function emitFreeCallFallback( if (narrowed.length === 1) { fnDef = narrowed[0]; } else if (narrowed.length === 0) { - handledSites.add(siteKey); + handledSites.add(key); continue; } else if (narrowed.length > 1) { + recordSuppressedOutcome(options.recordResolutionOutcome, { + phase: 'free-call-fallback', + filePath: parsed.filePath, + name: site.name, + range: site.atRange, + reason: suppressionReasonForOverload(narrowed, site.arity, { + conversionRankFn: options.conversionRankFn, + argumentTypes: site.argumentTypes, + }), + candidates: narrowed, + }); if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { - handledSites.add(siteKey); + handledSites.add(key); continue; } // Multiple survivors remain after conversion-rank scoring; // suppress instead of picking arbitrarily. - handledSites.add(siteKey); + handledSites.add(key); continue; } } @@ -295,7 +343,7 @@ export function emitFreeCallFallback( // Always mark the site as handled — even when the dedup-collapse // means we don't add a new edge — so `emit-references` skips its // potentially-wrong fallback for the same site. - handledSites.add(`${parsed.filePath}:${site.atRange.startLine}:${site.atRange.startCol}`); + handledSites.add(siteKey(parsed.filePath, site)); const relId = `rel:CALLS:${callerGraphId}->${tgtGraphId}`; if (seen.has(relId)) continue; seen.add(relId); @@ -315,6 +363,61 @@ export function emitFreeCallFallback( return emitted; } +function siteKey( + filePath: string, + site: { readonly atRange: { readonly startLine: number; readonly startCol: number } }, +): string { + return `${filePath}:${site.atRange.startLine}:${site.atRange.startCol}`; +} + +function suppressionReasonForOverload( + candidates: readonly SymbolDefinition[], + arity: number | undefined, + ctx: { + readonly conversionRankFn?: ConversionRankFn; + readonly argumentTypes?: readonly string[]; + }, +): ResolutionSuppressionReason { + if (isOverloadAmbiguousAfterNormalization(candidates, arity)) { + return 'overload-ambiguous-normalization'; + } + if ( + ctx.conversionRankFn !== undefined && + ctx.argumentTypes !== undefined && + ctx.argumentTypes.length > 0 + ) { + return 'conversion-rank-tied'; + } + return 'overload-ambiguous'; +} + +function recordSuppressedOutcome( + record: ResolutionOutcomeRecorder | undefined, + input: { + readonly phase: string; + readonly filePath: string; + readonly name: string; + readonly range: { + readonly startLine: number; + readonly startCol: number; + readonly endLine: number; + readonly endCol: number; + }; + readonly reason: ResolutionSuppressionReason; + readonly candidates: readonly SymbolDefinition[]; + }, +): void { + record?.({ + kind: 'suppressed', + phase: input.phase, + filePath: input.filePath, + name: input.name, + range: input.range, + reason: input.reason, + candidateIds: input.candidates.map((d) => d.nodeId), + }); +} + /** * Build a `simpleName -> callable defs` index from `scopes.defs` once per * pass. Mirrors the filter the old per-site scan applied: Function / diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index ac7164c70..59b6c323d 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -65,6 +65,10 @@ import { extractTemplateArguments, stripTemplateArguments, } from '../../utils/template-arguments.js'; +import type { + ResolutionOutcomeRecorder, + ResolutionSuppressionReason, +} from '../resolution-outcome.js'; /** Subset of `ScopeResolver` consumed by this pass. Accepting the * subset rather than the full provider keeps tests and partial @@ -140,6 +144,9 @@ export function emitReceiverBoundCalls( provider: ReceiverBoundProviderSubset, index: WorkspaceResolutionIndex, model: SemanticModel, + options: { + readonly recordResolutionOutcome?: ResolutionOutcomeRecorder; + } = {}, ): number { let emitted = 0; // Per-pass dedup so the multiple cases don't double-emit if two of @@ -498,6 +505,15 @@ export function emitReceiverBoundCalls( if (memberDef === 'ambiguous') { // Same-name ambiguity across inline-namespace children (#1564): // suppress edge emission, mark site handled. + options.recordResolutionOutcome?.({ + kind: 'suppressed', + phase: 'receiver-bound-calls', + filePath: parsed.filePath, + name: site.name, + range: site.atRange, + reason: 'inline-ns-ambiguous', + candidateIds: [], + }); handledSites.add(siteKey); continue; } @@ -702,6 +718,7 @@ export function emitReceiverBoundCalls( const chain = [ownerDef.nodeId, ...scopes.methodDispatch.mroFor(ownerDef.nodeId)]; let memberDef: SymbolDefinition | undefined; let ambiguous = false; + let ambiguousOwnerId: string | undefined; // Track whether the chain walk filtered out any static-only // candidates. When it did and the chain ended with no // legitimate instance member, we mark the site as handled so @@ -733,6 +750,7 @@ export function emitReceiverBoundCalls( const picked = pickFirstNonStaticOnly(ownerId, memberName, site, model, provider); if (picked === OVERLOAD_AMBIGUOUS) { ambiguous = true; + ambiguousOwnerId = ownerId; break; } if (picked === STATIC_ONLY_FILTERED) { @@ -754,6 +772,15 @@ export function emitReceiverBoundCalls( // Suppress and mark handled so `emitReferencesViaLookup` // doesn't re-emit the pre-resolved reference. See // OVERLOAD_AMBIGUOUS docstring for the upstream cause. + recordReceiverOverloadSuppression( + options.recordResolutionOutcome, + parsed.filePath, + site, + ambiguousOwnerId ?? ownerDef.nodeId, + memberName, + model, + provider, + ); handledSites.add(siteKey); continue; } @@ -830,6 +857,15 @@ export function emitReceiverBoundCalls( resolveDefGraphId(valueDef.filePath, valueDef, nodeLookup) ?? valueDef.nodeId; const picked = pickOverload(ownerGraphId, memberName, site, model, provider); if (picked === OVERLOAD_AMBIGUOUS) { + recordReceiverOverloadSuppression( + options.recordResolutionOutcome, + parsed.filePath, + site, + ownerGraphId, + memberName, + model, + provider, + ); handledSites.add(siteKey); continue; } @@ -1017,3 +1053,49 @@ function pickFirstNonStaticOnly( if (candidates.length > 1) return OVERLOAD_AMBIGUOUS; return candidates[0] ?? overloads[0]; } + +function recordReceiverOverloadSuppression( + record: ResolutionOutcomeRecorder | undefined, + filePath: string, + site: ParsedFile['referenceSites'][number], + ownerId: string, + memberName: string, + model: SemanticModel, + provider: ReceiverBoundProviderSubset, +): void { + if (record === undefined) return; + const overloads = model.methods.lookupAllByOwner(ownerId, memberName); + const candidates = narrowOverloadCandidates(overloads, site.arity, site.argumentTypes, { + argumentTypeClasses: site.argumentTypeClasses, + conversionRankFn: provider.conversionRankFn, + constraintCompatibility: provider.constraintCompatibility, + }); + const reason: ResolutionSuppressionReason = isOverloadAmbiguousAfterNormalization( + candidates, + site.arity, + ) + ? 'overload-ambiguous-normalization' + : hasConversionRankingSignal(site, provider) + ? 'conversion-rank-tied' + : 'overload-ambiguous'; + record({ + kind: 'suppressed', + phase: 'receiver-bound-calls', + filePath, + name: site.name, + range: site.atRange, + reason, + candidateIds: candidates.map((d) => d.nodeId), + }); +} + +function hasConversionRankingSignal( + site: ParsedFile['referenceSites'][number], + provider: ReceiverBoundProviderSubset, +): boolean { + return ( + provider.conversionRankFn !== undefined && + site.argumentTypes !== undefined && + site.argumentTypes.length > 0 + ); +} diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts index 98a9f8994..31b712d78 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts @@ -37,6 +37,7 @@ import { readFileContents } from '../../filesystem-walker.js'; import { runScopeResolution } from './run.js'; import { SCOPE_RESOLVERS } from './registry.js'; import { isDev, isSemanticModelValidatorEnabled } from '../../utils/env.js'; +import type { ResolutionOutcome } from '../resolution-outcome.js'; import { logger } from '../../../logger.js'; export interface ScopeResolutionOutput { @@ -48,6 +49,8 @@ export interface ScopeResolutionOutput { readonly importsEmitted: number; /** Reference (CALLS / ACCESSES / INHERITS / USES) edges emitted. */ readonly referenceEdgesEmitted: number; + /** Additive stream of resolver diagnostics; does not affect graph edges. */ + readonly resolutionOutcomes: readonly ResolutionOutcome[]; /** Per-language breakdown for telemetry / shadow-parity. */ readonly perLanguage: ReadonlyMap< SupportedLanguages, @@ -64,6 +67,7 @@ const NOOP_OUTPUT: ScopeResolutionOutput = Object.freeze({ filesProcessed: 0, importsEmitted: 0, referenceEdgesEmitted: 0, + resolutionOutcomes: [], perLanguage: new Map(), }); @@ -116,6 +120,7 @@ export const scopeResolutionPhase: PipelinePhase = { let totalImports = 0; let totalRefs = 0; let anyRan = false; + const resolutionOutcomes: ResolutionOutcome[] = []; const perLanguage = new Map< SupportedLanguages, { @@ -156,6 +161,9 @@ export const scopeResolutionPhase: PipelinePhase = { treeCache: scopeTreeCache, resolutionConfig, preExtractedParsedFiles: preExtractedByPath, + recordResolutionOutcome: (outcome) => { + resolutionOutcomes.push(outcome); + }, onWarn: (msg) => { if (isSemanticModelValidatorEnabled()) { logger.warn(`[scope-resolution:${lang}] ${msg}`); @@ -197,6 +205,7 @@ export const scopeResolutionPhase: PipelinePhase = { filesProcessed: totalFiles, importsEmitted: totalImports, referenceEdgesEmitted: totalRefs, + resolutionOutcomes, perLanguage, }; }, diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index ae6ec5a23..777cdb639 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -44,6 +44,7 @@ import { emitImportEdges } from '../graph-bridge/imports-to-edges.js'; import type { ScopeResolver } from '../contract/scope-resolver.js'; import { findClassBindingInScope, findEnclosingClassDef } from '../scope/walkers.js'; import { buildWorkspaceResolutionIndex } from '../workspace-index.js'; +import type { ResolutionOutcome, ResolutionOutcomeRecorder } from '../resolution-outcome.js'; import { logger } from '../../../logger.js'; @@ -161,6 +162,11 @@ interface RunScopeResolutionInput { * Cache miss is safe — falls back to fresh extract. */ readonly preExtractedParsedFiles?: ReadonlyMap; + /** + * Optional additive diagnostics sink. Resolver passes call this when they + * intentionally suppress an edge; the graph remains unchanged. + */ + readonly recordResolutionOutcome?: ResolutionOutcomeRecorder; } interface RunScopeResolutionStats { @@ -170,6 +176,7 @@ interface RunScopeResolutionStats { readonly resolve: ResolveStats; readonly referenceEdgesEmitted: number; readonly referenceSkipped: number; + readonly resolutionOutcomes: readonly ResolutionOutcome[]; } export function runScopeResolution( @@ -178,6 +185,11 @@ export function runScopeResolution( ): RunScopeResolutionStats { const { graph, files } = input; const onWarn = input.onWarn ?? (() => {}); + const resolutionOutcomes: ResolutionOutcome[] = []; + const recordResolutionOutcome: ResolutionOutcomeRecorder = (outcome) => { + resolutionOutcomes.push(outcome); + input.recordResolutionOutcome?.(outcome); + }; const PROF = process.env.PROF_SCOPE_RESOLUTION === '1'; const tStart = PROF ? process.hrtime.bigint() : 0n; let fileContents: Map | undefined; @@ -248,6 +260,7 @@ export function runScopeResolution( resolve: { sitesProcessed: 0, referencesEmitted: 0, unresolved: 0 }, referenceEdgesEmitted: 0, referenceSkipped: 0, + resolutionOutcomes, }; } @@ -359,6 +372,9 @@ export function runScopeResolution( provider, workspaceIndex, readonlyModel, + { + recordResolutionOutcome, + }, ); const unresolvedReceiverExtras = provider.emitUnresolvedReceiverEdges !== undefined @@ -387,6 +403,7 @@ export function runScopeResolution( resolveAdlCandidates: provider.resolveAdlCandidates, conversionRankFn: provider.conversionRankFn, constraintCompatibility: provider.constraintCompatibility, + recordResolutionOutcome, }, ); const { emitted, skipped } = emitReferencesViaLookup( @@ -424,5 +441,6 @@ export function runScopeResolution( resolve: resolveStats, referenceEdgesEmitted: emitted + receiverExtras + unresolvedReceiverExtras + freeCallExtras, referenceSkipped: skipped, + resolutionOutcomes, }; } diff --git a/gitnexus/src/core/ingestion/scope-resolution/resolution-outcome.ts b/gitnexus/src/core/ingestion/scope-resolution/resolution-outcome.ts new file mode 100644 index 000000000..4eabfa29b --- /dev/null +++ b/gitnexus/src/core/ingestion/scope-resolution/resolution-outcome.ts @@ -0,0 +1,34 @@ +import type { Range } from 'gitnexus-shared'; + +export type ResolutionSuppressionReason = + | 'adl-ordinary-lookup-blocked' + | 'conversion-rank-tied' + | 'inline-ns-ambiguous' + | 'overload-ambiguous' + | 'overload-ambiguous-normalization'; + +export type ResolutionOutcome = + | { + readonly kind: 'resolved'; + readonly targetId: string; + readonly phase: string; + readonly filePath: string; + readonly name: string; + readonly range: Range; + } + | { + readonly kind: 'suppressed'; + readonly reason: ResolutionSuppressionReason; + /** + * Scope-resolution definition IDs considered by the suppression decision. + * For `inline-ns-ambiguous` this is currently empty because the + * qualified namespace resolver returns only an `ambiguous` sentinel. + */ + readonly candidateIds: readonly string[]; + readonly phase: string; + readonly filePath: string; + readonly name: string; + readonly range: Range; + }; + +export type ResolutionOutcomeRecorder = (outcome: ResolutionOutcome) => void; diff --git a/gitnexus/src/types/pipeline.ts b/gitnexus/src/types/pipeline.ts index 331bdd6fc..becf81757 100644 --- a/gitnexus/src/types/pipeline.ts +++ b/gitnexus/src/types/pipeline.ts @@ -1,6 +1,7 @@ import type { KnowledgeGraph } from '../core/graph/types.js'; import { CommunityDetectionResult } from '../core/ingestion/community-processor.js'; import { ProcessDetectionResult } from '../core/ingestion/process-processor.js'; +import type { ResolutionOutcome } from '../core/ingestion/scope-resolution/resolution-outcome.js'; // CLI-specific: in-memory result with graph + detection results export interface PipelineResult { @@ -11,6 +12,12 @@ export interface PipelineResult { totalFileCount: number; communityResult?: CommunityDetectionResult; processResult?: ProcessDetectionResult; + /** + * Additive diagnostics for registry-primary resolution decisions that + * deliberately suppress edge emission. Empty means no diagnostic was + * produced; graph edge semantics are unchanged. + */ + resolutionOutcomes: readonly ResolutionOutcome[]; /** * True if the parse phase spawned a worker pool for this run. False means * the sequential fallback handled every chunk. Primarily a test affordance diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index 90bc75117..e33fa6632 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -9,6 +9,7 @@ import { getRelationships, getNodesByLabel, getNodesByLabelFull, + getResolutionOutcomes, edgeSet, runPipelineFromRepo, createResolverParityIt, @@ -1819,6 +1820,21 @@ describe('C++ ambiguous integer-width overloads', () => { // GitNexus does not have. The resolver must suppress entirely. expect(processCalls.length).toBe(0); }); + + it('records a structured suppression reason for normalization ambiguity', () => { + const outcomes = getResolutionOutcomes(result).filter( + (o) => + o.kind === 'suppressed' && + o.name === 'process' && + o.phase === 'receiver-bound-calls' && + o.filePath.endsWith('caller.cpp') && + o.reason === 'overload-ambiguous-normalization', + ); + + expect(outcomes.length).toBeGreaterThan(0); + expect(outcomes[0]?.candidateIds.length).toBe(2); + expect(outcomes[0]?.range.startLine).toBeGreaterThan(0); + }); }); // --------------------------------------------------------------------------- @@ -1893,6 +1909,20 @@ describe('C++ overload resolution — conversion-rank disambiguation (#1578)', ( // Contract: zero edges for ALL h() call sites combined (dedup). expect(hCalls.length).toBe(0); }); + + it('records a structured suppression reason for conversion-rank ties', () => { + const outcomes = getResolutionOutcomes(result).filter( + (o) => + o.kind === 'suppressed' && + o.name === 'h' && + o.phase === 'free-call-fallback' && + o.reason === 'conversion-rank-tied', + ); + + expect(outcomes.length).toBeGreaterThan(0); + expect(outcomes[0]?.candidateIds.length).toBe(2); + expect(outcomes[0]?.range.startLine).toBeGreaterThan(0); + }); }); // C++ overload resolution: pointer/nullptr/ellipsis conversion ranks (#1637) @@ -2722,6 +2752,20 @@ describe('C++ ADL — non-function ordinary lookup suppresses ADL', () => { // `e` is audit::Event, audit::record should NOT be discovered. expect(recordCalls.length).toBe(0); }); + + it('records a structured suppression reason for ADL blocker lookup', () => { + const outcomes = getResolutionOutcomes(result).filter( + (o) => + o.kind === 'suppressed' && + o.name === 'record' && + o.phase === 'free-call-fallback' && + o.reason === 'adl-ordinary-lookup-blocked', + ); + + expect(outcomes.length).toBeGreaterThan(0); + expect(outcomes[0]?.candidateIds.length).toBe(0); + expect(outcomes[0]?.range.startLine).toBeGreaterThan(0); + }); }); describe('C++ ADL — inner callable + outer non-callable: ADL not suppressed', () => { @@ -2984,6 +3028,20 @@ describe('C++ inline namespace — ambiguous same-name across inline children (# // the same name. The resolver must suppress rather than pick arbitrarily. expect(fooCalls.length).toBe(0); }); + + it('records a structured suppression reason for inline namespace ambiguity', () => { + const outcomes = getResolutionOutcomes(result).filter( + (o) => + o.kind === 'suppressed' && + o.name === 'foo' && + o.phase === 'receiver-bound-calls' && + o.reason === 'inline-ns-ambiguous', + ); + + expect(outcomes.length).toBeGreaterThan(0); + expect(outcomes[0]?.candidateIds.length).toBe(0); + expect(outcomes[0]?.range.startLine).toBeGreaterThan(0); + }); }); describe('C++ inline namespace — ambiguous distinct signatures (conservative suppress)', () => { diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index 74152c6ea..f599835b5 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -218,6 +218,7 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly { From a8a8a3710d11fe8c5a11ddb9f5e37ece75270564 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9on=20Simmons?= Date: Sun, 24 May 2026 03:05:27 -0400 Subject: [PATCH 03/10] fix(lbug): skip init lock and filesystem mutations for read-only opens (#1783) (#1784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `doInitLbug` unconditionally called `acquireInitLock`, which creates `${dbPath}.init.lock` inside the workspace. On a Docker `:ro` bind mount this fails with EROFS. The init lock prevents a TOCTOU race during DB creation — read-only opens never create databases and don't need it. Split the init path: - Read-only: skip path cleanup, init lock, orphan sidecar removal, and mkdir. Go straight to preflightLbugSidecars (allowQuarantine: false) then openLbugConnection with readOnly: true. - Writable: unchanged behavior (lock, cleanup, open). - Shadow-replay recovery: catch EROFS/EACCES/EPERM from the writable fallback in ensureReadOnlyConnectionUsable and surface an actionable error instead of a raw filesystem exception. Includes integration test verifying read-only open never creates lbug.init.lock on disk. Fixes #1783 Co-authored-by: Gergő Magyar --- gitnexus/src/core/lbug/lbug-adapter.ts | 195 ++++++++++-------- .../integration/lbug-readonly-init.test.ts | 29 +++ 2 files changed, 143 insertions(+), 81 deletions(-) create mode 100644 gitnexus/test/integration/lbug-readonly-init.test.ts diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 5e2a34601..b0f1d3ec0 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -525,6 +525,7 @@ const ensureReadOnlyConnectionUsable = async ( dbPath: string, handle: LbugConnectionHandle, ): Promise => { + let shadowReplayErr: unknown; try { await queryAndDrain(handle.conn, READ_ONLY_SHADOW_REPLAY_PROBE); return handle; @@ -537,11 +538,25 @@ const ensureReadOnlyConnectionUsable = async ( await closeLbugConnection(handle); throw err; } + shadowReplayErr = err; } await closeLbugConnection(handle); - const writable = await openLbugConnection(lbug, dbPath); + let writable: LbugConnectionHandle; + try { + writable = await openLbugConnection(lbug, dbPath); + } catch (openErr) { + const code = extractErrnoCode(openErr); + if (code === 'EROFS' || code === 'EACCES' || code === 'EPERM') { + throw new Error( + shadowSidecarRecoveryMessage(dbPath, shadowReplayErr) + + '\n The workspace appears to be read-only — mount it read-write to perform shadow replay recovery,' + + ' or re-run `gitnexus analyze` on a writable filesystem to rebuild the index.', + ); + } + throw openErr; + } let missingShadowError: unknown; try { await queryAndDrain(writable.conn, READ_ONLY_SHADOW_REPLAY_PROBE); @@ -691,94 +706,112 @@ const doInitLbug = async (dbPath: string, readOnly: boolean = false) => { ensuredFTSIndexes.clear(); } - // LadybugDB stores the database as a single file (not a directory). - // If the path already exists, it must be a valid LadybugDB database file. - // Remove stale empty directories or files from older versions. - try { - const stat = await fs.lstat(dbPath); - if (stat.isSymbolicLink()) { - // Never follow symlinks — just remove the link itself - await fs.unlink(dbPath); - } else if (stat.isDirectory()) { - // Verify path is within expected storage directory before deleting - const realPath = await fs.realpath(dbPath); - const parentDir = path.dirname(dbPath); - const realParent = await fs.realpath(parentDir); - if (!realPath.startsWith(realParent + path.sep) && realPath !== realParent) { - throw new Error( - `Refusing to delete ${dbPath}: resolved path ${realPath} is outside storage directory`, - ); - } - // Old-style directory database or empty leftover - remove it - await fs.rm(dbPath, { recursive: true, force: true }); - } - // If it's a file, assume it's an existing LadybugDB database - LadybugDB will open it - } catch (err) { - if (!isMissingFileError(err)) { - throw err; - } - // Path doesn't exist, which is what LadybugDB wants for a new database - } - // --------------------------------------------------------------------------- - // Cross-process critical section: acquire init lock, clean orphan sidecars, - // and open the database. The lock prevents a TOCTOU race where another - // process could create a fresh DB between our access() check and the - // unlink() of stale sidecars. + // Read-only fast path: skip all filesystem mutations (path cleanup, init + // lock, orphan sidecar removal, mkdir) so the open succeeds on read-only + // filesystems such as Docker `:ro` bind mounts. The init lock exists to + // prevent a TOCTOU race during DB *creation* — read-only opens never + // create databases and don't need the lock. // --------------------------------------------------------------------------- - const releaseInitLock = await acquireInitLock(dbPath); - try { - // Crash-recovery cleanup: if the main DB file is missing, stale sidecars - // from an interrupted run can block fresh opens indefinitely. - try { - await fs.access(dbPath); - } catch (err) { - if (isMissingFileError(err)) { - // `.shadow` is documented by LadybugDB checkpointing and `.wal.checkpoint` - // was observed in the #1618 crash loop that motivated this recovery path. - const orphanSidecars = [`${dbPath}.shadow`, `${dbPath}.wal.checkpoint`]; - for (const sidecar of orphanSidecars) { - try { - await fs.unlink(sidecar); - logger.warn( - `GitNexus: removed orphan sidecar ${path.basename(sidecar)} (no main DB file present)`, - ); - } catch (err) { - if (isMissingFileError(err)) { - continue; - } - const code = extractErrnoCode(err); - logger.warn( - `GitNexus: failed to remove orphan sidecar ${path.basename(sidecar)} (${code ?? 'UNKNOWN'}) while main DB file is missing; LadybugDB open may still fail: ${summarizeError(err)}`, - ); - } - } - } else { - const code = extractErrnoCode(err); - logger.warn( - `GitNexus: unable to verify main DB file before orphan sidecar cleanup (${code ?? 'UNKNOWN'}); skipping cleanup: ${summarizeError(err)}`, - ); - } - } - - // Ensure parent directory exists - const parentDir = path.dirname(dbPath); - await fs.mkdir(parentDir, { recursive: true }); + if (readOnly) { await preflightLbugSidecars(dbPath, { - mode: readOnly ? 'read-only' : 'write', + mode: 'read-only', logger, - allowQuarantine: true, + allowQuarantine: false, }); - const opened = readOnly - ? await openLbugConnection(lbug, dbPath, { readOnly: true }) - : await openLbugConnection(lbug, dbPath); - const usable = readOnly ? await ensureReadOnlyConnectionUsable(dbPath, opened) : opened; + const opened = await openLbugConnection(lbug, dbPath, { readOnly: true }); + const usable = await ensureReadOnlyConnectionUsable(dbPath, opened); db = usable.db; conn = usable.conn; - currentDbReadOnly = readOnly; - } finally { - await releaseInitLock(); + currentDbReadOnly = true; + } else { + // LadybugDB stores the database as a single file (not a directory). + // If the path already exists, it must be a valid LadybugDB database file. + // Remove stale empty directories or files from older versions. + try { + const stat = await fs.lstat(dbPath); + if (stat.isSymbolicLink()) { + // Never follow symlinks — just remove the link itself + await fs.unlink(dbPath); + } else if (stat.isDirectory()) { + // Verify path is within expected storage directory before deleting + const realPath = await fs.realpath(dbPath); + const parentDir = path.dirname(dbPath); + const realParent = await fs.realpath(parentDir); + if (!realPath.startsWith(realParent + path.sep) && realPath !== realParent) { + throw new Error( + `Refusing to delete ${dbPath}: resolved path ${realPath} is outside storage directory`, + ); + } + // Old-style directory database or empty leftover - remove it + await fs.rm(dbPath, { recursive: true, force: true }); + } + // If it's a file, assume it's an existing LadybugDB database - LadybugDB will open it + } catch (err) { + if (!isMissingFileError(err)) { + throw err; + } + // Path doesn't exist, which is what LadybugDB wants for a new database + } + + // ------------------------------------------------------------------------- + // Cross-process critical section: acquire init lock, clean orphan sidecars, + // and open the database. The lock prevents a TOCTOU race where another + // process could create a fresh DB between our access() check and the + // unlink() of stale sidecars. + // ------------------------------------------------------------------------- + const releaseInitLock = await acquireInitLock(dbPath); + try { + // Crash-recovery cleanup: if the main DB file is missing, stale sidecars + // from an interrupted run can block fresh opens indefinitely. + try { + await fs.access(dbPath); + } catch (err) { + if (isMissingFileError(err)) { + // `.shadow` is documented by LadybugDB checkpointing and `.wal.checkpoint` + // was observed in the #1618 crash loop that motivated this recovery path. + const orphanSidecars = [`${dbPath}.shadow`, `${dbPath}.wal.checkpoint`]; + for (const sidecar of orphanSidecars) { + try { + await fs.unlink(sidecar); + logger.warn( + `GitNexus: removed orphan sidecar ${path.basename(sidecar)} (no main DB file present)`, + ); + } catch (err) { + if (isMissingFileError(err)) { + continue; + } + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: failed to remove orphan sidecar ${path.basename(sidecar)} (${code ?? 'UNKNOWN'}) while main DB file is missing; LadybugDB open may still fail: ${summarizeError(err)}`, + ); + } + } + } else { + const code = extractErrnoCode(err); + logger.warn( + `GitNexus: unable to verify main DB file before orphan sidecar cleanup (${code ?? 'UNKNOWN'}); skipping cleanup: ${summarizeError(err)}`, + ); + } + } + + // Ensure parent directory exists + const parentDir = path.dirname(dbPath); + await fs.mkdir(parentDir, { recursive: true }); + await preflightLbugSidecars(dbPath, { + mode: 'write', + logger, + allowQuarantine: true, + }); + + const opened = await openLbugConnection(lbug, dbPath); + db = opened.db; + conn = opened.conn; + currentDbReadOnly = false; + } finally { + await releaseInitLock(); + } } if (!readOnly) { diff --git a/gitnexus/test/integration/lbug-readonly-init.test.ts b/gitnexus/test/integration/lbug-readonly-init.test.ts new file mode 100644 index 000000000..1d79ac39c --- /dev/null +++ b/gitnexus/test/integration/lbug-readonly-init.test.ts @@ -0,0 +1,29 @@ +/** + * Integration Tests: read-only doInitLbug path (#1783) + * + * Verifies that read-only LadybugDB opens skip filesystem mutations + * (init lock, orphan sidecar cleanup, mkdir) so they work on read-only + * filesystems such as Docker :ro bind mounts. + */ +import fs from 'fs/promises'; +import { it, expect } from 'vitest'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; +import { _initLockPathForTest } from '../../src/core/lbug/lbug-adapter.js'; + +withTestLbugDB('lbug-readonly-init', (handle) => { + it('read-only open never creates lbug.init.lock on disk', async () => { + const { dbPath } = handle; + const lockPath = _initLockPathForTest(dbPath); + + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + await adapter.closeLbug(); + + await expect(fs.access(lockPath)).rejects.toMatchObject({ code: 'ENOENT' }); + + await adapter.withLbugDb(dbPath, async () => {}, { readOnly: true }); + + await expect(fs.access(lockPath)).rejects.toMatchObject({ code: 'ENOENT' }); + + await adapter.closeLbug(); + }); +}); From 39e9b40136f6baab7b3ff6ea33af5ecf9c830952 Mon Sep 17 00:00:00 2001 From: ManniX-ITA <20623405+mann1x@users.noreply.github.com> Date: Sun, 24 May 2026 09:51:21 +0100 Subject: [PATCH 04/10] fix(windows): pass windowsHide:true to every child_process spawn-family call (#1794) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(hooks): pass windowsHide:true to every spawnSync to suppress flashing console windows on Windows On Windows, every PostToolUse and Stop event from Claude Code (and the Cursor integration variant) cold-spawns ``node`` / ``npx.cmd`` / ``git`` / ``lsof`` through ``child_process.spawnSync``. Without ``windowsHide: true`` in the options, Node's child_process module asks ``CreateProcess`` to use ``STARTF_USESHOWWINDOW`` with ``SW_SHOWDEFAULT``, and a black console window flashes onto the user's desktop for the duration of the call. Under active editor / agent use this means a near-continuous stream of pop-up windows — unusable in practice (reported live on a Windows 11 workstation running the gitnexus Claude plugin against an active project; the flashes stack on the taskbar and steal focus from the editor). The Node fix is one option flag per spawnSync: spawnSync(cmd, args, { encoding: 'utf-8', timeout, cwd, stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true, // <-- new }); ``windowsHide`` is a no-op on macOS/Linux (Node docs: "Hide the subprocess console window that would normally be created on Windows systems"), so the patch is platform-neutral and zero-risk on the other two majors. This commit touches every ``spawnSync`` call in the three sources that ship the hook layer: * gitnexus/hooks/claude/gitnexus-hook.cjs (4 sites) * gitnexus/hooks/claude/hook-db-lock-probe.cjs (3 sites) * gitnexus-claude-plugin/hooks/gitnexus-hook.js (6 sites) * gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs (3 sites) * gitnexus-cursor-integration/hooks/gitnexus-hook.cjs (3 sites) Total: 19 spawn sites guarded. ``hook-lock.cjs`` / ``hook-lock.js`` don't spawn subprocesses; nothing else in the hooks/ dirs touches ``child_process``. Verified on Windows 10 22H2 / Node 22.21 / gitnexus 1.6.5 by installing the locally-built tarball and running an active Claude Code session against a large mixed-language repo — no console window appears for any hook fire (pre-fix: ~2-3 visible flashes per edit). No behavioural change on Linux/macOS hosts. * test(hooks): regression — every hook spawnSync paired with windowsHide:true Source-level assertion that every ``spawnSync`` invocation in the hook layer has a matching ``windowsHide: true`` in its options object. Without the flag, Node's child_process module asks CreateProcess to use STARTF_USESHOWWINDOW with SW_SHOWDEFAULT and a black console window flashes onto the user's desktop for the duration of each call — see the parent fix commit. The check is source-level rather than behavioural because: * the flag's effect is observable only on Windows; * GitHub Actions runs vitest on Linux for the hook tests; * regressing this is easy (every new spawnSync site has to remember to add the flag), and a runtime check on a Windows-only CI leg would still let a PR land on the main branch first. Counts spawnSync occurrences and windowsHide:true occurrences per file (in code, ignoring comments) and asserts equality. Five files covered: * gitnexus/hooks/claude/gitnexus-hook.cjs * gitnexus/hooks/claude/hook-db-lock-probe.cjs * gitnexus-claude-plugin/hooks/gitnexus-hook.js * gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs * gitnexus-cursor-integration/hooks/gitnexus-hook.cjs Adding a new hook file requires updating the HOOK_FILES tuple. A sanity assertion ``spawnCount > 0`` catches accidental deletion of all spawn calls in a future refactor (would otherwise silently make the count-equality assertion trivially true). Sits next to the existing "no shell: true" and ".cmd extension" regression tests in test/unit/hooks.test.ts — same shape, same spirit. * fix(src): extend windowsHide:true to every spawn-family call in cli/core/mcp/server Companion to the hook-layer fix in this branch's first commit. The same Windows console-window flash bug applies to every ``spawn`` / ``spawnSync`` / ``execFile`` / ``execFileSync`` / ``execFileAsync`` / ``execSync`` call in the source tree — not just the hooks. The MCP local backend (``src/mcp/local/local-backend.ts``) and the ``gitnexus serve`` git helpers (``src/server/git-clone.ts``) are particularly bad because they run from daemonized processes that have no parent console; the spawned child auto-allocates one and it pops onto the user's desktop. The CLI sites are less visible (the user is at a terminal with an existing console; ``stdio: 'inherit'`` shares it) but the flag is harmless there — windowsHide only suppresses NEW console allocation, an inherited parent console is untouched. The visible output of ``gitnexus analyze`` and friends is preserved verbatim. The pre-existing fix at ``src/core/lbug/extension-loader.ts:96`` established the convention in this codebase. This commit applies it uniformly. Sites covered (21 new): | File | Sites | |---|---| | src/cli/analyze.ts | 1 | | src/cli/setup.ts | 2 | | src/cli/wiki.ts | 3 | | src/core/embeddings/embedder.ts | 1 | | src/core/git-staleness.ts | 3 | | src/core/run-analyze.ts | 1 | | src/core/wiki/cursor-client.ts | 2 | | src/core/wiki/generator.ts | 3 | | src/mcp/local/local-backend.ts | 2 | | src/server/git-clone.ts | 2 | | src/core/lbug/extension-loader.ts | (already had it, untouched) | Combined with the 19 hook sites from the first commit + the 1 pre-existing extension-loader site, the codebase now has uniform ``windowsHide: true`` on every spawn-family call. Behavioural notes: * ``windowsHide`` is documented by Node as a no-op on POSIX — Linux/macOS hosts see byte-identical behaviour. * ``stdio: 'inherit'`` callers (e.g. ``cli/wiki.ts:522`` opens the editor in the user's terminal) keep their interactive UX. The child inherits the parent's stdio handles; no new console is allocated; the flag has nothing to hide. * Piped callers (``stdio: ['pipe',…]``) continue to deliver every byte of stdout/stderr back to the parent for the parent to log / process / re-print. No output is swallowed. * ``execSync`` / ``execFileSync`` callers that previously had no ``stdio`` option (e.g. ``generator.ts:887`` ``execSync('git rev-parse HEAD', { cwd })``) keep their default pipe semantics (``.toString()`` still works) — windowsHide is added alongside the existing ``cwd`` option. Verified on Windows 10 22H2 / Node 22.21 by installing the locally built tarball and exercising: * MCP detect_changes via the local backend → no flash. * gitnexus serve → no flash on git clone/clone-pull. * gitnexus analyze interactively → output appears in terminal as before, no extra window. * test(windowsHide): extend regression to every spawn-family call in src/ Companion to the src/ patch. The hooks.test.ts regression now covers 16 files (5 hooks + 11 source files), and asserts the invariant for every spawn-family function — not just spawnSync. Changes: * Generalise countSpawnCalls() to also count spawn, execFile, execFileSync, execFileAsync, execSync (the entire spawn-family surface of child_process). Skip method calls (e.g. RegExp.exec) via a negative-lookbehind on ``.``. * Add SRC_FILES table with all 11 source-tree files that import spawn-family functions from child_process. * Loop over [...HOOK_FILES, ...SRC_FILES] so a regression in any file fails the same test name. * Tighten the assertion to ``hideCount >= spawnCount`` rather than strict equality, because some sites (e.g. setup.ts:534 using execFileAsync via shell:true on Windows) may legitimately add windowsHide to nested option objects in future refactors. * Sanity gate ``spawnCount > 0`` per file catches a refactor that deletes all spawn calls (would otherwise make the assertion trivially true). Manually exercised against the patched repo: 16 files, 28 total spawn-family calls, 28 windowsHide:true. All pass. The convention to keep this list in sync: every new file in gitnexus/src/ that imports from 'child_process' must be added to the SRC_FILES tuple. The cost is one line per file; the benefit is the next contributor never has to think about windowsHide again — the test will catch a miss before merge. * style: prettier --write on storage/git.ts + hooks.test.ts CI quality / format job flagged two formatting issues in the merge-resolution commit: a long single-line options object in storage/git.ts and similar in hooks.test.ts. prettier --write fixes both with the project's standard wrap-and-trailing-comma style. No semantic change. * test(git): include windowsHide in toHaveBeenCalledWith assertion The merge-resolution commit added windowsHide:true to the 'git rev-parse --is-inside-work-tree' execSync call in src/storage/git.ts, but the matching strict-shape assertion in git.test.ts:31-34 still expected the pre-patch two-key options object {cwd, stdio}. vitest's toHaveBeenCalledWith does a deep structural match, so the extra third key flipped the assertion to fail. Add windowsHide: true to the expected shape. Only this one assertion is strict; the two siblings ('passes the correct cwd' and the no-cwd-arg case) use expect.objectContaining and expect.any(String) and remain green without modification. * test(setup-codex): include windowsHide in execFile shape assertions Same root cause as the git.test.ts fix on this branch: the windowsHide patch added windowsHide:true to the execFile() options in src/cli/setup.ts, but three strict-shape toHaveBeenCalledWith assertions in setup-codex.test.ts still expected the pre-patch {shell:true} / {shell:false} two-key options. vitest does a deep structural match, so the extra key flipped the assertions to fail on every CI matrix leg (ubuntu coverage + macos + windows). Adding windowsHide:true alongside the existing 'shell' key in all three sites. * ci: retrigger checks go-parity failed on a flaky onnxruntime-node postinstall network timeout (AggregateError [ETIMEDOUT] in node ./script/install), which cascaded into the CI Gate. No code change — empty commit to re-run the pipeline. * fix(test): strengthen windowsHide regression assertions (PR #1794 review) - Replace toBeGreaterThanOrEqual with exact toBe per DoD §2.7 - Remove unused `m` variable in countSpawnCalls (CodeQL finding) - Add windowsHide: true to runGit test helper for consistency --------- Co-authored-by: Gergő Magyar Co-authored-by: ManniX-ITA <35522085+ManniX-ITA@users.noreply.github.com> Co-authored-by: Test --- gitnexus-claude-plugin/hooks/gitnexus-hook.js | 6 + .../hooks/hook-db-lock-probe.cjs | 3 + .../hooks/gitnexus-hook.cjs | 3 + gitnexus/hooks/claude/gitnexus-hook.cjs | 4 + gitnexus/hooks/claude/hook-db-lock-probe.cjs | 3 + gitnexus/src/cli/analyze.ts | 1 + gitnexus/src/cli/setup.ts | 2 + gitnexus/src/cli/wiki.ts | 6 +- gitnexus/src/core/embeddings/embedder.ts | 6 +- gitnexus/src/core/git-staleness.ts | 3 + gitnexus/src/core/run-analyze.ts | 1 + gitnexus/src/core/wiki/cursor-client.ts | 3 +- gitnexus/src/core/wiki/generator.ts | 9 +- gitnexus/src/mcp/local/local-backend.ts | 2 + gitnexus/src/server/git-clone.ts | 2 + gitnexus/src/storage/git.ts | 11 +- gitnexus/test/unit/git.test.ts | 1 + gitnexus/test/unit/hooks.test.ts | 171 ++++++++++++++++++ gitnexus/test/unit/setup-codex.test.ts | 6 +- 19 files changed, 233 insertions(+), 10 deletions(-) diff --git a/gitnexus-claude-plugin/hooks/gitnexus-hook.js b/gitnexus-claude-plugin/hooks/gitnexus-hook.js index 7ff03e430..91c20c9d2 100644 --- a/gitnexus-claude-plugin/hooks/gitnexus-hook.js +++ b/gitnexus-claude-plugin/hooks/gitnexus-hook.js @@ -77,6 +77,7 @@ function findCanonicalRepoRoot(cwd) { timeout: 2000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); if (result.error || result.status !== 0) return null; const commonDir = (result.stdout || '').trim(); @@ -200,6 +201,7 @@ function runGitNexusCli(args, cwd, timeout) { timeout, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } @@ -210,6 +212,7 @@ function runGitNexusCli(args, cwd, timeout) { encoding: 'utf-8', timeout: 3000, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); useDirectBinary = which.status === 0; } catch { @@ -222,6 +225,7 @@ function runGitNexusCli(args, cwd, timeout) { timeout, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } // npx fallback needs shell on Windows since npx is a .cmd script @@ -230,6 +234,7 @@ function runGitNexusCli(args, cwd, timeout) { timeout: timeout + 5000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } @@ -318,6 +323,7 @@ function handlePostToolUse(input) { timeout: 3000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); currentHead = (headResult.stdout || '').trim(); } catch { diff --git a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs index 783cd0804..752c114a7 100644 --- a/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs +++ b/gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs @@ -109,6 +109,7 @@ function hasGitNexusServerOwnerWindows(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 6000, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, env: { ...process.env, GITNEXUS_HOOK_RM_TARGET: dbPathAbs }, }, ); @@ -192,6 +193,7 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 1000, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); if (lsof.error) return lsof.error.code === 'ETIMEDOUT'; @@ -203,6 +205,7 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 500, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); if (ps.error) { if (ps.error.code === 'ETIMEDOUT') return true; diff --git a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs index 74c5587b3..ab495be84 100644 --- a/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs +++ b/gitnexus-cursor-integration/hooks/gitnexus-hook.cjs @@ -58,6 +58,7 @@ function findCanonicalRepoRoot(cwd) { timeout: 2000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); if (result.error || result.status !== 0) return null; const commonDir = (result.stdout || '').trim(); @@ -201,6 +202,7 @@ function runGitNexusCli(cliPath, args, cwd, timeout) { timeout, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } return spawnSync(isWin ? 'npx.cmd' : 'npx', ['-y', 'gitnexus', ...args], { @@ -208,6 +210,7 @@ function runGitNexusCli(cliPath, args, cwd, timeout) { timeout: timeout + 5000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } diff --git a/gitnexus/hooks/claude/gitnexus-hook.cjs b/gitnexus/hooks/claude/gitnexus-hook.cjs index e39fcf8e1..9793bd7bc 100755 --- a/gitnexus/hooks/claude/gitnexus-hook.cjs +++ b/gitnexus/hooks/claude/gitnexus-hook.cjs @@ -77,6 +77,7 @@ function findCanonicalRepoRoot(cwd) { timeout: 2000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); if (result.error || result.status !== 0) return null; const commonDir = (result.stdout || '').trim(); @@ -218,6 +219,7 @@ function runGitNexusCli(cliPath, args, cwd, timeout) { timeout, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } // On Windows, invoke npx.cmd directly (no shell needed) @@ -226,6 +228,7 @@ function runGitNexusCli(cliPath, args, cwd, timeout) { timeout: timeout + 5000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); } @@ -315,6 +318,7 @@ function handlePostToolUse(input) { timeout: 3000, cwd, stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); currentHead = (headResult.stdout || '').trim(); } catch { diff --git a/gitnexus/hooks/claude/hook-db-lock-probe.cjs b/gitnexus/hooks/claude/hook-db-lock-probe.cjs index 783cd0804..752c114a7 100644 --- a/gitnexus/hooks/claude/hook-db-lock-probe.cjs +++ b/gitnexus/hooks/claude/hook-db-lock-probe.cjs @@ -109,6 +109,7 @@ function hasGitNexusServerOwnerWindows(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 6000, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, env: { ...process.env, GITNEXUS_HOOK_RM_TARGET: dbPathAbs }, }, ); @@ -192,6 +193,7 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 1000, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); if (lsof.error) return lsof.error.code === 'ETIMEDOUT'; @@ -203,6 +205,7 @@ function unixLsofPsFindGitNexusServer(dbPathAbs, myPid) { encoding: 'utf-8', timeout: 500, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); if (ps.error) { if (ps.error.code === 'ETIMEDOUT') return true; diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 32ceaca62..c9d370f7f 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -312,6 +312,7 @@ const runRespawnedAnalyze = ( const child = spawn(process.execPath, [...args], { stdio: ['inherit', 'pipe', 'pipe'], + windowsHide: true, env, }); diff --git a/gitnexus/src/cli/setup.ts b/gitnexus/src/cli/setup.ts index fe9d86f52..915c19dec 100644 --- a/gitnexus/src/cli/setup.ts +++ b/gitnexus/src/cli/setup.ts @@ -54,6 +54,7 @@ function resolveGitnexusBin(): string | null { encoding: 'utf-8', timeout: 5000, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }); const lines = output .split('\n') @@ -532,6 +533,7 @@ async function setupCodex(result: SetupResult): Promise { const entry = getMcpEntry(); await execFileAsync('codex', ['mcp', 'add', 'gitnexus', '--', entry.command, ...entry.args], { shell: process.platform === 'win32', + windowsHide: true, }); result.configured.push('Codex'); return; diff --git a/gitnexus/src/cli/wiki.ts b/gitnexus/src/cli/wiki.ts index 6211d371c..97ac8fc6e 100644 --- a/gitnexus/src/cli/wiki.ts +++ b/gitnexus/src/cli/wiki.ts @@ -519,7 +519,7 @@ const wikiCommandImpl = async (inputPath?: string, options?: WikiCommandOptions) console.log(' Save and close the editor when done.\n'); try { - execFileSync(editor, [treeFile], { stdio: 'inherit' }); + execFileSync(editor, [treeFile], { stdio: 'inherit', windowsHide: true }); } catch { console.log(` Could not open editor. Please edit manually:\n ${treeFile}\n`); console.log(' Then run `gitnexus wiki` to continue.\n'); @@ -655,7 +655,7 @@ const wikiCommandImpl = async (inputPath?: string, options?: WikiCommandOptions) function hasGhCLI(): boolean { try { - execSync('gh --version', { stdio: 'ignore' }); + execSync('gh --version', { stdio: 'ignore', windowsHide: true }); return true; } catch { return false; @@ -699,7 +699,7 @@ function publishGist(htmlPath: string): { url: string; rawUrl: string } | null { const output = execFileSync( 'gh', ['gist', 'create', htmlPath, '--desc', 'Repository Wiki — generated by GitNexus', '--public'], - { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }, + { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }, ).trim(); // `gh gist create` prints the gist URL as a line in the output. Find the diff --git a/gitnexus/src/core/embeddings/embedder.ts b/gitnexus/src/core/embeddings/embedder.ts index 72ddcbd70..d2e9d0aff 100644 --- a/gitnexus/src/core/embeddings/embedder.ts +++ b/gitnexus/src/core/embeddings/embedder.ts @@ -78,7 +78,11 @@ function isCudaAvailable(): boolean { // Primary: query the dynamic linker cache — covers all architectures, // distro layouts, and custom install paths registered with ldconfig try { - const out = execFileSync('ldconfig', ['-p'], { timeout: 3000, encoding: 'utf-8' }); + const out = execFileSync('ldconfig', ['-p'], { + timeout: 3000, + encoding: 'utf-8', + windowsHide: true, + }); if (out.includes('libcublasLt.so.12')) return true; } catch { // ldconfig not available (e.g. non-standard container) diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index c90cef85e..2d6a1f8ec 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -26,6 +26,7 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI cwd: repoPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }).trim(); const commitsBehind = parseInt(result, 10) || 0; @@ -59,6 +60,7 @@ export async function checkStalenessAsync( const { stdout } = await execFileAsync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], { cwd: repoPath, encoding: 'utf-8', + windowsHide: true, }); const commitsBehind = parseInt(stdout.trim(), 10) || 0; @@ -90,6 +92,7 @@ function commitsAheadOfIndexed(siblingPath: string, indexedCommit: string): numb cwd: siblingPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }).trim(); return parseInt(result, 10) || 0; } catch { diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index c28504c4c..22405a026 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -332,6 +332,7 @@ export async function runFullAnalysis( { cwd: repoPath, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, encoding: 'utf8', }, ); diff --git a/gitnexus/src/core/wiki/cursor-client.ts b/gitnexus/src/core/wiki/cursor-client.ts index bf85f4183..cc24bdc4d 100644 --- a/gitnexus/src/core/wiki/cursor-client.ts +++ b/gitnexus/src/core/wiki/cursor-client.ts @@ -36,7 +36,7 @@ let cachedCursorBin: string | null | undefined; export function detectCursorCLI(): string | null { if (cachedCursorBin !== undefined) return cachedCursorBin; try { - execSync('agent --version', { stdio: 'ignore' }); + execSync('agent --version', { stdio: 'ignore', windowsHide: true }); cachedCursorBin = 'agent'; } catch { cachedCursorBin = null; @@ -109,6 +109,7 @@ export async function callCursorLLM( const child = spawn(cursorBin, args, { cwd: config.workingDirectory || process.cwd(), stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, env: { ...process.env, // Ensure non-interactive mode diff --git a/gitnexus/src/core/wiki/generator.ts b/gitnexus/src/core/wiki/generator.ts index 7bb8049c2..b17ad3006 100644 --- a/gitnexus/src/core/wiki/generator.ts +++ b/gitnexus/src/core/wiki/generator.ts @@ -884,7 +884,12 @@ export class WikiGenerator { private getCurrentCommit(): string { try { - return execSync('git rev-parse HEAD', { cwd: this.repoPath }).toString().trim(); + return execSync('git rev-parse HEAD', { + cwd: this.repoPath, + windowsHide: true, + }) + .toString() + .trim(); } catch { return ''; } @@ -899,6 +904,7 @@ export class WikiGenerator { execFileSync('git', ['merge-base', '--is-ancestor', fromCommit, toCommit], { cwd: this.repoPath, stdio: 'ignore', + windowsHide: true, }); return true; } catch { @@ -916,6 +922,7 @@ export class WikiGenerator { try { const output = execFileSync('git', ['diff', `${fromCommit}..${toCommit}`, '--name-only'], { cwd: this.repoPath, + windowsHide: true, }) .toString() .trim(); diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 331cffb2c..83c3f023f 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -2404,6 +2404,7 @@ export class LocalBackend { cwd: diffCwd, encoding: 'utf-8', maxBuffer: 256 * 1024 * 1024, + windowsHide: true, }); } catch (err: any) { return { error: `Git diff failed: ${err.message}` }; @@ -2680,6 +2681,7 @@ export class LocalBackend { timeout: 5000, // Avoid ENOBUFS on large repos: rg -l can list many files. maxBuffer: 256 * 1024 * 1024, + windowsHide: true, }); const files = output .trim() diff --git a/gitnexus/src/server/git-clone.ts b/gitnexus/src/server/git-clone.ts index 0ced1213a..d92e9c28f 100644 --- a/gitnexus/src/server/git-clone.ts +++ b/gitnexus/src/server/git-clone.ts @@ -304,6 +304,7 @@ export function getRemoteOriginUrl(cwd: string): Promise { const proc = spawn('git', ['config', '--get', 'remote.origin.url'], { cwd, stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, env: { ...process.env, GIT_TERMINAL_PROMPT: '0' }, }); let stdout = ''; @@ -427,6 +428,7 @@ function runGit(args: string[], cwd?: string): Promise { const proc = spawn('git', args, { cwd, stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, env: { ...process.env, // Prevent git from prompting for credentials (hangs the process) diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index 75e6e91d3..16ebe039a 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -6,7 +6,11 @@ import path from 'path'; export const isGitRepo = (repoPath: string): boolean => { try { - execSync('git rev-parse --is-inside-work-tree', { cwd: repoPath, stdio: 'ignore' }); + execSync('git rev-parse --is-inside-work-tree', { + cwd: repoPath, + stdio: 'ignore', + windowsHide: true, + }); return true; } catch { return false; @@ -23,6 +27,7 @@ export const getCurrentCommit = (repoPath: string): string => { // "fatal: not a git repository" to stderr, which leaks to the user's // terminal even though the error is caught here (#1172). stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }) .toString() .trim(); @@ -60,6 +65,7 @@ export const getRemoteUrl = (repoPath: string): string | undefined => { raw = execSync('git config --get remote.origin.url', { cwd: repoPath, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }) .toString() .trim(); @@ -100,6 +106,7 @@ export const getGitRoot = (fromPath: string): string | null => { cwd: fromPath, // Suppress stderr -- see getCurrentCommit comment and #1172. stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }) .toString() .trim(); @@ -142,6 +149,7 @@ export const getCanonicalRepoRoot = (fromPath: string): string | null => { const commonDir = execSync('git rev-parse --path-format=absolute --git-common-dir', { cwd: fromPath, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }) .toString() .trim(); @@ -245,6 +253,7 @@ export const getRemoteOriginUrl = (repoPath: string): string | null => { const url = execSync('git config --get remote.origin.url', { cwd: repoPath, stdio: ['ignore', 'pipe', 'ignore'], + windowsHide: true, }) .toString() .trim(); diff --git a/gitnexus/test/unit/git.test.ts b/gitnexus/test/unit/git.test.ts index 1bebf4143..9d3a424c9 100644 --- a/gitnexus/test/unit/git.test.ts +++ b/gitnexus/test/unit/git.test.ts @@ -31,6 +31,7 @@ describe('git utilities', () => { expect(mockExecSync).toHaveBeenCalledWith('git rev-parse --is-inside-work-tree', { cwd: '/project', stdio: 'ignore', + windowsHide: true, }); }); diff --git a/gitnexus/test/unit/hooks.test.ts b/gitnexus/test/unit/hooks.test.ts index a0ef2b8cb..141519c4d 100644 --- a/gitnexus/test/unit/hooks.test.ts +++ b/gitnexus/test/unit/hooks.test.ts @@ -94,6 +94,7 @@ function runGit(dir: string, args: string[]) { cwd: dir, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, }); if (result.status !== 0) { const message = result.stderr || result.stdout || result.error?.message || 'unknown error'; @@ -220,6 +221,176 @@ describe('Shell injection regression', () => { } }); +// ─── Source code regression: windowsHide:true on every spawn-family call ─── + +/** + * Every ``spawn`` / ``spawnSync`` / ``execFile`` / ``execFileSync`` / + * ``execFileAsync`` / ``execSync`` call in the hook layer **and the + * core/CLI/MCP/server source tree** must pass ``windowsHide: true`` + * in its options object. Without it, Node's ``child_process`` module + * asks ``CreateProcess`` to use ``STARTF_USESHOWWINDOW`` with + * ``SW_SHOWDEFAULT`` and a black console window flashes onto the + * user's desktop for each call. Under active Claude Code / MCP / + * gitnexus-serve use that's a near-continuous stream of pop-ups — + * unusable in practice on Windows. + * + * ``windowsHide`` is a no-op on POSIX (silently dropped), so the + * flag is safe to require unconditionally. ``stdio: 'inherit'`` + * callers (interactive editors etc.) are unaffected — windowsHide + * only suppresses NEW console allocation; an inherited parent + * console isn't touched. + * + * The check is source-level rather than behavioural because: + * - the flag's effect is observable only on Windows; + * - GitHub Actions runs vitest on Linux for these tests; + * - regressing this is easy (every new spawn site has to remember + * the flag), and a runtime check on a Windows-only CI leg would + * still let a PR land on the main branch first. + * + * The pre-existing fix at ``src/core/lbug/extension-loader.ts:96`` + * established the convention. This test enforces it everywhere. + */ +describe('windowsHide regression', () => { + // Hook-layer files. Adding a new hook file MUST be reflected here. + const HOOK_FILES: Array = [ + ['gitnexus/hooks/claude/gitnexus-hook.cjs', CJS_HOOK], + [ + 'gitnexus/hooks/claude/hook-db-lock-probe.cjs', + path.resolve(__dirname, '..', '..', 'hooks', 'claude', 'hook-db-lock-probe.cjs'), + ], + ['gitnexus-claude-plugin/hooks/gitnexus-hook.js', PLUGIN_HOOK], + [ + 'gitnexus-claude-plugin/hooks/hook-db-lock-probe.cjs', + path.resolve( + __dirname, + '..', + '..', + '..', + 'gitnexus-claude-plugin', + 'hooks', + 'hook-db-lock-probe.cjs', + ), + ], + [ + 'gitnexus-cursor-integration/hooks/gitnexus-hook.cjs', + path.resolve( + __dirname, + '..', + '..', + '..', + 'gitnexus-cursor-integration', + 'hooks', + 'gitnexus-hook.cjs', + ), + ], + ]; + + // Source-tree files. Every file that imports a spawn-family + // function from ``child_process`` belongs here. Discovered via + // grep -rn "from 'child_process'" -- gitnexus/src/ + // plus the explicit ``await import('child_process')`` callers in + // local-backend.ts. + const SRC_FILES: Array = [ + [ + 'gitnexus/src/cli/analyze.ts', + path.resolve(__dirname, '..', '..', 'src', 'cli', 'analyze.ts'), + ], + ['gitnexus/src/cli/setup.ts', path.resolve(__dirname, '..', '..', 'src', 'cli', 'setup.ts')], + ['gitnexus/src/cli/wiki.ts', path.resolve(__dirname, '..', '..', 'src', 'cli', 'wiki.ts')], + [ + 'gitnexus/src/core/embeddings/embedder.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'embeddings', 'embedder.ts'), + ], + [ + 'gitnexus/src/core/git-staleness.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'git-staleness.ts'), + ], + [ + 'gitnexus/src/core/lbug/extension-loader.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'lbug', 'extension-loader.ts'), + ], + [ + 'gitnexus/src/core/run-analyze.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'run-analyze.ts'), + ], + [ + 'gitnexus/src/core/wiki/cursor-client.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'wiki', 'cursor-client.ts'), + ], + [ + 'gitnexus/src/core/wiki/generator.ts', + path.resolve(__dirname, '..', '..', 'src', 'core', 'wiki', 'generator.ts'), + ], + [ + 'gitnexus/src/mcp/local/local-backend.ts', + path.resolve(__dirname, '..', '..', 'src', 'mcp', 'local', 'local-backend.ts'), + ], + [ + 'gitnexus/src/server/git-clone.ts', + path.resolve(__dirname, '..', '..', 'src', 'server', 'git-clone.ts'), + ], + // New post-upstream-merge (May 2026 sync): + [ + 'gitnexus/src/storage/git.ts', + path.resolve(__dirname, '..', '..', 'src', 'storage', 'git.ts'), + ], + ]; + + /** + * Strip pure-comment lines so prose mentions of ``spawn`` / + * ``exec`` don't inflate the call count. + */ + function stripComments(source: string): string { + return source + .split('\n') + .filter((l) => { + const t = l.trim(); + return !t.startsWith('//') && !t.startsWith('*') && !t.startsWith('/*'); + }) + .join('\n'); + } + + /** + * Count spawn-family invocations. The regex matches ``spawn(``, + * ``spawnSync(``, ``execFile(``, ``execFileSync(``, + * ``execFileAsync(``, ``execSync(`` as function calls — not + * destructures (``const { spawn } = ...``), not method calls + * (``.exec(``), not bare ``exec()`` (which collides with regex + * ``.exec()``; we explicitly drop it). + */ + function countSpawnCalls(codeSource: string): number { + const re = + /(^|[^a-zA-Z0-9_$.])(spawn|spawnSync|execFile|execFileSync|execFileAsync|execSync)\s*\(/gm; + let count = 0; + while (re.exec(codeSource) !== null) { + count++; + } + return count; + } + + for (const [label, file] of [...HOOK_FILES, ...SRC_FILES]) { + it(`${label}: every spawn-family options object contains windowsHide: true`, () => { + // The file must exist — silent-skip would mask a deletion. + expect(fs.existsSync(file)).toBe(true); + const source = fs.readFileSync(file, 'utf-8'); + const codeSource = stripComments(source); + + const spawnCount = countSpawnCalls(codeSource); + const hideCount = (codeSource.match(/windowsHide\s*:\s*true/g) ?? []).length; + + // Sanity: catch a refactor that accidentally deletes every + // spawn call (which would otherwise make the equality below + // trivially true at 0 == 0). + expect(spawnCount).toBeGreaterThan(0); + // One windowsHide per spawn-family call. We don't try to + // match brace structure — a same-count proxy is sufficient + // because every spawn site in these files passes an options + // object literal (no helper indirection). + expect(hideCount).toBe(spawnCount); + }); + } +}); + // ─── Source code regression: .cmd extensions for Windows ───────────── describe('Windows .cmd extension handling', () => { diff --git a/gitnexus/test/unit/setup-codex.test.ts b/gitnexus/test/unit/setup-codex.test.ts index 5951c9325..9ede4a67b 100644 --- a/gitnexus/test/unit/setup-codex.test.ts +++ b/gitnexus/test/unit/setup-codex.test.ts @@ -74,7 +74,7 @@ describe('setupCommand codex execution', () => { expect(execFileMock).toHaveBeenCalledWith( 'codex', ['mcp', 'add', 'gitnexus', '--', 'cmd', '/c', 'npx', '-y', NPX_REF, 'mcp'], - { shell: true }, + { shell: true, windowsHide: true }, expect.any(Function), ); }); @@ -89,7 +89,7 @@ describe('setupCommand codex execution', () => { expect(execFileMock).toHaveBeenCalledWith( 'codex', ['mcp', 'add', 'gitnexus', '--', 'cmd', '/c', 'npx', '-y', NPX_REF, 'mcp'], - { shell: true }, + { shell: true, windowsHide: true }, expect.any(Function), ); }); @@ -104,7 +104,7 @@ describe('setupCommand codex execution', () => { expect(execFileMock).toHaveBeenCalledWith( 'codex', ['mcp', 'add', 'gitnexus', '--', 'npx', '-y', NPX_REF, 'mcp'], - { shell: false }, + { shell: false, windowsHide: true }, expect.any(Function), ); From ac9a2ee12f956ecd451fb39bdc39a132009ffa76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sun, 24 May 2026 12:10:10 +0100 Subject: [PATCH 05/10] chore(ci): consolidate parity shards and narrow cross-platform matrix (#1798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(ci): reduce CI runner-minutes by consolidating parity and narrowing cross-platform Scope-resolution parity previously spawned 9 separate GitHub Actions jobs (one per migrated language), each doing full checkout + npm ci + build for a single test file. Consolidate into one job running scripts/run-parity.ts which loops through all migrated languages sequentially — same coverage, ~45 fewer runner-minutes of redundant setup per PR. Cross-platform (Windows/macOS) previously ran the full 373-file test suite. Narrow to 45 platform-sensitive files (native LadybugDB, process spawning, path separators, worker threads, filesystem behavior). Full suite still runs on Ubuntu with coverage. Also adds 2 missing lbug integration tests (lbug-orphan-sidecar-recovery, lbug-readonly-init) to the sequential lbug-db vitest project where they belong, and rewrites TESTING.md to document all test lanes. * fix: address code review findings on parity and cross-platform scripts - Capture stderr in run-parity.ts (vitest writes diagnostics to stderr) - Lower per-invocation timeout from 5min to 60s to stay within CI job limit - Add --language flag validation (error on missing value) - Add timeout diagnostic to run-cross-platform.ts catch block - Add analyze-wal-checkpoint-failure.test.ts to lbug-db sequential project - Expand cross-platform list: parser-loader, pipeline, pipeline-graph-golden, setup-skills, cli/tool-no-index-stderr (51 files, was 45) * fix: add shell:true for Windows npx resolution and simplify fs import execFileSync('npx', ...) fails with ENOENT on Windows because npx is npx.cmd — shell:true resolves this. Also replaces dynamic await import('fs') with static import, and fixes timeout detection to use err.killed instead of err.code. * fix(ci): raise parity per-invocation timeout to 120s and job timeout to 30min TypeScript and C++ resolver tests take 60-90s on CI runners, exceeding the 60s per-invocation timeout. Raise to 120s. Also bump the job-level timeout from 25 to 30 minutes for margin (realistic total is ~11 min). * fix(ci): raise parity per-invocation timeout to 180s for C++ resolver C++ resolver tests take 130-150s on CI runners due to template metaprogramming, ADL, and SFINAE fixture volume. 120s was still too tight. Realistic total across all 9 languages is ~12 min, well under the 30-min job timeout. * fix(ci): use stdio inherit for parity — no per-invocation timeout Switch from piped stdio with per-invocation timeouts to stdio: 'inherit'. Vitest output streams to CI console in real time, making failures immediately visible. The CI job-level timeout (30 min) is the only guard — no more artificial per-invocation timeouts that cut off slow resolver tests like C++ (which genuinely takes 3+ minutes). --------- Co-authored-by: Test --- .github/workflows/ci-scope-parity.yml | 62 +++------- .github/workflows/ci-tests.yml | 10 +- TESTING.md | 139 +++++++++++++++-------- gitnexus/package.json | 2 + gitnexus/scripts/cross-platform-tests.ts | 131 +++++++++++++++++++++ gitnexus/scripts/run-cross-platform.ts | 44 +++++++ gitnexus/scripts/run-parity.ts | 128 +++++++++++++++++++++ gitnexus/vitest.config.ts | 6 + 8 files changed, 428 insertions(+), 94 deletions(-) create mode 100644 gitnexus/scripts/cross-platform-tests.ts create mode 100644 gitnexus/scripts/run-cross-platform.ts create mode 100644 gitnexus/scripts/run-parity.ts diff --git a/.github/workflows/ci-scope-parity.yml b/.github/workflows/ci-scope-parity.yml index 8e2926ba8..039438a15 100644 --- a/.github/workflows/ci-scope-parity.yml +++ b/.github/workflows/ci-scope-parity.yml @@ -24,6 +24,19 @@ name: Scope Resolution Parity # When the set is empty (e.g. mid-Ring-3 for every language), the parity # matrix is skipped and the workflow reports success — no-op until a # language is explicitly claimed migrated. +# +# ── Consolidation (chore/vitest-speed-strategy) ──────────────────────── +# Previously each language was a separate GitHub Actions matrix job, +# meaning N languages × 1 checkout+install+build per shard. The build +# cost dwarfed the test cost (~5 min setup for ~15 sec test execution). +# +# Now a single job runs `scripts/run-parity.ts` which loops through all +# migrated languages sequentially (2 vitest invocations per language: +# legacy + registry-primary). All failures are collected and reported +# at the end (equivalent to the old fail-fast: false behavior). +# +# Adding a new language to MIGRATED_LANGUAGES still requires no workflow +# edit — the script auto-discovers the set at runtime. on: workflow_call: @@ -37,7 +50,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 outputs: - languages: ${{ steps.read.outputs.languages }} has-any: ${{ steps.read.outputs.has-any }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -49,65 +61,27 @@ jobs: working-directory: gitnexus run: | set -euo pipefail - # `tsx` evaluates the TS source directly (no build step), imports - # the exported `Set`, and emits a GH-Actions-friendly JSON matrix. LANGS=$(npx tsx scripts/ci-list-migrated-languages.ts) COUNT=$(printf '%s' "$LANGS" | jq 'length') HAS_ANY="false" if [[ "$COUNT" -gt 0 ]]; then HAS_ANY="true"; fi - echo "languages=$LANGS" >> "$GITHUB_OUTPUT" echo "has-any=$HAS_ANY" >> "$GITHUB_OUTPUT" echo "Discovered $COUNT migrated language(s): $LANGS" - echo "Parity matrix will run: $HAS_ANY" + echo "Parity will run: $HAS_ANY" parity: - name: ${{ matrix.lang.slug }} parity + name: scope-resolution parity needs: discover if: needs.discover.outputs.has-any == 'true' runs-on: ubuntu-latest - timeout-minutes: 20 - strategy: - # One language failing must not abort the others — we want the full - # parity matrix result on a single CI run so a reviewer sees every - # regression at once rather than one-at-a-time. - fail-fast: false - matrix: - lang: ${{ fromJSON(needs.discover.outputs.languages) }} + timeout-minutes: 30 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: ./.github/actions/setup-gitnexus with: build: 'true' - - name: Verify resolver test file exists + - name: Run parity for all migrated languages shell: bash working-directory: gitnexus - run: | - set -euo pipefail - TEST_FILE="test/integration/resolvers/${{ matrix.lang.slug }}.test.ts" - if [[ ! -f "$TEST_FILE" ]]; then - echo "::error title=Missing resolver test::\ - Expected $TEST_FILE for '${{ matrix.lang.slug }}' (listed in \ - MIGRATED_LANGUAGES). Either fix the slug or add the test file \ - before listing this language as migrated." - exit 1 - fi - - - name: Resolver tests — legacy DAG (REGISTRY_PRIMARY_${{ matrix.lang.envvar }}=0) - shell: bash - working-directory: gitnexus - env: - FLAG_NAME: REGISTRY_PRIMARY_${{ matrix.lang.envvar }} - # Explicitly force the flag to `0` even though it also defaults to - # `MIGRATED_LANGUAGES.has(lang)` — once a language is in the set, - # the default flips to registry-primary, so an unset env var would - # silently re-run the same path as step #2. `env FOO=0 cmd` spawns - # `cmd` with the override scoped to just this invocation. - run: env "$FLAG_NAME=0" npx vitest run "test/integration/resolvers/${{ matrix.lang.slug }}.test.ts" - - - name: Resolver tests — registry-primary (REGISTRY_PRIMARY_${{ matrix.lang.envvar }}=1) - shell: bash - working-directory: gitnexus - env: - FLAG_NAME: REGISTRY_PRIMARY_${{ matrix.lang.envvar }} - run: env "$FLAG_NAME=1" npx vitest run "test/integration/resolvers/${{ matrix.lang.slug }}.test.ts" + run: npx tsx scripts/run-parity.ts diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index f354e626b..c34d0f6ec 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -59,21 +59,25 @@ jobs: gitnexus-web/web-test-results.json retention-days: 5 + # Platform-sensitive subset only — the full suite runs on Ubuntu above. + # See gitnexus/scripts/cross-platform-tests.ts for the file list and + # rationale for each included test. cross-platform: - name: ${{ matrix.os }} + name: ${{ matrix.os }} (platform-sensitive) strategy: fail-fast: false matrix: # Ubuntu already covered by the coverage job above os: [windows-latest, macos-latest] runs-on: ${{ matrix.os }} - timeout-minutes: 25 + timeout-minutes: 20 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: ./.github/actions/setup-gitnexus with: build: 'true' - - run: npx vitest run + - name: Run platform-sensitive tests + run: npx tsx scripts/run-cross-platform.ts working-directory: gitnexus # End-to-end smoke test for the #1728 packaging fix: pack the published diff --git a/TESTING.md b/TESTING.md index cf481d32b..e69a4b8e9 100644 --- a/TESTING.md +++ b/TESTING.md @@ -10,32 +10,37 @@ How we structure tests and which commands to run locally and in CI. | Web UI | `gitnexus-web/`| Vitest | Unit/component tests | | Web UI E2E | `gitnexus-web/`| Playwright | Run when changing UI flows | -## Commands (local) +## Test lanes -From repository root, unless noted: +### `gitnexus/` commands -**`gitnexus` (CLI / library)** +From `gitnexus/`: + +| Command | What it runs | When to use | +| ------------------------ | ---------------------------------------------------- | ------------------------------- | +| `npm test` | Full suite (all 3 vitest projects) | Before opening a PR | +| `npm run test:unit` | Unit tests only (`test/unit/`) | Tight development loop | +| `npm run test:integration` | Integration tests (`test/integration/`) | After changing pipelines, DB, workers | +| `npm run test:coverage` | Full suite + v8 coverage with thresholds | Checking coverage impact | +| `npm run test:parity` | Scope-resolution parity for all migrated languages | After changing resolver or scope code | +| `npm run test:cross-platform` | Platform-sensitive subset only | Debugging a Windows/macOS issue | +| `npm run test:watch` | Vitest in watch mode | Active development | + +### `gitnexus-web/` commands + +From `gitnexus-web/`: + +| Command | What it runs | When to use | +| ---------------------- | --------------------------------- | ------------------------------ | +| `npm test` | Unit/component tests (vitest) | After changing web code | +| `npm run test:coverage`| Unit tests + coverage | Checking coverage impact | +| `npm run test:e2e` | Playwright browser tests | After changing UI flows (requires `gitnexus serve` + `npm run dev`) | + +### Before opening a PR ```bash -cd gitnexus -npm install -npm run build -npm test # full suite: vitest run -npm run test:unit # unit only: vitest run test/unit -npm run test:integration # integration suite -npm run test:coverage -npx tsc --noEmit # typecheck (matches CI) -``` - -**`gitnexus-web`** - -```bash -cd gitnexus-web -npm install -npm test # unit tests (vitest) -npx tsc -b --noEmit # typecheck (matches CI) -npm run test:coverage -npm run test:e2e # Playwright (requires gitnexus serve + npm run dev) +cd gitnexus && npx tsc --noEmit && npm test +cd ../gitnexus-web && npx tsc -b --noEmit && npm test ``` ## Pre-commit hook @@ -50,22 +55,79 @@ Tests do **not** run in the pre-commit hook — they run in CI (`ci-tests.yml`) Skip with `git commit --no-verify` (use sparingly). +## Vitest projects + +`gitnexus/vitest.config.ts` defines three projects for safety isolation: + +| Project | Files | Parallelism | Purpose | +| ---------- | ----------------------------- | ----------- | ---------------------------------------------- | +| `lbug-db` | Native LadybugDB integration tests (explicit list) | Sequential | Prevents file-lock conflicts from native mmap addon | +| `cli-e2e` | `skills-e2e.test.ts` | Sequential | CLI process spawning requires serial execution | +| `default` | Everything else | Parallel | Fast execution for pure logic and parser tests | + +When adding a new test that uses native LadybugDB (`@ladybugdb/core`), add it to the `lbug-db` project's explicit include list and the `default` project's exclude list. + ## Test categories - **Unit** — Pure logic, parsers, graph/query helpers; fast; no network. - **Integration** — Real combinations (filesystem, MCP wiring, larger pipelines) as already organized under `gitnexus/test/integration`. -- **Eval-style / golden sets** — For agent- or classification-style behavior, keep labeled inputs and expected outputs (JSON or table-driven tests) and run them in CI when relevant. +- **Resolver / parity** — Language-specific call-resolution tests in `test/integration/resolvers/`. - **E2E (web)** — Critical user paths only; prefer `data-testid` attributes for stable selectors. Tests run against real backend (`gitnexus serve`) and Vite dev server. -## Performance metrics (targets) +## Scope-resolution parity -Set targets to match team expectations, then tune to this repo’s CI reality: +Migrated languages (listed in `MIGRATED_LANGUAGES` in `src/core/ingestion/registry-primary-flag.ts`) are tested in both legacy and registry-primary modes on every PR. -| Metric | Target (initial) | Notes | -| ------------------- | ---------------- | ------------------------------------------ | -| Unit coverage | Align with CI | CI runs Vitest with coverage in `gitnexus` | -| Unit wall time | Fast PR feedback | Use `vitest run test/unit` for tight loop | -| Integration duration| < few minutes | Guard heavy tests with env flags if needed | +For each migrated language, CI runs the resolver test file twice: +1. `REGISTRY_PRIMARY_=0` — legacy DAG path +2. `REGISTRY_PRIMARY_=1` — registry-primary path + +Both must pass. Known legacy gaps are listed in `LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES` in `test/integration/resolvers/helpers.ts` and are automatically skipped in legacy mode. + +Adding a language to `MIGRATED_LANGUAGES` automatically enrolls it in parity — no workflow or config edit needed. The test file must exist at `test/integration/resolvers/.test.ts`. + +Run parity locally: `cd gitnexus && npm run test:parity` + +Run for a single language: `cd gitnexus && npx tsx scripts/run-parity.ts --language python` + +## Cross-platform testing + +Windows and macOS CI runs only the platform-sensitive test subset (~50 files out of 373). The full suite runs on Ubuntu. + +The subset is defined in `gitnexus/scripts/cross-platform-tests.ts` and includes: + +- **Platform-specific logic** — tests with `process.platform` guards, path.sep behavior, EPERM/EBUSY error classification +- **Native LadybugDB** — all `lbug-*` integration tests (N-API addon with known platform-varying behavior) +- **Process spawning / CLI** — tests using real `child_process.spawn`, shell quoting, CLI invocations +- **Worker threads** — tests spawning real `worker_threads` +- **Native addon loading** — tree-sitter grammar loading smoke tests +- **Filesystem behavior** — CRLF handling, directory walking, symlinks + +When adding a platform-sensitive test, add it to the appropriate section in `scripts/cross-platform-tests.ts`. + +### Confirming no tests are orphaned + +Every test file matches one of the three vitest projects. To verify: + +```bash +cd gitnexus +npx vitest list 2>/dev/null | wc -l # should match total test count +``` + +To check the cross-platform list is up to date, run `npm run test:cross-platform` — it fails fast if any listed file is missing. + +## CI integration + +GitHub Actions (`.github/workflows/ci.yml`) orchestrate: + +| Workflow | Jobs | Purpose | +| --------------------- | ------------------------------ | ------------------------------------------------ | +| `ci-quality.yml` | format, lint, typecheck, typecheck-web, workflow-convention | Code quality gates | +| `ci-tests.yml` | ubuntu/coverage, cross-platform (Win/Mac), packaged-install-smoke | Full suite + coverage on Ubuntu; platform-sensitive subset on Win/Mac | +| `ci-scope-parity.yml` | discover, parity | Scope-resolution parity for all migrated languages | +| `ci-e2e.yml` | e2e (chromium) | Playwright E2E, gated on `gitnexus-web/**` changes | + +The `CI Gate` job in `ci.yml` is the single required check for branch protection. It requires quality, tests, e2e, and scope-parity to all pass. ## Regression testing @@ -76,23 +138,6 @@ Re-run the full relevant suite when: - Graph schema, query contracts, or MCP tool shapes change - Dependencies with parsing or runtime impact upgrade -## CI integration - -GitHub Actions (`.github/workflows/ci.yml`) orchestrate: - -- **`ci-quality.yml`** — prettier format check, eslint lint, `tsc --noEmit` for `gitnexus/`, `tsc -b --noEmit` for `gitnexus-web/` -- **`ci-tests.yml`** — `vitest run` with coverage (ubuntu) + cross-platform (macOS, Windows) -- **`ci-e2e.yml`** — Playwright E2E tests, gated on `gitnexus-web/**` changes - -Local checks before pushing: - -```bash -cd gitnexus && npx tsc --noEmit && npm test -cd ../gitnexus-web && npx tsc -b --noEmit && npm test -``` - -Or rely on the pre-commit hook which runs these automatically for staged files. - ## User acceptance / beta (optional) For staged releases or UI betas: deploy to a staging environment, collect structured feedback, watch errors and latency, then iterate before a wider release. diff --git a/gitnexus/package.json b/gitnexus/package.json index fb40cba56..0e25ed25a 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -48,6 +48,8 @@ "test:integration": "vitest run test/integration", "test:watch": "vitest", "test:coverage": "vitest run --coverage", + "test:parity": "tsx scripts/run-parity.ts", + "test:cross-platform": "tsx scripts/run-cross-platform.ts", "postinstall": "node scripts/materialize-vendor-grammars.cjs && node scripts/build-tree-sitter-dart.cjs && node scripts/build-tree-sitter-proto.cjs && node scripts/build-tree-sitter-swift.cjs", "prepare": "node scripts/build.js", "prepack": "node scripts/build.js" diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts new file mode 100644 index 000000000..122ce064b --- /dev/null +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -0,0 +1,131 @@ +/** + * Cross-platform test subset runner. + * + * Runs only the tests that exercise platform-sensitive behavior on + * Windows and macOS. The full suite runs on Ubuntu; this narrows the + * cross-platform matrix to tests that actually vary across OSes. + * + * Categories included: + * - Platform-specific logic (path.sep, process.platform guards) + * - Native addon loading (LadybugDB, tree-sitter) + * - Process spawning and shell behavior + * - Filesystem locking and temp-dir behavior + * - Worker threads (real, not mocked) + * - CLI end-to-end tests + * + * When adding a new test that uses platform-varying APIs (native addons, + * child_process with real spawning, filesystem locking, path.sep), add + * it to the appropriate section below. + * + * Usage: + * npx vitest run $(npx tsx scripts/cross-platform-tests.ts) + * # or via the package script: + * npm run test:cross-platform + */ + +// Platform-specific logic tests — contain explicit process.platform guards +// or test behavior that differs across operating systems +const PLATFORM_LOGIC = [ + 'test/unit/setup.test.ts', + 'test/unit/setup-jsonc.test.ts', + 'test/unit/setup-codex.test.ts', + 'test/unit/platform-capabilities.test.ts', + 'test/unit/worker-pool-windows-quarantine.test.ts', + 'test/unit/lbug-pool-win-fts-probe.test.ts', + 'test/unit/repo-manager.test.ts', + 'test/unit/repo-manager-finalize-invariant.test.ts', + 'test/unit/hooks.test.ts', + 'test/unit/cursor-hook.test.ts', + 'test/unit/sidecar-recovery.test.ts', + 'test/unit/pool-wal-recovery.test.ts', + 'test/unit/detect-changes-worktree.test.ts', + 'test/unit/eval-server-bind-restriction.test.ts', + 'test/unit/ignore-service.test.ts', + 'test/unit/group/bridge-db.test.ts', + 'test/unit/group/bridge-db-edge.test.ts', +]; + +// Native LadybugDB integration tests — exercise the @ladybugdb/core +// N-API addon which has known platform-specific behavior (Windows +// file-lock lag after close, macOS N-API destructor segfaults) +const LBUG_NATIVE = [ + 'test/integration/lbug-core-adapter.test.ts', + 'test/integration/lbug-vector-extension.test.ts', + 'test/integration/lbug-pool.test.ts', + 'test/integration/lbug-pool-stability.test.ts', + 'test/integration/lbug-lock-retry.test.ts', + 'test/integration/lbug-open-retry.test.ts', + 'test/integration/lbug-close-handle-release.test.ts', + 'test/integration/lbug-orphan-sidecar-recovery.test.ts', + 'test/integration/lbug-readonly-init.test.ts', + 'test/integration/local-backend.test.ts', + 'test/integration/local-backend-calltool.test.ts', + 'test/integration/search-core.test.ts', + 'test/integration/search-pool.test.ts', + 'test/integration/staleness-and-stability.test.ts', + 'test/integration/analyze-wal-checkpoint-failure.test.ts', +]; + +// Process spawning and CLI tests — exercise child_process with real +// process spawning, which behaves differently across platforms (shell +// quoting, path resolution, signal handling) +const SPAWN_CLI = [ + 'test/integration/cli-e2e.test.ts', + 'test/integration/hooks-e2e.test.ts', + 'test/integration/skills-e2e.test.ts', + 'test/integration/server-http-startup.test.ts', + 'test/integration/mcp/server-startup.test.ts', + 'test/integration/analyze-heap-oom-e2e.test.ts', + 'test/integration/group/group-cli.test.ts', + 'test/integration/cli/tool-no-index-stderr.test.ts', + 'test/integration/setup-skills.test.ts', +]; + +// Worker threads tests — exercise real worker_threads which have +// platform-specific behavior (thread spawning, IPC, exit handling) +const WORKER_THREADS = [ + 'test/integration/worker-pool.test.ts', + 'test/integration/parse-impl-quarantine-cache-skip.test.ts', +]; + +// Tree-sitter native addon smoke tests — verify that native grammars +// load correctly on each platform (binary compatibility, .node loading) +const NATIVE_ADDON_SMOKE = [ + 'test/integration/tree-sitter-languages.test.ts', + 'test/integration/parsing.test.ts', + 'test/integration/pipeline.test.ts', + 'test/integration/pipeline-graph-golden.test.ts', + 'test/unit/parser-loader.test.ts', +]; + +// Filesystem behavior tests — exercise operations that vary across +// platforms (CRLF, symlinks, permissions, temp dirs) +const FILESYSTEM = [ + 'test/integration/filesystem-walker.test.ts', + 'test/integration/markdown-processor-crlf.test.ts', + 'test/integration/ignore-and-skip-e2e.test.ts', +]; + +const ALL_CROSS_PLATFORM = [ + ...PLATFORM_LOGIC, + ...LBUG_NATIVE, + ...SPAWN_CLI, + ...WORKER_THREADS, + ...NATIVE_ADDON_SMOKE, + ...FILESYSTEM, +]; + +// When invoked directly, print the file list for vitest consumption +if (process.argv[1]?.endsWith('cross-platform-tests.ts')) { + console.log(ALL_CROSS_PLATFORM.join('\n')); +} + +export { + ALL_CROSS_PLATFORM, + PLATFORM_LOGIC, + LBUG_NATIVE, + SPAWN_CLI, + WORKER_THREADS, + NATIVE_ADDON_SMOKE, + FILESYSTEM, +}; diff --git a/gitnexus/scripts/run-cross-platform.ts b/gitnexus/scripts/run-cross-platform.ts new file mode 100644 index 000000000..1f5464caf --- /dev/null +++ b/gitnexus/scripts/run-cross-platform.ts @@ -0,0 +1,44 @@ +/** + * Cross-platform test runner. + * + * Runs the platform-sensitive test subset defined in cross-platform-tests.ts + * via vitest. Used by `npm run test:cross-platform` and by the CI cross- + * platform matrix (ci-tests.yml). + * + * The main vitest.config.ts is used, so lbug-db project files get + * sequential execution and other safety constraints are preserved. + */ + +import { execFileSync } from 'child_process'; +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { ALL_CROSS_PLATFORM } from './cross-platform-tests.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(__dirname, '..'); + +// Verify all files exist +const missing = ALL_CROSS_PLATFORM.filter((f) => !fs.existsSync(path.resolve(ROOT, f))); +if (missing.length > 0) { + console.error(`Cross-platform test files not found (${missing.length}):`); + for (const f of missing) console.error(` ${f}`); + console.error('\nUpdate scripts/cross-platform-tests.ts if files were moved or removed.'); + process.exit(1); +} + +console.log(`Running ${ALL_CROSS_PLATFORM.length} platform-sensitive tests...\n`); + +try { + execFileSync('npx', ['vitest', 'run', ...ALL_CROSS_PLATFORM], { + cwd: ROOT, + stdio: 'inherit', + timeout: 15 * 60 * 1000, + shell: true, + }); +} catch (err: any) { + if (err.killed || err.signal) { + console.error('vitest timed out after 15 minutes'); + } + process.exit(1); +} diff --git a/gitnexus/scripts/run-parity.ts b/gitnexus/scripts/run-parity.ts new file mode 100644 index 000000000..f56db11bc --- /dev/null +++ b/gitnexus/scripts/run-parity.ts @@ -0,0 +1,128 @@ +/** + * Consolidated scope-resolution parity runner. + * + * Replaces the per-language matrix in ci-scope-parity.yml with a single + * job that runs all migrated languages sequentially in one process. This + * eliminates 8× redundant checkout + npm ci + build cycles (the old + * workflow created a separate GitHub Actions job per language). + * + * For each language in MIGRATED_LANGUAGES: + * 1. Run its resolver test with REGISTRY_PRIMARY_=0 (legacy DAG) + * 2. Run its resolver test with REGISTRY_PRIMARY_=1 (registry-primary) + * + * Both modes must pass. Failures are collected and reported at the end + * so all regressions are visible in a single CI run (equivalent to the + * old workflow's fail-fast: false behavior). + * + * Vitest output streams to the console in real time (stdio: 'inherit') + * so CI logs show the actual test output directly. No per-invocation + * timeout — the CI job-level timeout (30 min) is the outer guard. + * + * Usage: + * npx tsx scripts/run-parity.ts + * npx tsx scripts/run-parity.ts --language python # single language + */ + +import { execFileSync } from 'child_process'; +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; +import { MIGRATED_LANGUAGES } from '../src/core/ingestion/registry-primary-flag.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(__dirname, '..'); + +interface ParityFailure { + lang: string; + mode: 'legacy' | 'registry-primary'; +} + +function envVarName(slug: string): string { + return `REGISTRY_PRIMARY_${slug.toUpperCase().replace(/-/g, '_')}`; +} + +function testFilePath(slug: string): string { + return `test/integration/resolvers/${slug}.test.ts`; +} + +function runVitest(testFile: string, env: Record): boolean { + try { + execFileSync('npx', ['vitest', 'run', testFile], { + cwd: ROOT, + env: { ...process.env, ...env }, + stdio: 'inherit', + shell: true, + }); + return true; + } catch { + return false; + } +} + +// Parse CLI args +const args = process.argv.slice(2); +const langFlag = args.indexOf('--language'); +const singleLang = langFlag >= 0 ? args[langFlag + 1] : undefined; + +if (langFlag >= 0 && singleLang === undefined) { + console.error('--language requires a value'); + process.exit(1); +} + +const languages = singleLang ? [singleLang] : [...MIGRATED_LANGUAGES].map(String); + +// Verify test files exist before running +const missingFiles: string[] = []; +for (const lang of languages) { + const file = path.resolve(ROOT, testFilePath(lang)); + try { + fs.accessSync(file); + } catch { + missingFiles.push(`${testFilePath(lang)} (${lang})`); + } +} + +if (missingFiles.length > 0) { + console.error('Missing resolver test files:'); + for (const f of missingFiles) console.error(` ${f}`); + process.exit(1); +} + +console.log(`Scope-resolution parity: ${languages.length} language(s)`); +console.log(`Languages: ${languages.join(', ')}\n`); + +const failures: ParityFailure[] = []; + +for (const lang of languages) { + const file = testFilePath(lang); + const envVar = envVarName(lang); + + console.log(`\n── ${lang} — legacy DAG (${envVar}=0) ──`); + if (!runVitest(file, { [envVar]: '0' })) { + failures.push({ lang, mode: 'legacy' }); + } + + console.log(`\n── ${lang} — registry-primary (${envVar}=1) ──`); + if (!runVitest(file, { [envVar]: '1' })) { + failures.push({ lang, mode: 'registry-primary' }); + } +} + +// Summary +const total = languages.length * 2; +const passed = total - failures.length; + +console.log('\n═══════════════════════════════════════'); +console.log('PARITY SUMMARY'); +console.log('═══════════════════════════════════════'); +console.log(`Passed: ${passed}/${total}`); + +if (failures.length > 0) { + console.log(`\nFAILURES (${failures.length}):`); + for (const f of failures) { + console.log(` ✗ ${f.lang} [${f.mode}]`); + } + process.exit(1); +} + +console.log('\nAll parity checks passed.'); diff --git a/gitnexus/vitest.config.ts b/gitnexus/vitest.config.ts index 862357668..34ef25467 100644 --- a/gitnexus/vitest.config.ts +++ b/gitnexus/vitest.config.ts @@ -66,6 +66,9 @@ export default defineConfig({ 'test/integration/shape-check-regression.test.ts', 'test/integration/java-class-impact.test.ts', 'test/integration/class-impact-all-languages.test.ts', + 'test/integration/lbug-orphan-sidecar-recovery.test.ts', + 'test/integration/lbug-readonly-init.test.ts', + 'test/integration/analyze-wal-checkpoint-failure.test.ts', ], fileParallelism: false, sequence: { groupOrder: 1 }, @@ -95,6 +98,9 @@ export default defineConfig({ 'test/integration/shape-check-regression.test.ts', 'test/integration/java-class-impact.test.ts', 'test/integration/class-impact-all-languages.test.ts', + 'test/integration/lbug-orphan-sidecar-recovery.test.ts', + 'test/integration/lbug-readonly-init.test.ts', + 'test/integration/analyze-wal-checkpoint-failure.test.ts', 'test/integration/skills-e2e.test.ts', ], }, From 66f9ec8eff953a6cc19db2d77f35fa79b9effdb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sun, 24 May 2026 14:29:26 +0100 Subject: [PATCH 06/10] feat(java): add Java to MIGRATED_LANGUAGES with 100% scope-resolution parity (#1805) * feat(java): add Java to MIGRATED_LANGUAGES with 100% scope-resolution parity Route Java through the scope-resolution pipeline instead of the legacy single-threaded call processor, fixing the analyze hang on large Java codebases (issue #1741). Changes: - Add Java to MIGRATED_LANGUAGES (registry-primary-flag.ts) - Add tree-sitter queries for var type inference (call-result, alias, field-access, enhanced-for), instanceof/switch pattern bindings, and method references (User::getName, this::save, User::new) - Fix importedName to use simple class name instead of FQN so finalize binding materialization matches correctly - Implement buildJavaMro with IMPLEMENTS edge transitive closure for interface default method resolution - Implement populateJavaPackageSiblings for same-package implicit class visibility across files - Implement cross-file return-type mirroring from imported class files via populateRangeBindings hook - Add var type binding post-processing in captures.ts to resolve call-result and alias chains from same-file return types - Add variable-aware argument type inference for overload resolution - Fix pickConstructorOrClass to walk child scopes for Constructor defs (scope-resolution places them in Function scopes) - Remove over-aggressive field_access suppression in shouldEmitReadMember so ACCESSES edges emit for field steps in method chains - Enable collapseMemberCallsByCallerTarget for legacy parity - Update unit tests to use Ruby as unmigrated language example Parity: 178/178 integration tests pass in both registry-primary and legacy modes. * fix(java): address code review findings for scope-resolution migration - pickConstructorOrClass: skip inner Class scopes when walking children for Constructor defs (prevents resolving to wrong constructor in nested-class scenarios) - populateJavaCrossFileReturnTypes: filter out parameter-annotation bindings from class-scope mirroring to prevent foreign parameter types from shadowing local variables - resolveVarTypeBindings: detect ambiguous names (overloaded methods with different return types, same-named variables across scopes) and skip resolution rather than last-write-wins - sharedPrefixLength renamed to sharedSegmentCount: segment-based directory proximity for deterministic sort ordering - Add MAX_PACKAGE_FILES cap (500) to skip O(N^2) package-siblings injection for pathologically large packages * perf(java): optimize hot paths in scope-resolution migration - Replace O(D^2) list.some() dedup with O(1) Set lookup in populateJavaPackageSiblings binding injection - Replace queue.shift() O(N) with index-based O(1) iteration in closeInterfaces BFS traversal - Cache sharedSegmentCount results per file in sort comparator to avoid redundant path splitting * perf(ingestion): skip deferred accumulation for registry-primary languages The legacy call/import/heritage processing path accumulates extracted data from ALL files during the parse phase, then skips registry-primary files one-by-one during processing. For a 25K-file Java codebase this wastes ~150 MB holding calls that are never consumed. Gate the accumulation with a per-chunk file-path cache: calls, imports, heritage, constructor bindings, and assignments for registry-primary languages (Java, Python, TypeScript, Go, C#, C, C++, PHP, JavaScript, Kotlin) are no longer pushed into the deferred arrays. The scope- resolution pipeline handles these languages independently. Verified: 2258/2258 resolver integration tests pass across all languages. * fix(java): address Codex adversarial review findings - Cross-file return binding: detect ambiguous method names across imported classes (two classes with same-named methods but different return types) and delete the binding rather than first-wins - Package-siblings: only inject top-level classes (parent is Module scope) to prevent nested/inner classes from leaking to package scope - Add diagnostic log when MAX_PACKAGE_FILES cap fires so operators know same-package visibility was disabled for a large package * fix(test): force REGISTRY_PRIMARY_JAVA=false in legacy call-processor unit tests Three call-processor test suites use .java file paths to exercise legacy DAG features (MRO fast path, interface dispatch, class lookup fallback). Now that Java is in MIGRATED_LANGUAGES, the call-processor skips Java files. Force the flag off in beforeEach/afterEach so the legacy path runs, matching the existing Python pattern in the same file. --------- Co-authored-by: Test --- .../core/ingestion/languages/java/captures.ts | 130 ++++++++++- .../ingestion/languages/java/interpret.ts | 17 +- .../languages/java/package-siblings.ts | 156 +++++++++++++ .../core/ingestion/languages/java/query.ts | 75 ++++++ .../languages/java/scope-resolver.ts | 220 ++++++++++++++---- .../ingestion/pipeline-phases/parse-impl.ts | 35 ++- .../core/ingestion/registry-primary-flag.ts | 1 + .../passes/free-call-fallback.ts | 12 +- gitnexus/test/unit/call-processor.test.ts | 27 ++- .../test/unit/registry-primary-flag.test.ts | 24 +- 10 files changed, 613 insertions(+), 84 deletions(-) create mode 100644 gitnexus/src/core/ingestion/languages/java/package-siblings.ts diff --git a/gitnexus/src/core/ingestion/languages/java/captures.ts b/gitnexus/src/core/ingestion/languages/java/captures.ts index 73ea605fe..5ca470025 100644 --- a/gitnexus/src/core/ingestion/languages/java/captures.ts +++ b/gitnexus/src/core/ingestion/languages/java/captures.ts @@ -38,10 +38,6 @@ function shouldEmitReadMember(memberNode: SyntaxNode): boolean { if (parent === null) return true; switch (parent.type) { - case 'method_invocation': - // Don't emit read.member when the field_access is the object of a method_invocation - // (the method call already handles this relationship) - return parent.childForFieldName('object')?.id !== memberNode.id; case 'assignment_expression': return parent.childForFieldName('left')?.id !== memberNode.id; default: @@ -185,13 +181,137 @@ export function emitJavaScopeCaptures( callNode, JSON.stringify(argTypes), ); + + const argNames = args.map((a) => (a!.type === 'identifier' ? a!.text : '')); + if (argNames.some((n) => n !== '')) { + grouped['@reference.arg-names'] = syntheticCapture( + '@reference.arg-names', + callNode, + JSON.stringify(argNames), + ); + } } } out.push(grouped); } - return out; + return resolveVarTypeBindings(out); +} + +function resolveVarTypeBindings(matches: CaptureMatch[]): CaptureMatch[] { + const returnTypes = new Map(); + const varTypes = new Map(); + const ambiguousReturns = new Set(); + const ambiguousVars = new Set(); + + for (const m of matches) { + if ( + m['@type-binding.return'] !== undefined && + m['@type-binding.type'] !== undefined && + m['@type-binding.name'] !== undefined + ) { + const name = m['@type-binding.name'].text; + const type = m['@type-binding.type'].text; + const existing = returnTypes.get(name); + if (existing !== undefined && existing !== type) { + ambiguousReturns.add(name); + returnTypes.delete(name); + } else if (!ambiguousReturns.has(name)) { + returnTypes.set(name, type); + } + } + if ( + m['@type-binding.annotation'] !== undefined && + m['@type-binding.type'] !== undefined && + m['@type-binding.name'] !== undefined + ) { + const name = m['@type-binding.name'].text; + const t = m['@type-binding.type'].text; + if (t !== 'var') { + const existing = varTypes.get(name); + if (existing !== undefined && existing !== t) { + ambiguousVars.add(name); + varTypes.delete(name); + } else if (!ambiguousVars.has(name)) { + varTypes.set(name, t); + } + } + } + if ( + m['@type-binding.constructor'] !== undefined && + m['@type-binding.type'] !== undefined && + m['@type-binding.name'] !== undefined + ) { + const name = m['@type-binding.name'].text; + const type = m['@type-binding.type'].text; + const existing = varTypes.get(name); + if (existing !== undefined && existing !== type) { + ambiguousVars.add(name); + varTypes.delete(name); + } else if (!ambiguousVars.has(name)) { + varTypes.set(name, type); + } + } + } + + const resolved: CaptureMatch[] = []; + for (const m of matches) { + if (m['@type-binding.call-result'] !== undefined && m['@type-binding.type'] !== undefined) { + const methodName = m['@type-binding.type'].text; + const resolvedType = returnTypes.get(methodName); + if (resolvedType !== undefined) { + const patched: Record = { ...m }; + patched['@type-binding.type'] = { ...m['@type-binding.type']!, text: resolvedType }; + patched['@type-binding.annotation'] = m['@type-binding.call-result']!; + delete patched['@type-binding.call-result']; + resolved.push(patched); + continue; + } + } + if (m['@type-binding.alias'] !== undefined && m['@type-binding.type'] !== undefined) { + const sourceName = m['@type-binding.type'].text; + const resolvedType = varTypes.get(sourceName); + if (resolvedType !== undefined) { + const patched: Record = { ...m }; + patched['@type-binding.type'] = { ...m['@type-binding.type']!, text: resolvedType }; + patched['@type-binding.annotation'] = m['@type-binding.alias']!; + delete patched['@type-binding.alias']; + resolved.push(patched); + continue; + } + } + if (m['@reference.arg-names'] !== undefined && m['@reference.parameter-types'] !== undefined) { + try { + const types: string[] = JSON.parse(m['@reference.parameter-types'].text); + const names: string[] = JSON.parse(m['@reference.arg-names'].text); + let patched = false; + for (let i = 0; i < types.length; i++) { + if (types[i] === '' && names[i] !== undefined && names[i] !== '') { + const rt = varTypes.get(names[i]!); + if (rt !== undefined) { + types[i] = rt; + patched = true; + } + } + } + if (patched) { + const patchedMatch: Record = { ...m }; + patchedMatch['@reference.parameter-types'] = { + ...m['@reference.parameter-types']!, + text: JSON.stringify(types), + }; + delete patchedMatch['@reference.arg-names']; + resolved.push(patchedMatch); + continue; + } + } catch { + // pass through + } + } + resolved.push(m); + } + return resolved; } type SyntaxNode = ReturnType['parse']>['rootNode']; diff --git a/gitnexus/src/core/ingestion/languages/java/interpret.ts b/gitnexus/src/core/ingestion/languages/java/interpret.ts index 9c207d451..87da580b6 100644 --- a/gitnexus/src/core/ingestion/languages/java/interpret.ts +++ b/gitnexus/src/core/ingestion/languages/java/interpret.ts @@ -24,10 +24,11 @@ export function interpretJavaImport(captures: CaptureMatch): ParsedImport | null switch (kind) { case 'named': { // `import com.example.User;` + const simpleName = sourceCap.text.split('.').pop() ?? sourceCap.text; return { kind: 'named', - localName: nameCap?.text ?? sourceCap.text.split('.').pop() ?? sourceCap.text, - importedName: sourceCap.text, + localName: nameCap?.text ?? simpleName, + importedName: simpleName, targetRaw: sourceCap.text, }; } @@ -40,17 +41,14 @@ export function interpretJavaImport(captures: CaptureMatch): ParsedImport | null } case 'static': { // `import static com.example.Utils.format;` - // The source contains the full path including the member name - // (e.g. `com.example.Utils.format`). For file resolution we need - // the class path (`com.example.Utils`), so strip the final member - // segment. The local binding name is the member itself. const fullSource = sourceCap.text; const lastDot = fullSource.lastIndexOf('.'); + const memberName = lastDot >= 0 ? fullSource.slice(lastDot + 1) : fullSource; const classPath = lastDot >= 0 ? fullSource.slice(0, lastDot) : fullSource; return { kind: 'named', - localName: nameCap?.text ?? (lastDot >= 0 ? fullSource.slice(lastDot + 1) : fullSource), - importedName: fullSource, + localName: nameCap?.text ?? memberName, + importedName: memberName, targetRaw: classPath, }; } @@ -89,6 +87,9 @@ export function interpretJavaTypeBinding(captures: CaptureMatch): ParsedTypeBind let source: TypeRef['source'] = 'parameter-annotation'; if (captures['@type-binding.self'] !== undefined) source = 'self'; else if (captures['@type-binding.constructor'] !== undefined) source = 'constructor-inferred'; + else if (captures['@type-binding.pattern'] !== undefined) source = 'annotation'; + else if (captures['@type-binding.call-result'] !== undefined) source = 'annotation'; + else if (captures['@type-binding.alias'] !== undefined) source = 'annotation'; else if (captures['@type-binding.annotation'] !== undefined) source = 'annotation'; else if (captures['@type-binding.return'] !== undefined) source = 'return-annotation'; diff --git a/gitnexus/src/core/ingestion/languages/java/package-siblings.ts b/gitnexus/src/core/ingestion/languages/java/package-siblings.ts new file mode 100644 index 000000000..4ba5ac1e1 --- /dev/null +++ b/gitnexus/src/core/ingestion/languages/java/package-siblings.ts @@ -0,0 +1,156 @@ +/** + * Java package-scope implicit visibility. + * + * Classes in the same Java package see each other without explicit + * `import` statements. This hook groups files by `package` declaration, + * then injects cross-file class defs into each file's module-scope + * `bindingAugmentations` and mirrors type-bindings across same-package + * files — the Java equivalent of C#'s `populateNamespaceSiblings`. + */ + +import type { BindingRef, ParsedFile, ScopeId, TypeRef } from 'gitnexus-shared'; +import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import { isClassLike } from '../../scope-resolution/scope/walkers.js'; +import { getJavaParser } from './query.js'; +import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; +import { logger } from '../../../logger.js'; + +function extractPackageName(content: string, cachedTree?: unknown): string { + const tree = + (cachedTree as ReturnType['parse']> | undefined) ?? + parseSourceSafe(getJavaParser(), content); + for (const child of tree.rootNode.namedChildren) { + if (child.type === 'package_declaration') { + const scoped = child.namedChildren.find( + (c) => c.type === 'scoped_identifier' || c.type === 'identifier', + ); + return scoped?.text ?? ''; + } + } + return ''; +} + +interface PackageBucket { + readonly parsed: ParsedFile[]; + readonly moduleScopes: { filePath: string; scope: ParsedFile['scopes'][number] }[]; +} + +export function populateJavaPackageSiblings( + parsedFiles: readonly ParsedFile[], + indexes: ScopeResolutionIndexes, + ctx: { + readonly fileContents: ReadonlyMap; + readonly treeCache?: { get(filePath: string): unknown }; + }, +): void { + const buckets = new Map(); + + for (const parsed of parsedFiles) { + const content = ctx.fileContents.get(parsed.filePath); + if (content === undefined) continue; + const pkg = extractPackageName(content, ctx.treeCache?.get(parsed.filePath)); + let bucket = buckets.get(pkg); + if (bucket === undefined) { + bucket = { parsed: [], moduleScopes: [] }; + buckets.set(pkg, bucket); + } + bucket.parsed.push(parsed); + const ms = parsed.scopes.find((s) => s.kind === 'Module'); + if (ms !== undefined) { + bucket.moduleScopes.push({ filePath: parsed.filePath, scope: ms }); + } + } + + const augmentations = indexes.bindingAugmentations as Map>; + + const MAX_PACKAGE_FILES = 500; + + for (const bucket of buckets.values()) { + if (bucket.moduleScopes.length < 2) continue; + if (bucket.moduleScopes.length > MAX_PACKAGE_FILES) { + logger.warn( + `[java-package-siblings] skipping package with ${bucket.moduleScopes.length} files (cap=${MAX_PACKAGE_FILES}); same-package implicit visibility disabled for this package`, + ); + continue; + } + + const classDefs: { def: BindingRef['def']; filePath: string }[] = []; + for (const parsed of bucket.parsed) { + const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); + const moduleScopeId = moduleScope?.id; + for (const scope of parsed.scopes) { + if (scope.kind !== 'Class') continue; + if (scope.parent !== moduleScopeId) continue; + for (const def of scope.ownedDefs) { + if (isClassLike(def.type)) { + classDefs.push({ def, filePath: parsed.filePath }); + break; + } + } + } + } + + for (const { filePath, scope } of bucket.moduleScopes) { + let scopeAug = augmentations.get(scope.id); + if (scopeAug === undefined) { + scopeAug = new Map(); + augmentations.set(scope.id, scopeAug); + } + + const candidates = classDefs.filter((d) => d.filePath !== filePath); + const proximityCache = new Map(); + for (const c of candidates) { + if (!proximityCache.has(c.filePath)) { + proximityCache.set(c.filePath, sharedSegmentCount(c.filePath, filePath)); + } + } + const sorted = candidates.sort( + (a, b) => (proximityCache.get(b.filePath) ?? 0) - (proximityCache.get(a.filePath) ?? 0), + ); + + const injectedIds = new Set(); + for (const { def } of sorted) { + if (injectedIds.has(def.nodeId)) continue; + const qn = def.qualifiedName; + if (qn === undefined) continue; + injectedIds.add(def.nodeId); + const simpleName = qn.includes('.') ? qn.slice(qn.lastIndexOf('.') + 1) : qn; + let list = scopeAug.get(simpleName); + if (list === undefined) { + list = []; + scopeAug.set(simpleName, list); + } + list.push({ def, origin: 'namespace' }); + } + + const tb = scope.typeBindings as Map; + for (const sibling of bucket.moduleScopes) { + if (sibling.filePath === filePath) continue; + for (const [name, ref] of sibling.scope.typeBindings) { + if (tb.has(name)) continue; + tb.set(name, ref); + } + } + + for (const sibParsed of bucket.parsed) { + if (sibParsed.filePath === filePath) continue; + for (const sibScope of sibParsed.scopes) { + if (sibScope.kind !== 'Class') continue; + for (const [name, ref] of sibScope.typeBindings) { + if (ref.source === 'self') continue; + if (tb.has(name)) continue; + tb.set(name, ref); + } + } + } + } + } +} + +function sharedSegmentCount(a: string, b: string): number { + const sa = a.replace(/\\/g, '/').split('/'); + const sb = b.replace(/\\/g, '/').split('/'); + let i = 0; + while (i < sa.length && i < sb.length && sa[i] === sb[i]) i++; + return i; +} diff --git a/gitnexus/src/core/ingestion/languages/java/query.ts b/gitnexus/src/core/ingestion/languages/java/query.ts index 3fabbb7bf..e1e581ad5 100644 --- a/gitnexus/src/core/ingestion/languages/java/query.ts +++ b/gitnexus/src/core/ingestion/languages/java/query.ts @@ -102,6 +102,47 @@ const JAVA_SCOPE_QUERY = ` declarator: (variable_declarator name: (identifier) @type-binding.name)) @type-binding.annotation +;; Type bindings — var u = svc.getUser(); (Java 10+ call-result inference) +(local_variable_declaration + type: (type_identifier) @_var_type + (#eq? @_var_type "var") + declarator: (variable_declarator + name: (identifier) @type-binding.name + value: (method_invocation + name: (identifier) @type-binding.type))) @type-binding.call-result + +;; Type bindings — var alias = u; (Java 10+ alias inference) +(local_variable_declaration + type: (type_identifier) @_var_type + (#eq? @_var_type "var") + declarator: (variable_declarator + name: (identifier) @type-binding.name + value: (identifier) @type-binding.type)) @type-binding.alias + +;; Type bindings — var addr = user.address; (Java 10+ field-access alias) +(local_variable_declaration + type: (type_identifier) @_var_type + (#eq? @_var_type "var") + declarator: (variable_declarator + name: (identifier) @type-binding.name + value: (field_access + field: (identifier) @type-binding.type))) @type-binding.alias + +;; Type bindings — enhanced-for with var: for (var user : users) +(enhanced_for_statement + (type_identifier) @_var_type + (#eq? @_var_type "var") + (identifier) @type-binding.name + (identifier) @type-binding.type) @type-binding.alias + +;; Enhanced-for with var + method iterable: for (var user : data.values()) +(enhanced_for_statement + (type_identifier) @_var_type + (#eq? @_var_type "var") + (identifier) @type-binding.name + (method_invocation + object: (identifier) @type-binding.type)) @type-binding.alias + ;; Type bindings — var u = new User(); (Java 10+ local variable type inference) ;; tree-sitter-java parses \`var\` as a \`type_identifier\` with text "var". ;; The type-binding.constructor anchor fires when the rhs is an @@ -143,6 +184,16 @@ const JAVA_SCOPE_QUERY = ` type: (generic_type) @type-binding.type name: (identifier) @type-binding.name) @type-binding.annotation +;; Type bindings — instanceof pattern (Java 16+): if (obj instanceof User user) +(instanceof_expression + (type_identifier) @type-binding.type + (identifier) @type-binding.name) @type-binding.pattern + +;; Type bindings — switch case pattern (Java 21+): case User user -> +(type_pattern + (type_identifier) @type-binding.type + (identifier) @type-binding.name) @type-binding.pattern + ;; References — all method calls: foo() and obj.method() ;; tree-sitter-java's query engine drops negation-based \`!object\` ;; patterns when a positive \`object:\` pattern exists for the same @@ -166,6 +217,30 @@ const JAVA_SCOPE_QUERY = ` (object_creation_expression type: (scoped_type_identifier) @reference.call.constructor.qualified) @reference.call.constructor +;; References — method references: User::getName, obj::method +(method_reference + (identifier) @reference.receiver + (identifier) @reference.name) @reference.call.member + +;; References — this::method and super::method +(method_reference + (this) @reference.receiver + (identifier) @reference.name) @reference.call.member + +(method_reference + (super) @reference.receiver + (identifier) @reference.name) @reference.call.member + +;; References — field_access::method: responseBuilder::buildResponse +(method_reference + (field_access) @reference.receiver + (identifier) @reference.name) @reference.call.member + +;; References — constructor references: User::new +(method_reference + (identifier) @reference.name + "new") @reference.call.constructor + ;; References — field/property writes: obj.name = "x" (assignment_expression left: (field_access diff --git a/gitnexus/src/core/ingestion/languages/java/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/java/scope-resolver.ts index dac974cc7..a94c1c3b1 100644 --- a/gitnexus/src/core/ingestion/languages/java/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/java/scope-resolver.ts @@ -4,53 +4,29 @@ * * ## Registry-primary parity status * - * Java is **not** in `MIGRATED_LANGUAGES` — the scope-resolution - * registry runs in shadow mode only. Parity in forced registry mode - * (`REGISTRY_PRIMARY_JAVA=1`) is 143/172 (83%). The 29 gaps fall into: + * Java is in `MIGRATED_LANGUAGES` — the scope-resolution registry is + * the primary call-resolution path. Parity: 178/178 (100%). * - * - switch pattern binding / sealed-class exhaustiveness - * - Map.values() / entrySet() iteration type propagation - * - assignment / method chain return-type propagation across files - * - virtual dispatch / interface default methods - * - * These are the same category of advanced-resolution gaps seen in prior - * migrations (Python, C#, Go). Parity is below the ≥99% flip threshold - * per RFC §6.4. - * - * **CI visibility:** Because Java is absent from `MIGRATED_LANGUAGES`, - * the parity CI workflow (`ci-scope-parity.yml`) does not run Java in - * either `REGISTRY_PRIMARY_JAVA=0` or `=1` mode. Regressions in forced - * mode are only visible via manual `REGISTRY_PRIMARY_JAVA=1 npx vitest - * run java.test.ts`. Before flipping Java to registry-primary, a - * non-required CI step should be added to run Java tests in forced mode - * and report parity as a dashboard input. - * - * **Parity baseline (29 failures):** The 29 gaps in forced registry mode - * are tracked in this PR (#1482) and this JSDoc. If the gap count - * changes (up or down), update this baseline accordingly. - * - * ### Known flip-blockers (must fix before adding to MIGRATED_LANGUAGES) - * - * - Varargs arity: fixed-prefix count is now preserved, but no - * integration fixture exercises the 0-arg rejection path yet. - * - Static import resolution: `import static X.Y.m` now correctly - * resolves to `X/Y.java` (the class), not `X/Y/m.java` (the member). - * Edge cases with nested classes may remain. - * - Generic superclass receiver binding: `BaseModel` now strips - * to `BaseModel` via JVM type-erasure fallback in `stripGeneric`. - * - Wildcard import (`import com.example.*`) file selection is - * nondeterministic when multiple classes share a package directory. - * May produce wrong-file edges in forced mode. - * - Qualified generic type parameters in field/parameter annotations - * (`com.example.BaseModel`) — rare in practice but may miss - * resolution when the full qualifier is present with generics. + * **CI visibility:** The parity CI workflow (`ci-scope-parity.yml`) + * runs Java tests in both `REGISTRY_PRIMARY_JAVA=0` and `=1` modes + * automatically. */ -import type { ParsedFile } from 'gitnexus-shared'; +import type { ParsedFile, TypeRef } from 'gitnexus-shared'; import { SupportedLanguages } from 'gitnexus-shared'; +import type { KnowledgeGraph } from '../../../graph/types.js'; import { buildMro, defaultLinearize } from '../../scope-resolution/passes/mro.js'; -import { populateClassOwnedMembers } from '../../scope-resolution/scope/walkers.js'; +import { resolveDefGraphId } from '../../scope-resolution/graph-bridge/ids.js'; +import type { GraphNodeLookup } from '../../scope-resolution/graph-bridge/node-lookup.js'; +import { + isClassLike, + lookupBindingsAt, + namesAtScope, + populateClassOwnedMembers, +} from '../../scope-resolution/scope/walkers.js'; import type { ScopeResolver } from '../../scope-resolution/contract/scope-resolver.js'; +import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import { followChainPostFinalize } from '../../scope-resolution/passes/imported-return-types.js'; import { javaProvider } from '../java.js'; import { javaArityCompatibility, @@ -58,6 +34,7 @@ import { resolveJavaImportTarget, type JavaResolveContext, } from './index.js'; +import { populateJavaPackageSiblings } from './package-siblings.js'; const javaScopeResolver: ScopeResolver = { language: SupportedLanguages.Java, @@ -76,22 +53,167 @@ const javaScopeResolver: ScopeResolver = { arityCompatibility: (callsite, def) => javaArityCompatibility(def, callsite), - buildMro: (graph, parsedFiles, nodeLookup) => - buildMro(graph, parsedFiles, nodeLookup, defaultLinearize), + buildMro: buildJavaMro, populateOwners: (parsed: ParsedFile) => populateClassOwnedMembers(parsed), isSuperReceiver: (text) => text.trim() === 'super', - // Java is statically typed — field-fallback heuristic stays off fieldFallbackOnMethodLookup: false, propagatesReturnTypesAcrossImports: true, - - // Java doesn't collapse member calls - collapseMemberCallsByCallerTarget: false, - - // Hoist return-type bindings to Module scope for cross-file propagation + collapseMemberCallsByCallerTarget: true, hoistTypeBindingsToModule: true, + + populateNamespaceSiblings: populateJavaPackageSiblings, + populateRangeBindings: populateJavaCrossFileReturnTypes, }; export { javaScopeResolver }; + +function populateJavaCrossFileReturnTypes( + parsedFiles: readonly ParsedFile[], + indexes: ScopeResolutionIndexes, +): void { + const moduleScopeByFile = new Map(); + const classScopesByFile = new Map(); + for (const parsed of parsedFiles) { + const ms = parsed.scopes.find((s) => s.kind === 'Module'); + if (ms !== undefined) moduleScopeByFile.set(parsed.filePath, ms); + const cs = parsed.scopes.filter((s) => s.kind === 'Class'); + if (cs.length > 0) classScopesByFile.set(parsed.filePath, cs); + } + + for (const parsed of parsedFiles) { + const importerModule = moduleScopeByFile.get(parsed.filePath); + if (importerModule === undefined) continue; + + const ambiguousMirrors = new Set(); + for (const name of namesAtScope(importerModule.id, indexes)) { + const refs = lookupBindingsAt(importerModule.id, name, indexes); + for (const ref of refs) { + if (ref.origin !== 'import' && ref.origin !== 'reexport') continue; + if (!isClassLike(ref.def.type)) continue; + + const sourceModule = moduleScopeByFile.get(ref.def.filePath); + if (sourceModule === undefined) continue; + + const tb = importerModule.typeBindings as Map; + for (const [srcName, srcRef] of sourceModule.typeBindings) { + if (srcRef.source !== 'return-annotation') continue; + if (ambiguousMirrors.has(srcName)) continue; + const existing = tb.get(srcName); + if (existing !== undefined && existing.rawName !== srcRef.rawName) { + ambiguousMirrors.add(srcName); + tb.delete(srcName); + continue; + } + if (existing === undefined) tb.set(srcName, srcRef); + } + + for (const classScope of classScopesByFile.get(ref.def.filePath) ?? []) { + for (const [srcName, srcRef] of classScope.typeBindings) { + if (srcRef.source === 'self' || srcRef.source === 'parameter-annotation') continue; + if (ambiguousMirrors.has(srcName)) continue; + const existing = tb.get(srcName); + if (existing !== undefined && existing.rawName !== srcRef.rawName) { + ambiguousMirrors.add(srcName); + tb.delete(srcName); + continue; + } + if (existing === undefined) tb.set(srcName, srcRef); + } + } + } + } + + for (const [name, ref] of importerModule.typeBindings) { + const resolved = followChainPostFinalize(ref, importerModule.id, indexes); + if (resolved !== ref) { + (importerModule.typeBindings as Map).set(name, resolved); + } + } + } + + for (const parsed of parsedFiles) { + const moduleScopeId = moduleScopeByFile.get(parsed.filePath)?.id; + for (const scope of parsed.scopes) { + if (scope.id === moduleScopeId) continue; + for (const [name, ref] of scope.typeBindings) { + const resolved = followChainPostFinalize(ref, scope.id, indexes); + if (resolved !== ref) { + (scope.typeBindings as Map).set(name, resolved); + } + } + } + } +} + +function buildJavaMro( + graph: KnowledgeGraph, + parsedFiles: readonly ParsedFile[], + nodeLookup: GraphNodeLookup, +): Map { + const mro = buildMro(graph, parsedFiles, nodeLookup, defaultLinearize); + + const defIdByGraphId = new Map(); + for (const parsed of parsedFiles) { + for (const def of parsed.localDefs) { + if (!isClassLike(def.type)) continue; + const graphId = resolveDefGraphId(parsed.filePath, def, nodeLookup); + if (graphId !== undefined) defIdByGraphId.set(graphId, def.nodeId); + } + } + + const directImpls = new Map(); + for (const rel of graph.iterRelationshipsByType('IMPLEMENTS')) { + const source = defIdByGraphId.get(rel.sourceId); + const target = defIdByGraphId.get(rel.targetId); + if (source === undefined || target === undefined) continue; + let list = directImpls.get(source); + if (list === undefined) { + list = []; + directImpls.set(source, list); + } + if (!list.includes(target)) list.push(target); + } + + for (const [classDefId, extendsMro] of mro) { + const ancestorChain = [classDefId, ...extendsMro]; + const seeds: string[] = []; + for (const ancestorId of ancestorChain) { + for (const ifaceId of directImpls.get(ancestorId) ?? []) { + seeds.push(ifaceId); + } + } + if (seeds.length === 0) continue; + const interfaces = closeInterfaces(seeds, directImpls); + mro.set(classDefId, [...extendsMro, ...interfaces.filter((i) => !extendsMro.includes(i))]); + } + + for (const [classDefId, ifaces] of directImpls) { + if (mro.has(classDefId)) continue; + mro.set(classDefId, closeInterfaces([...ifaces], directImpls)); + } + + return mro; +} + +function closeInterfaces( + seeds: readonly string[], + directImpls: ReadonlyMap, +): string[] { + const out: string[] = []; + const seen = new Set(); + const queue: string[] = [...seeds]; + let head = 0; + while (head < queue.length) { + const cur = queue[head++]!; + if (seen.has(cur)) continue; + seen.add(cur); + out.push(cur); + for (const next of directImpls.get(cur) ?? []) { + if (!seen.has(next)) queue.push(next); + } + } + return out; +} diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index bf954b5cf..4d06b8d58 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -46,6 +46,7 @@ import { import { createResolutionContext } from '../model/resolution-context.js'; import { ASTCache, createASTCache } from '../ast-cache.js'; import { type PipelineProgress, getLanguageFromFilename } from 'gitnexus-shared'; +import { isRegistryPrimary } from '../registry-primary-flag.js'; import { readFileContents } from '../filesystem-walker.js'; import { isLanguageAvailable } from '../../tree-sitter/parser-loader.js'; import { createWorkerPool, WorkerPoolInitializationError } from '../workers/worker-pool.js'; @@ -606,11 +607,31 @@ export async function runChunkedParseAndResolve( if (chunkNeedsSynthesis[chunkIdx]) { anyChunkNeedsWildcardSynth = true; } - for (const item of chunkWorkerData.imports) deferredWorkerImports.push(item); - for (const item of chunkWorkerData.calls) deferredWorkerCalls.push(item); - for (const item of chunkWorkerData.heritage) deferredWorkerHeritage.push(item); - for (const item of chunkWorkerData.constructorBindings) - deferredConstructorBindings.push(item); + const skipFile = new Set(); + const checkFile = new Set(); + const shouldAccumulate = (filePath: string): boolean => { + if (checkFile.has(filePath)) return true; + if (skipFile.has(filePath)) return false; + const lang = getLanguageFromFilename(filePath); + if (lang !== null && isRegistryPrimary(lang)) { + skipFile.add(filePath); + return false; + } + checkFile.add(filePath); + return true; + }; + for (const item of chunkWorkerData.imports) { + if (shouldAccumulate(item.filePath)) deferredWorkerImports.push(item); + } + for (const item of chunkWorkerData.calls) { + if (shouldAccumulate(item.filePath)) deferredWorkerCalls.push(item); + } + for (const item of chunkWorkerData.heritage) { + if (shouldAccumulate(item.filePath)) deferredWorkerHeritage.push(item); + } + for (const item of chunkWorkerData.constructorBindings) { + if (shouldAccumulate(item.filePath)) deferredConstructorBindings.push(item); + } // Aggregate worker-produced ParsedFile artifacts so scope- // resolution can use them as a re-extraction cache (skips its // own tree-sitter re-parse on warm runs). @@ -618,7 +639,9 @@ export async function runChunkedParseAndResolve( for (const item of chunkWorkerData.parsedFiles) allParsedFiles.push(item); } if (chunkWorkerData.assignments?.length) { - for (const item of chunkWorkerData.assignments) deferredAssignments.push(item); + for (const item of chunkWorkerData.assignments) { + if (shouldAccumulate(item.filePath)) deferredAssignments.push(item); + } } if (chunkWorkerData.fileScopeBindings?.length) { diff --git a/gitnexus/src/core/ingestion/registry-primary-flag.ts b/gitnexus/src/core/ingestion/registry-primary-flag.ts index 9016a68b5..56e39962a 100644 --- a/gitnexus/src/core/ingestion/registry-primary-flag.ts +++ b/gitnexus/src/core/ingestion/registry-primary-flag.ts @@ -77,6 +77,7 @@ export const MIGRATED_LANGUAGES: ReadonlySet = new Set { describe('processCalls — Phase P class lookup fallback', () => { let graph: ReturnType; let ctx: ResolutionContext; + let prevRegistryJava: string | undefined; beforeEach(() => { graph = createKnowledgeGraph(); ctx = createResolutionContext(); + prevRegistryJava = process.env['REGISTRY_PRIMARY_JAVA']; + process.env['REGISTRY_PRIMARY_JAVA'] = 'false'; + }); + + afterEach(() => { + if (prevRegistryJava === undefined) delete process.env['REGISTRY_PRIMARY_JAVA']; + else process.env['REGISTRY_PRIMARY_JAVA'] = prevRegistryJava; }); it('uses lookupClassByName to override interface receiver types for cross-file virtual dispatch', async () => { @@ -2147,10 +2155,13 @@ describe('processNextjsFetchRoutes', () => { describe('processCallsFromExtracted — interface dispatch', () => { let graph: ReturnType; let ctx: ResolutionContext; + let prevRegistryJava: string | undefined; beforeEach(() => { graph = createKnowledgeGraph(); ctx = createResolutionContext(); + prevRegistryJava = process.env['REGISTRY_PRIMARY_JAVA']; + process.env['REGISTRY_PRIMARY_JAVA'] = 'false'; const ifaceFile = 'contracts/Action.java'; const runnerFile = 'runner.java'; const implA = 'impl/A.java'; @@ -2195,6 +2206,11 @@ describe('processCallsFromExtracted — interface dispatch', () => { }); }); + afterEach(() => { + if (prevRegistryJava === undefined) delete process.env['REGISTRY_PRIMARY_JAVA']; + else process.env['REGISTRY_PRIMARY_JAVA'] = prevRegistryJava; + }); + it('adds CALLS to interface method plus lower-confidence edges to implementing methods', async () => { const heritage: ExtractedHeritage[] = [ { filePath: 'impl/A.java', className: 'A', parentName: 'Action', kind: 'implements' }, @@ -2241,21 +2257,26 @@ describe('processCalls — D0 MRO fast path (SM-10)', () => { let graph: ReturnType; let ctx: ResolutionContext; let prevRegistryPython: string | undefined; + let prevRegistryJava: string | undefined; beforeEach(() => { graph = createKnowledgeGraph(); ctx = createResolutionContext(); // These tests exercise the LEGACY call-resolution DAG directly - // using .py fixtures. Python defaults to registry-primary now - // (MIGRATED_LANGUAGES), which gates call-processor out for - // Python files. Force the flag off so the legacy DAG runs. + // using .py/.java fixtures. Python and Java default to registry- + // primary now (MIGRATED_LANGUAGES), which gates call-processor + // out for those files. Force the flags off so the legacy DAG runs. prevRegistryPython = process.env['REGISTRY_PRIMARY_PYTHON']; process.env['REGISTRY_PRIMARY_PYTHON'] = 'false'; + prevRegistryJava = process.env['REGISTRY_PRIMARY_JAVA']; + process.env['REGISTRY_PRIMARY_JAVA'] = 'false'; }); afterEach(() => { if (prevRegistryPython === undefined) delete process.env['REGISTRY_PRIMARY_PYTHON']; else process.env['REGISTRY_PRIMARY_PYTHON'] = prevRegistryPython; + if (prevRegistryJava === undefined) delete process.env['REGISTRY_PRIMARY_JAVA']; + else process.env['REGISTRY_PRIMARY_JAVA'] = prevRegistryJava; }); const setupChildParent = () => { diff --git a/gitnexus/test/unit/registry-primary-flag.test.ts b/gitnexus/test/unit/registry-primary-flag.test.ts index e800bd14a..856dcef43 100644 --- a/gitnexus/test/unit/registry-primary-flag.test.ts +++ b/gitnexus/test/unit/registry-primary-flag.test.ts @@ -108,20 +108,20 @@ describe('isRegistryPrimary', () => { it('isolates flags per-language (one on does not affect others)', () => { process.env['REGISTRY_PRIMARY_PYTHON'] = 'true'; expect(isRegistryPrimary(SupportedLanguages.Python)).toBe(true); - // Java is not in MIGRATED_LANGUAGES — default false stays + // Ruby is not in MIGRATED_LANGUAGES — default false stays // false regardless of Python's flag. - expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false); + expect(isRegistryPrimary(SupportedLanguages.Ruby)).toBe(false); }); it('respects a mid-process env-var mutation (no stale cache)', () => { - // Use Java — not in MIGRATED_LANGUAGES — so the unset default is + // Use Ruby — not in MIGRATED_LANGUAGES — so the unset default is // deterministically `false`, independent of which languages have // been flipped to registry-primary. - expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false); - process.env['REGISTRY_PRIMARY_JAVA'] = 'true'; - expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(true); - delete process.env['REGISTRY_PRIMARY_JAVA']; - expect(isRegistryPrimary(SupportedLanguages.Java)).toBe(false); + expect(isRegistryPrimary(SupportedLanguages.Ruby)).toBe(false); + process.env['REGISTRY_PRIMARY_RUBY'] = 'true'; + expect(isRegistryPrimary(SupportedLanguages.Ruby)).toBe(true); + delete process.env['REGISTRY_PRIMARY_RUBY']; + expect(isRegistryPrimary(SupportedLanguages.Ruby)).toBe(false); }); it('handles the CPlusPlus → REGISTRY_PRIMARY_CPP mapping correctly', () => { @@ -150,7 +150,7 @@ describe('primaryLanguages', () => { it('returns exactly the flipped languages (env opts in unmigrated, opts out migrated)', () => { // Migrated languages are default-on; each must be opted out here when - // testing explicit env overrides. Java (unmigrated) opts in. + // testing explicit env overrides. Ruby (unmigrated) opts in. // Opt out every member of MIGRATED_LANGUAGES dynamically so this test // does not have to be updated each time a new language ships its // Ring 3 migration (C++ and PHP joined the set in their respective @@ -158,15 +158,15 @@ describe('primaryLanguages', () => { for (const lang of MIGRATED_LANGUAGES) { process.env[envVarNameFor(lang)] = 'false'; } - process.env['REGISTRY_PRIMARY_JAVA'] = '1'; + process.env['REGISTRY_PRIMARY_RUBY'] = '1'; const enabled = primaryLanguages(); expect(enabled.has(SupportedLanguages.Python)).toBe(false); expect(enabled.has(SupportedLanguages.CSharp)).toBe(false); expect(enabled.has(SupportedLanguages.Go)).toBe(false); expect(enabled.has(SupportedLanguages.CPlusPlus)).toBe(false); expect(enabled.has(SupportedLanguages.PHP)).toBe(false); - expect(enabled.has(SupportedLanguages.Java)).toBe(true); - // Only Java is on: migrated defaults overridden off, Java explicitly on. + expect(enabled.has(SupportedLanguages.Ruby)).toBe(true); + // Only Ruby is on: migrated defaults overridden off, Ruby explicitly on. expect(enabled.size).toBe(1); }); From 2006a3e5ac399a3553b0633818741d9604d66ae6 Mon Sep 17 00:00:00 2001 From: Lucas van Staden Date: Sun, 24 May 2026 22:53:20 +0800 Subject: [PATCH 07/10] fix(php): reduce memory during deferred-call accumulation and scope-resolution (#1800) --- .../core/ingestion/finalize-orchestrator.ts | 1 + .../languages/php/namespace-siblings.ts | 136 +++++++++++++----- .../model/scope-resolution-indexes.ts | 6 + .../scope-resolution/pipeline/phase.ts | 10 ++ .../scope-resolution/scope/walkers.ts | 32 +++-- .../unit/php-namespace-extraction.test.ts | 129 +++++++++++++++++ 6 files changed, 269 insertions(+), 45 deletions(-) create mode 100644 gitnexus/test/unit/php-namespace-extraction.test.ts diff --git a/gitnexus/src/core/ingestion/finalize-orchestrator.ts b/gitnexus/src/core/ingestion/finalize-orchestrator.ts index 70dff9874..50cbc6b2a 100644 --- a/gitnexus/src/core/ingestion/finalize-orchestrator.ts +++ b/gitnexus/src/core/ingestion/finalize-orchestrator.ts @@ -144,6 +144,7 @@ export function finalizeScopeModel( // AFTER `finalizeScopeModel` returns, before `resolveReferenceSites` // consumes the bundle. Most languages leave it empty. bindingAugmentations: new Map(), + workspaceFqnBindings: new Map(), referenceSites: Object.freeze([...allReferenceSites]), sccs: finalizeOut.sccs, stats: finalizeOut.stats, diff --git a/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts b/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts index 20b99d971..875e6642b 100644 --- a/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts +++ b/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts @@ -28,8 +28,6 @@ import type { BindingRef, ParsedFile, Scope, ScopeId, SymbolDefinition } from 'gitnexus-shared'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import { getPhpParser } from './query.js'; -import { getTreeSitterBufferSize } from '../../constants.js'; -import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js'; // ─── PHP file structure extraction ────────────────────────────────────────── @@ -40,21 +38,94 @@ interface PhpFileStructure { type PhpTree = ReturnType['parse']>; +const NAMESPACE_RE = /^\s*namespace\s+([\w\\]+)\s*[;{]/i; +const HEREDOC_START_RE = /<<<\s*['"]?(\w+)['"]?\s*$/; + +/** + * Extract a PHP namespace declaration from raw source without tree-sitter. + * + * Single-pass line scanner that skips heredoc/nowdoc bodies, block + * comments, and single-line comments before matching. This avoids the + * false positives that a multiline regex produces when `namespace` appears + * inside a heredoc, nowdoc, string, or comment. + */ +export function extractNamespaceViaScanner(content: string): string { + const lines = content.split('\n'); + let inBlockComment = false; + let heredocDelimiter: string | null = null; + + for (const raw of lines) { + if (heredocDelimiter !== null) { + const trimmed = raw.trim(); + if (trimmed === heredocDelimiter + ';' || trimmed === heredocDelimiter) { + heredocDelimiter = null; + } + continue; + } + + if (inBlockComment) { + if (raw.includes('*/')) { + inBlockComment = false; + } + continue; + } + + let line = raw; + + const blockStart = line.indexOf('/*'); + if (blockStart >= 0) { + const blockEnd = line.indexOf('*/', blockStart + 2); + if (blockEnd >= 0) { + line = line.slice(0, blockStart) + line.slice(blockEnd + 2); + } else { + line = line.slice(0, blockStart); + inBlockComment = true; + } + } + + const slashIdx = line.indexOf('//'); + const hashIdx = line.indexOf('#'); + if (slashIdx >= 0 && (hashIdx < 0 || slashIdx < hashIdx)) { + line = line.slice(0, slashIdx); + } else if (hashIdx >= 0) { + line = line.slice(0, hashIdx); + } + + const heredocMatch = raw.match(HEREDOC_START_RE); + if (heredocMatch) { + heredocDelimiter = heredocMatch[1]; + continue; + } + + const stripped = line.replace(/<\?php/gi, '').replace(/declare\s*\([^)]*\)\s*;?/gi, ''); + const nsMatch = stripped.match(NAMESPACE_RE); + if (nsMatch) { + return nsMatch[1]; + } + } + + return ''; +} + /** * Extract the declared namespace from a PHP file's source. * Uses the cached AST tree when available to avoid re-parsing. + * + * When no cached tree is available (worker-parsed files can't transfer + * native Tree objects across MessageChannels), uses a line scanner + * instead of re-parsing every file with tree-sitter. For 16K+ PHP files + * this eliminates ~16K tree-sitter re-parses during the namespace-siblings + * pass. See: https://github.com/abhigyanpatwari/GitNexus/issues/1741 */ -function extractPhpFileStructure(content: string, cachedTree: unknown): PhpFileStructure { - const tree = - (cachedTree as PhpTree | undefined) ?? - parseSourceSafe(getPhpParser(), content, undefined, { - bufferSize: getTreeSitterBufferSize(content), - }); +export function extractPhpFileStructure(content: string, cachedTree: unknown): PhpFileStructure { + if (!cachedTree) { + return { namespace: extractNamespaceViaScanner(content) }; + } // Walk top-level nodes looking for namespace_definition. // PHP files have at most one namespace declaration (PSR-4 convention). // `namespace_definition` has a `name:` field of type `namespace_name`. - const root = tree.rootNode; + const root = (cachedTree as PhpTree).rootNode; for (let i = 0; i < root.namedChildCount; i++) { const child = root.namedChild(i); if (child === null) continue; @@ -240,35 +311,28 @@ export function populatePhpNamespaceSiblings( } } - // Step 3b: Inject fully-qualified-name bindings into every PHP file's - // Module scope. PHP `\App\Models\User` (leading-backslash FQN) and - // `App\Models\User` (already-qualified relative) on a parameter or - // typed receiver must resolve to the exact namespace-qualified class - // regardless of which simple-name `User` the caller's `use` imports - // shadowed. The shared `findClassBindingInScope` scope-chain walk - // consumes these augmentations via `lookupBindingsAt`, so adding the - // qualified key on every file's module scope routes FQN-receivers to - // the right def. Codex PR #1497 review, finding 1. + // Step 3b: Register FQN bindings in a workspace-level map instead of + // per-scope augmentations. PHP `\App\Models\User` and `App\Models\User` + // must resolve regardless of which file the lookup originates from. + // `lookupBindingsAt` consults `workspaceFqnBindings` as a third source. // - // Cost: O(PHP files × class-like defs in the workspace) augmentation - // entries. Bounded and acceptable in practice — typical PHP projects - // have hundreds of files and classes, not tens of thousands. - for (const parsed of parsedFiles) { - const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); - if (moduleScope === undefined) continue; - const moduleScopeId = moduleScope.id; - - for (const [ns, bucket] of buckets) { - if (ns === '') continue; // global-namespace classes have no qualified form to register - for (const def of bucket.classDefs) { - const q = def.qualifiedName ?? ''; - const simpleName = q.includes('\\') ? q.slice(q.lastIndexOf('\\') + 1) : q; - if (simpleName === '') continue; - const fqn = `${ns}\\${simpleName}`; - const arr = getAugmentationBucket(augmentations, moduleScopeId, fqn); - if (arr.some((b) => b.def.nodeId === def.nodeId)) continue; - arr.push({ def, origin: 'namespace' }); + // Cost: O(class-like defs) entries — NOT O(files × classDefs). For 16K + // PHP files with 5K classes, this is 5K entries instead of 80M. + const fqnMap = indexes.workspaceFqnBindings as Map; + for (const [ns, bucket] of buckets) { + if (ns === '') continue; + for (const def of bucket.classDefs) { + const q = def.qualifiedName ?? ''; + const simpleName = q.includes('\\') ? q.slice(q.lastIndexOf('\\') + 1) : q; + if (simpleName === '') continue; + const fqn = `${ns}\\${simpleName}`; + let arr = fqnMap.get(fqn); + if (arr === undefined) { + arr = []; + fqnMap.set(fqn, arr); } + if (arr.some((b) => b.def.nodeId === def.nodeId)) continue; + arr.push({ def, origin: 'namespace' }); } } diff --git a/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts b/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts index a3fba7b30..561595ac5 100644 --- a/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts +++ b/gitnexus/src/core/ingestion/model/scope-resolution-indexes.ts @@ -77,6 +77,12 @@ export interface ScopeResolutionIndexes { * are returned first and win duplicate `def.nodeId` metadata, with * unique augmentations appended after. See I8. */ readonly bindingAugmentations: ReadonlyMap>; + /** Workspace-level FQN binding lookup. Populated by PHP namespace- + * siblings Step 3b as a shared map instead of per-scope duplication. + * Consulted by `lookupBindingsAt` as a third source after finalized + * and per-scope augmented bindings. Keys are backslash-separated FQNs + * (e.g. `App\Models\User`). */ + readonly workspaceFqnBindings: ReadonlyMap; /** Pre-resolution usage facts; consumed by the resolution phase. */ readonly referenceSites: readonly ReferenceSite[]; /** SCC condensation of the file-level import graph — callers that want diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts index 31b712d78..686d70fc7 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/phase.ts @@ -173,6 +173,16 @@ export const scopeResolutionPhase: PipelinePhase = { provider, ); + // Release file contents and pre-extracted entries after each language + // to reduce memory pressure. For large codebases (16K+ PHP files), + // holding all source code simultaneously with scope trees causes OOM. + // See: https://github.com/abhigyanpatwari/GitNexus/issues/1741 + files.length = 0; + contents.clear(); + for (const fp of filePaths) { + preExtractedByPath.delete(fp); + } + anyRan = true; totalFiles += stats.filesProcessed; totalImports += stats.importsEmitted; diff --git a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts index a9bb188d2..df6d259b5 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts @@ -55,20 +55,34 @@ export function lookupBindingsAt( ): readonly BindingRef[] { const finalized = scopes.bindings.get(scopeId)?.get(name); const augmented = scopes.bindingAugmentations.get(scopeId)?.get(name); + const workspace = scopes.workspaceFqnBindings?.get(name); const fLen = finalized?.length ?? 0; const aLen = augmented?.length ?? 0; - if (fLen === 0 && aLen === 0) return EMPTY_BINDINGS; - if (aLen === 0) return finalized!; - if (fLen === 0) return augmented!; + const wLen = workspace?.length ?? 0; + if (fLen === 0 && aLen === 0 && wLen === 0) return EMPTY_BINDINGS; + if (aLen === 0 && wLen === 0) return finalized!; + if (fLen === 0 && wLen === 0) return augmented!; + if (fLen === 0 && aLen === 0) return workspace!; const seen = new Set(); const out: BindingRef[] = []; - for (const r of finalized!) { - seen.add(r.def.nodeId); - out.push(r); + if (fLen > 0) { + for (const r of finalized!) { + seen.add(r.def.nodeId); + out.push(r); + } } - for (const r of augmented!) { - if (seen.has(r.def.nodeId)) continue; - out.push(r); + if (aLen > 0) { + for (const r of augmented!) { + if (seen.has(r.def.nodeId)) continue; + seen.add(r.def.nodeId); + out.push(r); + } + } + if (wLen > 0) { + for (const r of workspace!) { + if (seen.has(r.def.nodeId)) continue; + out.push(r); + } } return out; } diff --git a/gitnexus/test/unit/php-namespace-extraction.test.ts b/gitnexus/test/unit/php-namespace-extraction.test.ts new file mode 100644 index 000000000..589a4628a --- /dev/null +++ b/gitnexus/test/unit/php-namespace-extraction.test.ts @@ -0,0 +1,129 @@ +import { describe, it, expect } from 'vitest'; +import { extractNamespaceViaScanner } from '../../src/core/ingestion/languages/php/namespace-siblings.js'; + +describe('extractNamespaceViaScanner', () => { + it('extracts standard namespace declaration', () => { + const src = ` { + const src = ` { + const src = ` { + const src = ` { + const src = ` { + const src = ` { + expect(extractNamespaceViaScanner('')).toBe(''); + }); + + it('handles uppercase NAMESPACE keyword (case-insensitive)', () => { + const src = ` { + const src = ` { + const src = [ + ' { + const src = [ + ' { + const src = [' { + const src = ` { + const src = ` { + const src = ` { + const src = ` { + const src = [' { + const src = ` { + const src = [ + ' { + const src = [ + ' Date: Sun, 24 May 2026 19:53:07 +0100 Subject: [PATCH 08/10] fix(group): move manifest/workspace extraction before closeLbug (#1802) (#1807) --- gitnexus/src/core/group/sync.ts | 123 ++++++++++++-------------- gitnexus/test/unit/group/sync.test.ts | 123 ++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 64 deletions(-) diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index cd64fdf8c..5c535f29f 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -91,22 +91,23 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis let dbExecutors: Map | undefined; let registryEntries: RegistryEntry[] | undefined; - const eo = opts?.extractorOverride; - if (eo && eo.length === 0) { - autoContracts = await (eo as () => Promise)(); - } else { - registryEntries = await readRegistry(); - const entries = registryEntries; - const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries); - const httpEx = new HttpRouteExtractor(); - const grpcEx = new GrpcExtractor(); - const thriftEx = new ThriftExtractor(); - const topicEx = new TopicExtractor(); - const includeEx = new IncludeExtractor(); - dbExecutors = new Map(); - const openPoolIds: string[] = []; + const openPoolIds: string[] = []; + + try { + const eo = opts?.extractorOverride; + if (eo && eo.length === 0) { + autoContracts = await (eo as () => Promise)(); + } else { + registryEntries = await readRegistry(); + const entries = registryEntries; + const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries); + const httpEx = new HttpRouteExtractor(); + const grpcEx = new GrpcExtractor(); + const thriftEx = new ThriftExtractor(); + const topicEx = new TopicExtractor(); + const includeEx = new IncludeExtractor(); + dbExecutors = new Map(); - try { for (const [groupPath, regName] of Object.entries(config.repos)) { const handle = await resolve(regName, groupPath); if (!handle) { @@ -201,64 +202,58 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis missingRepos.push(groupPath); } } - } finally { - for (const id of [...new Set(openPoolIds)]) { - await closeLbug(id).catch(() => {}); + } + + // Workspace discovery and manifest extraction run inside this outer try + // block so dbExecutors closures resolve against live pools (issue #1802). + // The finally below closes pools after this completes (or throws). + let allLinks = [...config.links]; + + if (config.detect.workspace_deps) { + const repoPaths = new Map(); + if (!registryEntries) registryEntries = await readRegistry(); + for (const [groupPath, regName] of Object.entries(config.repos)) { + const e = registryEntries.find((en) => en.name === regName); + if (e) repoPaths.set(groupPath, e.path); } - } - } - // Auto-discover workspace dependency contracts (Rust Cargo workspaces, etc.) - // and merge them with explicit manifest links. Discovered links use the same - // ManifestExtractor pipeline as hand-written links in group.yaml. - let allLinks = [...config.links]; - - if (config.detect.workspace_deps) { - const repoPaths = new Map(); - if (!registryEntries) registryEntries = await readRegistry(); - for (const [groupPath, regName] of Object.entries(config.repos)) { - const e = registryEntries.find((en) => en.name === regName); - if (e) repoPaths.set(groupPath, e.path); - } - - const wsResult = await discoverWorkspaceLinks(config.repos, repoPaths, dbExecutors); - if (wsResult.links.length > 0) { - allLinks = [...allLinks, ...wsResult.links]; - if (opts?.verbose) { - for (const s of wsResult.stats) { - logger.info( - ` workspace-deps: discovered ${s.linkCount} cross-${s.ecosystem.toLowerCase()} links from ${s.projectCount} ${s.ecosystem} projects`, - ); + const wsResult = await discoverWorkspaceLinks(config.repos, repoPaths, dbExecutors); + if (wsResult.links.length > 0) { + allLinks = [...allLinks, ...wsResult.links]; + if (opts?.verbose) { + for (const s of wsResult.stats) { + logger.info( + ` workspace-deps: discovered ${s.linkCount} cross-${s.ecosystem.toLowerCase()} links from ${s.projectCount} ${s.ecosystem} projects`, + ); + } } } } - } - // Process manifest links declared in group.yaml (plus any auto-discovered). - // ManifestExtractor is fully implemented but was never wired into this - // pipeline — config.links were parsed and validated but silently dropped. - // Placed after the DB try/finally: resolveSymbol falls back to synthetic - // UIDs when dbExecutors is undefined or a pool is closed, so cross-links - // are always generated regardless of whether real DB executors are available. - if (allLinks.length > 0) { - const knownRepos = new Set(Object.keys(config.repos)); - for (const link of allLinks) { - const dangling = [link.from, link.to].filter((r) => !knownRepos.has(r)); - if (dangling.length > 0) { - logger.warn( - `[group/sync] manifest link ${link.type}:${link.contract} references repos not in config.repos: ${dangling.join(', ')} — cross-links will use synthetic UIDs`, + if (allLinks.length > 0) { + const knownRepos = new Set(Object.keys(config.repos)); + for (const link of allLinks) { + const dangling = [link.from, link.to].filter((r) => !knownRepos.has(r)); + if (dangling.length > 0) { + logger.warn( + `[group/sync] manifest link ${link.type}:${link.contract} references repos not in config.repos: ${dangling.join(', ')} — cross-links will use synthetic UIDs`, + ); + } + } + + const manifestEx = new ManifestExtractor(); + const manifestResult = await manifestEx.extractFromManifest(allLinks, dbExecutors); + autoContracts.push(...manifestResult.contracts); + manifestCrossLinks = manifestResult.crossLinks; + if (opts?.verbose) { + logger.info( + ` manifest: ${manifestCrossLinks.length} cross-links from ${allLinks.length} links (${config.links.length} declared + ${allLinks.length - config.links.length} discovered)`, ); } } - - const manifestEx = new ManifestExtractor(); - const manifestResult = await manifestEx.extractFromManifest(allLinks, dbExecutors); - autoContracts.push(...manifestResult.contracts); - manifestCrossLinks = manifestResult.crossLinks; - if (opts?.verbose) { - logger.info( - ` manifest: ${manifestCrossLinks.length} cross-links from ${allLinks.length} links (${config.links.length} declared + ${allLinks.length - config.links.length} discovered)`, - ); + } finally { + for (const id of [...new Set(openPoolIds)]) { + await closeLbug(id).catch(() => {}); } } diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 9f14db231..4fc320076 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -980,6 +980,129 @@ service OrderService { expect(nodeLink).toBeDefined(); }); }); + + it('manifest symbol resolution runs before closeLbug (issue #1802)', async () => { + const links: GroupManifestLink[] = [ + { + from: 'svc/orders', + to: 'svc/payments', + type: 'http', + contract: 'GET::/api/checkout', + role: 'consumer', + }, + ]; + + const config: GroupConfig = { + version: 1, + name: 'test', + description: '', + repos: { 'svc/orders': 'orders-repo', 'svc/payments': 'payments-repo' }, + links, + packages: {}, + detect: { + http: true, + grpc: false, + thrift: false, + topics: false, + shared_libs: false, + embedding_fallback: false, + workspace_deps: false, + }, + matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 }, + }; + + const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js'); + + let closeLbugCalled = false; + let manifestResolvedWhilePoolOpen = false; + + const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined); + const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockImplementation(async () => { + closeLbugCalled = true; + }); + const execSpy = vi + .spyOn(poolAdapter, 'executeParameterized') + .mockImplementation( + async (_poolId: string, query: string, _params: Record) => { + if (query.includes('HANDLES_ROUTE')) { + manifestResolvedWhilePoolOpen = !closeLbugCalled; + } + return [ + { uid: 'real-uid-checkout', name: 'CheckoutHandler', filePath: 'src/checkout.ts' }, + ]; + }, + ); + + try { + const result = await syncGroup(config, { + resolveRepoHandle: async (_name, groupPath) => ({ + id: groupPath.replace(/\//g, '-'), + path: groupPath, + repoPath: '/tmp/' + groupPath, + storagePath: '/tmp/' + groupPath + '/.gitnexus', + }), + skipWrite: true, + }); + + // Manifest symbol resolution must run while pools are still open + expect(manifestResolvedWhilePoolOpen).toBe(true); + expect(closeLbugCalled).toBe(true); + + // The manifest cross-link must use the real UID from the DB, not synthetic + const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest'); + expect(manifestLinks).toHaveLength(1); + expect(manifestLinks[0].to.symbolUid).toBe('real-uid-checkout'); + expect(manifestLinks[0].to.symbolUid).not.toContain('manifest::'); + + // closeLbug must fire exactly twice (one per repo) + expect(closeSpy).toHaveBeenCalledTimes(2); + } finally { + initSpy.mockRestore(); + closeSpy.mockRestore(); + execSpy.mockRestore(); + } + }); + + it('extractorOverride no-DB path still produces synthetic manifest UIDs', async () => { + const links: GroupManifestLink[] = [ + { + from: 'svc/orders', + to: 'svc/payments', + type: 'http', + contract: 'GET::/api/checkout', + role: 'consumer', + }, + ]; + + const config: GroupConfig = { + version: 1, + name: 'test', + description: '', + repos: { 'svc/orders': 'orders-repo', 'svc/payments': 'payments-repo' }, + links, + packages: {}, + detect: { + http: true, + grpc: false, + thrift: false, + topics: false, + shared_libs: false, + embedding_fallback: false, + workspace_deps: false, + }, + matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 }, + }; + + const result = await syncGroup(config, { + extractorOverride: async () => [], + skipWrite: true, + }); + + const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest'); + expect(manifestLinks).toHaveLength(1); + expect(manifestLinks[0].from.symbolUid).toBe('manifest::svc/orders::http::GET::/api/checkout'); + expect(manifestLinks[0].to.symbolUid).toBe('manifest::svc/payments::http::GET::/api/checkout'); + }); }); describe('stableRepoPoolId', () => { From 1c4993251c885813ce9a17a455a7530fb6b8eea9 Mon Sep 17 00:00:00 2001 From: Lucas van Staden Date: Mon, 25 May 2026 03:37:17 +0800 Subject: [PATCH 09/10] fix(php): synthesize module scope for namespace-less PHP files (.phtml) (#1801) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(php): phtml scope synthesis with full-file range + O(1) Step 4 lookup (#1801, #1803) Address PR #1801 review findings and complete #1803 fix: scope-extractor.ts: - Synthetic Module scope uses full-file range (computed from existing drafts) so positionIndex containment works for top-level references in ERROR-root .phtml files - Orphan scope re-parenting done on drafts in extract() by replacing with new drafts — no mutation of readonly fields, no PHP-specific logic in shared buildScopeTree - Dead matchCount parameter removed from ensureModuleScope namespace-siblings.ts: - Step 4 parsedFiles.find() replaced with pre-built Map for O(1) lookup (was O(n²) with 16K files = ~256M comparisons) * test(php): add pipeline benchmark for scaling regression detection Synthetic PHP fixture generator (N files × M namespaces × K classes) with cross-namespace imports and calls. Measures wall-clock, peak heap, node/edge counts at 100/250/500 file scales with worker pool enabled. Results on current branch: - 100 files: 982ms, 65MB (9.8ms/file) - 250 files: 1310ms, 70MB (5.2ms/file) - 500 files: 2006ms, 92MB (4.0ms/file) - Scaling: sublinear (0.53x-0.77x ratio) Gated behind GITNEXUS_BENCH=1 so it does not run in normal CI. * chore: trigger CI * fix: prettier formatting + update scope-extractor test for synthesis behavior * fix: extend synthetic Module range to all captures + update integration test Address CI failure and review findings: - ensureModuleScope now computes range from ALL captures (scope, declaration, reference, type-binding) not just scope drafts. This ensures top-level references after the last inner scope are covered. - Update parse-worker-scope-integration test for synthesis behavior. - Update extract() docstring to document synthesis contract. --------- Co-authored-by: Test Co-authored-by: Gergő Magyar --- .../languages/php/namespace-siblings.ts | 5 +- .../src/core/ingestion/scope-extractor.ts | 64 ++++-- .../php-pipeline-benchmark.test.ts | 199 ++++++++++++++++++ .../parse-worker-scope-integration.test.ts | 12 +- .../scope-resolution/scope-extractor.test.ts | 11 +- 5 files changed, 260 insertions(+), 31 deletions(-) create mode 100644 gitnexus/test/integration/php-pipeline-benchmark.test.ts diff --git a/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts b/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts index 875e6642b..93ad2b84e 100644 --- a/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts +++ b/gitnexus/src/core/ingestion/languages/php/namespace-siblings.ts @@ -345,6 +345,9 @@ export function populatePhpNamespaceSiblings( // // Additionally, mirror from files that are imported via `use` (different // namespace) so return types from dependencies are chain-followable too. + const parsedByPath = new Map(); + for (const p of parsedFiles) parsedByPath.set(p.filePath, p); + for (const parsed of parsedFiles) { const moduleScope = parsed.scopes.find((s) => s.kind === 'Module'); if (moduleScope === undefined) continue; @@ -386,7 +389,7 @@ export function populatePhpNamespaceSiblings( // Mirror return-type bindings from accessible files. for (const srcFilePath of accessibleFiles) { - const srcParsed = parsedFiles.find((p) => p.filePath === srcFilePath); + const srcParsed = parsedByPath.get(srcFilePath); if (srcParsed === undefined) continue; const srcModuleScope = srcParsed.scopes.find((s) => s.kind === 'Module'); if (srcModuleScope === undefined) continue; diff --git a/gitnexus/src/core/ingestion/scope-extractor.ts b/gitnexus/src/core/ingestion/scope-extractor.ts index a737214b3..7a57e704a 100644 --- a/gitnexus/src/core/ingestion/scope-extractor.ts +++ b/gitnexus/src/core/ingestion/scope-extractor.ts @@ -109,10 +109,12 @@ export type ScopeExtractorHooks = Pick< * Drive the five extraction passes and return a `ParsedFile`. * * Throws `ScopeTreeInvariantError` (from #912) when the provider emits - * captures that violate structural scope invariants. The error surfaces - * upward rather than being silently corrected — a malformed capture set - * is a bug in the provider's `emitScopeCaptures`, not a data condition - * to tolerate. + * captures that violate structural scope invariants (e.g., overlapping + * sibling scopes). When no `@scope.module` capture is present, a + * synthetic Module scope is created spanning all captures, and orphan + * non-Module scopes are re-parented under it. This enables indexing of + * files where tree-sitter produces an ERROR root (e.g., complex .phtml + * templates with mixed PHP/HTML/JS). */ export function extract( matches: readonly CaptureMatch[], @@ -124,7 +126,16 @@ export function extract( // ── Pass 1: build the scope tree ───────────────────────────────────── const scopeDrafts = pass1BuildScopes(partitioned.scope, filePath, provider); - const moduleScope = ensureModuleScope(scopeDrafts, matches.length, filePath); + const moduleScope = ensureModuleScope(scopeDrafts, filePath, matches); + // Re-parent orphan drafts (parent === null, non-Module) under the + // Module scope. Replaces drafts with new ones carrying the correct + // parent — runs before content passes so bindings/ownedDefs are empty. + for (let i = 0; i < scopeDrafts.length; i++) { + const d = scopeDrafts[i]; + if (d.parent === null && d.kind !== 'Module') { + scopeDrafts[i] = makeDraft(d.id, moduleScope.id, d.kind, d.range, d.filePath); + } + } const scopes = scopeDrafts.map(draftToScope); // buildScopeTree validates invariants (throws on violation) and exposes // the lookup contract consumed by Passes 2-5. @@ -280,29 +291,40 @@ interface ScopeDraft { function ensureModuleScope( scopeDrafts: ScopeDraft[], - matchCount: number, filePath: string, + allMatches: readonly CaptureMatch[], ): ScopeDraft { const moduleScope = scopeDrafts.find((s) => s.kind === 'Module'); if (moduleScope !== undefined) return moduleScope; - if (scopeDrafts.length === 0 && matchCount === 0) { - const range: Range = { startLine: 0, startCol: 0, endLine: 0, endCol: 0 }; - const synthetic = makeDraft( - makeScopeId({ filePath, range, kind: 'Module' }), - null, - 'Module', - range, - filePath, - ); - scopeDrafts.push(synthetic); - return synthetic; + // Synthesize a Module scope spanning all captures in the file. + // Computed from ALL captures (scope, declaration, reference, etc.) + // so the range covers top-level references that appear after the + // last inner scope — not just inner Function/Class scopes. + let endLine = 0; + let endCol = 0; + for (const match of allMatches) { + for (const capture of Object.values(match)) { + if ( + capture.range.endLine > endLine || + (capture.range.endLine === endLine && capture.range.endCol > endCol) + ) { + endLine = capture.range.endLine; + endCol = capture.range.endCol; + } + } } - - throw new Error( - `ScopeExtractor: no Module scope found for '${filePath}'. ` + - `Provider must emit at least one @scope.module capture per file.`, + const range: Range = { startLine: 0, startCol: 0, endLine, endCol }; + const synthetic = makeDraft( + makeScopeId({ filePath, range, kind: 'Module' }), + null, + 'Module', + range, + filePath, ); + + scopeDrafts.push(synthetic); + return synthetic; } function draftToScope(draft: ScopeDraft): Scope { diff --git a/gitnexus/test/integration/php-pipeline-benchmark.test.ts b/gitnexus/test/integration/php-pipeline-benchmark.test.ts new file mode 100644 index 000000000..9afd4c569 --- /dev/null +++ b/gitnexus/test/integration/php-pipeline-benchmark.test.ts @@ -0,0 +1,199 @@ +/** + * PHP ingestion pipeline benchmark. + * + * Generates synthetic PHP codebases at increasing scales and measures + * wall-clock time and peak heap through the full pipeline — parsing, + * scope extraction, namespace-siblings (Steps 1-4), and call resolution. + * + * Run: GITNEXUS_BENCH=1 npx vitest run test/integration/php-pipeline-benchmark.test.ts + * + * The benchmark uses workers (production path) by default. Set + * skipWorkers to test the sequential fallback path. + */ +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; + +const BENCH_ENABLED = process.env.GITNEXUS_BENCH === '1'; + +interface BenchResult { + fileCount: number; + classCount: number; + namespaceCount: number; + elapsedMs: number; + peakHeapMB: number; + nodeCount: number; + edgeCount: number; +} + +function generatePhpFixture( + fileCount: number, + namespacesPerLevel: number, +): { dir: string; classCount: number; namespaceCount: number } { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `php-bench-${fileCount}-`)); + const namespaces: string[] = []; + + for (let i = 0; i < namespacesPerLevel; i++) { + for (let j = 0; j < namespacesPerLevel; j++) { + namespaces.push(`App\\Module${i}\\Sub${j}`); + } + } + + const classCount = fileCount; + const namespaceCount = namespaces.length; + + for (let f = 0; f < fileCount; f++) { + const ns = namespaces[f % namespaces.length]; + const nsDir = ns.replace(/\\/g, '/'); + const className = `Class${f}`; + const targetDir = path.join(dir, nsDir); + fs.mkdirSync(targetDir, { recursive: true }); + + const siblingIdx = (f + 1) % fileCount; + const siblingClass = `Class${siblingIdx}`; + + const crossNsIdx = (f + Math.floor(fileCount / 3)) % fileCount; + const crossNs = namespaces[crossNsIdx % namespaces.length]; + const crossClass = `Class${crossNsIdx}`; + + const content = [ + 'id;', + ' }', + '', + ` public function process(): ${siblingClass}`, + ' {', + ` $sibling = new ${siblingClass}();`, + ' return $sibling;', + ' }', + '', + ns !== crossNs + ? [ + ` public function crossCall(): ${crossClass}`, + ' {', + ` $cross = new ${crossClass}();`, + ` $cross->getId();`, + ' return $cross;', + ' }', + ].join('\n') + : '', + '}', + '', + ] + .filter(Boolean) + .join('\n'); + + fs.writeFileSync(path.join(targetDir, `${className}.php`), content); + } + + const composerJson = { + name: 'bench/php-pipeline', + autoload: { 'psr-4': { 'App\\': 'App/' } }, + }; + fs.writeFileSync(path.join(dir, 'composer.json'), JSON.stringify(composerJson, null, 2)); + + return { dir, classCount, namespaceCount }; +} + +async function runBenchmark( + fileCount: number, + nsLevels: number, + budgetMs: number, +): Promise { + const { dir, classCount, namespaceCount } = generatePhpFixture(fileCount, nsLevels); + + let peakHeapMB = 0; + const heapSampler = setInterval(() => { + const heap = process.memoryUsage().heapUsed / 1024 / 1024; + if (heap > peakHeapMB) peakHeapMB = heap; + }, 50); + + try { + const start = Date.now(); + const result = await Promise.race([ + runPipelineFromRepo(dir, () => {}, { skipGraphPhases: true }), + new Promise((_, reject) => + setTimeout( + () => reject(new Error(`Pipeline exceeded ${budgetMs}ms at ${fileCount} files`)), + budgetMs, + ), + ), + ]); + const elapsedMs = Date.now() - start; + + return { + fileCount, + classCount, + namespaceCount, + elapsedMs, + peakHeapMB: Math.round(peakHeapMB), + nodeCount: result.graph.nodeCount, + edgeCount: result.graph.relationshipCount, + }; + } finally { + clearInterval(heapSampler); + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +function printResults(label: string, results: BenchResult[]) { + console.log(`\n${label}`); + console.log('┌──────────┬─────────┬──────────┬───────────┬──────────┬───────┬───────┐'); + console.log('│ Files │ Classes │ NS Count │ Time (ms) │ Heap MB │ Nodes │ Edges │'); + console.log('├──────────┼─────────┼──────────┼───────────┼──────────┼───────┼───────┤'); + for (const r of results) { + console.log( + `│ ${String(r.fileCount).padStart(8)} │ ${String(r.classCount).padStart(7)} │ ${String(r.namespaceCount).padStart(8)} │ ${String(r.elapsedMs).padStart(9)} │ ${String(r.peakHeapMB).padStart(8)} │ ${String(r.nodeCount).padStart(5)} │ ${String(r.edgeCount).padStart(5)} │`, + ); + } + console.log('└──────────┴─────────┴──────────┴───────────┴──────────┴───────┴───────┘'); + + if (results.length >= 2) { + console.log('\nScaling ratios (time_ratio / file_ratio):'); + for (let i = 1; i < results.length; i++) { + const fileRatio = results[i].fileCount / results[i - 1].fileCount; + const timeRatio = results[i].elapsedMs / results[i - 1].elapsedMs; + const scaling = timeRatio / fileRatio; + console.log( + ` ${results[i - 1].fileCount} → ${results[i].fileCount}: ${scaling.toFixed(2)}x (${scaling < 1.5 ? 'linear' : scaling < 3 ? 'superlinear' : 'WARNING: quadratic'})`, + ); + } + } +} + +describe.skipIf(!BENCH_ENABLED)('PHP pipeline benchmark', () => { + it('scales with file count (workers enabled)', async () => { + const scales = [100, 250, 500]; + const results: BenchResult[] = []; + + for (const fileCount of scales) { + const nsLevels = Math.max(2, Math.ceil(Math.sqrt(fileCount / 4))); + const result = await runBenchmark(fileCount, nsLevels, 180_000); + results.push(result); + console.log( + ` ${fileCount} files: ${result.elapsedMs}ms, ${result.peakHeapMB}MB heap, ${result.nodeCount} nodes, ${result.edgeCount} edges`, + ); + } + + printResults('PHP Pipeline — Workers Enabled', results); + + for (let i = 1; i < results.length; i++) { + const fileRatio = results[i].fileCount / results[i - 1].fileCount; + const timeRatio = results[i].elapsedMs / results[i - 1].elapsedMs; + expect(timeRatio / fileRatio).toBeLessThan(3); + } + }, 300_000); +}); diff --git a/gitnexus/test/unit/scope-resolution/parse-worker-scope-integration.test.ts b/gitnexus/test/unit/scope-resolution/parse-worker-scope-integration.test.ts index b34ef33be..36e4f0ead 100644 --- a/gitnexus/test/unit/scope-resolution/parse-worker-scope-integration.test.ts +++ b/gitnexus/test/unit/scope-resolution/parse-worker-scope-integration.test.ts @@ -146,15 +146,17 @@ describe('extractParsedFile', () => { expect(warnings[0]).toContain('provider boom'); }); - it('returns undefined when ScopeExtractor throws (missing Module scope)', () => { - // Emits a Class scope but no Module — extractor throws; helper - // swallows and returns undefined. Legacy parsing on the same file - // continues unaffected by this failure. + it('synthesizes Module scope and re-parents orphan Class when no Module is emitted', () => { const provider = fakeProvider({ emitScopeCaptures: () => [{ '@scope.class': cap('@scope.class', 5, 0, 10, 0) }], }); const result = extractParsedFile(provider, 'src', 'a.ts'); - expect(result).toBeUndefined(); + expect(result).toBeDefined(); + const moduleScope = result!.scopes.find((s) => s.kind === 'Module'); + expect(moduleScope).toBeDefined(); + const classScope = result!.scopes.find((s) => s.kind === 'Class'); + expect(classScope).toBeDefined(); + expect(classScope!.parent).toBe(moduleScope!.id); }); it('returns undefined when ScopeExtractor throws on malformed captures (overlap)', () => { diff --git a/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts b/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts index a13d57e2f..5143bce41 100644 --- a/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts +++ b/gitnexus/test/unit/scope-resolution/scope-extractor.test.ts @@ -195,10 +195,13 @@ describe('Pass 1: scope tree', () => { ).toThrow(/overlap/i); }); - it('throws when no Module scope is present', () => { - expect(() => extract([scopeMatch('function', 1, 0, 10, 0)], 'a.ts', mockProvider())).toThrow( - /Module/, - ); + it('synthesizes a Module scope and re-parents orphan Function when no Module is present', () => { + const result = extract([scopeMatch('function', 1, 0, 10, 0)], 'a.ts', mockProvider()); + const moduleScope = result.scopes.find((s) => s.kind === 'Module'); + expect(moduleScope).toBeDefined(); + const fnScope = result.scopes.find((s) => s.kind === 'Function'); + expect(fnScope).toBeDefined(); + expect(fnScope!.parent).toBe(moduleScope!.id); }); }); From 73a6a5376e8d007c96e6176cb0607dc88e1bc381 Mon Sep 17 00:00:00 2001 From: Sparsh <73558748+prajapatisparsh@users.noreply.github.com> Date: Mon, 25 May 2026 11:41:47 +0530 Subject: [PATCH 10/10] fix(cpp): thread call-site types into qualified member lookup (#1632) (#1810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cpp): thread call-site types into qualified member lookup (#1632) Widen Callsite (arity optional, add argumentTypes) and add optional callsite?: Callsite to ScopeResolver.resolveQualifiedReceiverMember. receiver-bound-calls.ts passes the ReferenceSite through structurally; resolveCppQualifiedNamespaceMember forwards it to narrowOverloadCandidates along with cppConversionRank, enabling exact-type and conversion-rank disambiguation across inline-namespace children. Behavior change: - outer::foo(42) where v1 declares foo(int) and v2 declares foo(double) now resolves to v1::foo (was: 0 edges, conservatively suppressed). - Same-name same-normalized-signature (e.g. foo(int) vs foo(long)) still suppresses at 0 edges via isOverloadAmbiguousAfterNormalization. - ADL using-import path (resolveAdlCandidates) unchanged — passes no callsite, narrowing degrades to existing pass-through behavior. Closes #1632. Part of #1564. * fix(cpp): update legacy parity expected-failure list for #1632 - Remove stale expected-failure entry for old diff-sigs test name (test now expects 1 edge; legacy DAG also emits 1 edge) - Add entry for normalized-signature ambiguity (int vs long) test - Rename describe block from 'conservative suppress' to 'distinct signatures resolved via call-site types' Verified both modes: REGISTRY_PRIMARY_CPP=1: 241/241 passed REGISTRY_PRIMARY_CPP=0: 194 passed, 47 skipped, 0 failed --- gitnexus-shared/src/scope-resolution/types.ts | 7 ++-- .../languages/cpp/inline-namespaces.ts | 28 +++++++-------- .../ingestion/languages/cpp/scope-resolver.ts | 10 ++++-- .../contract/scope-resolver.ts | 1 + .../passes/receiver-bound-calls.ts | 1 + .../caller.cpp | 5 +++ .../lib.h | 10 ++++++ .../test/integration/resolvers/cpp.test.ts | 35 +++++++++++++++---- .../test/integration/resolvers/helpers.ts | 13 ++++--- 9 files changed, 80 insertions(+), 30 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h diff --git a/gitnexus-shared/src/scope-resolution/types.ts b/gitnexus-shared/src/scope-resolution/types.ts index c3d8b80fb..828874a7f 100644 --- a/gitnexus-shared/src/scope-resolution/types.ts +++ b/gitnexus-shared/src/scope-resolution/types.ts @@ -267,8 +267,11 @@ export interface ScopeLookup { /** Call-site description passed to `arityCompatibility`. */ export interface Callsite { - /** Number of arguments at the call site. */ - readonly arity: number; + /** Number of arguments at the call site, if available. */ + readonly arity?: number; + /** Inferred argument types at the call site, one per argument. + * An empty string entry means the type was not inferred. */ + readonly argumentTypes?: readonly string[]; } // ─── §2.4 ImportEdge ──────────────────────────────────────────────────────── diff --git a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts index 604c50c0b..eec44c68f 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts @@ -27,12 +27,13 @@ * declaration transparently. */ -import type { ParsedFile, ScopeId, SymbolDefinition } from 'gitnexus-shared'; +import type { Callsite, ParsedFile, ScopeId, SymbolDefinition } from 'gitnexus-shared'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import { isOverloadAmbiguousAfterNormalization, narrowOverloadCandidates, } from '../../scope-resolution/passes/overload-narrowing.js'; +import { cppConversionRank } from './conversion-rank.js'; interface RangeKey { readonly startLine: number; @@ -107,6 +108,7 @@ export function resolveCppQualifiedNamespaceMember( memberName: string, parsedFiles: readonly ParsedFile[], _scopes: ScopeResolutionIndexes, + callsite?: Callsite, ): SymbolDefinition | 'ambiguous' | undefined { const allHits: SymbolDefinition[] = []; const seenNodeId = new Set(); @@ -132,19 +134,17 @@ export function resolveCppQualifiedNamespaceMember( if (allHits.length === 0) return undefined; if (allHits.length === 1) return allHits[0]; - // Multi-candidate: the `resolveQualifiedReceiverMember` hook has no - // access to call-site arity or argument types, so - // `narrowOverloadCandidates` cannot actually narrow here — the call - // with `(allHits, undefined, undefined)` is effectively a pass-through. - // We retain it so that `isOverloadAmbiguousAfterNormalization` can - // still detect int/long-style normalization collisions on this path, - // but for any multi-hit case where candidates have genuinely distinct - // signatures (e.g. `foo(int)` vs `foo(double)` in different inline - // children), we conservatively suppress rather than pick arbitrarily. - // A future enhancement could thread call-site argument info through - // the `resolveQualifiedReceiverMember` contract to enable real - // narrowing here. - const narrowed = narrowOverloadCandidates(allHits, undefined, undefined); + // Multi-candidate: thread call-site arity/argument-types through the + // `resolveQualifiedReceiverMember` contract so `narrowOverloadCandidates` + // can disambiguate via exact-type match and, when available, conversion-rank + // scoring (`cppConversionRank`). Same-signature ambiguity is still detected + // by `isOverloadAmbiguousAfterNormalization` below. + const narrowed = narrowOverloadCandidates( + allHits, + callsite?.arity, + callsite?.argumentTypes, + callsite !== undefined ? { conversionRankFn: cppConversionRank } : undefined, + ); if (narrowed.length === 1) return narrowed[0]; if (narrowed.length === 0) return undefined; if (isOverloadAmbiguousAfterNormalization(narrowed, undefined)) return 'ambiguous'; diff --git a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts index 4a1f343d1..9a5ac85a5 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts @@ -275,6 +275,12 @@ export const cppScopeResolver: ScopeResolver = { // descends transitively through inline-namespace children when // searching for the called member. Returns undefined for non-namespace // receivers so receiver-bound-calls Case 2 still gets a chance. - resolveQualifiedReceiverMember: (receiverName, memberName, _callerScope, scopes, parsedFiles) => - resolveCppQualifiedNamespaceMember(receiverName, memberName, parsedFiles, scopes), + resolveQualifiedReceiverMember: ( + receiverName, + memberName, + _callerScope, + scopes, + parsedFiles, + callsite, + ) => resolveCppQualifiedNamespaceMember(receiverName, memberName, parsedFiles, scopes, callsite), }; diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index cdfbe3d8e..2d0733684 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -705,6 +705,7 @@ export interface ScopeResolver { callerScope: ScopeId, scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], + callsite?: Callsite, ) => SymbolDefinition | 'ambiguous' | undefined; /** diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 59b6c323d..ef1d4a0eb 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -501,6 +501,7 @@ export function emitReceiverBoundCalls( site.inScope, scopes, parsedFiles, + site, ); if (memberDef === 'ambiguous') { // Same-name ambiguity across inline-namespace children (#1564): diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp new file mode 100644 index 000000000..fcb416b39 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp @@ -0,0 +1,5 @@ +#include "lib.h" + +void run() { + outer::foo(42); +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h new file mode 100644 index 000000000..703fb15e5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h @@ -0,0 +1,10 @@ +#pragma once + +namespace outer { + inline namespace v1 { + void foo(int x); + } + inline namespace v2 { + void foo(long y); + } +} diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index e33fa6632..0d31b3f18 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -3044,7 +3044,7 @@ describe('C++ inline namespace — ambiguous same-name across inline children (# }); }); -describe('C++ inline namespace — ambiguous distinct signatures (conservative suppress)', () => { +describe('C++ inline namespace — distinct signatures resolved via call-site types', () => { let result: PipelineResult; beforeAll(async () => { @@ -3054,14 +3054,35 @@ describe('C++ inline namespace — ambiguous distinct signatures (conservative s ); }, 60000); - it('outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(double)', () => { + it('outer::foo(42) emits exactly 1 CALLS edge to v1::foo(int) when v1 declares foo(int) and v2 declares foo(double)', () => { const calls = getRelationships(result, 'CALLS'); const fooCalls = calls.filter((c) => c.source === 'run' && c.target === 'foo'); - // Even though the two overloads have distinct signatures and a compiler - // could disambiguate via argument types, the `resolveQualifiedReceiverMember` - // hook lacks call-site arity/argument-type information, so multi-hit cases - // are conservatively suppressed. Documents the limitation noted in - // inline-namespaces.ts (Finding 1 of Claude review on #1600). + // Call-site arity and argument types are now threaded through the + // resolveQualifiedReceiverMember contract (#1632). narrowOverloadCandidates + // matches the exact type 'int' against v1::foo(int), producing exactly 1 edge. + expect(fooCalls).toHaveLength(1); + // Verify it resolved to v1::foo(int) at line 4 (0-indexed), not v2::foo(double) at line 7 + const targetNode = result.graph.getNode(fooCalls[0].rel.targetId); + expect(targetNode?.properties.startLine).toBe(4); + }); +}); + +describe('C++ inline namespace — ambiguous normalized signatures', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-inline-namespace-ambiguous-normalized'), + () => {}, + ); + }, 60000); + + it('outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int', () => { + const calls = getRelationships(result, 'CALLS'); + const fooCalls = calls.filter((c) => c.source === 'run' && c.target === 'foo'); + // int and long both normalize to 'int' via normalizeCppParamType, making + // the two candidates indistinguishable after normalization. The resolver + // must suppress rather than pick arbitrarily (isOverloadAmbiguousAfterNormalization). expect(fooCalls.length).toBe(0); }); }); diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index f599835b5..486449aa9 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -332,11 +332,14 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly