mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-20 00:11:37 +00:00
* feat: add IncludeExtractor for C++ cross-repo include tracking (group) * fix: address CodeQL warnings on include-extractor - Remove unused HEADER_GLOB constant in include-extractor.ts - Use fs.mkdtempSync for secure temp dir creation in tests (CodeQL: 'Insecure temporary file') * fix(group): close missing ); in manifest-extractor include branch The 'include' branch in ManifestExtractor.resolveSymbol was missing the closing ); for the executor() call, causing a syntax error that broke ESLint, Prettier, and the full test CI on all platforms. Reported by Claude PR review on #1156. * chore: drop test/global-setup.ts + test/vitest.d.ts Upstream removed these in commit3f0c74fe(ladybugdb 0.16.0 upgrade). Commit3f5d21c5accidentally restored them during a rebase dance. * style(group): reformat VALID_CONTRACT_TYPES array to satisfy prettier Adding 'include' pushed the array over prettier's 100-char limit, so prettier prefers multi-line. Apply the reformat to unbreak ci-quality/format job. * fix(include-extractor): address PR #1156 Claude review findings #3-#7 Claude Deep Review raised 7 findings on the IncludeExtractor. #1/#2 (BLOCKERs) were fixed earlier. This commit closes the remaining five. #3 HIGH case-sensitive FS -> provider contract-id collision Document the deliberate case-folding trade-off on normalizeIncludePath (matches C/C++ convention on Windows/macOS; collapses Foo.h & foo.h on Linux). Add a unit test pinning the behavior. #4 HIGH suffixResolve short-suffix match silently drops cross-repo include When a local file ends with the same basename as an external include (e.g. local internal/api.h vs. #include "ext/api.h"), suffixResolve returned a bogus local hit and suppressed the cross-repo consumer. Replace the suffixResolve lookup inside include-extractor with a strict isLocalInclude() that only accepts full-path hits via SuffixIndex.get / getInsensitive. Callers of suffixResolve elsewhere are unaffected. Add 3 unit tests covering the regression. #5 MEDIUM regex fallback matched #include inside /* ... */ Strip block comments before running the fallback regex scan. Add a unit test. #6 MEDIUM meta.source was hard-coded to 'tree_sitter' Track the actual extraction path with an extractionSource local and write it into meta.source so downstream audits can distinguish tree-sitter parses from regex fallbacks. Add 2 unit tests. #7 MEDIUM missing end-to-end coverage Add test/integration/group/include-extractor-sync.test.ts with 3 cases exercising extractor -> syncGroup -> CrossLink (mocked contracts, mixed-case/backslash normalization, real temp repos). Tests: 21 unit + 3 integration, all green. * fix(lbug): robust Windows lock acquisition for CI integration tests LadybugDB's `new Database()` raises `Could not set lock on file` from local_file_system.cpp synchronously inside the constructor — before any query is issued, so `withLbugDb`'s query-time retry never sees it. On Windows CI this surfaces as flaky integration tests due to AV-scanner holds, libuv handle-release lag, and stale `.wal` sidecars from aborted prior runs. This change closes the gap at *open time*: - `openLbugConnection` now wraps `new lbug.Database()` in a bounded busy-retry (5x100ms back-off) inside `lbug-config.ts`. Errors that exhaust the budget are tagged via `LBUG_OPEN_RETRY_EXHAUSTED` so `withLbugDb`'s outer 3x retry skips re-retrying a freshly-exhausted path (eliminates the 3x5=15-attempt / ~6s tail latency). - For recognized test fixtures only (immediate-parent dir matches a known prefix AND resolves under `os.tmpdir()`), one final stale- sidecar sweep removes `.wal`/`.lock` and retries once. Production paths never enter this branch. - `safeClose` on Windows runs a bounded `fs.open` probe to absorb native handle-release lag; logs a warning if the probe exhausts so operators can spot AV interference. - `isDbBusyError` is now defined in `lbug-config.ts` as the single source of truth, re-exported from `lbug-adapter.ts` for compatibility. - New tests cover open-time retry (happy/retry/exhaust/non-busy/tag), stale-sidecar sweep (test-fixture-only, production-rejection, preserves-original-error), `isTestFixturePath` direct unit suite (accept/reject/traversal/nested/trailing-sep), and `waitForWindowsHandleRelease` (openable/ENOENT/no-leak). - The two new test files are added to vitest's existing serialized `lbug-db` project (already `fileParallelism: false`). Closes the chronic Windows CI flake on lbug-touching integration tests while preserving the existing single-writable-Database-per-process LadybugDB contract. No public API surface changed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(lbug): drop isDbBusyError re-export, import from lbug-config directly The re-export from lbug-adapter.ts was a transitional convenience — with the matcher now living in lbug-config.ts, having two import paths for the same symbol invites future drift. Updated the two real consumers (lbug-lock-retry.test.ts, lbug-open-retry.test.ts) to import from lbug-config directly, removed the re-export equality test (now vacuous), and refreshed the explanatory comment so it no longer references a re-export pattern that doesn't exist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(lbug): silence benign LadybugDB v0.16.1 schema-init lock warnings on Windows doInitLbug logs "⚠️ Schema creation warning: ... Could not set lock on file" on every CREATE NODE TABLE call after the first init on a given dbPath, on Windows. The lock is internal to LadybugDB v0.16.1 and is resolved before the table is created — same tolerance pattern as the existing "already exists" filter. Genuine cross-process lock contention still surfaces on the next operation through withLbugDb's retry, so filtering at the schema-init catch only suppresses noise, not signal. Also extend the safeClose Windows handle-release probe to cover the .wal sidecar (the previous Database's WAL handle was the slowest to release, surfacing as the schema-query lock contention) and switch the probe back to 'r+' so it actually detects exclusive locks. Test loop in lbug-close-handle-release.test.ts simplified to 10 plain iterations now that the underlying noise is filtered upstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(lbug): isDbBusyError review fixes - Drop redundant `could not set lock` term — already subsumed by `lock`. - Document the intentionally-broad matcher: graph-DB lock-shaped errors ("deadlock", "unlock failed", "lock contention", "could not open lock file") are all treated as transient. If a non-transient surfaces, tighten the matcher rather than raise the retry budget. - Add positive test cases covering those lock-shaped strings so the intent is visible and a future tightening would deliberately break these. - Fix the open-retry back-off comment: max sleep is 100+200+300+400 = 1000ms (no sleep after the final attempt), not 1.5s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(group): address PR #1156 follow-up review findings Addresses two blockers and two mediums from the deep review. BLOCKER 1: Windows CI ENOTEMPTY in sync.test.ts After this PR added writeBridge() to syncGroup, the existing test "writes registry to groupDir when skipWrite is false" fails on windows-latest. LadybugDB's checkpoint thread briefly outlives closeBridgeDb, holding a Win32 lock on bridge.lbug; the test's fs.rmSync then fails with ENOTEMPTY. Switched the test cleanup to cleanupTempDir from test/helpers/test-db.ts which already tolerates EBUSY/EPERM/EACCES/ENOTEMPTY with bounded retries — same pattern used elsewhere for LadybugDB-touching tests. BLOCKER 2: Graph provider absolute-path bug extractProvidersGraph queried File.filePath from the LadybugDB graph but never stripped the repo root, so provider contract IDs ended up as include::/abs/path/foo.h while consumers emitted include::foo.h. These never matched through runExactMatch — silently producing 0 cross-links for any indexed C++ repo (the primary use case). Now passes repoPath into extractProvidersGraph and applies path.relative(); rows that resolve outside repoPath (stale absolute paths from another machine, system headers somehow indexed) are dropped instead of polluting the registry. MEDIUM: `../` relative includes produce spurious noise `#include "../foo.h"` is almost always intra-repo, but the suffix index can never match a `..`-prefixed path so it became a consumer contract no provider could satisfy. Now skipped before matching; covers both forward-slash and backslash forms. MEDIUM: writeBridge error in sync.ts propagates uncaught contracts.json is the canonical source of truth and was just written successfully when writeBridge runs. A bridge-only failure (disk full, schema error, permission denied) shouldn't mask the registry. Wrapped writeBridge in try/catch with a logger.warn surfacing the path and recovery instructions. Tests added: - extractProvidersGraph repo-relative ID generation (stub Cypher executor returns absolute paths) - extractProvidersGraph drops rows whose path resolves outside repo - `../foo.h` forward-slash skip - `..\foo.h` backslash-form skip Skipped findings: - canExtract() removal (#5, low): canExtract is part of the ContractExtractor interface; every other extractor implements the same `return true` shape. Removing it from IncludeExtractor would break the interface contract — keeping for consistency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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> * fix(review): apply autofix feedback ce-code-review surfaced 6 safe_auto findings on commita9936a9b: - T1 (testing, P2): the sync.ts:174 gate was untested with includes:false. Added a sync-level test mirroring the existing thrift-off pattern at sync.test.ts:545, asserting zero include contracts when the gate is disabled in a real syncGroup call. - T3 (testing, P3): third_party and 3rdparty entries in DEFAULT_IGNORE_LIST had no regression test. Added both to ignore-service.test.ts's dependency-directories it.each block. - M1 (maintainability, P3): discoverIndexableFiles JSDoc lacked a fork-warning relative to walkRepositoryPaths. Added a MAINTENANCE note explaining why the duplication is tolerated and the contract the two implementations must keep. - M2 (maintainability, P3): thrift-extractor still hand-rolls its ignore array with no signal that DEFAULT_IGNORE_LIST additions silently do not apply there. Added TODO(#1156-followup) comments above both call sites. - M3 (maintainability, P3): SOURCE_EXTENSIONS duplicated the four HEADER_EXTENSIONS entries with no expressed subset relationship. Spread HEADER_EXTENSIONS into SOURCE_EXTENSIONS so future header- extension additions propagate. - C1+T4 (correctness+testing, P3, cross-reviewer corroborated): discoverIndexableFiles swallowed all fs.stat errors silently, including EACCES/EMFILE/EIO. Narrowed the catch to ENOENT (the documented benign glob/stat race) and added a logger.warn for any other code so operators can spot permission/resource issues. All 629 tests pass; typecheck + prettier clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(group): use retryRename in writeContractRegistry to absorb Windows EPERM `storage.ts:62` used raw `fsp.rename` for the contracts.json atomic swap. On Windows, AV scanners and concurrent renames briefly hold the destination handle between rename calls, surfacing as EPERM/EBUSY. The `insecure-tempfile.test.ts > concurrent writes do not collide` test was flaking with `EPERM: operation not permitted, rename` on windows-latest CI. `bridge-db.ts` already has a battle-tested `retryRename(src, dst, 3)` helper used at six call sites for exactly this pattern. Reusing it here keeps the Windows-rename policy single-source-of-truth across the group package. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: HuangWenjie <zhoudeng.hwj@alibaba-inc.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
563 lines
23 KiB
TypeScript
563 lines
23 KiB
TypeScript
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<class T> 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 <stdio.h>
|
|
#include <vector>
|
|
#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;
|
|
}
|
|
});
|
|
});
|
|
});
|