scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-09-29 01:41:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(group): apply PR review fixes to all workspace extractors

Browse source 
Address review findings from PR #1256 across Node, Python, Go, Java,
and Elixir extractors:
- Replace hardcoded IGNORE sets with shared IgnoreService
  (shouldIgnorePath + loadIgnoreRules) to honor .gitnexusignore
- Qualify contract names with provider identifier to prevent
  contractId collisions across providers
- Warn and skip duplicate project/module/app names
- Update all test assertions for qualified contract format
This commit is contained in:
Christian C. Berclaz 2026-05-01 20:44:36 +02:00
parent 1040bc8902
commit dec909d8bd
No known key found for this signature in database
9 changed files with 108 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -2,6 +2,7 @@ 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, loadIgnoreRules } from '../../../config/ignore-service.js';
interface ElixirAppMeta {
appName: string;
@ -142,15 +143,7 @@ function extractTopModule(moduleName: string, prefix: string): string {
async function findElixirFiles(repoPath: string): Promise<string[]> {
const results: string[] = [];
const IGNORE = new Set([
'_build',
'deps',
'node_modules',
'.git',
'.gitnexus',
'.elixir_ls',
'cover',
]);
const ig = await loadIgnoreRules(repoPath);
async function walk(dir: string, rel: string): Promise<void> {
let entries;
@ -160,14 +153,16 @@ async function findElixirFiles(repoPath: string): Promise<string[]> {
return;
}
for (const entry of entries) {
if (IGNORE.has(entry.name)) continue;
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('.ex') || entry.name.endsWith('.exs')) {
if (entry.name !== 'mix.exs' && entry.name !== 'mix.lock') {
results.push(childRel);
}
if (entry.name === 'mix.exs' || entry.name === 'mix.lock') continue;
if (shouldIgnorePath(childRel)) continue;
if (ig && ig.ignores(childRel)) continue;
results.push(childRel);
}
}
}
@ -203,6 +198,13 @@ export async function extractElixirWorkspaceLinks(
repoPath,
deps: manifest.deps,
};
const existing = appsByName.get(manifest.appName);
if (existing) {
console.warn(
`[elixir-workspace-extractor] duplicate app "${manifest.appName}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`,
);
continue;
}
appsByName.set(manifest.appName, meta);
appsByGroupPath.set(groupPath, meta);
}

27
gitnexus/src/core/group/extractors/go-workspace-extractor.ts
View file

@ -2,6 +2,7 @@ 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, loadIgnoreRules } from '../../../config/ignore-service.js';
interface GoModuleMeta {
modulePath: string;
@ -163,13 +164,7 @@ function escapeRegex(s: string): string {
async function findGoFiles(repoPath: string): Promise<string[]> {
const results: string[] = [];
const IGNORE = new Set([
'vendor',
'node_modules',
'.git',
'.gitnexus',
'testdata',
]);
const ig = await loadIgnoreRules(repoPath);
async function walk(dir: string, rel: string): Promise<void> {
let entries;
@ -179,11 +174,14 @@ async function findGoFiles(repoPath: string): Promise<string[]> {
return;
}
for (const entry of entries) {
if (IGNORE.has(entry.name)) continue;
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('.go') && !entry.name.endsWith('_test.go')) {
if (shouldIgnorePath(childRel)) continue;
if (ig && ig.ignores(childRel)) continue;
results.push(childRel);
}
}
@ -219,6 +217,13 @@ export async function extractGoWorkspaceLinks(
repoPath,
requires: manifest.requires,
};
const existing = modulesByPath.get(manifest.modulePath);
if (existing) {
console.warn(
`[go-workspace-extractor] duplicate module "${manifest.modulePath}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`,
);
continue;
}
modulesByPath.set(manifest.modulePath, meta);
modulesByGroupPath.set(groupPath, meta);
}
@ -241,7 +246,9 @@ export async function extractGoWorkspaceLinks(
const providerMod = modulesByPath.get(imp.modulePath);
if (!providerMod) continue;
const key = `${mod.groupPath}→${providerMod.groupPath}::${imp.symbolName}`;
const shortModule = imp.modulePath.split('/').pop() || imp.modulePath;
const qualifiedContract = `${shortModule}::${imp.symbolName}`;
const key = `${mod.groupPath}→${providerMod.groupPath}::${qualifiedContract}`;
if (seen.has(key)) continue;
seen.add(key);
@ -249,7 +256,7 @@ export async function extractGoWorkspaceLinks(
from: providerMod.groupPath,
to: mod.groupPath,
type: 'custom',
contract: imp.symbolName,
contract: qualifiedContract,
role: 'provider' as ContractRole,
};
links.push(link);

33
gitnexus/src/core/group/extractors/java-workspace-extractor.ts
View file

@ -2,6 +2,7 @@ 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, loadIgnoreRules } from '../../../config/ignore-service.js';
interface JavaProjectMeta {
groupId: string;
@ -160,16 +161,7 @@ function isPascalCase(name: string): boolean {
async function findJavaFiles(repoPath: string): Promise<string[]> {
const results: string[] = [];
const IGNORE = new Set([
'node_modules',
'.git',
'.gitnexus',
'build',
'target',
'.gradle',
'.idea',
'bin',
]);
const ig = await loadIgnoreRules(repoPath);
async function walk(dir: string, rel: string): Promise<void> {
let entries;
@ -179,11 +171,14 @@ async function findJavaFiles(repoPath: string): Promise<string[]> {
return;
}
for (const entry of entries) {
if (IGNORE.has(entry.name)) continue;
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('.java') || entry.name.endsWith('.kt')) {
if (shouldIgnorePath(childRel)) continue;
if (ig && ig.ignores(childRel)) continue;
results.push(childRel);
}
}
@ -222,6 +217,13 @@ export async function extractJavaWorkspaceLinks(
repoPath,
deps: manifest.deps,
};
const existing = projectsByKey.get(key);
if (existing) {
console.warn(
`[java-workspace-extractor] duplicate artifact "${key}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`,
);
continue;
}
projectsByKey.set(key, meta);
projectsByGroupPath.set(groupPath, meta);
}
@ -245,15 +247,16 @@ export async function extractJavaWorkspaceLinks(
const providerProj = projectsByKey.get(imp.artifactKey);
if (!providerProj) continue;
const key = `${proj.groupPath}→${providerProj.groupPath}::${imp.symbolName}`;
if (seen.has(key)) continue;
seen.add(key);
const qualifiedContract = `${providerProj.artifactId}::${imp.symbolName}`;
const dedupKey = `${proj.groupPath}→${providerProj.groupPath}::${qualifiedContract}`;
if (seen.has(dedupKey)) continue;
seen.add(dedupKey);
const link: GroupManifestLink = {
from: providerProj.groupPath,
to: proj.groupPath,
type: 'custom',
contract: imp.symbolName,
contract: qualifiedContract,
role: 'provider' as ContractRole,
};
links.push(link);

30
gitnexus/src/core/group/extractors/node-workspace-extractor.ts
View file

@ -2,6 +2,7 @@ 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, loadIgnoreRules } from '../../../config/ignore-service.js';
interface PackageMeta {
name: string;
@ -148,18 +149,8 @@ function isExportedName(name: string): boolean {
async function findSourceFiles(repoPath: string): Promise<string[]> {
const results: string[] = [];
const IGNORE = new Set([
'node_modules',
'.git',
'.gitnexus',
'dist',
'build',
'coverage',
'.next',
'.nuxt',
'vendor',
]);
const EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mts', '.cts']);
const ig = await loadIgnoreRules(repoPath);
async function walk(dir: string, rel: string): Promise<void> {
let entries;
@ -169,13 +160,16 @@ async function findSourceFiles(repoPath: string): Promise<string[]> {
return;
}
for (const entry of entries) {
if (IGNORE.has(entry.name)) continue;
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 {
const ext = path.extname(entry.name);
if (EXTENSIONS.has(ext)) {
if (shouldIgnorePath(childRel)) continue;
if (ig && ig.ignores(childRel)) continue;
results.push(childRel);
}
}
@ -212,6 +206,13 @@ export async function extractNodeWorkspaceLinks(
repoPath,
workspaceDeps: manifest.workspaceDeps,
};
const existing = packagesByName.get(manifest.name);
if (existing) {
console.warn(
`[node-workspace-extractor] duplicate package name "${manifest.name}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`,
);
continue;
}
packagesByName.set(manifest.name, meta);
packagesByGroupPath.set(groupPath, meta);
}
@ -230,7 +231,8 @@ export async function extractNodeWorkspaceLinks(
const providerPkg = packagesByName.get(imp.packageName);
if (!providerPkg) continue;
const key = `${pkg.groupPath}→${providerPkg.groupPath}::${imp.symbolName}`;
const qualifiedContract = `${imp.packageName}::${imp.symbolName}`;
const key = `${pkg.groupPath}→${providerPkg.groupPath}::${qualifiedContract}`;
if (seen.has(key)) continue;
seen.add(key);
@ -238,7 +240,7 @@ export async function extractNodeWorkspaceLinks(
from: providerPkg.groupPath,
to: pkg.groupPath,
type: 'custom',
contract: imp.symbolName,
contract: qualifiedContract,
role: 'provider' as ContractRole,
};
links.push(link);

33
gitnexus/src/core/group/extractors/python-workspace-extractor.ts
View file

@ -2,6 +2,7 @@ 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, loadIgnoreRules } from '../../../config/ignore-service.js';
interface PythonPackageMeta {
name: string;
@ -152,20 +153,7 @@ function isPascalCase(name: string): boolean {
async function findPythonFiles(repoPath: string): Promise<string[]> {
const results: string[] = [];
const IGNORE = new Set([
'__pycache__',
'.git',
'.gitnexus',
'node_modules',
'.venv',
'venv',
'.tox',
'.eggs',
'dist',
'build',
'.mypy_cache',
'.pytest_cache',
]);
const ig = await loadIgnoreRules(repoPath);
async function walk(dir: string, rel: string): Promise<void> {
let entries;
@ -175,11 +163,14 @@ async function findPythonFiles(repoPath: string): Promise<string[]> {
return;
}
for (const entry of entries) {
if (IGNORE.has(entry.name)) continue;
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('.py')) {
if (shouldIgnorePath(childRel)) continue;
if (ig && ig.ignores(childRel)) continue;
results.push(childRel);
}
}
@ -216,6 +207,13 @@ export async function extractPythonWorkspaceLinks(
repoPath,
workspaceDeps: manifest.deps,
};
const existing = packagesByImportName.get(manifest.importName);
if (existing) {
console.warn(
`[python-workspace-extractor] duplicate package "${manifest.name}" in "${groupPath}" and "${existing.groupPath}" — skipping "${groupPath}"`,
);
continue;
}
packagesByImportName.set(manifest.importName, meta);
packagesByGroupPath.set(groupPath, meta);
}
@ -241,7 +239,8 @@ export async function extractPythonWorkspaceLinks(
const providerPkg = packagesByImportName.get(providerImportName);
if (!providerPkg) continue;
const key = `${pkg.groupPath}→${providerPkg.groupPath}::${imp.symbolName}`;
const qualifiedContract = `${providerPkg.name}::${imp.symbolName}`;
const key = `${pkg.groupPath}→${providerPkg.groupPath}::${qualifiedContract}`;
if (seen.has(key)) continue;
seen.add(key);
@ -249,7 +248,7 @@ export async function extractPythonWorkspaceLinks(
from: providerPkg.groupPath,
to: pkg.groupPath,
type: 'custom',
contract: imp.symbolName,
contract: qualifiedContract,
role: 'provider' as ContractRole,
};
links.push(link);

12
gitnexus/test/unit/group/go-workspace-extractor.test.ts
View file

@ -56,7 +56,7 @@ describe('GoWorkspaceExtractor', () => {
from: 'libs/models',
to: 'services/api',
type: 'custom',
contract: 'Schema',
contract: 'models::Schema',
role: 'provider',
});
});
@ -86,7 +86,7 @@ describe('GoWorkspaceExtractor', () => {
const result = await extractGoWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Token');
expect(result.links[0].contract).toBe('auth::Token');
});
it('handles subpackage imports (module/pkg)', async () => {
@ -117,7 +117,7 @@ describe('GoWorkspaceExtractor', () => {
const result = await extractGoWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Entity');
expect(result.links[0].contract).toBe('core::Entity');
});
it('handles replace directive with local paths', async () => {
@ -145,7 +145,7 @@ describe('GoWorkspaceExtractor', () => {
const result = await extractGoWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Config');
expect(result.links[0].contract).toBe('lib::Config');
});
it('ignores unexported (lowercase) identifiers', async () => {
@ -176,7 +176,7 @@ describe('GoWorkspaceExtractor', () => {
const result = await extractGoWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Config');
expect(result.links[0].contract).toBe('lib::Config');
});
it('skips repos without go.mod', async () => {
@ -251,6 +251,6 @@ describe('GoWorkspaceExtractor', () => {
expect(result.links).toHaveLength(2);
const contracts = result.links.map((l) => l.contract).sort();
expect(contracts).toEqual(['Request', 'Response']);
expect(contracts).toEqual(['lib::Request', 'lib::Response']);
});
});

12
gitnexus/test/unit/group/java-workspace-extractor.test.ts
View file

@ -60,7 +60,7 @@ describe('JavaWorkspaceExtractor', () => {
from: 'models',
to: 'api',
type: 'custom',
contract: 'User',
contract: 'models::User',
role: 'provider',
});
});
@ -93,7 +93,7 @@ describe('JavaWorkspaceExtractor', () => {
const result = await extractJavaWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Config');
expect(result.links[0].contract).toBe('core::Config');
});
it('handles Gradle project dependencies', async () => {
@ -124,7 +124,7 @@ describe('JavaWorkspaceExtractor', () => {
const result = await extractJavaWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Entity');
expect(result.links[0].contract).toBe('common::Entity');
});
it('handles static imports', async () => {
@ -152,7 +152,7 @@ describe('JavaWorkspaceExtractor', () => {
const result = await extractJavaWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Constants');
expect(result.links[0].contract).toBe('lib::Constants');
});
it('skips repos without Java manifest', async () => {
@ -223,7 +223,7 @@ describe('JavaWorkspaceExtractor', () => {
const result = await extractJavaWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Model');
expect(result.links[0].contract).toBe('lib::Model');
});
it('discovers multiple types from the same dependency', async () => {
@ -256,6 +256,6 @@ describe('JavaWorkspaceExtractor', () => {
expect(result.links).toHaveLength(2);
const contracts = result.links.map((l) => l.contract).sort();
expect(contracts).toEqual(['Request', 'Response']);
expect(contracts).toEqual(['lib::Request', 'lib::Response']);
});
});

14
gitnexus/test/unit/group/node-workspace-extractor.test.ts
View file

@ -57,7 +57,7 @@ describe('NodeWorkspaceExtractor', () => {
from: 'libs/shared',
to: 'services/api',
type: 'custom',
contract: 'Config',
contract: '@myorg/shared::Config',
role: 'provider',
});
});
@ -91,7 +91,7 @@ describe('NodeWorkspaceExtractor', () => {
const result = await extractNodeWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Button');
expect(result.links[0].contract).toBe('ui-components::Button');
});
it('handles CommonJS destructured require', async () => {
@ -123,7 +123,7 @@ describe('NodeWorkspaceExtractor', () => {
const result = await extractNodeWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Authenticator');
expect(result.links[0].contract).toBe('auth-lib::Authenticator');
});
it('handles scoped package imports with subpaths', async () => {
@ -155,7 +155,7 @@ describe('NodeWorkspaceExtractor', () => {
const result = await extractNodeWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('User');
expect(result.links[0].contract).toBe('@acme/core::User');
});
it('ignores camelCase/snake_case imports (non-type exports)', async () => {
@ -187,7 +187,7 @@ describe('NodeWorkspaceExtractor', () => {
const result = await extractNodeWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Formatter');
expect(result.links[0].contract).toBe('utils::Formatter');
});
it('skips repos without package.json', async () => {
@ -261,7 +261,7 @@ describe('NodeWorkspaceExtractor', () => {
const result = await extractNodeWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Entity');
expect(result.links[0].contract).toBe('models::Entity');
});
it('handles multiple packages importing from the same provider', async () => {
@ -307,6 +307,6 @@ describe('NodeWorkspaceExtractor', () => {
expect(result.links).toHaveLength(2);
const targets = result.links.map((l) => l.to).sort();
expect(targets).toEqual(['api', 'worker']);
expect(result.links.every((l) => l.contract === 'Schema')).toBe(true);
expect(result.links.every((l) => l.contract === '@org/shared::Schema')).toBe(true);
});
});

14
gitnexus/test/unit/group/python-workspace-extractor.test.ts
View file

@ -50,7 +50,7 @@ describe('PythonWorkspaceExtractor', () => {
from: 'models',
to: 'api',
type: 'custom',
contract: 'Schema',
contract: 'shared-models::Schema',
role: 'provider',
});
});
@ -77,7 +77,7 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Engine');
expect(result.links[0].contract).toBe('mycore::Engine');
});
it('handles hyphenated package names (normalized to underscore in imports)', async () => {
@ -102,7 +102,7 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Helper');
expect(result.links[0].contract).toBe('my-utils::Helper');
});
it('handles submodule imports (from pkg.sub import Class)', async () => {
@ -127,7 +127,7 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Record');
expect(result.links[0].contract).toBe('datalib::Record');
});
it('ignores snake_case imports (functions, not types)', async () => {
@ -152,7 +152,7 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Config');
expect(result.links[0].contract).toBe('utils::Config');
});
it('skips repos without Python manifest', async () => {
@ -217,7 +217,7 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Entity');
expect(result.links[0].contract).toBe('models::Entity');
});
it('reads optional-dependencies from pyproject.toml', async () => {
@ -242,6 +242,6 @@ describe('PythonWorkspaceExtractor', () => {
const result = await extractPythonWorkspaceLinks(repos, repoPaths);
expect(result.links).toHaveLength(1);
expect(result.links[0].contract).toBe('Plugin');
expect(result.links[0].contract).toBe('extras::Plugin');
});
});