mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: unify project path so path is always the local filesystem path
Cloud projects with bisync had a split-brain problem: `path` held a cloud slug while the actual local directory lived in `local_sync_path`. This caused `bm status` and file sync to fail for bisync'd cloud projects. Changes: - Config migration promotes `local_sync_path` → `path` for entries where `path` is a non-absolute cloud slug - `ensure_project_paths_exists` skips cloud-only projects with slug paths - `initialize_file_sync` and watch service now keep cloud projects that have an absolute local path (bisync copy) instead of skipping all cloud projects - `sync-setup` and `project add --cloud --local-path` set both `path` and `local_sync_path` to the local directory - `sync-setup` creates the project in the local DB for immediate MCP use - `_get_sync_project` falls back from `local_sync_path` to `path` - Config load errors now show user-friendly messages instead of stack traces Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
This commit is contained in:
@@ -5,6 +5,7 @@ project instances. These were previously in project.py but belong here since
|
||||
they are cloud-specific operations.
|
||||
"""
|
||||
|
||||
import os
|
||||
from datetime import datetime
|
||||
|
||||
import typer
|
||||
@@ -69,10 +70,11 @@ def _get_sync_project(
|
||||
Returns (sync_project, local_sync_path). Exits if no local_sync_path configured.
|
||||
"""
|
||||
sync_entry = config.projects.get(name)
|
||||
local_sync_path = sync_entry.local_sync_path if sync_entry else None
|
||||
# Support both new (path) and legacy (local_sync_path) configs
|
||||
local_sync_path = (sync_entry.local_sync_path or sync_entry.path) if sync_entry else None
|
||||
|
||||
if not local_sync_path:
|
||||
console.print(f"[red]Error: Project '{name}' has no local_sync_path configured[/red]")
|
||||
if not local_sync_path or not os.path.isabs(local_sync_path):
|
||||
console.print(f"[red]Error: Project '{name}' has no local sync path configured[/red]")
|
||||
console.print(f"\nConfigure sync with: bm cloud sync-setup {name} ~/path/to/local")
|
||||
raise typer.Exit(1)
|
||||
|
||||
@@ -334,9 +336,10 @@ def setup_project_sync(
|
||||
resolved_path = Path(os.path.abspath(os.path.expanduser(local_path)))
|
||||
resolved_path.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Update project entry with sync path
|
||||
# Update project entry with sync path — path is always the local directory
|
||||
entry = config.projects.get(name)
|
||||
if entry:
|
||||
entry.path = resolved_path.as_posix()
|
||||
entry.local_sync_path = resolved_path.as_posix()
|
||||
entry.bisync_initialized = False
|
||||
entry.last_sync = None
|
||||
@@ -347,6 +350,18 @@ def setup_project_sync(
|
||||
)
|
||||
config_manager.save_config(config)
|
||||
|
||||
# Create the project in the local DB so the MCP server can immediately use it
|
||||
async def _create_local_project():
|
||||
async with get_client() as client:
|
||||
data = {"name": name, "path": resolved_path.as_posix(), "set_default": False}
|
||||
return await ProjectClient(client).create_project(data)
|
||||
|
||||
with force_routing(local=True):
|
||||
try:
|
||||
run_with_cleanup(_create_local_project())
|
||||
except Exception:
|
||||
pass # Project may already exist locally; reconcile on next startup
|
||||
|
||||
console.print(f"[green]Sync configured for project '{name}'[/green]")
|
||||
console.print(f"\nLocal sync path: {resolved_path}")
|
||||
console.print("\nNext steps:")
|
||||
|
||||
@@ -285,9 +285,10 @@ def add_project(
|
||||
local_dir = Path(local_sync_path)
|
||||
local_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Update project entry with sync path
|
||||
# Update project entry — path is always the local directory
|
||||
entry = config.projects.get(name)
|
||||
if entry:
|
||||
entry.path = local_sync_path
|
||||
entry.local_sync_path = local_sync_path
|
||||
else:
|
||||
# Project may not be in local config yet (cloud-only add)
|
||||
|
||||
@@ -403,6 +403,18 @@ class BasicMemoryConfig(BaseSettings):
|
||||
data.pop("project_modes", None)
|
||||
data.pop("cloud_projects", None)
|
||||
|
||||
# --- Promote local_sync_path into path for cloud projects with slug paths ---
|
||||
# Trigger: project entry has local_sync_path set but path is a cloud slug (not absolute)
|
||||
# Why: path must always be the local filesystem path; the cloud remote is derivable
|
||||
# Outcome: path becomes the local directory, local_sync_path kept for backwards compat
|
||||
projects = data.get("projects", {})
|
||||
for name, entry in projects.items():
|
||||
if isinstance(entry, dict):
|
||||
lsp = entry.get("local_sync_path")
|
||||
path = entry.get("path", "")
|
||||
if lsp and not os.path.isabs(path):
|
||||
entry["path"] = lsp
|
||||
|
||||
return data
|
||||
|
||||
@property
|
||||
@@ -564,6 +576,9 @@ class BasicMemoryConfig(BaseSettings):
|
||||
|
||||
for name, entry in self.projects.items():
|
||||
path = Path(entry.path)
|
||||
# Skip cloud-only projects whose path is a slug, not a local directory
|
||||
if not path.is_absolute():
|
||||
continue
|
||||
if not path.exists():
|
||||
try:
|
||||
path.mkdir(parents=True)
|
||||
@@ -645,6 +660,17 @@ class ConfigManager:
|
||||
if isinstance(first_val, str):
|
||||
needs_resave = True
|
||||
|
||||
# Check if any project has local_sync_path set but path is a cloud slug
|
||||
# (will be migrated by migrate_legacy_projects validator)
|
||||
if not needs_resave:
|
||||
for entry_data in projects_raw.values():
|
||||
if isinstance(entry_data, dict):
|
||||
lsp = entry_data.get("local_sync_path")
|
||||
p = entry_data.get("path", "")
|
||||
if lsp and not os.path.isabs(p):
|
||||
needs_resave = True
|
||||
break
|
||||
|
||||
# First, create config from environment variables (Pydantic will read them)
|
||||
# Then overlay with file data for fields that aren't set via env vars
|
||||
# This ensures env vars take precedence
|
||||
@@ -673,9 +699,20 @@ class ConfigManager:
|
||||
save_basic_memory_config(self.config_file, _CONFIG_CACHE)
|
||||
|
||||
return _CONFIG_CACHE
|
||||
except json.JSONDecodeError as e: # pragma: no cover
|
||||
logger.error(f"Invalid JSON in config file {self.config_file}: {e}")
|
||||
raise SystemExit(
|
||||
f"Error: config file is not valid JSON: {self.config_file}\n"
|
||||
f" {e}\n"
|
||||
f"Fix or delete the file and re-run."
|
||||
)
|
||||
except Exception as e: # pragma: no cover
|
||||
logger.exception(f"Failed to load config: {e}")
|
||||
raise e
|
||||
logger.error(f"Failed to load config from {self.config_file}: {e}")
|
||||
raise SystemExit(
|
||||
f"Error: failed to load config from {self.config_file}\n"
|
||||
f" {e}\n"
|
||||
f"Fix or delete the file and re-run."
|
||||
)
|
||||
else:
|
||||
config = BasicMemoryConfig()
|
||||
self.save_config(config)
|
||||
|
||||
@@ -115,15 +115,18 @@ async def initialize_file_sync(
|
||||
active_projects = [p for p in active_projects if p.name == constrained_project]
|
||||
logger.info(f"Background sync constrained to project: {constrained_project}")
|
||||
|
||||
# Skip cloud-mode projects — their files live on the cloud instance, not locally
|
||||
cloud_projects = [
|
||||
p.name for p in active_projects if app_config.get_project_mode(p.name) == ProjectMode.CLOUD
|
||||
]
|
||||
if cloud_projects:
|
||||
active_projects = [
|
||||
p for p in active_projects if app_config.get_project_mode(p.name) != ProjectMode.CLOUD
|
||||
]
|
||||
logger.info(f"Skipping cloud-mode projects for local sync: {cloud_projects}")
|
||||
# Skip cloud-mode projects that have no local directory.
|
||||
# Cloud projects with a local bisync copy (absolute path) are kept for local sync.
|
||||
cloud_skip = []
|
||||
for p in active_projects:
|
||||
if app_config.get_project_mode(p.name) == ProjectMode.CLOUD:
|
||||
entry = app_config.projects.get(p.name)
|
||||
if entry and Path(entry.path).is_absolute():
|
||||
continue # Cloud project with local bisync copy — keep for local sync
|
||||
cloud_skip.append(p.name)
|
||||
if cloud_skip:
|
||||
active_projects = [p for p in active_projects if p.name not in cloud_skip]
|
||||
logger.info(f"Skipping cloud-mode projects for local sync: {cloud_skip}")
|
||||
|
||||
# Start sync for all projects as background tasks (non-blocking)
|
||||
async def sync_project_background(project: Project):
|
||||
|
||||
@@ -178,20 +178,19 @@ class WatchService:
|
||||
projects = await self.project_repository.get_active_projects()
|
||||
|
||||
# Trigger: project is configured for cloud routing
|
||||
# Why: cloud projects should not be watched/synced by local file watchers
|
||||
# Outcome: watch cycle only observes local-mode projects
|
||||
cloud_projects = [
|
||||
p.name
|
||||
for p in projects
|
||||
if self.app_config.get_project_mode(p.name) == ProjectMode.CLOUD
|
||||
]
|
||||
if cloud_projects:
|
||||
projects = [
|
||||
p
|
||||
for p in projects
|
||||
if self.app_config.get_project_mode(p.name) != ProjectMode.CLOUD
|
||||
]
|
||||
logger.debug(f"Skipping cloud-mode projects in watch cycle: {cloud_projects}")
|
||||
# Why: cloud-only projects (no local directory) should not be watched;
|
||||
# cloud projects with a local bisync copy (absolute path) need watching
|
||||
# Outcome: watch cycle skips cloud projects without a local directory
|
||||
cloud_skip = []
|
||||
for p in projects:
|
||||
if self.app_config.get_project_mode(p.name) == ProjectMode.CLOUD:
|
||||
entry = self.app_config.projects.get(p.name)
|
||||
if entry and Path(entry.path).is_absolute():
|
||||
continue # Cloud project with local bisync copy — keep watching
|
||||
cloud_skip.append(p.name)
|
||||
if cloud_skip:
|
||||
projects = [p for p in projects if p.name not in cloud_skip]
|
||||
logger.debug(f"Skipping cloud-mode projects in watch cycle: {cloud_skip}")
|
||||
|
||||
project_paths = [project.path for project in projects]
|
||||
logger.debug(f"Starting watch cycle for directories: {project_paths}")
|
||||
|
||||
@@ -144,23 +144,19 @@ async def test_run_reloads_projects_each_cycle(monkeypatch, tmp_path):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_filters_cloud_projects_each_cycle(monkeypatch, tmp_path):
|
||||
async def test_run_filters_cloud_only_projects_each_cycle(monkeypatch, tmp_path):
|
||||
"""Cloud-only projects (slug path, no local directory) are filtered out."""
|
||||
config = BasicMemoryConfig(
|
||||
watch_project_reload_interval=1,
|
||||
projects={
|
||||
"local-project": {"path": str(tmp_path / "local"), "mode": "local"},
|
||||
"cloud-project": {"path": str(tmp_path / "cloud"), "mode": "cloud"},
|
||||
"cloud-only": {"path": "cloud-slug", "mode": "cloud"},
|
||||
},
|
||||
)
|
||||
repo = _Repo(
|
||||
projects_return=[
|
||||
Project(id=1, name="local-project", path=str(tmp_path / "local"), permalink="local"),
|
||||
Project(
|
||||
id=2,
|
||||
name="cloud-project",
|
||||
path=str(tmp_path / "cloud"),
|
||||
permalink="cloud",
|
||||
),
|
||||
Project(id=2, name="cloud-only", path="cloud-slug", permalink="cloud-only"),
|
||||
]
|
||||
)
|
||||
watch_service = WatchService(config, repo, quiet=True)
|
||||
@@ -183,6 +179,47 @@ async def test_run_filters_cloud_projects_each_cycle(monkeypatch, tmp_path):
|
||||
assert seen_project_names == [["local-project"]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_keeps_cloud_projects_with_local_bisync(monkeypatch, tmp_path):
|
||||
"""Cloud projects with an absolute path (local bisync copy) are kept for watching."""
|
||||
config = BasicMemoryConfig(
|
||||
watch_project_reload_interval=1,
|
||||
projects={
|
||||
"local-project": {"path": str(tmp_path / "local"), "mode": "local"},
|
||||
"cloud-bisync": {"path": str(tmp_path / "cloud"), "mode": "cloud"},
|
||||
},
|
||||
)
|
||||
repo = _Repo(
|
||||
projects_return=[
|
||||
Project(id=1, name="local-project", path=str(tmp_path / "local"), permalink="local"),
|
||||
Project(
|
||||
id=2,
|
||||
name="cloud-bisync",
|
||||
path=str(tmp_path / "cloud"),
|
||||
permalink="cloud-bisync",
|
||||
),
|
||||
]
|
||||
)
|
||||
watch_service = WatchService(config, repo, quiet=True)
|
||||
|
||||
seen_project_names: list[list[str]] = []
|
||||
|
||||
async def watch_cycle_stub(projects, stop_event):
|
||||
seen_project_names.append([p.name for p in projects])
|
||||
watch_service.state.running = False
|
||||
stop_event.set()
|
||||
|
||||
async def fake_write_status():
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(watch_service, "_watch_projects_cycle", watch_cycle_stub)
|
||||
monkeypatch.setattr(watch_service, "write_status", fake_write_status)
|
||||
|
||||
await watch_service.run()
|
||||
|
||||
assert seen_project_names == [["local-project", "cloud-bisync"]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_continues_after_cycle_error(monkeypatch, tmp_path):
|
||||
config = BasicMemoryConfig()
|
||||
|
||||
@@ -1112,3 +1112,68 @@ class TestProjectMode:
|
||||
loaded.projects["research"].workspace_id == "11111111-1111-1111-1111-111111111111"
|
||||
)
|
||||
assert loaded.projects["main"].workspace_id is None
|
||||
|
||||
|
||||
class TestLocalSyncPathMigration:
|
||||
"""Test migration that promotes local_sync_path into path for cloud projects."""
|
||||
|
||||
def test_migrate_promotes_local_sync_path_to_path(self):
|
||||
"""When path is a cloud slug and local_sync_path is set, path becomes local_sync_path."""
|
||||
data = {
|
||||
"projects": {
|
||||
"specs": {
|
||||
"path": "specs",
|
||||
"mode": "cloud",
|
||||
"local_sync_path": "/Users/test/Documents/specs",
|
||||
}
|
||||
}
|
||||
}
|
||||
result = BasicMemoryConfig.migrate_legacy_projects(data)
|
||||
assert result["projects"]["specs"]["path"] == "/Users/test/Documents/specs"
|
||||
|
||||
def test_migrate_does_not_overwrite_absolute_path(self):
|
||||
"""When path is already absolute, migration should not change it."""
|
||||
data = {
|
||||
"projects": {
|
||||
"specs": {
|
||||
"path": "/Users/test/Documents/specs",
|
||||
"mode": "cloud",
|
||||
"local_sync_path": "/Users/test/Documents/specs",
|
||||
}
|
||||
}
|
||||
}
|
||||
result = BasicMemoryConfig.migrate_legacy_projects(data)
|
||||
assert result["projects"]["specs"]["path"] == "/Users/test/Documents/specs"
|
||||
|
||||
def test_migrate_skips_entries_without_local_sync_path(self):
|
||||
"""Entries without local_sync_path should not be modified."""
|
||||
data = {
|
||||
"projects": {
|
||||
"cloud-only": {
|
||||
"path": "cloud-only",
|
||||
"mode": "cloud",
|
||||
}
|
||||
}
|
||||
}
|
||||
result = BasicMemoryConfig.migrate_legacy_projects(data)
|
||||
assert result["projects"]["cloud-only"]["path"] == "cloud-only"
|
||||
|
||||
def test_migrate_handles_mixed_projects(self, tmp_path):
|
||||
"""Migration handles a mix of local, cloud-only, and cloud-with-bisync projects."""
|
||||
local_path = str(tmp_path / "local")
|
||||
bisync_path = str(tmp_path / "bisync")
|
||||
data = {
|
||||
"projects": {
|
||||
"local-proj": {"path": local_path, "mode": "local"},
|
||||
"cloud-only": {"path": "cloud-only", "mode": "cloud"},
|
||||
"cloud-bisync": {
|
||||
"path": "cloud-bisync",
|
||||
"mode": "cloud",
|
||||
"local_sync_path": bisync_path,
|
||||
},
|
||||
}
|
||||
}
|
||||
result = BasicMemoryConfig.migrate_legacy_projects(data)
|
||||
assert result["projects"]["local-proj"]["path"] == local_path
|
||||
assert result["projects"]["cloud-only"]["path"] == "cloud-only"
|
||||
assert result["projects"]["cloud-bisync"]["path"] == bisync_path
|
||||
|
||||
Reference in New Issue
Block a user