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

gitnexus/src/core/group/sync.ts

@@ -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(() => {});
-      }
     }
-  }
 
-  // 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<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);
+    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`,
+    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(() => {});
     }
   }
 

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<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', () => {