fix(group): move manifest/workspace extraction before closeLbug (#1802) (#1807)

This commit is contained in:
Gergő Magyar 2026-05-24 19:53:07 +01:00 • committed by GitHub
parent 1879624df8
commit 68adbb4743
2 changed files with 182 additions and 64 deletions

View file

@ -91,22 +91,23 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
let dbExecutors: Map<string, CypherExecutor> | undefined;
let registryEntries: RegistryEntry[] | undefined;
const eo = opts?.extractorOverride;
if (eo && eo.length === 0) {
autoContracts = await (eo as () => Promise<StoredContract[]>)();
} else {
registryEntries = await readRegistry();
const entries = registryEntries;
const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries);
const httpEx = new HttpRouteExtractor();
const grpcEx = new GrpcExtractor();
const thriftEx = new ThriftExtractor();
const topicEx = new TopicExtractor();
const includeEx = new IncludeExtractor();
dbExecutors = new Map<string, CypherExecutor>();
const openPoolIds: string[] = [];
const openPoolIds: string[] = [];
try {
const eo = opts?.extractorOverride;
if (eo && eo.length === 0) {
autoContracts = await (eo as () => Promise<StoredContract[]>)();
} else {
registryEntries = await readRegistry();
const entries = registryEntries;
const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries);
const httpEx = new HttpRouteExtractor();
const grpcEx = new GrpcExtractor();
const thriftEx = new ThriftExtractor();
const topicEx = new TopicExtractor();
const includeEx = new IncludeExtractor();
dbExecutors = new Map<string, CypherExecutor>();
try {
for (const [groupPath, regName] of Object.entries(config.repos)) {
const handle = await resolve(regName, groupPath);
if (!handle) {
@ -201,64 +202,58 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis
missingRepos.push(groupPath);
}
}
} finally {
for (const id of [...new Set(openPoolIds)]) {
await closeLbug(id).catch(() => {});
}
// Workspace discovery and manifest extraction run inside this outer try
// block so dbExecutors closures resolve against live pools (issue #1802).
// The finally below closes pools after this completes (or throws).
let allLinks = [...config.links];
if (config.detect.workspace_deps) {
const repoPaths = new Map<string, string>();
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);
}
}
}
// 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<string, string>();
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 discoverWorkspaceLinks(config.repos, repoPaths, dbExecutors);
if (wsResult.links.length > 0) {
allLinks = [...allLinks, ...wsResult.links];
if (opts?.verbose) {
for (const s of wsResult.stats) {
logger.info(
` workspace-deps: discovered ${s.linkCount} cross-${s.ecosystem.toLowerCase()} links from ${s.projectCount} ${s.ecosystem} projects`,
);
const wsResult = await discoverWorkspaceLinks(config.repos, repoPaths, dbExecutors);
if (wsResult.links.length > 0) {
allLinks = [...allLinks, ...wsResult.links];
if (opts?.verbose) {
for (const s of wsResult.stats) {
logger.info(
` workspace-deps: discovered ${s.linkCount} cross-${s.ecosystem.toLowerCase()} links from ${s.projectCount} ${s.ecosystem} projects`,
);
}
}
}
}
}
// 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 (allLinks.length > 0) {
const knownRepos = new Set(Object.keys(config.repos));
for (const link of allLinks) {
const dangling = [link.from, link.to].filter((r) => !knownRepos.has(r));
if (dangling.length > 0) {
logger.warn(
`[group/sync] manifest link ${link.type}:${link.contract} references repos not in config.repos: ${dangling.join(', ')} — cross-links will use synthetic UIDs`,
if (allLinks.length > 0) {
const knownRepos = new Set(Object.keys(config.repos));
for (const link of allLinks) {
const dangling = [link.from, link.to].filter((r) => !knownRepos.has(r));
if (dangling.length > 0) {
logger.warn(
`[group/sync] manifest link ${link.type}:${link.contract} references repos not in config.repos: ${dangling.join(', ')} — cross-links will use synthetic UIDs`,
);
}
}
const manifestEx = new ManifestExtractor();
const manifestResult = await manifestEx.extractFromManifest(allLinks, dbExecutors);
autoContracts.push(...manifestResult.contracts);
manifestCrossLinks = manifestResult.crossLinks;
if (opts?.verbose) {
logger.info(
` manifest: ${manifestCrossLinks.length} cross-links from ${allLinks.length} links (${config.links.length} declared + ${allLinks.length - config.links.length} discovered)`,
);
}
}
const manifestEx = new ManifestExtractor();
const manifestResult = await manifestEx.extractFromManifest(allLinks, dbExecutors);
autoContracts.push(...manifestResult.contracts);
manifestCrossLinks = manifestResult.crossLinks;
if (opts?.verbose) {
logger.info(
` manifest: ${manifestCrossLinks.length} cross-links from ${allLinks.length} links (${config.links.length} declared + ${allLinks.length - config.links.length} discovered)`,
);
} finally {
for (const id of [...new Set(openPoolIds)]) {
await closeLbug(id).catch(() => {});
}
}

View file

@ -980,6 +980,129 @@ service OrderService {
expect(nodeLink).toBeDefined();
});
});
it('manifest symbol resolution runs before closeLbug (issue #1802)', async () => {
const links: GroupManifestLink[] = [
{
from: 'svc/orders',
to: 'svc/payments',
type: 'http',
contract: 'GET::/api/checkout',
role: 'consumer',
},
];
const config: GroupConfig = {
version: 1,
name: 'test',
description: '',
repos: { 'svc/orders': 'orders-repo', 'svc/payments': 'payments-repo' },
links,
packages: {},
detect: {
http: true,
grpc: false,
thrift: false,
topics: false,
shared_libs: false,
embedding_fallback: false,
workspace_deps: false,
},
matching: { bm25_threshold: 0.7, embedding_threshold: 0.65, max_candidates_per_step: 3 },
};
const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js');
let closeLbugCalled = false;
let manifestResolvedWhilePoolOpen = false;
const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined);
const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockImplementation(async () => {
closeLbugCalled = true;
});
const execSpy = vi
.spyOn(poolAdapter, 'executeParameterized')
.mockImplementation(
async (_poolId: string, query: string, _params: Record<string, unknown>) => {
if (query.includes('HANDLES_ROUTE')) {
manifestResolvedWhilePoolOpen = !closeLbugCalled;
}
return [
{ uid: 'real-uid-checkout', name: 'CheckoutHandler', filePath: 'src/checkout.ts' },
];
},
);
try {
const result = await syncGroup(config, {
resolveRepoHandle: async (_name, groupPath) => ({
id: groupPath.replace(/\//g, '-'),
path: groupPath,
repoPath: '/tmp/' + groupPath,
storagePath: '/tmp/' + groupPath + '/.gitnexus',
}),
skipWrite: true,
});
// Manifest symbol resolution must run while pools are still open
expect(manifestResolvedWhilePoolOpen).toBe(true);
expect(closeLbugCalled).toBe(true);
// The manifest cross-link must use the real UID from the DB, not synthetic
const manifestLinks = result.crossLinks.filter((cl) => cl.matchType === 'manifest');
expect(manifestLinks).toHaveLength(1);
expect(manifestLinks[0].to.symbolUid).toBe('real-uid-checkout');
expect(manifestLinks[0].to.symbolUid).not.toContain('manifest::');
// closeLbug must fire exactly twice (one per repo)
expect(closeSpy).toHaveBeenCalledTimes(2);
} finally {
initSpy.mockRestore();
closeSpy.mockRestore();
execSpy.mockRestore();
}
});
it('extractorOverride no-DB path still produces synthetic manifest UIDs', async () => {
const links: GroupManifestLink[] = [
{
from: 'svc/orders',
to: 'svc/payments',
type: 'http',
contract: 'GET::/api/checkout',
role: 'consumer',
},
];
const config: GroupConfig = {
version: 1,
name: 'test',
description: '',
repos: { 'svc/orders': 'orders-repo', 'svc/payments': 'payments-repo' },
links,
packages: {},
detect: {
http: true,
grpc: false,
thrift: false,
topics: false,
shared_libs: false,
embedding_fallback: false,
workspace_deps: false,
},
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).toHaveLength(1);
expect(manifestLinks[0].from.symbolUid).toBe('manifest::svc/orders::http::GET::/api/checkout');
expect(manifestLinks[0].to.symbolUid).toBe('manifest::svc/payments::http::GET::/api/checkout');
});
});
describe('stableRepoPoolId', () => {