diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index 92ff504a..3da68d18 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -658,6 +658,26 @@ class BasicMemoryConfig(BaseSettings): entry = self.projects.get(project_name) return entry.mode if entry else ProjectMode.CLOUD + def is_locally_syncable(self, project_name: str, project_path: str) -> bool: + """Whether a project should be synced/watched on the local filesystem. + + Both conditions are required (issue #949): + + * The project is present in config. Config is the source of truth, so a + stale database row that was removed from config — but whose deletion + has not yet been reconciled, or whose reconciliation failed — must + not be synced even though it still has a real directory on disk. + * Its path is absolute. An empty or relative path resolves against the + process cwd, so syncing it would adopt whatever directory the server + was launched from as the project root and mutate unrelated files. + + Cloud-only projects (empty/slug path) and cloud projects with a real + local bisync copy (absolute path) are handled correctly by these two + conditions, so no separate mode check is needed. + """ + entry = self.projects.get(project_name) + return entry is not None and Path(project_path).is_absolute() + def set_project_mode(self, project_name: str, mode: ProjectMode) -> None: """Set the routing mode for a project. diff --git a/src/basic_memory/services/initialization.py b/src/basic_memory/services/initialization.py index bab14952..784b54de 100644 --- a/src/basic_memory/services/initialization.py +++ b/src/basic_memory/services/initialization.py @@ -120,20 +120,14 @@ 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}") - # Only projects with an absolute local path are safe to sync. - # Trigger: a project whose path is empty or relative. - # Why: Path("") and other relative paths resolve against the process cwd, so - # syncing such a project would adopt whatever directory the server was - # launched from as the project root and inject frontmatter into unrelated - # markdown files there (issue #949). Empty paths come from cloud-only - # projects, but also from any hand-edited config that left mode at the - # LOCAL default, so the gate is on the path itself, not the project mode. - # Outcome: such projects are excluded from local sync. Cloud projects that - # have a real local bisync copy keep their absolute path and still sync. - skip = [p.name for p in active_projects if not Path(p.path).is_absolute()] + # Only sync projects that are in config (source of truth) and have an + # absolute local path; see BasicMemoryConfig.is_locally_syncable. This keeps + # background sync from adopting the process cwd as a project root and + # mutating unrelated files (issue #949). + skip = [p.name for p in active_projects if not app_config.is_locally_syncable(p.name, p.path)] if skip: active_projects = [p for p in active_projects if p.name not in skip] - logger.info(f"Skipping projects without an absolute local path for sync: {skip}") + logger.info(f"Skipping projects that are not locally syncable for sync: {skip}") # Start sync for all projects as background tasks (non-blocking) async def sync_project_background(project: Project): diff --git a/src/basic_memory/sync/watch_service.py b/src/basic_memory/sync/watch_service.py index faab7c1e..ccf37d91 100644 --- a/src/basic_memory/sync/watch_service.py +++ b/src/basic_memory/sync/watch_service.py @@ -184,22 +184,24 @@ class WatchService: ``--project``, only that project is watched. This keeps concurrent MCP processes from producing duplicate watchers that race on the same files. - 2. Projects without an absolute local path are skipped. A non-absolute - path (empty string for cloud-only projects, or any relative value) - resolves against the process cwd, so watching it would observe and - mutate whatever directory the server was launched from (issue #949). - Cloud projects with a local bisync copy keep their absolute path and - are still watched. + 2. Projects that are not locally syncable are skipped — those missing + from config (config is the source of truth, so stale DB rows must + not be watched) or with a non-absolute path (which would resolve + against the process cwd and make the watcher observe and mutate the + directory the server was launched from). See + ``BasicMemoryConfig.is_locally_syncable`` (issue #949). Cloud + projects with a local bisync copy keep their absolute path and are + still watched. """ projects = await self.project_repository.get_active_projects() if self.constrained_project: projects = [p for p in projects if p.name == self.constrained_project] - skip = [p.name for p in projects if not Path(p.path).is_absolute()] + skip = [p.name for p in projects if not self.app_config.is_locally_syncable(p.name, p.path)] if skip: projects = [p for p in projects if p.name not in skip] - logger.debug(f"Skipping projects without an absolute local path in watch cycle: {skip}") + logger.debug(f"Skipping projects that are not locally syncable in watch cycle: {skip}") return list(projects) diff --git a/tests/services/test_initialization.py b/tests/services/test_initialization.py index 0a7d646c..8723cba6 100644 --- a/tests/services/test_initialization.py +++ b/tests/services/test_initialization.py @@ -265,7 +265,7 @@ async def test_initialize_file_sync_skips_project_with_non_absolute_path( await initialize_file_sync(updated, quiet=True) - skip_logs = [m for m in infos if "without an absolute local path" in m] + skip_logs = [m for m in infos if "not locally syncable" in m] assert skip_logs, "expected a skip log for the empty-path project" assert "empty-path" in skip_logs[0] assert "good" not in skip_logs[0] diff --git a/tests/sync/test_watch_service_reload.py b/tests/sync/test_watch_service_reload.py index c0b0377b..7b13ac44 100644 --- a/tests/sync/test_watch_service_reload.py +++ b/tests/sync/test_watch_service_reload.py @@ -279,6 +279,47 @@ async def test_run_filters_empty_path_local_mode_project(monkeypatch, tmp_path): assert seen_project_names == [["local-project"]] +@pytest.mark.asyncio +async def test_run_filters_orphan_db_project_absent_from_config(monkeypatch, tmp_path): + """A DB row not present in config is skipped even with an absolute path. + + Config is the source of truth. Reconciliation normally deletes orphan rows, + but if it is skipped or fails a stale row could remain; watching it would + mutate a directory the user already removed from config. + """ + config = BasicMemoryConfig( + watch_project_reload_interval=1, + projects={ + "local-project": {"path": str(tmp_path / "local"), "mode": "local"}, + }, + ) + repo = _Repo( + projects_return=[ + Project(id=1, name="local-project", path=str(tmp_path / "local"), permalink="local"), + # Absolute path, but no matching entry in config -> stale/orphan row. + Project(id=2, name="orphan", path=str(tmp_path / "orphan"), permalink="orphan"), + ] + ) + watch_service = _watch_service(config, repo) + + 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"]] + + @pytest.mark.asyncio async def test_run_continues_after_cycle_error(monkeypatch, tmp_path): config = BasicMemoryConfig( diff --git a/tests/test_config.py b/tests/test_config.py index f88ad839..14516c78 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1245,6 +1245,30 @@ class TestProjectMode: assert config.projects["research"].mode == ProjectMode.LOCAL assert config.get_project_mode("research") == ProjectMode.LOCAL + def test_is_locally_syncable_true_for_config_project_with_absolute_path(self, tmp_path): + """A project in config with an absolute path is locally syncable.""" + abs_path = str(tmp_path / "research") + config = BasicMemoryConfig(projects={"research": ProjectEntry(path=abs_path)}) + assert config.is_locally_syncable("research", abs_path) is True + + def test_is_locally_syncable_false_for_empty_path(self): + """An empty path resolves to cwd, so it is never locally syncable (#949).""" + config = BasicMemoryConfig(projects={"empty": ProjectEntry(path="")}) + assert config.is_locally_syncable("empty", "") is False + + def test_is_locally_syncable_false_for_relative_path(self): + """A relative (slug) path, as used by cloud-only projects, is not syncable.""" + config = BasicMemoryConfig(projects={"cloud": ProjectEntry(path="cloud-slug")}) + assert config.is_locally_syncable("cloud", "cloud-slug") is False + + def test_is_locally_syncable_false_for_orphan_not_in_config(self, tmp_path): + """A DB row absent from config is not syncable even with an absolute path. + + Config is the source of truth; stale rows must not be synced (#949). + """ + config = BasicMemoryConfig(projects={}) + assert config.is_locally_syncable("orphan", str(tmp_path / "orphan")) is False + def test_cloud_api_key_defaults_to_none(self): """Test that cloud_api_key defaults to None.""" config = BasicMemoryConfig()