Skip to content

Commit 8f26f15

Browse files
committed
test(cli): cover update_path and existing-row delete branches
Address reviewer follow-ups on #775: - Add type annotations to the two CLI helpers (BasicMemoryConfig). - Note the CLI-only assumption inline at each shutdown_db() call. - Add two tests that exercise the previously uncovered branches: - test_set_local_update_path_when_row_exists hits repo.update_path. - test_set_cloud_removes_existing_db_row hits repo.delete on the detach helper and asserts the user-visible cleanup message. Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 28c49b6 commit 8f26f15

2 files changed

Lines changed: 46 additions & 3 deletions

File tree

src/basic_memory/cli/commands/project.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
)
2727
from basic_memory.cli.commands.command_utils import get_project_info, run_with_cleanup
2828
from basic_memory.cli.commands.routing import force_routing, validate_routing_flags
29-
from basic_memory.config import ConfigManager, ProjectEntry, ProjectMode
29+
from basic_memory.config import BasicMemoryConfig, ConfigManager, ProjectEntry, ProjectMode
3030
from basic_memory.mcp.async_client import get_client, resolve_configured_workspace
3131
from basic_memory.mcp.clients import ProjectClient
3232
from basic_memory.schemas.cloud import (
@@ -838,7 +838,7 @@ async def _move_project():
838838
raise typer.Exit(1)
839839

840840

841-
async def _detach_local_project_row(app_config, name: str) -> bool:
841+
async def _detach_local_project_row(app_config: BasicMemoryConfig, name: str) -> bool:
842842
"""Drop the project's row from the local index DB.
843843
844844
Trigger: `bm project set-cloud` is making a project cloud-only.
@@ -863,10 +863,12 @@ async def _detach_local_project_row(app_config, name: str) -> bool:
863863
await repo.delete(existing.id)
864864
return True
865865
finally:
866+
# CLI-only: safe to tear down the global DB singleton here since
867+
# set-cloud/set-local never run inside a long-lived MCP/API server.
866868
await db.shutdown_db()
867869

868870

869-
async def _attach_local_project_row(app_config, name: str, path: str) -> None:
871+
async def _attach_local_project_row(app_config: BasicMemoryConfig, name: str, path: str) -> None:
870872
"""Ensure the project has a row in the local index DB at the given path.
871873
872874
Trigger: `bm project set-local` is making a previously cloud-only
@@ -901,6 +903,8 @@ async def _attach_local_project_row(app_config, name: str, path: str) -> None:
901903
if existing.path != path:
902904
await repo.update_path(existing.id, path)
903905
finally:
906+
# CLI-only: safe to tear down the global DB singleton here since
907+
# set-cloud/set-local never run inside a long-lived MCP/API server.
904908
await db.shutdown_db()
905909

906910

tests/cli/test_project_set_cloud_local.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,28 @@ def test_set_local_clears_workspace_id(self, runner, mock_config, tmp_path):
232232
assert updated_data["projects"]["research"]["workspace_id"] is None
233233
assert updated_data["projects"]["research"]["mode"] == "local"
234234

235+
def test_set_local_update_path_when_row_exists(self, runner, mock_config, tmp_path):
236+
"""Re-running set-local with a different --local-path must update the
237+
existing DB row (covers the repo.update_path() branch in
238+
_attach_local_project_row)."""
239+
path_a = tmp_path / "research_v1"
240+
path_b = tmp_path / "research_v2"
241+
242+
# First call seeds the DB row at path_a.
243+
result_a = runner.invoke(
244+
app, ["project", "set-local", "research", "--local-path", str(path_a)]
245+
)
246+
assert result_a.exit_code == 0
247+
248+
# Second call must update the existing row's path to path_b.
249+
result_b = runner.invoke(
250+
app, ["project", "set-local", "research", "--local-path", str(path_b)]
251+
)
252+
assert result_b.exit_code == 0
253+
254+
updated = json.loads(mock_config.read_text())
255+
assert updated["projects"]["research"]["path"] == path_b.as_posix()
256+
235257

236258
class TestSetCloudCutover:
237259
"""Regression tests for #680 — set-cloud as a one-way cutover."""
@@ -244,6 +266,23 @@ def test_set_cloud_clears_path(self, runner, mock_config):
244266
assert updated["projects"]["research"]["mode"] == "cloud"
245267
assert updated["projects"]["research"]["path"] == ""
246268

269+
def test_set_cloud_removes_existing_db_row(self, runner, mock_config, tmp_path):
270+
"""When set-cloud runs against a project with a DB row, the row must
271+
be removed and the user-facing message must mention the cleanup
272+
(covers the repo.delete() → return True branch in
273+
_detach_local_project_row)."""
274+
# Seed a DB row first by going through set-local.
275+
research_path = tmp_path / "research"
276+
seed = runner.invoke(
277+
app, ["project", "set-local", "research", "--local-path", str(research_path)]
278+
)
279+
assert seed.exit_code == 0
280+
281+
# Now flip to cloud — _detach_local_project_row should find and drop the row.
282+
result = runner.invoke(app, ["project", "set-cloud", "research"])
283+
assert result.exit_code == 0
284+
assert "local index entry removed" in result.stdout.lower()
285+
247286

248287
class TestSetCloudWithWorkspace:
249288
"""Tests for 'bm project set-cloud --workspace' option."""

0 commit comments

Comments
 (0)