feat(vault): opt-in empty-folder pruning for delete and move

[aliasunder]

Summary

Adds an opt-in prune_empty_folders flag (default false) to vault_delete_note and vault_move_note. When set, removes the note's now-empty parent folders, walking up to but never including the vault root.

Why default-off

The original task framed this as "match Obsidian," but an empirical check shows Obsidian leaves empty folders when the last note is moved/deleted — reliable auto-removal is exactly what the community "Remove Empty Folders" plugins exist to add. So default-off is Obsidian parity; pruning is an opt-in cleanup, and existing callers see no behavior change.

What's inside

• Shared pruneEmptyParents helper in vault-filesystem.ts — walks child→parent, stops at (never removes) the vault root.
• Only removes a zero-entry directory — a stray .DS_Store or any file blocks removal; it never deletes a file.
• Best-effort: a failed readdir/rmdir is logged at warn naming the folder and ends the walk, so pruning never fails the delete/move that already succeeded.
• Pruned-folder count rides the completion log and the tool response (MoveResult.pruned_empty_folders; delete confirmation notes it when > 0).
• Tool descriptions document the flag (TDQS self-score: vault_delete_note holds 5.0, vault_move_note holds ~4.8 — no regression).

Test plan

• npm test — 824 pass (13 new prune cases: default-off parity, multi-level walk, non-empty stop, vault-root guard, hidden-file, failure-isolation)
• Build (server + CLI), ESLint, Prettier clean
• Live e2e + Glama TDQS — require a deploy (default-off matches the currently deployed behavior)

🤖 Generated with Claude Code

---

Generated by Claude Code

Summary by CodeRabbit

• New Features

  • Added optional "prune empty folders" parameter to note deletion and move operations for automatic cleanup of empty parent directories.
  • Move operation now returns the count of pruned empty folders.

• Documentation

  • Updated test writing conventions to prevent common testing pitfalls including silent no-ops, incorrect error matching, and unintended early returns.

[aliasunder]

/@CodeRabbit

[coderabbitai]

@aliasunder Sure! Let me kick off a review of the PR changes.

✅ 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 an optional prune_empty_folders parameter to vault_delete_note and vault_move_note MCP tools. The core implementation introduces toVaultRelativePath and pruneEmptyParents in vault-filesystem.ts, updates deleteNote to return a pruned-folder count, and extends moveNote and MoveResult accordingly. New and updated tests cover all pruning scenarios. AGENTS.md gains test-quality guidance.

Changes

Prune Empty Folders Feature

Layer / File(s)	Summary
vault-filesystem: toVaultRelativePath, pruneEmptyParents, deleteNote update src/vault-mcp/vault-operations/vault-filesystem.ts	Exports toVaultRelativePath (POSIX path normalization) and pruneEmptyParents (recursive empty-dir walker returning a count). deleteNote now normalizes the path before protected-path checks, optionally calls pruneEmptyParents, and returns Promise<number> instead of Promise<void>.
vault-filesystem tests: updated call sites and new prune suite src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts	All existing deleteNote calls gain pruneEmptyFolders: false. Adds a traversal-into-protected-folder test and a full empty-folder prune suite covering parent removal, vault-root guard, hidden-file preservation, and rmdir failure handling.
note-mover: MoveResult, import consolidation, pruning wiring src/vault-mcp/vault-operations/note-mover.ts	MoveResult gains pruned_empty_folders: number. toVaultRelativePath is removed as a local export and imported from vault-filesystem. moveNote accepts pruneEmptyFolders and calls pruneEmptyParents on the old path after deletion.
note-mover tests: updated assertions and prune suite src/vault-mcp/vault-operations/__tests__/note-mover.test.ts	Test harness forwards pruneEmptyFolders and exposes folderExists. All existing MoveResult equality assertions gain pruned_empty_folders: 0. New describe block tests pruning on/off, nested parent removal, note-remaining preservation, same-folder rename, and subfolder-move edge cases.
MCP tool definitions: prune_empty_folders for delete and move src/vault-mcp/tool-definitions.ts	Adds prune_empty_folders boolean to Zod schemas, handler destructuring, request logging, and downstream calls for both vault_delete_note (pluralized confirmation message) and vault_move_note (return docs updated with pruned_empty_folders). Reorganizes imports.
AGENTS.md: test-quality conventions AGENTS.md	Adds test-writing rules against silent no-ops, overly-broad toThrow() matches, and early-return false positives; instructs asserting specific side effects and error messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• aliasunder/vault-cortex#160: Introduced vault_move_note and the noteMover.moveNote implementation whose signature, MoveResult type, and call site in tool-definitions.ts are directly extended in this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main feature: adding opt-in empty-folder pruning to delete and move operations. It is concise, actionable, and accurately summarizes the primary change across the changeset.
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-2qf15r

---

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: 2

🤖 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/vault-operations/__tests__/note-mover.test.ts`:
- Around line 552-605: The prune-path test cases are missing explicit assertions
that verify the source notes were actually removed during the move operation. In
the test "walks up removing nested empty source parents", add a noteExists check
to verify that "A/B/note.md" no longer exists after the move. Similarly, in the
tests "does not prune on an in-place rename within the same folder" and "does
not prune when moving into a subfolder of the source", add noteExists assertions
for the old note paths ("Notes/old.md" and "Parent/note.md" respectively) to
ensure the move actually occurred. This strengthens the tests so they fail if
the move operation doesn't execute but the pruning logic still produces the
expected folder state, following the guideline that tests must assert side
effects unique to the intended code path.

In `@src/vault-mcp/vault-operations/vault-filesystem.ts`:
- Around line 24-25: The toVaultRelativePath function on line 24-25 uses
posix.normalize which does not convert Windows backslash separators to forward
slashes, allowing paths like About Me\Principles.md to bypass the protected-path
checks in deleteNote and moveNote by failing the startsWith comparison. Modify
the toVaultRelativePath function to replace all backslashes with forward slashes
in the input string before applying posix.normalize to ensure consistent path
normalization across all platforms and prevent protected-path bypass
vulnerabilities.

🪄 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: 425cf318-20df-4090-b727-a88d3c616b6f

📥 Commits

Reviewing files that changed from the base of the PR and between 0828891 and 229cc06.

📒 Files selected for processing (6)

• AGENTS.md
• src/vault-mcp/tool-definitions.ts
• src/vault-mcp/vault-operations/__tests__/note-mover.test.ts
• src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts
• src/vault-mcp/vault-operations/note-mover.ts
• src/vault-mcp/vault-operations/vault-filesystem.ts