fix: complete project management special character support (#272) (#279)

jope-bm · claude[bot] · claude · web-flow · commit cd7cee650f66 · 2025-08-29T10:57:13.000-06:00
Signed-off-by: Joe P &lt;joe@basicmemory.com&gt;
Co-authored-by: claude[bot] &lt;209825114+claude[bot]@users.noreply.github.com&gt;
Co-authored-by: jope-bm &lt;jope-bm@users.noreply.github.com&gt;
Co-authored-by: Claude &lt;noreply@anthropic.com&gt;
diff --git a/src/basic_memory/cli/commands/project.py b/src/basic_memory/cli/commands/project.py
@@ -23,8 +23,8 @@
 from basic_memory.schemas.project_info import ProjectStatusResponse
 from basic_memory.mcp.tools.utils import call_delete
 from basic_memory.mcp.tools.utils import call_put
-from basic_memory.mcp.tools.utils import call_patch
 from basic_memory.utils import generate_permalink
+from basic_memory.mcp.tools.utils import call_patch
 
 console = Console()
 
@@ -100,8 +100,8 @@ def remove_project(
 ) -> None:
     """Remove a project from configuration."""
     try:
-        project_name = generate_permalink(name)
-        response = asyncio.run(call_delete(client, f"/projects/{project_name}"))
+        project_permalink = generate_permalink(name)
+        response = asyncio.run(call_delete(client, f"/projects/{project_permalink}"))
         result = ProjectStatusResponse.model_validate(response.json())
 
         console.print(f"[green]{result.message}[/green]")
@@ -119,9 +119,8 @@ def set_default_project(
 ) -> None:
     """Set the default project and activate it for the current session."""
     try:
-        project_name = generate_permalink(name)
-
-        response = asyncio.run(call_put(client, f"/projects/{project_name}/default"))
+        project_permalink = generate_permalink(name)
+        response = asyncio.run(call_put(client, f"/projects/{project_permalink}/default"))
         result = ProjectStatusResponse.model_validate(response.json())
 
         console.print(f"[green]{result.message}[/green]")
@@ -160,11 +159,11 @@ def move_project(
 
     try:
         data = {"path": resolved_path}
-        project_name = generate_permalink(name)
 
+        project_permalink = generate_permalink(name)
         current_project = session.get_current_project()
         response = asyncio.run(
-            call_patch(client, f"/{current_project}/project/{project_name}", json=data)
+            call_patch(client, f"/{current_project}/project/{project_permalink}", json=data)
         )
         result = ProjectStatusResponse.model_validate(response.json())
 
diff --git a/src/basic_memory/config.py b/src/basic_memory/config.py
@@ -247,7 +247,7 @@ def set_default_project(self, name: str) -> None:
 
         # Load config, modify, and save
         config = self.load_config()
-        config.default_project = name
+        config.default_project = project_name
         self.save_config(config)
 
     def get_project(self, name: str) -> Tuple[str, str] | Tuple[None, None]:
diff --git a/src/basic_memory/mcp/tools/project_management.py b/src/basic_memory/mcp/tools/project_management.py
@@ -221,8 +221,10 @@ async def set_default_project(project_name: str, ctx: Context | None = None) ->
     if ctx:  # pragma: no cover
         await ctx.info(f"Setting default project to: {project_name}")
 
-    # Call API to set default project
-    response = await call_put(client, f"/projects/{project_name}/default")
+    # Call API to set default project using URL encoding for special characters
+    from urllib.parse import quote
+    encoded_name = quote(project_name, safe='')
+    response = await call_put(client, f"/projects/{encoded_name}/default")
     status_response = ProjectStatusResponse.model_validate(response.json())
 
     result = f"✓ {status_response.message}\n\n"
@@ -323,16 +325,29 @@ async def delete_project(project_name: str, ctx: Context | None = None) -> str:
     response = await call_get(client, "/projects/projects")
     project_list = ProjectList.model_validate(response.json())
 
-    # Check if project exists
-    project_exists = any(p.name == project_name for p in project_list.projects)
-    if not project_exists:
+    # Find the project by name (case-insensitive) or permalink - same logic as switch_project
+    project_permalink = generate_permalink(project_name)
+    target_project = None
+    for p in project_list.projects:
+        # Match by permalink (handles case-insensitive input)
+        if p.permalink == project_permalink:
+            target_project = p
+            break
+        # Also match by name comparison (case-insensitive)
+        if p.name.lower() == project_name.lower():
+            target_project = p
+            break
+    
+    if not target_project:
         available_projects = [p.name for p in project_list.projects]
         raise ValueError(
             f"Project '{project_name}' not found. Available projects: {', '.join(available_projects)}"
         )
 
-    # Call API to delete project
-    response = await call_delete(client, f"/projects/{project_name}")
+    # Call API to delete project using URL encoding for special characters
+    from urllib.parse import quote
+    encoded_name = quote(target_project.name, safe='')
+    response = await call_delete(client, f"/projects/{encoded_name}")
     status_response = ProjectStatusResponse.model_validate(response.json())
 
     result = f"✓ {status_response.message}\n\n"
diff --git a/src/basic_memory/models/knowledge.py b/src/basic_memory/models/knowledge.py
@@ -1,6 +1,6 @@
 """Knowledge graph models."""
 
-from datetime import datetime, timezone
+from datetime import datetime
 from basic_memory.utils import ensure_timezone_aware
 from typing import Optional
 
diff --git a/src/basic_memory/schemas/base.py b/src/basic_memory/schemas/base.py
@@ -13,7 +13,7 @@
 
 import mimetypes
 import re
-from datetime import datetime, time, timezone
+from datetime import datetime, time
 from pathlib import Path
 from typing import List, Optional, Annotated, Dict
 
diff --git a/src/basic_memory/services/project_service.py b/src/basic_memory/services/project_service.py
@@ -139,8 +139,8 @@ async def remove_project(self, name: str) -> None:
         # First remove from config (this will validate the project exists and is not default)
         self.config_manager.remove_project(name)
 
-        # Then remove from database
-        project = await self.repository.get_by_name(name)
+        # Then remove from database using robust lookup
+        project = await self.get_project(name)
         if project:
             await self.repository.delete(project.id)
 
@@ -161,8 +161,8 @@ async def set_default_project(self, name: str) -> None:
         # First update config file (this will validate the project exists)
         self.config_manager.set_default_project(name)
 
-        # Then update database
-        project = await self.repository.get_by_name(name)
+        # Then update database using the same lookup logic as get_project
+        project = await self.get_project(name)
         if project:
             await self.repository.set_as_default(project.id)
         else:
@@ -338,8 +338,8 @@ async def move_project(self, name: str, new_path: str) -> None:
         config.projects[name] = resolved_path
         self.config_manager.save_config(config)
 
-        # Update in database
-        project = await self.repository.get_by_name(name)
+        # Update in database using robust lookup
+        project = await self.get_project(name)
         if project:
             await self.repository.update_path(project.id, resolved_path)
             logger.info(f"Moved project '{name}' from {old_path} to {resolved_path}")
@@ -370,8 +370,8 @@ async def update_project(  # pragma: no cover
         if name not in self.config_manager.projects:
             raise ValueError(f"Project '{name}' not found in configuration")
 
-        # Get project from database
-        project = await self.repository.get_by_name(name)
+        # Get project from database using robust lookup
+        project = await self.get_project(name)
         if not project:
             logger.error(f"Project '{name}' exists in config but not in database")
             return
diff --git a/test-int/mcp/test_project_management_integration.py b/test-int/mcp/test_project_management_integration.py
@@ -604,15 +604,15 @@ async def test_create_delete_project_edge_cases(mcp_server, app):
     """Test edge cases for create and delete project operations."""
 
     async with Client(mcp_server) as client:
-        # Test with special characters in project name (should be handled gracefully)
-        special_name = "test-project-with-dashes"
+        # Test with special characters and spaces in project name (should be handled gracefully)
+        special_name = "test project with spaces & symbols!"
 
         # Create project with special characters
         create_result = await client.call_tool(
             "create_memory_project",
             {
                 "project_name": special_name,
-                "project_path": f"/tmp/{special_name}",
+                "project_path": "/tmp/test-project-with-special-chars",
             },
         )
         assert "✓" in create_result.content[0].text
diff --git a/tests/cli/test_project_commands.py b/tests/cli/test_project_commands.py
@@ -184,3 +184,46 @@ def test_project_move_command_failure(mock_run, cli_env):
     # Should exit with code 1 and show error message
     assert result.exit_code == 1
     assert "Error moving project" in result.output
+
+
+@patch("basic_memory.cli.commands.project.call_patch")
+@patch("basic_memory.cli.commands.project.session")
+def test_project_move_command_uses_permalink(mock_session, mock_call_patch, cli_env):
+    """Test that the 'project move' command correctly generates and uses permalink in API call."""
+    # Mock the session to return a current project
+    mock_session.get_current_project.return_value = "current-project"
+    
+    # Mock successful API response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "message": "Project 'Test Project Name' updated successfully",
+        "status": "success",
+        "default": False,
+    }
+    mock_call_patch.return_value = mock_response
+    
+    runner = CliRunner()
+    
+    # Test with a project name that needs normalization (spaces, mixed case)
+    project_name = "Test Project Name"
+    new_path = os.path.join("new", "path", "to", "project")
+    
+    result = runner.invoke(cli_app, ["project", "move", project_name, new_path])
+    
+    # Verify command executed successfully
+    assert result.exit_code == 0
+    
+    # Verify call_patch was called with the correct permalink-formatted project name
+    mock_call_patch.assert_called_once()
+    args, kwargs = mock_call_patch.call_args
+    
+    # Check the API endpoint uses the normalized permalink
+    expected_endpoint = "/current-project/project/test-project-name"
+    assert args[1] == expected_endpoint  # Second argument is the endpoint URL
+    
+    # Verify the data contains the resolved path (using same normalization as the function)
+    from pathlib import Path
+    expected_path = Path(os.path.abspath(os.path.expanduser(new_path))).as_posix()
+    expected_data = {"path": expected_path}
+    assert kwargs["json"] == expected_data
diff --git a/tests/schemas/test_memory_serialization.py b/tests/schemas/test_memory_serialization.py
@@ -3,7 +3,6 @@
 import json
 from datetime import datetime
 
-import pytest
 
 from basic_memory.schemas.memory import (
     EntitySummary,
diff --git a/tests/schemas/test_schemas.py b/tests/schemas/test_schemas.py
@@ -1,7 +1,7 @@
 """Tests for Pydantic schema validation and conversion."""
 
 import pytest
-from datetime import datetime, time, timedelta, timezone
+from datetime import datetime, time, timedelta
 from pydantic import ValidationError, BaseModel
 
 from basic_memory.schemas import (
diff --git a/tests/services/test_project_service.py b/tests/services/test_project_service.py
@@ -713,4 +713,4 @@ async def test_synchronize_projects_handles_case_sensitivity_bug(
 
                     db_project = await project_service.repository.get_by_name(name)
                     if db_project:
-                        await project_service.repository.delete(db_project.id)
+                        await project_service.repository.delete(db_project.id)
diff --git a/tests/test_config.py b/tests/test_config.py
@@ -1,6 +1,9 @@
 """Test configuration management."""
 
-from basic_memory.config import BasicMemoryConfig
+import tempfile
+import pytest
+
+from basic_memory.config import BasicMemoryConfig, ConfigManager
 from pathlib import Path
 
 class TestBasicMemoryConfig:
@@ -76,3 +79,88 @@ def test_basic_memory_home_overrides_existing_main_project(self, config_home, mo
         # The default_factory should override with BASIC_MEMORY_HOME value
         # Note: This tests the current behavior where default_factory takes precedence
         assert config.projects["main"] == original_path
+
+
+class TestConfigManager:
+    """Test ConfigManager functionality."""
+    
+    @pytest.fixture
+    def temp_config_manager(self):
+        """Create a ConfigManager with temporary config file."""
+        with tempfile.TemporaryDirectory() as temp_dir:
+            temp_path = Path(temp_dir)
+            
+            # Create a test ConfigManager instance
+            config_manager = ConfigManager()
+            # Override config paths to use temp directory
+            config_manager.config_dir = temp_path / "basic-memory"
+            config_manager.config_file = config_manager.config_dir / "config.yaml"
+            config_manager.config_dir.mkdir(parents=True, exist_ok=True)
+            
+            # Create initial config with test projects
+            test_config = BasicMemoryConfig(
+                default_project="main",
+                projects={
+                    "main": str(temp_path / "main"),
+                    "test-project": str(temp_path / "test"),
+                    "special-chars": str(temp_path / "special")  # This will be the config key for "Special/Chars"
+                }
+            )
+            config_manager.save_config(test_config)
+            
+            yield config_manager
+    
+    def test_set_default_project_with_exact_name_match(self, temp_config_manager):
+        """Test set_default_project when project name matches config key exactly."""
+        config_manager = temp_config_manager
+        
+        # Set default to a project that exists with exact name match
+        config_manager.set_default_project("test-project")
+        
+        # Verify the config was updated
+        config = config_manager.load_config()
+        assert config.default_project == "test-project"
+    
+    def test_set_default_project_with_permalink_lookup(self, temp_config_manager):
+        """Test set_default_project when input needs permalink normalization."""
+        config_manager = temp_config_manager
+        
+        # Simulate a project that was created with special characters
+        # The config key would be the permalink, but user might type the original name
+        
+        # First add a project with original name that gets normalized
+        config = config_manager.load_config()
+        config.projects["special-chars-project"] = str(Path("/tmp/special"))
+        config_manager.save_config(config)
+        
+        # Now test setting default using a name that will normalize to the config key
+        config_manager.set_default_project("Special Chars Project")  # This should normalize to "special-chars-project"
+        
+        # Verify the config was updated with the correct config key
+        updated_config = config_manager.load_config()
+        assert updated_config.default_project == "special-chars-project"
+    
+    def test_set_default_project_uses_canonical_name(self, temp_config_manager):
+        """Test that set_default_project uses the canonical config key, not user input."""
+        config_manager = temp_config_manager
+        
+        # Add a project with a config key that differs from user input
+        config = config_manager.load_config()
+        config.projects["my-test-project"] = str(Path("/tmp/mytest"))
+        config_manager.save_config(config)
+        
+        # Set default using input that will match but is different from config key
+        config_manager.set_default_project("My Test Project")  # Should find "my-test-project"
+        
+        # Verify that the canonical config key is used, not the user input
+        updated_config = config_manager.load_config()
+        assert updated_config.default_project == "my-test-project"
+        # Should NOT be the user input
+        assert updated_config.default_project != "My Test Project"
+    
+    def test_set_default_project_nonexistent_project(self, temp_config_manager):
+        """Test set_default_project raises ValueError for nonexistent project."""
+        config_manager = temp_config_manager
+        
+        with pytest.raises(ValueError, match="Project 'nonexistent' not found"):
+            config_manager.set_default_project("nonexistent")
diff --git a/tests/utils/test_permalink_formatting.py b/tests/utils/test_permalink_formatting.py
@@ -79,6 +79,9 @@ async def test_permalink_formatting(
         ("archive/François Müller.md", "archive/francois-muller"),
         ("research/Søren Kierkegård.md", "research/soren-kierkegard"),
         ("articles/El Niño.md", "articles/el-nino"),
+        ("ArticlesElNiño.md", "articles-el-nino"),
+        ("articleselniño.md", "articleselnino"),
+        ("articles-El-Niño.md", "articles-el-nino"),
     ],
 )
 def test_latin_accents_transliteration(input_path, expected):
