fix: Add missing foreign key constraints for project removal (#254) (#258)

Signed-off-by: Joe P <joe@basicmemory.com>
Signed-off-by: joe@basicmemory.com
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: jope-bm <jope-bm@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>

src/basic_memory/alembic/versions/a1b2c3d4e5f6_fix_project_foreign_keys.py

@@ -0,0 +1,54 @@
+"""fix project foreign keys
+
+Revision ID: a1b2c3d4e5f6
+Revises: 647e7a75e2cd
+Create Date: 2025-08-19 22:06:00.000000
+
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision: str = "a1b2c3d4e5f6"
+down_revision: Union[str, None] = "647e7a75e2cd"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    """Re-establish foreign key constraints that were lost during project table recreation.
+    
+    The migration 647e7a75e2cd recreated the project table but did not re-establish
+    the foreign key constraint from entity.project_id to project.id, causing
+    foreign key constraint failures when trying to delete projects with related entities.
+    """
+    # SQLite doesn't allow adding foreign key constraints to existing tables easily
+    # We need to be careful and handle the case where the constraint might already exist
+    
+    with op.batch_alter_table("entity", schema=None) as batch_op:
+        # Try to drop existing foreign key constraint (may not exist)
+        try:
+            batch_op.drop_constraint("fk_entity_project_id", type_="foreignkey")
+        except Exception:
+            # Constraint may not exist, which is fine - we'll create it next
+            pass
+        
+        # Add the foreign key constraint with CASCADE DELETE
+        # This ensures that when a project is deleted, all related entities are also deleted
+        batch_op.create_foreign_key(
+            "fk_entity_project_id", 
+            "project", 
+            ["project_id"], 
+            ["id"],
+            ondelete="CASCADE"
+        )
+
+
+def downgrade() -> None:
+    """Remove the foreign key constraint."""
+    with op.batch_alter_table("entity", schema=None) as batch_op:
+        batch_op.drop_constraint("fk_entity_project_id", type_="foreignkey")

tests/db/test_issue_254_foreign_key_constraints.py

@@ -0,0 +1,154 @@
+"""Test to verify that issue #254 is fixed.
+
+Issue #254: Foreign key constraint failures when deleting projects with related entities.
+
+The issue was that when migration 647e7a75e2cd recreated the project table,
+it did not re-establish the foreign key constraint from entity.project_id to project.id
+with CASCADE DELETE, causing foreign key constraint failures when trying to delete
+projects that have related entities.
+
+Migration a1b2c3d4e5f6 was created to fix this by adding the missing foreign key
+constraint with CASCADE DELETE behavior.
+
+This test file verifies that the fix works correctly in production databases
+that have had the migration applied.
+"""
+from datetime import datetime, timezone
+
+import pytest
+
+from basic_memory.services.project_service import ProjectService
+
+
+#@pytest.mark.skip(reason="Issue #254 not fully resolved yet - foreign key constraint errors still occur")
+@pytest.mark.asyncio
+async def test_issue_254_foreign_key_constraint_fix(project_service: ProjectService, tmp_path):
+    """Test to verify issue #254 is fixed: project removal with foreign key constraints.
+    
+    This test reproduces the exact scenario from issue #254:
+    1. Create a project
+    2. Create entities, observations, and relations linked to that project  
+    3. Attempt to remove the project
+    4. Verify it succeeds without "FOREIGN KEY constraint failed" errors
+    5. Verify all related data is properly cleaned up via CASCADE DELETE
+    
+    Once issue #254 is fully fixed, remove the @pytest.mark.skip decorator.
+    """
+    test_project_name = "issue-254-verification"
+    test_project_path = str(tmp_path / "issue-254-verification")
+    
+    # Step 1: Create test project
+    await project_service.add_project(test_project_name, test_project_path)
+    project = await project_service.get_project(test_project_name)
+    assert project is not None, "Project should be created successfully"
+    
+    # Step 2: Create related entities that would cause foreign key constraint issues
+    from basic_memory.repository.entity_repository import EntityRepository
+    from basic_memory.repository.observation_repository import ObservationRepository
+    from basic_memory.repository.relation_repository import RelationRepository
+    
+    entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id)
+    obs_repo = ObservationRepository(project_service.repository.session_maker, project_id=project.id)
+    rel_repo = RelationRepository(project_service.repository.session_maker, project_id=project.id)
+    
+    # Create entity
+    entity_data = {
+        "title": "Issue 254 Test Entity",
+        "entity_type": "note",
+        "content_type": "text/markdown",
+        "project_id": project.id,
+        "permalink": "issue-254-entity",
+        "file_path": "issue-254-entity.md",
+        "checksum": "issue254test",
+        "created_at": datetime.now(timezone.utc),
+        "updated_at": datetime.now(timezone.utc),
+    }
+    entity = await entity_repo.create(entity_data)
+    
+    # Create observation linked to entity
+    observation_data = {
+        "entity_id": entity.id,
+        "content": "This observation should be cascade deleted",
+        "category": "test"
+    }
+    observation = await obs_repo.create(observation_data)
+    
+    # Create relation involving the entity
+    relation_data = {
+        "from_id": entity.id,
+        "to_name": "some-other-entity",
+        "relation_type": "relates-to"
+    }
+    relation = await rel_repo.create(relation_data)
+    
+    # Step 3: Attempt to remove the project
+    # This is where issue #254 manifested - should NOT raise "FOREIGN KEY constraint failed"
+    try:
+        await project_service.remove_project(test_project_name)
+    except Exception as e:
+        if "FOREIGN KEY constraint failed" in str(e):
+            pytest.fail(
+                f"Issue #254 not fixed - foreign key constraint error still occurs: {e}. "
+                f"The migration a1b2c3d4e5f6 may not have been applied correctly or "
+                f"the CASCADE DELETE constraint is not working as expected."
+            )
+        else:
+            # Re-raise unexpected errors
+            raise
+    
+    # Step 4: Verify project was successfully removed
+    removed_project = await project_service.get_project(test_project_name)
+    assert removed_project is None, "Project should have been removed"
+    
+    # Step 5: Verify related data was cascade deleted
+    remaining_entity = await entity_repo.find_by_id(entity.id)
+    assert remaining_entity is None, "Entity should have been cascade deleted"
+    
+    remaining_observation = await obs_repo.find_by_id(observation.id)
+    assert remaining_observation is None, "Observation should have been cascade deleted"
+    
+    remaining_relation = await rel_repo.find_by_id(relation.id)
+    assert remaining_relation is None, "Relation should have been cascade deleted"
+
+
+@pytest.mark.asyncio
+async def test_issue_254_reproduction(project_service: ProjectService, tmp_path):
+    """Test that reproduces issue #254 to document the current state.
+    
+    This test demonstrates the current behavior and will fail until the issue is fixed.
+    It serves as documentation of what the problem was.
+    """
+    test_project_name = "issue-254-reproduction"
+    test_project_path = str(tmp_path / "issue-254-reproduction")
+    
+    # Create project and entity
+    await project_service.add_project(test_project_name, test_project_path)
+    project = await project_service.get_project(test_project_name)
+    
+    from basic_memory.repository.entity_repository import EntityRepository
+    entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id)
+    
+    entity_data = {
+        "title": "Reproduction Entity",
+        "entity_type": "note",
+        "content_type": "text/markdown",
+        "project_id": project.id,
+        "permalink": "reproduction-entity",
+        "file_path": "reproduction-entity.md",
+        "checksum": "repro123",
+        "created_at": datetime.now(timezone.utc),
+        "updated_at": datetime.now(timezone.utc),
+    }
+    entity = await entity_repo.create(entity_data)
+    
+    # This should eventually work without errors once issue #254 is fixed
+    #with pytest.raises(Exception) as exc_info:
+    await project_service.remove_project(test_project_name)
+    
+    # Document the current error for tracking
+    # error_message = str(exc_info.value)
+    # assert any(keyword in error_message for keyword in [
+    #     "FOREIGN KEY constraint failed",
+    #     "constraint",
+    #     "integrity"
+    # ]), f"Expected foreign key or integrity constraint error, got: {error_message}"

tests/services/test_project_removal_bug.py

@@ -0,0 +1,125 @@
+"""Test for project removal bug #254."""
+
+import os
+from datetime import timezone, datetime
+
+import pytest
+
+from basic_memory.services.project_service import ProjectService
+
+
+@pytest.mark.asyncio
+async def test_remove_project_with_related_entities(project_service: ProjectService, tmp_path):
+    """Test removing a project that has related entities (reproduces issue #254).
+    
+    This test verifies that projects with related entities (entities, observations, relations)
+    can be properly deleted without foreign key constraint violations.
+    
+    The bug was caused by missing foreign key constraints with CASCADE DELETE after
+    the project table was recreated in migration 647e7a75e2cd.
+    """
+    test_project_name = f"test-remove-with-entities-{os.urandom(4).hex()}"
+    test_project_path = str(tmp_path / "test-remove-with-entities")
+
+    # Make sure the test directory exists
+    os.makedirs(test_project_path, exist_ok=True)
+
+    try:
+        # Step 1: Add the test project
+        await project_service.add_project(test_project_name, test_project_path)
+        
+        # Verify project exists
+        project = await project_service.get_project(test_project_name)
+        assert project is not None
+        
+        # Step 2: Create related entities for this project
+        from basic_memory.repository.entity_repository import EntityRepository
+        entity_repo = EntityRepository(project_service.repository.session_maker, project_id=project.id)
+        
+        entity_data = {
+            "title": "Test Entity for Deletion",
+            "entity_type": "note", 
+            "content_type": "text/markdown",
+            "project_id": project.id,
+            "permalink": "test-deletion-entity",
+            "file_path": "test-deletion-entity.md",
+            "checksum": "test123",
+            "created_at": datetime.now(timezone.utc),
+            "updated_at": datetime.now(timezone.utc),
+        }
+        entity = await entity_repo.create(entity_data)
+        assert entity is not None
+        
+        # Step 3: Create observations for the entity
+        from basic_memory.repository.observation_repository import ObservationRepository
+        obs_repo = ObservationRepository(project_service.repository.session_maker, project_id=project.id)
+        
+        observation_data = {
+            "entity_id": entity.id,
+            "content": "This is a test observation",
+            "category": "note"
+        }
+        observation = await obs_repo.create(observation_data)
+        assert observation is not None
+        
+        # Step 4: Create relations involving the entity 
+        from basic_memory.repository.relation_repository import RelationRepository
+        rel_repo = RelationRepository(project_service.repository.session_maker, project_id=project.id)
+        
+        relation_data = {
+            "from_id": entity.id,
+            "to_name": "some-target-entity",
+            "relation_type": "relates-to"
+        }
+        relation = await rel_repo.create(relation_data)
+        assert relation is not None
+        
+        # Step 5: Attempt to remove the project
+        # This should work with proper cascade delete, or fail with foreign key constraint
+        await project_service.remove_project(test_project_name)
+        
+        # Step 6: Verify everything was properly deleted
+        
+        # Project should be gone
+        removed_project = await project_service.get_project(test_project_name)
+        assert removed_project is None, "Project should have been removed"
+        
+        # Related entities should be cascade deleted
+        remaining_entity = await entity_repo.find_by_id(entity.id)
+        assert remaining_entity is None, "Entity should have been cascade deleted"
+        
+        # Observations should be cascade deleted
+        remaining_obs = await obs_repo.find_by_id(observation.id)
+        assert remaining_obs is None, "Observation should have been cascade deleted"
+        
+        # Relations should be cascade deleted  
+        remaining_rel = await rel_repo.find_by_id(relation.id)
+        assert remaining_rel is None, "Relation should have been cascade deleted"
+        
+    except Exception as e:
+        # Check if this is the specific foreign key constraint error from the bug report
+        if "FOREIGN KEY constraint failed" in str(e):
+            pytest.fail(
+                f"Bug #254 reproduced: {e}. "
+                "This indicates missing foreign key constraints with CASCADE DELETE. "
+                "Run migration a1b2c3d4e5f6_fix_project_foreign_keys.py to fix this."
+            )
+        else:
+            # Re-raise other unexpected errors
+            raise e
+    
+    finally:
+        # Clean up - remove project if it still exists
+        if test_project_name in project_service.projects:
+            try:
+                await project_service.remove_project(test_project_name)
+            except Exception:
+                # Manual cleanup if remove_project fails
+                try:
+                    project_service.config_manager.remove_project(test_project_name)
+                except Exception:
+                    pass
+                
+                project = await project_service.get_project(test_project_name)
+                if project:
+                    await project_service.repository.delete(project.id)

tests/test_production_cascade_delete.py

@@ -0,0 +1,328 @@
+#!/usr/bin/env python3
+"""
+Test script to verify cascade delete behavior on production SQLite database.
+
+This script tests whether foreign key constraints with CASCADE DELETE are properly
+configured in the production database at ~/.basic-memory/memory.db.
+
+Usage: python test_production_cascade_delete.py
+"""
+
+import asyncio
+import os
+import sqlite3
+import sys
+from datetime import datetime, timezone
+from pathlib import Path
+from typing import Optional
+
+import asyncio
+from sqlalchemy import text
+from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker, AsyncSession
+
+
+class ProductionCascadeTest:
+    """Test cascade delete behavior on production database."""
+    
+    def __init__(self, db_path: Optional[Path] = None):
+        """Initialize test with database path."""
+        if db_path is None:
+            # Default to standard Basic Memory location
+            home_dir = Path.home()
+            self.db_path = home_dir / ".basic-memory" / "memory.db"
+        else:
+            self.db_path = db_path
+            
+        # Create backup path
+        self.backup_path = self.db_path.with_suffix('.db.backup')
+        
+        self.engine = None
+        self.session_maker = None
+        
+    async def setup(self):
+        """Setup database connection."""
+        if not self.db_path.exists():
+            print(f"❌ Production database not found at: {self.db_path}")
+            print("Please ensure Basic Memory has been initialized and the database exists.")
+            sys.exit(1)
+            
+        print(f"📁 Using database: {self.db_path}")
+        
+        # Create backup
+        print(f"💾 Creating backup: {self.backup_path}")
+        import shutil
+        shutil.copy2(self.db_path, self.backup_path)
+        
+        # Connect to database
+        db_url = f"sqlite+aiosqlite:///{self.db_path}"
+        self.engine = create_async_engine(db_url, connect_args={"check_same_thread": False})
+        self.session_maker = async_sessionmaker(self.engine, expire_on_commit=False)
+        
+    async def cleanup(self):
+        """Cleanup database connection."""
+        if self.engine:
+            await self.engine.dispose()
+            
+    async def check_foreign_keys_enabled(self) -> bool:
+        """Check if foreign keys are enabled in this session."""
+        async with self.session_maker() as session:
+            # Enable foreign keys like production does
+            await session.execute(text("PRAGMA foreign_keys=ON"))
+            
+            result = await session.execute(text("PRAGMA foreign_keys"))
+            fk_enabled = result.fetchone()[0]
+            return bool(fk_enabled)
+    
+    async def check_schema(self):
+        """Check current database schema for foreign key constraints."""
+        async with self.session_maker() as session:
+            await session.execute(text("PRAGMA foreign_keys=ON"))
+            
+            # Check entity table foreign keys
+            result = await session.execute(text("PRAGMA foreign_key_list(entity)"))
+            entity_fks = result.fetchall()
+            
+            print("🔍 Current entity table foreign key constraints:")
+            for fk in entity_fks:
+                print(f"  - Column: {fk[3]} -> {fk[2]}.{fk[4]} (ON DELETE: {fk[6]})")
+                
+            # Check if CASCADE DELETE is configured
+            has_cascade = any(fk[6] == 'CASCADE' for fk in entity_fks)
+            
+            if has_cascade:
+                print("✅ CASCADE DELETE is configured")
+            else:
+                print("❌ CASCADE DELETE is NOT configured (uses NO ACTION)")
+                
+            return has_cascade
+    
+    async def create_test_data(self) -> tuple[int, int]:
+        """Create test project and entity. Returns (project_id, entity_id)."""
+        async with self.session_maker() as session:
+            await session.execute(text("PRAGMA foreign_keys=ON"))
+            
+            # Create test project
+            project_sql = """
+            INSERT INTO project (name, description, permalink, path, is_active, is_default, created_at, updated_at)
+            VALUES (:name, :description, :permalink, :path, :is_active, :is_default, :created_at, :updated_at)
+            """
+            now = datetime.now(timezone.utc)
+            
+            result = await session.execute(text(project_sql), {
+                "name": "cascade-test-project",
+                "description": "Test project for cascade delete verification", 
+                "permalink": "cascade-test-project",
+                "path": "/tmp/cascade-test",
+                "is_active": True,
+                "is_default": False,
+                "created_at": now,
+                "updated_at": now
+            })
+            project_id = result.lastrowid
+            
+            # Create test entity linked to project
+            entity_sql = """
+            INSERT INTO entity (title, entity_type, content_type, project_id, permalink, file_path, 
+                              checksum, created_at, updated_at)
+            VALUES (:title, :entity_type, :content_type, :project_id, :permalink, :file_path, 
+                    :checksum, :created_at, :updated_at)
+            """
+            
+            result = await session.execute(text(entity_sql), {
+                "title": "Cascade Test Entity",
+                "entity_type": "note",
+                "content_type": "text/markdown", 
+                "project_id": project_id,
+                "permalink": "cascade-test-entity",
+                "file_path": "cascade-test-entity.md",
+                "checksum": "test-checksum",
+                "created_at": now,
+                "updated_at": now
+            })
+            entity_id = result.lastrowid
+            
+            await session.commit()
+            
+            print(f"📝 Created test project (ID: {project_id}) and entity (ID: {entity_id})")
+            return project_id, entity_id
+    
+    async def verify_test_data_exists(self, project_id: int, entity_id: int) -> bool:
+        """Verify test data exists before deletion."""
+        async with self.session_maker() as session:
+            # Check project exists
+            result = await session.execute(
+                text("SELECT COUNT(*) FROM project WHERE id = :project_id"), {"project_id": project_id}
+            )
+            project_count = result.fetchone()[0]
+            
+            # Check entity exists
+            result = await session.execute(
+                text("SELECT COUNT(*) FROM entity WHERE id = :entity_id"), {"entity_id": entity_id}
+            )
+            entity_count = result.fetchone()[0]
+            
+            exists = project_count > 0 and entity_count > 0
+            if exists:
+                print(f"✅ Test data verified: project ({project_count}) and entity ({entity_count}) exist")
+            else:
+                print(f"❌ Test data missing: project ({project_count}) and entity ({entity_count})")
+                
+            return exists
+    
+    async def test_cascade_delete(self, project_id: int, entity_id: int) -> bool:
+        """Test if deleting project cascades to delete entity."""
+        async with self.session_maker() as session:
+            await session.execute(text("PRAGMA foreign_keys=ON"))
+            
+            try:
+                # Attempt to delete project
+                print(f"🗑️  Attempting to delete project (ID: {project_id})...")
+                
+                result = await session.execute(
+                    text("DELETE FROM project WHERE id = :project_id"), {"project_id": project_id}
+                )
+                
+                if result.rowcount == 0:
+                    print("❌ Project deletion failed - no rows affected")
+                    return False
+                    
+                await session.commit()
+                print("✅ Project deletion succeeded")
+                
+                # Check if entity was cascade deleted
+                result = await session.execute(
+                    text("SELECT COUNT(*) FROM entity WHERE id = :entity_id"), {"entity_id": entity_id}
+                )
+                entity_count = result.fetchone()[0]
+                
+                if entity_count == 0:
+                    print("✅ CASCADE DELETE working: Entity was automatically deleted")
+                    return True
+                else:
+                    print("❌ CASCADE DELETE NOT working: Entity still exists after project deletion")
+                    return False
+                    
+            except Exception as e:
+                await session.rollback()
+                print(f"❌ Project deletion failed with error: {e}")
+                
+                # Check if it's a foreign key constraint error
+                if "FOREIGN KEY constraint failed" in str(e):
+                    print("🔍 This confirms foreign key constraints are enforced but CASCADE DELETE is not configured")
+                
+                return False
+    
+    async def cleanup_test_data(self, project_id: int, entity_id: int):
+        """Clean up any remaining test data."""
+        async with self.session_maker() as session:
+            await session.execute(text("PRAGMA foreign_keys=ON"))
+            
+            try:
+                # Delete entity first (in case cascade didn't work)
+                await session.execute(text("DELETE FROM entity WHERE id = :entity_id"), {"entity_id": entity_id})
+                
+                # Delete project
+                await session.execute(text("DELETE FROM project WHERE id = :project_id"), {"project_id": project_id})
+                
+                await session.commit()
+                print("🧹 Cleaned up any remaining test data")
+                
+            except Exception as e:
+                print(f"⚠️  Error during cleanup: {e}")
+                await session.rollback()
+    
+    async def restore_backup(self):
+        """Restore database from backup."""
+        if self.backup_path.exists():
+            print(f"🔄 Restoring database from backup...")
+            import shutil
+            shutil.copy2(self.backup_path, self.db_path)
+            print("✅ Database restored from backup")
+            
+            # Remove backup file
+            self.backup_path.unlink()
+            print("🗑️  Backup file removed")
+        else:
+            print("⚠️  No backup file found to restore")
+    
+    async def run_test(self) -> bool:
+        """Run the complete cascade delete test."""
+        print("🧪 Production Database CASCADE DELETE Test")
+        print("=" * 50)
+        
+        try:
+            await self.setup()
+            
+            # Check if foreign keys are enabled
+            fk_enabled = await self.check_foreign_keys_enabled()
+            print(f"🔐 Foreign keys enabled: {fk_enabled}")
+            
+            if not fk_enabled:
+                print("❌ Foreign keys are not enabled - this test requires foreign key enforcement")
+                return False
+            
+            # Check current schema
+            has_cascade = await self.check_schema()
+            
+            # Create test data
+            project_id, entity_id = await self.create_test_data()
+            
+            # Verify test data exists
+            if not await self.verify_test_data_exists(project_id, entity_id):
+                return False
+            
+            # Test cascade delete
+            cascade_works = await self.test_cascade_delete(project_id, entity_id)
+            
+            # Clean up any remaining test data
+            await self.cleanup_test_data(project_id, entity_id)
+            
+            print("\n" + "=" * 50)
+            print("🧪 TEST RESULTS:")
+            print(f"  Schema has CASCADE DELETE: {has_cascade}")
+            print(f"  CASCADE DELETE works: {cascade_works}")
+            
+            if has_cascade and cascade_works:
+                print("✅ PASS: Foreign key constraints are properly configured with CASCADE DELETE")
+            elif not has_cascade and not cascade_works:
+                print("❌ FAIL: Foreign key constraints are missing CASCADE DELETE configuration")
+                print("💡 This confirms issue #254 - migration a1b2c3d4e5f6 is needed")
+            else:
+                print("⚠️  MIXED: Unexpected result combination")
+            
+            return cascade_works
+            
+        except Exception as e:
+            print(f"💥 Test failed with error: {e}")
+            return False
+        finally:
+            await self.cleanup()
+            # Always restore backup to avoid leaving test data
+            await self.restore_backup()
+
+
+async def main():
+    """Main test function."""
+    import argparse
+    
+    parser = argparse.ArgumentParser(description="Test cascade delete on production database")
+    parser.add_argument("--db-path", type=Path, help="Path to database file (default: ~/.basic-memory/memory.db)")
+    parser.add_argument("--no-backup", action="store_true", help="Skip creating backup (dangerous)")
+    
+    args = parser.parse_args()
+    
+    if args.no_backup:
+        print("⚠️  WARNING: Running without backup!")
+        response = input("Are you sure? Type 'yes' to continue: ")
+        if response.lower() != 'yes':
+            print("❌ Aborted")
+            return
+    
+    test = ProductionCascadeTest(args.db_path)
+    success = await test.run_test()
+    
+    sys.exit(0 if success else 1)
+
+
+if __name__ == "__main__":
+    asyncio.run(main())