From 59acfb226139d97307d3e1b5af06392cb951cc96 Mon Sep 17 00:00:00 2001 From: jelsco <58397194+jelsco@users.noreply.github.com> Date: Fri, 1 May 2026 09:27:07 -0600 Subject: [PATCH 01/10] fix(python): walk ancestors for multi-segment dotted imports (#1241) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(python): walk ancestors for multi-segment dotted imports (#1240) Single-segment Python imports (`from middleware import X`) already get an ancestor-directory walk in `resolvePythonImportInternal`, so they resolve correctly when the importer and the imported module share a parent directory (e.g. both under `backend/`). Multi-segment dotted imports (`from services.sync import X`) were only resolved against the workspace root. In a `backend/`-prefixed repo, `from services.sync import X` from `backend/routers/cron.py` would not resolve because `services/sync.py` does not exist at the workspace root — only `backend/services/sync.py` does. The IMPORTS edge was dropped, the imported names were never bound, and downstream CALLS edges to those names were silently lost. The fix mirrors the single-segment ancestor walk for multi-segment paths in `resolveAbsoluteFromFiles`, and widens `hasRepoCandidate` to accept nested `/segment/` matches so it does not bail before the walk runs. Includes a new fixture and 5 integration tests covering: - IMPORTS resolution for `from services.sync`, `from services.alerts`, `from routers.alerts` from `backend/routers/cron.py`. - CALLS edge counts for every multi-segment-imported callee. - Regression check: single-segment ancestor walk (`from auth_utils import …`) still resolves correctly. The django-app-imports regression suite (which prevents `accounts.apps` from spuriously matching a local `apps.py`) continues to pass — the new nested-namespace check in `hasRepoCandidate` is bounded by an explicit `/segment/` substring, and the workspace-root candidate check still runs first. * fix(python): scope hasRepoCandidate widening to importer ancestors + tighten ancestor-walk loop Address review findings from PR #1241: 1. hasRepoCandidate's nested check now requires the matching directory to sit on an ancestor of the importer. Previously any nested /SEGMENT/ path satisfied the gate, which would let a vendored copy of an external package (e.g. vendor/django/urls.py) gate-pass an external import like 'from django.urls import path' issued from app/main.py. 2. Loop bound in resolveAbsoluteFromFiles tightened from 'i >= 0' to 'i > 0' to skip a redundant root-candidate recheck (the workspace-root direct check above already covers that case). 3. Doc-comment in resolveAbsoluteFromFiles now states the precedence order explicitly: workspace root > closest ancestor > suffix fallback. Tests added: - Vendored-external false-positive guard (vendor/django/urls.py must not resolve from app/main.py). - Workspace-root vs ancestor precedence (root services/sync.py wins over backend/services/sync.py for a backend/routers/cron.py importer). 215/215 python integration tests pass (+4 from this change). tsc --noEmit green. --- .../languages/python/import-target.ts | 117 ++++++++-- .../backend/auth_utils.py | 6 + .../backend/routers/__init__.py | 0 .../backend/routers/alerts.py | 2 + .../backend/routers/cron.py | 24 ++ .../backend/services/__init__.py | 0 .../backend/services/alerts.py | 2 + .../backend/services/sync.py | 6 + .../test/integration/resolvers/python.test.ts | 207 ++++++++++++++++++ 9 files changed, 344 insertions(+), 20 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/auth_utils.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/__init__.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/alerts.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/cron.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/__init__.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/alerts.py create mode 100644 gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/sync.py diff --git a/gitnexus/src/core/ingestion/languages/python/import-target.ts b/gitnexus/src/core/ingestion/languages/python/import-target.ts index 3905f3301..677372966 100644 --- a/gitnexus/src/core/ingestion/languages/python/import-target.ts +++ b/gitnexus/src/core/ingestion/languages/python/import-target.ts @@ -61,37 +61,81 @@ export function resolvePythonImportTarget( const pathLike = parsedImport.targetRaw.replace(/\./g, '/'); if (pathLike.includes('/')) { const [leadingSegment] = pathLike.split('/').filter(Boolean); - if (!leadingSegment || !hasRepoCandidate(leadingSegment, ctx.allFilePaths)) { + if (!leadingSegment || !hasRepoCandidate(leadingSegment, ctx.allFilePaths, ctx.fromFile)) { return null; } } - // Multi-segment absolute resolve: try exact paths first, then suffix - // match in nested repos. Using direct `Set.has` + `endsWith` instead of - // `suffixResolve`'s shared helper because that helper requires a - // pre-built `SuffixIndex` to disambiguate ties — without one it falls - // back to an O(files) scan that silently picks the wrong file when - // the last segment collides across directories (e.g. `accounts.models` - // matching `billing/models.py` when both files exist). - return resolveAbsoluteFromFiles(pathLike, ctx.allFilePaths); + // Multi-segment absolute resolve: try exact paths first, then ancestor + // walk (mirrors the single-segment ancestor walk in + // `resolvePythonImportInternal`), then a suffix match in nested repos. + // Using direct `Set.has` + `endsWith` instead of `suffixResolve`'s shared + // helper because that helper requires a pre-built `SuffixIndex` to + // disambiguate ties — without one it falls back to an O(files) scan that + // silently picks the wrong file when the last segment collides across + // directories (e.g. `accounts.models` matching `billing/models.py` when + // both files exist). + return resolveAbsoluteFromFiles(pathLike, ctx.allFilePaths, ctx.fromFile); } /** * Resolve `package/sub/module` style paths (already dot-flattened) to a - * concrete file in `allFilePaths`. Tries the exact path first, then the - * `__init__.py` variant, then a suffix match for nested layouts. + * concrete file in `allFilePaths`. Tries the exact path first, then walks + * ancestors of `fromFile` looking for `/.py` (or + * `__init__.py`), then falls back to a suffix match for nested layouts. * Returns the original (un-normalized) path from the set. + * + * Precedence order: + * 1. Workspace-root direct hit (`.py`, `/__init__.py`). + * 2. Closest-ancestor match walking up from the importer's directory. + * 3. Suffix fallback (first match). + * + * Root wins over ancestor by construction — if both `services/sync.py` and + * `backend/services/sync.py` exist, `backend/routers/cron.py`'s + * `from services.sync import X` resolves to the root file. This mirrors + * Python's `sys.path` semantics where the project root is searched first. + * + * The ancestor walk mirrors the single-segment behavior in + * `resolvePythonImportInternal`. For `from services.sync import X` in + * `backend/routers/cron.py`, walk up: `backend/routers/services/sync.py` → + * `backend/services/sync.py` ✓. */ -function resolveAbsoluteFromFiles(pathLike: string, allFilePaths: Set): string | null { +function resolveAbsoluteFromFiles( + pathLike: string, + allFilePaths: Set, + fromFile: string, +): string | null { const directFile = `${pathLike}.py`; const directPkg = `${pathLike}/__init__.py`; + + // Direct hit at workspace root. + if (allFilePaths.has(directFile)) return directFile; + if (allFilePaths.has(directPkg)) return directPkg; + + // Ancestor walk — match the single-segment resolver's behavior at + // multi-segment granularity. Closest match wins. Stop at `i > 0` because + // `i === 0` would re-check the workspace-root candidates already covered + // by the direct check above. + const importerDir = fromFile.replace(/\\/g, '/').split('/').slice(0, -1).join('/'); + if (importerDir) { + const dirParts = importerDir.split('/').filter(Boolean); + for (let i = dirParts.length; i > 0; i--) { + const ancestor = dirParts.slice(0, i).join('/'); + const prefix = `${ancestor}/`; + const candidateFile = `${prefix}${directFile}`; + const candidatePkg = `${prefix}${directPkg}`; + if (allFilePaths.has(candidateFile)) return candidateFile; + if (allFilePaths.has(candidatePkg)) return candidatePkg; + } + } + + // Existing suffix-match fallback (preserved for monorepo/nested-repo + // layouts that don't share a directory ancestor with the importer). const suffixFile = `/${directFile}`; const suffixPkg = `/${directPkg}`; - let suffixMatch: string | null = null; for (const raw of allFilePaths) { const f = raw.replace(/\\/g, '/'); - if (f === directFile || f === directPkg) return raw; if (suffixMatch === null && (f.endsWith(suffixFile) || f.endsWith(suffixPkg))) { suffixMatch = raw; } @@ -100,21 +144,54 @@ function resolveAbsoluteFromFiles(pathLike: string, allFilePaths: Set): } /** - * Does the repo contain a module/package named `leadingSegment` at the top - * level? Used to guard against false-positive suffix matches on external - * dotted imports (e.g. `django.apps` matching a local `accounts/apps.py`). + * Does the repo contain a module/package named `leadingSegment` somewhere + * the importer can plausibly reach? * - * Checks, in order: `.py` root file, `/__init__.py` - * regular package, or any `/**.py` file (namespace package). + * Used to guard against false-positive suffix matches on external dotted + * imports (e.g. `django.apps` matching a local `accounts/apps.py`). + * + * Checks, in order: + * 1. `SEGMENT.py` root file or `SEGMENT/__init__.py` regular package. + * 2. Any `SEGMENT/...py` file at the workspace root (namespace package). + * 3. Any `/SEGMENT/...py` file (nested namespace + * package the importer could reach via an ancestor walk, e.g. + * `backend/services/sync.py` from `backend/routers/cron.py`). + * + * The nested case is bounded to the importer's own ancestors so a + * vendored copy of an external package (e.g. `vendor/django/urls.py`) + * does not gate-pass external imports like `from django.urls import path` + * issued from `app/main.py`. Files inside the vendored tree itself + * (importer under `vendor/django/...`) still resolve correctly because + * the ancestor walk includes their own parents. */ -function hasRepoCandidate(leadingSegment: string, allFilePaths: Set): boolean { +function hasRepoCandidate( + leadingSegment: string, + allFilePaths: Set, + fromFile: string, +): boolean { const prefix = `${leadingSegment}/`; const rootFile = `${leadingSegment}.py`; const initFile = `${leadingSegment}/__init__.py`; + + // Build importer-ancestor prefixes: for `backend/routers/cron.py`, + // produces `["backend/routers/services/", "backend/services/"]` for + // segment `services` (closest first, root excluded — covered above). + const importerDir = fromFile.replace(/\\/g, '/').split('/').slice(0, -1).join('/'); + const dirParts = importerDir ? importerDir.split('/').filter(Boolean) : []; + const ancestorPrefixes: string[] = []; + for (let i = dirParts.length; i > 0; i--) { + ancestorPrefixes.push(`${dirParts.slice(0, i).join('/')}/${leadingSegment}/`); + } + for (const raw of allFilePaths) { const f = raw.replace(/\\/g, '/'); if (f === rootFile || f === initFile) return true; if (f.startsWith(prefix) && f.endsWith('.py')) return true; + if (f.endsWith('.py')) { + for (const ap of ancestorPrefixes) { + if (f.startsWith(ap)) return true; + } + } } return false; } diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/auth_utils.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/auth_utils.py new file mode 100644 index 000000000..c384c28c6 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/auth_utils.py @@ -0,0 +1,6 @@ +def verify_cron_secret(token): + return token == "expected" + + +def get_org_id_from_header(headers): + return headers.get("x-org-id") if headers else None diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/__init__.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/alerts.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/alerts.py new file mode 100644 index 000000000..997a75a83 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/alerts.py @@ -0,0 +1,2 @@ +def send_daily_alerts(): + return "sent" diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/cron.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/cron.py new file mode 100644 index 000000000..f57dcf73c --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/routers/cron.py @@ -0,0 +1,24 @@ +from auth_utils import verify_cron_secret, get_org_id_from_header +from services.sync import _start_cron_run, _complete_cron_run +from services.alerts import _create_ops_alert +from routers.alerts import send_daily_alerts + + +def handler_a(): + if verify_cron_secret("x"): + org = get_org_id_from_header(None) + _start_cron_run("a") + _create_ops_alert("a") + send_daily_alerts() + _complete_cron_run("a") + return org + + +def handler_b(): + _start_cron_run("b") + _create_ops_alert("b") + send_daily_alerts() + + +def handler_c(): + _start_cron_run("c") diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/__init__.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/alerts.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/alerts.py new file mode 100644 index 000000000..92229fc68 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/alerts.py @@ -0,0 +1,2 @@ +def _create_ops_alert(name): + return f"alert:{name}" diff --git a/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/sync.py b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/sync.py new file mode 100644 index 000000000..ea3f1a77f --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/python-multi-segment-ancestor-import/backend/services/sync.py @@ -0,0 +1,6 @@ +def _start_cron_run(name): + return f"started:{name}" + + +def _complete_cron_run(name): + return f"completed:{name}" diff --git a/gitnexus/test/integration/resolvers/python.test.ts b/gitnexus/test/integration/resolvers/python.test.ts index 817483849..dd0104a5c 100644 --- a/gitnexus/test/integration/resolvers/python.test.ts +++ b/gitnexus/test/integration/resolvers/python.test.ts @@ -416,6 +416,213 @@ describe('Python ancestor directory import resolution (Issue #417)', () => { }); }); +// --------------------------------------------------------------------------- +// Multi-segment ancestor walk: `from services.sync import X` style imports +// from a sibling sub-package nested under a shared root directory. +// +// Before this fix, single-segment ancestor walks worked (`from middleware +// import X` from `backend/services/auth.py` → `backend/middleware.py`) but +// multi-segment dotted imports were only resolved against the workspace +// root. In a `backend/`-prefixed repo, `from services.sync import X` from +// `backend/routers/cron.py` would silently drop because `services/sync.py` +// does not exist at the workspace root — only `backend/services/sync.py` +// does. The fix mirrors the single-segment ancestor walk for multi-segment +// paths. +// --------------------------------------------------------------------------- + +describe('Python multi-segment ancestor directory import resolution', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'python-multi-segment-ancestor-import'), + () => {}, + ); + }, 60000); + + it('resolves from services.sync import to backend/services/sync.py via ancestor walk', () => { + const imports = getRelationships(result, 'IMPORTS'); + const syncImport = imports.find( + (i) => + i.sourceFilePath === 'backend/routers/cron.py' && + i.targetFilePath === 'backend/services/sync.py', + ); + expect(syncImport).toBeDefined(); + }); + + it('resolves from services.alerts import to backend/services/alerts.py via ancestor walk', () => { + const imports = getRelationships(result, 'IMPORTS'); + const alertsImport = imports.find( + (i) => + i.sourceFilePath === 'backend/routers/cron.py' && + i.targetFilePath === 'backend/services/alerts.py', + ); + expect(alertsImport).toBeDefined(); + }); + + it('resolves from routers.alerts import to backend/routers/alerts.py (sibling sub-package)', () => { + const imports = getRelationships(result, 'IMPORTS'); + const routerImport = imports.find( + (i) => + i.sourceFilePath === 'backend/routers/cron.py' && + i.targetFilePath === 'backend/routers/alerts.py', + ); + expect(routerImport).toBeDefined(); + }); + + it('emits CALLS edges for every multi-segment-imported callee', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'backend/routers/cron.py', + ); + + const startCronRunCalls = calls.filter((c) => c.target === '_start_cron_run'); + expect(startCronRunCalls.length).toBe(3); + expect(startCronRunCalls.every((c) => c.targetFilePath === 'backend/services/sync.py')).toBe( + true, + ); + + const completeCronRunCalls = calls.filter((c) => c.target === '_complete_cron_run'); + expect(completeCronRunCalls.length).toBe(1); + expect(completeCronRunCalls[0].targetFilePath).toBe('backend/services/sync.py'); + + const opsAlertCalls = calls.filter((c) => c.target === '_create_ops_alert'); + expect(opsAlertCalls.length).toBe(2); + expect(opsAlertCalls.every((c) => c.targetFilePath === 'backend/services/alerts.py')).toBe( + true, + ); + + const sendDailyCalls = calls.filter((c) => c.target === 'send_daily_alerts'); + expect(sendDailyCalls.length).toBe(2); + expect(sendDailyCalls.every((c) => c.targetFilePath === 'backend/routers/alerts.py')).toBe( + true, + ); + }); + + it('preserves single-segment ancestor walk (regression check for from auth_utils import X)', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'backend/routers/cron.py', + ); + + const verifyCalls = calls.filter((c) => c.target === 'verify_cron_secret'); + expect(verifyCalls.length).toBe(1); + expect(verifyCalls[0].targetFilePath).toBe('backend/auth_utils.py'); + + const orgCalls = calls.filter((c) => c.target === 'get_org_id_from_header'); + expect(orgCalls.length).toBe(1); + expect(orgCalls[0].targetFilePath).toBe('backend/auth_utils.py'); + }); +}); + +// --------------------------------------------------------------------------- +// Negative case for `hasRepoCandidate` widening: a vendored copy of an +// external package (e.g. `vendor/django/urls.py`) must not cause an external +// import like `from django.urls import path` issued from an unrelated file +// (`app/main.py`) to be treated as a local candidate. The ancestor-bounded +// nested check rejects vendored matches that don't sit on the importer's +// own ancestor path. +// --------------------------------------------------------------------------- + +describe('Python multi-segment widening: vendored external package false-positive guard', () => { + let repoDir: string; + let result: PipelineResult; + + beforeAll(async () => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-vendored-django-')); + writeFixtureRepo(repoDir, { + 'app/main.py': `from django.urls import path + +def boot(): + path("/") +`, + 'vendor/django/__init__.py': '', + 'vendor/django/urls.py': `def path(p): + return p +`, + }); + result = await runPipelineFromRepo(repoDir, () => {}); + }, 60000); + + afterAll(() => { + if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true }); + }); + + it('does not resolve from django.urls to vendor/django/urls.py from an unrelated importer', () => { + const imports = getRelationships(result, 'IMPORTS'); + const stray = imports.find( + (i) => i.sourceFilePath === 'app/main.py' && i.targetFilePath === 'vendor/django/urls.py', + ); + expect(stray).toBeUndefined(); + }); + + it('does not emit a CALLS edge from app/main.py:boot to vendor/django/urls.py:path', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'app/main.py', + ); + const stray = calls.find( + (c) => c.target === 'path' && c.targetFilePath === 'vendor/django/urls.py', + ); + expect(stray).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// Workspace-root precedence: when both `services/sync.py` (root) and +// `backend/services/sync.py` (ancestor) exist, an importer at +// `backend/routers/cron.py` doing `from services.sync import X` resolves to +// the root file. Mirrors Python's `sys.path` semantics where the project +// root is searched before package-local namespaces. +// --------------------------------------------------------------------------- + +describe('Python multi-segment resolution: workspace root wins over ancestor', () => { + let repoDir: string; + let result: PipelineResult; + + beforeAll(async () => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-root-precedence-')); + writeFixtureRepo(repoDir, { + 'services/__init__.py': '', + 'services/sync.py': `def root_marker(): + return "root" +`, + 'backend/__init__.py': '', + 'backend/services/__init__.py': '', + 'backend/services/sync.py': `def ancestor_marker(): + return "ancestor" +`, + 'backend/routers/__init__.py': '', + 'backend/routers/cron.py': `from services.sync import root_marker + +def handler(): + return root_marker() +`, + }); + result = await runPipelineFromRepo(repoDir, () => {}); + }, 60000); + + afterAll(() => { + if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true }); + }); + + it('resolves the import edge to the root services/sync.py, not backend/services/sync.py', () => { + const imports = getRelationships(result, 'IMPORTS').filter( + (i) => i.sourceFilePath === 'backend/routers/cron.py', + ); + const rootEdge = imports.find((i) => i.targetFilePath === 'services/sync.py'); + expect(rootEdge).toBeDefined(); + + const ancestorEdge = imports.find((i) => i.targetFilePath === 'backend/services/sync.py'); + expect(ancestorEdge).toBeUndefined(); + }); + + it('binds the imported name to the root file, not the ancestor copy', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'backend/routers/cron.py' && c.target === 'root_marker', + ); + expect(calls.length).toBe(1); + expect(calls[0].targetFilePath).toBe('services/sync.py'); + }); +}); + // --------------------------------------------------------------------------- // Re-export chain: from .base import X barrel pattern via __init__.py // --------------------------------------------------------------------------- From 4be4abe8e40d99c3851801c2e0da10b8ac0c6fa1 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Fri, 1 May 2026 16:42:21 +0100 Subject: [PATCH 02/10] fix(group): contract extractors honour .gitnexusignore via shared IgnoreService (#1185) (#1247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(group): contract extractors honour .gitnexusignore via shared IgnoreService (#1185) The HTTP, gRPC, and topic contract extractors each globbed the repo with a hardcoded `ignore: ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/build/**', '**/vendor/**']` array, bypassing the shared `IgnoreService` that the rest of the ingestion pipeline uses for `.gitnexusignore` and `.gitignore` parsing. Result: a vendored Python venv (`mentor_env/`), generated stubs, or any user-defined exclusion silently produced false-positive contracts. Replace each hardcoded array with `createIgnoreFilter(repoPath)`, mirroring the canonical pattern in `filesystem-walker.ts`. The 5 hardcoded names are all in `DEFAULT_IGNORE_LIST`, so default behaviour is preserved; users now also get `.gitnexusignore` patterns, the rest of the hardcoded list (e.g. `__pycache__`, `.pytest_cache`), and the `.gitnexusignore` negation semantics introduced in #771. The topic extractor additionally filters Go `*_test.go` at the glob level. That filter is preserved via a small wrapper around `createIgnoreFilter` that short-circuits before delegating, so glob-level pruning still applies and the existing `_test.go` skip test (with new content asserting the pruning is real) still passes. Tests added to all three `*-extractor.test.ts` files exercising `.gitnexusignore` honouring end-to-end via real temp directories. * test(group): exercise gRPC source-scan ignore + add .gitignore-only coverage (#1185) Addresses two findings from the @claude review on PR #1247: [medium] The gRPC ignore test claimed to cover both proto-context and source-scan paths but only wrote a .proto file under mentor_env/. Added a Python `_pb2_grpc.Stub(channel)` consumer file under the same ignored dir (mirroring the canonical pattern from `test_extract_python_stub_returns_consumer`); without the `.gitnexusignore` filter that file would emit a consumer contract. The test now exercises both `createIgnoreFilter` calls inside the gRPC extractor (`buildProtoContext` + `extract`) in a single run, with both defence-in-depth path-prefix assertions and a specific `role: consumer` LeakedService assertion. [low] Added one shared .gitignore-only test on the HTTP extractor. `createIgnoreFilter` reads both `.gitignore` and `.gitnexusignore` via `loadIgnoreRules`, but no extractor-level test exercised the `.gitignore` path. One shared test is sufficient because all three extractors consume the same filter object — verified at `IgnoreService` level already. The remaining [low] finding — "negation semantics (!pattern) not tested at extractor level" — is deferred deliberately, not skipped. Three reasons: 1. The negation logic (introduced in #771) lives entirely inside `createIgnoreFilter`'s `hasExplicitUnignore` ancestor-walk in `ignore-service.ts`. The extractors only consume the returned filter object — they never inspect patterns, never call `hasExplicitUnignore` directly, and have no code path that could diverge from the IgnoreService's negation behaviour. 2. Negation is already locked in by 8 dedicated unit tests in `test/unit/ignore-service.test.ts` (the #771 suite), plus the `!parent/` + `parent/child/` last-match-wins regression test added in PR #1046. An extractor-level negation test would re-prove the same code path and would not catch any failure mode the existing tests don't already catch. 3. The bot itself flagged the gap as "Acceptable to leave as follow-up referencing existing IgnoreService negation tests" — the deferral matches its own recommendation. If a future change inserts an extractor-side wrapper around the filter (as topic-extractor.ts already does for `*_test.go`) that could plausibly affect negation, an extractor-level negation test should be added at that point — not pre-emptively here. --- .../core/group/extractors/grpc-extractor.ts | 15 +++- .../group/extractors/http-route-extractor.ts | 10 ++- .../core/group/extractors/topic-extractor.ts | 28 +++--- .../test/unit/group/grpc-extractor.test.ts | 63 ++++++++++++++ .../unit/group/http-route-extractor.test.ts | 86 +++++++++++++++++++ .../test/unit/group/topic-extractor.test.ts | 55 ++++++++++++ 6 files changed, 240 insertions(+), 17 deletions(-) diff --git a/gitnexus/src/core/group/extractors/grpc-extractor.ts b/gitnexus/src/core/group/extractors/grpc-extractor.ts index b379a4dbd..b5782d9b3 100644 --- a/gitnexus/src/core/group/extractors/grpc-extractor.ts +++ b/gitnexus/src/core/group/extractors/grpc-extractor.ts @@ -1,6 +1,7 @@ import * as path from 'node:path'; import { glob } from 'glob'; import Parser from 'tree-sitter'; +import { createIgnoreFilter } from '../../../config/ignore-service.js'; import type { ContractExtractor, CypherExecutor } from '../contract-extractor.js'; import type { ExtractedContract, RepoHandle } from '../types.js'; import { readSafe } from './fs-utils.js'; @@ -227,11 +228,16 @@ async function buildProtoContext(repoPath: string): Promise<{ servicesByName: Map; }> { const servicesByName = new Map(); + // `.gitnexusignore` / `.gitignore` honoured via the shared IgnoreService — + // see `filesystem-walker.ts` for the canonical pattern. Replaces a + // hardcoded `[node_modules, .git, vendor]` array; those names plus the + // rest of `DEFAULT_IGNORE_LIST` are still excluded by default (#1185). + const protoIgnoreFilter = await createIgnoreFilter(repoPath); const protoFiles = await glob('**/*.proto', { cwd: repoPath, absolute: false, nodir: true, - ignore: ['**/node_modules/**', '**/.git/**', '**/vendor/**'], + ignore: protoIgnoreFilter, }); const contents = new Map(); @@ -401,9 +407,14 @@ export class GrpcExtractor implements ContractExtractor { } // ─── Source files (+ .proto when plugin available) ──────────── + // Honour `.gitnexusignore` / `.gitignore` via the shared IgnoreService — + // mirrors `filesystem-walker.ts`. Replaces a hardcoded + // `[node_modules, .git, vendor, dist, build]` array; those names are all + // in `DEFAULT_IGNORE_LIST`, so default behaviour is preserved (#1185). + const sourceIgnoreFilter = await createIgnoreFilter(repoPath); const sourceFiles = await glob(GRPC_SCAN_GLOB, { cwd: repoPath, - ignore: ['**/node_modules/**', '**/.git/**', '**/vendor/**', '**/dist/**', '**/build/**'], + ignore: sourceIgnoreFilter, nodir: true, }); diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index f2914613d..d989876a8 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -1,6 +1,7 @@ import * as path from 'node:path'; import { glob } from 'glob'; import Parser from 'tree-sitter'; +import { createIgnoreFilter } from '../../../config/ignore-service.js'; import type { ContractExtractor, CypherExecutor } from '../contract-extractor.js'; import type { ExtractedContract, RepoHandle } from '../types.js'; import { readSafe } from './fs-utils.js'; @@ -208,9 +209,16 @@ export class HttpRouteExtractor implements ContractExtractor { } private async scanFiles(repoPath: string): Promise { + // Honour `.gitnexusignore` and `.gitignore` via the shared IgnoreService + // so contract extraction respects the same exclusion rules as the rest of + // the ingestion pipeline. Mirrors `filesystem-walker.ts` which uses the + // same shape. Replaces a hardcoded `[node_modules, .git, dist, build, + // vendor]` array — those names are still in `DEFAULT_IGNORE_LIST`, so + // default behaviour is preserved (#1185). + const ignoreFilter = await createIgnoreFilter(repoPath); return glob(HTTP_SCAN_GLOB, { cwd: repoPath, - ignore: ['**/node_modules/**', '**/.git/**', '**/dist/**', '**/build/**', '**/vendor/**'], + ignore: ignoreFilter, nodir: true, }); } diff --git a/gitnexus/src/core/group/extractors/topic-extractor.ts b/gitnexus/src/core/group/extractors/topic-extractor.ts index 1fbccac8a..4f2128a1c 100644 --- a/gitnexus/src/core/group/extractors/topic-extractor.ts +++ b/gitnexus/src/core/group/extractors/topic-extractor.ts @@ -1,5 +1,6 @@ import { glob } from 'glob'; import Parser from 'tree-sitter'; +import { createIgnoreFilter } from '../../../config/ignore-service.js'; import type { ContractExtractor, CypherExecutor } from '../contract-extractor.js'; import type { ExtractedContract, RepoHandle } from '../types.js'; import { readSafe } from './fs-utils.js'; @@ -56,22 +57,21 @@ export class TopicExtractor implements ContractExtractor { repoPath: string, _repo: RepoHandle, ): Promise { + // Honour `.gitnexusignore` / `.gitignore` via the shared IgnoreService — + // mirrors `filesystem-walker.ts`. The 5-name hardcoded list + // (`node_modules, .git, vendor, dist, build`) is preserved because every + // entry is in `DEFAULT_IGNORE_LIST`, so default behaviour is unchanged + // (#1185). The Go-specific `**/*_test.go` filter is layered on top via a + // small wrapper so glob-level pruning is preserved (we never read those + // files); the wrapper short-circuits before calling the base filter. + const baseFilter = await createIgnoreFilter(repoPath); + const ignoreFilter: typeof baseFilter = { + ignored: (p) => p.relative().endsWith('_test.go') || baseFilter.ignored(p), + childrenIgnored: (p) => baseFilter.childrenIgnored(p), + }; const files = await glob(TOPIC_SCAN_GLOB, { cwd: repoPath, - ignore: [ - '**/node_modules/**', - '**/.git/**', - '**/vendor/**', - '**/dist/**', - '**/build/**', - // Language-level test file conventions. Go test files - // `*_test.go` live next to source; other languages either use - // separate test directories (Python's `tests/`, Java's - // `src/test/`) or are already covered by the dist/build ignores. - // Pushed to the glob level so the orchestrator stays - // language-agnostic. - '**/*_test.go', - ], + ignore: ignoreFilter, nodir: true, }); diff --git a/gitnexus/test/unit/group/grpc-extractor.test.ts b/gitnexus/test/unit/group/grpc-extractor.test.ts index 1664bd1a7..a586b5c0d 100644 --- a/gitnexus/test/unit/group/grpc-extractor.test.ts +++ b/gitnexus/test/unit/group/grpc-extractor.test.ts @@ -613,6 +613,69 @@ export class AuthGateway { expect(contracts).toHaveLength(0); }); }); + + // ─── #1185: gRPC extractor must honour .gitnexusignore ────────────── + // + // Both the `.proto` glob (in `buildProtoContext`) and the source-scan + // glob (in `extract`) used a hardcoded ignore array that bypassed + // `IgnoreService`. Both globs now consume the shared filter (mirrors + // `filesystem-walker.ts`) so any `.gitnexusignore` pattern is + // honoured. The single test below exercises BOTH paths in the same + // run: a `.proto` under `mentor_env/` (proto-context build) AND a + // Python `_pb2_grpc.Stub` consumer under `mentor_env/` + // (source-scan path) — neither produces a contract. + describe('respects .gitnexusignore (#1185)', () => { + it('proto + source globs both skip files matched by .gitnexusignore', async () => { + // Control: a regular .proto in a non-ignored dir. + writeFile( + 'proto/auth.proto', + `syntax = "proto3"; +package auth; +service AuthService { + rpc Login (LoginRequest) returns (LoginResponse); +}`, + ); + // Vendored proto under a venv-style dir — exercises proto-context glob. + writeFile( + 'mentor_env/lib/leaked.proto', + `syntax = "proto3"; +package leaked; +service LeakedService { + rpc Ping (PingRequest) returns (PingResponse); +}`, + ); + // Vendored Python consumer under the same venv-style dir — + // exercises the second glob in `extract()` (source-scan path). + // Mirrors the canonical pattern from + // `test_extract_python_stub_returns_consumer` above; without the + // `.gitnexusignore` filter this WOULD emit a `grpc::*/LeakedService` + // consumer contract. + writeFile( + 'mentor_env/lib/leaked_consumer.py', + `import grpc +from proto import leaked_pb2_grpc + +channel = grpc.insecure_channel('localhost:50051') +stub = leaked_pb2_grpc.LeakedServiceStub(channel)`, + ); + writeFile('.gitnexusignore', 'mentor_env/\n'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + // Control proto provider is still emitted. + expect(contracts.find((c) => c.contractId === 'grpc::auth.AuthService/Login')).toBeDefined(); + // Defence-in-depth: no contract — provider OR consumer — has a + // `symbolRef` path under the ignored directory. Catches both globs + // at once. + expect(contracts.some((c) => c.symbolRef?.filePath?.startsWith('mentor_env/'))).toBe(false); + // Specific assertions per glob path. + expect( + contracts.find((c) => c.contractId === 'grpc::leaked.LeakedService/Ping'), + ).toBeUndefined(); + expect( + contracts.some((c) => c.role === 'consumer' && /LeakedService/.test(c.contractId)), + ).toBe(false); + }); + }); }); describe('buildProtoMap', () => { diff --git a/gitnexus/test/unit/group/http-route-extractor.test.ts b/gitnexus/test/unit/group/http-route-extractor.test.ts index e81f1566e..69d3da2fa 100644 --- a/gitnexus/test/unit/group/http-route-extractor.test.ts +++ b/gitnexus/test/unit/group/http-route-extractor.test.ts @@ -729,4 +729,90 @@ router.get('/api/posts/{postId}', handler2); }); }); }); + + // ─── #1185: contract extractors must honour .gitnexusignore ───────── + // + // Pre-#1185 the source-scan path used a hardcoded + // `[node_modules, .git, dist, build, vendor]` glob ignore array, so a + // user's `.gitnexusignore` pattern (e.g. a Python venv `mentor_env/`, + // a generated stubs dir, a noisy fixture tree) was silently scanned + // anyway. Since #1185 the source-scan path consumes the shared + // `IgnoreService` (mirrors `filesystem-walker.ts`), so any pattern in + // `.gitnexusignore` (or `.gitignore`) prunes the glob. + describe('respects .gitnexusignore (#1185)', () => { + it('source-scan glob skips files matched by .gitnexusignore', async () => { + const dir = path.join(tmpDir, 'gitnexusignore-honoured'); + fs.mkdirSync(path.join(dir, 'src/routes'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'mentor_env/lib'), { recursive: true }); + // Control: a normal route file that SHOULD be discovered. + fs.writeFileSync( + path.join(dir, 'src/routes/users.ts'), + `import { Router } from 'express'; +const router = Router(); +router.get('/api/users', (req, res) => res.json([])); +export default router; +`, + ); + // Vendored source under a venv-style dir: the same Express + // pattern, but inside a directory the user wants excluded. + fs.writeFileSync( + path.join(dir, 'mentor_env/lib/leaked.ts'), + `import { Router } from 'express'; +const r = Router(); +r.get('/api/leaked', (req, res) => res.json([])); +export default r; +`, + ); + fs.writeFileSync(path.join(dir, '.gitnexusignore'), 'mentor_env/\n'); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const providers = contracts.filter((c) => c.role === 'provider'); + // Control survives. + expect(providers.find((c) => c.contractId === 'http::GET::/api/users')).toBeDefined(); + // Excluded path is pruned at the glob level — nothing emitted. + expect(providers.find((c) => c.contractId === 'http::GET::/api/leaked')).toBeUndefined(); + // Defence-in-depth: no contract whose symbolRef is under mentor_env/. + expect(contracts.some((c) => c.symbolRef?.filePath?.startsWith('mentor_env/'))).toBe(false); + }); + + // Pinned by the @claude review on PR #1247: above, only `.gitnexusignore` + // is exercised. `createIgnoreFilter` reads `.gitignore` too via + // `loadIgnoreRules`, but that integration is only proven at the + // `IgnoreService` level — no extractor-level test for the + // `.gitignore`-only code path. Adding one minimal extractor-level + // assertion here closes the gap (one shared test is sufficient + // because all three extractors consume the same filter object). + it('source-scan glob also skips files matched by `.gitignore` (no `.gitnexusignore`)', async () => { + const dir = path.join(tmpDir, 'gitignore-honoured'); + fs.mkdirSync(path.join(dir, 'src/routes'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'mentor_env/lib'), { recursive: true }); + // Same Express pattern as above so detection logic is identical. + fs.writeFileSync( + path.join(dir, 'src/routes/users.ts'), + `import { Router } from 'express'; +const router = Router(); +router.get('/api/users', (req, res) => res.json([])); +export default router; +`, + ); + fs.writeFileSync( + path.join(dir, 'mentor_env/lib/leaked.ts'), + `import { Router } from 'express'; +const r = Router(); +r.get('/api/leaked', (req, res) => res.json([])); +export default r; +`, + ); + // Note: NO .gitnexusignore — only `.gitignore`. This proves the + // `.gitignore` code path inside `createIgnoreFilter` is wired to + // the extractors' globs. + fs.writeFileSync(path.join(dir, '.gitignore'), 'mentor_env/\n'); + + const contracts = await extractor.extract(null, dir, makeRepo(dir)); + const providers = contracts.filter((c) => c.role === 'provider'); + expect(providers.find((c) => c.contractId === 'http::GET::/api/users')).toBeDefined(); + expect(providers.find((c) => c.contractId === 'http::GET::/api/leaked')).toBeUndefined(); + expect(contracts.some((c) => c.symbolRef?.filePath?.startsWith('mentor_env/'))).toBe(false); + }); + }); }); diff --git a/gitnexus/test/unit/group/topic-extractor.test.ts b/gitnexus/test/unit/group/topic-extractor.test.ts index bf821de63..754cf42f5 100644 --- a/gitnexus/test/unit/group/topic-extractor.test.ts +++ b/gitnexus/test/unit/group/topic-extractor.test.ts @@ -483,4 +483,59 @@ await consumer.subscribe({ topic: 'order.placed' });`, expect(contracts).toEqual([]); }); }); + + // ─── #1185: topic extractor must honour .gitnexusignore ───────────── + // + // The source-scan glob used to use a hardcoded ignore array; it now + // consumes the shared `IgnoreService` (mirrors `filesystem-walker.ts`) + // so any `.gitnexusignore` pattern excludes those files from contract + // extraction. Special case for this extractor: the Go-specific + // `_test.go` filter is preserved via a small wrapper around the base + // filter (so glob-level pruning still applies); the second test below + // pins that behaviour against accidental regressions. + describe('respects .gitnexusignore (#1185)', () => { + it('source-scan glob skips files matched by .gitnexusignore', async () => { + // Control: a regular @KafkaListener that SHOULD be discovered. + writeFile( + 'src/EventHandler.java', + `@KafkaListener(topics = "user.created") +public void handleUserCreated(ConsumerRecord record) {}`, + ); + // Vendored Java handler under a venv-style dir. + writeFile( + 'mentor_env/lib/LeakedHandler.java', + `@KafkaListener(topics = "leaked.event") +public void handleLeaked(ConsumerRecord r) {}`, + ); + writeFile('.gitnexusignore', 'mentor_env/\n'); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const ids = contracts.map((c) => c.contractId); + expect(ids).toContain('topic::user.created'); + expect(ids).not.toContain('topic::leaked.event'); + expect(contracts.some((c) => c.symbolRef?.filePath?.startsWith('mentor_env/'))).toBe(false); + }); + + it('still prunes `*_test.go` even when .gitnexusignore is empty (wrapper preserves glob-level filter)', async () => { + // Regression guard: replacing the hardcoded `**/*_test.go` glob + // entry with a wrapper around `createIgnoreFilter` must keep this + // file out of the scan. Without the wrapper, the previous test + // ('skips Go test files (`*_test.go`)') would still pass because + // the test scenario set up no detections, but here we WRITE a + // valid Sarama consumer call inside `_test.go` and assert the + // extractor never sees it. + writeFile( + 'src/orders_test.go', + `package orders +import "github.com/IBM/sarama" +func TestSomething() { + consumer.ConsumePartition("real-topic-from-test", 0, sarama.OffsetNewest) +}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + expect(contracts.find((c) => c.contractId === 'topic::real-topic-from-test')).toBeUndefined(); + expect(contracts).toEqual([]); + }); + }); }); From 0418cbb3478b2794d95ee1a50afefb05a332e39e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Fri, 1 May 2026 16:46:05 +0100 Subject: [PATCH 03/10] fix(cli): keep GitNexus ignores inside .gitnexus (#1248) * fix(cli): keep GitNexus ignores inside .gitnexus Avoid mutating analyzed repositories' root .gitignore while keeping generated GitNexus state untracked via .gitnexus/.gitignore. Made-with: Cursor * fix(cli): also use git info exclude for GitNexus storage When an analyzed repo has a real .git directory, add .gitnexus/ to .git/info/exclude so local Git metadata ignores generated storage without touching root .gitignore. Made-with: Cursor * fix(cli): keep skip-git subdir indexes ignored Ensure full analyze always writes the internal GitNexus ignore file so parent Git repositories stay clean for --skip-git subdirectory indexes. Made-with: Cursor --- gitnexus/src/cli/index-repo.ts | 4 +- gitnexus/src/core/run-analyze.ts | 9 +-- gitnexus/src/storage/repo-manager.ts | 47 ++++++++--- gitnexus/test/integration/cli-e2e.test.ts | 2 + gitnexus/test/unit/index-repo-command.test.ts | 14 ++-- gitnexus/test/unit/repo-manager.test.ts | 79 +++++++++++++++++++ gitnexus/test/unit/run-analyze.test.ts | 43 ++++++++++ gitnexus/test/unit/skip-git-cli.test.ts | 39 ++++++++- 8 files changed, 209 insertions(+), 28 deletions(-) diff --git a/gitnexus/src/cli/index-repo.ts b/gitnexus/src/cli/index-repo.ts index b909a40b5..52e8eb60d 100644 --- a/gitnexus/src/cli/index-repo.ts +++ b/gitnexus/src/cli/index-repo.ts @@ -14,7 +14,7 @@ import fs from 'fs/promises'; import { getStoragePaths, loadMeta, - addToGitignore, + ensureGitNexusIgnored, registerRepo, } from '../storage/repo-manager.js'; import { getGitRoot, getRemoteUrl, isGitRepo } from '../storage/git.js'; @@ -115,7 +115,7 @@ export const indexCommand = async (inputPathParts?: string[], options?: IndexOpt meta.remoteUrl = getRemoteUrl(repoPath); } await registerRepo(repoPath, meta); - await addToGitignore(repoPath); + await ensureGitNexusIgnored(repoPath); const projectName = path.basename(repoPath); const { stats } = meta; diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 854c65e24..e14f7a30f 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -26,7 +26,7 @@ import { getStoragePaths, saveMeta, loadMeta, - addToGitignore, + ensureGitNexusIgnored, registerRepo, cleanupOldKuzuFiles, } from '../storage/repo-manager.js'; @@ -166,6 +166,7 @@ export async function runFullAnalysis( if (existingMeta && !options.force && existingMeta.lastCommit === currentCommit) { // Non-git folders have currentCommit = '' — always rebuild since we can't detect changes if (currentCommit !== '') { + await ensureGitNexusIgnored(repoPath); return { repoName: options.registryName ?? getInferredRepoName(repoPath) ?? path.basename(repoPath), repoPath, @@ -447,10 +448,8 @@ export async function runFullAnalysis( allowDuplicateName: options.allowDuplicateName, }); - // Only attempt to update .gitignore when a .git directory is present. - if (hasGitDir(repoPath)) { - await addToGitignore(repoPath); - } + // Keep generated .gitnexus contents ignored without editing the user's root .gitignore. + await ensureGitNexusIgnored(repoPath); // ── Generate AI context files (best-effort) ─────────────────────── let aggregatedClusterCount = 0; diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index f0a322180..9741f54b4 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -96,6 +96,7 @@ export interface RegistryEntry { } const GITNEXUS_DIR = '.gitnexus'; +const GITNEXUS_EXCLUDE_ENTRY = `${GITNEXUS_DIR}/`; // ─── Local Storage Helpers ───────────────────────────────────────────── @@ -238,23 +239,45 @@ export const findRepo = async (startPath: string): Promise = }; /** - * Add .gitnexus to .gitignore if not already present + * Keep generated index files ignored without modifying the user's root .gitignore. */ -export const addToGitignore = async (repoPath: string): Promise => { - const gitignorePath = path.join(repoPath, '.gitignore'); +export const ensureGitNexusIgnored = async (repoPath: string): Promise => { + const gitignorePath = path.join(getStoragePath(repoPath), '.gitignore'); + + await fs.mkdir(path.dirname(gitignorePath), { recursive: true }); + await fs.writeFile(gitignorePath, '*\n', 'utf-8'); + + await ensureGitInfoExclude(repoPath); +}; + +const ensureGitInfoExclude = async (repoPath: string): Promise => { + const gitDirPath = path.join(path.resolve(repoPath), '.git'); + const excludePath = path.join(gitDirPath, 'info', 'exclude'); try { - const content = await fs.readFile(gitignorePath, 'utf-8'); - if (content.includes(GITNEXUS_DIR)) return; - - const newContent = content.endsWith('\n') - ? `${content}${GITNEXUS_DIR}\n` - : `${content}\n${GITNEXUS_DIR}\n`; - await fs.writeFile(gitignorePath, newContent, 'utf-8'); + const gitDir = await fs.stat(gitDirPath); + if (!gitDir.isDirectory()) return; } catch { - // .gitignore doesn't exist, create it - await fs.writeFile(gitignorePath, `${GITNEXUS_DIR}\n`, 'utf-8'); + return; } + + await fs.mkdir(path.dirname(excludePath), { recursive: true }); + + let content = ''; + try { + content = await fs.readFile(excludePath, 'utf-8'); + } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; + } + + const excludes = content + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line && !line.startsWith('#')); + if (excludes.includes(GITNEXUS_DIR) || excludes.includes(GITNEXUS_EXCLUDE_ENTRY)) return; + + const separator = content.length === 0 || content.endsWith('\n') ? '' : '\n'; + await fs.writeFile(excludePath, `${content}${separator}${GITNEXUS_EXCLUDE_ENTRY}\n`, 'utf-8'); }; // ─── Global Registry (~/.gitnexus/registry.json) ─────────────────────── diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index ecf1201ff..8037511bf 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -198,6 +198,8 @@ describe('CLI end-to-end', () => { const gitnexusDir = path.join(MINI_REPO, '.gitnexus'); expect(fs.existsSync(gitnexusDir)).toBe(true); expect(fs.statSync(gitnexusDir).isDirectory()).toBe(true); + expect(fs.existsSync(path.join(MINI_REPO, '.gitignore'))).toBe(false); + expect(fs.readFileSync(path.join(gitnexusDir, '.gitignore'), 'utf-8')).toBe('*\n'); }, 60_000); // Regression guard for issue #1169 — analyze must produce BOTH a diff --git a/gitnexus/test/unit/index-repo-command.test.ts b/gitnexus/test/unit/index-repo-command.test.ts index 3f7a57153..8e1994063 100644 --- a/gitnexus/test/unit/index-repo-command.test.ts +++ b/gitnexus/test/unit/index-repo-command.test.ts @@ -5,7 +5,7 @@ const mockAccess = vi.fn(); const mockGetStoragePaths = vi.fn(); const mockLoadMeta = vi.fn(); const mockRegisterRepo = vi.fn(); -const mockAddToGitignore = vi.fn(); +const mockEnsureGitNexusIgnored = vi.fn(); const mockGetGitRoot = vi.fn(); const mockIsGitRepo = vi.fn(); @@ -19,7 +19,7 @@ vi.mock('../../src/storage/repo-manager.js', () => ({ getStoragePaths: mockGetStoragePaths, loadMeta: mockLoadMeta, registerRepo: mockRegisterRepo, - addToGitignore: mockAddToGitignore, + ensureGitNexusIgnored: mockEnsureGitNexusIgnored, })); vi.mock('../../src/storage/git.js', () => ({ @@ -53,7 +53,7 @@ describe('indexCommand', () => { stats: { nodes: 10, edges: 20 }, }); mockAccess.mockResolvedValue(undefined); - mockAddToGitignore.mockResolvedValue(undefined); + mockEnsureGitNexusIgnored.mockResolvedValue(undefined); mockGetGitRoot.mockReturnValue(resolvedRepo); mockIsGitRepo.mockReturnValue(true); }); @@ -134,8 +134,8 @@ describe('indexCommand', () => { resolvedRepo, expect.objectContaining({ repoPath: resolvedRepo }), ); - expect(mockAddToGitignore).toHaveBeenCalledTimes(1); - expect(mockAddToGitignore).toHaveBeenCalledWith(resolvedRepo); + expect(mockEnsureGitNexusIgnored).toHaveBeenCalledTimes(1); + expect(mockEnsureGitNexusIgnored).toHaveBeenCalledWith(resolvedRepo); expect(process.exitCode).toBeUndefined(); }); @@ -170,7 +170,7 @@ describe('indexCommand', () => { resolvedRepo, expect.objectContaining({ repoPath: resolvedRepo }), ); - expect(mockAddToGitignore).toHaveBeenCalledWith(resolvedRepo); + expect(mockEnsureGitNexusIgnored).toHaveBeenCalledWith(resolvedRepo); expect(process.exitCode).toBeUndefined(); }); @@ -189,7 +189,7 @@ describe('indexCommand', () => { await indexCommand(['/repo', '/other']); expect(mockRegisterRepo).not.toHaveBeenCalled(); - expect(mockAddToGitignore).not.toHaveBeenCalled(); + expect(mockEnsureGitNexusIgnored).not.toHaveBeenCalled(); expect(process.exitCode).toBe(1); expect(logSpy).toHaveBeenCalledWith(' The `index` command accepts a single path only.'); }); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 12c56d67a..7169eb806 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -11,6 +11,7 @@ import fs from 'fs/promises'; import { getStoragePath, getStoragePaths, + ensureGitNexusIgnored, readRegistry, loadCLIConfig, registerRepo, @@ -62,6 +63,84 @@ describe('getStoragePaths', () => { }); }); +// ─── GitNexus ignore rules (#1233) ───────────────────────────────────── + +describe('ensureGitNexusIgnored (#1233)', () => { + let tmpRepo: Awaited>; + + beforeEach(async () => { + tmpRepo = await createTempDir('gitnexus-internal-gitignore-'); + }); + + afterEach(async () => { + await tmpRepo.cleanup(); + }); + + it('creates .gitnexus/.gitignore containing a catch-all ignore rule', async () => { + await ensureGitNexusIgnored(tmpRepo.dbPath); + + await expect( + fs.readFile(path.join(tmpRepo.dbPath, '.gitnexus', '.gitignore'), 'utf-8'), + ).resolves.toBe('*\n'); + }); + + it('does not create or modify the repository root .gitignore', async () => { + const rootGitignorePath = path.join(tmpRepo.dbPath, '.gitignore'); + await fs.writeFile(rootGitignorePath, 'node_modules/\n'); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + + await expect(fs.readFile(rootGitignorePath, 'utf-8')).resolves.toBe('node_modules/\n'); + }); + + it('adds .gitnexus/ to .git/info/exclude when the repo has a real .git directory', async () => { + const excludePath = path.join(tmpRepo.dbPath, '.git', 'info', 'exclude'); + await fs.mkdir(path.dirname(excludePath), { recursive: true }); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + + await expect(fs.readFile(excludePath, 'utf-8')).resolves.toBe('.gitnexus/\n'); + }); + + it('appends .gitnexus/ to .git/info/exclude once without disturbing existing rules', async () => { + const excludePath = path.join(tmpRepo.dbPath, '.git', 'info', 'exclude'); + await fs.mkdir(path.dirname(excludePath), { recursive: true }); + await fs.writeFile(excludePath, '# local excludes\nnode_modules/\n'); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + await ensureGitNexusIgnored(tmpRepo.dbPath); + + await expect(fs.readFile(excludePath, 'utf-8')).resolves.toBe( + '# local excludes\nnode_modules/\n.gitnexus/\n', + ); + }); + + it('does not create .git/info/exclude when .git is not a directory', async () => { + await fs.writeFile(path.join(tmpRepo.dbPath, '.git'), 'gitdir: ../real-git-dir\n'); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + + await expect(fs.access(path.join(tmpRepo.dbPath, '.git', 'info', 'exclude'))).rejects.toThrow(); + }); + + it('keeps generated .gitnexus files out of git status', async () => { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=test -c user.email=test@test commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + + await ensureGitNexusIgnored(tmpRepo.dbPath); + await fs.writeFile(path.join(tmpRepo.dbPath, '.gitnexus', 'meta.json'), '{}\n'); + + const status = execSync('git status --short', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }); + expect(status).toBe(''); + }); +}); + // ─── readRegistry ──────────────────────────────────────────────────── describe('readRegistry', () => { diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index 747fffe4a..a688d82d0 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -1,5 +1,10 @@ +import { execSync } from 'child_process'; +import fs from 'fs/promises'; +import path from 'path'; import { describe, it, expect } from 'vitest'; import { deriveEmbeddingMode } from '../../src/core/embedding-mode.js'; +import { getStoragePaths, saveMeta, type RepoMeta } from '../../src/storage/repo-manager.js'; +import { createTempDir } from '../helpers/test-db.js'; describe('run-analyze module', () => { it('exports runFullAnalysis as a function', async () => { @@ -12,6 +17,44 @@ describe('run-analyze module', () => { expect(mod.PHASE_LABELS).toBeDefined(); expect(mod.PHASE_LABELS.parsing).toBe('Parsing code'); }); + + it('creates .gitnexus/.gitignore on the already-up-to-date fast path (#1233)', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-fast-path-'); + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=test -c user.email=test@test commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + const currentCommit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: currentCommit, + indexedAt: new Date().toISOString(), + }; + await saveMeta(storagePath, meta); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + {}, + { + onProgress: () => {}, + }, + ); + + expect(result.alreadyUpToDate).toBe(true); + await expect( + fs.readFile(path.join(tmpRepo.dbPath, '.gitnexus', '.gitignore'), 'utf-8'), + ).resolves.toBe('*\n'); + } finally { + await tmpRepo.cleanup(); + } + }); }); describe('deriveEmbeddingMode', () => { diff --git a/gitnexus/test/unit/skip-git-cli.test.ts b/gitnexus/test/unit/skip-git-cli.test.ts index c8c433f35..e82bb7b13 100644 --- a/gitnexus/test/unit/skip-git-cli.test.ts +++ b/gitnexus/test/unit/skip-git-cli.test.ts @@ -40,18 +40,19 @@ describe('--skip-git CLI flag', () => { describe('--skip-git does not walk up to parent git repo (#1232)', () => { const cliPath = path.resolve(__dirname, '../../dist/cli/index.js'); let parentDir: string; + let gitnexusHome: string; function testEnv() { return { ...process.env, HOME: parentDir, - GITNEXUS_HOME: path.join(parentDir, '.gitnexus-home'), + GITNEXUS_HOME: gitnexusHome, GITNEXUS_LBUG_EXTENSION_INSTALL: 'never', }; } function readRegistry(): Array<{ name: string; path: string }> { - const registryPath = path.join(parentDir, '.gitnexus-home', 'registry.json'); + const registryPath = path.join(gitnexusHome, 'registry.json'); expect(fs.existsSync(registryPath)).toBe(true); return JSON.parse(fs.readFileSync(registryPath, 'utf8')); } @@ -99,6 +100,7 @@ describe('--skip-git CLI flag', () => { // package.json // src/index.ts parentDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-skip-git-')); + gitnexusHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-skip-git-home-')); initParentGitRepo(); fs.mkdirSync(path.join(parentDir, 'COOLIO', 'src'), { recursive: true }); fs.writeFileSync( @@ -125,6 +127,9 @@ describe('--skip-git CLI flag', () => { if (parentDir) { fs.rmSync(parentDir, { recursive: true, force: true }); } + if (gitnexusHome) { + fs.rmSync(gitnexusHome, { recursive: true, force: true }); + } } it('from subdir inside parent git repo, indexes subdir not parent', () => { @@ -155,6 +160,36 @@ describe('--skip-git CLI flag', () => { } }); + it('keeps parent git status clean for --skip-git subdir analyze (#1233)', () => { + createTestStructure(); + try { + fs.writeFileSync(path.join(parentDir, '.gitignore'), '.claude/\n'); + execSync('git add .gitignore COOLIO SubWooder', { cwd: parentDir, stdio: 'ignore' }); + execSync('git -c user.name=test -c user.email=test@example.com commit -m fixtures', { + cwd: parentDir, + stdio: 'ignore', + }); + + execSync(`node "${cliPath}" analyze --skip-git --skip-agents-md`, { + cwd: path.join(parentDir, 'COOLIO'), + encoding: 'utf8', + timeout: 60000, + env: testEnv(), + }); + + expect( + fs.readFileSync(path.join(parentDir, 'COOLIO', '.gitnexus', '.gitignore'), 'utf8'), + ).toBe('*\n'); + const status = execSync('git status --short', { + cwd: parentDir, + encoding: 'utf8', + }); + expect(status).toBe(''); + } finally { + cleanup(); + } + }); + it('explicit input path with --skip-git indexes subdir', () => { createTestStructure(); try { From 55d504284f1f36735cff951a9c61f473910504d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 2 May 2026 08:15:46 +0100 Subject: [PATCH 04/10] fix(ci): consolidate Claude review workflow (#1258) --- .github/workflows/claude-code-review.yml | 96 ------------------------ .github/workflows/claude.yml | 77 +++++++++++++++---- 2 files changed, 63 insertions(+), 110 deletions(-) delete mode 100644 .github/workflows/claude-code-review.yml diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml deleted file mode 100644 index e5642cb3e..000000000 --- a/.github/workflows/claude-code-review.yml +++ /dev/null @@ -1,96 +0,0 @@ -name: Claude Code Review - -# Uses pull_request_target so the workflow runs as defined on the default branch, -# which allows access to secrets for posting review comments on fork PRs. -# SECURITY: The checkout pins the fork's HEAD SHA (not the branch name) to -# prevent TOCTOU races (force-push between trigger and checkout). The -# claude-code-action sandboxes execution — it does NOT run arbitrary code -# from the checked-out source. - -on: - # Trigger only when explicitly requested: - # - Add the "claude-review" label to a PR, OR - # - Comment "@claude" or "/review" on a PR - pull_request_target: - types: [labeled] - issue_comment: - types: [created] - -# Concurrency convention: see CONTRIBUTING.md → "GitHub Actions — Concurrency Convention". -# Serialize per-PR to avoid racing review comments. -concurrency: - group: ${{ github.workflow }}-${{ github.event.issue.number || github.event.pull_request.number }} - cancel-in-progress: false - -jobs: - claude-review: - # Run only when: - # 1. The "claude-review" label is added to a non-draft PR by a trusted contributor, OR - # 2. A trusted contributor comments "@claude" or "/review" on a PR - if: | - ( - github.event_name == 'pull_request_target' && - github.event.label.name == 'claude-review' && - github.event.pull_request.draft == false && - (github.event.pull_request.author_association == 'OWNER' || - github.event.pull_request.author_association == 'MEMBER' || - github.event.pull_request.author_association == 'COLLABORATOR') - ) || - ( - github.event_name == 'issue_comment' && - github.event.issue.pull_request && - (contains(github.event.comment.body, '@claude') || - contains(github.event.comment.body, '/review')) && - (github.event.comment.author_association == 'OWNER' || - github.event.comment.author_association == 'MEMBER' || - github.event.comment.author_association == 'COLLABORATOR') - ) - runs-on: ubuntu-latest - timeout-minutes: 30 - permissions: - contents: read - pull-requests: write - issues: read - id-token: write - - steps: - # For issue_comment triggers, resolve the PR number, head SHA, and fork repo - - name: Resolve PR context - id: pr - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v7 - with: - script: | - let pr; - if (context.eventName === 'issue_comment') { - const resp = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.issue.number, - }); - pr = resp.data; - } else { - pr = context.payload.pull_request; - } - core.setOutput('number', pr.number); - core.setOutput('sha', pr.head.sha); - core.setOutput('repo', pr.head.repo.full_name); - core.setOutput('branch', pr.head.ref); - - - name: Checkout PR head - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - repository: ${{ steps.pr.outputs.repo }} - ref: ${{ steps.pr.outputs.sha }} - fetch-depth: 1 - - - name: Run Claude Code Review - id: claude-review - uses: anthropics/claude-code-action@9469d113c6afd29550c402740f22d1a97dd1209b # v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - github_token: ${{ secrets.GITHUB_TOKEN }} - allowed_non_write_users: '*' - show_full_output: true - plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' - plugins: 'code-review@claude-code-plugins' - prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ steps.pr.outputs.number }}' diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 553d3ab0d..cfba3ecbc 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -1,8 +1,17 @@ name: Claude Code +# Label-triggered code-review requests use pull_request_target so the workflow +# runs as defined on the default branch, which allows access to secrets for +# posting review comments on fork PRs. SECURITY: PR checkouts pin the fork's +# HEAD SHA (not the branch name) to prevent TOCTOU races. +# The claude-code-action sandboxes execution; it does not run arbitrary code +# from the checked-out source. + on: issue_comment: types: [created] + pull_request_target: + types: [labeled] pull_request_review_comment: types: [created] issues: @@ -21,7 +30,10 @@ jobs: if: | ( github.event_name == 'issue_comment' && - contains(github.event.comment.body, '@claude') && + ( + contains(github.event.comment.body, '@claude') || + (github.event.issue.pull_request && contains(github.event.comment.body, '/review')) + ) && (github.event.comment.author_association == 'OWNER' || github.event.comment.author_association == 'MEMBER' || github.event.comment.author_association == 'COLLABORATOR') @@ -46,6 +58,14 @@ jobs: (github.event.issue.author_association == 'OWNER' || github.event.issue.author_association == 'MEMBER' || github.event.issue.author_association == 'COLLABORATOR') + ) || + ( + github.event_name == 'pull_request_target' && + github.event.label.name == 'claude-review' && + github.event.pull_request.draft == false && + (github.event.pull_request.author_association == 'OWNER' || + github.event.pull_request.author_association == 'MEMBER' || + github.event.pull_request.author_association == 'COLLABORATOR') ) runs-on: ubuntu-latest timeout-minutes: 30 @@ -63,33 +83,48 @@ jobs: with: script: | // Determine if this event is PR-related - let prNumber = null; + let pr = null; if (context.eventName === 'issue_comment' && context.payload.issue.pull_request) { - prNumber = context.payload.issue.number; + const resp = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.issue.number, + }); + pr = resp.data; } else if (context.eventName === 'pull_request_review_comment') { - prNumber = context.payload.pull_request.number; + pr = context.payload.pull_request; } else if (context.eventName === 'pull_request_review') { - prNumber = context.payload.pull_request.number; + pr = context.payload.pull_request; + } else if (context.eventName === 'pull_request_target') { + pr = context.payload.pull_request; } - if (!prNumber) { + if (!pr) { core.setOutput('is_pr', 'false'); return; } - const resp = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: prNumber, - }); - const pr = resp.data; - core.setOutput('is_pr', 'true'); - core.setOutput('number', String(prNumber)); + core.setOutput('number', String(pr.number)); core.setOutput('sha', pr.head.sha); core.setOutput('repo', pr.head.repo.full_name); core.setOutput('branch', pr.head.ref); + - name: Resolve Claude mode + id: mode + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v7 + with: + script: | + const body = (context.payload.comment?.body ?? '').toLowerCase(); + const isCodeReview = + (context.eventName === 'pull_request_target' && + context.payload.label?.name === 'claude-review') || + (context.eventName === 'issue_comment' && + Boolean(context.payload.issue?.pull_request) && + body.includes('/review')); + + core.setOutput('code_review', isCodeReview ? 'true' : 'false'); + - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -98,6 +133,7 @@ jobs: fetch-depth: 1 - name: Run Claude Code + if: steps.mode.outputs.code_review != 'true' id: claude uses: anthropics/claude-code-action@9469d113c6afd29550c402740f22d1a97dd1209b # v1 with: @@ -109,3 +145,16 @@ jobs: # This is an optional setting that allows Claude to read CI results on PRs additional_permissions: | actions: read + + - name: Run Claude Code Review + if: steps.mode.outputs.code_review == 'true' + id: claude-review + uses: anthropics/claude-code-action@9469d113c6afd29550c402740f22d1a97dd1209b # v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + github_token: ${{ secrets.GITHUB_TOKEN }} + allowed_non_write_users: '*' + show_full_output: true + plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' + plugins: 'code-review@claude-code-plugins' + prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ steps.pr.outputs.number }}' From 368049576b3ddd31f93ebae7aa08610a9be1c55b Mon Sep 17 00:00:00 2001 From: jelsco <58397194+jelsco@users.noreply.github.com> Date: Sat, 2 May 2026 04:36:28 -0600 Subject: [PATCH 05/10] fix(python): make multi-segment suffix fallback deterministic (#1253) --- .../languages/python/import-target.ts | 38 ++- .../test/integration/resolvers/helpers.ts | 10 + .../test/integration/resolvers/python.test.ts | 227 +++++++++++++++++- 3 files changed, 266 insertions(+), 9 deletions(-) diff --git a/gitnexus/src/core/ingestion/languages/python/import-target.ts b/gitnexus/src/core/ingestion/languages/python/import-target.ts index 677372966..3a75868d9 100644 --- a/gitnexus/src/core/ingestion/languages/python/import-target.ts +++ b/gitnexus/src/core/ingestion/languages/python/import-target.ts @@ -88,7 +88,8 @@ export function resolvePythonImportTarget( * Precedence order: * 1. Workspace-root direct hit (`.py`, `/__init__.py`). * 2. Closest-ancestor match walking up from the importer's directory. - * 3. Suffix fallback (first match). + * 3. Suffix fallback (deterministic: fewest path segments, then + * lexicographic on the normalized path). * * Root wins over ancestor by construction — if both `services/sync.py` and * `backend/services/sync.py` exist, `backend/routers/cron.py`'s @@ -129,18 +130,39 @@ function resolveAbsoluteFromFiles( } } - // Existing suffix-match fallback (preserved for monorepo/nested-repo - // layouts that don't share a directory ancestor with the importer). + // Suffix-match fallback (preserved for monorepo/nested-repo layouts + // that don't share a directory ancestor with the importer). + // + // Tie-break order when multiple files match the same suffix: + // 1. Fewest path segments (shorter, more canonical paths win — `lib/x.py` + // beats `tooling/extras/x.py`). + // 2. Lexicographic order over the normalized path (final stable + // tiebreak independent of file-set insertion order). + // + // Without an explicit tie-break the previous implementation returned + // the first match in `Set` iteration order, which depended on file + // ingestion order and produced non-deterministic edges across runs in + // multi-directory collision repos. const suffixFile = `/${directFile}`; const suffixPkg = `/${directPkg}`; - let suffixMatch: string | null = null; + const matches: { raw: string; norm: string }[] = []; for (const raw of allFilePaths) { - const f = raw.replace(/\\/g, '/'); - if (suffixMatch === null && (f.endsWith(suffixFile) || f.endsWith(suffixPkg))) { - suffixMatch = raw; + const norm = raw.replace(/\\/g, '/'); + if (norm.endsWith(suffixFile) || norm.endsWith(suffixPkg)) { + matches.push({ raw, norm }); } } - return suffixMatch; + if (matches.length === 0) return null; + if (matches.length === 1) return matches[0].raw; + matches.sort((a, b) => { + const aDepth = a.norm.split('/').length; + const bDepth = b.norm.split('/').length; + if (aDepth !== bDepth) return aDepth - bDepth; + if (a.norm < b.norm) return -1; + if (a.norm > b.norm) return 1; + return 0; + }); + return matches[0].raw; } /** diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index 710fcddeb..427fd7e5f 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -12,6 +12,16 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly Models/User.cs through the scope-resolution path', ]), + python: new Set([ + // Suffix-fallback lex tiebreak depends on the registry-primary + // resolver's deterministic sort. The legacy resolver returns the + // first match in `Set` iteration order, which is insertion-order + // dependent and not aligned with this guarantee. Backporting the + // sort to legacy is out of scope. + 'picks the lexicographically smaller path on equal-depth ties', + 'binds the call to alpha/services/sync.py, not omega', + 'lex tiebreak still picks alpha/services/sync.py with reversed file-write order', + ]), }; type ResolverParityEnv = Readonly>; diff --git a/gitnexus/test/integration/resolvers/python.test.ts b/gitnexus/test/integration/resolvers/python.test.ts index dd0104a5c..450ba46fc 100644 --- a/gitnexus/test/integration/resolvers/python.test.ts +++ b/gitnexus/test/integration/resolvers/python.test.ts @@ -1,13 +1,14 @@ /** * Python: relative imports + class inheritance + ambiguous module disambiguation */ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, expect, beforeAll, afterAll } from 'vitest'; import path from 'path'; import fs from 'node:fs'; import os from 'node:os'; import { FIXTURES, CROSS_FILE_FIXTURES, + createResolverParityIt, getRelationships, getNodesByLabel, getNodesByLabelFull, @@ -16,6 +17,11 @@ import { type PipelineResult, } from './helpers.js'; +// Mirrors `csharp.test.ts`: skips tests in `LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.python` +// when the legacy-resolver parity sweep runs (`REGISTRY_PRIMARY_PYTHON=0`). For the +// default registry-primary CI run this is a transparent passthrough to vitest's `it`. +const it = createResolverParityIt('python'); + function writeFixtureRepo(root: string, files: Record): void { for (const [relPath, content] of Object.entries(files)) { const fullPath = path.join(root, relPath); @@ -623,6 +629,225 @@ def handler(): }); }); +// --------------------------------------------------------------------------- +// Suffix-fallback determinism: when both root + ancestor walk miss but the +// suffix scan finds multiple candidates in unrelated trees, the resolver +// must pick the same file regardless of file-set insertion order. The +// previous implementation returned the first match in `Set` iteration +// order, which depended on file ingestion order and produced flapping +// edges across runs in multi-directory collision repos. +// +// Tie-break order: fewest path segments, then lexicographic. +// --------------------------------------------------------------------------- + +describe('Python multi-segment resolution: suffix fallback determinism', () => { + let repoDir: string; + let result: PipelineResult; + + beforeAll(async () => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-determinism-')); + writeFixtureRepo(repoDir, { + // Importer's package. The `app/services/marker.py` file makes the + // `services` segment gate-pass under the ancestor-bounded + // `hasRepoCandidate` check, but `app/services/sync.py` is + // intentionally absent so the ancestor walk misses and the suffix + // fallback fires. + 'app/services/marker.py': `def _marker(): return True +`, + 'app/main.py': `from services.sync import handler + +def boot(): + return handler() +`, + // Two suffix candidates outside the importer's ancestor tree. + // `lib/services/sync.py` has 3 path segments, the alternative has + // 4 — the deterministic pick is `lib/services/sync.py`. + 'lib/services/sync.py': `def handler(): + return "lib" +`, + 'tooling/extras/services/sync.py': `def handler(): + return "tooling" +`, + }); + result = await runPipelineFromRepo(repoDir, () => {}); + }, 60000); + + afterAll(() => { + if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true }); + }); + + it('picks the shortest-path candidate (lib/services/sync.py) and only that one', () => { + const imports = getRelationships(result, 'IMPORTS').filter( + (i) => i.sourceFilePath === 'app/main.py', + ); + + const libEdge = imports.find((i) => i.targetFilePath === 'lib/services/sync.py'); + expect(libEdge).toBeDefined(); + + const toolingEdge = imports.find((i) => i.targetFilePath === 'tooling/extras/services/sync.py'); + expect(toolingEdge).toBeUndefined(); + }); + + it('binds the call to the deterministic pick, not the alternate copy', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'app/main.py' && c.target === 'handler', + ); + expect(calls.length).toBe(1); + expect(calls[0].targetFilePath).toBe('lib/services/sync.py'); + }); +}); + +// --------------------------------------------------------------------------- +// Lexicographic tiebreak: when two suffix candidates have the same +// directory depth, the lexicographically smaller path wins. Without this, +// equal-depth collisions would still depend on file-set insertion order. +// --------------------------------------------------------------------------- + +describe('Python multi-segment resolution: suffix fallback lexicographic tiebreak', () => { + let repoDir: string; + let result: PipelineResult; + + beforeAll(async () => { + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-lex-tiebreak-')); + writeFixtureRepo(repoDir, { + // Same gate-passing pattern as the determinism test — non-init + // marker file makes the `services` segment satisfy + // `hasRepoCandidate` for an importer at `app/main.py`. + 'app/services/marker.py': `def _marker(): return True +`, + 'app/main.py': `from services.sync import handler + +def boot(): + return handler() +`, + // Both candidates have depth 3, so directory-depth alone cannot + // disambiguate. Lexicographic order picks `alpha/...` over + // `omega/...` regardless of which file was ingested first. + 'alpha/services/sync.py': `def handler(): + return "alpha" +`, + 'omega/services/sync.py': `def handler(): + return "omega" +`, + }); + result = await runPipelineFromRepo(repoDir, () => {}); + }, 60000); + + afterAll(() => { + if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true }); + }); + + it('picks the lexicographically smaller path on equal-depth ties', () => { + const imports = getRelationships(result, 'IMPORTS').filter( + (i) => i.sourceFilePath === 'app/main.py', + ); + + const alphaEdge = imports.find((i) => i.targetFilePath === 'alpha/services/sync.py'); + expect(alphaEdge).toBeDefined(); + + const omegaEdge = imports.find((i) => i.targetFilePath === 'omega/services/sync.py'); + expect(omegaEdge).toBeUndefined(); + }); + + it('binds the call to alpha/services/sync.py, not omega', () => { + const calls = getRelationships(result, 'CALLS').filter( + (c) => c.sourceFilePath === 'app/main.py' && c.target === 'handler', + ); + expect(calls.length).toBe(1); + expect(calls[0].targetFilePath).toBe('alpha/services/sync.py'); + }); +}); + +// --------------------------------------------------------------------------- +// Insertion-order independence: re-runs the depth and lexicographic +// scenarios with the candidate files written in reverse order. The +// deterministic sort in `resolveAbsoluteFromFiles` should pick the same +// winner regardless. If a future refactor accidentally drops the sort +// and falls back to `Set` insertion order, these tests pin the +// regression directly. +// --------------------------------------------------------------------------- + +describe('Python multi-segment resolution: suffix fallback insertion-order independence', () => { + let depthRepoDir: string; + let lexRepoDir: string; + let depthResult: PipelineResult; + let lexResult: PipelineResult; + + beforeAll(async () => { + // Depth scenario, files written in reverse order: tooling first, lib + // second. `writeFixtureRepo` iterates in object-property insertion + // order, and the pipeline scanner's directory traversal is also + // affected by mtime/inode order on most filesystems. The expected + // winner is still `lib/services/sync.py` (depth 3 < depth 4). + depthRepoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-determinism-rev-')); + writeFixtureRepo(depthRepoDir, { + 'tooling/extras/services/sync.py': `def handler(): + return "tooling" +`, + 'lib/services/sync.py': `def handler(): + return "lib" +`, + 'app/services/marker.py': `def _marker(): return True +`, + 'app/main.py': `from services.sync import handler + +def boot(): + return handler() +`, + }); + depthResult = await runPipelineFromRepo(depthRepoDir, () => {}); + + // Lexicographic scenario, files written in reverse order: omega first. + // Expected winner is still `alpha/services/sync.py`. + lexRepoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-lex-rev-')); + writeFixtureRepo(lexRepoDir, { + 'omega/services/sync.py': `def handler(): + return "omega" +`, + 'alpha/services/sync.py': `def handler(): + return "alpha" +`, + 'app/services/marker.py': `def _marker(): return True +`, + 'app/main.py': `from services.sync import handler + +def boot(): + return handler() +`, + }); + lexResult = await runPipelineFromRepo(lexRepoDir, () => {}); + }, 120000); + + afterAll(() => { + if (depthRepoDir !== undefined) fs.rmSync(depthRepoDir, { recursive: true, force: true }); + if (lexRepoDir !== undefined) fs.rmSync(lexRepoDir, { recursive: true, force: true }); + }); + + it('depth tiebreak still picks lib/services/sync.py with reversed file-write order', () => { + const imports = getRelationships(depthResult, 'IMPORTS').filter( + (i) => i.sourceFilePath === 'app/main.py', + ); + + const libEdge = imports.find((i) => i.targetFilePath === 'lib/services/sync.py'); + expect(libEdge).toBeDefined(); + + const toolingEdge = imports.find((i) => i.targetFilePath === 'tooling/extras/services/sync.py'); + expect(toolingEdge).toBeUndefined(); + }); + + it('lex tiebreak still picks alpha/services/sync.py with reversed file-write order', () => { + const imports = getRelationships(lexResult, 'IMPORTS').filter( + (i) => i.sourceFilePath === 'app/main.py', + ); + + const alphaEdge = imports.find((i) => i.targetFilePath === 'alpha/services/sync.py'); + expect(alphaEdge).toBeDefined(); + + const omegaEdge = imports.find((i) => i.targetFilePath === 'omega/services/sync.py'); + expect(omegaEdge).toBeUndefined(); + }); +}); + // --------------------------------------------------------------------------- // Re-export chain: from .base import X barrel pattern via __init__.py // --------------------------------------------------------------------------- From bc722b9d8f6e945d8c70fc04415ccd2842e3fa0f Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Sun, 3 May 2026 02:40:29 +0200 Subject: [PATCH 06/10] fix(group): resolve custom manifest links against graph symbols (#1254) --- .../group/extractors/manifest-extractor.ts | 13 ++ .../unit/group/manifest-extractor.test.ts | 158 ++++++++++++++++++ 2 files changed, 171 insertions(+) diff --git a/gitnexus/src/core/group/extractors/manifest-extractor.ts b/gitnexus/src/core/group/extractors/manifest-extractor.ts index 83f5cab5e..b65b7712d 100644 --- a/gitnexus/src/core/group/extractors/manifest-extractor.ts +++ b/gitnexus/src/core/group/extractors/manifest-extractor.ts @@ -268,6 +268,19 @@ export class ManifestExtractor { LIMIT 1`, { contract: link.contract }, ); + } else if (link.type === 'custom') { + // V1: exact name-only match on code-definition nodes. + // Positive allowlist mirrors other contract types. If multiple code + // symbols share the same name, ORDER BY filePath ASC LIMIT 1 picks + // the alphabetically-first occurrence deterministically. + rows = await executor( + `MATCH (n:Function|Method|Class|Interface|Struct|Enum|Trait|Constructor|TypeAlias|Impl|Macro|Union|Typedef|Property|Record|Delegate|Annotation|Template|Const|Static|CodeElement) + WHERE n.name = $contract + RETURN n.id AS uid, n.name AS name, n.filePath AS filePath + ORDER BY n.filePath ASC + LIMIT 1`, + { contract: link.contract }, + ); } else { return null; } diff --git a/gitnexus/test/unit/group/manifest-extractor.test.ts b/gitnexus/test/unit/group/manifest-extractor.test.ts index 59725dc86..997445f2e 100644 --- a/gitnexus/test/unit/group/manifest-extractor.test.ts +++ b/gitnexus/test/unit/group/manifest-extractor.test.ts @@ -578,6 +578,164 @@ describe('ManifestExtractor', () => { expect(lowerContractId).toBe(upperContractId); }); + it('resolves custom manifest links by exact symbol name', async () => { + const links: GroupManifestLink[] = [ + { + from: 'parser/mathlex', + to: 'engine/thales', + type: 'custom', + contract: 'Expression', + role: 'provider', + }, + ]; + + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'engine/thales', + async (_cypher, params) => { + if (params?.contract === 'Expression') { + return [ + { + uid: 'uid-expression-struct', + name: 'Expression', + filePath: 'src/expression.rs', + }, + ]; + } + return []; + }, + ], + [ + 'parser/mathlex', + async (_cypher, params) => { + if (params?.contract === 'Expression') { + return [ + { + uid: 'uid-expression-enum', + name: 'Expression', + filePath: 'src/ast.rs', + }, + ]; + } + return []; + }, + ], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + + const provider = result.contracts.find((c) => c.role === 'provider'); + const consumer = result.contracts.find((c) => c.role === 'consumer'); + + expect(provider?.symbolUid).toBe('uid-expression-enum'); + expect(provider?.symbolRef.filePath).toBe('src/ast.rs'); + + expect(consumer?.symbolUid).toBe('uid-expression-struct'); + expect(consumer?.symbolRef.filePath).toBe('src/expression.rs'); + + expect(result.crossLinks).toHaveLength(1); + expect(result.crossLinks[0].matchType).toBe('manifest'); + }); + + it('falls back to synthetic uid when custom symbol not found in graph', async () => { + const links: GroupManifestLink[] = [ + { + from: 'core/units', + to: 'engine/thales', + type: 'custom', + contract: 'Dimension', + role: 'provider', + }, + ]; + + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + ['engine/thales', async () => []], + ['core/units', async () => []], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.symbolUid).toBe('manifest::core/units::custom::Dimension'); + }); + + it('custom contract query uses positive label allowlist (not negative exclusion)', async () => { + const links: GroupManifestLink[] = [ + { + from: 'parser/mathlex', + to: 'engine/thales', + type: 'custom', + contract: 'Route', + role: 'provider', + }, + ]; + let capturedCypher = ''; + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'parser/mathlex', + async (cypher) => { + capturedCypher = cypher; + return []; + }, + ], + ['engine/thales', async () => []], + ]); + + await extractor.extractFromManifest(links, dbExecutors); + + expect(capturedCypher).toContain('Function|Method|Class|Interface|Struct|Enum|Trait'); + expect(capturedCypher).not.toContain('NOT n:File'); + }); + + it('custom contract with ambiguous name returns first-by-filePath deterministically', async () => { + const links: GroupManifestLink[] = [ + { + from: 'parser/mathlex', + to: 'engine/thales', + type: 'custom', + contract: 'Token', + role: 'provider', + }, + ]; + + const dbExecutors = new Map< + string, + (cypher: string, params?: Record) => Promise[]> + >([ + [ + 'parser/mathlex', + async (_cypher, params) => { + if (params?.contract === 'Token') { + return [{ uid: 'uid-token-first', name: 'Token', filePath: 'src/ast.rs' }]; + } + return []; + }, + ], + [ + 'engine/thales', + async (_cypher, params) => { + if (params?.contract === 'Token') { + return [{ uid: 'uid-token-consumer', name: 'Token', filePath: 'src/lexer.rs' }]; + } + return []; + }, + ], + ]); + + const result = await extractor.extractFromManifest(links, dbExecutors); + const provider = result.contracts.find((c) => c.role === 'provider'); + expect(provider?.symbolUid).toBe('uid-token-first'); + }); + it('returns empty for no links', async () => { const result = await extractor.extractFromManifest([]); expect(result.contracts).toHaveLength(0); From b9a17f553d21340718b9e450dd6b9424260ac8d2 Mon Sep 17 00:00:00 2001 From: "Christian C. Berclaz" Date: Sun, 3 May 2026 02:43:22 +0200 Subject: [PATCH 07/10] feat(group): auto-discover Rust workspace cross-crate contracts (#1256) --- gitnexus/src/core/group/config-parser.ts | 1 + .../extractors/rust-workspace-extractor.ts | 270 ++++++++++++++ gitnexus/src/core/group/sync.ts | 42 ++- gitnexus/src/core/group/types.ts | 1 + .../group/rust-workspace-extractor.test.ts | 335 ++++++++++++++++++ gitnexus/test/unit/group/sync.test.ts | 197 +++++++++- 6 files changed, 836 insertions(+), 10 deletions(-) create mode 100644 gitnexus/src/core/group/extractors/rust-workspace-extractor.ts create mode 100644 gitnexus/test/unit/group/rust-workspace-extractor.test.ts diff --git a/gitnexus/src/core/group/config-parser.ts b/gitnexus/src/core/group/config-parser.ts index cf2141311..d55969a73 100644 --- a/gitnexus/src/core/group/config-parser.ts +++ b/gitnexus/src/core/group/config-parser.ts @@ -13,6 +13,7 @@ const DEFAULT_DETECT = { topics: true, shared_libs: true, embedding_fallback: true, + workspace_deps: true, }; const DEFAULT_MATCHING = { diff --git a/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts b/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts new file mode 100644 index 000000000..c19af07ca --- /dev/null +++ b/gitnexus/src/core/group/extractors/rust-workspace-extractor.ts @@ -0,0 +1,270 @@ +import fs from 'node:fs/promises'; +import path from 'node:path'; +import type { CypherExecutor } from '../contract-extractor.js'; +import type { GroupManifestLink, ContractRole } from '../types.js'; +import { shouldIgnorePath } from '../../../config/ignore-service.js'; +import { loadIgnoreRules } from '../../../config/ignore-service.js'; + +/** + * Discover cross-crate contracts in a Rust workspace by reading each + * member's `Cargo.toml` dependencies and scanning source files for + * `use ::` imports. + * + * Emits `GroupManifestLink[]` with `type: 'custom'` that feed into the + * existing ManifestExtractor pipeline — no new matching logic needed. + * + * Designed for the group-level sync pipeline: it receives all repos in + * a group and produces cross-repo links between them. + */ + +interface CrateMeta { + name: string; + groupPath: string; + repoPath: string; + workspaceDeps: string[]; +} + +interface ImportedSymbol { + crateName: string; + symbolName: string; + filePath: string; +} + +/** + * Parse a Cargo.toml to extract the crate name and workspace dependency + * names. Uses simple line-based parsing — no TOML library needed for + * the subset we care about. + */ +async function parseCrateManifest( + repoPath: string, +): Promise<{ name: string; workspaceDeps: string[] } | null> { + const cargoPath = path.join(repoPath, 'Cargo.toml'); + let content: string; + try { + content = await fs.readFile(cargoPath, 'utf-8'); + } catch { + return null; + } + + let name = ''; + const workspaceDeps: string[] = []; + + const nameMatch = content.match(/^\[package\]\s*\n(?:[^\[]*?\n)*?name\s*=\s*"([^"]+)"/m); + if (nameMatch) name = nameMatch[1]; + + // Match dependencies that use workspace = true, which indicates they + // are workspace-internal deps: + // dep_name = { workspace = true } + // dep_name.workspace = true + // + // Also match plain path dependencies: + // dep_name = { path = "../other" } + const depSections = content.matchAll( + /\[(dependencies|dev-dependencies|build-dependencies)\]\s*\n([\s\S]*?)(?=\n\[|$)/g, + ); + + for (const section of depSections) { + const sectionBody = section[2]; + // workspace = true style + const wsMatches = sectionBody.matchAll( + /^(\w[\w-]*)\s*=\s*\{[^}]*workspace\s*=\s*true[^}]*\}/gm, + ); + for (const m of wsMatches) workspaceDeps.push(m[1]); + + // dotted workspace style: dep_name.workspace = true + const dottedMatches = sectionBody.matchAll(/^(\w[\w-]*)\.workspace\s*=\s*true/gm); + for (const m of dottedMatches) workspaceDeps.push(m[1]); + + // path = "../other" style (local path deps within workspace) + const pathMatches = sectionBody.matchAll( + /^(\w[\w-]*)\s*=\s*\{[^}]*path\s*=\s*"[^"]*"[^}]*\}/gm, + ); + for (const m of pathMatches) workspaceDeps.push(m[1]); + } + + if (!name) return null; + return { name, workspaceDeps: [...new Set(workspaceDeps)] }; +} + +/** + * Scan Rust source files for `use ::::` patterns + * where is a known workspace dependency. + */ +async function scanImports(repoPath: string, knownCrates: Set): Promise { + const results: ImportedSymbol[] = []; + + const normalizedCrates = new Map(); + for (const c of knownCrates) { + normalizedCrates.set(c.replace(/-/g, '_'), c); + } + + const sourceFiles = await findRustFiles(repoPath); + for (const relFile of sourceFiles) { + const absPath = path.join(repoPath, relFile); + let content: string; + try { + content = await fs.readFile(absPath, 'utf-8'); + } catch { + continue; + } + + // Match patterns: + // use crate_name::Type; + // use crate_name::module::Type; + // use crate_name::{Type1, Type2}; + // use crate_name::module::{Type1, Type2}; + const useRegex = /^use\s+(\w+)::(.+);/gm; + let match; + while ((match = useRegex.exec(content)) !== null) { + const crateName = match[1]; + const originalCrateName = normalizedCrates.get(crateName); + if (!originalCrateName) continue; + + const importPath = match[2].trim(); + + // Handle grouped imports: {Type1, Type2, module::Type3} + const braceMatch = importPath.match(/\{([^}]+)\}/); + if (braceMatch) { + const items = braceMatch[1].split(',').map((s) => s.trim()); + for (const item of items) { + const symbolName = extractSymbolName(item); + if (symbolName && isTypeName(symbolName)) { + results.push({ crateName: originalCrateName, symbolName, filePath: relFile }); + } + } + } else { + const symbolName = extractSymbolName(importPath); + if (symbolName && isTypeName(symbolName)) { + results.push({ crateName: originalCrateName, symbolName, filePath: relFile }); + } + } + } + } + + return results; +} + +/** Extract the final symbol name from a path like `module::submod::TypeName`. */ +function extractSymbolName(importPath: string): string | null { + const trimmed = importPath.trim(); + if (!trimmed || trimmed === '*' || trimmed === 'self') return null; + const parts = trimmed.split('::'); + return parts[parts.length - 1].trim() || null; +} + +/** + * Heuristic: in Rust, types (structs, enums, traits) are PascalCase. + * Functions and modules are snake_case. We only want types as cross-crate + * contracts — functions are too granular and modules too broad. + */ +function isTypeName(name: string): boolean { + return /^[A-Z][A-Za-z0-9]*$/.test(name); +} + +async function findRustFiles(repoPath: string): Promise { + const results: string[] = []; + const ig = await loadIgnoreRules(repoPath); + + async function walk(dir: string, rel: string): Promise { + let entries; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + const childRel = rel ? `${rel}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + if (shouldIgnorePath(childRel)) continue; + if (ig && ig.ignores(childRel + '/')) continue; + await walk(path.join(dir, entry.name), childRel); + } else if (entry.name.endsWith('.rs')) { + if (shouldIgnorePath(childRel)) continue; + if (ig && ig.ignores(childRel)) continue; + results.push(childRel); + } + } + } + + await walk(repoPath, ''); + return results; +} + +export interface RustWorkspaceResult { + links: GroupManifestLink[]; + discoveredCrates: Map; +} + +/** + * Discover cross-crate contracts across all Rust repos in a group. + * + * Returns `GroupManifestLink[]` ready to feed into `ManifestExtractor`. + */ +export async function extractRustWorkspaceLinks( + repos: Record, + repoPaths: Map, + _dbExecutors?: Map, +): Promise { + // Phase 1: Parse all Cargo.toml files to build crate registry + const cratesByName = new Map(); + const cratesByGroupPath = new Map(); + + for (const [groupPath] of Object.entries(repos)) { + const repoPath = repoPaths.get(groupPath); + if (!repoPath) continue; + + const manifest = await parseCrateManifest(repoPath); + if (!manifest) continue; + + const meta: CrateMeta = { + name: manifest.name, + groupPath, + repoPath, + workspaceDeps: manifest.workspaceDeps, + }; + const existing = cratesByName.get(manifest.name); + if (existing) { + console.warn( + `[rust-workspace-extractor] duplicate crate name "${manifest.name}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`, + ); + continue; + } + cratesByName.set(manifest.name, meta); + cratesByGroupPath.set(groupPath, meta); + } + + // Phase 2: For each crate, identify which of its workspace deps are + // also in this group (i.e., repos we can link to) + const links: GroupManifestLink[] = []; + const seen = new Set(); + + for (const [, crate] of cratesByGroupPath) { + const groupCrateDeps = crate.workspaceDeps.filter((d) => cratesByName.has(d)); + if (groupCrateDeps.length === 0) continue; + + // Phase 3: Scan source files for imports from workspace deps + const knownCrates = new Set(groupCrateDeps); + const imports = await scanImports(crate.repoPath, knownCrates); + + for (const imp of imports) { + const providerCrate = cratesByName.get(imp.crateName); + if (!providerCrate) continue; + + const qualifiedContract = `${imp.crateName}::${imp.symbolName}`; + const key = `${crate.groupPath}→${providerCrate.groupPath}::${qualifiedContract}`; + if (seen.has(key)) continue; + seen.add(key); + + const link: GroupManifestLink = { + from: providerCrate.groupPath, + to: crate.groupPath, + type: 'custom', + contract: qualifiedContract, + role: 'provider' as ContractRole, + }; + links.push(link); + } + } + + return { links, discoveredCrates: cratesByGroupPath }; +} diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index bd2590ecd..a9ecb51f4 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -8,6 +8,7 @@ import { HttpRouteExtractor } from './extractors/http-route-extractor.js'; import { GrpcExtractor } from './extractors/grpc-extractor.js'; import { TopicExtractor } from './extractors/topic-extractor.js'; import { ManifestExtractor } from './extractors/manifest-extractor.js'; +import { extractRustWorkspaceLinks } from './extractors/rust-workspace-extractor.js'; import { runExactMatch } from './matching.js'; import { detectServiceBoundaries, assignService } from './service-boundary-detector.js'; import type { CypherExecutor } from './contract-extractor.js'; @@ -84,12 +85,14 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis let autoContracts: StoredContract[] = []; let manifestCrossLinks: CrossLink[] = []; let dbExecutors: Map | undefined; + let registryEntries: RegistryEntry[] | undefined; const eo = opts?.extractorOverride; if (eo && eo.length === 0) { autoContracts = await (eo as () => Promise)(); } else { - const entries = await readRegistry(); + registryEntries = await readRegistry(); + const entries = registryEntries; const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries); const httpEx = new HttpRouteExtractor(); const grpcEx = new GrpcExtractor(); @@ -177,18 +180,39 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis } } - // Process manifest links declared in group.yaml. + // Auto-discover workspace dependency contracts (Rust Cargo workspaces, etc.) + // and merge them with explicit manifest links. Discovered links use the same + // ManifestExtractor pipeline as hand-written links in group.yaml. + let allLinks = [...config.links]; + + if (config.detect.workspace_deps) { + const repoPaths = new Map(); + if (!registryEntries) registryEntries = await readRegistry(); + for (const [groupPath, regName] of Object.entries(config.repos)) { + const e = registryEntries.find((en) => en.name === regName); + if (e) repoPaths.set(groupPath, e.path); + } + + const wsResult = await extractRustWorkspaceLinks(config.repos, repoPaths, dbExecutors); + if (wsResult.links.length > 0) { + allLinks = [...allLinks, ...wsResult.links]; + if (opts?.verbose) { + console.log( + ` workspace-deps: discovered ${wsResult.links.length} cross-crate links from ${wsResult.discoveredCrates.size} Rust crates`, + ); + } + } + } + + // Process manifest links declared in group.yaml (plus any auto-discovered). // ManifestExtractor is fully implemented but was never wired into this // pipeline — config.links were parsed and validated but silently dropped. // Placed after the DB try/finally: resolveSymbol falls back to synthetic // UIDs when dbExecutors is undefined or a pool is closed, so cross-links // are always generated regardless of whether real DB executors are available. - if (config.links.length > 0) { - // Warn about dangling links that reference repos not declared in config.repos. - // They still generate cross-links via synthetic UIDs (determinism is preserved), - // but the operator probably meant something that now silently does nothing useful. + if (allLinks.length > 0) { const knownRepos = new Set(Object.keys(config.repos)); - for (const link of config.links) { + for (const link of allLinks) { const dangling = [link.from, link.to].filter((r) => !knownRepos.has(r)); if (dangling.length > 0) { console.warn( @@ -198,12 +222,12 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis } const manifestEx = new ManifestExtractor(); - const manifestResult = await manifestEx.extractFromManifest(config.links, dbExecutors); + const manifestResult = await manifestEx.extractFromManifest(allLinks, dbExecutors); autoContracts.push(...manifestResult.contracts); manifestCrossLinks = manifestResult.crossLinks; if (opts?.verbose) { console.log( - ` manifest: ${manifestCrossLinks.length} cross-links from ${config.links.length} declared links`, + ` manifest: ${manifestCrossLinks.length} cross-links from ${allLinks.length} links (${config.links.length} declared + ${allLinks.length - config.links.length} discovered)`, ); } } diff --git a/gitnexus/src/core/group/types.ts b/gitnexus/src/core/group/types.ts index 64ce53143..895bef6dc 100644 --- a/gitnexus/src/core/group/types.ts +++ b/gitnexus/src/core/group/types.ts @@ -27,6 +27,7 @@ export interface DetectConfig { topics: boolean; shared_libs: boolean; embedding_fallback: boolean; + workspace_deps: boolean; } export interface MatchingConfig { diff --git a/gitnexus/test/unit/group/rust-workspace-extractor.test.ts b/gitnexus/test/unit/group/rust-workspace-extractor.test.ts new file mode 100644 index 000000000..a9e277c36 --- /dev/null +++ b/gitnexus/test/unit/group/rust-workspace-extractor.test.ts @@ -0,0 +1,335 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; +import { extractRustWorkspaceLinks } from '../../../src/core/group/extractors/rust-workspace-extractor.js'; + +describe('RustWorkspaceExtractor', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-rust-ws-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + async function writeFile(relPath: string, content: string) { + const absPath = path.join(tmpDir, relPath); + await fs.mkdir(path.dirname(absPath), { recursive: true }); + await fs.writeFile(absPath, content, 'utf-8'); + } + + it('discovers cross-crate imports from workspace dependencies', async () => { + // Crate A: defines Expression + await writeFile( + 'crate-a/Cargo.toml', + `[package]\nname = "mathlex"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('crate-a/src/lib.rs', 'pub struct Expression {}\npub struct Token {}\n'); + + // Crate B: depends on A via workspace, imports Expression + await writeFile( + 'crate-b/Cargo.toml', + `[package]\nname = "thales"\nversion = "0.1.0"\n\n[dependencies]\nmathlex = { workspace = true }\n`, + ); + await writeFile('crate-b/src/main.rs', 'use mathlex::Expression;\nfn eval(e: Expression) {}\n'); + + const repos = { + 'parser/mathlex': 'mathlex', + 'engine/thales': 'thales', + }; + const repoPaths = new Map([ + ['parser/mathlex', path.join(tmpDir, 'crate-a')], + ['engine/thales', path.join(tmpDir, 'crate-b')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(1); + expect(result.links[0]).toEqual({ + from: 'parser/mathlex', + to: 'engine/thales', + type: 'custom', + contract: 'mathlex::Expression', + role: 'provider', + }); + }); + + it('handles hyphenated crate names (converted to underscores in use statements)', async () => { + await writeFile( + 'units/Cargo.toml', + `[package]\nname = "mathcore-units"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('units/src/lib.rs', 'pub struct Unit {}\npub struct Dimension {}\n'); + + await writeFile( + 'engine/Cargo.toml', + `[package]\nname = "thales"\nversion = "0.1.0"\n\n[dependencies]\nmathcore-units = { workspace = true }\n`, + ); + await writeFile( + 'engine/src/main.rs', + 'use mathcore_units::Unit;\nuse mathcore_units::Dimension;\n', + ); + + const repos = { + 'core/units': 'mathcore-units', + 'engine/thales': 'thales', + }; + const repoPaths = new Map([ + ['core/units', path.join(tmpDir, 'units')], + ['engine/thales', path.join(tmpDir, 'engine')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(2); + const contracts = result.links.map((l) => l.contract).sort(); + expect(contracts).toEqual(['mathcore-units::Dimension', 'mathcore-units::Unit']); + }); + + it('handles grouped imports (use crate::{Type1, Type2})', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "shared"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', 'pub struct Foo {}\npub struct Bar {}\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nshared = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use shared::{Foo, Bar};\n'); + + const repos = { lib: 'shared', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(2); + const contracts = result.links.map((l) => l.contract).sort(); + expect(contracts).toEqual(['shared::Bar', 'shared::Foo']); + }); + + it('ignores snake_case imports (functions/modules, not types)', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "utils"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', 'pub fn helper() {}\npub struct Config {}\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nutils = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use utils::helper;\nuse utils::Config;\n'); + + const repos = { lib: 'utils', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(1); + expect(result.links[0].contract).toBe('utils::Config'); + }); + + it('skips repos without Cargo.toml', async () => { + await writeFile('js-app/package.json', '{"name": "js-app"}'); + await writeFile('js-app/src/index.ts', 'export const x = 1;'); + + const repos = { app: 'js-app' }; + const repoPaths = new Map([['app', path.join(tmpDir, 'js-app')]]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(0); + expect(result.discoveredCrates.size).toBe(0); + }); + + it('deduplicates identical imports from multiple files', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "shared"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', 'pub struct Config {}\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nshared = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use shared::Config;\n'); + await writeFile('app/src/other.rs', 'use shared::Config;\n'); + + const repos = { lib: 'shared', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(1); + }); + + it('handles path dependencies alongside workspace deps', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "mylib"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', 'pub trait Handler {}\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nmylib = { path = "../lib" }\n`, + ); + await writeFile('app/src/main.rs', 'use mylib::Handler;\n'); + + const repos = { lib: 'mylib', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(1); + expect(result.links[0].contract).toBe('mylib::Handler'); + }); + + it('warns and skips duplicate crate names', async () => { + await writeFile( + 'repo-a/Cargo.toml', + `[package]\nname = "shared"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('repo-a/src/lib.rs', 'pub struct Alpha {}\n'); + + await writeFile( + 'repo-b/Cargo.toml', + `[package]\nname = "shared"\nversion = "0.2.0"\n\n[dependencies]\n`, + ); + await writeFile('repo-b/src/lib.rs', 'pub struct Beta {}\n'); + + await writeFile( + 'consumer/Cargo.toml', + `[package]\nname = "consumer"\nversion = "0.1.0"\n\n[dependencies]\nshared = { workspace = true }\n`, + ); + await writeFile('consumer/src/main.rs', 'use shared::Alpha;\n'); + + const repos = { a: 'shared-a', b: 'shared-b', consumer: 'consumer' }; + const repoPaths = new Map([ + ['a', path.join(tmpDir, 'repo-a')], + ['b', path.join(tmpDir, 'repo-b')], + ['consumer', path.join(tmpDir, 'consumer')], + ]); + + const warnings: string[] = []; + const origWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(String(args[0])); + }; + try { + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(warnings.some((w) => w.includes('duplicate crate name "shared"'))).toBe(true); + expect(result.links).toHaveLength(1); + expect(result.links[0].from).toBe('a'); + } finally { + console.warn = origWarn; + } + }); + + it('produces distinct contracts when two crates export same symbol name', async () => { + await writeFile( + 'lib-a/Cargo.toml', + `[package]\nname = "alpha"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib-a/src/lib.rs', 'pub struct Config {}\n'); + + await writeFile( + 'lib-b/Cargo.toml', + `[package]\nname = "beta"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib-b/src/lib.rs', 'pub struct Config {}\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nalpha = { workspace = true }\nbeta = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use alpha::Config;\nuse beta::Config;\n'); + + const repos = { alpha: 'alpha', beta: 'beta', app: 'myapp' }; + const repoPaths = new Map([ + ['alpha', path.join(tmpDir, 'lib-a')], + ['beta', path.join(tmpDir, 'lib-b')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(2); + const contracts = result.links.map((l) => l.contract).sort(); + expect(contracts).toEqual(['alpha::Config', 'beta::Config']); + }); + + it('respects .gitnexusignore patterns', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "mylib"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', 'pub struct Real {}\n'); + await writeFile('lib/generated/gen.rs', 'pub struct Fake {}\n'); + await writeFile('lib/.gitnexusignore', 'generated/\n'); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nmylib = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use mylib::Real;\n'); + await writeFile('app/generated/gen.rs', 'use mylib::Fake;\n'); + await writeFile('app/.gitnexusignore', 'generated/\n'); + + const repos = { lib: 'mylib', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(1); + expect(result.links[0].contract).toBe('mylib::Real'); + }); + + it('handles nested module imports (use crate::module::Type)', async () => { + await writeFile( + 'lib/Cargo.toml', + `[package]\nname = "shared"\nversion = "0.1.0"\n\n[dependencies]\n`, + ); + await writeFile('lib/src/lib.rs', ''); + + await writeFile( + 'app/Cargo.toml', + `[package]\nname = "myapp"\nversion = "0.1.0"\n\n[dependencies]\nshared = { workspace = true }\n`, + ); + await writeFile('app/src/main.rs', 'use shared::models::User;\nuse shared::auth::Token;\n'); + + const repos = { lib: 'shared', app: 'myapp' }; + const repoPaths = new Map([ + ['lib', path.join(tmpDir, 'lib')], + ['app', path.join(tmpDir, 'app')], + ]); + + const result = await extractRustWorkspaceLinks(repos, repoPaths); + + expect(result.links).toHaveLength(2); + const contracts = result.links.map((l) => l.contract).sort(); + expect(contracts).toEqual(['shared::Token', 'shared::User']); + }); +}); diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 5aa586c25..87a7c6645 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; @@ -335,6 +335,201 @@ describe('syncGroup', () => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + describe('workspace_deps integration', () => { + let tmpDir: string; + + function makeWsConfig(repos: Record, workspaceDeps: boolean): GroupConfig { + return { + version: 1, + name: 'test', + description: '', + repos, + links: [], + packages: {}, + detect: { + http: false, + grpc: false, + topics: false, + shared_libs: false, + embedding_fallback: false, + workspace_deps: workspaceDeps, + }, + matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 }, + }; + } + + function writeFileSync(relPath: string, content: string) { + const absPath = path.join(tmpDir, relPath); + fs.mkdirSync(path.dirname(absPath), { recursive: true }); + fs.writeFileSync(absPath, content, 'utf-8'); + } + + afterEach(() => { + vi.restoreAllMocks(); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('workspace_deps: true discovers Rust crate links through syncGroup', async () => { + tmpDir = path.join(os.tmpdir(), `gitnexus-sync-ws-${Date.now()}`); + fs.mkdirSync(tmpDir, { recursive: true }); + + writeFileSync( + 'crate-a/Cargo.toml', + '[package]\nname = "mathlex"\nversion = "0.1.0"\n\n[dependencies]\n', + ); + writeFileSync('crate-a/src/lib.rs', 'pub struct Expression {}\n'); + + writeFileSync( + 'crate-b/Cargo.toml', + '[package]\nname = "thales"\nversion = "0.1.0"\n\n[dependencies]\nmathlex = { workspace = true }\n', + ); + writeFileSync('crate-b/src/main.rs', 'use mathlex::Expression;\n'); + + const mockEntries: RegistryEntry[] = [ + { + name: 'mathlex', + path: path.join(tmpDir, 'crate-a'), + storagePath: path.join(tmpDir, 'crate-a', '.gitnexus'), + indexedAt: '', + lastCommit: '', + }, + { + name: 'thales', + path: path.join(tmpDir, 'crate-b'), + storagePath: path.join(tmpDir, 'crate-b', '.gitnexus'), + indexedAt: '', + lastCommit: '', + }, + ]; + + const repoManager = await import('../../../src/storage/repo-manager.js'); + vi.spyOn(repoManager, 'readRegistry').mockResolvedValue(mockEntries); + + const config = makeWsConfig({ 'parser/mathlex': 'mathlex', 'engine/thales': 'thales' }, true); + + const result = await syncGroup(config, { + extractorOverride: async () => [], + skipWrite: true, + }); + + const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest'); + expect(manifestLinks).toHaveLength(1); + expect(manifestLinks[0].contractId).toBe('custom::mathlex::Expression'); + expect(manifestLinks[0].from.repo).toBe('engine/thales'); + expect(manifestLinks[0].to.repo).toBe('parser/mathlex'); + }); + + it('workspace_deps: false skips Rust workspace extraction', async () => { + tmpDir = path.join(os.tmpdir(), `gitnexus-sync-ws-off-${Date.now()}`); + fs.mkdirSync(tmpDir, { recursive: true }); + + writeFileSync( + 'crate-a/Cargo.toml', + '[package]\nname = "mathlex"\nversion = "0.1.0"\n\n[dependencies]\n', + ); + writeFileSync('crate-a/src/lib.rs', 'pub struct Expression {}\n'); + + writeFileSync( + 'crate-b/Cargo.toml', + '[package]\nname = "thales"\nversion = "0.1.0"\n\n[dependencies]\nmathlex = { workspace = true }\n', + ); + writeFileSync('crate-b/src/main.rs', 'use mathlex::Expression;\n'); + + const repoManager = await import('../../../src/storage/repo-manager.js'); + vi.spyOn(repoManager, 'readRegistry').mockResolvedValue([]); + + const config = makeWsConfig( + { 'parser/mathlex': 'mathlex', 'engine/thales': 'thales' }, + false, + ); + + const result = await syncGroup(config, { + extractorOverride: async () => [], + skipWrite: true, + }); + + expect(result.crossLinks).toHaveLength(0); + expect(result.contracts).toHaveLength(0); + }); + + it('discovered workspace links merge with explicit manifest links', async () => { + tmpDir = path.join(os.tmpdir(), `gitnexus-sync-ws-merge-${Date.now()}`); + fs.mkdirSync(tmpDir, { recursive: true }); + + writeFileSync( + 'crate-a/Cargo.toml', + '[package]\nname = "mathlex"\nversion = "0.1.0"\n\n[dependencies]\n', + ); + writeFileSync('crate-a/src/lib.rs', 'pub struct Expression {}\n'); + + writeFileSync( + 'crate-b/Cargo.toml', + '[package]\nname = "thales"\nversion = "0.1.0"\n\n[dependencies]\nmathlex = { workspace = true }\n', + ); + writeFileSync('crate-b/src/main.rs', 'use mathlex::Expression;\n'); + + const mockEntries: RegistryEntry[] = [ + { + name: 'mathlex', + path: path.join(tmpDir, 'crate-a'), + storagePath: path.join(tmpDir, 'crate-a', '.gitnexus'), + indexedAt: '', + lastCommit: '', + }, + { + name: 'thales', + path: path.join(tmpDir, 'crate-b'), + storagePath: path.join(tmpDir, 'crate-b', '.gitnexus'), + indexedAt: '', + lastCommit: '', + }, + ]; + + const repoManager = await import('../../../src/storage/repo-manager.js'); + vi.spyOn(repoManager, 'readRegistry').mockResolvedValue(mockEntries); + + const explicitLinks: GroupManifestLink[] = [ + { + from: 'parser/mathlex', + to: 'engine/thales', + type: 'http', + contract: 'GET::/api/parse', + role: 'provider', + }, + ]; + + const config: GroupConfig = { + version: 1, + name: 'test', + description: '', + repos: { 'parser/mathlex': 'mathlex', 'engine/thales': 'thales' }, + links: explicitLinks, + packages: {}, + detect: { + http: false, + grpc: false, + topics: false, + shared_libs: false, + embedding_fallback: false, + workspace_deps: true, + }, + matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 }, + }; + + const result = await syncGroup(config, { + extractorOverride: async () => [], + skipWrite: true, + }); + + const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest'); + expect(manifestLinks.length).toBeGreaterThanOrEqual(2); + + const contractIds = manifestLinks.map((cl) => cl.contractId); + expect(contractIds).toContain('http::GET::/api/parse'); + expect(contractIds).toContain('custom::mathlex::Expression'); + }); + }); }); describe('stableRepoPoolId', () => { From db22a89021673aa14e3238c825b6da7556e3a74e Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sun, 3 May 2026 07:37:05 +0100 Subject: [PATCH 08/10] fix(embeddings): bridge HF_ENDPOINT env var to transformers.js env.remoteHost (#1205) (#1252) --- gitnexus/src/core/embeddings/embedder.ts | 12 ++-- gitnexus/src/core/embeddings/hf-env.ts | 62 +++++++++++++++++ gitnexus/src/mcp/core/embedder.ts | 13 ++-- gitnexus/test/unit/hf-env.test.ts | 84 ++++++++++++++++++++++++ 4 files changed, 158 insertions(+), 13 deletions(-) create mode 100644 gitnexus/src/core/embeddings/hf-env.ts create mode 100644 gitnexus/test/unit/hf-env.test.ts diff --git a/gitnexus/src/core/embeddings/embedder.ts b/gitnexus/src/core/embeddings/embedder.ts index caab198b4..0d7fe41df 100644 --- a/gitnexus/src/core/embeddings/embedder.ts +++ b/gitnexus/src/core/embeddings/embedder.ts @@ -15,7 +15,6 @@ if (!process.env.ORT_LOG_LEVEL) { } import { pipeline, env, type FeatureExtractionPipeline } from '@huggingface/transformers'; -import os from 'os'; import { existsSync } from 'fs'; import { execFileSync } from 'child_process'; import { join, dirname } from 'path'; @@ -23,6 +22,7 @@ import { createRequire } from 'module'; import { DEFAULT_EMBEDDING_CONFIG, type EmbeddingConfig, type ModelProgress } from './types.js'; import { isHttpMode, getHttpDimensions, httpEmbed } from './http-client.js'; import { resolveEmbeddingConfig } from './config.js'; +import { applyHfEnvOverrides } from './hf-env.js'; /** * Check whether the onnxruntime-node package that @huggingface/transformers @@ -158,11 +158,11 @@ export const initEmbedder = async ( try { // Configure transformers.js environment env.allowLocalModels = false; - // Default cache to user-writable location. transformers.js defaults to - // ./node_modules/.cache inside its own install dir, which is unwritable - // when gitnexus is installed globally (e.g. /usr/lib/node_modules/). - // Respect HF_HOME if set, otherwise fall back to ~/.cache/huggingface. - env.cacheDir = process.env.HF_HOME ?? join(os.homedir(), '.cache', 'huggingface'); + // Bridge user-controlled env vars to transformers.js: HF_HOME → + // env.cacheDir, HF_ENDPOINT → env.remoteHost (#1205). Centralised in + // applyHfEnvOverrides so the MCP embedder entry point behaves + // identically. + applyHfEnvOverrides(env); const isDev = process.env.NODE_ENV === 'development'; if (isDev) { diff --git a/gitnexus/src/core/embeddings/hf-env.ts b/gitnexus/src/core/embeddings/hf-env.ts new file mode 100644 index 000000000..6a977a76d --- /dev/null +++ b/gitnexus/src/core/embeddings/hf-env.ts @@ -0,0 +1,62 @@ +import os from 'node:os'; +import { join } from 'node:path'; + +/** + * @internal Exported only for unit tests and the two embedder entry points + * (`core/embeddings/embedder.ts` + `mcp/core/embedder.ts`). Not part of the + * public package API. + * + * Minimal subset of `@huggingface/transformers`' `env` object that gitnexus + * mutates. Defining a local structural type keeps this helper free of a + * transitive dependency on transformers' generated `.d.ts` while still + * giving full type-checking on the two fields we actually touch. + */ +export interface HfEnvSubset { + cacheDir: string; + remoteHost: string; +} + +/** + * @internal Exported only for unit tests and the two embedder entry points + * (`core/embeddings/embedder.ts` + `mcp/core/embedder.ts`). Not part of the + * public package API. + * + * Apply user-controlled HuggingFace environment overrides to the + * `@huggingface/transformers` `env` object. Centralises the two env-var + * bridges so every gitnexus embedder entry point (the analyze pipeline + * and the MCP server) behaves identically. + * + * - **`HF_HOME`** → `env.cacheDir` (default: `~/.cache/huggingface`). + * transformers.js otherwise defaults to `./node_modules/.cache` inside + * its own install dir, which is unwritable when gitnexus is installed + * globally (e.g. `/usr/lib/node_modules/`). + * + * - **`HF_ENDPOINT`** → `env.remoteHost` (#1205). transformers.js does + * not read `HF_ENDPOINT` on its own — it reads `env.remoteHost` — + * even though `HF_ENDPOINT` is the standard env var the upstream + * `huggingface_hub` Python client and the official HF mirror docs + * tell users to set. Bridging the two unblocks `--embeddings` for + * users behind networks where `huggingface.co` is unreachable + * (corporate proxies, the GFW, air-gapped mirrors). The trailing + * slash is normalised because transformers.js builds URLs by string + * concatenation and a missing slash silently falls through to its + * default `huggingface.co/...` host. + * + * Mutation rather than return-and-apply because callers already hold a + * reference to the live `env` object imported from + * `@huggingface/transformers` — passing the same reference in keeps the + * call site a single line at each entry point. + */ +export function applyHfEnvOverrides(env: HfEnvSubset): void { + env.cacheDir = process.env.HF_HOME ?? join(os.homedir(), '.cache', 'huggingface'); + // `.trim()` guards against the common copy-paste failure mode of + // `HF_ENDPOINT=" https://hf-mirror.com "` (leading/trailing whitespace + // from shell scripts or docs) — without it, a whitespace-only value + // would be truthy and produce an invalid `env.remoteHost = ' /'` that + // silently misroutes downloads. Empty string remains falsy in JS so the + // truthy guard already handles the unset/empty cases. + const endpoint = process.env.HF_ENDPOINT?.trim(); + if (endpoint) { + env.remoteHost = endpoint.endsWith('/') ? endpoint : endpoint + '/'; + } +} diff --git a/gitnexus/src/mcp/core/embedder.ts b/gitnexus/src/mcp/core/embedder.ts index b01755928..f506cdead 100644 --- a/gitnexus/src/mcp/core/embedder.ts +++ b/gitnexus/src/mcp/core/embedder.ts @@ -6,14 +6,13 @@ */ import { pipeline, env, type FeatureExtractionPipeline } from '@huggingface/transformers'; -import os from 'os'; -import { join } from 'path'; import { isHttpMode, getHttpDimensions, httpEmbedQuery, } from '../../core/embeddings/http-client.js'; import { resolveEmbeddingConfig } from '../../core/embeddings/config.js'; +import { applyHfEnvOverrides } from '../../core/embeddings/hf-env.js'; import { silenceStdout, restoreStdout, realStderrWrite } from '../../core/lbug/pool-adapter.js'; // Model config @@ -45,11 +44,11 @@ export const initEmbedder = async (): Promise => { initPromise = (async () => { try { env.allowLocalModels = false; - // Default cache to user-writable location. transformers.js defaults to - // ./node_modules/.cache inside its own install dir, which is unwritable - // when gitnexus is installed globally (e.g. /usr/lib/node_modules/). - // Respect HF_HOME if set, otherwise fall back to ~/.cache/huggingface. - env.cacheDir = process.env.HF_HOME ?? join(os.homedir(), '.cache', 'huggingface'); + // Bridge user-controlled env vars to transformers.js: HF_HOME → + // env.cacheDir, HF_ENDPOINT → env.remoteHost (#1205). Centralised in + // applyHfEnvOverrides so this MCP entry point behaves identically to + // the analyze pipeline embedder. + applyHfEnvOverrides(env); const embeddingConfig = resolveEmbeddingConfig(); console.error('GitNexus: Loading embedding model (first search may take a moment)...'); diff --git a/gitnexus/test/unit/hf-env.test.ts b/gitnexus/test/unit/hf-env.test.ts new file mode 100644 index 000000000..6a5697059 --- /dev/null +++ b/gitnexus/test/unit/hf-env.test.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import os from 'node:os'; +import { join } from 'node:path'; +import { applyHfEnvOverrides, type HfEnvSubset } from '../../src/core/embeddings/hf-env.js'; + +describe('applyHfEnvOverrides', () => { + let envStub: HfEnvSubset; + // Snapshot the two env vars so tests don't leak state into each other (or + // into the rest of the test run). `delete` + restore is the simplest pattern + // — vitest doesn't reset `process.env` between tests by default. + let originalHfHome: string | undefined; + let originalHfEndpoint: string | undefined; + + beforeEach(() => { + envStub = { cacheDir: '', remoteHost: '' }; + originalHfHome = process.env.HF_HOME; + originalHfEndpoint = process.env.HF_ENDPOINT; + delete process.env.HF_HOME; + delete process.env.HF_ENDPOINT; + }); + + afterEach(() => { + if (originalHfHome === undefined) delete process.env.HF_HOME; + else process.env.HF_HOME = originalHfHome; + if (originalHfEndpoint === undefined) delete process.env.HF_ENDPOINT; + else process.env.HF_ENDPOINT = originalHfEndpoint; + }); + + it('cacheDir defaults to ~/.cache/huggingface when HF_HOME is unset', () => { + applyHfEnvOverrides(envStub); + expect(envStub.cacheDir).toBe(join(os.homedir(), '.cache', 'huggingface')); + }); + + it('cacheDir respects HF_HOME when set', () => { + process.env.HF_HOME = '/custom/hf/cache'; + applyHfEnvOverrides(envStub); + expect(envStub.cacheDir).toBe('/custom/hf/cache'); + }); + + it('remoteHost is set when HF_ENDPOINT is set, with a trailing slash appended', () => { + process.env.HF_ENDPOINT = 'https://hf-mirror.com'; + applyHfEnvOverrides(envStub); + expect(envStub.remoteHost).toBe('https://hf-mirror.com/'); + }); + + it('remoteHost preserves existing trailing slash on HF_ENDPOINT', () => { + process.env.HF_ENDPOINT = 'https://hf-mirror.com/'; + applyHfEnvOverrides(envStub); + expect(envStub.remoteHost).toBe('https://hf-mirror.com/'); + }); + + it('remoteHost is left untouched when HF_ENDPOINT is unset', () => { + // Pre-populate to a sentinel so we can prove the function does NOT + // overwrite remoteHost when no env var is set. Without this guard a + // future refactor that always assigns `env.remoteHost = ...` would + // silently break consumers that have already configured it elsewhere. + envStub.remoteHost = 'pre-existing-do-not-touch'; + applyHfEnvOverrides(envStub); + expect(envStub.remoteHost).toBe('pre-existing-do-not-touch'); + }); + + it('remoteHost is left untouched when HF_ENDPOINT is whitespace-only', () => { + // Common copy-paste failure mode for users on restricted networks who + // pull `HF_ENDPOINT` values from shell scripts or docs with stray + // whitespace. The `.trim()` + truthiness guard ensures this is treated + // as "unset" rather than as an invalid host like `' /'` that would + // silently misroute model downloads. Pinned by the @claude review on + // PR #1252. + process.env.HF_ENDPOINT = ' '; + envStub.remoteHost = 'sentinel'; + applyHfEnvOverrides(envStub); + expect(envStub.remoteHost).toBe('sentinel'); + }); + + it('remoteHost trims surrounding whitespace from HF_ENDPOINT', () => { + // Compatible mirror of the previous test for the case where the env + // var is non-empty AFTER trimming. Without `.trim()`, the bogus + // leading/trailing space would survive into the URL and break + // downloads. + process.env.HF_ENDPOINT = ' https://hf-mirror.com '; + applyHfEnvOverrides(envStub); + expect(envStub.remoteHost).toBe('https://hf-mirror.com/'); + }); +}); From 1fe3bf939981c0b7591ca13503c62083a43bb11e Mon Sep 17 00:00:00 2001 From: Temirkhan <99467693+stemirkhan@users.noreply.github.com> Date: Sun, 3 May 2026 11:06:41 +0300 Subject: [PATCH 09/10] feat(mcp): add tool safety annotations (#1127) * feat(mcp): add tool safety annotations * test(mcp): address PR #1127 review follow-ups - Replace private `_requestHandlers` SDK access in server.test.ts with `Client` + `InMemoryTransport.createLinkedPair()` for the tools/list annotation propagation test. The new path uses supported public APIs and surfaces SDK changes loudly instead of silently degrading. - Extract `OPEN_WORLD_READ_ONLY_TOOLS` set in tools.test.ts so future read-only open-world tools can be added without rewriting the invariant; preserves the current "only `query` is open-world" guard. - Add inline rationale on `group_sync` annotations explaining the conservative `idempotentHint: false` (writes contracts.json on every call even when output is deterministic). No runtime behavior change. Annotations themselves and tools/list shape are unchanged. --------- Co-authored-by: Gergo Magyar --- gitnexus/src/mcp/server.ts | 1 + gitnexus/src/mcp/tools.ts | 39 +++++++++++++++++++++++++ gitnexus/test/unit/server.test.ts | 25 ++++++++++++++++ gitnexus/test/unit/tools.test.ts | 47 +++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+) diff --git a/gitnexus/src/mcp/server.ts b/gitnexus/src/mcp/server.ts index 4d540b8a2..be80f4c35 100644 --- a/gitnexus/src/mcp/server.ts +++ b/gitnexus/src/mcp/server.ts @@ -158,6 +158,7 @@ export function createMCPServer(backend: LocalBackend): Server { name: tool.name, description: tool.description, inputSchema: tool.inputSchema, + annotations: tool.annotations, })), })); diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 491c24557..a85298c04 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -5,9 +5,12 @@ * All tools support an optional `repo` parameter for multi-repo setups. */ +import type { ToolAnnotations } from '@modelcontextprotocol/sdk/types.js'; + export interface ToolDefinition { name: string; description: string; + annotations: ToolAnnotations; inputSchema: { type: 'object'; properties: Record< @@ -27,6 +30,27 @@ export interface ToolDefinition { }; } +const READ_ONLY_TOOL_ANNOTATIONS: ToolAnnotations = { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, +}; + +const QUERY_TOOL_ANNOTATIONS: ToolAnnotations = { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, +}; + +const DESTRUCTIVE_TOOL_ANNOTATIONS: ToolAnnotations = { + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: false, +}; + export const GITNEXUS_TOOLS: ToolDefinition[] = [ { name: 'list_repos', @@ -39,6 +63,7 @@ AFTER THIS: READ gitnexus://repo/{name}/context for the repo you want to work wi When multiple repos are indexed, you MUST specify the "repo" parameter on other tools (query, context, impact, etc.) to target the correct one.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: {}, @@ -63,6 +88,7 @@ Hybrid ranking: BM25 keyword + semantic vector search, ranked by Reciprocal Rank GROUP MODE: set "repo" to "@" to search all member repos in that group (merged via RRF), or "@/" to run against a single member (same path keys as in group.yaml). If you use "@" only, the member repo defaults to the lexicographically first key in group.yaml "repos". Prefer resources for contracts/status (see migration from legacy group_* tools). SERVICE: optional monorepo path prefix (POSIX-style, case-sensitive segments). When "repo" starts with "@", only processes whose symbols fall under that prefix are included. For a normal indexed repo name (no leading @), this field is currently ignored by the server.`, + annotations: QUERY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -156,6 +182,7 @@ TIPS: - Community = auto-detected functional area (Leiden algorithm). Properties: heuristicLabel, cohesion, symbolCount, keywords, description, enrichedBy - Process = execution flow trace from entry point to terminal. Properties: heuristicLabel, processType, stepCount, communities, entryPointId, terminalId - Use heuristicLabel (not label) for human-readable community/process names`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -183,6 +210,7 @@ NOTE: ACCESSES edges (field read/write tracking) are included in context results GROUP MODE: set "repo" to "@" to run context in each member repo (aggregated list), or "@/" for one member. If you use "@" only, the member defaults to the lexicographically first key in group.yaml "repos". SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", prefix-matches resolved symbol file paths; when a hit is outside the prefix, that member returns an empty payload for the symbol. Ignored for a normal indexed repo name.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -226,6 +254,7 @@ WHEN TO USE: Before committing — to understand what your changes affect. Pre-c AFTER THIS: Review affected processes. Use context() on high-risk symbols. READ gitnexus://repo/{name}/process/{name} for full traces. Returns: changed symbols, affected processes, and a risk summary.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -258,6 +287,7 @@ AFTER THIS: Run detect_changes() to verify no unexpected side effects. Each edit is tagged with confidence: - "graph": found via knowledge graph relationships (high confidence, safe to accept) - "text_search": found via regex text search (lower confidence, review carefully)`, + annotations: DESTRUCTIVE_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -311,6 +341,7 @@ Confidence: 1.0 = certain, <0.8 = fuzzy match GROUP MODE: set "repo" to "@" for cross-repo impact anchored at the default member (lexicographically first key in group.yaml "repos"), or "@/" to choose the member (same path keys as in group.yaml). Phase-1 walk runs in that member; cross-boundary fan-out uses the group bridge. SERVICE: optional monorepo path prefix (case-sensitive path segments). When "repo" starts with "@", scopes the local impact walk and cross-repo symbol paths to files under that prefix; ignored for a normal indexed repo name.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -404,6 +435,7 @@ WHEN TO USE: Understanding API consumption patterns, finding orphaned routes. Fo AFTER THIS: Use impact() on specific route handlers to see full blast radius. Returns: route nodes with their handlers, middleware wrapper chains (e.g., withAuth, withRateLimit), and consumers.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -426,6 +458,7 @@ Returns: route nodes with their handlers, middleware wrapper chains (e.g., withA WHEN TO USE: Understanding tool APIs, finding tool implementations, impact analysis for tool changes. Returns: tool nodes with their handler files and descriptions.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -443,6 +476,7 @@ WHEN TO USE: Detecting mismatches between what an API route returns and what con REQUIRES: Route nodes with responseKeys (extracted from .json({...}) calls during indexing). Returns routes that have both detected response keys AND consumers. Shows top-level keys each endpoint returns (e.g., data, pagination, error) and what keys each consumer accesses. Reports MISMATCH status when a consumer accesses keys not present in the route's response shape.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -467,6 +501,7 @@ WHEN TO USE: BEFORE modifying any API route handler. Shows what consumers depend Risk levels: LOW (0-3 consumers), MEDIUM (4-9 or any mismatches), HIGH (10+ consumers or mismatches with 4+ consumers). Mismatches with confidence "low" indicate the consumer file fetches multiple routes — property attribution is approximate. Returns: single route object when one match, or { routes: [...], total: N } for multiple matches. Combines route_map, shape_check, and impact data.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -482,6 +517,7 @@ Returns: single route object when one match, or { routes: [...], total: N } for description: `List all configured repository groups, or return details for one group (repos, manifest links). WHEN TO USE: Discover groups before group_sync. Optional "name" returns a single group's config.`, + annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { @@ -495,6 +531,9 @@ WHEN TO USE: Discover groups before group_sync. Optional "name" returns a single description: `Rebuild the Contract Registry (contracts.json) for a group: extract HTTP contracts, apply manifest links, exact-match cross-links. WHEN TO USE: After changing group.yaml or re-indexing member repos.`, + // Writes contracts.json on every call; conservatively non-idempotent + // even though output is deterministic for identical input. + annotations: DESTRUCTIVE_TOOL_ANNOTATIONS, inputSchema: { type: 'object', properties: { diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts index c556b48cc..e762c58ee 100644 --- a/gitnexus/test/unit/server.test.ts +++ b/gitnexus/test/unit/server.test.ts @@ -13,7 +13,10 @@ * directly through the MCP Server's handler dispatch. */ import { describe, it, expect, vi } from 'vitest'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { createMCPServer } from '../../src/mcp/server.js'; +import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js'; // ─── Mock backend ────────────────────────────────────────────────── @@ -52,6 +55,28 @@ describe('createMCPServer', () => { // The server has registered handlers — verify it was created without errors expect(server).toBeTruthy(); }); + + it('tools/list response includes tool annotations', async () => { + const backend = createMockBackend(); + const server = createMCPServer(backend); + const client = new Client({ name: 'test-client', version: '0.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + + try { + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + + const response = await client.listTools(); + expect(response.tools).toHaveLength(GITNEXUS_TOOLS.length); + + for (const tool of response.tools) { + const definition = GITNEXUS_TOOLS.find((t) => t.name === tool.name)!; + expect(tool.annotations).toEqual(definition.annotations); + } + } finally { + await client.close(); + await server.close(); + } + }); }); // ─── getNextStepHint (tested indirectly via server tool handler) ────── diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 231f55e3c..a9ce5cf25 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -11,6 +11,10 @@ import { describe, it, expect } from 'vitest'; import { GITNEXUS_TOOLS } from '../../src/mcp/tools.js'; const GROUP_TOOLS = new Set(['group_list', 'group_sync']); +const MUTATING_TOOLS = new Set(['rename', 'group_sync']); +// Read-only tools that legitimately reach external systems. Add a tool name +// here when introducing a read-only tool that needs openWorldHint: true. +const OPEN_WORLD_READ_ONLY_TOOLS = new Set(['query']); describe('GITNEXUS_TOOLS', () => { it('exports all tools (7 base + 3 route/tool/shape + 1 api_impact + 2 group)', () => { @@ -39,6 +43,7 @@ describe('GITNEXUS_TOOLS', () => { expect(typeof tool.name).toBe('string'); expect(tool.description).toBeTruthy(); expect(typeof tool.description).toBe('string'); + expect(tool.annotations).toBeDefined(); expect(tool.inputSchema).toBeDefined(); expect(tool.inputSchema.type).toBe('object'); expect(tool.inputSchema.properties).toBeDefined(); @@ -46,6 +51,48 @@ describe('GITNEXUS_TOOLS', () => { } }); + it('each tool exposes all MCP safety annotations', () => { + for (const tool of GITNEXUS_TOOLS) { + expect(typeof tool.annotations.readOnlyHint).toBe('boolean'); + expect(typeof tool.annotations.destructiveHint).toBe('boolean'); + expect(typeof tool.annotations.idempotentHint).toBe('boolean'); + expect(typeof tool.annotations.openWorldHint).toBe('boolean'); + } + }); + + it('read-only tools are marked non-destructive and idempotent', () => { + for (const tool of GITNEXUS_TOOLS) { + if (MUTATING_TOOLS.has(tool.name)) continue; + + expect(tool.annotations.readOnlyHint).toBe(true); + expect(tool.annotations.destructiveHint).toBe(false); + expect(tool.annotations.idempotentHint).toBe(true); + expect(tool.annotations.openWorldHint).toBe(OPEN_WORLD_READ_ONLY_TOOLS.has(tool.name)); + } + }); + + it('query is marked open-world because it may use external embeddings', () => { + const queryTool = GITNEXUS_TOOLS.find((t) => t.name === 'query')!; + expect(queryTool.annotations).toEqual({ + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, + }); + }); + + it('rename and group_sync are marked mutating and non-idempotent', () => { + for (const name of ['rename', 'group_sync'] as const) { + const tool = GITNEXUS_TOOLS.find((t) => t.name === name)!; + expect(tool.annotations).toEqual({ + readOnlyHint: false, + destructiveHint: true, + idempotentHint: false, + openWorldHint: false, + }); + } + }); + it('query tool requires "query" parameter', () => { const queryTool = GITNEXUS_TOOLS.find((t) => t.name === 'query')!; expect(queryTool.inputSchema.required).toContain('query'); From 114d5304d91d7500396cc22e196a20dd21fd29d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sun, 3 May 2026 09:17:12 +0100 Subject: [PATCH 10/10] fix(mcp): avoid git from non-repo cwd in sibling cwd match (#1138) (#1293) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): avoid git shellout from non-repo cwd for sibling match checkCwdMatch used getGitRoot(cwd), which runs git rev-parse from the launch cwd (often \C:\Users\gergo in MCP stdio). Resolve the cwd git root via ancestor .git checks first, then keep existing remote-based sibling logic. Fixes #1138 Co-authored-by: Cursor * test(mcp): address PR #1293 review follow-ups Three test gaps flagged by review on the #1138 fix: - sibling-clone-drift.test.ts: the existing "non-git cwd" test only asserted match=none, which the pre-fix code also returned (by silently failing the spawn). Wrap child_process / node:child_process with passthrough vi.fn() spies and assert no execSync/execFileSync call is recorded when checkCwdMatch runs against a non-git cwd, so a regression that re-introduces the spawn fails loudly. - git.test.ts: add coverage for findGitRootByDotGit's three untested inputs — a `.git` FILE (linked worktree / submodule), a path that does not exist, and a file path inside a repo (must walk from the parent dir). Each asserts no subprocess was spawned. No production code changes. Test additions only. --------- Co-authored-by: Cursor --- gitnexus/src/core/git-staleness.ts | 9 ++- gitnexus/src/storage/git.ts | 28 +++++++ gitnexus/test/unit/git.test.ts | 74 ++++++++++++++++++- .../test/unit/sibling-clone-drift.test.ts | 57 +++++++++++++- 4 files changed, 161 insertions(+), 7 deletions(-) diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index 93e556ab5..96f70ddd6 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -6,7 +6,7 @@ import { execFileSync } from 'node:child_process'; import path from 'path'; import { readRegistry, type RegistryEntry, type CwdMatch } from '../storage/repo-manager.js'; -import { getGitRoot, getCurrentCommit, getRemoteUrl } from '../storage/git.js'; +import { findGitRootByDotGit, getCurrentCommit, getRemoteUrl } from '../storage/git.js'; export interface StalenessInfo { isStale: boolean; @@ -101,9 +101,10 @@ export async function checkCwdMatch(cwd: string): Promise { } if (bestPath) return { match: 'path', entry: bestPath }; - // 2) Sibling-by-remote: locate the cwd's git root, get its remote - // URL, and look for any registered entry with the same fingerprint. - const cwdGitRoot = getGitRoot(cwdResolved); + // 2) Sibling-by-remote: locate the cwd's git root using only ancestor + // `.git` checks before shelling out. This keeps MCP startup from + // running git in an unrelated launch cwd such as $HOME (#1138). + const cwdGitRoot = findGitRootByDotGit(cwdResolved); if (!cwdGitRoot) return { match: 'none' }; const cwdRemote = getRemoteUrl(cwdGitRoot); diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index 8e0d6e555..9105a4da3 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -93,6 +93,34 @@ export const getGitRoot = (fromPath: string): string | null => { return null; } }; + +/** + * Find a git root by checking only `.git` entries on the ancestor chain. + * + * Unlike `getGitRoot`, this does not spawn `git`, so MCP can cheaply decide + * whether a launch cwd is a worktree before running any subprocess there. + */ +export const findGitRootByDotGit = (fromPath: string): string | null => { + let current = path.resolve(fromPath); + try { + if (!statSync(current).isDirectory()) { + current = path.dirname(current); + } + } catch { + return null; + } + + while (true) { + try { + statSync(path.join(current, '.git')); + return current; + } catch { + const parent = path.dirname(current); + if (parent === current) return null; + current = parent; + } + } +}; /** * Check whether a directory contains a .git entry (file or folder). * diff --git a/gitnexus/test/unit/git.test.ts b/gitnexus/test/unit/git.test.ts index 1e13facda..af339ce69 100644 --- a/gitnexus/test/unit/git.test.ts +++ b/gitnexus/test/unit/git.test.ts @@ -1,6 +1,14 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { execSync } from 'child_process'; -import { isGitRepo, getCurrentCommit, getGitRoot } from '../../src/storage/git.js'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + isGitRepo, + getCurrentCommit, + getGitRoot, + findGitRootByDotGit, +} from '../../src/storage/git.js'; // Mock child_process.execSync vi.mock('child_process', () => ({ @@ -92,4 +100,68 @@ describe('git utilities', () => { expect(result!.trim()).toBe(result); }); }); + + describe('findGitRootByDotGit', () => { + it('finds an ancestor .git directory without spawning git', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-dotgit-')); + try { + fs.mkdirSync(path.join(tmpDir, '.git')); + const nested = path.join(tmpDir, 'packages', 'app'); + fs.mkdirSync(nested, { recursive: true }); + + expect(findGitRootByDotGit(nested)).toBe(path.resolve(tmpDir)); + expect(mockExecSync).not.toHaveBeenCalled(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('returns null outside a git worktree without spawning git', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-nonrepo-')); + try { + expect(findGitRootByDotGit(tmpDir)).toBeNull(); + expect(mockExecSync).not.toHaveBeenCalled(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + // Linked worktrees and submodules use a `.git` file (not directory) that + // points at the real gitdir. statSync succeeds for both, so the ancestor + // walk should treat such roots identically to ordinary repos. + it('treats a .git file (linked worktree) as a valid root', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-worktree-')); + try { + fs.writeFileSync(path.join(tmpDir, '.git'), 'gitdir: /fake/worktrees/wt\n'); + const nested = path.join(tmpDir, 'src', 'pkg'); + fs.mkdirSync(nested, { recursive: true }); + + expect(findGitRootByDotGit(nested)).toBe(path.resolve(tmpDir)); + expect(mockExecSync).not.toHaveBeenCalled(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('returns null when the input path does not exist', () => { + const missing = path.join(os.tmpdir(), `gitnexus-missing-${Date.now()}-${Math.random()}`); + expect(findGitRootByDotGit(missing)).toBeNull(); + expect(mockExecSync).not.toHaveBeenCalled(); + }); + + it('walks from a file input by starting at its parent directory', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-fileinput-')); + try { + fs.mkdirSync(path.join(tmpDir, '.git')); + const filePath = path.join(tmpDir, 'pkg', 'index.ts'); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, 'export {};\n'); + + expect(findGitRootByDotGit(filePath)).toBe(path.resolve(tmpDir)); + expect(mockExecSync).not.toHaveBeenCalled(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + }); }); diff --git a/gitnexus/test/unit/sibling-clone-drift.test.ts b/gitnexus/test/unit/sibling-clone-drift.test.ts index cd063ceec..7f5b7339b 100644 --- a/gitnexus/test/unit/sibling-clone-drift.test.ts +++ b/gitnexus/test/unit/sibling-clone-drift.test.ts @@ -14,9 +14,24 @@ * `checkCwdMatch` API. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'path'; -import { execSync } from 'child_process'; +import { execSync, execFileSync } from 'child_process'; + +// Wrap child_process exports in spies that pass through to the real +// implementation. Test setup (initRepoWithCommit, etc.) keeps working +// against real git; individual tests can clear + assert call counts to +// prove no subprocess was launched. Both module specifiers are mocked +// because git-staleness.ts imports from 'node:child_process' while this +// file (and storage/git.ts) import from 'child_process'. +vi.mock('child_process', async () => { + const actual = await vi.importActual('child_process'); + return { ...actual, execSync: vi.fn(actual.execSync), execFileSync: vi.fn(actual.execFileSync) }; +}); +vi.mock('node:child_process', async () => { + const actual = await vi.importActual('node:child_process'); + return { ...actual, execSync: vi.fn(actual.execSync), execFileSync: vi.fn(actual.execFileSync) }; +}); import { registerRepo, readRegistry, @@ -233,6 +248,44 @@ describe('checkCwdMatch', () => { } }); + it('returns match=none for a non-git cwd before resolving sibling remotes', async () => { + const indexed = await createTempDir('cwd-home-indexed-'); + const nonGitCwd = await createTempDir('cwd-home-non-git-'); + try { + const indexedHead = initRepoWithCommit(indexed.dbPath, 'https://example.com/foo/bar'); + await registerRepo(indexed.dbPath, { + repoPath: indexed.dbPath, + lastCommit: indexedHead, + indexedAt: new Date().toISOString(), + remoteUrl: 'https://example.com/foo/bar', + }); + + // Clear after setup so we only count subprocess calls made by + // checkCwdMatch itself. The fix in #1138 must guarantee that no + // git subprocess is launched when the cwd is outside any .git + // ancestor — a return value of 'none' alone does not prove that + // (the pre-fix code also returned 'none', just by failing the + // spawn). The mocked module exports are vi.fn() wrappers that + // pass through to real implementations; clearing tracks only the + // calls made by checkCwdMatch below. + vi.mocked(execSync).mockClear(); + vi.mocked(execFileSync).mockClear(); + const nodeCp = await import('node:child_process'); + vi.mocked(nodeCp.execSync).mockClear(); + vi.mocked(nodeCp.execFileSync).mockClear(); + + const m = await checkCwdMatch(nonGitCwd.dbPath); + expect(m.match).toBe('none'); + expect(execSync).not.toHaveBeenCalled(); + expect(execFileSync).not.toHaveBeenCalled(); + expect(nodeCp.execSync).not.toHaveBeenCalled(); + expect(nodeCp.execFileSync).not.toHaveBeenCalled(); + } finally { + await indexed.cleanup(); + await nonGitCwd.cleanup(); + } + }); + it('reports sibling-by-remote with a stale hint when cwd HEAD has advanced', async () => { // Polecat-style scenario from the issue: index at path A, query // from cwd=path B (same repo), get a warning rather than