fix(agent-mention-sweep): retry GitHubClient on transient rate limit

scripts/ci/agent_mention_router.py

@@ -9,6 +9,7 @@
 import os
 import re
 import subprocess
+import time
 from dataclasses import dataclass
 from typing import Any, Sequence
 
@@ -35,6 +36,8 @@
 RECEIPT_RE = re.compile(r"<!-- cwl-agent-mention-receipt:(\d+) -->")
 REPOSITORY_DISPATCH_CLIENT_PAYLOAD_MAX_KEYS = 10
 GITHUB_API_TIMEOUT_SECONDS = 30
+GITHUB_API_MAX_ATTEMPTS = 6
+RATE_LIMIT_DIAGNOSTIC_RE = re.compile(r"API rate limit exceeded", re.IGNORECASE)
 
 
 @dataclass(frozen=True)
@@ -67,41 +70,64 @@ def request(
         *,
         input_payload: dict[str, Any] | None = None,
     ) -> Any:
-        """Execute one bounded ``gh api`` request and decode optional JSON."""
+        """Execute one bounded ``gh api`` request and decode optional JSON.
+
+        Retries only a completed request that failed on the shared GitHub
+        App installation-token rate limit, up to ``GITHUB_API_MAX_ATTEMPTS``
+        times with linear backoff, mirroring the established retry
+        convention in ``strix.yml``'s target-repository visibility lookup.
+        GitHub rejects a rate-limited request at admission time before it
+        mutates anything, so retrying it (including a POST, e.g. a
+        ``repository_dispatch`` or comment write) cannot double-apply an
+        already-completed write. Retry is intentionally NOT attempted for
+        other failures (permission errors, 404s, malformed requests): those
+        are not transient, and a non-idempotent write that failed for an
+        unknown reason must not be silently resent. A hung request
+        (``TimeoutExpired``) is likewise not retried; a stuck ``gh`` process
+        should fail immediately rather than compound the wait.
+        """
 
         command = ["gh", "api", *args]
         if input_payload is not None:
             command.extend(["--input", "-"])
         environment = os.environ.copy()
         environment["GH_TOKEN"] = self._token
-        try:
-            completed = subprocess.run(
-                command,
-                input=None if input_payload is None else json.dumps(input_payload),
-                text=True,
-                capture_output=True,
-                shell=False,
-                check=False,
-                env=environment,
-                timeout=GITHUB_API_TIMEOUT_SECONDS,
-            )
-        except subprocess.TimeoutExpired as exc:
-            raise RuntimeError(
-                "gh api timed out after "
-                f"{GITHUB_API_TIMEOUT_SECONDS} seconds"
-            ) from exc
-        return_code = int(getattr(completed, "returncode", 0))
-        if return_code:
-            diagnostic = " ".join(
-                str(getattr(completed, "stderr", "") or "").split()
-            )
+        payload = None if input_payload is None else json.dumps(input_payload)
+        attempt = 0
+        while True:
+            attempt += 1
+            try:
+                completed = subprocess.run(
+                    command,
+                    input=payload,
+                    text=True,
+                    capture_output=True,
+                    shell=False,
+                    check=False,
+                    env=environment,
+                    timeout=GITHUB_API_TIMEOUT_SECONDS,
+                )
+            except subprocess.TimeoutExpired as exc:
+                raise RuntimeError(
+                    "gh api timed out after "
+                    f"{GITHUB_API_TIMEOUT_SECONDS} seconds"
+                ) from exc
+            return_code = int(getattr(completed, "returncode", 0))
+            if not return_code:
+                output = completed.stdout.strip()
+                return None if not output else json.loads(output)
+            diagnostic = " ".join(str(getattr(completed, "stderr", "") or "").split())
             if not diagnostic:
                 diagnostic = "no stderr output"
+            retryable = RATE_LIMIT_DIAGNOSTIC_RE.search(diagnostic) is not None
+            if retryable and attempt < GITHUB_API_MAX_ATTEMPTS:
+                time.sleep(attempt * 5)
+                continue
+            suffix = f" after {attempt} attempts" if attempt > 1 else ""
             raise RuntimeError(
-                f"gh api failed with exit code {return_code}: {diagnostic[:2000]}"
+                f"gh api failed with exit code {return_code}{suffix}: "
+                f"{diagnostic[:2000]}"
             )
