fix(checklint): resolve pr number for fork prs via paginated sha lookup#25
Merged
Conversation
|
@calebephrem is attempting to deploy a commit to the calebephrem's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Summary by BeetleThis PR fixes a critical bug in the lint workflow bot where it failed to resolve PR numbers for fork-based pull requests. Previously, the bot only checked PRs from the same repository branch, causing it to silently fail when workflows ran on commits from forked repositories. The fix implements a paginated SHA-based lookup that searches through all open PRs (up to 1000 PRs across 10 pages) to match the workflow run's commit SHA, ensuring the bot can comment on both same-repo and fork PRs. 📁 File Changes Summary
Total Changes: 1 file changed, +108 additions, -53 deletions 🗺️ Walkthroughgraph TD
A["Workflow Run Completed Event"] --> B{"Is Lint Workflow?"}
B -->|"No"| Z["Exit"]
B -->|"Yes"| C["Resolve PR Number"]
C --> D{"Direct PR Link Exists?"}
D -->|"Yes"| E["Return PR Number"]
D -->|"No"| F["Try Same-Repo Branch Lookup"]
F --> G{"Found PR with Matching SHA?"}
G -->|"Yes"| E
G -->|"No"| H["Paginated Fork PR Lookup"]
H --> I["Iterate Pages 1-10"]
I --> J["Fetch 100 PRs per Page"]
J --> K{"Found PR with Matching SHA?"}
K -->|"Yes"| E
K -->|"No, More Pages"| I
K -->|"No, End of Pages"| L["Return null"]
E --> M{"Workflow Conclusion?"}
M -->|"Success"| N["Post Success Comment"]
M -->|"Failure"| O["Post Failure Comment with Logs"]
L --> P["Log Warning & Skip"]
N1["Note: Searches up to 1000 PRs to handle fork contributions"]
H -.-> N1
🎯 Key Changes
📊 Impact Assessment
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
![beetle-ai[bot]](https://avatars.githubusercontent.com/in/1636950?s=60&v=4){kind=small}
Comment on lines
+130
to
+151
| for (let page = 1; page <= MAX_PAGES; page++) { | ||
| const { data: prs } = await context.octokit.rest.pulls.list({ | ||
| owner, | ||
| repo, | ||
| state: "open", | ||
| per_page: PULLS_PER_PAGE, | ||
| page, | ||
| }); | ||
|
|
||
| if (prs.length === 0) { | ||
| break; | ||
| } | ||
|
|
||
| const forkMatch = prs.find((pr) => pr.head.sha === headSha); | ||
| if (forkMatch) { | ||
| return forkMatch.number; | ||
| } | ||
|
|
||
| if (prs.length < PULLS_PER_PAGE) { | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The paginated fork PR lookup could fetch up to 1000 PRs (10 pages × 100 per page) sequentially, which may cause significant latency and rate limit issues. For repositories with many open PRs, this could take several seconds and consume substantial API quota on every workflow run.
Performance Impact:
- Each API call adds ~200-500ms latency
- Up to 10 sequential API calls = 2-5 seconds worst case
- GitHub API rate limits could be exhausted quickly
- Blocks the workflow completion handler during this time
Recommendation: Consider adding a SHA-based search or limiting the pagination more aggressively (e.g., MAX_PAGES = 3-5) since most active PRs are recent. Also consider caching or using GitHub's search API with SHA filter if available.
Confidence: 5/5
Suggested Fix
Consider reducing MAX_PAGES to a more reasonable value like 3-5 pages (300-500 PRs), which should cover most active repositories while preventing excessive API calls:
Suggested change
| for (let page = 1; page <= MAX_PAGES; page++) { | |
| const { data: prs } = await context.octokit.rest.pulls.list({ | |
| owner, | |
| repo, | |
| state: "open", | |
| per_page: PULLS_PER_PAGE, | |
| page, | |
| }); | |
| if (prs.length === 0) { | |
| break; | |
| } | |
| const forkMatch = prs.find((pr) => pr.head.sha === headSha); | |
| if (forkMatch) { | |
| return forkMatch.number; | |
| } | |
| if (prs.length < PULLS_PER_PAGE) { | |
| break; | |
| } | |
| } | |
| const MAX_PAGES = 5; | |
Additionally, consider adding a comment explaining the trade-off:
// Limit pagination to avoid excessive API calls and latency.
// This covers ~500 most recent open PRs, which should be sufficient
// for most repositories. Very old fork PRs may not be found.
const MAX_PAGES = 5;Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/workflow_run.completed/checklint.ts around line 14, the MAX_PAGES constant is set to 10, which could cause the fork PR lookup to fetch up to 1000 PRs sequentially, leading to significant latency (2-5 seconds) and potential rate limit issues. Reduce MAX_PAGES to a more reasonable value like 5 (covering ~500 PRs) to balance between finding fork PRs and avoiding excessive API calls. Add a comment explaining this trade-off and the limitation that very old fork PRs may not be found.
📍 This suggestion applies to lines 130-151
Contributor
Summary by BeetleThis PR simplifies the fork pull request lookup logic by removing unnecessary pagination complexity. The change replaces a multi-page iteration approach (up to 10 pages, 100 PRs per page) with a single unpaginated API call that fetches up to 30 open PRs. This optimization reduces code complexity and API calls while maintaining functionality for typical use cases where fork PRs are among the most recent open PRs. 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 1 file changed, +9 additions, -26 deletions 🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
Contributor
|
✅ You're good to merge this PR! No issues found. Great job! Settings⚙️ SettingsSeverity Threshold: 📖 User Guide
|
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.
No description provided.