diff --git a/src/basic_memory/cli/commands/project.py b/src/basic_memory/cli/commands/project.py index 0173073a0..27bc21c9a 100644 --- a/src/basic_memory/cli/commands/project.py +++ b/src/basic_memory/cli/commands/project.py @@ -892,8 +892,10 @@ def remove_project( async def _remove_project(): # Resolve workspace so cloud-only projects auto-route without --cloud - config = ConfigManager().config - entry = config.projects.get(name) + config_manager = ConfigManager() + config = config_manager.config + entry_name, _ = config_manager.get_project(name) + entry = config.projects.get(entry_name) if entry_name else None ws = None if entry and entry.workspace_id: ws = entry.workspace_id @@ -910,26 +912,66 @@ async def _remove_project(): ) try: - # Get config to check for local sync path and bisync state - config = ConfigManager().config + # A display name and its permalink address the same entry, and the API + # accepts either, so resolve the config key the same permalink-aware way. + config_manager = ConfigManager() + config = config_manager.config + entry_name, _ = config_manager.get_project(name) + entry = config.projects.get(entry_name) if entry_name else None + # The delete is cloud-routed on an explicit --cloud or a cloud-mode entry + # (per-project routing); local-artifact cleanup must follow the route the + # delete actually takes, not just the flag. + cloud_routed = cloud or (entry is not None and entry.mode == ProjectMode.CLOUD) local_path_config = None - has_bisync_state = False + bisync_state_path: Path | None = None - entry = config.projects.get(name) - if cloud and entry and entry.local_sync_path: + if cloud_routed and entry is not None and entry_name is not None and entry.local_sync_path: local_path_config = entry.local_sync_path - # Check for bisync state + # Bisync state is keyed by the canonical config name, not the form + # the user typed. from basic_memory.cli.commands.cloud.rclone_commands import get_project_bisync_state - bisync_state_path = get_project_bisync_state(name) - has_bisync_state = bisync_state_path.exists() + bisync_state_path = get_project_bisync_state(entry_name) # Remove project from cloud/API with force_routing(local=local, cloud=cloud): result = run_with_cleanup(_remove_project()) console.print(f"[green]{result.message}[/green]") + # Trigger: the entry is a cloud-mode routing entry (written by + # `project add --cloud` or `set-cloud`). An explicit --cloud alone is only + # a routing override — a same-named local project keeps its entry. + # Why: the local API removes its own config entry, but a cloud delete + # never touches local config, so the routing stub outlived the project: + # list-projects kept reporting it as a local project at "/", a second + # `remove` routed to the cloud again and got "not found", and `add` + # refused the name as taken (#1340). + # Outcome: the stub goes with the project — persisted before the fallible + # filesystem cleanups below, so a failed rmtree cannot leave the remote + # project deleted and the stub alive. The default project is the one + # entry config must keep, so it is only scrubbed of sync state and the + # user is told how to retire it. An explicit --local hands the delete to + # the local service, which removes the config entry itself. + if ( + not local + and entry is not None + and entry_name is not None + and entry.mode == ProjectMode.CLOUD + ): + if config.default_project == entry_name: + entry.local_sync_path = None + entry.bisync_initialized = False + entry.last_sync = None + console.print( + f"[yellow]'{entry_name}' is still the default project in local config. " + "Choose another with `bm project default --local`, then run " + f"`bm project remove {entry_name} --local` to drop this entry.[/yellow]" + ) + else: + del config.projects[entry_name] + config_manager.save_config(config) + # Clean up local sync directory if it exists and delete_notes is True if delete_notes and local_path_config: local_dir = Path(local_path_config) @@ -940,21 +982,11 @@ async def _remove_project(): console.print(f"[green]Removed local sync directory: {local_path_config}[/green]") # Clean up bisync state if it exists - if has_bisync_state: - from basic_memory.cli.commands.cloud.rclone_commands import get_project_bisync_state + if bisync_state_path is not None and bisync_state_path.exists(): import shutil - bisync_state_path = get_project_bisync_state(name) - if bisync_state_path.exists(): - shutil.rmtree(bisync_state_path) - console.print("[green]Removed bisync state[/green]") - - # Clean up cloud sync fields on the project entry - if cloud and entry and entry.local_sync_path: - entry.local_sync_path = None - entry.bisync_initialized = False - entry.last_sync = None - ConfigManager().save_config(config) + shutil.rmtree(bisync_state_path) + console.print("[green]Removed bisync state[/green]") # Show informative message if files were not deleted if not delete_notes: diff --git a/tests/cli/test_project_remove_cloud_stub.py b/tests/cli/test_project_remove_cloud_stub.py new file mode 100644 index 000000000..67e85874b --- /dev/null +++ b/tests/cli/test_project_remove_cloud_stub.py @@ -0,0 +1,224 @@ +"""`bm project remove` must retire the local routing entry of a cloud project (#1340). + +`project add --cloud` writes a cloud-mode config entry (path "", workspace id) so +later commands route to the cloud. The cloud delete never touched local config, +so that stub outlived the project: list-projects kept showing it, a second +`remove` routed to the cloud and got "not found", and `add` refused the name. +""" + +import json +from contextlib import asynccontextmanager +from pathlib import Path +from types import SimpleNamespace + +import pytest +from typer.testing import CliRunner + +from basic_memory.cli.app import app +from basic_memory.cli.commands.cloud import rclone_commands +from basic_memory.mcp.clients.project import ProjectClient + +# Importing registers project subcommands on the shared app instance. +import basic_memory.cli.commands.project as project_cmd # noqa: F401 + + +@pytest.fixture +def runner(): + return CliRunner() + + +def _write_config(tmp_path: Path, monkeypatch, projects: dict[str, dict[str, object]]) -> Path: + """Write an isolated config with the given project entries and point HOME at it.""" + from basic_memory import config as config_module + + config_module._CONFIG_CACHE = None + config_module._CONFIG_MTIME = None + config_module._CONFIG_SIZE = None + + config_dir = tmp_path / ".basic-memory" + config_dir.mkdir(parents=True, exist_ok=True) + path = config_dir / "config.json" + path.write_text( + json.dumps({"env": "dev", "projects": projects, "default_project": "main"}, indent=2) + ) + monkeypatch.setenv("HOME", str(tmp_path)) + return path + + +def _projects(config_file: Path) -> dict[str, dict[str, object]]: + return json.loads(config_file.read_text())["projects"] + + +@pytest.fixture +def config_file(tmp_path, monkeypatch): + """A local default plus a cloud-only routing entry.""" + return _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "openclaw-demo": {"path": "", "mode": "cloud", "workspace_id": "team-drew"}, + }, + ) + + +@pytest.fixture +def cloud_delete(monkeypatch): + """Stub the API client so the cloud resolves and deletes the project.""" + seen: dict[str, str | None] = {} + + @asynccontextmanager + async def fake_get_client(*, project_name=None, workspace=None): + seen["workspace"] = workspace + yield object() + + async def fake_resolve_project(self, identifier): + return SimpleNamespace(external_id="ext-123") + + async def fake_delete_project(self, external_id, delete_notes=False): + seen["deleted"] = external_id + return SimpleNamespace(message="Project deletion queued") + + monkeypatch.setattr(project_cmd, "get_client", fake_get_client) + monkeypatch.setattr(ProjectClient, "resolve_project", fake_resolve_project) + monkeypatch.setattr(ProjectClient, "delete_project", fake_delete_project) + return seen + + +def test_removing_a_cloud_project_drops_its_local_routing_entry(runner, config_file, cloud_delete): + result = runner.invoke(app, ["project", "remove", "openclaw-demo"]) + + assert result.exit_code == 0, result.stdout + assert cloud_delete == {"workspace": "team-drew", "deleted": "ext-123"} + projects = _projects(config_file) + assert "openclaw-demo" not in projects + assert "main" in projects, "unrelated entries must survive" + + +def test_permalink_form_of_the_name_still_finds_the_entry( + tmp_path, monkeypatch, runner, cloud_delete +): + """The API resolves `my-research` for `My Research`; the config lookup must too.""" + config_file = _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "My Research": {"path": "", "mode": "cloud", "workspace_id": "team-drew"}, + }, + ) + + result = runner.invoke(app, ["project", "remove", "my-research"]) + + assert result.exit_code == 0, result.stdout + assert "My Research" not in _projects(config_file) + + +def test_cloud_flag_is_only_a_routing_override_for_a_local_entry( + tmp_path, monkeypatch, runner, cloud_delete +): + """`remove --cloud` on a same-named local project deletes the cloud copy, not local config.""" + config_file = _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "research": {"path": str(tmp_path / "research"), "mode": "local"}, + }, + ) + + result = runner.invoke(app, ["project", "remove", "research", "--cloud"]) + + assert result.exit_code == 0, result.stdout + assert cloud_delete["deleted"] == "ext-123" + assert "research" in _projects(config_file), "the local project keeps its entry" + + +def test_auto_routed_cloud_delete_cleans_local_sync_artifacts( + tmp_path, monkeypatch, runner, cloud_delete +): + """Cleanup follows the route the delete takes, not the raw --cloud flag.""" + local_sync = tmp_path / "research-sync" + local_sync.mkdir() + config_file = _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "research": { + "path": str(local_sync), + "mode": "cloud", + "workspace_id": "team-drew", + "local_sync_path": str(local_sync), + "bisync_initialized": True, + }, + }, + ) + bisync_state = tmp_path / "bisync-state" / "research" + bisync_state.mkdir(parents=True) + monkeypatch.setattr( + rclone_commands, "get_project_bisync_state", lambda project_name: bisync_state + ) + + result = runner.invoke(app, ["project", "remove", "research"]) + + assert result.exit_code == 0, result.stdout + assert not bisync_state.exists(), "stale bisync state would let a recreated name skip --resync" + assert local_sync.exists(), "notes stay on disk without --delete-notes" + # Rich wraps the long temp path across lines, so match the message alone. + assert "Local files remain at" in result.stdout + assert "research" not in _projects(config_file) + + +def test_bisync_state_cleanup_uses_the_canonical_entry_name( + tmp_path, monkeypatch, runner, cloud_delete +): + """Removing `My Research` as `my-research` must clear `bisync-state/My Research`.""" + local_sync = tmp_path / "research-sync" + local_sync.mkdir() + _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "My Research": { + "path": str(local_sync), + "mode": "cloud", + "workspace_id": "team-drew", + "local_sync_path": str(local_sync), + "bisync_initialized": True, + }, + }, + ) + states = {"My Research": tmp_path / "bisync-state" / "My Research"} + states["My Research"].mkdir(parents=True) + monkeypatch.setattr( + rclone_commands, + "get_project_bisync_state", + lambda project_name: states.get(project_name, tmp_path / "bisync-state" / project_name), + ) + + result = runner.invoke(app, ["project", "remove", "my-research"]) + + assert result.exit_code == 0, result.stdout + assert not states["My Research"].exists() + + +def test_explicit_local_route_leaves_config_removal_to_the_local_service( + tmp_path, monkeypatch, runner, cloud_delete +): + """`remove --local` on a cloud-mode entry with a local row: the local API owns the entry.""" + config_file = _write_config( + tmp_path, + monkeypatch, + { + "main": {"path": str(tmp_path / "main"), "mode": "local"}, + "research": {"path": "", "mode": "cloud", "workspace_id": "team-drew"}, + }, + ) + + result = runner.invoke(app, ["project", "remove", "research", "--local"]) + + assert result.exit_code == 0, result.stdout + # The stubbed API did not touch config; the CLI must not double-delete either. + assert "research" in _projects(config_file)