Merge pull request #883 from cbcoutinho/worktree-tender-stargazing-sundae

test: Pact consumer contract for astrolabe credentials status (ADR-029)

tests/contract/__init__.py

@@ -0,0 +1,12 @@
+"""Pact consumer/provider contract tests (ADR-029).
+
+This package contains the Pact contract tests that keep ``nextcloud-mcp-server``
+and the ``astrolabe`` Nextcloud app wire-compatible:
+
+- **Consumer** (this server -> astrolabe): ``test_astrolabe_credentials_consumer``
+  generates a pact for the background-sync credentials API that the
+  ``AstrolabeClient`` consumes.
+- **Provider** (astrolabe -> this server): ``test_mcp_provider_verification``
+  verifies the ``/api/v1/*`` HTTP API this server exposes against pacts that
+  the astrolabe app publishes to the broker.
+"""

tests/contract/conftest.py

@@ -0,0 +1,42 @@
+"""Fixtures shared by the Pact contract tests (ADR-029).
+
+The consumer tests build a fresh ``Pact`` per test and merge their interaction
+into a single pact file under ``tests/contract/pacts/``. A session-scoped
+autouse fixture wipes that directory once at the start of a run so merged pacts
+never accumulate stale interactions across runs (``write_file(overwrite=False)``
+merges into whatever is already on disk).
+"""
+
+import shutil
+from pathlib import Path
+
+import pytest
+from pact import Pact
+
+# Pact participant names. These MUST match the names used on the astrolabe side
+# and in the broker, so keep them in sync with the astrolabe repo's pact tests.
+CONSUMER = "nextcloud-mcp-server"
+PROVIDER = "astrolabe"
+
+PACT_DIR = Path(__file__).parent / "pacts"
+
+
+@pytest.fixture(scope="session", autouse=True)
+def _clean_pact_dir():
+    """Start each session from an empty pacts directory."""
+    if PACT_DIR.exists():
+        shutil.rmtree(PACT_DIR)
+    PACT_DIR.mkdir(parents=True, exist_ok=True)
+    yield
+
+
+@pytest.fixture
+def consumer_pact():
+    """A fresh Pact (consumer=nextcloud-mcp-server, provider=astrolabe).
+
+    The interaction added by each test is merged into the shared pact file on
+    teardown.
+    """
+    pact = Pact(CONSUMER, PROVIDER).with_specification("V4")
+    yield pact
+    pact.write_file(PACT_DIR, overwrite=False)

tests/contract/test_astrolabe_credentials_consumer.py

