Skip to content

fix: handle Ask GitHub rate limits - #1476

Merged
brendan-kellam merged 5 commits into
mainfrom
brendan/fix-sou-1546-sou-1547
Aug 5, 2026
Merged

fix: handle Ask GitHub rate limits#1476
brendan-kellam merged 5 commits into
mainfrom
brendan/fix-sou-1546-sou-1547

Conversation

@brendan-kellam

@brendan-kellam brendan-kellam commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Fixes SOU-1546
Fixes SOU-1547

Summary

  • authenticate Ask GitHub repository lookups with the configured deployment token
  • return a distinct 429 response for GitHub primary and secondary rate limits
  • show a recoverable Ask GitHub client state with a retry action

Test plan

  • yarn workspace @sourcebot/backend test src/errors.test.ts
  • yarn workspace @sourcebot/backend build
  • yarn workspace @sourcebot/web test src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.test.tsx
  • yarn workspace @sourcebot/web lint
  • yarn workspace @sourcebot/web build

Note

Medium Risk
Changes GitHub API auth and error classification for the experimental Ask GH add-repo path; misclassification could still surface wrong errors, but scope is limited and failures are user-recoverable rather than data or auth core paths.

Overview
Ask GitHub repository lookups now use the configured EXPERIMENT_ASK_GH_GITHUB_TOKEN when calling GitHub’s API, instead of unauthenticated Octokit requests.

When GitHub returns primary or secondary rate limits, the worker API responds with 429 instead of a generic failure. The web layer maps that to a new GITHUB_RATE_LIMITED service error, and the Ask GH repo page shows a recoverable “GitHub is temporarily busy” state with a Try again action (router.refresh()).

Backend adds isGitHubRateLimitError (429, 403 with rate-limit headers/message) with tests; the new UI component has a test for the retry button.

Reviewed by Cursor Bugbot for commit db5339f. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • GitHub repository lookups now use authentication, improving reliability and access to repositories.
    • GitHub API rate limits are detected and reported with a recoverable response instead of a generic error.
    • Added a clear “GitHub is temporarily busy” message with a Try again option to retry without a full page reload.
  • Tests
    • Added coverage for rate-limit detection, error handling, and retry behavior.

@github-actions

This comment has been minimized.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 05466cf2-f42d-4175-a057-594cff4a0150

📥 Commits

Reviewing files that changed from the base of the PR and between 0526125 and db5339f.

📒 Files selected for processing (1)
  • CHANGELOG.md

Walkthrough

Authenticated GitHub repository lookups now use a token, classify rate-limit failures, return HTTP 429, propagate a dedicated error code, and render a retryable busy state in the Ask GitHub page.

Changes

GitHub rate-limit flow

Layer / File(s) Summary
Backend detection and API response
packages/backend/src/api.ts, packages/backend/src/errors.ts, packages/backend/src/errors.test.ts, CHANGELOG.md
Authenticated Octokit requests detect primary and secondary GitHub rate limits, return HTTP 429, and document the fix. Tests cover recognized and unrelated errors.
Web error propagation
packages/web/src/lib/errorCodes.ts, packages/web/src/lib/serviceError.ts, packages/web/src/features/workerApi/actions.ts
HTTP 429 responses map to GITHUB_RATE_LIMITED through a dedicated service-error factory.
Rate-limit page and retry UI
packages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.test.tsx
The Ask GitHub page renders a busy message with a “Try again” button that refreshes the route. Component behavior is tested.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AskGithubPage
  participant addGithubRepo
  participant experimental_addGithubRepo
  participant GitHub
  AskGithubPage->>addGithubRepo: add repository
  addGithubRepo->>experimental_addGithubRepo: repository lookup request
  experimental_addGithubRepo->>GitHub: authenticated API request
  GitHub-->>experimental_addGithubRepo: rate-limit failure
  experimental_addGithubRepo-->>addGithubRepo: HTTP 429
  addGithubRepo-->>AskGithubPage: GITHUB_RATE_LIMITED
  AskGithubPage->>AskGithubPage: render busy state
  AskGithubPage->>AskGithubPage: refresh on retry
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the pull request's main change: handling Ask GitHub rate limits.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch brendan/fix-sou-1546-sou-1547

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/backend/src/api.ts`:
- Around line 159-161: Protect the /api/experimental/add-github-repo endpoint
with mandatory authentication before constructing the Octokit client or
performing any GitHub lookup. Replace the optional-auth behavior with the
backend’s required-auth guard and reject unauthenticated requests; preserve the
existing authenticated repository persistence and indexing flow.

In `@packages/backend/src/errors.ts`:
- Around line 43-48: The rate-limit classifier must inspect wrapped errors
consistently with getStatus. Update the logic around the responseHeaders and
message extraction to read metadata from cause.response and the wrapped error
message as well as the top-level fields, preserving existing top-level handling;
add a regression test covering a wrapped GitHub 403 with rate-limit metadata and
verifying it is classified as HTTP 429.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5201e88a-275b-4c9e-9367-1697a360b466

📥 Commits

Reviewing files that changed from the base of the PR and between e1ce46b and 6dd4ad0.

📒 Files selected for processing (10)
  • CHANGELOG.md
  • packages/backend/src/api.ts
  • packages/backend/src/errors.test.ts
  • packages/backend/src/errors.ts
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.test.tsx
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.tsx
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsx
  • packages/web/src/features/workerApi/actions.ts
  • packages/web/src/lib/errorCodes.ts
  • packages/web/src/lib/serviceError.ts

Comment thread packages/backend/src/api.ts
Comment thread packages/backend/src/errors.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit afd884c. Configure here.

Comment thread packages/backend/src/errors.ts
@brendan-kellam
brendan-kellam merged commit 895040a into main Aug 5, 2026
11 of 12 checks passed
@brendan-kellam
brendan-kellam deleted the brendan/fix-sou-1546-sou-1547 branch August 5, 2026 00:26
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