diff --git a/.github/workflows/hackbot-prod-deploy.yml b/.github/workflows/hackbot-prod-deploy.yml index e808a0d01ab..dd71bf1406a 100644 --- a/.github/workflows/hackbot-prod-deploy.yml +++ b/.github/workflows/hackbot-prod-deploy.yml @@ -61,6 +61,7 @@ on: - agents/test-repair - agents/test-repair-fix - services/hackbot-api + - services/hackbot-mcp - services/hackbot-ui action: description: "promote: what stage runs; rollback: prod's previous deployment" diff --git a/.taskcluster.yml b/.taskcluster.yml index a5a0177b6f7..2df50789e2c 100644 --- a/.taskcluster.yml +++ b/.taskcluster.yml @@ -199,6 +199,8 @@ tasks: uv run --locked --with pytest==9.1.0 --with pytest-cov==7.1.0 --with pytest-asyncio==1.4.0 pytest --cov=app --cov-append tests/ && cd ../hackbot-pulse-listener && uv run --locked --with pytest==9.1.0 --with pytest-cov==7.1.0 --with pytest-asyncio==1.4.0 pytest --cov=app --cov-append tests/ && + cd ../hackbot-mcp && + uv run --locked --with pytest==9.1.0 --with pytest-cov==7.1.0 --with pytest-asyncio==1.4.0 pytest --cov=hackbot_mcp --cov-append tests/ && cd ../mcp && uv run --locked --with pytest==9.1.0 --with pytest-cov==7.1.0 --with pytest-asyncio==1.4.0 --with pytest-mock==3.15.1 pytest --cov=bugbug_mcp --cov-append tests/ && cd ../.. && diff --git a/libs/hackbot-client/hackbot_client/__init__.py b/libs/hackbot-client/hackbot_client/__init__.py index 7b1996e6a72..f87c9111b00 100644 --- a/libs/hackbot-client/hackbot_client/__init__.py +++ b/libs/hackbot-client/hackbot_client/__init__.py @@ -4,20 +4,28 @@ HackbotInvalidResponseError, ) from hackbot_client.models import ( + AgentDescriptor, ApplyActionsResponse, + ArtifactRef, RunAction, + RunDoc, RunRef, RunStatus, + RunSummary, TriggeredRun, ) __all__ = [ + "AgentDescriptor", + "ApplyActionsResponse", + "ArtifactRef", "HackbotAPIError", "HackbotClient", "HackbotInvalidResponseError", "RunAction", - "ApplyActionsResponse", + "RunDoc", "RunRef", "RunStatus", + "RunSummary", "TriggeredRun", ] diff --git a/libs/hackbot-client/hackbot_client/client.py b/libs/hackbot-client/hackbot_client/client.py index b84cbea7e1c..2c8f0bd6afe 100644 --- a/libs/hackbot-client/hackbot_client/client.py +++ b/libs/hackbot-client/hackbot_client/client.py @@ -1,27 +1,62 @@ """Small async client for the public Hackbot API.""" import asyncio +import json from collections.abc import Mapping from typing import Any +from urllib.parse import quote from uuid import UUID import httpx from google.auth import compute_engine, credentials from google.auth.transport import requests as google_requests from google.oauth2 import service_account -from pydantic import ValidationError +from pydantic import BaseModel, ValidationError -from hackbot_client.models import ApplyActionsResponse, TriggeredRun +from hackbot_client.models import ( + AgentDescriptor, + ApplyActionsResponse, + RunDoc, + RunStatus, + TriggeredRun, +) class HackbotAPIError(Exception): - """A request to the Hackbot API failed (or answered with something unusable).""" + """A request to the Hackbot API failed (or answered with something unusable). + + `status_code` is the API's HTTP status when it answered with an error, and + `detail` the reason it gave, if any; both are None when it never answered. + """ + + def __init__( + self, + message: str, + *, + status_code: int | None = None, + detail: str | None = None, + ) -> None: + super().__init__(message) + self.status_code = status_code + self.detail = detail class HackbotInvalidResponseError(HackbotAPIError): """The Hackbot API answered successfully, but with a body that is unusable.""" +def _error_detail(response: httpx.Response) -> str | None: + """The reason an error response gives, from FastAPI's `{"detail": ...}`.""" + try: + body = response.json() + except ValueError: + return None + detail = body.get("detail") if isinstance(body, dict) else None + if detail is None: + return None + return detail if isinstance(detail, str) else json.dumps(detail) + + class HackbotClient: """Call the public Hackbot API with either API key or service account auth.""" @@ -83,50 +118,183 @@ async def _get_headers(self) -> dict[str, str]: await asyncio.to_thread(self._refresh_credentials) return {"Authorization": f"Bearer {self._credentials.token}"} - async def trigger_run( + async def _send( self, - agent_name: str, - inputs: Mapping[str, Any], + method: str, + path: str, *, on_behalf_of: str | None = None, - dedupe_key: str | None = None, - ) -> TriggeredRun: - """Create an agent run and return the API's typed run reference. - - `dedupe_key` keys the work the run does, and a key belongs to one run - for good: repeated triggers carrying it are no-ops, answered with the - same run reference and `is_new=False`. - """ + params: Mapping[str, Any] | None = None, + json: Any = None, + ) -> httpx.Response: + """Send one request, raising `HackbotAPIError` unless it succeeded.""" headers = await self._get_headers() if on_behalf_of is not None: headers["X-On-Behalf-Of"] = on_behalf_of - params = {} if dedupe_key is None else {"dedupe_key": dedupe_key} + kwargs: dict[str, Any] = {"headers": headers} + if params is not None: + kwargs["params"] = dict(params) + if json is not None: + kwargs["json"] = json try: - response = await self._http.post( - f"{self._base_url}/agents/{agent_name}/runs", - json=dict(inputs), - headers=headers, - params=params, - ) + if method == "GET": + response = await self._http.get(f"{self._base_url}{path}", **kwargs) + else: + response = await self._http.post(f"{self._base_url}{path}", **kwargs) except httpx.HTTPError as exc: raise HackbotAPIError(f"Hackbot API request failed: {exc}") from exc if not response.is_success: + detail = _error_detail(response) raise HackbotAPIError( f"Hackbot API returned HTTP {response.status_code} " f"({response.reason_phrase}) for {response.request.url}" + + (f": {detail}" if detail else ""), + status_code=response.status_code, + detail=detail, ) + return response + @staticmethod + def _body(response: httpx.Response, what: str) -> Any: try: - body = response.json() + return response.json() except ValueError as exc: raise HackbotInvalidResponseError( - f"Hackbot API returned an invalid run (HTTP {response.status_code}): " - "the body is not JSON" + f"Hackbot API returned an invalid {what} " + f"(HTTP {response.status_code}): the body is not JSON" ) from exc + @staticmethod + def _validate[M: BaseModel]( + model: type[M], body: Any, response: httpx.Response, what: str + ) -> M: + try: + return model.model_validate(body) + except ValidationError as exc: + raise HackbotInvalidResponseError( + f"Hackbot API returned an invalid {what} (HTTP {response.status_code})" + ) from exc + + async def list_agents( + self, *, on_behalf_of: str | None = None + ) -> list[AgentDescriptor]: + """Every agent the API can run, with its description and input schema.""" + response = await self._send("GET", "/agents", on_behalf_of=on_behalf_of) + body = self._body(response, "agent list") + if not isinstance(body, list): + raise HackbotInvalidResponseError( + f"Hackbot API returned an invalid agent list " + f"(HTTP {response.status_code}): the body is not a list" + ) + return [ + self._validate(AgentDescriptor, item, response, "agent list") + for item in body + ] + + async def get_run( + self, run_id: str | UUID, *, on_behalf_of: str | None = None + ) -> RunDoc: + response = await self._send("GET", f"/runs/{run_id}", on_behalf_of=on_behalf_of) + return self._validate(RunDoc, self._body(response, "run"), response, "run") + + async def list_runs( + self, + *, + agent: str | None = None, + status: RunStatus | str | None = None, + requested_by: str | None = None, + dedupe_key: str | None = None, + limit: int | None = None, + offset: int | None = None, + on_behalf_of: str | None = None, + ) -> list[RunDoc]: + """Runs matching every filter given, newest first.""" + params = { + name: value + for name, value in { + "agent": agent, + "status": RunStatus(status).value if status is not None else None, + "requested_by": requested_by, + "dedupe_key": dedupe_key, + "limit": limit, + "offset": offset, + }.items() + if value is not None + } + response = await self._send( + "GET", "/runs", on_behalf_of=on_behalf_of, params=params + ) + body = self._body(response, "run list") + if not isinstance(body, list): + raise HackbotInvalidResponseError( + f"Hackbot API returned an invalid run list " + f"(HTTP {response.status_code}): the body is not a list" + ) + return [self._validate(RunDoc, item, response, "run list") for item in body] + + async def list_actions( + self, run_id: str | UUID, *, on_behalf_of: str | None = None + ) -> ApplyActionsResponse: + """A run's recorded actions and their apply state. + + Empty until the API records them, which it does once a succeeded run is + processed (or on the first apply); until then, `summary.actions` on the + run lists what the agent proposed. + """ + response = await self._send( + "GET", f"/runs/{run_id}/actions", on_behalf_of=on_behalf_of + ) + return self._validate( + ApplyActionsResponse, + self._body(response, "action list"), + response, + "action list", + ) + + async def get_artifact_url( + self, run_id: str | UUID, artifact: str, *, on_behalf_of: str | None = None + ) -> str: + """A short-lived signed URL to download one of a run's artifacts.""" + response = await self._send( + "GET", + f"/runs/{run_id}/artifacts/{quote(artifact.lstrip('/'))}", + on_behalf_of=on_behalf_of, + ) + body = self._body(response, "artifact URL") + if not isinstance(body, dict) or not isinstance(body.get("url"), str): + raise HackbotInvalidResponseError( + f"Hackbot API returned an invalid artifact URL " + f"(HTTP {response.status_code})" + ) + return body["url"] + + async def trigger_run( + self, + agent_name: str, + inputs: Mapping[str, Any], + *, + on_behalf_of: str | None = None, + dedupe_key: str | None = None, + ) -> TriggeredRun: + """Create an agent run and return the API's typed run reference. + + `dedupe_key` keys the work the run does, and a key belongs to one run + for good: repeated triggers carrying it are no-ops, answered with the + same run reference and `is_new=False`. + """ + params = {} if dedupe_key is None else {"dedupe_key": dedupe_key} + response = await self._send( + "POST", + f"/agents/{agent_name}/runs", + on_behalf_of=on_behalf_of, + params=params, + json=dict(inputs), + ) + body = self._body(response, "run") + if not isinstance(body, dict): raise HackbotInvalidResponseError( f"Hackbot API returned an invalid run (HTTP {response.status_code}): " @@ -144,38 +312,20 @@ async def trigger_run( f"Hackbot API returned an invalid run (HTTP {response.status_code})" ) from exc - async def apply_actions(self, run_id: str | UUID) -> ApplyActionsResponse: + async def apply_actions( + self, run_id: str | UUID, *, on_behalf_of: str | None = None + ) -> ApplyActionsResponse: """Apply every one of a run's actions that has not landed yet. Idempotent, because the API skips rows already marked applied: calling this on a run whose actions all landed is a no-op. """ - try: - response = await self._http.post( - f"{self._base_url}/runs/{run_id}/actions/apply", - headers=await self._get_headers(), - ) - except httpx.HTTPError as exc: - raise HackbotAPIError(f"Hackbot API request failed: {exc}") from exc - - if not response.is_success: - raise HackbotAPIError( - f"Hackbot API returned HTTP {response.status_code} " - f"({response.reason_phrase}) for {response.request.url}" - ) - - try: - body = response.json() - except ValueError as exc: - raise HackbotInvalidResponseError( - "Hackbot API returned an invalid action list " - f"(HTTP {response.status_code}): the body is not JSON" - ) from exc - - try: - return ApplyActionsResponse.model_validate(body) - except ValidationError as exc: - raise HackbotInvalidResponseError( - "Hackbot API returned an invalid action list " - f"(HTTP {response.status_code})" - ) from exc + response = await self._send( + "POST", f"/runs/{run_id}/actions/apply", on_behalf_of=on_behalf_of + ) + return self._validate( + ApplyActionsResponse, + self._body(response, "action list"), + response, + "action list", + ) diff --git a/libs/hackbot-client/hackbot_client/models.py b/libs/hackbot-client/hackbot_client/models.py index b420ed7f025..cb372b9d7ad 100644 --- a/libs/hackbot-client/hackbot_client/models.py +++ b/libs/hackbot-client/hackbot_client/models.py @@ -37,6 +37,42 @@ class TriggeredRun(RunRef): is_new: bool +class AgentDescriptor(BaseModel): + name: str + description: str + input_schema: dict[str, Any] + + +class ArtifactRef(BaseModel): + name: str + size: int + content_type: str | None = None + + +class RunSummary(BaseModel): + status: str + error: str | None = None + findings: dict[str, Any] = {} + actions: list[dict[str, Any]] = [] + + +class RunDoc(BaseModel): + run_id: UUID + agent: str + status: RunStatus + inputs: dict[str, Any] + requested_by: str | None = None + requested_via: str | None = None + dedupe_key: str | None = None + created_at: datetime + updated_at: datetime + execution_name: str | None = None + results_prefix: str + summary: RunSummary | None = None + artifacts: list[ArtifactRef] = [] + error: str | None = None + + class RunAction(BaseModel): """One recorded action of a run, and whether it has landed. diff --git a/libs/hackbot-client/tests/test_client.py b/libs/hackbot-client/tests/test_client.py index 133b1d48901..5a54065db09 100644 --- a/libs/hackbot-client/tests/test_client.py +++ b/libs/hackbot-client/tests/test_client.py @@ -347,3 +347,139 @@ def mock_from_service_account_file(filename, target_audience): assert captured["filename"] == "/path/key.json" assert captured["audience"] == "https://hackbot.example" assert captured["headers"] == {"Authorization": "Bearer fake.jwt.token"} + + +def _mocked(handler) -> HackbotClient: + """A client whose requests ``handler`` answers.""" + client = _client() + client._http = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + return client + + +def _run_doc(**overrides) -> dict: + doc = { + "run_id": RUN_ID, + "agent": "bug-fix", + "status": "succeeded", + "inputs": {"bug_id": 1}, + "requested_by": "user@example.com", + "created_at": "2026-11-12T00:00:00Z", + "updated_at": "2026-11-12T00:00:00Z", + "results_prefix": f"runs/{RUN_ID}", + "summary": {"status": "ok", "actions": [{"type": "email.send"}]}, + "artifacts": [{"name": "changes/changes.patch", "size": 10}], + } + doc.update(overrides) + return doc + + +async def test_list_agents_returns_descriptors(): + seen = [] + + def handler(request): + seen.append(request) + return httpx.Response( + 200, + json=[{"name": "bug-fix", "description": "d", "input_schema": {}}], + ) + + agents = await _mocked(handler).list_agents(on_behalf_of="user@example.com") + + assert [agent.name for agent in agents] == ["bug-fix"] + assert str(seen[0].url) == "https://hackbot.example/agents" + assert seen[0].headers["X-On-Behalf-Of"] == "user@example.com" + assert seen[0].headers["X-API-Key"] == "secret" + + +async def test_get_run_returns_the_typed_run(): + run = await _mocked(lambda r: httpx.Response(200, json=_run_doc())).get_run(RUN_ID) + + assert run.run_id == UUID(RUN_ID) + assert run.status is RunStatus.succeeded + assert run.summary.actions == [{"type": "email.send"}] + assert run.artifacts[0].name == "changes/changes.patch" + + +async def test_get_run_rejects_an_invalid_run(): + client = _mocked(lambda r: httpx.Response(200, json={"run_id": RUN_ID})) + with pytest.raises(HackbotInvalidResponseError, match="invalid run"): + await client.get_run(RUN_ID) + + +async def test_list_runs_sends_only_the_filters_given(): + seen = [] + + def handler(request): + seen.append(request) + return httpx.Response(200, json=[_run_doc()]) + + runs = await _mocked(handler).list_runs( + requested_by="user@example.com", status="running", limit=5 + ) + + assert len(runs) == 1 + assert dict(seen[0].url.params) == { + "requested_by": "user@example.com", + "status": "running", + "limit": "5", + } + + +async def test_list_runs_rejects_a_body_that_is_not_a_list(): + client = _mocked(lambda r: httpx.Response(200, json={})) + with pytest.raises(HackbotInvalidResponseError, match="not a list"): + await client.list_runs() + + +async def test_list_actions_returns_each_action(): + client = _mocked( + lambda r: httpx.Response( + 200, + json=[{"idx": 0, "type": "email.send", "params": {}, "status": "pending"}], + ) + ) + + actions = await client.list_actions(RUN_ID) + + assert [action.status for action in actions] == ["pending"] + + +async def test_get_artifact_url_returns_the_signed_url(): + seen = [] + + def handler(request): + seen.append(request) + return httpx.Response(200, json={"url": "https://storage.example/x"}) + + url = await _mocked(handler).get_artifact_url(RUN_ID, "changes/changes.patch") + + assert url == "https://storage.example/x" + assert seen[0].url.path == f"/runs/{RUN_ID}/artifacts/changes/changes.patch" + + +async def test_get_artifact_url_rejects_a_body_without_a_url(): + client = _mocked(lambda r: httpx.Response(200, json={})) + with pytest.raises(HackbotInvalidResponseError, match="artifact URL"): + await client.get_artifact_url(RUN_ID, "x") + + +async def test_errors_carry_the_status_and_the_api_detail(): + client = _mocked( + lambda r: httpx.Response(422, json={"detail": "comment requires revision_id"}) + ) + + with pytest.raises(HackbotAPIError, match="comment requires revision_id") as exc: + await client.trigger_run("bug-fix", {"bug_id": 1, "comment": "x"}) + + assert exc.value.status_code == 422 + assert exc.value.detail == "comment requires revision_id" + + +async def test_unreachable_api_errors_have_no_status(): + def handler(request): + raise httpx.ConnectError("connection refused") + + with pytest.raises(HackbotAPIError) as exc: + await _mocked(handler).get_run(RUN_ID) + + assert exc.value.status_code is None diff --git a/services/hackbot-mcp/Dockerfile b/services/hackbot-mcp/Dockerfile new file mode 100644 index 00000000000..616f2425609 --- /dev/null +++ b/services/hackbot-mcp/Dockerfile @@ -0,0 +1,39 @@ +FROM python:3.14-slim AS builder + +COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ + +ENV UV_PROJECT_ENVIRONMENT=/opt/venv +ENV UV_PYTHON_DOWNLOADS=never + +WORKDIR /app + +# Install external deps without building workspace members. +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,source=pyproject.toml,target=pyproject.toml \ + --mount=type=bind,source=uv.lock,target=uv.lock \ + --mount=type=bind,source=VERSION,target=VERSION \ + uv sync --frozen --no-dev --no-install-workspace --package hackbot-mcp + +RUN --mount=type=cache,target=/root/.cache/uv \ + --mount=type=bind,target=/app,rw \ + uv sync --locked --no-dev --no-editable --package hackbot-mcp + +FROM python:3.14-slim AS base + +COPY --from=builder /opt/venv /opt/venv +WORKDIR /app + +ENV PYTHONUNBUFFERED=1 +ENV PYTHONDONTWRITEBYTECODE=1 +ENV PORT=8080 +ENV PATH="/opt/venv/bin:$PATH" + +ARG SENTRY_RELEASE +ENV SENTRY_RELEASE=$SENTRY_RELEASE + +RUN useradd --create-home --shell /bin/bash app +USER app + +EXPOSE 8080 + +CMD ["python", "-m", "hackbot_mcp.server"] diff --git a/services/hackbot-mcp/README.md b/services/hackbot-mcp/README.md new file mode 100644 index 00000000000..6fc616e4c74 --- /dev/null +++ b/services/hackbot-mcp/README.md @@ -0,0 +1,144 @@ +# hackbot-mcp + +A remote [MCP](https://modelcontextprotocol.io) server for Hackbot. Mozilla +developers can use it to start Hackbot agent runs, check on them, read their +results and apply their actions from an MCP client such as Claude Code or +Codex. They sign in with their own Mozilla Google account. + +It's a proxy, like [hackbot-ui](../hackbot-ui): + +- Users sign in through MCP OAuth, with Google as the identity provider. Only + verified `@mozilla.com` accounts get in: Google must report the account's + Workspace domain (`hd`) as `mozilla.com`, mark the email as verified, and + the email itself must be `@mozilla.com`. +- The server calls [hackbot-api](../hackbot-api) with its own credential and + sends the signed-in user's email as `X-On-Behalf-Of`. That email only ever + comes from the verified token, never from a tool argument. + +## Tools + +| Tool | What it does | +| ------------------ | ---------------------------------------------------------------------------------------- | +| `list_agents` | The agents, with their descriptions and input schemas. | +| `start_run` | Starts a run and returns right away. Holds actions for review unless `auto_apply=true`. | +| `get_run` | A run's current state. | +| `list_my_runs` | The signed-in user's runs, newest first. | +| `get_result` | A run's findings, artifacts and actions, with each action's state. | +| `get_artifact_url` | A short-lived download URL for an artifact, e.g. `changes/changes.patch` (for `git am`). | +| `apply_actions` | Applies all of a run's pending actions. Only the run's requester can do this. | + +No tool waits for a run to finish: runs take up to an hour or more. The +server's MCP instructions tell the model to confirm with the user before +`start_run`, to poll with `get_run`, and to list every pending action before +`apply_actions`. Each tool's annotations (read-only, or destructive for +`apply_actions`) also decide how the MCP client asks for permission. + +`start_run` doesn't accept `require_review` (`auto_apply` sets it) or +`bugzilla_needinfo_flag_id` (that input belongs to the Bugzilla webhook). + +## Connecting + +Claude Code: + +```sh +claude mcp add --transport http hackbot https:///mcp +``` + +Then run `/mcp` in Claude Code, pick `hackbot`, and sign in with your Mozilla +Google account in the browser window that opens. + +Codex: + +```sh +codex mcp add hackbot --url https:///mcp +codex mcp login hackbot +``` + +## Configuration + +Environment variables (pydantic-settings, see `src/hackbot_mcp/config.py`): + +| Variable | Secret | Description | +| ----------------------------------------------------- | --------- | -------------------------------------------------------------------------------------------------------- | +| `BASE_URL` | | Public URL of this server, without `/mcp`. Must stay the same, because OAuth metadata and tokens use it. | +| `GOOGLE_CLIENT_ID` | | Google OAuth client (Web application) with redirect URI `/auth/callback`. | +| `GOOGLE_CLIENT_SECRET` | yes | That client's secret. | +| `JWT_SIGNING_KEY` | yes | 32+ random characters. Signs the tokens issued to MCP clients. Rotating it signs everyone out. | +| `STORAGE_ENCRYPTION_KEY` | yes | 32+ random characters. Encrypts the stored OAuth state (users' Google tokens). | +| `CLOUD_SQL_INSTANCE` | | `project:region:instance` of the Postgres instance that holds the OAuth state. | +| `CLOUD_SQL_IP_TYPE` | | `PUBLIC` (default), `PRIVATE` or `PSC`. | +| `DB_USER` / `DB_PASS` / `DB_NAME` | `DB_PASS` | Database credentials; `DB_NAME` defaults to `hackbot_mcp`. The table is created on first use. | +| `DATABASE_URL` | yes | A plain `postgresql://` URL, instead of `CLOUD_SQL_INSTANCE` (local Postgres). | +| `HACKBOT_API_URL` | | hackbot-api's URL, which is also the audience of the ID token sent to it. | +| `HACKBOT_API_KEY` | yes | Optional. Without it, the server uses its service account's ID token (see below). | +| `HACKBOT_UI_URL` | | Base for the run links in tool results. | +| `ENVIRONMENT`, `SENTRY_DSN`, `SENTRY_RELEASE`, `PORT` | | As for hackbot-api. | + +Outside `ENVIRONMENT=development`, one of `CLOUD_SQL_INSTANCE` and +`DATABASE_URL` is required. Cloud Run runs several instances, a client's +requests can land on any of them, and all of them need to see the same client +registrations and tokens. + +### Authenticating to hackbot-api + +By default the server sends a Google ID token for its own service account, +from the metadata server, with audience `HACKBOT_API_URL`. The service account +must be in hackbot-api's `ALLOWED_SERVICE_ACCOUNTS`, and hackbot-api records it +as each run's `requested_via`. Set `HACKBOT_API_KEY` to use the shared key +instead. + +### Infrastructure (webservices-infra) + +- A Google OAuth client in the hackbot project with redirect URI + `/auth/callback`. An "Internal" consent screen also limits sign-in + to the Mozilla Workspace. +- A Cloud Run service, `hackbot-mcp` (with `-dev` and `-stage` suffixes outside + prod): + - allows unauthenticated invocations, since the server authenticates users + itself; + - runs as its own service account, with `roles/cloudsql.client` and access + to the secrets above; + - has a stable public URL. +- A `hackbot_mcp` database and user on hackbot's Cloud SQL instance. +- The service account added to hackbot-api's `ALLOWED_SERVICE_ACCOUNTS`. +- The `dev-hackbot-mcp` and `stage-hackbot-mcp` Cloud Build triggers for + `cloudbuild.yaml`, and a `hackbot-mcp` Sentry project. + +## Releasing + +As for hackbot-api: bump `VERSION` to release to stage. Push to +`dev/services/hackbot-mcp` to deploy to dev. Promote to prod with the +"hackbot prod deploy" workflow. See [cloudbuild.yaml](cloudbuild.yaml) and +[tools/hackbot-deploy](../../tools/hackbot-deploy). + +## Running locally + +Create a Google OAuth client (Web application) with the redirect URI +`http://localhost:8080/auth/callback`, then: + +```sh +cat > services/hackbot-mcp/.env < +HACKBOT_API_KEY=... +EOF +cd services/hackbot-mcp +uv run --package hackbot-mcp python -m hackbot_mcp.server +claude mcp add --transport http hackbot-local http://localhost:8080/mcp +``` + +Without `DATABASE_URL`, OAuth state is kept in memory, so restarting the +server signs everyone out. + +Tests: + +```sh +cd services/hackbot-mcp +uv run --locked --with pytest --with pytest-asyncio pytest tests/ +``` + +The tests mock hackbot-api and Google with `httpx.MockTransport`. diff --git a/services/hackbot-mcp/VERSION b/services/hackbot-mcp/VERSION new file mode 100644 index 00000000000..6e8bf73aa55 --- /dev/null +++ b/services/hackbot-mcp/VERSION @@ -0,0 +1 @@ +0.1.0 diff --git a/services/hackbot-mcp/cloudbuild.yaml b/services/hackbot-mcp/cloudbuild.yaml new file mode 100644 index 00000000000..e248552cd89 --- /dev/null +++ b/services/hackbot-mcp/cloudbuild.yaml @@ -0,0 +1,128 @@ +# Release hackbot-mcp: build it and deploy it to one environment. +# +# Two triggers (webservices-infra, hackbot/tf/modules/resources/build.tf) run +# this, telling it which environment in _ENV: +# stage-hackbot-mcp a push to the default branch touching +# services/hackbot-mcp/VERSION. Deploys only +# if its version isn't the one stage runs; run by hand, it +# deploys regardless. +# dev-hackbot-mcp a push to dev/services/hackbot-mcp or +# dev/services/hackbot-mcp/. Deploys to dev, +# always. +# Prod is a separate, approved promotion; nothing here reaches it. +# services/cloudbuild.yaml is hackbot-dev's older config and goes away at +# cutover. +# +# `release-plan` decides whether to release and `release` deploys it. The +# Sentry release is hackbot-mcp@ (dev builds add +), with the +# commits since what the environment ran before, and a deploy in its +# environment; prod's deploy is marked by the prod workflow. +# +# Cloud Build reads every `$NAME` as a substitution, and MUST_MATCH fails the +# build on unknown ones, so shell variables below are written `$$name`. + +steps: + # Builds of one trigger deploy to one environment: take turns. The wait + # allows for an older build's own work; the timeout is that plus this one's. + - id: Wait + name: ghcr.io/astral-sh/uv:python3.14-bookworm-slim + entrypoint: bash + args: + - -c + - uv run --frozen --package hackbot-deploy python -m hackbot_deploy wait-turn "$BUILD_ID" --project "$PROJECT_ID" --location "$LOCATION" --timeout 3600 + + # /workspace/plan.txt: "name version previous-commit - -", or empty when + # there is nothing to release (every step below then does nothing). + - id: Plan + name: ghcr.io/astral-sh/uv:python3.14-bookworm-slim + entrypoint: bash + # The tool reads these from the environment: in a command, a run by hand's + # unresolved pusher ("$(push.pusher.name)") would run as a command. + env: + - BRANCH_NAME=$BRANCH_NAME + - PUSHER=${_PUSHER} + args: + - -c + - uv run --frozen --package hackbot-deploy python -m hackbot_deploy release-plan services/hackbot-mcp --env "${_ENV}" --output /workspace/plan.txt + + - id: Build + name: gcr.io/cloud-builders/docker + entrypoint: bash + env: + - DOCKER_BUILDKIT=1 + args: + - -c + - | + set -euo pipefail + read -r _ version _ < /workspace/plan.txt || exit 0 + release="hackbot-mcp@$$version" + sha="$COMMIT_SHA" + if [ "${_ENV}" = dev ]; then release="$$release+$${sha:0:12}"; fi + image="${_AR_HOSTNAME}/${_AR_PROJECT_ID}/${_AR_REPOSITORY}/hackbot-mcp:$COMMIT_SHA" + docker build --no-cache -f services/hackbot-mcp/Dockerfile \ + --build-arg "SENTRY_RELEASE=$$release" \ + --label "org.opencontainers.image.version=$$version" \ + --label "org.opencontainers.image.revision=$COMMIT_SHA" \ + --label "org.opencontainers.image.created=$$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + -t "$$image" . + docker push "$$image" + docker inspect --format='{{index .RepoDigests 0}}' "$$image" > /workspace/hackbot-mcp.digest + + # Services have no shape gate: their infrastructure is written by hand in + # webservices-infra and changes there first. + - id: Deploy + name: ghcr.io/astral-sh/uv:python3.14-bookworm-slim + entrypoint: bash + # Recorded as deployed_by: whoever pushed, or the build when the trigger + # is run by hand. Substitutions aren't in a step's environment otherwise. + env: + - GITHUB_ACTOR=${_PUSHER} + - BUILD_ID=$BUILD_ID + args: + - -c + - uv run --frozen --package hackbot-deploy python -m hackbot_deploy release --env "${_ENV}" --plan /workspace/plan.txt --digests /workspace + + # After the deploy, so a Sentry outage can't hold one up. + - id: Sentry + name: getsentry/sentry-cli + entrypoint: sh + secretEnv: + - SENTRY_AUTH_TOKEN + args: + - -c + - | + set -eu + read -r _ version previous _ < /workspace/plan.txt || exit 0 + release="hackbot-mcp@$$version" + environment=staging + sha="$COMMIT_SHA" + if [ "${_ENV}" = dev ]; then + release="$$release+$$(echo "$$sha" | cut -c1-12)" + environment=development + fi + commits="mozilla/bugbug@$COMMIT_SHA" + if [ "$$previous" != - ]; then commits="mozilla/bugbug@$$previous..$COMMIT_SHA"; fi + sentry-cli releases new "$$release" + sentry-cli releases set-commits "$$release" --commit "$$commits" + sentry-cli releases finalize "$$release" + sentry-cli deploys new --release "$$release" -e "$$environment" + +# _ENV is set by each trigger; _PUSHER from the push event, empty when run +# any other way. +substitutions: + _ENV: stage + _PUSHER: "" + +timeout: 5400s + +options: + substitutionOption: MUST_MATCH + logging: CLOUD_LOGGING_ONLY + env: + - SENTRY_ORG=mozilla + - SENTRY_PROJECT=hackbot-mcp + +availableSecrets: + secretManager: + - versionName: projects/$PROJECT_ID/secrets/${_SENTRY_AUTH_TOKEN_SECRET}/versions/latest + env: SENTRY_AUTH_TOKEN diff --git a/services/hackbot-mcp/pyproject.toml b/services/hackbot-mcp/pyproject.toml new file mode 100644 index 00000000000..68d0736b34f --- /dev/null +++ b/services/hackbot-mcp/pyproject.toml @@ -0,0 +1,38 @@ +[project] +name = "hackbot-mcp" +dynamic = ["version"] +description = "Remote MCP server for running Hackbot agents, signed in with Google" +requires-python = ">=3.14" +dependencies = [ + "cloud-sql-python-connector[asyncpg]>=1.20.3", + "fastmcp>=3.4.7", + "hackbot-client", + "httpx>=0.28.1", + "jsonschema>=4.23.0", + "py-key-value-aio[postgresql,wrappers-encryption]>=0.4.5,<0.5.0", + "pydantic>=2.13.0", + "pydantic-settings>=2.13.0", + "sentry-sdk>=2.60.0", +] + +[project.optional-dependencies] +dev = ["pytest>=9.0.3", "pytest-asyncio>=1.4.0"] + +[tool.uv.sources] +hackbot-client = { workspace = true } + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.version] +path = "VERSION" +pattern = "(?P.+)" + +[tool.hatch.build.targets.wheel] +packages = ["src/hackbot_mcp"] + +[tool.pytest.ini_options] +asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" +testpaths = ["tests"] diff --git a/services/hackbot-mcp/src/hackbot_mcp/__init__.py b/services/hackbot-mcp/src/hackbot_mcp/__init__.py new file mode 100644 index 00000000000..5f47f31ca7a --- /dev/null +++ b/services/hackbot-mcp/src/hackbot_mcp/__init__.py @@ -0,0 +1,7 @@ +from importlib.metadata import PackageNotFoundError, version + +# The version lives in services/hackbot-mcp/VERSION; the package build reads it. +try: + __version__ = version("hackbot-mcp") +except PackageNotFoundError: # running from a source tree without installing + __version__ = "0+unknown" diff --git a/services/hackbot-mcp/src/hackbot_mcp/auth.py b/services/hackbot-mcp/src/hackbot_mcp/auth.py new file mode 100644 index 00000000000..a3ce3f450a8 --- /dev/null +++ b/services/hackbot-mcp/src/hackbot_mcp/auth.py @@ -0,0 +1,120 @@ +"""Google sign-in, restricted to verified @mozilla.com accounts.""" + +import logging +from typing import Any + +import httpx +from fastmcp.server.auth import AccessToken +from fastmcp.server.auth.providers.google import GoogleProvider, GoogleTokenVerifier +from fastmcp.server.dependencies import get_access_token +from key_value.aio.protocols import AsyncKeyValue + +log = logging.getLogger(__name__) + +ALLOWED_DOMAIN = "mozilla.com" + +SCOPES = ["openid", "https://www.googleapis.com/auth/userinfo.email"] + + +class UnauthorizedError(Exception): + """The request carries no identity this server accepts.""" + + +def mozilla_email(claims: dict[str, Any]) -> str | None: + """The caller's email if the claims are a verified Mozilla account, else None. + + Three checks, each of which alone is not enough: + - `hd` is the Workspace domain of the account. Only Google's userinfo + endpoint returns it (tokeninfo does not), and FastMCP ignores a failed + userinfo call, so a missing `hd` must fail closed. + - `email_verified`, which tokeninfo returns as the string "true". + - the email's own domain, so an account of the domain can't be signed in + under an alias of another one. + """ + user_data = claims.get("google_user_data") or {} + if user_data.get("hd") != ALLOWED_DOMAIN: + return None + if claims.get("email_verified") not in (True, "true"): + return None + email = claims.get("email") + if not isinstance(email, str): + return None + email = email.strip().lower() + local, at, domain = email.rpartition("@") + if not local or not at or domain != ALLOWED_DOMAIN: + return None + return email + + +class MozillaTokenVerifier(GoogleTokenVerifier): + """Google's verifier, accepting only verified accounts of `ALLOWED_DOMAIN`.""" + + def __init__(self, *, client_id: str, **kwargs: Any) -> None: + super().__init__(**kwargs) + self._client_id = client_id + + async def verify_token(self, token: str) -> AccessToken | None: + access_token = await super().verify_token(token) + if access_token is None: + return None + claims = access_token.claims + # The token must have been issued to this server's own OAuth client. + if claims.get("aud") != self._client_id: + log.warning("Rejected a Google token issued to another client") + return None + if mozilla_email(claims) is None: + log.info("Rejected a Google account outside %s", ALLOWED_DOMAIN) + return None + return access_token + + +class MozillaGoogleProvider(GoogleProvider): + """FastMCP's Google OAuth proxy, with `MozillaTokenVerifier` checking tokens. + + The proxy validates the upstream Google token on every request, through + the verifier, so swapping the verifier is what restricts every request. + """ + + def __init__( + self, + *, + client_id: str, + client_secret: str, + base_url: str, + jwt_signing_key: str, + client_storage: AsyncKeyValue, + http_client: httpx.AsyncClient | None = None, + ) -> None: + super().__init__( + client_id=client_id, + client_secret=client_secret, + base_url=base_url, + required_scopes=SCOPES, + jwt_signing_key=jwt_signing_key, + client_storage=client_storage, + # Only a hint for Google's account picker; the verifier decides. + extra_authorize_params={"hd": ALLOWED_DOMAIN}, + http_client=http_client, + ) + verifier = MozillaTokenVerifier( + client_id=client_id, + required_scopes=SCOPES, + http_client=http_client, + ) + # GoogleProvider builds its verifier itself. tests/test_auth.py pins + # that this attribute is the one the proxy checks tokens with, and + # every tool re-checks the claims (`current_user_email`) regardless. + if not isinstance(getattr(self, "_token_validator", None), GoogleTokenVerifier): + raise RuntimeError("FastMCP's OAuthProxy no longer has _token_validator") + self._token_validator = verifier + + +def current_user_email() -> str: + """The authenticated caller's email, re-checked against the domain rules.""" + token = get_access_token() + email = mozilla_email(token.claims) if token is not None else None + if email is None: + raise UnauthorizedError( + f"Sign in with a verified @{ALLOWED_DOMAIN} Google account." + ) + return email diff --git a/services/hackbot-mcp/src/hackbot_mcp/config.py b/services/hackbot-mcp/src/hackbot_mcp/config.py new file mode 100644 index 00000000000..9e53730ac26 --- /dev/null +++ b/services/hackbot-mcp/src/hackbot_mcp/config.py @@ -0,0 +1,68 @@ +from typing import Literal + +from pydantic import SecretStr, model_validator +from pydantic_settings import BaseSettings + + +class Settings(BaseSettings): + # Public URL of this server, as MCP clients reach it (no trailing /mcp). + # The OAuth metadata, the token audience and Google's redirect URI + # (/auth/callback) all derive from it, so it must be stable. + base_url: str + + # Google OAuth client (Web application) whose redirect URI is + # /auth/callback. + google_client_id: str + google_client_secret: SecretStr + + # Signs the tokens this server issues to MCP clients. + jwt_signing_key: SecretStr + # Encrypts the OAuth state at rest, which includes users' Google tokens. + storage_encryption_key: SecretStr + + # OAuth state storage, shared by every instance. A Cloud SQL instance in + # production; a plain Postgres URL locally; in memory only in development. + cloud_sql_instance: str = "" + cloud_sql_ip_type: Literal["PUBLIC", "PRIVATE", "PSC"] = "PUBLIC" + db_user: str = "" + db_pass: SecretStr = SecretStr("") + db_name: str = "hackbot_mcp" + database_url: SecretStr = SecretStr("") + + # hackbot-api. Without an API key, the client authenticates with this + # service's own Google identity (the metadata server's ID token, audience + # hackbot_api_url), which hackbot-api must list in ALLOWED_SERVICE_ACCOUNTS. + hackbot_api_url: str = "https://hackbot-api.moz.tools" + hackbot_api_key: SecretStr = SecretStr("") + hackbot_ui_url: str = "https://hackbot.moz.tools" + + port: int = 8080 + environment: str = "development" + sentry_dsn: str | None = None + + model_config = { + "env_file": ".env", + "env_file_encoding": "utf-8", + "extra": "ignore", + "hide_input_in_errors": True, + } + + @model_validator(mode="after") + def _check(self) -> "Settings": + for name in ("jwt_signing_key", "storage_encryption_key"): + if len(getattr(self, name).get_secret_value()) < 32: + raise ValueError(f"{name} must be at least 32 characters") + if ( + not self.cloud_sql_instance + and not self.database_url.get_secret_value() + and self.environment != "development" + ): + # Instances would each keep their own registrations and tokens, + # so clients would fail at random depending on the instance hit. + raise ValueError( + "CLOUD_SQL_INSTANCE or DATABASE_URL is required outside development" + ) + self.base_url = self.base_url.rstrip("/") + self.hackbot_api_url = self.hackbot_api_url.rstrip("/") + self.hackbot_ui_url = self.hackbot_ui_url.rstrip("/") + return self diff --git a/services/hackbot-mcp/src/hackbot_mcp/server.py b/services/hackbot-mcp/src/hackbot_mcp/server.py new file mode 100644 index 00000000000..e1194658f12 --- /dev/null +++ b/services/hackbot-mcp/src/hackbot_mcp/server.py @@ -0,0 +1,417 @@ +"""Remote MCP server for running Hackbot agents as the signed-in user. + +A proxy, like hackbot-ui: users sign in with Google (MCP OAuth), and this +server calls hackbot-api with its own credential, sending the signed-in +user's email as `X-On-Behalf-Of`. That email only ever comes from the +verified token, never from a tool argument. +""" + +import logging +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager +from typing import Annotated, Any +from uuid import UUID + +import httpx +import jsonschema +import sentry_sdk +from fastmcp import FastMCP +from fastmcp.exceptions import ToolError +from hackbot_client import ( + AgentDescriptor, + HackbotAPIError, + HackbotClient, + RunDoc, + RunStatus, +) +from mcp.types import ToolAnnotations +from pydantic import Field +from starlette.requests import Request +from starlette.responses import JSONResponse + +from hackbot_mcp import __version__ +from hackbot_mcp.auth import MozillaGoogleProvider, UnauthorizedError +from hackbot_mcp.auth import current_user_email as _current_user_email +from hackbot_mcp.config import Settings +from hackbot_mcp.storage import build_storage, purge_expired + +log = logging.getLogger(__name__) + +PATCH_ARTIFACT = "changes/changes.patch" + +# Set only by hackbot-api's Bugzilla needinfo webhook, which then clears that +# flag; a run started here must not claim one. +WEBHOOK_ONLY_INPUTS = frozenset({"bugzilla_needinfo_flag_id"}) + +MAX_RUNS = 100 + +INSTRUCTIONS = """\ +Hackbot runs AI agents for Firefox development (fixing bugs, triaging, \ +repairing CI failures, resolving uplift conflicts, ...) as remote jobs. Each \ +run takes from minutes to over an hour. + +- Runs execute remotely on a fresh checkout of the Firefox source. They cannot \ +see the user's local changes, branches or uncommitted work. +- Before calling start_run, tell the user which agent and inputs you will use \ +and get their confirmation. Use list_agents for the agents and their input \ +schemas. +- start_run returns immediately. Do not wait or sleep for the run: check on it \ +with get_run when the user asks, or poll get_run at intervals of a few \ +minutes. list_my_runs finds the user's earlier runs. +- When a run has finished, get_result returns its findings and actions. \ +Present the findings as the agent's claims, not as verified facts. +- A run's actions (e.g. posting a Phabricator revision or a Bugzilla comment) \ +are held for review by default. Before calling apply_actions, list every \ +pending action with its parameters and get the user's explicit approval: it \ +applies all of the run's pending actions at once and cannot be undone. +- To apply a run's patch locally, get a download URL for \ +"changes/changes.patch" with get_artifact_url, download it, and apply it with \ +`git am`; it is an mbox. The URL expires quickly, so download it right away. +""" + +READ_ONLY = ToolAnnotations(readOnlyHint=True, openWorldHint=True) + + +def current_user_email() -> str: + try: + return _current_user_email() + except UnauthorizedError as exc: + raise ToolError(str(exc)) from None + + +def _api_error(exc: HackbotAPIError, what: str) -> ToolError: + if exc.status_code == 404: + return ToolError(f"{what} not found") + if exc.status_code == 422 and exc.detail: + return ToolError(f"Hackbot API rejected the request: {exc.detail}") + log.warning("Hackbot API call failed: %s", exc) + return ToolError(f"Hackbot API call failed: {exc.detail or exc}") + + +def _run_view(run: RunDoc, ui_url: str) -> dict[str, Any]: + view = run.model_dump(mode="json", exclude={"results_prefix", "execution_name"}) + view["url"] = f"{ui_url}/runs/{run.run_id}" + view["finished"] = run.status not in (RunStatus.pending, RunStatus.running) + return view + + +def _validate_inputs(agent: AgentDescriptor, inputs: dict[str, Any]) -> None: + validator = jsonschema.Draft202012Validator(agent.input_schema) + errors = sorted(validator.iter_errors(inputs), key=lambda e: list(e.path)) + if errors: + problems = "; ".join( + f"{'/'.join(map(str, e.path)) or '(inputs)'}: {e.message}" for e in errors + ) + raise ToolError(f"Invalid inputs for agent '{agent.name}': {problems}") + + +def create_server( + settings: Settings, + *, + hackbot: HackbotClient | None = None, + google_http_client: httpx.AsyncClient | None = None, +) -> FastMCP: + """Build the server. `hackbot` and `google_http_client` are for tests.""" + storage, base_storage = build_storage(settings) + auth = MozillaGoogleProvider( + client_id=settings.google_client_id, + client_secret=settings.google_client_secret.get_secret_value(), + base_url=settings.base_url, + jwt_signing_key=settings.jwt_signing_key.get_secret_value(), + client_storage=storage, + http_client=google_http_client, + ) + client = hackbot or HackbotClient( + base_url=settings.hackbot_api_url, + api_key=settings.hackbot_api_key.get_secret_value(), + ) + ui_url = settings.hackbot_ui_url + + @asynccontextmanager + async def lifespan(server: FastMCP) -> AsyncIterator[dict]: + await purge_expired(base_storage) + try: + yield {} + finally: + await client.close() + + mcp = FastMCP( + "Hackbot", + instructions=INSTRUCTIONS, + version=__version__, + auth=auth, + lifespan=lifespan, + ) + + @mcp.custom_route("/health", methods=["GET"]) + async def health(request: Request) -> JSONResponse: + return JSONResponse({"status": "ok"}) + + async def _get_run(run_id: UUID, user: str) -> RunDoc: + try: + return await client.get_run(run_id, on_behalf_of=user) + except HackbotAPIError as exc: + raise _api_error(exc, f"Run {run_id}") from exc + + @mcp.tool(annotations=READ_ONLY) + async def list_agents() -> list[dict[str, Any]]: + """List the Hackbot agents: their names, descriptions and input schemas.""" + user = current_user_email() + try: + agents = await client.list_agents(on_behalf_of=user) + except HackbotAPIError as exc: + raise _api_error(exc, "Agents") from exc + return [agent.model_dump(mode="json") for agent in agents] + + @mcp.tool(annotations=ToolAnnotations(readOnlyHint=False, openWorldHint=True)) + async def start_run( + agent: Annotated[str, Field(description="Agent name, from list_agents.")], + inputs: Annotated[ + dict[str, Any], + Field(description="The agent's inputs, matching its input schema."), + ], + dedupe_key: Annotated[ + str | None, + Field( + description=( + "Optional key identifying the work. If this agent already " + "has a run with this key, that run is returned instead of " + "starting a new one." + ) + ), + ] = None, + auto_apply: Annotated[ + bool, + Field( + description=( + "Let the agent's actions (Phabricator, Bugzilla, ...) be " + "applied automatically when the run succeeds, as for " + "Hackbot's own triggers. Only with the user's explicit " + "request; by default actions wait for apply_actions." + ) + ), + ] = False, + ) -> dict[str, Any]: + """Start an agent run, returning as soon as it is queued. + + Runs take minutes to hours; check on one with get_run, never wait. + """ + user = current_user_email() + + if webhook_only := sorted(WEBHOOK_ONLY_INPUTS & inputs.keys()): + raise ToolError( + f"{', '.join(webhook_only)}: set by Hackbot's webhooks only" + ) + if "require_review" in inputs: + raise ToolError( + "Don't set 'require_review'; actions are held for review unless " + "auto_apply is true" + ) + + try: + agents = {a.name: a for a in await client.list_agents(on_behalf_of=user)} + except HackbotAPIError as exc: + raise _api_error(exc, "Agents") from exc + spec = agents.get(agent) + if spec is None: + raise ToolError( + f"Unknown agent '{agent}'. Available: {', '.join(sorted(agents))}" + ) + + payload = {**inputs, "require_review": not auto_apply} + _validate_inputs(spec, payload) + + try: + run = await client.trigger_run( + agent, payload, on_behalf_of=user, dedupe_key=dedupe_key + ) + except HackbotAPIError as exc: + raise _api_error(exc, f"Agent '{agent}'") from exc + + return { + "run_id": str(run.run_id), + "agent": run.agent, + "status": run.status.value, + # False: the dedupe key matched an existing run, which may be + # someone else's and may already have finished. + "is_new": run.is_new, + "require_review": payload["require_review"], + "url": f"{ui_url}/runs/{run.run_id}", + } + + @mcp.tool(annotations=READ_ONLY) + async def get_run(run_id: UUID) -> dict[str, Any]: + """Get a run's current state: status, inputs, summary and artifacts.""" + user = current_user_email() + return _run_view(await _get_run(run_id, user), ui_url) + + @mcp.tool(annotations=READ_ONLY) + async def list_my_runs( + agent: str | None = None, + status: RunStatus | None = None, + limit: Annotated[int, Field(ge=1, le=MAX_RUNS)] = 20, + ) -> list[dict[str, Any]]: + """List the signed-in user's runs, newest first.""" + user = current_user_email() + try: + runs = await client.list_runs( + agent=agent, + status=status, + requested_by=user, + limit=limit, + on_behalf_of=user, + ) + except HackbotAPIError as exc: + raise _api_error(exc, "Runs") from exc + return [ + { + "run_id": str(run.run_id), + "agent": run.agent, + "status": run.status.value, + "inputs": run.inputs, + "created_at": run.created_at.isoformat(), + "updated_at": run.updated_at.isoformat(), + "error": run.error, + "url": f"{ui_url}/runs/{run.run_id}", + } + for run in runs + ] + + @mcp.tool(annotations=READ_ONLY) + async def get_result(run_id: UUID) -> dict[str, Any]: + """Get a run's findings, artifacts and actions, with each action's state. + + Actions with status "pending" have not been applied; apply_actions + applies them, after the user approves. + """ + user = current_user_email() + run = await _get_run(run_id, user) + try: + actions = await client.list_actions(run_id, on_behalf_of=user) + except HackbotAPIError as exc: + raise _api_error(exc, f"Run {run_id}") from exc + + recorded = [action.model_dump(mode="json") for action in actions] + proposed = run.summary.actions if run.summary else [] + if not recorded and proposed: + # Not recorded by the API yet (it does once it has processed the + # finished run), so none of them can have been applied. + recorded = [ + { + "idx": idx, + "type": action.get("type"), + "params": action.get("params", {}), + "ref": action.get("ref"), + "status": "pending", + } + for idx, action in enumerate(proposed) + ] + + return { + "run_id": str(run.run_id), + "agent": run.agent, + "status": run.status.value, + "finished": run.status not in (RunStatus.pending, RunStatus.running), + "error": run.error or (run.summary.error if run.summary else None), + "findings": run.summary.findings if run.summary else {}, + "artifacts": [artifact.name for artifact in run.artifacts], + "actions": recorded, + "url": f"{ui_url}/runs/{run.run_id}", + } + + @mcp.tool(annotations=READ_ONLY) + async def get_artifact_url( + run_id: UUID, + artifact: Annotated[ + str, + Field(description="Artifact name, as listed by get_run or get_result."), + ], + ) -> dict[str, Any]: + """Get a short-lived signed URL to download one of a run's artifacts.""" + user = current_user_email() + try: + url = await client.get_artifact_url(run_id, artifact, on_behalf_of=user) + except HackbotAPIError as exc: + raise _api_error(exc, f"Artifact '{artifact}' of run {run_id}") from exc + result: dict[str, Any] = {"artifact": artifact, "url": url} + if artifact == PATCH_ARTIFACT: + result["note"] = ( + "An mbox of the run's commits: download it (the URL expires " + "soon), e.g. `curl -fsSL -o changes.patch ''`, then apply " + "it with `git am changes.patch`." + ) + return result + + @mcp.tool( + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=True, + idempotentHint=True, + openWorldHint=True, + ) + ) + async def apply_actions(run_id: UUID) -> dict[str, Any]: + """Apply ALL of a run's pending actions at once. Cannot be undone. + + Only after listing every pending action and its parameters to the + user (get_result) and getting their explicit approval. Only the user + who started the run may apply its actions. + """ + user = current_user_email() + run = await _get_run(run_id, user) + if (run.requested_by or "").strip().lower() != user: + raise ToolError( + "Only the user who started this run can apply its actions; " + "use the Hackbot UI otherwise" + ) + if run.status != RunStatus.succeeded: + raise ToolError( + f"The run has status '{run.status.value}'; only a succeeded " + "run's actions can be applied" + ) + try: + actions = await client.apply_actions(run_id, on_behalf_of=user) + except HackbotAPIError as exc: + raise _api_error(exc, f"Run {run_id}") from exc + return { + "run_id": str(run_id), + "all_applied": actions.all_applied, + "actions": [ + { + "idx": action.idx, + "type": action.type, + "status": action.status, + "result": action.result, + "error": action.error, + } + for action in actions + ], + } + + return mcp + + +def main() -> None: + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", + ) + settings = Settings() + if settings.sentry_dsn: + sentry_sdk.init( + dsn=settings.sentry_dsn, + environment=settings.environment, + send_default_pii=True, + ) + create_server(settings).run( + transport="streamable-http", + host="0.0.0.0", + port=settings.port, + # Cloud Run spreads a client's requests across instances, which share + # no session state. + stateless_http=True, + ) + + +if __name__ == "__main__": + main() diff --git a/services/hackbot-mcp/src/hackbot_mcp/storage.py b/services/hackbot-mcp/src/hackbot_mcp/storage.py new file mode 100644 index 00000000000..3abb8e31744 --- /dev/null +++ b/services/hackbot-mcp/src/hackbot_mcp/storage.py @@ -0,0 +1,98 @@ +"""Shared storage for the OAuth proxy's state. + +The proxy keeps client registrations, in-flight authorizations (PKCE), and +the Google tokens behind the tokens it issues. Cloud Run runs several +instances, and a client's requests land on any of them, so the state lives in +Postgres: a database of its own on hackbot's Cloud SQL instance, reached the +same way hackbot-api reaches its own. + +Everything is encrypted with a key of this service's, since the Google tokens +are users' credentials. FastMCP only does that for its default (local file) +store, not for a store passed to it. +""" + +import asyncio +import logging + +import asyncpg +from google.cloud.sql.connector import Connector +from key_value.aio.protocols import AsyncKeyValue +from key_value.aio.stores.memory import MemoryStore +from key_value.aio.stores.postgresql import PostgreSQLStore +from key_value.aio.wrappers.encryption import FernetEncryptionWrapper + +from hackbot_mcp.config import Settings + +log = logging.getLogger(__name__) + +TABLE = "oauth_state" + + +class CloudSQLStore(PostgreSQLStore): + """`PostgreSQLStore` connecting through the Cloud SQL Python connector. + + The pool is created on first use, inside the server's event loop, which + the connector needs. + """ + + def __init__(self, settings: Settings) -> None: + super().__init__(table_name=TABLE) + self._settings = settings + + async def _create_pool(self) -> asyncpg.Pool: + settings = self._settings + connector = Connector(loop=asyncio.get_running_loop()) + self._exit_stack.push_async_callback(connector.close_async) + + async def connect(*args, **kwargs) -> asyncpg.Connection: + return await connector.connect_async( + settings.cloud_sql_instance, + "asyncpg", + user=settings.db_user, + password=settings.db_pass.get_secret_value(), + db=settings.db_name, + ip_type=settings.cloud_sql_ip_type, + ) + + return await asyncpg.create_pool(connect=connect, min_size=1, max_size=5) + + +async def purge_expired(store: AsyncKeyValue) -> None: + """Delete entries past their expiry. + + Reads already ignore them; this only keeps the table from growing, so a + failure is logged rather than raised. + """ + if not isinstance(store, PostgreSQLStore): + return + try: + await store.setup() + result = await store._initialized_pool.execute( + f"DELETE FROM {TABLE} WHERE expires_at < now()" + ) + log.info("Purged expired OAuth state (%s)", result) + except Exception: + log.exception("Failed to purge expired OAuth state") + + +def build_storage(settings: Settings) -> tuple[AsyncKeyValue, AsyncKeyValue]: + """The encrypted store to give the OAuth proxy, and the store under it.""" + if settings.cloud_sql_instance: + base: AsyncKeyValue = CloudSQLStore(settings) + elif url := settings.database_url.get_secret_value(): + base = PostgreSQLStore(url=url, table_name=TABLE) + else: + # Development only (Settings refuses it elsewhere): one process, and + # every restart signs everyone out. + log.warning("Keeping OAuth state in memory; for development only") + base = MemoryStore() + + encrypted = FernetEncryptionWrapper( + key_value=base, + source_material=settings.storage_encryption_key.get_secret_value(), + salt="hackbot-mcp-oauth-state", + # A key rotation then reads old entries as missing, which signs users + # out instead of failing their requests. + raise_on_decryption_error=False, + ) + return encrypted, base diff --git a/services/hackbot-mcp/tests/conftest.py b/services/hackbot-mcp/tests/conftest.py new file mode 100644 index 00000000000..d1fa39ba47c --- /dev/null +++ b/services/hackbot-mcp/tests/conftest.py @@ -0,0 +1,160 @@ +from collections.abc import Callable +from datetime import datetime, timezone +from typing import Any + +import httpx +import pytest +from fastmcp import Client +from fastmcp.server.auth import AccessToken +from hackbot_client import HackbotClient +from hackbot_mcp import auth +from hackbot_mcp.config import Settings +from hackbot_mcp.server import create_server + +API = "https://hackbot-api.example" +UI = "https://hackbot.example" +USER = "dev@mozilla.com" +RUN_ID = "d3d5f21d-d716-4bb0-a812-8c9ef3e2f1c6" + +BUG_FIX_SCHEMA = { + "type": "object", + "properties": { + "require_review": {"type": "boolean", "default": False}, + "bug_id": {"type": "integer"}, + "revision_id": {"anyOf": [{"type": "integer"}, {"type": "null"}]}, + "comment": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + "bugzilla_needinfo_flag_id": { + "anyOf": [{"type": "integer", "exclusiveMinimum": 0}, {"type": "null"}] + }, + }, + "required": ["bug_id"], +} + +AGENTS = [ + { + "name": "bug-fix", + "description": "Fix a bug.", + "input_schema": BUG_FIX_SCHEMA, + } +] + + +def settings(**overrides: Any) -> Settings: + values = { + "base_url": "https://hackbot-mcp.example", + "google_client_id": "client-id.apps.googleusercontent.com", + "google_client_secret": "client-secret", + "jwt_signing_key": "j" * 32, + "storage_encryption_key": "s" * 32, + "hackbot_api_url": API, + "hackbot_api_key": "test-key", + "hackbot_ui_url": UI, + "environment": "development", + } + values.update(overrides) + return Settings(_env_file=None, **values) + + +def run_doc(**overrides: Any) -> dict[str, Any]: + now = datetime(2026, 11, 12, tzinfo=timezone.utc).isoformat() + doc = { + "run_id": RUN_ID, + "agent": "bug-fix", + "status": "succeeded", + "inputs": {"bug_id": 1, "require_review": True}, + "requested_by": USER, + "requested_via": "hackbot-mcp@example.iam.gserviceaccount.com", + "dedupe_key": None, + "created_at": now, + "updated_at": now, + "execution_name": "projects/p/executions/e", + "results_prefix": f"runs/{RUN_ID}", + "summary": {"status": "ok", "findings": {}, "actions": []}, + "artifacts": [], + "error": None, + } + doc.update(overrides) + return doc + + +def claims(**overrides: Any) -> dict[str, Any]: + values = { + "sub": "1234", + "aud": "client-id.apps.googleusercontent.com", + "email": USER, + "email_verified": "true", + "google_user_data": { + "email": USER, + "verified_email": True, + "hd": "mozilla.com", + }, + } + values.update(overrides) + return values + + +class FakeAPI: + """hackbot-api behind an httpx MockTransport: routes and recorded requests.""" + + def __init__(self) -> None: + self.requests: list[httpx.Request] = [] + self.routes: dict[ + tuple[str, str], Callable[[httpx.Request], httpx.Response] + ] = { + ("GET", "/agents"): lambda r: httpx.Response(200, json=AGENTS), + ("GET", f"/runs/{RUN_ID}"): lambda r: httpx.Response(200, json=run_doc()), + ("GET", f"/runs/{RUN_ID}/actions"): lambda r: httpx.Response(200, json=[]), + ("GET", "/runs"): lambda r: httpx.Response(200, json=[run_doc()]), + } + + def route(self, method: str, path: str, response: Any) -> None: + self.routes[(method, path)] = ( + response if callable(response) else (lambda r: response) + ) + + def handler(self, request: httpx.Request) -> httpx.Response: + self.requests.append(request) + respond = self.routes.get((request.method, request.url.path)) + if respond is None: + return httpx.Response(404, json={"detail": "Not found"}) + return respond(request) + + def sent(self, method: str, path: str) -> list[httpx.Request]: + return [r for r in self.requests if r.method == method and r.url.path == path] + + +@pytest.fixture +def api() -> FakeAPI: + return FakeAPI() + + +@pytest.fixture +def hackbot(api: FakeAPI) -> HackbotClient: + client = HackbotClient(base_url=API, api_key="test-key") + client._http = httpx.AsyncClient(transport=httpx.MockTransport(api.handler)) + return client + + +@pytest.fixture +def signed_in(monkeypatch) -> Callable[..., None]: + """Make tools see an access token with the given claims (default: USER).""" + + def sign_in(**overrides: Any) -> None: + token = AccessToken( + token="t", client_id="1234", scopes=[], claims=claims(**overrides) + ) + monkeypatch.setattr(auth, "get_access_token", lambda: token) + + sign_in() + return sign_in + + +@pytest.fixture +def server(hackbot: HackbotClient): + return create_server(settings(), hackbot=hackbot) + + +@pytest.fixture +async def mcp_client(server): + async with Client(server) as client: + yield client diff --git a/services/hackbot-mcp/tests/test_annotations.py b/services/hackbot-mcp/tests/test_annotations.py new file mode 100644 index 00000000000..fc8d93148f9 --- /dev/null +++ b/services/hackbot-mcp/tests/test_annotations.py @@ -0,0 +1,44 @@ +"""Tool annotations: the client's permission prompt is the user's confirmation.""" + +import pytest + +READ_ONLY = ["get_artifact_url", "get_result", "get_run", "list_agents", "list_my_runs"] + + +async def test_every_tool_is_covered(server): + names = {tool.name for tool in await server.list_tools()} + assert names == {*READ_ONLY, "start_run", "apply_actions"} + + +@pytest.mark.parametrize("name", READ_ONLY) +async def test_reads_are_read_only(name, server): + tool = await server.get_tool(name) + assert tool.annotations.readOnlyHint is True + + +async def test_start_run_is_not_read_only(server): + tool = await server.get_tool("start_run") + assert tool.annotations.readOnlyHint is False + # With auto_apply it can act on Bugzilla and Phabricator, so it doesn't + # claim to be non-destructive. + assert tool.annotations.destructiveHint is not False + + +async def test_apply_actions_is_destructive(server): + tool = await server.get_tool("apply_actions") + assert tool.annotations.readOnlyHint is False + assert tool.annotations.destructiveHint is True + + +async def test_instructions_cover_the_workflow(server): + instructions = server.instructions + for phrase in ( + "fresh checkout", + "get their confirmation", + "get_run", + "agent's claims", + "explicit approval", + "cannot be undone", + "git am", + ): + assert phrase in instructions diff --git a/services/hackbot-mcp/tests/test_auth.py b/services/hackbot-mcp/tests/test_auth.py new file mode 100644 index 00000000000..67102fbb685 --- /dev/null +++ b/services/hackbot-mcp/tests/test_auth.py @@ -0,0 +1,170 @@ +"""Sign-in: only verified @mozilla.com Google accounts get through.""" + +import httpx +import pytest +from conftest import USER, claims, settings +from fastmcp.server.auth.providers.google import GoogleTokenVerifier +from hackbot_mcp import auth +from hackbot_mcp.auth import MozillaGoogleProvider, MozillaTokenVerifier, mozilla_email +from hackbot_mcp.server import create_server +from hackbot_mcp.storage import build_storage + +CLIENT_ID = "client-id.apps.googleusercontent.com" + + +def google(tokeninfo: dict | None, userinfo: dict | None) -> httpx.AsyncClient: + """Google's tokeninfo and userinfo endpoints, answering with these bodies.""" + + def handler(request: httpx.Request) -> httpx.Response: + body = tokeninfo if request.url.path == "/tokeninfo" else userinfo + if body is None: + return httpx.Response(400, json={"error": "invalid_token"}) + return httpx.Response(200, json=body) + + return httpx.AsyncClient(transport=httpx.MockTransport(handler)) + + +def tokeninfo(**overrides) -> dict: + values = { + "aud": CLIENT_ID, + "sub": "1234", + "scope": "openid https://www.googleapis.com/auth/userinfo.email", + "email": USER, + "email_verified": "true", + "expires_in": "3599", + } + values.update(overrides) + return {k: v for k, v in values.items() if v is not None} + + +def userinfo(**overrides) -> dict: + values = {"id": "1234", "email": USER, "verified_email": True, "hd": "mozilla.com"} + values.update(overrides) + return {k: v for k, v in values.items() if v is not None} + + +def verifier(http_client: httpx.AsyncClient) -> MozillaTokenVerifier: + return MozillaTokenVerifier( + client_id=CLIENT_ID, required_scopes=auth.SCOPES, http_client=http_client + ) + + +async def test_a_verified_mozilla_account_is_accepted(): + token = await verifier(google(tokeninfo(), userinfo())).verify_token("t") + assert token is not None + assert mozilla_email(token.claims) == USER + + +@pytest.mark.parametrize( + ("info", "user"), + [ + pytest.param(tokeninfo(), userinfo(hd="example.com"), id="other-hd"), + pytest.param(tokeninfo(), userinfo(hd=None), id="no-hd"), + pytest.param(tokeninfo(), None, id="userinfo-failed"), + pytest.param( + tokeninfo(email="dev@gmail.com"), + userinfo(email="dev@gmail.com", hd=None), + id="gmail", + ), + pytest.param( + tokeninfo(email="dev@mozilla.com.evil.example"), + userinfo(email="dev@mozilla.com.evil.example"), + id="suffix-lookalike", + ), + pytest.param( + tokeninfo(email_verified="false"), + userinfo(verified_email=False), + id="unverified", + ), + pytest.param( + tokeninfo(email_verified=None), + userinfo(verified_email=None), + id="verification-unknown", + ), + pytest.param(tokeninfo(aud="another-client"), userinfo(), id="other-client"), + pytest.param(None, userinfo(), id="invalid-token"), + ], +) +async def test_other_accounts_are_rejected(info, user): + assert await verifier(google(info, user)).verify_token("t") is None + + +@pytest.mark.parametrize( + "overrides", + [ + {"google_user_data": None}, + {"google_user_data": {"hd": "example.com"}}, + {"email_verified": False}, + {"email_verified": "false"}, + {"email": "dev@example.com"}, + {"email": None}, + {"email": "@mozilla.com"}, + ], +) +def test_mozilla_email_fails_closed(overrides): + assert mozilla_email(claims(**overrides)) is None + + +def test_mozilla_email_normalizes_case(): + assert mozilla_email(claims(email=" Dev@Mozilla.COM ")) == USER + + +def test_the_proxy_checks_tokens_with_the_mozilla_verifier(): + # Pins the FastMCP internal the provider swaps: if a FastMCP upgrade + # renames it, this fails instead of every Google account being accepted + # (the tools would still refuse them, but the proxy would not). + storage, _ = build_storage(settings()) + provider = MozillaGoogleProvider( + client_id=CLIENT_ID, + client_secret="secret", + base_url="https://hackbot-mcp.example", + jwt_signing_key="j" * 32, + client_storage=storage, + ) + assert isinstance(provider._token_validator, MozillaTokenVerifier) + assert isinstance(provider._token_validator, GoogleTokenVerifier) + + +async def test_mcp_endpoint_requires_a_token(hackbot): + app = create_server(settings(), hackbot=hackbot).http_app(stateless_http=True) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, base_url="https://hackbot-mcp.example" + ) as client: + response = await client.post( + "/mcp", + json={"jsonrpc": "2.0", "id": 1, "method": "tools/list"}, + headers={"Accept": "application/json, text/event-stream"}, + ) + bad = await client.post( + "/mcp", + json={"jsonrpc": "2.0", "id": 1, "method": "tools/list"}, + headers={ + "Accept": "application/json, text/event-stream", + "Authorization": "Bearer not-a-token", + }, + ) + metadata = await client.get("/.well-known/oauth-protected-resource/mcp") + + assert response.status_code == 401 + assert "resource_metadata" in response.headers["www-authenticate"] + assert bad.status_code == 401 + assert metadata.status_code == 200 + + +async def test_tools_refuse_without_a_token(mcp_client): + from fastmcp.exceptions import ToolError + + with pytest.raises(ToolError, match="mozilla.com"): + await mcp_client.call_tool("list_agents", {}) + + +def test_storage_is_required_outside_development(): + with pytest.raises(ValueError, match="CLOUD_SQL_INSTANCE or DATABASE_URL"): + settings(environment="production") + assert settings(environment="production", cloud_sql_instance="p:r:i") + + +def test_short_keys_are_refused(): + with pytest.raises(ValueError, match="jwt_signing_key"): + settings(jwt_signing_key="short") diff --git a/services/hackbot-mcp/tests/test_tools.py b/services/hackbot-mcp/tests/test_tools.py new file mode 100644 index 00000000000..4f5b6186034 --- /dev/null +++ b/services/hackbot-mcp/tests/test_tools.py @@ -0,0 +1,384 @@ +"""The tools, against a mocked hackbot-api.""" + +import json + +import httpx +import pytest +from conftest import RUN_ID, UI, USER, run_doc +from fastmcp.exceptions import ToolError + +STARTED = {"run_id": RUN_ID, "agent": "bug-fix", "status": "pending"} + +TOOL_CALLS = { + "list_agents": {}, + "start_run": {"agent": "bug-fix", "inputs": {"bug_id": 1}}, + "get_run": {"run_id": RUN_ID}, + "list_my_runs": {}, + "get_result": {"run_id": RUN_ID}, + "get_artifact_url": {"run_id": RUN_ID, "artifact": "changes/changes.patch"}, + "apply_actions": {"run_id": RUN_ID}, +} + + +def _route_everything(api): + api.route("POST", "/agents/bug-fix/runs", httpx.Response(201, json=STARTED)) + api.route( + "GET", + f"/runs/{RUN_ID}/artifacts/changes/changes.patch", + httpx.Response(200, json={"url": "https://storage.example/signed"}), + ) + api.route("POST", f"/runs/{RUN_ID}/actions/apply", httpx.Response(200, json=[])) + + +@pytest.mark.parametrize("tool", sorted(TOOL_CALLS)) +async def test_every_call_is_on_behalf_of_the_authenticated_user( + tool, api, signed_in, mcp_client +): + _route_everything(api) + signed_in(email="Dev@Mozilla.com") + + await mcp_client.call_tool(tool, TOOL_CALLS[tool]) + + assert api.requests + for request in api.requests: + assert request.headers["X-On-Behalf-Of"] == USER + assert request.headers["X-API-Key"] == "test-key" + + +async def test_no_tool_takes_an_identity_argument(server): + for tool in await server.list_tools(): + params = set(tool.parameters.get("properties", {})) + assert not params & {"email", "user", "on_behalf_of", "requested_by"}, tool.name + + +@pytest.mark.parametrize("tool", sorted(TOOL_CALLS)) +async def test_tools_refuse_a_caller_without_a_mozilla_identity( + tool, api, signed_in, mcp_client +): + signed_in(google_user_data={"hd": "example.com"}) + + with pytest.raises(ToolError, match="mozilla.com"): + await mcp_client.call_tool(tool, TOOL_CALLS[tool]) + assert api.requests == [] + + +async def test_start_run_holds_actions_for_review_by_default( + api, signed_in, mcp_client +): + api.route("POST", "/agents/bug-fix/runs", httpx.Response(201, json=STARTED)) + + result = await mcp_client.call_tool( + "start_run", {"agent": "bug-fix", "inputs": {"bug_id": 1}} + ) + + (post,) = api.sent("POST", "/agents/bug-fix/runs") + assert json.loads(post.content) == {"bug_id": 1, "require_review": True} + assert result.data["require_review"] is True + + +async def test_start_run_auto_apply_turns_review_off(api, signed_in, mcp_client): + api.route("POST", "/agents/bug-fix/runs", httpx.Response(201, json=STARTED)) + + await mcp_client.call_tool( + "start_run", {"agent": "bug-fix", "inputs": {"bug_id": 1}, "auto_apply": True} + ) + + (post,) = api.sent("POST", "/agents/bug-fix/runs") + assert json.loads(post.content)["require_review"] is False + + +@pytest.mark.parametrize("value", [True, False]) +async def test_start_run_rejects_require_review_in_inputs( + value, api, signed_in, mcp_client +): + with pytest.raises(ToolError, match="auto_apply"): + await mcp_client.call_tool( + "start_run", + {"agent": "bug-fix", "inputs": {"bug_id": 1, "require_review": value}}, + ) + assert api.sent("POST", "/agents/bug-fix/runs") == [] + + +async def test_start_run_rejects_the_needinfo_input(api, signed_in, mcp_client): + with pytest.raises(ToolError, match="bugzilla_needinfo_flag_id"): + await mcp_client.call_tool( + "start_run", + { + "agent": "bug-fix", + "inputs": { + "bug_id": 1, + "comment": "please", + "bugzilla_needinfo_flag_id": 7, + }, + }, + ) + assert api.sent("POST", "/agents/bug-fix/runs") == [] + + +async def test_start_run_validates_inputs_against_the_agent_schema( + api, signed_in, mcp_client +): + with pytest.raises(ToolError, match="bug_id"): + await mcp_client.call_tool( + "start_run", {"agent": "bug-fix", "inputs": {"bug_id": "one"}} + ) + with pytest.raises(ToolError, match="'bug_id' is a required property"): + await mcp_client.call_tool("start_run", {"agent": "bug-fix", "inputs": {}}) + assert api.sent("POST", "/agents/bug-fix/runs") == [] + + +async def test_start_run_rejects_an_unknown_agent(api, signed_in, mcp_client): + with pytest.raises(ToolError, match="Unknown agent 'nope'.*bug-fix"): + await mcp_client.call_tool("start_run", {"agent": "nope", "inputs": {}}) + + +async def test_start_run_reports_the_api_validation_detail(api, signed_in, mcp_client): + api.route( + "POST", + "/agents/bug-fix/runs", + httpx.Response(422, json={"detail": "comment requires revision_id"}), + ) + with pytest.raises(ToolError, match="comment requires revision_id"): + await mcp_client.call_tool( + "start_run", {"agent": "bug-fix", "inputs": {"bug_id": 1, "comment": "x"}} + ) + + +@pytest.mark.parametrize(("status_code", "is_new"), [(201, True), (200, False)]) +async def test_start_run_says_whether_the_dedupe_key_matched_a_run( + status_code, is_new, api, signed_in, mcp_client +): + api.route("POST", "/agents/bug-fix/runs", httpx.Response(status_code, json=STARTED)) + + result = await mcp_client.call_tool( + "start_run", + {"agent": "bug-fix", "inputs": {"bug_id": 1}, "dedupe_key": "bug-1"}, + ) + + (post,) = api.sent("POST", "/agents/bug-fix/runs") + assert post.url.params["dedupe_key"] == "bug-1" + assert result.data["is_new"] is is_new + assert result.data["run_id"] == RUN_ID + assert result.data["url"] == f"{UI}/runs/{RUN_ID}" + + +async def test_get_run_returns_the_run_and_whether_it_finished( + api, signed_in, mcp_client +): + api.route( + "GET", f"/runs/{RUN_ID}", httpx.Response(200, json=run_doc(status="running")) + ) + + result = await mcp_client.call_tool("get_run", {"run_id": RUN_ID}) + + assert result.data["status"] == "running" + assert result.data["finished"] is False + assert "results_prefix" not in result.data + + +async def test_get_run_reports_an_unknown_run(api, signed_in, mcp_client): + api.route( + "GET", f"/runs/{RUN_ID}", httpx.Response(404, json={"detail": "Run not found"}) + ) + with pytest.raises(ToolError, match="not found"): + await mcp_client.call_tool("get_run", {"run_id": RUN_ID}) + + +async def test_list_my_runs_filters_on_the_caller(api, signed_in, mcp_client): + result = await mcp_client.call_tool( + "list_my_runs", {"agent": "bug-fix", "status": "running", "limit": 5} + ) + + (request,) = api.sent("GET", "/runs") + assert dict(request.url.params) == { + "agent": "bug-fix", + "status": "running", + "requested_by": USER, + "limit": "5", + } + assert [run["run_id"] for run in result.data] == [RUN_ID] + + +async def test_list_my_runs_caps_the_limit(api, signed_in, mcp_client): + with pytest.raises(ToolError): + await mcp_client.call_tool("list_my_runs", {"limit": 101}) + + +async def test_get_result_reports_summary_actions_as_pending_until_recorded( + api, signed_in, mcp_client +): + proposed = [ + {"type": "bugzilla.comment", "params": {"bug_id": 1, "text": "hi"}}, + {"type": "phabricator.submit", "params": {"bug_id": 1}, "ref": "patch"}, + ] + api.route( + "GET", + f"/runs/{RUN_ID}", + httpx.Response( + 200, + json=run_doc( + summary={"status": "ok", "findings": {"x": 1}, "actions": proposed} + ), + ), + ) + + result = await mcp_client.call_tool("get_result", {"run_id": RUN_ID}) + + assert result.data["findings"] == {"x": 1} + assert result.data["actions"] == [ + { + "idx": 0, + "type": "bugzilla.comment", + "params": {"bug_id": 1, "text": "hi"}, + "ref": None, + "status": "pending", + }, + { + "idx": 1, + "type": "phabricator.submit", + "params": {"bug_id": 1}, + "ref": "patch", + "status": "pending", + }, + ] + + +async def test_get_result_prefers_the_recorded_actions(api, signed_in, mcp_client): + api.route( + "GET", + f"/runs/{RUN_ID}", + httpx.Response( + 200, + json=run_doc( + summary={ + "status": "ok", + "findings": {}, + "actions": [{"type": "bugzilla.comment", "params": {}}], + } + ), + ), + ) + api.route( + "GET", + f"/runs/{RUN_ID}/actions", + httpx.Response( + 200, + json=[ + { + "idx": 0, + "type": "bugzilla.comment", + "params": {}, + "status": "applied", + "result": {"comment_id": 9}, + } + ], + ), + ) + + result = await mcp_client.call_tool("get_result", {"run_id": RUN_ID}) + + (action,) = result.data["actions"] + assert action["status"] == "applied" + assert action["result"] == {"comment_id": 9} + + +async def test_get_artifact_url_explains_how_to_apply_the_patch( + api, signed_in, mcp_client +): + _route_everything(api) + + result = await mcp_client.call_tool( + "get_artifact_url", {"run_id": RUN_ID, "artifact": "changes/changes.patch"} + ) + + assert result.data["url"] == "https://storage.example/signed" + assert "git am" in result.data["note"] + assert "mbox" in result.data["note"] + + +async def test_get_artifact_url_has_no_note_for_other_artifacts( + api, signed_in, mcp_client +): + api.route( + "GET", + f"/runs/{RUN_ID}/artifacts/findings.md", + httpx.Response(200, json={"url": "https://storage.example/f"}), + ) + + result = await mcp_client.call_tool( + "get_artifact_url", {"run_id": RUN_ID, "artifact": "findings.md"} + ) + + assert "note" not in result.data + + +async def test_apply_actions_returns_each_outcome(api, signed_in, mcp_client): + api.route( + "POST", + f"/runs/{RUN_ID}/actions/apply", + httpx.Response( + 200, + json=[ + { + "idx": 0, + "type": "bugzilla.comment", + "params": {"text": "hi"}, + "status": "applied", + "result": {"comment_id": 9}, + }, + { + "idx": 1, + "type": "phabricator.submit", + "params": {}, + "status": "failed", + "error": "conflict", + }, + ], + ), + ) + + result = await mcp_client.call_tool("apply_actions", {"run_id": RUN_ID}) + + assert result.data["all_applied"] is False + assert result.data["actions"] == [ + { + "idx": 0, + "type": "bugzilla.comment", + "status": "applied", + "result": {"comment_id": 9}, + "error": None, + }, + { + "idx": 1, + "type": "phabricator.submit", + "status": "failed", + "result": None, + "error": "conflict", + }, + ] + + +@pytest.mark.parametrize("requested_by", ["someone.else@mozilla.com", None]) +async def test_apply_actions_is_only_for_the_runs_requester( + requested_by, api, signed_in, mcp_client +): + api.route( + "GET", + f"/runs/{RUN_ID}", + httpx.Response(200, json=run_doc(requested_by=requested_by)), + ) + + with pytest.raises(ToolError, match="Only the user who started this run"): + await mcp_client.call_tool("apply_actions", {"run_id": RUN_ID}) + assert api.sent("POST", f"/runs/{RUN_ID}/actions/apply") == [] + + +@pytest.mark.parametrize("status", ["pending", "running", "failed", "timed_out"]) +async def test_apply_actions_needs_a_succeeded_run(status, api, signed_in, mcp_client): + api.route( + "GET", f"/runs/{RUN_ID}", httpx.Response(200, json=run_doc(status=status)) + ) + + with pytest.raises(ToolError, match="succeeded"): + await mcp_client.call_tool("apply_actions", {"run_id": RUN_ID}) + assert api.sent("POST", f"/runs/{RUN_ID}/actions/apply") == [] diff --git a/uv.lock b/uv.lock index 133589f7df0..f8b2f62cfdf 100644 --- a/uv.lock +++ b/uv.lock @@ -31,6 +31,7 @@ members = [ "hackbot-api", "hackbot-client", "hackbot-deploy", + "hackbot-mcp", "hackbot-pulse-listener", "hackbot-runtime", "lando-client", @@ -2846,6 +2847,43 @@ requires-dist = [ ] provides-extras = ["dev"] +[[package]] +name = "hackbot-mcp" +source = { editable = "services/hackbot-mcp" } +dependencies = [ + { name = "cloud-sql-python-connector", extra = ["asyncpg"] }, + { name = "fastmcp" }, + { name = "hackbot-client" }, + { name = "httpx" }, + { name = "jsonschema" }, + { name = "py-key-value-aio", extra = ["postgresql", "wrappers-encryption"] }, + { name = "pydantic" }, + { name = "pydantic-settings" }, + { name = "sentry-sdk" }, +] + +[package.optional-dependencies] +dev = [ + { name = "pytest" }, + { name = "pytest-asyncio" }, +] + +[package.metadata] +requires-dist = [ + { name = "cloud-sql-python-connector", extras = ["asyncpg"], specifier = ">=1.20.3" }, + { name = "fastmcp", specifier = ">=3.4.7" }, + { name = "hackbot-client", editable = "libs/hackbot-client" }, + { name = "httpx", specifier = ">=0.28.1" }, + { name = "jsonschema", specifier = ">=4.23.0" }, + { name = "py-key-value-aio", extras = ["postgresql", "wrappers-encryption"], specifier = ">=0.4.5,<0.5.0" }, + { name = "pydantic", specifier = ">=2.13.0" }, + { name = "pydantic-settings", specifier = ">=2.13.0" }, + { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.3" }, + { name = "pytest-asyncio", marker = "extra == 'dev'", specifier = ">=1.4.0" }, + { name = "sentry-sdk", specifier = ">=2.60.0" }, +] +provides-extras = ["dev"] + [[package]] name = "hackbot-pulse-listener" version = "0.1.0" @@ -5213,6 +5251,12 @@ keyring = [ memory = [ { name = "cachetools" }, ] +postgresql = [ + { name = "asyncpg" }, +] +wrappers-encryption = [ + { name = "cryptography" }, +] [[package]] name = "pyasn1"