Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 121 additions & 3 deletions src/basic_memory/cli/commands/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,72 @@ async def _move_project():
raise typer.Exit(1)


async def _detach_local_project_row(app_config, 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:
await db.shutdown_db()


async def _attach_local_project_row(app_config, 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:
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"),
Expand All @@ -855,6 +921,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-...
Expand Down Expand Up @@ -883,27 +956,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))

Comment on lines +967 to +968
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist config before deleting the local project row

The command deletes the local DB row before writing the updated config. If save_config fails (e.g., disk full/permissions), the project remains local in config but its DB row and indexed state are already removed, leaving the local project in a broken hybrid state until manual repair/reindex. This should be made atomic (or rollback the delete on save failure) so partial failure doesn’t corrupt local state.

Useful? React with 👍 / 👎.

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 = ""
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve local sync path during set-cloud cutover

Do not blank entry.path for every cloud cutover, because cloud projects with an existing local bisync copy rely on that path to stay eligible for local watchers/sync. After this line runs, local_sync_path may still be set but both cloud filters (services/initialization.py and sync/watch_service.py) check Path(entry.path).is_absolute() to decide whether to keep cloud projects, so rerunning set-cloud (for example to change workspace) can silently disable background sync/watch for that project’s local mirror.

Useful? React with 👍 / 👎.

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
Expand All @@ -913,11 +1011,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()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject relative saved paths when falling back in set-local

When --local-path is omitted, this command reuses entry.path and immediately runs abspath() on it. Legacy cloud entries can carry non-absolute placeholder paths (the config migration comments explicitly call out slug paths), so this silently rebases them to the current working directory and writes that to config/DB. In that scenario set-local can attach the project to an unintended directory without user confirmation; require an absolute fallback path (or force --local-path) before resolving.

Useful? React with 👍 / 👎.


# 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
Comment on lines +1034 to +1038
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Save config before reattaching local project row

set-local recreates/updates the local DB project row before persisting the config change. If save_config fails (for example due to disk/permission errors), config still says the project is cloud-routed while the local DB row has already been reintroduced, recreating the same hybrid local+cloud state this change is trying to eliminate and requiring manual cleanup. Make this transition atomic (or rollback the DB write on save failure) so partial failures do not corrupt project state.

Useful? React with 👍 / 👎.

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]")


Expand Down
52 changes: 43 additions & 9 deletions tests/cli/test_project_set_cloud_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand All @@ -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

Expand All @@ -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
Expand All @@ -211,6 +233,18 @@ def test_set_local_clears_workspace_id(self, runner, mock_config):
assert updated_data["projects"]["research"]["mode"] == "local"


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"] == ""


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

Expand Down
Loading