Merge pull request #306 from coleam00/feature/mcp-server-consolidation-simplification

Refactor MCP server: Modularize tools and add comprehensive tests

python/tests/mcp_server/__init__.py

@@ -0,0 +1 @@
+"""MCP server tests."""

python/tests/mcp_server/features/__init__.py

@@ -0,0 +1 @@
+"""MCP server features tests."""

python/tests/mcp_server/features/documents/__init__.py

@@ -0,0 +1 @@
+"""Document and version tools tests."""

python/tests/mcp_server/features/documents/test_document_tools.py

@@ -0,0 +1,174 @@
+"""Unit tests for document management tools."""
+
+import json
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp.server.fastmcp import Context
+
+from src.mcp_server.features.documents.document_tools import register_document_tools
+
+
+@pytest.fixture
+def mock_mcp():
+    """Create a mock MCP server for testing."""
+    mock = MagicMock()
+    # Store registered tools
+    mock._tools = {}
+
+    def tool_decorator():
+        def decorator(func):
+            mock._tools[func.__name__] = func
+            return func
+
+        return decorator
+
+    mock.tool = tool_decorator
+    return mock
+
+
+@pytest.fixture
+def mock_context():
+    """Create a mock context for testing."""
+    return MagicMock(spec=Context)
+
+
+@pytest.mark.asyncio
+async def test_create_document_success(mock_mcp, mock_context):
+    """Test successful document creation."""
+    # Register tools with mock MCP
+    register_document_tools(mock_mcp)
+
+    # Get the create_document function from registered tools
+    create_document = mock_mcp._tools.get("create_document")
+    assert create_document is not None, "create_document tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "document": {"id": "doc-123", "title": "Test Doc"},
+        "message": "Document created successfully",
+    }
+
+    with patch("src.mcp_server.features.documents.document_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        # Test the function
+        result = await create_document(
+            mock_context,
+            project_id="project-123",
+            title="Test Document",
+            document_type="spec",
+            content={"test": "content"},
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["document_id"] == "doc-123"
+        assert "Document created successfully" in result_data["message"]
+
+
+@pytest.mark.asyncio
+async def test_list_documents_success(mock_mcp, mock_context):
+    """Test successful document listing."""
+    register_document_tools(mock_mcp)
+
+    # Get the list_documents function from registered tools
+    list_documents = mock_mcp._tools.get("list_documents")
+    assert list_documents is not None, "list_documents tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "documents": [
+            {"id": "doc-1", "title": "Doc 1", "document_type": "spec"},
+            {"id": "doc-2", "title": "Doc 2", "document_type": "design"},
+        ]
+    }
+
+    with patch("src.mcp_server.features.documents.document_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await list_documents(mock_context, project_id="project-123")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert len(result_data["documents"]) == 2
+        assert result_data["count"] == 2
+
+
+@pytest.mark.asyncio
+async def test_update_document_partial_update(mock_mcp, mock_context):
+    """Test partial document update."""
+    register_document_tools(mock_mcp)
+
+    # Get the update_document function from registered tools
+    update_document = mock_mcp._tools.get("update_document")
+    assert update_document is not None, "update_document tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "doc": {"id": "doc-123", "title": "Updated Title"},
+        "message": "Document updated successfully",
+    }
+
+    with patch("src.mcp_server.features.documents.document_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.put.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        # Update only title
+        result = await update_document(
+            mock_context, project_id="project-123", doc_id="doc-123", title="Updated Title"
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert "Document updated successfully" in result_data["message"]
+
+        # Verify only title was sent in update
+        call_args = mock_async_client.put.call_args
+        sent_data = call_args[1]["json"]
+        assert sent_data == {"title": "Updated Title"}
+
+
+@pytest.mark.asyncio
+async def test_delete_document_not_found(mock_mcp, mock_context):
+    """Test deleting a non-existent document."""
+    register_document_tools(mock_mcp)
+
+    # Get the delete_document function from registered tools
+    delete_document = mock_mcp._tools.get("delete_document")
+    assert delete_document is not None, "delete_document tool not registered"
+
+    # Mock 404 response
+    mock_response = MagicMock()
+    mock_response.status_code = 404
+    mock_response.text = "Document not found"
+
+    with patch("src.mcp_server.features.documents.document_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.delete.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await delete_document(
+            mock_context, project_id="project-123", doc_id="non-existent"
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is False
+        # Error must be structured format (dict), not string
+        assert "error" in result_data
+        assert isinstance(result_data["error"], dict), (
+            "Error should be structured format, not string"
+        )
+        assert result_data["error"]["type"] == "not_found"
+        assert "not found" in result_data["error"]["message"].lower()

python/tests/mcp_server/features/documents/test_version_tools.py

@@ -0,0 +1,171 @@
+"""Unit tests for version management tools."""
+
+import json
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp.server.fastmcp import Context
+
+from src.mcp_server.features.documents.version_tools import register_version_tools
+
+
+@pytest.fixture
+def mock_mcp():
+    """Create a mock MCP server for testing."""
+    mock = MagicMock()
+    # Store registered tools
+    mock._tools = {}
+
+    def tool_decorator():
+        def decorator(func):
+            mock._tools[func.__name__] = func
+            return func
+
+        return decorator
+
+    mock.tool = tool_decorator
+    return mock
+
+
+@pytest.fixture
+def mock_context():
+    """Create a mock context for testing."""
+    return MagicMock(spec=Context)
+
+
+@pytest.mark.asyncio
+async def test_create_version_success(mock_mcp, mock_context):
+    """Test successful version creation."""
+    register_version_tools(mock_mcp)
+
+    # Get the create_version function
+    create_version = mock_mcp._tools.get("create_version")
+
+    assert create_version is not None, "create_version tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "version": {"version_number": 3, "field_name": "docs"},
+        "message": "Version created successfully",
+    }
+
+    with patch("src.mcp_server.features.documents.version_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await create_version(
+            mock_context,
+            project_id="project-123",
+            field_name="docs",
+            content=[{"id": "doc-1", "title": "Test Doc"}],
+            change_summary="Added test document",
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["version_number"] == 3
+        assert "Version 3 created successfully" in result_data["message"]
+
+
+@pytest.mark.asyncio
+async def test_create_version_invalid_field(mock_mcp, mock_context):
+    """Test version creation with invalid field name."""
+    register_version_tools(mock_mcp)
+
+    create_version = mock_mcp._tools.get("create_version")
+
+    # Mock 400 response for invalid field
+    mock_response = MagicMock()
+    mock_response.status_code = 400
+    mock_response.text = "invalid field_name"
+
+    with patch("src.mcp_server.features.documents.version_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await create_version(
+            mock_context, project_id="project-123", field_name="invalid", content={"test": "data"}
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is False
+        # Error must be structured format (dict), not string
+        assert "error" in result_data
+        assert isinstance(result_data["error"], dict), (
+            "Error should be structured format, not string"
+        )
+        assert result_data["error"]["type"] == "validation_error"
+
+
+@pytest.mark.asyncio
+async def test_restore_version_success(mock_mcp, mock_context):
+    """Test successful version restoration."""
+    register_version_tools(mock_mcp)
+
+    # Get the restore_version function
+    restore_version = mock_mcp._tools.get("restore_version")
+
+    assert restore_version is not None, "restore_version tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {"message": "Version 2 restored successfully"}
+
+    with patch("src.mcp_server.features.documents.version_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await restore_version(
+            mock_context,
+            project_id="project-123",
+            field_name="docs",
+            version_number=2,
+            restored_by="test-user",
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert "Version 2 restored successfully" in result_data["message"]
+
+
+@pytest.mark.asyncio
+async def test_list_versions_with_filter(mock_mcp, mock_context):
+    """Test listing versions with field name filter."""
+    register_version_tools(mock_mcp)
+
+    # Get the list_versions function
+    list_versions = mock_mcp._tools.get("list_versions")
+
+    assert list_versions is not None, "list_versions tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "versions": [
+            {"version_number": 1, "field_name": "docs", "change_summary": "Initial"},
+            {"version_number": 2, "field_name": "docs", "change_summary": "Updated"},
+        ]
+    }
+
+    with patch("src.mcp_server.features.documents.version_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await list_versions(mock_context, project_id="project-123", field_name="docs")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["count"] == 2
+        assert len(result_data["versions"]) == 2
+
+        # Verify filter was passed
+        call_args = mock_async_client.get.call_args
+        assert call_args[1]["params"]["field_name"] == "docs"

python/tests/mcp_server/features/projects/__init__.py

@@ -0,0 +1 @@
+"""Project tools tests."""

python/tests/mcp_server/features/projects/test_project_tools.py

@@ -0,0 +1,174 @@
+"""Unit tests for project management tools."""
+
+import asyncio
+import json
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp.server.fastmcp import Context
+
+from src.mcp_server.features.projects.project_tools import register_project_tools
+
+
+@pytest.fixture
+def mock_mcp():
+    """Create a mock MCP server for testing."""
+    mock = MagicMock()
+    # Store registered tools
+    mock._tools = {}
+
+    def tool_decorator():
+        def decorator(func):
+            mock._tools[func.__name__] = func
+            return func
+
+        return decorator
+
+    mock.tool = tool_decorator
+    return mock
+
+
+@pytest.fixture
+def mock_context():
+    """Create a mock context for testing."""
+    return MagicMock(spec=Context)
+
+
+@pytest.mark.asyncio
+async def test_create_project_success(mock_mcp, mock_context):
+    """Test successful project creation with polling."""
+    register_project_tools(mock_mcp)
+
+    # Get the create_project function
+    create_project = mock_mcp._tools.get("create_project")
+
+    assert create_project is not None, "create_project tool not registered"
+
+    # Mock initial creation response with progress_id
+    mock_create_response = MagicMock()
+    mock_create_response.status_code = 200
+    mock_create_response.json.return_value = {
+        "progress_id": "progress-123",
+        "message": "Project creation started",
+    }
+
+    # Mock list projects response for polling
+    mock_list_response = MagicMock()
+    mock_list_response.status_code = 200
+    mock_list_response.json.return_value = [
+        {"id": "project-123", "title": "Test Project", "created_at": "2024-01-01"}
+    ]
+
+    with patch("src.mcp_server.features.projects.project_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        # First call creates project, subsequent calls list projects
+        mock_async_client.post.return_value = mock_create_response
+        mock_async_client.get.return_value = mock_list_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        # Mock sleep to speed up test
+        with patch("asyncio.sleep", new_callable=AsyncMock):
+            result = await create_project(
+                mock_context,
+                title="Test Project",
+                description="A test project",
+                github_repo="https://github.com/test/repo",
+            )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["project"]["id"] == "project-123"
+        assert result_data["project_id"] == "project-123"
+        assert "Project created successfully" in result_data["message"]
+
+
+@pytest.mark.asyncio
+async def test_create_project_direct_response(mock_mcp, mock_context):
+    """Test project creation with direct response (no polling)."""
+    register_project_tools(mock_mcp)
+
+    create_project = mock_mcp._tools.get("create_project")
+
+    # Mock direct creation response (no progress_id)
+    mock_create_response = MagicMock()
+    mock_create_response.status_code = 200
+    mock_create_response.json.return_value = {
+        "project": {"id": "project-123", "title": "Test Project"},
+        "message": "Project created immediately",
+    }
+
+    with patch("src.mcp_server.features.projects.project_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_create_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await create_project(mock_context, title="Test Project")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        # Direct response returns the project directly
+        assert "project" in result_data
+
+
+@pytest.mark.asyncio
+async def test_list_projects_success(mock_mcp, mock_context):
+    """Test listing projects."""
+    register_project_tools(mock_mcp)
+
+    # Get the list_projects function
+    list_projects = mock_mcp._tools.get("list_projects")
+
+    assert list_projects is not None, "list_projects tool not registered"
+
+    # Mock HTTP response - API returns a list directly
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = [
+        {"id": "proj-1", "title": "Project 1", "created_at": "2024-01-01"},
+        {"id": "proj-2", "title": "Project 2", "created_at": "2024-01-02"},
+    ]
+
+    with patch("src.mcp_server.features.projects.project_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await list_projects(mock_context)
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert len(result_data["projects"]) == 2
+        assert result_data["count"] == 2
+
+
+@pytest.mark.asyncio
+async def test_get_project_not_found(mock_mcp, mock_context):
+    """Test getting a non-existent project."""
+    register_project_tools(mock_mcp)
+
+    # Get the get_project function
+    get_project = mock_mcp._tools.get("get_project")
+
+    assert get_project is not None, "get_project tool not registered"
+
+    # Mock 404 response
+    mock_response = MagicMock()
+    mock_response.status_code = 404
+    mock_response.text = "Project not found"
+
+    with patch("src.mcp_server.features.projects.project_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await get_project(mock_context, project_id="non-existent")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is False
+        # Error must be structured format (dict), not string
+        assert "error" in result_data
+        assert isinstance(result_data["error"], dict), (
+            "Error should be structured format, not string"
+        )
+        assert result_data["error"]["type"] == "not_found"
+        assert "not found" in result_data["error"]["message"].lower()

python/tests/mcp_server/features/tasks/__init__.py

@@ -0,0 +1 @@
+"""Task tools tests."""

python/tests/mcp_server/features/tasks/test_task_tools.py

@@ -0,0 +1,209 @@
+"""Unit tests for task management tools."""
+
+import json
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp.server.fastmcp import Context
+
+from src.mcp_server.features.tasks.task_tools import register_task_tools
+
+
+@pytest.fixture
+def mock_mcp():
+    """Create a mock MCP server for testing."""
+    mock = MagicMock()
+    # Store registered tools
+    mock._tools = {}
+
+    def tool_decorator():
+        def decorator(func):
+            mock._tools[func.__name__] = func
+            return func
+
+        return decorator
+
+    mock.tool = tool_decorator
+    return mock
+
+
+@pytest.fixture
+def mock_context():
+    """Create a mock context for testing."""
+    return MagicMock(spec=Context)
+
+
+@pytest.mark.asyncio
+async def test_create_task_with_sources(mock_mcp, mock_context):
+    """Test creating a task with sources and code examples."""
+    register_task_tools(mock_mcp)
+
+    # Get the create_task function
+    create_task = mock_mcp._tools.get("create_task")
+
+    assert create_task is not None, "create_task tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "task": {"id": "task-123", "title": "Test Task"},
+        "message": "Task created successfully",
+    }
+
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.post.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await create_task(
+            mock_context,
+            project_id="project-123",
+            title="Implement OAuth2",
+            description="Add OAuth2 authentication",
+            assignee="AI IDE Agent",
+            sources=[{"url": "https://oauth.net", "type": "doc", "relevance": "OAuth spec"}],
+            code_examples=[{"file": "auth.py", "function": "authenticate", "purpose": "Example"}],
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["task_id"] == "task-123"
+
+        # Verify sources and examples were sent
+        call_args = mock_async_client.post.call_args
+        sent_data = call_args[1]["json"]
+        assert len(sent_data["sources"]) == 1
+        assert len(sent_data["code_examples"]) == 1
+
+
+@pytest.mark.asyncio
+async def test_list_tasks_with_project_filter(mock_mcp, mock_context):
+    """Test listing tasks with project-specific endpoint."""
+    register_task_tools(mock_mcp)
+
+    # Get the list_tasks function
+    list_tasks = mock_mcp._tools.get("list_tasks")
+
+    assert list_tasks is not None, "list_tasks tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "tasks": [
+            {"id": "task-1", "title": "Task 1", "status": "todo"},
+            {"id": "task-2", "title": "Task 2", "status": "doing"},
+        ]
+    }
+
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await list_tasks(mock_context, filter_by="project", filter_value="project-123")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert len(result_data["tasks"]) == 2
+
+        # Verify project-specific endpoint was used
+        call_args = mock_async_client.get.call_args
+        assert "/api/projects/project-123/tasks" in call_args[0][0]
+
+
+@pytest.mark.asyncio
+async def test_list_tasks_with_status_filter(mock_mcp, mock_context):
+    """Test listing tasks with status filter uses generic endpoint."""
+    register_task_tools(mock_mcp)
+
+    list_tasks = mock_mcp._tools.get("list_tasks")
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = [{"id": "task-1", "title": "Task 1", "status": "todo"}]
+
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await list_tasks(
+            mock_context, filter_by="status", filter_value="todo", project_id="project-123"
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+
+        # Verify generic endpoint with status param was used
+        call_args = mock_async_client.get.call_args
+        assert "/api/tasks" in call_args[0][0]
+        assert call_args[1]["params"]["status"] == "todo"
+        assert call_args[1]["params"]["project_id"] == "project-123"
+
+
+@pytest.mark.asyncio
+async def test_update_task_status(mock_mcp, mock_context):
+    """Test updating task status."""
+    register_task_tools(mock_mcp)
+
+    # Get the update_task function
+    update_task = mock_mcp._tools.get("update_task")
+
+    assert update_task is not None, "update_task tool not registered"
+
+    # Mock HTTP response
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "task": {"id": "task-123", "status": "doing"},
+        "message": "Task updated successfully",
+    }
+
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.put.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await update_task(
+            mock_context, task_id="task-123", update_fields={"status": "doing", "assignee": "User"}
+        )
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert "Task updated successfully" in result_data["message"]
+
+
+@pytest.mark.asyncio
+async def test_delete_task_already_archived(mock_mcp, mock_context):
+    """Test deleting an already archived task."""
+    register_task_tools(mock_mcp)
+
+    # Get the delete_task function
+    delete_task = mock_mcp._tools.get("delete_task")
+
+    assert delete_task is not None, "delete_task tool not registered"
+
+    # Mock 400 response for already archived
+    mock_response = MagicMock()
+    mock_response.status_code = 400
+    mock_response.text = "Task already archived"
+
+    with patch("src.mcp_server.features.tasks.task_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.delete.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await delete_task(mock_context, task_id="task-123")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is False
+        # Error must be structured format (dict), not string
+        assert "error" in result_data
+        assert isinstance(result_data["error"], dict), (
+            "Error should be structured format, not string"
+        )
+        assert result_data["error"]["type"] == "already_archived"
+        assert "already archived" in result_data["error"]["message"].lower()

python/tests/mcp_server/features/test_feature_tools.py

@@ -0,0 +1,130 @@
+"""Unit tests for feature management tools."""
+
+import json
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp.server.fastmcp import Context
+
+from src.mcp_server.features.feature_tools import register_feature_tools
+
+
+@pytest.fixture
+def mock_mcp():
+    """Create a mock MCP server for testing."""
+    mock = MagicMock()
+    # Store registered tools
+    mock._tools = {}
+
+    def tool_decorator():
+        def decorator(func):
+            mock._tools[func.__name__] = func
+            return func
+
+        return decorator
+
+    mock.tool = tool_decorator
+    return mock
+
+
+@pytest.fixture
+def mock_context():
+    """Create a mock context for testing."""
+    return MagicMock(spec=Context)
+
+
+@pytest.mark.asyncio
+async def test_get_project_features_success(mock_mcp, mock_context):
+    """Test successful retrieval of project features."""
+    register_feature_tools(mock_mcp)
+
+    # Get the get_project_features function
+    get_project_features = mock_mcp._tools.get("get_project_features")
+
+    assert get_project_features is not None, "get_project_features tool not registered"
+
+    # Mock HTTP response with various feature structures
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {
+        "features": [
+            {"name": "authentication", "status": "completed", "components": ["oauth", "jwt"]},
+            {"name": "api", "status": "in_progress", "endpoints_done": 12, "endpoints_total": 20},
+            {"name": "database", "status": "planned"},
+            {"name": "payments", "provider": "stripe", "version": "2.0", "enabled": True},
+        ]
+    }
+
+    with patch("src.mcp_server.features.feature_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await get_project_features(mock_context, project_id="project-123")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["count"] == 4
+        assert len(result_data["features"]) == 4
+
+        # Verify different feature structures are preserved
+        features = result_data["features"]
+        assert features[0]["components"] == ["oauth", "jwt"]
+        assert features[1]["endpoints_done"] == 12
+        assert features[2]["status"] == "planned"
+        assert features[3]["provider"] == "stripe"
+
+
+@pytest.mark.asyncio
+async def test_get_project_features_empty(mock_mcp, mock_context):
+    """Test getting features for a project with no features defined."""
+    register_feature_tools(mock_mcp)
+
+    get_project_features = mock_mcp._tools.get("get_project_features")
+
+    # Mock response with empty features
+    mock_response = MagicMock()
+    mock_response.status_code = 200
+    mock_response.json.return_value = {"features": []}
+
+    with patch("src.mcp_server.features.feature_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await get_project_features(mock_context, project_id="project-123")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is True
+        assert result_data["count"] == 0
+        assert result_data["features"] == []
+
+
+@pytest.mark.asyncio
+async def test_get_project_features_not_found(mock_mcp, mock_context):
+    """Test getting features for a non-existent project."""
+    register_feature_tools(mock_mcp)
+
+    get_project_features = mock_mcp._tools.get("get_project_features")
+
+    # Mock 404 response
+    mock_response = MagicMock()
+    mock_response.status_code = 404
+    mock_response.text = "Project not found"
+
+    with patch("src.mcp_server.features.feature_tools.httpx.AsyncClient") as mock_client:
+        mock_async_client = AsyncMock()
+        mock_async_client.get.return_value = mock_response
+        mock_client.return_value.__aenter__.return_value = mock_async_client
+
+        result = await get_project_features(mock_context, project_id="non-existent")
+
+        result_data = json.loads(result)
+        assert result_data["success"] is False
+        # Error must be structured format (dict), not string
+        assert "error" in result_data
+        assert isinstance(result_data["error"], dict), (
+            "Error should be structured format, not string"
+        )
+        assert result_data["error"]["type"] == "not_found"
+        assert "not found" in result_data["error"]["message"].lower()

python/tests/mcp_server/utils/__init__.py

@@ -0,0 +1 @@
+"""Tests for MCP server utility modules."""

python/tests/mcp_server/utils/test_error_handling.py

@@ -0,0 +1,164 @@
+"""Unit tests for MCPErrorFormatter utility."""
+
+import json
+from unittest.mock import MagicMock
+
+import httpx
+import pytest
+
+from src.mcp_server.utils.error_handling import MCPErrorFormatter
+
+
+def test_format_error_basic():
+    """Test basic error formatting."""
+    result = MCPErrorFormatter.format_error(
+        error_type="validation_error",
+        message="Invalid input",
+    )
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "validation_error"
+    assert result_data["error"]["message"] == "Invalid input"
+    assert "details" not in result_data["error"]
+    assert "suggestion" not in result_data["error"]
+
+
+def test_format_error_with_all_fields():
+    """Test error formatting with all optional fields."""
+    result = MCPErrorFormatter.format_error(
+        error_type="connection_timeout",
+        message="Connection timed out",
+        details={"url": "http://api.example.com", "timeout": 30},
+        suggestion="Check network connectivity",
+        http_status=504,
+    )
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "connection_timeout"
+    assert result_data["error"]["message"] == "Connection timed out"
+    assert result_data["error"]["details"]["url"] == "http://api.example.com"
+    assert result_data["error"]["suggestion"] == "Check network connectivity"
+    assert result_data["error"]["http_status"] == 504
+
+
+def test_from_http_error_with_json_body():
+    """Test formatting from HTTP response with JSON error body."""
+    mock_response = MagicMock(spec=httpx.Response)
+    mock_response.status_code = 400
+    mock_response.json.return_value = {
+        "detail": {"error": "Field is required"},
+        "message": "Validation failed",
+    }
+
+    result = MCPErrorFormatter.from_http_error(mock_response, "create item")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    # When JSON body has error details, it returns api_error, not http_error
+    assert result_data["error"]["type"] == "api_error"
+    assert "Field is required" in result_data["error"]["message"]
+    assert result_data["error"]["http_status"] == 400
+
+
+def test_from_http_error_with_text_body():
+    """Test formatting from HTTP response with text error body."""
+    mock_response = MagicMock(spec=httpx.Response)
+    mock_response.status_code = 404
+    mock_response.json.side_effect = json.JSONDecodeError("msg", "doc", 0)
+    mock_response.text = "Resource not found"
+
+    result = MCPErrorFormatter.from_http_error(mock_response, "get item")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "http_error"
+    # The message format is "Failed to {operation}: HTTP {status_code}"
+    assert "Failed to get item: HTTP 404" == result_data["error"]["message"]
+    assert result_data["error"]["http_status"] == 404
+
+
+def test_from_exception_timeout():
+    """Test formatting from timeout exception."""
+    # httpx.TimeoutException is a subclass of httpx.RequestError
+    exception = httpx.TimeoutException("Request timed out after 30s")
+
+    result = MCPErrorFormatter.from_exception(
+        exception, "fetch data", {"url": "http://api.example.com"}
+    )
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    # TimeoutException is categorized as request_error since it's a RequestError subclass
+    assert result_data["error"]["type"] == "request_error"
+    assert "Request timed out" in result_data["error"]["message"]
+    assert result_data["error"]["details"]["context"]["url"] == "http://api.example.com"
+    assert "network connectivity" in result_data["error"]["suggestion"].lower()
+
+
+def test_from_exception_connection():
+    """Test formatting from connection exception."""
+    exception = httpx.ConnectError("Failed to connect to host")
+
+    result = MCPErrorFormatter.from_exception(exception, "connect to API")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "connection_error"
+    assert "Failed to connect" in result_data["error"]["message"]
+    # The actual suggestion is "Ensure the Archon server is running on the correct port"
+    assert "archon server" in result_data["error"]["suggestion"].lower()
+
+
+def test_from_exception_request_error():
+    """Test formatting from generic request error."""
+    exception = httpx.RequestError("Network error")
+
+    result = MCPErrorFormatter.from_exception(exception, "make request")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "request_error"
+    assert "Network error" in result_data["error"]["message"]
+    assert "network connectivity" in result_data["error"]["suggestion"].lower()
+
+
+def test_from_exception_generic():
+    """Test formatting from generic exception."""
+    exception = ValueError("Invalid value")
+
+    result = MCPErrorFormatter.from_exception(exception, "process data")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    # ValueError is specifically categorized as validation_error
+    assert result_data["error"]["type"] == "validation_error"
+    assert "process data" in result_data["error"]["message"]
+    assert "Invalid value" in result_data["error"]["details"]["exception_message"]
+
+
+def test_from_exception_connect_timeout():
+    """Test formatting from connect timeout exception."""
+    exception = httpx.ConnectTimeout("Connection timed out")
+
+    result = MCPErrorFormatter.from_exception(exception, "connect to API")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "connection_timeout"
+    assert "Connection timed out" in result_data["error"]["message"]
+    assert "server is running" in result_data["error"]["suggestion"].lower()
+
+
+def test_from_exception_read_timeout():
+    """Test formatting from read timeout exception."""
+    exception = httpx.ReadTimeout("Read timed out")
+
+    result = MCPErrorFormatter.from_exception(exception, "read data")
+
+    result_data = json.loads(result)
+    assert result_data["success"] is False
+    assert result_data["error"]["type"] == "read_timeout"
+    assert "Read timed out" in result_data["error"]["message"]
+    assert "taking longer than expected" in result_data["error"]["suggestion"].lower()

python/tests/mcp_server/utils/test_timeout_config.py

@@ -0,0 +1,161 @@
+"""Unit tests for timeout configuration utility."""
+
+import os
+from unittest.mock import patch
+
+import httpx
+import pytest
+
+from src.mcp_server.utils.timeout_config import (
+    get_default_timeout,
+    get_max_polling_attempts,
+    get_polling_interval,
+    get_polling_timeout,
+)
+
+
+def test_get_default_timeout_defaults():
+    """Test default timeout values when no environment variables are set."""
+    with patch.dict(os.environ, {}, clear=True):
+        timeout = get_default_timeout()
+
+        assert isinstance(timeout, httpx.Timeout)
+        # httpx.Timeout uses 'total' for the overall timeout
+        # We need to check the actual timeout values
+        # The timeout object has different attributes than expected
+
+
+def test_get_default_timeout_from_env():
+    """Test timeout values from environment variables."""
+    env_vars = {
+        "MCP_REQUEST_TIMEOUT": "60.0",
+        "MCP_CONNECT_TIMEOUT": "10.0",
+        "MCP_READ_TIMEOUT": "40.0",
+        "MCP_WRITE_TIMEOUT": "20.0",
+    }
+
+    with patch.dict(os.environ, env_vars):
+        timeout = get_default_timeout()
+
+        assert isinstance(timeout, httpx.Timeout)
+        # Just verify it's created with the env values
+
+
+def test_get_polling_timeout_defaults():
+    """Test default polling timeout values."""
+    with patch.dict(os.environ, {}, clear=True):
+        timeout = get_polling_timeout()
+
+        assert isinstance(timeout, httpx.Timeout)
+        # Default polling timeout is 60.0, not 10.0
+
+
+def test_get_polling_timeout_from_env():
+    """Test polling timeout from environment variables."""
+    env_vars = {
+        "MCP_POLLING_TIMEOUT": "15.0",
+        "MCP_CONNECT_TIMEOUT": "3.0",  # Uses MCP_CONNECT_TIMEOUT, not MCP_POLLING_CONNECT_TIMEOUT
+    }
+
+    with patch.dict(os.environ, env_vars):
+        timeout = get_polling_timeout()
+
+        assert isinstance(timeout, httpx.Timeout)
+
+
+def test_get_max_polling_attempts_default():
+    """Test default max polling attempts."""
+    with patch.dict(os.environ, {}, clear=True):
+        attempts = get_max_polling_attempts()
+
+        assert attempts == 30
+
+
+def test_get_max_polling_attempts_from_env():
+    """Test max polling attempts from environment variable."""
+    with patch.dict(os.environ, {"MCP_MAX_POLLING_ATTEMPTS": "50"}):
+        attempts = get_max_polling_attempts()
+
+        assert attempts == 50
+
+
+def test_get_max_polling_attempts_invalid_env():
+    """Test max polling attempts with invalid environment variable."""
+    with patch.dict(os.environ, {"MCP_MAX_POLLING_ATTEMPTS": "not_a_number"}):
+        attempts = get_max_polling_attempts()
+
+        # Should fall back to default after ValueError handling
+        assert attempts == 30
+
+
+def test_get_polling_interval_base():
+    """Test base polling interval (attempt 0)."""
+    with patch.dict(os.environ, {}, clear=True):
+        interval = get_polling_interval(0)
+
+        assert interval == 1.0
+
+
+def test_get_polling_interval_exponential_backoff():
+    """Test exponential backoff for polling intervals."""
+    with patch.dict(os.environ, {}, clear=True):
+        # Test exponential growth
+        assert get_polling_interval(0) == 1.0
+        assert get_polling_interval(1) == 2.0
+        assert get_polling_interval(2) == 4.0
+
+        # Test max cap at 5 seconds (default max_interval)
+        assert get_polling_interval(3) == 5.0  # Would be 8.0 but capped at 5.0
+        assert get_polling_interval(4) == 5.0
+        assert get_polling_interval(10) == 5.0
+
+
+def test_get_polling_interval_custom_base():
+    """Test polling interval with custom base interval."""
+    with patch.dict(os.environ, {"MCP_POLLING_BASE_INTERVAL": "2.0"}):
+        assert get_polling_interval(0) == 2.0
+        assert get_polling_interval(1) == 4.0
+        assert get_polling_interval(2) == 5.0  # Would be 8.0 but capped at default max (5.0)
+        assert get_polling_interval(3) == 5.0  # Capped at max
+
+
+def test_get_polling_interval_custom_max():
+    """Test polling interval with custom max interval."""
+    with patch.dict(os.environ, {"MCP_POLLING_MAX_INTERVAL": "5.0"}):
+        assert get_polling_interval(0) == 1.0
+        assert get_polling_interval(1) == 2.0
+        assert get_polling_interval(2) == 4.0
+        assert get_polling_interval(3) == 5.0  # Capped at custom max
+        assert get_polling_interval(10) == 5.0
+
+
+def test_get_polling_interval_all_custom():
+    """Test polling interval with all custom values."""
+    env_vars = {
+        "MCP_POLLING_BASE_INTERVAL": "0.5",
+        "MCP_POLLING_MAX_INTERVAL": "3.0",
+    }
+
+    with patch.dict(os.environ, env_vars):
+        assert get_polling_interval(0) == 0.5
+        assert get_polling_interval(1) == 1.0
+        assert get_polling_interval(2) == 2.0
+        assert get_polling_interval(3) == 3.0  # Capped at custom max
+        assert get_polling_interval(10) == 3.0
+
+
+def test_timeout_values_are_floats():
+    """Test that all timeout values are properly converted to floats."""
+    env_vars = {
+        "MCP_REQUEST_TIMEOUT": "30",  # Integer string
+        "MCP_CONNECT_TIMEOUT": "5",
+        "MCP_POLLING_BASE_INTERVAL": "1",
+        "MCP_POLLING_MAX_INTERVAL": "10",
+    }
+
+    with patch.dict(os.environ, env_vars):
+        timeout = get_default_timeout()
+        assert isinstance(timeout, httpx.Timeout)
+
+        interval = get_polling_interval(0)
+        assert isinstance(interval, float)