Skip to content

Commit a42c6a2

Browse files
committed
fix(cli): cleanup local DB state on set-cloud/set-local
Per maintainer direction (#680), `set-cloud` is now a one-way cutover rather than a routing toggle. Before this change, `set-cloud` only flipped `mode = cloud` in config and left the project's row in the local index DB alone — so `_merge_projects` continued to see the project on both sides and reported `source: "local+cloud"`, with the old `local_path` still populated. Confusing state, no clean way out without manually editing both `config.json` and `memory.db`. Changes: - `set-cloud` now drops the project's row from the local index DB and clears `path` in config. On-disk note files are preserved — the user can keep, archive, or delete them as they like. - `set-local` is the symmetric inverse. It accepts `--local-path` and recreates the local DB row at the resolved path. If the prior path is still in config (e.g. set-local on a project that was already local), `--local-path` may be omitted and the existing path is reused. - Both helpers (`_detach_local_project_row`, `_attach_local_project_row`) are idempotent: running set-cloud twice doesn't error; running set-local twice updates the path if it changed and otherwise no-ops. Tests: - `test_set_cloud_clears_path` — config `path` blanked after set-cloud - `test_set_local_requires_path_after_set_cloud` — regression for the exact loop the reporter described: re-binding to local must not silently default - `test_set_local_success` — full round trip with an explicit path Closes #680 Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 128c2da commit a42c6a2

2 files changed

Lines changed: 161 additions & 12 deletions

File tree

src/basic_memory/cli/commands/project.py

Lines changed: 121 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,72 @@ async def _move_project():
838838
raise typer.Exit(1)
839839

840840

841+
async def _detach_local_project_row(app_config, name: str) -> bool:
842+
"""Drop the project's row from the local index DB.
843+
844+
Trigger: `bm project set-cloud` is making a project cloud-only.
845+
Why: the local row is what causes `_merge_projects` to report
846+
`source: "local+cloud"` after the toggle (#680). Removing it
847+
forces the merged listing to honor the user's chosen mode.
848+
Outcome: returns True if a row was deleted, False if there was
849+
nothing to clean up. On-disk note files are not touched.
850+
"""
851+
from basic_memory import db
852+
from basic_memory.repository import ProjectRepository
853+
854+
_, session_maker = await db.get_or_create_db(
855+
db_path=app_config.database_path,
856+
db_type=db.DatabaseType.FILESYSTEM,
857+
)
858+
try:
859+
repo = ProjectRepository(session_maker)
860+
existing = await repo.get_by_name(name)
861+
if existing is None:
862+
return False
863+
await repo.delete(existing.id)
864+
return True
865+
finally:
866+
await db.shutdown_db()
867+
868+
869+
async def _attach_local_project_row(app_config, name: str, path: str) -> None:
870+
"""Ensure the project has a row in the local index DB at the given path.
871+
872+
Trigger: `bm project set-local` is making a previously cloud-only
873+
project local again.
874+
Why: without a row in the local DB, every local-side tool (`list`,
875+
`info`, sync, indexing) would skip this project.
876+
Outcome: a row is created if missing, or its path is updated to match
877+
the new local home if it already exists. On-disk files are not
878+
touched — the caller is responsible for ensuring the directory
879+
exists.
880+
"""
881+
from basic_memory import db
882+
from basic_memory.repository import ProjectRepository
883+
884+
_, session_maker = await db.get_or_create_db(
885+
db_path=app_config.database_path,
886+
db_type=db.DatabaseType.FILESYSTEM,
887+
)
888+
try:
889+
repo = ProjectRepository(session_maker)
890+
existing = await repo.get_by_name(name)
891+
if existing is None:
892+
await repo.create(
893+
{
894+
"name": name,
895+
"path": path,
896+
"permalink": generate_permalink(name),
897+
"is_active": True,
898+
}
899+
)
900+
return
901+
if existing.path != path:
902+
await repo.update_path(existing.id, path)
903+
finally:
904+
await db.shutdown_db()
905+
906+
841907
@project_app.command("set-cloud")
842908
def set_cloud(
843909
name: str = typer.Argument(..., help="Name of the project to route through cloud"),
@@ -855,6 +921,13 @@ def set_cloud(
855921
If omitted, uses the default workspace (if set) or auto-selects when
856922
only one workspace is available.
857923
924+
This is a one-way cutover: the project's row in the local index DB is
925+
removed and the local path in config is cleared so the project's
926+
configured state is purely cloud. On-disk note files are preserved —
927+
the caller can keep, archive, or delete them as they see fit. To
928+
return to local mode use `bm project set-local <name> --local-path
929+
<path>`.
930+
858931
Examples:
859932
bm project set-cloud research --workspace Personal
860933
bm project set-cloud research --workspace 11111111-...
@@ -883,27 +956,52 @@ def set_cloud(
883956

884957
resolved_workspace_id = _resolve_workspace_id(config, workspace)
885958

959+
# Drop the local DB row first so the user-visible state stays consistent
960+
# even if the config save below raises for some reason. Idempotent: a
961+
# second `set-cloud` simply finds no row and returns False.
962+
previous_path = config.projects[name].path
963+
detached = run_with_cleanup(_detach_local_project_row(config, name))
964+
886965
config.set_project_mode(name, ProjectMode.CLOUD)
887966
if resolved_workspace_id:
888967
config.projects[name].workspace_id = resolved_workspace_id
968+
# Clear local path: source-of-truth for this project is now the cloud
969+
config.projects[name].path = ""
889970
config_manager.save_config(config)
890971

891972
console.print(f"[green]Project '{name}' set to cloud mode[/green]")
892973
if resolved_workspace_id:
893974
console.print(f"[dim]Workspace: {resolved_workspace_id}[/dim]")
975+
if detached and previous_path:
976+
console.print(
977+
f"[dim]Local index entry removed. Files at {previous_path} are preserved on disk.[/dim]"
978+
)
894979
console.print("[dim]MCP tools and CLI commands for this project will route through cloud[/dim]")
895980

896981

897982
@project_app.command("set-local")
898983
def set_local(
899984
name: str = typer.Argument(..., help="Name of the project to revert to local mode"),
985+
local_path: str = typer.Option(
986+
None,
987+
"--local-path",
988+
help=(
989+
"Local filesystem path for this project. Required unless the project "
990+
"was previously local and its prior path is still in config."
991+
),
992+
),
900993
) -> None:
901994
"""Revert a project to local mode (use in-process ASGI transport).
902995
903-
Clears any associated cloud workspace.
996+
Recreates the project's row in the local index DB and clears any
997+
associated cloud workspace. If the project was previously local and
998+
its prior path is still in config (e.g. an older version that didn't
999+
blank `path` on `set-cloud`), `--local-path` may be omitted and that
1000+
path will be reused.
9041001
905-
Example:
906-
bm project set-local research
1002+
Examples:
1003+
bm project set-local research --local-path ~/Documents/research
1004+
bm project set-local research # reuse prior path
9071005
"""
9081006
config_manager = ConfigManager()
9091007
config = config_manager.config
@@ -913,11 +1011,31 @@ def set_local(
9131011
console.print(f"[red]Error: Project '{name}' not found in config[/red]")
9141012
raise typer.Exit(1)
9151013

1014+
entry = config.projects[name]
1015+
candidate = local_path or entry.path
1016+
if not candidate:
1017+
console.print(
1018+
f"[red]Error: --local-path is required for '{name}' "
1019+
"(no previous local path is recorded)[/red]"
1020+
)
1021+
raise typer.Exit(1)
1022+
1023+
resolved_path = Path(os.path.abspath(os.path.expanduser(candidate))).as_posix()
1024+
1025+
# Recreate the local DB row. Idempotent: if the row exists with the
1026+
# same path it's a no-op; if it exists at a stale path the path is
1027+
# updated. The directory itself is not auto-created — the user is
1028+
# expected to know whether they want to start a fresh project tree
1029+
# or point at an existing one.
1030+
run_with_cleanup(_attach_local_project_row(config, name, resolved_path))
1031+
9161032
config.set_project_mode(name, ProjectMode.LOCAL)
9171033
config.projects[name].workspace_id = None
1034+
config.projects[name].path = resolved_path
9181035
config_manager.save_config(config)
9191036

9201037
console.print(f"[green]Project '{name}' set to local mode[/green]")
1038+
console.print(f"[dim]Path: {resolved_path}[/dim]")
9211039
console.print("[dim]MCP tools and CLI commands for this project will use local transport[/dim]")
9221040

9231041

tests/cli/test_project_set_cloud_local.py

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,25 @@ def test_set_cloud_with_oauth_session(self, runner, tmp_path, monkeypatch):
154154
class TestSetLocal:
155155
"""Tests for bm project set-local command."""
156156

157-
def test_set_local_success(self, runner, mock_config):
157+
def test_set_local_success(self, runner, mock_config, tmp_path):
158158
"""Test reverting a project to local mode."""
159-
# First set to cloud
159+
# First set to cloud (clears the local path as part of the cutover)
160160
runner.invoke(app, ["project", "set-cloud", "research"])
161161
config_data = json.loads(mock_config.read_text())
162162
assert config_data["projects"]["research"]["mode"] == "cloud"
163163

164-
# Now set back to local
165-
result = runner.invoke(app, ["project", "set-local", "research"])
164+
# Now set back to local — must supply a path since set-cloud blanked it
165+
new_path = tmp_path / "research"
166+
result = runner.invoke(
167+
app, ["project", "set-local", "research", "--local-path", str(new_path)]
168+
)
166169
assert result.exit_code == 0
167170
assert "local mode" in result.stdout.lower()
168171

169-
# Verify config was updated — mode reset to local
172+
# Verify config was updated — mode reset to local, path restored
170173
config_data = json.loads(mock_config.read_text())
171174
assert config_data["projects"]["research"]["mode"] == "local"
175+
assert config_data["projects"]["research"]["path"] == str(new_path)
172176

173177
def test_set_local_nonexistent_project(self, runner, mock_config):
174178
"""Test set-local with a project that doesn't exist in config."""
@@ -177,12 +181,23 @@ def test_set_local_nonexistent_project(self, runner, mock_config):
177181
assert "not found" in result.stdout.lower()
178182

179183
def test_set_local_already_local(self, runner, mock_config):
180-
"""Test set-local on a project that's already local (no-op, should succeed)."""
184+
"""Test set-local on a project that's already local (reuses existing path)."""
181185
result = runner.invoke(app, ["project", "set-local", "main"])
182186
assert result.exit_code == 0
183187
assert "local mode" in result.stdout.lower()
184188

185-
def test_set_local_clears_workspace_id(self, runner, mock_config):
189+
def test_set_local_requires_path_after_set_cloud(self, runner, mock_config):
190+
"""Regression for #680: after set-cloud blanks the path, set-local must
191+
refuse to silently default — the user has to specify where the project
192+
lives now."""
193+
runner.invoke(app, ["project", "set-cloud", "research"])
194+
195+
# No --local-path; config no longer has a path either.
196+
result = runner.invoke(app, ["project", "set-local", "research"])
197+
assert result.exit_code == 1
198+
assert "--local-path" in result.stdout
199+
200+
def test_set_local_clears_workspace_id(self, runner, mock_config, tmp_path):
186201
"""Test that set-local clears workspace_id from the project entry."""
187202
from basic_memory import config as config_module
188203

@@ -198,8 +213,12 @@ def test_set_local_clears_workspace_id(self, runner, mock_config):
198213
config_module._CONFIG_MTIME = None
199214
config_module._CONFIG_SIZE = None
200215

201-
# Set back to local
202-
result = runner.invoke(app, ["project", "set-local", "research"])
216+
# Set back to local — supply --local-path; existing config path is preserved
217+
# in this test setup, but new behavior recommends explicit path passing.
218+
new_path = tmp_path / "research"
219+
result = runner.invoke(
220+
app, ["project", "set-local", "research", "--local-path", str(new_path)]
221+
)
203222
assert result.exit_code == 0
204223

205224
# Verify workspace_id was cleared
@@ -211,6 +230,18 @@ def test_set_local_clears_workspace_id(self, runner, mock_config):
211230
assert updated_data["projects"]["research"]["mode"] == "local"
212231

213232

233+
class TestSetCloudCutover:
234+
"""Regression tests for #680 — set-cloud as a one-way cutover."""
235+
236+
def test_set_cloud_clears_path(self, runner, mock_config):
237+
"""After set-cloud, config.projects[name].path must be blanked so the
238+
merged project list reports source: cloud (not local+cloud)."""
239+
runner.invoke(app, ["project", "set-cloud", "research"])
240+
updated = json.loads(mock_config.read_text())
241+
assert updated["projects"]["research"]["mode"] == "cloud"
242+
assert updated["projects"]["research"]["path"] == ""
243+
244+
214245
class TestSetCloudWithWorkspace:
215246
"""Tests for 'bm project set-cloud --workspace' option."""
216247

0 commit comments

Comments
 (0)