mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
@@ -133,6 +133,7 @@ Now searchable by: "coffee", "brewing", "equipment", or "Coffee Brewing Methods"
|
||||
- **`switch_project(project_name)`** - Change active project context during conversations
|
||||
- **`get_current_project()`** - Show currently active project with statistics
|
||||
- **`set_default_project(project_name)`** - Update default project configuration
|
||||
- **`sync_status()`** - Check file synchronization status and background operations
|
||||
|
||||
### New Note Operations Tools
|
||||
- **`edit_note()`** - Incremental note editing (append, prepend, find/replace, section replace)
|
||||
|
||||
@@ -185,7 +185,7 @@ async def delete_note(identifier: str, project: Optional[str] = None) -> bool |
|
||||
logger.warning(f"Delete operation completed but note was not deleted: {identifier}")
|
||||
return False
|
||||
|
||||
except Exception as e:
|
||||
except Exception as e: # pragma: no cover
|
||||
logger.error(f"Delete failed for '{identifier}': {e}")
|
||||
# Return formatted error message for better user experience
|
||||
return _format_delete_error_response(str(e), identifier)
|
||||
|
||||
@@ -56,7 +56,7 @@ async def read_note(
|
||||
from basic_memory.mcp.tools.utils import wait_for_migration_or_return_status
|
||||
|
||||
migration_status = await wait_for_migration_or_return_status(timeout=5.0)
|
||||
if migration_status: # pragma: no cover
|
||||
if migration_status: # pragma: no cover
|
||||
return f"# System Status\n\n{migration_status}\n\nPlease wait for migration to complete before reading notes."
|
||||
|
||||
active_project = get_active_project(project)
|
||||
|
||||
@@ -54,6 +54,23 @@ def _format_search_error_response(error_message: str, query: str, search_type: s
|
||||
Replace INSERT_CLEAN_QUERY_HERE with your simplified search terms.
|
||||
""").strip()
|
||||
|
||||
# Project not found errors (check before general "not found")
|
||||
if "project not found" in error_message.lower():
|
||||
return dedent(f"""
|
||||
# Search Failed - Project Not Found
|
||||
|
||||
The current project is not accessible or doesn't exist: {error_message}
|
||||
|
||||
## How to resolve:
|
||||
1. **Check available projects**: `list_projects()`
|
||||
2. **Switch to valid project**: `switch_project("valid-project-name")`
|
||||
3. **Verify project setup**: Ensure your project is properly configured
|
||||
|
||||
## Current session info:
|
||||
- Check current project: `get_current_project()`
|
||||
- See available projects: `list_projects()`
|
||||
""").strip()
|
||||
|
||||
# No results found
|
||||
if "no results" in error_message.lower() or "not found" in error_message.lower():
|
||||
simplified_query = (
|
||||
@@ -129,21 +146,6 @@ You don't have permission to search in the current project: {error_message}
|
||||
- Switch to accessible project: `switch_project("project-name")`
|
||||
- Check current project: `get_current_project()`"""
|
||||
|
||||
# Project not found errors
|
||||
if "project not found" in error_message.lower():
|
||||
return f"""# Search Failed - Project Not Found
|
||||
|
||||
The current project is not accessible or doesn't exist: {error_message}
|
||||
|
||||
## How to resolve:
|
||||
1. **Check available projects**: `list_projects()`
|
||||
2. **Switch to valid project**: `switch_project("valid-project-name")`
|
||||
3. **Verify project setup**: Ensure your project is properly configured
|
||||
|
||||
## Current session info:
|
||||
- Check current project: `get_current_project()`
|
||||
- See available projects: `list_projects()`"""
|
||||
|
||||
# Generic fallback
|
||||
return f"""# Search Failed
|
||||
|
||||
|
||||
@@ -550,6 +550,6 @@ async def wait_for_migration_or_return_status(timeout: float = 5.0) -> Optional[
|
||||
|
||||
# Still not ready after timeout
|
||||
return sync_status_tracker.get_summary()
|
||||
except Exception:
|
||||
except Exception: # pragma: no cover
|
||||
# If there's any error, assume ready
|
||||
return None
|
||||
|
||||
@@ -74,7 +74,7 @@ async def write_note(
|
||||
from basic_memory.mcp.tools.utils import wait_for_migration_or_return_status
|
||||
|
||||
migration_status = await wait_for_migration_or_return_status(timeout=5.0)
|
||||
if migration_status: # pragma: no cover
|
||||
if migration_status: # pragma: no cover
|
||||
return f"# System Status\n\n{migration_status}\n\nPlease wait for migration to complete before creating notes."
|
||||
|
||||
# Process tags using the helper function
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
"""Tests for delete_note MCP tool."""
|
||||
|
||||
from basic_memory.mcp.tools.delete_note import _format_delete_error_response
|
||||
|
||||
|
||||
class TestDeleteNoteErrorFormatting:
|
||||
"""Test the error formatting function for better user experience."""
|
||||
|
||||
def test_format_delete_error_note_not_found(self):
|
||||
"""Test formatting for note not found errors."""
|
||||
result = _format_delete_error_response("entity not found", "test-note")
|
||||
|
||||
assert "# Delete Failed - Note Not Found" in result
|
||||
assert "The note 'test-note' could not be found" in result
|
||||
assert 'search_notes("test-note")' in result
|
||||
assert "Already deleted" in result
|
||||
assert "Wrong identifier" in result
|
||||
|
||||
def test_format_delete_error_permission_denied(self):
|
||||
"""Test formatting for permission errors."""
|
||||
result = _format_delete_error_response("permission denied", "test-note")
|
||||
|
||||
assert "# Delete Failed - Permission Error" in result
|
||||
assert "You don't have permission to delete 'test-note'" in result
|
||||
assert "Check permissions" in result
|
||||
assert "File locks" in result
|
||||
assert "get_current_project()" in result
|
||||
|
||||
def test_format_delete_error_access_forbidden(self):
|
||||
"""Test formatting for access forbidden errors."""
|
||||
result = _format_delete_error_response("access forbidden", "test-note")
|
||||
|
||||
assert "# Delete Failed - Permission Error" in result
|
||||
assert "You don't have permission to delete 'test-note'" in result
|
||||
|
||||
def test_format_delete_error_server_error(self):
|
||||
"""Test formatting for server errors."""
|
||||
result = _format_delete_error_response("server error occurred", "test-note")
|
||||
|
||||
assert "# Delete Failed - System Error" in result
|
||||
assert "A system error occurred while deleting 'test-note'" in result
|
||||
assert "Try again" in result
|
||||
assert "Check file status" in result
|
||||
|
||||
def test_format_delete_error_filesystem_error(self):
|
||||
"""Test formatting for filesystem errors."""
|
||||
result = _format_delete_error_response("filesystem error", "test-note")
|
||||
|
||||
assert "# Delete Failed - System Error" in result
|
||||
assert "A system error occurred while deleting 'test-note'" in result
|
||||
|
||||
def test_format_delete_error_disk_error(self):
|
||||
"""Test formatting for disk errors."""
|
||||
result = _format_delete_error_response("disk full", "test-note")
|
||||
|
||||
assert "# Delete Failed - System Error" in result
|
||||
assert "A system error occurred while deleting 'test-note'" in result
|
||||
|
||||
def test_format_delete_error_database_error(self):
|
||||
"""Test formatting for database errors."""
|
||||
result = _format_delete_error_response("database error", "test-note")
|
||||
|
||||
assert "# Delete Failed - Database Error" in result
|
||||
assert "A database error occurred while deleting 'test-note'" in result
|
||||
assert "Sync conflict" in result
|
||||
assert "Database lock" in result
|
||||
|
||||
def test_format_delete_error_sync_error(self):
|
||||
"""Test formatting for sync errors."""
|
||||
result = _format_delete_error_response("sync failed", "test-note")
|
||||
|
||||
assert "# Delete Failed - Database Error" in result
|
||||
assert "A database error occurred while deleting 'test-note'" in result
|
||||
|
||||
def test_format_delete_error_generic(self):
|
||||
"""Test formatting for generic errors."""
|
||||
result = _format_delete_error_response("unknown error", "test-note")
|
||||
|
||||
assert "# Delete Failed" in result
|
||||
assert "Error deleting note 'test-note': unknown error" in result
|
||||
assert "General troubleshooting" in result
|
||||
assert "Verify the note exists" in result
|
||||
|
||||
def test_format_delete_error_with_complex_identifier(self):
|
||||
"""Test formatting with complex identifiers (permalinks)."""
|
||||
result = _format_delete_error_response("entity not found", "folder/note-title")
|
||||
|
||||
assert 'search_notes("note-title")' in result
|
||||
assert "Note Title" in result # Title format
|
||||
assert "folder/note-title" in result # Permalink format
|
||||
|
||||
|
||||
# Integration tests removed to focus on error formatting coverage
|
||||
# The error formatting tests above provide the necessary coverage for MCP tool error messaging
|
||||
@@ -1,8 +1,9 @@
|
||||
"""Tests for the move_note MCP tool."""
|
||||
|
||||
import pytest
|
||||
from unittest.mock import patch
|
||||
|
||||
from basic_memory.mcp.tools.move_note import move_note
|
||||
from basic_memory.mcp.tools.move_note import move_note, _format_move_error_response
|
||||
from basic_memory.mcp.tools.write_note import write_note
|
||||
from basic_memory.mcp.tools.read_note import read_note
|
||||
|
||||
@@ -419,3 +420,78 @@ async def test_move_note_preserves_frontmatter(app, client):
|
||||
assert "permalink: target/moved-custom-note" in content
|
||||
assert "# Custom Frontmatter Note" in content
|
||||
assert "Content with custom metadata" in content
|
||||
|
||||
|
||||
class TestMoveNoteErrorFormatting:
|
||||
"""Test move note error formatting for better user experience."""
|
||||
|
||||
def test_format_move_error_invalid_path(self):
|
||||
"""Test formatting for invalid path errors."""
|
||||
result = _format_move_error_response("invalid path format", "test-note", "/invalid/path.md")
|
||||
|
||||
assert "# Move Failed - Invalid Destination Path" in result
|
||||
assert "The destination path '/invalid/path.md' is not valid" in result
|
||||
assert "Relative paths only" in result
|
||||
assert "Include file extension" in result
|
||||
|
||||
def test_format_move_error_permission_denied(self):
|
||||
"""Test formatting for permission errors."""
|
||||
result = _format_move_error_response("permission denied", "test-note", "target/file.md")
|
||||
|
||||
assert "# Move Failed - Permission Error" in result
|
||||
assert "You don't have permission to move 'test-note'" in result
|
||||
assert "Check file permissions" in result
|
||||
assert "Check file locks" in result
|
||||
|
||||
def test_format_move_error_source_missing(self):
|
||||
"""Test formatting for source file missing errors."""
|
||||
result = _format_move_error_response("source file missing", "test-note", "target/file.md")
|
||||
|
||||
assert "# Move Failed - Source File Missing" in result
|
||||
assert "The source file for 'test-note' was not found on disk" in result
|
||||
assert "database and filesystem are out of sync" in result
|
||||
|
||||
def test_format_move_error_server_error(self):
|
||||
"""Test formatting for server errors."""
|
||||
result = _format_move_error_response("server error occurred", "test-note", "target/file.md")
|
||||
|
||||
assert "# Move Failed - System Error" in result
|
||||
assert "A system error occurred while moving 'test-note'" in result
|
||||
assert "Try again" in result
|
||||
assert "Check disk space" in result
|
||||
|
||||
|
||||
class TestMoveNoteErrorHandling:
|
||||
"""Test move note exception handling."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_exception_handling(self):
|
||||
"""Test exception handling in move_note."""
|
||||
with patch("basic_memory.mcp.tools.move_note.get_active_project") as mock_get_project:
|
||||
mock_get_project.return_value.project_url = "http://test"
|
||||
mock_get_project.return_value.name = "test-project"
|
||||
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.move_note.call_post",
|
||||
side_effect=Exception("entity not found"),
|
||||
):
|
||||
result = await move_note("test-note", "target/file.md")
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed - Note Not Found" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_move_note_permission_error_handling(self):
|
||||
"""Test permission error handling in move_note."""
|
||||
with patch("basic_memory.mcp.tools.move_note.get_active_project") as mock_get_project:
|
||||
mock_get_project.return_value.project_url = "http://test"
|
||||
mock_get_project.return_value.name = "test-project"
|
||||
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.move_note.call_post",
|
||||
side_effect=Exception("permission denied"),
|
||||
):
|
||||
result = await move_note("test-note", "target/file.md")
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Move Failed - Permission Error" in result
|
||||
|
||||
@@ -2,9 +2,10 @@
|
||||
|
||||
import pytest
|
||||
from datetime import datetime, timedelta
|
||||
from unittest.mock import patch
|
||||
|
||||
from basic_memory.mcp.tools import write_note
|
||||
from basic_memory.mcp.tools.search import search_notes
|
||||
from basic_memory.mcp.tools.search import search_notes, _format_search_error_response
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -157,3 +158,91 @@ async def test_search_with_date_filter(client):
|
||||
|
||||
# Verify we get results within timeframe
|
||||
assert len(response.results) > 0
|
||||
|
||||
|
||||
class TestSearchErrorFormatting:
|
||||
"""Test search error formatting for better user experience."""
|
||||
|
||||
def test_format_search_error_fts5_syntax(self):
|
||||
"""Test formatting for FTS5 syntax errors."""
|
||||
result = _format_search_error_response("syntax error in FTS5", "test query(")
|
||||
|
||||
assert "# Search Failed - Invalid Syntax" in result
|
||||
assert "The search query 'test query(' contains invalid syntax" in result
|
||||
assert "Special characters" in result
|
||||
assert "test query" in result # Clean query without special chars
|
||||
|
||||
def test_format_search_error_no_results(self):
|
||||
"""Test formatting for no results found."""
|
||||
result = _format_search_error_response("no results found", "very specific query")
|
||||
|
||||
assert "# Search Complete - No Results Found" in result
|
||||
assert "No content found matching 'very specific query'" in result
|
||||
assert "Broaden your search" in result
|
||||
assert "very" in result # Simplified query
|
||||
|
||||
def test_format_search_error_server_error(self):
|
||||
"""Test formatting for server errors."""
|
||||
result = _format_search_error_response("internal server error", "test query")
|
||||
|
||||
assert "# Search Failed - Server Error" in result
|
||||
assert "The search service encountered an error while processing 'test query'" in result
|
||||
assert "Try again" in result
|
||||
assert "Check project status" in result
|
||||
|
||||
def test_format_search_error_permission_denied(self):
|
||||
"""Test formatting for permission errors."""
|
||||
result = _format_search_error_response("permission denied", "test query")
|
||||
|
||||
assert "# Search Failed - Access Error" in result
|
||||
assert "You don't have permission to search" in result
|
||||
assert "Check your project access" in result
|
||||
|
||||
def test_format_search_error_project_not_found(self):
|
||||
"""Test formatting for project not found errors."""
|
||||
result = _format_search_error_response("current project not found", "test query")
|
||||
|
||||
assert "# Search Failed - Project Not Found" in result
|
||||
assert "The current project is not accessible" in result
|
||||
assert "Check available projects" in result
|
||||
|
||||
def test_format_search_error_generic(self):
|
||||
"""Test formatting for generic errors."""
|
||||
result = _format_search_error_response("unknown error", "test query")
|
||||
|
||||
assert "# Search Failed" in result
|
||||
assert "Error searching for 'test query': unknown error" in result
|
||||
assert "General troubleshooting" in result
|
||||
|
||||
|
||||
class TestSearchToolErrorHandling:
|
||||
"""Test search tool exception handling."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_notes_exception_handling(self):
|
||||
"""Test exception handling in search_notes."""
|
||||
with patch("basic_memory.mcp.tools.search.get_active_project") as mock_get_project:
|
||||
mock_get_project.return_value.project_url = "http://test"
|
||||
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.search.call_post", side_effect=Exception("syntax error")
|
||||
):
|
||||
result = await search_notes("test query")
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Search Failed - Invalid Syntax" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_notes_permission_error(self):
|
||||
"""Test search_notes with permission error."""
|
||||
with patch("basic_memory.mcp.tools.search.get_active_project") as mock_get_project:
|
||||
mock_get_project.return_value.project_url = "http://test"
|
||||
|
||||
with patch(
|
||||
"basic_memory.mcp.tools.search.call_post",
|
||||
side_effect=Exception("permission denied"),
|
||||
):
|
||||
result = await search_notes("test query")
|
||||
|
||||
assert isinstance(result, str)
|
||||
assert "# Search Failed - Access Error" in result
|
||||
|
||||
@@ -1,12 +1,20 @@
|
||||
"""Tests for MCP tool utilities."""
|
||||
|
||||
from unittest.mock import AsyncMock
|
||||
from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient, HTTPStatusError
|
||||
from mcp.server.fastmcp.exceptions import ToolError
|
||||
|
||||
from basic_memory.mcp.tools.utils import call_get, call_post, call_put, call_delete
|
||||
from basic_memory.mcp.tools.utils import (
|
||||
call_get,
|
||||
call_post,
|
||||
call_put,
|
||||
call_delete,
|
||||
get_error_message,
|
||||
check_migration_status,
|
||||
wait_for_migration_or_return_status,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -135,7 +143,6 @@ async def test_call_get_with_params(mock_response):
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_error_message():
|
||||
"""Test the get_error_message function."""
|
||||
from basic_memory.mcp.tools.utils import get_error_message
|
||||
|
||||
# Test 400 status code
|
||||
message = get_error_message(400, "http://test.com/resource", "GET")
|
||||
@@ -177,3 +184,82 @@ async def test_call_post_with_json(mock_response):
|
||||
mock_post.assert_called_once()
|
||||
call_kwargs = mock_post.call_args[1]
|
||||
assert call_kwargs["json"] == json_data
|
||||
|
||||
|
||||
class TestMigrationStatus:
|
||||
"""Test migration status checking functions."""
|
||||
|
||||
def test_check_migration_status_ready(self):
|
||||
"""Test check_migration_status when system is ready."""
|
||||
mock_tracker = MagicMock()
|
||||
mock_tracker.is_ready = True
|
||||
|
||||
with patch("basic_memory.services.sync_status_service.sync_status_tracker", mock_tracker):
|
||||
result = check_migration_status()
|
||||
assert result is None
|
||||
|
||||
def test_check_migration_status_not_ready(self):
|
||||
"""Test check_migration_status when sync is in progress."""
|
||||
mock_tracker = MagicMock()
|
||||
mock_tracker.is_ready = False
|
||||
mock_tracker.get_summary.return_value = "Sync in progress..."
|
||||
|
||||
with patch("basic_memory.services.sync_status_service.sync_status_tracker", mock_tracker):
|
||||
result = check_migration_status()
|
||||
assert result == "Sync in progress..."
|
||||
mock_tracker.get_summary.assert_called_once()
|
||||
|
||||
def test_check_migration_status_exception(self):
|
||||
"""Test check_migration_status with import/other exception."""
|
||||
# Mock the import itself to raise an exception
|
||||
with patch("builtins.__import__", side_effect=ImportError("Module not found")):
|
||||
result = check_migration_status()
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_migration_ready(self):
|
||||
"""Test wait_for_migration when system is already ready."""
|
||||
mock_tracker = MagicMock()
|
||||
mock_tracker.is_ready = True
|
||||
|
||||
with patch("basic_memory.services.sync_status_service.sync_status_tracker", mock_tracker):
|
||||
result = await wait_for_migration_or_return_status()
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_migration_becomes_ready(self):
|
||||
"""Test wait_for_migration when system becomes ready during wait."""
|
||||
mock_tracker = MagicMock()
|
||||
mock_tracker.is_ready = False
|
||||
|
||||
with patch("basic_memory.services.sync_status_service.sync_status_tracker", mock_tracker):
|
||||
# Mock asyncio.sleep to make tracker ready after first check
|
||||
async def mock_sleep(delay):
|
||||
mock_tracker.is_ready = True
|
||||
|
||||
with patch("asyncio.sleep", side_effect=mock_sleep):
|
||||
result = await wait_for_migration_or_return_status(timeout=1.0)
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_migration_timeout(self):
|
||||
"""Test wait_for_migration when timeout occurs."""
|
||||
mock_tracker = MagicMock()
|
||||
mock_tracker.is_ready = False
|
||||
mock_tracker.get_summary.return_value = "Still syncing..."
|
||||
|
||||
with patch("basic_memory.services.sync_status_service.sync_status_tracker", mock_tracker):
|
||||
with patch("asyncio.sleep", new_callable=AsyncMock):
|
||||
result = await wait_for_migration_or_return_status(timeout=0.1)
|
||||
assert result == "Still syncing..."
|
||||
mock_tracker.get_summary.assert_called_once()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_wait_for_migration_exception(self):
|
||||
"""Test wait_for_migration with exception during checking."""
|
||||
with patch(
|
||||
"basic_memory.services.sync_status_service.sync_status_tracker",
|
||||
side_effect=Exception("Test error"),
|
||||
):
|
||||
result = await wait_for_migration_or_return_status()
|
||||
assert result is None
|
||||
|
||||
Reference in New Issue
Block a user