Deduplicate Bugzilla webhook runs in the database

docs/hackbot/triggers.md

@@ -128,9 +128,9 @@ Guards, each closing a specific failure mode:
 - **Private bugs are never processed** — the check is `is_private is not False`, so a
   missing or non-boolean value fails closed.
 - **Dedupe** — retried deliveries are deduped by the **needinfo flag id**, which is globally
-  unique, so a later needinfo on the same bug gets a new id and still triggers. As on the
-  Phabricator side, the key is claimed **only after a successful trigger**, keeping a
-  transient failure retryable by BMO.
+  unique, so a later needinfo on the same bug gets a new id and still triggers. The flag id
+  is used as the run's database dedupe key, so retries reuse the existing run even after a
+  service restart.
 - **Latest flag wins** — BMO orders flags by id, so the last matching one is the newly
   requested one.
 

services/hackbot-api/app/config.py

@@ -34,8 +34,6 @@ class BugzillaWebhookSettings(BaseModel):
     secret: str
     # The Bugzilla account to which the needinfo request must be directed.
     bot_login: str = "hackbot@mozilla.tld"
-    # Best-effort in-memory dedupe of retried bug-modification deliveries.
-    dedupe_ttl_seconds: int = 6 * 60 * 60
 
 
 class SlackSettings(BaseModel):
@@ -79,8 +77,7 @@ class Settings(BaseSettings):
     webhook: WebhookSettings
 
     # Bugzilla uses a separate shared-secret header and bot identity. These map
-    # from BUGZILLA_WEBHOOK_SECRET, BUGZILLA_WEBHOOK_BOT_LOGIN, and
-    # BUGZILLA_WEBHOOK_DEDUPE_TTL_SECONDS.
+    # from BUGZILLA_WEBHOOK_SECRET and BUGZILLA_WEBHOOK_BOT_LOGIN.
     bugzilla_webhook: BugzillaWebhookSettings
 
     bugzilla_api_url: str = "https://bugzilla.mozilla.org/rest"

services/hackbot-api/app/routers/webhooks.py

@@ -2,7 +2,6 @@
 
 import logging
 
-from cachetools import TTLCache
 from fastapi import APIRouter, Depends, Request, Response, status
 from hackbot_client import HackbotClient
 from phabricator_client import PhabricatorClient
@@ -89,14 +88,6 @@ def get_bugzilla_authorizer(request: Request) -> BugzillaAuthorizer:
     return authorizer
 
 
