Skip to content

Failed move rewrites backlinks before rejecting the destination, leaving broken links #230

Closed

Description

@samkeen

Moving a note to a destination whose parent is a regular file returns an error after rewriting its backlinks. The target stays at its original path, but the authored Markdown now points to a nonexistent destination. Reindexing cannot repair this: it faithfully projects the already changed source files.

Reproduced on commit 1776e030eee3343652e6392f0233598fab52160c using the current source via cargo run, with the deterministic fake embedder and an isolated temporary vault.

Reproduction

  1. Create a vault containing a.md with Target note., b.md with See [[a]]., and a regular file named blocked.
  2. Run B2_EMBEDDER=fake cargo run --quiet -p b2-cli -- -C <vault> reindex.
  3. Run B2_EMBEDDER=fake cargo run --quiet -p b2-cli -- -C <vault> mv a.md blocked/a.md.

Observed:

  • The move exits 1 with the generic failure message.
  • a.md still exists at its original path.
  • b.md now contains See [[blocked/a]]..
  • The requested destination does not exist.

Expected: a rejected move leaves the original note and every inbound file byte-identical, with the original links resolving.

Cause and scope

move_note calls rewrite_inbound before rename_with_parents. The shared rewrite_inbound reads and writes inbound files sequentially with immediate error propagation and no rollback. A destination-parent failure therefore occurs after authored content has changed; a failure on a later inbound file can also leave earlier rewrites applied.

Resource and directory moves use the same helper and should be covered by the fix; the reproduction above exercises the note arm only. Both CLI and desktop inherit the core behavior.

This contradicts index-engine.md §8: the rewrite is described as transactional, with no partially updated vault. The disposable-index guarantee cannot recover lost original link text.

Acceptance criteria

  • Add a deterministic, model-free regression test for the destination-parent failure above before changing implementation; assert original file bytes, paths, and resolved links.
  • Exercise a failure after at least one inbound rewrite and ensure the operation restores original authored content, rather than returning an error with partial changes left behind.
  • Apply the failure-handling contract consistently to note, resource, and directory moves, preserving aliases and unrelated bytes.
  • Validate destinations before writing, but do not rely on preflight alone: later I/O failures still require handling.
  • Explicitly reconcile the documented transaction guarantee with the implemented behavior for ordinary I/O failures and interrupted operations. A SQLite transaction alone does not roll back filesystem writes.
  • Run the existing move/projection suites and make check; run make ci before pushing the fix.

Activity

  1. added a commit that references this issue on Sep 23, 2026
  2. samkeen commented on Sep 23, 2026

    @samkeen
    ContributorAuthor

    Fixed by #245 (merged). The PR referenced this issue without a closing keyword, so it's being closed by hand.

    How each acceptance criterion was met:

    • Regression test written first: a_move_under_a_file_is_refused_and_changes_nothing in crates/b2-core/tests/mv.rs is the reproduction above. It failed on the old code. It now asserts that every file's bytes and path are unchanged and that the original links still resolve, both straight after the refused move and after a reindex.
    • Failure after an inbound rewrite: a_move_whose_rename_fails_restores_every_rewritten_file and its resource-move twin make the rename fail after the rewrites have been written. Every file comes back byte-identical and the move leaves no empty folders behind. Unit tests in mv.rs cover a write that fails after an earlier one landed, and an undo that itself fails. That last case surfaces as Error::MoveIncomplete, which names the files still holding a rewrite and keeps the original error.
    • Note, resource and folder moves alike: all three now plan every rewrite before writing and run a single commit step that undoes itself on failure. Aliases, whitespace and unrelated bytes are left untouched.
    • Checks before writing, not only before: refuse_file_ancestor rejects a destination beneath a regular file, and later I/O failures are still undone.
    • Docs made to match the code: index-engine.md §8, data-model.md and invariant L1 no longer call the move transactional. They now say the move is all or nothing on the vault. A failure after the rename leaves correct Markdown that b2 reindex projects. A crash between the rewrites and the rename can't be undone, and re-running the same move finishes it; re_running_a_move_interrupted_before_its_rename_finishes_it pins that.
    • make check and make ci passed before each push, and CI was green on the PR.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions