From 3f8a8004bbd6d8dbaca950aafd5ffe5d80c720d0 Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 5 Jan 2026 19:43:40 -0600 Subject: [PATCH 1/4] fix: preserve search index across server restarts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove unconditional DROP TABLE from init_search_index() that was wiping the FTS5 search index on every MCP server startup. The bug was introduced in PR #439 (v0.16.3) when splitting SQLite and Postgres search implementations. The DROP TABLE was added with a comment about Base.metadata.create_all() potentially creating a regular table, but this caused all indexed data to be lost whenever Claude Desktop restarted the MCP server. Symptoms users experienced: - Notes searchable immediately after creation - Search returns empty results after ~30 minutes (server restart) - Files exist on disk but search index is empty - recent_activity() shows "No recent activity" The fix simply removes the DROP TABLE since CREATE_SEARCH_INDEX already uses "CREATE VIRTUAL TABLE IF NOT EXISTS" which safely handles both cases (table exists vs doesn't exist). Affected users should run `basic-memory reset` once after updating to rebuild their search index. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Signed-off-by: phernandez --- .../repository/sqlite_search_repository.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/basic_memory/repository/sqlite_search_repository.py b/src/basic_memory/repository/sqlite_search_repository.py index 3fd12235f..66a39ccfc 100644 --- a/src/basic_memory/repository/sqlite_search_repository.py +++ b/src/basic_memory/repository/sqlite_search_repository.py @@ -27,17 +27,15 @@ class SQLiteSearchRepository(SearchRepositoryBase): """ async def init_search_index(self): - """Create FTS5 virtual table for search. + """Create FTS5 virtual table for search if it doesn't exist. - Note: Drops any existing search_index table first to ensure FTS5 virtual table creation. - This is necessary because Base.metadata.create_all() might create a regular table. + Uses CREATE VIRTUAL TABLE IF NOT EXISTS to preserve existing indexed data + across server restarts. """ logger.info("Initializing SQLite FTS5 search index") try: async with db.scoped_session(self.session_maker) as session: - # Drop any existing regular or virtual table first - await session.execute(text("DROP TABLE IF EXISTS search_index")) - # Create FTS5 virtual table + # Create FTS5 virtual table if it doesn't exist await session.execute(CREATE_SEARCH_INDEX) await session.commit() except Exception as e: # pragma: no cover From 2950bb9971a909661296a963ec7390ea5042e6f9 Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 5 Jan 2026 19:53:21 -0600 Subject: [PATCH 2/4] fix: improve reset command reliability and UX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Skip database initialization for reset command (like mcp/status/etc) - Delete WAL files (-shm, -wal) along with main database file - Add user-friendly error messages when database is locked - Show reassuring message that note files won't be deleted 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Signed-off-by: phernandez --- src/basic_memory/cli/app.py | 5 ++-- src/basic_memory/cli/commands/db.py | 40 ++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/basic_memory/cli/app.py b/src/basic_memory/cli/app.py index ea7497357..d6e74f9a0 100644 --- a/src/basic_memory/cli/app.py +++ b/src/basic_memory/cli/app.py @@ -63,11 +63,12 @@ def app_callback( # Run initialization for commands that don't use the API # Skip for 'mcp' command - it has its own lifespan that handles initialization # Skip for API-using commands (status, sync, etc.) - they handle initialization via deps.py - api_commands = {"mcp", "status", "sync", "project", "tool"} + # Skip for 'reset' command - it manages its own database lifecycle + skip_init_commands = {"mcp", "status", "sync", "project", "tool", "reset"} if ( not version and ctx.invoked_subcommand is not None - and ctx.invoked_subcommand not in api_commands + and ctx.invoked_subcommand not in skip_init_commands ): from basic_memory.services.initialization import ensure_initialization diff --git a/src/basic_memory/cli/commands/db.py b/src/basic_memory/cli/commands/db.py index 93a421cd1..430b120d0 100644 --- a/src/basic_memory/cli/commands/db.py +++ b/src/basic_memory/cli/commands/db.py @@ -4,28 +4,46 @@ import typer from loguru import logger +from rich.console import Console +from sqlalchemy.exc import OperationalError from basic_memory import db from basic_memory.cli.app import app from basic_memory.config import ConfigManager, BasicMemoryConfig, save_basic_memory_config +console = Console() + @app.command() def reset( reindex: bool = typer.Option(False, "--reindex", help="Rebuild db index from filesystem"), ): # pragma: no cover """Reset database (drop all tables and recreate).""" - if typer.confirm("This will delete all data in your db. Are you sure?"): + console.print( + "[yellow]Note:[/yellow] This only deletes the index database. " + "Your markdown note files will not be affected." + ) + if typer.confirm("Reset the database index? (You can rebuild it with 'bm sync')"): logger.info("Resetting database...") config_manager = ConfigManager() app_config = config_manager.config # Get database path db_path = app_config.app_database_path - # Delete the database file if it exists - if db_path.exists(): - db_path.unlink() - logger.info(f"Database file deleted: {db_path}") + # Delete the database file and WAL files if they exist + for suffix in ["", "-shm", "-wal"]: + path = db_path.parent / f"{db_path.name}{suffix}" + if path.exists(): + try: + path.unlink() + logger.info(f"Deleted: {path}") + except OSError as e: + console.print( + f"[red]Error:[/red] Cannot delete {path.name}: {e}\n" + "The database may be in use by another process (e.g., MCP server).\n" + "Please close Claude Desktop or any other Basic Memory clients and try again." + ) + raise typer.Exit(1) # Reset project configuration config = BasicMemoryConfig() @@ -33,7 +51,17 @@ def reset( logger.info("Project configuration reset to default") # Create a new empty database - asyncio.run(db.run_migrations(app_config)) + try: + asyncio.run(db.run_migrations(app_config)) + except OperationalError as e: + if "disk I/O error" in str(e) or "database is locked" in str(e): + console.print( + "[red]Error:[/red] Cannot access database. " + "It may be in use by another process (e.g., MCP server).\n" + "Please close Claude Desktop or any other Basic Memory clients and try again." + ) + raise typer.Exit(1) + raise logger.info("Database reset complete") if reindex: From f2d906d487cffcfe3bf44b33b758185d733c2fd9 Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 5 Jan 2026 20:47:39 -0600 Subject: [PATCH 3/4] fix: resolve reset --reindex hanging and improve sync API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add run_in_background parameter to run_sync() for foreground sync - Handle both background and foreground response formats - Refactor reset command to use get_sync_service() directly - Avoid ASGI transport which caused process hang on completion - Ensure proper database cleanup with db.shutdown_db() - Remove unused imports in test files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 Signed-off-by: phernandez --- .../cli/commands/command_utils.py | 28 ++++++++- src/basic_memory/cli/commands/db.py | 61 ++++++++++++++----- tests/mcp/clients/test_clients.py | 2 +- tests/test_project_resolver.py | 1 - 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/src/basic_memory/cli/commands/command_utils.py b/src/basic_memory/cli/commands/command_utils.py index 5f34cf8f2..286c04306 100644 --- a/src/basic_memory/cli/commands/command_utils.py +++ b/src/basic_memory/cli/commands/command_utils.py @@ -42,23 +42,45 @@ async def _with_cleanup() -> T: return asyncio.run(_with_cleanup()) -async def run_sync(project: Optional[str] = None, force_full: bool = False): +async def run_sync( + project: Optional[str] = None, + force_full: bool = False, + run_in_background: bool = True, +): """Run sync operation via API endpoint. Args: project: Optional project name force_full: If True, force a full scan bypassing watermark optimization + run_in_background: If True, return immediately; if False, wait for completion """ try: async with get_client() as client: project_item = await get_active_project(client, project, None) url = f"{project_item.project_url}/project/sync" + params = [] if force_full: - url += "?force_full=true" + params.append("force_full=true") + if not run_in_background: + params.append("run_in_background=false") + if params: + url += "?" + "&".join(params) response = await call_post(client, url) data = response.json() - console.print(f"[green]{data['message']}[/green]") + # Background mode returns {"message": "..."}, foreground returns SyncReportResponse + if "message" in data: + console.print(f"[green]{data['message']}[/green]") + else: + # Foreground mode - show summary of sync results + total = data.get("total", 0) + new_count = len(data.get("new", [])) + modified_count = len(data.get("modified", [])) + deleted_count = len(data.get("deleted", [])) + console.print( + f"[green]Synced {total} files[/green] " + f"(new: {new_count}, modified: {modified_count}, deleted: {deleted_count})" + ) except (ToolError, ValueError) as e: console.print(f"[red]Sync failed: {e}[/red]") raise typer.Exit(1) diff --git a/src/basic_memory/cli/commands/db.py b/src/basic_memory/cli/commands/db.py index 430b120d0..a878252c8 100644 --- a/src/basic_memory/cli/commands/db.py +++ b/src/basic_memory/cli/commands/db.py @@ -1,6 +1,7 @@ """Database management commands.""" import asyncio +from pathlib import Path import typer from loguru import logger @@ -9,11 +10,43 @@ from basic_memory import db from basic_memory.cli.app import app -from basic_memory.config import ConfigManager, BasicMemoryConfig, save_basic_memory_config +from basic_memory.config import ConfigManager +from basic_memory.repository import ProjectRepository +from basic_memory.services.initialization import reconcile_projects_with_config +from basic_memory.sync.sync_service import get_sync_service console = Console() +async def _reindex_projects(app_config): + """Reindex all projects in a single async context. + + This ensures all database operations use the same event loop, + and proper cleanup happens when the function completes. + """ + try: + await reconcile_projects_with_config(app_config) + + # Get database session (migrations already run if needed) + _, session_maker = await db.get_or_create_db( + db_path=app_config.database_path, + db_type=db.DatabaseType.FILESYSTEM, + ) + project_repository = ProjectRepository(session_maker) + projects = await project_repository.get_active_projects() + + for project in projects: + console.print(f" Indexing [cyan]{project.name}[/cyan]...") + logger.info(f"Starting sync for project: {project.name}") + sync_service = await get_sync_service(project) + sync_dir = Path(project.path) + await sync_service.sync(sync_dir, project_name=project.name) + logger.info(f"Sync completed for project: {project.name}") + finally: + # Clean up database connections before event loop closes + await db.shutdown_db() + + @app.command() def reset( reindex: bool = typer.Option(False, "--reindex", help="Rebuild db index from filesystem"), @@ -21,9 +54,10 @@ def reset( """Reset database (drop all tables and recreate).""" console.print( "[yellow]Note:[/yellow] This only deletes the index database. " - "Your markdown note files will not be affected." + "Your markdown note files will not be affected.\n" + "Use [green]bm reset --reindex[/green] to automatically rebuild the index afterward." ) - if typer.confirm("Reset the database index? (You can rebuild it with 'bm sync')"): + if typer.confirm("Reset the database index?"): logger.info("Resetting database...") config_manager = ConfigManager() app_config = config_manager.config @@ -45,12 +79,7 @@ def reset( ) raise typer.Exit(1) - # Reset project configuration - config = BasicMemoryConfig() - save_basic_memory_config(config_manager.config_file, config) - logger.info("Project configuration reset to default") - - # Create a new empty database + # Create a new empty database (preserves project configuration) try: asyncio.run(db.run_migrations(app_config)) except OperationalError as e: @@ -62,11 +91,13 @@ def reset( ) raise typer.Exit(1) raise - logger.info("Database reset complete") + console.print("[green]Database reset complete[/green]") if reindex: - # Run database sync directly - from basic_memory.cli.commands.command_utils import run_sync - - logger.info("Rebuilding search index from filesystem...") - asyncio.run(run_sync(project=None)) + projects = list(app_config.projects) + if not projects: + console.print("[yellow]No projects configured. Skipping reindex.[/yellow]") + else: + console.print(f"Rebuilding search index for {len(projects)} project(s)...") + asyncio.run(_reindex_projects(app_config)) + console.print("[green]Reindex complete[/green]") diff --git a/tests/mcp/clients/test_clients.py b/tests/mcp/clients/test_clients.py index 333f9fa63..35300ba99 100644 --- a/tests/mcp/clients/test_clients.py +++ b/tests/mcp/clients/test_clients.py @@ -1,7 +1,7 @@ """Tests for typed API clients.""" import pytest -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import MagicMock from basic_memory.mcp.clients import ( KnowledgeClient, diff --git a/tests/test_project_resolver.py b/tests/test_project_resolver.py index 1d4b4e663..2383a2133 100644 --- a/tests/test_project_resolver.py +++ b/tests/test_project_resolver.py @@ -1,6 +1,5 @@ """Tests for ProjectResolver - unified project resolution logic.""" -import os import pytest from basic_memory.project_resolver import ( ProjectResolver, From f89c408a072009019b5e7612c59ec8799a437cca Mon Sep 17 00:00:00 2001 From: phernandez Date: Mon, 5 Jan 2026 20:55:04 -0600 Subject: [PATCH 4/4] test: add regression test for search index persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add test_init_search_index_preserves_data() to verify that calling init_search_index() twice preserves indexed data, simulating MCP server restart behavior. This test would have caught the bug fixed in PR #503 where init_search_index() was unconditionally dropping the search_index table on every call. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 Signed-off-by: phernandez --- tests/repository/test_search_repository.py | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/repository/test_search_repository.py b/tests/repository/test_search_repository.py index 9b167a77e..d19c994f6 100644 --- a/tests/repository/test_search_repository.py +++ b/tests/repository/test_search_repository.py @@ -107,6 +107,48 @@ async def test_init_search_index(search_repository, app_config): assert table_name == "search_index" +@pytest.mark.asyncio +async def test_init_search_index_preserves_data(search_repository, search_entity): + """Regression test: calling init_search_index() twice should preserve indexed data. + + This test prevents regression of the bug fixed in PR #503 where + init_search_index() was dropping existing data on every call due to + an unconditional DROP TABLE statement. + + The bug caused search to work immediately after creating notes, but + return empty results after MCP server restarts (~30 minutes in Claude Desktop). + """ + # Create and index a search item + search_row = SearchIndexRow( + id=search_entity.id, + type=SearchItemType.ENTITY.value, + title=search_entity.title, + content_stems="regression test content for server restart", + content_snippet="This content should persist across init_search_index calls", + permalink=search_entity.permalink, + file_path=search_entity.file_path, + entity_id=search_entity.id, + metadata={"entity_type": search_entity.entity_type}, + created_at=search_entity.created_at, + updated_at=search_entity.updated_at, + project_id=search_repository.project_id, + ) + await search_repository.index_item(search_row) + + # Verify it's searchable + results = await search_repository.search(search_text="regression test") + assert len(results) == 1 + assert results[0].title == search_entity.title + + # Re-initialize the search index (simulates MCP server restart) + await search_repository.init_search_index() + + # Verify data is still there after re-initialization + results_after = await search_repository.search(search_text="regression test") + assert len(results_after) == 1, "Search index data was lost after init_search_index()" + assert results_after[0].id == search_entity.id + + @pytest.mark.asyncio async def test_index_item(search_repository, search_entity): """Test indexing an item with project_id."""