From e59b5cb6d97acc93d1cb00a475cf2dd48b9d0a07 Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 26 Feb 2026 19:00:34 -0600 Subject: [PATCH] feat: edit_note append/prepend auto-create note if not found (#614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When append or prepend targets a non-existent note, the tool now creates the file automatically instead of returning an error. This eliminates silent failures for plugins (like openclaw) that use edit_note(append) to build daily conversation notes โ€” on the first message of each day, the note didn't exist yet. - find_replace and replace_section still require an existing note - JSON output now includes `fileCreated: bool` in all responses - Path traversal security check applied to auto-created directories - Updated error messages to suggest append/prepend for missing notes ๐Ÿงช 25 unit tests, 14 MCP integration tests, 10 CLI integration tests โ€” all passing Co-Authored-By: Claude Opus 4.6 Signed-off-by: phernandez --- src/basic_memory/mcp/tools/edit_note.py | 194 ++++++++++++++---- .../test_cli_tool_edit_note_integration.py | 31 ++- test-int/mcp/test_edit_note_integration.py | 81 +++++++- tests/mcp/test_tool_edit_note.py | 132 +++++++++++- 4 files changed, 384 insertions(+), 54 deletions(-) diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index eedaea3b..63d71b67 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -7,6 +7,35 @@ from fastmcp import Context from basic_memory.mcp.project_context import get_project_client, add_project_metadata from basic_memory.mcp.server import mcp +from basic_memory.schemas.base import Entity +from basic_memory.utils import validate_project_path + + +def _parse_identifier_to_title_and_directory(identifier: str) -> tuple[str, str]: + """Parse an identifier into (title, directory) for creating a new note. + + Strips memory:// prefix if present, then splits on the last '/' to + separate the directory path from the note title. + + Examples: + "conversations/my-note" โ†’ ("my-note", "conversations") + "my-note" โ†’ ("my-note", "") + "a/b/c/my-note" โ†’ ("my-note", "a/b/c") + "memory://a/b/note" โ†’ ("note", "a/b") + """ + cleaned = identifier + if cleaned.startswith("memory://"): + cleaned = cleaned[len("memory://"):] + + if "/" in cleaned: + last_slash = cleaned.rfind("/") + directory = cleaned[:last_slash] + title = cleaned[last_slash + 1:] + else: + directory = "" + title = cleaned + + return title, directory def _format_error_response( @@ -19,15 +48,19 @@ def _format_error_response( ) -> str: """Format helpful error responses for edit_note failures that guide the AI to retry successfully.""" - # Entity not found errors + # Entity not found errors โ€” only reachable for find_replace/replace_section + # because append/prepend auto-create the note when it doesn't exist if "Entity not found" in error_message or "entity not found" in error_message.lower(): return f"""# Edit Failed - Note Not Found -The note with identifier '{identifier}' could not be found. Edit operations require an exact match (no fuzzy matching). +The note with identifier '{identifier}' could not be found. The `find_replace` and `replace_section` operations require an existing note with content to modify. + +**Tip:** `append` and `prepend` operations automatically create the note if it doesn't exist. ## Suggestions to try: -1. **Search for the note first**: Use `search_notes("{project or "project-name"}", "{identifier.split("/")[-1]}")` to find similar notes with exact identifiers -2. **Try different exact identifier formats**: +1. **Use append/prepend instead**: These operations will create the note automatically if it doesn't exist +2. **Search for the note first**: Use `search_notes("{project or "project-name"}", "{identifier.split("/")[-1]}")` to find similar notes with exact identifiers +3. **Try different exact identifier formats**: - If you used a permalink like "folder/note-title", try the exact title: "{identifier.split("/")[-1].replace("-", " ").title()}" - If you used a title, try the exact permalink format: "{identifier.lower().replace(" ", "-")}" - Use `read_note("{project or "project-name"}", "{identifier}")` first to verify the note exists and get the exact identifier @@ -152,10 +185,10 @@ async def edit_note( Must be an exact match - fuzzy matching is not supported for edit operations. Use search_notes() or read_note() first to find the correct identifier if uncertain. operation: The editing operation to perform: - - "append": Add content to the end of the note - - "prepend": Add content to the beginning of the note - - "find_replace": Replace occurrences of find_text with content - - "replace_section": Replace content under a specific markdown header + - "append": Add content to the end of the note (creates the note if it doesn't exist) + - "prepend": Add content to the beginning of the note (creates the note if it doesn't exist) + - "find_replace": Replace occurrences of find_text with content (note must exist) + - "replace_section": Replace content under a specific markdown header (note must exist) content: The content to add or use for replacement project: Project name to edit in. Optional - server will resolve using hierarchy. If unknown, use list_memory_projects() to discover available projects. @@ -243,48 +276,118 @@ async def edit_note( # Use typed KnowledgeClient for API calls knowledge_client = KnowledgeClient(client, active_project.external_id) - # Resolve identifier to entity ID - entity_id = await knowledge_client.resolve_entity(identifier) + file_created = False - # Prepare the edit request data - edit_data = { - "operation": operation, - "content": content, - } + # Try to resolve the entity; for append/prepend, create it if not found + try: + entity_id = await knowledge_client.resolve_entity(identifier) + except Exception as resolve_error: + # Trigger: entity does not exist yet + # Why: append/prepend can meaningfully create a new note from the content, + # while find_replace/replace_section require existing content to modify + # Outcome: note is created via the same path as write_note + error_msg = str(resolve_error).lower() + is_not_found = "entity not found" in error_msg or "not found" in error_msg - # Add optional parameters - if section: - edit_data["section"] = section - if find_text: - edit_data["find_text"] = find_text - if effective_replacements != 1: # Only send if different from default - edit_data["expected_replacements"] = str(effective_replacements) + if is_not_found and operation in ("append", "prepend"): + title, directory = _parse_identifier_to_title_and_directory(identifier) - # Call the PATCH endpoint - result = await knowledge_client.patch_entity(entity_id, edit_data, fast=False) + # Validate directory path (same security check as write_note) + project_path = active_project.home + if directory and not validate_project_path(directory, project_path): + logger.warning( + "Attempted path traversal attack blocked", + directory=directory, + project=active_project.name, + ) + if output_format == "json": + return { + "title": title, + "permalink": None, + "file_path": None, + "checksum": None, + "operation": operation, + "fileCreated": False, + "error": "SECURITY_VALIDATION_ERROR", + } + return f"# Error\n\nDirectory path '{directory}' is not allowed - paths must stay within project boundaries" - # Format summary - summary = [ - f"# Edited note ({operation})", - f"project: {active_project.name}", - f"file_path: {result.file_path}", - f"permalink: {result.permalink}", - f"checksum: {result.checksum[:8] if result.checksum else 'unknown'}", - ] + entity = Entity( + title=title, + directory=directory, + content_type="text/markdown", + content=content, + ) - # Add operation-specific details - if operation == "append": + logger.info( + "Creating note via edit_note auto-create", + title=title, + directory=directory, + operation=operation, + ) + result = await knowledge_client.create_entity( + entity.model_dump(), fast=False + ) + file_created = True + else: + # find_replace/replace_section require existing content โ€” re-raise + raise resolve_error + + # --- Standard edit path (entity already existed) --- + if not file_created: + # Prepare the edit request data + edit_data = { + "operation": operation, + "content": content, + } + + # Add optional parameters + if section: + edit_data["section"] = section + if find_text: + edit_data["find_text"] = find_text + if effective_replacements != 1: # Only send if different from default + edit_data["expected_replacements"] = str(effective_replacements) + + # Call the PATCH endpoint + result = await knowledge_client.patch_entity(entity_id, edit_data, fast=False) + + # --- Format response --- + if file_created: + summary = [ + f"# Created note ({operation})", + f"project: {active_project.name}", + f"file_path: {result.file_path}", + f"permalink: {result.permalink}", + f"checksum: {result.checksum[:8] if result.checksum else 'unknown'}", + "fileCreated: true", + ] lines_added = len(content.split("\n")) - summary.append(f"operation: Added {lines_added} lines to end of note") - elif operation == "prepend": - lines_added = len(content.split("\n")) - summary.append(f"operation: Added {lines_added} lines to beginning of note") - elif operation == "find_replace": - # For find_replace, we can't easily count replacements from here - # since we don't have the original content, but the server handled it - summary.append("operation: Find and replace operation completed") - elif operation == "replace_section": - summary.append(f"operation: Replaced content under section '{section}'") + summary.append(f"operation: Created note with {lines_added} lines") + else: + summary = [ + f"# Edited note ({operation})", + f"project: {active_project.name}", + f"file_path: {result.file_path}", + f"permalink: {result.permalink}", + f"checksum: {result.checksum[:8] if result.checksum else 'unknown'}", + ] + + # Add operation-specific details + if operation == "append": + lines_added = len(content.split("\n")) + summary.append(f"operation: Added {lines_added} lines to end of note") + elif operation == "prepend": + lines_added = len(content.split("\n")) + summary.append( + f"operation: Added {lines_added} lines to beginning of note" + ) + elif operation == "find_replace": + # For find_replace, we can't easily count replacements from here + # since we don't have the original content, but the server handled it + summary.append("operation: Find and replace operation completed") + elif operation == "replace_section": + summary.append(f"operation: Replaced content under section '{section}'") # Count observations by category (reuse logic from write_note) categories = {} @@ -316,6 +419,7 @@ async def edit_note( permalink=result.permalink, observations_count=len(result.observations), relations_count=len(result.relations), + file_created=file_created, ) if output_format == "json": @@ -325,6 +429,7 @@ async def edit_note( "file_path": result.file_path, "checksum": result.checksum, "operation": operation, + "fileCreated": file_created, } summary_result = "\n".join(summary) @@ -339,6 +444,7 @@ async def edit_note( "file_path": None, "checksum": None, "operation": operation, + "fileCreated": False, "error": str(e), } return _format_error_response( diff --git a/test-int/cli/test_cli_tool_edit_note_integration.py b/test-int/cli/test_cli_tool_edit_note_integration.py index 3e4b552d..97ee074a 100644 --- a/test-int/cli/test_cli_tool_edit_note_integration.py +++ b/test-int/cli/test_cli_tool_edit_note_integration.py @@ -211,6 +211,34 @@ def test_edit_note_replace_section_fails_without_section( assert "section parameter is required for replace_section operation" in result.output +def test_edit_note_append_creates_nonexistent_note_cli( + app, app_config, test_project, config_manager +): + """append to a non-existent note via CLI should auto-create and include fileCreated.""" + result = runner.invoke( + cli_app, + [ + "tool", + "edit-note", + "cli-tests/auto-created-note", + "--operation", + "append", + "--content", + "# Auto Created\n\nCreated via CLI append.", + ], + ) + + assert result.exit_code == 0, result.output + data = json.loads(result.stdout) + assert data["fileCreated"] is True + assert data["operation"] == "append" + assert data["title"] is not None + + # Verify the note is readable + read_data = _read_note(data["permalink"]) + assert "Auto Created" in read_data["content"] + + def test_edit_note_json_format_contract(app, app_config, test_project, config_manager): """JSON output returns metadata keys required by contract.""" note = _write_note( @@ -234,8 +262,9 @@ def test_edit_note_json_format_contract(app, app_config, test_project, config_ma assert result.exit_code == 0, result.output data = json.loads(result.stdout) - assert set(data.keys()) == {"title", "permalink", "file_path", "operation", "checksum"} + assert set(data.keys()) == {"title", "permalink", "file_path", "operation", "checksum", "fileCreated"} assert data["operation"] == "append" + assert data["fileCreated"] is False assert data["title"] == "Edit JSON Note" diff --git a/test-int/mcp/test_edit_note_integration.py b/test-int/mcp/test_edit_note_integration.py index 650ffea0..d5fdf74e 100644 --- a/test-int/mcp/test_edit_note_integration.py +++ b/test-int/mcp/test_edit_note_integration.py @@ -323,17 +323,18 @@ Current endpoints include user management.""" @pytest.mark.asyncio async def test_edit_note_error_handling_note_not_found(mcp_server, app, test_project): - """Test error handling when trying to edit a non-existent note.""" + """Test error handling when using find_replace on a non-existent note.""" async with Client(mcp_server) as client: - # Try to edit a note that doesn't exist + # find_replace on a non-existent note should still error edit_result = await client.call_tool( "edit_note", { "project": test_project.name, "identifier": "Non-existent Note", - "operation": "append", - "content": "Some content to add", + "operation": "find_replace", + "content": "replacement", + "find_text": "old text", }, ) @@ -345,6 +346,78 @@ async def test_edit_note_error_handling_note_not_found(mcp_server, app, test_pro assert "search_notes(" in error_text +@pytest.mark.asyncio +async def test_edit_note_append_creates_nonexistent_note(mcp_server, app, test_project): + """append to a non-existent note should auto-create it and make it readable.""" + + async with Client(mcp_server) as client: + # Append to a note that doesn't exist yet + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "conversations/daily-log", + "operation": "append", + "content": "# Daily Log\n\nFirst entry for today.", + }, + ) + + # Should return a "Created note" summary + assert len(edit_result.content) == 1 + edit_text = edit_result.content[0].text + assert "Created note (append)" in edit_text + assert "fileCreated: true" in edit_text + + # The note should now be readable + read_result = await client.call_tool( + "read_note", + { + "project": test_project.name, + "identifier": "conversations/daily-log", + }, + ) + + content = read_result.content[0].text + assert "Daily Log" in content + assert "First entry for today." in content + + +@pytest.mark.asyncio +async def test_edit_note_prepend_creates_nonexistent_note(mcp_server, app, test_project): + """prepend to a non-existent note should auto-create it and make it readable.""" + + async with Client(mcp_server) as client: + # Prepend to a note that doesn't exist yet + edit_result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "notes/quick-thought", + "operation": "prepend", + "content": "# Quick Thought\n\nSomething important.", + }, + ) + + # Should return a "Created note" summary + assert len(edit_result.content) == 1 + edit_text = edit_result.content[0].text + assert "Created note (prepend)" in edit_text + assert "fileCreated: true" in edit_text + + # The note should now be readable + read_result = await client.call_tool( + "read_note", + { + "project": test_project.name, + "identifier": "notes/quick-thought", + }, + ) + + content = read_result.content[0].text + assert "Quick Thought" in content + assert "Something important." in content + + @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.""" diff --git a/tests/mcp/test_tool_edit_note.py b/tests/mcp/test_tool_edit_note.py index 5f2f3f9f..4d27f739 100644 --- a/tests/mcp/test_tool_edit_note.py +++ b/tests/mcp/test_tool_edit_note.py @@ -120,19 +120,141 @@ async def test_edit_note_replace_section_operation(client, test_project): @pytest.mark.asyncio -async def test_edit_note_nonexistent_note(client, test_project): - """Test editing a note that doesn't exist - should return helpful guidance.""" +async def test_edit_note_nonexistent_note_find_replace(client, test_project): + """Test find_replace on a note that doesn't exist - should return helpful guidance.""" result = await edit_note( project=test_project.name, identifier="nonexistent/note", - operation="append", - content="Some content", + operation="find_replace", + content="replacement", + find_text="old text", ) assert isinstance(result, str) assert "# Edit Failed" in result assert "search_notes" in result # Should suggest searching - assert "read_note" in result # Should suggest reading to verify + assert "append" in result # Should suggest using append/prepend instead + + +@pytest.mark.asyncio +async def test_edit_note_nonexistent_note_replace_section(client, test_project): + """Test replace_section on a note that doesn't exist - should return helpful guidance.""" + result = await edit_note( + project=test_project.name, + identifier="nonexistent/note", + operation="replace_section", + content="new section content", + section="## Missing Section", + ) + + assert isinstance(result, str) + assert "# Edit Failed" in result + assert "search_notes" in result # Should suggest searching + + +@pytest.mark.asyncio +async def test_edit_note_append_creates_note_if_not_found(client, test_project): + """append to a non-existent note should create it automatically.""" + result = await edit_note( + project=test_project.name, + identifier="auto-created-note", + operation="append", + content="# New Note\n\nCreated via append.", + ) + + assert isinstance(result, str) + assert "Created note (append)" in result + assert "fileCreated: true" in result + assert f"project: {test_project.name}" in result + + +@pytest.mark.asyncio +async def test_edit_note_prepend_creates_note_if_not_found(client, test_project): + """prepend to a non-existent note should create it automatically.""" + result = await edit_note( + project=test_project.name, + identifier="auto-created-prepend", + operation="prepend", + content="# Prepended Note\n\nCreated via prepend.", + ) + + assert isinstance(result, str) + assert "Created note (prepend)" in result + assert "fileCreated: true" in result + assert f"project: {test_project.name}" in result + + +@pytest.mark.asyncio +async def test_edit_note_append_creates_with_directory_from_identifier(client, test_project): + """Identifier 'conversations/my-note' should create in conversations/ directory.""" + result = await edit_note( + project=test_project.name, + identifier="conversations/my-note", + operation="append", + content="# My Note\n\nCreated in conversations directory.", + ) + + assert isinstance(result, str) + assert "Created note (append)" in result + assert "fileCreated: true" in result + assert "conversations/" in result + + +@pytest.mark.asyncio +async def test_edit_note_append_creates_at_root_when_no_directory(client, test_project): + """Identifier 'my-note' (no slash) should create at project root.""" + result = await edit_note( + project=test_project.name, + identifier="root-level-note", + operation="append", + content="# Root Note\n\nCreated at root.", + ) + + assert isinstance(result, str) + assert "Created note (append)" in result + assert "fileCreated: true" in result + + +@pytest.mark.asyncio +async def test_edit_note_append_creates_json_format(client, test_project): + """JSON output should include fileCreated: true when note is auto-created.""" + result = await edit_note( + project=test_project.name, + identifier="json-auto-create", + operation="append", + content="# JSON Test\n\nAuto-created.", + output_format="json", + ) + + assert isinstance(result, dict) + assert result["fileCreated"] is True + assert result["title"] is not None + assert result["operation"] == "append" + + +@pytest.mark.asyncio +async def test_edit_note_existing_note_json_includes_file_created_false(client, test_project): + """JSON output for editing an existing note should include fileCreated: false.""" + # Create the note first + await write_note( + project=test_project.name, + title="Existing JSON Note", + directory="test", + content="# Existing Note\nOriginal content.", + ) + + result = await edit_note( + project=test_project.name, + identifier="test/existing-json-note", + operation="append", + content="\nAppended content.", + output_format="json", + ) + + assert isinstance(result, dict) + assert result["fileCreated"] is False + assert result["title"] == "Existing JSON Note" + assert result["operation"] == "append" @pytest.mark.asyncio