Fix review findings and add regression tests

tests/integration/test_mcp_bridge_routes.py

@@ -0,0 +1,270 @@
+"""Integration tests for MCP HTTP bridge routes."""
+
+from __future__ import annotations
+
+from datetime import UTC, datetime
+from pathlib import Path
+
+import pytest
+import yaml
+from httpx import ASGITransport, AsyncClient
+
+from web2api.main import create_app
+from web2api.schemas import (
+    ApiResponse,
+    ErrorCode,
+    ErrorResponse,
+    MetadataResponse,
+    PaginationResponse,
+    SiteInfo,
+)
+
+
+class FakePool:
+    """Pool stub used for MCP bridge route tests."""
+
+    async def start(self) -> None:
+        return None
+
+    async def stop(self) -> None:
+        return None
+
+    @property
+    def health(self) -> dict[str, int | bool]:
+        return {
+            "browser_connected": True,
+            "total_contexts": 1,
+            "available_contexts": 1,
+            "queue_size": 0,
+            "total_requests_served": 0,
+        }
+
+
+def _write_recipe(recipes_dir: Path, slug: str) -> None:
+    recipe_dir = recipes_dir / slug
+    recipe_dir.mkdir(parents=True, exist_ok=True)
+    (recipe_dir / "recipe.yaml").write_text(
+        yaml.safe_dump(
+            {
+                "name": slug.title(),
+                "slug": slug,
+                "base_url": "https://example.com",
+                "description": f"{slug} fixture recipe",
+                "endpoints": {
+                    "read": {
+                        "url": "https://example.com/items?page={page}",
+                        "items": {
+                            "container": ".item",
+                            "fields": {"title": {"selector": ".title"}},
+                        },
+                        "pagination": {"type": "page_param", "param": "page"},
+                    },
+                },
+            }
+        ),
+        encoding="utf-8",
+    )
+
+
+def _success_response(*, slug: str, endpoint: str, page: int) -> ApiResponse:
+    return ApiResponse(
+        site=SiteInfo(name=slug.title(), slug=slug, url="https://example.com"),
+        endpoint=endpoint,
+        query=None,
+        items=[],
+        pagination=PaginationResponse(
+            current_page=page,
+            has_next=False,
+            has_prev=page > 1,
+            total_pages=None,
+            total_items=None,
+        ),
+        metadata=MetadataResponse(
+            scraped_at=datetime.now(UTC),
+            response_time_ms=1,
+            item_count=0,
+            cached=False,
+        ),
+        error=None,
+    )
+
+
+def _error_response(*, slug: str, endpoint: str, page: int, code: ErrorCode) -> ApiResponse:
+    return ApiResponse(
+        site=SiteInfo(name=slug.title(), slug=slug, url="https://example.com"),
+        endpoint=endpoint,
+        query=None,
+        items=[],
+        pagination=PaginationResponse(
+            current_page=page,
+            has_next=False,
+            has_prev=page > 1,
+            total_pages=None,
+            total_items=None,
+        ),
+        metadata=MetadataResponse(
+            scraped_at=datetime.now(UTC),
+            response_time_ms=1,
+            item_count=0,
+            cached=False,
+        ),
+        error=ErrorResponse(code=code, message="boom", details=None),
+    )
+
+
+@pytest.mark.asyncio
+async def test_mcp_call_tool_honors_page_param(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    recipes_dir = tmp_path / "recipes"
+    _write_recipe(recipes_dir, "demo")
+
+    seen_pages: list[int] = []
+
+    async def fake_scrape(
+        *,
+        pool: FakePool,
+        recipe,
+        endpoint: str,
+        page: int = 1,
+        query: str | None = None,
+        extra_params: dict[str, str] | None = None,
+        scrape_timeout: float = 30.0,
+    ) -> ApiResponse:
+        _ = pool, query, extra_params, scrape_timeout
+        seen_pages.append(page)
+        return _success_response(slug=recipe.config.slug, endpoint=endpoint, page=page)
+
+    monkeypatch.setattr("web2api.main.scrape", fake_scrape)
+
+    app = create_app(recipes_dir=recipes_dir, pool=FakePool())
+    async with app.router.lifespan_context(app):
+        transport = ASGITransport(app=app)
+        async with AsyncClient(transport=transport, base_url="http://testserver") as client:
+            resp = await client.post("/mcp/tools/demo_read", json={"page": 3})
+
+    assert resp.status_code == 200
+    assert seen_pages == [3]
+
+
+@pytest.mark.asyncio
+async def test_mcp_call_tool_error_uses_mapped_status_code(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    recipes_dir = tmp_path / "recipes"
+    _write_recipe(recipes_dir, "demo")
+
+    async def fake_scrape(
+        *,
+        pool: FakePool,
+        recipe,
+        endpoint: str,
+        page: int = 1,
+        query: str | None = None,
+        extra_params: dict[str, str] | None = None,
+        scrape_timeout: float = 30.0,
+    ) -> ApiResponse:
+        _ = pool, query, extra_params, scrape_timeout
+        return _error_response(
+            slug=recipe.config.slug,
+            endpoint=endpoint,
+            page=page,
+            code="INVALID_PARAMS",
+        )
+
+    monkeypatch.setattr("web2api.main.scrape", fake_scrape)
+
+    app = create_app(recipes_dir=recipes_dir, pool=FakePool())
+    async with app.router.lifespan_context(app):
+        transport = ASGITransport(app=app)
+        async with AsyncClient(transport=transport, base_url="http://testserver") as client:
+            resp = await client.post("/mcp/tools/demo_read", json={"q": "x"})
+
+    assert resp.status_code == 400
+    assert resp.json()["result"] == "Error: boom"
+
+
+@pytest.mark.asyncio
+async def test_mcp_call_tool_filtered_enforces_exclude(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    recipes_dir = tmp_path / "recipes"
+    _write_recipe(recipes_dir, "demo")
+
+    called = False
+
+    async def fake_scrape(
+        *,
+        pool: FakePool,
+        recipe,
+        endpoint: str,
+        page: int = 1,
+        query: str | None = None,
+        extra_params: dict[str, str] | None = None,
+        scrape_timeout: float = 30.0,
+    ) -> ApiResponse:
+        _ = pool, query, extra_params, scrape_timeout
+        nonlocal called
+        called = True
+        return _success_response(slug=recipe.config.slug, endpoint=endpoint, page=page)
+
+    monkeypatch.setattr("web2api.main.scrape", fake_scrape)
+
+    app = create_app(recipes_dir=recipes_dir, pool=FakePool())
+    async with app.router.lifespan_context(app):
+        transport = ASGITransport(app=app)
+        async with AsyncClient(transport=transport, base_url="http://testserver") as client:
+            resp = await client.post(
+                "/mcp/exclude/demo/tools/demo_read",
+                json={"q": "x"},
+            )
+
+    assert resp.status_code == 404
+    assert called is False
+
+
+@pytest.mark.asyncio
+async def test_mcp_call_tool_filtered_enforces_only(
+    tmp_path: Path,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    recipes_dir = tmp_path / "recipes"
+    _write_recipe(recipes_dir, "demo")
+
+    seen_pages: list[int] = []
+
+    async def fake_scrape(
+        *,
+        pool: FakePool,
+        recipe,
+        endpoint: str,
+        page: int = 1,
+        query: str | None = None,
+        extra_params: dict[str, str] | None = None,
+        scrape_timeout: float = 30.0,
+    ) -> ApiResponse:
+        _ = pool, query, extra_params, scrape_timeout
+        seen_pages.append(page)
+        return _success_response(slug=recipe.config.slug, endpoint=endpoint, page=page)
+
+    monkeypatch.setattr("web2api.main.scrape", fake_scrape)
+
+    app = create_app(recipes_dir=recipes_dir, pool=FakePool())
+    async with app.router.lifespan_context(app):
+        transport = ASGITransport(app=app)
+        async with AsyncClient(transport=transport, base_url="http://testserver") as client:
+            blocked = await client.post(
+                "/mcp/only/other/tools/demo_read",
+                json={"q": "x"},
+            )
+            allowed = await client.post(
+                "/mcp/only/demo/tools/demo_read",
+                json={"page": 2},
+            )
+
+    assert blocked.status_code == 404
+    assert allowed.status_code == 200
+    assert seen_pages == [2]

tests/unit/test_mcp_bridge.py

@@ -0,0 +1,79 @@
+"""Unit tests for MCP HTTP bridge tool resolution."""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+from web2api.config import RecipeConfig
+from web2api.mcp_bridge import _resolve_tool
+from web2api.registry import Recipe
+
+
+class _FakeRegistry:
+    """Minimal registry stub exposing ``list_all``."""
+
+    def __init__(self, recipes: list[Recipe]) -> None:
+        self._recipes = recipes
+
+    def list_all(self) -> list[Recipe]:
+        return self._recipes
+
+
+def _endpoint(**overrides: object) -> dict[str, object]:
+    endpoint: dict[str, object] = {
+        "url": "https://example.com/items?page={page}",
+        "items": {"container": ".item", "fields": {"title": {"selector": ".title"}}},
+        "pagination": {"type": "page_param", "param": "page"},
+    }
+    endpoint.update(overrides)
+    return endpoint
+
+
+def _recipe(slug: str, endpoints: dict[str, object]) -> Recipe:
+    config = RecipeConfig.model_validate(
+        {
+            "name": slug.title(),
+            "slug": slug,
+            "base_url": "https://example.com",
+            "description": "fixture recipe",
+            "endpoints": endpoints,
+        }
+    )
+    return Recipe(config=config, scraper=None, path=Path(f"recipes/{slug}"))
+
+
+def test_resolve_tool_standard_naming() -> None:
+    registry = _FakeRegistry([_recipe("demo", {"read": _endpoint()})])
+
+    assert _resolve_tool(registry, "demo__read") == ("demo", "read")
+
+
+def test_resolve_tool_legacy_single_underscore_name() -> None:
+    registry = _FakeRegistry([_recipe("demo", {"read": _endpoint()})])
+
+    assert _resolve_tool(registry, "demo_read") == ("demo", "read")
+
+
+def test_resolve_tool_handles_underscores_in_slug_and_endpoint() -> None:
+    registry = _FakeRegistry([_recipe("my_site", {"deep_read": _endpoint()})])
+
+    assert _resolve_tool(registry, "my_site__deep_read") == ("my_site", "deep_read")
+
+
+def test_resolve_tool_handles_double_underscores_in_slug() -> None:
+    registry = _FakeRegistry([_recipe("my__site", {"read": _endpoint()})])
+
+    # Splitting on "__" would mis-parse this as ("my", "site__read").
+    assert _resolve_tool(registry, "my__site__read") == ("my__site", "read")
+
+
+def test_resolve_tool_custom_tool_name_override() -> None:
+    registry = _FakeRegistry([_recipe("allenai", {"molmo2": _endpoint(tool_name="molmo2_vision")})])
+
+    assert _resolve_tool(registry, "molmo2_vision") == ("allenai", "molmo2")
+
+
+def test_resolve_tool_unknown_name_returns_none() -> None:
+    registry = _FakeRegistry([_recipe("demo", {"read": _endpoint()})])
+
+    assert _resolve_tool(registry, "missing_tool") == (None, None)

tests/unit/test_schemas.py

@@ -0,0 +1,35 @@
+"""Unit tests for unified schema helpers."""
+
+from __future__ import annotations
+
+import pytest
+
+from web2api.schemas import ErrorResponse, status_code_for_error
+
+
+def _error(code: str) -> ErrorResponse:
+    return ErrorResponse(code=code, message="boom", details=None)
+
+
+def test_status_code_for_error_returns_200_when_no_error() -> None:
+    assert status_code_for_error(None) == 200
+
+
+@pytest.mark.parametrize(
+    ("code", "expected"),
+    [
+        ("SITE_NOT_FOUND", 404),
+        ("CAPABILITY_NOT_SUPPORTED", 400),
+        ("INVALID_PARAMS", 400),
+        ("SCRAPE_FAILED", 502),
+        ("SCRAPE_TIMEOUT", 504),
+        ("INTERNAL_ERROR", 500),
+    ],
+)
+def test_status_code_mapping(code: str, expected: int) -> None:
+    assert status_code_for_error(_error(code)) == expected
+
+
+def test_status_code_defaults_to_500_for_unknown_code() -> None:
+    error = ErrorResponse.model_construct(code="UNKNOWN_CODE", message="boom", details=None)
+    assert status_code_for_error(error) == 500