diff --git a/agents/bug-fix/hackbot_agents/bug_fix/broker.py b/agents/bug-fix/hackbot_agents/bug_fix/broker.py index f10e69a637..001e2bc7fe 100644 --- a/agents/bug-fix/hackbot_agents/bug_fix/broker.py +++ b/agents/bug-fix/hackbot_agents/bug_fix/broker.py @@ -9,7 +9,6 @@ import logging from contextlib import asynccontextmanager -import bugsy import uvicorn from agent_tools import bugzilla from agent_tools.bugzilla import BugzillaContext @@ -32,10 +31,9 @@ class BrokerInputs(BaseSettings): def build_app(inputs: BrokerInputs) -> Starlette: - client = bugsy.Bugsy( - api_key=inputs.bugzilla_api_key, bugzilla_url=inputs.bugzilla_api_url + ctx = BugzillaContext( + api_url=inputs.bugzilla_api_url, api_key=inputs.bugzilla_api_key ) - ctx = BugzillaContext(client=client) sdk_config = build_sdk_server("bugzilla", ctx, bugzilla.TOOLS) mcp_server = sdk_config["instance"] diff --git a/agents/bug-fix/pyproject.toml b/agents/bug-fix/pyproject.toml index 492df37f36..f8cddb31ba 100644 --- a/agents/bug-fix/pyproject.toml +++ b/agents/bug-fix/pyproject.toml @@ -6,7 +6,6 @@ requires-python = ">=3.12" dependencies = [ "hackbot-runtime[claude-sdk]", "agent-tools[bugzilla,firefox]", - "bugsy", "claude-agent-sdk>=0.1.30", "mcp>=1.0.0", "starlette>=0.36.0", diff --git a/libs/agent-tools/agent_tools/bugzilla.py b/libs/agent-tools/agent_tools/bugzilla.py index 03f2bcf8a4..ecb15b2e92 100644 --- a/libs/agent-tools/agent_tools/bugzilla.py +++ b/libs/agent-tools/agent_tools/bugzilla.py @@ -1,9 +1,15 @@ -"""Read-only Bugzilla tools backed by bugsy. +"""Read-only Bugzilla tools backed by libmozdata. Framework-neutral: each tool is a ``@tool``-decorated handler whose first parameter is a :class:`BugzillaContext`. Handlers return plain data and surface proxy-level restrictions (code 101: endpoint not exposed, code 102: access denied) as a structured :class:`~agent_tools.registry.ToolError`. + +libmozdata exposes no raw-request passthrough; every call goes through its +handler-based ``Bugzilla(...).get_data().wait()`` API and its configuration is +process-global (set on class attributes). :class:`BugzillaContext` applies that +global configuration on construction, which is fine because the broker is a +single-tenant sidecar holding exactly one API key and URL. """ from __future__ import annotations @@ -12,7 +18,8 @@ from dataclasses import dataclass from typing import Annotated, Any -import bugsy +import requests +from libmozdata.bugzilla import Bugzilla, BugzillaBase from pydantic import Field from agent_tools.registry import ToolError, tool, tools_in @@ -20,24 +27,46 @@ @dataclass class BugzillaContext: - """Holds the live bugsy client. + """Carries the Bugzilla URL + API key and applies them to libmozdata. - Every tool receives the same instance, so they share auth and one TCP - connection pool. + libmozdata reads its credentials and endpoints from class attributes rather + than per-instance, so constructing the context configures the (process-wide) + libmozdata ``Bugzilla`` class. Every tool then builds short-lived + ``Bugzilla`` instances that share this auth. """ - client: bugsy.Bugsy + api_url: str + api_key: str + + def __post_init__(self) -> None: + base_url = self.api_url.rstrip("/") + BugzillaBase.TOKEN = self.api_key + BugzillaBase.URL = base_url + Bugzilla.API_URL = base_url + "/rest/bug" + Bugzilla.ATTACHMENT_API_URL = Bugzilla.API_URL + "/attachment" -def _bugsy_error(e: bugsy.BugsyException) -> ToolError: - """Turn a bugsy exception into a structured ToolError. +def _bugzilla_error(e: requests.HTTPError) -> ToolError: + """Turn a libmozdata HTTP error into a structured ToolError. The payload is friendly and machine-parseable so the agent can decide what to do (skip the bug, try a different endpoint, ...) rather than just seeing - a stack trace. + a stack trace. The Bugzilla proxy reports its restrictions in the JSON body + as ``{"code": ..., "message": ...}`` (code 101: endpoint not exposed, code + 102: access denied), which we recover from the failing response. """ - code = getattr(e, "code", None) - msg = getattr(e, "msg", str(e)) + code = None + msg = str(e) + resp = getattr(e, "response", None) + if resp is not None: + try: + body = resp.json() + except ValueError: + body = None + if isinstance(body, dict): + code = body.get("code") + msg = body.get("message", msg) + if code == 101: kind = "endpoint_not_exposed" hint = "This Bugzilla proxy does not expose this endpoint." @@ -77,11 +106,21 @@ async def search_bugs( component, status, resolution, priority, severity, assigned_to, whiteboard, include_fields, limit. """ + from urllib.parse import urlencode + + bugs: list[dict] = [] + + def bughandler(bug: dict) -> None: + bugs.append(bug) + + # Pass the query as a urlencoded string so libmozdata issues a single direct + # request (its dict-query path first fires a synchronous count_only probe + # the proxy may reject with code 101). + query = urlencode(params, doseq=True) try: - result = ctx.client.request("bug", params=params) - except bugsy.BugsyException as e: - raise _bugsy_error(e) from e - bugs = result.get("bugs", []) + Bugzilla(query, bughandler=bughandler).get_data().wait() + except requests.HTTPError as e: + raise _bugzilla_error(e) from e return {"count": len(bugs), "bugs": bugs} @@ -123,38 +162,33 @@ async def get_bugs( "creation_time,last_change_time,blocks,depends_on,see_also," "cf_crash_signature,url,version,op_sys,platform" ) - id_csv = ",".join(str(i) for i in ids) - try: - result = ctx.client.request( - "bug", params={"id": id_csv, "include_fields": include} - ) - except bugsy.BugsyException as e: - raise _bugsy_error(e) from e - bugs = result.get("bugs", []) - returned = {b["id"] for b in bugs} - inaccessible = [i for i in ids if i not in returned] - payload = {"count": len(bugs), "bugs": bugs, "inaccessible": inaccessible} + bugs_by_id: dict[int, dict] = {} - if include_comments and bugs: - # Bugzilla lets us fetch comments for many bugs in one call by hitting - # /bug/{first}/comment?ids=rest. One extra round trip total. - first, *rest = [b["id"] for b in bugs] - cparams = {"ids": ",".join(str(i) for i in rest)} if rest else {} - try: - cres = ctx.client.request(f"bug/{first}/comment", params=cparams) - comments_by_bug = { - int(bid): data["comments"] for bid, data in cres.get("bugs", {}).items() - } - for b in bugs: - b["comments"] = comments_by_bug.get(b["id"], []) - except bugsy.BugsyException as e: - payload["comments_error"] = { - "code": getattr(e, "code", None), - "message": getattr(e, "msg", str(e)), - } + def bughandler(bug: dict) -> None: + bugs_by_id[bug["id"]] = bug + + def commenthandler(data: dict, bug_id) -> None: + bug = bugs_by_id.get(int(bug_id)) + if bug is not None: + bug["comments"] = data["comments"] - return payload + kwargs: dict[str, Any] = { + "include_fields": include.split(","), + "bughandler": bughandler, + } + if include_comments: + kwargs["commenthandler"] = commenthandler + + try: + Bugzilla([str(i) for i in ids], **kwargs).get_data().wait() + except requests.HTTPError as e: + raise _bugzilla_error(e) from e + + bugs = list(bugs_by_id.values()) + returned = set(bugs_by_id) + inaccessible = [i for i in ids if i not in returned] + return {"count": len(bugs), "bugs": bugs, "inaccessible": inaccessible} @tool @@ -163,12 +197,16 @@ async def get_bug_comments( bug_id: Annotated[int, Field(description="Bug ID.")], ) -> dict: """Fetch all comments for a single bug.""" + collected: list[dict] = [] + + def commenthandler(data: dict, _bug_id) -> None: + collected.extend(data["comments"]) + try: - result = ctx.client.request(f"bug/{bug_id}/comment") - except bugsy.BugsyException as e: - raise _bugsy_error(e) from e - comments = result.get("bugs", {}).get(str(bug_id), {}).get("comments", []) - return {"bug_id": bug_id, "count": len(comments), "comments": comments} + Bugzilla([str(bug_id)], commenthandler=commenthandler).get_data().wait() + except requests.HTTPError as e: + raise _bugzilla_error(e) from e + return {"bug_id": bug_id, "count": len(collected), "comments": collected} @tool @@ -191,13 +229,42 @@ async def get_bug_attachments( include_data=true to also download the content — Bugzilla returns it base64-encoded in the 'data' field of each attachment. """ - params = {} if include_data else {"exclude_fields": "data"} + collected: list[dict] = [] + + def attachmenthandler(attachments: list, _bug_id) -> None: + collected.extend(attachments) + + # libmozdata has no exclude_fields; emulate it by requesting an explicit + # metadata-only field set unless the caller wants the (potentially large) + # base64 'data'. + include_fields = ( + None + if include_data + else [ + "id", + "file_name", + "content_type", + "size", + "creation_time", + "last_change_time", + "is_patch", + "is_obsolete", + "is_private", + "flags", + "summary", + "creator", + ] + ) + try: - result = ctx.client.request(f"bug/{bug_id}/attachment", params=params) - except bugsy.BugsyException as e: - raise _bugsy_error(e) from e - atts = result.get("bugs", {}).get(str(bug_id), []) - return {"bug_id": bug_id, "count": len(atts), "attachments": atts} + Bugzilla( + [str(bug_id)], + attachmenthandler=attachmenthandler, + attachment_include_fields=include_fields, + ).get_data().wait() + except requests.HTTPError as e: + raise _bugzilla_error(e) from e + return {"bug_id": bug_id, "count": len(collected), "attachments": collected} @tool @@ -223,12 +290,21 @@ async def download_attachment( get_bug_attachments first to discover attachment IDs. Returns the written path, size, and content_type. """ - try: - result = ctx.client.request(f"bug/attachment/{attachment_id}") - except bugsy.BugsyException as e: - raise _bugsy_error(e) from e + collected: list[dict] = [] + + def attachmenthandler(attachments: list) -> None: + collected.extend(attachments) - att = result.get("attachments", {}).get(str(attachment_id)) + try: + Bugzilla( + attachmentids=[str(attachment_id)], + attachmenthandler=attachmenthandler, + attachment_include_fields=["id", "data", "file_name", "content_type"], + ).get_data().wait() + except requests.HTTPError as e: + raise _bugzilla_error(e) from e + + att = next((a for a in collected if a.get("id") == attachment_id), None) if att is None: raise ToolError( f"attachment {attachment_id} not found", diff --git a/libs/agent-tools/pyproject.toml b/libs/agent-tools/pyproject.toml index 307e733045..1485037daa 100644 --- a/libs/agent-tools/pyproject.toml +++ b/libs/agent-tools/pyproject.toml @@ -8,7 +8,7 @@ dependencies = [ ] [project.optional-dependencies] -bugzilla = ["bugsy"] +bugzilla = ["libmozdata~=0.2.12"] firefox = ["grizzly-framework", "prefpicker"] claude-sdk = ["claude-agent-sdk>=0.1.30"] diff --git a/libs/agent-tools/tests/test_bugzilla.py b/libs/agent-tools/tests/test_bugzilla.py index 587202c2d2..2bf8c77132 100644 --- a/libs/agent-tools/tests/test_bugzilla.py +++ b/libs/agent-tools/tests/test_bugzilla.py @@ -1,8 +1,9 @@ """Tests for the Bugzilla read tools.""" -from unittest.mock import MagicMock +from unittest.mock import patch import pytest +import requests from agent_tools import bugzilla from agent_tools.bugzilla import BugzillaContext from agent_tools.claude_sdk import build_sdk_server @@ -10,6 +11,60 @@ from mcp.types import ListToolsRequest +def _ctx() -> BugzillaContext: + return BugzillaContext(api_url="https://bz.example", api_key="k") + + +class FakeBugzilla: + """Stand-in for libmozdata's Bugzilla. + + Captures the handler kwargs at construction and, on ``wait()``, replays + canned payloads through them (or raises a configured error) — mirroring the + real handler-based ``get_data().wait()`` flow. + """ + + bugs: list[dict] = [] + comments: list[dict] = [] + attachments: list[dict] = [] + error: requests.HTTPError | None = None + + def __init__(self, bugids=None, attachmentids=None, **kwargs): + self.bugids = bugids + self.attachmentids = attachmentids + self.kwargs = kwargs + + def get_data(self): + return self + + def wait(self): + if FakeBugzilla.error is not None: + raise FakeBugzilla.error + bughandler = self.kwargs.get("bughandler") + if bughandler: + for bug in FakeBugzilla.bugs: + bughandler(bug) + commenthandler = self.kwargs.get("commenthandler") + if commenthandler: + for bug in FakeBugzilla.bugs or [{"id": int(self.bugids[0])}]: + commenthandler({"comments": FakeBugzilla.comments}, str(bug["id"])) + attachmenthandler = self.kwargs.get("attachmenthandler") + if attachmenthandler: + if self.attachmentids is not None: + attachmenthandler(FakeBugzilla.attachments) + else: + attachmenthandler(FakeBugzilla.attachments, self.bugids[0]) + return self + + +@pytest.fixture(autouse=True) +def reset_fake(): + FakeBugzilla.bugs = [] + FakeBugzilla.comments = [] + FakeBugzilla.attachments = [] + FakeBugzilla.error = None + yield + + async def _list(server): return ( await server.request_handlers[ListToolsRequest]( @@ -19,9 +74,7 @@ async def _list(server): async def test_exposes_read_only_tools(): - config = build_sdk_server( - "bugzilla", BugzillaContext(client=MagicMock()), bugzilla.TOOLS - ) + config = build_sdk_server("bugzilla", _ctx(), bugzilla.TOOLS) assert config["type"] == "sdk" tools = await _list(config["instance"]) assert {t.name for t in tools} == { @@ -34,21 +87,82 @@ async def test_exposes_read_only_tools(): async def test_search_bugs_returns_data(): - client = MagicMock() - client.request.return_value = {"bugs": [{"id": 1}, {"id": 2}]} - result = await bugzilla.search_bugs( - BugzillaContext(client=client), params={"id": "1,2"} - ) + FakeBugzilla.bugs = [{"id": 1}, {"id": 2}] + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + result = await bugzilla.search_bugs(_ctx(), params={"id": "1,2"}) assert result == {"count": 2, "bugs": [{"id": 1}, {"id": 2}]} -async def test_search_bugs_raises_tool_error_on_bugsy_failure(): - import bugsy +async def test_get_bugs_reports_inaccessible(): + FakeBugzilla.bugs = [{"id": 1}] + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + result = await bugzilla.get_bugs(_ctx(), ids=[1, 2]) + assert result["count"] == 1 + assert result["bugs"] == [{"id": 1}] + assert result["inaccessible"] == [2] + + +async def test_get_bug_comments_returns_comments(): + FakeBugzilla.comments = [{"id": 10, "text": "hi"}] + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + result = await bugzilla.get_bug_comments(_ctx(), bug_id=1) + assert result == { + "bug_id": 1, + "count": 1, + "comments": [{"id": 10, "text": "hi"}], + } + + +async def test_get_bug_attachments_returns_attachments(): + FakeBugzilla.attachments = [{"id": 5, "file_name": "a.txt"}] + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + result = await bugzilla.get_bug_attachments(_ctx(), bug_id=1) + assert result == { + "bug_id": 1, + "count": 1, + "attachments": [{"id": 5, "file_name": "a.txt"}], + } + + +async def test_download_attachment_writes_file(tmp_path): + import base64 + + raw = b"hello" + FakeBugzilla.attachments = [ + { + "id": 5, + "data": base64.b64encode(raw).decode(), + "file_name": "a.txt", + "content_type": "text/plain", + } + ] + dest = tmp_path / "a.txt" + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + result = await bugzilla.download_attachment( + _ctx(), attachment_id=5, dest_path=str(dest) + ) + assert dest.read_bytes() == raw + assert result["size_bytes"] == len(raw) + assert result["file_name"] == "a.txt" + + +async def test_download_attachment_not_found(): + FakeBugzilla.attachments = [] + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + with pytest.raises(ToolError) as ei: + await bugzilla.download_attachment( + _ctx(), attachment_id=5, dest_path="/tmp/nope" + ) + assert ei.value.payload["error"] == "attachment_not_found" + - client = MagicMock() - err = bugsy.BugsyException("nope") - err.code = 102 - client.request.side_effect = err - with pytest.raises(ToolError) as ei: - await bugzilla.search_bugs(BugzillaContext(client=client), params={}) +async def test_search_bugs_raises_tool_error_on_http_failure(): + resp = requests.Response() + resp.status_code = 403 + resp._content = b'{"code": 102, "message": "nope"}' + FakeBugzilla.error = requests.HTTPError(response=resp) + with patch.object(bugzilla, "Bugzilla", FakeBugzilla): + with pytest.raises(ToolError) as ei: + await bugzilla.search_bugs(_ctx(), params={}) assert ei.value.payload["error"] == "access_denied" + assert ei.value.payload["code"] == 102 diff --git a/uv.lock b/uv.lock index 218fa0df13..452336cef7 100644 --- a/uv.lock +++ b/uv.lock @@ -57,7 +57,7 @@ dependencies = [ [package.optional-dependencies] bugzilla = [ - { name = "bugsy" }, + { name = "libmozdata" }, ] claude-sdk = [ { name = "claude-agent-sdk" }, @@ -69,9 +69,9 @@ firefox = [ [package.metadata] requires-dist = [ - { name = "bugsy", marker = "extra == 'bugzilla'" }, { name = "claude-agent-sdk", marker = "extra == 'claude-sdk'", specifier = ">=0.1.30" }, { name = "grizzly-framework", marker = "extra == 'firefox'" }, + { name = "libmozdata", marker = "extra == 'bugzilla'", specifier = "~=0.2.12" }, { name = "prefpicker", marker = "extra == 'firefox'" }, { name = "pydantic", specifier = ">=2.6.0" }, ] @@ -2137,7 +2137,6 @@ version = "0.1.0" source = { editable = "agents/bug-fix" } dependencies = [ { name = "agent-tools", extra = ["bugzilla", "firefox"] }, - { name = "bugsy" }, { name = "claude-agent-sdk" }, { name = "hackbot-runtime", extra = ["claude-sdk"] }, { name = "mcp" }, @@ -2148,7 +2147,6 @@ dependencies = [ [package.metadata] requires-dist = [ { name = "agent-tools", extras = ["bugzilla", "firefox"], editable = "libs/agent-tools" }, - { name = "bugsy" }, { name = "claude-agent-sdk", specifier = ">=0.1.30" }, { name = "hackbot-runtime", extras = ["claude-sdk"], editable = "libs/hackbot-runtime" }, { name = "mcp", specifier = ">=1.0.0" },