From d33832aba9caadbc832f61e9786854c4dfe0ae24 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 20:22:07 +0200 Subject: [PATCH 1/5] test: add Pact consumer contract for astrolabe credentials status (ADR-029) Introduce consumer-driven contract testing between nextcloud-mcp-server and the astrolabe Nextcloud app, published to the homelab Pact Broker and verified in CI. - pact-python dev dep + `contract` pytest marker - tests/contract/test_astrolabe_credentials_consumer.py: consumer pact for the background-sync *status* call (provisioned -> has_background_access:true, sync_type:"app_password", integer provisioned_at; unprovisioned -> false/null) - tests/contract/test_mcp_provider_verification.py: env-gated Verifier harness for this server's /api/v1/* provider role (provider-state handlers stubbed pending astrolabe's published pacts) - .github/workflows/pact.yml: join tailnet -> publish pacts -> provider verify -> can-i-deploy; broker steps skip when PACT_BROKER is unset (forks) - docs/ADR-029-pact-contract-testing.md Fix astrolabe_client.get_background_sync_status: it previously read a non-existent `app_password` field (always reporting no-access). Rewrite it to read the real status contract (has_background_access / sync_type / provisioned_at) and drop the unsatisfiable get_user_app_password. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/pact.yml | 155 ++++++++++++++++++ .gitignore | 3 + docs/ADR-029-pact-contract-testing.md | 109 ++++++++++++ nextcloud_mcp_server/auth/astrolabe_client.py | 60 +++---- pyproject.toml | 2 + tests/contract/__init__.py | 12 ++ tests/contract/conftest.py | 48 ++++++ .../test_astrolabe_credentials_consumer.py | 102 ++++++++++++ .../test_mcp_provider_verification.py | 110 +++++++++++++ uv.lock | 35 ++++ 10 files changed, 603 insertions(+), 33 deletions(-) create mode 100644 .github/workflows/pact.yml create mode 100644 docs/ADR-029-pact-contract-testing.md create mode 100644 tests/contract/__init__.py create mode 100644 tests/contract/conftest.py create mode 100644 tests/contract/test_astrolabe_credentials_consumer.py create mode 100644 tests/contract/test_mcp_provider_verification.py diff --git a/.github/workflows/pact.yml b/.github/workflows/pact.yml new file mode 100644 index 00000000..228f8f8b --- /dev/null +++ b/.github/workflows/pact.yml @@ -0,0 +1,155 @@ +name: Pact contract tests + +# Consumer-driven contract testing against the homelab-hosted Pact Broker +# (ADR-029). The broker is only reachable over Tailscale, so every job that +# talks to it first joins the tailnet with the shared github-runner OAuth +# client. Jobs no-op when the broker secrets are absent (e.g. on forks). +# +# Required repo/org secrets: +# TS_OAUTH_CLIENT_ID / TS_OAUTH_SECRET - Tailscale github-runner OAuth client +# PACT_BROKER - broker base URL (https://pact-broker.internal.coutinho.io) +# PACT_USERNAME / PACT_PASSWORD - broker basic-auth credentials + +on: + pull_request: + branches: + - master + push: + branches: + - master + +concurrency: + group: pact-${{ github.ref }} + cancel-in-progress: true + +env: + PACT_BROKER: ${{ secrets.PACT_BROKER }} + PACT_USERNAME: ${{ secrets.PACT_USERNAME }} + PACT_PASSWORD: ${{ secrets.PACT_PASSWORD }} + +jobs: + consumer: + name: Consumer pacts (mcp -> astrolabe) + runs-on: ubuntu-latest + # Skip on forks / when broker is not configured. + if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }} + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Install the latest version of uv + uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + + - name: Generate consumer pacts + run: uv run pytest -v -m contract -o "addopts=-p no:asyncio" tests/contract/ + + # Only publish from non-fork builds that have the broker secrets. + - name: Join tailnet + if: ${{ env.PACT_BROKER != '' }} + uses: tailscale/github-action@v3 + with: + oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} + oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} + tags: tag:github-runner + + - name: Install Pact CLI + if: ${{ env.PACT_BROKER != '' }} + run: | + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/master/install.sh | bash + echo "$PWD/pact/bin" >> "$GITHUB_PATH" + + - name: Publish pacts to broker + if: ${{ env.PACT_BROKER != '' }} + run: | + pact-broker publish tests/contract/pacts \ + --broker-base-url "$PACT_BROKER" \ + --broker-username "$PACT_USERNAME" \ + --broker-password "$PACT_PASSWORD" \ + --consumer-app-version "${{ github.sha }}" \ + --branch "${{ github.head_ref || github.ref_name }}" + + provider: + name: Provider verification (astrolabe -> mcp) + runs-on: ubuntu-latest + if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }} + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - name: Install the latest version of uv + uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + + # Stand up the MCP server. NOTE: single-user only exposes the *public* + # management endpoints (/api/v1/status, /api/v1/vector-sync/status) — see + # app.py:2248. The authenticated surface astrolabe also consumes + # (/api/v1/search, /webhooks, /apps, /chunk-context, /pdf-preview) needs + # an OAuth-capable profile (login-flow) plus Bearer-token injection into + # the verifier (Verifier.add_custom_header). That is the phase-4 follow-up; + # this job currently verifies the public-endpoint pacts. + - name: Generate ephemeral TOKEN_ENCRYPTION_KEY + run: | + KEY=$(openssl rand -base64 32 | tr '+/' '-_') + echo "TOKEN_ENCRYPTION_KEY=${KEY}" >> "$GITHUB_ENV" + + - name: Start MCP server + uses: hoverkraft-tech/compose-action@11beaa1c2dae4e8ed7b1665aa074723b6cecb0e4 # v3.0.0 + with: + compose-file: "./docker-compose.yml" + compose-flags: "--profile single-user" + up-flags: "--build" + env: + TOKEN_ENCRYPTION_KEY: ${{ env.TOKEN_ENCRYPTION_KEY }} + + - name: Wait for MCP server + run: | + for i in $(seq 1 30); do + code=$(curl -o /dev/null -s -w "%{http_code}" http://localhost:8000/api/v1/status || true) + [ "$code" = "200" ] && echo "ready" && exit 0 + sleep 5 + done + docker compose --profile single-user logs mcp + exit 1 + + - name: Join tailnet + if: ${{ env.PACT_BROKER != '' }} + uses: tailscale/github-action@v3 + with: + oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} + oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} + tags: tag:github-runner + + - name: Verify provider against broker pacts + if: ${{ env.PACT_BROKER != '' }} + env: + PACT_PROVIDER_URL: http://localhost:8000 + PACT_PROVIDER_VERSION: ${{ github.sha }} + PACT_PROVIDER_BRANCH: ${{ github.head_ref || github.ref_name }} + # Publish results only from master so PR runs don't pollute the matrix. + PACT_PUBLISH_RESULTS: ${{ github.ref == 'refs/heads/master' }} + run: uv run pytest -v -m contract -o "addopts=-p no:asyncio" tests/contract/test_mcp_provider_verification.py + + can-i-deploy: + name: can-i-deploy + runs-on: ubuntu-latest + needs: [consumer, provider] + if: ${{ github.ref == 'refs/heads/master' }} + steps: + - name: Join tailnet + uses: tailscale/github-action@v3 + with: + oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} + oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} + tags: tag:github-runner + + - name: Install Pact CLI + run: | + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/master/install.sh | bash + echo "$PWD/pact/bin" >> "$GITHUB_PATH" + + - name: Can I deploy nextcloud-mcp-server? + run: | + pact-broker can-i-deploy \ + --broker-base-url "$PACT_BROKER" \ + --broker-username "$PACT_USERNAME" \ + --broker-password "$PACT_PASSWORD" \ + --pacticipant nextcloud-mcp-server \ + --version "${{ github.sha }}" \ + --to-environment production diff --git a/.gitignore b/.gitignore index 469e4d90..fadb5b90 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,6 @@ docker-compose.override.yml # RAG Evaluation tests/rag_evaluation/fixtures/ + +# Pact contract tests — generated pacts are published to the broker, not committed +tests/contract/pacts/ diff --git a/docs/ADR-029-pact-contract-testing.md b/docs/ADR-029-pact-contract-testing.md new file mode 100644 index 00000000..7e9ae52b --- /dev/null +++ b/docs/ADR-029-pact-contract-testing.md @@ -0,0 +1,109 @@ +# ADR-029: Pact contract testing with astrolabe + +## Status + +Accepted — 2026-06-10 + +## Context + +`nextcloud-mcp-server` (Python/FastMCP) and the `astrolabe` Nextcloud app (PHP) +integrate over HTTP **in both directions**: + +- **MCP server → astrolabe** (one call): the background vector-sync reads a + user's provisioning **status** via + `GET /apps/astrolabe/api/v1/background-sync/credentials/{user_id}` + (`nextcloud_mcp_server/auth/astrolabe_client.py`). This returns status only — + `{success, user_id, has_background_access, sync_type, provisioned_at}`, always + HTTP 200, and **never the password**. The app password itself flows the other + way: astrolabe pushes it to the MCP server's `/api/v1/users/{uid}/app-password`. +- **astrolabe → MCP server** (~10 calls): astrolabe's `McpServerClient` consumes + this server's `/api/v1/*` HTTP API — `status`, `vector-sync/status`, `search`, + `vector-viz/search`, `webhooks` (GET/POST/DELETE), `apps`, `chunk-context`, + `pdf-preview` (`nextcloud_mcp_server/app.py` route table, `api/webhooks.py`). + +Today nothing guarantees the two stay wire-compatible. A renamed JSON field or a +changed status code on either side is only caught — if at all — by the heavy +integration matrix (docker-compose, real Nextcloud, minutes per run). We want a +fast, focused signal that fails the moment the contract drifts, runnable in +unit-style CI on either repo independently. + +## Decision + +Adopt **consumer-driven contract testing with Pact**, with a self-hosted **Pact +Broker in the homelab** as the system of record. GitHub Actions reach the broker +**over Tailscale** (the broker is not publicly exposed). + +Because the integration is bidirectional, each repo plays **both** Pact roles: + +| Contract | Consumer | Provider | Consumer tooling | Provider tooling | +|----------|----------|----------|------------------|------------------| +| credentials API | nextcloud-mcp-server | astrolabe | `pact-python` | `pact-php` verifier | +| `/api/v1/*` API | astrolabe | nextcloud-mcp-server | `pact-php` | `pact-python` `Verifier` | + +### This repo (Python side) + +- `pact-python` (v3 API, `from pact import Pact, Verifier, match`) is a **dev** + dependency. +- Contract tests live under `tests/contract/` behind a `contract` pytest marker, + kept out of the default `unit` run. + - **Consumer** (`test_astrolabe_credentials_consumer.py`): drives the real + `AstrolabeClient` against a Pact mock server, pinning the request shape and + the two status responses it branches on — provisioned + (`has_background_access: true`, `sync_type: "app_password"`, integer + `provisioned_at`) and unprovisioned (`has_background_access: false`). The + OAuth token fetch is stubbed so only the credentials call is exercised. + Interactions merge into `tests/contract/pacts/` (git-ignored — pacts are + published to the broker, not committed). + - **Provider** (`test_mcp_provider_verification.py`): a `Verifier` harness that + replays astrolabe's published pacts against a running MCP server. It is + **environment-gated** (`PACT_PROVIDER_URL` + a pact source) so it skips in + the consumer-only job and in local runs. Provider-state handlers are + registered by `given(...)` string in `_PROVIDER_STATES` and filled in as + astrolabe publishes its consumer pacts; unknown states no-op so state-less + interactions (`/api/v1/status`, `/api/v1/vector-sync/status`) verify + immediately. + +### CI (`.github/workflows/pact.yml`) + +Three jobs, each joining the tailnet with the shared `tag:github-runner` OAuth +client before touching the broker: + +1. **consumer** — generate pacts, publish to the broker tagged with the branch + and commit SHA. +2. **provider** — stand up the MCP server (single-user compose profile), verify + astrolabe's pacts against it, publish verification results (master only). +3. **can-i-deploy** — gate `master` on `pact-broker can-i-deploy … --pacticipant + nextcloud-mcp-server --to-environment production`. + +Broker-dependent steps are skipped when `PACT_BROKER` is unset (forks). + +### Infrastructure (other repos / sessions) + +- **Broker**: `pactfoundation/pact-broker` deployed via ArgoCD + (`homelab-argocd`), backed by the shared Zalando Postgres, reachable at + `pact-broker.internal.coutinho.io`. Single basic-auth credential from AWS + Secrets Manager via external-secrets. +- **CI access**: jobs join the tailnet (`tag:github-runner`) and reach the + broker via the `PACT_BROKER` secret (its Tailscale host). No new Terraform is + required. + +### Required secrets (this repo) + +`TS_OAUTH_CLIENT_ID`, `TS_OAUTH_SECRET` (Tailscale github-runner OAuth client), +`PACT_BROKER` (base URL), `PACT_USERNAME`, `PACT_PASSWORD` (broker basic auth). + +## Consequences + +- **Fast, independent signal**: either side detects an incompatible change in a + ~seconds-long job instead of waiting on the integration matrix. +- **Participant names are load-bearing**: the consumer/provider names + (`nextcloud-mcp-server`, `astrolabe`) must match exactly across both repos and + the broker. They live in `tests/contract/conftest.py` here and must mirror the + astrolabe pact tests. +- **Provider states are deferred work**: the `/api/v1/*` provider verification is + only as complete as the state handlers that seed its backends (webhooks DB, + Qdrant). These land incrementally as astrolabe publishes its consumer pacts. +- **Broker is a homelab dependency**: contract publication/verification needs the + tailnet and the broker up. Steps degrade to skipped (not failed) when the + broker is unreachable from a fork; on `master` an outage will fail + `can-i-deploy`. diff --git a/nextcloud_mcp_server/auth/astrolabe_client.py b/nextcloud_mcp_server/auth/astrolabe_client.py index 976290d5..8c316da8 100644 --- a/nextcloud_mcp_server/auth/astrolabe_client.py +++ b/nextcloud_mcp_server/auth/astrolabe_client.py @@ -91,24 +91,32 @@ class AstrolabeClient: logger.info("Obtained Astrolabe API token (expires in %ss)", expires_in) return data["access_token"] - async def get_user_app_password(self, user_id: str) -> Optional[str]: + async def get_background_sync_status(self, user_id: str) -> dict: """ - Retrieve user's app password for background sync. + Get background sync provisioning status for a user. + + Queries Astrolabe's admin credentials-metadata endpoint, which returns + presence/timestamps only — never the app password itself. The password + is delivered to the MCP server out-of-band (Astrolabe pushes it to + ``POST /api/v1/users/{user_id}/app-password``), so this endpoint exposes + only ``has_background_access`` / ``sync_type`` / ``provisioned_at``. Args: user_id: Nextcloud user ID Returns: - App password string, or None if user hasn't provisioned + Dict with keys: has_access (bool), credential_type (str | None), + provisioned_at (int | None) — Unix seconds, not an ISO string. Raises: - httpx.HTTPError: If API request fails (except 404) + httpx.HTTPError: If the API request fails (except 404, treated as + "not provisioned"). """ token = await self.get_access_token() url = f"{self.nextcloud_host}/apps/astrolabe/api/v1/background-sync/credentials/{user_id}" async with nextcloud_httpx_client() as client: - logger.debug("Retrieving app password for user: %s", user_id) + logger.debug("Fetching background-sync status for user: %s", user_id) response = await client.get( url, @@ -117,38 +125,24 @@ class AstrolabeClient: ) if response.status_code == 404: - logger.debug("No app password configured for user: %s", user_id) - return None + logger.debug("No background-sync credentials for user: %s", user_id) + return { + "has_access": False, + "credential_type": None, + "provisioned_at": None, + } response.raise_for_status() data = response.json() + has_access = bool(data.get("has_background_access")) logger.info( - "Retrieved app password for user: %s (type: %s)", + "Background-sync status for user %s: has_access=%s", user_id, - data.get("credential_type"), + has_access, ) - return data.get("app_password") - - async def get_background_sync_status(self, user_id: str) -> dict: - """ - Get background sync status for a user. - - Args: - user_id: Nextcloud user ID - - Returns: - Dict with keys: has_access, credential_type, provisioned_at - - Raises: - httpx.HTTPError: If API request fails - """ - # For now, check if app password exists - # In the future, this could query a dedicated status endpoint - app_password = await self.get_user_app_password(user_id) - - return { - "has_access": app_password is not None, - "credential_type": "app_password" if app_password else None, - "provisioned_at": None, # TODO: Get from API if available - } + return { + "has_access": has_access, + "credential_type": data.get("sync_type"), + "provisioned_at": data.get("provisioned_at"), + } diff --git a/pyproject.toml b/pyproject.toml index ffc69c34..efafdb77 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,6 +81,7 @@ markers = [ "login_flow: Login Flow v2 integration tests (ADR-022)", "multi_user_basic: Multi-user BasicAuth pass-through tests (ADR-020)", "postgres: Tests requiring the docker-compose postgres-test service (ADR-026)", + "contract: Pact consumer/provider contract tests (ADR-029)", ] testpaths = [ "tests", @@ -128,6 +129,7 @@ dev = [ "pytest-otel>=2.0.1", "procrastinate>=3.8", "psycopg[binary,pool]>=3.2", + "pact-python>=3.4.0", ] [project.scripts] diff --git a/tests/contract/__init__.py b/tests/contract/__init__.py new file mode 100644 index 00000000..8df19182 --- /dev/null +++ b/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. +""" diff --git a/tests/contract/conftest.py b/tests/contract/conftest.py new file mode 100644 index 00000000..639875ce --- /dev/null +++ b/tests/contract/conftest.py @@ -0,0 +1,48 @@ +"""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 pact_dir() -> Path: + """Directory the generated pact files are written to.""" + return PACT_DIR + + +@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) diff --git a/tests/contract/test_astrolabe_credentials_consumer.py b/tests/contract/test_astrolabe_credentials_consumer.py new file mode 100644 index 00000000..b0788758 --- /dev/null +++ b/tests/contract/test_astrolabe_credentials_consumer.py @@ -0,0 +1,102 @@ +"""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. + +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 `` 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 diff --git a/tests/contract/test_mcp_provider_verification.py b/tests/contract/test_mcp_provider_verification.py new file mode 100644 index 00000000..19bfd85e --- /dev/null +++ b/tests/contract/test_mcp_provider_verification.py @@ -0,0 +1,110 @@ +"""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 + +pytestmark = pytest.mark.contract + +logger = logging.getLogger(__name__) + +PROVIDER_NAME = "nextcloud-mcp-server" + +_PROVIDER_URL = os.environ.get("PACT_PROVIDER_URL") +_BROKER_URL = os.environ.get("PACT_BROKER") +_LOCAL_PACT_DIR = os.environ.get("PACT_PROVIDER_PACT_DIR") + +# 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_URL or _LOCAL_PACT_DIR), + reason=( + "Provider verification needs PACT_PROVIDER_URL and a pact source " + "(PACT_BROKER 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: + logger.warning("No provider-state handler registered for %r; no-op", state) + 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: + # Basic-auth creds accompany the broker URL (see module skipif). + username = os.environ["PACT_USERNAME"] + password = os.environ["PACT_PASSWORD"] + verifier.broker_source(_BROKER_URL, username=username, password=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() diff --git a/uv.lock b/uv.lock index b245fba6..070e9d02 100644 --- a/uv.lock +++ b/uv.lock @@ -2236,6 +2236,7 @@ dev = [ { name = "commitizen" }, { name = "datasets" }, { name = "ipython" }, + { name = "pact-python" }, { name = "playwright" }, { name = "procrastinate" }, { name = "psycopg", extra = ["binary", "pool"] }, @@ -2299,6 +2300,7 @@ dev = [ { name = "commitizen", specifier = ">=4.8.2" }, { name = "datasets", specifier = ">=3.3.0" }, { name = "ipython", specifier = ">=9.2.0" }, + { name = "pact-python", specifier = ">=3.4.0" }, { name = "playwright", specifier = ">=1.49.1" }, { name = "procrastinate", specifier = ">=3.8" }, { name = "psycopg", extras = ["binary", "pool"], specifier = ">=3.2" }, @@ -2718,6 +2720,39 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/20/12/38679034af332785aac8774540895e234f4d07f7545804097de4b666afd8/packaging-25.0-py3-none-any.whl", hash = "sha256:29572ef2b1f17581046b3a2227d5c611fb25ec70ca1ba8554b24b0e69331a484", size = 66469, upload-time = "2025-04-19T11:48:57.875Z" }, ] +[[package]] +name = "pact-python" +version = "3.4.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pact-python-ffi" }, + { name = "typing-extensions", marker = "python_full_version < '3.13'" }, + { name = "yarl" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/41/d0/0f7b44586c232e86383a6224eb9f6bfb511d2c9630b692312506a879ac40/pact_python-3.4.0.tar.gz", hash = "sha256:e4a500fa00e1fd586cecf473a74db1e604af5e1be97a312d88eced46c25395c0", size = 82088, upload-time = "2026-05-04T09:40:06.947Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/3a/63/fb5c76d6c23504d5838f4da4219c81399b68211735939d1657ad568a016b/pact_python-3.4.0-py3-none-any.whl", hash = "sha256:76e8fb55c403c2c87d45052b270535cd88af2cd4b60d7d17ae5714eb9d8a872a", size = 97691, upload-time = "2026-05-04T09:40:05.502Z" }, +] + +[[package]] +name = "pact-python-ffi" +version = "0.5.4.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cffi" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/a0/85/199a6d800563bbf6402b93d29b10542d2b0b70a07aa6e7a6c7b7382b48ac/pact_python_ffi-0.5.4.0.tar.gz", hash = "sha256:dd0d4eeab8fc183800bce0f499a90d1b8306b2f3122f95ab0c34d6dd1f066986", size = 43703, upload-time = "2026-05-04T09:40:05.348Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ed/7c/4c09c720a7b1c5842c7c4ffb6442170b901074fb1617227851014f6a5994/pact_python_ffi-0.5.4.0-cp310-abi3-macosx_12_0_arm64.whl", hash = "sha256:64ca6ff1b6054cc233e9543e6976609ad32e415ade993855cf4f167741c2b753", size = 5476847, upload-time = "2026-05-04T09:39:48.055Z" }, + { url = "https://files.pythonhosted.org/packages/c4/60/cb32cd3473af25bcd8766644bb98995916eed5a0928cd35ba1bc2da6423a/pact_python_ffi-0.5.4.0-cp310-abi3-macosx_12_0_x86_64.whl", hash = "sha256:7b90c0b5b17b70852b17fa1321d46a3272c9fd7fae7cf6dff2f3610dd6b30251", size = 5662311, upload-time = "2026-05-04T09:39:50.619Z" }, + { url = "https://files.pythonhosted.org/packages/ad/a9/e0aceaa45a0b58a57d4bb0319d841f1215805f5e4775727a20d7dd1967a1/pact_python_ffi-0.5.4.0-cp310-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:42238b9c2f96e541733f766fb1975e63f90b71dad7353ae6b1d8ba972f69f424", size = 6392810, upload-time = "2026-05-04T09:39:52.562Z" }, + { url = "https://files.pythonhosted.org/packages/1a/14/1b898aca91cd2d59cc8ae8142f519df4131f250ca13edad678693d09e5a6/pact_python_ffi-0.5.4.0-cp310-abi3-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0f7b34209976695551bd630a642f775cfb7b4bd510a1f03a658ec701e2a620d3", size = 6616030, upload-time = "2026-05-04T09:39:54.08Z" }, + { url = "https://files.pythonhosted.org/packages/a0/7b/91e436dc1b5b12aacccb1c8b24b7453afe9e54b76d3e4d8d009d6774bc8c/pact_python_ffi-0.5.4.0-cp310-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:563ed5af2e61fae9d2f9d3704056f0ac93d581174815433ee94eff60d7d63fc5", size = 15034468, upload-time = "2026-05-04T09:39:56.283Z" }, + { url = "https://files.pythonhosted.org/packages/aa/14/6fb062da39c3277a28727a41eeea3f5f9f88ca332f867f850da20bd2d6d9/pact_python_ffi-0.5.4.0-cp310-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:3afddaf461df078f94256157993680ea2bcbc8be215499bd2f54c3aa2f54afd3", size = 14285546, upload-time = "2026-05-04T09:39:58.634Z" }, + { url = "https://files.pythonhosted.org/packages/4a/f4/25e676f4414102dc3852ccc0aab894e91e8a627b6cc463686523064b527c/pact_python_ffi-0.5.4.0-cp310-abi3-win_amd64.whl", hash = "sha256:192761cc8a668e08b189f517417cdb71b846da09b20812fac9d690d7b9a5b152", size = 13278456, upload-time = "2026-05-04T09:40:00.908Z" }, + { url = "https://files.pythonhosted.org/packages/3d/1a/bbdb8ddf844b58dc6cb76c8ab0e27f965e3633d5fbc956d5f76a2296a8c3/pact_python_ffi-0.5.4.0-cp310-abi3-win_arm64.whl", hash = "sha256:94876f6a5a19b2f66214dcdd793ce7c911b88649dfabb98fddfcb10e40fed740", size = 10945281, upload-time = "2026-05-04T09:40:03.278Z" }, +] + [[package]] name = "pandas" version = "2.3.3" From 72592c3bca8f86f0058fb2368297501765619e76 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 20:44:53 +0200 Subject: [PATCH 2/5] test: address claude-review on Pact consumer contract (#883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pact.yml: pin tailscale/github-action@v3 to commit SHA (3 jobs) and pact-ruby-standalone install.sh to v2.6.1 (2 jobs) — supply-chain hardening - pact.yml: drop redundant `-o "addopts=..."` override (pyproject.toml already sets the same addopts; the override would silently mask future additions) - test_mcp_provider_verification.py: remove dead `pytestmark` shadowed by the list assignment; gate the module skip on PACT_USERNAME/PACT_PASSWORD too so a broker-set-but-creds-missing CI skips cleanly instead of raising KeyError - conftest.py: drop the unused `pact_dir` fixture Co-Authored-By: Claude Opus 4.8 --- .github/workflows/pact.yml | 14 +++++------ tests/contract/conftest.py | 6 ----- .../test_mcp_provider_verification.py | 25 +++++++++++-------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/.github/workflows/pact.yml b/.github/workflows/pact.yml index 228f8f8b..3c88ba4e 100644 --- a/.github/workflows/pact.yml +++ b/.github/workflows/pact.yml @@ -40,12 +40,12 @@ jobs: uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 - name: Generate consumer pacts - run: uv run pytest -v -m contract -o "addopts=-p no:asyncio" tests/contract/ + run: uv run pytest -v -m contract tests/contract/ # Only publish from non-fork builds that have the broker secrets. - name: Join tailnet if: ${{ env.PACT_BROKER != '' }} - uses: tailscale/github-action@v3 + uses: tailscale/github-action@6cae46e2d796f265265cfcf628b72a32b4d7cade # v3 with: oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} @@ -54,7 +54,7 @@ jobs: - name: Install Pact CLI if: ${{ env.PACT_BROKER != '' }} run: | - curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/master/install.sh | bash + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/v2.6.1/install.sh | bash echo "$PWD/pact/bin" >> "$GITHUB_PATH" - name: Publish pacts to broker @@ -110,7 +110,7 @@ jobs: - name: Join tailnet if: ${{ env.PACT_BROKER != '' }} - uses: tailscale/github-action@v3 + uses: tailscale/github-action@6cae46e2d796f265265cfcf628b72a32b4d7cade # v3 with: oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} @@ -124,7 +124,7 @@ jobs: PACT_PROVIDER_BRANCH: ${{ github.head_ref || github.ref_name }} # Publish results only from master so PR runs don't pollute the matrix. PACT_PUBLISH_RESULTS: ${{ github.ref == 'refs/heads/master' }} - run: uv run pytest -v -m contract -o "addopts=-p no:asyncio" tests/contract/test_mcp_provider_verification.py + run: uv run pytest -v -m contract tests/contract/test_mcp_provider_verification.py can-i-deploy: name: can-i-deploy @@ -133,7 +133,7 @@ jobs: if: ${{ github.ref == 'refs/heads/master' }} steps: - name: Join tailnet - uses: tailscale/github-action@v3 + uses: tailscale/github-action@6cae46e2d796f265265cfcf628b72a32b4d7cade # v3 with: oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} oauth-secret: ${{ secrets.TS_OAUTH_SECRET }} @@ -141,7 +141,7 @@ jobs: - name: Install Pact CLI run: | - curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/master/install.sh | bash + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/v2.6.1/install.sh | bash echo "$PWD/pact/bin" >> "$GITHUB_PATH" - name: Can I deploy nextcloud-mcp-server? diff --git a/tests/contract/conftest.py b/tests/contract/conftest.py index 639875ce..2ab72ab3 100644 --- a/tests/contract/conftest.py +++ b/tests/contract/conftest.py @@ -30,12 +30,6 @@ def _clean_pact_dir(): yield -@pytest.fixture -def pact_dir() -> Path: - """Directory the generated pact files are written to.""" - return PACT_DIR - - @pytest.fixture def consumer_pact(): """A fresh Pact (consumer=nextcloud-mcp-server, provider=astrolabe). diff --git a/tests/contract/test_mcp_provider_verification.py b/tests/contract/test_mcp_provider_verification.py index 19bfd85e..a2bcc6ed 100644 --- a/tests/contract/test_mcp_provider_verification.py +++ b/tests/contract/test_mcp_provider_verification.py @@ -35,24 +35,30 @@ from collections.abc import Callable import pytest from pact import Verifier -pytestmark = pytest.mark.contract - 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_URL or _LOCAL_PACT_DIR), + not _PROVIDER_URL or not (_BROKER_READY or _LOCAL_PACT_DIR), reason=( - "Provider verification needs PACT_PROVIDER_URL and a pact source " - "(PACT_BROKER or PACT_PROVIDER_PACT_DIR). Skipped outside CI." + "Provider verification needs PACT_PROVIDER_URL and a pact source: " + "PACT_BROKER (+ PACT_USERNAME/PACT_PASSWORD) or " + "PACT_PROVIDER_PACT_DIR. Skipped outside CI." ), ), ] @@ -90,11 +96,10 @@ def test_verify_astrolabe_consumer_pacts() -> None: verifier = Verifier(PROVIDER_NAME).add_transport(url=_PROVIDER_URL) verifier.state_handler(_dispatch_state, teardown=True) - if _BROKER_URL: - # Basic-auth creds accompany the broker URL (see module skipif). - username = os.environ["PACT_USERNAME"] - password = os.environ["PACT_PASSWORD"] - verifier.broker_source(_BROKER_URL, username=username, password=password) + 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) From 18baa501c9c49d98227c87ff05d7ed90a1565b0f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 20:53:54 +0200 Subject: [PATCH 3/5] test: address round-2 claude-review on #883 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pact.yml: guard `can-i-deploy` job on `env.PACT_BROKER != ''` so a secret rotation/fork can't break every master merge (the CLI errors on empty URL) - pact.yml: pin install.sh to the v2.6.1 commit SHA (immune to tag force-push) - astrolabe_client.py: `_token_cache` Optional[dict] -> `dict | None` and drop the now-unused `Optional` import (CLAUDE.md union syntax) - add tests/unit/test_astrolabe_client.py: mocked unit coverage for get_background_sync_status field mapping (200 provisioned / 200 not-provisioned / 404) — the layer that would have caught the original silent app_password bug - consumer pact test: note the 404 branch is internal defensive handling (covered by the unit test), not a contract obligation Co-Authored-By: Claude Opus 4.8 --- .github/workflows/pact.yml | 9 +- nextcloud_mcp_server/auth/astrolabe_client.py | 3 +- .../test_astrolabe_credentials_consumer.py | 4 + tests/unit/test_astrolabe_client.py | 104 ++++++++++++++++++ 4 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_astrolabe_client.py diff --git a/.github/workflows/pact.yml b/.github/workflows/pact.yml index 3c88ba4e..e7625ce5 100644 --- a/.github/workflows/pact.yml +++ b/.github/workflows/pact.yml @@ -54,7 +54,7 @@ jobs: - name: Install Pact CLI if: ${{ env.PACT_BROKER != '' }} run: | - curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/v2.6.1/install.sh | bash + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/f03e620e7552239b6ca59438c9beed9d1038c949/install.sh | bash # v2.6.1 echo "$PWD/pact/bin" >> "$GITHUB_PATH" - name: Publish pacts to broker @@ -130,7 +130,10 @@ jobs: name: can-i-deploy runs-on: ubuntu-latest needs: [consumer, provider] - if: ${{ github.ref == 'refs/heads/master' }} + # Gate on the broker secret too: the pact-broker CLI errors on an empty + # --broker-base-url, so without this guard a secret rotation/fork would + # break every master merge. + if: ${{ github.ref == 'refs/heads/master' && env.PACT_BROKER != '' }} steps: - name: Join tailnet uses: tailscale/github-action@6cae46e2d796f265265cfcf628b72a32b4d7cade # v3 @@ -141,7 +144,7 @@ jobs: - name: Install Pact CLI run: | - curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/v2.6.1/install.sh | bash + curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/f03e620e7552239b6ca59438c9beed9d1038c949/install.sh | bash # v2.6.1 echo "$PWD/pact/bin" >> "$GITHUB_PATH" - name: Can I deploy nextcloud-mcp-server? diff --git a/nextcloud_mcp_server/auth/astrolabe_client.py b/nextcloud_mcp_server/auth/astrolabe_client.py index 8c316da8..1e6da78b 100644 --- a/nextcloud_mcp_server/auth/astrolabe_client.py +++ b/nextcloud_mcp_server/auth/astrolabe_client.py @@ -7,7 +7,6 @@ and retrieve user app passwords for background sync operations. import logging import time -from typing import Optional from ..http import nextcloud_httpx_client @@ -38,7 +37,7 @@ class AstrolabeClient: self.nextcloud_host = nextcloud_host.rstrip("/") self.client_id = client_id self.client_secret = client_secret - self._token_cache: Optional[dict] = None # {access_token, expires_at} + self._token_cache: dict | None = None # {access_token, expires_at} async def get_access_token(self) -> str: """ diff --git a/tests/contract/test_astrolabe_credentials_consumer.py b/tests/contract/test_astrolabe_credentials_consumer.py index b0788758..5a5bb893 100644 --- a/tests/contract/test_astrolabe_credentials_consumer.py +++ b/tests/contract/test_astrolabe_credentials_consumer.py @@ -16,6 +16,10 @@ This pact pins the request shape and the two states the consumer branches on: 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. """ diff --git a/tests/unit/test_astrolabe_client.py b/tests/unit/test_astrolabe_client.py new file mode 100644 index 00000000..27d53a97 --- /dev/null +++ b/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() From c474f621903e8a8145be70a2d4a7206f7192315e Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 21:00:10 +0200 Subject: [PATCH 4/5] fix(ci): gate can-i-deploy broker steps individually, not at job level The `env` context is not available in a job-level `if:` (only `github`/`needs`/ `vars`/`inputs` are), so `if: ... && env.PACT_BROKER != ''` on the job was an invalid-context error that failed the whole workflow to parse. Move the broker guard onto each step (matching the consumer/provider jobs) and keep the job `if` on the master-branch check only. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/pact.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pact.yml b/.github/workflows/pact.yml index e7625ce5..031079d8 100644 --- a/.github/workflows/pact.yml +++ b/.github/workflows/pact.yml @@ -130,12 +130,13 @@ jobs: name: can-i-deploy runs-on: ubuntu-latest needs: [consumer, provider] - # Gate on the broker secret too: the pact-broker CLI errors on an empty - # --broker-base-url, so without this guard a secret rotation/fork would - # break every master merge. - if: ${{ github.ref == 'refs/heads/master' && env.PACT_BROKER != '' }} + # Only the `github` context is available in a job-level `if`, so the broker + # guard lives on each step below (the pact-broker CLI errors on an empty + # --broker-base-url, e.g. after a secret rotation or on a fork). + if: ${{ github.ref == 'refs/heads/master' }} steps: - name: Join tailnet + if: ${{ env.PACT_BROKER != '' }} uses: tailscale/github-action@6cae46e2d796f265265cfcf628b72a32b4d7cade # v3 with: oauth-client-id: ${{ secrets.TS_OAUTH_CLIENT_ID }} @@ -143,11 +144,13 @@ jobs: tags: tag:github-runner - name: Install Pact CLI + if: ${{ env.PACT_BROKER != '' }} run: | curl -fsSL https://raw.githubusercontent.com/pact-foundation/pact-ruby-standalone/f03e620e7552239b6ca59438c9beed9d1038c949/install.sh | bash # v2.6.1 echo "$PWD/pact/bin" >> "$GITHUB_PATH" - name: Can I deploy nextcloud-mcp-server? + if: ${{ env.PACT_BROKER != '' }} run: | pact-broker can-i-deploy \ --broker-base-url "$PACT_BROKER" \ From 69c40a04799de1f5952d3e10d69ec9a06c132868 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 10 Jun 2026 21:36:27 +0200 Subject: [PATCH 5/5] fix: convert astrolabe int provisioned_at to ISO before ProvisioningStatus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-4 review (real bug): get_background_sync_status now returns provisioned_at as Unix seconds (the wire/pact value), but ProvisioningStatus.provisioned_at is str | None (ISO). Constructing it for a provisioned user raised a Pydantic ValidationError — a path that was unreachable before the has_access fix. Convert int -> ISO at the oauth_tools boundary (mirroring the existing refresh_token branch), keeping the model schema and the int-asserting contract pact/unit tests intact. Add a regression test that drives the full _get_provisioning_status round-trip with an integer timestamp. Also surface dropped provider-state params in the verifier's _dispatch_state no-op branch (round-4 nit). Co-Authored-By: Claude Opus 4.8 --- nextcloud_mcp_server/server/oauth_tools.py | 8 ++++- .../test_mcp_provider_verification.py | 8 ++++- ...t_oauth_tools_app_password_provisioning.py | 34 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/nextcloud_mcp_server/server/oauth_tools.py b/nextcloud_mcp_server/server/oauth_tools.py index 7e0ee70f..81a7f513 100644 --- a/nextcloud_mcp_server/server/oauth_tools.py +++ b/nextcloud_mcp_server/server/oauth_tools.py @@ -118,7 +118,13 @@ async def _get_provisioning_status(ctx: Context, user_id: str) -> ProvisioningSt " get_provisioning_status: app password FOUND for user_id=%s", user_id, ) - provisioned_at_str = status.get("provisioned_at") + # Astrolabe returns provisioned_at as Unix seconds (see the + # contract pact); convert to the ISO string the model expects. + provisioned_at_str = None + provisioned_at_raw = status.get("provisioned_at") + if provisioned_at_raw: + dt = datetime.fromtimestamp(provisioned_at_raw, tz=timezone.utc) + provisioned_at_str = dt.isoformat() return ProvisioningStatus( is_provisioned=True, provisioned_at=provisioned_at_str, diff --git a/tests/contract/test_mcp_provider_verification.py b/tests/contract/test_mcp_provider_verification.py index a2bcc6ed..db5754ca 100644 --- a/tests/contract/test_mcp_provider_verification.py +++ b/tests/contract/test_mcp_provider_verification.py @@ -86,7 +86,13 @@ def _dispatch_state(state: str, **kwargs) -> None: """ handler = _PROVIDER_STATES.get(state) if handler is None: - logger.warning("No provider-state handler registered for %r; no-op", state) + # 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() diff --git a/tests/unit/test_oauth_tools_app_password_provisioning.py b/tests/unit/test_oauth_tools_app_password_provisioning.py index 81e96cdc..f710e43c 100644 --- a/tests/unit/test_oauth_tools_app_password_provisioning.py +++ b/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()