From 985f650a62889197d3d81c722ee254b2081ff59c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Mon, 3 Nov 2025 21:59:49 +0000 Subject: [PATCH] fix: Prevent deletion of user-created projects during reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add managed_by field to Project model to distinguish between: - CONFIG: Projects managed by config file (can be auto-deleted) - USER: Projects created via API/MCP (never auto-deleted) This fixes issue #413 where user-created projects were being deleted during system startup because they weren't in the config file. The fix preserves the behavior from issue #193 (cleaning up orphaned config projects) while protecting user-created projects. Changes: - Add ProjectManagementType enum and managed_by field to Project model - Add database migration for managed_by column (defaults to 'config') - Update add_project() to set managed_by=USER for API-created projects - Update synchronize_projects() to only delete CONFIG-managed projects - Add regression test to ensure USER projects are never auto-deleted - Update existing test to validate CONFIG projects are still cleaned up Closes #413 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Paul Hernandez --- .../f1a2b3c4d5e6_add_managed_by_to_project.py | 34 ++++++++ src/basic_memory/models/__init__.py | 3 +- src/basic_memory/models/project.py | 21 +++++ src/basic_memory/services/project_service.py | 27 +++++-- tests/services/test_project_service.py | 77 ++++++++++++++++--- 5 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 src/basic_memory/alembic/versions/f1a2b3c4d5e6_add_managed_by_to_project.py diff --git a/src/basic_memory/alembic/versions/f1a2b3c4d5e6_add_managed_by_to_project.py b/src/basic_memory/alembic/versions/f1a2b3c4d5e6_add_managed_by_to_project.py new file mode 100644 index 00000000..343bfccf --- /dev/null +++ b/src/basic_memory/alembic/versions/f1a2b3c4d5e6_add_managed_by_to_project.py @@ -0,0 +1,34 @@ +"""Add managed_by field to distinguish config vs user-created projects + +Revision ID: f1a2b3c4d5e6 +Revises: e7e1f4367280 +Create Date: 2025-11-03 21:57:00.000000 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = "f1a2b3c4d5e6" +down_revision: Union[str, None] = "e7e1f4367280" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # Add managed_by column with default value 'config' + # All existing projects are treated as config-managed for backward compatibility + with op.batch_alter_table("project", schema=None) as batch_op: + batch_op.add_column( + sa.Column("managed_by", sa.String(), nullable=False, server_default="config") + ) + + +def downgrade() -> None: + # Remove managed_by column + with op.batch_alter_table("project", schema=None) as batch_op: + batch_op.drop_column("managed_by") diff --git a/src/basic_memory/models/__init__.py b/src/basic_memory/models/__init__.py index acdc03b1..3c9a8ff6 100644 --- a/src/basic_memory/models/__init__.py +++ b/src/basic_memory/models/__init__.py @@ -3,7 +3,7 @@ import basic_memory from basic_memory.models.base import Base from basic_memory.models.knowledge import Entity, Observation, Relation -from basic_memory.models.project import Project +from basic_memory.models.project import Project, ProjectManagementType __all__ = [ "Base", @@ -11,5 +11,6 @@ __all__ = [ "Observation", "Relation", "Project", + "ProjectManagementType", "basic_memory", ] diff --git a/src/basic_memory/models/project.py b/src/basic_memory/models/project.py index 373eae6e..5b4edd17 100644 --- a/src/basic_memory/models/project.py +++ b/src/basic_memory/models/project.py @@ -1,6 +1,7 @@ """Project model for Basic Memory.""" from datetime import datetime, UTC +from enum import Enum from typing import Optional from sqlalchemy import ( @@ -19,6 +20,16 @@ from basic_memory.models.base import Base from basic_memory.utils import generate_permalink +class ProjectManagementType(str, Enum): + """Enum for project management types. + + - CONFIG: Project is managed by config file (system/default projects) + - USER: Project is user-created via API/MCP (never auto-deleted) + """ + CONFIG = "config" + USER = "user" + + class Project(Base): """Project model for Basic Memory. @@ -52,6 +63,16 @@ class Project(Base): is_active: Mapped[bool] = mapped_column(Boolean, default=True) is_default: Mapped[Optional[bool]] = mapped_column(Boolean, default=None, nullable=True) + # Management type - distinguishes config-managed vs user-created projects + # CONFIG: synced from config file, can be auto-deleted if removed from config + # USER: user-created via API/MCP, never auto-deleted during reconciliation + # Defaults to CONFIG for backward compatibility with existing projects + managed_by: Mapped[str] = mapped_column( + String, + default=ProjectManagementType.CONFIG.value, + nullable=False + ) + # Timestamps created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), default=lambda: datetime.now(UTC) diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index bd011968..8233bbbf 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -11,7 +11,7 @@ from typing import Dict, Optional, Sequence from loguru import logger from sqlalchemy import text -from basic_memory.models import Project +from basic_memory.models import Project, ProjectManagementType from basic_memory.repository.project_repository import ProjectRepository from basic_memory.schemas import ( ActivityMetrics, @@ -203,11 +203,14 @@ class ProjectService: project_config = self.config_manager.add_project(name, resolved_path) # Then add to database + # Set managed_by to USER for API-created projects + # Config-managed projects are created during synchronization project_data = { "name": name, "path": resolved_path, "permalink": generate_permalink(project_config.name), "is_active": True, + "managed_by": ProjectManagementType.USER.value, # Don't set is_default=False to avoid UNIQUE constraint issues # Let it default to NULL, only set to True when explicitly making default } @@ -386,19 +389,27 @@ class ProjectService: "path": path, "permalink": generate_permalink(name), "is_active": True, + "managed_by": ProjectManagementType.CONFIG.value, # Don't set is_default here - let the enforcement logic handle it } await self.repository.create(project_data) - # Remove projects that exist in DB but not in config - # Config is the source of truth - if a project was deleted from config, - # it should be deleted from DB too (fixes issue #193) + # Remove CONFIG-managed projects that exist in DB but not in config + # Config is the source of truth for CONFIG-managed projects - if a CONFIG project + # was deleted from config, it should be deleted from DB too (fixes issue #193) + # NEVER delete USER-managed projects - they are intentionally not in config (fixes issue #413) for name, project in db_projects_by_permalink.items(): if name not in config_projects: - logger.info( - f"Removing project '{name}' from database (deleted from config, source of truth)" - ) - await self.repository.delete(project.id) + # Only delete if it's a CONFIG-managed project + if project.managed_by == ProjectManagementType.CONFIG.value: + logger.info( + f"Removing CONFIG-managed project '{name}' from database (deleted from config)" + ) + await self.repository.delete(project.id) + else: + logger.debug( + f"Keeping USER-managed project '{name}' in database (not in config, but user-created)" + ) # Ensure database default project state is consistent await self._ensure_single_default_project() diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index 4b1cc8f3..8827220a 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -1201,13 +1201,14 @@ async def test_add_project_nested_validation_with_project_root( @pytest.mark.asyncio -async def test_synchronize_projects_removes_db_only_projects(project_service: ProjectService): - """Test that synchronize_projects removes projects that exist in DB but not in config. +async def test_synchronize_projects_removes_config_managed_db_only_projects( + project_service: ProjectService, +): + """Test that synchronize_projects removes CONFIG-managed projects that exist in DB but not in config. - This is a regression test for issue #193 where deleted projects would be re-added - to config during synchronization, causing them to reappear after deletion. - Config is the source of truth - if a project is deleted from config, it should be - removed from the database during synchronization. + This is a regression test for issue #193 where deleted CONFIG-managed projects + would be re-added to config during synchronization, causing them to reappear after deletion. + Config is the source of truth for CONFIG-managed projects only. """ test_project_name = f"test-db-only-{os.urandom(4).hex()}" with tempfile.TemporaryDirectory() as temp_dir: @@ -1218,28 +1219,30 @@ async def test_synchronize_projects_removes_db_only_projects(project_service: Pr os.makedirs(test_project_path, exist_ok=True) try: - # Add project to database only (not to config) - simulating orphaned DB entry + # Add CONFIG-managed project to database only (not to config) - simulating orphaned DB entry project_data = { "name": test_project_name, "path": test_project_path, "permalink": test_project_name.lower().replace(" ", "-"), "is_active": True, + "managed_by": "config", # CONFIG-managed project } await project_service.repository.create(project_data) # Verify it exists in DB but not in config db_project = await project_service.repository.get_by_name(test_project_name) assert db_project is not None + assert db_project.managed_by == "config" assert test_project_name not in project_service.projects - # Call synchronize_projects - this should remove the orphaned DB entry - # because config is the source of truth + # Call synchronize_projects - this should remove the orphaned CONFIG-managed DB entry + # because config is the source of truth for CONFIG-managed projects await project_service.synchronize_projects() # Verify project was removed from database db_project_after = await project_service.repository.get_by_name(test_project_name) assert db_project_after is None, ( - "Project should be removed from DB when not in config (config is source of truth)" + "CONFIG-managed project should be removed from DB when not in config" ) # Verify it's still not in config @@ -1252,6 +1255,60 @@ async def test_synchronize_projects_removes_db_only_projects(project_service: Pr await project_service.repository.delete(db_project.id) +@pytest.mark.asyncio +async def test_synchronize_projects_keeps_user_managed_projects(project_service: ProjectService): + """Test that synchronize_projects NEVER deletes USER-managed projects. + + This is a regression test for issue #413 where user-created projects were being + deleted during synchronization because they weren't in the config file. + USER-managed projects are intentionally not in config and should never be auto-deleted. + """ + test_project_name = f"test-user-project-{os.urandom(4).hex()}" + with tempfile.TemporaryDirectory() as temp_dir: + test_root = Path(temp_dir) + test_project_path = str(test_root / "test-user-project") + + # Make sure the test directory exists + os.makedirs(test_project_path, exist_ok=True) + + try: + # Add USER-managed project to database only (not to config) - simulating user-created project + project_data = { + "name": test_project_name, + "path": test_project_path, + "permalink": test_project_name.lower().replace(" ", "-"), + "is_active": True, + "managed_by": "user", # USER-managed project + } + await project_service.repository.create(project_data) + + # Verify it exists in DB but not in config + db_project = await project_service.repository.get_by_name(test_project_name) + assert db_project is not None + assert db_project.managed_by == "user" + assert test_project_name not in project_service.projects + + # Call synchronize_projects - this should KEEP the USER-managed project + # USER projects are not in config by design and should never be auto-deleted + await project_service.synchronize_projects() + + # Verify project still exists in database (NOT deleted) + db_project_after = await project_service.repository.get_by_name(test_project_name) + assert db_project_after is not None, ( + "USER-managed project should NEVER be removed from DB during synchronization" + ) + assert db_project_after.managed_by == "user" + + # Verify it's still not in config (as expected for USER projects) + assert test_project_name not in project_service.projects + + finally: + # Clean up + db_project = await project_service.repository.get_by_name(test_project_name) + if db_project: + await project_service.repository.delete(db_project.id) + + @pytest.mark.asyncio async def test_remove_project_with_delete_notes_false(project_service: ProjectService): """Test that remove_project with delete_notes=False keeps directory intact."""