fix(scheduler): retry rate-limited comment fetches with backoff

CHANGELOG.md

@@ -5,6 +5,31 @@ this file. The format follows Keep a Changelog, and versioned releases follow
 Semantic Versioning where the repository publishes a release.
 
 ## [Unreleased]
+- Root-cause the hourly PR-review-fix scheduler's silent `autofix_dispatches: 0`
+  on nearly every run (surfaced while investigating why 40 of `.github`'s 81
+  open PRs were stuck reporting "This branch has conflicts that must be
+  resolved"): `github-hourly-review-repair.yml`'s most recent run inspected
+  50 PRs and dispatched zero autofixes, with every candidate PR's decision
+  reading `"error": "API rate limit exceeded for installation ID ..."`. Two
+  compounding causes in `scripts/ci/pr_review_fix_scheduler.py`: (1)
+  `issue_comments()` fetched a PR's *entire* issue-comment history with the
+  default 30-per-page pagination even though `recent_fix_marker_exists()`
+  only ever needs the most recent marker; (2) `process_queue()`'s concurrent
+  comment-prefetch (up to 10 simultaneous `gh api --paginate` calls against
+  the same shared, org-wide-contended OpenCode app installation) silently
+  swallowed a failed fetch and then had `inspect_pr()` immediately retry the
+  *same* doomed call sequentially with zero backoff, doubling the wasted
+  request volume for every already-failing PR. `issue_comments()` now
+  requests `per_page=100` (cutting page count for long comment threads by
+  up to 3x) and retries a detected rate-limit error with a short linear
+  backoff (up to 2 attempts) before propagating; `process_queue()` now
+  caps prefetch concurrency at 4 workers instead of 10, and a PR whose
+  comment fetch still fails after retries is deferred to the next scheduled
+  pass (`"wait"`) instead of silently prefetch-swallowed and then
+  redundantly re-fetched and reported as a scary `"error"`. This is a
+  single shared script, so the fix applies identically to every one of the
+  ~19 product-specific hourly review-repair callers, not just `.github`'s
+  own.
 - Fix a Devin Review finding on PR #1456: the REST fallback path
   (`rest_pr_node`, used when GraphQL is unavailable) only ever fetched a
   head commit's CheckRuns (`commits/{sha}/check-runs`), never its classic
@@ -443,8 +468,7 @@ Semantic Versioning where the repository publishes a release.
   Informational, no change: the gap-baseline's repeated review-round
   narrative is this repo's own documented, intentional convention
   (ADR-0002: the baseline is "an operational snapshot," not a duplicate of
-  the ADR's design record), not accidental redundancy.
-- Raise `contextual_orchestrator_review_sidecar.sh`'s
+  the ADR's design record), not accidental redundancy.- Raise `contextual_orchestrator_review_sidecar.sh`'s
   `ORCHESTRATOR_CATALOG_FAMILY_CAP` default from 4 to 8: root-caused the
   live "no provider route passed the Strix plain-chat preflight" outage
   blocking `noema-review`/`opencode-review`/`strix` org-wide to

scripts/ci/pr_review_fix_scheduler.py

@@ -76,12 +76,47 @@ def run_json(args: list[str]) -> Any:
     return json.loads(run(["gh", *args]) or "null")
 
 
+RATE_LIMIT_ERROR_MARKERS = ("api rate limit exceeded", "secondary rate limit")
+ISSUE_COMMENTS_RETRY_ATTEMPTS = 2
+ISSUE_COMMENTS_RETRY_BACKOFF_SECONDS = 15
+
+
+def is_rate_limit_error(exc: BaseException) -> bool:
+    """Return whether an exception's message names a GitHub API rate limit."""
+    message = str(exc).lower()
+    return any(marker in message for marker in RATE_LIMIT_ERROR_MARKERS)
+
+
 def issue_comments(repo: str, number: int) -> list[dict[str, Any]]:
-    """Return issue comments for a PR."""
-    pages = run_json(
-        ["api", f"repos/{repo}/issues/{number}/comments", "--paginate", "--slurp"]
-    )
-    return [comment for page in pages for comment in page]
+    """Return issue comments for a PR, retrying transient rate-limit errors.
+
+    The shared OpenCode app installation's API budget is contended by many
+    concurrent org-wide scheduled workflows, so a rate-limit error here is
+    often transient. Retry it with a short linear backoff up to
+    ISSUE_COMMENTS_RETRY_ATTEMPTS times before propagating; any other error,
+    or a rate-limit error past the retry budget, propagates immediately.
+    ``per_page=100`` bounds the paginated request count for PRs with a long
+    review-comment history.
+    """
+    attempt = 0
+    while True:
+        try:
+            pages = run_json(
+                [
+                    "api",
+                    f"repos/{repo}/issues/{number}/comments",
+                    "--paginate",
+                    "--slurp",
+                    "-f",
+                    "per_page=100",
+                ]
+            )
+            return [comment for page in pages for comment in page]
+        except RuntimeError as exc:
+            if attempt >= ISSUE_COMMENTS_RETRY_ATTEMPTS or not is_rate_limit_error(exc):
+                raise
+            attempt += 1
+            time.sleep(ISSUE_COMMENTS_RETRY_BACKOFF_SECONDS * attempt)
 
 
 def recent_fix_marker_exists(
@@ -384,12 +419,20 @@ def process_queue(args: argparse.Namespace) -> int:
             prs_needing_comments.append(pr)
 
     comments_by_pr: dict[int, list[dict[str, Any]]] = {}
+    comment_fetch_errors: dict[int, str] = {}
     if len(prs_needing_comments) <= 1:
         for pr in prs_needing_comments:
             pr_number = int(pr["number"])
-            comments_by_pr[pr_number] = issue_comments(args.repo, pr_number)
+            try:
+                comments_by_pr[pr_number] = issue_comments(args.repo, pr_number)
+            except Exception as exc:
+                comment_fetch_errors[pr_number] = str(exc)
     else:
-        max_workers = min(10, len(prs_needing_comments))
+        # Bounded well below GitHub's per-installation rate-limit budget: this
+        # scheduler is one of many concurrent org-wide callers sharing the
+        # same OpenCode app installation, so a wide burst of simultaneous
+        # comment fetches here can exhaust that shared budget on its own.
+        max_workers = min(4, len(prs_needing_comments))
         with concurrent.futures.ThreadPoolExecutor(
             max_workers=max_workers
         ) as executor:
@@ -400,16 +443,17 @@ def fetch_comments(
                 """Fetch one PR's issue comments for parallel queue inspection."""
                 return pr_number, issue_comments(args.repo, pr_number)
 
-            futures = [
-                executor.submit(fetch_comments, int(pr["number"]))
+            futures = {
+                executor.submit(fetch_comments, int(pr["number"])): int(pr["number"])
                 for pr in prs_needing_comments
-            ]
+            }
             for future in concurrent.futures.as_completed(futures):
+                pr_number = futures[future]
                 try:
-                    pr_number, comments = future.result()
+                    _, comments = future.result()
                     comments_by_pr[pr_number] = comments
-                except Exception:
-                    pass
+                except Exception as exc:
+                    comment_fetch_errors[pr_number] = str(exc)
 
     for pr in prs:
         inspected += 1
@@ -422,8 +466,24 @@ def fetch_comments(
                 }
             )
             continue
+        pr_number = int(pr["number"])
+        if pr_number in comment_fetch_errors:
+            decisions.append(
+                {
+                    "pr": pr["number"],
+                    "action": "wait",
+                    "reasons": [
+                        "issue comment fetch failed; deferring to next scheduled "
+                        f"pass: {comment_fetch_errors[pr_number]}"
+                    ],
+                }
+            )
+            print(
+                f"PR #{pr['number']}: wait: issue comment fetch failed; "
+                "deferring to next scheduled pass"
+            )
+            continue
         try:
-            pr_number = int(pr["number"])
             action, reasons = inspect_pr(
                 args.repo,
                 pr,

tests/test_pr_review_fix_scheduler.py

@@ -628,6 +628,122 @@ def fake_run(argv, *, stdin=None):
     assert payload["client_payload"]["target_repository"] == "owner/repo"
 
 
+def test_is_rate_limit_error_matches_known_github_signatures():
+    """Rate-limit detection matches GitHub's primary and secondary wording."""
+    assert fix.is_rate_limit_error(RuntimeError("gh: API rate limit exceeded for installation ID 1"))
+    assert fix.is_rate_limit_error(RuntimeError("You have exceeded a Secondary rate limit"))
+    assert not fix.is_rate_limit_error(RuntimeError("gh: Resource not accessible by integration"))
+
+
+def test_issue_comments_retries_rate_limit_then_succeeds(monkeypatch):
+    """A transient rate-limit error is retried with backoff before succeeding."""
+    calls = []
+    sleeps = []
+    attempts = {"count": 0}
+
+    def fake_run(argv, *, stdin=None):
+        calls.append(argv)
+        attempts["count"] += 1
+        if attempts["count"] < 2:
+            raise RuntimeError("gh: API rate limit exceeded for installation ID 1")
+        return "[[{\"id\": 1}]]"
+
+    monkeypatch.setattr(fix, "run", fake_run)
+    monkeypatch.setattr(fix.time, "sleep", lambda seconds: sleeps.append(seconds))
+
+    assert fix.issue_comments("owner/repo", 7) == [{"id": 1}]
+    assert len(calls) == 2
+    assert sleeps == [fix.ISSUE_COMMENTS_RETRY_BACKOFF_SECONDS]
+    assert all("per_page=100" in argv for argv in calls)
+
+
+def test_issue_comments_exhausts_retries_and_raises(monkeypatch):
+    """A persistent rate-limit error still propagates once retries are spent."""
+    sleeps = []
+
+    def always_rate_limited(argv, *, stdin=None):
+        raise RuntimeError("gh: API rate limit exceeded for installation ID 1")
+
+    monkeypatch.setattr(fix, "run", always_rate_limited)
+    monkeypatch.setattr(fix.time, "sleep", lambda seconds: sleeps.append(seconds))
+
+    with pytest.raises(RuntimeError, match="rate limit exceeded"):
+        fix.issue_comments("owner/repo", 7)
+    assert len(sleeps) == fix.ISSUE_COMMENTS_RETRY_ATTEMPTS
+
+
+def test_issue_comments_does_not_retry_non_rate_limit_errors(monkeypatch):
+    """A non-rate-limit failure propagates immediately without backoff."""
+    sleeps = []
+
+    def fail_once(argv, *, stdin=None):
+        raise RuntimeError("gh: Resource not accessible by integration")
+
+    monkeypatch.setattr(fix, "run", fail_once)
+    monkeypatch.setattr(fix.time, "sleep", lambda seconds: sleeps.append(seconds))
+
+    with pytest.raises(RuntimeError, match="not accessible"):
+        fix.issue_comments("owner/repo", 7)
+    assert sleeps == []
+
+
+def test_process_queue_defers_prs_whose_comment_fetch_failed(monkeypatch, capsys):
+    """A single failing comment fetch defers that PR instead of erroring."""
+    pr = make_pr()
+    monkeypatch.setattr(fix, "fetch_open_prs", lambda repo, max_prs: [pr])
+    monkeypatch.setattr(fix, "needs_autofix", lambda pr: (True, ("reason",)))
+
+    def failing_issue_comments(repo, number):
+        raise RuntimeError("gh: API rate limit exceeded for installation ID 1")
+
+    monkeypatch.setattr(fix, "issue_comments", failing_issue_comments)
+    inspect_calls = []
+    monkeypatch.setattr(
+        fix,
+        "inspect_pr",
+        lambda repo, pr, args, **kwargs: inspect_calls.append(kwargs) or ("dispatch", ("reason",)),
+    )
+
+    assert fix.main(["--repo", "owner/repo", "--base-branch", "main", "--dry-run"]) == 0
+
+    assert inspect_calls == []
+    payload = json.loads(capsys.readouterr().out.strip().splitlines()[-1])
+    assert payload["autofix_dispatches"] == 0
+    assert payload["decisions"][0]["action"] == "wait"
+    assert "deferring to next scheduled pass" in payload["decisions"][0]["reasons"][0]
+
+
+def test_process_queue_concurrent_fetch_defers_only_the_failing_pr(monkeypatch, capsys):
+    """The concurrent comment-fetch path defers only the PR whose fetch failed."""
+    pr1 = make_pr(number=1)
+    pr2 = make_pr(number=2)
+    monkeypatch.setattr(fix, "fetch_open_prs", lambda repo, max_prs: [pr1, pr2])
+    monkeypatch.setattr(fix, "needs_autofix", lambda pr: (True, ("reason",)))
+
+    def flaky_issue_comments(repo, number):
+        if number == 1:
+            raise RuntimeError("gh: API rate limit exceeded for installation ID 1")
+        return []
+
+    monkeypatch.setattr(fix, "issue_comments", flaky_issue_comments)
+    inspect_calls = []
+
+    def fake_inspect_pr(repo, pr, args, **kwargs):
+        inspect_calls.append((pr["number"], kwargs.get("comments")))
+        return "dispatch", ("reason",)
+
+    monkeypatch.setattr(fix, "inspect_pr", fake_inspect_pr)
+
+    assert fix.main(["--repo", "owner/repo", "--base-branch", "main", "--dry-run", "--max-dispatches", "2"]) == 0
+
+    assert inspect_calls == [(2, [])]
+    payload = json.loads(capsys.readouterr().out.strip().splitlines()[-1])
+    decisions_by_pr = {d["pr"]: d for d in payload["decisions"]}
+    assert decisions_by_pr[1]["action"] == "wait"
+    assert "deferring to next scheduled pass" in decisions_by_pr[1]["reasons"][0]
+    assert decisions_by_pr[2]["action"] == "dispatch"
+
+
 def _approved_dirty_pr(**overrides):
     """Return an approved PR that GitHub reports as conflicting."""
     fields = {