[feature] Automate assigning/unassigning issues #571#572
[feature] Automate assigning/unassigning issues #571#572Eeshu-Yadav wants to merge 8 commits intoopenwisp:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a GitHub automation suite under .github/actions/bot-autoassign: three bots (IssueAssignmentBot, StalePRBot, PRReopenBot) built on a shared GitHubBot base, utilities for extracting/validating linked issues, a dispatcher entrypoint, comprehensive unit tests, packaging update (PyGithub in extras), and four GitHub Actions workflows to run bots on issue comments, pull_request_target events, scheduled cron, and manual dispatch. Workflows generate App tokens, set up Python, install deps, and invoke the dispatcher with event payloads or tokens. No existing public API surface was modified. Sequence Diagram(s)sequenceDiagram
actor Contributor as Contributor
participant GitHub as GitHub
participant Bot as IssueAssignmentBot
participant API as GitHub API
Contributor->>GitHub: Posts "please assign" comment
GitHub->>Bot: issue_comment event payload
Bot->>Bot: Detect assignment request
Bot->>API: Fetch issue details/labels
API-->>Bot: Issue data
Bot->>API: Post guidance comment
API-->>GitHub: Comment created
sequenceDiagram
participant Contributor as Contributor
participant GitHub as GitHub
participant Bot as IssueAssignmentBot
participant Utils as utils.extract_linked_issues
participant API as GitHub API
Contributor->>GitHub: Opens PR with linked issues
GitHub->>Bot: pull_request_target event payload
Bot->>Utils: Parse linked issues from PR body
loop for each linked issue
Bot->>API: Fetch issue via repo.get_issue
API-->>Bot: Issue data
alt valid & not already assigned
Bot->>API: Add assignee
Bot->>API: Post assignment comment
API-->>GitHub: Issue updated
end
end
sequenceDiagram
participant Scheduler as Cron
participant Bot as StalePRBot
participant API as GitHub API
participant PR as Pull Request
Scheduler->>Bot: Scheduled run
Bot->>API: List open PRs
API-->>Bot: PR list
loop For each PR
Bot->>API: Fetch commits, reviews, comments
API-->>Bot: Activity data
Bot->>Bot: Compute inactivity days
alt > 60 days
Bot->>API: Comment & close PR
Bot->>API: Unassign linked issues
else > 14 days
Bot->>API: Mark PR stale & unassign issues
else > 7 days
Bot->>API: Post inactivity warning
end
end
sequenceDiagram
participant GitHub as GitHub
participant Bot as PRReopenBot
participant Utils as utils.extract_linked_issues
participant API as GitHub API
GitHub->>Bot: PR reopened or contributor activity
Bot->>Utils: Parse linked issues from PR body
loop For each linked issue
Bot->>API: Fetch issue
API-->>Bot: Issue data
alt reassignment needed
Bot->>API: Assign issue to PR author
Bot->>API: Post welcome comment
end
end
Bot->>API: Remove stale label from PR if present
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Pull request overview
This PR introduces GitHub Actions workflows to automate issue assignment/unassignment based on PR linking and contributor activity, and to manage stale PRs after changes are requested.
Changes:
- Adds an “assign me” responder that directs contributors to the guidelines and encourages opening a linked PR.
- Auto-assigns linked issues to PR authors when PRs are opened/reopened.
- Introduces stale PR management that warns, marks stale/unassigns, and closes PRs after defined inactivity windows.
- Reassigns linked issues and removes stale status when PRs are reopened or when the PR author comments on a stale PR.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| .github/workflows/issue-assignment.yml | Responds to “assign me” issue comments with contribution guidance and PR-linking instructions. |
| .github/workflows/pr-issue-link.yml | Auto-assigns linked issues to PR authors on PR open/reopen. |
| .github/workflows/stale-pr-management.yml | Daily job to warn, mark stale/unassign, and close PRs after inactivity post “changes requested”. |
| .github/workflows/pr-reopen-reassign.yml | Reassigns issues / removes stale label on PR reopen and on PR-author follow-up comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| issue_comment: | ||
| types: [created] | ||
| push: | ||
|
|
There was a problem hiding this comment.
on: push is configured, but both jobs are gated to pull_request_target or issue_comment, so push events will trigger workflow runs that immediately skip all jobs. Removing the push trigger avoids noisy/empty runs and reduces Actions usage.
| @@ -0,0 +1,77 @@ | |||
| name: PR Issue Auto-Assignment | |||
There was a problem hiding this comment.
PR description ends with “Fixes #541”, but #541 appears to be an unrelated (already-merged) UI PR in this repository’s history. If the intent is to close an issue for this automation feature, the reference likely needs to point to the correct issue number (or be removed to avoid auto-closing the wrong item).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/issue-assignment.yml:
- Line 28: The contributingUrl constant is using an insecure HTTP URL; update
the value of contributingUrl to use the HTTPS scheme (i.e., replace
'http://openwisp.io/docs/developer/contributing.html' with
'https://openwisp.io/docs/developer/contributing.html') so the link is served
over TLS and avoids mixed-content/MITM risks.
In @.github/workflows/stale-pr-management.yml:
- Around line 26-59: The list calls (pulls.list, pulls.listReviews,
pulls.listCommits, issues.listComments) currently use per_page: 100 and will
miss results beyond the first page; replace each call with github.paginate(...)
using the corresponding endpoint.merge({...}) parameters (keep owner, repo,
pull_number/issue_number and state where applicable) to populate pullRequests,
reviews, commits, and comments with the full arrays; remove reliance on
single-page responses and ensure downstream code (e.g., filtering
changesRequestedReviews and accessing commits/comments) works with the full
arrays returned by paginate.
🧹 Nitpick comments (1)
.github/workflows/pr-reopen-reassign.yml (1)
3-9: Drop the unusedpushtrigger to avoid empty workflow runs.No job is enabled for
push, so these runs will always be skipped.🧹 Minimal cleanup
on: pull_request_target: types: [reopened] issue_comment: types: [created] - push:
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/issue-assignment.yml.github/workflows/pr-issue-link.yml.github/workflows/pr-reopen-reassign.yml.github/workflows/stale-pr-management.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Agent
🔇 Additional comments (1)
.github/workflows/pr-issue-link.yml (1)
20-75: Solid auto-assignment flow — looks safe and well-guarded.Nice dedupe + PR/assignee checks with clear logging.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
2 times, most recently
from
a34efd6 to
a116660
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/issue-assignment.yml:
- Around line 13-57: The job 'if' condition uses unsupported toLower() calls
around github.event.comment.body; update the conditional to remove the toLower()
wrappers and call contains() directly on github.event.comment.body (e.g.,
replace contains(toLower(github.event.comment.body), 'assign me') with
contains(github.event.comment.body, 'assign me')) so the expressions use the
built-in case-insensitive contains() correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/issue-assignment.yml.github/workflows/pr-issue-link.yml.github/workflows/pr-reopen-reassign.yml.github/workflows/stale-pr-management.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-reopen-reassign.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/issue-assignment.yml
[error] 15-15: undefined function "toLower". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
[error] 16-16: undefined function "toLower". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
[error] 17-17: undefined function "toLower". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
[error] 18-18: undefined function "toLower". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
[error] 19-19: undefined function "toLower". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
🔇 Additional comments (7)
.github/workflows/pr-issue-link.yml (2)
3-10: Workflow trigger and permissions look good.Clear event scope and minimal permissions for the task.
20-85: Solid issue-link parsing and assignment flow.Dedupe + max cap reduce rate‑limit risk; skip logic for PRs/assigned issues is sensible.
.github/workflows/stale-pr-management.yml (5)
3-10: Schedule and permissions are appropriate.Daily cadence and write permissions align with automation responsibilities.
81-94: Bot-comment detection for stale actions is clear and consistent.The
botComments+ flags avoid duplicate warnings.
95-155: Closure flow is clear and user-friendly.Closes after the threshold and unassigns linked issues as expected.
156-219: Unassign + stale labeling branch looks solid.The messaging and unassign behavior align with the workflow intent.
220-244: Stale warning messaging is concise and actionable.Good guidance for contributors before escalation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
@Eeshu-Yadav look at the work we're doing here:
#562
The same recommendations I've been giving there apply here, we should make something consistent:
- reusable actions that we can enable in all repos
- automated tests to give us confidence when maintaining/changing/iterating
- documentation for usage
- enable this in a test repo where we can test it and link to it in the PR description
- use Python
github-project-automation
bot
moved this to In progress
in OpenWISP Priorities for next releases
nemesifier
changed the title
okk |
|
@nemesifier https://github.com/issue-automation-org/issue-bot
stale one will be updated and after doing that , here updated code will be pushed based on the tested ymls and also python code for the bot will be updated |
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
Good to see that you're testing this, I'd like to see the testing done after the python port, otherwise we'll need to test the same things twice which is time consuming.
See my comment below.
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
a116660 to
30dfeb4
Compare
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
30dfeb4 to
3ccd173
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py`:
- Line 110: The failing test assumes detect_issue_type returns "bug" but the
mock_issue used in issue_assignment_bot_tests.py has no bug/feature keywords, so
detect_issue_type returns None and the response won't contain the literal "Fixes
`#123`"; fix this by updating the test setup to give mock_issue a title or body
containing a bug keyword (e.g., include "bug") so detect_issue_type(...) yields
"bug" and the generated call_args will include "Fixes `#123`", or alternatively
change the assertion to match the generic linking output (e.g., assert both
"Fixes `#123`" and "Closes `#123`" are handled or assert the generic format) —
locate references to mock_issue and the detect_issue_type call in the test to
apply the change.
- Around line 99-110: The failing tests are caused by mock_issue.labels not
being iterable when detect_issue_type(issue) enumerates issue.labels; update the
test setup in test_respond_to_assignment_request_success (and similarly in
test_handle_issue_comment_assignment_request) to set mock_issue.labels to an
iterable (e.g., an empty list or list of Label-like mocks) so detect_issue_type
can iterate over it; locate the mock object named mock_issue in these tests and
assign mock_issue.labels = [] (or a list of mocks) before calling
IssueAssignmentBot.respond_to_assignment_request / the handler.
- Around line 6-12: Remove the unused imports PRReopenBot and PRActivityBot from
the import block so this test module only imports IssueAssignmentBot;
specifically, edit the try/except import section where IssueAssignmentBot,
PRReopenBot, and PRActivityBot are imported and delete PRReopenBot and
PRActivityBot (and their corresponding None assignments in the except block)
leaving only IssueAssignmentBot to avoid unused symbols in
issue_assignment_bot_tests.py.
In `@openwisp_utils/bots/auto_assign/issue_assignment_bot.py`:
- Around line 116-126: The current detect_issue_type function uses substring
checks against combined_text with bug_keywords and feature_keywords (variables
referenced) which causes false positives; update detect_issue_type to perform
whole-word, case-insensitive matching instead (e.g., compile each keyword into a
word-boundary regex like r"\bkeyword\b" or tokenize combined_text and compare
tokens) and use re.search or token membership for scoring so words like "tissue"
or "renew" do not match "issue"/"new"; preserve existing variables
(bug_keywords, feature_keywords, combined_text) and keep the scoring logic same
but based on whole-word matches.
In `@openwisp_utils/bots/auto_assign/pr_reopen_bot.py`:
- Around line 37-45: Duplicate extract_linked_issues logic exists in multiple
bots; extract to a shared module-level function and import it. Create a new
utility function (e.g., extract_linked_issues(pr_body)) in a shared utils module
(utils.py) with the regex logic and move the import re to the top of that
module; then replace PRReopenBot.extract_linked_issues,
PRActivityBot.extract_linked_issues, StalePRBot.extract_linked_issues, and
IssueAssignmentBot.extract_linked_issues to call the shared
utils.extract_linked_issues; ensure signatures remain identical and remove the
in-method import re occurrences.
- Around line 248-252: The current code always posts encouragement_message via
pr.create_issue_comment even when no stale label was removed or no issues were
reassigned; update the flow to check the actual outcomes of the operations
(e.g., the result/boolean returned by remove_stale_label/remove_label_from_pr
and the count/list returned by reassign_linked_issues or similar functions) and
only compose/post pr.create_issue_comment with the “stale status removed and
issues reassigned” text when stale_label_removed is true and
reassigned_issues_count > 0; otherwise craft and post a different, accurate
message (or skip posting) reflecting the real result. Ensure you reference the
existing encouragement_message variable and only call pr.create_issue_comment
once with the appropriate conditional message.
- Around line 285-290: The branch selecting PRActivityBot vs PRReopenBot uses
sys.argv[0] which is never the user mode, so PRActivityBot is unreachable;
change the selection to read an explicit mode argument (e.g., sys.argv[1]) or
use argparse/cli parsing in the module that contains the current logic (the
block that instantiates PRActivityBot and PRReopenBot), validate that an
argument like "activity" is provided, and default to PRReopenBot otherwise;
update the startup/entrypoint logic where main()/if __name__ == "__main__"
triggers the bot so it expects and documents the mode argument or register
separate entry points per bot.
In `@openwisp_utils/bots/auto_assign/pr_reopen_tests.py`:
- Around line 14-209: Remove the duplicated TestStalePRBot class from
pr_reopen_tests.py (the one defining TestStalePRBot, setUp/tearDown, and all
tests) so only the copy in stale_pr_tests.py remains; also remove any related
imports or fixtures in pr_reopen_tests.py that are only used by that class, and
delete the incorrect patch targeting
"openwisp_utils.bots.auto_assign.pr_reopen_bot.Github" (used in setUp) so tests
no longer attempt to mock the wrong symbol — this leaves StalePRBot tests only
in stale_pr_tests.py where the correct import and patch targeting
(stale_pr_bot.Github) is used.
- Around line 322-328: The test payload in test_handle_contributor_activity is
missing the "pull_request" key inside the "issue" object so
handle_contributor_activity short-circuits; update the event_payload in the test
to include an appropriate "pull_request" entry (e.g., an empty object or minimal
fields) inside issue so PRActivityBot.handle_contributor_activity sees a PR and
returns True as the test expects.
- Around line 350-358: The test function
test_handle_contributor_activity_not_author constructs an event_payload missing
the "pull_request" key so the PR-detection guard returns False; to exercise the
author-check logic, add a "pull_request" key (e.g.,
event_payload["issue"]["pull_request"] = {}) to the payload in that test (or
include it inline in the dict) so PRActivityBot.handle_contributor_activity
reaches the author check path that verifies the commenter is not the author.
In `@openwisp_utils/bots/auto_assign/stale_pr_bot.py`:
- Around line 93-107: The current has_bot_comment function uses fragile
substring matching (comment_type.lower() in comment.body.lower()) causing false
matches; change it to look for a dedicated HTML comment marker token instead
(e.g., "<!-- bot:stale_warning -->" or "<!-- bot:stale_mark -->") so checks are
unambiguous: update has_bot_comment to search for the exact marker string in
comment.body and then update the places that post comments (e.g., functions that
post the warning and mark comments such as warn_pr_stale and mark_pr_stale) to
include the corresponding marker in the posted body so the bot can reliably
detect which comment type exists.
- Around line 305-317: The substring check in has_bot_comment(pr, "stale")
incorrectly matches the earlier "stale warning" comment and blocks escalation to
mark_pr_stale; update the conditional logic so the checks are unambiguous —
e.g., change calls to has_bot_comment to look for exact comment types/keys
(distinct tokens like "stale" vs "stale warning" or a dedicated tag argument) or
add a stricter exact-match flag, then use those exact matches in the branches
around DAYS_BEFORE_UNASSIGN and DAYS_BEFORE_STALE_WARNING when deciding to call
mark_pr_stale or send_stale_warning and increment processed_count.
In `@openwisp_utils/bots/auto_assign/stale_pr_tests.py`:
- Around line 1-362: This file duplicates three test classes; keep only
TestStalePRBot here and remove the duplicated TestPRReopenBot and
TestPRActivityBot definitions from this file; then ensure the other test file
(pr_reopen_tests.py) contains TestPRReopenBot and TestPRActivityBot (if missing,
move their code there) so each test class exists in a single place, update
imports/patch targets if needed to reflect the single-location change, and run
tests to confirm no duplicated runs remain.
🧹 Nitpick comments (6)
setup.py (1)
66-68: Consider adding an upper-bound constraint on PyGithub.Other extras in this file use
~=or explicit upper bounds to prevent unexpected major-version breakage. An open-ended>=2.0.0could pull in a future PyGithub 3.x with breaking API changes.Suggested fix
"bots": [ - "PyGithub>=2.0.0", + "PyGithub>=2.0.0,<3.0.0", ],openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
8-31: Duplicated__init__boilerplate across all bot classes — consider a shared base class.
PRReopenBot.__init__,PRActivityBot.__init__,StalePRBot.__init__, andIssueAssignmentBot.__init__all contain nearly identical GitHub client initialization logic. ABaseBotMixinorBaseBotclass would eliminate ~25 lines of duplication per class and centralize configuration.Also applies to: 159-176
openwisp_utils/bots/auto_assign/stale_pr_bot.py (2)
44-54: Performance: fetches all branch commits, not only those after changes were requested.
self.repo.get_commits(sha=pr.head.sha)returns every commit reachable from the head, then materializes the full list. For long-lived branches this can be hundreds or thousands of commits and API pages. Consider using thesinceparameter to limit to commits afterlast_changes_requested:Proposed fix
- commits = list(self.repo.get_commits(sha=pr.head.sha)) + commits = list( + self.repo.get_commits(sha=pr.head.sha, since=last_changes_requested) + )
277-284: Loading all open PRs into a list upfront can be expensive.
list(self.repo.get_pulls(state="open"))eagerly fetches every page. PyGithub'sget_pullsreturns aPaginatedListthat supports lazy iteration. Iterating directly avoids holding all PR objects in memory.Proposed fix
- open_prs = list(self.repo.get_pulls(state="open")) - print(f"Found {len(open_prs)} open pull requests") + open_prs = self.repo.get_pulls(state="open") + print(f"Processing open pull requests (count: {open_prs.totalCount})")openwisp_utils/bots/auto_assign/__main__.py (1)
4-36: LGTM on the dispatcher structure.The bot-type dispatch with lazy imports and
sys.argvnormalization is clean. Minor nit: Ruff suggests[sys.argv[0], *sys.argv[2:]]over concatenation on lines 17 and 26.Nit: more idiomatic list construction
- sys.argv = [sys.argv[0]] + sys.argv[2:] + sys.argv = [sys.argv[0], *sys.argv[2:]]openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py (1)
12-13: Missing blank line before class definition.PEP 8 requires two blank lines before a top-level class definition. There's no blank line between the
exceptblock and theclassstatement.PRActivityBot = None + + class TestIssueAssignmentBot(TestCase):
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/issue-assignment.yml.github/workflows/pr-issue-link.yml.github/workflows/pr-reopen-reassign.yml.github/workflows/stale-pr-management.ymlopenwisp_utils/bots/__init__.pyopenwisp_utils/bots/auto_assign/__init__.pyopenwisp_utils/bots/auto_assign/__main__.pyopenwisp_utils/bots/auto_assign/issue_assignment_bot.pyopenwisp_utils/bots/auto_assign/issue_assignment_bot_tests.pyopenwisp_utils/bots/auto_assign/pr_reopen_bot.pyopenwisp_utils/bots/auto_assign/pr_reopen_tests.pyopenwisp_utils/bots/auto_assign/stale_pr_bot.pyopenwisp_utils/bots/auto_assign/stale_pr_tests.pysetup.py
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/issue-assignment.yml
- .github/workflows/pr-reopen-reassign.yml
- .github/workflows/stale-pr-management.yml
- .github/workflows/pr-issue-link.yml
🧰 Additional context used
🧬 Code graph analysis (5)
openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py (1)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (10)
IssueAssignmentBot(8-403)is_assignment_request(37-52)extract_linked_issues(217-223)respond_to_assignment_request(128-215)auto_assign_issues_from_pr(225-282)unassign_issues_from_pr(284-317)load_event_payload(33-35)handle_issue_comment(319-349)handle_pull_request(351-380)run(382-403)
openwisp_utils/bots/auto_assign/__main__.py (3)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (1)
main(404-418)openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
main(281-300)openwisp_utils/bots/auto_assign/stale_pr_bot.py (1)
main(346-349)
openwisp_utils/bots/auto_assign/stale_pr_tests.py (3)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (4)
extract_linked_issues(37-45)extract_linked_issues(182-190)load_event_payload(33-35)load_event_payload(178-180)openwisp_utils/bots/auto_assign/stale_pr_bot.py (2)
extract_linked_issues(109-115)get_last_changes_requested(75-91)openwisp_utils/bots/auto_assign/issue_assignment_bot.py (2)
extract_linked_issues(217-223)load_event_payload(33-35)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (3)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (4)
load_event_payload(33-35)extract_linked_issues(217-223)run(382-403)main(404-418)openwisp_utils/bots/auto_assign/stale_pr_bot.py (3)
extract_linked_issues(109-115)run(330-345)main(346-349)openwisp_utils/bots/auto_assign/__main__.py (1)
main(4-36)
openwisp_utils/bots/auto_assign/stale_pr_bot.py (3)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (3)
extract_linked_issues(217-223)run(382-403)main(404-418)openwisp_utils/bots/auto_assign/pr_reopen_bot.py (5)
extract_linked_issues(37-45)extract_linked_issues(182-190)run(139-158)run(261-280)main(281-300)openwisp_utils/bots/auto_assign/__main__.py (1)
main(4-36)
🪛 Ruff (0.14.14)
openwisp_utils/bots/auto_assign/__main__.py
[warning] 17-17: Consider [sys.argv[0], *sys.argv[2:]] instead of concatenation
Replace with [sys.argv[0], *sys.argv[2:]]
(RUF005)
[warning] 26-26: Consider [sys.argv[0], *sys.argv[2:]] instead of concatenation
Replace with [sys.argv[0], *sys.argv[2:]]
(RUF005)
[warning] 33-33: Consider moving this statement to an else block
(TRY300)
[warning] 34-34: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py
[warning] 19-19: Do not catch blind exception: Exception
(BLE001)
[warning] 212-212: Consider moving this statement to an else block
(TRY300)
[warning] 213-213: Do not catch blind exception: Exception
(BLE001)
[warning] 274-274: Do not catch blind exception: Exception
(BLE001)
[warning] 279-279: Consider moving this statement to an else block
(TRY300)
[warning] 280-280: Do not catch blind exception: Exception
(BLE001)
[warning] 310-310: Do not catch blind exception: Exception
(BLE001)
[warning] 313-313: Consider moving this statement to an else block
(TRY300)
[warning] 315-315: Do not catch blind exception: Exception
(BLE001)
[warning] 345-345: Consider moving this statement to an else block
(TRY300)
[warning] 347-347: Do not catch blind exception: Exception
(BLE001)
[warning] 376-376: Consider moving this statement to an else block
(TRY300)
[warning] 378-378: Do not catch blind exception: Exception
(BLE001)
[warning] 399-399: Do not catch blind exception: Exception
(BLE001)
[warning] 415-415: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py
[warning] 19-19: Do not catch blind exception: Exception
(BLE001)
[warning] 82-82: Do not catch blind exception: Exception
(BLE001)
[warning] 88-88: Consider moving this statement to an else block
(TRY300)
[warning] 90-90: Do not catch blind exception: Exception
(BLE001)
[warning] 108-108: Do not catch blind exception: Exception
(BLE001)
[warning] 135-135: Do not catch blind exception: Exception
(BLE001)
[warning] 154-154: Do not catch blind exception: Exception
(BLE001)
[warning] 170-170: Do not catch blind exception: Exception
(BLE001)
[warning] 225-225: Do not catch blind exception: Exception
(BLE001)
[warning] 245-245: Do not catch blind exception: Exception
(BLE001)
[warning] 255-255: Consider moving this statement to an else block
(TRY300)
[warning] 257-257: Do not catch blind exception: Exception
(BLE001)
[warning] 276-276: Do not catch blind exception: Exception
(BLE001)
[warning] 295-295: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/stale_pr_bot.py
[warning] 22-22: Do not catch blind exception: Exception
(BLE001)
[warning] 71-71: Do not catch blind exception: Exception
(BLE001)
[warning] 89-89: Do not catch blind exception: Exception
(BLE001)
[warning] 104-104: Consider moving this statement to an else block
(TRY300)
[warning] 105-105: Do not catch blind exception: Exception
(BLE001)
[warning] 136-136: Do not catch blind exception: Exception
(BLE001)
[warning] 139-139: Consider moving this statement to an else block
(TRY300)
[warning] 141-141: Do not catch blind exception: Exception
(BLE001)
[warning] 182-182: Consider moving this statement to an else block
(TRY300)
[warning] 184-184: Do not catch blind exception: Exception
(BLE001)
[warning] 225-225: Do not catch blind exception: Exception
(BLE001)
[warning] 232-232: Consider moving this statement to an else block
(TRY300)
[warning] 234-234: Do not catch blind exception: Exception
(BLE001)
[warning] 271-271: Consider moving this statement to an else block
(TRY300)
[warning] 273-273: Do not catch blind exception: Exception
(BLE001)
[warning] 321-321: Do not catch blind exception: Exception
(BLE001)
[warning] 327-327: Do not catch blind exception: Exception
(BLE001)
[warning] 340-340: Consider moving this statement to an else block
(TRY300)
[warning] 341-341: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
🔇 Additional comments (9)
openwisp_utils/bots/auto_assign/stale_pr_bot.py (1)
145-186: LGTM on the stale lifecycle actions (close, mark stale, warn).The three-tier approach (warn at 7d, mark stale + unassign at 14d, close at 60d) aligns with the PR objectives. The messages are well-crafted and user-friendly. The
unassign_linked_issuesreuse is clean.Also applies to: 188-236, 238-275
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (2)
37-52: LGTM on assignment-request detection.The phrase-based matching is appropriate for this use case — the phrases are specific enough to avoid false positives while covering common variations. Good approach.
225-282: LGTM onauto_assign_issues_from_pr.The
max_issuesguard, PR-vs-issue check, already-assigned check, and confirmation comment are all well-handled. Good defensive coding.openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
148-148: The review comment is incorrect. According to official GitHub Actions documentation,GITHUB_EVENT_NAMEis"pull_request_target"when a workflow is triggered by thepull_request_targetevent. The code comparisons at line 148 (pr_reopen_bot.py) and line 393 (issue_assignment_bot.py) are correct. Both bots properly retrieveGITHUB_EVENT_NAMEviaos.environ.get("GITHUB_EVENT_NAME")and correctly check against"pull_request_target". No changes are needed.openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py (5)
165-179: LGTM!The rate-limiting test correctly validates that only
max_issuesare processed, and the assertion on count is order-independent.
316-338: LGTM!Proper end-to-end test for the
pull_request_targetevent path with correctly configured mocks.
38-49: LGTM!Initialization tests correctly verify both the happy path and missing-env-var scenarios.
51-79: LGTM!Good coverage of both positive and negative cases for assignment request detection, including edge cases like empty strings and
None.
81-97: LGTM!Solid test for linked issue extraction with deduplication and edge cases.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
4 times, most recently
from
92c7e63 to
374ace8
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py`:
- Around line 307-323: The test test_run_issue_comment_event creates mock_issue
= Mock() but doesn't set iterable labels or strings for title/body, causing
detect_issue_type to iterate over a Mock and raise; fix by configuring
mock_issue in that test: set mock_issue.labels to an iterable (e.g., an empty
list), and set mock_issue.title and mock_issue.body to simple string values so
detect_issue_type and respond_to_assignment_request can operate normally; locate
the mock_issue assignment in test_run_issue_comment_event and update those
attributes accordingly.
- Around line 109-113: The test's expectation/comment is wrong because
mock_issue.title contains the bug keyword "issue", so detect_issue_type returns
"bug" and the code emits the bug-specific backreference format; either update
the test data to avoid bug/feature keywords (change mock_issue.title from "Test
issue title" to a neutral title like "Test task title" or remove "issue" from
body) so the generic branch is exercised, or else update the assertion/comment
to reflect the bug-specific path (assert that the comment contains the exact
bug-format string such as "Fixes `#123`" and update the inline comment on the
assertion); locate this in the test where mock_issue.create_comment is inspected
(call_args variable) and where mock_issue.title is set.
In `@openwisp_utils/bots/auto_assign/issue_assignment_bot.py`:
- Around line 60-135: The detect_issue_type function currently biases results
because the token "issue" in bug_keywords will match almost every GitHub issue;
update detect_issue_type by removing "issue" from the bug_keywords list (or
alternatively treating it as a low-weight token) and adjust the scoring logic
that computes bug_score/feature_score so common generic tokens don't dominate
(refer to bug_keywords, feature_keywords, detect_issue_type, bug_score and
feature_score); implement the change by either deleting "issue" from
bug_keywords or subtracting a small weight when matching that specific keyword
before comparing bug_score and feature_score.
In `@openwisp_utils/bots/auto_assign/pr_reopen_bot.py`:
- Around line 114-139: handle_pr_reopen currently returns True only if
reassign_issues_to_author returned >0, so removing a stale label without
reassigning still yields False; update handle_pr_reopen to capture the result of
remove_stale_label (e.g., removed = self.remove_stale_label(pr_number)) and
return True if either reassigned has length > 0 OR removed is truthy; if
remove_stale_label doesn't return a boolean yet, adjust it to return True on
successful removal (or at least a truthy value) so you can combine: return
(len(reassigned) > 0) or bool(removed). Ensure references to handle_pr_reopen,
reassign_issues_to_author, and remove_stale_label are used to locate the
changes.
In `@openwisp_utils/bots/auto_assign/stale_pr_bot.py`:
- Around line 43-71: The code currently calls
self.repo.get_commits(sha=pr.head.sha) which returns the entire reachable
history and can be huge; replace that call with pr.get_commits() to iterate only
the commits belonging to the pull request (keep the existing commit/date logic
and last_author_activity variable), and avoid materializing all commits at once
(iterate the PR commits generator instead of wrapping with list()) to reduce API
pagination and rate-limit usage in the methods like the stale PR age computation
used by process_stale_prs.
- Around line 317-322: The dedupe check in process_stale_prs is using
has_bot_comment(pr, "stale warning") and has_bot_comment(pr, "reminder") which
don't match the actual marker posted by send_stale_warning (<!--
bot:stale_warning -->) and no code ever posts <!-- bot:reminder -->; update
process_stale_prs to call has_bot_comment(pr, "stale_warning") (underscore)
instead of "stale warning" and remove the dead has_bot_comment(pr, "reminder")
check (or replace it only if you add a matching reminder marker elsewhere),
ensuring deduplication uses the same marker string as send_stale_warning.
- Around line 310-315: The check for has_bot_comment(pr, "unassigned") is dead
code because no method ever posts the "<!-- bot:unassigned -->" marker; remove
that check from the conditional in stale_pr_bot.py (the block around
DAYS_BEFORE_UNASSIGN and mark_pr_stale) so the branch simply tests
has_bot_comment(pr, "stale") and then calls mark_pr_stale(pr, days_inactive) as
before, leaving DAYS_BEFORE_UNASSIGN, has_bot_comment, and mark_pr_stale intact.
In `@openwisp_utils/bots/auto_assign/stale_pr_tests.py`:
- Around line 139-152: The test test_has_bot_comment sets mock_comment.body to
"This is a stale warning" but has_bot_comment (StalePRBot.has_bot_comment) looks
for the HTML marker "<!-- bot:stale -->"; update the test so mock_comment.body
includes that exact marker (e.g., include "<!-- bot:stale --> This is a stale
warning") when calling bot.has_bot_comment(mock_pr, "stale"), and keep a body
without the marker for the "closed" assertion to ensure the True/False
expectations are correct.
🧹 Nitpick comments (4)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
10-33: Duplicated__init__and GitHub client setup across bot classes — consider a shared base class.
PRReopenBot.__init__,PRActivityBot.__init__,StalePRBot.__init__, andIssueAssignmentBot.__init__all contain the same token/repo/client initialization logic. ABaseBotMixinor base class would eliminate ~25 lines of duplication per bot.Also applies to: 163-180
openwisp_utils/bots/auto_assign/__main__.py (1)
15-29: Mutatingsys.argvglobally is a side effect that persists after exceptions.If
issue_main()orpr_main()raises,sys.argvhas already been overwritten on lines 19/28. This is acceptable for a CLI entry point that exits after one run, but consider passing the payload path as a function argument instead for testability.openwisp_utils/bots/auto_assign/stale_pr_bot.py (1)
148-189:close_stale_prdoes not include a<!-- bot:closed -->HTML marker in the close comment.This isn't currently a problem since closed PRs are filtered out by
get_pulls(state="open"), but adding a marker would be consistent with the other bot comments and useful if the check is ever needed.openwisp_utils/bots/auto_assign/issue_assignment_bot.py (1)
415-433:IssueAssignmentBot()is instantiated (and GitHub client initialized) before argument validation.Line 420 creates the bot and authenticates with GitHub even when invoked with no arguments. Moving instantiation after the
sys.argvcheck avoids unnecessary API calls.Proposed fix
def main(): """Entry point for command line usage""" import json import sys - bot = IssueAssignmentBot() if len(sys.argv) > 1: + bot = IssueAssignmentBot() try: with open(sys.argv[1], "r") as f: event_payload = json.load(f) bot.load_event_payload(event_payload) except Exception as e: print(f"Could not load event payload: {e}") - - bot.run() + bot.run() + else: + print("Usage: python issue_assignment_bot.py <event_payload.json>")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/issue-assignment.yml.github/workflows/pr-issue-link.yml.github/workflows/pr-reopen-reassign.yml.github/workflows/stale-pr-management.ymlopenwisp_utils/bots/__init__.pyopenwisp_utils/bots/auto_assign/__init__.pyopenwisp_utils/bots/auto_assign/__main__.pyopenwisp_utils/bots/auto_assign/issue_assignment_bot.pyopenwisp_utils/bots/auto_assign/issue_assignment_bot_tests.pyopenwisp_utils/bots/auto_assign/pr_reopen_bot.pyopenwisp_utils/bots/auto_assign/pr_reopen_tests.pyopenwisp_utils/bots/auto_assign/stale_pr_bot.pyopenwisp_utils/bots/auto_assign/stale_pr_tests.pysetup.py
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/issue-assignment.yml
- .github/workflows/pr-reopen-reassign.yml
- .github/workflows/stale-pr-management.yml
- openwisp_utils/bots/auto_assign/pr_reopen_tests.py
- .github/workflows/pr-issue-link.yml
- setup.py
🧰 Additional context used
🧬 Code graph analysis (5)
openwisp_utils/bots/auto_assign/stale_pr_tests.py (1)
openwisp_utils/bots/auto_assign/stale_pr_bot.py (9)
StalePRBot(12-350)extract_linked_issues(112-118)get_last_changes_requested(77-93)get_days_since_activity(38-75)unassign_linked_issues(120-146)has_bot_comment(95-110)send_stale_warning(242-280)mark_pr_stale(191-240)close_stale_pr(148-189)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (3)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (4)
load_event_payload(35-37)extract_linked_issues(226-232)run(391-412)main(415-429)openwisp_utils/bots/auto_assign/stale_pr_bot.py (3)
extract_linked_issues(112-118)run(335-350)main(353-356)openwisp_utils/bots/auto_assign/__main__.py (1)
main(6-38)
openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py (1)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (10)
IssueAssignmentBot(10-412)is_assignment_request(39-54)extract_linked_issues(226-232)respond_to_assignment_request(137-224)auto_assign_issues_from_pr(234-291)unassign_issues_from_pr(293-326)load_event_payload(35-37)handle_issue_comment(328-358)handle_pull_request(360-389)run(391-412)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (2)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (7)
load_event_payload(35-37)load_event_payload(182-184)extract_linked_issues(39-47)extract_linked_issues(186-194)run(141-160)run(265-284)main(287-309)openwisp_utils/bots/auto_assign/stale_pr_bot.py (3)
extract_linked_issues(112-118)run(335-350)main(353-356)
openwisp_utils/bots/auto_assign/__main__.py (3)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py (1)
main(415-429)openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
main(287-309)openwisp_utils/bots/auto_assign/stale_pr_bot.py (1)
main(353-356)
🪛 Ruff (0.14.14)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py
[warning] 21-21: Do not catch blind exception: Exception
(BLE001)
[warning] 84-84: Do not catch blind exception: Exception
(BLE001)
[warning] 90-90: Consider moving this statement to an else block
(TRY300)
[warning] 92-92: Do not catch blind exception: Exception
(BLE001)
[warning] 110-110: Do not catch blind exception: Exception
(BLE001)
[warning] 137-137: Do not catch blind exception: Exception
(BLE001)
[warning] 156-156: Do not catch blind exception: Exception
(BLE001)
[warning] 174-174: Do not catch blind exception: Exception
(BLE001)
[warning] 229-229: Do not catch blind exception: Exception
(BLE001)
[warning] 249-249: Do not catch blind exception: Exception
(BLE001)
[warning] 259-259: Consider moving this statement to an else block
(TRY300)
[warning] 261-261: Do not catch blind exception: Exception
(BLE001)
[warning] 280-280: Do not catch blind exception: Exception
(BLE001)
[warning] 306-306: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/stale_pr_bot.py
[warning] 24-24: Do not catch blind exception: Exception
(BLE001)
[warning] 73-73: Do not catch blind exception: Exception
(BLE001)
[warning] 91-91: Do not catch blind exception: Exception
(BLE001)
[warning] 107-107: Consider moving this statement to an else block
(TRY300)
[warning] 108-108: Do not catch blind exception: Exception
(BLE001)
[warning] 139-139: Do not catch blind exception: Exception
(BLE001)
[warning] 142-142: Consider moving this statement to an else block
(TRY300)
[warning] 144-144: Do not catch blind exception: Exception
(BLE001)
[warning] 185-185: Consider moving this statement to an else block
(TRY300)
[warning] 187-187: Do not catch blind exception: Exception
(BLE001)
[warning] 229-229: Do not catch blind exception: Exception
(BLE001)
[warning] 236-236: Consider moving this statement to an else block
(TRY300)
[warning] 238-238: Do not catch blind exception: Exception
(BLE001)
[warning] 276-276: Consider moving this statement to an else block
(TRY300)
[warning] 278-278: Do not catch blind exception: Exception
(BLE001)
[warning] 326-326: Do not catch blind exception: Exception
(BLE001)
[warning] 332-332: Do not catch blind exception: Exception
(BLE001)
[warning] 345-345: Consider moving this statement to an else block
(TRY300)
[warning] 346-346: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/issue_assignment_bot.py
[warning] 21-21: Do not catch blind exception: Exception
(BLE001)
[warning] 221-221: Consider moving this statement to an else block
(TRY300)
[warning] 222-222: Do not catch blind exception: Exception
(BLE001)
[warning] 283-283: Do not catch blind exception: Exception
(BLE001)
[warning] 288-288: Consider moving this statement to an else block
(TRY300)
[warning] 289-289: Do not catch blind exception: Exception
(BLE001)
[warning] 319-319: Do not catch blind exception: Exception
(BLE001)
[warning] 322-322: Consider moving this statement to an else block
(TRY300)
[warning] 324-324: Do not catch blind exception: Exception
(BLE001)
[warning] 354-354: Consider moving this statement to an else block
(TRY300)
[warning] 356-356: Do not catch blind exception: Exception
(BLE001)
[warning] 385-385: Consider moving this statement to an else block
(TRY300)
[warning] 387-387: Do not catch blind exception: Exception
(BLE001)
[warning] 408-408: Do not catch blind exception: Exception
(BLE001)
[warning] 426-426: Do not catch blind exception: Exception
(BLE001)
openwisp_utils/bots/auto_assign/__main__.py
[warning] 19-19: Consider [sys.argv[0], *sys.argv[2:]] instead of concatenation
Replace with [sys.argv[0], *sys.argv[2:]]
(RUF005)
[warning] 28-28: Consider [sys.argv[0], *sys.argv[2:]] instead of concatenation
Replace with [sys.argv[0], *sys.argv[2:]]
(RUF005)
[warning] 35-35: Consider moving this statement to an else block
(TRY300)
[warning] 36-36: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
🔇 Additional comments (10)
openwisp_utils/bots/auto_assign/pr_reopen_bot.py (1)
287-309:main()continues tobot.run()even when payload loading fails.If the
json.loadon line 295 oropenon line 294 raises, the exception is caught and printed (line 307), butbot.run()is never reached either because theexceptends thetryblock. However, ifbot.load_event_payloaditself raised,bot.run()would still be skipped since it's inside the sametry. On closer inspection, the entire dispatch is inside thetry, so a payload error aborts execution. This is fine.openwisp_utils/bots/auto_assign/stale_pr_tests.py (3)
1-11: Test setup and structure look solid.The
setUp/tearDownproperly patch environment variables and theGithubclass. TheskipTestguard for missing imports is a good defensive pattern.
46-62: Good coverage ofextract_linked_issuesedge cases including duplicates, empty, and None.
96-118: The datetime mock is safe—no action needed.The code imports
datetimeandtimezoneseparately (from datetime import datetime, timezone), and uses onlydatetime.now(timezone.utc). Sincetimezoneis not accessed as an attribute of thedatetimeobject (nodatetime.timezonepatterns found), patching only thedatetimeclass poses no risk.openwisp_utils/bots/auto_assign/__main__.py (1)
1-42: Dispatcher logic is clear and functional.The bot-type routing, lazy imports, and error handling are well-structured for a CLI entry point.
openwisp_utils/bots/auto_assign/issue_assignment_bot_tests.py (3)
12-36: Test setup and teardown are well-structured.Clean environment patching with proper cleanup, and the
Githubclass is correctly mocked at the right import path.
50-96: Thorough coverage of assignment request detection and linked issues extraction.Good use of
subTestfor parameterized cases, including edge cases (empty, None, duplicates).
121-169: Auto-assign tests cover key scenarios well: success, already-assigned, PR references, and rate limiting.openwisp_utils/bots/auto_assign/stale_pr_bot.py (1)
95-110: HTML marker approach forhas_bot_commentis a solid improvement over substring matching.openwisp_utils/bots/auto_assign/issue_assignment_bot.py (1)
234-291:auto_assign_issues_from_pr— rate-limiting by truncating the list is a good safeguard.The
max_issuesparameter and early truncation on line 251 prevent excessive API calls when a PR body references many issues.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Eeshu-Yadav
force-pushed
the
issues/571-automate-assigning-unassigning-issues
branch
from
374ace8 to
2315ec8
Compare
✅ Actions performedReview triggered.
|
|
@coderabbitai are you happy? If so approve the PR. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
runtests.py (1)
27-32:⚠️ Potential issue | 🟠 MajorLocal test runs without
github_actionsextra will fail during collection.Line 31 adds
.github/actions/bot-autoassign/teststo the default pytest run. However,.github/actions/bot-autoassign/base.pyimportsfrom github import Githubat module level (line 3), andPyGithubis only inextras_require["github_actions"]. Developers following the installation docs (pip install -e .[qa,rest,selenium]) won't havePyGithubinstalled, causing collection to fail before any bot tests run.Either add
PyGithubto the default test dependencies, or conditionally skip the bot test suite when the extra is absent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtests.py` around lines 27 - 32, The test collection fails when PyGithub isn't installed because runtests.py unconditionally includes ".github/actions/bot-autoassign/tests" and that package imports "from github import Github" at module import time; update the pytest.main invocation in runtests.py (the pytest_exit_code list construction) to conditionally include ".github/actions/bot-autoassign/tests" only if the github package is available (use importlib.util.find_spec('github') or a try/except import to detect availability) and otherwise omit that path (optionally log or print a short message), so that pytest collection succeeds for dev installs without the "github_actions" extra.
♻️ Duplicate comments (1)
docs/developer/reusable-github-utils.rst (1)
89-95:⚠️ Potential issue | 🟡 MinorFix the install path in the cross-repo setup steps.
Lines 93-94 say to checkout
openwisp-utils, but Line 94 then tells readers to runpip install -e .[github_actions]. In another repo, that installs the caller repo instead of the checked-outopenwisp-utilscopy; the example below already usesopenwisp-utils/.[github_actions]. The numbered setup steps should match the working example.Proposed doc fix
-3. Install the bot dependencies via ``pip install -e .[github_actions]``. +3. Install the bot dependencies via ``pip install -e openwisp-utils/.[github_actions]``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer/reusable-github-utils.rst` around lines 89 - 95, The numbered setup step that currently says to "Checkout `openwisp-utils`" but then instructs to run `pip install -e .[github_actions]` is incorrect for cross-repo usage; update the instruction so the pip install targets the checked-out checkout (e.g., `openwisp-utils/.[github_actions]`) to match the working example. Specifically, change the step text that advises running `pip install -e .[github_actions]` to reference the checked-out path (the `openwisp-utils` directory) so it installs the library you checked out rather than the caller repo, and ensure consistency with the example that uses `openwisp-utils/.[github_actions]`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-autoassign/__main__.py:
- Around line 18-29: The dispatcher currently masks failures by using
`issue_main() or 0`, `stale_main() or 0`, `pr_main() or 0`; change each branch
to call the downstream entrypoint (issue_assignment_bot.main, stale_pr_bot.main,
pr_reopen_bot.main), capture its return value, and explicitly return integer
exit codes: return 0 when the captured result indicates success (e.g., explicit
0 or True) and return 1 when the result is None/False or an exception occurs; do
not use `or 0`. Ensure you propagate failures by converting None/False into exit
code 1 and let exceptions bubble or catch them to return 1 so failures are not
silently treated as success.
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 28-29: The code slices pr.get_commits() with [:50], which keeps
the oldest commits and can miscompute last_author_activity; instead iterate the
paginated generator returned by pr.get_commits() (do not slice) or collect
commits into a rolling window of the newest N (e.g., use a deque(maxlen=50))
before computing last_author_activity. Update the loop that assigns commits/for
commit in commits to either iterate pr.get_commits() directly or populate a
deque with pr.get_commits() so you always examine the most recent commits when
updating last_author_activity.
In @.github/actions/bot-autoassign/utils.py:
- Around line 15-18: The parser currently strips repository context from
qualified refs (owner/repo#NN) using issue_pattern so extract_linked_issues()
returns only the issue number; update extract_linked_issues() to preserve and
return the repo full name along with the issue number (e.g., return tuples like
(repo_full_name, issue_number)) or alternatively ignore qualified refs entirely,
and then update get_valid_linked_issues() to accept and filter on that tuple
before calling repo.get_issue(); specifically adjust the behavior around
issue_pattern and the logic in extract_linked_issues() and
get_valid_linked_issues() (also update the code paths around the similar logic
in lines 29-35) so cross-repo references cannot be resolved against the wrong
local repository.
In @.github/workflows/bot-autoassign-pr-issue-link.yml:
- Around line 35-45: The workflow installs and runs a repo-local bot (pip
install -e .[github_actions] and python
.github/actions/bot-autoassign/__main__.py issue_assignment
"$GITHUB_EVENT_PATH"), which prevents reuse across repos; either make the action
reusable by publishing the bot as a package or composite action and update the
steps to install/run the published artifact (replace the editable install +
local __main__.py invocation with a pip install of the published package or use
uses: owner/repo/.github/workflows/... for a reusable workflow), or explicitly
document that consumers must vendor the .github/actions/bot-autoassign directory
alongside the YAML and update the workflow env/steps to match that documented
installation method.
---
Outside diff comments:
In `@runtests.py`:
- Around line 27-32: The test collection fails when PyGithub isn't installed
because runtests.py unconditionally includes
".github/actions/bot-autoassign/tests" and that package imports "from github
import Github" at module import time; update the pytest.main invocation in
runtests.py (the pytest_exit_code list construction) to conditionally include
".github/actions/bot-autoassign/tests" only if the github package is available
(use importlib.util.find_spec('github') or a try/except import to detect
availability) and otherwise omit that path (optionally log or print a short
message), so that pytest collection succeeds for dev installs without the
"github_actions" extra.
---
Duplicate comments:
In `@docs/developer/reusable-github-utils.rst`:
- Around line 89-95: The numbered setup step that currently says to "Checkout
`openwisp-utils`" but then instructs to run `pip install -e .[github_actions]`
is incorrect for cross-repo usage; update the instruction so the pip install
targets the checked-out checkout (e.g., `openwisp-utils/.[github_actions]`) to
match the working example. Specifically, change the step text that advises
running `pip install -e .[github_actions]` to reference the checked-out path
(the `openwisp-utils` directory) so it installs the library you checked out
rather than the caller repo, and ensure consistency with the example that uses
`openwisp-utils/.[github_actions]`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5cb46c28-f55a-4e62-a869-eea587366994
📒 Files selected for processing (16)
.github/actions/bot-autoassign/__main__.py.github/actions/bot-autoassign/base.py.github/actions/bot-autoassign/issue_assignment_bot.py.github/actions/bot-autoassign/pr_reopen_bot.py.github/actions/bot-autoassign/stale_pr_bot.py.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py.github/actions/bot-autoassign/tests/test_stale_pr_bot.py.github/actions/bot-autoassign/utils.py.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-pr-issue-link.yml.github/workflows/bot-autoassign-pr-reopen.yml.github/workflows/bot-autoassign-stale-pr.ymldocs/developer/reusable-github-utils.rstruntests.pysetup.py
📜 Review details
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
📚 Learning: 2026-03-05T14:23:55.528Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Applied to files:
.github/workflows/bot-autoassign-pr-issue-link.yml.github/workflows/bot-autoassign-stale-pr.yml.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-pr-reopen.yml
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/workflows/bot-autoassign-pr-issue-link.yml.github/workflows/bot-autoassign-stale-pr.yml.github/workflows/bot-autoassign-pr-reopen.yml.github/actions/bot-autoassign/utils.py.github/actions/bot-autoassign/tests/test_pr_reopen_bot.pydocs/developer/reusable-github-utils.rst.github/actions/bot-autoassign/stale_pr_bot.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/workflows/bot-autoassign-issue.ymlruntests.pydocs/developer/reusable-github-utils.rst
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-02-10T20:38:27.593Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 593
File: openwisp_utils/releaser/commitizen.py:5-9
Timestamp: 2026-02-10T20:38:27.593Z
Learning: In openwisp-utils commitizen implementation, the `_TITLE_ISSUE_EXTRACT_RE` pattern intentionally matches any space-preceded `#<number>` in commit titles without anchoring to the end. This is acceptable because 99.9% of such patterns are actual issue references, and rare edge cases (like version numbers) will be handled manually if they occur.
Applied to files:
.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py.github/actions/bot-autoassign/utils.py
📚 Learning: 2026-03-05T20:55:29.968Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 0
File: :0-0
Timestamp: 2026-03-05T20:55:29.968Z
Learning: For the openwisp/openwisp-utils repository (and the broader openwisp org), the maintainers have explicitly decided to use mutable version tags (e.g. `v2`, `v6`) for GitHub Actions and to use `ref: master` for internal script checkouts. Do NOT flag unpinned action versions or `master`/version-tag usage as security issues in any openwisp-utils workflow file. The maintainers are aware of the trade-offs and are intentionally fine with this approach.
Applied to files:
docs/developer/reusable-github-utils.rst
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me do a final check on the current state of the key files before approving. 🏁 Script executed: Length of output: 32183
The current code in pr.create_issue_comment("\n".join(close_lines)) # comment posted first
pr.edit(state="closed") # close happens after
unassigned_count = self.unassign_linked_issues(pr)This is the same order The safe fix is to reverse the order: close first, then comment, then unassign: pr.edit(state="closed")
pr.create_issue_comment("\n".join(close_lines))
unassigned_count = self.unassign_linked_issues(pr)This is a minor edge case (requires a transient GitHub API failure at exactly the right moment), so I'll defer to you on whether it's worth blocking the merge. All other previously flagged issues are genuinely resolved. If you're comfortable with this edge case, I'm happy to approve now. 🧠 Learnings used |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
addresess , wrap it in a try/finally to ensure both operations are attempted regardless |
Commit Message Format ViolationHello @Eeshu-Yadav, The CI pipeline failed because the commit message does not adhere to the OpenWISP commit message conventions. Specifically, the commit message is missing a closing keyword (like Explanation: OpenWISP enforces a strict commit message format to ensure clarity and traceability. When an issue number is included in the commit title (e.g., Remediation: Please update your commit message to include a closing keyword in the body that references the issue number. Here's an example of the correct format: Ensure that the keyword ( |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
.github/actions/bot-autoassign/pr_reopen_bot.py (1)
57-76:⚠️ Potential issue | 🟠 MajorTreat a no-op reopen as success.
A reopened PR with no linked issues and no
stalelabel currently returnsFalsehere, andmain()turns that into exit code 1. Since reopen is a normal event path, this still produces false workflow failures.Suggested fix
reassigned = self.reassign_issues_to_author(pr_number, pr_author, pr_body) label_removed = self.remove_stale_label(pr_number) print(f"Reassigned {len(reassigned)}" f" issues to {pr_author}") - return len(reassigned) > 0 or label_removed + return True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-autoassign/pr_reopen_bot.py around lines 57 - 76, handle_pr_reopen currently returns False for a no-op reopen (no linked issues and no stale label) which causes workflows to fail; update the method (handle_pr_reopen) so that after calling reassign_issues_to_author(pr_number, pr_author, pr_body) and remove_stale_label(pr_number) it treats a no-op as success by returning True when both reassigned is empty and label_removed is False (i.e., replace the final return len(reassigned) > 0 or label_removed with logic that returns True for successful no-ops and only returns False on exceptions or missing PR data).docs/developer/reusable-github-utils.rst (1)
89-95:⚠️ Potential issue | 🟡 MinorFix the install path in the reusable setup steps.
Step 2 checks out
openwisp-utilsinto a subdirectory, but Step 3 still sayspip install -e .[github_actions]. In another repository that installs the caller repo, not the checked-out bot repo, so the documented setup is wrong unless the caller also defines that extra.Suggested doc fix
-3. Install the bot dependencies via ``pip install -e .[github_actions]``. +3. Install the bot dependencies via + ``pip install -e openwisp-utils/.[github_actions]``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/developer/reusable-github-utils.rst` around lines 89 - 95, The docs currently instruct checking out openwisp-utils into a subdirectory (Step 2) but still use pip install -e .[github_actions] (Step 3), which installs the caller repo instead of the checked-out bot; update the instructions to either change Step 3 to pip install -e openwisp-utils[github_actions] (or equivalent path) so the checked-out package is installed, or add a step to cd into the checked-out openwisp-utils directory before running pip install -e .[github_actions]; update the text referencing "openwisp-utils" and the install command accordingly..github/actions/bot-autoassign/tests/test_stale_pr_bot.py (2)
82-86: 🧹 Nitpick | 🔵 TrivialAvoid rebuilding
datetimebehavior withside_effect.This mixes a fake
now()with a callableMagicMock, which makes the test depend on mock internals instead of the activity calculation. Wrap or freeze the real time source instead so constructor behavior stays native.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py around lines 82 - 86, The test currently patches stale_pr_bot.datetime and sets mock_datetime.side_effect to rebuild datetime constructors; instead freeze real time so constructors remain native: replace the `@patch`("stale_pr_bot.datetime") usage in test_with_author_commit with a real-time freeze (e.g., use freezegun.freeze_time("2024-01-10T00:00:00Z") as decorator or context) and remove mock_datetime and side_effect logic so stale_pr_bot uses the frozen real datetime for now() while datetime(...) constructors behave normally.
209-228: 🧹 Nitpick | 🔵 TrivialCover the real stale-transition path.
This file still only exercises
run()when the GitHub client is missing, and the close helper test does not lock in close-before-comment ordering. Add one threshold-drivenprocess_stale_prsor positiverun()test that covers warning → stale → close and asserts the PR is closed before the close notice is posted.Also applies to: 239-244
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py around lines 209 - 228, Add a new positive integration-style test that drives the full warning → stale → close transition using StalePRBot.run() or process_stale_prs() so the real transition path is exercised; set up bot_env["repo"] to return a PR/Issue pair and configure thresholds (e.g., warning_days, stale_days, close_days) so the PR moves through warning and stale into close, then assert ordering by checking mock_pr.edit(state="closed") was called before mock_pr.create_issue_comment (and that the close comment contains "<!-- bot:closed -->"), and also assert mock_issue.remove_from_assignees was called; place this alongside TestCloseStalePR and update any existing tests that relied only on missing-client behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-autoassign/issue_assignment_bot.py:
- Around line 277-299: handle_issue_comment currently returns False for benign
cases (comment on a PR, missing comment fields, or a non-assignment comment),
which causes main() to treat normal discussion as a failure; update
handle_issue_comment so that benign/expected cases return True instead of False:
change the branches that print "Comment is on a PR, not an issue - skipping",
"Missing required comment data", and "Comment does not contain assignment
request" to return True, while preserving the existing True/False return for an
actual assignment flow (is_assignment_request -> respond_to_assignment_request)
and keeping the except Exception handler returning False; reference
handle_issue_comment, is_assignment_request, and respond_to_assignment_request
when making the changes.
In @.github/actions/bot-autoassign/stale_pr_bot.py:
- Around line 185-193: The code posts a “has been automatically closed” comment
before attempting to close the PR, which can leave a false notice if
pr.edit(state="closed") fails; change the order and error handling so you call
pr.edit(state="closed") first and only create the closing comment
(pr.create_issue_comment) if the edit succeeds, catching and logging exceptions
separately for the edit and the comment (referencing pr.edit and
pr.create_issue_comment) so you never announce closure on an open PR.
In @.github/workflows/bot-autoassign-pr-reopen.yml:
- Around line 50-52: Add an author-check to the handle-pr-activity job guard by
updating its if condition (the job named handle-pr-activity) to include
github.event.issue.user.login == github.event.comment.user.login so the job only
runs when the commenter is the PR author; retain the existing github.event_name
== 'issue_comment' && github.event.issue.pull_request check and keep the
stale/label logic inside the bot code.
---
Duplicate comments:
In @.github/actions/bot-autoassign/pr_reopen_bot.py:
- Around line 57-76: handle_pr_reopen currently returns False for a no-op reopen
(no linked issues and no stale label) which causes workflows to fail; update the
method (handle_pr_reopen) so that after calling
reassign_issues_to_author(pr_number, pr_author, pr_body) and
remove_stale_label(pr_number) it treats a no-op as success by returning True
when both reassigned is empty and label_removed is False (i.e., replace the
final return len(reassigned) > 0 or label_removed with logic that returns True
for successful no-ops and only returns False on exceptions or missing PR data).
In @.github/actions/bot-autoassign/tests/test_stale_pr_bot.py:
- Around line 82-86: The test currently patches stale_pr_bot.datetime and sets
mock_datetime.side_effect to rebuild datetime constructors; instead freeze real
time so constructors remain native: replace the `@patch`("stale_pr_bot.datetime")
usage in test_with_author_commit with a real-time freeze (e.g., use
freezegun.freeze_time("2024-01-10T00:00:00Z") as decorator or context) and
remove mock_datetime and side_effect logic so stale_pr_bot uses the frozen real
datetime for now() while datetime(...) constructors behave normally.
- Around line 209-228: Add a new positive integration-style test that drives the
full warning → stale → close transition using StalePRBot.run() or
process_stale_prs() so the real transition path is exercised; set up
bot_env["repo"] to return a PR/Issue pair and configure thresholds (e.g.,
warning_days, stale_days, close_days) so the PR moves through warning and stale
into close, then assert ordering by checking mock_pr.edit(state="closed") was
called before mock_pr.create_issue_comment (and that the close comment contains
"<!-- bot:closed -->"), and also assert mock_issue.remove_from_assignees was
called; place this alongside TestCloseStalePR and update any existing tests that
relied only on missing-client behavior.
In `@docs/developer/reusable-github-utils.rst`:
- Around line 89-95: The docs currently instruct checking out openwisp-utils
into a subdirectory (Step 2) but still use pip install -e .[github_actions]
(Step 3), which installs the caller repo instead of the checked-out bot; update
the instructions to either change Step 3 to pip install -e
openwisp-utils[github_actions] (or equivalent path) so the checked-out package
is installed, or add a step to cd into the checked-out openwisp-utils directory
before running pip install -e .[github_actions]; update the text referencing
"openwisp-utils" and the install command accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f3769448-74ea-427e-9cf7-f0d50ee93e21
📒 Files selected for processing (16)
.github/actions/bot-autoassign/__main__.py.github/actions/bot-autoassign/base.py.github/actions/bot-autoassign/issue_assignment_bot.py.github/actions/bot-autoassign/pr_reopen_bot.py.github/actions/bot-autoassign/stale_pr_bot.py.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py.github/actions/bot-autoassign/tests/test_stale_pr_bot.py.github/actions/bot-autoassign/utils.py.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-pr-issue-link.yml.github/workflows/bot-autoassign-pr-reopen.yml.github/workflows/bot-autoassign-stale-pr.ymldocs/developer/reusable-github-utils.rstruntests.pysetup.py
📜 Review details
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
📚 Learning: 2026-03-05T14:23:55.528Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:22-24
Timestamp: 2026-03-05T14:23:55.528Z
Learning: In `.github/workflows/reusable-bot-changelog.yml`, the maintainer (nemesifier) has explicitly decided that `github.event.review.author_association == 'COLLABORATOR'` should be allowed (alongside `OWNER` and `MEMBER`) to trigger the changelog bot workflow. The rationale is that the workflow is non-destructive and only posts a PR comment — it cannot make code changes. Do not flag `COLLABORATOR` as a security issue for this workflow.
Applied to files:
.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-stale-pr.yml.github/workflows/bot-autoassign-pr-reopen.yml.github/workflows/bot-autoassign-pr-issue-link.yml.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-pr-issue-link.ymlruntests.pydocs/developer/reusable-github-utils.rst
📚 Learning: 2026-03-05T20:55:29.968Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 0
File: :0-0
Timestamp: 2026-03-05T20:55:29.968Z
Learning: For the openwisp/openwisp-utils repository (and the broader openwisp org), the maintainers have explicitly decided to use mutable version tags (e.g. `v2`, `v6`) for GitHub Actions and to use `ref: master` for internal script checkouts. Do NOT flag unpinned action versions or `master`/version-tag usage as security issues in any openwisp-utils workflow file. The maintainers are aware of the trade-offs and are intentionally fine with this approach.
Applied to files:
.github/workflows/bot-autoassign-issue.yml.github/workflows/bot-autoassign-stale-pr.yml.github/workflows/bot-autoassign-pr-reopen.yml.github/workflows/bot-autoassign-pr-issue-link.ymldocs/developer/reusable-github-utils.rst
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/workflows/bot-autoassign-stale-pr.yml.github/actions/bot-autoassign/tests/test_pr_reopen_bot.py.github/workflows/bot-autoassign-pr-reopen.yml.github/workflows/bot-autoassign-pr-issue-link.yml.github/actions/bot-autoassign/utils.py.github/actions/bot-autoassign/stale_pr_bot.pydocs/developer/reusable-github-utils.rst
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/workflows/bot-autoassign-pr-issue-link.yml.github/actions/bot-autoassign/tests/test_stale_pr_bot.py
📚 Learning: 2026-02-10T20:38:27.593Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 593
File: openwisp_utils/releaser/commitizen.py:5-9
Timestamp: 2026-02-10T20:38:27.593Z
Learning: In openwisp-utils commitizen implementation, the `_TITLE_ISSUE_EXTRACT_RE` pattern intentionally matches any space-preceded `#<number>` in commit titles without anchoring to the end. This is acceptable because 99.9% of such patterns are actual issue references, and rare edge cases (like version numbers) will be handled manually if they occur.
Applied to files:
.github/actions/bot-autoassign/utils.py.github/actions/bot-autoassign/tests/test_issue_assignment_bot.py
🔇 Additional comments (1)
runtests.py (1)
27-32: No action required — bot tests are not silently skipped.The main CI workflow (
.github/workflows/ci.yml) installs.[ga,rest,selenium,releaser,github_actions]before runningruntests.py, ensuringPyGithubis available. Bot-specific workflows also explicitly install.[github_actions]. All execution paths pairruntests.pywith the required dependencies.
- Refactored auto_assign_issues_from_pr to use get_valid_linked_issues instead of duplicating cross-repo and PR-type filtering - Fixed regex to match 'related to' keyword - Moved time.sleep to finally block for consistent rate limiting - Return True from handle_pr_reopen for handled events Fixes openwisp#571
Commit Message FailureHello @Eeshu-Yadav, The CI build failed because the commit message does not adhere to the OpenWISP commit message conventions. Explanation: Remediation: Here is an example of the correct format: |
|
@coderabbitai review focusing on security and efficiency, let us know if any previous comment was not addressed but give up on trivialities. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 6969 🏁 Script executed: Length of output: 752 🏁 Script executed: Length of output: 42045
✅ All Previous Issues — Confirmed Fixed
🟡 Minor Efficiency Note (non-blocking)
commits = []
for commit in pr.get_commits(): # fetches ALL pages from the API
commits.append(commit)
commits = commits[-50:] # discards everything before the last 50GitHub's PR commits API returns commits ascending-only with no from collections import deque
commits = deque(pr.get_commits(), maxlen=50)This doesn't reduce API calls but avoids storing the full list before slicing. Up to you whether it's worth the change. No security issues found. The [approve] 🧠 Learnings used |
above told to do relative import , add init , so have change this also. |
okk will revert it |
|
@nemesifier all done |
| if len(assigned_issues) >= max_issues: | ||
| break | ||
| try: | ||
| current_assignees = [assignee.login for assignee in issue.assignees] |
There was a problem hiding this comment.
CRITICAL: Potential AttributeError if assignee lacks login attribute
The code directly accesses assignee.login without checking if the attribute exists. This could cause a runtime error if an assignee object doesn't have a login attribute. The utils.py file correctly uses hasattr(assignee, "login") check (line 57), but this code doesn't.
| current_assignees = [assignee.login for assignee in issue.assignees] | |
| current_assignees = [assignee.login for assignee in issue.assignees if hasattr(assignee, "login")] |
| self.repo, self.repository_name, pr_body | ||
| ): | ||
| try: | ||
| current_assignees = [assignee.login for assignee in issue.assignees] |
There was a problem hiding this comment.
CRITICAL: Potential AttributeError if assignee lacks login attribute
Direct access to assignee.login without checking attribute existence. Use hasattr(assignee, "login") guard as done in utils.py.
| current_assignees = [assignee.login for assignee in issue.assignees] | |
| current_assignees = [assignee.login for assignee in issue.assignees if hasattr(assignee, "login")] |
| self.repo, self.repository_name, pr.body or "" | ||
| ): | ||
| try: | ||
| current_assignees = [assignee.login for assignee in issue.assignees] |
There was a problem hiding this comment.
CRITICAL: Potential AttributeError if assignee lacks login attribute
Direct access to assignee.login without checking attribute existence. Use hasattr(assignee, "login") guard as done in utils.py.
| current_assignees = [assignee.login for assignee in issue.assignees] | |
| current_assignees = [assignee.login for assignee in issue.assignees if hasattr(assignee, "login")] |
Checklist
Created Workflows
1.
issue-assignment.ymlAuto-response to "assign me" requests
2.
pr-issue-link.ymlAuto-assign on PR
Fixes #123,Closes #123, etc.)3.
stale-pr-management.ymlStale PR management (tracks contributor inactivity only)
stalelabel, unassigns linked issues4.
pr-reopen-reassign.ymlHandle contributor follow-up
stalelabelFixes #571