mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix(sync): also exclude orphan DB projects absent from config (#949)
Address Codex review feedback. The previous path-only filter dropped an implicit protection: get_project_mode() defaults projects missing from config to CLOUD, so the old mode-based guard skipped stale DB rows that had been removed from config. With a path-only check, an orphan row with an absolute path would pass and background sync/watch could still mutate a directory the user already removed from config (config is the source of truth) if reconciliation was skipped or failed. Introduce BasicMemoryConfig.is_locally_syncable(name, path), which requires both config membership and an absolute path, and use it from both the background sync selection and the watch cycle so the two paths cannot diverge. Add direct unit tests for the helper plus an orphan-row regression test for the watch selection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Drew Cain <groksrc@gmail.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user