Skip to content

fix(compliance): cache mothership config per sweep, add rate-limit backoff - #66

Merged
aabusair merged 2 commits into
mainfrom
fix/sweeper-rate-limit-and-config-caching
Aug 26, 2026
Merged

fix(compliance): cache mothership config per sweep, add rate-limit backoff#66
aabusair merged 2 commits into
mainfrom
fix/sweeper-rate-limit-and-config-caching

Conversation

@aabusair

Copy link
Copy Markdown
Contributor

Summary

Pre-rollout fixes ahead of scaling CLA/DCO enforcement from the current 3 gated repos to ~130+ additional public repos. Both are operational/scale fixes only — the CLA-vs-DCO decision logic itself is unchanged and already proven correct across all code paths (member bypass, commit sign-off, comment+sweeper) on the 3 existing test repos.

  • Rate-limit backoff (policy_selector.py's github_api()): previously had zero rate-limit awareness, unlike the batch reporting pipeline's detect_org_repo_licenses.py. A 403/429 just silently returned None, which could misclassify a repo's policy, falsely mark an already-signed contributor as non-compliant, or leave a required status check stuck pending forever. Now checks X-RateLimit-Remaining/-Reset, sleeps preemptively when low, and retries once (only) on an actual rate-limit 403 — distinguished from a permissions 403 (e.g. the App not being installed on a repo), which now fails loudly instead of looking identical to a rate limit.
  • Per-sweep config caching: process_single_pr() re-fetched cla/allowlist.yml and both license catalogs (one ~5.9MB, fetched via a full blob download) on every single call, including once per PR inside cla_sweeper.py's sweep loop. Harmless at today's scale; at 130+ repos (open-vm-tools alone has 44 open PRs) this would redundantly re-download that blob hundreds of times per 5-minute sweep cycle. Extracted the fetch into fetch_shared_config(), now called once per sweep and passed through, instead of once per PR. required-compliance.yml's single-PR-per-run usage is unaffected — it already only fetches once.

Test plan

  • 12 unit tests (mocked HTTP) covering: normal responses unaffected, preemptive sleep on low remaining, rate-limited 403 retries exactly once then succeeds, persistent rate-limiting gives up after one retry, a permissions 403 does not retry (previously indistinguishable from a rate limit), 404s still silently return None, fetch_shared_config() shape, and process_single_pr() correctly using vs. re-fetching config based on shared_config.
  • Live end-to-end test: temporarily pointed cla_sweeper.yml's checkout at this branch, ran the real Compliance Sweeper via workflow_dispatch (run 33015966605), confirmed the allowlist/license-catalog fetch happened exactly once for the whole run (not once per PR), zero errors, and compared directly against the prior scheduled run on main (33013958831) — identical outcome: same PR (#802), same policy decision (CLA), same commit painted success.
  • Temporary test-only ref change reverted before this PR — cla_sweeper.yml has zero diff against main other than the two real script changes.

🤖 Generated with Claude Code

Amr AbuSair and others added 2 commits August 26, 2026 16:00
…ckoff

Two pre-rollout fixes ahead of scaling CLA/DCO enforcement from 3 repos
to 130+:

- policy_selector.py's github_api() had no rate-limit awareness, unlike
  the batch reporting pipeline's detect_org_repo_licenses.py. A 403/429
  just silently returned None, which could misclassify a repo's policy,
  falsely mark an already-signed contributor as non-compliant, or leave
  a required status check stuck pending forever. Now checks
  X-RateLimit-Remaining/-Reset, sleeps preemptively when low, and
  retries once (only) on an actual rate-limit 403 — distinguished from a
  permissions 403 (e.g. the App not being installed on a repo), which
  now fails loudly instead of looking identical.

- process_single_pr() re-fetched cla/allowlist.yml and both license
  catalogs (one ~5.9MB) on every single call, including once per PR
  inside cla_sweeper.py's sweep loop. Harmless at today's handful of
  PRs; at 130+ repos (open-vm-tools alone has 43 open PRs) this would
  redundantly re-download that blob hundreds of times per 5-minute
  sweep. Extracted the fetch into fetch_shared_config(), now called
  once per sweep and passed through, instead of once per PR.
  required-compliance.yml's single-PR-per-run usage is unaffected.

TEMPORARY: cla_sweeper.yml's checkout ref points at this branch for a
live test before merge — revert to `main` before merging.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was pointed at this branch to live-test the rate-limit/caching fix via
a real workflow_dispatch run before merge. Restoring ref: main now
that testing is done.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@aabusair
aabusair merged commit 764897d into main Aug 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant