fix(mv): resolve self-reference removal when source path shape mismatches

src/core/mv.rs

@@ -11,8 +11,8 @@ use super::{
     model::{LinkReplacement, MoveChange, MoveChangeKind, MovePreview, MoveTransaction},
     progress::ProgressReporter,
     util::{
-        collect_markdown_files, is_external_url, relative_path, strip_utf8_bom_prefix,
-        url_decode_link,
+        collect_markdown_files, is_external_url, relative_path, resolve_path,
+        strip_utf8_bom_prefix, url_decode_link,
     },
 };
 use crate::{LinkType, MdrefError, Reference, Result, core::pathdiff::diff_paths, find_links};
@@ -29,6 +29,37 @@ enum RegularFileMoveMethod {
 
 // LinkReplacement and MoveTransaction are now defined in the model module
 
+/// Resolve a path to a canonical form suitable for **semantic comparison**
+/// between `ReplacementPlan` keys.
+///
+/// Paths enter the plan from several sources — user-supplied `source`/`dest`
+/// arguments, `WalkBuilder` output via `collect_markdown_files`, and
+/// `validate_move_paths` results — each with a different textual shape for the
+/// same filesystem entry (`examples/main.md` vs `./examples/main.md` vs a fully
+/// canonicalised absolute path). Direct `PathBuf` equality misses these, which
+/// is how #6 sneaked in.
+///
+/// We deliberately **do not** rewrite the plan's keys with the canonical form,
+/// because those keys are surfaced downstream (e.g. as `MoveChange.path` in the
+/// dry-run JSON) and should preserve the caller-supplied shape. Use this helper
+/// only for transient comparisons via [`find_matching_plan_key`].
+fn canonical_plan_key(path: &Path) -> PathBuf {
+    path.canonicalize()
+        .or_else(|_| resolve_path(path))
+        .unwrap_or_else(|_| path.to_path_buf())
+}
+
+/// Find the existing `ReplacementPlan` key that refers to the same filesystem
+/// entry as `target`, comparing via [`canonical_plan_key`]. Returns a cloned
+/// `PathBuf` so callers can pass it to `HashMap::remove` / `contains_key`
+/// without fighting the borrow checker.
+fn find_matching_plan_key(plan: &ReplacementPlan, target: &Path) -> Option<PathBuf> {
+    let target_key = canonical_plan_key(target);
+    plan.keys()
+        .find(|key| canonical_plan_key(key) == target_key)
+        .cloned()
+}
+
 /// Execute a fallible closure within a transaction context.
 /// If the closure returns an error, the transaction is rolled back automatically.
 fn execute_with_rollback<F>(transaction: &MoveTransaction, operation: F) -> Result<()>
@@ -503,12 +534,16 @@ fn move_source_replacements_to_destination(
     source: &Path,
     destination: &Path,
 ) {
-    if let Some(source_replacements) = replacements_by_file.remove(source) {
-        let destination_replacements = replacements_by_file
-            .entry(destination.to_path_buf())
-            .or_default();
-        extend_unique_replacements(destination_replacements, source_replacements);
-    }
+    let Some(existing_source_key) = find_matching_plan_key(replacements_by_file, source) else {
+        return;
+    };
+    let Some(source_replacements) = replacements_by_file.remove(&existing_source_key) else {
+        return;
+    };
+    let destination_key = find_matching_plan_key(replacements_by_file, destination)
+        .unwrap_or_else(|| destination.to_path_buf());
+    let destination_replacements = replacements_by_file.entry(destination_key).or_default();
+    extend_unique_replacements(destination_replacements, source_replacements);
 }
 
 fn add_destination_replacements(
@@ -520,9 +555,9 @@ fn add_destination_replacements(
         return;
     }
 
-    let destination_replacements = replacements_by_file
-        .entry(destination.to_path_buf())
-        .or_default();
+    let destination_key = find_matching_plan_key(replacements_by_file, destination)
+        .unwrap_or_else(|| destination.to_path_buf());
+    let destination_replacements = replacements_by_file.entry(destination_key).or_default();
     extend_unique_replacements(destination_replacements, replacements);
 }
 
@@ -625,7 +660,9 @@ fn preview_regular_file_move(
     progress.set_message("Scanning references...");
     let references = find_references(source, root, progress)?;
     let mut replacements_by_file = plan_external_replacements(&references, &resolved_dest)?;
-    replacements_by_file.remove(source);
+    if let Some(source_key) = find_matching_plan_key(&replacements_by_file, source) {
+        replacements_by_file.remove(&source_key);
+    }
     let internal_replacements = plan_internal_replacements(source, source, &resolved_dest)?;
     add_destination_replacements(
         &mut replacements_by_file,
@@ -718,7 +755,9 @@ fn mv_regular_file(
     progress.set_message("Scanning references...");
     let references = find_references(source, root, progress)?;
     let mut replacements_by_file = plan_external_replacements(&references, &resolved_dest)?;
-    replacements_by_file.remove(source);
+    if let Some(source_key) = find_matching_plan_key(&replacements_by_file, source) {
+        replacements_by_file.remove(&source_key);
+    }
     let internal_replacements = plan_internal_replacements(source, source, &resolved_dest)?;
 
     if dry_run {
@@ -740,7 +779,9 @@ fn mv_regular_file(
         transaction.snapshot_file(file_path)?;
     }
 
-    if !internal_replacements.is_empty() && !replacements_by_file.contains_key(source) {
+    if !internal_replacements.is_empty()
+        && find_matching_plan_key(&replacements_by_file, source).is_none()
+    {
         transaction.snapshot_file(source)?;
     }
 
@@ -821,12 +862,13 @@ fn mv_case_only_file(
     }
 
     let mut transaction = MoveTransaction::new(source.to_path_buf(), resolved_dest.to_path_buf());
-    if replacements_by_file.contains_key(resolved_dest) {
+    let dest_key_in_plan = find_matching_plan_key(&replacements_by_file, resolved_dest);
+    if dest_key_in_plan.is_some() {
         transaction.snapshot_file(source)?;
     }
     for file_path in replacements_by_file
         .keys()
-        .filter(|path| *path != resolved_dest)
+        .filter(|path| Some(*path) != dest_key_in_plan.as_ref())
     {
         transaction.snapshot_file(file_path)?;
     }

tests/lib_mv_tests.rs

@@ -2785,3 +2785,74 @@ fn test_mv_directory_dry_run_with_images() {
         "Dry-run should not modify references"
     );
 }
+
+// ============= Regression tests for #6: relative source + self-reference =============
+
+/// Regression test for #6.
+///
+/// When the user invokes `mv` from a shell cwd with a **relative** source path
+/// (no leading `./`) and `root="."`, the self-reference inside the source file
+/// must still be recognised and rewritten — previously the `HashMap` key
+/// comparison missed the self-reference entry because `WalkBuilder` produced
+/// paths with a `./` prefix while the user supplied a bare relative path,
+/// causing `apply_replacements` to read the stale pre-move path and fail.
+#[test]
+#[allow(clippy::unwrap_used)]
+fn test_mv_relative_source_with_self_reference_succeeds() {
+    let temp_dir = TempDir::new().unwrap();
+    write_file(temp_dir.path().join("page.md"), "[Self](page.md)\n");
+
+    with_current_dir(temp_dir.path(), || {
+        let result = mv("page.md", "./moved.md", ".", false, &NoopProgress);
+        assert!(
+            result.is_ok(),
+            "mv with bare relative source should succeed, got: {:?}",
+            result.err()
+        );
+    });
+
+    let source = temp_dir.path().join("page.md");
+    let target = temp_dir.path().join("moved.md");
+    assert!(!source.exists(), "source should have been removed");
+    assert!(target.exists(), "target should have been created");
+
+    let content = fs::read_to_string(&target).unwrap();
+    assert!(
+        content.contains("moved.md"),
+        "self-reference should be rewritten to new filename, got: {content}"
+    );
+    assert!(
+        !content.contains("page.md"),
+        "old self-reference must not survive, got: {content}"
+    );
+}
+
+/// Regression test for #6.
+///
+/// A cross-reference from another file must still be rewritten when the user
+/// supplies `source` as a bare relative path and `root="."`. Before the fix
+/// the external-reference path happened to work (the `WalkBuilder`-produced
+/// path was consistently used on both sides), but we lock in the end-to-end
+/// guarantee here so future refactors can't regress it.
+#[test]
+#[allow(clippy::unwrap_used)]
+fn test_mv_relative_source_rewrites_external_reference() {
+    let temp_dir = TempDir::new().unwrap();
+    write_file(temp_dir.path().join("page.md"), "# Page\n");
+    write_file(temp_dir.path().join("ref.md"), "[Link](page.md)\n");
+
+    with_current_dir(temp_dir.path(), || {
+        let result = mv("page.md", "./moved.md", ".", false, &NoopProgress);
+        assert!(result.is_ok(), "mv should succeed, got: {:?}", result.err());
+    });
+
+    let ref_content = fs::read_to_string(temp_dir.path().join("ref.md")).unwrap();
+    assert!(
+        ref_content.contains("moved.md"),
+        "external reference should point to the new filename, got: {ref_content}"
+    );
+    assert!(
+        !ref_content.contains("page.md"),
+        "old external reference must not survive, got: {ref_content}"
+    );
+}