From 5b2ea2a23fd9c2fae7dd067638be74ad1faa9078 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 5 Feb 2026 21:19:18 -0600 Subject: [PATCH] Refactor file path handling in JobThreadContext to support combined rename and ClassName changes - Updated the logic to use the effective file path when handling script-to-script transitions, ensuring that the correct path is used even after a rename operation. - Enhanced the test suite with a new test case for combined rename and ClassName changes, verifying that the extension change is applied correctly after a file rename. - Improved comments for clarity on the renaming process and its implications for file handling during updates. --- src/change_processor.rs | 22 +++++++++----- tests/tests/two_way_sync.rs | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/change_processor.rs b/src/change_processor.rs index 7af5dd75..88e727cb 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -654,21 +654,29 @@ impl JobThreadContext { "ModuleScript" | "Script" | "LocalScript" ); - if old_is_script && new_is_script && path.exists() { + // If a rename handler already moved the file, + // use the new path instead of the stale + // instigating_source path. + let effective_path = overridden_source_path + .as_deref() + .unwrap_or(path.as_path()); + + if old_is_script && new_is_script && effective_path.exists() { // Script-to-script transition: rename the file extension. // For directory-format scripts, the path is the directory // (e.g., src/MyModule/), not the init file inside. We must // find and rename the init file, not the directory. // Resolve the actual file to rename. For directories, // find the init file inside; for files, use directly. - let init_result = if path.is_dir() { - Self::find_init_file(path) - .map(|f| (f.clone(), path.to_path_buf())) + let init_result = if effective_path.is_dir() { + Self::find_init_file(effective_path) + .map(|f| (f.clone(), effective_path.to_path_buf())) } else { Some(( - path.clone(), - path.parent() - .unwrap_or(path.as_path()) + effective_path.to_path_buf(), + effective_path + .parent() + .unwrap_or(effective_path) .to_path_buf(), )) }; diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index 66728a60..74c6a631 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -932,6 +932,64 @@ fn combined_classname_and_source_update() { }); } +/// Test 19b: Combined rename + ClassName change in a single update. +/// The rename handler moves the file first (existing.luau → Renamed.luau), +/// then the ClassName handler must use the renamed path to apply the extension +/// change (Renamed.luau → Renamed.server.luau). Without the fix, the ClassName +/// handler checks the OLD path which no longer exists, silently skipping the +/// extension rename. +#[test] +fn combined_rename_and_classname_change() { + 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"); + // After rename + ClassName change: should end up as Renamed.server.luau + let final_path = session.path().join("src").join("Renamed.server.luau"); + // Should NOT exist: renamed but without extension change + let renamed_only = session.path().join("src").join("Renamed.luau"); + + assert_file_exists(&old_path, "existing.luau before combined update"); + + let mut props = UstrMap::default(); + props.insert( + ustr("Source"), + Some(Variant::String( + "-- After rename + class change".to_string(), + )), + ); + send_update( + &session, + &session_id, + InstanceUpdate { + id: existing_id, + changed_name: Some("Renamed".to_string()), + changed_class_name: Some(ustr("Script")), + changed_properties: props, + changed_metadata: None, + }, + ); + + assert_not_exists(&old_path, "Original file after combined rename+classname"); + assert_not_exists( + &renamed_only, + "Renamed.luau should not exist — ClassName handler should have \ + applied the .server extension", + ); + assert_file_exists( + &final_path, + "Renamed.server.luau should exist after combined rename+classname", + ); + + let content = fs::read_to_string(&final_path).unwrap(); + assert!( + content.contains("After rename + class change"), + "Source should be in the final file, got: {}", + content + ); + }); +} + // --------------------------------------------------------------------------- // Tests: Standalone → directory conversion & suffix-aware removal // ---------------------------------------------------------------------------