From 651a4e15ac8fdb85dcc7e73ab9b4fbf27da46af3 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sat, 9 May 2026 06:49:18 +0100 Subject: [PATCH] fix(group): drop macro-style #include from consumer contracts Tree-sitter's `(_) @import.source` wildcard matches the identifier node of `#include PLATFORM_HEADER`, so the cleaned value `PLATFORM_HEADER` slipped past the system-header / `..` filters and was emitted as a permanently orphaned consumer contract (no file is named after a macro identifier, so no provider can ever match). Add a shape guard that skips cleaned values lacking both a path separator and an extension dot, plus regression tests for single and multi-macro files. Also document `IncludeExtractor.canExtract()` as unused by sync.ts (gated via `config.detect.includes` instead) and kept solely for ContractExtractor interface uniformity. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../group/extractors/include-extractor.ts | 18 +++++++++ .../test/unit/group/include-extractor.test.ts | 38 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/gitnexus/src/core/group/extractors/include-extractor.ts b/gitnexus/src/core/group/extractors/include-extractor.ts index c27212a93..7bbfd61ed 100644 --- a/gitnexus/src/core/group/extractors/include-extractor.ts +++ b/gitnexus/src/core/group/extractors/include-extractor.ts @@ -296,6 +296,11 @@ function isLocalInclude(cleaned: string, suffixIndex: SuffixIndex): boolean { 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; } @@ -544,6 +549,19 @@ export class IncludeExtractor implements ContractExtractor { // 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 diff --git a/gitnexus/test/unit/group/include-extractor.test.ts b/gitnexus/test/unit/group/include-extractor.test.ts index 0a5a11bd3..3956cd5f2 100644 --- a/gitnexus/test/unit/group/include-extractor.test.ts +++ b/gitnexus/test/unit/group/include-extractor.test.ts @@ -398,6 +398,44 @@ int main() { return 0; }`, }); }); + // ---- 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', () => {