diff --git a/README.rst b/README.rst index 4db53cd63..6e8ccded5 100644 --- a/README.rst +++ b/README.rst @@ -58,9 +58,12 @@ Before running: "phab_api_key": "xxxxxxxxxxxxxx", "iam_client_secret": "xxxxxxxxxxxxxx", "iam_client_id": "xxxxxxxxxxxxxx", - "socorro_token": "xxxxxxxxxxxxxx" + "socorro_token": "xxxxxxxxxxxxxx", + "hackbot_api_key": "xxxxxxxxxxxxxx" } +The ``hackbot_api_key`` is only needed by rules that start a `hackbot `_ agent run (currently ``frontend_triage``). Those rules talk to ``https://hackbot-api.moz.tools`` by default; set ``HACKBOT_API_URL`` to point at a different deployment. + Do a dryrun:: uv run -m bugbot.rules.stalled diff --git a/bugbot/hackbot_utils.py b/bugbot/hackbot_utils.py new file mode 100644 index 000000000..6a332ab05 --- /dev/null +++ b/bugbot/hackbot_utils.py @@ -0,0 +1,57 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +import os + +import requests + +from bugbot import utils + +DEFAULT_API_URL = "https://hackbot-api.moz.tools" + +TIMEOUT = 60 + + +def api_url() -> str: + """The hackbot API base URL. + + hackbot runs the LLM agents (see mozilla/bugbug's services/hackbot-api). We + only ever start a run from here: the agent applies its own findings to + Bugzilla and reports the result to Slack itself, so nothing comes back to us. + + Defaults to the production deployment so the cron host needs no extra + environment, the way ``BUGBUG_HTTP_SERVER`` does. Override + ``HACKBOT_API_URL`` to point at another deployment — that is the name + hackbot's other clients already use (the pulse listener's setting and the + console's deploy env), so there is one name to remember across all three. + + Read per call rather than at import so the override is settable by anything + that configures the environment after this module loads. + """ + return os.environ.get("HACKBOT_API_URL", DEFAULT_API_URL) + + +def trigger_agent_run(agent: str, inputs: dict) -> str: + """Start a hackbot agent run. + + Args: + agent: The agent to run, e.g. `frontend-triage`. + inputs: The agent's per-run inputs, e.g. `{"bug_id": 1234567}`. + + Returns: + The id of the created run. + """ + base_url = api_url() + if not base_url: + raise ValueError("HACKBOT_API_URL is not set") + + response = requests.post( + f"{base_url.rstrip('/')}/agents/{agent}/runs", + headers={"X-API-Key": utils.get_login_info()["hackbot_api_key"]}, + json=inputs, + timeout=TIMEOUT, + ) + response.raise_for_status() + + return response.json()["run_id"] diff --git a/bugbot/rules/frontend_triage.py b/bugbot/rules/frontend_triage.py new file mode 100644 index 000000000..a2b267409 --- /dev/null +++ b/bugbot/rules/frontend_triage.py @@ -0,0 +1,130 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +from bugbot import logger +from bugbot.bzcleaner import BzCleaner +from bugbot.hackbot_utils import api_url, trigger_agent_run +from bugbot.people import People + +AGENT = "frontend-triage" + + +class FrontendTriage(BzCleaner): + """Ask hackbot's frontend-triage agent to triage newly filed frontend bugs. + + Scoped to bugs filed by Mozilla staff (which includes QA) in a small set of + components, because the agent's analysis is posted to the bug unattended when + it is confident. This rule only starts runs: the agent investigates the + source, comments on the bug, and reports to Slack itself, so nothing is + written to Bugzilla from here. + """ + + def __init__(self, people: People | None = None) -> None: + super().__init__() + # Injectable because configs/people.json is gitignored and absent in CI. + self.people = people or People.get_instance() + self.max_triggers = self.get_config("max_triggers", 10) + self.left_for_next_run = 0 + + def description(self): + return "[Using AI] Bugs sent for automatic frontend triage" + + def columns(self): + return ["id", "summary", "creator", "run_id"] + + def has_default_products(self): + # The query names its own product; the 19-product default list would put + # the whole tree in scope. + return False + + def get_bz_params(self, date): + start_date, _ = self.get_dates(date) + components = self.get_config("components", {}) + + return { + "include_fields": ["id", "summary", "creator", "component"], + "product": list(components.keys()), + "component": [c for cs in components.values() for c in cs], + # Defects only: the agent triages broken behaviour, not feature work. + "bug_type": "defect", + "resolution": "---", + "f1": "creation_ts", + "o1": "greaterthan", + "v1": start_date, + } + + def handle_bug(self, bug, data): + # The IAM roster covers QA too, now that they file from @mozilla.com + # addresses, so staff membership is the whole filter. It beats a check on + # the address itself, which would miss the employees who file from a + # personal Bugzilla account. + if not self.people.is_mozilla(bug["creator"]): + return None + + # `bughandler` rebuilds each row from id + summary alone, so the reporter + # has to be stashed here to reach the report. + data[str(bug["id"])] = {"creator": bug["creator"]} + + return bug + + def get_bugs(self, date="today", bug_ids=[], chunk_size=None): + return self.trigger_runs( + super().get_bugs(date=date, bug_ids=bug_ids, chunk_size=chunk_size) + ) + + def trigger_runs(self, bugs: dict) -> dict: + """Start a triage run per bug, and return the ones we actually started. + + Bugs we did not trigger are dropped from the result so they are neither + reported as triaged nor added to the cache — a bug skipped over the cap, + or one whose trigger failed, gets another chance on the next run. + """ + if self.dryrun: + logger.info( + "Dry run: would trigger %s for bugs %s", + AGENT, + ", ".join(bugs) or "(none)", + ) + for bug in bugs.values(): + bug["run_id"] = "(dry run)" + return bugs + + if not api_url(): + # A misconfiguration, not a transient failure: fail once here rather + # than letting every bug look like its own flaky trigger below. The + # rule aborts and the cron error digest picks it up. + raise ValueError(f"HACKBOT_API_URL is not set; cannot start {AGENT} runs") + + triggered: dict = {} + for bugid, bug in bugs.items(): + if len(triggered) >= self.max_triggers: + # Each run is real LLM spend, so a flood of filings must not turn + # into a flood of agent runs. + logger.warning( + "Reached the cap of %d %s runs; the rest wait for the next run", + self.max_triggers, + AGENT, + ) + break + + try: + bug["run_id"] = trigger_agent_run(AGENT, {"bug_id": int(bugid)}) + except Exception: + logger.exception("Failed to trigger %s for bug %s", AGENT, bugid) + continue + + triggered[bugid] = bug + + # Covers both causes — over the cap, and a trigger that failed — since + # either way the bug stays out of the cache and comes back next run. + self.left_for_next_run = len(bugs) - len(triggered) + + return triggered + + def get_extra_for_template(self): + return {"left_for_next_run": self.left_for_next_run} + + +if __name__ == "__main__": + FrontendTriage().run() diff --git a/configs/rules.json b/configs/rules.json index 4184c4b26..c5a9ca922 100644 --- a/configs/rules.json +++ b/configs/rules.json @@ -419,6 +419,14 @@ "confidence_threshold": 0.95, "cc": [] }, + "frontend_triage": { + "days_lookup": 1, + "max_days_in_cache": 30, + "max_triggers": 3, + "components": { + "Firefox": ["New Tab Page"] + } + }, "performancebug": { "max_days_in_cache": 7, "days_lookup": 7 diff --git a/scripts/check_rules_on_wiki.py b/scripts/check_rules_on_wiki.py index 9f1702242..0d2a9dccd 100644 --- a/scripts/check_rules_on_wiki.py +++ b/scripts/check_rules_on_wiki.py @@ -56,6 +56,7 @@ class CheckWikiPage: # Experimental rules: "accessibilitybug.py", "performancebug.py", + "frontend_triage.py", # piloting on one component before documenting it } def __init__(self) -> None: diff --git a/scripts/cron_run_hourly.sh b/scripts/cron_run_hourly.sh index 1cfc99191..34d033691 100755 --- a/scripts/cron_run_hourly.sh +++ b/scripts/cron_run_hourly.sh @@ -62,6 +62,9 @@ python -m bugbot.rules.closed_dupeme --production # Detect spam bugs using bugbug python -m bugbot.rules.spambug --production +# Send newly filed frontend bugs from employees and QA to the frontend-triage agent +python -m bugbot.rules.frontend_triage --production + # Suggest components for untriaged bugs (hourly, list only bugs on which we acted) python -m bugbot.rules.component --frequency hourly --production diff --git a/templates/frontend_triage.html b/templates/frontend_triage.html new file mode 100644 index 000000000..f78a33c6d --- /dev/null +++ b/templates/frontend_triage.html @@ -0,0 +1,32 @@ +

