fix(scheduler): skip dead REST mergeability refresh for draft PRs - #1670
Conversation
org-queue-sweep investigation (rate-limit incident 2026-09-02): draft PRs never reach a merge/branch-update decision in inspect_pr, so the 2 REST calls enrich_rest_mergeable_states spent per draft PR every sweep tick (fetch_rest_mergeable_state + fetch_compare_branch_freshness) were pure dead work. Skip them for isDraft PRs; non-draft enrichment behavior is unchanged. This does not fix the incident itself: live evidence gathered during the investigation shows GitHub's *secondary* (concurrency) rate limiter is what's firing (primary quota was full and unused when the identical "API rate limit exceeded for user ID 8172694" error reproduced on the very first unrelated REST call this investigation made), and org-queue-sweep's own sequential, hourly-bounded call volume is too small and too bounded in concurrency to plausibly be the primary driver on its own. Native GitHub Actions primitives cannot replace the sweep either: docs/org-required- workflow-rollout.md's own verified required-workflow ruleset trigger support excludes schedule/check_suite/check_run, and `schedule` triggers are single-repository by GitHub Actions design. See docs/doctoring/org-queue-sweep-rate-limit-investigation-20260902.md for the full evidence trail, call-volume quantification, and what was deliberately left unchanged (and why). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 11 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Summary
Investigation of the 2026-09-02 GraphQL secondary rate-limit incident, requested by the repo owner: is
org-queue-sweepsomething GitHub Actions' own native primitives could replace, or does it need an improvement plan because it's causing rate-limit pressure?Full evidence trail:
docs/doctoring/org-queue-sweep-rate-limit-investigation-20260902.md..github/workflows/pr-review-merge-scheduler.yml:568-1265): an hourly (0 * * * *, not the*/30cron) organization-wide reconciliation sweep that catches PRs which became mergeable after their last GitHub Actions triggering event, with no later event to re-wake the per-repo scheduler. Confirmed by reading the code, not assumed.API rate limit exceeded for user ID 8172694error on its first try, with/rate_limitimmediately after showing full, unused primary quota (5000/5000 REST and GraphQL) — the signature of GitHub's secondary (concurrency) limiter, not exhausted call volume.org-queue-sweep's own footprint is sequential across repos, bounded to 10-way concurrency within one repo, and on the order of ~100 REST + ~15-25 GraphQL calls per hourly run — nowhere near enough on its own to trigger that.ps auxduring this investigation showed multiple other concurrent Claude sessions running on the same host, corroborating the "many parallel autonomous agent sessions sharing the org's GitHub identity" hypothesis as the more likely driver.docs/org-required-workflow-rollout.md:25records this org's own verified finding that the required-workflow ruleset (the exact fan-out mechanism Strix/OpenCode/Noema/this scheduler already use) supports onlypull_request/pull_request_target/push/workflow_run— notschedule,check_suite, orcheck_run. Separately, GitHub Actions'scheduletrigger only fires in the repo that owns the workflow file, so there's no way to fan a cron tick out org-wide without per-repo file copies (which the whole central-required-workflow architecture exists to avoid). Elimination is not safe/possible; the improvement-plan path applies.enrich_rest_mergeable_statesnow skips the 2 REST calls per PR (fetch_rest_mergeable_state+fetch_compare_branch_freshness) for draft PRs. Traced every reader ofrestMergeableState/compareStatus/compareBehindBy— all three live strictly afterinspect_pr's early return for draft PRs, so this evidence was refreshed but never consulted for drafts. Pure dead-call elimination, no behavior change for non-draft PRs.docs/doctoring/actions-queue-saturation-hourly-sweep.md); the no-open-PR repo skip (already implemented); GraphQL PR-list batching (already one query per ≤25 PRs, not N+1); the REST mergeability refresh itself for non-draft PRs (deliberate, tested correctness fix from commit5c6f0694protecting against GraphQL merge-state staleness — not touched, per the task's own instruction not to weaken correctness-critical code without certainty);resolveReviewThread(the exact mutation named in the incident) — architecturally GraphQL-only, no REST equivalent exists.Test plan
PYTHONPATH=. python3 -m coverage run -m pytest tests— 2602 passed, 1 skippedpython3 -m coverage report --show-missing—scripts/ci/pr_review_merge_scheduler.py100% line/100% branch;TOTAL100%/100%python3 -m interrogate—RESULT: PASSED (minimum: 100.0%, actual: 100.0%)max_workersexcludes drafts) — pre-existing tests for the same function pass unmodified since none setisDrafton fixturesorg-queue-sweep/pr-review-merge-schedulercontract test needed updating (change is internal to the Python script's REST-enrichment step)🤖 Generated with Claude Code