@@ -0,0 +1,106 @@
+"""Consumer contract: nextcloud-mcp-server -> astrolabe credentials status API.
+
+The MCP server checks a user's background-sync provisioning status via
+:meth:`AstrolabeClient.get_background_sync_status`, which calls astrolabe's
+admin credentials-metadata endpoint
+(``GET /apps/astrolabe/api/v1/background-sync/credentials/{user_id}``). That
+endpoint returns **presence/timestamps only — never the app password itself**
+(the password reaches the MCP server out-of-band, pushed by astrolabe to
+``POST /api/v1/users/{user_id}/app-password``).
+
+This pact pins the request shape and the two states the consumer branches on:
+
+- provisioned   -> ``has_background_access: true``,  ``sync_type: "app_password"``
+- unprovisioned -> ``has_background_access: false``, ``sync_type: null``
+
+The OAuth token fetch (:meth:`AstrolabeClient.get_access_token`) is stubbed so
+only the status call hits the Pact mock server.
+
+The client's 404 branch is internal defensive handling (astrolabe always returns
+200 for this endpoint), not a contract obligation, so it is covered by a unit
+test (``tests/unit/test_astrolabe_client.py``) rather than a pact interaction.
+
+See ADR-029 for the overall contract-testing architecture.
+"""
+
+import pytest
+from pact import match
+
+from nextcloud_mcp_server.auth.astrolabe_client import AstrolabeClient
+
+pytestmark = pytest.mark.contract
+
+# Matches the ``Authorization: Bearer <token>`` header the client always sends.
+_BEARER = match.regex("Bearer test-token", regex=r"Bearer .+")
+
+
+async def test_status_reports_access_for_provisioned_user(consumer_pact, mocker):
+    """A provisioned user is reported as having background access."""
+    (
+        consumer_pact.upon_receiving(
+            "a request for a provisioned user's background-sync status"
+        )
+        .given("user alice has provisioned background-sync credentials")
+        .with_request("GET", "/apps/astrolabe/api/v1/background-sync/credentials/alice")
+        .with_header("Authorization", _BEARER)
+        .will_respond_with(200)
+        .with_body(
+            {
+                "success": True,
+                "user_id": "alice",
+                "has_background_access": True,
+                "sync_type": "app_password",
+                # Unix seconds (astrolabe BackgroundSyncCredentialStorage::getProvisionedAt),
+                # not an ISO string.
+                "provisioned_at": match.integer(1717000000),
+            },
+            content_type="application/json",
+        )
+    )
+
+    with consumer_pact.serve() as srv:
+        client = AstrolabeClient(
+            nextcloud_host=str(srv.url), client_id="mcp", client_secret="secret"
+        )
+        mocker.patch.object(client, "get_access_token", return_value="test-token")
+
+        status = await client.get_background_sync_status("alice")
+
+    assert status["has_access"] is True
+    assert status["credential_type"] == "app_password"
+    assert status["provisioned_at"] == 1717000000
+
+
+async def test_status_reports_no_access_for_unprovisioned_user(consumer_pact, mocker):
+    """An unprovisioned user is reported as having no background access."""
+    (
+        consumer_pact.upon_receiving(
+            "a request for an unprovisioned user's background-sync status"
+        )
+        .given("user bob has no background-sync credentials")
+        .with_request("GET", "/apps/astrolabe/api/v1/background-sync/credentials/bob")
+        .with_header("Authorization", _BEARER)
+        .will_respond_with(200)
+        .with_body(
+            {
+                "success": True,
+                "user_id": "bob",
+                "has_background_access": False,
+                "sync_type": None,
+                "provisioned_at": None,
+            },
+            content_type="application/json",
+        )
+    )
+
+    with consumer_pact.serve() as srv:
+        client = AstrolabeClient(
+            nextcloud_host=str(srv.url), client_id="mcp", client_secret="secret"
+        )
+        mocker.patch.object(client, "get_access_token", return_value="test-token")
+
+        status = await client.get_background_sync_status("bob")
+
+    assert status["has_access"] is False
+    assert status["credential_type"] is None
+    assert status["provisioned_at"] is None

tests/contract/test_mcp_provider_verification.py

