From 43834f357e196202e81d8a6641e0eb8e663c027e Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 19 Feb 2026 04:36:44 -0600 Subject: [PATCH] Update submodule and implement parallel I/O audit fixes - Updated the `vscode-rojo` submodule to the latest commit. - Added a new plan for addressing issues found during the audit of parallelization and optimization commits, including fixes for a data loss bug in the orphan scanner, a duplicate-children bug in the prefetch cache, and a nested-detection correctness issue. - Implemented a small DRY refactor by extracting a shared helper function for UNC path stripping. - Enhanced the `syncback` logic to ensure hidden directories are preserved during clean mode operations and added tests to validate these changes. These updates aim to improve the reliability and correctness of the parallel I/O functionality while maintaining code clarity and performance. --- .../parallel_io_audit_fixes_35f49f83.plan.md | 267 ++++++++++++++++++ src/serve_session.rs | 9 + src/syncback/mod.rs | 98 +++---- tests/tests/build.rs | 164 +++++++++++ tests/tests/clean_mode.rs | 53 ++++ vscode-rojo | 2 +- 6 files changed, 536 insertions(+), 57 deletions(-) create mode 100644 .cursor/plans/parallel_io_audit_fixes_35f49f83.plan.md diff --git a/.cursor/plans/parallel_io_audit_fixes_35f49f83.plan.md b/.cursor/plans/parallel_io_audit_fixes_35f49f83.plan.md new file mode 100644 index 00000000..88f4c885 --- /dev/null +++ b/.cursor/plans/parallel_io_audit_fixes_35f49f83.plan.md @@ -0,0 +1,267 @@ +--- +name: Parallel IO audit fixes +overview: "Fix three issues found during audit of the parallelization/optimization commits: a data loss bug in the orphan scanner, a duplicate-children bug in the prefetch cache, and a nested-detection correctness issue, plus a small DRY refactor." +todos: + - id: fix1-hidden-dirs + content: Fix orphan scanner walkdir to prune hidden directories using filter_entry() + status: completed + - id: fix2-dedup-roots + content: Deduplicate overlapping $path roots in prefetch_project_files before walking + status: completed + - id: fix3-nested-detection + content: Replace broken binary search nested detection with O(n log n) ancestor-tracking approach + status: completed + - id: fix4-unc-refactor + content: Extract shared strip_unc_prefix helper from 3 inline copies + status: completed + - id: test-hidden-dir + content: Add integration test for hidden directory survival in clean syncback + status: completed + - id: test-overlap + content: Add test for overlapping $path roots with prefetch cache + status: completed + - id: test-nested + content: Add unit test for nested path detection correctness + status: completed + - id: run-ci + content: Run full CI pipeline (cargo test, clippy, fmt) + status: completed +isProject: false +--- + +# Parallel I/O Audit Fixes + +> This plan was generated by `/audit` from a Plan-mode session. +> Refer to `.cursor/rules/atlas.mdc` for full project standards. + +## Standards + +These standards govern every fix in this plan. An implementer MUST read `.cursor/rules/atlas.mdc` before starting. + +### Round-Trip Identity (from `atlas.mdc` - Quality Standard) + +Syncback (or two-way sync) writes a directory tree. Building an rbxl from that directory tree and forward-syncing it back must produce a **bit-identical instance tree** -- same names, same classes, same properties, same hierarchy, same ref targets. Any deviation is a bug. + +### CLI Syncback Parity (from `atlas.mdc` - Two-Way Sync Strategy) + +Plugin-based sync must produce **exactly the same filesystem output** that `atlas syncback` would give for the same input. Byte-for-byte identical files, identical directory structures, identical naming. Any divergence is a bug until proven otherwise. + +### Code Quality (from `atlas.mdc` - Code Quality Standard) + +DRY code that is easy to reason about and hard to get wrong. The same slugify/dedup/meta-update pattern should not be copy-pasted across files. Small refactors (2-3 call sites) are included as fixes. Major rewrites (10+ call sites) are deferred. + +## Context + +Audited the last 7 commits on the current branch (from `1ffed121` through `56e1ab4a`), which add: + +- VFS prefetch cache for parallel file reading at startup +- Parallel snapshot building via rayon in `dir.rs` +- Parallel RefPathIndex population via rayon + walkdir +- Parallel tree hashing in syncback +- Orphan scanner rewrite using walkdir +- Placeholder-based ref path substitution optimization +- ServeCommand refactor and dead code cleanup + +## Fixes + +### Fix 1: Prune hidden directories from walkdir orphan scanner + +- **Status:** Approved +- **Finding:** Orphan scanner walkdir replacement doesn't prune hidden directories +- **Files:** `[src/syncback/mod.rs](src/syncback/mod.rs)` +- **Problem:** Lines ~406-424. The `walkdir::WalkDir` iterator yields ALL entries including children of hidden directories. The hidden-name filter only checks `file_name().starts_with('.')`, so `settings.json` inside `.vscode/` passes the filter and gets added to `existing_paths`. In clean mode, these files would be deleted. +The old recursive `scan_directory` function skipped hidden directories AND never recursed into them. The walkdir replacement must replicate this behavior. +- **Solution:** Replace the flat `.flatten()` + post-hoc hidden check with `filter_entry()` to prune the walk at hidden directories: + +```rust + for entry in walkdir::WalkDir::new(&canonical_dir) + .follow_links(true) + .into_iter() + .filter_entry(|e| { + if e.depth() == 0 { + return true; + } + e.file_name() + .to_str() + .map_or(true, |n| !n.starts_with('.') || n == ".gitkeep") + }) + .flatten() + { + if entry.depth() == 0 { + continue; + } + let path = entry.path().to_path_buf(); + if !is_valid_path(&ignore_patterns, project_path, &path) { + continue; + } + // Hidden check no longer needed here -- filter_entry handles it + paths.insert(path); + } + + +``` + + `filter_entry` controls which directories walkdir descends into. When it returns `false` for a directory, walkdir skips the directory AND all its descendants. This exactly matches the old recursive behavior. + +- **Risk:** Low -- only touches the orphan scanner loop. `filter_entry` is walkdir's standard pruning mechanism. +- **Verify round-trip:** Clean mode syncback with a `.vscode/` directory inside a `$path` source dir should NOT delete `.vscode/` contents. +- **Verify syncback parity:** N/A -- this only affects clean mode orphan removal, not the sync pipeline. +- **Tests required:** + - Integration test: create a temp project with `src/.hidden/keep.txt`, run clean-mode syncback, verify `.hidden/keep.txt` survives. + +### Fix 2: Deduplicate overlapping roots in prefetch cache + +- **Status:** Approved +- **Finding:** Overlapping `$path` roots cause duplicate children in prefetch cache +- **Files:** `[src/serve_session.rs](src/serve_session.rs)` +- **Problem:** Lines ~136-156 in `prefetch_project_files`. `collect_path_roots` collects ALL `$path` entries including nested ones. For a project with `$path: "src"` and `$path: "src/shared"`, walkdir walks `src/shared` twice -- once as part of `src` and once as its own root. The `children_map` accumulates duplicate entries for `src/shared`'s children. When consumed by VFS `read_dir`, these duplicates cause duplicate instance snapshots. +- **Solution:** After collecting roots in `prefetch_project_files`, deduplicate by removing any root that is a descendant of another root: + +```rust + let mut roots: Vec = Vec::new(); + collect_path_roots(&project.tree, folder, &mut roots); + + // Remove roots that are descendants of other roots to avoid + // walking the same directory twice and producing duplicate children. + roots.sort(); + let mut deduped: Vec = Vec::new(); + for root in roots { + if !deduped.iter().any(|existing| root.starts_with(existing)) { + deduped.push(root); + } + } + let roots = deduped; + + +``` + + After sorting, a parent path always sorts before its descendants. By iterating in order and only keeping roots that are NOT descendants of already-kept roots, we eliminate all overlaps in O(n^2) worst case but typically O(n) since most projects have few `$path` entries. + +- **Risk:** Low -- only modifies which directories are walked during prefetch. The dedup logic is simple and well-understood. +- **Verify round-trip:** Build a project with overlapping `$path` entries. Verify snapshot tree has no duplicate instances. Run build 5 times, verify deterministic output. +- **Verify syncback parity:** N/A -- prefetch cache only affects forward sync initial load. +- **Tests required:** + - Unit test in `tests/tests/build.rs`: create a project with `$path: "src"` and a child with `$path: "src/shared"`. Run `snapshot_from_vfs` with prefetch cache. Verify no duplicate children under the `shared` directory instance. + +### Fix 3: Fix nested path detection in orphan removal + +- **Status:** Approved (O(n log n) approach) +- **Finding:** Binary search nested detection misses sibling paths +- **Files:** `[src/syncback/mod.rs](src/syncback/mod.rs)` +- **Problem:** Lines ~869-888. The optimization uses `binary_search` to find the current path's position, then checks only `sorted_removals[idx - 1]`. This misses cases where a sibling path sits between the parent and the current path in sorted order. +Example: `sorted = ["a/b", "a/b/c", "a/b/d"]`. For `a/b/d` at idx 2, the code checks `sorted[1] = "a/b/c"` (a sibling, not an ancestor). The actual ancestor `"a/b"` at idx 0 is not checked. +- **Solution:** Replace binary search with a sorted single-pass using an `current_ancestor` tracker: + +```rust + let mut sorted_removals: Vec<_> = paths_to_remove.iter().cloned().collect(); + sorted_removals.sort(); + + let mut current_ancestor: Option<&PathBuf> = None; + for old_path in &sorted_removals { + if let Some(ancestor) = current_ancestor { + if old_path.starts_with(ancestor) && old_path != ancestor { + continue; // nested under current ancestor, skip + } + } + current_ancestor = Some(old_path); + + let relative_path = old_path + .strip_prefix(&canonical_project_path) + .unwrap_or(old_path); + + log::debug!("Removing orphaned path: {}", relative_path.display()); + if old_path.is_dir() { + fs_snapshot.remove_dir(relative_path); + } else { + fs_snapshot.remove_file(relative_path); + } + } + + +``` + + In sorted path order, all descendants of a path appear consecutively after it (because `Path` comparison is component-based). So a single variable tracking the current top-level ancestor is sufficient. When we encounter a non-descendant, it becomes the new ancestor. This is O(n log n) for the sort + O(n) for the walk. + + This also removes the intermediate `paths_vec` and the `binary_search` code, simplifying the function. + +- **Risk:** Low -- the logic is simpler than what it replaces. The correctness argument is straightforward: sorted paths guarantee all descendants appear consecutively. +- **Verify round-trip:** Clean mode syncback with nested directories. Verify parent-only removal (no redundant child removals). +- **Verify syncback parity:** N/A -- orphan removal is internal to clean mode. +- **Tests required:** + - Unit test: create paths `["a/b", "a/b/c", "a/b/d", "x/y", "x/y/z"]`, verify only `"a/b"` and `"x/y"` are top-level removals. + +### Fix 4: Extract shared `strip_unc_prefix` helper + +- **Status:** Approved +- **Finding:** Duplicated UNC prefix stripping logic in 3 locations +- **Files:** `[src/syncback/mod.rs](src/syncback/mod.rs)` +- **Problem:** The same UNC prefix stripping pattern appears at: + 1. Lines ~396-404 (orphan scanner: canonicalize `dirs_to_scan` entries) + 2. Lines ~649-656 (`strip_unc_prefix` helper definition inside orphan removal block) + 3. Lines ~703-711 (inline canonicalize in `collect_all_path_refs_and_mappings`) + Location 2 defines a proper `fn strip_unc_prefix`, but locations 1 and 3 inline the same logic. +- **Solution:** Move `strip_unc_prefix` to module level (or just above the clean-mode block), and replace all 3 inline instances with calls to it. The function is already defined at location 2: + +```rust + fn strip_unc_prefix(p: PathBuf) -> PathBuf { + let path_str = p.to_string_lossy(); + if path_str.starts_with(r"\\?\") || path_str.starts_with("//?/") { + PathBuf::from(&path_str[4..]) + } else { + p + } + } + + +``` + + Move this definition before the `existing_paths` block (around line ~258), then replace the inline code at lines ~396-404 with `strip_unc_prefix(std::fs::canonicalize(dir).unwrap_or_else(|_| dir.clone()))` and at lines ~703-711 with `strip_unc_prefix(p)`. + +- **Risk:** Zero -- pure refactor, no behavior change. +- **Verify round-trip:** N/A -- refactor only. +- **Verify syncback parity:** N/A -- refactor only. +- **Tests required:** None -- existing tests cover this code path. + +## Skipped Fixes + +### Skipped: Serve command loads project twice + +- **Finding:** `src/cli/serve.rs` loads the project file via `Project::load_initial_project` for port/address, then `ServeSession::new` loads it again in `init_tree`. +- **Problem:** Minor performance -- project file read twice from disk at startup. +- **Reason skipped:** Acceptable overhead. Project files are small and this only happens once. + +### Skipped: Commented-out git config in init.rs + +- **Finding:** Four commented-out git config lines in `src/cli/init.rs` lines 134-137. +- **Reason skipped:** User indicated these are planned for future enablement. + +## Accepted Limitations + +- **Limitation:** Symlinks within project `$path` directories may cause path mismatches +- **Scenario:** If a symlink inside a `$path` source directory points elsewhere, walkdir-collected paths won't match `added_paths` during clean-mode orphan removal. Files behind the symlink could be incorrectly removed. +- **User decision:** Accepted for this release +- **Mitigation:** Symlinks within source directories are not a supported configuration in Roblox development. Root-level symlinks (project folder itself) still work because `canonicalize` is called on the root. + +## Deferred Refactors + +None identified beyond the small `strip_unc_prefix` extraction (included as Fix 4). + +## Test Plan + +### Rust Integration Tests (`tests/tests/`) + +- **Hidden directory survival in clean syncback** (Fix 1): Create temp project with `src/.hidden/keep.txt`, run clean-mode syncback, assert `.hidden/keep.txt` still exists after syncback. +- **Overlapping `$path` roots don't create duplicates** (Fix 2): Create project with overlapping `$path` entries (`"src"` and `"src/shared"`), run `snapshot_from_vfs` with prefetch cache, verify no duplicate children. + +### Rust Unit Tests + +- **Nested path detection correctness** (Fix 3): Test the ancestor-tracking algorithm with paths `["a/b", "a/b/c", "a/b/d", "x/y", "x/y/z"]`, verify only `"a/b"` and `"x/y"` are emitted as top-level. + +## Test Rules + +- Every bug fix MUST have a test that would have FAILED before the fix and PASSES after. +- Fix 4 (refactor) does not need new tests -- existing test coverage is sufficient. + +## Final Step: Run CI + +After ALL fixes and tests are implemented, run the full CI pipeline (`cargo test --locked`, `cargo clippy`, `cargo fmt --check`). Every fix must pass CI before the plan is considered complete. \ No newline at end of file diff --git a/src/serve_session.rs b/src/serve_session.rs index 6bd5ddf0..198d2c94 100644 --- a/src/serve_session.rs +++ b/src/serve_session.rs @@ -136,6 +136,15 @@ fn prefetch_project_files(project: &Project) -> io::Result { let mut roots: Vec = Vec::new(); collect_path_roots(&project.tree, folder, &mut roots); + roots.sort(); + let mut deduped: Vec = Vec::new(); + for root in roots { + if !deduped.iter().any(|existing| root.starts_with(existing)) { + deduped.push(root); + } + } + let roots = deduped; + if roots.is_empty() { return Ok(PrefetchCache { files: HashMap::new(), diff --git a/src/syncback/mod.rs b/src/syncback/mod.rs index 1ad47f54..667db2c7 100644 --- a/src/syncback/mod.rs +++ b/src/syncback/mod.rs @@ -254,6 +254,15 @@ pub fn syncback_loop_with_stats( let project_path = project.folder_location(); + fn strip_unc_prefix(p: PathBuf) -> PathBuf { + let path_str = p.to_string_lossy(); + if path_str.starts_with(r"\\?\") || path_str.starts_with("//?/") { + PathBuf::from(&path_str[4..]) + } else { + p + } + } + let phase_timer = std::time::Instant::now(); let existing_paths: HashSet = if !incremental { let mut paths = HashSet::new(); @@ -393,19 +402,25 @@ pub fn syncback_loop_with_stats( if !dir.is_dir() { continue; } + if let Some(name) = dir.file_name().and_then(|n| n.to_str()) { + if name.starts_with('.') && name != ".gitkeep" { + continue; + } + } let canonical_dir = std::fs::canonicalize(dir) - .map(|p| { - let s = p.to_string_lossy(); - if s.starts_with(r"\\?\") || s.starts_with("//?/") { - PathBuf::from(&s[4..]) - } else { - p - } - }) + .map(strip_unc_prefix) .unwrap_or_else(|_| dir.clone()); for entry in walkdir::WalkDir::new(&canonical_dir) .follow_links(true) .into_iter() + .filter_entry(|e| { + if e.depth() == 0 { + return true; + } + e.file_name() + .to_str() + .map_or(true, |n| !n.starts_with('.') || n == ".gitkeep") + }) .flatten() { if entry.depth() == 0 { @@ -415,11 +430,6 @@ pub fn syncback_loop_with_stats( if !is_valid_path(&ignore_patterns, project_path, &path) { continue; } - if let Some(name) = path.file_name().and_then(|n| n.to_str()) { - if name.starts_with('.') && name != ".gitkeep" { - continue; - } - } paths.insert(path); } } @@ -561,6 +571,12 @@ pub fn syncback_loop_with_stats( 'remove: for inst in &syncback.removed_children { let path = inst.metadata().instigating_source.as_ref().unwrap().path(); let inst_path = snapshot.get_old_inst_path(inst.id()); + if let Some(name) = path.file_name().and_then(|n| n.to_str()) { + if name.starts_with('.') && name != ".gitkeep" { + log::debug!("Skipping removing {} (hidden path)", path.display()); + continue; + } + } if !is_valid_path(&ignore_patterns, project_path, path) { log::debug!( "Skipping removing {} because its matches an ignore pattern", @@ -645,16 +661,6 @@ pub fn syncback_loop_with_stats( if !incremental && !existing_paths.is_empty() { log::debug!("Clean mode: checking for orphaned files to remove"); - // Helper to strip UNC prefix on Windows - fn strip_unc_prefix(p: PathBuf) -> PathBuf { - let path_str = p.to_string_lossy(); - if path_str.starts_with(r"\\?\") || path_str.starts_with("//?/") { - PathBuf::from(&path_str[4..]) - } else { - p - } - } - let canonical_project_path = strip_unc_prefix( std::fs::canonicalize(project_path).unwrap_or_else(|_| project_path.to_path_buf()), ); @@ -700,17 +706,9 @@ pub fn syncback_loop_with_stats( ) { if let Some(ref path_node) = node.path { let resolved = base_path.join(path_node.path()); - let canonical = match std::fs::canonicalize(&resolved) { - Ok(p) => { - let s = p.to_string_lossy(); - if s.starts_with(r"\\?\") || s.starts_with("//?/") { - PathBuf::from(&s[4..]) - } else { - p - } - } - Err(_) => resolved.clone(), - }; + let canonical = std::fs::canonicalize(&resolved) + .map(strip_unc_prefix) + .unwrap_or_else(|_| resolved.clone()); protected.insert(canonical.clone()); if !instance_path.is_empty() { mappings.push((canonical, instance_path.to_string())); @@ -869,32 +867,20 @@ pub fn syncback_loop_with_stats( } // Second pass: only remove top-level orphaned paths. - let mut sorted_removals: Vec<_> = paths_to_remove.iter().cloned().collect(); + // Sorted path order guarantees all descendants appear consecutively + // after their ancestor, so a single ancestor tracker suffices. + let mut sorted_removals: Vec<_> = paths_to_remove.into_iter().collect::>(); sorted_removals.sort(); - let paths_vec: Vec<_> = paths_to_remove.iter().collect(); - for old_path in &paths_vec { - let is_nested = sorted_removals - .binary_search(old_path) - .ok() - .and_then(|idx| { - if idx > 0 { - Some( - old_path.starts_with(&sorted_removals[idx - 1]) - && *old_path != &sorted_removals[idx - 1], - ) - } else { - None - } - }) - .unwrap_or(false); - - if is_nested { - continue; + let mut current_ancestor: Option<&PathBuf> = None; + for old_path in &sorted_removals { + if let Some(ancestor) = current_ancestor { + if old_path.starts_with(ancestor) && old_path != ancestor { + continue; + } } + current_ancestor = Some(old_path); - // Convert absolute path to relative path for FsSnapshot. - // FsSnapshot expects relative paths and joins them with base_path in write_to_vfs. let relative_path = old_path .strip_prefix(&canonical_project_path) .unwrap_or(old_path); diff --git a/tests/tests/build.rs b/tests/tests/build.rs index 62bef5fe..5b32b320 100644 --- a/tests/tests/build.rs +++ b/tests/tests/build.rs @@ -302,3 +302,167 @@ fn parallel_snapshot_error_propagation() { "Should propagate JSON parse error from parallel snapshot" ); } + +#[test] +fn overlapping_path_roots_no_duplicate_children() { + let _ = env_logger::try_init(); + + let dir = tempdir().expect("couldn't create temp dir"); + let root = dir.path(); + let src = root.join("src"); + let shared = src.join("shared"); + fs::create_dir_all(&shared).unwrap(); + + fs::write(src.join("top.luau"), "return 1").unwrap(); + fs::write(shared.join("a.luau"), "return 2").unwrap(); + fs::write(shared.join("b.luau"), "return 3").unwrap(); + + fs::write( + root.join("default.project.json5"), + r#"{ + "name": "OverlapTest", + "tree": { + "$path": "src", + "shared": { + "$path": "src/shared" + } + } + }"#, + ) + .unwrap(); + + let ctx = librojo::InstanceContext::default(); + let project_path = root.join("default.project.json5"); + + use std::collections::HashMap; + let mut files = HashMap::new(); + let mut canonical = HashMap::new(); + let mut is_file = HashMap::new(); + let mut children_map: HashMap> = HashMap::new(); + + for entry in walkdir::WalkDir::new(root).follow_links(true) { + let entry = entry.unwrap(); + let path = entry.path().to_path_buf(); + is_file.insert(path.clone(), entry.file_type().is_file()); + if entry.file_type().is_file() { + files.insert(path.clone(), fs::read(&path).unwrap()); + } + if let Ok(c) = fs::canonicalize(&path) { + canonical.insert(path.clone(), c); + } + if entry.depth() > 0 { + if let Some(parent) = entry.path().parent() { + children_map + .entry(parent.to_path_buf()) + .or_default() + .push(path); + } + } + } + for children in children_map.values_mut() { + children.sort_by(|a, b| a.file_name().cmp(&b.file_name())); + } + + let vfs = memofs::Vfs::new_default(); + vfs.set_prefetch_cache(memofs::PrefetchCache { + files, + canonical, + is_file, + children: children_map, + }); + + let snap = librojo::snapshot_from_vfs(&ctx, &vfs, &project_path) + .expect("snapshot failed") + .expect("snapshot returned None"); + + fn find_child<'a>( + snap: &'a librojo::InstanceSnapshot, + name: &str, + ) -> Option<&'a librojo::InstanceSnapshot> { + snap.children.iter().find(|c| c.name.as_ref() == name) + } + + let shared_snap = find_child(&snap, "shared").expect("shared child should exist"); + let shared_names: Vec<&str> = shared_snap + .children + .iter() + .map(|c| c.name.as_ref()) + .collect(); + + let mut deduped = shared_names.clone(); + deduped.sort(); + deduped.dedup(); + assert_eq!( + shared_names.len(), + deduped.len(), + "No duplicate children should exist under shared. Got: {:?}", + shared_names + ); +} + +/// Test the nested path detection algorithm used in clean-mode orphan removal. +/// Sorted path walk with ancestor tracking should only emit top-level paths. +#[test] +fn nested_path_detection_correctness() { + use std::path::PathBuf; + + fn top_level_paths(input: &[&str]) -> Vec { + let mut sorted: Vec = input.iter().map(PathBuf::from).collect(); + sorted.sort(); + + let mut result = Vec::new(); + let mut current_ancestor: Option<&PathBuf> = None; + for path in &sorted { + if let Some(ancestor) = current_ancestor { + if path.starts_with(ancestor) && path != ancestor { + continue; + } + } + current_ancestor = Some(path); + result.push(path.clone()); + } + result + } + + let result = top_level_paths(&["a/b", "a/b/c", "a/b/d", "x/y", "x/y/z"]); + assert_eq!( + result, + vec![PathBuf::from("a/b"), PathBuf::from("x/y")], + "Should only keep top-level ancestors" + ); + + let result = top_level_paths(&["a/b/c", "a/b/d", "a/b"]); + assert_eq!( + result, + vec![PathBuf::from("a/b")], + "Should detect ancestors regardless of input order" + ); + + let result = top_level_paths(&["a", "a/b", "a/b/c"]); + assert_eq!( + result, + vec![PathBuf::from("a")], + "Single ancestor should subsume all descendants" + ); + + let result = top_level_paths(&["a/b", "a/c", "b/d"]); + assert_eq!( + result, + vec![ + PathBuf::from("a/b"), + PathBuf::from("a/c"), + PathBuf::from("b/d") + ], + "Non-nested siblings should all be kept" + ); + + let result = top_level_paths(&["x"]); + assert_eq!( + result, + vec![PathBuf::from("x")], + "Single path should be kept" + ); + + let result: Vec = top_level_paths(&[]); + assert!(result.is_empty(), "Empty input should produce empty output"); +} diff --git a/tests/tests/clean_mode.rs b/tests/tests/clean_mode.rs index 0edb4cae..3ded9e4e 100644 --- a/tests/tests/clean_mode.rs +++ b/tests/tests/clean_mode.rs @@ -328,3 +328,56 @@ fn clean_mode_is_idempotent() { "File content should be identical after second syncback" ); } + +/// Test that clean mode does NOT delete hidden directories or their contents +/// inside $path source directories. Hidden directories like .vscode/ or +/// .cursor/ should be left untouched by the orphan scanner. +#[test] +fn clean_mode_preserves_hidden_directories() { + let _ = env_logger::try_init(); + + let source_path = Path::new(SYNCBACK_TESTS_PATH) + .join("sync_rules") + .join("input-project"); + let input_file = Path::new(SYNCBACK_TESTS_PATH) + .join("sync_rules") + .join("input.rbxm"); + + let test_dir = tempdir().expect("Couldn't create temporary directory"); + let project_path = test_dir.path().join("test_project"); + + fs_err::create_dir(&project_path).expect("Couldn't create project directory"); + copy_recursive(&source_path, &project_path).expect("Couldn't copy project"); + + let src_dir = project_path.join("src"); + let hidden_dir = src_dir.join(".hidden"); + fs_err::create_dir_all(&hidden_dir).expect("Couldn't create hidden directory"); + fs_err::write(hidden_dir.join("keep.txt"), "do not delete").unwrap(); + fs_err::write(hidden_dir.join("config.txt"), "some config").unwrap(); + + let nested_hidden = hidden_dir.join("nested"); + fs_err::create_dir_all(&nested_hidden).unwrap(); + fs_err::write(nested_hidden.join("deep.txt"), "also keep").unwrap(); + + assert!( + run_syncback(&project_path, &input_file, false), + "Syncback should succeed" + ); + + assert!( + hidden_dir.exists(), + ".hidden directory should survive clean mode syncback" + ); + assert!( + hidden_dir.join("keep.txt").exists(), + ".hidden/keep.txt should survive" + ); + assert!( + hidden_dir.join("config.txt").exists(), + ".hidden/config.txt should survive" + ); + assert!( + nested_hidden.join("deep.txt").exists(), + ".hidden/nested/deep.txt should survive" + ); +} diff --git a/vscode-rojo b/vscode-rojo index 531a5a2c..08a8e230 160000 --- a/vscode-rojo +++ b/vscode-rojo @@ -1 +1 @@ -Subproject commit 531a5a2ca5743982fb5af55ef5741a79fb512e4b +Subproject commit 08a8e230435011ddaf32668a099bf610ef5d22e5