From bc5fb2efa38f53aefebd09c203be9bc2af34cc0e Mon Sep 17 00:00:00 2001 From: phernandez Date: Tue, 30 Dec 2025 08:42:31 -0600 Subject: [PATCH] fix: set_default_project skips config file update in cloud mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Validate project exists in database before updating (fail-fast) - Only update config file in local mode; cloud mode uses database only - Raise ValueError if project not found instead of just logging error - Update tests to expect ValueError for non-existent projects 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 Signed-off-by: phernandez --- src/basic_memory/services/project_service.py | 18 ++++++------ tests/mcp/test_project_context.py | 30 ++++++-------------- tests/schemas/test_schemas.py | 6 +++- tests/services/test_project_service.py | 16 +++-------- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py index 678d19e2..af9073b9 100644 --- a/src/basic_memory/services/project_service.py +++ b/src/basic_memory/services/project_service.py @@ -283,15 +283,17 @@ async def set_default_project(self, name: str) -> None: if not self.repository: # pragma: no cover raise ValueError("Repository is required for set_default_project") - # First update config file (this will validate the project exists) - self.config_manager.set_default_project(name) - - # Then update database using the same lookup logic as get_project + # Look up project in database first to validate it exists project = await self.get_project(name) - if project: - await self.repository.set_as_default(project.id) - else: - logger.error(f"Project '{name}' exists in config but not in database") + if not project: + raise ValueError(f"Project '{name}' not found") + + # Update database + await self.repository.set_as_default(project.id) + + # Update config file only in local mode (cloud mode uses database only) + if not self.config_manager.config.cloud_mode: + self.config_manager.set_default_project(name) logger.info(f"Project '{name}' set as default in configuration and database") diff --git a/tests/mcp/test_project_context.py b/tests/mcp/test_project_context.py index 49e046af..6d8d44f1 100644 --- a/tests/mcp/test_project_context.py +++ b/tests/mcp/test_project_context.py @@ -17,9 +17,7 @@ async def test_cloud_mode_requires_project_by_default(self): mock_config = MagicMock() mock_config.cloud_mode = True - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config with pytest.raises(ValueError) as exc_info: @@ -36,9 +34,7 @@ async def test_cloud_mode_allows_discovery_when_enabled(self): mock_config = MagicMock() mock_config.cloud_mode = True - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config result = await resolve_project_parameter(project=None, allow_discovery=True) @@ -53,9 +49,7 @@ async def test_cloud_mode_returns_project_when_specified(self): mock_config = MagicMock() mock_config.cloud_mode = True - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config result = await resolve_project_parameter(project="my-project") @@ -70,9 +64,7 @@ async def test_local_mode_uses_env_var_priority(self): mock_config = MagicMock() mock_config.cloud_mode = False - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config with patch.dict(os.environ, {"BASIC_MEMORY_MCP_PROJECT": "env-project"}): @@ -90,9 +82,7 @@ async def test_local_mode_uses_explicit_project(self): mock_config.cloud_mode = False mock_config.default_project_mode = False - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config with patch.dict(os.environ, {}, clear=True): @@ -112,9 +102,7 @@ async def test_local_mode_uses_default_project(self): mock_config.default_project_mode = True mock_config.default_project = "default-project" - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config with patch.dict(os.environ, {}, clear=True): @@ -132,13 +120,11 @@ async def test_local_mode_returns_none_when_no_resolution(self): mock_config.cloud_mode = False mock_config.default_project_mode = False - with patch( - "basic_memory.mcp.project_context.ConfigManager" - ) as mock_config_manager: + with patch("basic_memory.mcp.project_context.ConfigManager") as mock_config_manager: mock_config_manager.return_value.config = mock_config with patch.dict(os.environ, {}, clear=True): os.environ.pop("BASIC_MEMORY_MCP_PROJECT", None) result = await resolve_project_parameter(project=None) - assert result is None \ No newline at end of file + assert result is None diff --git a/tests/schemas/test_schemas.py b/tests/schemas/test_schemas.py index baf43fc6..f374f90f 100644 --- a/tests/schemas/test_schemas.py +++ b/tests/schemas/test_schemas.py @@ -106,7 +106,11 @@ def test_relation_response_with_null_permalink(): "permalink": "test/relation/123", "relation_type": "relates_to", "from_entity": {"permalink": None, "file_path": "notes/source-note.md"}, - "to_entity": {"permalink": None, "file_path": "notes/target-note.md", "title": "Target Note"}, + "to_entity": { + "permalink": None, + "file_path": "notes/target-note.md", + "title": "Target Note", + }, } relation = RelationResponse.model_validate(data) # Falls back to file_path directly (not converted to permalink) diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py index 8b73da1e..a2fb9a96 100644 --- a/tests/services/test_project_service.py +++ b/tests/services/test_project_service.py @@ -284,7 +284,7 @@ async def test_get_project_method(project_service: ProjectService): async def test_set_default_project_config_db_mismatch( project_service: ProjectService, config_manager: ConfigManager ): - """Test set_default_project when project exists in config but not in database.""" + """Test set_default_project raises error when project exists in config but not in database.""" test_project_name = f"test-mismatch-project-{os.urandom(4).hex()}" with tempfile.TemporaryDirectory() as temp_dir: test_root = Path(temp_dir) @@ -293,8 +293,6 @@ async def test_set_default_project_config_db_mismatch( # Make sure the test directory exists os.makedirs(test_project_path, exist_ok=True) - original_default = project_service.default_project - try: # Add project to config only (not to database) config_manager.add_project(test_project_name, test_project_path) @@ -304,17 +302,11 @@ async def test_set_default_project_config_db_mismatch( db_project = await project_service.repository.get_by_name(test_project_name) assert db_project is None - # Try to set as default - this should trigger the error log on line 142 - await project_service.set_default_project(test_project_name) - - # Should still update config despite database mismatch - assert project_service.default_project == test_project_name + # Try to set as default - should raise ValueError since project not in database + with pytest.raises(ValueError, match=f"Project '{test_project_name}' not found"): + await project_service.set_default_project(test_project_name) finally: - # Restore original default - if original_default: - config_manager.set_default_project(original_default) - # Clean up if test_project_name in project_service.projects: config_manager.remove_project(test_project_name)