fix: concurrent delete race conditions in delete_entity (#702)

Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/basic_memory/services/entity_service.py

@@ -735,6 +735,10 @@ class EntityService(BaseService[EntityModel]):
                 entity = await self.get_by_permalink(permalink_or_id)
             else:
                 entities = await self.get_entities_by_id([permalink_or_id])
+                if len(entities) == 0:
+                    # Entity already deleted (concurrent delete or race condition)
+                    logger.info("Entity already deleted", entity_id=permalink_or_id)
+                    return True
                 if len(entities) != 1:  # pragma: no cover
                     logger.error(
                         "Entity lookup error", entity_id=permalink_or_id, found_count=len(entities)
@@ -746,13 +750,28 @@ class EntityService(BaseService[EntityModel]):
 
             # Delete from search index first (if search_service is available)
             if self.search_service:
-                await self.search_service.handle_delete(entity)
+                try:
+                    await self.search_service.handle_delete(entity)
+                except Exception:
+                    # Search cleanup is best-effort during concurrent deletes.
+                    # Relationships may have been cascade-deleted by a concurrent request.
+                    logger.warning(
+                        "Search cleanup failed for entity (likely concurrent delete)",
+                        permalink_or_id=permalink_or_id,
+                        exc_info=True,
+                    )
 
             # Delete file
             await self.file_service.delete_entity_file(entity)
 
             # Delete from DB (this will cascade to observations/relations)
-            return await self.repository.delete(entity.id)
+            # Trigger: repository.delete returns False when entity is already gone (NoResultFound)
+            # Why: concurrent delete_directory requests can race to delete the same entity
+            # Outcome: treat as success since the entity is deleted either way
+            deleted = await self.repository.delete(entity.id)
+            if not deleted:
+                logger.info("Entity already removed from DB", entity_id=permalink_or_id)
+            return True
 
         except EntityNotFoundError:
             logger.info(f"Entity not found: {permalink_or_id}")

tests/services/test_entity_service.py

@@ -2426,3 +2426,116 @@ async def test_fast_write_entity_null_user_id(entity_service: EntityService):
     entity = await entity_service.fast_write_entity(schema, external_id=str(uuid.uuid4()))
     assert entity.created_by is None
     assert entity.last_updated_by is None
+
+
+# --- Concurrent Delete Resilience ---
+
+
+@pytest.mark.asyncio
+async def test_delete_entity_by_id_already_deleted(entity_service: EntityService):
+    """delete_entity returns True when entity was already deleted (concurrent delete)."""
+    entity_data = EntitySchema(
+        title="ConcurrentDeleteTarget",
+        directory="test",
+        note_type="test",
+    )
+    created = await entity_service.create_entity(entity_data)
+    entity_id = created.id
+
+    # Delete once - should succeed
+    assert await entity_service.delete_entity(entity_id) is True
+
+    # Delete again by ID - should return True (already deleted), not raise ValueError
+    assert await entity_service.delete_entity(entity_id) is True
+
+
+@pytest.mark.asyncio
+async def test_delete_entity_by_permalink_already_deleted(entity_service: EntityService):
+    """delete_entity returns True when entity was already deleted by permalink."""
+    entity_data = EntitySchema(
+        title="ConcurrentDeleteByPermalink",
+        directory="test",
+        note_type="test",
+    )
+    created = await entity_service.create_entity(entity_data)
+
+    # Delete once
+    assert await entity_service.delete_entity(created.id) is True
+
+    # Delete again by permalink - should return True (EntityNotFoundError caught)
+    assert await entity_service.delete_entity(entity_data.permalink) is True
+
+
+@pytest.mark.asyncio
+async def test_delete_directory_concurrent_resilience(entity_service: EntityService):
+    """delete_directory succeeds even when entities are concurrently deleted."""
+    # Create entities in a directory
+    entities = []
+    for i in range(5):
+        entity_data = EntitySchema(
+            title=f"DirEntity{i}",
+            directory="concurrent-test",
+            note_type="test",
+        )
+        created = await entity_service.create_entity(entity_data)
+        entities.append(created)
+
+    # Delete some entities directly to simulate concurrent delete
+    await entity_service.delete_entity(entities[0].id)
+    await entity_service.delete_entity(entities[2].id)
+
+    # Now delete the directory - should handle already-deleted entities gracefully
+    result = await entity_service.delete_directory("concurrent-test")
+
+    # Only 3 remain in DB (2 were already deleted before the directory query)
+    assert result.total_files == 3
+    assert result.successful_deletes == 3
+    assert result.failed_deletes == 0
+    assert len(result.errors) == 0
+
+
+@pytest.mark.asyncio
+async def test_delete_directory_all_already_deleted(entity_service: EntityService):
+    """delete_directory handles case where all entities were concurrently deleted."""
+    # Create entities
+    created_entities = []
+    for i in range(3):
+        entity_data = EntitySchema(
+            title=f"AllGone{i}",
+            directory="all-deleted",
+            note_type="test",
+        )
+        created = await entity_service.create_entity(entity_data)
+        created_entities.append(created)
+
+    # Delete all entities directly
+    for e in created_entities:
+        await entity_service.delete_entity(e.id)
+
+    # Directory delete should report empty (entities no longer found in prefix query)
+    result = await entity_service.delete_directory("all-deleted")
+    assert result.total_files == 0
+    assert result.successful_deletes == 0
+    assert result.failed_deletes == 0
+
+
+@pytest.mark.asyncio
+async def test_delete_directory_entity_deleted_between_query_and_delete(
+    entity_service: EntityService,
+):
+    """Simulates the real race condition: entity exists in prefix query but is deleted
+    by a concurrent request before delete_entity is called."""
+    # Create entities
+    entity_data = EntitySchema(title="RaceTarget", directory="race-dir", note_type="test")
+    created = await entity_service.create_entity(entity_data)
+
+    # Get the entities via prefix query (as delete_directory does)
+    entities = await entity_service.repository.find_by_directory_prefix("race-dir")
+    assert len(entities) == 1
+
+    # Now delete the entity behind the scenes (simulating a concurrent request)
+    await entity_service.delete_entity(created.id)
+
+    # Call delete_entity with the stale entity ID - should return True, not raise
+    result = await entity_service.delete_entity(entities[0].id)
+    assert result is True