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 736b6f5db..3e7faa7be 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -437,9 +437,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. 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 23ed69e2a..65f244ffc 100644 --- a/src/basic_memory/services/note_preparation.py +++ b/src/basic_memory/services/note_preparation.py @@ -689,18 +689,20 @@ 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"}) 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: diff --git a/test-int/mcp/test_edit_note_integration.py b/test-int/mcp/test_edit_note_integration.py index 0f5c0783c..b00a8ab61 100644 --- a/test-int/mcp/test_edit_note_integration.py +++ b/test-int/mcp/test_edit_note_integration.py @@ -876,7 +876,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 +898,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 +921,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..fe058e98b 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"}, + ) + + prepared_frontmatter = parse_frontmatter(prepared.markdown_content) + assert prepared_frontmatter["type"] == "decision" + assert prepared.entity_fields.note_type == "decision" + # 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"