From 523817bc03e1f8fd0b0e68a3166af44a9762b660 Mon Sep 17 00:00:00 2001 From: Nir Hadassi Date: Mon, 20 Apr 2026 17:27:57 +0300 Subject: [PATCH 1/2] fix: apply root fallback for source files outside sourceRoot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source-typed config files at project root (e.g. jest.config.js, webpack.config.ts when sourceRoot is "/src") are classified as source by extension but never walked by the analyzer, which only descends sourceRoot. Step 6a used to silently `continue` on such files, so the root fallback added in #61 never got a chance to run for them — changes to project-level config files went undetected. When a changed source file isn't in `analyzer.files`, fall back to `ProjectIndex::get_package_names_by_path` and mark the owning projects affected, mirroring how Step 6b handles assets. The new `root_entries` index from #61 now actually covers this case end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core.rs | 44 +++++++++++++++++++++++++++++++++++++-- tests/integration_test.rs | 23 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/core.rs b/src/core.rs index 90c32d7..064c695 100644 --- a/src/core.rs +++ b/src/core.rs @@ -135,9 +135,49 @@ fn find_affected_internal( for changed_file in &source_files { let file_path = &changed_file.file_path; - // Check if file exists in our analyzed files + // Check if file exists in our analyzed files. A source-typed file (.ts/.tsx/.js/.jsx) + // can live inside a project's root but outside its sourceRoot (e.g. jest.config.js, + // webpack.config.js at project root when sourceRoot = "/src"). The semantic + // analyzer only walks sourceRoot, so such files never reach it — but they still + // belong to the project and changing them must mark it affected. Fall back to the + // same root-based ownership lookup used for assets. if !analyzer.files.contains_key(file_path) { - debug!("Skipping unanalyzed source file: {:?}", file_path); + debug!( + "Source file not in analyzer.files, using root fallback: {:?}", + file_path + ); + let owning_packages = project_index.get_package_names_by_path(file_path); + for pkg in &owning_packages { + debug!( + "File {:?} belongs to package '{}' (via root fallback)", + file_path, pkg + ); + affected_packages.insert(pkg.clone()); + + if generate_report { + if changed_file.changed_lines.is_empty() { + project_causes + .entry(pkg.clone()) + .or_default() + .push(AffectCause::DirectChange { + file: file_path.clone(), + symbol: None, + line: 0, + }); + } else { + for &line in &changed_file.changed_lines { + project_causes + .entry(pkg.clone()) + .or_default() + .push(AffectCause::DirectChange { + file: file_path.clone(), + symbol: None, + line, + }); + } + } + } + } continue; } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 6abd72f..ea27e4e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -3293,6 +3293,29 @@ fn test_named_inputs_negation_with_root_differs_from_source_root() { ); } +#[test] +fn test_source_file_outside_sourceroot_affects_owning_project() { + // lib-a has sourceRoot = "libs/lib-a/src" but project root = "libs/lib-a". + // A source-typed config file (jest.config.js) at project root lives OUTSIDE + // sourceRoot, so the semantic analyzer never parses it. It must still mark + // its owning project as affected via the root fallback — otherwise changes + // to project-level config files would be silently ignored. + let repo = TempNxRepo::new(r#"{}"#); + + repo.change_and_commit( + "libs/lib-a/jest.config.js", + "module.exports = { workerIdleMemoryLimit: '2048MB' };\n", + ); + + let affected = repo.get_affected(); + + assert!( + affected.contains(&"lib-a".to_string()), + "lib-a should be affected (jest.config.js at project root, outside sourceRoot). Got: {:?}", + affected + ); +} + #[test] fn test_head_flag_commit_to_commit_diff() { let branch = TestBranch::new("test-head-flag"); From b1cf76fe176031cd3eb7868ad7b33e1d1470d50c Mon Sep 17 00:00:00 2001 From: Nir Hadassi Date: Mon, 20 Apr 2026 17:48:53 +0300 Subject: [PATCH 2/2] fix(utils): exclude workspace-root projects from root fallback; extract cause helper; tighten tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #65 review: - src/utils.rs: skip projects with `root == ""` or `root == "."` when building `root_entries`. The Nx loader produces `root = ""` via `strip_prefix(cwd)` for workspace-root projects; an empty path is a prefix of every path in the repo and would over-attribute every fallback lookup to the workspace project. (`.` is a no-op under `Path::starts_with` component semantics but guarded for loaders that preserve it literally / paths that arrive with a `./` prefix.) Adds two unit tests covering both cases. - src/core.rs: extract `record_direct_change_causes` helper; reuse it from both the new source root-fallback path (Step 6a) and the asset path (Step 6b) which had identical inlined blocks. - tests/integration_test.rs: tighten `test_source_file_outside_sourceroot_affects_owning_project` to an exact-match assertion (previous `.contains(...)` passed even if every project leaked through). Add `test_workspace_root_project_not_over_attributed` — a root-level project with a nested project next to it; changing the nested project's `jest.config.js` must affect only the nested project. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core.rs | 88 +++++++++++++++---------------- src/utils.rs | 106 +++++++++++++++++++++++++++++++++++++- tests/integration_test.rs | 91 ++++++++++++++++++++++++++++++-- 3 files changed, 235 insertions(+), 50 deletions(-) diff --git a/src/core.rs b/src/core.rs index 064c695..b395c0c 100644 --- a/src/core.rs +++ b/src/core.rs @@ -22,6 +22,39 @@ struct AffectedState<'a> { visited: &'a mut FxHashSet<(PathBuf, String)>, } +/// Record a `DirectChange` cause for `pkg` for each changed line (or a +/// single line-0 cause when no lines are known). Shared by both the source +/// root-fallback path (Step 6a) and the asset path (Step 6b), which +/// otherwise had identical inlined blocks. +fn record_direct_change_causes( + project_causes: &mut FxHashMap>, + pkg: &str, + file: &Path, + changed_lines: &[usize], +) { + if changed_lines.is_empty() { + project_causes + .entry(pkg.to_string()) + .or_default() + .push(AffectCause::DirectChange { + file: file.to_path_buf(), + symbol: None, + line: 0, + }); + } else { + for &line in changed_lines { + project_causes + .entry(pkg.to_string()) + .or_default() + .push(AffectCause::DirectChange { + file: file.to_path_buf(), + symbol: None, + line, + }); + } + } +} + /// Main true-affected algorithm implementation pub fn find_affected( config: TrueAffectedConfig, @@ -155,27 +188,12 @@ fn find_affected_internal( affected_packages.insert(pkg.clone()); if generate_report { - if changed_file.changed_lines.is_empty() { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: file_path.clone(), - symbol: None, - line: 0, - }); - } else { - for &line in &changed_file.changed_lines { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: file_path.clone(), - symbol: None, - line, - }); - } - } + record_direct_change_causes( + &mut project_causes, + pkg, + file_path, + &changed_file.changed_lines, + ); } } continue; @@ -304,29 +322,13 @@ fn find_affected_internal( debug!("Asset {:?} belongs to package '{}'", asset_path, pkg); affected_packages.insert(pkg.clone()); - // Record direct change cause if generating report if generate_report { - if asset_file.changed_lines.is_empty() { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: asset_path.clone(), - symbol: None, - line: 0, - }); - } else { - for &line in &asset_file.changed_lines { - project_causes - .entry(pkg.clone()) - .or_default() - .push(AffectCause::DirectChange { - file: asset_path.clone(), - symbol: None, - line, - }); - } - } + record_direct_change_causes( + &mut project_causes, + pkg, + asset_path, + &asset_file.changed_lines, + ); } } diff --git a/src/utils.rs b/src/utils.rs index b3b9d5d..69429f3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -53,8 +53,18 @@ impl ProjectIndex { map.push((project.source_root.clone(), vec![project.name.clone()])); } - // Index by root (fallback) — only when root differs from sourceRoot - if project.root != project.source_root { + // Index by root (fallback) — only when root differs from sourceRoot. + // Skip workspace-root projects (root == "" or "."): their root is a + // prefix of every path in the repo, so they'd match every fallback + // lookup and cause massive false positives on asset/config changes. + // The Nx loader produces `root = ""` via `strip_prefix(cwd)` when the + // project lives at the workspace root; guard against "." too for + // loaders that preserve it literally or when paths come in with a + // `./` prefix. + if project.root != project.source_root + && !project.root.as_os_str().is_empty() + && project.root != Path::new(".") + { if let Some(entry) = root_map.iter_mut().find(|(root, _)| *root == project.root) { entry.1.push(project.name.clone()); } else { @@ -536,4 +546,96 @@ mod tests { "file inside parent only must not attribute to child" ); } + + #[test] + fn test_project_index_workspace_root_project_excluded_from_fallback() { + // Nx workspaces commonly have a root-level project with `root: "."`. + // Such a project must NOT be added to `root_entries` — otherwise every + // file that falls through the sourceRoot check would match it (since + // every path `starts_with(".")`), producing massive false positives + // on asset and source-file changes. + let tmp = tempfile::TempDir::new().unwrap(); + let projects = vec![ + Project { + name: "workspace".to_string(), + root: PathBuf::from("."), + source_root: PathBuf::from("src"), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + Project { + name: "my-lib".to_string(), + root: "libs/my-lib".into(), + source_root: "libs/my-lib/src".into(), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + ]; + + let index = ProjectIndex::new(&projects, tmp.path()); + + // A config file at a nested project root (outside its sourceRoot) must + // resolve ONLY to that project — not to the workspace-root project. + let mut result = index.get_package_names_by_path(Path::new("libs/my-lib/jest.config.js")); + result.sort(); + assert_eq!( + result, + vec!["my-lib"], + "config file inside nested project must NOT also attribute to root workspace project" + ); + + // Likewise for a file inside the nested project's sourceRoot. + let mut result = index.get_package_names_by_path(Path::new("libs/my-lib/src/index.ts")); + result.sort(); + assert_eq!( + result, + vec!["my-lib"], + "source file inside nested project must NOT also attribute to root workspace project" + ); + + // Workspace-root sourceRoot still resolves normally — the filter only + // affects root_entries, not entries. + let result = index.get_package_names_by_path(Path::new("src/main.ts")); + assert_eq!( + result, + vec!["workspace"], + "file inside workspace sourceRoot must still attribute to workspace" + ); + } + + #[test] + fn test_project_index_empty_root_excluded_from_fallback() { + // Defensive: an empty-path root would also match everything via + // `starts_with(Path::new(""))`. Treat empty the same as ".". + let tmp = tempfile::TempDir::new().unwrap(); + let projects = vec![ + Project { + name: "workspace".to_string(), + root: PathBuf::new(), + source_root: PathBuf::from("src"), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + Project { + name: "my-lib".to_string(), + root: "libs/my-lib".into(), + source_root: "libs/my-lib/src".into(), + ts_config: None, + implicit_dependencies: vec![], + targets: vec![], + }, + ]; + + let index = ProjectIndex::new(&projects, tmp.path()); + + let result = index.get_package_names_by_path(Path::new("libs/my-lib/jest.config.js")); + assert_eq!( + result, + vec!["my-lib"], + "empty-path root must not over-attribute" + ); + } } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index ea27e4e..d6fd74e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -3307,12 +3307,93 @@ fn test_source_file_outside_sourceroot_affects_owning_project() { "module.exports = { workerIdleMemoryLimit: '2048MB' };\n", ); - let affected = repo.get_affected(); + let mut affected = repo.get_affected(); + affected.sort(); - assert!( - affected.contains(&"lib-a".to_string()), - "lib-a should be affected (jest.config.js at project root, outside sourceRoot). Got: {:?}", - affected + // Exact match — guards against the root-fallback over-attributing. lib-b + // owns nothing at this path and must not appear; a workspace-root project + // (if one existed) must not appear either. + assert_eq!( + affected, + vec!["lib-a".to_string()], + "Only lib-a should be affected by its own jest.config.js" + ); +} + +#[test] +fn test_workspace_root_project_not_over_attributed() { + // Nx workspaces commonly have a root-level project (e.g. the workspace itself + // registered with `root: ""` when loaded via strip_prefix(cwd)). Without the + // root==""/"." guard in ProjectIndex::new(), its root would prefix-match every + // path in the repo and a change to any nested project's config file would + // incorrectly cascade to the workspace project. + let tmp = tempfile::TempDir::new().unwrap(); + let root = tmp.path(); + + git_in(root, &["init", "-q"]); + git_in(root, &["config", "user.email", "test@example.com"]); + git_in(root, &["config", "user.name", "Test"]); + git_in(root, &["branch", "-M", "main"]); + + fs::write(root.join("nx.json"), r#"{}"#).unwrap(); + + // Workspace-root project (root == cwd) + fs::write( + root.join("project.json"), + r#"{ "name": "workspace", "sourceRoot": "src" }"#, + ) + .unwrap(); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/main.ts"), "export const a = 1;\n").unwrap(); + + // Nested project with sourceRoot != root + fs::create_dir_all(root.join("libs/lib-a/src")).unwrap(); + fs::write( + root.join("libs/lib-a/project.json"), + r#"{ "name": "lib-a", "sourceRoot": "libs/lib-a/src" }"#, + ) + .unwrap(); + fs::write( + root.join("libs/lib-a/src/index.ts"), + "export const b = 2;\n", + ) + .unwrap(); + + git_in(root, &["add", "."]); + git_in(root, &["commit", "-q", "-m", "init"]); + git_in(root, &["checkout", "-q", "-b", "test-branch"]); + + // Change a config file inside lib-a's root but outside lib-a's sourceRoot. + fs::write( + root.join("libs/lib-a/jest.config.js"), + "module.exports = {};\n", + ) + .unwrap(); + git_in(root, &["add", "."]); + git_in(root, &["commit", "-q", "-m", "change jest config"]); + + let projects = domino::workspace::discover_projects(root).unwrap(); + let config = TrueAffectedConfig { + cwd: root.to_path_buf(), + base: "main".to_string(), + head: None, + root_ts_config: None, + projects, + include: vec![], + ignored_paths: vec![], + lockfile_strategy: LockfileStrategy::None, + }; + + let profiler = Arc::new(Profiler::new(false)); + let mut affected = find_affected(config, profiler) + .expect("find_affected failed") + .affected_projects; + affected.sort(); + + assert_eq!( + affected, + vec!["lib-a".to_string()], + "Only lib-a should be affected — workspace-root project must not match via root fallback" ); }