diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index ce1fda913..2c3eebe3b 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -25,6 +25,8 @@ const DEFAULT_IGNORE_LIST = new Set([ 'bower_components', 'jspm_packages', 'vendor', // PHP/Go + 'third_party', // C/C++ (Google-style vendored dependencies) + '3rdparty', // C/C++ (alternate spelling, also Qt convention) // 'packages' removed - commonly used for monorepo source code (lerna, pnpm, yarn workspaces) 'venv', '.venv', diff --git a/gitnexus/src/core/group/config-parser.ts b/gitnexus/src/core/group/config-parser.ts index 73a9021b9..29c868171 100644 --- a/gitnexus/src/core/group/config-parser.ts +++ b/gitnexus/src/core/group/config-parser.ts @@ -4,9 +4,26 @@ import type { GroupConfig, GroupManifestLink, ContractType, ContractRole } from const _require = createRequire(import.meta.url); const yaml = _require('js-yaml') as typeof import('js-yaml'); -const VALID_CONTRACT_TYPES: ContractType[] = ['http', 'grpc', 'thrift', 'topic', 'lib', 'custom']; +const VALID_CONTRACT_TYPES: ContractType[] = [ + 'http', + 'grpc', + 'thrift', + 'topic', + 'lib', + 'custom', + 'include', +]; const VALID_ROLES: ContractRole[] = ['provider', 'consumer']; +// Defaults matter for backward compatibility: any group.yaml that omits a +// `detect.` key inherits its value from this constant. Adding a new +// extractor that defaults to `true` silently changes the behavior of every +// existing group on the next sync. New extractors must default to `false` +// (opt-in) so operators consciously enable them via group.yaml. +// +// `includes`: opt-in. The C/C++ IncludeExtractor (PR #1156) ships disabled by +// default; enable with `detect.includes: true` for groups containing C/C++ +// repos that need cross-repo header tracking. const DEFAULT_DETECT = { http: true, grpc: true, @@ -14,6 +31,7 @@ const DEFAULT_DETECT = { topics: true, shared_libs: true, embedding_fallback: true, + includes: false, workspace_deps: false, }; diff --git a/gitnexus/src/core/group/extractors/include-extractor.ts b/gitnexus/src/core/group/extractors/include-extractor.ts new file mode 100644 index 000000000..7bbfd61ed --- /dev/null +++ b/gitnexus/src/core/group/extractors/include-extractor.ts @@ -0,0 +1,610 @@ +import * as path from 'node:path'; +import * as fs from 'node:fs/promises'; +import { glob } from 'glob'; +import Parser from 'tree-sitter'; +import C from 'tree-sitter-c'; +import Cpp from 'tree-sitter-cpp'; +import type { ContractExtractor, CypherExecutor } from '../contract-extractor.js'; +import type { ExtractedContract, RepoHandle } from '../types.js'; +import { readSafe } from './fs-utils.js'; +import { buildSuffixIndex, type SuffixIndex } from '../../ingestion/import-resolvers/utils.js'; +import { createIgnoreFilter } from '../../../config/ignore-service.js'; +import { getMaxFileSizeBytes } from '../../ingestion/utils/max-file-size.js'; +import { logger } from '../../logger.js'; + +/** + * Cross-repo C/C++ `#include` dependency extractor. + * + * **Provider side:** registers every `.h/.hpp/.hxx/.hh` file in the repo + * as a provider contract with `include::`. + * + * **Consumer side:** parses all C/C++ source/header files for `#include "…"` + * directives, attempts suffix-based resolution against the repo's own file + * list (reusing the same algorithm as the single-repo ingestion pipeline), + * and emits unresolved include paths as consumer contracts. + * + * Matching: a consumer's `include::map/base/dice_map_view.h` in repo A + * matches a provider's `include::map/base/dice_map_view.h` in repo B via + * exact contract-id equality in `runExactMatch`. + */ + +// ---------- constants ---------- + +const HEADER_EXTENSIONS = new Set(['.h', '.hpp', '.hxx', '.hh']); + +// Source = headers (provider-eligible) ∪ implementation files (.c/.cpp/.cc/.cxx). +// Spread keeps the subset relationship explicit so a future contributor adding +// a new header extension to HEADER_EXTENSIONS does not have to remember to +// also add it here. +const SOURCE_EXTENSIONS = new Set([...HEADER_EXTENSIONS, '.c', '.cpp', '.cc', '.cxx']); + +const INCLUDE_QUERY_SRC = '(preproc_include path: (_) @import.source) @import'; + +/** + * Well-known C/C++ standard library headers that can appear in `#include "…"` + * form (some projects use quotes for system headers). + */ +const SYSTEM_HEADERS = new Set([ + // C standard + 'assert.h', + 'complex.h', + 'ctype.h', + 'errno.h', + 'fenv.h', + 'float.h', + 'inttypes.h', + 'iso646.h', + 'limits.h', + 'locale.h', + 'math.h', + 'setjmp.h', + 'signal.h', + 'stdalign.h', + 'stdarg.h', + 'stdatomic.h', + 'stdbool.h', + 'stddef.h', + 'stdint.h', + 'stdio.h', + 'stdlib.h', + 'stdnoreturn.h', + 'string.h', + 'tgmath.h', + 'threads.h', + 'time.h', + 'uchar.h', + 'wchar.h', + 'wctype.h', + // C++ standard (extensionless) + 'algorithm', + 'any', + 'array', + 'atomic', + 'barrier', + 'bit', + 'bitset', + 'cassert', + 'cctype', + 'cerrno', + 'cfenv', + 'cfloat', + 'charconv', + 'chrono', + 'cinttypes', + 'climits', + 'clocale', + 'cmath', + 'codecvt', + 'compare', + 'complex', + 'concepts', + 'condition_variable', + 'coroutine', + 'csetjmp', + 'csignal', + 'cstdarg', + 'cstddef', + 'cstdint', + 'cstdio', + 'cstdlib', + 'cstring', + 'ctime', + 'cuchar', + 'cwchar', + 'cwctype', + 'deque', + 'exception', + 'execution', + 'expected', + 'filesystem', + 'format', + 'forward_list', + 'fstream', + 'functional', + 'future', + 'generator', + 'initializer_list', + 'iomanip', + 'ios', + 'iosfwd', + 'iostream', + 'istream', + 'iterator', + 'latch', + 'limits', + 'list', + 'locale', + 'map', + 'mdspan', + 'memory', + 'memory_resource', + 'mutex', + 'new', + 'numbers', + 'numeric', + 'optional', + 'ostream', + 'print', + 'queue', + 'random', + 'ranges', + 'ratio', + 'regex', + 'scoped_allocator', + 'semaphore', + 'set', + 'shared_mutex', + 'source_location', + 'span', + 'spanstream', + 'sstream', + 'stack', + 'stacktrace', + 'stdexcept', + 'stdfloat', + 'stop_token', + 'streambuf', + 'string', + 'string_view', + 'strstream', + 'syncstream', + 'system_error', + 'thread', + 'tuple', + 'type_traits', + 'typeindex', + 'typeinfo', + 'unordered_map', + 'unordered_set', + 'utility', + 'valarray', + 'variant', + 'vector', + 'version', +]); + +/** Path prefixes that indicate system/kernel headers. */ +const SYSTEM_PATH_PREFIXES = [ + 'sys/', + 'net/', + 'netinet/', + 'arpa/', + 'linux/', + 'asm/', + 'bits/', + 'gnu/', + 'mach/', + 'machine/', + 'xlocale/', +]; + +/** Regex fallback for files that exceed tree-sitter's 32 KB parse limit. */ +const INCLUDE_REGEX = /^[ \t]*#\s*include\s*"([^"]+)"/gm; + +// ---------- helpers ---------- + +/** + * Normalize an include path to a canonical lowercase forward-slash form. + * + * IMPORTANT — case-folding caveat (PR #1156 review finding #3): + * Header paths are lowercased so consumer `#include "Foo/Bar.h"` and + * provider file `Foo/Bar.h` normalize to the same contract-id. This is + * the right trade-off on case-insensitive filesystems (macOS, Windows) + * but on case-sensitive Linux filesystems two distinct headers `Foo.h` + * and `foo.h` in the same repo will collide onto the same provider + * contract-id; only one survives `dedupe()`. The gain (reliable + * cross-platform matching) outweighs the cost (extremely rare header + * casing collisions inside a single repo). + */ +function normalizeIncludePath(raw: string): string { + return raw.replace(/\\/g, '/').replace(/^\.\//, '').replace(/\/+/g, '/').toLowerCase(); +} + +/** + * Strip C/C++ block comments from a source blob. Used only by the + * regex-fallback path to avoid emitting consumer contracts for + * commented-out #include directives. Line comments (`// …`) cannot hide + * #include directives because the regex anchors on start-of-line. + * See PR #1156 review finding #5. + */ +function stripBlockComments(src: string): string { + return src.replace(/\/\*[\s\S]*?\*\//g, ''); +} + +function isAngleBracketInclude(rawNodeText: string): boolean { + const trimmed = rawNodeText.trim(); + return trimmed.startsWith('<') && trimmed.endsWith('>'); +} + +function isSystemHeader(cleanedPath: string): boolean { + // Check well-known standard headers + if (SYSTEM_HEADERS.has(cleanedPath)) return true; + // Check system path prefixes + const lower = cleanedPath.toLowerCase(); + return SYSTEM_PATH_PREFIXES.some((prefix) => lower.startsWith(prefix)); +} + +function isHeaderFile(filePath: string): boolean { + return HEADER_EXTENSIONS.has(path.extname(filePath).toLowerCase()); +} + +function getLanguageForFile(filePath: string): unknown | null { + const ext = path.extname(filePath).toLowerCase(); + switch (ext) { + case '.c': + case '.h': + return C; + case '.cpp': + case '.cc': + case '.cxx': + case '.hpp': + case '.hxx': + case '.hh': + return Cpp; + default: + return null; + } +} + +/** + * Check whether an include path resolves to a file inside the local repo. + * + * Uses *exact full-path* matching on the suffix index — we never accept a + * truncated suffix match. For `#include "foo/bar.h"` this checks: + * (a) a file whose path ends with the full `foo/bar.h` + * (b) if the include omitted the extension, a file whose path ends with + * the include + one of the C/C++ header extensions + * + * Returns `true` when a local file matches — caller should suppress the + * cross-repo consumer contract. + * + * See PR #1156 review finding #4 (suffixResolve ambiguity). + */ +function isLocalInclude(cleaned: string, suffixIndex: SuffixIndex): boolean { + const candidates = [cleaned]; + if (!/\.[a-zA-Z0-9]+$/.test(cleaned)) { + for (const ext of ['.h', '.hpp', '.hxx', '.hh']) candidates.push(cleaned + ext); + } + for (const c of candidates) { + if (suffixIndex.get(c) || suffixIndex.getInsensitive(c)) return true; + } + return false; +} + +// ---------- main class ---------- + +export class IncludeExtractor implements ContractExtractor { + type = 'include' as const; + + /** + * Always returns `true`. NOT called by `sync.ts`, which gates extraction via + * `config.detect.includes` instead (see `sync.ts:174`). Kept solely to satisfy + * the `ContractExtractor` interface so the type stays uniform across extractors. + */ + async canExtract(_repo: RepoHandle): Promise { + return true; + } + + async extract( + dbExecutor: CypherExecutor | null, + repoPath: string, + _repo: RepoHandle, + ): Promise { + // 1. Build the local file list using the same discovery as ingestion + // (createIgnoreFilter + getMaxFileSizeBytes). This guarantees the + // universe of provider/consumer paths matches the universe of File + // nodes in the LadybugDB graph — so no cross-link points at a UID + // that group impact cannot fan out to. + // (PR #1156 Codex follow-up: discovery aligned with ingestion.) + const allFiles = await this.discoverIndexableFiles(repoPath); + const normalizedFiles = allFiles.map((f) => f.replace(/\\/g, '/')); + const suffixIndex = buildSuffixIndex(normalizedFiles, allFiles); + + // 2. Provider: register all header files + const providers = await this.extractProviders(dbExecutor, repoPath, allFiles); + + // 3. Consumer: filter the shared discovery list for source extensions + // and parse #include directives in those files. + const sourceFiles = allFiles.filter((f) => + SOURCE_EXTENSIONS.has(path.extname(f).toLowerCase()), + ); + const consumers = await this.extractConsumers(repoPath, sourceFiles, suffixIndex); + + return this.dedupe([...providers, ...consumers]); + } + + /** + * Discover repo-relative file paths using exactly the same rules the + * ingestion pipeline uses (`walkRepositoryPaths` in + * `gitnexus/src/core/ingestion/filesystem-walker.ts`): + * - `createIgnoreFilter` honors `.gitignore`, `.gitnexusignore`, the + * hardcoded ignore list, and `.gitnexusignore` last-match-wins + * negation. + * - `getMaxFileSizeBytes()` drops files larger than the cap so we + * never emit `File:` UIDs for files ingestion would skip. + * + * Uses sequential stat — there is no `READ_CONCURRENCY` batching here + * because group sync runs at startup-time, not the ingestion hot path, + * and parallelism gains are not worth the import-graph weight. + * + * MAINTENANCE: if `walkRepositoryPaths` changes its glob options, ignore + * filter shape, or size-cap logic, mirror those changes here. The two + * implementations exist because the consumers need different return + * shapes (string[] vs ScannedFile[]) and different concurrency, but + * they MUST agree on which files are reachable — that is what makes + * `File:` UIDs in cross-links correspond to graph File nodes. + */ + private async discoverIndexableFiles(repoPath: string): Promise { + const ignoreFilter = await createIgnoreFilter(repoPath); + const maxFileSizeBytes = getMaxFileSizeBytes(); + + const candidates = await glob('**/*', { + cwd: repoPath, + nodir: true, + dot: false, + ignore: ignoreFilter, + }); + + const survivors: string[] = []; + for (const rel of candidates) { + try { + const stat = await fs.stat(path.join(repoPath, rel)); + if (stat.size > maxFileSizeBytes) continue; + survivors.push(rel); + } catch (err) { + // ENOENT is the documented benign race (glob enumerated a file + // that was deleted before we stat'd it — same race + // walkRepositoryPaths absorbs via Promise.allSettled). Anything + // else (EACCES, EMFILE, EIO) deserves a warning so an operator + // can spot a permission/resource problem instead of silently + // shipping fewer contracts than expected. + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (code !== 'ENOENT') { + logger.warn( + { err: (err as Error).message, file: rel, repoPath }, + '⚠️ IncludeExtractor: stat failed during discovery; skipping file', + ); + } + } + } + return survivors; + } + + // ---------- provider extraction ---------- + + private async extractProviders( + dbExecutor: CypherExecutor | null, + repoPath: string, + allFiles: string[], + ): Promise { + // Strategy A: graph-assisted + if (dbExecutor) { + const graphProviders = await this.extractProvidersGraph(dbExecutor, repoPath); + if (graphProviders.length > 0) return graphProviders; + } + // Strategy B: filesystem fallback + return this.extractProvidersFallback(repoPath, allFiles); + } + + private async extractProvidersGraph( + db: CypherExecutor, + repoPath: string, + ): Promise { + try { + const rows = await db( + `MATCH (f:File) + WHERE f.filePath =~ '.*\\\\.(h|hpp|hxx|hh)$' + RETURN f.filePath AS filePath, f.id AS fileId`, + ); + // gitnexus analyze stores absolute paths in the File.filePath column. + // Provider contract IDs MUST be repo-relative — otherwise the consumer + // emits `include::map/base/view.h` and the provider emits + // `include::/abs/path/to/repo/map/base/view.h`, which never match + // through runExactMatch and the cross-link silently disappears. + // (PR #1156 follow-up review: graph provider absolute-path bug.) + const normalizedRepoPath = path.resolve(repoPath); + const out: ExtractedContract[] = []; + for (const r of rows) { + if (typeof r.filePath !== 'string' || !r.filePath) continue; + const absolute = r.filePath as string; + const rel = path.relative(normalizedRepoPath, absolute); + // Skip rows that resolve outside the repo (e.g., system headers + // somehow indexed, or stale absolute paths from a different machine). + // path.relative returns a `..`-prefixed path or an absolute path + // when the target is outside the base — both are wrong for our IDs. + if (!rel || rel.startsWith('..') || path.isAbsolute(rel)) continue; + const normalizedRel = rel.replace(/\\/g, '/'); + out.push({ + contractId: `include::${normalizeIncludePath(normalizedRel)}`, + type: 'include' as const, + role: 'provider' as const, + symbolUid: String(r.fileId ?? ''), + symbolRef: { filePath: normalizedRel, name: path.basename(normalizedRel) }, + symbolName: path.basename(normalizedRel), + confidence: 1.0, + meta: { source: 'graph' }, + }); + } + return out; + } catch { + return []; + } + } + + private extractProvidersFallback(_repoPath: string, allFiles: string[]): ExtractedContract[] { + return allFiles + .filter((f) => isHeaderFile(f)) + .map((f) => { + const filePath = f.replace(/\\/g, '/'); + return { + contractId: `include::${normalizeIncludePath(filePath)}`, + type: 'include' as const, + role: 'provider' as const, + symbolUid: `File:${filePath}`, + symbolRef: { filePath, name: path.basename(filePath) }, + symbolName: path.basename(filePath), + confidence: 0.95, + meta: { source: 'filesystem' }, + }; + }); + } + + // ---------- consumer extraction ---------- + + private async extractConsumers( + repoPath: string, + sourceFiles: string[], + suffixIndex: SuffixIndex, + ): Promise { + const parser = new Parser(); + const out: ExtractedContract[] = []; + // Compile the include query once per grammar to avoid re-compilation per file + const queryCache = new Map(); + + for (const rel of sourceFiles) { + const lang = getLanguageForFile(rel); + if (!lang) continue; + + const content = readSafe(repoPath, rel); + if (!content) continue; + + let query = queryCache.get(lang); + if (!query) { + try { + query = new Parser.Query(lang, INCLUDE_QUERY_SRC); + queryCache.set(lang, query); + } catch { + continue; + } + } + + // Collect raw include paths: tree-sitter first, regex fallback for large files. + // `extractionSource` is stamped on each emitted consumer contract so + // regex-fallback contracts stay auditable post-hoc (PR #1156 review finding #6). + let rawIncludes: string[]; + let extractionSource: 'tree_sitter' | 'regex_fallback'; + try { + parser.setLanguage(lang); + const tree = parser.parse(content); + let matches: Parser.QueryMatch[]; + try { + matches = query.matches(tree.rootNode); + } catch { + matches = []; + } + rawIncludes = []; + extractionSource = 'tree_sitter'; + for (const match of matches) { + const sourceNode = match.captures.find((c) => c.name === 'import.source'); + if (!sourceNode) continue; + const rawText = sourceNode.node.text; + if (isAngleBracketInclude(rawText)) continue; + const cleaned = rawText.replace(/['"<>]/g, ''); + if (cleaned && cleaned.length <= 2048) rawIncludes.push(cleaned); + } + } catch { + // tree-sitter failed (e.g. file > 32 KB) — fall back to regex. + // Strip block comments first so we don't emit a consumer contract + // for a commented-out #include (PR #1156 review finding #5). + rawIncludes = []; + extractionSource = 'regex_fallback'; + const scanTarget = stripBlockComments(content); + INCLUDE_REGEX.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = INCLUDE_REGEX.exec(scanTarget)) !== null) { + if (m[1] && m[1].length <= 2048) rawIncludes.push(m[1]); + } + } + + for (const cleaned of rawIncludes) { + // Filter: skip known system headers and system path prefixes + if (isSystemHeader(cleaned)) continue; + + // Skip relative-up includes: `#include "../include/foo.h"` is + // almost always an intra-repo reference. The suffix index is built + // from repo-relative paths, so isLocalInclude can never match + // `../foo.h`, and emitting it as a consumer contract just pollutes + // the registry with an entry no provider can ever satisfy. + // (PR #1156 follow-up review: `../` relative includes produce + // spurious consumer contracts.) + if (cleaned.startsWith('../') || cleaned.startsWith('..\\')) continue; + + // Skip macro-style includes: `#include PLATFORM_HEADER` parses as an + // identifier under tree-sitter's `(_) @import.source` wildcard. The + // identifier text passes the strip/clean step unchanged, so without + // this guard we would emit `include::platform_header` as a consumer + // contract — and no provider in any repo will ever expose a contract + // for a macro identifier (no file is named `PLATFORM_HEADER`). The + // contract would sit permanently orphaned in the registry. Real + // header references always contain a path separator (`/`, `\`) or an + // extension dot (`foo.h`), so an absent both is a reliable signal we + // are looking at a macro identifier. (PR #1156 follow-up review: + // macro includes emit orphaned consumer contracts.) + if (!/[./\\]/.test(cleaned)) continue; + + // Local resolution (PR #1156 review finding #4): only accept an + // exact-suffix match on the *full* include path. The generic + // suffixResolve() iterates all truncated suffixes, which would + // silently suppress a cross-repo `#include "map/base/view.h"` + // when the local repo has any `internal/view.h` — a realistic + // false-negative in large C++ codebases. Here we only resolve + // locally if a file path ends with the complete include string + // (optionally re-appending one of the C/C++ header extensions + // when the include already omits it). + if (isLocalInclude(cleaned, suffixIndex)) continue; + + // Unresolved: emit as consumer contract + const normalizedRel = rel.replace(/\\/g, '/'); + out.push({ + contractId: `include::${normalizeIncludePath(cleaned)}`, + type: 'include' as const, + role: 'consumer' as const, + symbolUid: `File:${normalizedRel}`, + symbolRef: { filePath: normalizedRel, name: cleaned }, + symbolName: cleaned, + confidence: 0.85, + meta: { + source: extractionSource, + includePath: cleaned, + }, + }); + } + } + + return out; + } + + // ---------- deduplication ---------- + + private dedupe(items: ExtractedContract[]): ExtractedContract[] { + const seen = new Set(); + const out: ExtractedContract[] = []; + for (const c of items) { + const k = `${c.contractId}|${c.role}|${c.symbolRef.filePath}`; + if (seen.has(k)) continue; + seen.add(k); + out.push(c); + } + return out; + } +} diff --git a/gitnexus/src/core/group/extractors/manifest-extractor.ts b/gitnexus/src/core/group/extractors/manifest-extractor.ts index 2af3db595..f4d0f77cf 100644 --- a/gitnexus/src/core/group/extractors/manifest-extractor.ts +++ b/gitnexus/src/core/group/extractors/manifest-extractor.ts @@ -274,6 +274,14 @@ export class ManifestExtractor { LIMIT 1`, { contract: link.contract }, ); + } else if (link.type === 'include') { + rows = await executor( + `MATCH (f:File) WHERE f.filePath = $contract + RETURN f.id AS uid, f.name AS name, f.filePath AS filePath + ORDER BY f.filePath ASC + LIMIT 1`, + { contract: link.contract }, + ); } else if (link.type === 'custom') { // Workspace extractors produce qualified contracts like "mathlex::Expression". // Graph nodes store the unqualified symbol name ("Expression"), so strip @@ -358,6 +366,8 @@ export class ManifestExtractor { return `lib::${contract}`; case 'custom': return `custom::${contract}`; + case 'include': + return `include::${contract}`; default: { const _exhaustive: never = type; throw new Error(`Unhandled ContractType: ${String(_exhaustive)}`); diff --git a/gitnexus/src/core/group/extractors/thrift-extractor.ts b/gitnexus/src/core/group/extractors/thrift-extractor.ts index cfd8fef02..709968790 100644 --- a/gitnexus/src/core/group/extractors/thrift-extractor.ts +++ b/gitnexus/src/core/group/extractors/thrift-extractor.ts @@ -217,6 +217,10 @@ export async function buildThriftContext(repoPath: string): Promise(); @@ -290,6 +294,10 @@ export class ThriftExtractor implements ContractExtractor { cwd: repoPath, absolute: false, nodir: true, + // TODO(#1156-followup): replace this hand-rolled list with createIgnoreFilter + // (the canonical ingestion ignore filter, like include-extractor.ts now uses). + // New entries to DEFAULT_IGNORE_LIST in src/config/ignore-service.ts (e.g. + // third_party, 3rdparty added in commit a9936a9b) silently do not apply here. ignore: ['**/node_modules/**', '**/.git/**', '**/vendor/**', '**/dist/**', '**/build/**'], }); diff --git a/gitnexus/src/core/group/matching.ts b/gitnexus/src/core/group/matching.ts index 3431f8ddf..0b27655c6 100644 --- a/gitnexus/src/core/group/matching.ts +++ b/gitnexus/src/core/group/matching.ts @@ -107,6 +107,8 @@ export function normalizeContractId(id: string): string { return `topic::${rest.trim().toLowerCase()}`; case 'lib': return `lib::${rest.toLowerCase()}`; + case 'include': + return `include::${rest.replace(/\\/g, '/').replace(/^\.\//, '').replace(/\/+/g, '/').toLowerCase()}`; default: return id; } diff --git a/gitnexus/src/core/group/storage.ts b/gitnexus/src/core/group/storage.ts index cc3dbfdc9..bc08fd7f9 100644 --- a/gitnexus/src/core/group/storage.ts +++ b/gitnexus/src/core/group/storage.ts @@ -4,6 +4,7 @@ import * as path from 'node:path'; import * as os from 'node:os'; import { randomBytes } from 'node:crypto'; import type { ContractRegistry } from './types.js'; +import { retryRename } from './bridge-db.js'; /** * Build an unpredictable suffix for atomic-write tmp files. Replaces the @@ -59,7 +60,13 @@ export async function writeContractRegistry( } finally { await handle.close(); } - await fsp.rename(tmpPath, targetPath); + // retryRename absorbs the documented Windows EPERM/EBUSY/EACCES race that + // fires when AV scanners or another concurrent rename briefly hold the + // destination handle between rename calls. Same helper bridge-db.ts uses + // (lines 304, 583, 587, 595, 605, 677) for the bridge.lbug atomic swap — + // single source of truth for the Windows-rename pattern across the group + // package. + await retryRename(tmpPath, targetPath); } export async function readContractRegistry(groupDir: string): Promise { diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index 7ed065131..cd64fdf8c 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -8,12 +8,14 @@ import { HttpRouteExtractor } from './extractors/http-route-extractor.js'; import { GrpcExtractor } from './extractors/grpc-extractor.js'; import { ThriftExtractor } from './extractors/thrift-extractor.js'; import { TopicExtractor } from './extractors/topic-extractor.js'; +import { IncludeExtractor } from './extractors/include-extractor.js'; import { ManifestExtractor } from './extractors/manifest-extractor.js'; import { discoverWorkspaceLinks } from './extractors/workspace-extractor.js'; import { buildProviderIndex, runExactMatch, runWildcardMatch } from './matching.js'; import { detectServiceBoundaries, assignService } from './service-boundary-detector.js'; import type { CypherExecutor } from './contract-extractor.js'; import { writeContractRegistry } from './storage.js'; +import { writeBridge } from './bridge-db.js'; import type { ContractRegistry } from './types.js'; import { logger } from '../logger.js'; @@ -100,6 +102,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis const grpcEx = new GrpcExtractor(); const thriftEx = new ThriftExtractor(); const topicEx = new TopicExtractor(); + const includeEx = new IncludeExtractor(); dbExecutors = new Map(); const openPoolIds: string[] = []; @@ -168,6 +171,17 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis } } + if (config.detect.includes) { + const extracted = await includeEx.extract(executor, handle.repoPath, handle); + for (const c of extracted) { + autoContracts.push({ + ...c, + repo: groupPath, + service: assignService(c.symbolRef.filePath, boundaries), + }); + } + } + const metaPath = path.join(handle.storagePath, 'meta.json'); try { const raw = await fs.readFile(metaPath, 'utf-8'); @@ -270,6 +284,28 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis if (opts?.groupDir && !opts.skipWrite) { await writeContractRegistry(opts.groupDir, registry); + // writeBridge failure (disk full, schema error, permission denied) must + // not mask the registry — contracts.json was just written successfully + // and is the canonical source of truth. A stale or absent bridge + // degrades impact queries to empty results, which is recoverable on + // the next sync. Surface the failure as a warning so operators can + // act, but do not propagate it. + // (PR #1156 follow-up review: writeBridge error in sync.ts propagates + // uncaught.) + try { + await writeBridge(opts.groupDir, { + contracts: allContracts, + crossLinks, + repoSnapshots, + missingRepos, + }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + logger.warn( + { err: msg, groupDir: opts.groupDir }, + '⚠️ writeBridge failed; contracts.json is intact but bridge.lbug is stale. Re-run `gitnexus group sync` to retry.', + ); + } } return { diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index 7d0a14251..8e43ff78f 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -1,4 +1,4 @@ -export type ContractType = 'http' | 'grpc' | 'thrift' | 'topic' | 'lib' | 'custom'; +export type ContractType = 'http' | 'grpc' | 'thrift' | 'topic' | 'lib' | 'custom' | 'include'; export type MatchType = 'exact' | 'manifest' | 'wildcard' | 'bm25' | 'embedding'; export type ContractRole = 'provider' | 'consumer'; @@ -28,6 +28,7 @@ export interface DetectConfig { topics: boolean; shared_libs: boolean; embedding_fallback: boolean; + includes: boolean; workspace_deps: boolean; } diff --git a/gitnexus/test/integration/group/include-extractor-sync.test.ts b/gitnexus/test/integration/group/include-extractor-sync.test.ts new file mode 100644 index 000000000..908664bdc --- /dev/null +++ b/gitnexus/test/integration/group/include-extractor-sync.test.ts @@ -0,0 +1,195 @@ +/** + * Integration test: IncludeExtractor output → group matching → bridge DB. + * + * Covers PR #1156 review finding #7: verifies that the full runtime path + * (IncludeExtractor → StoredContract → runExactMatch → CrossLinks → writeBridge) + * stays wired up. A regression in either normalizeContractId or the include + * branch of ManifestExtractor.resolveSymbol would produce 0 cross-links and + * fail this test. + */ +import { describe, it, expect } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { parseGroupConfig } from '../../../src/core/group/config-parser.js'; +import { syncGroup } from '../../../src/core/group/sync.js'; +import type { StoredContract } from '../../../src/core/group/types.js'; +import { IncludeExtractor } from '../../../src/core/group/extractors/include-extractor.js'; +import { normalizeContractId } from '../../../src/core/group/matching.js'; + +const GROUP_YAML = [ + 'version: 1', + 'name: include-test-group', + 'description: "IncludeExtractor integration test"', + '', + 'repos:', + ' app/provider: include-provider', + ' app/consumer: include-consumer', + '', + 'links: []', + 'packages: {}', + '', + 'detect:', + ' http: false', + ' grpc: false', + ' topics: false', + ' shared_libs: false', + ' includes: true', + ' embedding_fallback: false', + '', + 'matching:', + ' bm25_threshold: 0.7', + ' embedding_threshold: 0.65', + ' max_candidates_per_step: 3', +].join('\n'); + +describe('IncludeExtractor → syncGroup integration (finding #7)', () => { + it('produces a CrossLink when provider and consumer emit the same include contract-id', async () => { + const config = parseGroupConfig(GROUP_YAML); + + // Mock the IncludeExtractor output directly — a header provider in one + // repo and a quoted #include consumer in the other, both normalized to + // the same include::map/base/view.h contract-id. + const mockContracts: StoredContract[] = [ + { + contractId: 'include::map/base/view.h', + type: 'include', + role: 'provider', + symbolUid: 'File:map/base/view.h', + symbolRef: { filePath: 'map/base/view.h', name: 'view.h' }, + symbolName: 'view.h', + confidence: 0.95, + meta: { source: 'filesystem' }, + repo: 'app/provider', + }, + { + contractId: 'include::map/base/view.h', + type: 'include', + role: 'consumer', + symbolUid: 'File:src/controller.cpp', + symbolRef: { filePath: 'src/controller.cpp', name: 'map/base/view.h' }, + symbolName: 'map/base/view.h', + confidence: 0.85, + meta: { source: 'tree_sitter', includePath: 'map/base/view.h' }, + repo: 'app/consumer', + }, + ]; + + const result = await syncGroup(config, { + extractorOverride: async () => mockContracts, + skipWrite: true, + }); + + const includeLinks = result.crossLinks.filter((l) => l.type === 'include'); + expect(includeLinks.length).toBeGreaterThanOrEqual(1); + + const link = includeLinks[0]; + expect(link.contractId).toBe('include::map/base/view.h'); + expect(link.matchType).toBe('exact'); + expect(link.from.repo).toBe('app/consumer'); + expect(link.to.repo).toBe('app/provider'); + }); + + it('normalizes mixed-case / backslash include paths to the same contract-id end-to-end', async () => { + const config = parseGroupConfig(GROUP_YAML); + + // Provider writes the canonical form; consumer's include has mixed case + // and a backslash. After normalizeContractId they must still match. + const providerId = 'include::map/base/view.h'; + const rawConsumerId = 'include::Map\\Base\\View.h'; + + // Sanity — normalizeContractId must collapse them. + expect(normalizeContractId(rawConsumerId)).toBe(providerId); + + const mockContracts: StoredContract[] = [ + { + contractId: providerId, + type: 'include', + role: 'provider', + symbolUid: 'File:map/base/view.h', + symbolRef: { filePath: 'map/base/view.h', name: 'view.h' }, + symbolName: 'view.h', + confidence: 0.95, + meta: { source: 'filesystem' }, + repo: 'app/provider', + }, + { + contractId: rawConsumerId, + type: 'include', + role: 'consumer', + symbolUid: 'File:src/controller.cpp', + symbolRef: { filePath: 'src/controller.cpp', name: 'Map/Base/View.h' }, + symbolName: 'Map/Base/View.h', + confidence: 0.85, + meta: { source: 'tree_sitter', includePath: 'Map\\Base\\View.h' }, + repo: 'app/consumer', + }, + ]; + + const result = await syncGroup(config, { + extractorOverride: async () => mockContracts, + skipWrite: true, + }); + + const includeLinks = result.crossLinks.filter((l) => l.type === 'include'); + expect(includeLinks.length).toBeGreaterThanOrEqual(1); + }); + + it('round-trip: extractor output from two real temp repos produces matching contract-ids', async () => { + // Drives the extractor directly (no `syncGroup`) against two on-disk + // fixture repos, then hands the StoredContract-shaped output to + // syncGroup via extractorOverride. This exercises the real extraction + // code + the matching pipeline together. + const providerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-include-int-provider-')); + const consumerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-include-int-consumer-')); + try { + fs.mkdirSync(path.join(providerDir, 'shared/api'), { recursive: true }); + fs.writeFileSync( + path.join(providerDir, 'shared/api/client.h'), + '#pragma once\nstruct Client {};', + ); + fs.mkdirSync(path.join(consumerDir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(consumerDir, 'src/main.cpp'), + '#include "shared/api/client.h"\nint main(){return 0;}', + ); + + const extractor = new IncludeExtractor(); + const providerOutput = await extractor.extract(null, providerDir, { + id: 'provider', + path: 'app/provider', + repoPath: providerDir, + storagePath: path.join(providerDir, '.gitnexus'), + }); + const consumerOutput = await extractor.extract(null, consumerDir, { + id: 'consumer', + path: 'app/consumer', + repoPath: consumerDir, + storagePath: path.join(consumerDir, '.gitnexus'), + }); + + const stored: StoredContract[] = [ + ...providerOutput + .filter((c) => c.role === 'provider') + .map((c) => ({ ...c, repo: 'app/provider' })), + ...consumerOutput + .filter((c) => c.role === 'consumer') + .map((c) => ({ ...c, repo: 'app/consumer' })), + ]; + + const config = parseGroupConfig(GROUP_YAML); + const result = await syncGroup(config, { + extractorOverride: async () => stored, + skipWrite: true, + }); + + const includeLinks = result.crossLinks.filter((l) => l.type === 'include'); + expect(includeLinks.length).toBeGreaterThanOrEqual(1); + expect(includeLinks[0].contractId).toBe('include::shared/api/client.h'); + expect(includeLinks[0].matchType).toBe('exact'); + } finally { + fs.rmSync(providerDir, { recursive: true, force: true }); + fs.rmSync(consumerDir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/group/config-parser.test.ts b/gitnexus/test/unit/group/config-parser.test.ts index e1d3b540f..22bb2ad26 100644 --- a/gitnexus/test/unit/group/config-parser.test.ts +++ b/gitnexus/test/unit/group/config-parser.test.ts @@ -75,6 +75,62 @@ repos: expect(config.detect.thrift).toBe(true); }); + // PR #1156 Codex follow-up: include extraction is opt-in. Existing + // group.yaml files that do not declare `detect.includes` must not gain + // a wave of new include::* contracts on the next sync after upgrade. + describe('detect.includes opt-in default', () => { + it('defaults includes detection to false when detect block omits it', () => { + const minimal = ` +version: 1 +name: test +repos: + app: my-app +`; + const config = parseGroupConfig(minimal); + expect(config.detect.includes).toBe(false); + }); + + it('defaults includes detection to false when detect block is present but omits the key', () => { + const yaml = ` +version: 1 +name: test +repos: + app: my-app +detect: + http: true + grpc: false +`; + const config = parseGroupConfig(yaml); + expect(config.detect.includes).toBe(false); + }); + + it('honors explicit detect.includes: true (opt-in works)', () => { + const yaml = ` +version: 1 +name: test +repos: + app: my-app +detect: + includes: true +`; + const config = parseGroupConfig(yaml); + expect(config.detect.includes).toBe(true); + }); + + it('honors explicit detect.includes: false', () => { + const yaml = ` +version: 1 +name: test +repos: + app: my-app +detect: + includes: false +`; + const config = parseGroupConfig(yaml); + expect(config.detect.includes).toBe(false); + }); + }); + it('parses thrift manifest links', () => { const yaml = ` version: 1 diff --git a/gitnexus/test/unit/group/include-extractor.test.ts b/gitnexus/test/unit/group/include-extractor.test.ts new file mode 100644 index 000000000..3956cd5f2 --- /dev/null +++ b/gitnexus/test/unit/group/include-extractor.test.ts @@ -0,0 +1,563 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { IncludeExtractor } from '../../../src/core/group/extractors/include-extractor.js'; +import type { RepoHandle } from '../../../src/core/group/types.js'; +import { normalizeContractId } from '../../../src/core/group/matching.js'; + +describe('IncludeExtractor', () => { + let tmpDir: string; + let extractor: IncludeExtractor; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-include-')); + extractor = new IncludeExtractor(); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function writeFile(relPath: string, content: string): void { + const full = path.join(tmpDir, relPath); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } + + const makeRepo = (repoPath: string): RepoHandle => ({ + id: 'test-repo', + path: 'test/app', + repoPath, + storagePath: path.join(repoPath, '.gitnexus'), + }); + + // ---- Provider detection ---- + + describe('provider extraction', () => { + it('registers .h files as providers', async () => { + writeFile('map/base/view.h', '#pragma once\nclass View {};'); + writeFile('map/base/types.h', '#pragma once\nstruct Point {};'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + expect(providers).toHaveLength(2); + const ids = providers.map((p) => p.contractId).sort(); + expect(ids).toEqual(['include::map/base/types.h', 'include::map/base/view.h']); + expect(providers[0].type).toBe('include'); + expect(providers[0].confidence).toBeGreaterThanOrEqual(0.95); + }); + + it('registers .hpp files as providers', async () => { + writeFile('utils/helper.hpp', '#pragma once\ntemplate T id(T x) { return x; }'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + expect(providers).toHaveLength(1); + expect(providers[0].contractId).toBe('include::utils/helper.hpp'); + }); + + it('does not register .cpp files as providers', async () => { + writeFile('src/main.cpp', 'int main() { return 0; }'); + writeFile('src/utils.h', '#pragma once'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + expect(providers).toHaveLength(1); + expect(providers[0].contractId).toBe('include::src/utils.h'); + }); + }); + + // ---- Consumer detection ---- + + describe('consumer extraction', () => { + it('emits unresolved includes as consumers', async () => { + writeFile( + 'src/main.cpp', + `#include "map/base/view.h" +#include "map/base/types.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(2); + const ids = consumers.map((c) => c.contractId).sort(); + expect(ids).toEqual(['include::map/base/types.h', 'include::map/base/view.h']); + expect(consumers[0].type).toBe('include'); + expect(consumers[0].confidence).toBe(0.85); + }); + + it('skips locally resolved includes', async () => { + writeFile('map/base/view.h', '#pragma once\nclass View {};'); + writeFile( + 'src/main.cpp', + `#include "map/base/view.h" +#include "external/lib.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // Only external/lib.h should be a consumer — map/base/view.h resolves locally + expect(consumers).toHaveLength(1); + expect(consumers[0].contractId).toBe('include::external/lib.h'); + }); + + it('skips angle-bracket includes', async () => { + writeFile( + 'src/main.cpp', + `#include +#include +#include "app/interface.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(1); + expect(consumers[0].contractId).toBe('include::app/interface.h'); + }); + + it('skips well-known system headers in quotes', async () => { + writeFile( + 'src/main.cpp', + `#include "stdio.h" +#include "stdlib.h" +#include "app/config.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(1); + expect(consumers[0].contractId).toBe('include::app/config.h'); + }); + + it('skips system path prefixes', async () => { + writeFile( + 'src/main.c', + `#include "sys/types.h" +#include "linux/input.h" +#include "mylib/types.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(1); + expect(consumers[0].contractId).toBe('include::mylib/types.h'); + }); + }); + + // ---- Cross-repo matching scenario ---- + + describe('cross-repo matching', () => { + it('provider and consumer produce matching contractIds', async () => { + // Simulate provider repo (header-only) + const providerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-include-provider-')); + const providerFile = path.join(providerDir, 'map/base/dice_map_view.h'); + fs.mkdirSync(path.dirname(providerFile), { recursive: true }); + fs.writeFileSync(providerFile, '#pragma once\nclass DiceMapView {};'); + + // Simulate consumer repo + const consumerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-include-consumer-')); + const consumerFile = path.join(consumerDir, 'src/controller.cpp'); + fs.mkdirSync(path.dirname(consumerFile), { recursive: true }); + fs.writeFileSync(consumerFile, '#include "map/base/dice_map_view.h"\nvoid init() {}'); + + try { + const providerContracts = await extractor.extract(null, providerDir, makeRepo(providerDir)); + const consumerContracts = await extractor.extract(null, consumerDir, makeRepo(consumerDir)); + + const providers = providerContracts.filter((c) => c.role === 'provider'); + const consumers = consumerContracts.filter((c) => c.role === 'consumer'); + + expect(providers.length).toBeGreaterThanOrEqual(1); + expect(consumers.length).toBeGreaterThanOrEqual(1); + + const providerIds = new Set(providers.map((p) => normalizeContractId(p.contractId))); + const consumerIds = consumers.map((c) => normalizeContractId(c.contractId)); + + // The consumer's include path should match a provider's file path + expect(providerIds.has(consumerIds[0])).toBe(true); + } finally { + fs.rmSync(providerDir, { recursive: true, force: true }); + fs.rmSync(consumerDir, { recursive: true, force: true }); + } + }); + }); + + // ---- Review finding #4: suffixResolve ambiguity ---- + + describe('finding #4: suffix-ambiguity does not silently suppress cross-repo include', () => { + it('emits a cross-repo contract when the include path does not match any local file (even if a shorter suffix does)', async () => { + // local repo has `internal/api.h` but NOT `ext/api.h` + writeFile('internal/api.h', '#pragma once'); + writeFile( + 'src/main.cpp', + `#include "ext/api.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // Previously suffixResolve would match `api.h` against `internal/api.h` + // and drop the cross-repo contract. After finding #4 fix, we only + // accept exact full-path matches — so `ext/api.h` must still be + // emitted as a consumer contract. + expect(consumers).toHaveLength(1); + expect(consumers[0].contractId).toBe('include::ext/api.h'); + }); + + it('still suppresses a local include when the FULL path matches', async () => { + writeFile('ext/api.h', '#pragma once'); + writeFile('src/main.cpp', '#include "ext/api.h"\nint main(){return 0;}'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(0); + }); + + it('resolves locally when include omits extension and a matching .h exists', async () => { + writeFile('foo/bar.h', '#pragma once'); + writeFile('src/main.cpp', '#include "foo/bar"\nint main(){return 0;}'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(0); + }); + }); + + // ---- Review finding #5: regex fallback must strip block comments ---- + + describe('finding #5: regex fallback ignores block-commented includes', () => { + it('does not emit a contract for an #include inside /* ... */', async () => { + // Force regex fallback by producing a file larger than tree-sitter's + // 32 KB hard cap. The include we care about lives inside a block + // comment that spans the file. + const filler = 'int dummy_' + 'x'.repeat(32) + ' = 0;\n'.repeat(1200); + const content = `/* + * Historical include, kept for reference only: + * #include "legacy/old-api.h" + */ +${filler} +#include "real/api.h" +int main(){return 0;}`; + writeFile('src/huge.cpp', content); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + const ids = consumers.map((c) => c.contractId); + + // The live include should appear; the commented-out one must NOT. + expect(ids).toContain('include::real/api.h'); + expect(ids).not.toContain('include::legacy/old-api.h'); + }); + }); + + // ---- Review finding #6: meta.source must reflect which extraction path ran ---- + + describe('finding #6: meta.source reflects extraction path', () => { + it('stamps `tree_sitter` on contracts produced via AST walking', async () => { + writeFile('src/main.cpp', '#include "app/small.h"\nint main(){return 0;}'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(1); + expect((consumers[0].meta as { source?: string } | undefined)?.source).toBe('tree_sitter'); + }); + + it('meta.source is one of the two documented values (tree_sitter | regex_fallback)', async () => { + // Regex fallback is a defensive branch that only fires if + // parser.setLanguage() or parser.parse() throws. In practice + // tree-sitter-c/cpp handles realistic inputs, so we only assert + // the meta.source contract: it is always present and always one of + // the two documented values. This guards against future regressions + // that might hard-code the wrong string. + writeFile('src/main.cpp', '#include "ext/whatever.h"\nint main(){return 0;}'); + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumer = contracts.find((c) => c.role === 'consumer'); + expect(consumer).toBeDefined(); + const src = (consumer?.meta as { source?: string } | undefined)?.source; + expect(['tree_sitter', 'regex_fallback']).toContain(src); + }); + }); + + // ---- Review finding #3: provider id collision on case-sensitive FS ---- + + describe('finding #3: case-folding is documented and deterministic', () => { + it('collapses `Foo.h` and `foo.h` onto the same provider contract-id (documented trade-off)', async () => { + writeFile('Foo.h', '#pragma once\n// Capital Foo'); + // On case-insensitive filesystems (macOS default) the second writeFile + // will overwrite the first, so we only create this when distinct files + // can coexist (case-sensitive FS, e.g. Linux CI). + try { + fs.writeFileSync(path.join(tmpDir, 'foo.h'), '#pragma once\n// lowercase foo'); + } catch { + // Ignore — some FS won't allow both names to coexist. + } + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + const ids = providers.map((p) => p.contractId); + + // Both files (if they coexist) must normalize to the same id. + // dedupe() keeps only one; caller code must be aware of this. + expect(ids).toContain('include::foo.h'); + // Never see a mixed-case contract-id leak out. + expect(ids.every((id) => id === id.toLowerCase())).toBe(true); + }); + }); + + // ---- Deduplication ---- + + describe('deduplication', () => { + it('deduplicates same include from multiple source files', async () => { + writeFile('src/a.cpp', '#include "ext/api.h"\nvoid a() {}'); + writeFile('src/b.cpp', '#include "ext/api.h"\nvoid b() {}'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // Both files include "ext/api.h" — each should produce a separate + // consumer contract (different symbolRef.filePath) + expect(consumers).toHaveLength(2); + const files = consumers.map((c) => c.symbolRef.filePath).sort(); + expect(files).toEqual(['src/a.cpp', 'src/b.cpp']); + }); + }); + + // ---- normalizeContractId ---- + + describe('normalizeContractId for include', () => { + it('lowercases the path', () => { + expect(normalizeContractId('include::Map/Base/Foo.h')).toBe('include::map/base/foo.h'); + }); + + it('normalizes backslashes', () => { + expect(normalizeContractId('include::map\\base\\foo.h')).toBe('include::map/base/foo.h'); + }); + + it('strips leading ./', () => { + expect(normalizeContractId('include::./foo.h')).toBe('include::foo.h'); + }); + + it('collapses consecutive slashes', () => { + expect(normalizeContractId('include::map//base///foo.h')).toBe('include::map/base/foo.h'); + }); + }); + + // ---- PR #1156 follow-up: `../` relative includes ---- + + describe('follow-up: `../` relative includes are skipped', () => { + it('does not emit a consumer contract for `#include "../foo.h"`', async () => { + // Producer: a header that exists locally but only via parent reference + writeFile('include/foo.h', '#pragma once'); + writeFile( + 'src/sub/main.cpp', + `#include "../../include/foo.h" +#include "real/cross_repo.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + // Only `real/cross_repo.h` should remain — the `..`-prefixed include + // is intra-repo noise that no provider can ever satisfy. + expect(consumers.map((c) => c.contractId)).toEqual(['include::real/cross_repo.h']); + }); + + it('skips backslash-form `..\\` for completeness', async () => { + writeFile( + 'src/main.cpp', + `#include "..\\\\sibling\\\\foo.h" +#include "remote/header.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + const ids = consumers.map((c) => c.contractId); + expect(ids).toContain('include::remote/header.h'); + expect(ids.some((id) => id.includes('..'))).toBe(false); + }); + }); + + // ---- PR #1156 follow-up: macro-style includes ---- + + describe('follow-up: macro-style #include emits no consumer contract', () => { + it('does not emit a consumer contract for `#include PLATFORM_HEADER` (no separator, no dot)', async () => { + // `#include PLATFORM_HEADER` parses under tree-sitter as an identifier + // node, slips past the existing system-header / `..` filters, and used + // to leak through as a permanently orphaned consumer contract because + // no file is ever named `PLATFORM_HEADER`. Verify the macro guard + // suppresses it while preserving the real cross-repo include. + writeFile( + 'src/main.cpp', + `#include PLATFORM_HEADER +#include "real/api.h" +int main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers.map((c) => c.contractId)).toEqual(['include::real/api.h']); + }); + + it('skips multiple macro identifiers in the same translation unit', async () => { + writeFile( + 'src/cfg.cpp', + `#include CONFIG_HEADER +#include PLATFORM_HEADER +#include ASSERT_H_ +int main(){return 0;}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(0); + }); + }); + + // ---- PR #1156 follow-up: graph provider absolute paths ---- + + describe('follow-up: extractProvidersGraph strips repo root from absolute paths', () => { + it('produces repo-relative contract IDs when the graph returns absolute paths', async () => { + writeFile('map/base/view.h', '#pragma once\nclass View {};'); + writeFile('utils/types.hpp', '#pragma once'); + + // Stub the Cypher executor to return absolute paths the way + // gitnexus analyze actually persists them. + const absolute1 = path.join(tmpDir, 'map/base/view.h'); + const absolute2 = path.join(tmpDir, 'utils/types.hpp'); + const stubDb = async () => [ + { filePath: absolute1, fileId: 'File:abs:1' }, + { filePath: absolute2, fileId: 'File:abs:2' }, + ]; + + const contracts = await extractor.extract(stubDb, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + const ids = providers.map((p) => p.contractId).sort(); + expect(ids).toEqual(['include::map/base/view.h', 'include::utils/types.hpp']); + expect(providers.every((p) => p.meta?.source === 'graph')).toBe(true); + }); + + it('drops graph rows whose path resolves outside the repo root', async () => { + writeFile('local/header.h', '#pragma once'); + const absoluteLocal = path.join(tmpDir, 'local/header.h'); + const stubDb = async () => [ + { filePath: absoluteLocal, fileId: 'File:1' }, + // Stale absolute path from a different machine — must be skipped. + { filePath: '/some/other/repo/foreign.h', fileId: 'File:2' }, + ]; + + const contracts = await extractor.extract(stubDb, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.role === 'provider'); + + expect(providers.map((p) => p.contractId)).toEqual(['include::local/header.h']); + }); + }); + + // ---- PR #1156 Codex follow-up: discovery aligned with ingestion ---- + + describe('follow-up: file discovery honors createIgnoreFilter and getMaxFileSizeBytes', () => { + it('does not emit a provider contract for a header excluded by .gitignore', async () => { + writeFile('.gitignore', 'vendor-headers/\n'); + writeFile('vendor-headers/blocked.h', '#pragma once'); + writeFile('src/wanted.h', '#pragma once'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providerIds = contracts.filter((c) => c.role === 'provider').map((p) => p.contractId); + + expect(providerIds).toContain('include::src/wanted.h'); + expect(providerIds).not.toContain('include::vendor-headers/blocked.h'); + }); + + it('does not emit a provider contract for a header excluded by .gitnexusignore', async () => { + writeFile('.gitnexusignore', 'legacy/\n'); + writeFile('legacy/old.h', '#pragma once'); + writeFile('src/current.h', '#pragma once'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providerIds = contracts.filter((c) => c.role === 'provider').map((p) => p.contractId); + + expect(providerIds).toContain('include::src/current.h'); + expect(providerIds).not.toContain('include::legacy/old.h'); + }); + + it('does not parse #include directives in a source file excluded by .gitignore', async () => { + // The ignored source file references a header that would otherwise be + // a cross-repo consumer. After alignment, the ignored file is invisible + // to the consumer scan — no consumer contract should appear. + writeFile('.gitignore', 'generated/\n'); + writeFile( + 'generated/auto.cpp', + `#include "remote/should_not_appear.h" +int auto_main() { return 0; }`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumerIds = contracts.filter((c) => c.role === 'consumer').map((c) => c.contractId); + + expect(consumerIds).not.toContain('include::remote/should_not_appear.h'); + }); + + it('skips a provider header whose size exceeds GITNEXUS_MAX_FILE_SIZE', async () => { + const previous = process.env.GITNEXUS_MAX_FILE_SIZE; + process.env.GITNEXUS_MAX_FILE_SIZE = '1'; // 1 KB cap + try { + // 4 KB header — comfortably exceeds the cap. + const oversized = '#pragma once\n' + 'x'.repeat(4 * 1024); + writeFile('huge/big.h', oversized); + writeFile('small/tiny.h', '#pragma once'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providerIds = contracts.filter((c) => c.role === 'provider').map((p) => p.contractId); + + expect(providerIds).toContain('include::small/tiny.h'); + expect(providerIds).not.toContain('include::huge/big.h'); + } finally { + if (previous === undefined) delete process.env.GITNEXUS_MAX_FILE_SIZE; + else process.env.GITNEXUS_MAX_FILE_SIZE = previous; + } + }); + + it('skips parsing #include directives in source files exceeding GITNEXUS_MAX_FILE_SIZE', async () => { + const previous = process.env.GITNEXUS_MAX_FILE_SIZE; + process.env.GITNEXUS_MAX_FILE_SIZE = '1'; + try { + const oversized = + '#include "remote/should_not_appear.h"\n' + + '// padding to push the file past 1 KB\n' + + 'x'.repeat(4 * 1024); + writeFile('big/main.cpp', oversized); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumerIds = contracts.filter((c) => c.role === 'consumer').map((c) => c.contractId); + + expect(consumerIds).not.toContain('include::remote/should_not_appear.h'); + } finally { + if (previous === undefined) delete process.env.GITNEXUS_MAX_FILE_SIZE; + else process.env.GITNEXUS_MAX_FILE_SIZE = previous; + } + }); + }); +}); diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 88bc3af2d..9f14db231 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -3,6 +3,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; import { syncGroup, stableRepoPoolId } from '../../../src/core/group/sync.js'; +import { cleanupTempDir } from '../../helpers/test-db.js'; import { _captureLogger } from '../../../src/core/logger.js'; import type { GroupConfig, @@ -583,6 +584,47 @@ service OrderService { } }); + it('does not extract include contracts during real sync when includes detection is disabled', async () => { + // PR #1156 Codex follow-up: ce-code-review T1 — verifies the gate at + // sync.ts:174 honors `detect.includes: false`. Mirrors the existing + // thrift-off pattern at sync.test.ts:545. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-sync-includes-off-')); + const storageDir = path.join(tmpDir, '.gitnexus'); + fs.mkdirSync(path.join(tmpDir, 'src'), { recursive: true }); + fs.mkdirSync(storageDir, { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'src', 'view.h'), '#pragma once\nclass View {};'); + + const config = makeConfig({ 'app/cpp-lib': 'cpp-lib-repo' }); + config.detect.http = false; + config.detect.grpc = false; + config.detect.thrift = false; + config.detect.topics = false; + config.detect.includes = false; + + const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js'); + const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined); + const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockResolvedValue(undefined); + + try { + const result = await syncGroup(config, { + resolveRepoHandle: async (_name, groupPath) => ({ + id: 'cpp-lib-repo', + path: groupPath, + repoPath: tmpDir, + storagePath: storageDir, + }), + skipWrite: true, + }); + + expect(result.missingRepos).toHaveLength(0); + expect(result.contracts.filter((c) => c.type === 'include')).toHaveLength(0); + } finally { + initSpy.mockRestore(); + closeSpy.mockRestore(); + await cleanupTempDir(tmpDir); + } + }); + it('dedupes duplicate wildcard cross-links during sync', async () => { const config = makeConfig({ 'app/provider': 'provider-repo', 'app/consumer': 'consumer-repo' }); const provider: StoredContract = { @@ -689,7 +731,12 @@ service OrderService { expect(registry.version).toBe(1); expect(registry.contracts).toHaveLength(0); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + // syncGroup now writes bridge.lbug + WAL/shadow sidecars when + // skipWrite is false. On Windows, LadybugDB's checkpoint thread can + // briefly outlive closeBridgeDb, holding a Win32 lock on the file. + // cleanupTempDir tolerates the documented Windows-native lock codes + // (EBUSY/EPERM/EACCES/ENOTEMPTY) with bounded retries. + await cleanupTempDir(tmpDir); } }); diff --git a/gitnexus/test/unit/ignore-service.test.ts b/gitnexus/test/unit/ignore-service.test.ts index b4e5cdca1..1e1908137 100644 --- a/gitnexus/test/unit/ignore-service.test.ts +++ b/gitnexus/test/unit/ignore-service.test.ts @@ -28,6 +28,8 @@ describe('shouldIgnorePath', () => { it.each([ 'node_modules', 'vendor', + 'third_party', + '3rdparty', 'venv', '.venv', '__pycache__',