diff --git a/app/commands/move/move_apply.py b/app/commands/move/move_apply.py index c676db6..d3fb445 100644 --- a/app/commands/move/move_apply.py +++ b/app/commands/move/move_apply.py @@ -48,27 +48,18 @@ def apply_file_move( Path(dest_abs).parent.mkdir(parents=True, exist_ok=True) written_files: dict[str, str] = {} - - # 1. Snapshot all original file contents explicitly BEFORE the filesystem mutates - # We do NOT snapshot source_abs into written_files because the rollback - # strategy is handled separately by `_rollback_move_target`. We only snapshot - # files whose contents we are going to modify in place. + try: for filepath in importer_changes: if filepath in new_contents and Path(filepath).exists(): written_files[filepath] = Path(filepath).read_text() - - # 2. Execute the filesystem move + shutil.move(source_abs, dest_abs) - # 3. Apply memory modifications to the new destination if dest_abs in new_contents: - # We don't snapshot dest_abs before this write because if a rollback happens, - # `dest_abs` ceases to exist (it gets moved back to source_abs by `_rollback_move_target`). - # In fact, writing to dest_abs during rollback would recreate an orphaned file. + written_files[dest_abs] = Path(dest_abs).read_text() safe_write_text(dest_abs, new_contents[dest_abs]) - # 4. Apply rest of imports for filepath in importer_changes: if filepath in new_contents: safe_write_text(filepath, new_contents[filepath]) @@ -93,7 +84,6 @@ def apply_directory_move( written_files: dict[str, str] = {} try: - # Snapshot content files prior to mutating for filepath, replacements in external_changes.items(): if Path(filepath).exists(): written_files[filepath] = Path(filepath).read_text() @@ -109,6 +99,7 @@ def apply_directory_move( content = original for old_str, new_str in changes: content = content.replace(old_str, new_str) + written_files[str(dest_file)] = original safe_write_text(dest_file, content) for filepath, replacements in external_changes.items(): diff --git a/tests/commands/test_transitive_engine.py b/tests/commands/test_transitive_engine.py index 6658760..a453ecc 100644 --- a/tests/commands/test_transitive_engine.py +++ b/tests/commands/test_transitive_engine.py @@ -768,6 +768,36 @@ def test_file_move_rollback_on_write_error(self, tmp_path): [], ) + def test_file_move_rollback_restores_self_changes(self, tmp_path, monkeypatch): + """Rollback restores the original source contents after self changes.""" + src = tmp_path / "a.py" + dest = tmp_path / "b.py" + importer = tmp_path / "user.py" + src.write_text("from old import thing\n") + importer.write_text("from a import thing\n") + + original_safe_write_text = move_apply_mod.safe_write_text + + def flaky_safe_write_text(filepath: str | Path, content: str) -> None: + if str(filepath) == str(importer): + raise OSError("boom") + original_safe_write_text(filepath, content) + + monkeypatch.setattr(move_apply_mod, "safe_write_text", flaky_safe_write_text) + + with pytest.raises(OSError, match="boom"): + move_apply_mod.apply_file_move( + str(src), + str(dest), + {str(importer): [("from a import thing", "from b import thing")]}, + [("from old import thing", "from new import thing")], + ) + + assert src.exists() + assert not dest.exists() + assert src.read_text() == "from old import thing\n" + assert importer.read_text() == "from a import thing\n" + class TestApplyDirectoryMove: def test_directory_move_basic(self, tmp_path): @@ -800,6 +830,40 @@ def test_directory_move_with_internal_changes(self, tmp_path): ) assert (dest / "a.py").read_text() == "from new_pkg.b import f" + def test_directory_move_rollback_restores_internal_changes( + self, tmp_path, monkeypatch + ): + """Rollback restores original moved-directory contents after internal rewrites.""" + src = tmp_path / "pkg" + src.mkdir() + (src / "a.py").write_text("from pkg.b import f\n") + consumer = tmp_path / "consumer.py" + consumer.write_text("from pkg.a import f\n") + + dest = tmp_path / "new_pkg" + original_safe_write_text = move_apply_mod.safe_write_text + + def flaky_safe_write_text(filepath: str | Path, content: str) -> None: + if str(filepath) == str(consumer): + raise OSError("boom") + original_safe_write_text(filepath, content) + + monkeypatch.setattr(move_apply_mod, "safe_write_text", flaky_safe_write_text) + + with pytest.raises(OSError, match="boom"): + move_apply_mod.apply_directory_move( + str(src), + str(dest), + src, + {str(consumer): [("from pkg.a import f", "from new_pkg.a import f")]}, + {str(src / "a.py"): [("from pkg.b import f", "from new_pkg.b import f")]}, + ) + + assert src.exists() + assert not dest.exists() + assert (src / "a.py").read_text() == "from pkg.b import f\n" + assert consumer.read_text() == "from pkg.a import f\n" + # ===================================================================== # Module 5: shared_phases.py