From dc27512be5677f370e9d620bce357a4d82f049fb Mon Sep 17 00:00:00 2001 From: phernandez Date: Thu, 16 Oct 2025 10:41:21 -0500 Subject: [PATCH] fix: Terminate sync immediately when project is deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #188 where sync fails with FOREIGN KEY constraint violations when a project is deleted during sync operations. Changes: - Add SyncFatalError exception to distinguish fatal from recoverable errors - Detect FOREIGN KEY failures in entity_repository.upsert_entity() - Check exception chain in sync_service.sync_file() and re-raise fatal errors - Add repository-level test for invalid project_id handling - Add integration test to verify fatal errors terminate sync immediately Before: Circuit breaker retries each file 3 times, wastes resources After: First FOREIGN KEY error terminates sync immediately with clear message Fatal errors (project deleted, database corrupt) now bypass the circuit breaker and terminate sync immediately, while recoverable file-level errors (bad markdown, parse errors) continue to use circuit breaker as intended. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Signed-off-by: phernandez --- .../repository/entity_repository.py | 15 +++- src/basic_memory/services/exceptions.py | 15 ++++ src/basic_memory/sync/sync_service.py | 8 ++ .../test_entity_repository_upsert.py | 30 ++++++++ tests/sync/test_sync_service.py | 76 +++++++++++++++++++ 5 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/basic_memory/repository/entity_repository.py b/src/basic_memory/repository/entity_repository.py index 486b23e64..3393584f5 100644 --- a/src/basic_memory/repository/entity_repository.py +++ b/src/basic_memory/repository/entity_repository.py @@ -135,7 +135,20 @@ async def upsert_entity(self, entity: Entity) -> Entity: ) return found - except IntegrityError: + except IntegrityError as e: + # Check if this is a FOREIGN KEY constraint failure + error_str = str(e) + if "FOREIGN KEY constraint failed" in error_str: + # Import locally to avoid circular dependency (repository -> services -> repository) + from basic_memory.services.exceptions import SyncFatalError + + # Project doesn't exist in database - this is a fatal sync error + raise SyncFatalError( + f"Cannot sync file '{entity.file_path}': " + f"project_id={entity.project_id} does not exist in database. " + f"The project may have been deleted. This sync will be terminated." + ) from e + await session.rollback() # Re-query after rollback to get a fresh, attached entity diff --git a/src/basic_memory/services/exceptions.py b/src/basic_memory/services/exceptions.py index 7adb0a18b..ac5cc6115 100644 --- a/src/basic_memory/services/exceptions.py +++ b/src/basic_memory/services/exceptions.py @@ -20,3 +20,18 @@ class DirectoryOperationError(Exception): """Raised when directory operations fail""" pass + + +class SyncFatalError(Exception): + """Raised when sync encounters a fatal error that prevents continuation. + + Fatal errors include: + - Project deleted during sync (FOREIGN KEY constraint) + - Database corruption + - Critical system failures + + When this exception is raised, the entire sync operation should be terminated + immediately rather than attempting to continue with remaining files. + """ + + pass diff --git a/src/basic_memory/sync/sync_service.py b/src/basic_memory/sync/sync_service.py index 0080da9b4..52f0c6b83 100644 --- a/src/basic_memory/sync/sync_service.py +++ b/src/basic_memory/sync/sync_service.py @@ -22,6 +22,7 @@ from basic_memory.repository import EntityRepository, RelationRepository, ObservationRepository from basic_memory.repository.search_repository import SearchRepository from basic_memory.services import EntityService, FileService +from basic_memory.services.exceptions import SyncFatalError from basic_memory.services.link_resolver import LinkResolver from basic_memory.services.search_service import SearchService from basic_memory.services.sync_status_service import sync_status_tracker, SyncStatus @@ -514,6 +515,13 @@ async def sync_file( return entity, checksum except Exception as e: + # Check if this is a fatal error (or caused by one) + # Fatal errors like project deletion should terminate sync immediately + if isinstance(e, SyncFatalError) or isinstance(e.__cause__, SyncFatalError): + logger.error(f"Fatal sync error encountered, terminating sync: path={path}") + raise + + # Otherwise treat as recoverable file-level error error_msg = str(e) logger.error(f"Failed to sync file: path={path}, error={error_msg}") diff --git a/tests/repository/test_entity_repository_upsert.py b/tests/repository/test_entity_repository_upsert.py index 037189db5..072416e78 100644 --- a/tests/repository/test_entity_repository_upsert.py +++ b/tests/repository/test_entity_repository_upsert.py @@ -6,6 +6,7 @@ from basic_memory.models.knowledge import Entity from basic_memory.repository.entity_repository import EntityRepository from basic_memory.repository.project_repository import ProjectRepository +from basic_memory.services.exceptions import SyncFatalError @pytest.mark.asyncio @@ -436,3 +437,32 @@ async def test_upsert_entity_permalink_conflict_within_project_only(session_make assert result1.project_id == project1.id assert result2.project_id == project2.id assert result3.project_id == project1.id + + +@pytest.mark.asyncio +async def test_upsert_entity_with_invalid_project_id(entity_repository: EntityRepository): + """Test that upserting with non-existent project_id raises clear error. + + This tests the fix for issue #188 where sync fails with FOREIGN KEY constraint + violations when a project is deleted during sync operations. + """ + # Create entity with non-existent project_id + entity = Entity( + title="Test Entity", + entity_type="note", + file_path="test.md", + permalink="test", + project_id=99999, # This project doesn't exist + content_type="text/markdown", + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc), + ) + + # Should raise SyncFatalError with clear message about missing project + with pytest.raises(SyncFatalError) as exc_info: + await entity_repository.upsert_entity(entity) + + error_msg = str(exc_info.value) + assert "project_id=99999 does not exist" in error_msg + assert "project may have been deleted" in error_msg.lower() + assert "sync will be terminated" in error_msg.lower() diff --git a/tests/sync/test_sync_service.py b/tests/sync/test_sync_service.py index 7bef4c9c3..38e68f6af 100644 --- a/tests/sync/test_sync_service.py +++ b/tests/sync/test_sync_service.py @@ -1565,3 +1565,79 @@ async def mock_compute_checksum(path): failure_info = sync_service._file_failures["checksum_fail.md"] assert failure_info.count == 1 assert failure_info.last_checksum == "" # Empty when checksum fails + + +@pytest.mark.asyncio +async def test_sync_fatal_error_terminates_sync_immediately( + sync_service: SyncService, project_config: ProjectConfig, entity_service: EntityService +): + """Test that SyncFatalError terminates sync immediately without circuit breaker retry. + + This tests the fix for issue #188 where project deletion during sync should + terminate immediately rather than retrying each file 3 times. + """ + from unittest.mock import patch + from basic_memory.services.exceptions import SyncFatalError + + project_dir = project_config.home + + # Create multiple test files + await create_test_file( + project_dir / "file1.md", + dedent( + """ + --- + type: knowledge + --- + # File 1 + Content 1 + """ + ), + ) + await create_test_file( + project_dir / "file2.md", + dedent( + """ + --- + type: knowledge + --- + # File 2 + Content 2 + """ + ), + ) + await create_test_file( + project_dir / "file3.md", + dedent( + """ + --- + type: knowledge + --- + # File 3 + Content 3 + """ + ), + ) + + # Mock entity_service.create_entity_from_markdown to raise SyncFatalError on first file + # This simulates project being deleted during sync + async def mock_create_entity_from_markdown(*args, **kwargs): + raise SyncFatalError( + "Cannot sync file 'file1.md': project_id=99999 does not exist in database. " + "The project may have been deleted. This sync will be terminated." + ) + + with patch.object( + entity_service, "create_entity_from_markdown", side_effect=mock_create_entity_from_markdown + ): + # Sync should raise SyncFatalError and terminate immediately + with pytest.raises(SyncFatalError, match="project_id=99999 does not exist"): + await sync_service.sync(project_dir) + + # Verify that circuit breaker did NOT record this as a file-level failure + # (SyncFatalError should bypass circuit breaker and re-raise immediately) + assert "file1.md" not in sync_service._file_failures + + # Verify that no other files were attempted (sync terminated on first error) + # If circuit breaker was used, we'd see file1 in failures + # If sync continued, we'd see attempts for file2 and file3