diff --git a/src/basic_memory/cli/commands/project.py b/src/basic_memory/cli/commands/project.py index c7db3abf..2bc4b411 100644 --- a/src/basic_memory/cli/commands/project.py +++ b/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 --local-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]") diff --git a/tests/cli/test_project_set_cloud_local.py b/tests/cli/test_project_set_cloud_local.py index e8024274..a00cf8b6 100644 --- a/tests/cli/test_project_set_cloud_local.py +++ b/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."""