feat(tools): add vault_move_note with vault-wide link rewriting

[aliasunder]

Problem

Moving or renaming a note meant write-new + delete-old, which silently orphaned every backlink: the links table kept stale rows pointing at the old path, and source notes still contained [[Old]] text that no longer resolved. Obsidian's own rename updates every link to a moved note, so this is a parity gap — a strict subset of Obsidian's link handling is a bug, per AGENTS.md.

Fix

New vault_move_note(old_path, new_path) tool that moves the file and rewrites every link across the vault that resolves to it.

• Reuses the indexer's link logic — resolveLink plus the wikilink/markdown/fence/inline-code regexes, now exported from search-index.ts — so the rewriter can't disagree with the index about what a link is or where it points. No reimplemented link parsing.
• Form-preserving — basename, full-path, and source-relative (../) forms are each kept; aliases (|), heading anchors (#), embeds (!), markdown links, and frontmatter links are all preserved. Reserved characters in markdown link paths are percent-encoded per segment.
• Minimal-churn — a link is rewritten only when leaving it would break it. A short [[Note]] survives a folder move when its name stays unambiguous, exactly as Obsidian behaves.
• The moved note's own relative links are recomputed so they still resolve from the new folder.
• Path normalization — old_path/new_path are canonicalized before the protected-path guard and the backlink lookup, so .. segments can't bypass protection or miss backlinks.
• Failure safety — a preflight reads and plans every rewrite before any write (a read failure aborts with the vault untouched); the destination is created with an atomic no-clobber write (link-based, never overwrites); the original is deleted last; each failure point logs structured context, and the completion summary logs success/failure counts.
• Safety — refuses to overwrite an existing destination; refuses moves into or out of protected paths (memory, daily notes); leaves links inside code fences and inline code untouched.
• Index refresh rides the existing file-watcher, consistent with vault_write_note / vault_delete_note; the tool performs filesystem operations only.

Returns { moved_to, links_updated, updated_notes }.

Changes

Area	Detail
New tool	note-mover.ts — orchestration + form-preserving, minimal-churn link rewriting
Reuse	resolveLink / WIKILINK_RE / MD_LINK_RE / FENCE_OPEN / INLINE_CODE_RE exported from search-index.ts
New utility	src/utils/map-with-concurrency.ts — bounded-concurrency async map
Filesystem	atomicCreateFile — atomic no-clobber create (temp file + link)
Registration	vault_move_note added to tool-definitions.ts (TDQS-scored description; snake_case old_path/new_path)
Docs	tool count 23 → 24 across README, ARCHITECTURE, cli/llms-install, server.json, Dockerfile, deploy workflow, social-preview; AGENTS.md structure + logging lists updated

Tests

• 32 tests in note-mover.test.ts — every link form, minimal-churn no-ops, basename-collision fallback, the moved note's own relative links, code-fence and inline-code exclusion, reserved-character link encoding, normalized-protected-path refusal, all guards (overwrite, missing source, protected paths, identical paths, non-.md, traversal), and failure safety (preflight abort + structured-log assertions). Whole-object assertions on the result and exact file-content assertions.
• 8 tests in map-with-concurrency.test.ts — order preservation, the concurrency cap actually holding, head-of-line blocking, rejection short-circuiting later batches, empty input, and input validation.
• 4 tests in vault-filesystem.test.ts for atomicCreateFile — creates a new file, throws EEXIST without modifying existing content, and strands no temp file in either case.
• Mutation-checked: every behaviour-breaking mutation fails only the test(s) that target it.
• Full suite: 811 pass; lint, build, and prettier clean.

---

Generated by Claude Code

Summary by CodeRabbit

• New Features

  • Added vault_move_note tool to move or rename notes while automatically updating all vault-wide links that reference the moved note.

• Documentation

  • Updated documentation to reflect 24 available tools (previously 23).

[aliasunder]

/@CodeRabbit

[coderabbitai]

@aliasunder Sure! I'll start reviewing this PR right away.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a vault_move_note MCP tool that moves or renames Obsidian notes while rewriting all vault-wide links (wikilinks, markdown links, frontmatter). Introduces a mapWithConcurrency batch utility used by the note mover, exports four link-parsing regex constants from search-index.ts for reuse, and updates all tool-count references across docs and metadata from 23 to 24.

Changes

vault_move_note tool with link rewriting

Layer / File(s)	Summary
mapWithConcurrency utility and tests src/utils/map-with-concurrency.ts, src/utils/__tests__/map-with-concurrency.test.ts	Adds mapWithConcurrency, a batch-based async utility that processes items with bounded concurrency, preserves result order, validates concurrency as a positive integer, and propagates mapper rejection. Tests cover ordering, batch head-of-line blocking, concurrency limits, rejection behavior, empty input, and invalid concurrency values.
Export link-parsing regex constants src/vault-mcp/search/search-index.ts	Changes FENCE_OPEN, WIKILINK_RE, MD_LINK_RE, and INLINE_CODE_RE from file-local constants to exported constants so the note-mover can reuse the same link-parsing rules as the indexer.
Note-mover link rewriting engine src/vault-mcp/vault-operations/note-mover.ts (lines 1–410)	Implements MoveResult type, link-form classification, rewriteTarget minimal-churn computation, per-line body rewriting (skipping inline code spans and fenced blocks via fence-state machine), recursive frontmatter wikilink rewriting, and rewriteNoteContent combining both passes into a single serialized output.
moveNote orchestration and public API src/vault-mcp/vault-operations/note-mover.ts (lines 412–637)	Adds path guard helpers (protected-folder detection, vault-relative normalization, safe existence probe), REWRITE_CONCURRENCY = 10, and moveNote with preflight/commit semantics: input validation, source/destination checks, concurrent backlink reads, concurrent writes, original deletion, MoveResult construction, and the exported noteMover object.
notesMover test suite src/vault-mcp/vault-operations/__tests__/note-mover.test.ts	Comprehensive Vitest suite using a per-test temporary vault, covering file relocation, all supported link syntaxes and edge cases, selectivity (fenced/inline-code skipping, unrelated link preservation), count/summary correctness, guard errors (protected paths, traversal, non-.md, vault escape), and failure-safety (abort on unreadable source, error/completion logging).
vault_move_note MCP tool registration src/vault-mcp/tool-definitions.ts, src/vault-mcp/__tests__/tool-definitions.test.ts	Imports noteMover, adds VAULT_MOVE_NOTE to TOOL_NAMES, registers the tool with old_path/new_path schema and a safeHandler that calls search.getBacklinks, vaultFs.listNotes, and noteMover.moveNote, then returns JSON-stringified MoveResult. Test file adds VAULT_MOVE_NOTE to the destructive and write-tool classification lists.
Documentation and metadata (24 tools) README.md, ARCHITECTURE.md, llms-install.md, server.json, cli/README.md, Dockerfile, .github/workflows/deploy.yml	Updates tool count from 23 to 24 in all docs, metadata labels, and install verification text, and adds the vault_move_note entry to the README tools table and the ARCHITECTURE Phase 1 tools list.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• aliasunder/vault-cortex#66: Also modifies src/vault-mcp/tool-definitions.ts and tool-definition tests, and establishes the "every tool must have a Returns: section" test rule that the new vault_move_note registration must satisfy.
• aliasunder/vault-cortex#81: Updates the same destructive/additive tool classification arrays in src/vault-mcp/__tests__/tool-definitions.test.ts, using the same pattern this PR extends for VAULT_MOVE_NOTE.
• aliasunder/vault-cortex#151: Modifies src/vault-mcp/search/search-index.ts around wikilink parsing and link extraction from YAML frontmatter, directly intersecting with the regex constants this PR exports for reuse by the link-rewriter.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(tools): add vault_move_note with vault-wide link rewriting' directly summarizes the main feature addition: a new MCP tool for moving notes while rewriting vault-wide links, matching the PR's primary objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/vault-bootstrap-setup-m6ssvv

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (2)

src/vault-mcp/vault-operations/note-mover.ts (2)

518-527: ⚡ Quick win

Use try/catch for logged async operations.

The chained .catch() blocks diverge from the project async style; wrapping these awaited operations in try/catch also makes it easier to include planning failures in the moved-note preflight log, not just read failures.

As per coding guidelines, **/*.ts: “prefer async/await over .then()/.catch() for async operations”.

Also applies to: 576-586

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vault-mcp/vault-operations/note-mover.ts` around lines 518 - 527, The
readFile call in the movedRawContent assignment uses a chained .catch() block,
which diverges from the project's async/await style preference. Convert this to
use try/catch wrapping the await expression instead. Wrap the readFile operation
in a try block and move the error handling logic from the catch handler into a
corresponding catch block, maintaining the same error logging and throwing
behavior. Apply the same pattern to the similar code block at lines 576-586.

Source: Coding guidelines

---

468-486: ⚡ Quick win

Create an operation child logger before emitting move logs.

moveNote receives a logger but logs directly. Create a child logger after path normalization, e.g. with { tool: "vault_move_note", from: oldPath, to: newPath }, then use it for all move logs so request/session context is consistently carried.

As per coding guidelines, src/vault-mcp/vault-operations/**/*.ts: “Data-layer functions must use two-argument pattern: (params, logger) with named params, and carry logger context via child loggers with .child() for sessionId, clientIp, requestId, and tool”.

Also applies to: 520-526, 563-566, 576-586, 600-608, 620-626

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/vault-mcp/vault-operations/note-mover.ts` around lines 468 - 486, The
moveNote function receives a logger parameter but logs directly without creating
a child logger context. After normalizing the paths (oldPath and newPath),
create a child logger using the logger.child() method with context properties
including tool set to "vault_move_note", from set to oldPath, and to set to
newPath. Then use this child logger instance for all error logs and subsequent
logging throughout the moveNote function instead of using the original logger
parameter directly. Apply the same pattern to all other logging calls mentioned
in the affected line ranges (520-526, 563-566, 576-586, 600-608, 620-626) to
ensure consistent request/session context is carried through all move
operations.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/vault-mcp/tool-definitions.ts`:
- Around line 607-625: The backlink lookup using search.getBacklinks at line 616
receives a raw oldPath without normalization, while noteMover.moveNote
normalizes paths internally, causing a mismatch where non-canonical paths (like
Folder/../Note.md) can move successfully but backlink sources are not found,
leaving stale inbound links. Normalize oldPath before passing it to
search.getBacklinks to ensure it matches the canonical path format that
noteMover.moveNote expects, so backlinks are properly identified and updated
during the move operation.

In `@src/vault-mcp/vault-operations/__tests__/note-mover.test.ts`:
- Around line 398-400: The test assertion for the moveNote function guard is
using a substring match instead of the exact error message. Replace the
substring "only moves .md" in the toThrow() call with the complete,
deterministic error message that the guard actually throws. This ensures the
test catches any unintended changes to the error message contract and provides
more precise validation than a partial string match.
- Around line 325-330: Replace the loose boolean assertion that uses `.every()`
on the `allRewritten` array with an exact whole-value assertion. Instead of
checking if every element matches a condition and asserting the resulting
boolean is true, directly assert that the entire `allRewritten` array equals the
expected array of rewritten notes. This will provide better diagnostics if the
assertion fails and make the test deterministic by comparing the full array
values directly.

In `@src/vault-mcp/vault-operations/note-mover.ts`:
- Line 613: The unlink(oldFullPath) call at line 613 in the note-mover.ts file
lacks error handling that provides recovery context when the file deletion fails
after successful destination and backlink rewrites. Wrap the unlink(oldFullPath)
call in a try-catch block and in the catch handler, log a structured error
message that includes the source path (from), destination path (to),
planned/written backlink counts, and guidance for recovery operations before
rethrowing the error. This ensures debugging and recovery information is
captured when the vault is left with both old and new notes due to a deletion
failure.
- Around line 491-496: The fileExists precheck for newFullPath followed by
atomicWriteFile is vulnerable to race conditions where another process can
create the file between the check and write, causing silent data loss. Replace
the fileExists guard with atomic no-clobber semantics by modifying
atomicWriteFile to use the 'wx' flag in fs.writeFile for exclusive file
creation, or adopt a no-clobber atomic write library that combines exclusive
creation with atomic rename semantics. This ensures the destination cannot be
overwritten between the check and the actual write operation. Apply this fix to
all atomicWriteFile call sites where the destination is expected to be new,
including the locations around lines 491-496 and lines 576-586.
- Around line 12-13: The code imports `posix` from `node:path` and uses
`posix.dirname()` on filesystem paths returned from `resolveSafePath()`, which
causes issues on Windows. Update the import statement on line 12 to also import
the native `dirname` function from `node:path` (not posix), then locate where
`posix.dirname(newFullPath)` is called and replace it with the native
`dirname(newFullPath)` function to ensure proper cross-platform path handling.

---

Nitpick comments:
In `@src/vault-mcp/vault-operations/note-mover.ts`:
- Around line 518-527: The readFile call in the movedRawContent assignment uses
a chained .catch() block, which diverges from the project's async/await style
preference. Convert this to use try/catch wrapping the await expression instead.
Wrap the readFile operation in a try block and move the error handling logic
from the catch handler into a corresponding catch block, maintaining the same
error logging and throwing behavior. Apply the same pattern to the similar code
block at lines 576-586.
- Around line 468-486: The moveNote function receives a logger parameter but
logs directly without creating a child logger context. After normalizing the
paths (oldPath and newPath), create a child logger using the logger.child()
method with context properties including tool set to "vault_move_note", from set
to oldPath, and to set to newPath. Then use this child logger instance for all
error logs and subsequent logging throughout the moveNote function instead of
using the original logger parameter directly. Apply the same pattern to all
other logging calls mentioned in the affected line ranges (520-526, 563-566,
576-586, 600-608, 620-626) to ensure consistent request/session context is
carried through all move operations.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dc80f3b-072e-4a9e-91ad-cc8050b89508

📥 Commits

Reviewing files that changed from the base of the PR and between 9336ebf and 0fc573b.

⛔ Files ignored due to path filters (1)

• assets/social-preview.svg is excluded by !**/*.svg

📒 Files selected for processing (14)

• .github/workflows/deploy.yml
• ARCHITECTURE.md
• Dockerfile
• README.md
• cli/README.md
• llms-install.md
• server.json
• src/utils/__tests__/map-with-concurrency.test.ts
• src/utils/map-with-concurrency.ts
• src/vault-mcp/__tests__/tool-definitions.test.ts
• src/vault-mcp/search/search-index.ts
• src/vault-mcp/tool-definitions.ts
• src/vault-mcp/vault-operations/__tests__/note-mover.test.ts
• src/vault-mcp/vault-operations/note-mover.ts

[aliasunder]

Review — feat(tools): add vault_move_note with vault-wide link rewriting

Verdict: Ship it. No bugs. 1 medium convention finding, 3 low.

---

What's good

The design is sound — reusing resolveLink and the 4 link-syntax regexes from search-index.ts means the rewriter literally cannot disagree with the indexer about what a link is or where it points. This is the right call and the PR description communicates it well.

The two-phase preflight + commit architecture (read everything → compute all rewrites → write destination → rewrite sources → delete original last) is the correct failure-safety model: a read failure aborts cleanly, a mid-write failure never loses the original. Good.

Form-preserving, minimal-churn rewriting — a bare [[Foo]] surviving a folder move when its short name stays unambiguous — mirrors Obsidian's actual behavior rather than the lazy "always write the full path" approach. The classifyLinkForm → buildReplacementTarget → verify-with-resolveLink pipeline is clean and well-defended.

mapWithConcurrency is a clean utility with a thorough 8-test suite covering every meaningful behavioral contract (order preservation, peak concurrency, head-of-line blocking, error propagation, input validation).

31 tests in note-mover.test.ts (see nit below) covering all link forms, minimal-churn no-ops, basename-collision fallback, code-fence/inline-code exclusion, all guards, and failure safety with structured-log assertions. Assertion-strong throughout — full file-content assertions, not substring checks.

---

Findings

1. Doc comments on exported regexes name a specific caller (medium — convention)

The 4 exported regex doc-comment additions in search-index.ts each reference note-mover.ts by name:

"Exported so the link rewriter (note-mover.ts) skips code the same way indexing does."
"Exported as the single source of truth for wikilink syntax, reused by note-mover.ts."
etc.

Per AGENTS.md's comment rules (and the global CLAUDE.md): comments should not reference callers — "those belong in the PR description and rot as the codebase evolves." If a second consumer appears, these comments become stale or misleading. The why ("single source of truth for wikilink syntax") is exactly the right thing to say; it just shouldn't be coupled to who.

Suggested fix: keep the "single source of truth" / "so rewriters skip code the same way indexing does" framing, drop the "(note-mover.ts)" references. See inline comment.

2. posix.dirname on an OS path (low — consistency)

note-mover.ts uses posix.dirname(newFullPath) for the mkdir parent, where newFullPath is an absolute OS path from resolveSafePath. Not a bug (Linux/macOS paths are POSIX), but inconsistent with vault-filesystem.ts which imports dirname from node:path for filesystem paths. The codebase convention is posix for vault-relative paths, node:path for OS paths. See inline comment.

3. AGENTS.md structure section not updated (low — doc lockstep)

note-mover.ts is a primary feature module (adds a new MCP tool) but isn't listed in the AGENTS.md structure section alongside its peers (vault-filesystem.ts, vault-patcher.ts, etc.). The logging section's data-layer function list (vault-filesystem, vault-patcher, memory-store, search-index) also doesn't mention note-mover. Per working-style memory: "Keep convention docs in lockstep with the code" (2026-05-27).

src/utils/map-with-concurrency.ts introduces a new directory (src/utils/) — could also be listed, though it's a general utility rather than a domain module.

4. PR body says "26 new tests," actual count is 31 (nit)

Counted 31 it() blocks in note-mover.test.ts: 3 (file relocation) + 12 (link rewriting forms) + 4 (selectivity) + 2 (counts/summary) + 7 (guards) + 3 (failure safety). Plus 8 in map-with-concurrency.test.ts. The "26" is likely from an earlier draft.

---

Observations (not findings)

• Fence state machine duplication: rewriteBody in note-mover.ts reimplements the same fence-tracking state machine from extractLinks in search-index.ts. The regex (FENCE_OPEN) is shared, but the state-machine logic could diverge if one is changed without the other. This is defensible — the two consumers use different iteration patterns (reduce vs for-loop) — but worth noting for future maintainers. Not a finding since extraction would be a refactor beyond this PR's scope.

• Test coverage gaps (low risk, not findings): No test for a markdown link with a heading anchor ([text](path.md#heading) being moved), and no test for nested frontmatter objects containing wikilinks. Both are structurally handled by the same code paths that ARE tested.

---

Reviewed with: fable-mode staged review (6 stages), sequential thinking, fresh vault-memory pull (Opinions + Principles). Mutation analysis claims in PR description accepted — verified the tests' assertion strength independently.

[aliasunder]

Correction to review: Finding #2 ("posix.dirname on an OS path") is invalid — the review was based on the initial commit's diff, but the branch was force-pushed before the review landed. The latest code already imports dirname from node:path and uses it correctly, with an explanatory comment:

// dirname must be the native variant — newFullPath is an absolute filesystem
// path from resolveSafePath, not a vault-relative ("/"-separated) path.
await mkdir(dirname(newFullPath), { recursive: true })

The inline comment for that finding was deleted. Remaining findings (1, 3, 4) are verified against the latest commit (04b1fcd).

Also noting: the latest branch has additional improvements over the initial commit — preflight+commit phase separation, mapWithConcurrency for bounded concurrency, proper error handling with descriptive structured logs at each failure point, toVaultRelativePath normalization, and robust encodeMarkdownLinkPath. These are all good changes.

Updated test count: 32 tests in note-mover.test.ts (not 26 as stated in the PR body, finding #4), plus 8 in map-with-concurrency.test.ts.

[aliasunder]

Local README: Windows section placement + verbosity

The Windows (Docker Desktop) section feels too verbose and lands in the wrong spot.

Placement: It's inserted between "Start the server" (step 4) and "Connect your MCP client" — right in the middle of the happy path. A Windows user who needs this info needs it before step 3 (when they're setting VAULT_PATH), not after step 4. And for everyone else it's a 30-line detour in a quickstart.

Verbosity: The section explains the why (filesystem boundary, inotify, hard-link atomics) at the same depth as the fix. In a quickstart, the user just needs to know what to do — the mechanism is secondary.

Suggested fix:

1. Add a one-liner at step 3 (where VAULT_PATH is set): "On Windows, use a path inside the WSL2 filesystem (e.g. /home/you/vaults/MyVault), not a C:\ drive — see Windows notes below."

2. Move the full section after Troubleshooting (or as a subsection of it), and trim the mechanism explanation. The essential content is: use WSL2 filesystem, here's the path, you can still open it in Obsidian via \\wsl$\.... The "two things that break" list and the cross-filesystem-boundary explanation can shrink to one sentence.

[aliasunder]

Done in 7f6f681:

• Step 3 pointer — added a one-liner under the VAULT_PATH row directing Windows users to a WSL2 path (not C:\) with a link down to the Windows section.
• Moved + trimmed — the full section now lives below Troubleshooting and is down to the essentials: keep the vault on the WSL2 filesystem, one sentence on the two affected features (live re-indexing, vault_move_note), the path command, and the \\wsl$\… note that Obsidian on Windows can still open it. The mechanism deep-dive (inotify, hard-link atomics, the cross-boundary explanation) is gone.

---

Generated by Claude Code

[aliasunder]

Follow-up: eliminate RewriteContext threading

RewriteContext is passed through 5 layers of functions (rewriteBody → rewriteBodyLine → collectLineEdits → rewriteWikilinkText → rewriteTarget) but only rewriteTarget at the bottom actually reads from it. The intermediate functions just pass it through — a reader has to trace 5 levels deep to understand why it's there.

Fix: Partially apply rewriteTarget to its context once (producing a (rawTarget: string) => string | null callback), then thread that callback instead. The intermediate functions don't need to know about RewriteContext at all — they just need "here's how to rewrite a target."