diff --git a/.alfonso/release-notes/next/skip-nested-worktrees.md b/.alfonso/release-notes/next/skip-nested-worktrees.md new file mode 100644 index 000000000..b09cdb83b --- /dev/null +++ b/.alfonso/release-notes/next/skip-nested-worktrees.md @@ -0,0 +1 @@ +AFT no longer indexes linked worktrees nested inside a project folder a second time, so search and inspect stay fast and return no duplicate hits (#746, thanks @moynzzz). diff --git a/benchmarks/aft-search/slice-descriptors/land-skip-nested-worktrees.json b/benchmarks/aft-search/slice-descriptors/land-skip-nested-worktrees.json new file mode 100644 index 000000000..c3d528a59 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-skip-nested-worktrees.json @@ -0,0 +1,15 @@ +{ + "slice_class": "engine_unwired", + "targeted_mechanism": "none", + "kind": "harness", + "fixtures": [ + "crates/aft/tests/integration/tool_call_parity_test.rs", + "crates/aft/src/search_index.rs", + "crates/aft/src/commands/semantic_search/mod.rs", + "crates/aft/src/lib.rs" + ], + "slices": [ + "skip-nested-worktrees" + ], + "note": "The ranking fence hits are search_index.rs and semantic_search/mod.rs, whose project walks now prune linked worktrees of the same repository nested under the root, and lib.rs, which registers the new test module. No benchmark corpus contains such a worktree, so ranked rows must remain byte-identical to the committed reference; scoring, admission and routing are unchanged." +} diff --git a/crates/aft/src/callgraph.rs b/crates/aft/src/callgraph.rs index 09f45bf7d..0df37cb00 100644 --- a/crates/aft/src/callgraph.rs +++ b/crates/aft/src/callgraph.rs @@ -3961,6 +3961,9 @@ fn walk_project_paths_filtered_while_with_builder( configure_builder(&mut builder); let filter_keep_walking = std::sync::Arc::new(keep_walking); let filter_budget = std::sync::Arc::clone(&filter_keep_walking); + // Skip nested linked worktrees of this repository: their files are indexed + // under their own roots. The walk root itself is always admitted (depth 0). + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(root); let mut walker = crate::context::apply_project_ignore_rules(&mut builder, root) .filter_entry(move |entry| { #[cfg(test)] @@ -3982,6 +3985,7 @@ fn walk_project_paths_filtered_while_with_builder( let file_type = entry.file_type(); if crate::project_content::is_excluded_with_file_type(entry.path(), file_type.as_ref()) || !include(entry.path()) + || crate::search_index::is_nested_worktree_entry(entry, &worktrees) { return false; } diff --git a/crates/aft/src/commands/configure.rs b/crates/aft/src/commands/configure.rs index 22f703d3a..7b36b38e7 100644 --- a/crates/aft/src/commands/configure.rs +++ b/crates/aft/src/commands/configure.rs @@ -3044,6 +3044,44 @@ fn walk_semantic_project_files_bounded( walk_project_files_bounded_matching(root, &filters, max_files, is_semantic_indexed_extension) } +#[cfg(test)] +mod nested_worktree_semantic_tests { + use super::*; + + #[test] + fn semantic_enumeration_skips_nested_linked_worktrees() { + let dir = tempfile::tempdir().unwrap(); + let fixture = + crate::git_probe::init_repo_with_nested_worktree_for_test(dir.path(), "semantic"); + let files = walk_semantic_project_files_bounded(&fixture.root, 1000) + .expect("fixture walk must complete") + .into_iter() + .map(|path| { + path.strip_prefix(&fixture.root) + .unwrap() + .to_string_lossy() + .replace('\\', "/") + }) + .collect::>(); + assert!( + files.contains("src/main.rs"), + "missing main file: {files:?}" + ); + assert!( + files.contains("vendor/independent/inner.rs"), + "nested independent repository must still be walked: {files:?}" + ); + assert!( + files.contains("submod/code.rs"), + "submodule-style checkout must still be walked: {files:?}" + ); + assert!( + !files.iter().any(|path| path.starts_with("linked/")), + "nested linked worktree files were enumerated: {files:?}" + ); + } +} + #[cfg(debug_assertions)] fn delay_search_rebuild_publish_for_debug() { let Some(delay_ms) = std::env::var("AFT_TEST_SEARCH_REBUILD_PUBLISH_DELAY_MS") diff --git a/crates/aft/src/commands/semantic_search/mod.rs b/crates/aft/src/commands/semantic_search/mod.rs index 7ee71f76e..32f3443fd 100644 --- a/crates/aft/src/commands/semantic_search/mod.rs +++ b/crates/aft/src/commands/semantic_search/mod.rs @@ -7164,6 +7164,9 @@ fn collect_degraded_grep_files( // abort the daemon if a disappearing child mount reports ENXIO. let skipped_foreign_mounts = Arc::new(AtomicUsize::new(0)); let boundary = crate::walk_boundary::DeviceBoundary::for_root(project_root).ok(); + // Skip nested linked worktrees of this repository; the walk root itself is + // always admitted (depth 0). + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(project_root); let mut builder = ignore::WalkBuilder::new(project_root); builder.same_file_system(true).hidden(false); let walker = crate::context::apply_project_ignore_rules(&mut builder, project_root) @@ -7186,7 +7189,8 @@ fn collect_degraded_grep_files( if crate::project_content::is_excluded_with_file_type( entry.path(), file_type.as_ref(), - ) { + ) || crate::search_index::is_nested_worktree_entry(entry, &worktrees) + { return false; } let name = entry.file_name().to_string_lossy(); diff --git a/crates/aft/src/git_probe.rs b/crates/aft/src/git_probe.rs index ce61d413b..e9d7240a6 100644 --- a/crates/aft/src/git_probe.rs +++ b/crates/aft/src/git_probe.rs @@ -446,6 +446,87 @@ pub(crate) fn init_repository_with_corrupt_head_for_test(path: &Path) { std::fs::write(path.join(".git").join("HEAD"), "not a ref\n").unwrap(); } +/// A real repository with every nested-checkout shape below its root: +/// - `linked/`: a linked worktree of this repository (`git worktree add`), to +/// be skipped by project walks; +/// - `vendor/independent/`: a nested independent repository (its own `.git` +/// directory), still walked; +/// - `submod/`: a submodule-style checkout (a `.git` file naming +/// `/.git/modules/submod`), still walked. +#[cfg(test)] +pub(crate) struct NestedWorktreeFixture { + pub root: PathBuf, + pub worktree: PathBuf, + pub independent: PathBuf, + pub submodule: PathBuf, +} + +/// Test fixture: `root` becomes the repository above, with `marker` written +/// into one committed source file. Asserts Git runs and every step succeeds. +#[cfg(test)] +pub(crate) fn init_repo_with_nested_worktree_for_test( + root: &Path, + marker: &str, +) -> NestedWorktreeFixture { + assert!( + crate::developer_tools::git_usable(), + "nested-worktree fixtures need git" + ); + let root = std::fs::canonicalize(root).unwrap_or_else(|_| root.to_path_buf()); + let git = |dir: &Path, args: &[&str]| { + let output = command() + .args([ + "-c", + "user.name=AFT Tests", + "-c", + "user.email=aft-tests@example.com", + "-c", + "commit.gpgsign=false", + ]) + .args(args) + .current_dir(dir) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .env("GIT_CONFIG_COUNT", "0") + .output() + .expect("git runs"); + assert!(output.status.success(), "git {args:?}: {output:?}"); + }; + let source = format!("pub fn probe_marker() {{ let _ = \"{marker}\"; }}\n"); + for relative in ["src/main.rs", "lib/util.rs"] { + let path = root.join(relative); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, &source).unwrap(); + } + git(&root, &["init", "--quiet"]); + git(&root, &["add", "."]); + git(&root, &["commit", "--quiet", "-m", "initial"]); + // A linked worktree of this repository, nested in a subfolder of the main + // checkout: the worktree's `.git` file names its admin dir under the + // main checkout's `.git/worktrees/`. + git(&root, &["worktree", "add", "--quiet", "linked"]); + let worktree = root.join("linked"); + std::fs::write(worktree.join("extra.rs"), &source).unwrap(); + // A nested independent repository: its own `.git` directory. + let independent = root.join("vendor").join("independent"); + std::fs::create_dir_all(&independent).unwrap(); + git(&independent, &["init", "--quiet"]); + std::fs::write(independent.join("inner.rs"), &source).unwrap(); + // A submodule-style checkout: a `.git` file whose gitdir lives under + // `.git/modules/`, not `worktrees/`. + let submodule = root.join("submod"); + std::fs::create_dir_all(&submodule).unwrap(); + std::fs::create_dir_all(root.join(".git").join("modules").join("submod")).unwrap(); + std::fs::write(submodule.join(".git"), "gitdir: ../.git/modules/submod\n").unwrap(); + std::fs::write(submodule.join("code.rs"), &source).unwrap(); + NestedWorktreeFixture { + root, + worktree, + independent, + submodule, + } +} + /// Test fixture: a `.git` folder holding only `info/exclude` and no HEAD, which /// Git itself does not treat as a repository. #[cfg(test)] diff --git a/crates/aft/src/grep_executor.rs b/crates/aft/src/grep_executor.rs index fa4f043df..809dc34d1 100644 --- a/crates/aft/src/grep_executor.rs +++ b/crates/aft/src/grep_executor.rs @@ -1215,6 +1215,12 @@ fn target_walk_builder( // A disappearing child mount can make ReadDir::drop panic on ENXIO and abort // the daemon, so never open directories outside this walk root's filesystem. builder.same_file_system(true).hidden(false); + // Skip nested linked worktrees of this repository: their files are indexed + // under their own roots, and walking them here duplicates every file. + // The named starting path itself is always admitted (depth 0), so an + // explicitly named target inside a worktree still walks, exactly like an + // explicitly named ignored directory. + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(search_root); crate::context::apply_project_ignore_rules(&mut builder, search_root) .parents(!crate::context::confinement::active()) .filter_entry(move |entry| { @@ -1224,6 +1230,10 @@ fn target_walk_builder( floor_disclosure.record_source_exclusion(entry.path()); return false; } + if crate::search_index::is_nested_worktree_entry(entry, &worktrees) { + floor_disclosure.record_source_exclusion(entry.path()); + return false; + } if !crate::bash_background::registry::safe_task_entry(entry.path()) { return false; } diff --git a/crates/aft/src/inspect/cache.rs b/crates/aft/src/inspect/cache.rs index 0a5dd3b63..fa9b59145 100644 --- a/crates/aft/src/inspect/cache.rs +++ b/crates/aft/src/inspect/cache.rs @@ -2572,8 +2572,14 @@ fn walk_resolver_config_files(project_root: &Path) -> BTreeSet { // Prevent a disappearing child mount from making ReadDir::drop abort on ENXIO. let mut builder = ignore::WalkBuilder::new(project_root); builder.same_file_system(true).hidden(true); + // Resolver configs inside a nested linked worktree of this repository + // belong to that checkout's own scope, not this one. + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(project_root); let walker = crate::context::apply_project_ignore_rules(&mut builder, project_root) - .filter_entry(|entry| { + .filter_entry(move |entry| { + if crate::search_index::is_nested_worktree_entry(entry, &worktrees) { + return false; + } let name = entry.file_name().to_string_lossy(); if entry .file_type() @@ -2866,6 +2872,34 @@ fn now_nanos() -> u128 { pub(crate) mod tests { use super::*; + #[test] + fn resolver_configs_skip_nested_linked_worktrees() { + // tsconfig files inside a nested linked worktree belong to that + // checkout's own scope, not this one's. + let dir = tempfile::tempdir().unwrap(); + let fixture = + crate::git_probe::init_repo_with_nested_worktree_for_test(dir.path(), "resolver"); + for config_dir in [&fixture.root, &fixture.worktree, &fixture.independent] { + std::fs::write(config_dir.join("tsconfig.json"), r#"{"files": []}"#).unwrap(); + } + let configs = walk_resolver_config_files(&fixture.root); + assert!( + configs.contains(&canonical_file_path(fixture.root.join("tsconfig.json")).unwrap()), + "missing main config: {configs:?}" + ); + assert!( + configs + .contains(&canonical_file_path(fixture.independent.join("tsconfig.json")).unwrap()), + "nested independent repository must still be walked: {configs:?}" + ); + assert!( + !configs + .iter() + .any(|path| path.starts_with(&fixture.worktree)), + "nested linked worktree configs were collected: {configs:?}" + ); + } + fn cache_epoch_fixture( marker: Option, ) -> (tempfile::TempDir, PathBuf, PathBuf, JobKey, String) { diff --git a/crates/aft/src/inspect/entry_points.rs b/crates/aft/src/inspect/entry_points.rs index 2b97d87c4..f764ecaa7 100644 --- a/crates/aft/src/inspect/entry_points.rs +++ b/crates/aft/src/inspect/entry_points.rs @@ -154,6 +154,9 @@ fn collect_manifests(project_root: &Path, manifests: &mut ManifestPaths) { } fn entry_point_walk_files(project_root: &Path) -> Vec { + // Skip nested linked worktrees of this repository; the walk root itself + // is always admitted (depth 0). + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(project_root); let mut builder = ignore::WalkBuilder::new(project_root); // Prevent a disappearing child mount from making ReadDir::drop abort on ENXIO. builder @@ -164,7 +167,12 @@ fn entry_point_walk_files(project_root: &Path) -> Vec { .git_exclude(true) .require_git(false) .add_custom_ignore_filename(".aftignore") - .filter_entry(|entry| { + .filter_entry(move |entry| { + // Manifests inside a nested linked worktree of this repository + // belong to that checkout's own scope, not this one. + if crate::search_index::is_nested_worktree_entry(entry, &worktrees) { + return false; + } !entry .file_type() .is_some_and(|file_type| file_type.is_dir()) @@ -581,6 +589,9 @@ fn collect_framework_route_files( fn framework_route_walk_files(package_dir: &Path) -> Vec { let package_dir = snapshot_path(package_dir); + // Skip nested linked worktrees of this repository; the walk root itself + // is always admitted (depth 0). + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(&package_dir); // A vanished framework mount could otherwise make ReadDir::drop abort on ENXIO. let mut builder = ignore::WalkBuilder::new(&package_dir); let package_root = package_dir.clone(); @@ -593,6 +604,9 @@ fn framework_route_walk_files(package_dir: &Path) -> Vec { .require_git(false) .add_custom_ignore_filename(".aftignore") .filter_entry(move |entry| { + if crate::search_index::is_nested_worktree_entry(entry, &worktrees) { + return false; + } if !entry .file_type() .is_some_and(|file_type| file_type.is_dir()) @@ -1129,6 +1143,29 @@ pub(crate) fn top_preview_symbols(items: &[Value]) -> Vec { mod tests { use super::*; + #[test] + fn entry_point_manifests_skip_nested_linked_worktrees() { + // package.json files inside a nested linked worktree belong to that + // checkout's own scope, not this one's. + let dir = tempfile::tempdir().unwrap(); + let fixture = + crate::git_probe::init_repo_with_nested_worktree_for_test(dir.path(), "entry_points"); + for manifest_dir in [&fixture.root, &fixture.worktree] { + std::fs::write(manifest_dir.join("package.json"), r#"{"name": "fixture"}"#).unwrap(); + } + let manifests = collect_entry_point_manifests(&fixture.root); + assert!( + manifests.contains(&snapshot_path(&fixture.root.join("package.json"))), + "missing main manifest: {manifests:?}" + ); + assert!( + !manifests + .iter() + .any(|path| path.starts_with(snapshot_path(&fixture.worktree))), + "nested linked worktree manifests were collected: {manifests:?}" + ); + } + fn published_api_fixture(wildcard: bool, index: &str, default_live: bool) { use crate::inspect::oxc_engine::{ analyze_files_with_cache, AnalyzeOptions, LivenessVerdict, OxcFactsCache, diff --git a/crates/aft/src/lib.rs b/crates/aft/src/lib.rs index c36fb03cb..869c1a3df 100644 --- a/crates/aft/src/lib.rs +++ b/crates/aft/src/lib.rs @@ -143,6 +143,8 @@ pub mod memory; pub mod migrate_storage; pub mod migration; pub mod ndjson_text; +#[cfg(test)] +mod nested_worktree_tests; pub mod ort_lifecycle; pub mod ort_pin; pub(crate) mod os_metadata; diff --git a/crates/aft/src/nested_worktree_tests.rs b/crates/aft/src/nested_worktree_tests.rs new file mode 100644 index 000000000..c7ef5a3c5 --- /dev/null +++ b/crates/aft/src/nested_worktree_tests.rs @@ -0,0 +1,298 @@ +//! Cross-walker regression tests: a directory that is a nested linked +//! worktree of this repository must be skipped by every content-reading walk, +//! while a nested independent repository and a submodule-style checkout are +//! still walked. An explicitly named target inside a worktree keeps the rule +//! that already applies to explicitly named gitignored paths: a named file is +//! read directly, and a named directory is walked with the index bypassed. + +use std::collections::BTreeSet; +use std::path::PathBuf; + +const MARKER: &str = "nested_worktree_probe_746"; + +fn fixture() -> (tempfile::TempDir, crate::git_probe::NestedWorktreeFixture) { + let dir = tempfile::tempdir().unwrap(); + let fixture = crate::git_probe::init_repo_with_nested_worktree_for_test(dir.path(), MARKER); + (dir, fixture) +} + +fn relative_set( + root: &std::path::Path, + paths: impl IntoIterator, +) -> BTreeSet { + paths + .into_iter() + .map(|path| { + path.strip_prefix(root) + .unwrap_or_else(|_| panic!("{} is outside {}", path.display(), root.display())) + .to_string_lossy() + .replace('\\', "/") + }) + .collect() +} + +#[test] +fn boundary_marks_only_this_repositories_linked_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let boundary = crate::project_content::NestedWorktreeBoundary::for_root(root); + assert!( + boundary.is_nested_worktree_root(&fixture.worktree), + "linked worktree not detected" + ); + // Repeated lookups agree (per-directory verdict cache). + assert!(boundary.is_nested_worktree_root(&fixture.worktree)); + assert!( + !boundary.is_nested_worktree_root(root), + "project root is never a worktree" + ); + assert!( + !boundary.is_nested_worktree_root(&fixture.independent), + "nested independent repository must still be walked" + ); + assert!( + !boundary.is_nested_worktree_root(&fixture.submodule), + "submodule-style gitdir must still be walked" + ); + assert!(!boundary.is_nested_worktree_root(&root.join("src"))); + assert!(boundary.is_under_nested_worktree(&fixture.worktree.join("src/main.rs"))); + assert!(boundary.is_under_nested_worktree(&fixture.worktree)); + assert!(!boundary.is_under_nested_worktree(&root.join("src/main.rs"))); + assert!(!boundary.is_under_nested_worktree(root)); + assert!(!boundary.is_under_nested_worktree(&fixture.independent.join("inner.rs"))); + assert!(!boundary.is_under_nested_worktree(&fixture.submodule.join("code.rs"))); +} + +#[test] +fn boundary_ignores_a_worktree_of_another_repository() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + // A `.git` file naming a `worktrees/` entry of a different common dir is + // a foreign checkout nested here, not this repository's worktree. + let foreign = root.join("foreign"); + let foreign_admin = root.join("elsewhere.git").join("worktrees").join("foreign"); + std::fs::create_dir_all(&foreign_admin).unwrap(); + std::fs::create_dir_all(&foreign).unwrap(); + std::fs::write( + foreign.join(".git"), + format!("gitdir: {}\n", foreign_admin.display()), + ) + .unwrap(); + assert!( + !crate::project_content::NestedWorktreeBoundary::for_root(root) + .is_nested_worktree_root(&foreign), + "foreign worktrees entry must still be walked" + ); +} + +#[test] +fn trigram_project_walk_skips_nested_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let walked = relative_set( + root, + crate::search_index::project_walk_builder(root) + .build() + .filter_map(Result::ok) + .filter(|entry| entry.file_type().is_some_and(|kind| kind.is_file())) + .map(|entry| entry.into_path()), + ); + assert!( + walked.contains("src/main.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("lib/util.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("vendor/independent/inner.rs"), + "nested independent repository must still be walked: {walked:?}" + ); + assert!( + walked.contains("submod/code.rs"), + "submodule-style checkout must still be walked: {walked:?}" + ); + assert!( + !walked.iter().any(|path| path.starts_with("linked/")), + "nested linked worktree files were walked: {walked:?}" + ); +} + +#[test] +fn callgraph_walk_skips_nested_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let walked = relative_set(root, crate::callgraph::walk_project_files(root)); + assert!( + walked.contains("src/main.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("lib/util.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("vendor/independent/inner.rs"), + "nested independent repository must still be walked: {walked:?}" + ); + assert!( + walked.contains("submod/code.rs"), + "submodule-style checkout must still be walked: {walked:?}" + ); + assert!( + !walked.iter().any(|path| path.starts_with("linked/")), + "nested linked worktree files were walked: {walked:?}" + ); +} + +#[test] +fn grep_fallback_walk_skips_nested_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let filters = crate::search_index::PathFilters::default(); + let outcome = crate::grep_executor::bounded_fallback_walk_files(root, root, &filters); + assert!(!outcome.walk_truncated, "fixture walk must complete"); + let walked = relative_set(root, outcome.files); + assert!( + walked.contains("src/main.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("vendor/independent/inner.rs"), + "nested independent repository must still be walked: {walked:?}" + ); + assert!( + walked.contains("submod/code.rs"), + "submodule-style checkout must still be walked: {walked:?}" + ); + assert!( + !walked.iter().any(|path| path.starts_with("linked/")), + "nested linked worktree files were walked: {walked:?}" + ); +} + +#[test] +fn explicitly_named_worktree_target_still_walks_like_an_ignored_target() { + let (_dir, fixture) = fixture(); + let filters = crate::search_index::PathFilters::default(); + // A directory explicitly named inside the worktree is walked: the boundary + // only prunes entries below the walk's own root, never the root itself, + // the same rule explicitly named gitignored directories follow. + let outcome = crate::grep_executor::bounded_fallback_walk_files( + &fixture.worktree, + &fixture.worktree, + &filters, + ); + assert!(!outcome.walk_truncated, "fixture walk must complete"); + let walked = relative_set(&fixture.worktree, outcome.files); + assert!( + walked.contains("src/main.rs"), + "explicitly named worktree target must walk: {walked:?}" + ); + assert!( + walked.contains("extra.rs"), + "explicitly named worktree target must walk: {walked:?}" + ); + // A file explicitly named inside the worktree stays readable, exactly like + // an explicitly named gitignored file: only storage and SQLite names are + // refused by the guarded readers. + let named = fixture.worktree.join("src/main.rs"); + assert!( + !crate::project_content::is_excluded(&named), + "explicitly named worktree file must stay readable" + ); + assert!(crate::project_content::read(&named).is_ok()); +} + +#[cfg(unix)] +#[test] +fn glob_fallback_skips_nested_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let ctx = crate::context::AppContext::new( + crate::context::default_language_provider_factory(), + crate::config::Config { + project_root: Some(root.clone()), + ..Default::default() + }, + ); + // No index is installed, so glob answers from the fallback walk. + let (files, source, _) = + crate::commands::glob::glob_root_files_for_test(&ctx, root, root, "**/*.rs", 100, None); + assert_eq!(source, "fallback"); + let walked = relative_set(root, files); + assert!( + walked.contains("src/main.rs"), + "missing main file: {walked:?}" + ); + assert!( + walked.contains("vendor/independent/inner.rs"), + "nested independent repository must still be walked: {walked:?}" + ); + assert!( + walked.contains("submod/code.rs"), + "submodule-style checkout must still be walked: {walked:?}" + ); + assert!( + !walked.iter().any(|path| path.starts_with("linked/")), + "nested linked worktree files were walked: {walked:?}" + ); +} + +#[test] +fn trigram_index_holds_no_nested_worktree_files() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let index = crate::search_index::SearchIndex::build(root); + assert!(index.is_ready(), "fixture index must build"); + let snapshot = index.snapshot(); + assert!(snapshot.has_file_in_scope(&root.join("src/main.rs"))); + assert!(snapshot.has_file_in_scope(&root.join("vendor/independent/inner.rs"))); + assert!(!snapshot.has_file_in_scope(&fixture.worktree.join("src/main.rs"))); + assert!(!snapshot.has_file_in_scope(&fixture.worktree.join("extra.rs"))); +} + +#[test] +fn watcher_drops_events_under_nested_worktrees() { + let (_dir, fixture) = fixture(); + let root = &fixture.root; + let linked_file = fixture.worktree.join("src/main.rs"); + let main_file = root.join("src/main.rs"); + let independent_file = fixture.independent.join("inner.rs"); + let submodule_file = fixture.submodule.join("code.rs"); + assert!( + crate::watcher_filter::watcher_path_is_infra_skip_under(root, &linked_file), + "events under a nested linked worktree must be dropped" + ); + assert!( + crate::watcher_filter::watcher_path_is_infra_skip_under(root, &fixture.worktree), + "the worktree root itself must be dropped" + ); + for path in [&main_file, &independent_file, &submodule_file] { + assert!( + !crate::watcher_filter::watcher_path_is_infra_skip_under(root, path), + "{} must still be watched", + path.display() + ); + } + // End to end through one filter flush: the worktree event is dropped, the + // others are kept. + let config = crate::watcher_filter::WatcherFilterConfig::new(root.clone(), None); + let matcher = std::sync::Arc::new(std::sync::RwLock::new(None)); + let filtered = crate::watcher_filter::filter_watcher_raw_paths_for_test( + &config, + &matcher, + [linked_file.clone(), main_file.clone()], + ); + assert!( + !filtered.changed.contains(&linked_file), + "worktree event reached the checkout: {:?}", + filtered.changed + ); + assert!( + filtered.changed.contains(&main_file), + "main checkout event was dropped: {:?}", + filtered.changed + ); +} diff --git a/crates/aft/src/project_content.rs b/crates/aft/src/project_content.rs index 177984614..08e0fc602 100644 --- a/crates/aft/src/project_content.rs +++ b/crates/aft/src/project_content.rs @@ -1,8 +1,8 @@ //! Files that must not be opened as project source by indexers and walkers. -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashMap}; use std::path::{Path, PathBuf}; -use std::sync::{OnceLock, RwLock}; +use std::sync::{Mutex, OnceLock, RwLock}; fn storage_roots() -> &'static RwLock> { static ROOTS: OnceLock>> = OnceLock::new(); @@ -26,6 +26,242 @@ pub(crate) fn register_storage_root(path: &Path) { roots.insert(canonical); } +/// A walk-scoped boundary that skips nested linked worktrees of this +/// repository: directories under the project root whose `.git` is a file +/// naming this repository's common git dir `worktrees/` entry (the layout +/// `git worktree add` creates). Indexing those checkouts again duplicates +/// every file, so every content-reading walker prunes them; an explicitly +/// named target inside one still walks, because the boundary only prunes +/// entries below the walk's own root, never the root itself. +/// +/// Not a worktree root: a nested independent repository (its own `.git` +/// directory, or a `.git` file naming another common dir), a submodule +/// (its gitdir lives under `.git/modules/`, not `worktrees/`), and the +/// boundary's own root. +/// +/// The check is cheap by construction: a directory is only examined past one +/// `symlink_metadata` when it holds a `.git` entry that is a file, the +/// repository's common dir is resolved once per boundary, and each verdict is +/// cached per directory for the boundary's life (one walk, or one watcher +/// filter call). +pub(crate) struct NestedWorktreeBoundary { + root: PathBuf, + common_dir: OnceLock>, + verdicts: Mutex>, +} + +impl NestedWorktreeBoundary { + /// One boundary per walk, rooted at the walk's root. The root itself is + /// never reported, so a project opened inside a worktree still walks. + pub(crate) fn for_root(root: &Path) -> Self { + Self { + root: root.to_path_buf(), + common_dir: OnceLock::new(), + verdicts: Mutex::new(HashMap::new()), + } + } + + fn common_dir(&self) -> Option<&Path> { + self.common_dir + .get_or_init(|| project_git_common_dir(&self.root)) + .as_deref() + } + + /// True when `dir` itself is a nested linked worktree of this repository. + /// Only directories below the walk root are ever reported; callers check + /// `entry.depth() > 0` (or an equivalent) so the named root of an + /// explicitly targeted walk is always admitted. + pub(crate) fn is_nested_worktree_root(&self, dir: &Path) -> bool { + if dir == self.root { + return false; + } + if let Some(verdict) = self + .verdicts + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(dir) + { + return *verdict; + } + // Gate the `.git` file read on the entry existing as a file: ordinary + // directories cost one metadata probe and never resolve the common dir. + let verdict = std::fs::symlink_metadata(dir.join(".git")) + .is_ok_and(|metadata| metadata.is_file()) + && self + .common_dir() + .is_some_and(|common| worktree_gitdir_points_at(dir, common)); + self.verdicts + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert(dir.to_path_buf(), verdict); + verdict + } + + /// True when `path` is at or under a nested linked worktree of this + /// repository. The boundary root itself never counts, even when the + /// project is opened inside a worktree. + pub(crate) fn is_under_nested_worktree(&self, path: &Path) -> bool { + let mut current = Some(path); + while let Some(dir) = current { + if dir == self.root || !dir.starts_with(&self.root) { + return false; + } + if self.is_nested_worktree_root(dir) { + return true; + } + current = dir.parent(); + } + false + } +} + +/// This repository's common git dir for walks rooted at `root`: the `.git` +/// directory of a main checkout, the `commondir` target of a linked-worktree +/// checkout, or a submodule admin dir for a submodule checkout. `None` when +/// no ancestor holds `.git`, so there is no repository whose worktrees could +/// be nested below the root. +fn project_git_common_dir(root: &Path) -> Option { + let mut current = if root.is_absolute() { + root.to_path_buf() + } else { + std::env::current_dir().ok()?.join(root) + }; + loop { + let dot_git = current.join(".git"); + match std::fs::symlink_metadata(&dot_git) { + Ok(metadata) if metadata.is_dir() => return canonicalized(&dot_git), + Ok(metadata) if metadata.is_file() => { + let target = read_gitdir_target(&dot_git)?; + let target = if target.is_absolute() { + target + } else { + current.join(target) + }; + // A linked-worktree checkout names its admin dir, which carries + // a `commondir` file pointing at the shared git dir. Anything + // else (a submodule admin dir, for example) is its own common + // dir. + if let Some(common) = read_commondir_target(&target) { + let common = if common.is_absolute() { + common + } else { + target.join(common) + }; + return canonicalized(&common); + } + return canonicalized(&target); + } + Ok(_) => return None, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + let Some(parent) = current.parent().map(Path::to_path_buf) else { + return None; + }; + current = parent; + } + Err(_) => return None, + } + } +} + +/// True when `dir/.git` is a file whose `gitdir:` resolves to +/// `/worktrees/`. Relative gitdir lines resolve against +/// `dir`, matching git; both sides are canonical, so symlinked checkouts and +/// platform spellings compare equal. +fn worktree_gitdir_points_at(dir: &Path, common_dir: &Path) -> bool { + let target = match read_gitdir_target(&dir.join(".git")) { + Some(target) => target, + None => return false, + }; + let target = if target.is_absolute() { + target + } else { + dir.join(target) + }; + worktree_common_candidate(&target).is_some_and(|candidate| candidate == *common_dir) +} + +/// The canonical common dir when `gitdir_target` names a linked-worktree +/// admin dir (`/worktrees/`); `None` for submodule-style and +/// other gitdirs. Pure path logic, so Windows spellings (forward slashes, +/// drive paths) are covered without touching the filesystem. +fn worktree_common_candidate(gitdir_target: &Path) -> Option { + let worktrees = gitdir_target.parent()?; + if worktrees.file_name()? != std::ffi::OsStr::new("worktrees") { + return None; + } + canonicalized(worktrees.parent()?) +} + +fn canonicalized(path: &Path) -> Option { + std::fs::canonicalize(path).ok() +} + +/// Parse a `.git` file's `gitdir:` line into its (possibly relative) target. +/// The read is capped: a `.git` file is one short line, never bulk data. +fn read_gitdir_target(dot_git: &Path) -> Option { + parse_gitdir_bytes(&capped_read(dot_git, 4096)?) +} + +/// The `commondir` file a linked worktree's gitdir carries, naming the shared +/// git dir (absolute, or relative to the gitdir). +fn read_commondir_target(gitdir: &Path) -> Option { + let bytes = capped_read(&gitdir.join("commondir"), 4096)?; + let line = bytes.split(|byte| *byte == b'\n').next()?; + let trimmed = trim_ascii_whitespace(line); + if trimmed.is_empty() { + return None; + } + bytes_to_path(trimmed) +} + +fn parse_gitdir_bytes(bytes: &[u8]) -> Option { + let line = bytes.split(|byte| *byte == b'\n').next()?; + let line = line.strip_suffix(b"\r").unwrap_or(line); + let line = trim_ascii_whitespace(line); + let rest = trim_ascii_whitespace(line.strip_prefix(b"gitdir:")?); + if rest.is_empty() { + return None; + } + bytes_to_path(rest) +} + +fn trim_ascii_whitespace(mut bytes: &[u8]) -> &[u8] { + while let Some((first, rest)) = bytes.split_first() { + if first.is_ascii_whitespace() { + bytes = rest; + } else { + break; + } + } + while let Some((last, rest)) = bytes.split_last() { + if last.is_ascii_whitespace() { + bytes = rest; + } else { + break; + } + } + bytes +} + +#[cfg(unix)] +fn bytes_to_path(bytes: &[u8]) -> Option { + use std::os::unix::ffi::OsStringExt; + Some(PathBuf::from(std::ffi::OsString::from_vec(bytes.to_vec()))) +} + +#[cfg(not(unix))] +fn bytes_to_path(bytes: &[u8]) -> Option { + std::str::from_utf8(bytes).ok().map(PathBuf::from) +} + +fn capped_read(path: &Path, cap: u64) -> Option> { + use std::io::Read; + let file = std::fs::File::open(path).ok()?; + let mut bytes = Vec::new(); + file.take(cap).read_to_end(&mut bytes).ok()?; + Some(bytes) +} + /// SQLite uses `-shm` files as shared-memory indexes and protects them with /// process-wide file locks on macOS and Linux. Closing any descriptor to one of /// these files, even a read-only indexing probe, releases SQLite's locks. A @@ -616,4 +852,96 @@ mod tests { "alive" ); } + + #[test] + fn gitdir_lines_parse_absolute_relative_and_malformed() { + // Absolute, relative, and CRLF-terminated lines resolve; anything else + // is not a worktree marker and must not prune. + assert_eq!( + parse_gitdir_bytes(b"gitdir: /repo/.git/worktrees/wt\n"), + Some(PathBuf::from("/repo/.git/worktrees/wt")) + ); + assert_eq!( + parse_gitdir_bytes(b" gitdir: ../.git/worktrees/wt \r\n"), + Some(PathBuf::from("../.git/worktrees/wt")) + ); + for malformed in [ + b"gitdir:".as_slice(), + b"gitdir: \n", + b"not a gitdir\n", + b"", + b"gitdir : /repo/.git/worktrees/wt\n", + ] { + assert_eq!(parse_gitdir_bytes(malformed), None, "{malformed:?}"); + } + } + + #[test] + fn windows_gitdir_lines_name_a_worktrees_admin_dir() { + // Worktree gitdir lines on Windows use forward slashes or drive paths. + // Pure path logic, so this runs on every platform without git. + for line in [ + "gitdir: C:/repo/.git/worktrees/wt\n", + "gitdir: C:/repo/.git/worktrees/wt", + ] { + let target = + parse_gitdir_bytes(line.as_bytes()).expect("Windows gitdir line must parse"); + assert_eq!( + target.parent().and_then(|parent| parent.file_name()), + Some(std::ffi::OsStr::new("worktrees")) + ); + assert_eq!( + target.parent().and_then(|parent| parent.parent()), + Some(std::path::Path::new("C:/repo/.git")) + ); + } + // A submodule-style gitdir never names `worktrees/`. + let target = parse_gitdir_bytes(b"gitdir: ../.git/modules/sub\n").unwrap(); + assert_ne!( + target.parent().and_then(|parent| parent.file_name()), + Some(std::ffi::OsStr::new("worktrees")) + ); + } + + #[test] + fn commondir_points_at_the_shared_git_dir() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + let common = root.join(".git"); + let admin = common.join("worktrees").join("wt"); + std::fs::create_dir_all(&admin).unwrap(); + std::fs::write(admin.join("commondir"), "../../\n").unwrap(); + assert_eq!(read_commondir_target(&admin), Some(PathBuf::from("../../"))); + assert_eq!(read_commondir_target(&common), None); + } + + #[test] + fn hand_rolled_worktree_layout_resolves_without_git() { + // The same layout `git worktree add` creates, without needing git: + // main/.git is the common dir, main/linked/.git names its admin entry. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + let main = root.join("main"); + let linked = main.join("linked"); + let admin = main.join(".git").join("worktrees").join("linked"); + std::fs::create_dir_all(&admin).unwrap(); + std::fs::create_dir_all(&linked).unwrap(); + std::fs::write( + linked.join(".git"), + format!("gitdir: {}\n", admin.display()), + ) + .unwrap(); + std::fs::write( + admin.join("gitdir"), + format!("{}\n", linked.join(".git").display()), + ) + .unwrap(); + std::fs::write(admin.join("commondir"), "../..\n").unwrap(); + let boundary = NestedWorktreeBoundary::for_root(&main); + assert!(boundary.is_nested_worktree_root(&linked)); + assert!(!boundary.is_nested_worktree_root(&main)); + assert!(!boundary.is_nested_worktree_root(&root)); + assert!(boundary.is_under_nested_worktree(&linked.join("file.rs"))); + assert!(!boundary.is_under_nested_worktree(&main.join("file.rs"))); + } } diff --git a/crates/aft/src/search_index.rs b/crates/aft/src/search_index.rs index e96cafd0c..feb69d5bd 100644 --- a/crates/aft/src/search_index.rs +++ b/crates/aft/src/search_index.rs @@ -6642,12 +6642,37 @@ fn project_walk_builder_pruned( // A disappearing child mount can make ReadDir::drop panic on ENXIO and abort // the daemon, so never open directories outside this walk root's filesystem. builder.same_file_system(true).hidden(false); + // Nested linked worktrees of this repository index again under their own + // roots; prune them here so every walk sharing this builder skips them. + // The named root itself is always admitted (depth 0), so an explicitly + // targeted walk inside a worktree still reads it. + let worktrees = crate::project_content::NestedWorktreeBoundary::for_root(search_root); crate::context::apply_project_ignore_rules(&mut builder, search_root).filter_entry( - move |entry| project_walk_admits_entry(entry, excludes) && descend(entry.path()), + move |entry| { + project_walk_admits_entry(entry, excludes) + && descend(entry.path()) + && !is_nested_worktree_entry(entry, &worktrees) + }, ); builder } +/// True when `entry` is a directory below the walk root that is itself a +/// nested linked worktree of this repository, which the walk must prune. +/// Shared by every walker built on the project walk; the depth check keeps +/// an explicitly named root (depth 0) admitted. +pub(crate) fn is_nested_worktree_entry( + entry: &ignore::DirEntry, + worktrees: &crate::project_content::NestedWorktreeBoundary, +) -> bool { + entry.depth() > 0 + && entry + .file_type() + .as_ref() + .is_some_and(std::fs::FileType::is_dir) + && worktrees.is_nested_worktree_root(entry.path()) +} + /// The membership rule the project walk applies to each entry on top of the /// ignore rules: content exclusions, skipped directory names and OS metadata /// files. diff --git a/crates/aft/src/watcher_filter.rs b/crates/aft/src/watcher_filter.rs index eb4933d39..26767695b 100644 --- a/crates/aft/src/watcher_filter.rs +++ b/crates/aft/src/watcher_filter.rs @@ -308,6 +308,12 @@ pub(crate) fn watcher_path_is_infra_skip_under(root: &Path, path: &Path) -> bool || absolute .strip_prefix(root) .is_ok_and(watcher_path_has_infra_component) + // Events under a nested linked worktree of this repository belong to + // that checkout's own session, like other excluded paths. The boundary + // resolves the common dir lazily and only reads a `.git` file for + // directories that hold one. + || crate::project_content::NestedWorktreeBoundary::for_root(root) + .is_under_nested_worktree(&absolute) } /// High-churn ignored directories that can be dropped from the raw event stream