diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index cd64fdf8c6..5c535f29f4 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -91,22 +91,23 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis let dbExecutors: Map | undefined; let registryEntries: RegistryEntry[] | undefined; - const eo = opts?.extractorOverride; - if (eo && eo.length === 0) { - autoContracts = await (eo as () => Promise)(); - } 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(); - const openPoolIds: string[] = []; + const openPoolIds: string[] = []; + + try { + const eo = opts?.extractorOverride; + if (eo && eo.length === 0) { + autoContracts = await (eo as () => Promise)(); + } 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(); - 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(() => {}); - } } - } - // 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]; + // 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(); - 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); + 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 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`, + 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`, ); } } - } - } - // 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`, + 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(() => {}); } } diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 9f14db2318..4fc320076c 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -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) => { + 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', () => {