Add directory name derivation functions in ApiService for improved file handling

- Introduced `dir_name_from_script_path` and `dir_name_from_instance_path` functions to derive directory names from script and instance paths, respectively, enhancing consistency in file operations.
- Updated the `convert_standalone_script_to_directory` and related methods to utilize the new functions for accurate directory naming, ensuring proper handling of various script suffixes.
- Added comprehensive unit tests for the new functions to validate their behavior across different file types and edge cases, improving overall test coverage and reliability.

src/web/api.rs

@@ -159,6 +159,56 @@ pub struct ApiService {
     suppressed_paths: Arc<Mutex<HashMap<PathBuf, usize>>>,
 }
 
+/// Derives the directory name from a standalone script's filesystem path.
+///
+/// Uses `file_stem()` with script suffix stripping instead of the decoded
+/// instance name, so that Windows-invalid character encoding (e.g., `%3F`
+/// for `?`) is preserved.
+///
+/// Examples:
+/// - `What%3F.server.luau` → `What%3F`
+/// - `MyModule.luau` → `MyModule`
+/// - `Handler.client.lua` → `Handler`
+fn dir_name_from_script_path(standalone_path: &Path) -> &str {
+    let file_stem = standalone_path
+        .file_stem()
+        .and_then(|s| s.to_str())
+        .unwrap_or("");
+    file_stem
+        .strip_suffix(".server")
+        .or_else(|| file_stem.strip_suffix(".client"))
+        .or_else(|| file_stem.strip_suffix(".plugin"))
+        .or_else(|| file_stem.strip_suffix(".local"))
+        .or_else(|| file_stem.strip_suffix(".legacy"))
+        .unwrap_or(file_stem)
+}
+
+/// Derives the directory name from a standalone non-script instance's filesystem path.
+///
+/// Strips compound extensions (`.model.json5`, `.model.json`) or falls back
+/// to `file_stem()` for single-extension types (`.txt`, `.csv`, etc.).
+///
+/// Examples:
+/// - `What%3F.model.json5` → `What%3F`
+/// - `MyPart.model.json` → `MyPart`
+/// - `Greeting.txt` → `Greeting`
+/// - `Translations.csv` → `Translations`
+fn dir_name_from_instance_path(standalone_path: &Path) -> &str {
+    let file_name = standalone_path
+        .file_name()
+        .and_then(|f| f.to_str())
+        .unwrap_or("");
+    file_name
+        .strip_suffix(".model.json5")
+        .or_else(|| file_name.strip_suffix(".model.json"))
+        .unwrap_or_else(|| {
+            standalone_path
+                .file_stem()
+                .and_then(|s| s.to_str())
+                .unwrap_or(file_name)
+        })
+}
+
 impl ApiService {
     pub fn new(serve_session: Arc<ServeSession>) -> Self {
         let suppressed_paths = serve_session.suppressed_paths();
@@ -921,7 +971,7 @@ impl ApiService {
     fn convert_standalone_script_to_directory(
         &self,
         standalone_path: &std::path::Path,
-        script_name: &str,
+        _script_name: &str,
         class_name: &str,
         containing_dir: &std::path::Path,
     ) -> anyhow::Result<std::path::PathBuf> {
@@ -934,8 +984,10 @@ impl ApiService {
             String::new()
         };
 
+        let dir_name = dir_name_from_script_path(standalone_path);
+
         // Create the directory
-        let new_dir = containing_dir.join(script_name);
+        let new_dir = containing_dir.join(dir_name);
         self.suppress_path(&new_dir);
         fs::create_dir_all(&new_dir).with_context(|| {
             format!(
@@ -970,7 +1022,7 @@ impl ApiService {
         }
 
         // Move adjacent meta file into directory if it exists
-        let meta_path = containing_dir.join(format!("{}.meta.json5", script_name));
+        let meta_path = containing_dir.join(format!("{}.meta.json5", dir_name));
         if meta_path.exists() {
             let init_meta_path = new_dir.join("init.meta.json5");
             self.suppress_path(&meta_path);
@@ -1012,7 +1064,9 @@ impl ApiService {
     ) -> anyhow::Result<std::path::PathBuf> {
         use anyhow::Context;
 
-        let new_dir = containing_dir.join(instance_name);
+        let dir_name = dir_name_from_instance_path(standalone_path);
+
+        let new_dir = containing_dir.join(dir_name);
         self.suppress_path(&new_dir);
         fs::create_dir_all(&new_dir).with_context(|| {
             format!(
@@ -5140,4 +5194,163 @@ mod tests {
             );
         }
     }
+
+    mod dir_name_from_script_path_tests {
+        use super::*;
+
+        #[test]
+        fn module_script_luau() {
+            let path = Path::new("src/MyModule.luau");
+            assert_eq!(dir_name_from_script_path(path), "MyModule");
+        }
+
+        #[test]
+        fn server_script_luau() {
+            let path = Path::new("src/Handler.server.luau");
+            assert_eq!(dir_name_from_script_path(path), "Handler");
+        }
+
+        #[test]
+        fn client_script_luau() {
+            let path = Path::new("src/UI.client.luau");
+            assert_eq!(dir_name_from_script_path(path), "UI");
+        }
+
+        #[test]
+        fn local_script_luau() {
+            let path = Path::new("src/Input.local.luau");
+            assert_eq!(dir_name_from_script_path(path), "Input");
+        }
+
+        #[test]
+        fn plugin_script_luau() {
+            let path = Path::new("src/MyPlugin.plugin.luau");
+            assert_eq!(dir_name_from_script_path(path), "MyPlugin");
+        }
+
+        #[test]
+        fn legacy_script_luau() {
+            let path = Path::new("src/OldCode.legacy.luau");
+            assert_eq!(dir_name_from_script_path(path), "OldCode");
+        }
+
+        #[test]
+        fn legacy_lua_extension() {
+            let path = Path::new("src/OldModule.lua");
+            assert_eq!(dir_name_from_script_path(path), "OldModule");
+        }
+
+        #[test]
+        fn legacy_server_lua() {
+            let path = Path::new("src/Main.server.lua");
+            assert_eq!(dir_name_from_script_path(path), "Main");
+        }
+
+        #[test]
+        fn legacy_client_lua() {
+            let path = Path::new("src/Client.client.lua");
+            assert_eq!(dir_name_from_script_path(path), "Client");
+        }
+
+        #[test]
+        fn encoded_windows_chars_module() {
+            // ? is invalid on Windows, encoded as %3F
+            let path = Path::new("src/What%3F.luau");
+            assert_eq!(dir_name_from_script_path(path), "What%3F");
+        }
+
+        #[test]
+        fn encoded_windows_chars_server() {
+            let path = Path::new("src/What%3F.server.luau");
+            assert_eq!(dir_name_from_script_path(path), "What%3F");
+        }
+
+        #[test]
+        fn encoded_colon_char() {
+            // : is invalid on Windows, encoded as %3A
+            let path = Path::new("src/Key%3AValue.client.luau");
+            assert_eq!(dir_name_from_script_path(path), "Key%3AValue");
+        }
+
+        #[test]
+        fn multiple_dots_in_name() {
+            // Name like "my.module.luau" — file_stem is "my.module", no suffix match
+            let path = Path::new("src/my.module.luau");
+            assert_eq!(dir_name_from_script_path(path), "my.module");
+        }
+
+        #[test]
+        fn name_ending_with_suffix_substring() {
+            // "MyServer" should NOT be stripped to "My" — ".server" stripping only
+            // applies to the suffix after file_stem strips ".luau"
+            let path = Path::new("src/MyServer.luau");
+            assert_eq!(dir_name_from_script_path(path), "MyServer");
+        }
+    }
+
+    mod dir_name_from_instance_path_tests {
+        use super::*;
+
+        #[test]
+        fn model_json5() {
+            let path = Path::new("src/MyPart.model.json5");
+            assert_eq!(dir_name_from_instance_path(path), "MyPart");
+        }
+
+        #[test]
+        fn model_json_legacy() {
+            let path = Path::new("src/MyPart.model.json");
+            assert_eq!(dir_name_from_instance_path(path), "MyPart");
+        }
+
+        #[test]
+        fn txt_file() {
+            let path = Path::new("src/Greeting.txt");
+            assert_eq!(dir_name_from_instance_path(path), "Greeting");
+        }
+
+        #[test]
+        fn csv_file() {
+            let path = Path::new("src/Translations.csv");
+            assert_eq!(dir_name_from_instance_path(path), "Translations");
+        }
+
+        #[test]
+        fn encoded_windows_chars_model() {
+            let path = Path::new("src/What%3F.model.json5");
+            assert_eq!(dir_name_from_instance_path(path), "What%3F");
+        }
+
+        #[test]
+        fn encoded_windows_chars_txt() {
+            let path = Path::new("src/Ask%3F.txt");
+            assert_eq!(dir_name_from_instance_path(path), "Ask%3F");
+        }
+
+        #[test]
+        fn encoded_colon_model() {
+            let path = Path::new("src/Key%3AValue.model.json5");
+            assert_eq!(dir_name_from_instance_path(path), "Key%3AValue");
+        }
+
+        #[test]
+        fn encoded_colon_legacy_model() {
+            let path = Path::new("src/Key%3AValue.model.json");
+            assert_eq!(dir_name_from_instance_path(path), "Key%3AValue");
+        }
+
+        #[test]
+        fn multiple_dots_in_name_model() {
+            // "my.part.model.json5" — should strip ".model.json5"
+            let path = Path::new("src/my.part.model.json5");
+            assert_eq!(dir_name_from_instance_path(path), "my.part");
+        }
+
+        #[test]
+        fn unknown_extension_uses_file_stem() {
+            // Fallback to file_stem for unrecognized extensions
+            let path = Path::new("src/Something.toml");
+            assert_eq!(dir_name_from_instance_path(path), "Something");
+        }
+    }
 }

tests/tests/two_way_sync.rs

@@ -634,3 +634,435 @@ fn echo_suppression_prevents_redundant_patches() {
         );
     });
 }
+
+// ---------------------------------------------------------------------------
+// Helpers (syncback_format_transitions fixture)
+// ---------------------------------------------------------------------------
+
+/// Look up an instance by name under ReplicatedStorage in the format_transitions fixture.
+fn get_format_transitions_instance(
+    session: &crate::rojo_test::serve_util::TestServeSession,
+    instance_name: &str,
+) -> (librojo::SessionId, Ref) {
+    let info = session.get_api_rojo().unwrap();
+    let root_read = session.get_api_read(info.root_instance_id).unwrap();
+    let (rs_id, _) = find_by_class(&root_read.instances, "ReplicatedStorage");
+    let rs_read = session.get_api_read(rs_id).unwrap();
+    let (instance_id, _) = find_by_name(&rs_read.instances, instance_name);
+    (info.session_id, instance_id)
+}
+
+// ---------------------------------------------------------------------------
+// Tests: Directory-format operations
+// ---------------------------------------------------------------------------
+
+/// Test 13: Renaming a directory-format script renames the parent directory,
+/// not the init file inside it. Children should move with it.
+#[test]
+fn rename_directory_format_script() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, dir_module_id) =
+            get_format_transitions_instance(&session, "DirModuleWithChildren");
+
+        let old_dir = session.path().join("src").join("DirModuleWithChildren");
+        let new_dir = session.path().join("src").join("RenamedDirModule");
+
+        assert!(old_dir.is_dir(), "Old directory should exist before rename");
+
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: dir_module_id,
+                changed_name: Some("RenamedDirModule".to_string()),
+                changed_class_name: None,
+                changed_properties: UstrMap::default(),
+                changed_metadata: None,
+            },
+        );
+
+        assert!(
+            !old_dir.exists(),
+            "Old directory should be gone after rename: {}",
+            old_dir.display()
+        );
+        assert!(
+            new_dir.is_dir(),
+            "New directory should exist after rename: {}",
+            new_dir.display()
+        );
+
+        // init.luau should be inside the renamed directory
+        let init_file = new_dir.join("init.luau");
+        assert_file_exists(&init_file, "init.luau inside renamed directory");
+
+        // Children should also be present in the renamed directory
+        let child_a = new_dir.join("ChildA.luau");
+        assert_file_exists(&child_a, "ChildA.luau inside renamed directory");
+    });
+}
+
+/// Test 14: Changing ClassName on a directory-format ModuleScript to Script
+/// renames the init file inside the directory (init.luau → init.server.luau).
+#[test]
+fn classname_change_directory_module_to_script() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, dir_module_id) =
+            get_format_transitions_instance(&session, "DirModuleWithChildren");
+
+        let dir = session.path().join("src").join("DirModuleWithChildren");
+        let old_init = dir.join("init.luau");
+        let new_init = dir.join("init.server.luau");
+
+        assert_file_exists(&old_init, "init.luau before class change");
+        assert_not_exists(&new_init, "init.server.luau before class change");
+
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: dir_module_id,
+                changed_name: None,
+                changed_class_name: Some(ustr("Script")),
+                changed_properties: UstrMap::default(),
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_init, "init.luau after class change to Script");
+        assert_file_exists(&new_init, "init.server.luau after class change to Script");
+
+        // Children should be unaffected
+        let child_a = dir.join("ChildA.luau");
+        assert_file_exists(&child_a, "ChildA.luau unaffected by class change");
+    });
+}
+
+// ---------------------------------------------------------------------------
+// Tests: Reverse ClassName transitions (code paths added in these commits)
+// ---------------------------------------------------------------------------
+
+/// Test 15: Script → ModuleScript ClassName transition (reverse of test 6).
+/// Validates that the .server suffix is correctly stripped.
+#[test]
+fn classname_change_script_to_module() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, script_id) =
+            get_format_transitions_instance(&session, "StandaloneScript");
+
+        let old_path = session
+            .path()
+            .join("src")
+            .join("StandaloneScript.server.luau");
+        let new_path = session.path().join("src").join("StandaloneScript.luau");
+
+        assert_file_exists(&old_path, "Script file before class change");
+
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: script_id,
+                changed_name: None,
+                changed_class_name: Some(ustr("ModuleScript")),
+                changed_properties: UstrMap::default(),
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_path, "Script file after class change to ModuleScript");
+        assert_file_exists(&new_path, "ModuleScript file after class change");
+    });
+}
+
+/// Test 16: Script → LocalScript ClassName transition.
+/// Validates that .server is replaced with .local.
+#[test]
+fn classname_change_script_to_localscript() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, script_id) =
+            get_format_transitions_instance(&session, "StandaloneScript");
+
+        let old_path = session
+            .path()
+            .join("src")
+            .join("StandaloneScript.server.luau");
+        let new_path = session
+            .path()
+            .join("src")
+            .join("StandaloneScript.local.luau");
+
+        assert_file_exists(&old_path, "Script file before class change");
+
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: script_id,
+                changed_name: None,
+                changed_class_name: Some(ustr("LocalScript")),
+                changed_properties: UstrMap::default(),
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_path, "Script file after class change to LocalScript");
+        assert_file_exists(&new_path, "LocalScript file after class change");
+    });
+}
+
+/// Test 17: LocalScript → Script ClassName transition.
+/// Exercises .client suffix handling (the fixture uses .client.luau for LocalScript).
+#[test]
+fn classname_change_localscript_to_script() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, local_id) =
+            get_format_transitions_instance(&session, "StandaloneLocalScript");
+
+        let old_path = session
+            .path()
+            .join("src")
+            .join("StandaloneLocalScript.client.luau");
+        let new_path = session
+            .path()
+            .join("src")
+            .join("StandaloneLocalScript.server.luau");
+
+        assert_file_exists(&old_path, "LocalScript file before class change");
+
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: local_id,
+                changed_name: None,
+                changed_class_name: Some(ustr("Script")),
+                changed_properties: UstrMap::default(),
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_path, "LocalScript file after class change to Script");
+        assert_file_exists(&new_path, "Script file after class change");
+    });
+}
+
+// ---------------------------------------------------------------------------
+// Tests: Combined operations (overridden_source_path logic)
+// ---------------------------------------------------------------------------
+
+/// Test 18: Combined rename + Source update in a single request.
+/// Validates that Source is written to the RENAMED file, not the old location.
+/// This exercises the `overridden_source_path` tracking in ChangeProcessor.
+#[test]
+fn combined_rename_and_source_update() {
+    run_serve_test("syncback_write", |session, _redactions| {
+        let (session_id, _rs_id, existing_id) = get_rs_and_existing(&session);
+
+        let old_path = session.path().join("src").join("existing.luau");
+        let new_path = session.path().join("src").join("CombinedRename.luau");
+
+        assert_file_exists(&old_path, "existing.luau before combined update");
+
+        let mut props = UstrMap::default();
+        props.insert(
+            ustr("Source"),
+            Some(Variant::String("-- Written after rename".to_string())),
+        );
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: existing_id,
+                changed_name: Some("CombinedRename".to_string()),
+                changed_class_name: None,
+                changed_properties: props,
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_path, "Old file after combined rename+source");
+        assert_file_exists(&new_path, "Renamed file after combined rename+source");
+
+        let content = fs::read_to_string(&new_path).unwrap();
+        assert!(
+            content.contains("Written after rename"),
+            "Source should be written to the NEW file location, got: {}",
+            content
+        );
+    });
+}
+
+/// Test 19: Combined ClassName change + Source update in a single request.
+/// Validates that Source is written to the file with the NEW extension.
+#[test]
+fn combined_classname_and_source_update() {
+    run_serve_test("syncback_write", |session, _redactions| {
+        let (session_id, _rs_id, existing_id) = get_rs_and_existing(&session);
+
+        let old_path = session.path().join("src").join("existing.luau");
+        let new_path = session.path().join("src").join("existing.server.luau");
+
+        assert_file_exists(&old_path, "existing.luau before combined update");
+
+        let mut props = UstrMap::default();
+        props.insert(
+            ustr("Source"),
+            Some(Variant::String(
+                "-- Source after class change".to_string(),
+            )),
+        );
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: existing_id,
+                changed_name: None,
+                changed_class_name: Some(ustr("Script")),
+                changed_properties: props,
+                changed_metadata: None,
+            },
+        );
+
+        assert_not_exists(&old_path, "ModuleScript file after ClassName+Source");
+        assert_file_exists(&new_path, "Script file after ClassName+Source");
+
+        let content = fs::read_to_string(&new_path).unwrap();
+        assert!(
+            content.contains("Source after class change"),
+            "Source should be in the new file, got: {}",
+            content
+        );
+    });
+}
+
+// ---------------------------------------------------------------------------
+// Tests: Standalone → directory conversion & suffix-aware removal
+// ---------------------------------------------------------------------------
+
+/// Test 20: Adding a child instance to a standalone script converts it to
+/// directory format (e.g., StandaloneModule.luau → StandaloneModule/init.luau).
+/// This exercises `convert_standalone_script_to_directory` in ApiService.
+#[test]
+fn add_child_converts_standalone_to_directory() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let info = session.get_api_rojo().unwrap();
+        let root_read = session.get_api_read(info.root_instance_id).unwrap();
+        let (rs_id, _) = find_by_class(&root_read.instances, "ReplicatedStorage");
+        let rs_read = session.get_api_read(rs_id).unwrap();
+        let (standalone_id, _) = find_by_name(&rs_read.instances, "StandaloneModule");
+
+        let standalone_file = session.path().join("src").join("StandaloneModule.luau");
+        let dir_path = session.path().join("src").join("StandaloneModule");
+
+        assert_file_exists(&standalone_file, "Standalone file before child add");
+        assert!(
+            !dir_path.exists(),
+            "Directory should NOT exist before child add"
+        );
+
+        // Add a child ModuleScript to StandaloneModule
+        let mut properties = HashMap::new();
+        properties.insert(
+            "Source".to_string(),
+            Variant::String("-- New child module".to_string()),
+        );
+        let added = AddedInstance {
+            parent: Some(standalone_id),
+            name: "NewChild".to_string(),
+            class_name: "ModuleScript".to_string(),
+            properties,
+            children: vec![],
+        };
+        let mut added_map = HashMap::new();
+        added_map.insert(Ref::new(), added);
+        let write_request = WriteRequest {
+            session_id: info.session_id,
+            removed: vec![],
+            added: added_map,
+            updated: vec![],
+        };
+        session.post_api_write(&write_request).unwrap();
+        thread::sleep(Duration::from_millis(500));
+
+        // Standalone file should be gone, replaced by directory
+        assert_not_exists(
+            &standalone_file,
+            "Standalone file after child add (converted to directory)",
+        );
+        assert!(
+            dir_path.is_dir(),
+            "Directory should exist after conversion: {}",
+            dir_path.display()
+        );
+
+        // init.luau should contain the original source
+        let init_file = dir_path.join("init.luau");
+        assert_file_exists(&init_file, "init.luau after standalone→directory conversion");
+
+        let init_content = fs::read_to_string(&init_file).unwrap();
+        assert!(
+            init_content.contains("Standalone ModuleScript"),
+            "init.luau should contain original source, got: {}",
+            init_content
+        );
+
+        // Child should be created inside the directory
+        let child_file = dir_path.join("NewChild.luau");
+        assert_file_exists(&child_file, "NewChild.luau inside converted directory");
+    });
+}
+
+/// Test 21: Removing a Script (with .server suffix) properly cleans up
+/// the adjacent meta file by stripping the script suffix to find the
+/// correct meta file name (e.g., StandaloneScript.meta.json5).
+#[test]
+fn remove_script_with_suffix_cleans_meta() {
+    run_serve_test("syncback_format_transitions", |session, _redactions| {
+        let (session_id, script_id) =
+            get_format_transitions_instance(&session, "StandaloneScript");
+
+        let script_path = session
+            .path()
+            .join("src")
+            .join("StandaloneScript.server.luau");
+        let meta_path = session
+            .path()
+            .join("src")
+            .join("StandaloneScript.meta.json5");
+
+        assert_file_exists(&script_path, "Script file before operations");
+
+        // First, create a meta file by sending a non-Source property update
+        let mut attrs = rbx_dom_weak::types::Attributes::new();
+        attrs.insert("ScriptAttr".to_string(), Variant::Float64(7.0));
+        let mut props = UstrMap::default();
+        props.insert(ustr("Attributes"), Some(Variant::Attributes(attrs)));
+        send_update(
+            &session,
+            &session_id,
+            InstanceUpdate {
+                id: script_id,
+                changed_name: None,
+                changed_class_name: None,
+                changed_properties: props,
+                changed_metadata: None,
+            },
+        );
+
+        assert_file_exists(&meta_path, "Meta file after property update");
+
+        // Re-read ID in case tree was rebuilt after the update
+        let (session_id, script_id) =
+            get_format_transitions_instance(&session, "StandaloneScript");
+
+        // Now remove the instance — both script and meta should be deleted
+        send_removal(&session, &session_id, vec![script_id]);
+
+        assert_not_exists(&script_path, "Script file after removal");
+        assert_not_exists(
+            &meta_path,
+            "Meta file after removal (suffix-stripped cleanup)",
+        );
+    });
+}