mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-08 22:22:52 +00:00
* fix(group/sync): wire ManifestExtractor into syncGroup pipeline ManifestExtractor was fully implemented in extractors/manifest-extractor.ts but never imported or called in sync.ts. As a result, any links declared in group.yaml were parsed and validated by config-parser.ts but silently dropped — config.links was always an empty dead-end as far as syncGroup was concerned. Changes: - Import ManifestExtractor in sync.ts - Call extractFromManifest(config.links, dbExecutors) inside the outer try block, after all repos are processed but before the finally closes the DB pools (symbol resolution via resolveSymbol requires open executors) - Collect the resulting contracts into autoContracts and the cross-links into a separate manifestCrossLinks array - Merge manifestCrossLinks into the final crossLinks alongside runExactMatch results Without this fix, users who declare explicit service dependencies in group.yaml links (the documented workaround for HTTP clients that use absolute URLs and are invisible to the auto-extractors) get 0 cross-links regardless of what they configure. * test(group/sync): cover manifest links producing cross-links Add a unit test that asserts config.links entries produce contract pairs and a manifest cross-link (matchType: 'manifest') via syncGroup. Also refactors the manifest extraction call to sit outside the else/try block so it runs regardless of extractorOverride arity — makes the code testable without mocked DB pools and ensures links work when callers supply a zero-arity override (e.g. in tests or programmatic usage). * style: prettier format sync.ts and sync.test.ts Also removes the stray empty line in the finally block (noted in review). * fix(group/sync): dedupe cross-links and warn on dangling manifest repos Addresses review feedback on PR #827: 1. Dedupe cross-links. Manifest contracts participate in runExactMatch, so a manifest-declared link also emitted a duplicate matchType:'exact' CrossLink for the same endpoint pair. Dedupe by (from, to, type, contractId) and prefer manifest (operator-declared intent). 2. Warn on dangling repos. When a manifest link references a repo not in config.repos, log a warning. Synthetic UIDs keep the cross-link deterministic, but the operator probably meant something else. 3. Tests: - Assert no duplicate 'exact' CrossLink is emitted alongside the manifest one. - Assert synthetic UID format when no DB executors are available. - New test: dangling manifest repo still produces a cross-link + logs a warning. * perf(group/manifest): parallelize and memoize symbol resolution Previous implementation ran 2N sequential Cypher round-trips per manifest (one for provider side, one for consumer, awaited in-order per link). For manifests with tens of links this dominated syncGroup latency in groups with many declared cross-repo contracts. Changes: - Resolve provider + consumer in parallel per link (Promise.all). - Resolve all links in parallel (outer Promise.all over links.map). Each repo's executor pool is independent, so cross-repo fan-out scales with the number of distinct repos in the manifest. - Memoize by (repo, type, contract). Manifests frequently declare the same contract from both directions or across sibling groups, so duplicate triples now hit the DB once instead of 2× per link. Correctness: - resolveSymbol is a pure LIMIT 1 read, so caching + concurrent invocation is safe. - Iteration order over links is preserved in the final contracts / crossLinks arrays — result shape is identical. Test: - New test asserts that two links sharing (repo, type, contract) produce exactly one DB call per distinct repo-tuple. --------- Co-authored-by: jonasvanderhaegen-xve <> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com>
377 lines
12 KiB
TypeScript
377 lines
12 KiB
TypeScript
import { describe, it, expect } from 'vitest';
|
|
import * as fs from 'node:fs';
|
|
import * as path from 'node:path';
|
|
import * as os from 'node:os';
|
|
import { syncGroup, stableRepoPoolId } from '../../../src/core/group/sync.js';
|
|
import type {
|
|
GroupConfig,
|
|
StoredContract,
|
|
RepoHandle,
|
|
GroupManifestLink,
|
|
} from '../../../src/core/group/types.js';
|
|
import type { RegistryEntry } from '../../../src/storage/repo-manager.js';
|
|
|
|
describe('syncGroup', () => {
|
|
const makeConfig = (repos: Record<string, string>): GroupConfig => ({
|
|
version: 1,
|
|
name: 'test',
|
|
description: '',
|
|
repos,
|
|
links: [],
|
|
packages: {},
|
|
detect: {
|
|
http: true,
|
|
grpc: false,
|
|
topics: false,
|
|
shared_libs: false,
|
|
embedding_fallback: false,
|
|
},
|
|
matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 },
|
|
});
|
|
|
|
it('returns SyncResult with contracts and cross-links', async () => {
|
|
const config = makeConfig({ 'app/backend': 'backend-repo', 'app/frontend': 'frontend-repo' });
|
|
|
|
const mockContracts: StoredContract[] = [
|
|
{
|
|
contractId: 'http::GET::/api/users',
|
|
type: 'http',
|
|
role: 'provider',
|
|
symbolUid: 'uid-1',
|
|
symbolRef: { filePath: 'src/ctrl.ts', name: 'UserController.list' },
|
|
symbolName: 'UserController.list',
|
|
confidence: 0.8,
|
|
meta: { method: 'GET', path: '/api/users' },
|
|
repo: 'app/backend',
|
|
},
|
|
{
|
|
contractId: 'http::GET::/api/users',
|
|
type: 'http',
|
|
role: 'consumer',
|
|
symbolUid: 'uid-2',
|
|
symbolRef: { filePath: 'src/api.ts', name: 'fetchUsers' },
|
|
symbolName: 'fetchUsers',
|
|
confidence: 0.7,
|
|
meta: { method: 'GET', path: '/api/users' },
|
|
repo: 'app/frontend',
|
|
},
|
|
];
|
|
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => mockContracts,
|
|
skipWrite: true,
|
|
});
|
|
|
|
expect(result.contracts).toHaveLength(2);
|
|
expect(result.crossLinks).toHaveLength(1);
|
|
expect(result.crossLinks[0].matchType).toBe('exact');
|
|
expect(result.crossLinks[0].confidence).toBe(1.0);
|
|
expect(result.unmatched).toHaveLength(0);
|
|
});
|
|
|
|
it('reports missing repos', async () => {
|
|
const config = makeConfig({ 'app/backend': 'nonexistent-repo' });
|
|
|
|
const result = await syncGroup(config, {
|
|
resolveRepoHandle: async () => null,
|
|
skipWrite: true,
|
|
});
|
|
|
|
expect(result.missingRepos).toContain('app/backend');
|
|
expect(result.contracts).toHaveLength(0);
|
|
});
|
|
|
|
it('handles empty repos config', async () => {
|
|
const config = makeConfig({});
|
|
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => [],
|
|
skipWrite: true,
|
|
});
|
|
|
|
expect(result.contracts).toHaveLength(0);
|
|
expect(result.crossLinks).toHaveLength(0);
|
|
expect(result.missingRepos).toHaveLength(0);
|
|
});
|
|
|
|
it('intra-repo matching works with service field via extractorOverride', async () => {
|
|
const config = makeConfig({ 'platform/monorepo': 'monorepo' });
|
|
|
|
const mockContracts: StoredContract[] = [
|
|
{
|
|
...makeContract('http::GET::/api/users', 'provider', 'platform/monorepo'),
|
|
service: 'services/auth',
|
|
},
|
|
{
|
|
...makeContract('http::GET::/api/users', 'consumer', 'platform/monorepo'),
|
|
service: 'services/gateway',
|
|
},
|
|
];
|
|
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => mockContracts,
|
|
skipWrite: true,
|
|
});
|
|
|
|
expect(result.crossLinks).toHaveLength(1);
|
|
expect(result.crossLinks[0].from.service).toBe('services/gateway');
|
|
expect(result.crossLinks[0].to.service).toBe('services/auth');
|
|
});
|
|
|
|
function makeContract(id: string, role: 'provider' | 'consumer', repo: string): StoredContract {
|
|
return {
|
|
contractId: id,
|
|
type: 'http',
|
|
role,
|
|
symbolUid: `uid-${repo}-${id}`,
|
|
symbolRef: { filePath: `src/${repo}.ts`, name: `fn-${id}` },
|
|
symbolName: `fn-${id}`,
|
|
confidence: 0.8,
|
|
meta: {},
|
|
repo,
|
|
};
|
|
}
|
|
|
|
it('per-repo extractorOverride receives repo handle and extracts per repo', async () => {
|
|
const config = makeConfig({
|
|
'app/backend': 'backend-repo',
|
|
'app/frontend': 'frontend-repo',
|
|
});
|
|
|
|
const perRepoOverride = async (repo: RepoHandle) => {
|
|
if (repo.path === 'app/backend') {
|
|
return [makeContract('http::GET::/api/users', 'provider', 'app/backend')];
|
|
}
|
|
return [makeContract('http::GET::/api/users', 'consumer', 'app/frontend')];
|
|
};
|
|
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: perRepoOverride,
|
|
resolveRepoHandle: async (_name, groupPath) => ({
|
|
id: groupPath,
|
|
path: groupPath,
|
|
repoPath: '/tmp/' + groupPath,
|
|
storagePath: '/tmp/' + groupPath + '/.gitnexus',
|
|
}),
|
|
skipWrite: true,
|
|
});
|
|
|
|
// per-repo override goes through the initLbug path which will fail
|
|
// but the extractorOverride with arity > 0 triggers the else branch
|
|
// At minimum, the function should not throw
|
|
expect(result).toBeDefined();
|
|
});
|
|
|
|
it('test_syncGroup_closes_only_opened_pools', async () => {
|
|
const config = makeConfig({
|
|
'app/backend': 'backend-repo',
|
|
'app/frontend': 'frontend-repo',
|
|
});
|
|
|
|
const closedIds: string[] = [];
|
|
|
|
const { vi } = await import('vitest');
|
|
const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js');
|
|
const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined);
|
|
const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockImplementation(async (id?: string) => {
|
|
if (id) closedIds.push(id);
|
|
});
|
|
|
|
try {
|
|
await syncGroup(config, {
|
|
resolveRepoHandle: async (_name, groupPath) => ({
|
|
id: groupPath.replace(/\//g, '-'),
|
|
path: groupPath,
|
|
repoPath: '/tmp/' + groupPath,
|
|
storagePath: '/tmp/' + groupPath + '/.gitnexus',
|
|
}),
|
|
skipWrite: true,
|
|
}).catch(() => {});
|
|
|
|
// closeLbug must have been called at least once with specific pool ids
|
|
expect(closeSpy.mock.calls.length).toBeGreaterThan(0);
|
|
expect(closedIds).toContain('app-backend');
|
|
expect(closedIds).toContain('app-frontend');
|
|
|
|
// Every call must have a truthy string id
|
|
for (const id of closedIds) {
|
|
expect(id).toBeTruthy();
|
|
expect(typeof id).toBe('string');
|
|
}
|
|
// No blanket close (no-arg or empty-string or undefined)
|
|
const blanketCalls = closeSpy.mock.calls.filter((args) => args.length === 0 || !args[0]);
|
|
expect(blanketCalls).toHaveLength(0);
|
|
} finally {
|
|
initSpy.mockRestore();
|
|
closeSpy.mockRestore();
|
|
}
|
|
});
|
|
|
|
it('manifest links in config.links produce cross-links with matchType manifest', async () => {
|
|
const links: GroupManifestLink[] = [
|
|
{
|
|
from: 'app/consumer',
|
|
to: 'app/provider',
|
|
type: 'http',
|
|
contract: 'GET::/api/orders',
|
|
role: 'consumer',
|
|
},
|
|
];
|
|
|
|
const config: GroupConfig = {
|
|
version: 1,
|
|
name: 'test',
|
|
description: '',
|
|
repos: { 'app/consumer': 'consumer-repo', 'app/provider': 'provider-repo' },
|
|
links,
|
|
packages: {},
|
|
detect: {
|
|
http: true,
|
|
grpc: false,
|
|
topics: false,
|
|
shared_libs: false,
|
|
embedding_fallback: false,
|
|
},
|
|
matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 },
|
|
};
|
|
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => [],
|
|
skipWrite: true,
|
|
});
|
|
|
|
// ManifestExtractor should inject 2 contracts (provider + consumer) and 1 cross-link
|
|
expect(result.contracts).toHaveLength(2);
|
|
const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest');
|
|
expect(manifestLinks).toHaveLength(1);
|
|
expect(manifestLinks[0].contractId).toBe('http::GET::/api/orders');
|
|
expect(manifestLinks[0].from.repo).toBe('app/consumer');
|
|
expect(manifestLinks[0].to.repo).toBe('app/provider');
|
|
expect(manifestLinks[0].confidence).toBe(1.0);
|
|
|
|
// With no DB executors available, UIDs fall back to the deterministic
|
|
// synthetic form `manifest::<repo>::<contractId>`.
|
|
expect(manifestLinks[0].from.symbolUid).toBe('manifest::app/consumer::http::GET::/api/orders');
|
|
expect(manifestLinks[0].to.symbolUid).toBe('manifest::app/provider::http::GET::/api/orders');
|
|
|
|
// Manifest contracts also participate in runExactMatch; we must not emit a
|
|
// duplicate matchType:'exact' cross-link for the same endpoint pair.
|
|
const exactForSameContract = result.crossLinks.filter(
|
|
(cl) => cl.matchType === 'exact' && cl.contractId === 'http::GET::/api/orders',
|
|
);
|
|
expect(exactForSameContract).toHaveLength(0);
|
|
expect(result.crossLinks).toHaveLength(1);
|
|
});
|
|
|
|
it('manifest links referencing unknown repos still produce cross-links via synthetic UIDs', async () => {
|
|
const links: GroupManifestLink[] = [
|
|
{
|
|
from: 'app/known',
|
|
to: 'app/dangling', // not present in config.repos
|
|
type: 'http',
|
|
contract: 'POST::/api/missing',
|
|
role: 'consumer',
|
|
},
|
|
];
|
|
|
|
const config: GroupConfig = {
|
|
version: 1,
|
|
name: 'test',
|
|
description: '',
|
|
repos: { 'app/known': 'known-repo' },
|
|
links,
|
|
packages: {},
|
|
detect: {
|
|
http: true,
|
|
grpc: false,
|
|
topics: false,
|
|
shared_libs: false,
|
|
embedding_fallback: false,
|
|
},
|
|
matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 },
|
|
};
|
|
|
|
const warnings: string[] = [];
|
|
const origWarn = console.warn;
|
|
console.warn = (msg: string) => warnings.push(String(msg));
|
|
try {
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => [],
|
|
skipWrite: true,
|
|
});
|
|
|
|
expect(result.crossLinks).toHaveLength(1);
|
|
expect(result.crossLinks[0].matchType).toBe('manifest');
|
|
expect(result.crossLinks[0].to.symbolUid).toBe(
|
|
'manifest::app/dangling::http::POST::/api/missing',
|
|
);
|
|
expect(warnings.some((w) => w.includes('app/dangling'))).toBe(true);
|
|
} finally {
|
|
console.warn = origWarn;
|
|
}
|
|
});
|
|
|
|
it('writes registry to groupDir when skipWrite is false', async () => {
|
|
const tmpDir = path.join(os.tmpdir(), `gitnexus-sync-write-${Date.now()}`);
|
|
fs.mkdirSync(tmpDir, { recursive: true });
|
|
|
|
try {
|
|
const config = makeConfig({});
|
|
const result = await syncGroup(config, {
|
|
extractorOverride: async () => [],
|
|
groupDir: tmpDir,
|
|
skipWrite: false,
|
|
});
|
|
|
|
expect(result.contracts).toHaveLength(0);
|
|
|
|
const registryPath = path.join(tmpDir, 'contracts.json');
|
|
expect(fs.existsSync(registryPath)).toBe(true);
|
|
|
|
const registry = JSON.parse(fs.readFileSync(registryPath, 'utf-8'));
|
|
expect(registry.version).toBe(1);
|
|
expect(registry.contracts).toHaveLength(0);
|
|
} finally {
|
|
fs.rmSync(tmpDir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|
|
|
|
describe('stableRepoPoolId', () => {
|
|
it('returns lowercase name when no collision', () => {
|
|
const entry: RegistryEntry = {
|
|
name: 'MyRepo',
|
|
path: '/a/MyRepo',
|
|
storagePath: '/a/MyRepo/.gitnexus',
|
|
indexedAt: '',
|
|
lastCommit: '',
|
|
};
|
|
const all = [entry];
|
|
expect(stableRepoPoolId(entry, all)).toBe('myrepo');
|
|
});
|
|
|
|
it('appends hash suffix on name collision with different path', () => {
|
|
const entry1: RegistryEntry = {
|
|
name: 'repo',
|
|
path: '/a/repo',
|
|
storagePath: '/a/repo/.gitnexus',
|
|
indexedAt: '',
|
|
lastCommit: '',
|
|
};
|
|
const entry2: RegistryEntry = {
|
|
name: 'repo',
|
|
path: '/b/repo',
|
|
storagePath: '/b/repo/.gitnexus',
|
|
indexedAt: '',
|
|
lastCommit: '',
|
|
};
|
|
const all = [entry1, entry2];
|
|
|
|
const id1 = stableRepoPoolId(entry1, all);
|
|
const id2 = stableRepoPoolId(entry2, all);
|
|
|
|
expect(id1).toMatch(/^repo-/);
|
|
expect(id2).toMatch(/^repo-/);
|
|
expect(id1).not.toBe(id2);
|
|
});
|
|
});
|