-# Best-effort dedupe of retried BMO deliveries, keyed by the globally unique
-# needinfo flag ID. A later needinfo on the same bug receives a new flag ID.
-# TODO: Replace with DB-level deduplication (#6716).
-_seen_bugzilla_events: TTLCache = TTLCache(
-    maxsize=4096, ttl=settings.bugzilla_webhook.dedupe_ttl_seconds
-)
-
-
 @router.post(
     "/phabricator",
     status_code=status.HTTP_202_ACCEPTED,
@@ -188,14 +179,6 @@ async def bugzilla_webhook(
     )
     if detected is None:
         return {"status": "ignored", "reason": "no actionable Hackbot needinfo"}
-    dedupe_key = f"ni{detected.flag_id}"
-    if dedupe_key in _seen_bugzilla_events:
-        log.info(
-            "Ignored duplicate Bugzilla needinfo webhook for bug %s (flag: %s)",
-            detected.bug_id,
-            detected.flag_id,
-        )
-        return {"status": "ignored", "reason": "duplicate delivery"}
 
     if not await authorizer.is_authorized(detected.user_login):
         log.info(
@@ -212,13 +195,24 @@ async def bugzilla_webhook(
             "bugzilla_needinfo_flag_id": detected.flag_id,
             "comment": detected.comment,
         },
+        dedupe_key=f"ni{detected.flag_id}",
     )
-    # Do not consume an event until run creation succeeds; a transient failure
-    # must remain retryable by Bugzilla.
-    _seen_bugzilla_events[dedupe_key] = True
+    if not run.is_new:
+        log.info(
+            "Duplicate Bugzilla delivery for bug %s (flag: %s) resolved to run %s",
+            detected.bug_id,
+            detected.flag_id,
+            run.run_id,
+        )
+        return {
+            "status": "ignored",
+            "reason": "duplicate delivery",
+            "run_id": run.run_id,
+        }
     log.info(
-        "Triggered bug-fix run %s for Bugzilla bug %s from needinfo request",
+        "Triggered bug-fix run %s for Bugzilla bug %s from needinfo request (flag: %s)",
         run.run_id,
         detected.bug_id,
+        detected.flag_id,
     )
     return {"status": "triggered", "run_id": run.run_id}

services/hackbot-api/tests/test_webhooks.py

@@ -614,8 +614,6 @@ def client(monkeypatch, authorizer, bugzilla_authorizer, phab_client):
     monkeypatch.setattr(settings.webhook, "secret", SECRET)
     monkeypatch.setattr(settings.bugzilla_webhook, "secret", BUGZILLA_SECRET)
     monkeypatch.setattr(settings.bugzilla_webhook, "bot_login", BUGZILLA_BOT_LOGIN)
-    # Fresh dedupe cache per test.
-    webhooks._seen_bugzilla_events.clear()
     app.dependency_overrides[webhooks.get_phabricator_client] = lambda: phab_client
     app.dependency_overrides[webhooks.get_phabricator_authorizer] = lambda: authorizer
     app.dependency_overrides[webhooks.get_bugzilla_authorizer] = lambda: (
@@ -868,47 +866,43 @@ def test_bugzilla_route_ignores_unauthorized_actor(client, bugzilla_authorizer):
     fake_api = _FakeHackbotClient()
     app.dependency_overrides[webhooks.get_hackbot_client] = lambda: fake_api
     payload = _bugzilla_payload()
-    detected = detect_needinfo_request(payload, bot_login=BUGZILLA_BOT_LOGIN)
 
     response = _post_bugzilla(client, payload)
 
     assert response.status_code == 202
     assert response.json() == {"status": "ignored", "reason": "unauthorized user"}
     assert fake_api.calls == []
-    # The event stays unconsumed: the same flag can still trigger a run once
-    # the actor is authorized.
-    assert f"ni{detected.flag_id}" not in webhooks._seen_bugzilla_events
 
 
-def test_bugzilla_route_dedupes_retry_but_not_later_event(client):
+def test_bugzilla_route_keys_retry_same_but_later_event_differently(client):
     fake_api = _FakeHackbotClient()
     app.dependency_overrides[webhooks.get_hackbot_client] = lambda: fake_api
     payload = _bugzilla_payload()
 
     first = _post_bugzilla(client, payload)
-    duplicate = _post_bugzilla(client, payload)
+    retry = _post_bugzilla(client, payload)
     later = _post_bugzilla(
         client,
         _bugzilla_payload(flag_id=2187234, event_time="2026-08-07T19:00:05"),
     )
 
     assert first.json()["status"] == "triggered"
-    assert duplicate.json()["reason"] == "duplicate delivery"
+    # The retry is answered with the existing run rather than claimed as new.
+    assert retry.json() == {
+        "status": "ignored",
+        "reason": "duplicate delivery",
+        "run_id": "d3d5f21d-d716-4bb0-a812-8c9ef3e2f1c6",
+    }
     assert later.json()["status"] == "triggered"
-    assert len(fake_api.calls) == 2
+    assert fake_api.dedupe_keys == ["ni2187233", "ni2187233", "ni2187234"]
 
 
-def test_bugzilla_route_does_not_dedupe_failed_dispatch(client):
+def test_bugzilla_route_surfaces_failed_dispatch(client):
     class _FailingClient:
-        async def trigger_run(self, agent_name, inputs):
+        async def trigger_run(self, agent_name, inputs, *, dedupe_key=None):
             raise RuntimeError("run creation failed")
 
-    payload = _bugzilla_payload()
-    detected = detect_needinfo_request(payload, bot_login=BUGZILLA_BOT_LOGIN)
-    assert detected is not None
     app.dependency_overrides[webhooks.get_hackbot_client] = lambda: _FailingClient()
 
     with pytest.raises(RuntimeError, match="run creation failed"):
-        _post_bugzilla(client, payload)
-
-    assert f"ni{detected.flag_id}" not in webhooks._seen_bugzilla_events
+        _post_bugzilla(client, _bugzilla_payload())