diff --git a/gitnexus-web/src/hooks/useAutoScroll.ts b/gitnexus-web/src/hooks/useAutoScroll.ts index 58a0aedcc..55c2946f7 100644 --- a/gitnexus-web/src/hooks/useAutoScroll.ts +++ b/gitnexus-web/src/hooks/useAutoScroll.ts @@ -142,4 +142,4 @@ export function useAutoScroll( isAtBottom, scrollToBottom, }; -} \ No newline at end of file +} diff --git a/gitnexus-web/test/unit/use-auto-scroll.test.tsx b/gitnexus-web/test/unit/use-auto-scroll.test.tsx index a8bc1a795..e58227d67 100644 --- a/gitnexus-web/test/unit/use-auto-scroll.test.tsx +++ b/gitnexus-web/test/unit/use-auto-scroll.test.tsx @@ -267,9 +267,7 @@ describe('useAutoScroll', () => { }); it('attaches the observer when the messages wrapper first appears and disconnects on unmount', () => { - const { rerender, unmount } = render( - , - ); + const { rerender, unmount } = render(); expect(screen.queryByTestId('messages-container')).toBeNull(); expect(resizeObserverInstances).toHaveLength(0); diff --git a/gitnexus/README.md b/gitnexus/README.md index 7e87c93b4..8c7888d66 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -234,6 +234,56 @@ Installed automatically by both `gitnexus analyze` (per-repo) and `gitnexus setu - Node.js >= 18 - Git repository (uses git for commit tracking) +## Troubleshooting + +### `Cannot destructure property 'package' of 'node.target' as it is null` + +This crash was caused by a dependency URL format that is incompatible with +certain npm/arborist versions ([npm/cli#8126](https://github.com/npm/cli/issues/8126)). +It is fixed in **gitnexus v1.6.2+**. Upgrade to the latest version: + +```bash +npx gitnexus@latest analyze # always uses the newest release +# — or — +npm install -g gitnexus@latest # upgrade a global install +``` + +If you still hit npm install issues after upgrading, these generic workarounds +may help: + +```bash +npm install -g npm@latest # update npm itself +npm cache clean --force # clear a possibly corrupt cache +``` + +### Installation fails with native module errors + +Some optional language grammars (Dart, Kotlin, Swift) require native compilation. If they fail, GitNexus still works — those languages will be skipped. + +If `npm install -g gitnexus` fails on native modules: + +```bash +# Ensure build tools are available (Linux/macOS) +# Ubuntu/Debian: sudo apt install python3 make g++ +# macOS: xcode-select --install + +# Retry installation +npm install -g gitnexus +``` + +### Analysis runs out of memory + +For very large repositories: + +```bash +# Increase Node.js heap size +NODE_OPTIONS="--max-old-space-size=16384" npx gitnexus analyze + +# Exclude large directories +echo "vendor/" >> .gitnexusignore +echo "dist/" >> .gitnexusignore +``` + ## Privacy - All processing happens locally on your machine diff --git a/gitnexus/package-lock.json b/gitnexus/package-lock.json index c9ca9017f..bc4fc7b20 100644 --- a/gitnexus/package-lock.json +++ b/gitnexus/package-lock.json @@ -62,7 +62,7 @@ "node": ">=20.0.0" }, "optionalDependencies": { - "tree-sitter-dart": "https://github.com/UserNobody14/tree-sitter-dart/archive/80e23c07b64494f7e21090bb3450223ef0b192f4.tar.gz", + "tree-sitter-dart": "git+https://github.com/UserNobody14/tree-sitter-dart.git#80e23c07b64494f7e21090bb3450223ef0b192f4", "tree-sitter-kotlin": "^0.3.8", "tree-sitter-proto": "file:./vendor/tree-sitter-proto", "tree-sitter-swift": "^0.6.0" @@ -5132,8 +5132,8 @@ }, "node_modules/tree-sitter-dart": { "version": "1.0.0", - "resolved": "https://github.com/UserNobody14/tree-sitter-dart/archive/80e23c07b64494f7e21090bb3450223ef0b192f4.tar.gz", - "integrity": "sha512-aqLZTEji2vAZPdbaCSjR0SXJGzFRKD//7VtrSV3st9bgrCM2tsXxXAHZlMlQLOCt7K2yKxM5K3gNXYph8TCjCQ==", + "resolved": "git+https://github.com/UserNobody14/tree-sitter-dart.git#80e23c07b64494f7e21090bb3450223ef0b192f4", + "integrity": "sha512-Bs/1wAOIJ2akPEXlE/XVpuES19Oo3NqoSJRJ/0N2r38qAd9nTXdqmaGHQ44/JXnA6QHcbgD2YzCCc4wUc98cyQ==", "hasInstallScript": true, "license": "ISC", "optional": true, diff --git a/gitnexus/package.json b/gitnexus/package.json index d0d0704f5..571a341c5 100644 --- a/gitnexus/package.json +++ b/gitnexus/package.json @@ -84,7 +84,7 @@ "uuid": "^13.0.0" }, "optionalDependencies": { - "tree-sitter-dart": "https://github.com/UserNobody14/tree-sitter-dart/archive/80e23c07b64494f7e21090bb3450223ef0b192f4.tar.gz", + "tree-sitter-dart": "git+https://github.com/UserNobody14/tree-sitter-dart.git#80e23c07b64494f7e21090bb3450223ef0b192f4", "tree-sitter-kotlin": "^0.3.8", "tree-sitter-proto": "file:./vendor/tree-sitter-proto", "tree-sitter-swift": "^0.6.0" diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index dc8fd87a9..26d1ae8c6 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -297,7 +297,7 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption const msg = err.message || String(err); console.error(`\n Analysis failed: ${msg}\n`); - // Provide helpful guidance for known large-repo failure modes + // Provide helpful guidance for known failure modes if ( msg.includes('Maximum call stack size exceeded') || msg.includes('call stack') || @@ -314,6 +314,28 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption console.error(' 2. Increase Node.js heap: NODE_OPTIONS="--max-old-space-size=16384"'); console.error(' 3. Increase stack size: NODE_OPTIONS="--stack-size=4096"'); console.error(''); + } else if (msg.includes('ERESOLVE') || msg.includes('Could not resolve dependency')) { + // Note: the original arborist "Cannot destructure property 'package' of + // 'node.target'" crash happens inside npm *before* gitnexus code runs, + // so it can't be caught here. This branch handles dependency-resolution + // errors that surface at runtime (e.g. dynamic require failures). + console.error(' This looks like an npm dependency resolution issue.'); + console.error(' Suggestions:'); + console.error(' 1. Clear the npm cache: npm cache clean --force'); + console.error(' 2. Update npm: npm install -g npm@latest'); + console.error(' 3. Reinstall gitnexus: npm install -g gitnexus@latest'); + console.error(' 4. Or try npx directly: npx gitnexus@latest analyze'); + console.error(''); + } else if ( + msg.includes('MODULE_NOT_FOUND') || + msg.includes('Cannot find module') || + msg.includes('ERR_MODULE_NOT_FOUND') + ) { + console.error(' A required module could not be loaded. The installation may be corrupt.'); + console.error(' Suggestions:'); + console.error(' 1. Reinstall: npm install -g gitnexus@latest'); + console.error(' 2. Clear cache: npm cache clean --force && npx gitnexus@latest analyze'); + console.error(''); } process.exitCode = 1; diff --git a/gitnexus/src/core/group/extractors/grpc-extractor.ts b/gitnexus/src/core/group/extractors/grpc-extractor.ts index c6af9138a..b379a4dbd 100644 --- a/gitnexus/src/core/group/extractors/grpc-extractor.ts +++ b/gitnexus/src/core/group/extractors/grpc-extractor.ts @@ -314,7 +314,7 @@ export async function buildProtoMap(repoPath: string): Promise { const protoDir = normalizeProtoPath(path.dirname(c.protoPath)); - const sharedRun = longestSharedSegmentRun(sourceDir, protoDir); - if (sharedRun > bestScore) { - bestScore = sharedRun; - best = c; - } + return { candidate: c, score: longestSharedSegmentRun(sourceDir, protoDir) }; + }); + + let maxScore = -1; + for (const s of scored) { + if (s.score > maxScore) maxScore = s.score; } - return best; + const winners = scored.filter((s) => s.score === maxScore); + + // Path heuristic cannot uniquely identify a winner — refuse to guess. + // Ties (including all-zero ties) would otherwise silently merge unrelated + // services under a fabricated package-qualified contract id. + if (winners.length !== 1) { + const paths = candidates.map((c) => c.protoPath).join(', '); + console.warn( + `[grpc-extractor] Ambiguous proto resolution for service "${serviceName}" from ${sourceFilePath}: ${winners.length} candidates tied at score ${maxScore} among [${paths}] — skipping canonical contract`, + ); + return null; + } + + return winners[0].candidate; } export function serviceContractId(pkg: string, serviceName: string): string { @@ -410,7 +422,8 @@ export class GrpcExtractor implements ContractExtractor { continue; } for (const d of detections) { - out.push(this.detectionToContract(d, rel, protoMap)); + const contract = this.detectionToContract(d, rel, protoMap); + if (contract) out.push(contract); } } @@ -428,9 +441,13 @@ export class GrpcExtractor implements ContractExtractor { d: GrpcDetection, filePath: string, protoMap: Map, - ): ExtractedContract { - const candidates = protoMap.get(d.serviceName); - const proto = resolveProtoConflict(d.serviceName, filePath, candidates ?? []); + ): ExtractedContract | null { + const candidates = protoMap.get(d.serviceName) ?? []; + const proto = resolveProtoConflict(d.serviceName, filePath, candidates); + // If there were proto candidates but resolution was ambiguous, skip + // contract emission rather than fabricating a package-qualified id from + // an arbitrary candidate. resolveProtoConflict already warned. + if (candidates.length > 0 && proto === null) return null; const pkg = proto?.package ?? ''; const cid = d.methodName ? contractId(pkg, d.serviceName, d.methodName) diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index 0b07090e1..f2914613d 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -245,7 +245,30 @@ export class HttpRouteExtractor implements ContractExtractor { const providerDetections = detections.filter((d) => d.role === 'provider'); let handlerName: string | null = null; const normalizedRoute = normalizeHttpPath(routePath); - const match = providerDetections.find((d) => normalizeHttpPath(d.path) === normalizedRoute); + // Candidates share the same normalized path. When multiple + // detections at the same path exist (e.g. GET + POST /api/orders + // in one router), a blind `.find()` silently returned the first + // verb — attaching the wrong handler and, when method was not + // already pinned by the route reason, the wrong method too. + // Disambiguate by method when we know it; refuse to guess when + // we don't. + const candidates = providerDetections.filter( + (d) => normalizeHttpPath(d.path) === normalizedRoute, + ); + let match: (typeof candidates)[number] | undefined; + const ambiguousCandidates = !method && candidates.length > 1; + if (method) { + match = candidates.find((d) => d.method === method); + } else if (candidates.length === 1) { + match = candidates[0]; + } + // else: multiple candidates + unknown method → leave match + // undefined so handlerName stays null and skip symbol + // enrichment below, keeping the file-basename fallback instead + // of letting pickSymbolUid silently pick the first Function / + // Method in the file (which reintroduces the mis-attribution + // we were trying to avoid). Method stays at the conservative + // 'GET' default set below. if (match) { if (!method) method = match.method; handlerName = match.name; @@ -259,7 +282,7 @@ export class HttpRouteExtractor implements ContractExtractor { let symbolName = path.basename(filePath) || 'handler'; let symPath = filePath; const fileId = row.fileId ?? row[0]; - if (fileId) { + if (fileId && !ambiguousCandidates) { try { const syms = await db(CONTAINS_QUERY, { fileId }); if (syms.length > 0) { @@ -347,10 +370,19 @@ export class HttpRouteExtractor implements ContractExtractor { // Prefer the plugin's detected method if we can find a matching // fetch/axios call in the same file. const detections = filePath ? getDetections(filePath) : []; - const inferred = detections.find( + // Symmetric to the provider path: if multiple consumer calls in + // the same file share the same normalized path (e.g. a GET + // fetch AND a POST fetch to `/api/orders`), `.find()` silently + // picked the first verb and keyed the contract id on the wrong + // method. With no upstream method signal here, refuse to guess + // when candidates are ambiguous — leave `method` at its + // conservative 'GET' default. + const consumerCandidates = detections.filter( (d) => d.role === 'consumer' && normalizeConsumerPath(d.path) === pathNorm, ); - if (inferred) method = inferred.method; + if (consumerCandidates.length === 1) { + method = consumerCandidates[0].method; + } const cid = contractIdFor(method, pathNorm); let symbolUid = ''; diff --git a/gitnexus/src/core/group/extractors/manifest-extractor.ts b/gitnexus/src/core/group/extractors/manifest-extractor.ts index 29c8f9b21..4c0d737b7 100644 --- a/gitnexus/src/core/group/extractors/manifest-extractor.ts +++ b/gitnexus/src/core/group/extractors/manifest-extractor.ts @@ -23,6 +23,34 @@ function normalizeRoutePath(raw: string): string { return collapsed.replace(/\/+$/, ''); } +/** + * Split a manifest HTTP contract into its optional `METHOD::` prefix and + * its path portion. + * + * `buildContractId` recommends the explicit-method form `GET::/api/orders` + * in group.yaml; if we hand that raw string to `normalizeRoutePath` we get + * `/GET::/api/orders`, which can never match `Route.name = "/api/orders"` + * in the graph. This helper extracts the path portion so the Cypher + * lookup uses the canonical route name. + * + * The method prefix regex mirrors `buildContractId` (line ~251) for + * symmetry: case-insensitive `[A-Za-z]+` followed by `::`. The captured + * method is upper-cased for downstream use; method-constrained matching + * against `HANDLES_ROUTE` is a future enhancement (not yet wired). + * + * Edge cases: + * - `"::/api/orders"` — empty method portion, no alpha prefix match, so + * the whole string is treated as a bare path (matches buildContractId + * which also requires `[A-Za-z]+`). + * - `"GET::"` — method with empty path, returns `{ method: 'GET', path: '' }`; + * `normalizeRoutePath('')` resolves to `/` for caller. + */ +function parseHttpContract(raw: string): { method: string | null; path: string } { + const match = raw.match(/^([A-Za-z]+)::/); + if (!match) return { method: null, path: raw }; + return { method: match[1].toUpperCase(), path: raw.slice(match[0].length) }; +} + /** * Stable synthetic symbolUid for a manifest-declared contract whose target * symbol could not be resolved against the per-repo graph (resolveSymbol @@ -134,7 +162,15 @@ export class ManifestExtractor { // core/ingestion/pipeline.ts ensureSlash + generateId('Route', ...)). // Normalize the manifest contract the same way so a user-written // "/api/orders" matches "api/orders" in the graph. - const normalized = normalizeRoutePath(link.contract); + // + // The contract may also use the explicit-method form "GET::/api/orders" + // recommended by buildContractId. Strip the METHOD:: prefix before + // normalizing — otherwise `normalizeRoutePath('GET::/api/orders')` + // returns `/GET::/api/orders` and never matches Route.name. The + // captured method is not yet used to constrain the Cypher query + // (method-aware HANDLES_ROUTE matching is a future enhancement). + const parsed = parseHttpContract(link.contract); + const normalized = normalizeRoutePath(parsed.path); rows = await executor( `MATCH (handler)-[r:CodeRelation {type: 'HANDLES_ROUTE'}]->(route:Route) WHERE route.name = $normalized @@ -248,8 +284,15 @@ export class ManifestExtractor { private buildContractId(type: ContractType, contract: string): string { switch (type) { case 'http': { - if (/^[A-Za-z]+::/.test(contract)) return `http::${contract}`; - return `http::*::${contract}`; + // Canonicalize method casing and path separators so logically + // equivalent inputs (`get::/api/orders` vs `GET::/api/orders`, + // or trailing-slash variants) produce the same contractId and + // matching `manifestSymbolUid` fallback. Without this, raw + // user casing leaks into cross-impact join keys and fragments + // matches across repos. + const { method, path: rawPath } = parseHttpContract(contract); + const normalizedPath = normalizeRoutePath(rawPath); + return method ? `http::${method}::${normalizedPath}` : `http::*::${normalizedPath}`; } case 'grpc': return `grpc::${contract}`; diff --git a/gitnexus/src/core/ingestion/language-provider.ts b/gitnexus/src/core/ingestion/language-provider.ts index fae696be6..736c3b666 100644 --- a/gitnexus/src/core/ingestion/language-provider.ts +++ b/gitnexus/src/core/ingestion/language-provider.ts @@ -30,8 +30,30 @@ export type CaptureMap = Record; // so `core/ingestion/model/resolve.ts` can consume it without importing from // this file (which would pull in the full language-registry dependency graph). -/** How a language handles imports — determines wildcard synthesis behavior. */ -export type ImportSemantics = 'named' | 'wildcard' | 'namespace'; +/** + * How a language handles imports — determines wildcard synthesis behavior. + * + * Import resolution is a graph-traversal policy with multiple distinct strategies, + * analogous to MRO for method resolution. Each tag picks a strategy: + * + * | Tag | Mechanism | Traversal | Languages | + * |-----------------------|------------------------------------------------|---------------------|--------------------------------------------| + * | `named` | Per-symbol imports | None (use-site) | JS/TS, Java, C#, Rust, PHP, Kotlin, Vue | + * | `wildcard-transitive` | Textual paste, symbols chain through files | BFS closure | C, C++ (future: Obj-C, Fortran, Nim) | + * | `wildcard-leaf` | Whole public API, single hop | None (direct only) | Go, Ruby, Swift, Dart | + * | `namespace` | Qualified handle; symbols resolved at call site| None at import | Python | + * | `explicit-reexport` | Opt-in per-symbol re-export (SCAFFOLD) | Topological DAG | (future: TS `export *`, Rust `pub use`) | + * + * The `explicit-reexport` tag is a compile-time scaffold; no provider claims it yet. + * It falls through to `wildcard-leaf` behavior in synthesis so today's TS/Rust + * handling is unchanged. A future PR will implement the DAG walk for `export *`. + */ +export type ImportSemantics = + | 'named' + | 'wildcard-transitive' + | 'wildcard-leaf' + | 'namespace' + | 'explicit-reexport'; /** * Everything a language needs to provide. @@ -68,10 +90,12 @@ interface LanguageProviderConfig { /** Named binding extraction from import statements. * Default: undefined (language uses wildcard/whole-module imports). */ readonly namedBindingExtractor?: NamedBindingExtractorFn; - /** How this language handles imports. + /** How this language handles imports. See `ImportSemantics` for the full taxonomy. * - 'named': per-symbol imports (JS/TS, Java, C#, Rust, PHP, Kotlin) - * - 'wildcard': whole-module imports, needs synthesis (Go, Ruby, C/C++, Swift) - * - 'namespace': namespace imports, needs moduleAliasMap (Python) + * - 'wildcard-transitive': textual-include closure; imports chain through files (C, C++) + * - 'wildcard-leaf': whole-module single-hop imports; no transitive chaining (Go, Ruby, Swift, Dart) + * - 'namespace': qualified namespace imports, needs moduleAliasMap (Python) + * - 'explicit-reexport': opt-in per-symbol re-export (scaffold; no provider uses yet) * Default: 'named'. */ readonly importSemantics?: ImportSemantics; /** Language-specific transformation of raw import path text before resolution. diff --git a/gitnexus/src/core/ingestion/languages/c-cpp.ts b/gitnexus/src/core/ingestion/languages/c-cpp.ts index a0508ff5b..5b887634f 100644 --- a/gitnexus/src/core/ingestion/languages/c-cpp.ts +++ b/gitnexus/src/core/ingestion/languages/c-cpp.ts @@ -321,7 +321,7 @@ export const cProvider = defineLanguage({ typeConfig: cCppConfig, exportChecker: cCppExportChecker, importResolver: resolveCImport, - importSemantics: 'wildcard', + importSemantics: 'wildcard-transitive', fieldExtractor: createFieldExtractor(cFieldConfig), methodExtractor: createMethodExtractor({ ...cMethodConfig, @@ -339,7 +339,7 @@ export const cppProvider = defineLanguage({ typeConfig: cCppConfig, exportChecker: cCppExportChecker, importResolver: resolveCppImport, - importSemantics: 'wildcard', + importSemantics: 'wildcard-transitive', mroStrategy: 'leftmost-base', fieldExtractor: createFieldExtractor(cppFieldConfig), methodExtractor: createMethodExtractor({ diff --git a/gitnexus/src/core/ingestion/languages/dart.ts b/gitnexus/src/core/ingestion/languages/dart.ts index 591519895..7dc6769c6 100644 --- a/gitnexus/src/core/ingestion/languages/dart.ts +++ b/gitnexus/src/core/ingestion/languages/dart.ts @@ -2,7 +2,7 @@ * Dart Language Provider * * Dart traits: - * - importSemantics: 'wildcard' (Dart imports bring everything public into scope) + * - importSemantics: 'wildcard-leaf' (Dart imports bring everything public into scope) * - exportChecker: public if no leading underscore * - Dart SDK imports (dart:*) and external packages are skipped * - enclosingFunctionFinder: Dart's tree-sitter grammar places function_body @@ -90,7 +90,7 @@ export const dartProvider = defineLanguage({ typeConfig: dartConfig, exportChecker: dartExportChecker, importResolver: resolveDartImport, - importSemantics: 'wildcard', + importSemantics: 'wildcard-leaf', fieldExtractor: createFieldExtractor(dartFieldConfig), methodExtractor: createMethodExtractor(dartMethodConfig), classExtractor: createClassExtractor({ diff --git a/gitnexus/src/core/ingestion/languages/go.ts b/gitnexus/src/core/ingestion/languages/go.ts index 803e70bbb..2a2b35f50 100644 --- a/gitnexus/src/core/ingestion/languages/go.ts +++ b/gitnexus/src/core/ingestion/languages/go.ts @@ -5,7 +5,7 @@ * LanguageProvider, following the Strategy pattern used by the pipeline. * * Key Go traits: - * - importSemantics: 'wildcard' (Go imports entire packages) + * - importSemantics: 'wildcard-leaf' (Go imports entire packages) * - callRouter: present (Go method calls may need routing) */ @@ -28,7 +28,7 @@ export const goProvider = defineLanguage({ typeConfig: goConfig, exportChecker: goExportChecker, importResolver: resolveGoImport, - importSemantics: 'wildcard', + importSemantics: 'wildcard-leaf', fieldExtractor: createFieldExtractor(goFieldConfig), methodExtractor: createMethodExtractor(goMethodConfig), classExtractor: createClassExtractor({ diff --git a/gitnexus/src/core/ingestion/languages/ruby.ts b/gitnexus/src/core/ingestion/languages/ruby.ts index 00b566068..a15b5439c 100644 --- a/gitnexus/src/core/ingestion/languages/ruby.ts +++ b/gitnexus/src/core/ingestion/languages/ruby.ts @@ -107,7 +107,7 @@ export const rubyProvider = defineLanguage({ exportChecker: rubyExportChecker, importResolver: resolveRubyImport, callRouter: routeRubyCall, - importSemantics: 'wildcard', + importSemantics: 'wildcard-leaf', resolveEnclosingOwner(node) { // Ruby singleton_class (class << self) should resolve to the enclosing // class or module for owner/container resolution (HAS_METHOD edges, class IDs). diff --git a/gitnexus/src/core/ingestion/languages/swift.ts b/gitnexus/src/core/ingestion/languages/swift.ts index 7d01b4912..314c27c1d 100644 --- a/gitnexus/src/core/ingestion/languages/swift.ts +++ b/gitnexus/src/core/ingestion/languages/swift.ts @@ -5,7 +5,7 @@ * LanguageProvider, following the Strategy pattern used by the pipeline. * * Key Swift traits: - * - importSemantics: 'wildcard' (Swift imports entire modules) + * - importSemantics: 'wildcard-leaf' (Swift imports entire modules) * - heritageDefaultEdge: 'IMPLEMENTS' (protocols are more common than class inheritance) * - implicitImportWirer: all files in the same SPM target see each other */ @@ -238,7 +238,7 @@ export const swiftProvider = defineLanguage({ typeConfig: swiftConfig, exportChecker: swiftExportChecker, importResolver: resolveSwiftImport, - importSemantics: 'wildcard', + importSemantics: 'wildcard-leaf', heritageDefaultEdge: 'IMPLEMENTS', fieldExtractor: createFieldExtractor(swiftFieldConfig), methodExtractor: createMethodExtractor({ diff --git a/gitnexus/src/core/ingestion/pipeline-phases/wildcard-synthesis.ts b/gitnexus/src/core/ingestion/pipeline-phases/wildcard-synthesis.ts index c2c0f986d..83f38bc60 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/wildcard-synthesis.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/wildcard-synthesis.ts @@ -15,8 +15,10 @@ import type { KnowledgeGraph } from '../../graph/types.js'; import type { createResolutionContext } from '../model/resolution-context.js'; -import { getLanguageFromFilename, SupportedLanguages } from 'gitnexus-shared'; +import { getLanguageFromFilename } from 'gitnexus-shared'; +import type { SupportedLanguages } from 'gitnexus-shared'; import { providers, getProviderForFile } from '../languages/index.js'; +import type { LanguageProvider, ImportSemantics } from '../language-provider.js'; // ── Constants ────────────────────────────────────────────────────────────── @@ -41,10 +43,29 @@ const IMPORTABLE_SYMBOL_LABELS = new Set([ * for C/C++ files that include many large headers. */ const MAX_SYNTHETIC_BINDINGS_PER_FILE = 1000; +/** Max files allowed in a single transitive include closure. Guards against + * OOM on pathological C/C++ codebases (boost, Linux kernel-style monoheaders) + * where a single translation unit can transitively reach many thousands of + * headers. When the cap is hit, BFS expansion stops early — the file still + * synthesizes bindings from the partial closure rather than failing. */ +const MAX_TRANSITIVE_CLOSURE_SIZE = 5000; + +/** Import semantics tags whose languages need synthesis of whole-module imports. + * `wildcard-transitive` (C/C++) and `wildcard-leaf` (Go, Ruby, Swift, Dart) are + * the file-based wildcard strategies. `explicit-reexport` is a scaffold tag — + * no provider uses it yet, but it goes through the same leaf-style synthesis + * path today because a re-exporter is still an importer; only the extra DAG + * walk to surface re-exported symbols is missing (future work). */ +const WILDCARD_SEMANTICS: ReadonlySet = new Set([ + 'wildcard-transitive', + 'wildcard-leaf', + 'explicit-reexport', +]); + /** Languages with whole-module import semantics (derived from providers at module load). */ const WILDCARD_LANGUAGES = new Set( Object.values(providers) - .filter((p) => p.importSemantics === 'wildcard') + .filter((p) => WILDCARD_SEMANTICS.has(p.importSemantics)) .map((p) => p.id), ); @@ -66,6 +87,84 @@ export function needsSynthesis(lang: SupportedLanguages): boolean { return SYNTHESIS_LANGUAGES.has(lang); } +// ── Strategy implementations ─────────────────────────────────────────────── + +/** + * Strategy implementation for `importSemantics: 'wildcard-transitive'` (C, C++). + * + * Textual-include languages chain symbols through files: if `dict.c` includes + * `server.h` and `server.h` includes `dict.h`, then `dict.c` sees symbols from + * all three files. This helper walks the include graph (combining both the + * ingestion-context `importMap` and the graph-level IMPORTS edges) until the + * closure is stable. + * + * **Order matters.** The returned `Set` preserves iteration order (insertion + * order). `synthesizeWildcardImportBindings` dedupes bindings by symbol name + * on a first-seen-wins basis, so this closure's ordering determines which + * declaration wins when multiple headers export the same name (e.g. overloaded + * free functions like `write_audit()` vs `write_audit(const char*)` in + * different headers). We therefore: + * 1. Seed the closure with direct imports in declaration order (matches the + * order of `#include` directives in the source file). + * 2. Use FIFO / true BFS (`queue.shift()`) for transitive expansion, so + * closer headers are seen before deeper ones. + * + * Cycle-safe: the `closure.has(file)` guard prevents infinite loops on circular + * header includes, which are valid C/C++ when paired with `#pragma once` or + * include guards. + * + * Size-bounded: the closure is capped at `MAX_TRANSITIVE_CLOSURE_SIZE` files to + * prevent OOM on pathological codebases (e.g. boost, monoheader kernel code) + * where one translation unit can transitively reach tens of thousands of + * headers. Partial closures still yield useful bindings for the cluster of + * headers closest to the importer, which is what overload resolution and + * cross-file call resolution care about. + * + * Queue implementation: uses a head-index over a growing array (O(1) dequeue) + * instead of `Array.prototype.shift()` (O(n)) so deep chains stay linear. + */ +export function expandTransitiveIncludeClosure( + directImports: Iterable, + importMap: ReadonlyMap>, + graphImports: ReadonlyMap>, +): Set { + const closure = new Set(); + const queue: string[] = []; + let head = 0; // O(1) dequeue: advance the head index instead of shift()-ing. + + const tryEnqueue = (file: string): boolean => { + if (closure.has(file)) return true; + if (closure.size >= MAX_TRANSITIVE_CLOSURE_SIZE) return false; + closure.add(file); + queue.push(file); + return true; + }; + + // Seed direct imports in declaration order (see JSDoc on order-sensitivity). + for (const f of directImports) { + if (!tryEnqueue(f)) break; + } + // True BFS for transitive reach: head-index FIFO preserves the "closer + // headers first" ordering that overload resolution depends on. + while (head < queue.length) { + if (closure.size >= MAX_TRANSITIVE_CLOSURE_SIZE) break; + const file = queue[head++]!; + const nested = importMap.get(file); + if (nested) { + for (const n of nested) { + if (!tryEnqueue(n)) break; + } + } + const nestedGraph = graphImports.get(file); + if (nestedGraph) { + for (const n of nestedGraph) { + if (!tryEnqueue(n)) break; + } + } + } + return closure; +} + // ── Main synthesis function ──────────────────────────────────────────────── /** @@ -149,16 +248,67 @@ export function synthesizeWildcardImportBindings( } }; - // Synthesize from ctx.importMap (Ruby, C/C++, Swift file-based imports) + /** + * Dispatch wildcard synthesis by the file's language provider strategy. + * + * Strategy tags (see `ImportSemantics`): + * - `wildcard-transitive`: expand the include closure first (C/C++ #include + * chains — e.g. `dict.c` → `server.h` → `dict.h` so `dictFind` resolves + * across header chains) + * - `wildcard-leaf`: synthesize from direct imports only (Go, Ruby, Swift, Dart) + * - `explicit-reexport`: scaffold tag; falls through to leaf behavior. + * TODO(#821): implement re-export DAG walk for TS `export *` / Rust + * `pub use`. The leaf fallthrough preserves today's TS/Rust behavior + * (their direct imports still synthesize correctly); only the extra + * re-export DAG walk for barrel-file correctness is missing. + * - `namespace` / `named`: no-op here (namespace handled in Loop 3 below, + * named needs no synthesis). + * + * Used by both Loop 1 (ctx.importMap) and Loop 2 (graphImports) so a future + * transitive-import language whose edges arrive via graphImports gets closure + * expansion consistently regardless of edge source. + */ + const dispatchSynthesis = ( + filePath: string, + importedFiles: ReadonlySet, + provider: LanguageProvider, + ) => { + switch (provider.importSemantics) { + case 'wildcard-transitive': + synthesizeForFile( + filePath, + expandTransitiveIncludeClosure(importedFiles, ctx.importMap, graphImports), + ); + return; + case 'wildcard-leaf': + case 'explicit-reexport': + synthesizeForFile(filePath, importedFiles); + return; + case 'namespace': + case 'named': + return; + default: { + const _exhaustive: never = provider.importSemantics; + void _exhaustive; + } + } + }; + + // Loop 1: synthesize from ctx.importMap (Ruby, C/C++, Swift, Dart file-based imports). for (const [filePath, importedFiles] of ctx.importMap) { const lang = getLanguageFromFilename(filePath); if (!lang || !isWildcardImportLanguage(lang)) continue; - synthesizeForFile(filePath, importedFiles); + const provider = getProviderForFile(filePath); + if (!provider) continue; + dispatchSynthesis(filePath, importedFiles, provider); } - // Synthesize from graph IMPORTS edges (Go and other wildcard-import languages) + // Loop 2: synthesize from graph IMPORTS edges (Go and other wildcard-import + // languages whose edges live in the graph rather than ctx.importMap). for (const [filePath, importedFiles] of graphImports) { - synthesizeForFile(filePath, importedFiles); + const provider = getProviderForFile(filePath); + if (!provider) continue; + dispatchSynthesis(filePath, importedFiles, provider); } // Build Python module-alias maps for namespace-import languages. diff --git a/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/db.c b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/db.c new file mode 100644 index 000000000..deb76c90e --- /dev/null +++ b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/db.c @@ -0,0 +1,12 @@ +#include "server.h" + +void lookupKey(const char *key) { + dictEntry *entry = dictFind(key); + if (entry) { + void *val = entry->val; + } +} + +void dbGet(const char *key) { + void *val = dictFetchValue(key); +} diff --git a/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.c b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.c new file mode 100644 index 000000000..514a6c866 --- /dev/null +++ b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.c @@ -0,0 +1,12 @@ +#include "dict.h" +#include + +dictEntry *dictFind(const char *key) { + return NULL; +} + +void *dictFetchValue(const char *key) { + dictEntry *entry = dictFind(key); + if (entry) return entry->val; + return NULL; +} diff --git a/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.h b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.h new file mode 100644 index 000000000..9d17d19bf --- /dev/null +++ b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/dict.h @@ -0,0 +1,12 @@ +#ifndef DICT_H +#define DICT_H + +typedef struct dictEntry { + void *key; + void *val; +} dictEntry; + +dictEntry *dictFind(const char *key); +void *dictFetchValue(const char *key); + +#endif diff --git a/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/server.h b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/server.h new file mode 100644 index 000000000..e7ccf7fdd --- /dev/null +++ b/gitnexus/test/fixtures/cross-file-binding/c-cross-file/src/server.h @@ -0,0 +1,8 @@ +#ifndef SERVER_H +#define SERVER_H + +#include "dict.h" + +void processCommand(const char *cmd); + +#endif diff --git a/gitnexus/test/integration/cross-file-binding.test.ts b/gitnexus/test/integration/cross-file-binding.test.ts index 33063f94f..88ba776ed 100644 --- a/gitnexus/test/integration/cross-file-binding.test.ts +++ b/gitnexus/test/integration/cross-file-binding.test.ts @@ -436,6 +436,42 @@ describe('Phase 9 — Cross-File Call-Result Binding: C++', () => { }); }); +describe('Cross-File Call Resolution: pure C transitive #include', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(CROSS_FILE_FIXTURES, 'c-cross-file'), () => {}); + }, 60000); + + it('detects dictFind and dictFetchValue functions', () => { + expect(getNodesByLabel(result, 'Function')).toContain('dictFind'); + expect(getNodesByLabel(result, 'Function')).toContain('dictFetchValue'); + }); + + it('detects lookupKey and dbGet in db.c', () => { + expect(getNodesByLabel(result, 'Function')).toContain('lookupKey'); + expect(getNodesByLabel(result, 'Function')).toContain('dbGet'); + }); + + it('resolves dictFind() call in db.c to dict via transitive header chain', () => { + const calls = getRelationships(result, 'CALLS'); + const crossFileCall = calls.find( + (c) => + c.target === 'dictFind' && c.source === 'lookupKey' && c.targetFilePath.includes('dict'), + ); + expect(crossFileCall).toBeDefined(); + }); + + it('resolves dictFetchValue() call in db.c to dict via transitive header chain', () => { + const calls = getRelationships(result, 'CALLS'); + const crossFileCall = calls.find( + (c) => + c.target === 'dictFetchValue' && c.source === 'dbGet' && c.targetFilePath.includes('dict'), + ); + expect(crossFileCall).toBeDefined(); + }); +}); + describe('Phase 9 — Cross-File Call-Result Binding: C#', () => { let result: PipelineResult; diff --git a/gitnexus/test/unit/group/grpc-extractor.test.ts b/gitnexus/test/unit/group/grpc-extractor.test.ts index 82d79cbd6..1664bd1a7 100644 --- a/gitnexus/test/unit/group/grpc-extractor.test.ts +++ b/gitnexus/test/unit/group/grpc-extractor.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import * as fs from 'node:fs'; import fsp from 'node:fs/promises'; import * as path from 'node:path'; @@ -732,6 +732,120 @@ describe('resolveProtoConflict', () => { it('test_no_candidates_returns_null', () => { expect(resolveProtoConflict('Svc', 'src/main.go', [])).toBeNull(); }); + + it('test_all_zero_tie_returns_null', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const candidates = [ + makeInfo('pkgA', 'totally/unrelated/a/svc.proto'), + makeInfo('pkgB', 'completely/different/b/svc.proto'), + ]; + const result = resolveProtoConflict('Svc', 'src/main.go', candidates); + expect(result).toBeNull(); + warnSpy.mockRestore(); + }); + + it('test_positive_score_tie_returns_null', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Both candidates share `src/proto` with the source dir — equal shared runs. + const candidates = [ + makeInfo('pkgA', 'src/proto/a/svc.proto'), + makeInfo('pkgB', 'src/proto/b/svc.proto'), + ]; + const result = resolveProtoConflict('Svc', 'src/proto/main.go', candidates); + expect(result).toBeNull(); + warnSpy.mockRestore(); + }); + + it('test_three_way_zero_tie_returns_null', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const candidates = [ + makeInfo('pkgA', 'aaa/svc.proto'), + makeInfo('pkgB', 'bbb/svc.proto'), + makeInfo('pkgC', 'ccc/svc.proto'), + ]; + const result = resolveProtoConflict('Svc', 'src/main.go', candidates); + expect(result).toBeNull(); + warnSpy.mockRestore(); + }); + + it('test_unique_winner_among_ties', () => { + // Winner with shared run 2 (services/auth), two losers with score 0. + const candidates = [ + makeInfo('winner', 'services/auth/proto/svc.proto'), + makeInfo('loserA', 'totally/unrelated/a/svc.proto'), + makeInfo('loserB', 'elsewhere/b/svc.proto'), + ]; + const result = resolveProtoConflict('Svc', 'services/auth/src/server.ts', candidates); + expect(result?.package).toBe('winner'); + }); + + it('test_ambiguous_emits_single_warn_with_service_and_paths', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const candidates = [ + makeInfo('pkgA', 'totally/unrelated/a/svc.proto'), + makeInfo('pkgB', 'completely/different/b/svc.proto'), + ]; + resolveProtoConflict('MyService', 'src/main.go', candidates); + expect(warnSpy).toHaveBeenCalledTimes(1); + const msg = String(warnSpy.mock.calls[0][0]); + expect(msg).toContain('MyService'); + expect(msg).toContain('src/main.go'); + expect(msg).toContain('totally/unrelated/a/svc.proto'); + expect(msg).toContain('completely/different/b/svc.proto'); + warnSpy.mockRestore(); + }); +}); + +describe('GrpcExtractor.extract ambiguous proto resolution', () => { + let tmpDir: string; + let extractor: GrpcExtractor; + + beforeEach(async () => { + tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'gitnexus-grpc-ambig-')); + extractor = new GrpcExtractor(); + }); + afterEach(async () => { + await fsp.rm(tmpDir, { recursive: true, force: true }); + }); + + const makeRepo = (repoPath: string): RepoHandle => ({ + id: 'test-repo', + path: '', + repoPath, + storagePath: '', + }); + + it('test_ambiguous_short_name_across_unrelated_protos_yields_no_source_contract', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // Two unrelated proto files defining the same short name `UserService` in + // unrelated directories, neither sharing path segments with the Go source. + await fsp.mkdir(path.join(tmpDir, 'billing-team', 'proto'), { recursive: true }); + await fsp.writeFile( + path.join(tmpDir, 'billing-team', 'proto', 'user.proto'), + 'package billing.v1;\nservice UserService { rpc GetUser (R) returns (R); }', + ); + await fsp.mkdir(path.join(tmpDir, 'auth-team', 'proto'), { recursive: true }); + await fsp.writeFile( + path.join(tmpDir, 'auth-team', 'proto', 'user.proto'), + 'package auth.v1;\nservice UserService { rpc GetUser (R) returns (R); }', + ); + // Consumer in an unrelated directory. + await fsp.mkdir(path.join(tmpDir, 'apps', 'gateway'), { recursive: true }); + await fsp.writeFile( + path.join(tmpDir, 'apps', 'gateway', 'client.go'), + 'package main\nfunc init() { client := pb.NewUserServiceClient(conn) }', + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + + // No source-attributed contract for UserService should be emitted. + const sourceContracts = contracts.filter( + (c) => c.meta.source === 'go_client' && c.meta.service === 'UserService', + ); + expect(sourceContracts).toHaveLength(0); + expect(warnSpy).toHaveBeenCalled(); + warnSpy.mockRestore(); + }); }); describe('serviceContractId', () => { diff --git a/gitnexus/test/unit/group/http-route-multi-verb.test.ts b/gitnexus/test/unit/group/http-route-multi-verb.test.ts new file mode 100644 index 000000000..b2884fc60 --- /dev/null +++ b/gitnexus/test/unit/group/http-route-multi-verb.test.ts @@ -0,0 +1,393 @@ +/** + * Coverage tests for `HttpRouteExtractor` graph-assisted paths — + * specifically the multi-verb same-path regression (Codex finding F2). + * + * The bug: `extractProvidersGraph` / `extractConsumersGraph` used + * `detections.find(d => normalizeHttpPath(d.path) === routePath)` to + * backfill handler name and (for providers) method. On a file with + * multiple verbs at the same normalized path (e.g. `GET /api/orders` + * and `POST /api/orders` in one router), `.find()` returned the first + * match, silently attaching the wrong handler and/or method. + * + * Strategy: mock `./http-patterns/index.js` + `./fs-utils.js` so we + * can inject a synthetic `HttpDetection[]` per file without needing + * real tree-sitter grammars. The `db` executor is a vi.fn() that + * returns stubbed rows for the HANDLES_ROUTE / FETCHES / CONTAINS + * queries. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type Parser from 'tree-sitter'; +import type { HttpDetection } from '../../../src/core/group/extractors/http-patterns/types.js'; + +// Per-file detections injected into the mocked plugin. +const FILE_DETECTIONS = new Map(); + +vi.mock('../../../src/core/group/extractors/fs-utils.js', () => ({ + readSafe: (_repo: string, _rel: string) => 'stub content', +})); + +vi.mock('../../../src/core/group/extractors/http-patterns/index.js', () => { + return { + HTTP_SCAN_GLOB: '**/*.fake', + getPluginForFile: (rel: string) => ({ + name: 'fake', + language: {}, + scan: (_tree: Parser.Tree) => FILE_DETECTIONS.get(rel) ?? [], + }), + }; +}); + +// Patch tree-sitter Parser so `.setLanguage()` + `.parse()` don't +// require a real grammar — the mocked plugin's scan() ignores the +// tree anyway. +vi.mock('tree-sitter', () => { + class FakeParser { + setLanguage(_lang: unknown) {} + parse(_src: string) { + return {} as Parser.Tree; + } + } + return { default: FakeParser }; +}); + +import { HttpRouteExtractor } from '../../../src/core/group/extractors/http-route-extractor.js'; + +function detection( + role: 'provider' | 'consumer', + method: string, + p: string, + name: string | null, +): HttpDetection { + return { role, framework: 'test', method, path: p, name, confidence: 0.8 }; +} + +describe('HttpRouteExtractor — graph-assisted multi-verb disambiguation', () => { + beforeEach(() => { + FILE_DETECTIONS.clear(); + }); + + // Helper to build a CONTAINS response covering all handler names in a file. + const containsFor = (names: string[]) => + names.map((name, i) => ({ + uid: `uid-${name}`, + name, + filePath: 'routes.ts', + labels: ['Function'], + 0: `uid-${name}`, + 1: name, + 2: 'routes.ts', + 3: ['Function'], + })); + + // ── Provider: happy path (single match) ──────────────────────────── + it('provider: single detection backfills handler name as today', async () => { + FILE_DETECTIONS.set('routes.ts', [detection('provider', 'GET', '/api/orders', 'listOrders')]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeId: 'r1', + responseKeys: [], + routeSource: 'decorator-Get', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders']); + return []; + }); + + const ex = new HttpRouteExtractor(); + const out = await ex.extract(db, '/repo', { name: 'r', url: 'r' } as never); + expect(out).toHaveLength(1); + expect(out[0].symbolName).toBe('listOrders'); + expect(out[0].meta.method).toBe('GET'); + }); + + // ── Provider: multi-verb, method KNOWN (POST) ────────────────────── + it('provider: multi-verb with method known picks the matching verb (POST)', async () => { + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'decorator-Post', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders', 'createOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + expect(out[0].symbolName).toBe('createOrder'); + expect(out[0].meta.method).toBe('POST'); + }); + + it('provider: multi-verb with method known picks the matching verb (GET)', async () => { + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'decorator-Get', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders', 'createOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + expect(out[0].symbolName).toBe('listOrders'); + expect(out[0].meta.method).toBe('GET'); + }); + + // ── Provider: multi-verb, method UNKNOWN → refuse to guess ───────── + it('provider: multi-verb with method unknown skips backfill (no silent inheritance)', async () => { + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'unknown-reason', // methodFromRouteReason → null + }, + ]; + } + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + // CRITICAL: must NOT silently inherit POST from createOrder via .find() + expect(out[0].meta.method).toBe('GET'); // conservative default + // CRITICAL: must NOT silently attach createOrder as handler + expect(out[0].symbolName).not.toBe('createOrder'); + // With no CONTAINS rows, handlerName stays null and file-basename fallback wins. + expect(out[0].symbolName).toBe('routes.ts'); + }); + + // ── Provider: multi-verb + CONTAINS rows → must still refuse to guess ── + it('provider: ambiguous multi-verb skips CONTAINS enrichment (no silent pool[0] pick)', async () => { + // Regression test for Copilot's review on PR #817. Before the fix, + // the ambiguous-case code path left `handlerName` null but still ran + // the CONTAINS DB query, and `pickSymbolUid(syms, null)` silently + // picked pool[0] — reintroducing handler mis-attribution via a + // different route than `.find()`. + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'unknown-reason', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders', 'createOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + // Ambiguous → do not attribute to any real handler in the file. + expect(out[0].symbolName).not.toBe('listOrders'); + expect(out[0].symbolName).not.toBe('createOrder'); + expect(out[0].symbolUid).toBe(''); + expect(out[0].symbolName).toBe('routes.ts'); + expect(out[0].meta.method).toBe('GET'); + // CONTAINS query must have been skipped entirely under ambiguity. + const calls = db.mock.calls.map(([q]) => q as string); + expect(calls.some((q) => q.includes('CONTAINS'))).toBe(false); + }); + + // ── Provider: three-verb method known ────────────────────────────── + it('provider: three verbs at same path with method known still matches correctly', async () => { + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + detection('provider', 'PUT', '/api/orders', 'replaceOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'decorator-Put', + }, + ]; + } + if (query.includes('CONTAINS')) + return containsFor(['listOrders', 'createOrder', 'replaceOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out[0].symbolName).toBe('replaceOrder'); + expect(out[0].meta.method).toBe('PUT'); + }); + + // ── Provider: unrelated path detections don't false-positive ─────── + it('provider: detection for unrelated path does not backfill', async () => { + FILE_DETECTIONS.set('routes.ts', [detection('provider', 'POST', '/api/users', 'createUser')]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'unknown', + }, + ]; + } + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out[0].meta.method).toBe('GET'); + expect(out[0].symbolName).not.toBe('createUser'); + }); + + // ── Integration: one row, two detections, one out.push ───────────── + it('integration: one db row with two same-path detections yields exactly one contract', async () => { + FILE_DETECTIONS.set('routes.ts', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'decorator-Post', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders', 'createOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + expect(out[0].meta.method).toBe('POST'); + expect(out[0].symbolName).toBe('createOrder'); + }); + + // ── Consumer: single match ───────────────────────────────────────── + it('consumer: single detection backfills method as today', async () => { + FILE_DETECTIONS.set('client.ts', [detection('consumer', 'POST', '/api/orders', null)]); + + const db = vi.fn(async (query: string) => { + if (query.includes('FETCHES')) { + return [ + { + fileId: 'f1', + filePath: 'client.ts', + routePath: '/api/orders', + fetchReason: 'fetch', + }, + ]; + } + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + expect(out[0].meta.method).toBe('POST'); + }); + + // ── Consumer: multi-verb skips backfill ──────────────────────────── + it('consumer: multi-verb at same path skips backfill (conservative GET)', async () => { + FILE_DETECTIONS.set('client.ts', [ + detection('consumer', 'GET', '/api/orders', null), + detection('consumer', 'POST', '/api/orders', null), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('FETCHES')) { + return [ + { + fileId: 'f1', + filePath: 'client.ts', + routePath: '/api/orders', + fetchReason: 'fetch', + }, + ]; + } + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + // CRITICAL: must NOT silently pick POST (first/last via .find) + expect(out[0].meta.method).toBe('GET'); // conservative default + expect(out[0].contractId).toBe('http::GET::/api/orders'); + }); +}); diff --git a/gitnexus/test/unit/group/manifest-extractor.test.ts b/gitnexus/test/unit/group/manifest-extractor.test.ts index c2c67a33a..42ed86b84 100644 --- a/gitnexus/test/unit/group/manifest-extractor.test.ts +++ b/gitnexus/test/unit/group/manifest-extractor.test.ts @@ -300,6 +300,284 @@ describe('ManifestExtractor', () => { } }); + it('resolves http contract with explicit METHOD prefix (GET::/api/orders)', async () => { + // Regression test for Codex finding F1: resolveSymbol was passing the + // raw `link.contract` through normalizeRoutePath, which turned + // "GET::/api/orders" into "/GET::/api/orders" and never matched + // Route.name = "/api/orders". The extractor must strip the METHOD:: + // prefix and pass only the path portion to the Cypher executor. + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'GET::/api/orders', + role: 'consumer', + }, + ]; + + let seenParam: string | undefined; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'orders-svc', + async (_cypher, params) => { + seenParam = params?.normalized as string; + if (seenParam === '/api/orders') { + return [ + { + uid: 'uid-orders-list', + name: 'listOrders', + filePath: 'src/orders.ts', + }, + ]; + } + return []; + }, + ], + ['gateway', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + + // The key assertion: $normalized must be the path only, NOT "/GET::/api/orders". + expect(seenParam).toBe('/api/orders'); + + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.symbolUid).toBe('uid-orders-list'); + expect(provider?.symbolRef.filePath).toBe('src/orders.ts'); + }); + + it('resolves http contract with parameterised path (POST::/users/:id)', async () => { + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'users-svc', + type: 'http', + contract: 'POST::/users/:id', + role: 'consumer', + }, + ]; + + let seenParam: string | undefined; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'users-svc', + async (_cypher, params) => { + seenParam = params?.normalized as string; + if (seenParam === '/users/:id') { + return [ + { + uid: 'uid-update-user', + name: 'updateUser', + filePath: 'src/users.ts', + }, + ]; + } + return []; + }, + ], + ['gateway', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + expect(seenParam).toBe('/users/:id'); + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.symbolUid).toBe('uid-update-user'); + }); + + it('handles http contract with empty path after METHOD:: (GET::)', async () => { + // Edge case: "GET::" (empty path after prefix). Normalizer produces "/" + // — either resolves to a root route or returns null cleanly. + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'GET::', + role: 'consumer', + }, + ]; + + let seenParam: string | undefined; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'orders-svc', + async (_cypher, params) => { + seenParam = params?.normalized as string; + return []; + }, + ], + ['gateway', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + expect(seenParam).toBe('/'); + // No match → synthetic uid, no crash. + const provider = result.contracts.find((c) => c.role === 'provider'); + // buildContractId canonicalizes the empty path to `/` so contract ids + // match regardless of trailing-slash variants in the manifest input. + expect(provider?.symbolUid).toBe('manifest::orders-svc::http::GET::/'); + }); + + it('treats empty method portion (::/api/orders) as a bare path', async () => { + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: '::/api/orders', + role: 'consumer', + }, + ]; + + let seenParam: string | undefined; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'orders-svc', + async (_cypher, params) => { + seenParam = params?.normalized as string; + return []; + }, + ], + ['gateway', async () => []], + ]); + + await extractor.extractFromManifest(links, dbExecutors); + // "::/api/orders" has no method prefix per buildContractId's regex + // (`[A-Za-z]+::`), so the whole string is treated as a bare path. + // Normalizer collapses leading slashes, so "::/api/orders" stays + // essentially as-is (no alpha prefix match). + expect(seenParam).toBe('/::/api/orders'); + }); + + it('resolves http contract with lowercase verb (get::/api/orders)', async () => { + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'get::/api/orders', + role: 'consumer', + }, + ]; + + let seenParam: string | undefined; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'orders-svc', + async (_cypher, params) => { + seenParam = params?.normalized as string; + if (seenParam === '/api/orders') { + return [ + { + uid: 'uid-orders-list', + name: 'listOrders', + filePath: 'src/orders.ts', + }, + ]; + } + return []; + }, + ], + ['gateway', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + expect(seenParam).toBe('/api/orders'); + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.symbolUid).toBe('uid-orders-list'); + }); + + it('returns null cleanly when no Route matches explicit-method http contract', async () => { + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'GET::/api/orders', + role: 'consumer', + }, + ]; + + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + ['orders-svc', async () => []], + ['gateway', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + const provider = result.contracts.find((c) => c.role === 'provider'); + // No match → synthetic uid, caller falls back as today. + expect(provider?.symbolUid).toBe('manifest::orders-svc::http::GET::/api/orders'); + }); + + it('buildContractId round-trip regression for GET::/api/orders', async () => { + // Verifies buildContractId still produces http::GET::/api/orders for + // explicit-method form — i.e. the fix to resolveSymbol did not touch + // buildContractId. + const links: GroupManifestLink[] = [ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'GET::/api/orders', + role: 'consumer', + }, + ]; + + const result = await extractor.extractFromManifest(links); + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.contractId).toBe('http::GET::/api/orders'); + }); + + it('canonicalizes method casing so get::/api/orders and GET::/api/orders share a contractId', async () => { + // Regression for Copilot's review on PR #817: without canonicalization, + // `buildContractId` passed raw casing through (`http::get::/api/orders`) + // while `parseHttpContract` upper-cased during lookup, fragmenting + // cross-impact joins between providers and consumers that happened to + // use different casing conventions in their group.yaml. + const lower = await extractor.extractFromManifest([ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'get::/api/orders', + role: 'consumer', + }, + ]); + const upper = await extractor.extractFromManifest([ + { + from: 'gateway', + to: 'orders-svc', + type: 'http', + contract: 'GET::/api/orders', + role: 'consumer', + }, + ]); + const lowerContractId = lower.contracts.find((c) => c.role === 'provider')?.contractId; + const upperContractId = upper.contracts.find((c) => c.role === 'provider')?.contractId; + expect(lowerContractId).toBe('http::GET::/api/orders'); + expect(upperContractId).toBe('http::GET::/api/orders'); + expect(lowerContractId).toBe(upperContractId); + }); + it('returns empty for no links', async () => { const result = await extractor.extractFromManifest([]); expect(result.contracts).toHaveLength(0); diff --git a/gitnexus/test/unit/transitive-include-closure.test.ts b/gitnexus/test/unit/transitive-include-closure.test.ts new file mode 100644 index 000000000..fa9c0e6c9 --- /dev/null +++ b/gitnexus/test/unit/transitive-include-closure.test.ts @@ -0,0 +1,101 @@ +/** + * Unit tests for `expandTransitiveIncludeClosure` — the C/C++ Strategy 1 + * (`wildcard-transitive`) implementation extracted from `wildcard-synthesis.ts`. + * + * These tests exercise the BFS/DFS closure algorithm in isolation, without + * running the full pipeline. They cover edge cases flagged in PR #816 review: + * circular header includes, deep chains, and graphImports-only transitive paths. + */ + +import { describe, it, expect } from 'vitest'; +import { expandTransitiveIncludeClosure } from '../../src/core/ingestion/pipeline-phases/wildcard-synthesis.js'; + +const EMPTY = new Map>(); + +describe('expandTransitiveIncludeClosure', () => { + it('returns the direct imports when none are chained', () => { + const direct = new Set(['a.h', 'b.h']); + const closure = expandTransitiveIncludeClosure(direct, EMPTY, EMPTY); + expect([...closure].sort()).toEqual(['a.h', 'b.h']); + }); + + it('expands a two-hop chain via importMap (a.c → b.h → c.h)', () => { + const importMap = new Map>([['b.h', new Set(['c.h'])]]); + const closure = expandTransitiveIncludeClosure(new Set(['b.h']), importMap, EMPTY); + expect([...closure].sort()).toEqual(['b.h', 'c.h']); + }); + + it('expands a deep 5-level chain (A → B → C → D → E)', () => { + const importMap = new Map>([ + ['B.h', new Set(['C.h'])], + ['C.h', new Set(['D.h'])], + ['D.h', new Set(['E.h'])], + ]); + const closure = expandTransitiveIncludeClosure(new Set(['B.h']), importMap, EMPTY); + expect([...closure].sort()).toEqual(['B.h', 'C.h', 'D.h', 'E.h']); + }); + + it('terminates on circular header includes (A.h ↔ B.h)', () => { + const importMap = new Map>([ + ['A.h', new Set(['B.h'])], + ['B.h', new Set(['A.h'])], + ]); + const closure = expandTransitiveIncludeClosure(new Set(['A.h']), importMap, EMPTY); + expect([...closure].sort()).toEqual(['A.h', 'B.h']); + }); + + it('terminates on self-referential include (A.h includes A.h)', () => { + const importMap = new Map>([['A.h', new Set(['A.h'])]]); + const closure = expandTransitiveIncludeClosure(new Set(['A.h']), importMap, EMPTY); + expect([...closure]).toEqual(['A.h']); + }); + + it('expands through graphImports edges when importMap is empty', () => { + const graphImports = new Map>([['b.h', new Set(['c.h'])]]); + const closure = expandTransitiveIncludeClosure(new Set(['b.h']), EMPTY, graphImports); + expect([...closure].sort()).toEqual(['b.h', 'c.h']); + }); + + it('combines importMap and graphImports in one traversal', () => { + const importMap = new Map>([['b.h', new Set(['c.h'])]]); + const graphImports = new Map>([['c.h', new Set(['d.h'])]]); + const closure = expandTransitiveIncludeClosure(new Set(['b.h']), importMap, graphImports); + expect([...closure].sort()).toEqual(['b.h', 'c.h', 'd.h']); + }); + + it('returns an empty set when given no direct imports', () => { + const closure = expandTransitiveIncludeClosure(new Set(), EMPTY, EMPTY); + expect(closure.size).toBe(0); + }); + + it('caps closure size to prevent OOM on pathological codebases', () => { + // Build a synthetic include graph of 10,000 files, each including the next. + // The cap (5000) should halt BFS early with a partial but bounded closure. + const importMap = new Map>(); + for (let i = 0; i < 10_000; i++) { + importMap.set(`h${i}.h`, new Set([`h${i + 1}.h`])); + } + const closure = expandTransitiveIncludeClosure(new Set(['h0.h']), importMap, EMPTY); + expect(closure.size).toBe(5000); + // Partial closure still starts from the importer's side (BFS ordering). + expect(closure.has('h0.h')).toBe(true); + expect(closure.has('h1.h')).toBe(true); + expect(closure.has('h9999.h')).toBe(false); + }); + + it('deduplicates when a file is reachable through multiple paths (diamond)', () => { + // A + // / \ + // B C + // \ / + // D + const importMap = new Map>([ + ['A.h', new Set(['B.h', 'C.h'])], + ['B.h', new Set(['D.h'])], + ['C.h', new Set(['D.h'])], + ]); + const closure = expandTransitiveIncludeClosure(new Set(['A.h']), importMap, EMPTY); + expect([...closure].sort()).toEqual(['A.h', 'B.h', 'C.h', 'D.h']); + expect(closure.size).toBe(4); // D.h appears once + }); +});