merge: resolve conflicts with main

Impact: 21 functions changed, 39 affected

Author: carlos-alm

crates/codegraph-core/src/cfg.rs

@@ -803,6 +803,8 @@ impl<'a> CfgBuilder<'a> {
                 for_stmt.child_by_field_name(field).is_some()
                     || for_stmt.child_by_field_name("right").is_some()
                     || for_stmt.child_by_field_name("value").is_some()
+                    // Go: for-range has a range_clause child (no condition/right/value fields)
+                    || has_child_of_kind(for_stmt, "range_clause")
                     // Explicit iterator-style node kinds across supported languages:
                     // JS: for_in_statement, Java: enhanced_for_statement,
                     // C#/PHP: foreach_statement, Ruby: for
@@ -1134,6 +1136,14 @@ impl<'a> CfgBuilder<'a> {
 
 // ─── Helpers ────────────────────────────────────────────────────────────
 
+/// Returns true if `node` has a direct child whose node kind equals `kind`.
+/// This is a shallow (one-level) check — it does not recurse into grandchildren.
+fn has_child_of_kind(node: &Node, kind: &str) -> bool {
+    let mut cursor = node.walk();
+    let result = node.children(&mut cursor).any(|c| c.kind() == kind);
+    result
+}
+
 fn node_line(node: &Node) -> u32 {
     node.start_position().row as u32 + 1
 }

crates/codegraph-core/src/import_resolution.rs

@@ -10,10 +10,33 @@ fn file_exists(path: &str, known: Option<&HashSet<String>>) -> bool {
     known.map_or_else(|| Path::new(path).exists(), |set| set.contains(path))
 }
 
+/// Resolve `.` and `..` components in a path without touching the filesystem.
+/// Unlike `PathBuf::components().collect()`, this properly collapses `..` by
+/// popping the previous component from the result.
+///
+/// NOTE: if the path begins with more `..` components than there are preceding
+/// components to pop (e.g. a purely relative `../../foo`), the excess `..`
+/// components are silently dropped.  This function is therefore only correct
+/// when called on paths that have already been joined to a base directory with
+/// sufficient depth.
+fn clean_path(p: &Path) -> PathBuf {
+    let mut result = PathBuf::new();
+    for c in p.components() {
+        match c {
+            std::path::Component::ParentDir => {
+                result.pop();
+            }
+            std::path::Component::CurDir => {}
+            _ => result.push(c),
+        }
+    }
+    result
+}
+
 /// Normalize a path to use forward slashes and clean `.` / `..` segments
 /// (cross-platform consistency).
 fn normalize_path(p: &str) -> String {
-    let cleaned: PathBuf = Path::new(p).components().collect();
+    let cleaned = clean_path(Path::new(p));
     cleaned.display().to_string().replace('\\', "/")
 }
 
@@ -111,7 +134,7 @@ fn resolve_import_path_inner(
 
     // Relative import — normalize immediately to remove `.` / `..` segments
     let dir = Path::new(from_file).parent().unwrap_or(Path::new(""));
-    let resolved: PathBuf = dir.join(import_source).components().collect();
+    let resolved = clean_path(&dir.join(import_source));
     let resolved_str = resolved.display().to_string().replace('\\', "/");
 
     // .js → .ts remap
@@ -246,3 +269,49 @@ pub fn resolve_imports_batch(
         })
         .collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn clean_path_collapses_parent_dirs() {
+        assert_eq!(
+            clean_path(Path::new("src/cli/commands/../../domain/graph/builder.js")),
+            PathBuf::from("src/domain/graph/builder.js")
+        );
+    }
+
+    #[test]
+    fn clean_path_skips_cur_dir() {
+        assert_eq!(
+            clean_path(Path::new("src/./foo.ts")),
+            PathBuf::from("src/foo.ts")
+        );
+    }
+
+    #[test]
+    fn clean_path_handles_absolute_root() {
+        assert_eq!(
+            clean_path(Path::new("/src/../foo.ts")),
+            PathBuf::from("/foo.ts")
+        );
+    }
+
+    #[test]
+    fn clean_path_mixed_segments() {
+        assert_eq!(
+            clean_path(Path::new("a/b/../c/./d/../e.js")),
+            PathBuf::from("a/c/e.js")
+        );
+    }
+
+    #[test]
+    fn clean_path_excess_parent_dirs_silently_dropped() {
+        // Documents the known limitation: excess leading `..` are dropped
+        assert_eq!(
+            clean_path(Path::new("../../foo")),
+            PathBuf::from("foo")
+        );
+    }
+}

scripts/incremental-benchmark.ts

@@ -58,7 +58,7 @@ if (!isWorker()) {
 		function walk(dir) {
 			for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
 				if (entry.isDirectory()) { walk(path.join(dir, entry.name)); continue; }
-				if (!entry.name.endsWith('.js')) continue;
+				if (!entry.name.endsWith('.js') && !entry.name.endsWith('.ts') && !entry.name.endsWith('.tsx')) continue;
 				const absFile = path.join(dir, entry.name);
 				const content = fs.readFileSync(absFile, 'utf8');
 				let match;

src/domain/graph/cycles.ts

@@ -4,6 +4,25 @@ import { CodeGraph } from '../../graph/model.js';
 import { loadNative } from '../../infrastructure/native.js';
 import type { BetterSqlite3Database } from '../../types.js';
 
+/**
+ * Engine parity note — function-level cycle counts
+ *
+ * The native (Rust) and WASM engines may report different function-level cycle
+ * counts even on the same codebase. This is expected behavior, not a bug in
+ * the cycle detection algorithm (Tarjan SCC is identical in both engines).
+ *
+ * Root cause: the native engine extracts slightly more symbols and resolves
+ * more call edges than WASM (e.g. 10883 nodes / 4000 calls native vs 10857
+ * nodes / 3986 calls WASM on the codegraph repo). The additional precision
+ * can both create new edges and — more commonly — resolve previously ambiguous
+ * calls to their correct targets, which breaks false cycles that WASM reports.
+ *
+ * For file-level cycles the engines are in parity because import edges are
+ * resolved identically. The gap only manifests at function-level granularity
+ * where call-site extraction differs between the Rust and WASM parsers.
+ *
+ * See: https://github.com/optave/codegraph/issues/597
+ */
 export function findCycles(
   db: BetterSqlite3Database,
   opts: { fileLevel?: boolean; noTests?: boolean } = {},

src/features/cfg.ts

@@ -84,6 +84,24 @@ interface FileSymbols {
   _langId?: string;
 }
 
+/**
+ * Check whether all function/method definitions in a single file already
+ * have native CFG data (blocks populated by the Rust extractor).
+ * cfg === null means no body (expected); cfg with empty blocks means not computed.
+ */
+function hasNativeCfgForFile(symbols: FileSymbols): boolean {
+  return symbols.definitions
+    .filter(
+      (d) =>
+        (d.kind === 'function' || d.kind === 'method') &&
+        d.line > 0 &&
+        d.endLine != null &&
+        d.endLine > d.line &&
+        !d.name.includes('.'),
+    )
+    .every((d) => d.cfg === null || (d.cfg?.blocks?.length ?? 0) > 0);
+}
+
 async function initCfgParsers(
   fileSymbols: Map<string, FileSymbols>,
 ): Promise<{ parsers: unknown; getParserFn: unknown }> {
@@ -93,17 +111,7 @@ async function initCfgParsers(
     if (!symbols._tree) {
       const ext = path.extname(relPath).toLowerCase();
       if (CFG_EXTENSIONS.has(ext)) {
-        const hasNativeCfg = symbols.definitions
-          .filter(
-            (d) =>
-              (d.kind === 'function' || d.kind === 'method') &&
-              d.line > 0 &&
-              d.endLine != null &&
-              d.endLine > d.line &&
-              !d.name.includes('.'),
-          )
-          .every((d) => d.cfg === null || (d.cfg?.blocks?.length ?? 0) > 0);
-        if (!hasNativeCfg) {
+        if (!hasNativeCfgForFile(symbols)) {
           needsFallback = true;
           break;
         }
@@ -136,11 +144,7 @@ function getTreeAndLang(
   let tree = symbols._tree;
   let langId = symbols._langId;
 
-  const allNative = symbols.definitions
-    .filter((d) => (d.kind === 'function' || d.kind === 'method') && d.line)
-    .every((d) => d.cfg === null || (d.cfg?.blocks?.length ?? 0) > 0);
-
-  if (!tree && !allNative) {
+  if (!tree && !hasNativeCfgForFile(symbols)) {
     if (!getParserFn) return null;
     langId = extToLang.get(ext);
     if (!langId || !CFG_RULES.has(langId)) return null;
@@ -253,14 +257,43 @@ function persistCfg(
 
 // ─── Build-Time: Compute CFG for Changed Files ─────────────────────────
 
+/**
+ * Check if all function/method definitions across all files already have
+ * native CFG data (blocks array populated by the Rust extractor).
+ * When true, the WASM parser and JS CFG visitor can be fully bypassed.
+ */
+function allCfgNative(fileSymbols: Map<string, FileSymbols>): boolean {
+  let hasCfgFile = false;
+  for (const [relPath, symbols] of fileSymbols) {
+    if (symbols._tree) continue; // already parsed via WASM; will use _tree in slow path
+    const ext = path.extname(relPath).toLowerCase();
+    if (!CFG_EXTENSIONS.has(ext)) continue;
+    hasCfgFile = true;
+
+    if (!hasNativeCfgForFile(symbols)) return false;
+  }
+  // Return false when no CFG files found (empty map, all _tree, or all non-CFG
+  // extensions) to avoid vacuously triggering the fast path.
+  return hasCfgFile;
+}
+
 export async function buildCFGData(
   db: BetterSqlite3Database,
   fileSymbols: Map<string, FileSymbols>,
   rootDir: string,
   _engineOpts?: unknown,
 ): Promise<void> {
+  // Fast path: when all function/method defs already have native CFG data,
+  // skip WASM parser init, tree parsing, and JS visitor entirely — just persist.
+  const allNative = allCfgNative(fileSymbols);
+
   const extToLang = buildExtToLangMap();
-  const { parsers, getParserFn } = await initCfgParsers(fileSymbols);
+  let parsers: unknown = null;
+  let getParserFn: unknown = null;
+
+  if (!allNative) {
+    ({ parsers, getParserFn } = await initCfgParsers(fileSymbols));
+  }
 
   const insertBlock = db.prepare(
     `INSERT INTO cfg_blocks (function_node_id, block_index, block_type, start_line, end_line, label)
@@ -277,6 +310,35 @@ export async function buildCFGData(
       const ext = path.extname(relPath).toLowerCase();
       if (!CFG_EXTENSIONS.has(ext)) continue;
 
+      // Native fast path: skip tree/visitor setup when all CFG is pre-computed.
+      // Only apply to files without _tree — files with _tree were WASM-parsed
+      // and need the slow path (visitor) to compute CFG.
+      if (allNative && !symbols._tree) {
+        for (const def of symbols.definitions) {
+          if (def.kind !== 'function' && def.kind !== 'method') continue;
+          if (!def.line) continue;
+
+          const nodeId = getFunctionNodeId(db, def.name, relPath, def.line);
+          if (!nodeId) continue;
+
+          // Always delete stale CFG rows (handles body-removed case)
+          deleteCfgForNode(db, nodeId);
+          if (!def.cfg?.blocks?.length) continue;
+
+          persistCfg(
+            def.cfg as unknown as { blocks: CfgBuildBlock[]; edges: CfgBuildEdge[] },
+            nodeId,
+            insertBlock,
+            insertEdge,
+          );
+          analyzed++;
+        }
+        continue;
+      }
+
+      // When allNative=true, parsers/getParserFn are null. This is safe because
+      // _tree files use symbols._tree directly in getTreeAndLang (the parser
+      // code path is never reached). Non-_tree files are handled by the fast path above.
       const treeLang = getTreeAndLang(symbols, relPath, rootDir, extToLang, parsers, getParserFn);
       if (!treeLang) continue;
       const { tree, langId } = treeLang;
@@ -309,9 +371,10 @@ export async function buildCFGData(
           if (r) cfg = { blocks: r.blocks, edges: r.edges };
         }
 
+        // Always purge stale rows (handles body-removed case)
+        deleteCfgForNode(db, nodeId);
         if (!cfg || cfg.blocks.length === 0) continue;
 
-        deleteCfgForNode(db, nodeId);
         persistCfg(cfg, nodeId, insertBlock, insertEdge);
         analyzed++;
       }

src/mcp/server.ts

@@ -19,6 +19,14 @@ import { initMcpDefaults } from './middleware.js';
 import { buildToolList } from './tool-registry.js';
 import { TOOL_HANDLERS } from './tools/index.js';
 
+/**
+ * Module-level guard to register shutdown handlers only once per process.
+ * Because tests use vi.resetModules(), this module-level variable resets
+ * on each re-import — but the process-level flag on `process` persists.
+ */
+// biome-ignore lint/suspicious/noExplicitAny: MCP SDK server type is lazy-loaded
+let _activeServer: any = null;
+
 export interface McpToolContext {
   dbPath: string | undefined;
   // biome-ignore lint/suspicious/noExplicitAny: lazy-loaded queries module
@@ -179,5 +187,63 @@ export async function startMCPServer(
 
   // biome-ignore lint/suspicious/noExplicitAny: MCP SDK types are lazy-loaded and untyped
   const transport = new (StdioServerTransport as any)();
-  await server.connect(transport);
+
+  // Graceful shutdown — when the client disconnects (e.g. session clear),
+  // close the server cleanly so the process exits without error.
+  // Track the active server at module level so handlers always reference
+  // the latest instance (matters when tests call startMCPServer repeatedly).
+  _activeServer = server;
+
+  // Register handlers once per process to avoid listener accumulation.
+  // Use a process-level flag so it survives vi.resetModules() in tests.
+  const g = globalThis as Record<string, unknown>;
+  // biome-ignore lint/complexity/useLiteralKeys: bracket notation required by TS noPropertyAccessFromIndexSignature
+  if (!g['__codegraph_shutdown_installed']) {
+    // biome-ignore lint/complexity/useLiteralKeys: bracket notation required by TS noPropertyAccessFromIndexSignature
+    g['__codegraph_shutdown_installed'] = true;
+
+    const shutdown = async () => {
+      try {
+        await _activeServer?.close();
+      } catch {}
+      process.exit(0);
+    };
+    const silentExit = (err: Error & { code?: string }) => {
+      // Only suppress broken-pipe errors from closed stdio transport;
+      // let real bugs surface with a non-zero exit code.
+      if (err.code === 'EPIPE' || err.code === 'ERR_STREAM_DESTROYED') {
+        process.exit(0);
+      }
+      process.stderr.write(`Uncaught exception: ${err.stack ?? err.message}\n`);
+      process.exit(1);
+    };
+    const silentReject = (reason: unknown) => {
+      const err = reason instanceof Error ? reason : new Error(String(reason));
+      const code = (err as Error & { code?: string }).code;
+      if (code === 'EPIPE' || code === 'ERR_STREAM_DESTROYED') {
+        process.exit(0);
+      }
+      process.stderr.write(`Unhandled rejection: ${err.stack ?? err.message}\n`);
+      process.exit(1);
+    };
+
+    process.on('SIGINT', shutdown);
+    process.on('SIGTERM', shutdown);
+    process.on('SIGHUP', shutdown);
+    process.on('uncaughtException', silentExit);
+    process.on('unhandledRejection', silentReject);
+  }
+
+  try {
+    await server.connect(transport);
+  } catch (err) {
+    const code = (err as Error & { code?: string }).code;
+    if (code === 'EPIPE' || code === 'ERR_STREAM_DESTROYED') {
+      process.exit(0);
+    }
+    process.stderr.write(
+      `MCP transport connect failed: ${(err as Error).stack ?? (err as Error).message}\n`,
+    );
+    process.exit(1);
+  }
 }