+ The following {{ plural('bug', data, pword='bugs') }} {{ plural('was', data, pword='were') }} sent to the frontend-triage agent. The agent posts its analysis to the bug itself when it is confident; otherwise the result waits for review in the hackbot UI. +

+ + + + + + + + + + + {% for i, (bugid, summary, creator, run_id) in enumerate(data) -%} + + + + + + + {% endfor -%} + +
BugSummaryReporterRun
+ {{ bugid }} + {{ summary | e }}{{ creator | e }}{{ run_id }}
+{% if extra.left_for_next_run %} +

+ {{ extra.left_for_next_run }} more {{ plural('bug', extra.left_for_next_run, pword='bugs') }} matched but {{ plural('was', extra.left_for_next_run, pword='were') }} not sent this time — either over the cap on how many agent runs one invocation may start, or the run failed to start. {{ plural('It', extra.left_for_next_run, pword='They') }} will be picked up on the next run. +

+{% endif %} diff --git a/tests/rules/test_frontend_triage.py b/tests/rules/test_frontend_triage.py new file mode 100644 index 000000000..974d2a336 --- /dev/null +++ b/tests/rules/test_frontend_triage.py @@ -0,0 +1,307 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at http://mozilla.org/MPL/2.0/. + +import pytest +from jinja2 import Environment, FileSystemLoader + +from bugbot import hackbot_utils, utils +from bugbot.people import People +from bugbot.rules.frontend_triage import FrontendTriage + +STAFF_MAIL = "staffer@mozilla.com" +QA_MAIL = "tester@mozilla.com" +# An employee who files from a personal Bugzilla account, which is why the filter +# reads the roster rather than the address. +STAFF_PERSONAL_MAIL = "staffer@example.net" + + +def _person(mail, bzmail=None, cn="A Staffer"): + return { + "mail": mail, + "bugzillaEmail": bzmail or mail, + "cn": cn, + "dn": f"mail={mail},o=com,dc=mozilla", + "ismanager": "FALSE", + "isdirector": "FALSE", + "title": "Engineer", + "manager": {"cn": "A Manager", "dn": "mail=boss,o=com,dc=mozilla"}, + } + + +def _people(): + # `People` is normally loaded from configs/people.json, which is gitignored + # and absent in CI, so the roster is injected here. QA are on the roster too, + # now that they file from @mozilla.com addresses. + return People( + [ + _person(STAFF_MAIL), + _person(QA_MAIL, cn="A Tester"), + _person("staffer@mozilla.com.example", bzmail=STAFF_PERSONAL_MAIL), + ] + ) + + +def _rule(**over): + rule = FrontendTriage(people=_people()) + for key, value in over.items(): + setattr(rule, key, value) + return rule + + +def _bug(creator, bug_id=1): + return { + "id": bug_id, + "summary": "New Tab weather widget vanishes", + "creator": creator, + "component": "New Tab Page", + # `amend_bzparams` adds `groups` to include_fields for every rule, and + # `get_summary` reads it to redact security bugs. + "groups": [], + } + + +# --- who gets triaged --------------------------------------------------- # + + +def test_keeps_employee_filed_bug(): + rule = _rule() + assert rule.handle_bug(_bug(STAFF_MAIL), {}) is not None + + +def test_keeps_qa_filed_bug(): + # QA are on the staff roster now that they file from @mozilla.com, so they + # need no separate rule of their own. + rule = _rule() + assert rule.handle_bug(_bug(QA_MAIL), {}) is not None + + +def test_keeps_bug_from_an_employees_personal_bugzilla_account(): + # The roster maps a staffer's Bugzilla address to them, so an employee who + # doesn't file under @mozilla.com is still in scope. This is what a check on + # the address alone would miss. + rule = _rule() + assert rule.handle_bug(_bug(STAFF_PERSONAL_MAIL), {}) is not None + + +def test_drops_community_filed_bug(): + # The pilot is scoped to reporters we expect to file well; everyone else is + # left to the humans. + rule = _rule() + assert rule.handle_bug(_bug("someone@example.org"), {}) is None + + +def test_drops_a_mozilla_com_address_that_is_not_on_the_roster(): + # The roster is the source of truth, not the domain: a @mozilla.com address + # IAM doesn't know about (a bot, a departed account) is not triaged. + rule = _rule() + assert rule.handle_bug(_bug("automation@mozilla.com"), {}) is None + + +def test_reporter_reaches_the_report(): + # `bughandler` rebuilds each row from scratch and keeps only id/summary, so + # anything else a column needs has to be stashed in `data` (the idiom in + # rules/defectenhancementtask.py). Going through `bughandler` rather than + # `handle_bug` is what catches that. + rule = _rule(dryrun=True) + rule.cache.set_dry_run(True) + data: dict = {} + rule.bughandler(_bug(STAFF_MAIL), data) + assert data["1"]["creator"] == STAFF_MAIL + # And every column the template reads is present, so organize() won't raise. + assert set(rule.columns()) <= set(data["1"]) | {"run_id"} + + +# --- which bugs are queried --------------------------------------------- # + + +def test_does_not_use_the_default_product_list(): + # The query names Firefox itself; inheriting the 19-product default list + # would triage the whole tree. + assert _rule().has_default_products() is False + + +def test_queries_the_configured_product_and_components(): + params = _rule().get_bz_params("2026-07-28") + assert params["product"] == ["Firefox"] + assert params["component"] == ["New Tab Page"] + + +def test_queries_only_open_defects(): + params = _rule().get_bz_params("2026-07-28") + assert params["bug_type"] == "defect" + assert params["resolution"] == "---" + + +def test_queries_only_recently_filed_bugs(): + rule = _rule() + params = rule.get_bz_params("2026-07-28") + start_date, _ = rule.get_dates("2026-07-28") + triplet = { + (params[f"f{i}"], params[f"o{i}"], params[f"v{i}"]) + for i in range(1, 10) + if f"f{i}" in params + } + assert ("creation_ts", "greaterthan", start_date) in triplet + + +def test_requests_the_fields_the_filter_needs(): + params = _rule().get_bz_params("2026-07-28") + assert {"id", "summary", "creator", "component"} <= set(params["include_fields"]) + + +# --- triggering the agent ----------------------------------------------- # + + +@pytest.fixture +def triggered(monkeypatch): + """Capture the runs the rule would trigger, instead of calling hackbot.""" + monkeypatch.setenv("HACKBOT_API_URL", "https://hackbot.example") + calls = [] + + def fake_trigger(agent, inputs): + calls.append((agent, inputs)) + return f"run-{inputs['bug_id']}" + + monkeypatch.setattr("bugbot.rules.frontend_triage.trigger_agent_run", fake_trigger) + return calls + + +def _bugs(*ids): + return {str(i): _bug(STAFF_MAIL, bug_id=i) for i in ids} + + +def test_dry_run_triggers_nothing(triggered): + rule = _rule(dryrun=True) + rule.trigger_runs(_bugs(1, 2)) + assert triggered == [] + + +def test_dry_run_still_reports_which_bugs_it_would_have_triaged(triggered): + # The dry run is how a human checks the filter before enabling the cron, so + # it has to survive `organize()`, which does a bare lookup of every column. + rule = _rule(dryrun=True) + bugs = rule.trigger_runs(_bugs(1, 2)) + assert set(bugs) == {"1", "2"} + assert rule.organize(bugs) + + +def test_triggers_one_run_per_bug(triggered): + rule = _rule(dryrun=False) + rule.trigger_runs(_bugs(1, 2)) + assert triggered == [ + ("frontend-triage", {"bug_id": 1}), + ("frontend-triage", {"bug_id": 2}), + ] + + +def test_records_the_run_id_on_each_bug(triggered): + rule = _rule(dryrun=False) + bugs = rule.trigger_runs(_bugs(1)) + assert bugs["1"]["run_id"] == "run-1" + + +def test_caps_the_number_of_runs(triggered): + # Each run is real LLM spend, so a flood of filings must not turn into a + # flood of agent runs. + rule = _rule(dryrun=False, max_triggers=2) + bugs = rule.trigger_runs(_bugs(1, 2, 3, 4)) + assert len(triggered) == 2 + # Only the bugs actually triaged are reported, so the cap isn't silent. + assert set(bugs) == {"1", "2"} + assert rule.left_for_next_run == 2 + + +def test_reports_nothing_skipped_when_under_the_cap(triggered): + rule = _rule(dryrun=False) + rule.trigger_runs(_bugs(1, 2)) + assert rule.get_extra_for_template() == {"left_for_next_run": 0} + + +def test_defaults_to_the_production_api(monkeypatch): + # No env var needed on the cron host: the deployment is at a stable custom + # domain, so it can be the default (as BUGBUG_HTTP_SERVER's is). + monkeypatch.delenv("HACKBOT_API_URL", raising=False) + assert hackbot_utils.api_url() == "https://hackbot-api.moz.tools" + + +def test_env_var_overrides_the_default(monkeypatch): + monkeypatch.setenv("HACKBOT_API_URL", "https://hackbot-api-staging.example") + assert hackbot_utils.api_url() == "https://hackbot-api-staging.example" + + +def test_explicitly_blanked_api_url_fails_the_rule_up_front(monkeypatch): + # Deliberately pointing at nothing is a misconfiguration, not a transient + # failure. Failing once before the loop keeps it from looking like N flaky + # triggers, and lets the cron error digest surface it. + monkeypatch.setenv("HACKBOT_API_URL", "") + attempts = [] + monkeypatch.setattr( + "bugbot.rules.frontend_triage.trigger_agent_run", + lambda agent, inputs: attempts.append(inputs), + ) + + rule = _rule(dryrun=False) + with pytest.raises(ValueError, match="HACKBOT_API_URL"): + rule.trigger_runs(_bugs(1, 2)) + assert attempts == [] + + +def test_a_failing_trigger_does_not_stop_the_others(triggered, monkeypatch): + def flaky(agent, inputs): + if inputs["bug_id"] == 1: + raise RuntimeError("hackbot down") + triggered.append((agent, inputs)) + return f"run-{inputs['bug_id']}" + + monkeypatch.setattr("bugbot.rules.frontend_triage.trigger_agent_run", flaky) + + rule = _rule(dryrun=False) + bugs = rule.trigger_runs(_bugs(1, 2)) + assert triggered == [("frontend-triage", {"bug_id": 2})] + # The bug we failed to trigger is not reported as triaged, so it stays out + # of the cache and gets another chance on the next run. + assert set(bugs) == {"2"} + # And the report says so, rather than quietly losing it. + assert rule.left_for_next_run == 1 + + +# --- the email report --------------------------------------------------- # + + +def _render(rule, bugs): + env = Environment(loader=FileSystemLoader("templates")) + return env.get_template(rule.template()).render( + date="2026-07-28", + data=rule.organize(bugs), + extra=rule.get_extra_for_template(), + str=str, + enumerate=enumerate, + plural=utils.plural, + no_manager=rule.no_manager, + table_attrs="", + ) + + +def test_template_renders_a_row_per_triaged_bug(triggered): + # The email path is otherwise only exercised in production, where a template + # typo would be a silent 500 in the cron log. + rule = _rule(dryrun=False) + html = _render(rule, rule.trigger_runs(_bugs(1, 2))) + assert "show_bug.cgi?id=1" in html + assert "show_bug.cgi?id=2" in html + assert STAFF_MAIL in html + assert "run-1" in html + + +def test_template_reports_the_bugs_left_for_next_run(triggered): + rule = _rule(dryrun=False, max_triggers=1) + html = _render(rule, rule.trigger_runs(_bugs(1, 2, 3))) + assert "2 more bugs" in html + assert "next run" in html + + +def test_template_omits_the_cap_note_when_it_does_not(triggered): + rule = _rule(dryrun=False) + html = _render(rule, rule.trigger_runs(_bugs(1))) + assert "more bug" not in html