fix: UNIQUE constraint failed: entity.permalink issue #139 (#140)

Signed-off-by: Drew Cain <groksrc@users.noreply.github.com>
Signed-off-by: phernandez <paul@basicmachines.co>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: phernandez <paul@basicmachines.co>
Co-authored-by: Claude <noreply@anthropic.com>

Author: 5 people

src/basic_memory/alembic/env.py

@@ -8,12 +8,12 @@
 
 from alembic import context
 
-from basic_memory.models import Base
-
 # set config.env to "test" for pytest to prevent logging to file in utils.setup_logging()
 os.environ["BASIC_MEMORY_ENV"] = "test"
 
-from basic_memory.config import app_config
+# Import after setting environment variable  # noqa: E402
+from basic_memory.config import app_config  # noqa: E402
+from basic_memory.models import Base  # noqa: E402
 
 # this is the Alembic Config object, which provides
 # access to the values within the .ini file in use.

src/basic_memory/repository/entity_repository.py

@@ -3,10 +3,13 @@
 from pathlib import Path
 from typing import List, Optional, Sequence, Union
 
+from sqlalchemy import select
+from sqlalchemy.exc import IntegrityError
 from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker
 from sqlalchemy.orm import selectinload
 from sqlalchemy.orm.interfaces import LoaderOption
 
+from basic_memory import db
 from basic_memory.models.knowledge import Entity, Observation, Relation
 from basic_memory.repository.repository import Repository
 
@@ -96,3 +99,153 @@ async def find_by_permalinks(self, permalinks: List[str]) -> Sequence[Entity]:
 
         result = await self.execute_query(query)
         return list(result.scalars().all())
+
+    async def upsert_entity(self, entity: Entity) -> Entity:
+        """Insert or update entity using a hybrid approach.
+        
+        This method provides a cleaner alternative to the try/catch approach
+        for handling permalink and file_path conflicts. It first tries direct 
+        insertion, then handles conflicts intelligently.
+        
+        Args:
+            entity: The entity to insert or update
+            
+        Returns:
+            The inserted or updated entity
+        """
+
+        async with db.scoped_session(self.session_maker) as session:
+            # Set project_id if applicable and not already set
+            self._set_project_id_if_needed(entity)
+            
+            # Check for existing entity with same file_path first
+            existing_by_path = await session.execute(
+                select(Entity).where(
+                    Entity.file_path == entity.file_path,
+                    Entity.project_id == entity.project_id
+                )
+            )
+            existing_path_entity = existing_by_path.scalar_one_or_none()
+            
+            if existing_path_entity:
+                # Update existing entity with same file path
+                for key, value in {
+                    'title': entity.title,
+                    'entity_type': entity.entity_type,
+                    'entity_metadata': entity.entity_metadata,
+                    'content_type': entity.content_type,
+                    'permalink': entity.permalink,
+                    'checksum': entity.checksum,
+                    'updated_at': entity.updated_at,
+                }.items():
+                    setattr(existing_path_entity, key, value)
+                
+                await session.flush()
+                # Return with relationships loaded
+                query = (
+                    select(Entity)
+                    .where(Entity.file_path == entity.file_path)
+                    .options(*self.get_load_options())
+                )
+                result = await session.execute(query)
+                found = result.scalar_one_or_none()
+                if not found:  # pragma: no cover
+                    raise RuntimeError(f"Failed to retrieve entity after update: {entity.file_path}")
+                return found
+            
+            # No existing entity with same file_path, try insert
+            try:
+                # Simple insert for new entity
+                session.add(entity)
+                await session.flush()
+                
+                # Return with relationships loaded
+                query = (
+                    select(Entity)
+                    .where(Entity.file_path == entity.file_path)
+                    .options(*self.get_load_options())
+                )
+                result = await session.execute(query)
+                found = result.scalar_one_or_none()
+                if not found:  # pragma: no cover
+                    raise RuntimeError(f"Failed to retrieve entity after insert: {entity.file_path}")
+                return found
+                
+            except IntegrityError:
+                # Could be either file_path or permalink conflict
+                await session.rollback()
+                
+                # Check if it's a file_path conflict (race condition)
+                existing_by_path_check = await session.execute(
+                    select(Entity).where(
+                        Entity.file_path == entity.file_path,
+                        Entity.project_id == entity.project_id
+                    )
+                )
+                race_condition_entity = existing_by_path_check.scalar_one_or_none()
+                
+                if race_condition_entity:
+                    # Race condition: file_path conflict detected after our initial check
+                    # Update the existing entity instead
+                    for key, value in {
+                        'title': entity.title,
+                        'entity_type': entity.entity_type,
+                        'entity_metadata': entity.entity_metadata,
+                        'content_type': entity.content_type,
+                        'permalink': entity.permalink,
+                        'checksum': entity.checksum,
+                        'updated_at': entity.updated_at,
+                    }.items():
+                        setattr(race_condition_entity, key, value)
+                    
+                    await session.flush()
+                    # Return the updated entity with relationships loaded
+                    query = (
+                        select(Entity)
+                        .where(Entity.file_path == entity.file_path)
+                        .options(*self.get_load_options())
+                    )
+                    result = await session.execute(query)
+                    found = result.scalar_one_or_none()
+                    if not found:  # pragma: no cover
+                        raise RuntimeError(f"Failed to retrieve entity after race condition update: {entity.file_path}")
+                    return found
+                else:
+                    # Must be permalink conflict - generate unique permalink
+                    return await self._handle_permalink_conflict(entity, session)
+
+    async def _handle_permalink_conflict(self, entity: Entity, session: AsyncSession) -> Entity:
+        """Handle permalink conflicts by generating a unique permalink."""
+        base_permalink = entity.permalink
+        suffix = 1
+        
+        # Find a unique permalink
+        while True:
+            test_permalink = f"{base_permalink}-{suffix}"
+            existing = await session.execute(
+                select(Entity).where(
+                    Entity.permalink == test_permalink,
+                    Entity.project_id == entity.project_id
+                )
+            )
+            if existing.scalar_one_or_none() is None:
+                # Found unique permalink
+                entity.permalink = test_permalink
+                break
+            suffix += 1
+            
+        # Insert with unique permalink (no conflict possible now)
+        session.add(entity)
+        await session.flush()
+        
+        # Return the inserted entity with relationships loaded
+        query = (
+            select(Entity)
+            .where(Entity.file_path == entity.file_path)
+            .options(*self.get_load_options())
+        )
+        result = await session.execute(query)
+        found = result.scalar_one_or_none()
+        if not found:  # pragma: no cover
+            raise RuntimeError(f"Failed to retrieve entity after insert: {entity.file_path}")
+        return found

