fix(group): close PR #1156 Codex adversarial findings

Two HIGH findings from the Codex adversarial review on
feat/group-include-extractor:

1. Default-on extraction silently changes existing groups (BLOCKER)
   DEFAULT_DETECT.includes was true, so any pre-existing group.yaml
   that omits the new field would gain a wave of include::* contracts
   on the next sync after upgrade. Flipped to false (opt-in). The
   integration test already declares includes: true explicitly so it
   survives unchanged; the unit extractor tests bypass parseGroupConfig
   entirely; the sync test uses extractorOverride. Only config-parser
   needed regression tests covering omitted/explicit/false variants.

2. IncludeExtractor scans outside the indexed file universe (BLOCKER)
   The extractor was running glob('**/*', { ignore: STANDARD_IGNORES })
   twice with a hand-rolled 9-pattern list, no .gitignore/.gitnexusignore
   honoring, and no max-file-size cap. That meant File:<path> contracts
   could appear for files ingestion would never index, producing
   cross-links group impact cannot fan out to (silent false-negatives).
   Refactored to a single discoverIndexableFiles() helper that mirrors
   walkRepositoryPaths exactly: createIgnoreFilter + getMaxFileSizeBytes,
   one discovery pass shared by provider and consumer paths. Dropped
   STANDARD_IGNORES and SOURCE_GLOB entirely.

   third_party and 3rdparty (the C/C++ vendored-deps conventions) were
   in the local ignore list but not in the canonical DEFAULT_IGNORE_LIST
   used by ingestion. Folded both into the canonical set rather than
   keep a parallel list — the whole point of the Codex finding is that
   two file-discovery implementations drift. Single source of truth.

Tests: 5 new regression tests for the discovery alignment (.gitignore,
.gitnexusignore, max-file-size on both provider and consumer paths)
plus 4 for the opt-in default. All 30 include-extractor tests + the
494-test group suite + ignore-service tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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',

gitnexus/src/core/group/config-parser.ts

@@ -15,6 +15,15 @@ const VALID_CONTRACT_TYPES: ContractType[] = [
 ];
 const VALID_ROLES: ContractRole[] = ['provider', 'consumer'];
 
+// Defaults matter for backward compatibility: any group.yaml that omits a
+// `detect.<type>` 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,
@@ -22,7 +31,7 @@ const DEFAULT_DETECT = {
   topics: true,
   shared_libs: true,
   embedding_fallback: true,
-  includes: true,
+  includes: false,
   workspace_deps: false,
 };
 

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';
@@ -7,6 +8,8 @@ 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';
 
 /**
  * Cross-repo C/C++ `#include` dependency extractor.
@@ -28,19 +31,7 @@ import { buildSuffixIndex, type SuffixIndex } from '../../ingestion/import-resol
 
 const HEADER_EXTENSIONS = new Set(['.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/**',
-];
+const SOURCE_EXTENSIONS = new Set(['.c', '.cpp', '.cc', '.cxx', '.h', '.hpp', '.hxx', '.hh']);
 
 const INCLUDE_QUERY_SRC = '(preproc_include path: (_) @import.source) @import';
 
@@ -309,24 +300,67 @@ export class IncludeExtractor implements ContractExtractor {
     repoPath: string,
     _repo: RepoHandle,
   ): Promise<ExtractedContract[]> {
-    // 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:<rel>` 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.
+   */
+  private async discoverIndexableFiles(repoPath: string): Promise<string[]> {
+    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 {
+        /* file disappeared between glob and stat — skip */
+      }
+    }
+    return survivors;
+  }
+
   // ---------- provider extraction ----------
 
   private async extractProviders(
@@ -410,16 +444,9 @@ export class IncludeExtractor implements ContractExtractor {
 
   private async extractConsumers(
     repoPath: string,
-    normalizedFiles: string[],
-    allFiles: string[],
+    sourceFiles: string[],
     suffixIndex: SuffixIndex,
   ): Promise<ExtractedContract[]> {
-    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

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

gitnexus/test/unit/group/include-extractor.test.ts

@@ -437,4 +437,89 @@ int main() { return 0; }`,
       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;
+      }
+    });
+  });
 });