diff --git a/src/basic_memory/cli/commands/project.py b/src/basic_memory/cli/commands/project.py index 0173073a0..73a069ec1 100644 --- a/src/basic_memory/cli/commands/project.py +++ b/src/basic_memory/cli/commands/project.py @@ -1184,8 +1184,13 @@ def set_cloud( 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 + # Clear the local path and sync metadata: the source of truth for this + # project is now the cloud. A leftover local_sync_path would read as a local + # copy and let reconciliation recreate the row this cutover just dropped. config.projects[name].path = "" + config.projects[name].local_sync_path = None + config.projects[name].bisync_initialized = False + config.projects[name].last_sync = None config_manager.save_config(config) console.print(f"[green]Project '{name}' set to cloud mode[/green]") diff --git a/src/basic_memory/deps/db.py b/src/basic_memory/deps/db.py index daecb107c..a24a365c9 100644 --- a/src/basic_memory/deps/db.py +++ b/src/basic_memory/deps/db.py @@ -44,6 +44,13 @@ async def get_engine_factory( app_config = resolve_container().config engine, session_maker = await db.get_or_create_db(app_config.database_path) + # Deferred import for the same reason as resolve_container above: the + # services layer is reached through api.app -> routers -> deps. + from basic_memory.services.initialization import reconcile_projects_with_config_once + + # No server lifespan ran initialize_app() for this process, so seed + # config.json's projects into the database here (#1334). + await reconcile_projects_with_config_once(app_config) return engine, session_maker diff --git a/src/basic_memory/services/initialization.py b/src/basic_memory/services/initialization.py index c25965874..1864e3926 100644 --- a/src/basic_memory/services/initialization.py +++ b/src/basic_memory/services/initialization.py @@ -146,9 +146,12 @@ async def initialize_database(app_config: BasicMemoryConfig) -> None: raise -async def reconcile_projects_with_config(app_config: BasicMemoryConfig): +async def reconcile_projects_with_config(app_config: BasicMemoryConfig) -> bool: """Ensure all projects in config.json exist in the projects table and vice versa. + Returns True when synchronization completed, False when it failed (the + failure is logged; startup continues either way). + This uses the ProjectService's synchronize_projects method to ensure bidirectional synchronization between the configuration file and the database. @@ -171,10 +174,41 @@ async def reconcile_projects_with_config(app_config: BasicMemoryConfig): project_service = ProjectService(repository=project_repository, session_maker=session_maker) try: await project_service.synchronize_projects() - logger.info("Projects successfully reconciled between config and database") except Exception as e: logger.error(f"Error during project synchronization: {e}") logger.info("Continuing with initialization despite synchronization error") + return False + logger.info("Projects successfully reconciled between config and database") + return True + + +# Database paths this process has already reconciled config projects into. +_reconciled_database_paths: set[Path] = set() + + +async def reconcile_projects_with_config_once(app_config: BasicMemoryConfig) -> None: + """Reconcile config projects into the database the first time a process opens it. + + Trigger: a request opens the local database outside a server lifespan — the + one-shot CLI and the MCP local flow drive the API over an in-process ASGI + transport, so nothing else seeds config.json's projects into a fresh database. + Why: initialize_app() only runs for the API/MCP servers and a few CLI + commands, so a CLI-only fresh install had `main` in config.json but no + projects row, and every default-project command failed while + `project add main` refused with "already exists" (#1334, regression of #974). + Outcome: the same reconciliation the servers run, once per process; cloud and + stateless deployments stay untouched, matching initialize_app(). + """ + if app_config.skip_local_initialization: + return + database_path = Path(app_config.database_path) + if database_path in _reconciled_database_paths: + return + # Only a completed reconciliation retires this path; a transient failure + # (logged inside) leaves the next request to try again rather than pinning + # a long-lived MCP process to an unseeded database. + if await reconcile_projects_with_config(app_config): + _reconciled_database_paths.add(database_path) # Strong references for fire-and-forget startup index tasks; the event loop diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index d866bbd70..336744238 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -39,6 +39,7 @@ WATCH_STATUS_JSON, ConfigManager, ProjectEntry, + ProjectMode, get_project_config, ProjectConfig, ) @@ -50,6 +51,26 @@ type ProjectSearchRepositoryFactory = Callable[[int], SearchRepository] +def _is_cloud_only(entry: ProjectEntry) -> bool: + """Whether a config entry routes to the cloud with no local copy of the notes. + + A local copy is recorded in ``local_sync_path``; older entries recorded it only + in ``path``, which ``_require_local_sync_path`` still honors — but only when + absolute. A relative value on a cloud entry is the remote slug + (``config_migrations``), and, like ``is_locally_syncable``, it must never be + taken for a local directory: a row for it would resolve against the process + cwd. ``set-cloud`` clears both fields; ``add --cloud`` without ``--local-path`` + writes neither. + """ + if entry.mode != ProjectMode.CLOUD: + return False + # Same rule as _require_local_sync_path: the sync copy is local_sync_path, + # falling back to path, and only an absolute one is a directory on this + # machine. ProjectEntry accepts a relative local_sync_path, so check it too. + local_copy = entry.local_sync_path or entry.path + return not (local_copy and os.path.isabs(local_copy)) + + class ProjectService: """Service for managing Basic Memory projects.""" @@ -521,6 +542,11 @@ async def synchronize_projects(self) -> None: # pragma: no cover if normalized_name != name: logger.info(f"Normalizing project name in config: '{name}' -> '{normalized_name}'") config_updated = True + # The default must follow its key: config validation resets a + # default that no longer matches a project key to the first + # project, which would silently replace a cloud default. + if config.default_project == name: + config.default_project = normalized_name updated_config[normalized_name] = entry @@ -538,8 +564,25 @@ async def synchronize_projects(self) -> None: # pragma: no cover db_projects = await self.repository.get_active_projects(session) db_projects_by_permalink = {p.permalink: p for p in db_projects} + # Trigger: a cloud-only entry — cloud mode with no local sync copy + # (`add --cloud` without --local-path, or a `set-cloud` + # cutover, which clears the sync metadata). + # Why: set-cloud deliberately deletes the local row so the project's + # configured state is purely cloud; a local row for it would undo + # that cutover and admit the project to local indexing/watching + # (#1334 review). A cloud project with a local sync path still + # needs its row for local-side operations. + # Outcome: cloud-only entries are absent from the local-row set — never + # created, and a stale row left by an older reconciliation is + # removed like any other row config no longer claims. + local_entries = { + name: entry + for name, entry in config_project_names.items() + if not _is_cloud_only(entry) + } + # Add projects that exist in config but not in DB - for name, entry in config_project_names.items(): + for name, entry in local_entries.items(): if name not in db_projects_by_permalink: logger.info(f"Adding project '{name}' to database") project_data = { @@ -555,7 +598,7 @@ async def synchronize_projects(self) -> None: # pragma: no cover # Config is the source of truth - if a project was deleted from config, # it should be deleted from DB too (fixes issue #193) for name, project in db_projects_by_permalink.items(): - if name not in config_project_names: + if name not in local_entries: logger.info( f"Removing project '{name}' from database (deleted from config, source of truth)" ) @@ -572,8 +615,17 @@ async def synchronize_projects(self) -> None: # pragma: no cover # Make sure default project is synchronized between config and database db_default = await self.repository.get_default_project(session) config_default = self.config_manager.default_project + config_default_entry = ( + config_project_names.get(config_default) if config_default else None + ) - if db_default and db_default.name != config_default: + # Trigger: the configured default is cloud-only (no local sync copy). + # Why: it has no local row by design, so the database default is only + # the local fallback and must not overwrite the user's choice. + # Outcome: config keeps the cloud default; the DB default stays local. + if config_default_entry is not None and _is_cloud_only(config_default_entry): + pass + elif db_default and db_default.name != config_default: # Update config to match DB default logger.info(f"Updating default project in config to '{db_default.name}'") self.config_manager.set_default_project(db_default.name) diff --git a/tests/cli/test_fresh_install.py b/tests/cli/test_fresh_install.py new file mode 100644 index 000000000..bf344bcbb --- /dev/null +++ b/tests/cli/test_fresh_install.py @@ -0,0 +1,56 @@ +"""Regression tests for a fresh, CLI-only install (#1334, #974).""" + +import os +import subprocess +from pathlib import Path + +PROJECT_ROOT = Path(__file__).parent.parent.parent + + +def _pristine_env(home: Path) -> dict[str, str]: + """A profile with no Basic Memory state, and none inherited from the developer or CI.""" + env = { + key: value + for key, value in os.environ.items() + if not key.startswith("BASIC_MEMORY_") and key != "PYTEST_CURRENT_TEST" + } + env.update( + HOME=str(home), + USERPROFILE=str(home), + BASIC_MEMORY_HOME=str(home / "basic-memory"), + BASIC_MEMORY_CONFIG_DIR=str(home / ".basic-memory"), + BASIC_MEMORY_NO_PROMOS="1", + ) + return env + + +def _bm(args: list[str], env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["uv", "run", "bm", *args], + capture_output=True, + text=True, + env=env, + # Guards a wedged install, not a performance budget: first run creates + # the database and runs migrations. + timeout=90, + cwd=PROJECT_ROOT, + ) + + +def test_fresh_cli_install_can_use_the_bootstrapped_default_project(tmp_path): + """The auto-created `main` project must exist in the database, not just config.json. + + A fresh install seeds `main` into config.json. Only the API/MCP server lifespan + used to reconcile that into the projects table, so a CLI-only flow hit + "Project not found: 'main'" on every default-project command while + `project add main` refused with "already exists" (#1334). + """ + home = tmp_path / "home" + home.mkdir() + env = _pristine_env(home) + + status = _bm(["status"], env) + + assert status.returncode == 0, status.stderr + assert "Project not found" not in status.stdout + status.stderr + assert "main" in status.stdout diff --git a/tests/cli/test_project_set_cloud_local.py b/tests/cli/test_project_set_cloud_local.py index a00cf8b63..e627902c9 100644 --- a/tests/cli/test_project_set_cloud_local.py +++ b/tests/cli/test_project_set_cloud_local.py @@ -383,3 +383,25 @@ def test_set_cloud_uses_default_workspace_when_no_flag(self, runner, mock_config config_module._CONFIG_SIZE = None updated_data = json.loads(mock_config.read_text()) assert updated_data["projects"]["research"]["workspace_id"] == "global-default-tenant-id" + + +def test_set_cloud_clears_sync_metadata(runner, mock_config, tmp_path): + """A cutover leaves no local_sync_path behind, so reconciliation cannot recreate the row. + + `add --cloud --local-path` entries carry a sync path; set-cloud blanked only + `path`, and a surviving local_sync_path read as a local copy (#1334 review). + """ + config = json.loads(mock_config.read_text()) + config["projects"]["research"].update( + {"local_sync_path": str(tmp_path / "research"), "bisync_initialized": True} + ) + mock_config.write_text(json.dumps(config, indent=2)) + + result = runner.invoke(app, ["project", "set-cloud", "research"]) + + assert result.exit_code == 0, result.stdout + entry = json.loads(mock_config.read_text())["projects"]["research"] + assert entry["mode"] == "cloud" + assert entry["path"] == "" + assert entry["local_sync_path"] is None + assert entry["bisync_initialized"] is False diff --git a/tests/mcp/test_async_client_modes.py b/tests/mcp/test_async_client_modes.py index 70235fd0c..f09d86984 100644 --- a/tests/mcp/test_async_client_modes.py +++ b/tests/mcp/test_async_client_modes.py @@ -7,6 +7,7 @@ from basic_memory.cli.auth import CLIAuth from basic_memory.config import ProjectMode from basic_memory.mcp import async_client as async_client_module +from basic_memory.services import initialization from basic_memory.mcp.async_client import ( get_client, get_cloud_control_plane_client, @@ -78,7 +79,13 @@ async def fake_get_or_create_db(db_path): calls.append(db_path) return engine, session_maker + async def skip_project_reconciliation(app_config): + """The engine/session maker here are stand-ins; reconciliation needs a real database.""" + monkeypatch.setattr(db, "get_or_create_db", fake_get_or_create_db) + monkeypatch.setattr( + initialization, "reconcile_projects_with_config_once", skip_project_reconciliation + ) try: async with get_client() as client: @@ -238,7 +245,13 @@ async def fake_get_or_create_db(db_path): calls.append(db_path) return engine, session_maker + async def skip_project_reconciliation(app_config): + """The engine/session maker here are stand-ins; reconciliation needs a real database.""" + monkeypatch.setattr(db, "get_or_create_db", fake_get_or_create_db) + monkeypatch.setattr( + initialization, "reconcile_projects_with_config_once", skip_project_reconciliation + ) first_context = get_client() second_context = get_client() diff --git a/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index c6474e5b6..431b4ffa3 100644 --- a/tests/mcp/test_project_context.py +++ b/tests/mcp/test_project_context.py @@ -2407,6 +2407,9 @@ class TestGetProjectClientRoutingOrder: @pytest.mark.asyncio async def test_local_flag_skips_workspace_resolution(self, config_manager, monkeypatch): """--local flag should never trigger workspace resolution, even for cloud projects.""" + import httpx + + import basic_memory.mcp.project_context as project_context from basic_memory.mcp.project_context import get_project_client from basic_memory.config import ProjectEntry, ProjectMode @@ -2422,14 +2425,18 @@ async def test_local_flag_skips_workspace_resolution(self, config_manager, monke monkeypatch.setenv("BASIC_MEMORY_FORCE_LOCAL", "true") monkeypatch.delenv("BASIC_MEMORY_FORCE_CLOUD", raising=False) - # Should not raise "Multiple workspaces" — it should skip workspace entirely - # It will fail at project validation (no API running), which proves routing worked - with pytest.raises(Exception) as exc_info: - async with get_project_client(project="cloud-proj"): - pass + # Any workspace lookup is the failure being guarded against. + async def fail_on_workspace_lookup(*args, **kwargs): + raise AssertionError("--local must not resolve workspaces") + + monkeypatch.setattr(project_context, "get_available_workspaces", fail_on_workspace_lookup) - # The error should NOT be about workspaces - assert "workspace" not in str(exc_info.value).lower() + # This entry is the legacy cloud-with-local-path shape, so the local ASGI + # client seeds it into the database (#1334) and validation succeeds on the + # local route; any workspace lookup would have tripped the guard above. + async with get_project_client(project="cloud-proj") as (client, active_project): + assert isinstance(client._transport, httpx.ASGITransport) # pyright: ignore[reportPrivateUsage] + assert active_project.name == "cloud-proj" @pytest.mark.asyncio async def test_local_route_clears_stale_cached_workspace(self, config_manager, monkeypatch): diff --git a/tests/services/test_initialization_reconcile_once.py b/tests/services/test_initialization_reconcile_once.py new file mode 100644 index 000000000..584b9bfaf --- /dev/null +++ b/tests/services/test_initialization_reconcile_once.py @@ -0,0 +1,64 @@ +"""The once-per-process project reconciliation used by the CLI's API path (#1334).""" + +import pytest + +from basic_memory.services import initialization + + +@pytest.mark.asyncio +async def test_reconcile_projects_with_config_once_runs_once_per_database(app_config, monkeypatch): + """Every local ASGI request hits the dependency; only the first may reconcile.""" + calls = [] + + async def fake_reconcile(config): + calls.append(config) + return True + + monkeypatch.setattr(initialization, "reconcile_projects_with_config", fake_reconcile) + monkeypatch.setattr(initialization, "_reconciled_database_paths", set()) + + await initialization.reconcile_projects_with_config_once(app_config) + await initialization.reconcile_projects_with_config_once(app_config) + + assert calls == [app_config] + + +@pytest.mark.asyncio +async def test_reconcile_projects_with_config_once_skips_cloud_deployments(app_config, monkeypatch): + """Cloud/stateless deployments own their project rows; reconciling would delete them.""" + calls = [] + + async def fake_reconcile(config): + calls.append(config) + return True + + monkeypatch.setattr(initialization, "reconcile_projects_with_config", fake_reconcile) + monkeypatch.setattr(initialization, "_reconciled_database_paths", set()) + monkeypatch.setenv("BASIC_MEMORY_CLOUD_MODE", "true") + assert app_config.skip_local_initialization + + await initialization.reconcile_projects_with_config_once(app_config) + + assert calls == [] + + +@pytest.mark.asyncio +async def test_reconcile_projects_with_config_once_retries_after_a_failed_attempt( + app_config, monkeypatch +): + """A transient failure must not pin a long-lived process to an unseeded database.""" + outcomes = iter([False, True]) + calls = [] + + async def fake_reconcile(config): + calls.append(config) + return next(outcomes) + + monkeypatch.setattr(initialization, "reconcile_projects_with_config", fake_reconcile) + monkeypatch.setattr(initialization, "_reconciled_database_paths", set()) + + await initialization.reconcile_projects_with_config_once(app_config) + await initialization.reconcile_projects_with_config_once(app_config) + await initialization.reconcile_projects_with_config_once(app_config) + + assert calls == [app_config, app_config], "retry once after failure, then stop" diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index ac351c8bd..b22d6d21b 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -1781,3 +1781,166 @@ async def test_add_doctor_project_requires_project_root(project_service: Project """Without a configured root there is no nesting problem for doctor to solve.""" with pytest.raises(ValueError, match="BASIC_MEMORY_PROJECT_ROOT"): await project_service.add_doctor_project() + + +@pytest.mark.asyncio +async def test_synchronize_projects_skips_cloud_mode_entries(project_service: ProjectService): + """A cloud-mode config entry must not get a local projects row (#1334 review). + + `set-cloud` deletes the local row on purpose so the project's configured state + is purely cloud. Reconciliation now runs on every CLI process, so recreating the + row here would undo that cutover each time. + """ + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-cloud"] = ProjectEntry(path="", mode=ProjectMode.CLOUD) + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-cloud") is None + + +@pytest.mark.asyncio +async def test_synchronize_projects_keeps_a_cloud_default_in_config( + project_service: ProjectService, +): + """The database default is only the local fallback; a cloud default must survive.""" + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-cloud"] = ProjectEntry(path="", mode=ProjectMode.CLOUD) + config.default_project = "research-cloud" + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert config_manager.load_config().default_project == "research-cloud" + db_default = await _get_default_project(project_service) + assert db_default is not None and db_default.name != "research-cloud" + + +@pytest.mark.asyncio +async def test_synchronize_projects_keeps_a_row_for_cloud_projects_with_a_local_sync_copy( + project_service: ProjectService, tmp_path +): + """`add --cloud --local-path` entries stay cloud-routed but still need their local row.""" + from basic_memory.config import ProjectEntry, ProjectMode + + sync_dir = tmp_path / "research-sync" + sync_dir.mkdir() + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-synced"] = ProjectEntry( + path=str(sync_dir), mode=ProjectMode.CLOUD, local_sync_path=str(sync_dir) + ) + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-synced") is not None + + +@pytest.mark.asyncio +async def test_synchronize_projects_removes_a_stale_row_for_a_cloud_only_entry( + project_service: ProjectService, tmp_path +): + """A row an older reconciliation recreated for a cut-over project converges away.""" + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-cloud"] = ProjectEntry(path="", mode=ProjectMode.CLOUD) + config_manager.save_config(config) + await _create_project( + project_service, + { + "name": "research-cloud", + "path": str(tmp_path / "stale-research"), + "permalink": "research-cloud", + "is_active": True, + }, + ) + assert await _get_project(project_service, "research-cloud") is not None + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-cloud") is None + + +@pytest.mark.asyncio +async def test_synchronize_projects_keeps_a_cloud_default_written_in_display_form( + project_service: ProjectService, +): + """`default_project: Research Cloud` must match the normalized `research-cloud` key.""" + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["Research Cloud"] = ProjectEntry(path="", mode=ProjectMode.CLOUD) + config.default_project = "Research Cloud" + config_manager.save_config(config) + + await project_service.synchronize_projects() + + # Keys are normalized on sync; the default must follow its key, not fall + # back to the first local project. + assert config_manager.load_config().default_project == "research-cloud" + + +@pytest.mark.asyncio +async def test_synchronize_projects_keeps_a_row_for_a_legacy_path_only_cloud_copy( + project_service: ProjectService, tmp_path +): + """Older entries record the local copy only in `path`; that is still a local copy.""" + from basic_memory.config import ProjectEntry, ProjectMode + + sync_dir = tmp_path / "legacy-sync" + sync_dir.mkdir() + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-legacy"] = ProjectEntry(path=str(sync_dir), mode=ProjectMode.CLOUD) + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-legacy") is not None + + +@pytest.mark.asyncio +async def test_synchronize_projects_treats_a_relative_cloud_path_as_the_remote_slug( + project_service: ProjectService, +): + """A cloud entry whose `path` is a slug like `research` has no local copy.""" + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-slug"] = ProjectEntry(path="research", mode=ProjectMode.CLOUD) + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-slug") is None + + +@pytest.mark.asyncio +async def test_synchronize_projects_treats_a_relative_local_sync_path_as_no_local_copy( + project_service: ProjectService, +): + """`_require_local_sync_path` rejects a relative sync path; reconciliation must agree.""" + from basic_memory.config import ProjectEntry, ProjectMode + + config_manager = project_service.config_manager + config = config_manager.load_config() + config.projects["research-rel"] = ProjectEntry( + path="", mode=ProjectMode.CLOUD, local_sync_path="research" + ) + config_manager.save_config(config) + + await project_service.synchronize_projects() + + assert await _get_project(project_service, "research-rel") is None