diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py index d5f849815..02abd0d1b 100644 --- a/src/basic_memory/config.py +++ b/src/basic_memory/config.py @@ -686,11 +686,8 @@ def add_project(self, name: str, path: str) -> ProjectConfig: if project_name: # pragma: no cover raise ValueError(f"Project '{name}' already exists") - # Ensure the path exists - project_path = Path(path) - project_path.mkdir(parents=True, exist_ok=True) # pragma: no cover - # Load config, modify it, and save it + project_path = Path(path) config = self.load_config() config.projects[name] = ProjectEntry(path=str(project_path)) self.save_config(config) diff --git a/src/basic_memory/deps/services.py b/src/basic_memory/deps/services.py index 6e41c469e..7d7572f40 100644 --- a/src/basic_memory/deps/services.py +++ b/src/basic_memory/deps/services.py @@ -9,6 +9,7 @@ import asyncio import os +from pathlib import Path from typing import Annotated, Any, Callable, Coroutine, Mapping, Protocol from fastapi import Depends @@ -549,9 +550,15 @@ async def _reindex_project(**_: Any) -> None: async def get_project_service( project_repository: ProjectRepositoryDep, + app_config: AppConfigDep, ) -> ProjectService: - """Create ProjectService with repository.""" - return ProjectService(repository=project_repository) + """Create ProjectService with repository and a system-level FileService for directory operations.""" + # A system-level FileService for project directory creation (no project-specific base_path needed). + # ensure_directory() accepts absolute paths and ignores base_path for those, so Path.home() is safe. + entity_parser = EntityParser(Path.home()) + markdown_processor = MarkdownProcessor(entity_parser, app_config=app_config) + file_service = FileService(Path.home(), markdown_processor, app_config=app_config) + return ProjectService(repository=project_repository, file_service=file_service) ProjectServiceDep = Annotated[ProjectService, Depends(get_project_service)] diff --git a/src/basic_memory/mcp/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py index 27af35ca5..f4323bf93 100644 --- a/src/basic_memory/mcp/tools/project_management.py +++ b/src/basic_memory/mcp/tools/project_management.py @@ -64,7 +64,12 @@ async def list_memory_projects( result = "Available projects:\n" for project in project_list.projects: - result += f"• {project.name}\n" + label = ( + f"{project.display_name} ({project.name})" + if project.display_name + else project.name + ) + result += f"• {label}\n" result += "\n" + "─" * 40 + "\n" result += "Next: Ask which project to use for this session.\n" diff --git a/src/basic_memory/repository/fastembed_provider.py b/src/basic_memory/repository/fastembed_provider.py index b8c2bde39..ef257eb51 100644 --- a/src/basic_memory/repository/fastembed_provider.py +++ b/src/basic_memory/repository/fastembed_provider.py @@ -9,7 +9,7 @@ from basic_memory.repository.semantic_errors import SemanticDependenciesMissingError if TYPE_CHECKING: - from fastembed import TextEmbedding # pragma: no cover + from fastembed import TextEmbedding # type: ignore[import-not-found] # pragma: no cover class FastEmbedEmbeddingProvider(EmbeddingProvider): @@ -42,7 +42,7 @@ async def _load_model(self) -> "TextEmbedding": def _create_model() -> "TextEmbedding": try: - from fastembed import TextEmbedding + from fastembed import TextEmbedding # type: ignore[import-not-found] except ( ImportError ) as exc: # pragma: no cover - exercised via tests with monkeypatch diff --git a/src/basic_memory/repository/openai_provider.py b/src/basic_memory/repository/openai_provider.py index 69c4acfa6..4d3198b9c 100644 --- a/src/basic_memory/repository/openai_provider.py +++ b/src/basic_memory/repository/openai_provider.py @@ -41,7 +41,7 @@ async def _get_client(self) -> Any: return self._client try: - from openai import AsyncOpenAI + from openai import AsyncOpenAI # type: ignore[import-not-found] except ImportError as exc: # pragma: no cover - covered via monkeypatch tests raise SemanticDependenciesMissingError( "OpenAI dependency is missing. " diff --git a/src/basic_memory/repository/sqlite_search_repository.py b/src/basic_memory/repository/sqlite_search_repository.py index a0bdb20d7..b63ebdc9c 100644 --- a/src/basic_memory/repository/sqlite_search_repository.py +++ b/src/basic_memory/repository/sqlite_search_repository.py @@ -343,7 +343,7 @@ async def _ensure_sqlite_vec_loaded(self, session) -> None: pass try: - import sqlite_vec + import sqlite_vec # type: ignore[import-not-found] except ImportError as exc: raise SemanticDependenciesMissingError( "sqlite-vec package is missing. " diff --git a/src/basic_memory/schemas/project_info.py b/src/basic_memory/schemas/project_info.py index 4168b70ed..c1dd18f29 100644 --- a/src/basic_memory/schemas/project_info.py +++ b/src/basic_memory/schemas/project_info.py @@ -178,6 +178,9 @@ class ProjectItem(BaseModel): name: str path: str is_default: bool = False + # Optional metadata injected by cloud hosting layer (not stored in DB) + display_name: Optional[str] = None + is_private: bool = False @property def permalink(self) -> str: # pragma: no cover diff --git a/src/basic_memory/services/file_service.py b/src/basic_memory/services/file_service.py index 044ac952c..a1d0ea808 100644 --- a/src/basic_memory/services/file_service.py +++ b/src/basic_memory/services/file_service.py @@ -43,7 +43,7 @@ class FileService: def __init__( self, base_path: Path, - markdown_processor: MarkdownProcessor, + markdown_processor: Optional[MarkdownProcessor] = None, max_concurrent_files: int = 10, app_config: Optional["BasicMemoryConfig"] = None, ): @@ -79,6 +79,10 @@ async def read_entity_content(self, entity: EntityModel) -> str: """ logger.debug(f"Reading entity content, entity_id={entity.id}, permalink={entity.permalink}") + # markdown_processor is required for entity content reads — fail fast if not configured + if self.markdown_processor is None: + raise ValueError("markdown_processor is required for read_entity_content") + file_path = self.get_entity_path(entity) markdown = await self.markdown_processor.read_file(file_path) return markdown.content or "" diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index 36d314e74..d45b405da 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -6,7 +6,7 @@ import shutil from datetime import datetime from pathlib import Path -from typing import Dict, Optional, Sequence +from typing import TYPE_CHECKING, Dict, Optional, Sequence from loguru import logger @@ -30,16 +30,22 @@ ) from basic_memory.utils import generate_permalink +if TYPE_CHECKING: # pragma: no cover + from basic_memory.services.file_service import FileService + class ProjectService: """Service for managing Basic Memory projects.""" repository: ProjectRepository - def __init__(self, repository: ProjectRepository): + def __init__( + self, repository: ProjectRepository, file_service: Optional["FileService"] = None + ): """Initialize the project service.""" super().__init__() self.repository = repository + self.file_service = file_service @property def config_manager(self) -> ConfigManager: @@ -205,6 +211,16 @@ async def add_project(self, name: str, path: str, set_default: bool = False) -> f"Projects cannot share directory trees." ) + # Ensure the project directory exists on disk. + # Trigger: project_root not set means local filesystem mode (not S3/cloud) + # Why: FileService (or future S3FileService) provides cloud-compatible directory creation; + # direct Path.mkdir() bypasses this abstraction + # Outcome: directory exists before config/DB entries are written + if not self.config_manager.config.project_root: + if self.file_service is None: + raise ValueError("file_service is required for local project directory creation") + await self.file_service.ensure_directory(Path(resolved_path)) + # First add to config file (this validates project uniqueness and keeps # config + database aligned for all backends). self.config_manager.add_project(name, resolved_path) @@ -459,8 +475,14 @@ async def move_project(self, name: str, new_path: str) -> None: if name not in self.config_manager.projects: raise ValueError(f"Project '{name}' not found in configuration") - # Create the new directory if it doesn't exist - Path(resolved_path).mkdir(parents=True, exist_ok=True) + # Create the new directory if it doesn't exist (skip in cloud mode where storage is S3) + # Trigger: project_root not set means local filesystem mode + # Why: FileService (or future S3FileService) provides cloud-compatible directory creation + # Outcome: destination directory exists before config/DB are updated + if not self.config_manager.config.project_root: + if self.file_service is None: + raise ValueError("file_service is required for local project directory creation") + await self.file_service.ensure_directory(Path(resolved_path)) # Update in configuration config = self.config_manager.load_config() diff --git a/tests/conftest.py b/tests/conftest.py index 2606aa1d3..662a56495 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -466,9 +466,10 @@ async def sample_entity(entity_repository: EntityRepository) -> Entity: @pytest_asyncio.fixture async def project_service( project_repository: ProjectRepository, + file_service: FileService, ) -> ProjectService: - """Create ProjectService with repository.""" - return ProjectService(repository=project_repository) + """Create ProjectService with repository and file service for directory operations.""" + return ProjectService(repository=project_repository, file_service=file_service) @pytest_asyncio.fixture diff --git a/tests/mcp/test_tool_project_management.py b/tests/mcp/test_tool_project_management.py index a059c5876..aacb96f58 100644 --- a/tests/mcp/test_tool_project_management.py +++ b/tests/mcp/test_tool_project_management.py @@ -15,6 +15,75 @@ async def test_list_memory_projects_unconstrained(app, test_project): assert f"• {test_project.name}" in result +@pytest.mark.asyncio +async def test_list_memory_projects_shows_display_name(app, client, test_project): + """When a project has display_name set, list_memory_projects shows 'display_name (name)' format.""" + # Inject display_name into the project list response by patching the API response. + # In production, the cloud proxy adds display_name to the JSON before deserialization. + from unittest.mock import AsyncMock, patch + from basic_memory.schemas.project_info import ProjectItem, ProjectList + + mock_project = ProjectItem( + id=1, + external_id="00000000-0000-0000-0000-000000000001", + name="private-fb83af23", + path="/tmp/private", + is_default=False, + display_name="My Notes", + is_private=True, + ) + regular_project = ProjectItem( + id=2, + external_id="00000000-0000-0000-0000-000000000002", + name="main", + path="/tmp/main", + is_default=True, + ) + mock_list = ProjectList( + projects=[regular_project, mock_project], + default_project="main", + ) + + with patch( + "basic_memory.mcp.clients.project.ProjectClient.list_projects", + new_callable=AsyncMock, + return_value=mock_list, + ): + result = await list_memory_projects.fn() + + # Regular project shows just the name + assert "• main\n" in result + # Private project shows display_name with slug in parentheses + assert "• My Notes (private-fb83af23)" in result + + +@pytest.mark.asyncio +async def test_list_memory_projects_no_display_name_shows_name_only(app, client, test_project): + """When a project has no display_name, list_memory_projects shows just the name.""" + from unittest.mock import AsyncMock, patch + from basic_memory.schemas.project_info import ProjectItem, ProjectList + + project = ProjectItem( + id=1, + external_id="00000000-0000-0000-0000-000000000001", + name="my-project", + path="/tmp/my-project", + is_default=True, + ) + mock_list = ProjectList(projects=[project], default_project="my-project") + + with patch( + "basic_memory.mcp.clients.project.ProjectClient.list_projects", + new_callable=AsyncMock, + return_value=mock_list, + ): + result = await list_memory_projects.fn() + + assert "• my-project\n" in result + # Should NOT have parenthetical format + assert "(" not in result.split("• my-project")[1].split("\n")[0] + + @pytest.mark.asyncio async def test_list_memory_projects_constrained_env(monkeypatch, app, test_project): monkeypatch.setenv("BASIC_MEMORY_MCP_PROJECT", test_project.name) diff --git a/tests/schemas/test_schemas.py b/tests/schemas/test_schemas.py index 3f6dc279b..42f9e7db0 100644 --- a/tests/schemas/test_schemas.py +++ b/tests/schemas/test_schemas.py @@ -511,6 +511,106 @@ class TestModel(BaseModel): assert time_diff < 3600, f"'today' and '1d' should be similar times, diff: {time_diff}s" +class TestProjectItemSchema: + """Test ProjectItem schema with optional cloud-injected fields.""" + + def test_project_item_defaults(self): + """ProjectItem has sensible defaults for cloud-injected fields.""" + from basic_memory.schemas.project_info import ProjectItem + + project = ProjectItem( + id=1, + external_id="00000000-0000-0000-0000-000000000001", + name="main", + path="/tmp/main", + ) + assert project.display_name is None + assert project.is_private is False + assert project.is_default is False + + def test_project_item_with_display_name(self): + """ProjectItem accepts display_name from cloud proxy enrichment.""" + from basic_memory.schemas.project_info import ProjectItem + + project = ProjectItem( + id=1, + external_id="00000000-0000-0000-0000-000000000001", + name="private-fb83af23", + path="/tmp/private", + display_name="My Notes", + is_private=True, + ) + assert project.display_name == "My Notes" + assert project.is_private is True + assert project.name == "private-fb83af23" + + def test_project_item_deserialization_from_json(self): + """ProjectItem correctly deserializes display_name and is_private from JSON. + + This is the actual path: the cloud proxy enriches the JSON response from + basic-memory API, and the MCP tools deserialize it back into ProjectItem. + """ + from basic_memory.schemas.project_info import ProjectItem + + json_data = { + "id": 1, + "external_id": "00000000-0000-0000-0000-000000000001", + "name": "private-fb83af23", + "path": "/tmp/private", + "is_default": False, + "display_name": "My Notes", + "is_private": True, + } + project = ProjectItem.model_validate(json_data) + assert project.display_name == "My Notes" + assert project.is_private is True + + def test_project_item_deserialization_without_cloud_fields(self): + """ProjectItem works when cloud fields are absent (non-cloud usage).""" + from basic_memory.schemas.project_info import ProjectItem + + json_data = { + "id": 1, + "external_id": "00000000-0000-0000-0000-000000000001", + "name": "main", + "path": "/tmp/main", + "is_default": True, + } + project = ProjectItem.model_validate(json_data) + assert project.display_name is None + assert project.is_private is False + + def test_project_list_with_mixed_projects(self): + """ProjectList can contain a mix of regular and private projects.""" + from basic_memory.schemas.project_info import ProjectItem, ProjectList + + projects = ProjectList( + projects=[ + ProjectItem( + id=1, + external_id="00000000-0000-0000-0000-000000000001", + name="main", + path="/tmp/main", + is_default=True, + ), + ProjectItem( + id=2, + external_id="00000000-0000-0000-0000-000000000002", + name="private-fb83af23", + path="/tmp/private", + display_name="My Notes", + is_private=True, + ), + ], + default_project="main", + ) + assert len(projects.projects) == 2 + assert projects.projects[0].display_name is None + assert projects.projects[0].is_private is False + assert projects.projects[1].display_name == "My Notes" + assert projects.projects[1].is_private is True + + class TestObservationContentLength: """Test observation content length validation matches DB schema.""" diff --git a/tests/test_config.py b/tests/test_config.py index 63961b5ef..294bc60a4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -580,6 +580,36 @@ def test_add_project_uses_platform_native_separators(self, monkeypatch): assert "/" in saved_path assert saved_path == str(project_path) + def test_add_project_never_creates_directory(self): + """Test that ConfigManager.add_project() is pure config management — no mkdir. + + Directory creation is delegated to ProjectService via FileService, which + supports both local and cloud (S3) backends. + """ + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + + config_manager = ConfigManager() + config_manager.config_dir = temp_path / "basic-memory" + config_manager.config_file = config_manager.config_dir / "config.json" + config_manager.config_dir.mkdir(parents=True, exist_ok=True) + + initial_config = BasicMemoryConfig(projects={}) + config_manager.save_config(initial_config) + + # Use a path that does not exist — ConfigManager should not create it + nonexistent_path = str(temp_path / "nonexistent" / "project") + config_manager.add_project("test-project", nonexistent_path) + + # Check directory does NOT exist right after add_project(), + # before load_config() which triggers the model validator + assert not Path(nonexistent_path).exists() + + # Verify project was persisted in config + config = config_manager.load_config() + assert "test-project" in config.projects + assert config.projects["test-project"].path == nonexistent_path + def test_model_post_init_uses_platform_native_separators(self, config_home, monkeypatch): """Test that model_post_init uses platform-native separators.""" import platform