Make note/resource moves all-or-nothing (GH #230)

crates/b2-cli/src/main.rs

@@ -1671,6 +1671,10 @@ fn user_message(err: &CliError) -> String {
         CliError::Core(b2_core::Error::MoveDestination(_)) => {
             "That move destination isn't valid. Give a vault-relative path like `notes/new-name.md`.".to_string()
         }
+        CliError::Core(b2_core::Error::MoveIncomplete { paths, .. }) => format!(
+            "The move failed, and B2 could not undo its link changes in: {}. Check the links in those files; `b2 reindex` then shows any that no longer resolve.",
+            paths.join(", ")
+        ),
         CliError::Core(b2_core::Error::DirNotFound(p)) => format!(
             "Folder not found: '{p}'. Check the path (folders are vault-relative, like `notes/archive`)."
         ),
@@ -1791,6 +1795,19 @@ mod tests {
         );
     }
 
+    #[test]
+    fn a_move_that_could_not_be_undone_names_the_files_to_check() {
+        let msg = user_message(&CliError::Core(b2_core::Error::MoveIncomplete {
+            paths: vec!["b.md".into(), "notes/c.md".into()],
+            source: Box::new(b2_core::Error::Io(std::io::Error::other("disk full"))),
+        }));
+        assert!(msg.contains("in: b.md, notes/c.md."), "{msg}");
+        assert!(
+            !msg.contains("disk full"),
+            "the cause stays internal: {msg}"
+        );
+    }
+
     fn view(vouched: Option<bool>, n: usize) -> SearchEvidenceView {
         SearchEvidenceView {
             results: (0..n)

crates/b2-core/src/error.rs

@@ -64,6 +64,17 @@ pub enum Error {
     #[error("move target already exists: {0}")]
     MoveTargetExists(String),
 
+    /// A move failed part-way and B2 could not put back every file it had already
+    /// changed (GH #230). Rare: an ordinary failure is rolled back and surfaces as
+    /// its own error. Carries the vault-relative paths still holding the move's
+    /// rewrite, so the user can check those links by hand, and the failure that
+    /// started the undo, so the adapters' internal log keeps the root cause.
+    #[error("move failed and could not be fully undone: {} (cause: {source})", paths.join(", "))]
+    MoveIncomplete {
+        paths: Vec<String>,
+        source: Box<Error>,
+    },
+
     /// `b2 mv` was given a source folder that doesn't exist in the vault — the
     /// directory sibling of [`Error::NoteNotFound`] (a folder is a path prefix,
     /// never an indexed row, so it resolves against the filesystem).

crates/b2-core/tests/mv.rs

@@ -562,3 +562,239 @@ fn note_vectors(conn: &rusqlite::Connection, note_path: &str) -> Vec<Vec<u8>> {
     let rows = stmt.query_map([note_path], |r| r.get(0)).unwrap();
     rows.map(Result::unwrap).collect()
 }
+
+// --- a failed move changes nothing (GH #230) ------------------------------------
+//
+// A move writes the vault in two kinds of step: the inbound files' link text, then the
+// rename. A move that fails must leave every file byte-identical — reindexing cannot
+// repair rewritten link text, because it faithfully projects whatever the Markdown now
+// says. Two routes to a failure: a destination refused up front, and an I/O failure
+// after the rewrites have been written, which must be rolled back.
+
+/// A purpose-built vault under `dir/vault` from `(path, contents)` pairs, reindexed.
+fn small_vault(dir: &Path, files: &[(&str, &str)]) -> (Vault, std::path::PathBuf) {
+    let root = dir.join("vault");
+    for (path, contents) in files {
+        let abs = root.join(path);
+        fs::create_dir_all(abs.parent().unwrap()).unwrap();
+        fs::write(abs, contents).unwrap();
+    }
+    let vault = Vault::open(&root).unwrap();
+    vault.reindex().unwrap();
+    (vault, root)
+}
+
+/// Every file under `root` (outside `.b2/`) with its bytes, sorted by path — the
+/// whole authored vault, so an assertion on it catches a stray write anywhere.
+fn vault_bytes(root: &Path) -> Vec<(String, Vec<u8>)> {
+    fn walk(root: &Path, dir: &Path, out: &mut Vec<(String, Vec<u8>)>) {
+        for entry in fs::read_dir(dir).unwrap() {
+            let path = entry.unwrap().path();
+            let rel = path
+                .strip_prefix(root)
+                .unwrap()
+                .to_string_lossy()
+                .into_owned();
+            if rel == ".b2" {
+                continue;
+            }
+            if path.is_dir() {
+                out.push((format!("{rel}/"), Vec::new()));
+                walk(root, &path, out);
+            } else {
+                out.push((rel, fs::read(&path).unwrap()));
+            }
+        }
+    }
+    let mut out = Vec::new();
+    walk(root, root, &mut out);
+    out.sort();
+    out
+}
+
+#[test]
+fn a_move_under_a_file_is_refused_and_changes_nothing() {
+    // The issue's reproduction: `blocked` is a regular file, so `blocked/a.md` can
+    // never exist. The move must be refused before a single inbound link is touched.
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("a.md", "Target note.\n"),
+            ("b.md", "See [[a]].\n"),
+            ("c.md", "Also [[ a | the target ]] and [[a.md]].\n"),
+            ("blocked", "a plain file\n"),
+        ],
+    );
+    let before = vault_bytes(&root);
+    let backlinks = inbound(&vault, "a.md");
+    assert_eq!(
+        backlinks,
+        vec![
+            ("referenced-by".to_string(), "b.md".to_string()),
+            ("referenced-by".to_string(), "c.md".to_string()),
+            ("referenced-by".to_string(), "c.md".to_string()),
+        ],
+        "one per authored link: c.md links twice"
+    );
+
+    let err = vault.move_note("a.md", "blocked/a.md").unwrap_err();
+    assert!(matches!(err, Error::MoveDestination(_)), "{err:?}");
+
+    assert_eq!(vault_bytes(&root), before, "every file byte-identical");
+    assert_eq!(inbound(&vault, "a.md"), backlinks, "the index is untouched");
+    // And the Markdown still says what it said: a rebuild resolves the same links.
+    vault.reindex().unwrap();
+    assert_eq!(
+        inbound(&vault, "a.md"),
+        backlinks,
+        "the original links resolve"
+    );
+}
+
+#[test]
+fn a_move_deeper_under_a_file_is_refused_too() {
+    // The blocking file may be any ancestor, not just the immediate parent.
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("a.md", "Target note.\n"),
+            ("b.md", "See [[a]].\n"),
+            ("blocked", "a plain file\n"),
+        ],
+    );
+    let before = vault_bytes(&root);
+
+    let err = vault.move_note("a.md", "blocked/deeper/a.md").unwrap_err();
+    assert!(matches!(err, Error::MoveDestination(_)), "{err:?}");
+    assert_eq!(vault_bytes(&root), before);
+}
+
+#[test]
+fn a_move_whose_rename_fails_restores_every_rewritten_file() {
+    // A failure *after* the rewrites: the note vanished from disk (an out-of-band
+    // delete the index hasn't seen), so the destination checks pass, both inbound
+    // files are rewritten, and only then does the rename fail. The rewrites and the
+    // destination folder the move created must all be undone.
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("a.md", "Target note.\n"),
+            ("b.md", "See [[a]].\n"),
+            ("c.md", "Also [[ a | the target ]] and [[a.md]].\n"),
+        ],
+    );
+    let backlinks = inbound(&vault, "a.md");
+    fs::remove_file(root.join("a.md")).unwrap();
+    let before = vault_bytes(&root);
+
+    let err = vault.move_note("a.md", "archive/2026/a.md").unwrap_err();
+    assert!(matches!(err, Error::Io(_)), "{err:?}");
+
+    assert_eq!(
+        vault_bytes(&root),
+        before,
+        "inbound files restored, no archive/ folder left behind"
+    );
+    assert_eq!(inbound(&vault, "a.md"), backlinks, "the index is untouched");
+}
+
+#[test]
+fn a_resource_move_under_a_file_is_refused_and_changes_nothing() {
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("img.png", "png bytes"),
+            ("n.md", "![[img.png]] and ![alt](img.png)\n"),
+            ("blocked", "a plain file\n"),
+        ],
+    );
+    let before = vault_bytes(&root);
+
+    let err = vault
+        .move_resource("img.png", "blocked/img.png")
+        .unwrap_err();
+    assert!(matches!(err, Error::MoveDestination(_)), "{err:?}");
+    assert_eq!(vault_bytes(&root), before);
+}
+
+#[test]
+fn a_resource_move_whose_rename_fails_restores_every_rewritten_note() {
+    // The resource arm of the rollback: both link syntaxes are rewritten, then the
+    // rename fails because the file is gone from disk.
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("img.png", "png bytes"),
+            ("n.md", "![[img.png|caption]] and ![alt]( img.png )\n"),
+        ],
+    );
+    let backlinks = vault.explain_resource("img.png").unwrap().backlinks;
+    assert_eq!(backlinks.len(), 2);
+    fs::remove_file(root.join("img.png")).unwrap();
+    let before = vault_bytes(&root);
+
+    let err = vault.move_resource("img.png", "media/img.png").unwrap_err();
+    assert!(matches!(err, Error::Io(_)), "{err:?}");
+
+    assert_eq!(
+        vault_bytes(&root),
+        before,
+        "n.md restored, no media/ folder"
+    );
+    assert_eq!(
+        vault.explain_resource("img.png").unwrap().backlinks,
+        backlinks,
+        "the index is untouched"
+    );
+}
+
+#[test]
+fn move_dir_under_a_file_is_refused_and_changes_nothing() {
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[
+            ("docs/x.md", "Inside, see [[docs/y]].\n"),
+            ("docs/y.md", "Also inside.\n"),
+            ("hub.md", "See [[docs/x|X]].\n"),
+            ("blocked", "a plain file\n"),
+        ],
+    );
+    let before = vault_bytes(&root);
+
+    let err = vault.move_dir("docs", "blocked/docs").unwrap_err();
+    assert!(matches!(err, Error::MoveDestination(_)), "{err:?}");
+    assert_eq!(vault_bytes(&root), before);
+}
+
+#[test]
+fn re_running_a_move_interrupted_before_its_rename_finishes_it() {
+    // The one window no undo covers is a crash between the rewrites and the rename:
+    // the inbound links already name the destination, the note is still at its source,
+    // and the index hasn't heard. Simulated by writing that state by hand; re-running
+    // the same move must complete it rather than refuse or double-rewrite.
+    let tmp = tempfile::TempDir::new().unwrap();
+    let (vault, root) = small_vault(
+        tmp.path(),
+        &[("a.md", "Target note.\n"), ("b.md", "See [[a|A]].\n")],
+    );
+    fs::write(root.join("b.md"), "See [[archive/a|A]].\n").unwrap();
+
+    let report = vault.move_note("a.md", "archive/a.md").unwrap();
+    assert_eq!(report.links_rewritten, 0, "the rewrite was already done");
+    assert!(root.join("archive/a.md").exists() && !root.join("a.md").exists());
+    assert_eq!(
+        fs::read_to_string(root.join("b.md")).unwrap(),
+        "See [[archive/a|A]].\n"
+    );
+    assert_eq!(
+        inbound(&vault, "archive/a.md"),
+        vec![("referenced-by".to_string(), "b.md".to_string())],
+        "the link resolves at the destination"
+    );
+}

