From 03423efbdeac9048563a8766acbcb2f8689a8acb Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Wed, 8 Jul 2026 15:26:05 -0600 Subject: [PATCH] fix(native): resolve monorepo workspace package imports in the native engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_import_path_inner had no workspace-awareness at all: a bare monorepo-package specifier (e.g. import "@myorg/lib") fell straight through to the raw-specifier fallback under the native engine, unlike the WASM/JS engine's resolveViaWorkspace(), which resolves it to the package's real entry file and grants the resulting call edges a 0.95 confidence floor. - Port resolveViaWorkspace()/parseBareSpecifier() into crates/codegraph-core/src/domain/graph/resolve.rs (resolve_via_workspace, parse_bare_specifier), threaded through resolve_import_path/ resolve_imports_batch and both the per-call FFI path (lib.rs) and the full Rust build orchestrator (pipeline.rs, import_edges.rs). - Add a process-lifetime workspace-resolved-paths cache in resolve.rs, mirroring _workspaceResolvedPaths in resolve.ts, so compute_confidence grants the same 0.95 confidence floor natively. - Thread the already-detected workspace map (from detectWorkspaces() in infrastructure/config.ts — no native equivalent, matching the existing config.rs JSON-handoff pattern) across the FFI boundary: new NativeWorkspacePackage/WorkspacePackage type, new optional parameters on resolveImport/resolveImports/buildGraph, and a new getWorkspacesForNative() helper in resolve.ts. - Add a monorepo-workspace fixture and wire it into the native/WASM build-parity hard gate, plus a dedicated regression test asserting the resolved edge, its target file, and the 0.95 confidence floor for both engines. Native still has no package.json "exports" field resolver (a separate, broader gap tracked in #2060) — resolve_via_workspace falls back to main/source/index-file resolution for that case, same as a repo without an exports field would get under either engine. --- crates/codegraph-core/src/db/connection.rs | 8 + .../src/domain/graph/builder/pipeline.rs | 54 ++- .../graph/builder/stages/import_edges.rs | 7 + .../src/domain/graph/resolve.rs | 446 +++++++++++++++++- crates/codegraph-core/src/lib.rs | 32 +- crates/codegraph-core/src/types.rs | 26 + .../builder/stages/native-orchestrator.ts | 3 +- src/domain/graph/resolve.ts | 26 +- src/types.ts | 29 +- .../monorepo-workspace/apps/web/index.js | 5 + .../monorepo-workspace/apps/web/package.json | 5 + .../fixtures/monorepo-workspace/package.json | 8 + .../packages/lib/package.json | 5 + .../packages/lib/src/index.js | 7 + tests/integration/build-parity.test.ts | 10 + ...e-1927-native-workspace-resolution.test.ts | 144 ++++++ 16 files changed, 796 insertions(+), 19 deletions(-) create mode 100644 tests/fixtures/monorepo-workspace/apps/web/index.js create mode 100644 tests/fixtures/monorepo-workspace/apps/web/package.json create mode 100644 tests/fixtures/monorepo-workspace/package.json create mode 100644 tests/fixtures/monorepo-workspace/packages/lib/package.json create mode 100644 tests/fixtures/monorepo-workspace/packages/lib/src/index.js create mode 100644 tests/integration/issue-1927-native-workspace-resolution.test.ts diff --git a/crates/codegraph-core/src/db/connection.rs b/crates/codegraph-core/src/db/connection.rs index 5297796e1..aca94537b 100644 --- a/crates/codegraph-core/src/db/connection.rs +++ b/crates/codegraph-core/src/db/connection.rs @@ -1464,14 +1464,22 @@ impl NativeDatabase { config_json: String, aliases_json: String, opts_json: String, + // Monorepo workspace packages (JSON array of `WorkspacePackage`), + // detected on the JS side by `detectWorkspaces()` — see + // `resolve::resolve_via_workspace`'s doc comment (issue #1927). + // `Option` for compatibility with older JS callers built against a + // pre-#1927 native binary that never passes this argument. + workspaces_json: Option, ) -> napi::Result { let conn = self.conn()?; + let workspaces_json = workspaces_json.unwrap_or_default(); let result = crate::domain::graph::builder::pipeline::run_pipeline( conn, &root_dir, &config_json, &aliases_json, &opts_json, + &workspaces_json, ) .map_err(|e| napi::Error::from_reason(format!("build_graph failed: {e}")))?; serde_json::to_string(&result) diff --git a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs index b8f094080..6af4975bc 100644 --- a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs +++ b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs @@ -116,6 +116,12 @@ struct PipelineSetup { include_dataflow: bool, include_ast: bool, force_full_rebuild: bool, + /// Monorepo workspace packages, keyed by package name. Detected by the JS + /// caller (`detectWorkspaces()` in infrastructure/config.ts — no Rust + /// equivalent; see `resolve::resolve_via_workspace`'s doc comment) and + /// serialized alongside aliases/opts. Empty when the project has no + /// workspace config (issue #1927). + workspaces: HashMap, } fn pipeline_setup( @@ -123,6 +129,7 @@ fn pipeline_setup( config_json: &str, aliases_json: &str, opts_json: &str, + workspaces_json: &str, ) -> Result { let config: BuildConfig = serde_json::from_str(config_json).map_err(|e| format!("config parse error: {e}"))?; @@ -130,12 +137,22 @@ fn pipeline_setup( serde_json::from_str(aliases_json).map_err(|e| format!("aliases parse error: {e}"))?; let opts: BuildOpts = serde_json::from_str(opts_json).map_err(|e| format!("opts parse error: {e}"))?; + let workspace_packages: Vec = if workspaces_json.trim().is_empty() { + Vec::new() + } else { + serde_json::from_str(workspaces_json).map_err(|e| format!("workspaces parse error: {e}"))? + }; let napi_aliases = aliases.to_napi_aliases(); let incremental = opts.incremental.unwrap_or(config.build.incremental); let include_dataflow = opts.dataflow.unwrap_or(true); let include_ast = opts.ast.unwrap_or(true); let force_full_rebuild = check_version_mismatch(conn); + let workspaces = resolve::workspaces_from_packages(&workspace_packages); + // Reset once per build, mirroring `setWorkspaces()`'s + // `_workspaceResolvedPaths.clear()` in resolve.ts — must happen before + // Stage 6/6b resolve any imports below. + resolve::reset_workspace_resolved_paths(); Ok(PipelineSetup { config, @@ -145,6 +162,7 @@ fn pipeline_setup( include_dataflow, include_ast, force_full_rebuild, + workspaces, }) } @@ -278,6 +296,7 @@ fn resolve_pipeline_imports( collect_files: &[String], root_dir: &str, napi_aliases: &crate::types::PathAliases, + workspaces: &HashMap, ) -> (HashMap, HashSet) { let mut batch_inputs: Vec = Vec::new(); for (rel_path, symbols) in file_symbols { @@ -295,8 +314,13 @@ fn resolve_pipeline_imports( } let known_files: HashSet = collect_files.iter().map(|f| relative_path(root_dir, f)).collect(); - let resolved = - resolve::resolve_imports_batch(&batch_inputs, root_dir, napi_aliases, Some(&known_files)); + let resolved = resolve::resolve_imports_batch( + &batch_inputs, + root_dir, + napi_aliases, + Some(&known_files), + Some(workspaces), + ); let mut batch_resolved: HashMap = HashMap::new(); for r in &resolved { let key = format!("{}|{}", r.from_file, r.import_source); @@ -493,13 +517,14 @@ pub fn run_pipeline( config_json: &str, aliases_json: &str, opts_json: &str, + workspaces_json: &str, ) -> Result { let total_start = Instant::now(); let mut timing = PipelineTiming::default(); // ── Stage 1: Deserialize config ──────────────────────────────────── let t0 = Instant::now(); - let setup = pipeline_setup(conn, config_json, aliases_json, opts_json)?; + let setup = pipeline_setup(conn, config_json, aliases_json, opts_json, workspaces_json)?; let PipelineSetup { config, napi_aliases, @@ -508,6 +533,7 @@ pub fn run_pipeline( include_dataflow, include_ast, force_full_rebuild, + workspaces, } = setup; timing.setup_ms = t0.elapsed().as_secs_f64() * 1000.0; @@ -596,8 +622,13 @@ pub fn run_pipeline( // ── Stage 6: Resolve imports ─────────────────────────────────────── let t0 = Instant::now(); - let (mut batch_resolved, known_files) = - resolve_pipeline_imports(&file_symbols, &collect_result.files, root_dir, &napi_aliases); + let (mut batch_resolved, known_files) = resolve_pipeline_imports( + &file_symbols, + &collect_result.files, + root_dir, + &napi_aliases, + &workspaces, + ); timing.resolve_ms = t0.elapsed().as_secs_f64() * 1000.0; // ── Stage 6b: Re-parse barrel candidates (incremental only) ───────── @@ -610,8 +641,13 @@ pub fn run_pipeline( // than recomputing it over the whole `fileSymbols` map — #1848). let barrel_candidates_added: Vec = if !change_result.is_full_build { reparse_barrel_candidates( - conn, root_dir, &napi_aliases, &known_files, - &mut file_symbols, &mut batch_resolved, + conn, + root_dir, + &napi_aliases, + &known_files, + &workspaces, + &mut file_symbols, + &mut batch_resolved, ) } else { Vec::new() @@ -629,6 +665,7 @@ pub fn run_pipeline( root_dir: root_dir.to_string(), aliases: napi_aliases.clone(), known_files, + workspaces: workspaces.clone(), }; // Build reexport map and detect barrel files. Classification is scoped to @@ -883,6 +920,7 @@ fn reparse_barrel_candidates( root_dir: &str, napi_aliases: &crate::types::PathAliases, known_files: &HashSet, + workspaces: &HashMap, file_symbols: &mut BTreeMap, batch_resolved: &mut HashMap, ) -> Vec { @@ -981,6 +1019,7 @@ fn reparse_barrel_candidates( root_dir, napi_aliases, Some(known_files), + Some(workspaces), ); for r in &resolved_batch { let key = format!("{}|{}", r.from_file, r.import_source); @@ -2198,6 +2237,7 @@ mod tests { root_dir: "/repo".to_string(), aliases: PathAliases { base_url: None, paths: vec![] }, known_files: HashSet::new(), + workspaces: HashMap::new(), } } diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs b/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs index db058aa03..0a5ae03ec 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/import_edges.rs @@ -39,6 +39,8 @@ pub struct ImportEdgeContext { pub aliases: PathAliases, /// All known file paths (for resolution). pub known_files: HashSet, + /// Monorepo workspace packages, keyed by package name (issue #1927). + pub workspaces: HashMap, } /// An edge to insert into the database. @@ -65,6 +67,7 @@ impl ImportEdgeContext { import_source, &self.root_dir, &self.aliases, + Some(&self.workspaces), ) } @@ -796,6 +799,7 @@ mod tests { paths: vec![], }, known_files: HashSet::new(), + workspaces: HashMap::new(), }; assert!(ctx.is_barrel_file("src/index.ts")); @@ -843,6 +847,7 @@ mod tests { paths: vec![], }, known_files: HashSet::new(), + workspaces: HashMap::new(), }; let candidates = vec![ @@ -881,6 +886,7 @@ mod tests { paths: vec![], }, known_files: HashSet::new(), + workspaces: HashMap::new(), }; assert!(detect_barrel_only_files(&ctx, &[]).is_empty()); @@ -943,6 +949,7 @@ mod tests { root_dir: "/project".to_string(), aliases: PathAliases { base_url: None, paths: vec![] }, known_files: HashSet::new(), + workspaces: HashMap::new(), } } diff --git a/crates/codegraph-core/src/domain/graph/resolve.rs b/crates/codegraph-core/src/domain/graph/resolve.rs index 0ff060f6c..ca8df7e82 100644 --- a/crates/codegraph-core/src/domain/graph/resolve.rs +++ b/crates/codegraph-core/src/domain/graph/resolve.rs @@ -1,10 +1,11 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; +use std::sync::{Mutex, OnceLock}; use rayon::prelude::*; use crate::domain::parser::LanguageKind; -use crate::types::{ImportResolutionInput, PathAliases, ResolvedImport}; +use crate::types::{ImportResolutionInput, PathAliases, ResolvedImport, WorkspacePackage}; /// Check file existence using known_files set when available, falling back to FS. /// @@ -124,14 +125,191 @@ fn resolve_via_alias( None } +// ── Monorepo workspace resolution ─────────────────────────────────── +// +// Mirrors `resolveViaWorkspace()`/`setWorkspaces()`/`isWorkspaceResolved()` +// in `src/domain/graph/resolve.ts`. The JS side owns workspace *detection* +// (parsing pnpm-workspace.yaml / package.json / lerna.json — no equivalent +// exists in Rust, matching the established split documented in +// `infrastructure/config.rs`); this module only consumes the already-detected +// `{ packageName -> { dir, entry } }` map, passed in from JS on every call. + +/// A single workspace package's resolution data. Mirrors the `WorkspaceEntry` +/// interface in `src/infrastructure/config.ts`. Plain (non-napi) struct built +/// from the napi-facing [`WorkspacePackage`] list. +#[derive(Debug, Clone)] +pub struct WorkspaceEntry { + pub dir: String, + pub entry: Option, +} + +/// Convert the napi-facing workspace package list into a lookup map, keyed +/// by package name. +pub fn workspaces_from_packages(packages: &[WorkspacePackage]) -> HashMap { + packages + .iter() + .map(|p| { + ( + p.package_name.clone(), + WorkspaceEntry { + dir: p.dir.clone(), + entry: p.entry.clone(), + }, + ) + }) + .collect() +} + +/// Parse a bare specifier into `(packageName, subpath)`. Mirrors +/// `parseBareSpecifier()` in resolve.ts. +/// Scoped: `"@scope/pkg/sub"` → `("@scope/pkg", "./sub")` +/// Plain: `"pkg/sub"` → `("pkg", "./sub")` +/// No sub: `"pkg"` → `("pkg", ".")` +fn parse_bare_specifier(specifier: &str) -> Option<(String, String)> { + let (package_name, rest) = if specifier.starts_with('@') { + let parts: Vec<&str> = specifier.splitn(3, '/').collect(); + if parts.len() < 2 { + return None; + } + let package_name = format!("{}/{}", parts[0], parts[1]); + let rest = parts.get(2).copied().unwrap_or("").to_string(); + (package_name, rest) + } else { + match specifier.find('/') { + None => (specifier.to_string(), String::new()), + Some(idx) => ( + specifier[..idx].to_string(), + specifier[idx + 1..].to_string(), + ), + } + }; + let subpath = if rest.is_empty() { + ".".to_string() + } else { + format!("./{rest}") + }; + Some((package_name, subpath)) +} + +/// Extensions probed when resolving a workspace subpath import against the +/// filesystem. Mirrors the extension list in `resolveViaWorkspace()`. +const WORKSPACE_PROBE_EXTENSIONS: &[&str] = &[ + "", + ".ts", + ".tsx", + ".js", + ".jsx", + ".mjs", + "/index.ts", + "/index.tsx", + "/index.js", +]; + +/// Resolve a bare specifier through monorepo workspace packages. +/// +/// For `"@myorg/utils"` → finds the workspace package dir → resolves to its +/// entry point. For `"@myorg/utils/sub"` → finds the package dir → filesystem +/// probes `dir/sub` then `dir/src/sub`. +/// +/// Unlike `resolveViaWorkspace()` in resolve.ts, this does not attempt a +/// `package.json` `exports`-field lookup first — the native engine has no +/// `exports`-field resolver at all (tracked separately; see +/// `resolveViaExports()`'s absence from this module). This only affects +/// workspace packages that rely on a conditional `exports` map instead of +/// `main`/`source`/index-file resolution. +fn resolve_via_workspace( + specifier: &str, + workspaces: &HashMap, + root_dir: &str, + known_files: Option<&HashSet>, +) -> Option { + if workspaces.is_empty() { + return None; + } + let (package_name, subpath) = parse_bare_specifier(specifier)?; + let info = workspaces.get(&package_name)?; + + if subpath == "." { + return info.entry.clone(); + } + + let sub_rel = &subpath[2..]; // strip leading "./" + + let base = format!("{}/{}", info.dir.trim_end_matches('/'), sub_rel); + for ext in WORKSPACE_PROBE_EXTENSIONS { + let candidate = format!("{base}{ext}"); + if file_exists(&candidate, known_files, root_dir) { + return Some(candidate); + } + } + + let src_base = format!("{}/src/{}", info.dir.trim_end_matches('/'), sub_rel); + for ext in WORKSPACE_PROBE_EXTENSIONS { + let candidate = format!("{src_base}{ext}"); + if file_exists(&candidate, known_files, root_dir) { + return Some(candidate); + } + } + + None +} + +/// Process-lifetime cache of root-relative paths resolved via a workspace +/// import. Mirrors `_workspaceResolvedPaths` in resolve.ts — read by +/// `compute_confidence()` to grant workspace-resolved imports a 0.95 +/// confidence floor regardless of directory distance. +/// +/// Populated as a side effect of `resolve_import_path`/`resolve_imports_batch` +/// and reset once per build by the callers that own "start of build" timing: +/// `resolve_imports` (the per-call FFI entry point, called exactly once per +/// JS-driven build by `resolveImportsBatch()`) and `pipeline_setup` (the Rust +/// orchestrator's once-per-build setup stage). Later same-build calls (e.g. +/// the barrel re-parse loop, which calls `resolve_imports_batch` repeatedly) +/// only add to the set, matching `setWorkspaces()`'s clear-once-then-accumulate +/// contract on the JS side. +fn workspace_resolved_cache() -> &'static Mutex> { + static CACHE: OnceLock>> = OnceLock::new(); + CACHE.get_or_init(|| Mutex::new(HashSet::new())) +} + +/// Clear the workspace-resolved-paths cache. Call once per build, before any +/// resolution runs, mirroring `_workspaceResolvedPaths.clear()` inside +/// `setWorkspaces()`. +pub fn reset_workspace_resolved_paths() { + if let Ok(mut set) = workspace_resolved_cache().lock() { + set.clear(); + } +} + +fn mark_workspace_resolved(path: &str) { + if let Ok(mut set) = workspace_resolved_cache().lock() { + set.insert(path.to_string()); + } +} + +fn is_workspace_resolved(path: &str) -> bool { + workspace_resolved_cache() + .lock() + .map(|set| set.contains(path)) + .unwrap_or(false) +} + /// Resolve a single import path, mirroring `resolveImportPath()` in builder.js. pub fn resolve_import_path( from_file: &str, import_source: &str, root_dir: &str, aliases: &PathAliases, + workspaces: Option<&HashMap>, ) -> String { - resolve_import_path_inner(from_file, import_source, root_dir, aliases, None) + resolve_import_path_inner( + from_file, + import_source, + root_dir, + aliases, + None, + workspaces, + ) } /// Inner implementation with optional known_files cache. @@ -147,17 +325,32 @@ fn relativize_to_root(candidate: &str, root_dir: &str) -> String { } } -/// Resolve a non-relative (alias or bare) import source. Returns the -/// resolved path or the raw source if no alias matches (bare specifier). +/// Resolve a non-relative (alias, workspace, or bare) import source. Returns +/// the resolved path or the raw source if nothing matches (bare specifier). +/// +/// Order mirrors `resolveImportPathJS()`: aliases take priority (tsconfig/ +/// jsconfig path mappings), then workspace packages ("workspace packages +/// take priority over node_modules" — resolve.ts), then the raw specifier is +/// returned unresolved. fn resolve_non_relative_import( import_source: &str, root_dir: &str, aliases: &PathAliases, known_files: Option<&HashSet>, + workspaces: Option<&HashMap>, ) -> String { if let Some(alias_resolved) = resolve_via_alias(import_source, aliases, root_dir, known_files) { return relativize_to_root(&alias_resolved, root_dir); } + if let Some(workspaces) = workspaces { + if let Some(ws_resolved) = + resolve_via_workspace(import_source, workspaces, root_dir, known_files) + { + let rel = relativize_to_root(&ws_resolved, root_dir); + mark_workspace_resolved(&rel); + return rel; + } + } import_source.to_string() } @@ -227,9 +420,16 @@ fn resolve_import_path_inner( root_dir: &str, aliases: &PathAliases, known_files: Option<&HashSet>, + workspaces: Option<&HashMap>, ) -> String { if !import_source.starts_with('.') { - return resolve_non_relative_import(import_source, root_dir, aliases, known_files); + return resolve_non_relative_import( + import_source, + root_dir, + aliases, + known_files, + workspaces, + ); } let dir = Path::new(from_file).parent().unwrap_or(Path::new("")); @@ -333,6 +533,14 @@ pub fn compute_confidence( if imp == target_file { return 1.0; } + // Workspace-resolved imports get high confidence even across package + // boundaries — mirrors the `_workspaceResolvedPaths` check in + // `computeConfidenceJS()` (resolve.ts), backed here by the + // process-lifetime cache populated by `resolve_import_path`/ + // `resolve_imports_batch` (issue #1927). + if is_workspace_resolved(imp) { + return 0.95; + } } // Cross-language candidates are never legitimate call targets (#1783) — // reject before scoring proximity so a same-directory, same-named symbol @@ -364,6 +572,7 @@ pub fn resolve_imports_batch( root_dir: &str, aliases: &PathAliases, known_files: Option<&HashSet>, + workspaces: Option<&HashMap>, ) -> Vec { inputs .par_iter() @@ -374,6 +583,7 @@ pub fn resolve_imports_batch( root_dir, aliases, known_files, + workspaces, ); ResolvedImport { from_file: input.from_file.clone(), @@ -469,6 +679,7 @@ mod tests { "/project", &aliases, Some(&known), + None, ); assert_eq!(result, "src/bar.ts"); } @@ -490,6 +701,7 @@ mod tests { "/project", &aliases, Some(&known), + None, ); assert_eq!(result, "src/utils.ts"); } @@ -655,4 +867,226 @@ mod tests { let conf = compute_confidence("src/graph/a.ts", "src/graph/b.js", None); assert_eq!(conf, 0.7); } + + // Regression tests for #1927: `resolve_import_path_inner` had no + // workspace-awareness at all, so a bare monorepo-package specifier (e.g. + // `import "@myorg/lib"`) fell straight through to `resolve_non_relative_import`'s + // raw-specifier fallback under the native engine, unlike the WASM/JS engine's + // `resolveViaWorkspace()`. + + fn make_workspaces(entries: &[(&str, &str, Option<&str>)]) -> HashMap { + entries + .iter() + .map(|(name, dir, entry)| { + ( + name.to_string(), + WorkspaceEntry { + dir: dir.to_string(), + entry: entry.map(|e| e.to_string()), + }, + ) + }) + .collect() + } + + #[test] + fn parse_bare_specifier_scoped_package_root() { + assert_eq!( + parse_bare_specifier("@myorg/core"), + Some(("@myorg/core".to_string(), ".".to_string())) + ); + } + + #[test] + fn parse_bare_specifier_scoped_package_subpath() { + assert_eq!( + parse_bare_specifier("@myorg/core/src/helpers"), + Some(("@myorg/core".to_string(), "./src/helpers".to_string())) + ); + } + + #[test] + fn parse_bare_specifier_plain_package() { + assert_eq!( + parse_bare_specifier("lodash"), + Some(("lodash".to_string(), ".".to_string())) + ); + assert_eq!( + parse_bare_specifier("lodash/fp"), + Some(("lodash".to_string(), "./fp".to_string())) + ); + } + + #[test] + fn parse_bare_specifier_rejects_malformed_scoped_specifier() { + assert_eq!(parse_bare_specifier("@myorg"), None); + } + + #[test] + fn resolve_via_workspace_resolves_root_import_to_entry() { + let workspaces = make_workspaces(&[( + "@myorg/core", + "packages/core", + Some("packages/core/src/index.js"), + )]); + let result = resolve_via_workspace("@myorg/core", &workspaces, "/project", None); + assert_eq!(result, Some("packages/core/src/index.js".to_string())); + } + + #[test] + fn resolve_via_workspace_returns_none_when_entry_missing() { + let workspaces = make_workspaces(&[("@myorg/broken", "packages/broken", None)]); + let result = resolve_via_workspace("@myorg/broken", &workspaces, "/project", None); + assert_eq!(result, None); + } + + #[test] + fn resolve_via_workspace_resolves_subpath_via_known_files_probe() { + let mut known = HashSet::new(); + known.insert("packages/core/src/helpers.js".to_string()); + let workspaces = make_workspaces(&[("@myorg/core", "packages/core", None)]); + let result = resolve_via_workspace( + "@myorg/core/src/helpers", + &workspaces, + "/project", + Some(&known), + ); + assert_eq!(result, Some("packages/core/src/helpers.js".to_string())); + } + + #[test] + fn resolve_via_workspace_resolves_subpath_via_src_convention() { + let mut known = HashSet::new(); + known.insert("packages/core/src/helpers.js".to_string()); + let workspaces = make_workspaces(&[("@myorg/core", "packages/core", None)]); + let result = + resolve_via_workspace("@myorg/core/helpers", &workspaces, "/project", Some(&known)); + assert_eq!(result, Some("packages/core/src/helpers.js".to_string())); + } + + #[test] + fn resolve_via_workspace_returns_none_for_unknown_package() { + let workspaces = make_workspaces(&[("@myorg/core", "packages/core", None)]); + assert_eq!( + resolve_via_workspace("@myorg/unknown", &workspaces, "/project", None), + None + ); + } + + #[test] + fn resolve_via_workspace_returns_none_when_no_workspaces_registered() { + let workspaces: HashMap = HashMap::new(); + assert_eq!( + resolve_via_workspace("@myorg/core", &workspaces, "/project", None), + None + ); + } + + #[test] + fn resolve_non_relative_import_prefers_workspace_over_raw_specifier() { + let workspaces = make_workspaces(&[( + "@myorg/lib", + "packages/lib", + Some("/project/packages/lib/src/index.js"), + )]); + let aliases = PathAliases { + base_url: None, + paths: vec![], + }; + let resolved = resolve_non_relative_import( + "@myorg/lib", + "/project", + &aliases, + None, + Some(&workspaces), + ); + assert_eq!(resolved, "packages/lib/src/index.js"); + } + + #[test] + fn resolve_non_relative_import_falls_back_to_raw_specifier_without_workspace_match() { + let workspaces = make_workspaces(&[("@myorg/lib", "packages/lib", None)]); + let aliases = PathAliases { + base_url: None, + paths: vec![], + }; + let resolved = + resolve_non_relative_import("lodash", "/project", &aliases, None, Some(&workspaces)); + assert_eq!(resolved, "lodash"); + } + + // Serializes access to the process-lifetime workspace-resolved-paths + // cache: `cargo test` runs tests in parallel threads within one process, + // and `reset_workspace_resolved_paths()` would otherwise race with + // concurrent assertions in other tests below. + static WORKSPACE_CACHE_TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn with_workspace_cache_lock(f: F) { + let guard = WORKSPACE_CACHE_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + reset_workspace_resolved_paths(); + f(); + reset_workspace_resolved_paths(); + drop(guard); + } + + #[test] + fn resolve_import_path_marks_workspace_resolved_paths() { + with_workspace_cache_lock(|| { + let workspaces = make_workspaces(&[( + "@myorg/lib", + "packages/lib", + Some("/project/packages/lib/src/index.js"), + )]); + let aliases = PathAliases { + base_url: None, + paths: vec![], + }; + let resolved = resolve_import_path( + "/project/apps/web/src/app.js", + "@myorg/lib", + "/project", + &aliases, + Some(&workspaces), + ); + assert_eq!(resolved, "packages/lib/src/index.js"); + assert!(is_workspace_resolved("packages/lib/src/index.js")); + }); + } + + #[test] + fn compute_confidence_returns_0_95_for_workspace_resolved_import() { + with_workspace_cache_lock(|| { + mark_workspace_resolved("packages/lib/src/index.js"); + let conf = compute_confidence( + "apps/web/src/app.js", + "packages/lib/src/utils.js", + Some("packages/lib/src/index.js"), + ); + assert_eq!(conf, 0.95); + }); + } + + #[test] + fn compute_confidence_does_not_boost_non_workspace_imports() { + with_workspace_cache_lock(|| { + let conf = compute_confidence( + "apps/web/src/app.js", + "some/distant/file.js", + Some("some/other/import.js"), + ); + assert!(conf < 0.95); + }); + } + + #[test] + fn reset_workspace_resolved_paths_clears_previously_marked_entries() { + with_workspace_cache_lock(|| { + mark_workspace_resolved("packages/lib/src/index.js"); + assert!(is_workspace_resolved("packages/lib/src/index.js")); + reset_workspace_resolved_paths(); + assert!(!is_workspace_resolved("packages/lib/src/index.js")); + }); + } } diff --git a/crates/codegraph-core/src/lib.rs b/crates/codegraph-core/src/lib.rs index a4e7a3a29..bfea5f561 100644 --- a/crates/codegraph-core/src/lib.rs +++ b/crates/codegraph-core/src/lib.rs @@ -92,27 +92,47 @@ pub fn parse_files_full( } /// Resolve a single import path. +/// +/// `workspaces` carries the caller's already-detected monorepo workspace +/// packages (see `detectWorkspaces()`/`setWorkspaces()` in +/// `src/infrastructure/config.ts` — the native engine has no workspace +/// *detection* of its own, only resolution against a supplied map; issue #1927). #[napi] pub fn resolve_import( from_file: String, import_source: String, root_dir: String, aliases: Option, + workspaces: Option>, ) -> String { let aliases = aliases.unwrap_or(PathAliases { base_url: None, paths: vec![], }); - domain::graph::resolve::resolve_import_path(&from_file, &import_source, &root_dir, &aliases) + let workspace_map = workspaces.map(|w| domain::graph::resolve::workspaces_from_packages(&w)); + domain::graph::resolve::resolve_import_path( + &from_file, + &import_source, + &root_dir, + &aliases, + workspace_map.as_ref(), + ) } /// Batch resolve multiple imports. +/// +/// Resets the process-lifetime workspace-resolved-paths cache (read by +/// `compute_confidence()`) before resolving — this is the once-per-build +/// entry point on the per-call FFI path (called exactly once per build by +/// `resolveImportsBatch()` in resolve.ts); see +/// `reset_workspace_resolved_paths()`'s doc comment for the full contract. #[napi] pub fn resolve_imports( inputs: Vec, root_dir: String, aliases: Option, known_files: Option>, + workspaces: Option>, ) -> Vec { let aliases = aliases.unwrap_or(PathAliases { base_url: None, @@ -120,7 +140,15 @@ pub fn resolve_imports( }); let known_set = known_files.map(|v| v.into_iter().collect::>()); - domain::graph::resolve::resolve_imports_batch(&inputs, &root_dir, &aliases, known_set.as_ref()) + let workspace_map = workspaces.map(|w| domain::graph::resolve::workspaces_from_packages(&w)); + domain::graph::resolve::reset_workspace_resolved_paths(); + domain::graph::resolve::resolve_imports_batch( + &inputs, + &root_dir, + &aliases, + known_set.as_ref(), + workspace_map.as_ref(), + ) } /// Compute proximity-based confidence for call resolution. diff --git a/crates/codegraph-core/src/types.rs b/crates/codegraph-core/src/types.rs index b22cf2e7c..fea6d4fb6 100644 --- a/crates/codegraph-core/src/types.rs +++ b/crates/codegraph-core/src/types.rs @@ -584,6 +584,32 @@ pub struct AliasMapping { pub targets: Vec, } +/// A single monorepo workspace package, mirroring `WorkspaceEntry` in +/// `src/infrastructure/config.ts`. `entry` is `None` when no resolvable +/// entry point was found for the package (missing `main`/`source`/index +/// file). Serves double duty: passed as a napi array argument to +/// `resolve_import`/`resolve_imports` for the per-call FFI path, and +/// deserialized from the `workspaces_json` blob `NativeDatabase::build_graph` +/// receives for the full Rust orchestrator path (both use the same +/// `{ packageName, dir, entry }` JSON shape). +/// +/// `#[serde(rename_all = "camelCase")]` is required here even though +/// `#[napi(object)]` already camelCases fields for direct FFI calls — that +/// conversion is a separate mechanism from `serde_json`, which only sees the +/// literal Rust field names unless told otherwise. Without it, +/// `serde_json::from_str` on `workspaces_json` (camelCase, produced by +/// `JSON.stringify(getWorkspacesForNative(...))`) fails with "missing field +/// `package_name`" (mirrors `BuildPathAliases`'s reason for existing +/// alongside `PathAliases` in infrastructure/config.rs). +#[napi(object)] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct WorkspacePackage { + pub package_name: String, + pub dir: String, + pub entry: Option, +} + #[napi(object)] #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ImportResolutionInput { diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index 73bce2e28..e60862794 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -43,7 +43,7 @@ import { parseFilesWasmForBackfill, patchDataflowResult, } from '../../../parser.js'; -import { computeConfidence } from '../../resolve.js'; +import { computeConfidence, getWorkspacesForNative } from '../../resolve.js'; import type { CallNodeLookup } from '../call-resolver.js'; import { resolveDefinePropertyAccessorTarget } from '../call-resolver.js'; import type { ChaContext } from '../cha.js'; @@ -2445,6 +2445,7 @@ class NativeOrchestrationSession { JSON.stringify(this.ctx.config), JSON.stringify(this.ctx.aliases), JSON.stringify(this.ctx.opts), + JSON.stringify(getWorkspacesForNative(this.ctx.rootDir)), ); } finally { // Restore FK enforcement so any subsequent writes to this connection diff --git a/src/domain/graph/resolve.ts b/src/domain/graph/resolve.ts index 0c262664f..6a1093e69 100644 --- a/src/domain/graph/resolve.ts +++ b/src/domain/graph/resolve.ts @@ -4,7 +4,13 @@ import { debug } from '../../infrastructure/logger.js'; import { loadNative } from '../../infrastructure/native.js'; import { normalizePath } from '../../shared/constants.js'; import { toErrorMessage } from '../../shared/errors.js'; -import type { BareSpecifier, BatchResolvedMap, ImportBatchItem, PathAliases } from '../../types.js'; +import type { + BareSpecifier, + BatchResolvedMap, + ImportBatchItem, + NativeWorkspacePackage, + PathAliases, +} from '../../types.js'; import { LANGUAGE_REGISTRY } from '../parser.js'; // ── package.json exports resolution ───────────────────────────────── @@ -215,6 +221,22 @@ function getWorkspaces(rootDir: string): Map ({ + packageName, + dir: info.dir, + entry: info.entry, + })); +} + /** * Resolve a bare specifier through monorepo workspace packages. * @@ -600,6 +622,7 @@ export function resolveImportPath( importSource, rootDir, convertAliasesForNative(aliases), + getWorkspacesForNative(rootDir), ); const normalized = normalizePath(path.normalize(result)); // The native resolver's .js → .ts remap fails when paths contain @@ -660,6 +683,7 @@ export function resolveImportsBatch( rootDir, convertAliasesForNative(aliases), knownFiles || null, + getWorkspacesForNative(rootDir), ); const map: BatchResolvedMap = new Map(); for (const r of results) { diff --git a/src/types.ts b/src/types.ts index 5629fed0c..905524a2e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2315,17 +2315,36 @@ export type StmtCache = WeakMap, rootDir: string, aliases: unknown, knownFiles: string[] | null, + workspaces?: NativeWorkspacePackage[] | null, ): Array<{ fromFile: string; importSource: string; resolvedPath: string }>; computeConfidence(callerFile: string, targetFile: string, importedFrom: string | null): number; detectCycles(edges: Array<{ source: string; target: string }>): string[][]; @@ -2971,7 +2990,13 @@ export interface NativeDatabase { * Returns a JSON string with timing and build result data. * When unavailable, the JS pipeline (runPipelineStages) is used as fallback. */ - buildGraph?(rootDir: string, configJson: string, aliasesJson: string, optsJson: string): string; + buildGraph?( + rootDir: string, + configJson: string, + aliasesJson: string, + optsJson: string, + workspacesJson?: string, + ): string; } // ════════════════════════════════════════════════════════════════════════ diff --git a/tests/fixtures/monorepo-workspace/apps/web/index.js b/tests/fixtures/monorepo-workspace/apps/web/index.js new file mode 100644 index 000000000..5bf80c2c2 --- /dev/null +++ b/tests/fixtures/monorepo-workspace/apps/web/index.js @@ -0,0 +1,5 @@ +import { add, multiply } from '@myorg/lib'; + +export function calculate(a, b) { + return add(a, b) + multiply(a, b); +} diff --git a/tests/fixtures/monorepo-workspace/apps/web/package.json b/tests/fixtures/monorepo-workspace/apps/web/package.json new file mode 100644 index 000000000..1ac316914 --- /dev/null +++ b/tests/fixtures/monorepo-workspace/apps/web/package.json @@ -0,0 +1,5 @@ +{ + "name": "@myorg/web", + "version": "1.0.0", + "main": "./index.js" +} diff --git a/tests/fixtures/monorepo-workspace/package.json b/tests/fixtures/monorepo-workspace/package.json new file mode 100644 index 000000000..7f033d23f --- /dev/null +++ b/tests/fixtures/monorepo-workspace/package.json @@ -0,0 +1,8 @@ +{ + "name": "monorepo-workspace-fixture", + "private": true, + "workspaces": [ + "packages/*", + "apps/*" + ] +} diff --git a/tests/fixtures/monorepo-workspace/packages/lib/package.json b/tests/fixtures/monorepo-workspace/packages/lib/package.json new file mode 100644 index 000000000..da924355f --- /dev/null +++ b/tests/fixtures/monorepo-workspace/packages/lib/package.json @@ -0,0 +1,5 @@ +{ + "name": "@myorg/lib", + "version": "1.0.0", + "main": "./src/index.js" +} diff --git a/tests/fixtures/monorepo-workspace/packages/lib/src/index.js b/tests/fixtures/monorepo-workspace/packages/lib/src/index.js new file mode 100644 index 000000000..e059eb589 --- /dev/null +++ b/tests/fixtures/monorepo-workspace/packages/lib/src/index.js @@ -0,0 +1,7 @@ +export function add(a, b) { + return a + b; +} + +export function multiply(a, b) { + return a * b; +} diff --git a/tests/integration/build-parity.test.ts b/tests/integration/build-parity.test.ts index a50535e66..e3585a89d 100644 --- a/tests/integration/build-parity.test.ts +++ b/tests/integration/build-parity.test.ts @@ -26,6 +26,16 @@ const PARITY_FIXTURES: Array<{ label: string; dir: string }> = [ label: 'csharp Repository', dir: path.join(FIXTURES_ROOT, 'benchmarks', 'resolution', 'fixtures', 'csharp'), }, + // Regression fixture for issue #1927: a bare monorepo workspace package + // import (`import "@myorg/lib"`) must resolve identically under both + // engines — `resolve_import_path_inner` in the native engine previously had + // no workspace-awareness at all, so the import stayed unresolved and its + // downstream call edges either dropped or lost the workspace confidence + // boost that the WASM/JS engine's `resolveViaWorkspace()` grants. + { + label: 'monorepo-workspace (JS)', + dir: path.join(FIXTURES_ROOT, 'fixtures', 'monorepo-workspace'), + }, ]; const hasNative = isNativeAvailable(); diff --git a/tests/integration/issue-1927-native-workspace-resolution.test.ts b/tests/integration/issue-1927-native-workspace-resolution.test.ts new file mode 100644 index 000000000..46c4dd9c8 --- /dev/null +++ b/tests/integration/issue-1927-native-workspace-resolution.test.ts @@ -0,0 +1,144 @@ +/** + * Regression test for #1927: the native engine's `resolve_import_path_inner` + * had no workspace-awareness at all, so a bare monorepo workspace-package + * specifier (e.g. `import "@myorg/lib"`) fell straight through to the raw- + * specifier fallback under the native engine — unlike the WASM/JS engine's + * `resolveViaWorkspace()`, which resolves it to the package's real entry file. + * + * Fixture: a two-package monorepo — + * packages/lib/package.json { name: "@myorg/lib", main: "./src/index.js" } + * packages/lib/src/index.js exports `add`/`multiply` + * apps/web/index.js `import { add, multiply } from "@myorg/lib"` + * and calls both from `calculate()` + * + * Verifies, for both engines: + * 1. The `imports` edge from apps/web/index.js resolves to the workspace + * package's real entry file, not the raw `@myorg/lib` specifier. + * 2. `calculate` gets `calls` edges to both `add` and `multiply`. + * 3. Those `calls` edges carry the 0.95 workspace-resolved confidence floor + * (mirrors `computeConfidenceJS`'s `_workspaceResolvedPaths` check). + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import type { EngineMode } from '../../src/types.js'; + +const FIXTURE: Record = { + 'package.json': JSON.stringify({ + name: 'issue-1927-fixture', + private: true, + workspaces: ['packages/*', 'apps/*'], + }), + 'packages/lib/package.json': JSON.stringify({ + name: '@myorg/lib', + version: '1.0.0', + main: './src/index.js', + }), + 'packages/lib/src/index.js': ` +export function add(a, b) { + return a + b; +} + +export function multiply(a, b) { + return a * b; +} +`, + 'apps/web/package.json': JSON.stringify({ + name: '@myorg/web', + version: '1.0.0', + main: './index.js', + }), + 'apps/web/index.js': ` +import { add, multiply } from '@myorg/lib'; + +export function calculate(a, b) { + return add(a, b) + multiply(a, b); +} +`, +}; + +const ENGINES: EngineMode[] = ['wasm', 'native']; + +describe.each(ENGINES)('monorepo workspace import resolution (#1927, %s)', (engine) => { + let tmpDir: string; + let importEdges: Array<{ src_file: string; tgt_file: string }>; + let callEdges: Array<{ src: string; tgt: string; tgt_file: string; confidence: number }>; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-1927-${engine}-`)); + for (const [rel, content] of Object.entries(FIXTURE)) { + const full = path.join(tmpDir, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + } + await buildGraph(tmpDir, { incremental: false, skipRegistry: true, engine }); + + const dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const db = new Database(dbPath, { readonly: true }); + try { + importEdges = db + .prepare( + `SELECT n1.file AS src_file, n2.file AS tgt_file + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'imports' AND n1.file = 'apps/web/index.js'`, + ) + .all() as Array<{ src_file: string; tgt_file: string }>; + callEdges = db + .prepare( + `SELECT n1.name AS src, n2.name AS tgt, n2.file AS tgt_file, e.confidence AS confidence + FROM edges e + JOIN nodes n1 ON e.source_id = n1.id + JOIN nodes n2 ON e.target_id = n2.id + WHERE e.kind = 'calls' AND n1.name = 'calculate' + ORDER BY n2.name`, + ) + .all() as Array<{ src: string; tgt: string; tgt_file: string; confidence: number }>; + } finally { + db.close(); + } + }, 60_000); + + afterAll(() => { + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('resolves the workspace-package import to the package entry file', () => { + expect( + importEdges, + `Expected an imports edge from apps/web/index.js resolved into packages/lib.\nEdges: ${JSON.stringify(importEdges, null, 2)}`, + ).toContainEqual( + expect.objectContaining({ + src_file: 'apps/web/index.js', + tgt_file: 'packages/lib/src/index.js', + }), + ); + }); + + it('emits calls edges from calculate to both workspace-imported functions', () => { + const targets = callEdges.map((e) => e.tgt); + expect(targets, `All calls edges: ${JSON.stringify(callEdges, null, 2)}`).toEqual([ + 'add', + 'multiply', + ]); + for (const edge of callEdges) { + expect(edge.tgt_file, `Edge to ${edge.tgt} should resolve into packages/lib`).toBe( + 'packages/lib/src/index.js', + ); + } + }); + + it('grants the workspace-resolved confidence floor (0.95) to cross-package calls', () => { + for (const edge of callEdges) { + expect( + edge.confidence, + `calculate -> ${edge.tgt} confidence should be >= 0.95 (workspace-resolved)`, + ).toBeGreaterThanOrEqual(0.95); + } + }); +});