fix(cli): cleanup local DB state on set-cloud/set-local (#775)

Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

src/basic_memory/cli/commands/project.py

@@ -26,7 +26,7 @@
 )
 from basic_memory.cli.commands.command_utils import get_project_info, run_with_cleanup
 from basic_memory.cli.commands.routing import force_routing, validate_routing_flags
-from basic_memory.config import ConfigManager, ProjectEntry, ProjectMode
+from basic_memory.config import BasicMemoryConfig, ConfigManager, ProjectEntry, ProjectMode
 from basic_memory.mcp.async_client import get_client, resolve_configured_workspace
 from basic_memory.mcp.clients import ProjectClient
 from basic_memory.schemas.cloud import (
@@ -838,6 +838,76 @@ async def _move_project():
         raise typer.Exit(1)
 
 
+async def _detach_local_project_row(app_config: BasicMemoryConfig, name: str) -> bool:
+    """Drop the project's row from the local index DB.
+
+    Trigger: `bm project set-cloud` is making a project cloud-only.
+    Why: the local row is what causes `_merge_projects` to report
+         `source: "local+cloud"` after the toggle (#680). Removing it
+         forces the merged listing to honor the user's chosen mode.
+    Outcome: returns True if a row was deleted, False if there was
+         nothing to clean up. On-disk note files are not touched.
+    """
+    from basic_memory import db
+    from basic_memory.repository import ProjectRepository
+
+    _, session_maker = await db.get_or_create_db(
+        db_path=app_config.database_path,
+        db_type=db.DatabaseType.FILESYSTEM,
+    )
+    try:
+        repo = ProjectRepository(session_maker)
+        existing = await repo.get_by_name(name)
+        if existing is None:
+            return False
+        await repo.delete(existing.id)
+        return True
+    finally:
+        # CLI-only: safe to tear down the global DB singleton here since
+        # set-cloud/set-local never run inside a long-lived MCP/API server.
+        await db.shutdown_db()
+
+
+async def _attach_local_project_row(app_config: BasicMemoryConfig, name: str, path: str) -> None:
+    """Ensure the project has a row in the local index DB at the given path.
+
+    Trigger: `bm project set-local` is making a previously cloud-only
+        project local again.
+    Why: without a row in the local DB, every local-side tool (`list`,
+        `info`, sync, indexing) would skip this project.
+    Outcome: a row is created if missing, or its path is updated to match
+        the new local home if it already exists. On-disk files are not
+        touched — the caller is responsible for ensuring the directory
+        exists.
+    """
+    from basic_memory import db
+    from basic_memory.repository import ProjectRepository
+
+    _, session_maker = await db.get_or_create_db(
+        db_path=app_config.database_path,
+        db_type=db.DatabaseType.FILESYSTEM,
+    )
+    try:
+        repo = ProjectRepository(session_maker)
+        existing = await repo.get_by_name(name)
+        if existing is None:
+            await repo.create(
+                {
+                    "name": name,
+                    "path": path,
+                    "permalink": generate_permalink(name),
+                    "is_active": True,
+                }
+            )
+            return
+        if existing.path != path:
+            await repo.update_path(existing.id, path)
+    finally:
+        # CLI-only: safe to tear down the global DB singleton here since
+        # set-cloud/set-local never run inside a long-lived MCP/API server.
+        await db.shutdown_db()
+
+
 @project_app.command("set-cloud")
 def set_cloud(
     name: str = typer.Argument(..., help="Name of the project to route through cloud"),
@@ -855,6 +925,13 @@ def set_cloud(
     If omitted, uses the default workspace (if set) or auto-selects when
     only one workspace is available.
 
+    This is a one-way cutover: the project's row in the local index DB is
+    removed and the local path in config is cleared so the project's
+    configured state is purely cloud. On-disk note files are preserved —
+    the caller can keep, archive, or delete them as they see fit. To
+    return to local mode use `bm project set-local <name> --local-path
+    <path>`.
+
     Examples:
       bm project set-cloud research --workspace Personal
       bm project set-cloud research --workspace 11111111-...
@@ -883,27 +960,52 @@ def set_cloud(
 
     resolved_workspace_id = _resolve_workspace_id(config, workspace)
 
+    # Drop the local DB row first so the user-visible state stays consistent
+    # even if the config save below raises for some reason. Idempotent: a
+    # second `set-cloud` simply finds no row and returns False.
+    previous_path = config.projects[name].path
+    detached = run_with_cleanup(_detach_local_project_row(config, name))
+
     config.set_project_mode(name, ProjectMode.CLOUD)
     if resolved_workspace_id:
         config.projects[name].workspace_id = resolved_workspace_id
+    # Clear local path: source-of-truth for this project is now the cloud
+    config.projects[name].path = ""
     config_manager.save_config(config)
 
     console.print(f"[green]Project '{name}' set to cloud mode[/green]")
     if resolved_workspace_id:
         console.print(f"[dim]Workspace: {resolved_workspace_id}[/dim]")
+    if detached and previous_path:
+        console.print(
+            f"[dim]Local index entry removed. Files at {previous_path} are preserved on disk.[/dim]"
+        )
     console.print("[dim]MCP tools and CLI commands for this project will route through cloud[/dim]")
 
 
 @project_app.command("set-local")
 def set_local(
     name: str = typer.Argument(..., help="Name of the project to revert to local mode"),
+    local_path: str = typer.Option(
+        None,
+        "--local-path",
+        help=(
+            "Local filesystem path for this project. Required unless the project "
+            "was previously local and its prior path is still in config."
+        ),
+    ),
 ) -> None:
     """Revert a project to local mode (use in-process ASGI transport).
 
-    Clears any associated cloud workspace.
+    Recreates the project's row in the local index DB and clears any
+    associated cloud workspace. If the project was previously local and
+    its prior path is still in config (e.g. an older version that didn't
+    blank `path` on `set-cloud`), `--local-path` may be omitted and that
+    path will be reused.
 
-    Example:
-      bm project set-local research
+    Examples:
+      bm project set-local research --local-path ~/Documents/research
+      bm project set-local research                 # reuse prior path
     """
     config_manager = ConfigManager()
     config = config_manager.config
@@ -913,11 +1015,31 @@ def set_local(
         console.print(f"[red]Error: Project '{name}' not found in config[/red]")
         raise typer.Exit(1)
 
+    entry = config.projects[name]
+    candidate = local_path or entry.path
+    if not candidate:
+        console.print(
+            f"[red]Error: --local-path is required for '{name}' "
+            "(no previous local path is recorded)[/red]"
+        )
+        raise typer.Exit(1)
+
+    resolved_path = Path(os.path.abspath(os.path.expanduser(candidate))).as_posix()
+
+    # Recreate the local DB row. Idempotent: if the row exists with the
+    # same path it's a no-op; if it exists at a stale path the path is
+    # updated. The directory itself is not auto-created — the user is
+    # expected to know whether they want to start a fresh project tree
+    # or point at an existing one.
+    run_with_cleanup(_attach_local_project_row(config, name, resolved_path))
+
     config.set_project_mode(name, ProjectMode.LOCAL)
     config.projects[name].workspace_id = None
+    config.projects[name].path = resolved_path
     config_manager.save_config(config)
 
     console.print(f"[green]Project '{name}' set to local mode[/green]")
+    console.print(f"[dim]Path: {resolved_path}[/dim]")
     console.print("[dim]MCP tools and CLI commands for this project will use local transport[/dim]")
 
 

tests/cli/test_project_set_cloud_local.py

@@ -154,21 +154,28 @@ def test_set_cloud_with_oauth_session(self, runner, tmp_path, monkeypatch):
 class TestSetLocal:
     """Tests for bm project set-local command."""
 
-    def test_set_local_success(self, runner, mock_config):
+    def test_set_local_success(self, runner, mock_config, tmp_path):
         """Test reverting a project to local mode."""
-        # First set to cloud
+        # First set to cloud (clears the local path as part of the cutover)
         runner.invoke(app, ["project", "set-cloud", "research"])
         config_data = json.loads(mock_config.read_text())
         assert config_data["projects"]["research"]["mode"] == "cloud"
 
-        # Now set back to local
-        result = runner.invoke(app, ["project", "set-local", "research"])
+        # Now set back to local — must supply a path since set-cloud blanked it
+        new_path = tmp_path / "research"
+        result = runner.invoke(
+            app, ["project", "set-local", "research", "--local-path", str(new_path)]
+        )
         assert result.exit_code == 0
         assert "local mode" in result.stdout.lower()
 
-        # Verify config was updated — mode reset to local
+        # Verify config was updated — mode reset to local, path restored.
+        # set-local normalizes the path via Path.as_posix(), matching the
+        # convention used by `bm project add`. On Windows that means
+        # backslashes are converted to forward slashes.
         config_data = json.loads(mock_config.read_text())
         assert config_data["projects"]["research"]["mode"] == "local"
+        assert config_data["projects"]["research"]["path"] == new_path.as_posix()
 
     def test_set_local_nonexistent_project(self, runner, mock_config):
         """Test set-local with a project that doesn't exist in config."""
@@ -177,12 +184,23 @@ def test_set_local_nonexistent_project(self, runner, mock_config):
         assert "not found" in result.stdout.lower()
 
     def test_set_local_already_local(self, runner, mock_config):
-        """Test set-local on a project that's already local (no-op, should succeed)."""
+        """Test set-local on a project that's already local (reuses existing path)."""
         result = runner.invoke(app, ["project", "set-local", "main"])
         assert result.exit_code == 0
         assert "local mode" in result.stdout.lower()
 
-    def test_set_local_clears_workspace_id(self, runner, mock_config):
+    def test_set_local_requires_path_after_set_cloud(self, runner, mock_config):
+        """Regression for #680: after set-cloud blanks the path, set-local must
+        refuse to silently default — the user has to specify where the project
+        lives now."""
+        runner.invoke(app, ["project", "set-cloud", "research"])
+
+        # No --local-path; config no longer has a path either.
+        result = runner.invoke(app, ["project", "set-local", "research"])
+        assert result.exit_code == 1
+        assert "--local-path" in result.stdout
+
+    def test_set_local_clears_workspace_id(self, runner, mock_config, tmp_path):
         """Test that set-local clears workspace_id from the project entry."""
         from basic_memory import config as config_module
 
@@ -198,8 +216,12 @@ def test_set_local_clears_workspace_id(self, runner, mock_config):
         config_module._CONFIG_MTIME = None
         config_module._CONFIG_SIZE = None
 
-        # Set back to local
-        result = runner.invoke(app, ["project", "set-local", "research"])
+        # Set back to local — supply --local-path; existing config path is preserved
+        # in this test setup, but new behavior recommends explicit path passing.
+        new_path = tmp_path / "research"
+        result = runner.invoke(
+            app, ["project", "set-local", "research", "--local-path", str(new_path)]
+        )
         assert result.exit_code == 0
 
         # Verify workspace_id was cleared
@@ -210,6 +232,57 @@ def test_set_local_clears_workspace_id(self, runner, mock_config):
         assert updated_data["projects"]["research"]["workspace_id"] is None
         assert updated_data["projects"]["research"]["mode"] == "local"
 
+    def test_set_local_update_path_when_row_exists(self, runner, mock_config, tmp_path):
+        """Re-running set-local with a different --local-path must update the
+        existing DB row (covers the repo.update_path() branch in
+        _attach_local_project_row)."""
+        path_a = tmp_path / "research_v1"
+        path_b = tmp_path / "research_v2"
+
+        # First call seeds the DB row at path_a.
+        result_a = runner.invoke(
+            app, ["project", "set-local", "research", "--local-path", str(path_a)]
+        )
+        assert result_a.exit_code == 0
+
+        # Second call must update the existing row's path to path_b.
+        result_b = runner.invoke(
+            app, ["project", "set-local", "research", "--local-path", str(path_b)]
+        )
+        assert result_b.exit_code == 0
+
+        updated = json.loads(mock_config.read_text())
+        assert updated["projects"]["research"]["path"] == path_b.as_posix()
+
+
+class TestSetCloudCutover:
+    """Regression tests for #680 — set-cloud as a one-way cutover."""
+
+    def test_set_cloud_clears_path(self, runner, mock_config):
+        """After set-cloud, config.projects[name].path must be blanked so the
+        merged project list reports source: cloud (not local+cloud)."""
+        runner.invoke(app, ["project", "set-cloud", "research"])
+        updated = json.loads(mock_config.read_text())
+        assert updated["projects"]["research"]["mode"] == "cloud"
+        assert updated["projects"]["research"]["path"] == ""
+
+    def test_set_cloud_removes_existing_db_row(self, runner, mock_config, tmp_path):
+        """When set-cloud runs against a project with a DB row, the row must
+        be removed and the user-facing message must mention the cleanup
+        (covers the repo.delete() → return True branch in
+        _detach_local_project_row)."""
+        # Seed a DB row first by going through set-local.
+        research_path = tmp_path / "research"
+        seed = runner.invoke(
+            app, ["project", "set-local", "research", "--local-path", str(research_path)]
+        )
+        assert seed.exit_code == 0
+
+        # Now flip to cloud — _detach_local_project_row should find and drop the row.
+        result = runner.invoke(app, ["project", "set-cloud", "research"])
+        assert result.exit_code == 0
+        assert "local index entry removed" in result.stdout.lower()
+
 
 class TestSetCloudWithWorkspace:
     """Tests for 'bm project set-cloud --workspace' option."""