diff --git a/src/basic_memory/man/man3/edit-note(3).md b/src/basic_memory/man/man3/edit-note(3).md index 644edbe00..d179667f7 100644 --- a/src/basic_memory/man/man3/edit-note(3).md +++ b/src/basic_memory/man/man3/edit-note(3).md @@ -68,7 +68,8 @@ permalink, or memory:// URL — there is no fuzzy fallback for edits. heading of any level and preserves subsections - **metadata** — dict of frontmatter fields merged in alongside any operation; given keys overwrite or add, other keys and the body are untouched. - `title`, `type`, and `permalink` are ignored; keys cannot be deleted + `title` and `permalink` are ignored; `type` is applied like any other + frontmatter field; keys cannot be deleted - **project** / **project_id** / **workspace** — routing; same semantics as [[write-note(3)]] diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index adcb33ed6..0b0dc7aa9 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -2,6 +2,7 @@ from typing import Any, TYPE_CHECKING, Annotated, Literal, Optional +import frontmatter import logfire from httpx import HTTPStatusError from loguru import logger @@ -13,6 +14,12 @@ from basic_memory.mcp.clients import KnowledgeClient from basic_memory.config import ConfigManager +from basic_memory.file_utils import ( + dump_frontmatter, + has_frontmatter, + parse_frontmatter, + remove_frontmatter, +) from basic_memory.ignore_utils import IGNORED_PATH_REJECTION_DETAIL from basic_memory.mcp.project_context import ( UnresolvedProjectRouteError, @@ -437,9 +444,9 @@ async def edit_note( metadata: Optional dict of frontmatter fields to merge, independent of `operation`. Provided keys overwrite existing frontmatter values (or are added if new); unrelated frontmatter keys and the note body are left untouched. Can be - combined with any operation in the same call. `title`, `type`, and `permalink` - are ignored since those have their own dedicated handling. Key deletion is - not supported. + combined with any operation in the same call. `title` and `permalink` are + ignored since those have their own dedicated handling; `type` is applied like + any other frontmatter field. Key deletion is not supported. output_format: "text" returns the existing markdown summary. "json" returns machine-readable edit metadata. context: Optional FastMCP context for performance caching. @@ -720,11 +727,23 @@ async def edit_note( entity = Entity( title=title, directory=directory, + note_type=metadata.get("type", "note") if metadata else "note", content_type="text/markdown", content=content, entity_metadata=metadata, ) + # Trigger: auto-created content already declares a different type. + # Why: explicit metadata follows edit semantics and must win over the + # content frontmatter that create preparation reads back into note_type. + # Outcome: the canonical create path sees the validated metadata type. + if metadata and "type" in metadata and has_frontmatter(content): + content_metadata = parse_frontmatter(content) + content_metadata["type"] = entity.note_type + post = frontmatter.Post(remove_frontmatter(content, strip=False)) + post.metadata.update(content_metadata) + entity.content = dump_frontmatter(post) + logger.info( "Creating note via edit_note auto-create", title=title, diff --git a/src/basic_memory/schemas/request.py b/src/basic_memory/schemas/request.py index 1f19c4120..37239826f 100644 --- a/src/basic_memory/schemas/request.py +++ b/src/basic_memory/schemas/request.py @@ -83,7 +83,7 @@ class EditEntityRequest(BaseModel): replace_subsections: bool = True # Frontmatter fields to merge, independent of `operation` (issue #1011). Set/overwrite # semantics: provided keys overwrite existing values, unrelated keys and the body are - # untouched. title/type/permalink are ignored — they have their own resolution paths. + # untouched. title/permalink are ignored — they have their own resolution paths. metadata: Optional[dict[str, Any]] = None @field_validator("section") diff --git a/src/basic_memory/services/note_preparation.py b/src/basic_memory/services/note_preparation.py index e2508ed4d..e1e5ff4bb 100644 --- a/src/basic_memory/services/note_preparation.py +++ b/src/basic_memory/services/note_preparation.py @@ -11,6 +11,7 @@ import frontmatter import yaml from loguru import logger +from pydantic import TypeAdapter from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from basic_memory import db @@ -39,7 +40,7 @@ from basic_memory.repository.project_repository import ProjectRepository from basic_memory.runtime.note_move import normalize_note_move_destination_path from basic_memory.schemas import Entity as EntitySchema -from basic_memory.schemas.base import Permalink +from basic_memory.schemas.base import NoteType, Permalink from basic_memory.services.exceptions import EntityAlreadyExistsError from basic_memory.services.file_service import FileService from basic_memory.utils import build_canonical_permalink @@ -690,18 +691,21 @@ def apply_edit_operation( raise ValueError(f"Unsupported operation: {operation}") -# title/type/permalink already have dedicated resolution paths in -# prepare_edit_entity_content (H1 title reconciliation, permalink resolver). Letting a -# metadata merge touch them would race with those paths and could be silently reverted. -_METADATA_IDENTITY_FIELDS = frozenset({"title", "type", "permalink"}) +# title and permalink get reworked after the merge in prepare_edit_entity_content — +# title by H1 reconciliation, permalink by the collision-suffixing resolver. Either can +# hand back a value the caller did not ask for, so a metadata merge that set them would +# be silently reverted. `type` has no such second opinion: prepare_edit_entity_content +# just reads it back out of the frontmatter, so writing it there is how you set it. +_METADATA_IDENTITY_FIELDS = frozenset({"title", "permalink"}) +_NOTE_TYPE_ADAPTER = TypeAdapter(NoteType) def _merge_metadata_into_markdown(markdown_content: str, metadata: dict[str, Any]) -> str: """Merge caller-supplied fields into a markdown string's YAML frontmatter. - Identity fields (title/type/permalink) are dropped from the merge; every other key - overwrites the existing frontmatter value or is added new. The note body, and any - frontmatter keys not present in ``metadata``, are left untouched. + Identity fields (title/permalink) are dropped from the merge; every other key, + ``type`` included, overwrites the existing frontmatter value or is added new. The + note body, and any frontmatter keys not present in ``metadata``, are left untouched. """ null_keys = sorted(key for key, value in metadata.items() if value is None) if null_keys: @@ -714,6 +718,13 @@ def _merge_metadata_into_markdown(markdown_content: str, metadata: dict[str, Any sanitized = {k: v for k, v in metadata.items() if k not in _METADATA_IDENTITY_FIELDS} if not sanitized: return markdown_content + if "type" in sanitized: + raw_note_type = sanitized["type"] + if not isinstance(raw_note_type, str): + raise ValueError("metadata type must be a string") + # `type` now changes the note's classification, so it must cross the + # same normalization and length boundary as write_note's note_type. + sanitized["type"] = _NOTE_TYPE_ADAPTER.validate_python(raw_note_type) had_separator = True if has_frontmatter(markdown_content): diff --git a/test-int/mcp/test_edit_note_integration.py b/test-int/mcp/test_edit_note_integration.py index 0f5c0783c..fe25fe683 100644 --- a/test-int/mcp/test_edit_note_integration.py +++ b/test-int/mcp/test_edit_note_integration.py @@ -422,6 +422,144 @@ async def test_edit_note_prepend_creates_nonexistent_note(mcp_server, app, test_ assert "Something important." in content +@pytest.mark.asyncio +async def test_edit_note_append_metadata_type_applies_when_auto_creating_note( + mcp_server, app, test_project +): + """Append auto-create must honor type metadata and normalize it.""" + async with Client(mcp_server) as client: + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/append-decision", + "operation": "append", + "content": "# Append Decision\n\nDecision body.", + "metadata": {"type": "Decision Log"}, + }, + ) + + assert "Created note (append)" in edit_result.content[0].text + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "notes/append-decision"}, + ) + assert parse_frontmatter(read_result.content[0].text)["type"] == "decision_log" + + +@pytest.mark.asyncio +async def test_edit_note_prepend_metadata_type_applies_when_auto_creating_note( + mcp_server, app, test_project +): + """Prepend auto-create must follow the same type path as append.""" + async with Client(mcp_server) as client: + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/prepend-decision", + "operation": "prepend", + "content": "# Prepend Decision\n\nDecision body.", + "metadata": {"type": "decision"}, + }, + ) + + assert "Created note (prepend)" in edit_result.content[0].text + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "notes/prepend-decision"}, + ) + assert parse_frontmatter(read_result.content[0].text)["type"] == "decision" + + +@pytest.mark.asyncio +async def test_edit_note_auto_create_metadata_type_overrides_content_frontmatter( + mcp_server, app, test_project +): + """Explicit edit metadata must win over embedded frontmatter during creation.""" + async with Client(mcp_server) as client: + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/frontmatter-conflict", + "operation": "append", + "content": "---\ntype: incident\n---\n# Frontmatter Conflict\n\nDecision body.", + "metadata": {"type": "Decision Log"}, + }, + ) + + assert "Created note (append)" in edit_result.content[0].text + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "notes/frontmatter-conflict"}, + ) + assert parse_frontmatter(read_result.content[0].text)["type"] == "decision_log" + assert "Decision body." in read_result.content[0].text + + +@pytest.mark.asyncio +async def test_edit_note_auto_create_type_preserves_bom_frontmatter(mcp_server, app, test_project): + """Type precedence must not turn BOM-prefixed frontmatter into note body text.""" + async with Client(mcp_server) as client: + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/bom-frontmatter", + "operation": "append", + "content": "\ufeff---\ntype: incident\nstatus: draft\n---\n# BOM Note\n\nBody.", + "metadata": {"type": "decision"}, + }, + ) + + assert "Created note (append)" in edit_result.content[0].text + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "notes/bom-frontmatter"}, + ) + persisted = read_result.content[0].text + persisted_metadata = parse_frontmatter(persisted) + assert persisted_metadata["type"] == "decision" + assert persisted_metadata["status"] == "draft" + assert persisted.count("---") == 2 + assert "# BOM Note\n\nBody." in persisted + + +@pytest.mark.asyncio +async def test_edit_note_rejects_blank_metadata_type(mcp_server, app, test_project): + """A writable type field must reject an empty note classification.""" + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Type Validation Note", + "directory": "notes", + "content": "Original body.", + }, + ) + + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/type-validation-note", + "operation": "append", + "content": "", + "metadata": {"type": ""}, + }, + ) + + assert "Edit Failed" in edit_result.content[0].text + assert "at least 1 item" in edit_result.content[0].text + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "notes/type-validation-note"}, + ) + assert parse_frontmatter(read_result.content[0].text)["type"] == "note" + + @pytest.mark.asyncio async def test_edit_note_error_handling_text_not_found(mcp_server, app, test_project): """Test error handling when find_text is not found in the note.""" @@ -876,7 +1014,7 @@ async def test_edit_note_metadata_merges_frontmatter(mcp_server, app, test_proje @pytest.mark.asyncio async def test_edit_note_metadata_ignores_identity_fields(mcp_server, app, test_project): - """title/type/permalink in `metadata` are ignored rather than hijacking the note's identity.""" + """title/permalink in `metadata` are ignored rather than hijacking the note's identity.""" async with Client(mcp_server) as client: await client.call_tool( @@ -898,7 +1036,6 @@ async def test_edit_note_metadata_ignores_identity_fields(mcp_server, app, test_ "content": "", "metadata": { "title": "Hijacked Title", - "type": "hijacked", "permalink": "hijacked/permalink", "status": "resolved", }, @@ -922,6 +1059,52 @@ async def test_edit_note_metadata_ignores_identity_fields(mcp_server, app, test_ assert "status: draft" not in content +@pytest.mark.asyncio +async def test_edit_note_metadata_sets_note_type(mcp_server, app, test_project): + """`type` in `metadata` reaches both the file's frontmatter and the indexed entity.""" + + async with Client(mcp_server) as client: + await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Type Change Note", + "directory": "tickets", + "content": "# Type Change Note\n\nBody.", + }, + ) + + await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "tickets/type-change-note", + "operation": "append", + "content": "", + "metadata": {"type": "decision"}, + }, + ) + + read_result = await client.call_tool( + "read_note", + {"project": test_project.name, "identifier": "tickets/type-change-note"}, + ) + content = read_result.content[0].text + assert parse_frontmatter(content)["type"] == "decision" + assert "Body." in content + + # The index has to agree with the file, otherwise the note reverts on the next sync. + search_result = await client.call_tool( + "search_notes", + { + "project": test_project.name, + "query": "Type Change Note", + "note_types": ["decision"], + }, + ) + assert "tickets/type-change-note" in search_result.content[0].text + + @pytest.mark.asyncio async def test_edit_note_metadata_null_values_rejected_before_auto_create( mcp_server, app, test_project diff --git a/tests/services/test_entity_service_prepare.py b/tests/services/test_entity_service_prepare.py index 58b19613a..bd62d4506 100644 --- a/tests/services/test_entity_service_prepare.py +++ b/tests/services/test_entity_service_prepare.py @@ -680,6 +680,7 @@ async def test_prepare_edit_entity_content_metadata_ignores_identity_fields( entity_service, file_service, ) -> None: + """title and permalink stay under their own resolvers, whatever `metadata` says.""" created = await entity_service.create_entity( EntitySchema( title="Metadata Identity Guard", @@ -697,7 +698,6 @@ async def test_prepare_edit_entity_content_metadata_ignores_identity_fields( content="", metadata={ "title": "Hijacked Title", - "type": "hijacked", "permalink": "hijacked/permalink", "status": "resolved", }, @@ -709,6 +709,41 @@ async def test_prepare_edit_entity_content_metadata_ignores_identity_fields( assert prepared.entity_fields.permalink == created.permalink assert prepared_frontmatter["title"] == "Metadata Identity Guard" assert prepared_frontmatter["permalink"] == created.permalink + # An untouched note type is not collateral damage of the guard above. + assert prepared.entity_fields.note_type == "note" + + +@pytest.mark.asyncio +async def test_prepare_edit_entity_content_metadata_sets_note_type( + entity_service, + file_service, +) -> None: + """`type` is a plain frontmatter field: the merge writes it and the read-back keeps it.""" + created = await entity_service.create_entity( + EntitySchema( + title="Metadata Type Change", + directory="notes", + note_type="note", + content="Original body", + ) + ) + + current_content = await file_service.read_file_content(created.file_path) + prepared = await entity_service.prepare_edit_entity_content( + created, + current_content, + operation="append", + content="", + metadata={"type": "Decision Log"}, + ) + + prepared_frontmatter = parse_frontmatter(prepared.markdown_content) + assert prepared_frontmatter["type"] == "decision_log" + assert prepared.entity_fields.note_type == "decision_log" + # Setting the type is not a license to move the note. + assert prepared.entity_fields.title == "Metadata Type Change" + assert prepared.entity_fields.permalink == created.permalink + assert "Original body" in remove_frontmatter(prepared.markdown_content) @pytest.mark.asyncio @@ -826,10 +861,22 @@ async def test_prepare_edit_entity_content_metadata_rejects_null_values( def test_merge_metadata_into_markdown_identity_only_metadata_is_noop(): """A merge holding only identity fields must leave the markdown byte-identical.""" markdown = "---\nstatus: draft\n---\n\nBody \n" - merged = _merge_metadata_into_markdown(markdown, {"title": "X", "type": "y", "permalink": "z"}) + merged = _merge_metadata_into_markdown(markdown, {"title": "X", "permalink": "z"}) assert merged == markdown +def test_merge_metadata_into_markdown_writes_type(): + """`type` is merged like any other field, while title and permalink are still dropped.""" + markdown = "---\ntitle: Keep Me\ntype: note\npermalink: notes/keep-me\n---\n\nBody\n" + merged = _merge_metadata_into_markdown( + markdown, {"title": "X", "type": "decision", "permalink": "z"} + ) + merged_frontmatter = parse_frontmatter(merged) + assert merged_frontmatter["type"] == "decision" + assert merged_frontmatter["title"] == "Keep Me" + assert merged_frontmatter["permalink"] == "notes/keep-me" + + def test_merge_metadata_into_markdown_preserves_crlf_body(): """CRLF notes keep their body when the separator line is dropped for re-dumping.""" markdown = "---\r\nstatus: draft\r\n---\r\n\r\nBody line\r\n"