fix: vault_update_properties treats null as property deletion

[aliasunder]

Problem

vault_update_properties was merge-only with no property-removal path. Passing { "key": null } carried the null straight through the spread merge into the YAML serializer, which wrote a literal key: null into frontmatter. Hit live 2026-06-10 cleaning a stray property off a note — recovery required a manual file edit.

Fix

Contract: null means delete. A key explicitly set to null in the update is removed from frontmatter.

• New mergeFrontmatter helper in frontmatter.ts — used by both merge paths (updateProperties and vault_write_note's serializeNote, including its new-file branch) so the contract is consistent everywhere.
• Only the caller's nulls delete. Nulls already present in existing frontmatter — Obsidian empty properties like due: parse as null — are preserved when unmentioned; filtering all nulls would silently strip empty template properties on every update.
• nullStr: "" added to the YAML stringify options so preserved nulls round-trip Obsidian-style as due: instead of due: null.
• Deleting the last remaining property removes the frontmatter block entirely (no empty --- stub).
• Tool descriptions + Zod describe() document the contract on both tools.

⚠️ destructiveHint flip

vault_update_properties annotations change destructiveHint: false → true — the tool can now irreversibly remove data (and already overwrote values). Some MCP clients may start gating it behind a confirmation prompt; this is intended. Moved from ADDITIVE_WRITE_TOOLS to DESTRUCTIVE_TOOLS in the annotation tests (vault_update_memory remains the only genuinely additive writer).

Tests

9 new tests, all byte-exact content assertions. Mutation-audited:

• Plain-spread revert at both call sites → exactly the 6 delete-behavior tests fail
• Helper mutated to filter all nulls → exactly the preserve-empty-property guard fails

529 tests pass; lint, build, prettier clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Implemented proper null semantics for property handling—setting a property to null now correctly deletes that key from notes while preserving unmodified properties.

• Documentation

  • Expanded tool descriptions with examples demonstrating null-deletes-key behavior.
  • Reclassified property update tool as destructive, accurately reflecting its capability to remove data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c5f60a21-4404-44bf-acf9-4692d881d799

📥 Commits

Reviewing files that changed from the base of the PR and between bddec20 and e9dbab3.

📒 Files selected for processing (5)

• src/vault-mcp/__tests__/tool-definitions.test.ts
• src/vault-mcp/tool-definitions.ts
• src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts
• src/vault-mcp/vault-operations/frontmatter.ts
• src/vault-mcp/vault-operations/vault-filesystem.ts

---

📝 Walkthrough

Walkthrough

This PR implements null-value property deletion for vault notes. A new mergeFrontmatter() helper enables safe property merging where setting a property to null removes that key. The behavior is integrated into write and update operations, documented in tool metadata, and covered by comprehensive test cases.

Changes

Note Property Null-Deletion Semantics

Layer / File(s)	Summary
Frontmatter merge contract and null-deletion implementation src/vault-mcp/vault-operations/frontmatter.ts	New mergeFrontmatter(existing, updates) helper merges property updates while treating null values as key deletions. YAML serialization options updated to emit null values as empty properties instead of null literals.
Vault filesystem operations using mergeFrontmatter src/vault-mcp/vault-operations/vault-filesystem.ts	serializeNote() and updateProperties() now use mergeFrontmatter() to safely merge properties, enabling null-deletion semantics during note write and property update operations.
Filesystem operation tests for null-deletion src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts	Test coverage verifies null keys are removed during merge, null properties are omitted from new files, and frontmatter block is removed when all keys are deleted via null values.
Tool metadata and classification updates src/vault-mcp/tool-definitions.ts	Tool descriptions for vault_write_note and vault_update_properties document the null-deletion behavior. VAULT_UPDATE_PROPERTIES reclassified as destructive with destructiveHint set to true.
Tool metadata description and classification tests src/vault-mcp/__tests__/tool-definitions.test.ts	Test suite verifies tool metadata documents null-deletion semantics and that VAULT_UPDATE_PROPERTIES is classified as destructive in both annotations and description assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• aliasunder/vault-cortex#85: Both PRs modify the shared gray-matter/YAML frontmatter serialization path via MATTER_OPTIONS (main PR adds nullStr: "" and mergeFrontmatter() null-deletion semantics; retrieved PR introduces/threads MATTER_OPTIONS with a YAML engine to preserve datetime scalars as strings).
• aliasunder/vault-cortex#81: Both PRs update MCP tool metadata/tests in tool-definitions.ts and tool-definitions.test.ts around the destructive vs non-destructive (destructiveHint) categorization sets (main: VAULT_UPDATE_PROPERTIES null-delete semantics plus moves to destructive; retrieved: reclassifies writer tools and asserts destructiveHint for additive tools).

Poem

🐰 A null walks through the frontmatter deep,
And keys that matched it fall asleep,
No empty values left to weep—
Just deletion, clean and lean! 🗝️

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: vault_update_properties now treats null values as property deletion, which is the primary fix addressed by this PR.
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.

_{✏️ 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 fix/property-delete-null

---

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.}