-        output = completed.stdout.strip()
-        return None if not output else json.loads(output)
 
 
 def exact_mentions(body: str) -> tuple[str, ...]:

tests/test_agent_mention_timeout_bounds.py

@@ -100,6 +100,95 @@ def timeout_run(command, **kwargs):
         )
 
 
+def test_github_client_retries_a_completed_failure_with_backoff(monkeypatch) -> None:
+    """A transient rate limit succeeds after retrying with linear backoff."""
+
+    router = router_module()
+    attempts = []
+    sleeps = []
+
+    def flaky_run(command, **kwargs):
+        """Fail with a rate-limit diagnostic twice, then succeed."""
+        del kwargs
+        attempts.append(command)
+        if len(attempts) < 3:
+            return SimpleNamespace(
+                stdout="",
+                stderr="gh: API rate limit exceeded for installation ID 1",
+                returncode=1,
+            )
+        return SimpleNamespace(stdout='{"ok": true}\n', returncode=0)
+
+    monkeypatch.setattr(router.subprocess, "run", flaky_run)
+    monkeypatch.setattr(router.time, "sleep", lambda seconds: sleeps.append(seconds))
+    result = router.GitHubClient("token").request(["repos/x/y"])
+
+    assert result == {"ok": True}
+    assert len(attempts) == 3
+    assert sleeps == [5, 10]
+
+
+def test_github_client_fails_closed_after_exhausting_retries(monkeypatch) -> None:
+    """A persistent rate limit still fails after all retries, not silently."""
+
+    router = router_module()
+    attempts = []
+    sleeps = []
+
+    def always_fails(command, **kwargs):
+        """Fail every attempt with a stable rate-limit diagnostic."""
+        del kwargs
+        attempts.append(command)
+        return SimpleNamespace(
+            stdout="",
+            stderr="gh: API rate limit exceeded for installation ID 1",
+            returncode=1,
+        )
+
+    monkeypatch.setattr(router.subprocess, "run", always_fails)
+    monkeypatch.setattr(router.time, "sleep", lambda seconds: sleeps.append(seconds))
+    with pytest.raises(
+        RuntimeError,
+        match=r"gh api failed with exit code 1 after 6 attempts: "
+        r"gh: API rate limit exceeded",
+    ):
+        router.GitHubClient("token").request(["repos/x/y"])
+
+    assert len(attempts) == router.GITHUB_API_MAX_ATTEMPTS == 6
+    assert sleeps == [5, 10, 15, 20, 25]
+
+
+def test_github_client_does_not_retry_a_non_rate_limit_failure(monkeypatch) -> None:
+    """A permanent failure (e.g. 404, permission denied) fails on the first attempt.
+
+    Retrying an unknown failure could resend a non-idempotent write (a
+    ``repository_dispatch`` POST or an acknowledgement comment) that may
+    have already applied server-side; only the specific, safe-to-retry
+    rate-limit diagnostic is retried.
+    """
+
+    router = router_module()
+    attempts = []
+    sleeps = []
+
+    def permission_denied(command, **kwargs):
+        """Fail once with a diagnostic that is not a rate limit."""
+        del kwargs
+        attempts.append(command)
+        return SimpleNamespace(stdout="", stderr="permission denied", returncode=1)
+
+    monkeypatch.setattr(router.subprocess, "run", permission_denied)
+    monkeypatch.setattr(router.time, "sleep", lambda seconds: sleeps.append(seconds))
+    with pytest.raises(
+        RuntimeError,
+        match=r"gh api failed with exit code 1: permission denied",
+    ):
+        router.GitHubClient("token").request(["repos/x/y"])
+
+    assert len(attempts) == 1
+    assert sleeps == []
+
+
 def test_repository_fanout_uses_exactly_four_workers_at_scale(monkeypatch) -> None:
     """Five repositories exercise the fixed four-worker production ceiling."""