Add PR-closed-assignee remove logic to be used by workflows #103
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughOn PR close (not merged), linked issues are labeled Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PR as Pull Request
participant GH as GitHub API (Octokit)
participant Issues as Issues
participant Scheduler as Scheduler
User->>PR: Close PR (not merged)
PR->>GH: pull_request_target (closed)
GH->>GH: extractLinkedIssuesFromPRBody()
GH->>GH: findLinkedIssuesFromTimeline()
GH->>Issues: Add `pending-unassign` label to linked issues
GH->>Issues: Post warning comment (starts 12-hour grace)
Note over Issues: 12-hour grace period
Scheduler->>GH: schedule / workflow_dispatch
GH->>Issues: List issues with `pending-unassign`
GH->>GH: Check warning timestamp and active PRs
alt No active PR and grace expired
GH->>Issues: Unassign user
GH->>Issues: Remove label & warning comment
GH->>User: Post unassignment notification
else Active PR exists or grace not expired
GH->>Issues: Keep label & assignments
end
alt PR reopened during grace
User->>PR: Reopen PR
PR->>GH: pull_request_target (reopened)
GH->>Issues: Remove `pending-unassign` label
GH->>Issues: Delete warning comment
GH->>User: Post reopen notification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/index.js`:
- Around line 390-395: The code calls an undefined function hasOpenPR to get {
hasOpen, prNumber, error }; replace that call by using
getLinkedPRsWithDetails(octokit, owner, repoName, issue.number) (or the existing
helper with that signature) and derive hasOpen = linkedPRs.length > 0, prNumber
= linkedPRs[0].number (or choose the first/most-relevant PR), and error = any
error returned by getLinkedPRsWithDetails; update the surrounding logic to use
these computed values (keep the existing console log and continue behavior when
error is truthy). Ensure you reference getLinkedPRsWithDetails and the variables
hasOpen, prNumber, error in the change.
- Around line 150-165: The catch in ensureClosedPRLabel silences all errors from
octokit.rest.issues.getLabel; update the catch to distinguish 404 from other
errors: if e.status === 404 continue to create the label as now, otherwise log
the error (including e.message/stack and context like
owner/repo/CLOSED_PR_LABEL) and rethrow the error so failures (auth, rate-limit,
network) surface immediately; keep the existing createLabel flow and console.log
for the 404 path.
- Around line 427-434: The code currently builds assignees from
issue.assignees.map(...) and calls octokit.rest.issues.removeAssignees(...)
which removes every assignee; change this to only unassign the PR author
referenced in the warning by checking pr.user.login and, if present in
issue.assignees, call octokit.rest.issues.removeAssignees with an array
containing only pr.user.login (use the existing pr and issue variables and the
octokit.rest.issues.removeAssignees method) so unrelated contributors are not
unassigned.
- Around line 167-172: The extractLinkedIssuesFromPRBody function uses a regex
that misses colon variants and full GitHub issue URLs; update the regex variable
inside this function to also accept an optional colon/whitespace after the
keyword and to capture numeric IDs from full issue URLs (e.g., match
/issues/123), so matches include forms like "Closes: `#123`" and "Fixes
https://github.com/owner/repo/issues/123"; keep the rest of the function logic
(matches, parseInt, de-dup) unchanged so returned IDs remain numeric and unique.
🧹 Nitpick comments (1)
src/index.js (1)
174-194: Paginate timeline events to avoid missing linked issues.
listEventsForTimelineis paginated; limiting to the first 100 events can miss older “connected” events in busy PRs. Consider usingoctokit.paginate.♻️ Proposed refactor
- const timeline = await octokit.rest.issues.listEventsForTimeline({ + const timeline = await octokit.paginate(octokit.rest.issues.listEventsForTimeline, { owner, repo: repoName, issue_number: prNumber, per_page: 100 }); const linked = new Set(); - for (const event of timeline.data) { + for (const event of timeline) { if (event.event === 'connected' && event.issue && !event.issue.pull_request) { linked.add(event.issue.number); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (1)
src/index.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.js (1)
src/mock-test.js (1)
attribution(5-5)
🔇 Additional comments (4)
src/index.js (4)
6-9: LGTM: closed‑PR constants are clear and self‑contained.
196-260: LGTM: closed‑PR flow and safeguards look solid.
262-333: LGTM: reopen/open cleanup path is straightforward.
492-508: LGTM: event routing additions look consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/index.js`:
- Around line 433-479: The comment message incorrectly claims an unassignment
regardless of whether removeAssignees ran; modify the flow in the block around
prAuthorMatch/prAuthor and isAssigned so you track if an assignee was actually
removed (e.g., a boolean like wasUnassigned set true only after
octokit.rest.issues.removeAssignees succeeds) and then adjust the
issues.createComment body accordingly (use a conditional message when
wasUnassigned is false to avoid saying "You've been unassigned"); ensure you
reference and update the existing symbols prAuthorMatch, prAuthor, isAssigned,
removeAssignees (octokit.rest.issues.removeAssignees), warningComment, and the
final createComment call so the notification accurately reflects whether an
unassignment occurred.
♻️ Duplicate comments (1)
src/index.js (1)
170-174: Handle backtick‑wrapped issue references in PR body parsing.The regex won’t match common formats like
Closes#123`` because it expects#or a URL immediately after whitespace/colon. This can cause linked issues to be missed and skip the grace‑period flow.💡 Suggested tweak
- const regex = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)(?:\s*:\s*|\s+)(?:#(\d+)|https?:\/\/github\.com\/[^\/]+\/[^\/]+\/issues\/(\d+))/gi; + const regex = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)(?:\s*:\s*|\s+)(?:`?#(\d+)`?|`?https?:\/\/github\.com\/[^\/]+\/[^\/]+\/issues\/(\d+)`?)/gi;
🧹 Nitpick comments (1)
src/index.js (1)
308-322: Remove all warning comments, not just the first.If a PR is closed/reopened multiple times, multiple warning comments with the marker can accumulate. Deleting only the first leaves stale warnings.
♻️ Suggested refactor
- const warningComment = comments.data.find(c => c.body?.includes(CLOSED_PR_COMMENT_MARKER)); - if (warningComment) { - await octokit.rest.issues.deleteComment({ - owner, - repo: repoName, - comment_id: warningComment.id - }); - } + const warningComments = comments.data.filter(c => c.body?.includes(CLOSED_PR_COMMENT_MARKER)); + for (const c of warningComments) { + await octokit.rest.issues.deleteComment({ + owner, + repo: repoName, + comment_id: c.id + }); + }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/index.js`:
- Around line 6-9: The grace period constant and all user-facing copy currently
state 12 hours but the requirement is 6 hours; change CLOSED_PR_GRACE_PERIOD_MS
from 12 * 60 * 60 * 1000 to 6 * 60 * 60 * 1000 and update any user-visible
strings that mention "12 hours" to "6 hours" (search for usages of
CLOSED_PR_GRACE_PERIOD_MS and the literal "12 hours" including places
referencing CLOSED_PR_LABEL/CLOSED_PR_COMMENT_MARKER to keep messages
consistent). Ensure the numeric constant and all copies (including any comments,
logs, labels, or warning text tied to CLOSED_PR_COMMENT_MARKER) consistently
reflect the 6‑hour policy.
- Around line 357-365: The comments listing uses
octokit.rest.issues.listComments and then .find(), which only fetches one page
and may pick the oldest match; replace these calls (the ones in the closed-PR
handling block where comments is declared and the similar one in handleOpenedPR)
with octokit.paginate(octokit.rest.issues.listComments, { owner, repo: repoName,
issue_number: issue.number, per_page: 100 }) to retrieve all pages, then sort
the returned array by created_at descending and pick the first comment whose
body includes CLOSED_PR_COMMENT_MARKER (so you evaluate the most recent warning
timestamp); update the deletion logic in handleOpenedPR to search the full
paginated list similarly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/index.js (1)
139-143: Error flag is never set ingetLinkedPRsWithDetails, causing silent failures during API errors.
getLinkedPRsWithDetailsinitializesallPRswithout anerrorproperty (line 82) and the catch block logs non-404 errors (lines 139-141) but never setsallPRs.error = true. Callers likeenforceClosedPRGracePeriodchecklinkedPRs.error(line 398) and skip enforcement on error, but since the flag is never set, rate-limit and network failures go undetected and can cause incorrect unassignment.Add
error: falseto theallPRsinitialization and setallPRs.error = truein the catch block for non-404 errors.
🤖 Fix all issues with AI agents
In `@src/index.js`:
- Around line 170-174: The current extractLinkedIssuesFromPRBody function strips
owner/repo context from GitHub URL references and returns bare issue numbers,
causing cross-repo URLs like https://github.com/other/rep/issues/42 to be
applied to the current repo; update extractLinkedIssuesFromPRBody to return
structured items (e.g. {owner, repo, issueNumber}) or at minimum ignore URL
matches whose owner/repo don't match the current repoName/owner before
converting to issue numbers, and update any callers that call
octokit.rest.issues.get({ owner, repo: repoName, issue_number }) to use the
parsed owner/repo when present (or skip non-matching URLs) so only same-repo
URLs or explicitly-matched owner/repo references are processed.
1 participant
Summary
Adds automated workflow to unassign contributors when their pull request is closed (not merged), with a 12-hour grace period to open a new PR.
Problem
When a contributor's PR is closed, they remain assigned to the issue indefinitely. This blocks other contributors from picking up the issue.
Solution
Integrates closed PR management directly into the BLT-Action code:
On PR close (not merged):
Posts a warning comment
Adds
pr-closed-pending-unassignlabelOn new PR open:
Cancels the warning
Removes the label
Every 12 hours (scheduled):
Checks labeled issues
Unassigns contributors if no new PR was opened
Changes
Added
pull_request_targetevent handling for closed/opened/reopened PRsAdded 12-hour grace period enforcement to scheduled checks
New helper functions for PR-issue linking and grace period management
Reuses existing
hasOpenLinkedPRfunction for consistencySummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.