From 752be0d56fb930108639f16dbed312c3f48ea7d3 Mon Sep 17 00:00:00 2001 From: warsam Date: Tue, 27 Jan 2026 16:03:44 -0800 Subject: [PATCH 1/3] Fix server crash when deleting folders Handle the case where a folder's parent path no longer exists when processing VFS removal events. This can happen when a folder and its contents are deleted together, causing a race between removal events. Added a regression test for this scenario. --- ...nd__tests__serve__remove_folder_all-2.snap | 16 ++++++++ ..._end__tests__serve__remove_folder_all.snap | 38 +++++++++++++++++++ ...end__tests__serve__remove_folder_info.snap | 13 +++++++ ...tests__serve__remove_folder_subscribe.snap | 13 +++++++ .../remove_folder/default.project.json | 6 +++ .../remove_folder/src/my-folder/test.txt | 1 + src/change_processor.rs | 19 ++++++++-- tests/tests/serve.rs | 35 +++++++++++++++++ 8 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap create mode 100644 rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap create mode 100644 rojo-test/serve-tests/remove_folder/default.project.json create mode 100644 rojo-test/serve-tests/remove_folder/src/my-folder/test.txt diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap new file mode 100644 index 00000000..f5203d43 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all-2.snap @@ -0,0 +1,16 @@ +--- +source: tests/tests/serve.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: [] + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: remove_folder + Parent: "00000000000000000000000000000000" + Properties: {} +messageCursor: 1 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap new file mode 100644 index 00000000..ca429826 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap @@ -0,0 +1,38 @@ +--- +source: tests/tests/serve.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Children: + - id-3 + ClassName: Folder + Id: id-2 + Metadata: + ignoreUnknownInstances: false + Name: remove_folder + Parent: "00000000000000000000000000000000" + Properties: {} + id-3: + Children: + - id-4 + ClassName: Folder + Id: id-3 + Metadata: + ignoreUnknownInstances: false + Name: my-folder + Parent: id-2 + Properties: {} + id-4: + Children: [] + ClassName: StringValue + Id: id-4 + Metadata: + ignoreUnknownInstances: false + Name: test + Parent: id-3 + Properties: + Value: + String: "This file is inside my-folder\r\n" +messageCursor: 0 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap new file mode 100644 index 00000000..6796e4ec --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_info.snap @@ -0,0 +1,13 @@ +--- +source: tests/tests/serve.rs +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +gameId: ~ +placeId: ~ +projectName: remove_folder +protocolVersion: 5 +rootInstanceId: id-2 +serverVersion: "[server-version]" +sessionId: id-1 +unexpectedPlaceIds: ~ diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap new file mode 100644 index 00000000..e9e39e04 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_subscribe.snap @@ -0,0 +1,13 @@ +--- +source: tests/tests/serve.rs +expression: "socket_packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 1 + messages: + - added: {} + removed: + - id-3 + updated: [] +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-tests/remove_folder/default.project.json b/rojo-test/serve-tests/remove_folder/default.project.json new file mode 100644 index 00000000..28d0bc72 --- /dev/null +++ b/rojo-test/serve-tests/remove_folder/default.project.json @@ -0,0 +1,6 @@ +{ + "name": "remove_folder", + "tree": { + "$path": "src" + } +} diff --git a/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt b/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt new file mode 100644 index 00000000..44c07d0f --- /dev/null +++ b/rojo-test/serve-tests/remove_folder/src/my-folder/test.txt @@ -0,0 +1 @@ +This file is inside my-folder diff --git a/src/change_processor.rs b/src/change_processor.rs index 6c5c8224..78ca2858 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -172,12 +172,23 @@ impl JobThreadContext { self.apply_patches(self.vfs.canonicalize(&path).unwrap()) } VfsEvent::Remove(path) => { - // MemoFS does not track parent removals yet, so we can canonicalize - // the parent path safely and then append the removed path's file name. + // Try to canonicalize the parent path and append the removed path's + // file name. If the parent no longer exists (e.g., a folder and its + // contents were deleted together), we skip processing since the + // parent's removal event will handle cleaning up the instances. let parent = path.parent().unwrap(); let file_name = path.file_name().unwrap(); - let parent_normalized = self.vfs.canonicalize(parent).unwrap(); - self.apply_patches(parent_normalized.join(file_name)) + match self.vfs.canonicalize(parent) { + Ok(parent_normalized) => self.apply_patches(parent_normalized.join(file_name)), + Err(err) => { + log::debug!( + "Parent path no longer exists for removed path {}: {}", + path.display(), + err + ); + Vec::new() + } + } } _ => { log::warn!("Unhandled VFS event: {:?}", event); diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 748f1687..d41ee72e 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -126,6 +126,41 @@ fn remove_file() { }); } +/// Test that removing a folder (directory) with contents doesn't crash the server. +/// This is a regression test for a bug where deleting a folder would cause a panic +/// due to the parent path no longer existing when processing child removal events. +#[test] +fn remove_folder() { + run_serve_test("remove_folder", |session, mut redactions| { + let info = session.get_api_rojo().unwrap(); + let root_id = info.root_instance_id; + + assert_yaml_snapshot!("remove_folder_info", redactions.redacted_yaml(info)); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "remove_folder_all", + read_response.intern_and_redact(&mut redactions, root_id) + ); + + fs::remove_dir_all(session.path().join("src/my-folder")).unwrap(); + + let socket_packet = session + .get_api_socket_packet(SocketPacketType::Messages, 0) + .unwrap(); + assert_yaml_snapshot!( + "remove_folder_subscribe", + socket_packet.intern_and_redact(&mut redactions, ()) + ); + + let read_response = session.get_api_read(root_id).unwrap(); + assert_yaml_snapshot!( + "remove_folder_all-2", + read_response.intern_and_redact(&mut redactions, root_id) + ); + }); +} + #[test] fn edit_init() { run_serve_test("edit_init", |session, mut redactions| { From c89bb1da749c2f962902492ab93bd97ab861e773 Mon Sep 17 00:00:00 2001 From: warsam Date: Fri, 13 Feb 2026 10:16:49 -0800 Subject: [PATCH 2/3] Fix cross-platform snapshot failure in remove_folder test Force LF line endings for test fixtures via .gitattributes and update snapshot to match. --- .gitattributes | 3 ++- .../end_to_end__tests__serve__remove_folder_all.snap | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitattributes b/.gitattributes index ba715681..dd2c4ca0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ -*.lua linguist-language=Luau \ No newline at end of file +*.lua linguist-language=Luau +rojo-test/**/*.txt text eol=lf \ No newline at end of file diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap index ca429826..d861c560 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__remove_folder_all.snap @@ -33,6 +33,6 @@ instances: Parent: id-3 Properties: Value: - String: "This file is inside my-folder\r\n" + String: "This file is inside my-folder\n" messageCursor: 0 sessionId: id-1 From 974693412f096eaa7bd4a6d1ba6a7ae78499db8a Mon Sep 17 00:00:00 2001 From: warsam Date: Fri, 13 Feb 2026 15:34:12 -0800 Subject: [PATCH 3/3] Add CHANGELOG.md entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a246ebf6..3c5d95cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,12 +36,14 @@ Making a new release? Simply add the new header with the version and date undern * Fixed a bug where MacOS paths weren't being handled correctly. ([#1201]) * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211]) * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215]) +* Fixed server crash when deleting folders ([#1220]) [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 [#1192]: https://github.com/rojo-rbx/rojo/pull/1192 [#1201]: https://github.com/rojo-rbx/rojo/pull/1201 [#1211]: https://github.com/rojo-rbx/rojo/pull/1211 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215 +[#1220]: https://github.com/rojo-rbx/rojo/pull/1220 ## [7.7.0-rc.1] (November 27th, 2025)