fix: prevent nested project paths to avoid data conflicts (#338)

Signed-off-by: phernandez <[email protected]>
Co-authored-by: Claude <[email protected]>

Author: phernandez

src/basic_memory/services/project_service.py

@@ -88,6 +88,40 @@ async def get_project(self, name: str) -> Optional[Project]:
             name
         )
 
+    def _check_nested_paths(self, path1: str, path2: str) -> bool:
+        """Check if two paths are nested (one is a prefix of the other).
+
+        Args:
+            path1: First path to compare
+            path2: Second path to compare
+
+        Returns:
+            True if one path is nested within the other, False otherwise
+
+        Examples:
+            _check_nested_paths("/foo", "/foo/bar")     # True (child under parent)
+            _check_nested_paths("/foo/bar", "/foo")     # True (parent over child)
+            _check_nested_paths("/foo", "/bar")         # False (siblings)
+        """
+        # Normalize paths to ensure proper comparison
+        p1 = Path(path1).resolve()
+        p2 = Path(path2).resolve()
+
+        # Check if either path is a parent of the other
+        try:
+            # Check if p2 is under p1
+            p2.relative_to(p1)
+            return True
+        except ValueError:
+            # Not nested in this direction, check the other
+            try:
+                # Check if p1 is under p2
+                p1.relative_to(p2)
+                return True
+            except ValueError:
+                # Not nested in either direction
+                return False
+
     async def add_project(self, name: str, path: str, set_default: bool = False) -> None:
         """Add a new project to the configuration and database.
 
@@ -142,6 +176,29 @@ async def add_project(self, name: str, path: str, set_default: bool = False) ->
         else:
             resolved_path = Path(os.path.abspath(os.path.expanduser(path))).as_posix()
 
+        # Check for nested paths with existing projects
+        existing_projects = await self.list_projects()
+        for existing in existing_projects:
+            if self._check_nested_paths(resolved_path, existing.path):
+                # Determine which path is nested within which for appropriate error message
+                p_new = Path(resolved_path).resolve()
+                p_existing = Path(existing.path).resolve()
+
+                # Check if new path is nested under existing project
+                if p_new.is_relative_to(p_existing):
+                    raise ValueError(
+                        f"Cannot create project at '{resolved_path}': "
+                        f"path is nested within existing project '{existing.name}' at '{existing.path}'. "
+                        f"Projects cannot share directory trees."
+                    )
+                else:
+                    # Existing project is nested under new path
+                    raise ValueError(
+                        f"Cannot create project at '{resolved_path}': "
+                        f"existing project '{existing.name}' at '{existing.path}' is nested within this path. "
+                        f"Projects cannot share directory trees."
+                    )
+
         # First add to config file (this will validate the project doesn't exist)
         project_config = self.config_manager.add_project(name, resolved_path)
 

test-int/cli/test_project_commands_integration.py

@@ -1,5 +1,8 @@
 """Integration tests for project CLI commands."""
 
+import tempfile
+from pathlib import Path
+
 from typer.testing import CliRunner
 
 from basic_memory.cli.main import app
@@ -52,61 +55,67 @@ def test_project_info_json(app_config, test_project, config_manager):
     assert "system" in data
 
 
-def test_project_add_and_remove(app_config, tmp_path, config_manager):
+def test_project_add_and_remove(app_config, config_manager):
     """Test adding and removing a project."""
     runner = CliRunner()
-    new_project_path = tmp_path / "new-project"
-    new_project_path.mkdir()
 
-    # Add project
-    result = runner.invoke(app, ["project", "add", "new-project", str(new_project_path)])
+    # Use a separate temporary directory to avoid nested path conflicts
+    with tempfile.TemporaryDirectory() as temp_dir:
+        new_project_path = Path(temp_dir) / "new-project"
+        new_project_path.mkdir()
 
-    if result.exit_code != 0:
-        print(f"STDOUT: {result.stdout}")
-        print(f"STDERR: {result.stderr}")
-    assert result.exit_code == 0
-    assert (
-        "Project 'new-project' added successfully" in result.stdout
-        or "added" in result.stdout.lower()
-    )
+        # Add project
+        result = runner.invoke(app, ["project", "add", "new-project", str(new_project_path)])
 
-    # Verify it shows up in list
-    result = runner.invoke(app, ["project", "list"])
-    assert result.exit_code == 0
-    assert "new-project" in result.stdout
+        if result.exit_code != 0:
+            print(f"STDOUT: {result.stdout}")
+            print(f"STDERR: {result.stderr}")
+        assert result.exit_code == 0
+        assert (
+            "Project 'new-project' added successfully" in result.stdout
+            or "added" in result.stdout.lower()
+        )
 
-    # Remove project
-    result = runner.invoke(app, ["project", "remove", "new-project"])
-    assert result.exit_code == 0
-    assert "removed" in result.stdout.lower() or "deleted" in result.stdout.lower()
+        # Verify it shows up in list
+        result = runner.invoke(app, ["project", "list"])
+        assert result.exit_code == 0
+        assert "new-project" in result.stdout
+
+        # Remove project
+        result = runner.invoke(app, ["project", "remove", "new-project"])
+        assert result.exit_code == 0
+        assert "removed" in result.stdout.lower() or "deleted" in result.stdout.lower()
 
 
-def test_project_set_default(app_config, tmp_path, config_manager):
+def test_project_set_default(app_config, config_manager):
     """Test setting default project."""
     runner = CliRunner()
-    new_project_path = tmp_path / "another-project"
-    new_project_path.mkdir()
 
-    # Add a second project
-    result = runner.invoke(app, ["project", "add", "another-project", str(new_project_path)])
-    if result.exit_code != 0:
-        print(f"STDOUT: {result.stdout}")
-        print(f"STDERR: {result.stderr}")
-    assert result.exit_code == 0
-
-    # Set as default
-    result = runner.invoke(app, ["project", "default", "another-project"])
-    if result.exit_code != 0:
-        print(f"STDOUT: {result.stdout}")
-        print(f"STDERR: {result.stderr}")
-    assert result.exit_code == 0
-    assert "default" in result.stdout.lower()
-
-    # Verify in list
-    result = runner.invoke(app, ["project", "list"])
-    assert result.exit_code == 0
-    # The new project should have the checkmark now
-    lines = result.stdout.split("\n")
-    for line in lines:
-        if "another-project" in line:
-            assert "✓" in line
+    # Use a separate temporary directory to avoid nested path conflicts
+    with tempfile.TemporaryDirectory() as temp_dir:
+        new_project_path = Path(temp_dir) / "another-project"
+        new_project_path.mkdir()
+
+        # Add a second project
+        result = runner.invoke(app, ["project", "add", "another-project", str(new_project_path)])
+        if result.exit_code != 0:
+            print(f"STDOUT: {result.stdout}")
+            print(f"STDERR: {result.stderr}")
+        assert result.exit_code == 0
+
+        # Set as default
+        result = runner.invoke(app, ["project", "default", "another-project"])
+        if result.exit_code != 0:
+            print(f"STDOUT: {result.stdout}")
+            print(f"STDERR: {result.stderr}")
+        assert result.exit_code == 0
+        assert "default" in result.stdout.lower()
+
+        # Verify in list
+        result = runner.invoke(app, ["project", "list"])
+        assert result.exit_code == 0
+        # The new project should have the checkmark now
+        lines = result.stdout.split("\n")
+        for line in lines:
+            if "another-project" in line:
+                assert "✓" in line

test-int/mcp/test_project_management_integration.py

@@ -512,3 +512,42 @@ async def test_case_preservation_in_project_list(mcp_server, app, test_project):
         # Clean up - delete test projects
         for project_name in test_projects:
             await client.call_tool("delete_project", {"project_name": project_name})
+
+
+@pytest.mark.asyncio
+async def test_nested_project_paths_rejected(mcp_server, app, test_project):
+    """Test that creating nested project paths is rejected with clear error message."""
+
+    async with Client(mcp_server) as client:
+        # Create a parent project
+        parent_name = "parent-project"
+        parent_path = "/tmp/nested-test/parent"
+
+        await client.call_tool(
+            "create_memory_project",
+            {
+                "project_name": parent_name,
+                "project_path": parent_path,
+            },
+        )
+
+        # Try to create a child project nested under the parent
+        child_name = "child-project"
+        child_path = "/tmp/nested-test/parent/child"
+
+        with pytest.raises(Exception) as exc_info:
+            await client.call_tool(
+                "create_memory_project",
+                {
+                    "project_name": child_name,
+                    "project_path": child_path,
+                },
+            )
+
+        # Verify error message mentions nested paths
+        error_message = str(exc_info.value)
+        assert "nested" in error_message.lower()
+        assert parent_name in error_message or parent_path in error_message
+
+        # Clean up parent project
+        await client.call_tool("delete_project", {"project_name": parent_name})