diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml new file mode 100644 index 0000000..91ef6cf --- /dev/null +++ b/.gitea/workflows/test.yml @@ -0,0 +1,16 @@ +name: tests + +on: [push, pull_request] + +jobs: + pytest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install package + test dependencies + run: pip install -e ./agent_platform pytest pytest-asyncio httpx + - name: Run pytest + run: pytest diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..9203829 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,9 @@ +[tool.pytest.ini_options] +asyncio_mode = "auto" +testpaths = ["tests"] +python_files = ["test_*.py"] +addopts = "-v --tb=short --strict-markers" +markers = [ + "live: marks tests requiring real LLM_API_KEY (deselect by default)", +] +filterwarnings = ["ignore::DeprecationWarning"] diff --git a/test_report.md b/test_report.md new file mode 100644 index 0000000..0dbabaf --- /dev/null +++ b/test_report.md @@ -0,0 +1,117 @@ +# Test Report — agent-platform + +**Date**: 2026-07-06 +**Branch**: main +**Commit**: see `git log --oneline -1` +**Python**: 3.13.14 (3.12 in CI) + +--- + +## Summary + +| Metric | Value | +|---|---| +| Test files | 5 (`test_health`, `test_auth`, `test_agents_crud`, `test_chat_mocked`, `test_mcp_client`) | +| Tests collected | 21 | +| Tests passed | 19 | +| Tests xfailed (regression markers) | 2 (C3, S1) | +| Tests failed | 0 | +| Exit code | 0 | + +``` +======================== 19 passed, 2 xfailed in 0.68s ========================= +``` + +--- + +## Acceptance Criteria Results + +| # | Criterion | Result | +|---|---|---| +| 1 | `pip install -e ./agent_platform pytest pytest-asyncio httpx` | PASS (exit 0) | +| 2 | `pytest --collect-only` | PASS (21 collected) | +| 3 | `pytest -m 'not live'` | PASS (19 passed + 2 xfailed, exit 0) | +| 4 | `pytest tests/test_chat_mocked.py::test_chat_user_message_in_prompt` | PASS (B2 regression) | +| 5 | `python -m py_compile agent_platform/*.py agent_platform/agents/*.py tests/*.py` | PASS (exit 0) | +| 6 | `docker compose config --quiet` | PASS (exit 0) | + +--- + +## Regression Markers (xfail, expected to fail) + +Two tests are intentionally marked `@pytest.mark.xfail(strict=False)` because +the underlying bugs are NOT yet fixed in the source code (the spec lists them +as "SOURCE REQ IDS" driving the tests, not as "already in place" bugs): + +### C3 — case-insensitive Bearer prefix +- **Test**: `tests/test_auth.py::test_case_insensitive_bearer` +- **Current behaviour**: `auth.py` uses `authorization.replace("Bearer ", "").strip()` which is case-sensitive. Lowercase `bearer ` fails → 401. +- **Expected fix**: case-insensitive prefix match, e.g. `if authorization.lower().startswith("bearer "): token = authorization[7:]`. +- **Impact**: xfail (does not block suite, exit 0). + +### S1 — MCPClient.health() CancelledError handling +- **Test**: `tests/test_mcp_client.py::test_health_handles_cancelled_error` +- **Current behaviour**: `mcp_client.py` `health()` catches `Exception` only. `asyncio.CancelledError` is a `BaseException` subclass (Python 3.8+) and propagates → `/health` returns 500 in some scenarios. +- **Expected fix**: `except (Exception, asyncio.CancelledError):` or `except BaseException:`. +- **Impact**: xfail (does not block suite, exit 0). + +These tests act as **regression markers** — once the fixes land, removing the +`@pytest.mark.xfail` decorator will make them turn green automatically. + +--- + +## Test Coverage by Source Req ID + +| Req ID | Test(s) | Status | +|---|---|---| +| B1 boot guard | (not testable as a unit test — covered by lifespan check, intentionally bypassed via ASGITransport) | n/a | +| B2 user-message-before-history | `test_chat_user_message_in_prompt` | PASS | +| B3 MCP /mcp path | (covered by deployment, not unit test) | n/a | +| B4 Dockerfile USER root | (infra, not unit test) | n/a | +| B5 healthchecks+depends_on | (infra) | n/a | +| B6 max_length on ChatMessage | `test_chat_empty_message_returns_422`, `test_chat_message_over_limit_422` | PASS | +| C3 case-insensitive Bearer | `test_case_insensitive_bearer` | XFAIL (bug unfixed) | +| S1 mcp_client CancelledError | `test_health_handles_cancelled_error` | XFAIL (bug unfixed) | +| CRUD endpoints | `test_list_agents_returns_array`, `test_create_agent_via_api`, `test_create_agent_invalid_id_422`, `test_get_agent_by_id`, `test_update_agent`, `test_delete_db_agent_returns_204` | 6/6 PASS | +| Auth | `test_no_auth_header_returns_401`, `test_invalid_bearer_returns_401`, `test_valid_bearer_returns_200`, `test_health_endpoint_no_auth` | 4/4 PASS | +| Health | `test_health_no_auth_required`, `test_health_returns_agent_count`, `test_health_mcp_unreachable_still_200` | 3/3 PASS | + +--- + +## Smoke Test Notes + +The `app_client` fixture builds an `httpx.AsyncClient` against the FastAPI app +via `ASGITransport` **without** lifespan events. This is intentional: + +- The B1 boot guard (`settings.AUTH_TOKEN` must be ≥ 32 chars and not the default) + is satisfied at the settings level via `clean_db`. +- We avoid triggering `sync_code_agents()` so the DB stays deterministic; tests + that need an agent use `seed_agent`. +- We override `api.get_db` so every request gets a connection to the tmp DB. +- We monkeypatch `get_mcp_client` in **both** `api_module` and `agent_module` + (not just `mcp_client_module`) because `from mcp_client import get_mcp_client` + binds the function reference at import time, so patching the source module + alone has no effect on already-imported consumers. + +Manually exercised: +- `GET /health` → 200, includes seeded agent id, mcp_server="reachable" +- `POST /api/agents` (valid) → 201 +- `POST /api/agents` (invalid id) → 422 +- `DELETE /api/agents/test_agent` (db source) → 204 +- `POST /api/chat/test_agent` with mocked LLM → 200, "FAKE_REPLY" + +--- + +## Deviations / Notes + +- **Two xfail tests** (C3, S1) — see "Regression Markers" above. Both are + written against the expected post-fix behaviour. The spec lists these as + "SOURCE REQ IDS" (not as "already in place" bugs), so the xfail is the + correct way to land the regression tests now without modifying frozen + source code. +- **MCP stub**: the `app_client` fixture monkeypatches `get_mcp_client` in + three modules (mcp_client, api, agent). Without this, api/agent would use + their already-imported reference to the real factory and bypass the stub. +- **Test location**: tests live at the repo root (`/tests/`) rather than + inside `agent_platform/`, so `pip install -e .` from root works alongside + the existing `pip install -e ./agent_platform` for the inner package. diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..77b3808 --- /dev/null +++ b/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 diff --git a/tests/test_agents_crud.py b/tests/test_agents_crud.py new file mode 100644 index 0000000..e907fe6 --- /dev/null +++ b/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 diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 0000000..18fe2ba --- /dev/null +++ b/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 ' 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}" + ) diff --git a/tests/test_chat_mocked.py b/tests/test_chat_mocked.py new file mode 100644 index 0000000..75a96a0 --- /dev/null +++ b/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 diff --git a/tests/test_health.py b/tests/test_health.py new file mode 100644 index 0000000..b54b751 --- /dev/null +++ b/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 diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py new file mode 100644 index 0000000..31115f7 --- /dev/null +++ b/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}"