From 602c55fe90c1d45540e09d4f587eb7096bb3a1dc Mon Sep 17 00:00:00 2001 From: phernandez Date: Wed, 4 Jun 2025 23:37:29 -0500 Subject: [PATCH] only allow edit_note, move_note using strict identifier match Signed-off-by: phernandez --- .claude/commands/test-live.md | 8 +- src/basic_memory/mcp/tools/edit_note.py | 28 ++-- src/basic_memory/mcp/tools/move_note.py | 33 +++-- src/basic_memory/services/entity_service.py | 8 +- src/basic_memory/services/link_resolver.py | 17 ++- tests/services/test_link_resolver.py | 136 ++++++++++++++++++++ 6 files changed, 199 insertions(+), 31 deletions(-) diff --git a/.claude/commands/test-live.md b/.claude/commands/test-live.md index 36fd0132..11a7c70e 100644 --- a/.claude/commands/test-live.md +++ b/.claude/commands/test-live.md @@ -12,7 +12,8 @@ Execute comprehensive real-world testing of Basic Memory using the installed ver ## Implementation -You are an expert QA engineer conducting live testing of Basic Memory. When the user runs `/project:test-live`, execute comprehensive testing following the TESTING.md methodology: +You are an expert QA engineer conducting live testing of Basic Memory. +When the user runs `/project:test-live`, execute comprehensive testing following the TESTING.md methodology: ### Pre-Test Setup @@ -22,12 +23,17 @@ You are an expert QA engineer conducting live testing of Basic Memory. When the - Test MCP connection and tool availability 2. **Test Project Creation** + +Run the bash `date` command to get the current date/time. + ``` Create project: "basic-memory-testing-[timestamp]" Location: ~/basic-memory-testing-[timestamp] Purpose: Record all test observations and results ``` +Make sure to switch to the newly created project with the `switch_project()` tool. + 3. **Baseline Documentation** Create initial test session note with: - Test environment details diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index 79d70d55..35caf602 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -24,14 +24,14 @@ def _format_error_response( 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. +The note with identifier '{identifier}' could not be found. Edit operations require an exact match (no fuzzy matching). ## Suggestions to try: -1. **Search for the note first**: Use `search_notes("{identifier.split("/")[-1]}")` to find similar notes -2. **Try different identifier formats**: - - If you used a permalink like "folder/note-title", try just the title: "{identifier.split("/")[-1].replace("-", " ").title()}" - - If you used a title, try the permalink format: "{identifier.lower().replace(" ", "-")}" - - Use `read_note()` first to verify the note exists and get the correct identifiers +1. **Search for the note first**: Use `search_notes("{identifier.split("/")[-1]}")` to find similar notes with exact identifiers +2. **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()` first to verify the note exists and get the exact identifier ## Alternative approach: Use `write_note()` to create the note first, then edit it.""" @@ -142,7 +142,9 @@ async def edit_note( It supports various operations for different editing scenarios. Args: - identifier: The title, permalink, or memory:// URL of the note to edit + identifier: The exact title, permalink, or memory:// URL of the note to edit. + 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 @@ -179,10 +181,14 @@ async def edit_note( # Replace subsection with more specific header edit_note("docs/setup", "replace_section", "Updated install steps\\n", section="### Installation") - # Using different identifier formats - edit_note("Meeting Notes", "append", "\\n- Follow up on action items") # title - edit_note("docs/meeting-notes", "append", "\\n- Follow up tasks") # permalink - edit_note("docs/Meeting Notes", "append", "\\n- Next steps") # folder/title + # Using different identifier formats (must be exact matches) + edit_note("Meeting Notes", "append", "\\n- Follow up on action items") # exact title + edit_note("docs/meeting-notes", "append", "\\n- Follow up tasks") # exact permalink + edit_note("docs/Meeting Notes", "append", "\\n- Next steps") # exact folder/title + + # If uncertain about identifier, search first: + # search_notes("meeting") # Find available notes + # edit_note("docs/meeting-notes-2025", "append", "content") # Use exact result # Add new section to document edit_note("project-plan", "replace_section", "TBD - needs research\\n", section="## Future Work") diff --git a/src/basic_memory/mcp/tools/move_note.py b/src/basic_memory/mcp/tools/move_note.py index a439e008..1b774faf 100644 --- a/src/basic_memory/mcp/tools/move_note.py +++ b/src/basic_memory/mcp/tools/move_note.py @@ -26,14 +26,14 @@ def _format_move_error_response(error_message: str, identifier: str, destination return dedent(f""" # Move Failed - Note Not Found - The note '{identifier}' could not be found for moving. + The note '{identifier}' could not be found for moving. Move operations require an exact match (no fuzzy matching). ## Suggestions to try: - 1. **Search for the note first**: Use `search_notes("{search_term}")` to find it - 2. **Try different identifier formats**: - - If you used a permalink like "folder/note-title", try just the title: "{title_format}" - - If you used a title, try the permalink format: "{permalink_format}" - - Use `read_note()` first to verify the note exists and get the correct identifier + 1. **Search for the note first**: Use `search_notes("{search_term}")` to find it with exact identifiers + 2. **Try different exact identifier formats**: + - If you used a permalink like "folder/note-title", try the exact title: "{title_format}" + - If you used a title, try the exact permalink format: "{permalink_format}" + - Use `read_note()` first to verify the note exists and get the exact identifier 3. **Check current project**: Use `get_current_project()` to verify you're in the right project 4. **List available notes**: Use `list_directory("/")` to see what notes exist @@ -43,7 +43,7 @@ def _format_move_error_response(error_message: str, identifier: str, destination # First, verify the note exists: search_notes("{identifier}") - # Then use the correct identifier from search results: + # Then use the exact identifier from search results: move_note("correct-identifier-here", "{destination_path}") ``` """).strip() @@ -220,7 +220,9 @@ async def move_note( """Move a note to a new file location within the same project. Args: - identifier: Entity identifier (title, permalink, or memory:// URL) + identifier: Exact entity identifier (title, permalink, or memory:// URL). + Must be an exact match - fuzzy matching is not supported for move operations. + Use search_notes() or read_note() first to find the correct identifier if uncertain. destination_path: New path relative to project root (e.g., "work/meetings/2025-05-26.md") project: Optional project name (defaults to current session project) @@ -228,9 +230,18 @@ async def move_note( Success message with move details Examples: - - Move to new folder: move_note("My Note", "work/notes/my-note.md") - - Move by permalink: move_note("my-note-permalink", "archive/old-notes/my-note.md") - - Specify project: move_note("My Note", "archive/my-note.md", project="work-project") + # Move to new folder (exact title match) + move_note("My Note", "work/notes/my-note.md") + + # Move by exact permalink + move_note("my-note-permalink", "archive/old-notes/my-note.md") + + # Specify project with exact identifier + move_note("My Note", "archive/my-note.md", project="work-project") + + # If uncertain about identifier, search first: + # search_notes("my note") # Find available notes + # move_note("docs/my-note-2025", "archive/my-note.md") # Use exact result Note: This operation moves notes within the specified project only. Moving notes between different projects is not currently supported. diff --git a/src/basic_memory/services/entity_service.py b/src/basic_memory/services/entity_service.py index 6ca8c51b..dd3ea7a9 100644 --- a/src/basic_memory/services/entity_service.py +++ b/src/basic_memory/services/entity_service.py @@ -413,8 +413,8 @@ class EntityService(BaseService[EntityModel]): """ logger.debug(f"Editing entity: {identifier}, operation: {operation}") - # Find the entity using the link resolver - entity = await self.link_resolver.resolve_link(identifier) + # Find the entity using the link resolver with strict mode for destructive operations + entity = await self.link_resolver.resolve_link(identifier, strict=True) if not entity: raise EntityNotFoundError(f"Entity not found: {identifier}") @@ -630,8 +630,8 @@ class EntityService(BaseService[EntityModel]): """ logger.debug(f"Moving entity: {identifier} to {destination_path}") - # 1. Resolve identifier to entity - entity = await self.link_resolver.resolve_link(identifier) + # 1. Resolve identifier to entity with strict mode for destructive operations + entity = await self.link_resolver.resolve_link(identifier, strict=True) if not entity: raise EntityNotFoundError(f"Entity not found: {identifier}") diff --git a/src/basic_memory/services/link_resolver.py b/src/basic_memory/services/link_resolver.py index ec8cfa1f..e36baf1d 100644 --- a/src/basic_memory/services/link_resolver.py +++ b/src/basic_memory/services/link_resolver.py @@ -26,8 +26,14 @@ class LinkResolver: self.entity_repository = entity_repository self.search_service = search_service - async def resolve_link(self, link_text: str, use_search: bool = True) -> Optional[Entity]: - """Resolve a markdown link to a permalink.""" + async def resolve_link(self, link_text: str, use_search: bool = True, strict: bool = False) -> Optional[Entity]: + """Resolve a markdown link to a permalink. + + Args: + link_text: The link text to resolve + use_search: Whether to use search-based fuzzy matching as fallback + strict: If True, only exact matches are allowed (no fuzzy search fallback) + """ logger.trace(f"Resolving link: {link_text}") # Clean link text and extract any alias @@ -60,9 +66,12 @@ class LinkResolver: logger.debug(f"Found entity with path (with .md): {found_path_md.file_path}") return found_path_md - # search if indicated + # In strict mode, don't try fuzzy search - return None if no exact match found + if strict: + return None + + # 5. Fall back to search for fuzzy matching (only if not in strict mode) if use_search and "*" not in clean_text: - # 5. Fall back to search for fuzzy matching on title (use text search for prefix matching) results = await self.search_service.search( query=SearchQuery(text=clean_text, entity_types=[SearchItemType.ENTITY]), ) diff --git a/tests/services/test_link_resolver.py b/tests/services/test_link_resolver.py index 3e3c13ca..dd05b902 100644 --- a/tests/services/test_link_resolver.py +++ b/tests/services/test_link_resolver.py @@ -220,3 +220,139 @@ async def test_folder_title_pattern_with_md_extension(link_resolver, test_entiti entity = await link_resolver.resolve_link("components/core-service") assert entity is not None assert entity.permalink == "components/core-service" + + +# Tests for strict mode parameter combinations +@pytest.mark.asyncio +async def test_strict_mode_parameter_combinations(link_resolver, test_entities): + """Test all combinations of use_search and strict parameters.""" + + # Test queries + exact_match = "Auth Service" # Should always work (unique title) + fuzzy_match = "Auth Serv" # Should only work with fuzzy search enabled + non_existent = "Does Not Exist" # Should never work + + # Case 1: use_search=True, strict=False (default behavior - fuzzy matching allowed) + result = await link_resolver.resolve_link(exact_match, use_search=True, strict=False) + assert result is not None + assert result.permalink == "components/auth-service" + + result = await link_resolver.resolve_link(fuzzy_match, use_search=True, strict=False) + assert result is not None # Should find "Auth Service" via fuzzy matching + assert result.permalink == "components/auth-service" + + result = await link_resolver.resolve_link(non_existent, use_search=True, strict=False) + assert result is None + + # Case 2: use_search=True, strict=True (exact matches only, even with search enabled) + result = await link_resolver.resolve_link(exact_match, use_search=True, strict=True) + assert result is not None + assert result.permalink == "components/auth-service" + + result = await link_resolver.resolve_link(fuzzy_match, use_search=True, strict=True) + assert result is None # Should NOT find via fuzzy matching in strict mode + + result = await link_resolver.resolve_link(non_existent, use_search=True, strict=True) + assert result is None + + # Case 3: use_search=False, strict=False (no search, exact repository matches only) + result = await link_resolver.resolve_link(exact_match, use_search=False, strict=False) + assert result is not None + assert result.permalink == "components/auth-service" + + result = await link_resolver.resolve_link(fuzzy_match, use_search=False, strict=False) + assert result is None # No search means no fuzzy matching + + result = await link_resolver.resolve_link(non_existent, use_search=False, strict=False) + assert result is None + + # Case 4: use_search=False, strict=True (redundant but should work same as case 3) + result = await link_resolver.resolve_link(exact_match, use_search=False, strict=True) + assert result is not None + assert result.permalink == "components/auth-service" + + result = await link_resolver.resolve_link(fuzzy_match, use_search=False, strict=True) + assert result is None # No search means no fuzzy matching + + result = await link_resolver.resolve_link(non_existent, use_search=False, strict=True) + assert result is None + + +@pytest.mark.asyncio +async def test_exact_match_types_in_strict_mode(link_resolver, test_entities): + """Test that all types of exact matches work in strict mode.""" + + # 1. Exact permalink match + result = await link_resolver.resolve_link("components/core-service", strict=True) + assert result is not None + assert result.permalink == "components/core-service" + + # 2. Exact title match + result = await link_resolver.resolve_link("Core Service", strict=True) + assert result is not None + assert result.permalink == "components/core-service" + + # 3. Exact file path match + result = await link_resolver.resolve_link("components/Core Service.md", strict=True) + assert result is not None + assert result.permalink == "components/core-service" + + # 4. Folder/title pattern with .md extension added + result = await link_resolver.resolve_link("components/Core Service", strict=True) + assert result is not None + assert result.permalink == "components/core-service" + + # 5. Non-markdown file (Image.png) + result = await link_resolver.resolve_link("Image.png", strict=True) + assert result is not None + assert result.title == "Image.png" + + +@pytest.mark.asyncio +async def test_fuzzy_matching_blocked_in_strict_mode(link_resolver, test_entities): + """Test that various fuzzy matching scenarios are blocked in strict mode.""" + + # Partial matches that would work in normal mode + fuzzy_queries = [ + "Auth Serv", # Partial title + "auth-service", # Lowercase permalink variation + "Core", # Single word from title + "Service", # Common word + "Serv", # Partial word + ] + + for query in fuzzy_queries: + # Should NOT work in strict mode + strict_result = await link_resolver.resolve_link(query, strict=True) + assert strict_result is None, f"Query '{query}' should return None in strict mode" + + +@pytest.mark.asyncio +async def test_link_normalization_with_strict_mode(link_resolver, test_entities): + """Test that link normalization still works in strict mode.""" + + # Test bracket removal and alias handling in strict mode + queries_and_expected = [ + ("[[Core Service]]", "components/core-service"), + ("[[Core Service|Main]]", "components/core-service"), # Alias should be ignored + (" [[ Core Service ]] ", "components/core-service"), # Extra whitespace + ] + + for query, expected_permalink in queries_and_expected: + result = await link_resolver.resolve_link(query, strict=True) + assert result is not None, f"Query '{query}' should find entity in strict mode" + assert result.permalink == expected_permalink + + +@pytest.mark.asyncio +async def test_duplicate_title_handling_in_strict_mode(link_resolver, test_entities): + """Test how duplicate titles are handled in strict mode.""" + + # "Core Service" appears twice in test data (components/core-service and components2/core-service) + # In strict mode, if there are multiple exact title matches, it should still return the first one + # (same behavior as normal mode for exact matches) + + result = await link_resolver.resolve_link("Core Service", strict=True) + assert result is not None + # Should return the first match (components/core-service based on test fixture order) + assert result.permalink == "components/core-service"