test: add pytest suite + CI workflow (21 tests)

tests/conftest.py

@@ -0,0 +1,232 @@
+"""Shared pytest fixtures for agent-platform test suite.
+
+The agent_platform package uses flat module imports (e.g. ``from api import app``)
+configured by ``agent_platform/pyproject.toml``'s ``py-modules``. We mirror that
+by adding ``agent_platform/`` to ``sys.path`` so the same import paths work in
+tests without needing the package to be installed.
+"""
+from __future__ import annotations
+
+import os
+import sys
+from pathlib import Path
+from typing import AsyncIterator
+
+import aiosqlite
+import pytest
+import pytest_asyncio
+
+# Make ``agent_platform/`` flat modules importable as if the package was installed.
+ROOT = Path(__file__).resolve().parent.parent
+AGENT_PLATFORM_DIR = ROOT / "agent_platform"
+if str(AGENT_PLATFORM_DIR) not in sys.path:
+    sys.path.insert(0, str(AGENT_PLATFORM_DIR))
+
+# Set AUTH_TOKEN env var BEFORE importing config so the settings instance
+# sees a strong token (the production boot-guard requires >= 32 chars).
+# Individual fixtures monkeypatch settings.AUTH_TOKEN back to the test value.
+os.environ.setdefault("AUTH_TOKEN", "x" * 64)
+os.environ.setdefault("DB_PATH", str(ROOT / "tests" / ".unused.db"))
+os.environ.setdefault("LLM_API_KEY", "")
+os.environ.setdefault("MCP_SERVER_URL", "http://localhost:8501/mcp")
+
+from config import settings  # noqa: E402  (must come after sys.path/env setup)
+import config as config_module  # noqa: E402
+import api as api_module  # noqa: E402
+import auth as auth_module  # noqa: E402
+import db as db_module  # noqa: E402
+import llm as llm_module  # noqa: E402
+import mcp_client as mcp_client_module  # noqa: E402
+import agent as agent_module  # noqa: E402
+from api import app  # noqa: E402
+
+
+# === Paths & Auth ===
+
+AUTH_TOKEN_VALUE = "change-me-in-production"
+AUTH_HEADERS = {"Authorization": f"Bearer {AUTH_TOKEN_VALUE}"}
+
+
+@pytest.fixture
+def tmp_db_path(tmp_path) -> str:
+    """Return a unique DB path under pytest's tmp_path."""
+    return str(tmp_path / "test.db")
+
+
+@pytest.fixture
+def auth_headers() -> dict:
+    """Authorization header dict using the literal default AUTH_TOKEN."""
+    return dict(AUTH_HEADERS)
+
+
+# === DB + Settings ===
+
+@pytest_asyncio.fixture
+async def clean_db(tmp_db_path, monkeypatch) -> AsyncIterator[aiosqlite.Connection]:
+    """Point settings at tmp_db_path, init schema, yield a live connection."""
+    monkeypatch.setattr(config_module.settings, "DB_PATH", tmp_db_path)
+    monkeypatch.setattr(config_module.settings, "AUTH_TOKEN", AUTH_TOKEN_VALUE)
+
+    await db_module.init_db()
+    db = await aiosqlite.connect(tmp_db_path)
+    try:
+        yield db
+    finally:
+        await db.close()
+
+
+# === MCP Stub ===
+
+class _StubMCPClient:
+    """In-memory MCP replacement used by API + agent layer."""
+
+    def __init__(self, tools=("echo", "calculate"), healthy: bool = True,
+                 health_exc: BaseException | None = None,
+                 connect_exc: BaseException | None = None,
+                 list_tools_exc: BaseException | None = None):
+        self._tools = list(tools)
+        self._healthy = healthy
+        self._health_exc = health_exc
+        self._connect_exc = connect_exc
+        self._list_tools_exc = list_tools_exc
+
+    async def list_tools(self):
+        if self._list_tools_exc:
+            raise self._list_tools_exc
+        return [{"name": t, "description": f"stub:{t}", "inputSchema": {"type": "object", "properties": {}}} for t in self._tools]
+
+    async def health(self) -> bool:
+        if self._health_exc:
+            raise self._health_exc
+        if self._connect_exc:
+            raise self._connect_exc
+        return self._healthy
+
+    async def connect(self):
+        if self._connect_exc:
+            raise self._connect_exc
+        return None
+
+    async def disconnect(self):
+        return None
+
+    async def call_tool(self, name, arguments):
+        return {"name": name, "result": f"stub-{name}"}
+
+    async def get_tools_for_llm(self, allowed=None):
+        tools = await self.list_tools()
+        result = []
+        for t in tools:
+            name = t.get("name")
+            if allowed and name not in allowed:
+                continue
+            result.append({
+                "type": "function",
+                "function": {
+                    "name": name,
+                    "description": t.get("description", ""),
+                    "parameters": t.get("inputSchema", {"type": "object", "properties": {}}),
+                },
+            })
+        return result
+
+
+@pytest.fixture
+def stub_mcp():
+    """Default healthy stub returning ['echo', 'calculate']."""
+    return _StubMCPClient()
+
+
+@pytest.fixture
+def stub_mcp_unreachable():
+    """Stub whose health() raises CancelledError (S1 regression fixture)."""
+    import asyncio
+    return _StubMCPClient(connect_exc=asyncio.CancelledError())
+
+
+def AsyncMock_dummy():
+    """Async no-op function to replace close_mcp_client during tests."""
+    async def _noop():
+        return None
+    return _noop
+
+
+# === HTTP Client ===
+
+async def _override_get_db() -> AsyncIterator[aiosqlite.Connection]:
+    """Yield a connection to settings.DB_PATH for the duration of one request."""
+    db = await aiosqlite.connect(config_module.settings.DB_PATH)
+    try:
+        yield db
+    finally:
+        await db.close()
+
+
+@pytest_asyncio.fixture
+async def app_client(clean_db, monkeypatch, stub_mcp) -> AsyncIterator:
+    """httpx.AsyncClient against the FastAPI app via ASGI transport.
+
+    * Uses ASGITransport WITHOUT lifespan events (boot guard would otherwise
+      require a 32+ char AUTH_TOKEN, which is set via clean_db).
+    * Overrides ``api.get_db`` so every request gets the tmp DB connection.
+    * Stubs ``get_mcp_client()`` (in api + agent modules, not just mcp_client)
+      so list_tools/health never hit the network.
+    """
+    import httpx
+
+    # Override DB dependency
+    app.dependency_overrides[api_module.get_db] = _override_get_db
+
+    # Stub MCP client in ALL modules that imported it via `from mcp_client import`
+    # (api + agent both have their own references that monkeypatch on the source
+    # module alone won't affect).
+    factory = lambda: stub_mcp
+    monkeypatch.setattr(mcp_client_module, "get_mcp_client", factory)
+    monkeypatch.setattr(api_module, "get_mcp_client", factory)
+    monkeypatch.setattr(agent_module, "get_mcp_client", factory)
+    monkeypatch.setattr(mcp_client_module, "close_mcp_client", AsyncMock_dummy())
+
+    transport = httpx.ASGITransport(app=app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
+        yield client
+
+    app.dependency_overrides.clear()
+
+
+@pytest_asyncio.fixture
+async def app_client_mcp_down(clean_db, monkeypatch, stub_mcp_unreachable) -> AsyncIterator:
+    """Same as app_client but MCP client raises CancelledError on health()."""
+    import httpx
+
+    app.dependency_overrides[api_module.get_db] = _override_get_db
+    factory = lambda: stub_mcp_unreachable
+    monkeypatch.setattr(mcp_client_module, "get_mcp_client", factory)
+    monkeypatch.setattr(api_module, "get_mcp_client", factory)
+    monkeypatch.setattr(agent_module, "get_mcp_client", factory)
+    monkeypatch.setattr(mcp_client_module, "close_mcp_client", AsyncMock_dummy())
+
+    transport = httpx.ASGITransport(app=app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
+        yield client
+
+    app.dependency_overrides.clear()
+
+
+# === Seeding ===
+
+@pytest_asyncio.fixture
+async def seed_agent(clean_db) -> dict:
+    """Insert one db-source agent 'test_agent' for tests that need it."""
+    payload = {
+        "id": "test_agent",
+        "name": "Test Agent",
+        "description": "Seeded test agent",
+        "system_prompt": "You are a test agent. Reply concisely.",
+        "allowed_tools": ["echo", "calculate"],
+        "model": None,
+        "temperature": 0.7,
+        "max_tokens": 2000,
+        "enabled": True,
+    }
+    await db_module.upsert_agent(clean_db, payload, source="db")
+    return payload

tests/test_agents_crud.py

@@ -0,0 +1,90 @@
+"""CRUD tests for /api/agents endpoints (admin operations)."""
+import pytest
+
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_list_agents_returns_array(app_client, auth_headers, seed_agent):
+    """GET /api/agents -> 200 with a JSON array of agents."""
+    resp = await app_client.get("/api/agents", headers=auth_headers)
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert isinstance(body, list)
+    assert len(body) >= 1
+    assert any(a["id"] == "test_agent" for a in body)
+
+
+async def test_create_agent_via_api(app_client, auth_headers):
+    """POST /api/agents with valid payload -> 201 and body has id."""
+    payload = {
+        "id": "crud_test_agent",
+        "name": "CRUD Test Agent",
+        "description": "created via test",
+        "system_prompt": "You are a CRUD test agent.",
+        "allowed_tools": ["echo"],
+        "model": None,
+        "temperature": 0.5,
+        "max_tokens": 1000,
+        "enabled": True,
+    }
+    resp = await app_client.post("/api/agents", json=payload, headers=auth_headers)
+    assert resp.status_code == 201, resp.text
+    body = resp.json()
+    assert body["id"] == "crud_test_agent"
+    assert body["name"] == "CRUD Test Agent"
+    assert body["enabled"] is True
+
+
+async def test_create_agent_invalid_id_422(app_client, auth_headers):
+    """POST /api/agents with invalid id 'BadId' (uppercase, invalid chars) -> 422."""
+    payload = {
+        "id": "BadId",  # fails pattern ^[a-z0-9_-]+$
+        "name": "Invalid ID Agent",
+        "system_prompt": "x",
+    }
+    resp = await app_client.post("/api/agents", json=payload, headers=auth_headers)
+    assert resp.status_code == 422, resp.text
+
+
+async def test_get_agent_by_id(app_client, auth_headers, seed_agent):
+    """GET /api/agents/{id} -> 200 with the agent body."""
+    resp = await app_client.get("/api/agents/test_agent", headers=auth_headers)
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert body["id"] == "test_agent"
+    assert body["name"] == "Test Agent"
+
+
+async def test_update_agent(app_client, auth_headers, seed_agent, clean_db):
+    """PUT /api/agents/{id} -> 200 and updated_at changes (proves write)."""
+    # Read current updated_at
+    before = await app_client.get("/api/agents/test_agent", headers=auth_headers)
+    assert before.status_code == 200
+    before_updated_at = before.json()["updated_at"]
+
+    # Apply update
+    update_payload = {"description": "Updated by test", "temperature": 0.42}
+    resp = await app_client.put(
+        "/api/agents/test_agent", json=update_payload, headers=auth_headers
+    )
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert body["description"] == "Updated by test"
+    assert body["temperature"] == 0.42
+    # updated_at should be present (CURRENT_TIMESTAMP == same or later)
+    assert "updated_at" in body
+    assert body["updated_at"] is not None
+    # If the DB clock granularity is the same string, that's still a successful write.
+    # We assert at minimum that updated_at is present and the write completed.
+
+
+async def test_delete_db_agent_returns_204(app_client, auth_headers, seed_agent):
+    """DELETE /api/agents/{db_agent_id} -> 204 (db-source agents are hard-deleted)."""
+    # seed_agent is a db-source agent, so DELETE should return 204
+    resp = await app_client.delete("/api/agents/test_agent", headers=auth_headers)
+    assert resp.status_code == 204, resp.text
+
+    # And subsequent GET should return 404
+    follow = await app_client.get("/api/agents/test_agent", headers=auth_headers)
+    assert follow.status_code == 404

tests/test_auth.py

@@ -0,0 +1,46 @@
+"""Tests for token-based auth (Bearer header validation)."""
+import pytest
+
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_no_auth_header_returns_401(app_client):
+    """GET /api/agents without Authorization header -> 401."""
+    resp = await app_client.get("/api/agents")
+    assert resp.status_code == 401
+
+
+async def test_invalid_bearer_returns_401(app_client):
+    """GET /api/agents with wrong Bearer token -> 401."""
+    resp = await app_client.get("/api/agents", headers={"Authorization": "Bearer wrongtoken"})
+    assert resp.status_code == 401
+
+
+async def test_valid_bearer_returns_200(app_client, auth_headers, seed_agent):
+    """GET /api/agents with correct Bearer -> 200 and list payload."""
+    resp = await app_client.get("/api/agents", headers=auth_headers)
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert isinstance(body, list)
+    assert any(a["id"] == "test_agent" for a in body)
+
+
+async def test_health_endpoint_no_auth(app_client):
+    """The /health endpoint is open: no auth header needed, returns 200."""
+    resp = await app_client.get("/health")
+    assert resp.status_code == 200
+
+
+@pytest.mark.xfail(
+    reason="C3 regression: auth.py uses .replace('Bearer ', '') which is case-sensitive. "
+           "Lowercase 'bearer <token>' currently fails. Fix: use a case-insensitive prefix match.",
+    strict=False,
+)
+async def test_case_insensitive_bearer(app_client, auth_headers, seed_agent):
+    """C3 regression: lowercase 'bearer change-me-in-production' must work."""
+    lower = {"Authorization": auth_headers["Authorization"].replace("Bearer ", "bearer ")}
+    resp = await app_client.get("/api/agents", headers=lower)
+    assert resp.status_code == 200, (
+        f"expected 200 with lowercase 'bearer' prefix, got {resp.status_code}: {resp.text}"
+    )

tests/test_chat_mocked.py

@@ -0,0 +1,100 @@
+"""Tests for /api/chat/{agent_id} with a mocked LLM.
+
+B2 regression: the user's message must reach the LLM (was broken when history
+slice dropped the new turn-message). Test by capturing the messages list
+passed to ``llm.chat`` and asserting the last item is the user's input.
+"""
+import pytest
+
+from llm import LLMResponse
+
+
+pytestmark = pytest.mark.asyncio
+
+
+def _fake_llm_response(content: str = "FAKE_REPLY", tool_calls=None, usage=None) -> LLMResponse:
+    return LLMResponse(content=content, tool_calls=tool_calls or [], usage=usage or {
+        "prompt_tokens": 1,
+        "completion_tokens": 1,
+        "total_tokens": 2,
+        "finish_reason": "stop",
+    })
+
+
+async def test_chat_empty_message_returns_422(app_client, auth_headers, seed_agent):
+    """POST /api/chat with empty message -> 422 (B6 min_length=1)."""
+    resp = await app_client.post(
+        "/api/chat/test_agent",
+        json={"message": ""},
+        headers=auth_headers,
+    )
+    assert resp.status_code == 422, resp.text
+
+
+async def test_chat_message_over_limit_422(app_client, auth_headers, seed_agent):
+    """POST /api/chat with 40000-char message -> 422 (B6 max_length=32000)."""
+    resp = await app_client.post(
+        "/api/chat/test_agent",
+        json={"message": "x" * 40000},
+        headers=auth_headers,
+    )
+    assert resp.status_code == 422, resp.text
+
+
+async def test_chat_with_mocked_llm_returns_assistant(app_client, auth_headers, seed_agent, monkeypatch, stub_mcp, clean_db):
+    """Stub llm.chat -> 200 with assistant message; messages table has 2 rows (user + assistant)."""
+    async def fake_chat(messages, tools=None, model=None, temperature=0.7, max_tokens=2000, **_kw):
+        return _fake_llm_response(content="FAKE_REPLY")
+
+    monkeypatch.setattr("agent.chat", fake_chat)
+
+    resp = await app_client.post(
+        "/api/chat/test_agent",
+        json={"message": "hello world"},
+        headers=auth_headers,
+    )
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert body["content"] == "FAKE_REPLY"
+    assert "conversation_id" in body
+    conv_id = body["conversation_id"]
+
+    # Verify messages table: 1 user + 1 assistant = 2 rows
+    async with clean_db.execute(
+        "SELECT role, content FROM messages WHERE conversation_id = ? ORDER BY id ASC",
+        (conv_id,),
+    ) as cur:
+        rows = await cur.fetchall()
+    assert len(rows) == 2, f"expected 2 rows (user+assistant), got {len(rows)}"
+    assert rows[0][0] == "user"
+    assert rows[0][1] == "hello world"
+    assert rows[1][0] == "assistant"
+    assert rows[1][1] == "FAKE_REPLY"
+
+
+async def test_chat_user_message_in_prompt(app_client, auth_headers, seed_agent, monkeypatch, stub_mcp):
+    """B2 regression: capture messages list passed to llm.chat, assert last item == user input."""
+    captured = {}
+
+    async def capturing_chat(messages, tools=None, model=None, temperature=0.7, max_tokens=2000, **_kw):
+        captured["messages"] = list(messages)
+        return _fake_llm_response(content="FAKE_REPLY")
+
+    monkeypatch.setattr("agent.chat", capturing_chat)
+
+    user_input = "what is the meaning of life, the universe, and everything?"
+    resp = await app_client.post(
+        "/api/chat/test_agent",
+        json={"message": user_input},
+        headers=auth_headers,
+    )
+    assert resp.status_code == 200, resp.text
+
+    assert "messages" in captured, "llm.chat was not invoked"
+    msgs = captured["messages"]
+    assert len(msgs) >= 2
+    # First message must be the system prompt
+    assert msgs[0]["role"] == "system"
+    # Last message must be the user's input (B2 regression)
+    assert msgs[-1]["role"] == "user"
+    assert msgs[-1]["content"] == user_input

tests/test_health.py

@@ -0,0 +1,35 @@
+"""Tests for the /health endpoint (no auth required, MCP failure tolerant)."""
+import pytest
+
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_health_no_auth_required(app_client):
+    """GET /health must return 200 without any Authorization header."""
+    resp = await app_client.get("/health")
+    assert resp.status_code == 200, resp.text
+
+
+async def test_health_returns_agent_count(app_client, seed_agent):
+    """Body must include `agents >= 1` and an `agent_ids` list."""
+    resp = await app_client.get("/health")
+    assert resp.status_code == 200
+    body = resp.json()
+    assert "agents" in body
+    assert "agent_ids" in body
+    assert isinstance(body["agent_ids"], list)
+    assert body["agents"] >= 1
+    assert "test_agent" in body["agent_ids"]
+    assert body["status"] == "ok"
+    assert body["mcp_server"] == "reachable"
+
+
+async def test_health_mcp_unreachable_still_200(app_client_mcp_down, seed_agent):
+    """If MCP health raises CancelledError, /health still returns 200 with mcp_server=unreachable."""
+    resp = await app_client_mcp_down.get("/health")
+    assert resp.status_code == 200, resp.text
+    body = resp.json()
+    assert body["status"] == "ok"
+    assert body["mcp_server"] == "unreachable"
+    assert body["agents"] >= 1

tests/test_mcp_client.py

@@ -0,0 +1,71 @@
+"""Tests for the MCPClient (tool listing + health check + CancelledError handling).
+
+S1 regression: the original MCPClient.health() only caught ``Exception``.
+``asyncio.CancelledError`` is a ``BaseException`` subclass (Python 3.8+) so
+it propagated up and broke callers. health() must swallow it and return False.
+"""
+import asyncio
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from mcp_client import MCPClient
+
+
+pytestmark = pytest.mark.asyncio
+
+
+async def test_list_tools_returns_array(monkeypatch):
+    """list_tools() returns a list (from stubbed MCP session)."""
+    client = MCPClient(server_url="http://stub:0/mcp")
+
+    fake_tool = MagicMock()
+    fake_tool.model_dump.return_value = {
+        "name": "echo",
+        "description": "echo input",
+        "inputSchema": {"type": "object", "properties": {}},
+    }
+
+    session = MagicMock()
+    session.list_tools = AsyncMock(
+        return_value=MagicMock(tools=[fake_tool])
+    )
+    session.initialize = AsyncMock(return_value=None)
+
+    # Manually wire the internal session to bypass real streamable_http
+    client._session = session
+    tools = await client.list_tools()
+    assert isinstance(tools, list)
+    assert tools[0]["name"] == "echo"
+    assert tools[0]["description"] == "echo input"
+
+
+async def test_health_check_returns_bool(monkeypatch):
+    """health() returns a bool, even when connect() fails."""
+    client = MCPClient(server_url="http://stub:0/mcp")
+
+    async def fail_connect():
+        raise ConnectionError("simulated unreachable")
+
+    monkeypatch.setattr(client, "connect", fail_connect)
+    result = await client.health()
+    assert isinstance(result, bool)
+    assert result is False
+
+
+@pytest.mark.xfail(
+    reason="S1 regression: MCPClient.health() catches 'Exception' only. "
+           "asyncio.CancelledError is a BaseException subclass and propagates. "
+           "Fix: catch (Exception, asyncio.CancelledError) or use BaseException.",
+    strict=False,
+)
+async def test_health_handles_cancelled_error(monkeypatch):
+    """S1 regression: health() must return False when connect() raises CancelledError."""
+    client = MCPClient(server_url="http://stub:0/mcp")
+
+    async def boom():
+        raise asyncio.CancelledError()
+
+    monkeypatch.setattr(client, "connect", boom)
+    result = await client.health()
+    assert result is False, f"expected False, got {result}"