@@ -0,0 +1,121 @@
+"""Provider verification: astrolabe -> nextcloud-mcp-server /api/v1 API.
+
+The astrolabe Nextcloud app consumes this server's ``/api/v1/*`` HTTP API
+(``lib/Service/McpServerClient.php``: ``search``, ``webhooks`` CRUD, ``apps``,
+``status``, ``vector-sync/status``, ``chunk-context``, ``pdf-preview``,
+``vector-viz/search``). This test plays the **provider** role: it pulls the
+pacts astrolabe published to the broker and replays each interaction against a
+running MCP server, failing if a response no longer matches the contract.
+
+It is **environment-gated** and skips unless a running provider and a pact
+source are configured, so it is a no-op in the consumer-only job and in local
+unit runs. Wire it into CI against the integration docker stack (see
+``.github/workflows/pact.yml``).
+
+Required environment:
+- ``PACT_PROVIDER_URL`` — base URL of the running MCP server to verify against
+  (e.g. ``http://localhost:8000``).
+- One pact source, either:
+  - ``PACT_BROKER`` (+ ``PACT_USERNAME`` / ``PACT_PASSWORD``) — verify against
+    pacts in the broker, or
+  - ``PACT_PROVIDER_PACT_DIR`` — verify against a local directory of pacts.
+
+Optional:
+- ``PACT_PROVIDER_VERSION`` — provider version (git SHA) to publish results under.
+- ``PACT_PROVIDER_BRANCH`` — provider branch for the published results.
+- ``PACT_PUBLISH_RESULTS=true`` — publish verification results to the broker.
+
+See ADR-029.
+"""
+
+import logging
+import os
+from collections.abc import Callable
+
+import pytest
+from pact import Verifier
+
+logger = logging.getLogger(__name__)
+
+PROVIDER_NAME = "nextcloud-mcp-server"
+
+_PROVIDER_URL = os.environ.get("PACT_PROVIDER_URL")
+_BROKER_URL = os.environ.get("PACT_BROKER")
+_BROKER_USERNAME = os.environ.get("PACT_USERNAME")
+_BROKER_PASSWORD = os.environ.get("PACT_PASSWORD")
+_LOCAL_PACT_DIR = os.environ.get("PACT_PROVIDER_PACT_DIR")
+
+# A usable broker source needs the URL *and* its basic-auth credentials; gating
+# on all three keeps a misconfigured CI (broker set, creds missing) a clean skip
+# rather than a confusing KeyError at verify time.
+_BROKER_READY = bool(_BROKER_URL and _BROKER_USERNAME and _BROKER_PASSWORD)
+
+# Skip the whole module unless we have a provider to hit AND a pact source.
+pytestmark = [
+    pytest.mark.contract,
+    pytest.mark.skipif(
+        not _PROVIDER_URL or not (_BROKER_READY or _LOCAL_PACT_DIR),
+        reason=(
+            "Provider verification needs PACT_PROVIDER_URL and a pact source: "
+            "PACT_BROKER (+ PACT_USERNAME/PACT_PASSWORD) or "
+            "PACT_PROVIDER_PACT_DIR. Skipped outside CI."
+        ),
+    ),
+]
+
+
+# Map astrolabe-side provider-state strings -> setup callables. astrolabe's
+# consumer pacts declare the ``given(...)`` provider states; add one handler per
+# state name here as those pacts are written (seeding webhooks DB, qdrant
+# fixtures, etc.). Keep the keys identical to the astrolabe ``given(...)``
+# strings. Unhandled states fall through to ``_dispatch_state`` which logs and
+# no-ops, so state-less interactions still verify.
+_PROVIDER_STATES: dict[str, Callable[[], None]] = {
+    # "a webhook is registered for user alice": _state_webhook_registered,
+    # "vector sync has indexed documents": _state_vector_sync_ran,
+    # "the search index returns a hit for 'budget'": _state_search_has_hit,
+}
+
+
+def _dispatch_state(state: str, **kwargs) -> None:
+    """Provider-state dispatcher passed to the verifier.
+
+    Looks up a registered handler by state name; logs and no-ops for unknown
+    states so contracts that don't require seeded state (``/api/v1/status``,
+    ``/api/v1/vector-sync/status``) verify without a handler.
+    """
+    handler = _PROVIDER_STATES.get(state)
+    if handler is None:
+        # Log any params astrolabe passed so they're visible once real handlers
+        # need them (e.g. given("user X exists", params={"user_id": ...})).
+        logger.warning(
+            "No provider-state handler registered for %r (params=%s); no-op",
+            state,
+            kwargs,
+        )
+        return
+    handler()
+
+
+def test_verify_astrolabe_consumer_pacts() -> None:
+    """Verify the MCP server honours every interaction astrolabe published."""
+    verifier = Verifier(PROVIDER_NAME).add_transport(url=_PROVIDER_URL)
+    verifier.state_handler(_dispatch_state, teardown=True)
+
+    if _BROKER_URL and _BROKER_USERNAME and _BROKER_PASSWORD:
+        verifier.broker_source(
+            _BROKER_URL, username=_BROKER_USERNAME, password=_BROKER_PASSWORD
+        )
+    else:
+        assert _LOCAL_PACT_DIR is not None  # guaranteed by module skipif
+        verifier.add_source(_LOCAL_PACT_DIR)
+
+    if os.environ.get("PACT_PUBLISH_RESULTS", "").lower() == "true":
+        version = os.environ.get("PACT_PROVIDER_VERSION", "dev")
+        verifier.set_publish_options(
+            version=version,
+            branch=os.environ.get("PACT_PROVIDER_BRANCH"),
+        )
+
+    # Raises (failing the test) if any interaction does not match.
+    verifier.verify()

tests/unit/test_astrolabe_client.py

