fix: resolve pr number via branch fallback when pull_requests is empty#24
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. |
Summary by BeetleThis PR removes the visual glow effect from the header text by eliminating the 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 1 file changed, +0 additions, -3 deletions 🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
|
✅ You're good to merge this PR! No issues found. Great job! Settings⚙️ SettingsSeverity Threshold: 📖 User Guide
|
Summary by BeetleThis PR fixes a critical issue in the workflow run completion handler for linting checks. When the 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 1 file changed, +36 additions, -13 deletions 🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
calebephrem
changed the title
![beetle-ai[bot]](https://avatars.githubusercontent.com/in/1636950?s=60&v=4){kind=small}
|
Important Review skipped Bot user detected. To trigger a single review, invoke the ⚙️ SettingsSeverity Threshold: 📖 User Guide
|
Co-authored-by: beetle-ai[bot] <221859081+beetle-ai[bot]@users.noreply.github.com>
Summary by BeetleThis PR addresses two distinct issues: a visual styling fix and a critical bug fix for PR number resolution in workflow automation. The main focus is on improving the reliability of the 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 2 files changed, +37 additions, -17 deletions 🗺️ Walkthrough:sequenceDiagram
participant WF as Workflow Run Event
participant Handler as checklint Handler
participant Resolver as resolvePullNumber
participant GitHub as GitHub API
participant PR as Pull Request
WF->>Handler: workflow_run.completed event
Handler->>Resolver: Request PR number resolution
alt Direct PR reference exists
Resolver->>Resolver: Check pull_requests array
Resolver-->>Handler: Return PR number
else Fallback to branch lookup
Resolver->>Resolver: Extract head_branch & head_sha
Resolver->>GitHub: List open PRs for branch
GitHub-->>Resolver: Return matching PRs
Resolver->>Resolver: Find PR by SHA match
Resolver-->>Handler: Return PR number or null
end
alt PR number resolved
Handler->>Handler: Check workflow conclusion
alt Success
Handler->>PR: Post success comment
else Failure
Handler->>PR: Post failure comment with logs
end
else No PR found
Handler->>Handler: Exit silently
end
🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
| const { data: prs } = await context.octokit.rest.pulls.list({ | ||
| owner, | ||
| repo, | ||
| state: "open", | ||
| head: `${owner}:${headBranch}`, | ||
| }); |
There was a problem hiding this comment.
The context.octokit.rest.pulls.list() API call lacks error handling. If this call fails (network issues, rate limits, permission errors), the entire workflow handler will crash and no comment will be posted to the PR. This is a critical path that should handle failures gracefully.
Confidence: 5/5
Suggested Fix
| const { data: prs } = await context.octokit.rest.pulls.list({ | |
| owner, | |
| repo, | |
| state: "open", | |
| head: `${owner}:${headBranch}`, | |
| }); | |
| try { | |
| const { data: prs } = await context.octokit.rest.pulls.list({ | |
| owner, | |
| repo, | |
| state: "open", | |
| head: `${owner}:${headBranch}`, | |
| }); | |
| } catch (error) { | |
| context.log.error("Failed to fetch pull requests", error); | |
| return null; | |
| } | |
Wrap the API call in a try-catch block to handle potential failures gracefully. This ensures the workflow handler doesn't crash and can return null to skip processing when the API is unavailable.
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 93, the context.octokit.rest.pulls.list() API call lacks error handling and can cause the entire workflow handler to crash if the GitHub API fails; wrap the API call (lines 93-98) in a try-catch block that logs the error and returns null on failure, ensuring graceful degradation when the API is unavailable.
📍 This suggestion applies to lines 93-98
| head: `${owner}:${headBranch}`, | ||
| }); | ||
|
|
||
| const match = prs.find((pr) => pr.head.sha === headSha); |
There was a problem hiding this comment.
The fallback logic ?? prs[0] is dangerous. If no PR matches the head_sha, it falls back to the first open PR with the same branch name, which could be a completely different PR. This could result in posting workflow comments to the wrong PR, confusing developers and potentially exposing information about one PR in another.
Confidence: 5/5
Suggested Fix
| const match = prs.find((pr) => pr.head.sha === headSha); | |
| const match = prs.find((pr) => pr.head.sha === headSha); | |
| return match?.number ?? null; | |
Remove the ?? prs[0] fallback. If no PR matches the exact SHA, return null to skip processing rather than risk commenting on the wrong PR. This ensures comments are only posted to the correct PR.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/workflow_run.completed/checklint.ts at line 100, the fallback logic `?? prs[0]` can cause comments to be posted to the wrong PR if the SHA doesn't match; remove the `?? prs[0]` fallback and return null when no matching PR is found, ensuring comments are only posted to the correct PR with the matching SHA.
There was a problem hiding this comment.
@calebephrem I encountered an error while generating a response. Please try again.
1 participant
No description provided.