crates/b2-desktop/src/error.rs

@@ -103,6 +103,10 @@ pub fn user_message(err: &CmdError) -> String {
             "That destination isn't valid. Give a vault-relative name like `notes/new-name`."
                 .to_string()
         }
+        CmdError::Core(b2_core::Error::MoveIncomplete { paths, .. }) => format!(
+            "The move failed, and B2 could not undo its link changes in: {}. Check the links in those files, then reindex.",
+            paths.join(", ")
+        ),
         CmdError::Core(b2_core::Error::DirNotFound(p)) => {
             format!("Folder not found: '{p}'. It may have been moved — reindex and try again.")
         }
@@ -283,5 +287,17 @@ mod tests {
 
         let dir = CmdError::Core(b2_core::Error::DirNotFound("old-folder".into()));
         assert!(user_message(&dir).contains("Folder not found: 'old-folder'"));
+
+        let incomplete = CmdError::Core(b2_core::Error::MoveIncomplete {
+            paths: vec!["b.md".into(), "notes/c.md".into()],
+            source: Box::new(b2_core::Error::Io(std::io::Error::other("disk full"))),
+        });
+        let msg = user_message(&incomplete);
+        assert!(msg.contains("in: b.md, notes/c.md."));
+        assert!(!msg.contains("disk full"), "the cause stays internal");
+        assert!(
+            incomplete.to_string().contains("disk full"),
+            "the internal log (`log_internal`) keeps the cause"
+        );
     }
 }

