-
Notifications
You must be signed in to change notification settings - Fork 0
fix(scheduler): retry and gracefully defer shared installation rate limits #1245
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
base: main
Are you sure you want to change the base?
Changes from all commits
c895eec
d67ead6
d007bce
9262430
7046ba9
d696be5
d8bdb24
c62145e
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 |
|---|---|---|
|
|
@@ -974,6 +974,8 @@ jobs: | |
| failures=0 | ||
| unavailable=0 | ||
| unavailable_repos=() | ||
| rate_limited=0 | ||
| rate_limited_repos=() | ||
| # These are organization-wide budgets. They must be consumed across | ||
| # the repository loop, not reset for every target repository; resetting | ||
| # them here can enqueue hundreds of long-running review jobs per sweep. | ||
|
|
@@ -1075,12 +1077,33 @@ jobs: | |
| # repository at all — the OpenCode app is not installed there or | ||
| # PR_REVIEW_MERGE_TOKEN does not cover it. The automation can never | ||
| # merge those PRs regardless, so this is a skipped, non-fatal | ||
| # "unavailable" repository, not a failure the sweep can act on. Any | ||
| # other non-zero exit is a genuine per-repository failure. | ||
| # "unavailable" repository, not a failure the sweep can act on. | ||
| # | ||
| # "API rate limit exceeded" means the shared GitHub App | ||
| # installation-token bucket (5,000-12,500 requests/hour, pooled | ||
| # across at least eight other central workflows that mint tokens | ||
| # for the same installation) is exhausted for this hourly window. | ||
| # That is routine cross-workflow contention, not a defect in this | ||
| # repository, and it self-heals on GitHub's own reset schedule; | ||
| # treating it as a hard failure previously turned one exhausted | ||
| # bucket into a permanently red */15 * * * * cron for as long as | ||
| # the contention lasted. Because the installation bucket is shared | ||
| # by every remaining repository, the current rotation stops after | ||
| # recording the first exhausted request instead of repeating the | ||
| # same bounded retries and queue-hygiene calls for every target. | ||
| # Deferred work is picked up on a later rotation after reset. | ||
| # | ||
| # Any other non-zero exit is a genuine per-repository failure. | ||
| if printf '%s' "$sweep_output" | grep -qF "Resource not accessible by integration"; then | ||
| echo "::warning::Skipping ${repo_full_name}: the sweep credential lacks access (HTTP 403 Resource not accessible by integration). Install the OpenCode app on this repository or grant PR_REVIEW_MERGE_TOKEN access to include it in the sweep." | ||
| unavailable=$((unavailable + 1)) | ||
| unavailable_repos+=("$repo_full_name") | ||
| elif printf '%s' "$sweep_output" | grep -qiF "API rate limit exceeded"; then | ||
| echo "::warning::Deferring ${repo_full_name} and stopping this rotation: the shared GitHub App installation-token rate limit is exhausted (HTTP 403 API rate limit exceeded). Deferred repositories are retried automatically on the next sweep rotation once the bucket resets." | ||
| rate_limited=$((rate_limited + 1)) | ||
| rate_limited_repos+=("$repo_full_name") | ||
| echo "::endgroup::" | ||
| break | ||
|
Comment on lines
+1101
to
+1106
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. 📝 Info: Queue-hygiene calls not covered by defer logic The defer-and-stop branch fires only when the Python scheduler exits non-zero. When it succeeds but the following queue-hygiene Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+1101
to
+1106
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. 📝 Info: Rate-limit branch classifies via substring grep Classification uses Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+1101
to
+1106
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. 🟡 Mid-scan rate limits bypass deferral When a per-PR request hits the shared limit, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| else | ||
| echo "::error::Queue sweep failed for ${repo_full_name}; see the decision log above for the concrete per-PR reason." | ||
| failures=$((failures + 1)) | ||
|
|
@@ -1240,6 +1263,13 @@ jobs: | |
| if [ "$unavailable" -gt 0 ]; then | ||
| echo "::warning::${unavailable} repository(ies) were skipped as unreachable by the sweep credential (HTTP 403): ${unavailable_repos[*]}. These do not fail the sweep; install the OpenCode app or grant PR_REVIEW_MERGE_TOKEN access to include them." | ||
| fi | ||
| if [ "$rate_limited" -gt 0 ]; then | ||
| # No fail-closed ceiling here, unlike ORG_SWEEP_MAX_UNAVAILABLE below: | ||
| # one exhausted shared installation-token bucket affects every | ||
| # remaining repository, so the rotation stops after the first | ||
| # observed exhaustion instead of multiplying retries and API calls. | ||
| echo "::warning::The organization sweep stopped after ${rate_limited} observed rate-limit exhaustion(s): ${rate_limited_repos[*]}. Deferred work does not fail this sweep and is retried automatically once the shared bucket resets." | ||
| fi | ||
| # Fail-closed guard: a handful of un-enrolled repositories is expected, | ||
| # but if MORE than ORG_SWEEP_MAX_UNAVAILABLE repositories become | ||
| # unreachable at once the sweep credential itself has regressed and the | ||
|
|
||
|
seonghobae marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -721,6 +721,16 @@ def repository_dispatch_wait_reason(repo: str, workflow: str) -> str | None: | |
| "unexpected EOF", | ||
| "received from peer", | ||
| ) | ||
| # The exact diagnostic GitHub emits when a GitHub App installation token's | ||
| # shared primary rate limit (5,000-12,500 requests/hour, pooled across every | ||
| # workflow that mints a token for the same installation -- at least eight | ||
| # other central workflows in this repository alone) is exhausted. Matches | ||
| # the pattern scripts/ci/agent_mention_router.py already retries on. Kept | ||
| # distinct from TRANSIENT_GITHUB_API_ERRORS because this is routine | ||
| # cross-workflow contention, not infrastructure flakiness, and needs a | ||
| # reset-time-aware wait rather than a short fixed backoff. | ||
| RATE_LIMIT_DIAGNOSTIC_RE = re.compile(r"API rate limit exceeded", re.IGNORECASE) | ||
| GITHUB_API_RATE_LIMIT_RETRY_CAP_SECONDS = 60 | ||
|
|
||
|
|
||
| def is_transient_github_api_error(exc: Exception) -> bool: | ||
|
|
@@ -732,6 +742,49 @@ def is_transient_github_api_error(exc: Exception) -> bool: | |
| return any(marker in message or marker.lower() in folded for marker in TRANSIENT_GITHUB_API_ERRORS) | ||
|
|
||
|
|
||
| def is_rate_limited_error(exc: Exception) -> bool: | ||
| """Return whether a GitHub API failure is the shared installation rate limit. | ||
|
|
||
| Distinct from :func:`is_transient_github_api_error`: this is routine | ||
| contention from sibling workflows sharing one GitHub App installation's | ||
| request bucket, not an infrastructure error, so callers give it a | ||
| reset-time-aware wait via :func:`rate_limit_retry_delay_seconds` instead | ||
| of the short fixed backoff used for a passing transient failure. | ||
| """ | ||
| return RATE_LIMIT_DIAGNOSTIC_RE.search(str(exc)) is not None | ||
|
|
||
|
|
||
| def rate_limit_retry_delay_seconds(resource: str, attempt: int) -> int: | ||
| """Return how long to wait before retrying a rate-limited GitHub API call. | ||
|
|
||
| Prefers GitHub's own reported reset time for ``resource`` (``"core"`` | ||
| for REST, ``"graphql"`` for GraphQL), read from ``GET /rate_limit`` -- | ||
| which GitHub documents as exempt from the primary rate limit it reports, | ||
| so checking it does not deepen the exhaustion it is diagnosing. Falls | ||
| back to the same capped exponential backoff already used for other | ||
| transient errors when that lookup is itself unavailable or does not | ||
| confirm the bucket is empty, and never waits longer than | ||
| ``GITHUB_API_RATE_LIMIT_RETRY_CAP_SECONDS`` for any one retry interval. | ||
| After the bounded attempts are exhausted, the error reaches the calling | ||
| workflow's skip-and-defer handling so the repository can be picked back | ||
| up on the next sweep rotation. | ||
| """ | ||
| fallback = min(2 ** (attempt - 1), GITHUB_API_RATE_LIMIT_RETRY_CAP_SECONDS) | ||
| try: | ||
| status = json.loads(run_github_read(["gh", "api", "rate_limit"])) | ||
| bucket = (status.get("resources") or {}).get(resource) or {} | ||
| remaining = bucket.get("remaining") | ||
| reset_epoch = bucket.get("reset") | ||
| except (RuntimeError, json.JSONDecodeError, AttributeError): | ||
|
Comment on lines
+774
to
+778
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. |
||
| return fallback | ||
| if remaining != 0 or not isinstance(reset_epoch, int): | ||
| return fallback | ||
| delay = reset_epoch - int(time.time()) + 5 | ||
| if delay <= 0: | ||
| return fallback | ||
| return min(delay, GITHUB_API_RATE_LIMIT_RETRY_CAP_SECONDS) | ||
|
seonghobae marked this conversation as resolved.
seonghobae marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def gh_graphql(query: str, **fields: str | int) -> dict[str, Any]: | ||
| """Run a GitHub GraphQL query through gh and decode the JSON response.""" | ||
| cmd = ["gh", "api", "graphql", "-F", "query=@-"] | ||
|
|
@@ -743,13 +796,21 @@ def gh_graphql(query: str, **fields: str | int) -> dict[str, Any]: | |
| try: | ||
| return json.loads(run_github_read(cmd, stdin=query)) | ||
| except (RuntimeError, json.JSONDecodeError) as exc: | ||
| if attempt >= max_attempts or not is_transient_github_api_error(exc): | ||
| rate_limited = is_rate_limited_error(exc) | ||
| if attempt >= max_attempts or not (rate_limited or is_transient_github_api_error(exc)): | ||
| raise | ||
| delay = min(2 ** (attempt - 1), 8) | ||
| print( | ||
| f"Transient GitHub GraphQL error on attempt {attempt}/{max_attempts}; retrying in {delay}s", | ||
| file=sys.stderr, | ||
| ) | ||
| if rate_limited: | ||
| delay = rate_limit_retry_delay_seconds("graphql", attempt) | ||
| print( | ||
| f"Rate-limited GitHub GraphQL error on attempt {attempt}/{max_attempts}; retrying in {delay}s", | ||
| file=sys.stderr, | ||
| ) | ||
| else: | ||
| delay = min(2 ** (attempt - 1), 8) | ||
| print( | ||
| f"Transient GitHub GraphQL error on attempt {attempt}/{max_attempts}; retrying in {delay}s", | ||
| file=sys.stderr, | ||
| ) | ||
| time.sleep(delay) | ||
|
|
||
|
|
||
|
|
@@ -760,9 +821,34 @@ def github_resource_inaccessible(exc: RuntimeError) -> bool: | |
|
|
||
|
|
||
| def gh_api_json(path: str) -> Any: | ||
| """Run a GitHub REST API request through gh and decode the JSON response.""" | ||
| """Run a GitHub REST API request through gh and decode the JSON response. | ||
|
|
||
| return json.loads(run_github_read(["gh", "api", path])) | ||
| Retries the shared installation rate limit or another transient GitHub | ||
| API error up to ``max_attempts`` times, mirroring :func:`gh_graphql`'s | ||
| existing retry convention; any other failure raises immediately exactly | ||
| as before. | ||
| """ | ||
| max_attempts = 4 | ||
| for attempt in range(1, max_attempts + 1): # pragma: no branch - last failed attempt always raises | ||
| try: | ||
| return json.loads(run_github_read(["gh", "api", path])) | ||
| except (RuntimeError, json.JSONDecodeError) as exc: | ||
| rate_limited = is_rate_limited_error(exc) | ||
| if attempt >= max_attempts or not (rate_limited or is_transient_github_api_error(exc)): | ||
| raise | ||
| if rate_limited: | ||
| delay = rate_limit_retry_delay_seconds("core", attempt) | ||
| print( | ||
| f"Rate-limited GitHub REST error on attempt {attempt}/{max_attempts} for {path}; retrying in {delay}s", | ||
| file=sys.stderr, | ||
| ) | ||
| else: | ||
| delay = min(2 ** (attempt - 1), 8) | ||
| print( | ||
| f"Transient GitHub REST error on attempt {attempt}/{max_attempts} for {path}; retrying in {delay}s", | ||
| file=sys.stderr, | ||
| ) | ||
| time.sleep(delay) | ||
|
seonghobae marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def rest_review_node(review: dict[str, Any]) -> dict[str, Any]: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.