src/basic_memory/services/entity_service.py

@@ -292,27 +292,21 @@ async def create_entity_from_markdown(
 
         Creates the entity with null checksum to indicate sync not complete.
         Relations will be added in second pass.
+        
+        Uses UPSERT approach to handle permalink/file_path conflicts cleanly.
         """
         logger.debug(f"Creating entity: {markdown.frontmatter.title} file_path: {file_path}")
         model = entity_model_from_markdown(file_path, markdown)
 
         # Mark as incomplete because we still need to add relations
         model.checksum = None
-        # Repository will set project_id automatically
+        
+        # Use UPSERT to handle conflicts cleanly
         try:
-            return await self.repository.add(model)
-        except IntegrityError as e:
-            # Handle race condition where entity was created by another process
-            if "UNIQUE constraint failed: entity.file_path" in str(
-                e
-            ) or "UNIQUE constraint failed: entity.permalink" in str(e):
-                logger.info(
-                    f"Entity already exists for file_path={file_path} (file_path or permalink conflict), updating instead of creating"
-                )
-                return await self.update_entity_and_observations(file_path, markdown)
-            else:
-                # Re-raise if it's a different integrity error
-                raise
+            return await self.repository.upsert_entity(model)
+        except Exception as e:
+            logger.error(f"Failed to upsert entity for {file_path}: {e}")
+            raise EntityCreationError(f"Failed to create entity: {str(e)}") from e
 
     async def update_entity_and_observations(
         self, file_path: Path, markdown: EntityMarkdown

tests/mcp/test_tool_write_note.py

@@ -413,3 +413,66 @@ async def test_write_note_preserves_content_frontmatter(app):
         ).strip()
         in content
     )
+
+
+@pytest.mark.asyncio
+async def test_write_note_permalink_collision_fix_issue_139(app):
+    """Test fix for GitHub Issue #139: UNIQUE constraint failed: entity.permalink.
+    
+    This reproduces the exact scenario described in the issue:
+    1. Create a note with title "Note 1" 
+    2. Create another note with title "Note 2"
+    3. Try to create/replace first note again with same title "Note 1"
+    
+    Before the fix, step 3 would fail with UNIQUE constraint error.
+    After the fix, it should either update the existing note or create with unique permalink.
+    """
+    # Step 1: Create first note
+    result1 = await write_note.fn(
+        title="Note 1",
+        folder="test",
+        content="Original content for note 1"
+    )
+    assert "# Created note" in result1
+    assert "permalink: test/note-1" in result1
+    
+    # Step 2: Create second note with different title
+    result2 = await write_note.fn(
+        title="Note 2", 
+        folder="test",
+        content="Content for note 2"
+    )
+    assert "# Created note" in result2
+    assert "permalink: test/note-2" in result2
+    
+    # Step 3: Try to create/replace first note again
+    # This scenario would trigger the UNIQUE constraint failure before the fix
+    result3 = await write_note.fn(
+        title="Note 1",  # Same title as first note
+        folder="test",   # Same folder as first note
+        content="Replacement content for note 1"  # Different content
+    )
+    
+    # This should not raise a UNIQUE constraint failure error
+    # It should succeed and either:
+    # 1. Update the existing note (preferred behavior)
+    # 2. Create a new note with unique permalink (fallback behavior)
+    
+    assert result3 is not None
+    assert ("Updated note" in result3 or "Created note" in result3)
+    
+    # The result should contain either the original permalink or a unique one
+    assert ("permalink: test/note-1" in result3 or "permalink: test/note-1-1" in result3)
+    
+    # Verify we can read back the content
+    if "permalink: test/note-1" in result3:
+        # Updated existing note case
+        content = await read_note.fn("test/note-1")
+        assert "Replacement content for note 1" in content
+    else:
+        # Created new note with unique permalink case  
+        content = await read_note.fn("test/note-1-1")
+        assert "Replacement content for note 1" in content
+        # Original note should still exist
+        original_content = await read_note.fn("test/note-1")
+        assert "Original content for note 1" in original_content