fix: handle Ask GitHub rate limits - #1476
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAuthenticated 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. ChangesGitHub rate-limit flow
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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
📒 Files selected for processing (10)
CHANGELOG.mdpackages/backend/src/api.tspackages/backend/src/errors.test.tspackages/backend/src/errors.tspackages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.test.tsxpackages/web/src/app/(app)/askgh/[owner]/[repo]/components/githubRateLimitExceeded.tsxpackages/web/src/app/(app)/askgh/[owner]/[repo]/page.tsxpackages/web/src/features/workerApi/actions.tspackages/web/src/lib/errorCodes.tspackages/web/src/lib/serviceError.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SOU-1546
Fixes SOU-1547
Summary
Test plan
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_TOKENwhen 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_LIMITEDservice 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