From 31f551851db5a3a1f87347deaeca82509a2ac7e3 Mon Sep 17 00:00:00 2001 From: Test Date: Sun, 24 May 2026 18:08:27 +0100 Subject: [PATCH 1/3] fix(group): move manifest/workspace extraction before closeLbug (#1802) syncGroup closed all LadybugDB pools in `finally` before running discoverWorkspaceLinks and ManifestExtractor.extractFromManifest, causing executor closures to resolve against dead pools. Restructure so both run inside an outer try block with pools still open; the finally now fires only after manifest/workspace processing completes. Adds regression test proving executeParameterized fires before closeLbug and that the resulting cross-link uses the real DB UID. --- gitnexus/src/core/group/sync.ts | 117 ++++++++++++------------ gitnexus/test/unit/group/sync.test.ts | 122 ++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 61 deletions(-) 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..5b734daf35 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -980,6 +980,128 @@ 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 closeLbugCalledAt = Infinity; + let executeCalledAt = -1; + let callCounter = 0; + + const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined); + const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockImplementation(async () => { + closeLbugCalledAt = Math.min(closeLbugCalledAt, ++callCounter); + }); + const execSpy = vi + .spyOn(poolAdapter, 'executeParameterized') + .mockImplementation( + async (_poolId: string, _query: string, _params: Record) => { + executeCalledAt = ++callCounter; + 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, + }); + + // executeParameterized (manifest symbol resolution) must fire before closeLbug + expect(executeCalledAt).toBeGreaterThan(0); + expect(executeCalledAt).toBeLessThan(closeLbugCalledAt); + + // 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', () => { From 7d582ebf4d774acc620e6c321f8a3e3df6cbde63 Mon Sep 17 00:00:00 2001 From: Test Date: Sun, 24 May 2026 18:46:03 +0100 Subject: [PATCH 2/3] fix(test): use exact .toBe() assertions in ordering test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace toBeGreaterThan(0) with exact counter values per DoD ยง2.7. --- gitnexus/test/unit/group/sync.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index 5b734daf35..cc2a265460 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -1044,7 +1044,8 @@ service OrderService { }); // executeParameterized (manifest symbol resolution) must fire before closeLbug - expect(executeCalledAt).toBeGreaterThan(0); + expect(executeCalledAt).toBe(6); + expect(closeLbugCalledAt).toBe(7); expect(executeCalledAt).toBeLessThan(closeLbugCalledAt); // The manifest cross-link must use the real UID from the DB, not synthetic From af897e7d1cda247e69c74e25317a83047f732359 Mon Sep 17 00:00:00 2001 From: Test Date: Sun, 24 May 2026 19:30:47 +0100 Subject: [PATCH 3/3] fix(test): replace fragile counter assertions with query-aware ordering check Instead of hard-coding executeParameterized call counts (which depend on internal HttpRouteExtractor behavior), detect manifest-specific resolution by matching the HANDLES_ROUTE query and record whether closeLbug had already fired at that point. This decouples the ordering proof from unrelated extractor internals. --- gitnexus/test/unit/group/sync.test.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/gitnexus/test/unit/group/sync.test.ts b/gitnexus/test/unit/group/sync.test.ts index cc2a265460..4fc320076c 100644 --- a/gitnexus/test/unit/group/sync.test.ts +++ b/gitnexus/test/unit/group/sync.test.ts @@ -1013,19 +1013,20 @@ service OrderService { const poolAdapter = await import('../../../src/core/lbug/pool-adapter.js'); - let closeLbugCalledAt = Infinity; - let executeCalledAt = -1; - let callCounter = 0; + let closeLbugCalled = false; + let manifestResolvedWhilePoolOpen = false; const initSpy = vi.spyOn(poolAdapter, 'initLbug').mockResolvedValue(undefined); const closeSpy = vi.spyOn(poolAdapter, 'closeLbug').mockImplementation(async () => { - closeLbugCalledAt = Math.min(closeLbugCalledAt, ++callCounter); + closeLbugCalled = true; }); const execSpy = vi .spyOn(poolAdapter, 'executeParameterized') .mockImplementation( - async (_poolId: string, _query: string, _params: Record) => { - executeCalledAt = ++callCounter; + 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' }, ]; @@ -1043,10 +1044,9 @@ service OrderService { skipWrite: true, }); - // executeParameterized (manifest symbol resolution) must fire before closeLbug - expect(executeCalledAt).toBe(6); - expect(closeLbugCalledAt).toBe(7); - expect(executeCalledAt).toBeLessThan(closeLbugCalledAt); + // 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');