diff --git a/src/web/api.rs b/src/web/api.rs index 75e15ca9..92c33582 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -1091,7 +1091,7 @@ impl ApiService { } else { None }; - self.write_init_meta_json(existing_path, added, instance_name)?; + self.write_init_meta_json(existing_path, added, instance_name, true)?; log::info!( "Syncback: Updated existing {} at {}/init.meta.json5", class_name, @@ -1736,23 +1736,19 @@ impl ApiService { added.name ); if old_path.exists() { - self.suppress_path_remove(old_path); let _ = fs::remove_file(old_path); } let meta_path = parent_dir.join(format!("{}.meta.json5", encoded_name)); if meta_path.exists() { - self.suppress_path_remove(&meta_path); let _ = fs::remove_file(&meta_path); } } let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; let init_path = dir_path.join("init.luau"); - self.suppress_path(&init_path); fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; @@ -1761,7 +1757,6 @@ impl ApiService { self.process_children_incremental(&unique_children, &dir_path, stats)?; } else { let file_path = parent_dir.join(format!("{}.luau", encoded_name)); - self.suppress_path(&file_path); fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -1791,23 +1786,19 @@ impl ApiService { added.name ); if old_path.exists() { - self.suppress_path_remove(old_path); let _ = fs::remove_file(old_path); } let meta_path = parent_dir.join(format!("{}.meta.json5", encoded_name)); if meta_path.exists() { - self.suppress_path_remove(&meta_path); let _ = fs::remove_file(&meta_path); } } let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; let init_path = dir_path.join(format!("init.{}.luau", script_suffix)); - self.suppress_path(&init_path); fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; @@ -1817,7 +1808,6 @@ impl ApiService { } else { let file_path = parent_dir.join(format!("{}.{}.luau", encoded_name, script_suffix)); - self.suppress_path(&file_path); fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -1846,23 +1836,19 @@ impl ApiService { added.name ); if old_path.exists() { - self.suppress_path_remove(old_path); let _ = fs::remove_file(old_path); } let meta_path = parent_dir.join(format!("{}.meta.json5", encoded_name)); if meta_path.exists() { - self.suppress_path_remove(&meta_path); let _ = fs::remove_file(&meta_path); } } let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; let init_path = dir_path.join("init.local.luau"); - self.suppress_path(&init_path); fs::write(&init_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; @@ -1871,7 +1857,6 @@ impl ApiService { self.process_children_incremental(&unique_children, &dir_path, stats)?; } else { let file_path = parent_dir.join(format!("{}.local.luau", encoded_name)); - self.suppress_path(&file_path); fs::write(&file_path, source.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -1889,7 +1874,6 @@ impl ApiService { "Folder" | "Configuration" | "Tool" | "ScreenGui" | "SurfaceGui" | "BillboardGui" | "AdGui" => { let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; @@ -1906,7 +1890,6 @@ impl ApiService { // Uses !has_children which accounts for filtered duplicate children if !has_children && !has_metadata { let gitkeep = dir_path.join(".gitkeep"); - self.suppress_path(&gitkeep); fs::write(gitkeep, b"").with_context(|| "Failed to write .gitkeep")?; } @@ -1925,11 +1908,10 @@ impl ApiService { if has_children { // Must become directory - store StringValue data in init.meta.json5 let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; - self.write_init_meta_json(&dir_path, added, meta_name_field)?; + self.write_init_meta_json(&dir_path, added, meta_name_field, false)?; log::info!( "Syncback: Created StringValue directory at {}", dir_path.display() @@ -1945,7 +1927,6 @@ impl ApiService { }) .unwrap_or_default(); let file_path = parent_dir.join(format!("{}.txt", encoded_name)); - self.suppress_path(&file_path); fs::write(&file_path, value.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -1960,7 +1941,6 @@ impl ApiService { let meta_path = parent_dir.join(format!("{}.meta.json5", encoded_name)); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize meta")?; - self.suppress_path(&meta_path); fs::write(&meta_path, &content).with_context(|| { format!("Failed to write meta: {}", meta_path.display()) })?; @@ -1977,17 +1957,15 @@ impl ApiService { if has_children { // Must become directory with init.csv let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; let init_path = dir_path.join("init.csv"); - self.suppress_path(&init_path); fs::write(&init_path, content.as_bytes()).with_context(|| { format!("Failed to write file: {}", init_path.display()) })?; // Write init.meta.json5 for className and name preservation - self.write_init_meta_json(&dir_path, added, meta_name_field)?; + self.write_init_meta_json(&dir_path, added, meta_name_field, false)?; log::info!( "Syncback: Created LocalizationTable directory at {}", dir_path.display() @@ -1995,7 +1973,6 @@ impl ApiService { self.process_children_incremental(&unique_children, &dir_path, stats)?; } else { let file_path = parent_dir.join(format!("{}.csv", encoded_name)); - self.suppress_path(&file_path); fs::write(&file_path, content.as_bytes()).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -2010,7 +1987,6 @@ impl ApiService { let meta_path = parent_dir.join(format!("{}.meta.json5", encoded_name)); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize meta")?; - self.suppress_path(&meta_path); fs::write(&meta_path, &content).with_context(|| { format!("Failed to write meta: {}", meta_path.display()) })?; @@ -2039,19 +2015,17 @@ impl ApiService { added.name ); if old_path.exists() { - self.suppress_path_remove(old_path); let _ = fs::remove_file(old_path); } } let dir_path = parent_dir.join(&encoded_name); - self.suppress_path(&dir_path); fs::create_dir_all(&dir_path).with_context(|| { format!("Failed to create directory: {}", dir_path.display()) })?; // Write init.meta.json5 with class and properties - self.write_init_meta_json(&dir_path, added, meta_name_field)?; + self.write_init_meta_json(&dir_path, added, meta_name_field, false)?; log::info!( "Syncback: Updated {} at {}/init.meta.json5", @@ -2069,7 +2043,6 @@ impl ApiService { ExistingFileFormat::Standalone(p) => p.clone(), _ => parent_dir.join(format!("{}.model.json5", encoded_name)), }; - self.suppress_path(&file_path); fs::write(&file_path, &content).with_context(|| { format!("Failed to write file: {}", file_path.display()) })?; @@ -2188,7 +2161,6 @@ impl ApiService { let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; - self.suppress_path(&meta_path); fs::write(&meta_path, &content) .with_context(|| format!("Failed to write meta file: {}", meta_path.display()))?; log::info!( @@ -2221,7 +2193,6 @@ impl ApiService { let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; - self.suppress_path(&meta_path); fs::write(&meta_path, &content) .with_context(|| format!("Failed to write meta file: {}", meta_path.display()))?; log::info!( @@ -2239,6 +2210,7 @@ impl ApiService { dir_path: &std::path::Path, added: &crate::web::interface::AddedInstance, instance_name: Option<&str>, + suppress_vfs: bool, ) -> anyhow::Result<()> { use anyhow::Context; @@ -2255,7 +2227,9 @@ impl ApiService { let meta_path = dir_path.join("init.meta.json5"); let content = crate::json::to_vec_pretty_sorted(&meta) .context("Failed to serialize init.meta.json5")?; - self.suppress_path(&meta_path); + if suppress_vfs { + self.suppress_path(&meta_path); + } fs::write(&meta_path, &content) .with_context(|| format!("Failed to write meta file: {}", meta_path.display()))?; log::info!( @@ -2466,7 +2440,6 @@ impl ApiService { let meta_path = parent_dir.join(format!("{}.meta.json5", name)); let content = crate::json::to_vec_pretty_sorted(&meta).context("Failed to serialize meta.json5")?; - self.suppress_path(&meta_path); fs::write(&meta_path, &content) .with_context(|| format!("Failed to write meta file: {}", meta_path.display()))?; log::info!( diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index 0a8132d4..ec80aa55 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -12,13 +12,10 @@ //! 7. ProjectNode guard (no project file corruption) //! 8. Echo suppression (no redundant VFS patches) //! -//! NOTE: Tests operate on pre-existing instances already in the Rojo tree -//! (from the initial snapshot). Adding new instances via the API writes files -//! to disk but relies on VFS events for tree updates; with echo suppression, -//! those events are suppressed for the paths the API wrote. This is correct -//! behavior — the plugin reconciles its own tree and doesn't need Rojo to -//! re-snapshot what it just sent. Tests that need to interact with the tree -//! (read back IDs, send updates) must use instances from the initial snapshot. +//! NOTE: Adding new instances via the API writes files to disk WITHOUT +//! VFS echo suppression — the watcher must pick up the new files to add +//! them to the tree. Updates and removals of EXISTING instances DO suppress +//! their VFS echoes since the tree is already mutated via handle_tree_event. use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -721,13 +718,13 @@ fn echo_suppression_prevents_redundant_patches() { let read_after = session.get_api_read(root_id).unwrap(); let cursor_delta = read_after.message_cursor - initial_cursor; - // With proper echo suppression, the cursor should not advance excessively. - // For a single add: 0–3 messages (the suppressed echo plus potential - // metadata/re-snapshot). Anything higher means echo events are leaking. + // For a single add: ~2 messages (1 from handle_tree_event broadcast + + // 1 from VFS watcher adding the new instance to the tree). Allowing 3 + // accounts for platform-specific directory-level VFS events. assert!( cursor_delta <= 3, - "Echo suppression failure: cursor advanced by {} (expected <= 3). \ - VFS echo events are leaking through suppression.", + "Echo suppression: cursor advanced by {} (expected ~2 for add: \ + 1 tree mutation + 1 VFS pickup)", cursor_delta ); @@ -2355,12 +2352,14 @@ fn echo_suppression_rapid_adds_10x() { ); } - // Server should still be responsive + // Server should still be responsive. 10 adds x ~2 events each + // (1 handle_tree_event + 1 VFS pickup) = ~20, with margin for + // platform-specific directory events. let read_after = session.get_api_read(root_id).unwrap(); let cursor_delta = read_after.message_cursor - initial_cursor; assert!( - cursor_delta < 50, - "Cursor delta {} should be reasonable with echo suppression", + cursor_delta <= 30, + "Cursor delta {} should be bounded (10 adds x ~2 events + margin)", cursor_delta );