-
Notifications
You must be signed in to change notification settings - Fork 0
fix(automation): stop mention sweep after shared rate-limit exhaustion #1012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
30a5c13
dd9a8d2
965896e
0cccd7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Agent mention sweep rate-limit fail-fast boundary | ||
|
|
||
| Updated: 2026-08-15 | ||
|
|
||
| ## Incident | ||
|
|
||
| Scheduled `Review Agent Mention Router` run `31868885733` exhausted the OpenCode GitHub App installation REST budget before processing the requested review queue. The sweep continued traversing repositories after the first installation-wide `API rate limit exceeded` response and finished with zero dispatches plus 116 isolated failures. Repeating requests after the shared budget is exhausted cannot recover candidate-local work and consumes runner time while obscuring the single control-plane cause. | ||
|
|
||
| ## Decision | ||
|
|
||
| Treat explicit GitHub primary- or secondary-rate-limit messages as **sweep-global capacity exhaustion**, not candidate-local failures. The sweep records the first failed scope, then raises `SweepRateLimitExhausted` immediately. Ordinary repository, pull-request, review, acknowledgement, and dispatch failures remain isolated exactly as before. | ||
|
|
||
| This change is intentionally narrow. It does not retry, sleep, change credentials, widen permissions, alter the canonical invocation key, modify the exact-name artifact ledger, or claim that a failed request was dispatched. A later scheduled invocation may run after GitHub restores capacity. Interactive/local routing and the separate concurrency-isolation repair remain independent control-plane lanes. | ||
|
|
||
| The scheduled CLI emits an error with the budget-reset action and exits 1, so | ||
| operators see a bounded failure rather than an unhandled traceback. | ||
|
|
||
| ## Why fail-fast | ||
|
|
||
| GitHub documents that installation access tokens share an installation-level primary REST budget. When a primary limit is exceeded, requests return HTTP 403 or 429 and callers should not retry until the reset time. GitHub also states that integrations should stop and wait on secondary-rate-limit responses; continuing to make requests while rate-limited may lead to integration bans. The current sweep cannot safely infer reset headers from the `gh` exception string, so the bounded action is to stop the current scheduled traversal rather than amplify the exhausted state. | ||
|
|
||
| ## Verification contract | ||
|
|
||
| - a synthetic installation-wide primary-limit error on the first PR aborts before the second PR is touched; | ||
| - exactly one failure is recorded for the first exhausted scope; | ||
| - secondary-rate-limit messages are classified as sweep-global exhaustion; | ||
| - unrelated authorization/resource errors remain candidate-local and preserve existing failure isolation; | ||
| - the permanent agent-mention quality suite continues to require 100% owned production statement/branch and public docstring coverage. | ||
|
|
||
| ## Rollback | ||
|
|
||
| Revert `SweepRateLimitExhausted`, `is_rate_limit_exhaustion`, and their focused regression if GitHub changes the CLI error contract or the router gains structured response-header handling. Do not restore repeated API calls after a proven shared rate-limit exhaustion without an equivalent bounded backoff/stop mechanism. | ||
|
|
||
| ## References | ||
|
|
||
| GitHub. (n.d.). *Rate limits for the REST API*. GitHub Docs. Retrieved August 15, 2026, from https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api | ||
|
|
||
| GitHub. (n.d.). *Rate limits for GitHub Apps*. GitHub Docs. Retrieved August 15, 2026, from https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/rate-limits-for-github-apps |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,21 @@ class SweepMetrics: | |
| failures: int = 0 | ||
|
|
||
|
|
||
| class SweepRateLimitExhausted(RuntimeError): | ||
| """Signal that shared GitHub API capacity is unavailable for this sweep.""" | ||
|
|
||
|
|
||
| def is_rate_limit_exhaustion(error: Exception) -> bool: | ||
| """Return whether an API error says the shared primary/secondary budget is exhausted.""" | ||
|
|
||
| message = " ".join(str(error).split()).casefold() | ||
| return ( | ||
| "api rate limit exceeded" in message | ||
| or "api rate limit already exceeded" in message | ||
| or "secondary rate limit" in message | ||
| ) | ||
|
|
||
|
|
||
| def parse_timestamp(value: str) -> datetime: | ||
| """Parse one GitHub ISO-8601 timestamp into timezone-aware UTC.""" | ||
|
|
||
|
|
@@ -301,13 +316,19 @@ def sweep( | |
| dispatched = 0 | ||
|
|
||
| def record_failure(scope: str, error: Exception) -> None: | ||
| """Record one isolated error and preserve the remaining sweep.""" | ||
| """Record isolated errors but stop when the shared API budget is exhausted.""" | ||
|
|
||
| counters.failures += 1 | ||
| message = " ".join(str(error).split()) or error.__class__.__name__ | ||
| print( | ||
| f"::warning::Agent mention sweep skipped {scope}: {message[:1000]}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still says the scope was skipped, then raises. Operators will treat it as a local skip and re-run. Emit |
||
| ) | ||
| if is_rate_limit_exhaustion(error): | ||
| raise SweepRateLimitExhausted( | ||
| "GitHub API rate limit exhausted; stopping organization sweep " | ||
| "to preserve the shared installation budget; wait for the " | ||
| "budget reset before retrying" | ||
| ) from error | ||
|
|
||
| for issue in list_recent_pull_requests( | ||
| target_client, | ||
|
|
@@ -374,19 +395,23 @@ def main(argv: Sequence[str] | None = None) -> int: | |
| os.environ.get("OPENCODE_REPOSITORY_DISPATCH_TARGETS", "") | ||
| ) | ||
| metrics = SweepMetrics() | ||
| sweep( | ||
| target_client=GitHubClient( | ||
| os.environ.get("TARGET_REPOSITORY_TOKEN", "") | ||
| ), | ||
| dispatch_client=GitHubClient(os.environ.get("AGENT_DISPATCH_TOKEN", "")), | ||
| organization=args.organization, | ||
| repository_source=args.repository_source, | ||
| lookback_hours=args.lookback_hours, | ||
| max_dispatches=args.max_dispatches, | ||
| opencode_allowlist=allowlist, | ||
| dry_run=args.dry_run, | ||
| metrics=metrics, | ||
| ) | ||
| try: | ||
| sweep( | ||
| target_client=GitHubClient( | ||
| os.environ.get("TARGET_REPOSITORY_TOKEN", "") | ||
| ), | ||
| dispatch_client=GitHubClient(os.environ.get("AGENT_DISPATCH_TOKEN", "")), | ||
| organization=args.organization, | ||
| repository_source=args.repository_source, | ||
| lookback_hours=args.lookback_hours, | ||
| max_dispatches=args.max_dispatches, | ||
| opencode_allowlist=allowlist, | ||
| dry_run=args.dry_run, | ||
| metrics=metrics, | ||
| ) | ||
| except SweepRateLimitExhausted as exc: | ||
| print(f"::error::{exc}") | ||
| return 1 | ||
| return 1 if metrics.failures else 0 | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| """Fail-fast regressions for exhausted GitHub mention-router API budgets.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import importlib | ||
| import sys | ||
| from datetime import datetime, timezone | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
| SCRIPTS = ROOT / "scripts" / "ci" | ||
| sys.path.insert(0, str(SCRIPTS)) | ||
|
|
||
|
|
||
| def module(): | ||
| """Reload the sweep module for isolated monkeypatching.""" | ||
|
|
||
| return importlib.reload(importlib.import_module("agent_mention_sweep")) | ||
|
|
||
|
|
||
| def test_primary_rate_limit_exhaustion_stops_the_sweep(monkeypatch) -> None: | ||
| """A shared installation budget exhaustion must stop further API work.""" | ||
|
|
||
| sweep = module() | ||
| issues = [ | ||
| {"repository": "ContextualWisdomLab/first", "number": 1}, | ||
| {"repository": "ContextualWisdomLab/second", "number": 2}, | ||
| ] | ||
| monkeypatch.setattr( | ||
| sweep, | ||
| "list_recent_pull_requests", | ||
| lambda *args, **kwargs: iter(issues), | ||
| ) | ||
| visited: list[str] = [] | ||
|
|
||
| def build_requests(client, *, issue, since): | ||
| del client, since | ||
| visited.append(issue["repository"]) | ||
| raise RuntimeError( | ||
| "gh: API rate limit exceeded for installation ID 141441800 (HTTP 403)" | ||
| ) | ||
|
|
||
| monkeypatch.setattr(sweep, "build_requests_for_pull_request", build_requests) | ||
| metrics = sweep.SweepMetrics() | ||
|
|
||
| with pytest.raises(sweep.SweepRateLimitExhausted, match="rate limit"): | ||
| sweep.sweep( | ||
| target_client=object(), | ||
| dispatch_client=object(), | ||
| organization="ContextualWisdomLab", | ||
| repository_source="installation", | ||
| lookback_hours=24, | ||
| max_dispatches=5, | ||
| opencode_allowlist=frozenset(), | ||
| now=datetime(2026, 8, 15, tzinfo=timezone.utc), | ||
| metrics=metrics, | ||
| ) | ||
|
|
||
| assert visited == ["ContextualWisdomLab/first"] | ||
| assert metrics.failures == 1 | ||
|
|
||
|
|
||
| def test_secondary_rate_limit_exhaustion_is_global() -> None: | ||
| """Secondary-limit messages are classified as sweep-global exhaustion.""" | ||
|
|
||
| sweep = module() | ||
| assert sweep.is_rate_limit_exhaustion( | ||
| RuntimeError("You have exceeded a secondary rate limit. Please retry later.") | ||
| ) | ||
| assert sweep.is_rate_limit_exhaustion( | ||
| RuntimeError("API rate limit already exceeded for installation ID 141441800") | ||
| ) | ||
| assert not sweep.is_rate_limit_exhaustion(RuntimeError("Resource not accessible")) | ||
|
|
||
|
|
||
| def test_main_reports_rate_limit_reset_and_returns_failure(monkeypatch, capsys) -> None: | ||
| """The scheduled CLI reports the shared-budget stop without a traceback.""" | ||
|
|
||
| sweep = module() | ||
| monkeypatch.setenv("TARGET_REPOSITORY_TOKEN", "target") | ||
| monkeypatch.setenv("AGENT_DISPATCH_TOKEN", "dispatch") | ||
| def fail_for_rate_limit(**kwargs): | ||
| del kwargs | ||
| raise sweep.SweepRateLimitExhausted( | ||
| "wait for the budget reset before retrying" | ||
| ) | ||
|
|
||
| monkeypatch.setattr(sweep, "sweep", fail_for_rate_limit) | ||
|
|
||
| assert sweep.main([]) == 1 | ||
| assert "wait for the budget reset before retrying" in capsys.readouterr().out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This contiguous needle misses
API rate limit already exceeded. That GitHub wording insertsalreadybetweenlimitandexceeded, so the first exhausted repository would be recorded as an isolated skip and later repositories would keep consuming the empty installation budget.Match
rate limitplusexceeded/exhausted, orsecondary rate limit. Add a regression that feeds thealready exceededstring and proves the next repository is never requested.