From f50650763dbd4322c132e4bdc959ce4bf074374b Mon Sep 17 00:00:00 2001 From: Paul Hernandez <60959+phernandez@users.noreply.github.com> Date: Wed, 25 Jun 2025 12:57:44 -0500 Subject: [PATCH] fix: ensure permalinks are generated for entities with null permalinks during move operations (#162) Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Paul Hernandez --- src/basic_memory/services/entity_service.py | 9 ++- tests/services/test_entity_service.py | 66 +++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/services/entity_service.py b/src/basic_memory/services/entity_service.py index 78e4381c..3c0da241 100644 --- a/src/basic_memory/services/entity_service.py +++ b/src/basic_memory/services/entity_service.py @@ -682,8 +682,8 @@ class EntityService(BaseService[EntityModel]): # 6. Prepare database updates updates = {"file_path": destination_path} - # 7. Update permalink if configured - if app_config.update_permalinks_on_move: + # 7. Update permalink if configured or if entity has null permalink + if app_config.update_permalinks_on_move or old_permalink is None: # Generate new permalink from destination path new_permalink = await self.resolve_permalink(destination_path) @@ -693,7 +693,10 @@ class EntityService(BaseService[EntityModel]): ) updates["permalink"] = new_permalink - logger.info(f"Updated permalink: {old_permalink} -> {new_permalink}") + if old_permalink is None: + logger.info(f"Generated permalink for entity with null permalink: {new_permalink}") + else: + logger.info(f"Updated permalink: {old_permalink} -> {new_permalink}") # 8. Recalculate checksum new_checksum = await self.file_service.compute_checksum(destination_path) diff --git a/tests/services/test_entity_service.py b/tests/services/test_entity_service.py index 47942acd..fbad95fa 100644 --- a/tests/services/test_entity_service.py +++ b/tests/services/test_entity_service.py @@ -1728,3 +1728,69 @@ async def test_move_entity_with_complex_observations( assert "Branch Strategy" in relation_targets assert "Multiple" in relation_targets assert "Links" in relation_targets + + +@pytest.mark.asyncio +async def test_move_entity_with_null_permalink_generates_permalink( + entity_service: EntityService, + project_config: ProjectConfig, + entity_repository: EntityRepository, +): + """Test that moving entity with null permalink generates a new permalink automatically. + + This tests the fix for issue #155 where entities with null permalinks from the database + migration would fail validation when being moved. The fix ensures that entities with + null permalinks get a generated permalink during move operations, regardless of the + update_permalinks_on_move setting. + """ + # Create entity through direct database insertion to simulate migrated entity with null permalink + from basic_memory.models.knowledge import Entity as EntityModel + from datetime import datetime, timezone + + # Create an entity with null permalink directly in database (simulating migrated data) + entity_data = { + "title": "Test Entity", + "file_path": "test/null-permalink-entity.md", + "entity_type": "note", + "content_type": "text/markdown", + "permalink": None, # This is the key - null permalink from migration + "created_at": datetime.now(timezone.utc), + "updated_at": datetime.now(timezone.utc), + } + + # Create the entity directly in database + created_entity = await entity_repository.create(entity_data) + assert created_entity.permalink is None + + # Create the physical file + file_path = project_config.home / created_entity.file_path + file_path.parent.mkdir(parents=True, exist_ok=True) + file_path.write_text("# Test Entity\n\nContent here.") + + # Configure move without permalink updates (the default setting that previously triggered the bug) + app_config = BasicMemoryConfig(update_permalinks_on_move=False) + + # Move entity - this should now succeed and generate a permalink + moved_entity = await entity_service.move_entity( + identifier=created_entity.title, # Use title since permalink is None + destination_path="moved/test-entity.md", + project_config=project_config, + app_config=app_config, + ) + + # Verify the move succeeded and a permalink was generated + assert moved_entity is not None + assert moved_entity.file_path == "moved/test-entity.md" + assert moved_entity.permalink is not None + assert moved_entity.permalink != "" + + # Verify the moved entity can be used to create an EntityResponse without validation errors + from basic_memory.schemas.response import EntityResponse + response = EntityResponse.model_validate(moved_entity) + assert response.permalink == moved_entity.permalink + + # Verify the physical file was moved + old_path = project_config.home / "test/null-permalink-entity.md" + new_path = project_config.home / "moved/test-entity.md" + assert not old_path.exists() + assert new_path.exists()