scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-03 02:21:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

fix(group): contract extractors honour .gitnexusignore via shared IgnoreService (#1185) (#1247)

Browse source 
* 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.<Name>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.
This commit is contained in:
azizur100389 2026-05-01 16:42:21 +01:00 • committed by Evan Wang
parent efed04af51
commit b692d1691f
6 changed files with 240 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
gitnexus/src/core/group/extractors/grpc-extractor.ts
View file

@ -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<string, ProtoServiceInfo[]>;
}> {
const servicesByName = new Map<string, ProtoServiceInfo[]>();
// `.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<string, string>();
@ -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,
});

10
gitnexus/src/core/group/extractors/http-route-extractor.ts
View file

@ -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<string[]> {
// 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,
});
}

28
gitnexus/src/core/group/extractors/topic-extractor.ts
View file

@ -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<ExtractedContract[]> {
// 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,
});

63
gitnexus/test/unit/group/grpc-extractor.test.ts
View file

@ -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.<Name>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', () => {

86
gitnexus/test/unit/group/http-route-extractor.test.ts
View file

@ -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);
});
});
});

55
gitnexus/test/unit/group/topic-extractor.test.ts
View file

@ -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<String, String> record) {}`,
);
// Vendored Java handler under a venv-style dir.
writeFile(
'mentor_env/lib/LeakedHandler.java',
`@KafkaListener(topics = "leaked.event")
public void handleLeaked(ConsumerRecord<String, String> 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([]);
});
});
});