diff --git a/gitnexus/src/core/group/extractors/include-extractor.ts b/gitnexus/src/core/group/extractors/include-extractor.ts index 2cdc0cf59..98cbd371f 100644 --- a/gitnexus/src/core/group/extractors/include-extractor.ts +++ b/gitnexus/src/core/group/extractors/include-extractor.ts @@ -1,4 +1,5 @@ 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'; @@ -6,12 +7,11 @@ 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, - suffixResolve, - type SuffixIndex, -} from '../../ingestion/import-resolvers/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 { parseSourceSafe } from '../../tree-sitter/safe-parse.js'; +import { logger } from '../../logger.js'; /** * Cross-repo C/C++ `#include` dependency extractor. @@ -33,20 +33,11 @@ import { parseSourceSafe } from '../../tree-sitter/safe-parse.js'; const HEADER_EXTENSIONS = new Set(['.h', '.hpp', '.hxx', '.hh']); -const _HEADER_GLOB = '**/*.{h,hpp,hxx,hh}'; -const SOURCE_GLOB = '**/*.{c,cpp,cc,cxx,h,hpp,hxx,hh}'; - -const STANDARD_IGNORES = [ - '**/node_modules/**', - '**/.git/**', - '**/vendor/**', - '**/dist/**', - '**/build/**', - '**/.gitnexus/**', - '**/third_party/**', - '**/3rdparty/**', - '**/external/**', -]; +// 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'; @@ -213,10 +204,34 @@ 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('>'); @@ -252,11 +267,41 @@ function getLanguageForFile(filePath: string): unknown | 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; } @@ -266,24 +311,86 @@ export class IncludeExtractor implements ContractExtractor { repoPath: string, _repo: RepoHandle, ): Promise { - // 1. Build the local file list (for suffix resolution) - const allFiles = await glob('**/*', { - cwd: repoPath, - ignore: STANDARD_IGNORES, - nodir: true, - }); + // 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: find unresolved #include directives - const consumers = await this.extractConsumers(repoPath, normalizedFiles, allFiles, suffixIndex); + // 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( @@ -293,35 +400,53 @@ export class IncludeExtractor implements ContractExtractor { ): Promise { // Strategy A: graph-assisted if (dbExecutor) { - const graphProviders = await this.extractProvidersGraph(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): Promise { + 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`, ); - return rows - .filter((r) => typeof r.filePath === 'string' && r.filePath) - .map((r) => { - const filePath = (r.filePath as string).replace(/\\/g, '/'); - return { - contractId: `include::${normalizeIncludePath(filePath)}`, - type: 'include' as const, - role: 'provider' as const, - symbolUid: String(r.fileId ?? ''), - symbolRef: { filePath, name: path.basename(filePath) }, - symbolName: path.basename(filePath), - confidence: 1.0, - meta: { source: 'graph' }, - }; + // 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 []; } @@ -349,16 +474,9 @@ export class IncludeExtractor implements ContractExtractor { private async extractConsumers( repoPath: string, - normalizedFiles: string[], - allFiles: string[], + sourceFiles: string[], suffixIndex: SuffixIndex, ): Promise { - const sourceFiles = await glob(SOURCE_GLOB, { - cwd: repoPath, - ignore: STANDARD_IGNORES, - nodir: true, - }); - const parser = new Parser(); const out: ExtractedContract[] = []; // Compile the include query once per grammar to avoid re-compilation per file @@ -381,8 +499,11 @@ export class IncludeExtractor implements ContractExtractor { } } - // Collect raw include paths: tree-sitter first, regex fallback for large files + // 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 = parseSourceSafe(parser, content); @@ -393,6 +514,7 @@ export class IncludeExtractor implements ContractExtractor { matches = []; } rawIncludes = []; + extractionSource = 'tree_sitter'; for (const match of matches) { const sourceNode = match.captures.find((c) => c.name === 'import.source'); if (!sourceNode) continue; @@ -402,11 +524,15 @@ export class IncludeExtractor implements ContractExtractor { if (cleaned && cleaned.length <= 2048) rawIncludes.push(cleaned); } } catch { - // tree-sitter failed (e.g. file > 32 KB) — fall back to regex + // 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(content)) !== null) { + while ((m = INCLUDE_REGEX.exec(scanTarget)) !== null) { if (m[1] && m[1].length <= 2048) rawIncludes.push(m[1]); } } @@ -415,10 +541,38 @@ export class IncludeExtractor implements ContractExtractor { // Filter: skip known system headers and system path prefixes if (isSystemHeader(cleaned)) continue; - // Local resolution: try to resolve against this repo's own files - const pathParts = cleaned.split('/').filter(Boolean); - const resolved = suffixResolve(pathParts, normalizedFiles, allFiles, suffixIndex); - if (resolved !== null) continue; // Local include — not cross-repo + // 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, '/'); @@ -431,7 +585,7 @@ export class IncludeExtractor implements ContractExtractor { symbolName: cleaned, confidence: 0.85, meta: { - source: 'tree_sitter', + source: extractionSource, includePath: cleaned, }, }); diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/compound-receiver.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/compound-receiver.ts index 3177336ec..b89551689 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/compound-receiver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/compound-receiver.ts @@ -32,7 +32,7 @@ import { /** Max depth for compound-receiver chain resolution (`a().b().c().d()`). * Practical code rarely exceeds 3-4 hops; the cap prevents * pathological recursion if the receiver text is malformed. */ -const COMPOUND_RECEIVER_MAX_DEPTH = 6; +const COMPOUND_RECEIVER_MAX_DEPTH = 4; const MAP_TUPLE_SENTINEL_RE = /^__MAP_TUPLE_(\d+)__:(.+)$/; diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index cbe105494..5a1edc11f 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -941,9 +941,12 @@ describe('Write access tracking (C++)', () => { const accesses = getRelationships(result, 'ACCESSES'); const writes = accesses.filter((e) => e.rel.reason === 'write'); expect(writes.length).toBe(3); - const fieldNames = writes.map((e) => e.target); - expect(fieldNames).toContain('name'); - expect(fieldNames).toContain('address'); + // Per-field exact counts: both `user.name = ...` and `user.name += ...` + // must produce distinct edges (no dedup); single write to `address`. + const nameWrites = writes.filter((e) => e.target === 'name'); + expect(nameWrites.length).toBe(2); + const addrWrites = writes.filter((e) => e.target === 'address'); + expect(addrWrites.length).toBe(1); const sources = writes.map((e) => e.source); expect(sources).toContain('updateUser'); }); diff --git a/gitnexus/test/unit/group/include-extractor.test.ts b/gitnexus/test/unit/group/include-extractor.test.ts index 7bf71a6f0..321773518 100644 --- a/gitnexus/test/unit/group/include-extractor.test.ts +++ b/gitnexus/test/unit/group/include-extractor.test.ts @@ -1,7 +1,15 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; + +const { parseSourceSafeSpy } = vi.hoisted(() => ({ parseSourceSafeSpy: vi.fn() })); + +vi.mock('../../../src/core/tree-sitter/safe-parse.js', async () => { + const { buildSafeParseMock } = await import('../../helpers/parse-source-safe-mock.js'); + return buildSafeParseMock(parseSourceSafeSpy); +}); + 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'; @@ -196,6 +204,132 @@ int main() { return 0; }`, }); }); + // ---- 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', () => { @@ -233,4 +367,236 @@ int main() { return 0; }`, 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; + } + }); + }); + + describe('Windows SIGSEGV regression — large input must route through parseSourceSafe', () => { + it('routes >32 767-char header file through parseSourceSafe (not direct parser.parse)', async () => { + parseSourceSafeSpy.mockClear(); + + // Bump the file-size cap so the >40 000-char file isn't filtered before + // it ever reaches the parser. Direct parser.parse(content) on a string + // this size SIGSEGVs the process on Windows. The spy assertion catches + // the regression — a "no throw" assertion alone is satisfied by the + // bypass on Linux/macOS where parser.parse(40 000 chars) succeeds. + const previousLimit = process.env.GITNEXUS_MAX_FILE_SIZE; + process.env.GITNEXUS_MAX_FILE_SIZE = '512'; + try { + const includes = Array.from( + { length: 1500 }, + (_, i) => `#include "lib/header_${i}.h"\n`, + ).join(''); + const largeHeader = `#pragma once\n${includes}\nstruct Big {};\n`; + expect(largeHeader.length).toBeGreaterThan(40_000); + + writeFile('big/big.cpp', largeHeader); + + await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + + expect(parseSourceSafeSpy).toHaveBeenCalled(); + } finally { + if (previousLimit === undefined) delete process.env.GITNEXUS_MAX_FILE_SIZE; + else process.env.GITNEXUS_MAX_FILE_SIZE = previousLimit; + } + }); + }); });