docs/data-model.md

@@ -280,9 +280,10 @@ Every edge projects to one record. This is the shape the [index-engine.md](index
   `[[path]]` is normalized by the resolver (the wikilink `+ ".md"` ladder) and stored as the
   path it named. `dst_path_raw` keeps the text exactly as written. So an edge stores what you
   authored, and the graph carries no identifier the vault does not. A B2-performed move is
-  therefore one transaction: rewrite the inbound `[[path|title]]` text, re-key the moved
-  note's rows, re-project the inbound sources. That is how L1's "rename keeps every backlink
-  resolving" holds. A move made outside B2 is a delete plus a create, and its inbound links
+  therefore one operation: rewrite the inbound `[[path|title]]` text and rename the file
+  (all or nothing on the vault), then re-key the moved note's rows and re-project the
+  inbound sources ([index-engine.md](index-engine.md) §8). That is how L1's "rename keeps
+  every backlink resolving" holds. A move made outside B2 is a delete plus a create, and its inbound links
   project as surfaced dangling edges (G5).
 - **The edge set is the union of the two homes, deduped** (G2). A frontmatter entry with a
   *different* verb than a body link to the same target is no duplicate; that is the augment

docs/index-engine.md

@@ -96,8 +96,8 @@ Why this shape fits B2:
   key; `chunks.note_path`, `note_aliases.note_path`, `note_centroids.note_path`, and
   `edges.src_path` are `REFERENCES notes(path) ON DELETE CASCADE ON UPDATE CASCADE`. That
   makes a B2-performed move a path re-key rather than a rebuild: `UPDATE notes SET path = …`
-  cascades through every child in one transaction, alongside the inbound link-text rewrite
-  and a re-projection of the inbound sources. `edges.dst_path` is deliberately *not* a
+  cascades through every child in one statement, after the inbound link-text rewrite and
+  before a re-projection of the inbound sources. `edges.dst_path` is deliberately *not* a
   foreign key: it must be allowed to be NULL, the dangling case (G5). "Rename keeps every
   backlink resolving" is therefore a property of the move operation, not of the key; the
   price is that a move made outside B2 is a delete plus a create (§8).
@@ -589,10 +589,27 @@ be budgeted, tested, and watched.
   inbound link text in every file that points at it: an N-file write. It is bounded and
   mechanical (the materialized edges name exactly which files and links to touch, Markdown
   first, then the index), but moving a heavily linked note is proportional to its backlink
-  count, not O(1). The rewrite is transactional, so a partial move never half-updates the
-  vault. The index side is one cascading `UPDATE` plus re-projection of the inbound sources,
-  bounded by the same count. Only the exact files the graph names are touched: a
-  prefix-sharing `[[foo-bar]]` is never rewritten when moving `foo`.
+  count, not O(1). The index side is one cascading `UPDATE` plus re-projection of the
+  inbound sources, bounded by the same count. Only the exact files the graph names are
+  touched: a prefix-sharing `[[foo-bar]]` is never rewritten when moving `foo`.
+- **A failed move leaves the vault as it was.** A SQLite transaction can't roll back file
+  writes, and reindexing can't repair rewritten link text (it projects whatever the
+  Markdown now says), so the vault half carries its own all-or-nothing contract (GH #230):
+  - *Refused before any write:* a destination that exists, or that sits beneath a regular
+    file.
+  - *Planned before any write:* every inbound file is read and its rewrite computed in
+    memory, so an unreadable file stops the move with nothing changed.
+  - *Undone on failure:* the rewrites, any destination folders the move creates, and the
+    rename run as one unit. If a step fails, the earlier ones are undone in reverse and
+    the original error is returned, with every file byte-identical. If an undo step
+    itself fails, the move reports the files still holding a rewrite
+    (`Error::MoveIncomplete`) rather than claiming a clean vault.
+  - *After the rename, the vault is final.* A failure re-keying or re-projecting the index
+    leaves correct Markdown and a stale index; `b2 reindex` projects it.
+  - *A crash can't be undone.* The one unguarded window is between the first rewrite and
+    the rename: the rewritten links name the destination while the note is still at its
+    source, and they surface as dangling (G5). Re-running the same move finishes it,
+    because the rewrites are already done and only the rename is left.
 - **Out-of-band moves are identified, not repaired, and that is the scope decision.** A
   `git mv` or Finder move is, to a path-keyed index, a delete plus a create: the old path's
   rows prune, the new path projects fresh, and every inbound `[[oldpath]]` becomes a surfaced

docs/invariants.md

@@ -94,9 +94,10 @@ drops in without a redesign.
   link homes are already written by path: a body `[[path]]`, or a frontmatter `b2_relations:`
   entry. So an edge stores what you authored, resolved at projection time, with no machine id
   in the file. Consequence: a rename keeps every backlink resolving *when B2 does the move*. A
-  B2 move rewrites the inbound path text and re-keys the moved note's rows in one transaction.
-  A move made outside B2 is a delete plus a create: the inbound links surface as dangling
-  (G5), identified rather than silently dropped.
+  B2 move rewrites the inbound path text and re-keys the moved note's rows as one operation,
+  and a move that fails leaves every file as it was (GH #230). A move made outside B2 is a
+  delete plus a create: the inbound links surface as dangling (G5), identified rather than
+  silently dropped.
   ([data-model.md](data-model.md) §1, §3,
   [GH #170](https://github.com/AlteredCraft/B2/issues/170))
 - **L2. A note's title is its filename.** The frontmatter `title:` key is recognized but