Avoid clone in src/syncback/file_names.rs

Co-authored-by: krakow10 <krakow20@gmail.com>

CHANGELOG.md

@@ -31,10 +31,8 @@ Making a new release? Simply add the new header with the version and date undern
 
 ## Unreleased
 * Fixed a bug caused by having reference properties (such as `ObjectValue.Value`) that point to an Instance not included in syncback. ([#1179])
-* Fixed instance replacement fallback failing when too many instances needed to be replaced. ([#1192])
 
 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179
-[#1192]: https://github.com/rojo-rbx/rojo/pull/1192
 
 ## [7.7.0-rc.1] (November 27th, 2025)
 

plugin/src/ApiContext.lua

@@ -290,39 +290,31 @@ function ApiContext:open(id)
 end
 
 function ApiContext:serialize(ids: { string })
-	local url = ("%s/api/serialize"):format(self.__baseUrl)
-	local request_body = Http.jsonEncode({ sessionId = self.__sessionId, ids = ids })
+	local url = ("%s/api/serialize/%s"):format(self.__baseUrl, table.concat(ids, ","))
 
-	return Http.post(url, request_body)
-		:andThen(rejectFailedRequests)
-		:andThen(Http.Response.json)
-		:andThen(function(response_body)
-			if response_body.sessionId ~= self.__sessionId then
-				return Promise.reject("Server changed ID")
-			end
+	return Http.get(url):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(body)
+		if body.sessionId ~= self.__sessionId then
+			return Promise.reject("Server changed ID")
+		end
 
-			assert(validateApiSerialize(response_body))
+		assert(validateApiSerialize(body))
 
-			return response_body
-		end)
+		return body
+	end)
 end
 
 function ApiContext:refPatch(ids: { string })
-	local url = ("%s/api/ref-patch"):format(self.__baseUrl)
-	local request_body = Http.jsonEncode({ sessionId = self.__sessionId, ids = ids })
+	local url = ("%s/api/ref-patch/%s"):format(self.__baseUrl, table.concat(ids, ","))
 
-	return Http.post(url, request_body)
-		:andThen(rejectFailedRequests)
-		:andThen(Http.Response.json)
-		:andThen(function(response_body)
-			if response_body.sessionId ~= self.__sessionId then
-				return Promise.reject("Server changed ID")
-			end
+	return Http.get(url):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(body)
+		if body.sessionId ~= self.__sessionId then
+			return Promise.reject("Server changed ID")
+		end
 
-			assert(validateApiRefPatch(response_body))
+		assert(validateApiRefPatch(body))
 
-			return response_body
-		end)
+		return body
+	end)
 end
 
 return ApiContext

rojo-test/build-test-snapshots/end_to_end__tests__build__json_model_legacy_name.snap

@@ -1,16 +0,0 @@
----
-source: tests/tests/build.rs
-expression: contents
----
-<roblox version="4">
-  <Item class="Folder" referent="0">
-    <Properties>
-      <string name="Name">json_model_legacy_name</string>
-    </Properties>
-    <Item class="Folder" referent="1">
-      <Properties>
-        <string name="Name">Expected Name</string>
-      </Properties>
-    </Item>
-  </Item>
-</roblox>

rojo-test/build-test-snapshots/end_to_end__tests__build__model_json_name_input.snap

@@ -0,0 +1,23 @@
+---
+source: tests/tests/build.rs
+assertion_line: 109
+expression: contents
+---
+<roblox version="4">
+  <Item class="DataModel" referent="0">
+    <Properties>
+      <string name="Name">model_json_name_input</string>
+    </Properties>
+    <Item class="Workspace" referent="1">
+      <Properties>
+        <string name="Name">Workspace</string>
+        <bool name="NeedsPivotMigration">false</bool>
+      </Properties>
+      <Item class="StringValue" referent="2">
+        <Properties>
+          <string name="Name">/Bar</string>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/end_to_end__tests__build__slugified_name_roundtrip.snap

@@ -0,0 +1,20 @@
+---
+source: tests/tests/build.rs
+assertion_line: 108
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">slugified_name_roundtrip</string>
+    </Properties>
+    <Item class="Script" referent="1">
+      <Properties>
+        <string name="Name">/Script</string>
+        <token name="RunContext">0</token>
+        <string name="Source"><![CDATA[print("Hello world!")
+]]></string>
+      </Properties>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-tests/json_model_legacy_name/default.project.json

@@ -1,6 +0,0 @@
-{
-  "name": "json_model_legacy_name",
-  "tree": {
-    "$path": "folder"
-  }
-}

rojo-test/build-tests/json_model_legacy_name/folder/Expected Name.model.json

@@ -1,4 +0,0 @@
-{
-  "Name": "Overridden Name",
-  "ClassName": "Folder"
-}

rojo-test/build-tests/model_json_name_input/default.project.json

@@ -0,0 +1,11 @@
+{
+  "name": "model_json_name_input",
+  "tree": {
+    "$className": "DataModel",
+    "Workspace": {
+      "$className": "Workspace",
+      "$path": "src"
+    }
+  }
+}
+

rojo-test/build-tests/model_json_name_input/src/Bar.model.json

@@ -0,0 +1,5 @@
+{
+  "name": "/Bar",
+  "className": "StringValue"
+}
+

rojo-test/build-tests/slugified_name_roundtrip/_Script/init.meta.json

@@ -0,0 +1,4 @@
+{
+  "name": "/Script"
+}
+

rojo-test/build-tests/slugified_name_roundtrip/_Script/init.server.luau

@@ -0,0 +1,2 @@
+print("Hello world!")
+

rojo-test/build-tests/slugified_name_roundtrip/default.project.json

@@ -0,0 +1,6 @@
+{
+  "name": "slugified_name_roundtrip",
+  "tree": {
+    "$path": "src"
+  }
+}

rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.meta.json

@@ -0,0 +1,3 @@
+{
+  "name": "/Script"
+}

rojo-test/build-tests/slugified_name_roundtrip/src/_Script/init.server.luau

@@ -0,0 +1 @@
+print("Hello world!")

rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__model_json_name-stdout.snap

@@ -0,0 +1,6 @@
+---
+source: tests/rojo_test/syncback_util.rs
+assertion_line: 101
+expression: "String::from_utf8_lossy(&output.stdout)"
+---
+

rojo-test/syncback-test-snapshots/end_to_end__rojo_test__syncback_util__slugified_name-stdout.snap

@@ -0,0 +1,13 @@
+---
+source: tests/rojo_test/syncback_util.rs
+assertion_line: 101
+expression: "String::from_utf8_lossy(&output.stdout)"
+---
+Writing default.project.json
+Writing src/Camera.rbxm
+Writing src/Terrain.rbxm
+Writing src/_Folder/init.meta.json
+Writing src/_Script.meta.json
+Writing src/_Script.server.luau
+Writing src
+Writing src/_Folder

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__model_json_name-src__foo.model.json.snap

@@ -0,0 +1,9 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/foo.model.json
+---
+{
+  "name": "/Bar",
+  "className": "StringValue"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder.model.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Folder.model.json
+---
+{
+  "className": "Folder"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Folder__init.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Folder/init.meta.json
+---
+{
+  "name": "/Folder"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script.meta.json
+---
+{
+  "name": "/Script"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script.server.luau.snap

@@ -0,0 +1,6 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script.server.luau
+---
+print("Hello world!")

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.meta.json.snap

@@ -0,0 +1,8 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script/init.meta.json
+---
+{
+  "name": "/Script"
+}

rojo-test/syncback-test-snapshots/end_to_end__tests__syncback__slugified_name-src___Script__init.server.luau.snap

@@ -0,0 +1,6 @@
+---
+source: tests/tests/syncback.rs
+assertion_line: 31
+expression: src/_Script/init.server.luau
+---
+print("Hello world!")

rojo-test/syncback-tests/model_json_name/input-project/default.project.json

@@ -0,0 +1,11 @@
+{
+  "name": "model_json_name",
+  "tree": {
+    "$className": "DataModel",
+    "Workspace": {
+      "$className": "Workspace",
+      "$path": "src"
+    }
+  }
+}
+

rojo-test/syncback-tests/model_json_name/input-project/src/foo.model.json

@@ -0,0 +1,5 @@
+{
+  "name": "/Bar",
+  "className": "StringValue"
+}
+

rojo-test/syncback-tests/slugified_name/input-project/default.project.json

@@ -0,0 +1,10 @@
+{
+  "name": "slugified_name",
+  "tree": {
+    "$className": "DataModel",
+    "Workspace": {
+      "$className": "Workspace",
+      "$path": "src"
+    }
+  }
+}

src/cli/syncback.rs

@@ -54,11 +54,6 @@ pub struct SyncbackCommand {
     /// If provided, the prompt for writing to the file system is skipped.
     #[clap(long, short = 'y')]
     pub non_interactive: bool,
-
-    /// If provided, forces syncback to use JSON model files instead of binary
-    /// .rbxm files for instances that would otherwise serialize as binary.
-    #[clap(long)]
-    pub dangerously_force_json: bool,
 }
 
 impl SyncbackCommand {
@@ -109,7 +104,6 @@ impl SyncbackCommand {
             &mut dom_old,
             dom_new,
             session_old.root_project(),
-            self.dangerously_force_json,
         )?;
         log::debug!(
             "Syncback finished in {:.02}s!",

src/snapshot/metadata.rs

@@ -70,6 +70,12 @@ pub struct InstanceMetadata {
     /// A schema provided via a JSON file, if one exists. Will be `None` for
     /// all non-JSON middleware.
     pub schema: Option<String>,
+
+    /// A custom name specified via meta.json or model.json files. If present,
+    /// this name will be used for the instance while the filesystem name will
+    /// be slugified to remove illegal characters.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub specified_name: Option<String>,
 }
 
 impl InstanceMetadata {
@@ -82,6 +88,7 @@ impl InstanceMetadata {
             specified_id: None,
             middleware: None,
             schema: None,
+            specified_name: None,
         }
     }
 
@@ -130,6 +137,13 @@ impl InstanceMetadata {
     pub fn schema(self, schema: Option<String>) -> Self {
         Self { schema, ..self }
     }
+
+    pub fn specified_name(self, specified_name: Option<String>) -> Self {
+        Self {
+            specified_name,
+            ..self
+        }
+    }
 }
 
 impl Default for InstanceMetadata {

src/snapshot_middleware/json_model.rs

@@ -35,20 +35,14 @@ pub fn snapshot_json_model(
         format!("File is not a valid JSON model: {}", path.display())
     })?;
 
-    if let Some(top_level_name) = &instance.name {
-        let new_name = format!("{}.model.json", top_level_name);
+    // If the JSON has a name property, preserve it in metadata for syncback
+    let specified_name = instance.name.clone();
 
-        log::warn!(
-            "Model at path {} had a top-level Name field. \
-            This field has been ignored since Rojo 6.0.\n\
-            Consider removing this field and renaming the file to {}.",
-            new_name,
-            path.display()
-        );
+    // Use the name from JSON if present, otherwise fall back to filename-derived name
+    if instance.name.is_none() {
+        instance.name = Some(name.to_owned());
     }
 
-    instance.name = Some(name.to_owned());
-
     let id = instance.id.take().map(RojoRef::new);
     let schema = instance.schema.take();
 
@@ -62,7 +56,8 @@ pub fn snapshot_json_model(
         .relevant_paths(vec![path.to_path_buf()])
         .context(context)
         .specified_id(id)
-        .schema(schema);
+        .schema(schema)
+        .specified_name(specified_name);
 
     Ok(Some(snapshot))
 }
@@ -81,6 +76,7 @@ pub fn syncback_json_model<'sync>(
         // schemas will ever exist in one project for it to matter, but it
         // could have a performance cost.
         model.schema = old_inst.metadata().schema.clone();
+        model.name = old_inst.metadata().specified_name.clone();
     }
 
     Ok(SyncbackReturn {

src/snapshot_middleware/lua.rs

@@ -158,8 +158,16 @@ pub fn syncback_lua<'sync>(
 
         if !meta.is_empty() {
             let parent_location = snapshot.path.parent_err()?;
+            let instance_name = &snapshot.new_inst().name;
+            let slugified;
+            let meta_name = if crate::syncback::validate_file_name(instance_name).is_err() {
+                slugified = crate::syncback::slugify_name(instance_name);
+                &slugified
+            } else {
+                instance_name
+            };
             fs_snapshot.add_file(
-                parent_location.join(format!("{}.meta.json", new_inst.name)),
+                parent_location.join(format!("{}.meta.json", meta_name)),
                 serde_json::to_vec_pretty(&meta).context("cannot serialize metadata")?,
             );
         }

src/snapshot_middleware/meta_file.rs

@@ -10,7 +10,10 @@ use rbx_dom_weak::{
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, syncback::SyncbackSnapshot,
+    json,
+    resolution::UnresolvedValue,
+    snapshot::InstanceSnapshot,
+    syncback::{validate_file_name, SyncbackSnapshot},
     RojoRef,
 };
 
@@ -36,6 +39,9 @@ pub struct AdjacentMetadata {
     #[serde(default, skip_serializing_if = "IndexMap::is_empty")]
     pub attributes: IndexMap<String, UnresolvedValue>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub name: Option<String>,
+
     #[serde(skip)]
     pub path: PathBuf,
 }
@@ -144,6 +150,24 @@ impl AdjacentMetadata {
             }
         }
 
+        let name = snapshot
+            .old_inst()
+            .and_then(|inst| inst.metadata().specified_name.clone())
+            .or_else(|| {
+                // If this is a new instance and its name is invalid for the filesystem,
+                // we need to specify the name in meta.json so it can be preserved
+                if snapshot.old_inst().is_none() {
+                    let instance_name = &snapshot.new_inst().name;
+                    if validate_file_name(instance_name).is_err() {
+                        Some(instance_name.clone())
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            });
+
         Ok(Some(Self {
             ignore_unknown_instances: if ignore_unknown_instances {
                 Some(true)
@@ -155,6 +179,7 @@ impl AdjacentMetadata {
             path,
             id: None,
             schema,
+            name,
         }))
     }
 
@@ -213,11 +238,26 @@ impl AdjacentMetadata {
         Ok(())
     }
 
+    fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
+        if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
+            anyhow::bail!(
+                "cannot specify a name using {} (instance has a name from somewhere else)",
+                self.path.display()
+            );
+        }
+        if let Some(name) = &self.name {
+            snapshot.name = name.clone().into();
+        }
+        snapshot.metadata.specified_name = self.name.take();
+        Ok(())
+    }
+
     pub fn apply_all(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
         self.apply_ignore_unknown_instances(snapshot);
         self.apply_properties(snapshot)?;
         self.apply_id(snapshot)?;
         self.apply_schema(snapshot)?;
+        self.apply_name(snapshot)?;
         Ok(())
     }
 
@@ -226,11 +266,13 @@ impl AdjacentMetadata {
     ///
     /// - The number of properties and attributes is 0
     /// - `ignore_unknown_instances` is None
+    /// - `name` is None
     #[inline]
     pub fn is_empty(&self) -> bool {
         self.attributes.is_empty()
             && self.properties.is_empty()
             && self.ignore_unknown_instances.is_none()
+            && self.name.is_none()
     }
 
     // TODO: Add method to allow selectively applying parts of metadata and
@@ -262,6 +304,9 @@ pub struct DirectoryMetadata {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub class_name: Option<Ustr>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub name: Option<String>,
+
     #[serde(skip)]
     pub path: PathBuf,
 }
@@ -372,6 +417,24 @@ impl DirectoryMetadata {
             }
         }
 
+        let name = snapshot
+            .old_inst()
+            .and_then(|inst| inst.metadata().specified_name.clone())
+            .or_else(|| {
+                // If this is a new instance and its name is invalid for the filesystem,
+                // we need to specify the name in meta.json so it can be preserved
+                if snapshot.old_inst().is_none() {
+                    let instance_name = &snapshot.new_inst().name;
+                    if validate_file_name(instance_name).is_err() {
+                        Some(instance_name.clone())
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            });
+
         Ok(Some(Self {
             ignore_unknown_instances: if ignore_unknown_instances {
                 Some(true)
@@ -384,6 +447,7 @@ impl DirectoryMetadata {
             path,
             id: None,
             schema,
+            name,
         }))
     }
 
@@ -393,6 +457,7 @@ impl DirectoryMetadata {
         self.apply_properties(snapshot)?;
         self.apply_id(snapshot)?;
         self.apply_schema(snapshot)?;
+        self.apply_name(snapshot)?;
 
         Ok(())
     }
@@ -464,17 +529,33 @@ impl DirectoryMetadata {
         snapshot.metadata.schema = self.schema.take();
         Ok(())
     }
+
+    fn apply_name(&mut self, snapshot: &mut InstanceSnapshot) -> anyhow::Result<()> {
+        if self.name.is_some() && snapshot.metadata.specified_name.is_some() {
+            anyhow::bail!(
+                "cannot specify a name using {} (instance has a name from somewhere else)",
+                self.path.display()
+            );
+        }
+        if let Some(name) = &self.name {
+            snapshot.name = name.clone().into();
+        }
+        snapshot.metadata.specified_name = self.name.take();
+        Ok(())
+    }
     /// Returns whether the metadata is 'empty', meaning it doesn't have anything
     /// worth persisting in it. Specifically:
     ///
     /// - The number of properties and attributes is 0
     /// - `ignore_unknown_instances` is None
     /// - `class_name` is either None or not Some("Folder")
+    /// - `name` is None
     #[inline]
     pub fn is_empty(&self) -> bool {
         self.attributes.is_empty()
             && self.properties.is_empty()
             && self.ignore_unknown_instances.is_none()
+            && self.name.is_none()
             && if let Some(class) = &self.class_name {
                 class == "Folder"
             } else {

src/syncback/file_names.rs

@@ -8,11 +8,11 @@ use rbx_dom_weak::Instance;
 
 use crate::{snapshot::InstanceWithMeta, snapshot_middleware::Middleware};
 
-pub fn name_for_inst<'old>(
+pub fn name_for_inst<'a>(
     middleware: Middleware,
-    new_inst: &Instance,
-    old_inst: Option<InstanceWithMeta<'old>>,
-) -> anyhow::Result<Cow<'old, str>> {
+    new_inst: &'a Instance,
+    old_inst: Option<InstanceWithMeta<'a>>,
+) -> anyhow::Result<Cow<'a, str>> {
     if let Some(old_inst) = old_inst {
         if let Some(source) = old_inst.metadata().relevant_paths.first() {
             source
@@ -35,14 +35,24 @@ pub fn name_for_inst<'old>(
             | Middleware::CsvDir
             | Middleware::ServerScriptDir
             | Middleware::ClientScriptDir
-            | Middleware::ModuleScriptDir => Cow::Owned(new_inst.name.clone()),
+            | Middleware::ModuleScriptDir => {
+                if validate_file_name(&new_inst.name).is_err() {
+                    Cow::Owned(slugify_name(&new_inst.name))
+                } else {
+                    Cow::Borrowed(&new_inst.name)
+                }
+            }
             _ => {
                 let extension = extension_for_middleware(middleware);
-                let name = &new_inst.name;
-                validate_file_name(name).with_context(|| {
-                    format!("name '{name}' is not legal to write to the file system")
-                })?;
-                Cow::Owned(format!("{name}.{extension}"))
+                let slugified;
+                let final_name = if validate_file_name(&new_inst.name).is_err() {
+                    slugified = slugify_name(&new_inst.name);
+                    &slugified
+                } else {
+                    &new_inst.name
+                };
+
+                Cow::Owned(format!("{final_name}.{extension}"))
             }
         })
     }
@@ -94,6 +104,39 @@ const INVALID_WINDOWS_NAMES: [&str; 22] = [
 /// in a file's name.
 const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '|', '?', '*', '\\'];
 
+/// Slugifies a name by replacing forbidden characters with underscores
+/// and ensuring the result is a valid file name
+pub fn slugify_name(name: &str) -> String {
+    let mut result = String::with_capacity(name.len());
+
+    for ch in name.chars() {
+        if FORBIDDEN_CHARS.contains(&ch) {
+            result.push('_');
+        } else {
+            result.push(ch);
+        }
+    }
+
+    // Handle Windows reserved names by appending an underscore
+    let result_lower = result.to_lowercase();
+    for forbidden in INVALID_WINDOWS_NAMES {
+        if result_lower == forbidden.to_lowercase() {
+            result.push('_');
+            break;
+        }
+    }
+
+    while result.ends_with(' ') || result.ends_with('.') {
+        result.pop();
+    }
+
+    if result.is_empty() || result.chars().all(|c| c == '_') {
+        result = "instance".to_string();
+    }
+
+    result
+}
+
 /// Validates a provided file name to ensure it's allowed on the file system. An
 /// error is returned if the name isn't allowed, indicating why.
 /// This takes into account rules for Windows, MacOS, and Linux.

src/syncback/mod.rs

@@ -28,7 +28,7 @@ use crate::{
     Project,
 };
 
-pub use file_names::{extension_for_middleware, name_for_inst, validate_file_name};
+pub use file_names::{extension_for_middleware, name_for_inst, slugify_name, validate_file_name};
 pub use fs_snapshot::FsSnapshot;
 pub use hash::*;
 pub use property_filter::{filter_properties, filter_properties_preallocated};
@@ -52,7 +52,6 @@ pub fn syncback_loop(
     old_tree: &mut RojoTree,
     mut new_tree: WeakDom,
     project: &Project,
-    force_json: bool,
 ) -> anyhow::Result<FsSnapshot> {
     let ignore_patterns = project
         .syncback_rules
@@ -154,7 +153,6 @@ pub fn syncback_loop(
         old_tree,
         new_tree: &new_tree,
         project,
-        force_json,
     };
 
     let mut snapshots = vec![SyncbackSnapshot {
@@ -199,7 +197,7 @@ pub fn syncback_loop(
             }
         }
 
-        let middleware = get_best_middleware(&snapshot, force_json);
+        let middleware = get_best_middleware(&snapshot);
 
         log::trace!(
             "Middleware for {inst_path} is {:?} (path is {})",
@@ -215,14 +213,10 @@ pub fn syncback_loop(
         let syncback = match middleware.syncback(&snapshot) {
             Ok(syncback) => syncback,
             Err(err) if middleware == Middleware::Dir => {
-                let new_middleware = if force_json {
-                    Middleware::JsonModel
-                } else {
-                    match env::var(DEBUG_MODEL_FORMAT_VAR) {
-                        Ok(value) if value == "1" => Middleware::Rbxmx,
-                        Ok(value) if value == "2" => Middleware::JsonModel,
-                        _ => Middleware::Rbxm,
-                    }
+                let new_middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
+                    Ok(value) if value == "1" => Middleware::Rbxmx,
+                    Ok(value) if value == "2" => Middleware::JsonModel,
+                    _ => Middleware::Rbxm,
                 };
                 let file_name = snapshot
                     .path
@@ -301,12 +295,13 @@ pub struct SyncbackReturn<'sync> {
     pub removed_children: Vec<InstanceWithMeta<'sync>>,
 }
 
-pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Middleware {
+pub fn get_best_middleware(snapshot: &SyncbackSnapshot) -> Middleware {
     // At some point, we're better off using an O(1) method for checking
     // equality for classes like this.
     static JSON_MODEL_CLASSES: OnceLock<HashSet<&str>> = OnceLock::new();
     let json_model_classes = JSON_MODEL_CLASSES.get_or_init(|| {
         [
+            "Actor",
             "Sound",
             "SoundGroup",
             "Sky",
@@ -339,8 +334,6 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Mid
         return override_middleware;
     } else if let Some(old_middleware) = old_middleware {
         return old_middleware;
-    } else if json_model_classes.contains(inst.class.as_str()) {
-        middleware = Middleware::JsonModel;
     } else {
         middleware = match inst.class.as_str() {
             "Folder" | "Configuration" | "Tool" => Middleware::Dir,
@@ -351,6 +344,7 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Mid
             "LocalizationTable" => Middleware::Csv,
             // This isn't the ideal way to handle this but it works.
             name if name.ends_with("Value") => Middleware::JsonModel,
+            _ if json_model_classes.contains(inst.class.as_str()) => Middleware::JsonModel,
             _ => Middleware::Rbxm,
         }
     }
@@ -366,19 +360,33 @@ pub fn get_best_middleware(snapshot: &SyncbackSnapshot, force_json: bool) -> Mid
         }
     }
 
+    // If the name is invalid but the instance has no descendants and isn't a
+    // folder/config/tool, prefer slugified files over creating a directory.
+    // Only promote to directory when there are children (or dir-like classes).
+    if crate::syncback::file_names::validate_file_name(&inst.name).is_err() {
+        middleware = match middleware {
+            Middleware::ServerScript | Middleware::ClientScript | Middleware::ModuleScript
+                if inst.children().is_empty() =>
+            {
+                middleware
+            }
+            Middleware::JsonModel | Middleware::Text if inst.children().is_empty() => middleware,
+            _ => match middleware {
+                Middleware::ServerScript => Middleware::ServerScriptDir,
+                Middleware::ClientScript => Middleware::ClientScriptDir,
+                Middleware::ModuleScript => Middleware::ModuleScriptDir,
+                Middleware::Csv => Middleware::CsvDir,
+                Middleware::JsonModel | Middleware::Text => Middleware::Dir,
+                _ => middleware,
+            },
+        }
+    }
+
     if middleware == Middleware::Rbxm {
-        middleware = if force_json {
-            if !inst.children().is_empty() {
-                Middleware::Dir
-            } else {
-                Middleware::JsonModel
-            }
-        } else {
-            match env::var(DEBUG_MODEL_FORMAT_VAR) {
-                Ok(value) if value == "1" => Middleware::Rbxmx,
-                Ok(value) if value == "2" => Middleware::JsonModel,
-                _ => Middleware::Rbxm,
-            }
+        middleware = match env::var(DEBUG_MODEL_FORMAT_VAR) {
+            Ok(value) if value == "1" => Middleware::Rbxmx,
+            Ok(value) if value == "2" => Middleware::JsonModel,
+            _ => Middleware::Rbxm,
         }
     }
 

src/syncback/snapshot.rs

@@ -20,7 +20,6 @@ pub struct SyncbackData<'sync> {
     pub(super) old_tree: &'sync RojoTree,
     pub(super) new_tree: &'sync WeakDom,
     pub(super) project: &'sync Project,
-    pub(super) force_json: bool,
 }
 
 pub struct SyncbackSnapshot<'sync> {
@@ -44,7 +43,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
             path: PathBuf::new(),
             middleware: None,
         };
-        let middleware = get_best_middleware(&snapshot, self.data.force_json);
+        let middleware = get_best_middleware(&snapshot);
         let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
         snapshot.path = self.path.join(name.as_ref());
 
@@ -70,7 +69,7 @@ impl<'sync> SyncbackSnapshot<'sync> {
             path: PathBuf::new(),
             middleware: None,
         };
-        let middleware = get_best_middleware(&snapshot, self.data.force_json);
+        let middleware = get_best_middleware(&snapshot);
         let name = name_for_inst(middleware, snapshot.new_inst(), snapshot.old_inst())?;
         snapshot.path = base_path.join(name.as_ref());
 

src/web/api.rs

@@ -1,7 +1,13 @@
 //! Defines Rojo's HTTP API, all under /api. These endpoints generally return
 //! JSON.
 
-use std::{collections::HashMap, fs, path::PathBuf, str::FromStr, sync::Arc};
+use std::{
+    collections::{HashMap, HashSet},
+    fs,
+    path::PathBuf,
+    str::FromStr,
+    sync::Arc,
+};
 
 use futures::{sink::SinkExt, stream::StreamExt};
 use hyper::{body, Body, Method, Request, Response, StatusCode};
@@ -24,10 +30,7 @@ use crate::{
         },
         util::{json, json_ok},
     },
-    web_api::{
-        BufferEncode, InstanceUpdate, RefPatchRequest, RefPatchResponse, SerializeRequest,
-        SerializeResponse,
-    },
+    web_api::{BufferEncode, InstanceUpdate, RefPatchResponse, SerializeResponse},
 };
 
 pub async fn call(serve_session: Arc<ServeSession>, mut request: Request<Body>) -> Response<Body> {
@@ -50,8 +53,12 @@ pub async fn call(serve_session: Arc<ServeSession>, mut request: Request<Body>)
                 )
             }
         }
-        (&Method::POST, "/api/serialize") => service.handle_api_serialize(request).await,
-        (&Method::POST, "/api/ref-patch") => service.handle_api_ref_patch(request).await,
+        (&Method::GET, path) if path.starts_with("/api/serialize/") => {
+            service.handle_api_serialize(request).await
+        }
+        (&Method::GET, path) if path.starts_with("/api/ref-patch/") => {
+            service.handle_api_ref_patch(request).await
+        }
 
         (&Method::POST, path) if path.starts_with("/api/open/") => {
             service.handle_api_open(request).await
@@ -222,30 +229,22 @@ impl ApiService {
     /// that correspond to the requested Instances. These values have their
     /// `Value` property set to point to the requested Instance.
     async fn handle_api_serialize(&self, request: Request<Body>) -> Response<Body> {
-        let session_id = self.serve_session.session_id();
-        let body = body::to_bytes(request.into_body()).await.unwrap();
+        let argument = &request.uri().path()["/api/serialize/".len()..];
+        let requested_ids: Result<Vec<Ref>, _> = argument.split(',').map(Ref::from_str).collect();
 
-        let request: SerializeRequest = match json::from_slice(&body) {
-            Ok(request) => request,
-            Err(err) => {
+        let requested_ids = match requested_ids {
+            Ok(ids) => ids,
+            Err(_) => {
                 return json(
-                    ErrorResponse::bad_request(format!("Invalid body: {}", err)),
+                    ErrorResponse::bad_request("Malformed ID list"),
                     StatusCode::BAD_REQUEST,
                 );
             }
         };
-
-        if request.session_id != session_id {
-            return json(
-                ErrorResponse::bad_request("Wrong session ID"),
-                StatusCode::BAD_REQUEST,
-            );
-        }
-
         let mut response_dom = WeakDom::new(InstanceBuilder::new("Folder"));
 
         let tree = self.serve_session.tree();
-        for id in &request.ids {
+        for id in &requested_ids {
             if let Some(instance) = tree.get_instance(*id) {
                 let clone = response_dom.insert(
                     Ref::none(),
@@ -291,26 +290,20 @@ impl ApiService {
     /// and referent properties need to be updated after the serialize
     /// endpoint is used.
     async fn handle_api_ref_patch(self, request: Request<Body>) -> Response<Body> {
-        let session_id = self.serve_session.session_id();
-        let body = body::to_bytes(request.into_body()).await.unwrap();
+        let argument = &request.uri().path()["/api/ref-patch/".len()..];
+        let requested_ids: Result<HashSet<Ref>, _> =
+            argument.split(',').map(Ref::from_str).collect();
 
-        let request: RefPatchRequest = match json::from_slice(&body) {
-            Ok(request) => request,
-            Err(err) => {
+        let requested_ids = match requested_ids {
+            Ok(ids) => ids,
+            Err(_) => {
                 return json(
-                    ErrorResponse::bad_request(format!("Invalid body: {}", err)),
+                    ErrorResponse::bad_request("Malformed ID list"),
                     StatusCode::BAD_REQUEST,
                 );
             }
         };
 
-        if request.session_id != session_id {
-            return json(
-                ErrorResponse::bad_request("Wrong session ID"),
-                StatusCode::BAD_REQUEST,
-            );
-        }
-
         let mut instance_updates: HashMap<Ref, InstanceUpdate> = HashMap::new();
 
         let tree = self.serve_session.tree();
@@ -319,7 +312,7 @@ impl ApiService {
                 let Variant::Ref(prop_value) = prop_value else {
                     continue;
                 };
-                if let Some(target_id) = request.ids.get(prop_value) {
+                if let Some(target_id) = requested_ids.get(prop_value) {
                     let instance_id = instance.id();
                     let update =
                         instance_updates

src/web/interface.rs

@@ -238,13 +238,6 @@ pub struct OpenResponse {
     pub session_id: SessionId,
 }
 
-#[derive(Debug, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct SerializeRequest {
-    pub session_id: SessionId,
-    pub ids: Vec<Ref>,
-}
-
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SerializeResponse {
@@ -276,13 +269,6 @@ impl BufferEncode {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub struct RefPatchRequest {
-    pub session_id: SessionId,
-    pub ids: HashSet<Ref>,
-}
-
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RefPatchResponse<'a> {

tests/rojo_test/serve_util.rs

@@ -1,4 +1,5 @@
 use std::{
+    fmt::Write as _,
     fs,
     path::{Path, PathBuf},
     process::Command,
@@ -12,12 +13,8 @@ use rbx_dom_weak::types::Ref;
 
 use tempfile::{tempdir, TempDir};
 
-use librojo::{
-    web_api::{
-        ReadResponse, SerializeRequest, SerializeResponse, ServerInfoResponse, SocketPacket,
-        SocketPacketType,
-    },
-    SessionId,
+use librojo::web_api::{
+    ReadResponse, SerializeResponse, ServerInfoResponse, SocketPacket, SocketPacketType,
 };
 use rojo_insta_ext::RedactionMap;
 
@@ -229,19 +226,16 @@ impl TestServeSession {
         }
     }
 
-    pub fn get_api_serialize(
-        &self,
-        ids: &[Ref],
-        session_id: SessionId,
-    ) -> Result<SerializeResponse, reqwest::Error> {
-        let client = reqwest::blocking::Client::new();
-        let url = format!("http://localhost:{}/api/serialize", self.port);
-        let body = serde_json::to_string(&SerializeRequest {
-            session_id,
-            ids: ids.to_vec(),
-        });
+    pub fn get_api_serialize(&self, ids: &[Ref]) -> Result<SerializeResponse, reqwest::Error> {
+        let mut id_list = String::with_capacity(ids.len() * 33);
+        for id in ids {
+            write!(id_list, "{id},").unwrap();
+        }
+        id_list.pop();
 
-        client.post(url).body((body).unwrap()).send()?.json()
+        let url = format!("http://localhost:{}/api/serialize/{}", self.port, id_list);
+
+        reqwest::blocking::get(url)?.json()
     }
 }
 

tests/tests/build.rs

@@ -41,7 +41,6 @@ gen_build_tests! {
     issue_546,
     json_as_lua,
     json_model_in_folder,
-    json_model_legacy_name,
     module_in_folder,
     module_init,
     nested_runcontext,
@@ -55,6 +54,8 @@ gen_build_tests! {
     script_meta_disabled,
     server_in_folder,
     server_init,
+    slugified_name_roundtrip,
+    model_json_name_input,
     txt,
     txt_in_folder,
     unresolved_values,

tests/tests/serve.rs

@@ -646,7 +646,7 @@ fn meshpart_with_id() {
             .unwrap();
 
         let serialize_response = session
-            .get_api_serialize(&[*meshpart, *objectvalue], info.session_id)
+            .get_api_serialize(&[*meshpart, *objectvalue])
             .unwrap();
 
         // We don't assert a snapshot on the SerializeResponse because the model includes the
@@ -673,9 +673,7 @@ fn forced_parent() {
             read_response.intern_and_redact(&mut redactions, root_id)
         );
 
-        let serialize_response = session
-            .get_api_serialize(&[root_id], info.session_id)
-            .unwrap();
+        let serialize_response = session.get_api_serialize(&[root_id]).unwrap();
 
         assert_eq!(serialize_response.session_id, info.session_id);
 

tests/tests/syncback.rs

@@ -86,4 +86,9 @@ syncback_tests! {
     sync_rules => ["src/module.modulescript", "src/text.text"],
     // Ensures that the `syncUnscriptable` setting works
     unscriptable_properties => ["default.project.json"],
+    // Ensures that instances with names containing illegal characters get slugified filenames
+    // and preserve their original names in meta.json without forcing directories for leaf scripts
+    slugified_name => ["src/_Script.meta.json", "src/_Script.server.luau", "src/_Folder/init.meta.json"],
+    // Ensures that .model.json files preserve the name property
+    model_json_name => ["src/foo.model.json"],
 }