@@ -0,0 +1,104 @@
+"""Unit tests for ``AstrolabeClient.get_background_sync_status`` (ADR-029).
+
+The Pact consumer contract (``tests/contract/``) pins the wire shape; these
+mocked tests pin the *field mapping* that the original silent bug got wrong —
+it read a non-existent ``app_password`` field, so ``has_access`` was always
+``False``. A regression here now fails fast at the unit layer.
+"""
+
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from nextcloud_mcp_server.auth.astrolabe_client import AstrolabeClient
+
+pytestmark = pytest.mark.unit
+
+
+def _patch_outbound_client(mocker, response: MagicMock) -> MagicMock:
+    """Patch the httpx client used by AstrolabeClient; return the mock client."""
+    mock_client = AsyncMock()
+    mock_client.get = AsyncMock(return_value=response)
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mocker.patch(
+        "nextcloud_mcp_server.auth.astrolabe_client.nextcloud_httpx_client",
+        MagicMock(return_value=mock_client),
+    )
+    return mock_client
+
+
+def _client(mocker) -> AstrolabeClient:
+    client = AstrolabeClient(
+        nextcloud_host="https://cloud.example.com",
+        client_id="mcp",
+        client_secret="secret",
+    )
+    mocker.patch.object(client, "get_access_token", AsyncMock(return_value="tok"))
+    return client
+
+
+async def test_provisioned_user_maps_status_fields(mocker):
+    """200 + has_background_access=True maps to has_access/credential_type/provisioned_at."""
+    response = MagicMock()
+    response.status_code = 200
+    response.json.return_value = {
+        "success": True,
+        "user_id": "alice",
+        "has_background_access": True,
+        "sync_type": "app_password",
+        "provisioned_at": 1717000000,
+    }
+    mock_client = _patch_outbound_client(mocker, response)
+    client = _client(mocker)
+
+    status = await client.get_background_sync_status("alice")
+
+    assert status == {
+        "has_access": True,
+        "credential_type": "app_password",
+        "provisioned_at": 1717000000,
+    }
+    response.raise_for_status.assert_called_once()
+    # The bearer token from get_access_token is forwarded on the request.
+    _, kwargs = mock_client.get.call_args
+    assert kwargs["headers"]["Authorization"] == "Bearer tok"
+
+
+async def test_provisioned_false_reports_no_access(mocker):
+    """200 + has_background_access=False reports no access (the bug's regression guard)."""
+    response = MagicMock()
+    response.status_code = 200
+    response.json.return_value = {
+        "success": True,
+        "user_id": "bob",
+        "has_background_access": False,
+        "sync_type": None,
+        "provisioned_at": None,
+    }
+    _patch_outbound_client(mocker, response)
+    client = _client(mocker)
+
+    status = await client.get_background_sync_status("bob")
+
+    assert status["has_access"] is False
+    assert status["credential_type"] is None
+    assert status["provisioned_at"] is None
+
+
+async def test_missing_credentials_404_returns_no_access(mocker):
+    """404 short-circuits to no-access without touching raise_for_status/json."""
+    response = MagicMock()
+    response.status_code = 404
+    _patch_outbound_client(mocker, response)
+    client = _client(mocker)
+
+    status = await client.get_background_sync_status("carol")
+
+    assert status == {
+        "has_access": False,
+        "credential_type": None,
+        "provisioned_at": None,
+    }
+    response.raise_for_status.assert_not_called()
+    response.json.assert_not_called()

tests/unit/test_oauth_tools_app_password_provisioning.py

@@ -8,6 +8,7 @@ read and clear that store too, otherwise they report "not provisioned" while
 tools still work, and "nothing to revoke" while the credential persists.
 """
 
+from datetime import datetime
 from types import SimpleNamespace
 from unittest.mock import AsyncMock, MagicMock
 
@@ -58,6 +59,39 @@ async def test_status_reports_provisioned_for_app_password_store(
     storage.get_refresh_token.assert_not_awaited()  # app password short-circuits
 
 
+async def test_status_converts_astrolabe_int_timestamp_to_iso(mocker):
+    """Astrolabe returns provisioned_at as Unix seconds (per the contract pact),
+    but ProvisioningStatus.provisioned_at is an ISO string. The int must be
+    converted at the boundary, else constructing the model raises ValidationError
+    for every provisioned user."""
+    mocker.patch.object(
+        oauth_tools,
+        "get_settings",
+        return_value=SimpleNamespace(
+            oidc_client_id="mcp",
+            oidc_client_secret="secret",
+            nextcloud_host="https://cloud.example.com",
+        ),
+    )
+    astrolabe = MagicMock()
+    astrolabe.get_background_sync_status = AsyncMock(
+        return_value={
+            "has_access": True,
+            "credential_type": "app_password",
+            "provisioned_at": 1717000000,
+        }
+    )
+    mocker.patch.object(oauth_tools, "AstrolabeClient", return_value=astrolabe)
+
+    status = await _get_provisioning_status(MagicMock(), "alice")
+
+    assert status.is_provisioned is True
+    assert status.credential_type == "app_password"
+    # Converted from Unix seconds to an ISO-8601 string that round-trips back.
+    assert isinstance(status.provisioned_at, str)
+    assert datetime.fromisoformat(status.provisioned_at).timestamp() == 1717000000
+
+
 async def test_revoke_deletes_app_password(mocker, _no_astrolabe_settings):
     """Revoke must delete the app password from storage (not just refresh tokens)."""
     storage = MagicMock()